Nested Virtualization: cpuid

PATCH In review Christoph Egger 2 participants 2 messages Apr 15, 2010 – Apr 16, 2010 2 messages not in the archive

lore

Share & tools

lore

Christoph Egger

author

2010-04-15 12:35 UTC #0 lore raw

PATCH 11/18 Nested Virtualization: cpuid

Signed-off-by Christoph Egger <[email protected]>

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

# HG changeset patch
# User cegger
# Date 1271330304 -7200
Pass cpuid feature bits to guest

xen/arch/x86/hvm/hvm.c +26 -0

diff -r e905f1d81e63 -r 01758c6195f4 xen/arch/x86/hvm/hvm.c
@@ -1970,12 +1970,38 @@ void hvm_cpuid(unsigned int input, unsig
             }
         }
         break;
+    case 0x80000000:
+        if ( !nestedhvm_enabled(v->domain) )
+            break;
+        if (*eax < 0x8000000a)
+            *eax = 0x8000000a;
+        break;
     case 0x80000001:
         /* We expose RDTSCP feature to guest only when
            tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
         if ( v->domain->arch.tsc_mode != TSC_MODE_DEFAULT ||
              !host_tsc_is_safe() )
             *edx &= ~bitmaskof(X86_FEATURE_RDTSCP);
+
+        /* Don't expose SVM feature bit when host does not run with
+         * nested paging enabled. This check is needed as long as
+         * we don't support shadow-on-shadow.
+         */
+        if ( !paging_mode_hap(v->domain) )
+            *ecx &= ~bitmaskof(X86_FEATURE_SVME);
+
+        /* Don't expose SVM feature bit when nestedhvm is disabled. */
+        if ( !nestedhvm_enabled(v->domain) )
+            *ecx &= ~bitmaskof(X86_FEATURE_SVME);
+        break;
+    case 0x8000000a:
+        /* Don't expose SVM feature bits when nestedhvm is disabled. */
+        if ( !nestedhvm_enabled(v->domain) ) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
+
+        hvm_nestedhvm_vcpu_features(v, eax, ebx, ecx, edx);
         break;
     }
 }

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Tim Deegan

2010-04-16 14:33 UTC #3 lore raw

Quoted 26 lines · Christoph Egger

Quoted 16 lines · On Friday 16 April 2010 15:00:47 Tim Deegan wrote:
Content-Description: xen_nh11_cpuid.diff
Quoted 14 lines
# HG changeset patch
# User cegger
# Date 1271330304 -7200
Pass cpuid feature bits to guest
diff -r e905f1d81e63 -r 01758c6195f4 xen/arch/x86/hvm/hvm.c
xen/arch/x86/hvm/hvm.c +2 -0
@@ -1970,12 +1970,38 @@ void hvm_cpuid(unsigned int input, unsig
             }
         }
         break;
+    case 0x80000000:
+        if ( !nestedhvm_enabled(v->domain) )
Quoted 8 lines
xen/arch/x86/hvm/hvm.c void hvm_cpuid(unsigned int input, unsig · line 1977
Quoted 3 lines
xen/arch/x86/hvm/hvm.c +3 -0
+            break;
+        if (*eax < 0x8000000a)
+            *eax = 0x8000000a;
I don't understand this. Surely you can't have set nestedhvm_enabled on a CPU that doesn't supoprt SVM (and therefore has at least that level of CPUID available). If that's been overwritten by the tools then it should be fixed in the tools, not the hypervisor.

Quoted 3 lines

Xen reads SVM feature bits only if cpuid level 0x8000000a is available. When you run Xen as HVM guest then Xen reads 0x80000001 if I remember correctly and the L2 guest can't use hap then.

Okay, but you're fixing this in the wrong place. Find out what's causing the cpuid to be capped at 0x80000001 (I think it'll be libxc) and fix it there.

Quoted 15 lines

Quoted 14 lines

xen/arch/x86/hvm/hvm.c +8 -0

+        break;
     case 0x80000001:
         /* We expose RDTSCP feature to guest only when
            tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1
 */ if ( v->domain->arch.tsc_mode != TSC_MODE_DEFAULT ||
              !host_tsc_is_safe() )
             *edx &= ~bitmaskof(X86_FEATURE_RDTSCP);
+
+        /* Don't expose SVM feature bit when host does not run with
+         * nested paging enabled. This check is needed as long as
+         * we don't support shadow-on-shadow.
+         */
+        if ( !paging_mode_hap(v->domain) )
+            *ecx &= ~bitmaskof(X86_FEATURE_SVME);

Quoted 7 lines

Quoted 3 lines
Surely !paging_mode_hap(d) -> !nestedhvm_enabled(d), and you don't need to check it here. Otherwise you're in for worse surprises than getting CPUID wrong.
This check is supposed to go away when shadow-on-shadow is implemented. This check makes sure that SVM feature bit is not propagated to the guest if you have hap=0 and nestedhvm=1 in the guest config file.

If you haven't implemented shadow-on-shadow then it's not safe to let nestedhvm_enabled(d) == 1 when paging_mode_hap(d) == 0.

If you've enforced that, then this check isn't needed. If you haven't than masking the CPUID bits won't make it safe, just safe from well-behaved guests.

Cheers,

Tim.

Quoted 38 lines

Quoted 27 lines

Cheers,
Tim.
Quoted 5 lines
xen/arch/x86/hvm/hvm.c +5 -0
+        /* Don't expose SVM feature bit when nestedhvm is disabled. */
+        if ( !nestedhvm_enabled(v->domain) )
+            *ecx &= ~bitmaskof(X86_FEATURE_SVME);
+        break;
+    case 0x8000000a:
xen/arch/x86/hvm/hvm.c void hvm_cpuid(unsigned int input, unsig · line 2007
Quoted 10 lines
xen/arch/x86/hvm/hvm.c +7 -0
+        /* Don't expose SVM feature bits when nestedhvm is disabled. */
+        if ( !nestedhvm_enabled(v->domain) ) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
+
+        hvm_nestedhvm_vcpu_features(v, eax, ebx, ecx, edx);
         break;
     }
 }
Content-Description: ATT00001.txt
Quoted 4 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Signature · 3 lines

Tim Deegan <[email protected]>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)