Hi,
> On Apr 25, 2025, at 2:02 PM, Jason Andryuk <jason.andr...@amd.com> wrote:
>
> On 2025-04-25 12:51, 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 <agarc...@amd.com>
>> Cc: Alexander M. Merritt <alexan...@edera.dev>
>> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>> ---
>> xen/arch/x86/guest/hyperv/hyperv.c | 39 ++++++++++---------
>> xen/arch/x86/include/asm/fixmap.h | 3 --
>> xen/arch/x86/include/asm/guest/hyperv-hcall.h | 12 +++---
>> xen/arch/x86/include/asm/guest/hyperv-tlfs.h | 2 +
>> xen/arch/x86/include/asm/guest/hyperv.h | 3 --
>> xen/arch/x86/xen.lds.S | 4 --
>> 6 files changed, 28 insertions(+), 35 deletions(-)
>> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c
>> b/xen/arch/x86/guest/hyperv/hyperv.c
>> index 6989af38f1..637b4bf335 100644
>> --- a/xen/arch/x86/guest/hyperv/hyperv.c
>> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
>
>> @@ -98,10 +97,22 @@ static void __init setup_hypercall_page(void)
>> rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>> if ( !hypercall_msr.enable )
>> {
>> - mfn = HV_HCALL_MFN;
>> + hv_hcall_page = alloc_xenheap_page();
>> + if ( !hv_hcall_page )
>> + {
>> + printk("Hyper-V: Failed to allocate hypercall trampoline
>> page\n");
>
> Minor, but maybe panic() here and avoid changing the return type?
Sure, can do that.
>
>> + return -ENOMEM;
>> + }
>> +
>> + printk("Hyper-V: Allocated hypercall page @ %p.\n", hv_hcall_page);
>> +
>> + mfn = virt_to_mfn(hv_hcall_page);
>> hypercall_msr.enable = 1;
>> hypercall_msr.guest_physical_address = mfn;
>> wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>> +
>> + start = (unsigned long) hv_hcall_page;
>> + modify_xen_mappings(start, start + PAGE_SIZE, PAGE_HYPERVISOR_RX);
>> }
>> else
>> mfn = hypercall_msr.guest_physical_address;
>
>> diff --git a/xen/arch/x86/include/asm/guest/hyperv-hcall.h
>> b/xen/arch/x86/include/asm/guest/hyperv-hcall.h
>> index b76dbf9ccc..b73edca7c6 100644
>> --- a/xen/arch/x86/include/asm/guest/hyperv-hcall.h
>> +++ b/xen/arch/x86/include/asm/guest/hyperv-hcall.h
>> @@ -20,13 +20,13 @@ static inline uint64_t hv_do_hypercall(uint64_t control,
>> paddr_t input_addr,
>> paddr_t output_addr)
>> {
>> uint64_t status;
>> - register unsigned long r8 asm ( "r8" ) = output_addr;
>> /* See TLFS for volatile registers */
>> - asm volatile ( "call hv_hcall_page"
>> + asm volatile ( "mov %[output_addr], %%r8\n"
>
> Don't you need to list r8 as clobbered? Or maybe just retain the r8 handling
> above and below to avoid this mov.
This can probably mostly be reverted if we get the fixmap working.
The issue we were facing before was a situation where calling hv_hcall_page
directly resulted in a page fault due to NX-bit being set on the page.
>> + "call *%[target_addr]"
>
> It might be preferable to retain a direct call which can still be installed
> with __set_fixmap_x. Otherwise, __set_fixmap_x can be removed
I think we should use fixmap, I just need to figure out what that looks like.
Will send v2 patch on Monday.
Ariadne
