All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH 5/5] x86/cpuid: Fix handling of xsave dynamic leaves
Date: Mon, 3 May 2021 16:39:38 +0100	[thread overview]
Message-ID: <20210503153938.14109-6-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20210503153938.14109-1-andrew.cooper3@citrix.com>

If max leaf is greater than 0xd but xsave not available to the guest, then the
current XSAVE size should not be filled in.  This is a latent bug for now as
the guest max leaf is 0xd, but will become problematic in the future.

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.

Introduce {xstate,hw}_compressed_size() helpers to mirror the uncompressed
ones.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpuid.c         | 23 +++++++--------------
 xen/arch/x86/xstate.c        | 49 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/xstate.h |  1 +
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index c7f8388e5d..92745aa63f 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -1041,24 +1041,15 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
     case XSTATE_CPUID:
         switch ( subleaf )
         {
+        case 0:
+            if ( p->basic.xsave )
+                res->b = xstate_uncompressed_size(v->arch.xcr0);
+            break;
+
         case 1:
             if ( 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.
-                 */
-        case 0:
-                res->b = cpuid_count_ebx(leaf, subleaf);
-            }
+                res->b = xstate_compressed_size(v->arch.xcr0 |
+                                                v->arch.msrs->xss.raw);
             break;
         }
         break;
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index d4c01da574..03489f0cf4 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -602,6 +602,55 @@ unsigned int xstate_uncompressed_size(uint64_t xcr0)
     return size;
 }
 
+static unsigned int hw_compressed_size(uint64_t xstates)
+{
+    uint64_t curr_xcr0 = get_xcr0(), curr_xss = get_msr_xss();
+    unsigned int size;
+    bool ok;
+
+    ok = set_xcr0(xstates & ~XSTATE_XSAVES_ONLY);
+    ASSERT(ok);
+    set_msr_xss(xstates & XSTATE_XSAVES_ONLY);
+
+    size = cpuid_count_ebx(XSTATE_CPUID, 1);
+
+    ok = set_xcr0(curr_xcr0);
+    ASSERT(ok);
+    set_msr_xss(curr_xss);
+
+    return size;
+}
+
+unsigned int xstate_compressed_size(uint64_t xstates)
+{
+    unsigned int i, size = XSTATE_AREA_MIN_SIZE;
+
+    xstates &= ~XSTATE_FP_SSE;
+    for_each_set_bit ( i, &xstates, 63 )
+    {
+        if ( test_bit(i, &xstate_align) )
+            size = ROUNDUP(size, 64);
+
+        size += xstate_sizes[i];
+    }
+
+    /* In debug builds, cross-check our calculation with hardware. */
+    if ( IS_ENABLED(CONFIG_DEBUG) )
+    {
+        unsigned int hwsize;
+
+        xstates |= XSTATE_FP_SSE;
+        hwsize = hw_compressed_size(xstates);
+
+        if ( size != hwsize )
+            printk_once(XENLOG_ERR "%s(%#"PRIx64") size %#x != hwsize %#x\n",
+                        __func__, xstates, size, hwsize);
+        size = hwsize;
+    }
+
+    return size;
+}
+
 /* Collect the information of processor's extended state */
 void xstate_init(struct cpuinfo_x86 *c)
 {
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 02d6f171b8..ecf7bbc5cd 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -108,6 +108,7 @@ void xstate_free_save_area(struct vcpu *v);
 int xstate_alloc_save_area(struct vcpu *v);
 void xstate_init(struct cpuinfo_x86 *c);
 unsigned int xstate_uncompressed_size(uint64_t xcr0);
+unsigned int xstate_compressed_size(uint64_t states);
 
 static inline uint64_t xgetbv(unsigned int index)
 {
-- 
2.11.0



  parent reply	other threads:[~2021-05-03 15:40 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 15:39 [PATCH 0/5] x86/xstate: Fixes to size calculations Andrew Cooper
2021-05-03 15:39 ` [PATCH 1/5] x86/xstate: Elide redundant writes in set_xcr0() Andrew Cooper
2021-05-04 11:51   ` Jan Beulich
2021-05-03 15:39 ` [PATCH 2/5] x86/xstate: Rename _xstate_ctxt_size() to hw_uncompressed_size() Andrew Cooper
2021-05-04 11:53   ` Jan Beulich
2021-05-03 15:39 ` [PATCH 3/5] x86/xstate: Rework xstate_ctxt_size() as xstate_uncompressed_size() Andrew Cooper
2021-05-03 18:17   ` Andrew Cooper
2021-05-04 12:08     ` Jan Beulich
2021-05-04 12:15       ` Andrew Cooper
2021-05-04 12:20   ` Jan Beulich
2021-05-04 12:22     ` Andrew Cooper
2021-05-04 12:45       ` Jan Beulich
2021-05-03 15:39 ` [PATCH 4/5] x86/cpuid: Simplify recalculate_xstate() Andrew Cooper
2021-05-04 12:43   ` Jan Beulich
2021-05-04 13:58     ` Andrew Cooper
2021-05-05  8:19       ` Jan Beulich
2021-05-05 14:53         ` Andrew Cooper
2021-05-05 15:14           ` Jan Beulich
2021-05-03 15:39 ` Andrew Cooper [this message]
2021-05-04 12:56   ` [PATCH 5/5] x86/cpuid: Fix handling of xsave dynamic leaves Jan Beulich
2021-05-04 14:17     ` Andrew Cooper
2021-05-05  8:33       ` Jan Beulich
2021-05-05 16:59         ` Andrew Cooper
2021-05-06  6:17           ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210503153938.14109-6-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.