[PATCH] x86/hyperv: Adjust hypercall page placement

[PATCH] x86/hyperv: Adjust hypercall page placement

[Alejandro Vallejo]

Xen nowadays crashes under some Hyper-V configurations when
paddr_bits>36. At the 44bit boundary we reach an edge case in which the
end of the guest physical address space is not representable using 32bit
MFNs. Furthermore, it's an act of faith that the tail of the physical
address space has no reserved regions already.

This commit uses the first unused MFN rather than the last, thus
ensuring the hypercall page placement is more resilient against such
corner cases.

While at this, add an extra BUG_ON() to explicitly test for the
hypercall page being correctly set, and mark hcall_page_ready as
__ro_after_init.

Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
 xen/arch/x86/e820.c                     | 14 ++++++++++++++
 xen/arch/x86/guest/hyperv/hyperv.c      | 20 ++++++++++----------
 xen/arch/x86/include/asm/e820.h         |  1 +
 xen/arch/x86/include/asm/guest/hyperv.h |  3 ---
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index ca577c0bde..7be9a8db34 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -582,6 +582,20 @@ int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
     return 1;
 }
 
+uint64_t __init e820_reserve_hole(void)
+{
+    for ( unsigned int i = 0; i < e820.nr_map; i++ )
+    {
+        uint64_t hole = e820.map[i].addr + e820.map[i].size;
+        if ( e820_add_range(hole, hole + PAGE_SIZE, E820_RESERVED) )
+            return hole;
+    }
+
+    print_e820_memory_map(e820.map, e820.nr_map);
+    panic("Unable to find a hole in e820");
+}
+
+
 int __init e820_change_range_type(
     struct e820map *map, uint64_t s, uint64_t e,
     uint32_t orig_type, uint32_t new_type)
diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
index 6989af38f1..7617f94808 100644
--- a/xen/arch/x86/guest/hyperv/hyperv.c
+++ b/xen/arch/x86/guest/hyperv/hyperv.c
@@ -22,7 +22,8 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
 DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
 
 unsigned int __read_mostly hv_max_vp_index;
-static bool __read_mostly hcall_page_ready;
+static bool __ro_after_init hcall_page_ready;
+static unsigned long __ro_after_init hcall_page_mfn = INVALID_MFN_RAW;
 
 static uint64_t generate_guest_id(void)
 {
@@ -84,7 +85,6 @@ static void __init setup_hypercall_page(void)
 {
     union hv_x64_msr_hypercall_contents hypercall_msr;
     union hv_guest_os_id guest_id;
-    unsigned long mfn;
 
     BUILD_BUG_ON(HV_HYP_PAGE_SHIFT != PAGE_SHIFT);
 
@@ -98,18 +98,18 @@ static void __init setup_hypercall_page(void)
     rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
     if ( !hypercall_msr.enable )
     {
-        mfn = HV_HCALL_MFN;
         hypercall_msr.enable = 1;
-        hypercall_msr.guest_physical_address = mfn;
+        hypercall_msr.guest_physical_address = hcall_page_mfn;
         wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
     }
     else
-        mfn = hypercall_msr.guest_physical_address;
+        hcall_page_mfn = hypercall_msr.guest_physical_address;
 
     rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
     BUG_ON(!hypercall_msr.enable);
+    BUG_ON(hypercall_msr.guest_physical_address != hcall_page_mfn);
 
-    set_fixmap_x(FIX_X_HYPERV_HCALL, mfn << PAGE_SHIFT);
+    set_fixmap_x(FIX_X_HYPERV_HCALL, hcall_page_mfn << PAGE_SHIFT);
 
     hcall_page_ready = true;
 }
@@ -189,10 +189,10 @@ static int cf_check ap_setup(void)
 
 static void __init cf_check e820_fixup(void)
 {
-    uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
-
-    if ( !e820_add_range(s, s + PAGE_SIZE, E820_RESERVED) )
-        panic("Unable to reserve Hyper-V hypercall range\n");
+    printk(XENLOG_DEBUG "reserving hole for the hypercall page");
+    hcall_page_mfn = e820_reserve_hole() >> PAGE_SHIFT;
+    printk(XENLOG_INFO "hyperv: hypercall page %#lx",
+           hcall_page_mfn << PAGE_SHIFT);
 }
 
 static int cf_check flush_tlb(
diff --git a/xen/arch/x86/include/asm/e820.h b/xen/arch/x86/include/asm/e820.h
index 8e7644f887..74cbbea62d 100644
--- a/xen/arch/x86/include/asm/e820.h
+++ b/xen/arch/x86/include/asm/e820.h
@@ -26,6 +26,7 @@ struct e820map {
 extern int sanitize_e820_map(struct e820entry *biosmap, unsigned int *pnr_map);
 extern int e820_all_mapped(u64 start, u64 end, unsigned type);
 extern int reserve_e820_ram(struct e820map *map, uint64_t s, uint64_t e);
+extern uint64_t e820_reserve_hole(void);
 extern int e820_change_range_type(
     struct e820map *map, uint64_t s, uint64_t e,
     uint32_t orig_type, uint32_t new_type);
diff --git a/xen/arch/x86/include/asm/guest/hyperv.h b/xen/arch/x86/include/asm/guest/hyperv.h
index c05efdce71..5792e77104 100644
--- a/xen/arch/x86/include/asm/guest/hyperv.h
+++ b/xen/arch/x86/include/asm/guest/hyperv.h
@@ -10,9 +10,6 @@
 
 #include <xen/types.h>
 
-/* Use top-most MFN for hypercall page */
-#define HV_HCALL_MFN   (((1ull << paddr_bits) - 1) >> HV_HYP_PAGE_SHIFT)
-
 /*
  * The specification says: "The partition reference time is computed
  * by the following formula:
-- 
2.43.0

Re: [PATCH] x86/hyperv: Adjust hypercall page placement

[Alejandro Vallejo]

On Thu Apr 24, 2025 at 1:45 PM BST, Alejandro Vallejo wrote:
> Xen nowadays crashes under some Hyper-V configurations when
> paddr_bits>36. At the 44bit boundary we reach an edge case in which the
> end of the guest physical address space is not representable using 32bit
> MFNs. Furthermore, it's an act of faith that the tail of the physical
> address space has no reserved regions already.
>
> This commit uses the first unused MFN rather than the last, thus
> ensuring the hypercall page placement is more resilient against such
> corner cases.
>
> While at this, add an extra BUG_ON() to explicitly test for the
> hypercall page being correctly set, and mark hcall_page_ready as
> __ro_after_init.
>
> Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>

After a side discussion, this seems on the unsafe side of things due to
potential collision with MMIO. I'll resend (though not today) with the
page overlapping a RAM page instead. Possibly the last page of actual
RAM.

Cheers,
Alejandro

Re: [PATCH] x86/hyperv: Adjust hypercall page placement

[Ariadne Conill]

Hi,

> On Apr 24, 2025, at 6:48 AM, Alejandro Vallejo <agarciav@amd.com> wrote:
> 
> On Thu Apr 24, 2025 at 1:45 PM BST, Alejandro Vallejo wrote:
>> Xen nowadays crashes under some Hyper-V configurations when
>> paddr_bits>36. At the 44bit boundary we reach an edge case in which the
>> end of the guest physical address space is not representable using 32bit
>> MFNs. Furthermore, it's an act of faith that the tail of the physical
>> address space has no reserved regions already.
>> 
>> This commit uses the first unused MFN rather than the last, thus
>> ensuring the hypercall page placement is more resilient against such
>> corner cases.
>> 
>> While at this, add an extra BUG_ON() to explicitly test for the
>> hypercall page being correctly set, and mark hcall_page_ready as
>> __ro_after_init.
>> 
>> Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> 
> After a side discussion, this seems on the unsafe side of things due to
> potential collision with MMIO. I'll resend (though not today) with the
> page overlapping a RAM page instead. Possibly the last page of actual
> RAM.

We have been working on bringing Xen up on Azure over at Edera, and have encountered this problem.  Our solution to this problem was to change Xen to handle the hypercall trampoline page in the same way as Linux: dynamically allocating a page from the heap and then marking it as executable.

This approach should avoid the issues with MMIO and page overlaps.  Would it be more interesting to start with our patch instead?

Ariadne

Re: [PATCH] x86/hyperv: Adjust hypercall page placement

[Alejandro Vallejo]

On Thu Apr 24, 2025 at 7:22 PM BST, Ariadne Conill wrote:
> Hi,
>
>> On Apr 24, 2025, at 6:48 AM, Alejandro Vallejo <agarciav@amd.com> wrote:
>> 
>> On Thu Apr 24, 2025 at 1:45 PM BST, Alejandro Vallejo wrote:
>>> Xen nowadays crashes under some Hyper-V configurations when
>>> paddr_bits>36. At the 44bit boundary we reach an edge case in which the
>>> end of the guest physical address space is not representable using 32bit
>>> MFNs. Furthermore, it's an act of faith that the tail of the physical
>>> address space has no reserved regions already.
>>> 
>>> This commit uses the first unused MFN rather than the last, thus
>>> ensuring the hypercall page placement is more resilient against such
>>> corner cases.
>>> 
>>> While at this, add an extra BUG_ON() to explicitly test for the
>>> hypercall page being correctly set, and mark hcall_page_ready as
>>> __ro_after_init.
>>> 
>>> Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
>>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> 
>> After a side discussion, this seems on the unsafe side of things due to
>> potential collision with MMIO. I'll resend (though not today) with the
>> page overlapping a RAM page instead. Possibly the last page of actual
>> RAM.
>
> We have been working on bringing Xen up on Azure over at Edera, and
> have encountered this problem.  Our solution to this problem was to
> change Xen to handle the hypercall trampoline page in the same way as
> Linux: dynamically allocating a page from the heap and then marking it
> as executable.
>
> This approach should avoid the issues with MMIO and page overlaps.

Yes, that's what I meant by overlapping RAM. Overlaying the hypercall
page on top of existing RAM rather than trying to find a suitable hole.

> Would it be more interesting to start with our patch instead?

If you have it ready to go, for sure. My ability to test any of this is
fairly limited. I suspect the VM is just not getting 48 bits worth of
guest-physical address space, and so making any hypercall turns into an
EPT violation.

I couldn't run the tests that would definitely prove it though

From the little I saw of the dmesg going forward, I suspect there's more
required (at least in time handling) to enable support in gen2
insteances.

Cheers,
Alejandro

Re: [PATCH] x86/hyperv: Adjust hypercall page placement

[Trolle Selander]

[-- Attachment #1: Type: text/plain, Size: 3270 bytes --]

I worked on Xen-on-Azure last summer in my previous position. Allocating a
heap page was how we solved this particular issue in our branch as well.

I see you say you're still 'working on bringing Xen up on Azure', so I'm
not sure how far along your branch / patch set is, but for what it's worth,
we had "everything" working, including use of VMBus and passthrough devices
in dom0.

The Azure version of the product got put on ice, and the code wasn't in a
very 'upstreamable' state due to how patches/branches were managed, but
bottom line is I know every bump on the Xen-on-Azure road, so may be able
to help get an upstream version of this 'the rest of the way' if you start
pushing it to xen-devel.

Cheers,
Trolle

On Thu, Apr 24, 2025 at 4:47 PM Alejandro Vallejo <agarciav@amd.com> wrote:

> On Thu Apr 24, 2025 at 7:22 PM BST, Ariadne Conill wrote:
> > Hi,
> >
> >> On Apr 24, 2025, at 6:48 AM, Alejandro Vallejo <agarciav@amd.com>
> wrote:
> >>
> >> On Thu Apr 24, 2025 at 1:45 PM BST, Alejandro Vallejo wrote:
> >>> Xen nowadays crashes under some Hyper-V configurations when
> >>> paddr_bits>36. At the 44bit boundary we reach an edge case in which the
> >>> end of the guest physical address space is not representable using
> 32bit
> >>> MFNs. Furthermore, it's an act of faith that the tail of the physical
> >>> address space has no reserved regions already.
> >>>
> >>> This commit uses the first unused MFN rather than the last, thus
> >>> ensuring the hypercall page placement is more resilient against such
> >>> corner cases.
> >>>
> >>> While at this, add an extra BUG_ON() to explicitly test for the
> >>> hypercall page being correctly set, and mark hcall_page_ready as
> >>> __ro_after_init.
> >>>
> >>> Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
> >>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> >>
> >> After a side discussion, this seems on the unsafe side of things due to
> >> potential collision with MMIO. I'll resend (though not today) with the
> >> page overlapping a RAM page instead. Possibly the last page of actual
> >> RAM.
> >
> > We have been working on bringing Xen up on Azure over at Edera, and
> > have encountered this problem.  Our solution to this problem was to
> > change Xen to handle the hypercall trampoline page in the same way as
> > Linux: dynamically allocating a page from the heap and then marking it
> > as executable.
> >
> > This approach should avoid the issues with MMIO and page overlaps.
>
> Yes, that's what I meant by overlapping RAM. Overlaying the hypercall
> page on top of existing RAM rather than trying to find a suitable hole.
>
> > Would it be more interesting to start with our patch instead?
>
> If you have it ready to go, for sure. My ability to test any of this is
> fairly limited. I suspect the VM is just not getting 48 bits worth of
> guest-physical address space, and so making any hypercall turns into an
> EPT violation.
>
> I couldn't run the tests that would definitely prove it though
>
> From the little I saw of the dmesg going forward, I suspect there's more
> required (at least in time handling) to enable support in gen2
> insteances.
>
> Cheers,
> Alejandro
>
>
>

[-- Attachment #2: Type: text/html, Size: 4147 bytes --]

Re: [PATCH] x86/hyperv: Adjust hypercall page placement

[Jan Beulich]

On 24.04.2025 14:45, Alejandro Vallejo wrote:
> Xen nowadays crashes under some Hyper-V configurations when
> paddr_bits>36. At the 44bit boundary we reach an edge case in which the
> end of the guest physical address space is not representable using 32bit
> MFNs. Furthermore, it's an act of faith that the tail of the physical
> address space has no reserved regions already.
> 
> This commit uses the first unused MFN rather than the last, thus
> ensuring the hypercall page placement is more resilient against such
> corner cases.
> 
> While at this, add an extra BUG_ON() to explicitly test for the
> hypercall page being correctly set, and mark hcall_page_ready as
> __ro_after_init.
> 
> Fixes: 620fc734f854("x86/hyperv: setup hypercall page")
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>

Just in case you re-submit (as per later discussion), and respective code
was to stay, two style nits:

> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -582,6 +582,20 @@ int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
>      return 1;
>  }
>  
> +uint64_t __init e820_reserve_hole(void)
> +{
> +    for ( unsigned int i = 0; i < e820.nr_map; i++ )
> +    {
> +        uint64_t hole = e820.map[i].addr + e820.map[i].size;
> +        if ( e820_add_range(hole, hole + PAGE_SIZE, E820_RESERVED) )

Blank line between declaration(s) and statement8s) please.

> +            return hole;
> +    }
> +
> +    print_e820_memory_map(e820.map, e820.nr_map);
> +    panic("Unable to find a hole in e820");
> +}
> +
> +

No double blank lines please.

Jan