[for-4.18] x86/cpuid: Fix handling of XSAVE dynamic leaves

Commit Message

Andrew Cooper June 20, 2024, 11:14 a.m. UTC

[ This is a minimal backport of commit 71cacfb035f4 ("x86/cpuid: Fix handling
  of XSAVE dynamic leaves") to fix the bugs without depending on large rework of
  XSTATE handling in Xen 4.19 ]

First, if XSAVE is available in hardware but not visible to the guest, the
dynamic leaves shouldn't be filled in.

Second, the comment concerning XSS state is wrong.  VT-x doesn't manage
host/guest state automatically, but there is provision for "host only" bits to
be set, so the implications are still accurate.

In Xen 4.18, no XSS states are supported, so it's safe to keep deferring to
real hardware.

Signed-off-by: Andrew Cooper <[e-mail]>
(cherry picked from commit 71cacfb035f4a78ee10970dc38a3baa04d387451)
---
CC: Jan Beulich <[e-mail]>
---
 xen/arch/x86/cpuid.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)


base-commit: 01f7a3c792241d348a4e454a30afdf6c0d6cd71c

Comments

Jan Beulich June 20, 2024, 12:10 p.m. UTC | #1

On 20.06.2024 13:14, Andrew Cooper wrote:
> [ This is a minimal backport of commit 71cacfb035f4 ("x86/cpuid: Fix handling
>   of XSAVE dynamic leaves") to fix the bugs without depending on large rework of
>   XSTATE handling in Xen 4.19 ]
> 
> First, if XSAVE is available in hardware but not visible to the guest, the
> dynamic leaves shouldn't be filled in.
> 
> Second, the comment concerning XSS state is wrong.  VT-x doesn't manage
> host/guest state automatically, but there is provision for "host only" bits to
> be set, so the implications are still accurate.
> 
> In Xen 4.18, no XSS states are supported, so it's safe to keep deferring to
> real hardware.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

I'll try to not forget to include this in my next backporting batch. Thanks
for getting this sorted so quickly.

Jan

Andrew Cooper June 20, 2024, 12:28 p.m. UTC | #2

On 20/06/2024 1:10 pm, Jan Beulich wrote:
> On 20.06.2024 13:14, Andrew Cooper wrote:
>> [ This is a minimal backport of commit 71cacfb035f4 ("x86/cpuid: Fix handling
>>   of XSAVE dynamic leaves") to fix the bugs without depending on large rework of
>>   XSTATE handling in Xen 4.19 ]
>>
>> First, if XSAVE is available in hardware but not visible to the guest, the
>> dynamic leaves shouldn't be filled in.
>>
>> Second, the comment concerning XSS state is wrong.  VT-x doesn't manage
>> host/guest state automatically, but there is provision for "host only" bits to
>> be set, so the implications are still accurate.
>>
>> In Xen 4.18, no XSS states are supported, so it's safe to keep deferring to
>> real hardware.
>>
>> Signed-off-by: Andrew Cooper <[e-mail]>
> Reviewed-by: Jan Beulich <[e-mail]>
>
> I'll try to not forget to include this in my next backporting batch. Thanks
> for getting this sorted so quickly.

No problem.  If you're intending to commit this, could you fix:

  "depending on large" => "depending on the large"

in the first sentence please.  Only just spotted it.

~Andrew

Patch

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 455a09b2dd22..f6fd6cc6b32c 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -330,24 +330,20 @@  void guest_cpuid(const struct vcpu *v, uint32_t leaf,
     case XSTATE_CPUID:
         switch ( subleaf )
         {
-        case 1:
-            if ( p->xstate.xsavec || p->xstate.xsaves )
-            {
-                /*
-                 * TODO: Figure out what to do for XSS state.  VT-x manages
-                 * host vs guest MSR_XSS automatically, so as soon as we start
-                 * supporting any XSS states, the wrong XSS will be in
-                 * context.
-                 */
-                BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
-
-                /*
-                 * Read CPUID[0xD,0/1].EBX from hardware.  They vary with
-                 * enabled XSTATE, and appropraite XCR0|XSS are in context.
-                 */
+            /*
+             * Read CPUID[0xd,0/1].EBX from hardware.  They vary with enabled
+             * XSTATE, and the appropriate XCR0 is in context.
+             */
         case 0:
-                res->b = cpuid_count_ebx(leaf, subleaf);
-            }
+            if ( p->basic.xsave )
+                res->b = cpuid_count_ebx(0xd, 0);
+            break;
+
+        case 1:
+            /* This only works because Xen doesn't support XSS states yet. */
+            BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
+            if ( p->xstate.xsavec )
+                res->b = cpuid_count_ebx(0xd, 1);
             break;
         }
         break;