All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave"
@ 2015-11-27 11:07 Jan Beulich
  2015-11-27 16:00 ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2015-11-27 11:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]

... or when the guest has the XSAVE feature hidden by CPUID policy.
Not doing so is at best confusing to guests.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
One question is whether we shouldn't switch leaf 7 to white listing,
just like done for PV guests.

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -265,7 +265,8 @@ static void __cpuinit generic_identify(s
 	if ( c->cpuid_level >= 0x00000007 )
 		cpuid_count(0x00000007, 0, &tmp,
 			    &c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)],
-			    &tmp, &tmp);
+			    &c->x86_capability[cpufeat_word(X86_FEATURE_PREFETCHWT1)],
+			    &tmp);
 }
 
 /*
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4556,10 +4556,15 @@ void hvm_cpuid(unsigned int input, unsig
         if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
             __clear_bit(X86_FEATURE_APIC & 31, edx);
 
-        /* Fix up OSXSAVE. */
-        if ( cpu_has_xsave )
-            *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ?
-                     cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;
+        /* Fix up XSAVE, OSXSAVE, and AVX. */
+        if ( !cpu_has_xsave )
+            *ecx &= ~cpufeat_mask(X86_FEATURE_XSAVE);
+        if ( !(*ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ecx &= ~(cpufeat_mask(X86_FEATURE_OSXSAVE) |
+                      cpufeat_mask(X86_FEATURE_AVX) |
+                      cpufeat_mask(X86_FEATURE_F16C));
+        else if ( v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE )
+            *ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);
 
         /* Don't expose PCID to non-hap hvm. */
         if ( !hap_enabled(d) )
@@ -4585,12 +4590,36 @@ void hvm_cpuid(unsigned int input, unsig
         /* Don't expose INVPCID to non-hap hvm. */
         if ( (count == 0) && !hap_enabled(d) )
             *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
+
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ebx &= ~(cpufeat_mask(X86_FEATURE_AVX2) |
+                      cpufeat_mask(X86_FEATURE_MPX) |
+                      cpufeat_mask(X86_FEATURE_AVX512F));
+        if ( !(*ebx & cpufeat_mask(X86_FEATURE_AVX512F)) )
+        {
+            *ebx &= ~(cpufeat_mask(X86_FEATURE_AVX512DQ) |
+                      cpufeat_mask(X86_FEATURE_AVX512IFMA) |
+                      cpufeat_mask(X86_FEATURE_AVX512PF) |
+                      cpufeat_mask(X86_FEATURE_AVX512ER) |
+                      cpufeat_mask(X86_FEATURE_AVX512CD) |
+                      cpufeat_mask(X86_FEATURE_AVX512BW) |
+                      cpufeat_mask(X86_FEATURE_AVX512VL));
+            *ecx &= ~cpufeat_mask(X86_FEATURE_AVX512VBMI);
+        }
         break;
     case 0xb:
         /* Fix the x2APIC identifier. */
         *edx = v->vcpu_id * 2;
         break;
-    case 0xd:
+
+    case XSTATE_CPUID:
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+        {
+            *edx = *ecx = *ebx = *eax = 0;
+            break;
+        }
         /* EBX value of main leaf 0 depends on enabled xsave features */
         if ( count == 0 && v->arch.xcr0 ) 
         {
@@ -4637,6 +4666,11 @@ void hvm_cpuid(unsigned int input, unsig
         /* Hide data breakpoint extensions if the hardware has no support. */
         if ( !boot_cpu_has(X86_FEATURE_DBEXT) )
             *ecx &= ~cpufeat_mask(X86_FEATURE_DBEXT);
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ecx &= ~(cpufeat_mask(X86_FEATURE_XOP) |
+                      cpufeat_mask(X86_FEATURE_LWP) |
+                      cpufeat_mask(X86_FEATURE_FMA4));
         break;
 
     case 0x80000008:
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -11,7 +11,7 @@
 
 #include <xen/const.h>
 
-#define NCAPINTS	8	/* N 32-bit words worth of info */
+#define NCAPINTS	9	/* N 32-bit words worth of info */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */
 #define X86_FEATURE_FPU		(0*32+ 0) /* Onboard FPU */
@@ -159,9 +159,21 @@
 #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
 #define X86_FEATURE_MPX		(7*32+14) /* Memory Protection Extensions */
 #define X86_FEATURE_CAT 	(7*32+15) /* Cache Allocation Technology */
+#define X86_FEATURE_AVX512F	(7*32+16) /* AVX-512 instructions */
+#define X86_FEATURE_AVX512DQ	(7*32+17) /* AVX-512 double/quad instructions */
 #define X86_FEATURE_RDSEED	(7*32+18) /* RDSEED instruction */
 #define X86_FEATURE_ADX		(7*32+19) /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
+#define X86_FEATURE_AVX512IFMA	(7*32+21) /* AVX-512 integer FMA instructions */
+#define X86_FEATURE_AVX512PF	(7*32+26) /* AVX-512 prefetch instructions */
+#define X86_FEATURE_AVX512ER	(7*32+27) /* AVX-512 exp/recip instructions */
+#define X86_FEATURE_AVX512CD	(7*32+28) /* AVX-512 conflict detect instructions */
+#define X86_FEATURE_AVX512BW	(7*32+30) /* AVX-512 byte/word instructions */
+#define X86_FEATURE_AVX512VL	(7*32+31) /* AVX-512 variable length instructions */
+
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 8 */
+#define X86_FEATURE_PREFETCHWT1	(8*32+ 0) /* prefetchwt1 instruction */
+#define X86_FEATURE_AVX512VBMI	(8*32+ 1) /* AVX-512 byte manipulate instructions */
 
 #define cpufeat_word(idx)	((idx) / 32)
 #define cpufeat_bit(idx)	((idx) % 32)



[-- Attachment #2: x86-HVM-hide-XSAVE-dependents.patch --]
[-- Type: text/plain, Size: 5676 bytes --]

x86/HVM: hide features dependent on XSAVE when booted with "no-xsave"

... or when the guest has the XSAVE feature hidden by CPUID policy.
Not doing so is at best confusing to guests.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
One question is whether we shouldn't switch leaf 7 to white listing,
just like done for PV guests.

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -265,7 +265,8 @@ static void __cpuinit generic_identify(s
 	if ( c->cpuid_level >= 0x00000007 )
 		cpuid_count(0x00000007, 0, &tmp,
 			    &c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)],
-			    &tmp, &tmp);
+			    &c->x86_capability[cpufeat_word(X86_FEATURE_PREFETCHWT1)],
+			    &tmp);
 }
 
 /*
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4556,10 +4556,15 @@ void hvm_cpuid(unsigned int input, unsig
         if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
             __clear_bit(X86_FEATURE_APIC & 31, edx);
 
-        /* Fix up OSXSAVE. */
-        if ( cpu_has_xsave )
-            *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ?
-                     cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;
+        /* Fix up XSAVE, OSXSAVE, and AVX. */
+        if ( !cpu_has_xsave )
+            *ecx &= ~cpufeat_mask(X86_FEATURE_XSAVE);
+        if ( !(*ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ecx &= ~(cpufeat_mask(X86_FEATURE_OSXSAVE) |
+                      cpufeat_mask(X86_FEATURE_AVX) |
+                      cpufeat_mask(X86_FEATURE_F16C));
+        else if ( v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE )
+            *ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);
 
         /* Don't expose PCID to non-hap hvm. */
         if ( !hap_enabled(d) )
@@ -4585,12 +4590,36 @@ void hvm_cpuid(unsigned int input, unsig
         /* Don't expose INVPCID to non-hap hvm. */
         if ( (count == 0) && !hap_enabled(d) )
             *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
+
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ebx &= ~(cpufeat_mask(X86_FEATURE_AVX2) |
+                      cpufeat_mask(X86_FEATURE_MPX) |
+                      cpufeat_mask(X86_FEATURE_AVX512F));
+        if ( !(*ebx & cpufeat_mask(X86_FEATURE_AVX512F)) )
+        {
+            *ebx &= ~(cpufeat_mask(X86_FEATURE_AVX512DQ) |
+                      cpufeat_mask(X86_FEATURE_AVX512IFMA) |
+                      cpufeat_mask(X86_FEATURE_AVX512PF) |
+                      cpufeat_mask(X86_FEATURE_AVX512ER) |
+                      cpufeat_mask(X86_FEATURE_AVX512CD) |
+                      cpufeat_mask(X86_FEATURE_AVX512BW) |
+                      cpufeat_mask(X86_FEATURE_AVX512VL));
+            *ecx &= ~cpufeat_mask(X86_FEATURE_AVX512VBMI);
+        }
         break;
     case 0xb:
         /* Fix the x2APIC identifier. */
         *edx = v->vcpu_id * 2;
         break;
-    case 0xd:
+
+    case XSTATE_CPUID:
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+        {
+            *edx = *ecx = *ebx = *eax = 0;
+            break;
+        }
         /* EBX value of main leaf 0 depends on enabled xsave features */
         if ( count == 0 && v->arch.xcr0 ) 
         {
@@ -4637,6 +4666,11 @@ void hvm_cpuid(unsigned int input, unsig
         /* Hide data breakpoint extensions if the hardware has no support. */
         if ( !boot_cpu_has(X86_FEATURE_DBEXT) )
             *ecx &= ~cpufeat_mask(X86_FEATURE_DBEXT);
+        hvm_cpuid(1, NULL, NULL, &_ecx, NULL);
+        if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) )
+            *ecx &= ~(cpufeat_mask(X86_FEATURE_XOP) |
+                      cpufeat_mask(X86_FEATURE_LWP) |
+                      cpufeat_mask(X86_FEATURE_FMA4));
         break;
 
     case 0x80000008:
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -11,7 +11,7 @@
 
 #include <xen/const.h>
 
-#define NCAPINTS	8	/* N 32-bit words worth of info */
+#define NCAPINTS	9	/* N 32-bit words worth of info */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */
 #define X86_FEATURE_FPU		(0*32+ 0) /* Onboard FPU */
@@ -159,9 +159,21 @@
 #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
 #define X86_FEATURE_MPX		(7*32+14) /* Memory Protection Extensions */
 #define X86_FEATURE_CAT 	(7*32+15) /* Cache Allocation Technology */
+#define X86_FEATURE_AVX512F	(7*32+16) /* AVX-512 instructions */
+#define X86_FEATURE_AVX512DQ	(7*32+17) /* AVX-512 double/quad instructions */
 #define X86_FEATURE_RDSEED	(7*32+18) /* RDSEED instruction */
 #define X86_FEATURE_ADX		(7*32+19) /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
+#define X86_FEATURE_AVX512IFMA	(7*32+21) /* AVX-512 integer FMA instructions */
+#define X86_FEATURE_AVX512PF	(7*32+26) /* AVX-512 prefetch instructions */
+#define X86_FEATURE_AVX512ER	(7*32+27) /* AVX-512 exp/recip instructions */
+#define X86_FEATURE_AVX512CD	(7*32+28) /* AVX-512 conflict detect instructions */
+#define X86_FEATURE_AVX512BW	(7*32+30) /* AVX-512 byte/word instructions */
+#define X86_FEATURE_AVX512VL	(7*32+31) /* AVX-512 variable length instructions */
+
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 8 */
+#define X86_FEATURE_PREFETCHWT1	(8*32+ 0) /* prefetchwt1 instruction */
+#define X86_FEATURE_AVX512VBMI	(8*32+ 1) /* AVX-512 byte manipulate instructions */
 
 #define cpufeat_word(idx)	((idx) / 32)
 #define cpufeat_bit(idx)	((idx) % 32)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave"
  2015-11-27 11:07 [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave" Jan Beulich
@ 2015-11-27 16:00 ` Andrew Cooper
  2015-11-27 17:10   ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2015-11-27 16:00 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Keir Fraser

On 27/11/15 11:07, Jan Beulich wrote:
> ... or when the guest has the XSAVE feature hidden by CPUID policy.
> Not doing so is at best confusing to guests.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> One question is whether we shouldn't switch leaf 7 to white listing,
> just like done for PV guests.

The general comments from my PV side review apply here as well.

My levelling series has also moved all cpuid information to
whitelist-based.  It is the only safe way of doing things.

I chose to defer AVX512 work from my levelling series, because it is
rather more complicated than this, and there was insufficient details in
the Extention reference.  In particular, there is an suggestion that one
of the feature bits indicates support for non-EVEX encoded instructions
with k$N opmasks, which is not dependent on AVX512F.

It is also odd to see it added to HVM guests but not PV.  I would be
tempted to leave all the AVX512 bits until the groundwork from my series
is in.  It is destined for Skylake-EP which is still a year off.

~Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave"
  2015-11-27 16:00 ` Andrew Cooper
@ 2015-11-27 17:10   ` Jan Beulich
  2015-11-27 17:18     ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2015-11-27 17:10 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, Keir Fraser

>>> On 27.11.15 at 17:00, <andrew.cooper3@citrix.com> wrote:
> It is also odd to see it added to HVM guests but not PV.

But that's a result of PV using white listing on leaf 7, as opposed to
HVM's black listing.

But yes, if you CPUID series is about to arrive, these two patches
may not be worthwhile to commit. I've put them together because
of noticing the dependency issue, and knowing only that you've
been talking for a while about your plans, without it being clear
when they would land.

Jan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave"
  2015-11-27 17:10   ` Jan Beulich
@ 2015-11-27 17:18     ` Andrew Cooper
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2015-11-27 17:18 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Keir Fraser

On 27/11/15 17:10, Jan Beulich wrote:
>>>> On 27.11.15 at 17:00, <andrew.cooper3@citrix.com> wrote:
>> It is also odd to see it added to HVM guests but not PV.
> But that's a result of PV using white listing on leaf 7, as opposed to
> HVM's black listing.
>
> But yes, if you CPUID series is about to arrive, these two patches
> may not be worthwhile to commit. I've put them together because
> of noticing the dependency issue, and knowing only that you've
> been talking for a while about your plans, without it being clear
> when they would land.

I will post an RFC once I get some testing on my msr mask context
switching logic.

~Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-27 17:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-27 11:07 [PATCH] x86/HVM: hide features dependent on XSAVE when booted with "no-xsave" Jan Beulich
2015-11-27 16:00 ` Andrew Cooper
2015-11-27 17:10   ` Jan Beulich
2015-11-27 17:18     ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.