[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 07/12] x86/hyperv: setup hypercall page



On Wed, Jan 29, 2020 at 08:20:29PM +0000, Wei Liu wrote:
> Hyper-V uses a technique called overlay page for its hypercall page. It
> will insert a backing page to the guest when the hypercall functionality
> is enabled. That means we can use a page that is not backed by real
> memory for hypercall page.
> 
> Use the top-most addressable page for that purpose. Adjust e820 code
> accordingly.
> 
> We also need to register Xen's guest OS ID to Hyper-V. Use 0x3 as the
> vendor ID. Fix the comment in hyperv-tlfs.h while at it.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>
> ---
> v5:
> 1. use hypervisor_reserve_top_pages
> 2. add a macro for hypercall page mfn
> 3. address other misc comments
> 
> v4:
> 1. Use fixmap
> 2. Follow routines listed in TLFS
> ---
>  xen/arch/x86/e820.c                     |  5 +++
>  xen/arch/x86/guest/hyperv/hyperv.c      | 57 +++++++++++++++++++++++--
>  xen/include/asm-x86/guest/hyperv-tlfs.h |  5 ++-
>  xen/include/asm-x86/guest/hyperv.h      |  3 ++
>  4 files changed, 65 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> index 3892c9cfb7..99643f3ea0 100644
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -343,6 +343,7 @@ static unsigned long __init find_max_pfn(void)
>  {
>      unsigned int i;
>      unsigned long max_pfn = 0;
> +    unsigned long top_pfn = ((1ull << paddr_bits) - 1) >> PAGE_SHIFT;
>  
>      for (i = 0; i < e820.nr_map; i++) {
>          unsigned long start, end;
> @@ -357,6 +358,10 @@ static unsigned long __init find_max_pfn(void)
>              max_pfn = end;
>      }
>  
> +    top_pfn -= hypervisor_reserve_top_pages();
> +    if ( max_pfn >= top_pfn )
> +        max_pfn = top_pfn;

Hm, I'm not sure I see the point of this. The value returned by
find_max_pfn is the maximum RAM address found in the memory map, but
the physical address you are using to map the hypercall page is almost
certainly much higher than the maximum address found in the physmap
(and certainly not RAM), and hence I'm not sure what's the point of
this.

Also you haven't introduced a HyperV implementation of
hypervisor_reserve_top_pages so far, so it's hard to tell the intend
of this.

> +
>      return max_pfn;
>  }
>  
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c 
> b/xen/arch/x86/guest/hyperv/hyperv.c
> index 8d38313d7a..2bedcc438c 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -19,15 +19,26 @@
>   * Copyright (c) 2019 Microsoft.
>   */
>  #include <xen/init.h>
> +#include <xen/version.h>
>  
> +#include <asm/fixmap.h>
>  #include <asm/guest.h>
>  #include <asm/guest/hyperv-tlfs.h>
> +#include <asm/processor.h>
>  
>  struct ms_hyperv_info __read_mostly ms_hyperv;
>  
> -static const struct hypervisor_ops ops = {
> -    .name = "Hyper-V",
> -};
> +static uint64_t generate_guest_id(void)
> +{
> +    uint64_t id;
> +
> +    id = (uint64_t)HV_XEN_VENDOR_ID << 48;
> +    id |= (xen_major_version() << 16) | xen_minor_version();
> +
> +    return id;
> +}
> +
> +static const struct hypervisor_ops ops;
>  
>  const struct hypervisor_ops *__init hyperv_probe(void)
>  {
> @@ -72,6 +83,46 @@ const struct hypervisor_ops *__init hyperv_probe(void)
>      return &ops;
>  }
>  
> +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;
> +
> +    rdmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +    if ( !guest_id.raw )
> +    {
> +        guest_id.raw = generate_guest_id();
> +        wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id.raw);
> +    }
> +
> +    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;
> +        wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +    } else {
> +        mfn = hypercall_msr.guest_physical_address;
> +    }
> +
> +    rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +    BUG_ON(!hypercall_msr.enable);
> +
> +    set_fixmap_x(FIX_X_HYPERV_HCALL, mfn << PAGE_SHIFT);
> +}
> +
> +static void __init setup(void)
> +{
> +    setup_hypercall_page();
> +}
> +
> +static const struct hypervisor_ops ops = {
> +    .name = "Hyper-V",
> +    .setup = setup,
> +};
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/guest/hyperv-tlfs.h 
> b/xen/include/asm-x86/guest/hyperv-tlfs.h
> index 05c4044976..07db57b55f 100644
> --- a/xen/include/asm-x86/guest/hyperv-tlfs.h
> +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
> @@ -318,15 +318,16 @@ struct ms_hyperv_tsc_page {
>   *
>   * Bit(s)
>   * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
> - * 62:56 - Os Type; Linux is 0x100
> + * 62:56 - Os Type; Linux 0x1, FreeBSD 0x2, Xen 0x3
>   * 55:48 - Distro specific identification
> - * 47:16 - Linux kernel version number
> + * 47:16 - Guest OS version number
>   * 15:0  - Distro specific identification
>   *
>   *
>   */
>  
>  #define HV_LINUX_VENDOR_ID              0x8100
> +#define HV_XEN_VENDOR_ID                0x8300
>  union hv_guest_os_id
>  {
>      uint64_t raw;
> diff --git a/xen/include/asm-x86/guest/hyperv.h 
> b/xen/include/asm-x86/guest/hyperv.h
> index c7a7f32bd5..0dcd8082ad 100644
> --- a/xen/include/asm-x86/guest/hyperv.h
> +++ b/xen/include/asm-x86/guest/hyperv.h
> @@ -21,6 +21,9 @@
>  
>  #include <xen/types.h>
>  
> +/* Use top-most MFN for hypercall page */
> +#define HV_HCALL_MFN (((1ull << paddr_bits) - 1) >> HV_HYP_PAGE_SHIFT)

I think you should use PAGE_SHIFT here, or else you also need to make
sure the fixmap reserved entry is of size ((1 << HV_HYP_PAGE_SHIFT) -
1), and the call to set_fixmap_x in setup_hypercall_page needs to take
this into account AFAICT and not use PAGE_SHIFT.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.