On 29.07.2025 10:38, Juergen Gross wrote:
> --- a/arch/x86/x86_64.S
> +++ b/arch/x86/x86_64.S
> @@ -33,13 +33,8 @@ _start:
> stack_start:
> .quad stack+(2*__STACK_SIZE)
>
> -.globl shared_info, hypercall_page
> - /* Unpleasant -- the PTE that maps this page is actually overwritten
> */
> - /* to map the real shared-info page! :-)
> */
> .align __PAGE_SIZE
> -shared_info:
> - .fill __PAGE_SIZE,1,0
> -
> +.globl hypercall_page
While touching this line, may I suggest to indent this directive to match all
other directives in context? Even if assemblers accept them for most targets,
directives starting in the first column strictly speaking are misplaced.
> --- a/hypervisor.c
> +++ b/hypervisor.c
> @@ -27,8 +27,10 @@
>
> #include <mini-os/os.h>
> #include <mini-os/lib.h>
> +#include <mini-os/e820.h>
> #include <mini-os/hypervisor.h>
> #include <mini-os/events.h>
> +#include <mini-os/mm.h>
> #include <xen/memory.h>
>
> EXPORT_SYMBOL(hypercall_page);
> @@ -37,7 +39,8 @@ EXPORT_SYMBOL(hypercall_page);
> ((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
>
> #ifndef CONFIG_PARAVIRT
> -extern shared_info_t shared_info;
> +static unsigned long shinfo_pfn;
> +static unsigned long shinfo_va;
>
> int hvm_get_parameter(int idx, uint64_t *value)
> {
> @@ -69,24 +72,31 @@ shared_info_t *map_shared_info(void)
> {
> struct xen_add_to_physmap xatp;
>
> + shinfo_pfn = e820_get_reserved_pfns(1);
> xatp.domid = DOMID_SELF;
> xatp.idx = 0;
> xatp.space = XENMAPSPACE_shared_info;
> - xatp.gpfn = virt_to_pfn(&shared_info);
> + xatp.gpfn = shinfo_pfn;
> if ( HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
> BUG();
> + if ( !shinfo_va )
> + shinfo_va = alloc_virt_kernel(1);
> + if ( !shinfo_va || map_frame_rw(shinfo_va, shinfo_pfn) )
> + BUG();
Now there's a new asymmetry: Here you check whether alloc_virt_kernel()
(appears to have) failed, whereas in the PV variant you don't. And it's
really only "appears to", as the function won't return 0 in the failure
case, afaics. I therefore think that extra condition simply wants
dropping here. Then
Reviewed-by: Jan Beulich <jbeul...@suse.com>
As for the other patch, happy to make both adjustments while committing.
As long as you agree, of course.
Jan
