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

Re: [Xen-devel] [PATCH v4 01/16] xen: Add support for VMware cpuid leaves



On 09/11/14 15:49, Andrew Cooper wrote:
On 11/09/2014 19:36, Don Slutz wrote:
This is done by adding HVM_PARAM_VMWARE_HW. It is set to the VMware
virtual hardware version.

Currently 0, 3-4, 6-11 are good values.  However the
code only checks for == 0 or != 0.

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:

Xen at 0x40000000 or
Viridian or VMware at 0x40000000 and Xen at 0x40000100

If both Viridian and VMware selected, report an error.

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
---
  xen/arch/x86/hvm/Makefile        |  3 +-
  xen/arch/x86/hvm/hvm.c           | 32 +++++++++++++++++++
  xen/arch/x86/hvm/vmware/Makefile |  1 +
  xen/arch/x86/hvm/vmware/cpuid.c  | 69 ++++++++++++++++++++++++++++++++++++++++
  xen/arch/x86/traps.c             |  8 +++--
  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, 148 insertions(+), 4 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
This hunk is unrelated, but is perhaps something better fixed.  A
passing note in the commit message perhaps?


Sure.

[snip -- different thread]


+#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;
+
+    if ( !is_vmware_domain(d) )
+        return 0;
+
+    switch ( idx - 0x40000000 )
+    {
+    case 0x0:
Do these leaves have semantic names from a spec, which could live in an
appropriate header file?

Not sure.

http://lwn.net/Articles/301888/

Which is from Oct 2008 -- Looks like VMware went with this and no one
else.

I can only find list postings that refer the the statement:

"
VMware hardware version 7 defines some of these cpuid levels, below is a brief description about those. These levels can be implemented by other hypervisors
too so that Linux has a standard way of communicating to any hypervisor.

Leaf 0x40000000, Hypervisor CPUID information
# EAX: The maximum input value for hypervisor CPUID info (0x40000010).
# EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware"

Leaf 0x40000010, Timing information.
# EAX: (Virtual) TSC frequency in kHz.
# EBX: (Virtual) Bus (local apic timer) frequency in kHz.
# ECX, EDX: RESERVED
"

There are a few about 0x40000010 as HYPERVISOR_TIMING_LEAF
or  HYPERVISOR_VMWARE_TIMING_LEAF.

All from late Sep 2008 (https://lkml.org/lkml/2008/9/29/45)

But I also found that I should be checking for >= 7:

The hypervisor present bit and hypervisor information leaf are only defined for products based on VMware hardware version 7.

So I will be changing this.



+    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;
At least 1 pair of brackets please, especially as the placement of
brackets affects the result of this particular calculation.

See other thread.

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 10fc2ca..f353f42 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -685,8 +685,12 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t 
sub_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;
+    /*
+     * Optionally shift out of the way of Viridian or VMware
+     * architectural leaves.
+     */
+    uint32_t base = is_viridian_domain(d) | is_vmware_domain(d) ?
+        0x40000100 : 0x40000000;
Again, brackets please for binary operators.  (We have had one recent
slipup because of the precedence of | and ?:)

I do not think they are needed, but can add them.  A separate
thread also talks about this.


Furthermore, I think you mean (is_viridian_domain(d) ||
is_vmware_domain(d)) ? 0x40000100 : 0x40000000; which allows for short
circuiting of is_vmware_domain().


Boris Ostrovsky siad the same.  I will switch to "||".


      uint32_t limit, dummy;
idx -= base;
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__
#include <xen/types.h> (IIRC) please, or uint32_t could cause a
compilation failure given specific orderings of #include's in
translation units.

I am fine with adding this.

    -Don Slutz

~Andrew




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


 


Rackspace

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