xen/arch/x86/guest/hyperv/hyperv.c | 19 +++++++++---------- xen/arch/x86/include/asm/guest/hyperv.h | 3 --- 2 files changed, 9 insertions(+), 13 deletions(-)
Previously Xen placed the hypercall page at the highest possible MFN,
but this caused problems on systems where there is more than 36 bits
of physical address space.
In general, it also seems unreliable to assume that the highest possible
MFN is not already reserved for some other purpose.
Fixes: 620fc734f854 ("x86/hyperv: setup hypercall page")
Cc: Alejandro Vallejo <agarciav@amd.com>
Cc: Alexander M. Merritt <alexander@edera.dev>
Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
---
Changes from v2:
- Style fixes
- Change hypercall page notice to print the MFN instead, and use
dprintk
Changes from v1:
- Continue to use fixmap infrastructure
- Use panic in Hyper-V setup() function instead of returning -ENOMEM
on hypercall page allocation failure
xen/arch/x86/guest/hyperv/hyperv.c | 19 +++++++++----------
xen/arch/x86/include/asm/guest/hyperv.h | 3 ---
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
index 6989af38f1..f69f596441 100644
--- a/xen/arch/x86/guest/hyperv/hyperv.c
+++ b/xen/arch/x86/guest/hyperv/hyperv.c
@@ -98,10 +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;
+ void *hcall_page = alloc_xenheap_page();
+
+ if ( !hcall_page )
+ panic("Hyper-V: Failed to allocate hypercall trampoline page\n");
+
+ mfn = virt_to_mfn(hcall_page);
hypercall_msr.enable = 1;
hypercall_msr.guest_physical_address = mfn;
wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+
+ dprintk(XENLOG_INFO,
+ "Hyper-V: Allocated hypercall page at MFN %lx\n", mfn);
}
else
mfn = hypercall_msr.guest_physical_address;
@@ -187,14 +195,6 @@ static int cf_check ap_setup(void)
return setup_vp_assist();
}
-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");
-}
-
static int cf_check flush_tlb(
const cpumask_t *mask, const void *va, unsigned int flags)
{
@@ -211,7 +211,6 @@ static const struct hypervisor_ops __initconst_cf_clobber ops = {
.name = "Hyper-V",
.setup = setup,
.ap_setup = ap_setup,
- .e820_fixup = e820_fixup,
.flush_tlb = flush_tlb,
};
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.39.5 (Apple Git-154)On Mon Apr 28, 2025 at 8:57 PM BST, Ariadne Conill wrote:
> Previously Xen placed the hypercall page at the highest possible MFN,
> but this caused problems on systems where there is more than 36 bits
> of physical address space.
>
> In general, it also seems unreliable to assume that the highest possible
> MFN is not already reserved for some other purpose.
>
> Fixes: 620fc734f854 ("x86/hyperv: setup hypercall page")
> Cc: Alejandro Vallejo <agarciav@amd.com>
> Cc: Alexander M. Merritt <alexander@edera.dev>
> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
Reviewed-by: Alejandro Vallejo <agarciav@amd.com>
I'm happy with the patch as-is, and I'm equally happy...
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> index 6989af38f1..f69f596441 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -98,10 +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;
> + void *hcall_page = alloc_xenheap_page();
> +
> + if ( !hcall_page )
> + panic("Hyper-V: Failed to allocate hypercall trampoline page\n");
... with Roger's suggestion to probe <4G first with MEMF(32) here.
> +
> + mfn = virt_to_mfn(hcall_page);
> hypercall_msr.enable = 1;
> hypercall_msr.guest_physical_address = mfn;
> wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
> + dprintk(XENLOG_INFO,
> + "Hyper-V: Allocated hypercall page at MFN %lx\n", mfn);
nit: This is a personal preference thing, but I find %lx is very
unhelpful when the variable is something like 0x123, as 123 might be
misinterpreted to be in decimal. An mfn is not nearly as problematic as
an error code, but %#lx avoids the ambiguity altogether.
Cheers,
Alejandro
On 28.04.2025 21:57, Ariadne Conill wrote: > Previously Xen placed the hypercall page at the highest possible MFN, > but this caused problems on systems where there is more than 36 bits > of physical address space. Hmm, I should have asked already on the earlier version: What kinds of problems are these, beyond ... > In general, it also seems unreliable to assume that the highest possible > MFN is not already reserved for some other purpose. ... this particular aspect? I find it puzzling that such problems would depend on the number of physical address bits. Jan
On Tue Apr 29, 2025 at 9:28 AM BST, Jan Beulich wrote: > On 28.04.2025 21:57, Ariadne Conill wrote: >> Previously Xen placed the hypercall page at the highest possible MFN, >> but this caused problems on systems where there is more than 36 bits >> of physical address space. > > Hmm, I should have asked already on the earlier version: What kinds of > problems are these, beyond ... > >> In general, it also seems unreliable to assume that the highest possible >> MFN is not already reserved for some other purpose. > > ... this particular aspect? I find it puzzling that such problems would > depend on the number of physical address bits. > > Jan Pagefault on access (due to reserved bits being set) on access to the hypercall page. The available guest-physical address space doesn't seem to be as wide as advertised, though I didn't carry enough tests to single this as the only explanation. Seeing how we don't really know what's already on the last mfn this seems like a strict improvement irrespective of the actual cause of the fault. Cheers, Alejandro
On 29.04.2025 12:48, Alejandro Vallejo wrote: > On Tue Apr 29, 2025 at 9:28 AM BST, Jan Beulich wrote: >> On 28.04.2025 21:57, Ariadne Conill wrote: >>> Previously Xen placed the hypercall page at the highest possible MFN, >>> but this caused problems on systems where there is more than 36 bits >>> of physical address space. >> >> Hmm, I should have asked already on the earlier version: What kinds of >> problems are these, beyond ... >> >>> In general, it also seems unreliable to assume that the highest possible >>> MFN is not already reserved for some other purpose. >> >> ... this particular aspect? I find it puzzling that such problems would >> depend on the number of physical address bits. > > Pagefault on access (due to reserved bits being set) on access to the > hypercall page. The available guest-physical address space doesn't seem > to be as wide as advertised, though I didn't carry enough tests to > single this as the only explanation. Seeing how we don't really know > what's already on the last mfn this seems like a strict improvement > irrespective of the actual cause of the fault. No question there, yet the first paragraph is a little too vague for my taste. It'll also not help people later finding this commit and wondering what the issue was. Jan
On 2025-04-28 15:57, Ariadne Conill wrote:
> Previously Xen placed the hypercall page at the highest possible MFN,
> but this caused problems on systems where there is more than 36 bits
> of physical address space.
>
> In general, it also seems unreliable to assume that the highest possible
> MFN is not already reserved for some other purpose.
>
> Fixes: 620fc734f854 ("x86/hyperv: setup hypercall page")
> Cc: Alejandro Vallejo <agarciav@amd.com>
> Cc: Alexander M. Merritt <alexander@edera.dev>
> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Thanks,
Jason
© 2016 - 2025 Red Hat, Inc.