Re: [Xen-devel] [PATCH v3 01/16] hypervisor part of add vmware_hw to xl.cfg

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Sep 8, 2014 at 2:15 PM, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
> If non-zero then
>   Return VMware's cpuid leaves.
>
> The support of hypervisor cpuid leaves has not been agreed to.
>
> MicroSoft Hyper-V (AKA viridian) currently must be at 0x40000000.
>
> VMware currently must be at 0x40000000.
>
> KVM currently must be at 0x40000000 (from Seabios).
>
> Seabios will find xen at 0x40000000, 0x40000100, 0x40000200 ..
> 0x40010000.
>
> http://download.microsoft.com/download/F/B/0/FB0D01A3-8E3A-4F5F-AA59-08C8026D3B8A/requirements-for-implementing-microsoft-hypervisor-interface.docx
>
> http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458
>
> http://lwn.net/Articles/301888/
>   Attempted to get this cleaned up.
>
> So based on this, I picked the order:
>
> 0x40000000 is viridian, vmware or xen
> 0x40000100 is vmware or xen
> 0x40000200 is xen
>
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>

It looks like you need to update the description here.  The subject /
first line should be "xen: Add support for VMWare cpuid leaves" (since
you're doing stuff in the hypervisor and not in xl.cfg).  The
description should say that you're introducing HVM_PARAM_VMWARE_HW.

Re the viridian / vmware coexisting thing:

If the vmware tools supported looking for the cpuid leaf at
0x40000100, then I'd say we should allow them both to be set: after
all, at some point in the future vmware might enable viridian support,
at which case a guest running in vmware might expect to find both
viridian and vmware leaves.

But if they don't, then there's really not  much point in allowing the
vmware leaf to be at 0x40000100, because no one will read it.  In that
case we should disallow setting both.

Patch looks fine to me otherwise.

 -George

> ---
>  xen/arch/x86/hvm/Makefile        |  3 +-
>  xen/arch/x86/hvm/hvm.c           | 22 +++++++++++++
>  xen/arch/x86/hvm/vmware/Makefile |  1 +
>  xen/arch/x86/hvm/vmware/cpuid.c  | 71 
> ++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/traps.c             |  7 +++-
>  xen/include/asm-x86/hvm/hvm.h    |  3 ++
>  xen/include/asm-x86/hvm/vmware.h | 31 ++++++++++++++++++
>  xen/include/public/hvm/params.h  |  5 ++-
>  8 files changed, 140 insertions(+), 3 deletions(-)
>  create mode 100644 xen/arch/x86/hvm/vmware/Makefile
>  create mode 100644 xen/arch/x86/hvm/vmware/cpuid.c
>  create mode 100644 xen/include/asm-x86/hvm/vmware.h
>
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index eea5555..77598a6 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -1,5 +1,6 @@
>  subdir-y += svm
>  subdir-y += vmx
> +subdir-y += vmware
>
>  obj-y += asid.o
>  obj-y += emulate.o
> @@ -22,4 +23,4 @@ obj-y += vlapic.o
>  obj-y += vmsi.o
>  obj-y += vpic.o
>  obj-y += vpt.o
> -obj-y += vpmu.o
> \ No newline at end of file
> +obj-y += vpmu.o
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 8d905d3..365331a 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -57,6 +57,7 @@
>  #include <asm/hvm/cacheattr.h>
>  #include <asm/hvm/trace.h>
>  #include <asm/hvm/nestedhvm.h>
> +#include <asm/hvm/vmware.h>
>  #include <asm/mtrr.h>
>  #include <asm/apic.h>
>  #include <public/sched.h>
> @@ -4228,6 +4229,9 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>      if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )
>          return;
>
> +    if ( cpuid_vmware_leaves(input, eax, ebx, ecx, edx) )
> +        return;
> +
>      if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
>          return;
>
> @@ -5692,6 +5696,24 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>
>                  break;
>              }
> +            case HVM_PARAM_VMWARE_HW:
> +                /*
> +                 * This should only ever be set non-zero one time by
> +                 * the tools and is read only by the guest.
> +                 */
> +                if ( d == current->domain )
> +                {
> +                    rc = -EPERM;
> +                    break;
> +                }
> +                if ( d->arch.hvm_domain.params[HVM_PARAM_VMWARE_HW] &&
> +                     d->arch.hvm_domain.params[HVM_PARAM_VMWARE_HW] !=
> +                     a.value )
> +                {
> +                    rc = -EEXIST;
> +                    break;
> +                }
> +                break;
>              }
>
>              if ( rc == 0 )
> diff --git a/xen/arch/x86/hvm/vmware/Makefile 
> b/xen/arch/x86/hvm/vmware/Makefile
> new file mode 100644
> index 0000000..3fb2e0b
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmware/Makefile
> @@ -0,0 +1 @@
> +obj-y += cpuid.o
> diff --git a/xen/arch/x86/hvm/vmware/cpuid.c b/xen/arch/x86/hvm/vmware/cpuid.c
> new file mode 100644
> index 0000000..03b74bf
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmware/cpuid.c
> @@ -0,0 +1,71 @@
> +/*
> + * arch/x86/hvm/vmware/cpuid.c
> + *
> + * Copyright (C) 2012 Verizon Corporation
> + *
> + * This file is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License Version 2 (GPLv2)
> + * as published by the Free Software Foundation.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details. <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/sched.h>
> +
> +#include <asm/hvm/hvm.h>
> +#include <asm/hvm/vmware.h>
> +
> +int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
> +                        uint32_t *ecx, uint32_t *edx)
> +{
> +    struct domain *d = current->domain;
> +    /* Optionally shift out of the way of Viridian architectural leaves. */
> +    uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
> +
> +    if ( !is_vmware_domain(d) )
> +        return 0;
> +
> +    switch ( idx - base )
> +    {
> +    case 0x0:
> +        *eax = base + 0x10; /* Largest leaf */
> +        *ebx = 0x61774d56;  /* "VMwa" */
> +        *ecx = 0x4d566572;  /* "reVM" */
> +        *edx = 0x65726177;  /* "ware" */
> +        break;
> +
> +    case 0x1 ... 0xf:
> +        *eax = 0;          /* Reserved */
> +        *ebx = 0;          /* Reserved */
> +        *ecx = 0;          /* Reserved */
> +        *edx = 0;          /* Reserved */
> +        break;
> +
> +    case 0x10:
> +        /* (Virtual) TSC frequency in kHz. */
> +        *eax =  d->arch.tsc_khz;
> +        /* (Virtual) Bus (local apic timer) frequency in kHz. */
> +        *ebx = 1000000000ull / APIC_BUS_CYCLE_NS / 1000ull;
> +        *ecx = 0;          /* Reserved */
> +        *edx = 0;          /* Reserved */
> +        break;
> +
> +    default:
> +        return 0;
> +    }
> +
> +    return 1;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 10fc2ca..932cd3d 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -686,9 +686,14 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t 
> sub_idx,
>  {
>      struct domain *d = current->domain;
>      /* Optionally shift out of the way of Viridian architectural leaves. */
> -    uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
> +    uint32_t base = 0x40000000;
>      uint32_t limit, dummy;
>
> +    if ( is_viridian_domain(d) )
> +        base += 0x100;
> +    if ( is_vmware_domain(d) )
> +        base += 0x100;
> +
>      idx -= base;
>      if ( idx > XEN_CPUID_MAX_NUM_LEAVES )
>          return 0; /* Avoid unnecessary pass through domain_cpuid() */
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 1123857..546210a 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -347,6 +347,9 @@ static inline unsigned long hvm_get_shadow_gs_base(struct 
> vcpu *v)
>  #define is_viridian_domain(_d)                                             \
>   (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
>
> +#define is_vmware_domain(_d)                                             \
> + (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VMWARE_HW]))
> +
>  void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
>                                 uint32_t *eax, uint32_t *ebx,
>                                 uint32_t *ecx, uint32_t *edx);
> diff --git a/xen/include/asm-x86/hvm/vmware.h 
> b/xen/include/asm-x86/hvm/vmware.h
> new file mode 100644
> index 0000000..f254106
> --- /dev/null
> +++ b/xen/include/asm-x86/hvm/vmware.h
> @@ -0,0 +1,31 @@
> +/*
> + * asm-x86/hvm/vmware.h
> + *
> + * Copyright (C) 2012 Verizon Corporation
> + *
> + * This file is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License Version 2 (GPLv2)
> + * as published by the Free Software Foundation.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details. <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef ASM_X86_HVM_VMWARE_H__
> +#define ASM_X86_HVM_VMWARE_H__
> +
> +int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
> +                        uint32_t *ecx, uint32_t *edx);
> +
> +#endif /* ASM_X86_HVM_VMWARE_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 614ff5f..dee6d68 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -151,6 +151,9 @@
>  /* Location of the VM Generation ID in guest physical address space. */
>  #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
>
> -#define HVM_NR_PARAMS          35
> +/* Params for VMware */
> +#define HVM_PARAM_VMWARE_HW                 35
> +
> +#define HVM_NR_PARAMS          36
>
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> --
> 1.8.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel