All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets
@ 2023-05-24 11:25 Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 01/10] x86/boot: Rework dom0 feature configuration Andrew Cooper
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

Also combined with "x86: Feature check cleanup" for simplicity.

See individual patches for v2 deltas.

Andrew Cooper (10):
  x86/boot: Rework dom0 feature configuration
  x86/boot: Adjust MSR_ARCH_CAPS handling for the Host policy
  x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS
  x86/cpu-policy: MSR_ARCH_CAPS feature names
  x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy
  x86/boot: Expose MSR_ARCH_CAPS data in guest max policies
  x86/cpufeature: Rework {boot_,}cpu_has()
  x86/vtx: Remove opencoded MSR_ARCH_CAPS check
  x86/tsx: Remove opencoded MSR_ARCH_CAPS check
  x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check

 tools/misc/xen-cpuid.c                      | 57 +++++++++-----
 tools/tests/cpu-policy/test-cpu-policy.c    |  5 --
 xen/arch/x86/cpu-policy.c                   | 83 ++++++++++-----------
 xen/arch/x86/cpu/common.c                   |  5 ++
 xen/arch/x86/hvm/vmx/vmx.c                  |  8 +-
 xen/arch/x86/include/asm/cpufeature.h       | 23 +++++-
 xen/arch/x86/include/asm/processor.h        |  2 +-
 xen/arch/x86/spec_ctrl.c                    | 56 +++++++-------
 xen/arch/x86/tsx.c                          | 13 ++--
 xen/include/public/arch-x86/cpufeatureset.h | 29 ++++++-
 xen/include/xen/lib/x86/cpu-policy.h        | 50 ++++++-------
 xen/lib/x86/cpuid.c                         | 11 ++-
 xen/tools/gen-cpuid.py                      |  3 +
 13 files changed, 208 insertions(+), 137 deletions(-)

-- 
2.30.2



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

* [PATCH v2 01/10] x86/boot: Rework dom0 feature configuration
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 02/10] x86/boot: Adjust MSR_ARCH_CAPS handling for the Host policy Andrew Cooper
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu

Right now, dom0's feature configuration is split between between the common
path and a dom0-specific one.  This mostly is by accident, and causes some
very subtle bugs.

First, start by clearly defining init_dom0_cpuid_policy() to be the domain
that Xen builds automatically.  The late hwdom case is still constructed in a
mostly normal way, with the control domain having full discretion over the CPU
policy.

Identifying this highlights a latent bug - the two halves of the MSR_ARCH_CAPS
bodge are asymmetric with respect to the hardware domain.  This means that
shim, or a control-only dom0 sees the MSR_ARCH_CAPS CPUID bit but none of the
MSR content.  This in turn declares the hardware to be retpoline-safe by
failing to advertise the {R,}RSBA bits appropriately.  Restrict this logic to
the hardware domain, although the special case will cease to exist shortly.

For the CPUID Faulting adjustment, the comment in ctxt_switch_levelling()
isn't actually relevant.  Provide a better explanation.

Move the recalculate_cpuid_policy() call outside of the dom0-cpuid= case.
This is no change for now, but will become necessary shortly.

Finally, place the second half of the MSR_ARCH_CAPS bodge after the
recalculate_cpuid_policy() call.  This is necessary to avoid transiently
breaking the hardware domain's view while the handling is cleaned up.  This
special case will cease to exist shortly.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpu-policy.c | 57 +++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index ef6a2d0d180a..5e7e19fbcda8 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -687,29 +687,6 @@ int init_domain_cpu_policy(struct domain *d)
     if ( !p )
         return -ENOMEM;
 
-    /* See comment in ctxt_switch_levelling() */
-    if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
-        p->platform_info.cpuid_faulting = false;
-
-    /*
-     * Expose the "hardware speculation behaviour" bits of ARCH_CAPS to dom0,
-     * so dom0 can turn off workarounds as appropriate.  Temporary, until the
-     * domain policy logic gains a better understanding of MSRs.
-     */
-    if ( is_hardware_domain(d) && cpu_has_arch_caps )
-    {
-        uint64_t val;
-
-        rdmsrl(MSR_ARCH_CAPABILITIES, val);
-
-        p->arch_caps.raw = val &
-            (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
-             ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | ARCH_CAPS_IF_PSCHANGE_MC_NO |
-             ARCH_CAPS_TAA_NO | ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO |
-             ARCH_CAPS_PSDP_NO | ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA |
-             ARCH_CAPS_BHI_NO | ARCH_CAPS_PBRSB_NO);
-    }
-
     d->arch.cpu_policy = p;
 
     recalculate_cpuid_policy(d);
@@ -845,11 +822,15 @@ void recalculate_cpuid_policy(struct domain *d)
         p->extd.raw[0x19] = EMPTY_LEAF;
 }
 
+/*
+ * Adjust the CPU policy for dom0.  Really, this is "the domain Xen builds
+ * automatically on boot", and might not have the domid 0 (e.g. pvshim).
+ */
 void __init init_dom0_cpuid_policy(struct domain *d)
 {
     struct cpu_policy *p = d->arch.cpuid;
 
-    /* dom0 can't migrate.  Give it ITSC if available. */
+    /* Dom0 doesn't migrate relative to Xen.  Give it ITSC if available. */
     if ( cpu_has_itsc )
         p->extd.itsc = true;
 
@@ -858,7 +839,7 @@ void __init init_dom0_cpuid_policy(struct domain *d)
      * so dom0 can turn off workarounds as appropriate.  Temporary, until the
      * domain policy logic gains a better understanding of MSRs.
      */
-    if ( cpu_has_arch_caps )
+    if ( is_hardware_domain(d) && cpu_has_arch_caps )
         p->feat.arch_caps = true;
 
     /* Apply dom0-cpuid= command line settings, if provided. */
@@ -876,8 +857,32 @@ void __init init_dom0_cpuid_policy(struct domain *d)
         }
 
         x86_cpu_featureset_to_policy(fs, p);
+    }
+
+    /*
+     * PV Control domains used to require unfiltered CPUID.  This was fixed in
+     * Xen 4.13, but there is an cmdline knob to restore the prior behaviour.
+     *
+     * If the domain is getting unfiltered CPUID, don't let the guest kernel
+     * play with CPUID faulting either, as Xen's CPUID path won't cope.
+     */
+    if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
+        p->platform_info.cpuid_faulting = false;
 
-        recalculate_cpuid_policy(d);
+    recalculate_cpuid_policy(d);
+
+    if ( is_hardware_domain(d) && cpu_has_arch_caps )
+    {
+        uint64_t val;
+
+        rdmsrl(MSR_ARCH_CAPABILITIES, val);
+
+        p->arch_caps.raw = val &
+            (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
+             ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | ARCH_CAPS_IF_PSCHANGE_MC_NO |
+             ARCH_CAPS_TAA_NO | ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO |
+             ARCH_CAPS_PSDP_NO | ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA |
+             ARCH_CAPS_BHI_NO | ARCH_CAPS_PBRSB_NO);
     }
 }
 
-- 
2.30.2



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

* [PATCH v2 02/10] x86/boot: Adjust MSR_ARCH_CAPS handling for the Host policy
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 01/10] x86/boot: Rework dom0 feature configuration Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS Andrew Cooper
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu

We are about to move MSR_ARCH_CAPS into featureset, but the order of
operations (copy raw policy, then copy x86_capabilitiles[] in) will end up
clobbering the ARCH_CAPS value.

Some toolstacks use this information to handle TSX compatibility across the
CPUs and microcode versions where support was removed.

To avoid this transient breakage, read from raw_cpu_policy rather than
modifying it in place.  This logic will be removed entirely in due course.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpu-policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 5e7e19fbcda8..49f5465ec445 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -411,7 +411,7 @@ static void __init calculate_host_policy(void)
     p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting;
 
     /* Temporary, until we have known_features[] for feature bits in MSRs. */
-    p->arch_caps.raw &=
+    p->arch_caps.raw = raw_cpu_policy.arch_caps.raw &
         (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
          ARCH_CAPS_SKIP_L1DFL | ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO |
          ARCH_CAPS_IF_PSCHANGE_MC_NO | ARCH_CAPS_TSX_CTRL | ARCH_CAPS_TAA_NO |
-- 
2.30.2



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

* [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 01/10] x86/boot: Rework dom0 feature configuration Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 02/10] x86/boot: Adjust MSR_ARCH_CAPS handling for the Host policy Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 14:53   ` Jan Beulich
  2023-05-24 11:25 ` [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names Andrew Cooper
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

Bits through 24 are already defined, meaning that we're not far off needing
the second word.  Put both in right away.

As both halves are present now, the arch_caps field is full width.  Adjust the
unit test, which notices.

The bool bitfield names in the arch_caps union are unused, and somewhat out of
date.  They'll shortly be automatically generated.

Add CPUID and MSR prefixes to the ./xen-cpuid verbose output, now that there
are a mix of the two.

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>

v2:
 * Adjust the unit test.
 * Use an m prefix on the short name.
 * Add CPUID/MSR to the verbose name.
---
 tools/misc/xen-cpuid.c                      | 44 +++++++++++-------
 tools/tests/cpu-policy/test-cpu-policy.c    |  5 ---
 xen/include/public/arch-x86/cpufeatureset.h |  4 ++
 xen/include/xen/lib/x86/cpu-policy.h        | 50 ++++++++++-----------
 xen/lib/x86/cpuid.c                         |  4 ++
 5 files changed, 59 insertions(+), 48 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 8ec143ebc854..15ad2d33e2a1 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -226,31 +226,41 @@ static const char *const str_7d2[32] =
     [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
 };
 
+static const char *const str_10Al[32] =
+{
+};
+
+static const char *const str_10Ah[32] =
+{
+};
+
 static const struct {
     const char *name;
     const char *abbr;
     const char *const *strs;
 } decodes[] =
 {
-    { "0x00000001.edx",   "1d",  str_1d },
-    { "0x00000001.ecx",   "1c",  str_1c },
-    { "0x80000001.edx",   "e1d", str_e1d },
-    { "0x80000001.ecx",   "e1c", str_e1c },
-    { "0x0000000d:1.eax", "Da1", str_Da1 },
-    { "0x00000007:0.ebx", "7b0", str_7b0 },
-    { "0x00000007:0.ecx", "7c0", str_7c0 },
-    { "0x80000007.edx",   "e7d", str_e7d },
-    { "0x80000008.ebx",   "e8b", str_e8b },
-    { "0x00000007:0.edx", "7d0", str_7d0 },
-    { "0x00000007:1.eax", "7a1", str_7a1 },
-    { "0x80000021.eax",  "e21a", str_e21a },
-    { "0x00000007:1.ebx", "7b1", str_7b1 },
-    { "0x00000007:2.edx", "7d2", str_7d2 },
-    { "0x00000007:1.ecx", "7c1", str_7c1 },
-    { "0x00000007:1.edx", "7d1", str_7d1 },
+    { "CPUID 0x00000001.edx",        "1d", str_1d },
+    { "CPUID 0x00000001.ecx",        "1c", str_1c },
+    { "CPUID 0x80000001.edx",       "e1d", str_e1d },
+    { "CPUID 0x80000001.ecx",       "e1c", str_e1c },
+    { "CPUID 0x0000000d:1.eax",     "Da1", str_Da1 },
+    { "CPUID 0x00000007:0.ebx",     "7b0", str_7b0 },
+    { "CPUID 0x00000007:0.ecx",     "7c0", str_7c0 },
+    { "CPUID 0x80000007.edx",       "e7d", str_e7d },
+    { "CPUID 0x80000008.ebx",       "e8b", str_e8b },
+    { "CPUID 0x00000007:0.edx",     "7d0", str_7d0 },
+    { "CPUID 0x00000007:1.eax",     "7a1", str_7a1 },
+    { "CPUID 0x80000021.eax",      "e21a", str_e21a },
+    { "CPUID 0x00000007:1.ebx",     "7b1", str_7b1 },
+    { "CPUID 0x00000007:2.edx",     "7d2", str_7d2 },
+    { "CPUID 0x00000007:1.ecx",     "7c1", str_7c1 },
+    { "CPUID 0x00000007:1.edx",     "7d1", str_7d1 },
+    { "MSR   0x0000010a.lo",      "m10Al", str_10Al },
+    { "MSR   0x0000010a.hi",      "m10Ah", str_10Ah },
 };
 
-#define COL_ALIGN "18"
+#define COL_ALIGN "24"
 
 static const char *const fs_names[] = {
     [XEN_SYSCTL_cpu_featureset_raw]     = "Raw",
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index f1d968adfc39..301df2c00285 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -391,11 +391,6 @@ static void test_msr_deserialise_failure(void)
             .msr = { .idx = 0xce, .val = ~0ull },
             .rc = -EOVERFLOW,
         },
-        {
-            .name = "truncated val",
-            .msr = { .idx = 0x10a, .val = ~0ull },
-            .rc = -EOVERFLOW,
-        },
     };
 
     printf("Testing MSR deserialise failure:\n");
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 8de73aebc3e0..032cec3ccba2 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -307,6 +307,10 @@ XEN_CPUFEATURE(AVX_VNNI_INT8,      15*32+ 4) /*A  AVX-VNNI-INT8 Instructions */
 XEN_CPUFEATURE(AVX_NE_CONVERT,     15*32+ 5) /*A  AVX-NE-CONVERT Instructions */
 XEN_CPUFEATURE(CET_SSS,            15*32+18) /*   CET Supervisor Shadow Stacks safe to use */
 
+/* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.eax, word 16 */
+
+/* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
+
 #endif /* XEN_CPUFEATURE */
 
 /* Clean up from a default include.  Close the enum (for C). */
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index bfa425060464..6d5e9edd269b 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -4,22 +4,24 @@
 
 #include <xen/lib/x86/cpuid-autogen.h>
 
-#define FEATURESET_1d     0 /* 0x00000001.edx      */
-#define FEATURESET_1c     1 /* 0x00000001.ecx      */
-#define FEATURESET_e1d    2 /* 0x80000001.edx      */
-#define FEATURESET_e1c    3 /* 0x80000001.ecx      */
-#define FEATURESET_Da1    4 /* 0x0000000d:1.eax    */
-#define FEATURESET_7b0    5 /* 0x00000007:0.ebx    */
-#define FEATURESET_7c0    6 /* 0x00000007:0.ecx    */
-#define FEATURESET_e7d    7 /* 0x80000007.edx      */
-#define FEATURESET_e8b    8 /* 0x80000008.ebx      */
-#define FEATURESET_7d0    9 /* 0x00000007:0.edx    */
-#define FEATURESET_7a1   10 /* 0x00000007:1.eax    */
-#define FEATURESET_e21a  11 /* 0x80000021.eax      */
-#define FEATURESET_7b1   12 /* 0x00000007:1.ebx    */
-#define FEATURESET_7d2   13 /* 0x00000007:2.edx    */
-#define FEATURESET_7c1   14 /* 0x00000007:1.ecx    */
-#define FEATURESET_7d1   15 /* 0x00000007:1.edx    */
+#define FEATURESET_1d         0 /* 0x00000001.edx      */
+#define FEATURESET_1c         1 /* 0x00000001.ecx      */
+#define FEATURESET_e1d        2 /* 0x80000001.edx      */
+#define FEATURESET_e1c        3 /* 0x80000001.ecx      */
+#define FEATURESET_Da1        4 /* 0x0000000d:1.eax    */
+#define FEATURESET_7b0        5 /* 0x00000007:0.ebx    */
+#define FEATURESET_7c0        6 /* 0x00000007:0.ecx    */
+#define FEATURESET_e7d        7 /* 0x80000007.edx      */
+#define FEATURESET_e8b        8 /* 0x80000008.ebx      */
+#define FEATURESET_7d0        9 /* 0x00000007:0.edx    */
+#define FEATURESET_7a1       10 /* 0x00000007:1.eax    */
+#define FEATURESET_e21a      11 /* 0x80000021.eax      */
+#define FEATURESET_7b1       12 /* 0x00000007:1.ebx    */
+#define FEATURESET_7d2       13 /* 0x00000007:2.edx    */
+#define FEATURESET_7c1       14 /* 0x00000007:1.ecx    */
+#define FEATURESET_7d1       15 /* 0x00000007:1.edx    */
+#define FEATURESET_m10Al     16 /* 0x0000010a.eax      */
+#define FEATURESET_m10Ah     17 /* 0x0000010a.edx      */
 
 struct cpuid_leaf
 {
@@ -350,17 +352,13 @@ struct cpu_policy
      * fixed in hardware.
      */
     union {
-        uint32_t raw;
+        uint64_t raw;
+        struct {
+            uint32_t lo, hi;
+        };
         struct {
-            bool rdcl_no:1;
-            bool ibrs_all:1;
-            bool rsba:1;
-            bool skip_l1dfl:1;
-            bool ssb_no:1;
-            bool mds_no:1;
-            bool if_pschange_mc_no:1;
-            bool tsx_ctrl:1;
-            bool taa_no:1;
+            DECL_BITFIELD(m10Al);
+            DECL_BITFIELD(m10Ah);
         };
     } arch_caps;
 
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 68aafb404927..e795ce375032 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -79,6 +79,8 @@ void x86_cpu_policy_to_featureset(
     fs[FEATURESET_7d2]       = p->feat._7d2;
     fs[FEATURESET_7c1]       = p->feat._7c1;
     fs[FEATURESET_7d1]       = p->feat._7d1;
+    fs[FEATURESET_m10Al]     = p->arch_caps.lo;
+    fs[FEATURESET_m10Ah]     = p->arch_caps.hi;
 }
 
 void x86_cpu_featureset_to_policy(
@@ -100,6 +102,8 @@ void x86_cpu_featureset_to_policy(
     p->feat._7d2             = fs[FEATURESET_7d2];
     p->feat._7c1             = fs[FEATURESET_7c1];
     p->feat._7d1             = fs[FEATURESET_7d1];
+    p->arch_caps.lo          = fs[FEATURESET_m10Al];
+    p->arch_caps.hi          = fs[FEATURESET_m10Ah];
 }
 
 void x86_cpu_policy_recalc_synth(struct cpu_policy *p)
-- 
2.30.2



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

* [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (2 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 14:56   ` Jan Beulich
  2023-05-24 11:25 ` [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy Andrew Cooper
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

Seed the default visibility from the dom0 special case, which for the most
part just exposes the *_NO bits.  EIBRS is the one non-*_NO bit, which is
"just" a status bit to the guest indicating a change in implemention of IBRS
which is already fully supported.

Insert a block dependency from the ARCH_CAPS CPUID bit to the entire content
of the MSR.  This is because MSRs have no structure information similar to
CPUID, and used by x86_cpu_policy_clear_out_of_range_leaves(), in order to
bulk-clear inaccessable words.

The overall CPUID bit is still max-only, so all of MSR_ARCH_CAPS is hidden in
the default policies.

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>

There is no libxl logic because libxl still uses the older xend format which
is specific to CPUID data.  That is going to need untangling at some other
point.

v2:
 * Don't expose SKIP_L1DFL to guests (it's only applicable for nested virt)
 * Fix SBDR_SSDP_NO and FBSDP_NO names.
 * Extend the commit message.
---
 tools/misc/xen-cpuid.c                      | 13 ++++++++++++
 xen/include/public/arch-x86/cpufeatureset.h | 23 +++++++++++++++++++++
 xen/tools/gen-cpuid.py                      |  3 +++
 3 files changed, 39 insertions(+)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 15ad2d33e2a1..8925a583edd5 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -228,6 +228,19 @@ static const char *const str_7d2[32] =
 
 static const char *const str_10Al[32] =
 {
+    [ 0] = "rdcl-no",             [ 1] = "eibrs",
+    [ 2] = "rsba",                [ 3] = "skip-l1dfl",
+    [ 4] = "intel-ssb-no",        [ 5] = "mds-no",
+    [ 6] = "if-pschange-mc-no",   [ 7] = "tsx-ctrl",
+    [ 8] = "taa-no",              [ 9] = "mcu-ctrl",
+    [10] = "misc-pkg-ctrl",       [11] = "energy-ctrl",
+    [12] = "doitm",               [13] = "sbdr-ssdp-no",
+    [14] = "fbsdp-no",            [15] = "psdp-no",
+    /* 16 */                      [17] = "fb-clear",
+    [18] = "fb-clear-ctrl",       [19] = "rrsba",
+    [20] = "bhi-no",              [21] = "xapic-status",
+    /* 22 */                      [23] = "ovrclk-status",
+    [24] = "pbrsb-no",
 };
 
 static const char *const str_10Ah[32] =
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 032cec3ccba2..033b1a72feea 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -308,6 +308,29 @@ XEN_CPUFEATURE(AVX_NE_CONVERT,     15*32+ 5) /*A  AVX-NE-CONVERT Instructions */
 XEN_CPUFEATURE(CET_SSS,            15*32+18) /*   CET Supervisor Shadow Stacks safe to use */
 
 /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.eax, word 16 */
+XEN_CPUFEATURE(RDCL_NO,            16*32+ 0) /*A  No Rogue Data Cache Load (Meltdown) */
+XEN_CPUFEATURE(EIBRS,              16*32+ 1) /*A  Enhanced IBRS */
+XEN_CPUFEATURE(RSBA,               16*32+ 2) /*!A RSB Alternative (Retpoline not safe) */
+XEN_CPUFEATURE(SKIP_L1DFL,         16*32+ 3) /*   Don't need to flush L1D on VMEntry */
+XEN_CPUFEATURE(INTEL_SSB_NO,       16*32+ 4) /*A  No Speculative Store Bypass */
+XEN_CPUFEATURE(MDS_NO,             16*32+ 5) /*A  No Microarchitectural Data Sampling */
+XEN_CPUFEATURE(IF_PSCHANGE_MC_NO,  16*32+ 6) /*A  No Instruction fetch #MC */
+XEN_CPUFEATURE(TSX_CTRL,           16*32+ 7) /*   MSR_TSX_CTRL */
+XEN_CPUFEATURE(TAA_NO,             16*32+ 8) /*A  No TSX Async Abort */
+XEN_CPUFEATURE(MCU_CTRL,           16*32+ 9) /*   MSR_MCU_CTRL */
+XEN_CPUFEATURE(MISC_PKG_CTRL,      16*32+10) /*   MSR_MISC_PKG_CTRL */
+XEN_CPUFEATURE(ENERGY_FILTERING,   16*32+11) /*   MSR_MISC_PKG_CTRL.ENERGY_FILTERING */
+XEN_CPUFEATURE(DOITM,              16*32+12) /*   Data Operand Invariant Timing Mode */
+XEN_CPUFEATURE(SBDR_SSDP_NO,       16*32+13) /*A  No Shared Buffer Data Read or Sideband Stale Data Propagation */
+XEN_CPUFEATURE(FBSDP_NO,           16*32+14) /*A  No Fill Buffer Stale Data Propagation */
+XEN_CPUFEATURE(PSDP_NO,            16*32+15) /*A  No Primary Stale Data Propagation */
+XEN_CPUFEATURE(FB_CLEAR,           16*32+17) /*A  Fill Buffers cleared by VERW */
+XEN_CPUFEATURE(FB_CLEAR_CTRL,      16*32+18) /*   MSR_OPT_CPU_CTRL.FB_CLEAR_DIS */
+XEN_CPUFEATURE(RRSBA,              16*32+19) /*!A Restricted RSB Alternative */
+XEN_CPUFEATURE(BHI_NO,             16*32+20) /*A  No Branch History Injection  */
+XEN_CPUFEATURE(XAPIC_STATUS,       16*32+21) /*   MSR_XAPIC_DISABLE_STATUS */
+XEN_CPUFEATURE(OVRCLK_STATUS,      16*32+23) /*   MSR_OVERCLOCKING_STATUS */
+XEN_CPUFEATURE(PBRSB_NO,           16*32+24) /*A  No Post-Barrier RSB predictions */
 
 /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
 
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 86d00bb3c273..f28ff708a2fc 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -325,6 +325,9 @@ def crunch_numbers(state):
 
         # In principle the TSXLDTRK insns could also be considered independent.
         RTM: [TSXLDTRK],
+
+        # The ARCH_CAPS CPUID bit enumerates the availability of the whole register.
+        ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)),
     }
 
     deep_features = tuple(sorted(deps.keys()))
-- 
2.30.2



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

* [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (3 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 14:03   ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies Andrew Cooper
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu

Extend x86_cpu_policy_fill_native() with a read of ARCH_CAPS based on the
CPUID information just read, removing the specially handling in
calculate_raw_cpu_policy().

Right now, the only use of x86_cpu_policy_fill_native() outside of Xen is the
unit tests.  Getting MSR data in this context is left to whomever first
encounters a genuine need to have it.

Extend generic_identify() to read ARCH_CAPS into x86_capability[], which is
fed into the Host Policy.  This in turn means there's no need to special case
arch_caps in calculate_host_policy().

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Extend commit message to discuss the absence of MSRs outside of __XEN__
---
 xen/arch/x86/cpu-policy.c | 12 ------------
 xen/arch/x86/cpu/common.c |  5 +++++
 xen/lib/x86/cpuid.c       |  7 ++++++-
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 49f5465ec445..dfd9abd8564c 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -354,9 +354,6 @@ void calculate_raw_cpu_policy(void)
 
     /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     /* Was already added by probe_cpuid_faulting() */
-
-    if ( cpu_has_arch_caps )
-        rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw);
 }
 
 static void __init calculate_host_policy(void)
@@ -409,15 +406,6 @@ static void __init calculate_host_policy(void)
     /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     /* probe_cpuid_faulting() sanity checks presence of MISC_FEATURES_ENABLES */
     p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting;
-
-    /* Temporary, until we have known_features[] for feature bits in MSRs. */
-    p->arch_caps.raw = raw_cpu_policy.arch_caps.raw &
-        (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
-         ARCH_CAPS_SKIP_L1DFL | ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO |
-         ARCH_CAPS_IF_PSCHANGE_MC_NO | ARCH_CAPS_TSX_CTRL | ARCH_CAPS_TAA_NO |
-         ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO | ARCH_CAPS_PSDP_NO |
-         ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA | ARCH_CAPS_BHI_NO |
-         ARCH_CAPS_PBRSB_NO);
 }
 
 static void __init guest_common_default_feature_adjustments(uint32_t *fs)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 9bbb385db42d..f1084bb1ed36 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -477,6 +477,11 @@ static void generic_identify(struct cpuinfo_x86 *c)
 		cpuid_count(0xd, 1,
 			    &c->x86_capability[FEATURESET_Da1],
 			    &tmp, &tmp, &tmp);
+
+	if (test_bit(X86_FEATURE_ARCH_CAPS, c->x86_capability))
+		rdmsr(MSR_ARCH_CAPABILITIES,
+		      c->x86_capability[FEATURESET_10Al],
+		      c->x86_capability[FEATURESET_10Ah]);
 }
 
 /*
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index e795ce375032..07e550191448 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -226,7 +226,12 @@ void x86_cpu_policy_fill_native(struct cpu_policy *p)
     p->hv_limit = 0;
     p->hv2_limit = 0;
 
-    /* TODO MSRs */
+#ifdef __XEN__
+    /* TODO MSR_PLATFORM_INFO */
+
+    if ( p->feat.arch_caps )
+        rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw);
+#endif
 
     x86_cpu_policy_recalc_synth(p);
 }
-- 
2.30.2



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

* [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (4 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 15:02   ` Jan Beulich
  2023-05-24 11:25 ` [PATCH v2 07/10] x86/cpufeature: Rework {boot_,}cpu_has() Andrew Cooper
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

We already have common and default feature adjustment helpers.  Introduce one
for max featuresets too.

Offer MSR_ARCH_CAPS unconditionally in the max policy, and stop clobbering the
data inherited from the Host policy.  This will be necessary to level a VM
safely for migration.  Annotate the ARCH_CAPS CPUID bit as special.  Note:
ARCH_CAPS is still max-only for now, so will not be inhereted by the default
policies.

With this done, the special case for dom0 can be shrunk to just resampling the
Host policy (as ARCH_CAPS isn't visible by default yet).

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>

v2:
 * Annotate ARCH_CAPS as special.
---
 xen/arch/x86/cpu-policy.c                   | 42 ++++++++++++---------
 xen/include/public/arch-x86/cpufeatureset.h |  2 +-
 2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index dfd9abd8564c..74266d30b551 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -408,6 +408,25 @@ static void __init calculate_host_policy(void)
     p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting;
 }
 
+static void __init guest_common_max_feature_adjustments(uint32_t *fs)
+{
+    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+    {
+        /*
+         * MSR_ARCH_CAPS is just feature data, and we can offer it to guests
+         * unconditionally, although limit it to Intel systems as it is highly
+         * uarch-specific.
+         *
+         * In particular, the RSBA and RRSBA bits mean "you might migrate to a
+         * system where RSB underflow uses alternative predictors (a.k.a
+         * Retpoline not safe)", so these need to be visible to a guest in all
+         * cases, even when it's only some other server in the pool which
+         * suffers the identified behaviour.
+         */
+        __set_bit(X86_FEATURE_ARCH_CAPS, fs);
+    }
+}
+
 static void __init guest_common_default_feature_adjustments(uint32_t *fs)
 {
     /*
@@ -483,6 +502,7 @@ static void __init calculate_pv_max_policy(void)
         __clear_bit(X86_FEATURE_IBRS, fs);
     }
 
+    guest_common_max_feature_adjustments(fs);
     guest_common_feature_adjustments(fs);
 
     sanitise_featureset(fs);
@@ -490,8 +510,6 @@ static void __init calculate_pv_max_policy(void)
     recalculate_xstate(p);
 
     p->extd.raw[0xa] = EMPTY_LEAF; /* No SVM for PV guests. */
-
-    p->arch_caps.raw = 0; /* Not supported yet. */
 }
 
 static void __init calculate_pv_def_policy(void)
@@ -598,6 +616,7 @@ static void __init calculate_hvm_max_policy(void)
     if ( !cpu_has_vmx )
         __clear_bit(X86_FEATURE_PKS, fs);
 
+    guest_common_max_feature_adjustments(fs);
     guest_common_feature_adjustments(fs);
 
     sanitise_featureset(fs);
@@ -606,8 +625,6 @@ static void __init calculate_hvm_max_policy(void)
 
     /* It's always possible to emulate CPUID faulting for HVM guests */
     p->platform_info.cpuid_faulting = true;
-
-    p->arch_caps.raw = 0; /* Not supported yet. */
 }
 
 static void __init calculate_hvm_def_policy(void)
@@ -828,7 +845,10 @@ void __init init_dom0_cpuid_policy(struct domain *d)
      * domain policy logic gains a better understanding of MSRs.
      */
     if ( is_hardware_domain(d) && cpu_has_arch_caps )
+    {
         p->feat.arch_caps = true;
+        p->arch_caps.raw = host_cpu_policy.arch_caps.raw;
+    }
 
     /* Apply dom0-cpuid= command line settings, if provided. */
     if ( dom0_cpuid_cmdline )
@@ -858,20 +878,6 @@ void __init init_dom0_cpuid_policy(struct domain *d)
         p->platform_info.cpuid_faulting = false;
 
     recalculate_cpuid_policy(d);
-
-    if ( is_hardware_domain(d) && cpu_has_arch_caps )
-    {
-        uint64_t val;
-
-        rdmsrl(MSR_ARCH_CAPABILITIES, val);
-
-        p->arch_caps.raw = val &
-            (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
-             ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | ARCH_CAPS_IF_PSCHANGE_MC_NO |
-             ARCH_CAPS_TAA_NO | ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO |
-             ARCH_CAPS_PSDP_NO | ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA |
-             ARCH_CAPS_BHI_NO | ARCH_CAPS_PBRSB_NO);
-    }
 }
 
 static void __init __maybe_unused build_assertions(void)
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 033b1a72feea..777041425e0a 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -271,7 +271,7 @@ XEN_CPUFEATURE(AVX512_FP16,   9*32+23) /*   AVX512 FP16 instructions */
 XEN_CPUFEATURE(IBRSB,         9*32+26) /*A  IBRS and IBPB support (used by Intel) */
 XEN_CPUFEATURE(STIBP,         9*32+27) /*A  STIBP */
 XEN_CPUFEATURE(L1D_FLUSH,     9*32+28) /*S  MSR_FLUSH_CMD and L1D flush. */
-XEN_CPUFEATURE(ARCH_CAPS,     9*32+29) /*a  IA32_ARCH_CAPABILITIES MSR */
+XEN_CPUFEATURE(ARCH_CAPS,     9*32+29) /*!a IA32_ARCH_CAPABILITIES MSR */
 XEN_CPUFEATURE(CORE_CAPS,     9*32+30) /*   IA32_CORE_CAPABILITIES MSR */
 XEN_CPUFEATURE(SSBD,          9*32+31) /*A  MSR_SPEC_CTRL.SSBD available */
 
-- 
2.30.2



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

* [PATCH v2 07/10] x86/cpufeature: Rework {boot_,}cpu_has()
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (5 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 08/10] x86/vtx: Remove opencoded MSR_ARCH_CAPS check Andrew Cooper
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu

One area where Xen deviates from Linux is that test_bit() forces a volatile
read.  This leads to poor code generation, because the optimiser cannot merge
bit operations on the same word.

Drop the use of test_bit(), and write the expressions in regular C.  This
removes the include of bitops.h (which is a frequent source of header
tangles), and it offers the optimiser far more flexibility.

Bloat-o-meter reports a net change of:

  add/remove: 0/0 grow/shrink: 21/87 up/down: 641/-2751 (-2110)

with half of that in x86_emulate() alone.  vmx_ctxt_switch_to() seems to be
the fastpath with the greatest delta at -24, where the optimiser has
successfully removed the branch hidden in cpu_has_msr_tsc_aux.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Drop stdbool.  It is already covered by other includes.
---
 xen/arch/x86/include/asm/cpufeature.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 4140ec0938b2..d0ead8e7a51e 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -17,7 +17,6 @@
 #define X86_FEATURE_ALWAYS      X86_FEATURE_LM
 
 #ifndef __ASSEMBLY__
-#include <xen/bitops.h>
 
 struct cpuinfo_x86 {
     unsigned char x86;                 /* CPU family */
@@ -43,8 +42,15 @@ struct cpuinfo_x86 {
 
 extern struct cpuinfo_x86 boot_cpu_data;
 
-#define cpu_has(c, bit)		test_bit(bit, (c)->x86_capability)
-#define boot_cpu_has(bit)	test_bit(bit, boot_cpu_data.x86_capability)
+static inline bool cpu_has(const struct cpuinfo_x86 *info, unsigned int feat)
+{
+    return info->x86_capability[cpufeat_word(feat)] & cpufeat_mask(feat);
+}
+
+static inline bool boot_cpu_has(unsigned int feat)
+{
+    return cpu_has(&boot_cpu_data, feat);
+}
 
 #define CPUID_PM_LEAF                    6
 #define CPUID6_ECX_APERFMPERF_CAPABILITY 0x1
-- 
2.30.2



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

* [PATCH v2 08/10] x86/vtx: Remove opencoded MSR_ARCH_CAPS check
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (6 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 07/10] x86/cpufeature: Rework {boot_,}cpu_has() Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 09/10] x86/tsx: " Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 10/10] x86/spec-ctrl: " Andrew Cooper
  9 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu, Jun Nakajima, Kevin Tian

MSR_ARCH_CAPS data is now included in featureset information.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/vmx/vmx.c            | 8 ++------
 xen/arch/x86/include/asm/cpufeature.h | 3 +++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 096c69251d58..9dc16d0cc6b9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2849,8 +2849,6 @@ static void __init ler_to_fixup_check(void);
  */
 static bool __init has_if_pschange_mc(void)
 {
-    uint64_t caps = 0;
-
     /*
      * If we are virtualised, there is nothing we can do.  Our EPT tables are
      * shadowed by our hypervisor, and not walked by hardware.
@@ -2858,10 +2856,8 @@ static bool __init has_if_pschange_mc(void)
     if ( cpu_has_hypervisor )
         return false;
 
-    if ( cpu_has_arch_caps )
-        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
-
-    if ( caps & ARCH_CAPS_IF_PSCHANGE_MC_NO )
+    /* Hardware reports itself as fixed. */
+    if ( cpu_has_if_pschange_mc_no )
         return false;
 
     /*
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index d0ead8e7a51e..e3154ec5800d 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -182,6 +182,9 @@ static inline bool boot_cpu_has(unsigned int feat)
 #define cpu_has_avx_vnni_int8   boot_cpu_has(X86_FEATURE_AVX_VNNI_INT8)
 #define cpu_has_avx_ne_convert  boot_cpu_has(X86_FEATURE_AVX_NE_CONVERT)
 
+/* MSR_ARCH_CAPS */
+#define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
+
 /* Synthesized. */
 #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
 #define cpu_has_cpuid_faulting  boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
-- 
2.30.2



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

* [PATCH v2 09/10] x86/tsx: Remove opencoded MSR_ARCH_CAPS check
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (7 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 08/10] x86/vtx: Remove opencoded MSR_ARCH_CAPS check Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 11:25 ` [PATCH v2 10/10] x86/spec-ctrl: " Andrew Cooper
  9 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Jan Beulich, Jan Beulich, Roger Pau Monné,
	Wei Liu

The current cpu_has_tsx_ctrl tristate is serving double pupose; to signal the
first pass through tsx_init(), and the availability of MSR_TSX_CTRL.

Drop the variable, replacing it with a once boolean, and altering
cpu_has_tsx_ctrl to come out of the feature information.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/include/asm/cpufeature.h |  1 +
 xen/arch/x86/include/asm/processor.h  |  2 +-
 xen/arch/x86/tsx.c                    | 13 ++++++++-----
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index e3154ec5800d..9047ea43f503 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -184,6 +184,7 @@ static inline bool boot_cpu_has(unsigned int feat)
 
 /* MSR_ARCH_CAPS */
 #define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
+#define cpu_has_tsx_ctrl        boot_cpu_has(X86_FEATURE_TSX_CTRL)
 
 /* Synthesized. */
 #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index 0eaa2c3094d0..f983ff501d95 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -535,7 +535,7 @@ static inline uint8_t get_cpu_family(uint32_t raw, uint8_t *model,
     return fam;
 }
 
-extern int8_t opt_tsx, cpu_has_tsx_ctrl;
+extern int8_t opt_tsx;
 extern bool rtm_disabled;
 void tsx_init(void);
 
diff --git a/xen/arch/x86/tsx.c b/xen/arch/x86/tsx.c
index 41b6092cfe16..fc199815994d 100644
--- a/xen/arch/x86/tsx.c
+++ b/xen/arch/x86/tsx.c
@@ -19,7 +19,6 @@
  * controlling TSX behaviour, and where TSX isn't force-disabled by firmware.
  */
 int8_t __read_mostly opt_tsx = -1;
-int8_t __read_mostly cpu_has_tsx_ctrl = -1;
 bool __read_mostly rtm_disabled;
 
 static int __init cf_check parse_tsx(const char *s)
@@ -37,24 +36,28 @@ custom_param("tsx", parse_tsx);
 
 void tsx_init(void)
 {
+    static bool __read_mostly once;
+
     /*
      * This function is first called between microcode being loaded, and CPUID
      * being scanned generally.  Read into boot_cpu_data.x86_capability[] for
      * the cpu_has_* bits we care about using here.
      */
-    if ( unlikely(cpu_has_tsx_ctrl < 0) )
+    if ( unlikely(!once) )
     {
-        uint64_t caps = 0;
         bool has_rtm_always_abort;
 
+        once = true;
+
         if ( boot_cpu_data.cpuid_level >= 7 )
             boot_cpu_data.x86_capability[FEATURESET_7d0]
                 = cpuid_count_edx(7, 0);
 
         if ( cpu_has_arch_caps )
-            rdmsrl(MSR_ARCH_CAPABILITIES, caps);
+            rdmsr(MSR_ARCH_CAPABILITIES,
+                  boot_cpu_data.x86_capability[FEATURESET_10Al],
+                  boot_cpu_data.x86_capability[FEATURESET_10Ah]);
 
-        cpu_has_tsx_ctrl = !!(caps & ARCH_CAPS_TSX_CTRL);
         has_rtm_always_abort = cpu_has_rtm_always_abort;
 
         if ( cpu_has_tsx_ctrl && cpu_has_srbds_ctrl )
-- 
2.30.2



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

* [PATCH v2 10/10] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check
  2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
                   ` (8 preceding siblings ...)
  2023-05-24 11:25 ` [PATCH v2 09/10] x86/tsx: " Andrew Cooper
@ 2023-05-24 11:25 ` Andrew Cooper
  2023-05-24 15:06   ` Jan Beulich
  9 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 11:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

MSR_ARCH_CAPS data is now included in featureset information.  Replace
opencoded checks with regular feature ones.

No functional change.

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/include/asm/cpufeature.h |  7 ++++
 xen/arch/x86/spec_ctrl.c              | 56 +++++++++++++--------------
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 9047ea43f503..50235f098d70 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -183,8 +183,15 @@ static inline bool boot_cpu_has(unsigned int feat)
 #define cpu_has_avx_ne_convert  boot_cpu_has(X86_FEATURE_AVX_NE_CONVERT)
 
 /* MSR_ARCH_CAPS */
+#define cpu_has_rdcl_no         boot_cpu_has(X86_FEATURE_RDCL_NO)
+#define cpu_has_eibrs           boot_cpu_has(X86_FEATURE_EIBRS)
+#define cpu_has_rsba            boot_cpu_has(X86_FEATURE_RSBA)
+#define cpu_has_skip_l1dfl      boot_cpu_has(X86_FEATURE_SKIP_L1DFL)
+#define cpu_has_mds_no          boot_cpu_has(X86_FEATURE_MDS_NO)
 #define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
 #define cpu_has_tsx_ctrl        boot_cpu_has(X86_FEATURE_TSX_CTRL)
+#define cpu_has_taa_no          boot_cpu_has(X86_FEATURE_TAA_NO)
+#define cpu_has_fb_clear        boot_cpu_has(X86_FEATURE_FB_CLEAR)
 
 /* Synthesized. */
 #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index f81db2143328..50d467f74cf8 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -282,12 +282,10 @@ custom_param("spec-ctrl", parse_spec_ctrl);
 int8_t __read_mostly opt_xpti_hwdom = -1;
 int8_t __read_mostly opt_xpti_domu = -1;
 
-static __init void xpti_init_default(uint64_t caps)
+static __init void xpti_init_default(void)
 {
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
-        caps = ARCH_CAPS_RDCL_NO;
-
-    if ( caps & ARCH_CAPS_RDCL_NO )
+    if ( (boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
+         cpu_has_rdcl_no )
     {
         if ( opt_xpti_hwdom < 0 )
             opt_xpti_hwdom = 0;
@@ -390,9 +388,10 @@ static int __init cf_check parse_pv_l1tf(const char *s)
 }
 custom_param("pv-l1tf", parse_pv_l1tf);
 
-static void __init print_details(enum ind_thunk thunk, uint64_t caps)
+static void __init print_details(enum ind_thunk thunk)
 {
     unsigned int _7d0 = 0, _7d2 = 0, e8b = 0, max = 0, tmp;
+    uint64_t caps = 0;
 
     /* Collect diagnostics about available mitigations. */
     if ( boot_cpu_data.cpuid_level >= 7 )
@@ -401,6 +400,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
         cpuid_count(7, 2, &tmp, &tmp, &tmp, &_7d2);
     if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
         cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
+    if ( cpu_has_arch_caps )
+        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
 
     printk("Speculative mitigation facilities:\n");
 
@@ -578,7 +579,7 @@ static bool __init check_smt_enabled(void)
 }
 
 /* Calculate whether Retpoline is known-safe on this CPU. */
-static bool __init retpoline_safe(uint64_t caps)
+static bool __init retpoline_safe(void)
 {
     unsigned int ucode_rev = this_cpu(cpu_sig).rev;
 
@@ -596,7 +597,7 @@ static bool __init retpoline_safe(uint64_t caps)
      * Processors offering Enhanced IBRS are not guarenteed to be
      * repoline-safe.
      */
-    if ( caps & (ARCH_CAPS_RSBA | ARCH_CAPS_IBRS_ALL) )
+    if ( cpu_has_rsba || cpu_has_eibrs )
         return false;
 
     switch ( boot_cpu_data.x86_model )
@@ -845,7 +846,7 @@ static void __init ibpb_calculations(void)
 }
 
 /* Calculate whether this CPU is vulnerable to L1TF. */
-static __init void l1tf_calculations(uint64_t caps)
+static __init void l1tf_calculations(void)
 {
     bool hit_default = false;
 
@@ -933,7 +934,7 @@ static __init void l1tf_calculations(uint64_t caps)
     }
 
     /* Any processor advertising RDCL_NO should be not vulnerable to L1TF. */
-    if ( caps & ARCH_CAPS_RDCL_NO )
+    if ( cpu_has_rdcl_no )
         cpu_has_bug_l1tf = false;
 
     if ( cpu_has_bug_l1tf && hit_default )
@@ -992,7 +993,7 @@ static __init void l1tf_calculations(uint64_t caps)
 }
 
 /* Calculate whether this CPU is vulnerable to MDS. */
-static __init void mds_calculations(uint64_t caps)
+static __init void mds_calculations(void)
 {
     /* MDS is only known to affect Intel Family 6 processors at this time. */
     if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
@@ -1000,7 +1001,7 @@ static __init void mds_calculations(uint64_t caps)
         return;
 
     /* Any processor advertising MDS_NO should be not vulnerable to MDS. */
-    if ( caps & ARCH_CAPS_MDS_NO )
+    if ( cpu_has_mds_no )
         return;
 
     switch ( boot_cpu_data.x86_model )
@@ -1113,10 +1114,6 @@ void __init init_speculation_mitigations(void)
     enum ind_thunk thunk = THUNK_DEFAULT;
     bool has_spec_ctrl, ibrs = false, hw_smt_enabled;
     bool cpu_has_bug_taa;
-    uint64_t caps = 0;
-
-    if ( cpu_has_arch_caps )
-        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
 
     hw_smt_enabled = check_smt_enabled();
 
@@ -1163,7 +1160,7 @@ void __init init_speculation_mitigations(void)
              * On all hardware, we'd like to use retpoline in preference to
              * IBRS, but only if it is safe on this hardware.
              */
-            if ( retpoline_safe(caps) )
+            if ( retpoline_safe() )
                 thunk = THUNK_RETPOLINE;
             else if ( has_spec_ctrl )
                 ibrs = true;
@@ -1392,13 +1389,13 @@ void __init init_speculation_mitigations(void)
      * threads.  Activate this if SMT is enabled, and Xen is using a non-zero
      * MSR_SPEC_CTRL setting.
      */
-    if ( boot_cpu_has(X86_FEATURE_IBRSB) && !(caps & ARCH_CAPS_IBRS_ALL) &&
+    if ( boot_cpu_has(X86_FEATURE_IBRSB) && !cpu_has_eibrs &&
          hw_smt_enabled && default_xen_spec_ctrl )
         setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
 
-    xpti_init_default(caps);
+    xpti_init_default();
 
-    l1tf_calculations(caps);
+    l1tf_calculations();
 
     /*
      * By default, enable PV domU L1TF mitigations on all L1TF-vulnerable
@@ -1419,7 +1416,7 @@ void __init init_speculation_mitigations(void)
     if ( !boot_cpu_has(X86_FEATURE_L1D_FLUSH) )
         opt_l1d_flush = 0;
     else if ( opt_l1d_flush == -1 )
-        opt_l1d_flush = cpu_has_bug_l1tf && !(caps & ARCH_CAPS_SKIP_L1DFL);
+        opt_l1d_flush = cpu_has_bug_l1tf && !cpu_has_skip_l1dfl;
 
     /* We compile lfence's in by default, and nop them out if requested. */
     if ( !opt_branch_harden )
@@ -1442,7 +1439,7 @@ void __init init_speculation_mitigations(void)
             "enabled.  Please assess your configuration and choose an\n"
             "explicit 'smt=<bool>' setting.  See XSA-273.\n");
 
-    mds_calculations(caps);
+    mds_calculations();
 
     /*
      * Parts which enumerate FB_CLEAR are those which are post-MDS_NO and have
@@ -1454,7 +1451,7 @@ void __init init_speculation_mitigations(void)
      * the return-to-guest path.
      */
     if ( opt_unpriv_mmio )
-        opt_fb_clear_mmio = caps & ARCH_CAPS_FB_CLEAR;
+        opt_fb_clear_mmio = cpu_has_fb_clear;
 
     /*
      * By default, enable PV and HVM mitigations on MDS-vulnerable hardware.
@@ -1484,7 +1481,7 @@ void __init init_speculation_mitigations(void)
      */
     if ( opt_md_clear_pv || opt_md_clear_hvm || opt_fb_clear_mmio )
         setup_force_cpu_cap(X86_FEATURE_SC_VERW_IDLE);
-    opt_md_clear_hvm &= !(caps & ARCH_CAPS_SKIP_L1DFL) && !opt_l1d_flush;
+    opt_md_clear_hvm &= !cpu_has_skip_l1dfl && !opt_l1d_flush;
 
     /*
      * Warn the user if they are on MLPDS/MFBDS-vulnerable hardware with HT
@@ -1515,8 +1512,7 @@ void __init init_speculation_mitigations(void)
      *       we check both to spot TSX in a microcode/cmdline independent way.
      */
     cpu_has_bug_taa =
-        (cpu_has_rtm || (caps & ARCH_CAPS_TSX_CTRL)) &&
-        (caps & (ARCH_CAPS_MDS_NO | ARCH_CAPS_TAA_NO)) == ARCH_CAPS_MDS_NO;
+        (cpu_has_rtm || cpu_has_tsx_ctrl) && cpu_has_mds_no && !cpu_has_taa_no;
 
     /*
      * On TAA-affected hardware, disabling TSX is the preferred mitigation, vs
@@ -1535,7 +1531,7 @@ void __init init_speculation_mitigations(void)
      * plausibly value TSX higher than Hyperthreading...), disable TSX to
      * mitigate TAA.
      */
-    if ( opt_tsx == -1 && cpu_has_bug_taa && (caps & ARCH_CAPS_TSX_CTRL) &&
+    if ( opt_tsx == -1 && cpu_has_bug_taa && cpu_has_tsx_ctrl &&
          ((hw_smt_enabled && opt_smt) ||
           !boot_cpu_has(X86_FEATURE_SC_VERW_IDLE)) )
     {
@@ -1560,15 +1556,15 @@ void __init init_speculation_mitigations(void)
     if ( cpu_has_srbds_ctrl )
     {
         if ( opt_srb_lock == -1 && !opt_unpriv_mmio &&
-             (caps & (ARCH_CAPS_MDS_NO|ARCH_CAPS_TAA_NO)) == ARCH_CAPS_MDS_NO &&
-             (!cpu_has_hle || ((caps & ARCH_CAPS_TSX_CTRL) && rtm_disabled)) )
+             cpu_has_mds_no && !cpu_has_taa_no &&
+             (!cpu_has_hle || (cpu_has_tsx_ctrl && rtm_disabled)) )
             opt_srb_lock = 0;
 
         set_in_mcu_opt_ctrl(MCU_OPT_CTRL_RNGDS_MITG_DIS,
                             opt_srb_lock ? 0 : MCU_OPT_CTRL_RNGDS_MITG_DIS);
     }
 
-    print_details(thunk, caps);
+    print_details(thunk);
 
     /*
      * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
-- 
2.30.2



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

* Re: [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy
  2023-05-24 11:25 ` [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy Andrew Cooper
@ 2023-05-24 14:03   ` Andrew Cooper
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 14:03 UTC (permalink / raw)
  To: Xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu

On 24/05/2023 12:25 pm, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 9bbb385db42d..f1084bb1ed36 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -477,6 +477,11 @@ static void generic_identify(struct cpuinfo_x86 *c)
>  		cpuid_count(0xd, 1,
>  			    &c->x86_capability[FEATURESET_Da1],
>  			    &tmp, &tmp, &tmp);
> +
> +	if (test_bit(X86_FEATURE_ARCH_CAPS, c->x86_capability))
> +		rdmsr(MSR_ARCH_CAPABILITIES,
> +		      c->x86_capability[FEATURESET_10Al],
> +		      c->x86_capability[FEATURESET_10Ah]);

I managed to send out a stale version.  I've corrected this and the
other instances locally.

~Andrew


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

* Re: [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS
  2023-05-24 11:25 ` [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS Andrew Cooper
@ 2023-05-24 14:53   ` Jan Beulich
  2023-05-24 18:02     ` Andrew Cooper
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2023-05-24 14:53 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 24.05.2023 13:25, Andrew Cooper wrote:
> Bits through 24 are already defined, meaning that we're not far off needing
> the second word.  Put both in right away.
> 
> As both halves are present now, the arch_caps field is full width.  Adjust the
> unit test, which notices.
> 
> The bool bitfield names in the arch_caps union are unused, and somewhat out of
> date.  They'll shortly be automatically generated.
> 
> Add CPUID and MSR prefixes to the ./xen-cpuid verbose output, now that there
> are a mix of the two.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit ...

> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -226,31 +226,41 @@ static const char *const str_7d2[32] =
>      [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
>  };
>  
> +static const char *const str_10Al[32] =
> +{
> +};
> +
> +static const char *const str_10Ah[32] =
> +{
> +};
> +
>  static const struct {
>      const char *name;
>      const char *abbr;
>      const char *const *strs;
>  } decodes[] =
>  {
> -    { "0x00000001.edx",   "1d",  str_1d },
> -    { "0x00000001.ecx",   "1c",  str_1c },
> -    { "0x80000001.edx",   "e1d", str_e1d },
> -    { "0x80000001.ecx",   "e1c", str_e1c },
> -    { "0x0000000d:1.eax", "Da1", str_Da1 },
> -    { "0x00000007:0.ebx", "7b0", str_7b0 },
> -    { "0x00000007:0.ecx", "7c0", str_7c0 },
> -    { "0x80000007.edx",   "e7d", str_e7d },
> -    { "0x80000008.ebx",   "e8b", str_e8b },
> -    { "0x00000007:0.edx", "7d0", str_7d0 },
> -    { "0x00000007:1.eax", "7a1", str_7a1 },
> -    { "0x80000021.eax",  "e21a", str_e21a },
> -    { "0x00000007:1.ebx", "7b1", str_7b1 },
> -    { "0x00000007:2.edx", "7d2", str_7d2 },
> -    { "0x00000007:1.ecx", "7c1", str_7c1 },
> -    { "0x00000007:1.edx", "7d1", str_7d1 },
> +    { "CPUID 0x00000001.edx",        "1d", str_1d },
> +    { "CPUID 0x00000001.ecx",        "1c", str_1c },
> +    { "CPUID 0x80000001.edx",       "e1d", str_e1d },
> +    { "CPUID 0x80000001.ecx",       "e1c", str_e1c },
> +    { "CPUID 0x0000000d:1.eax",     "Da1", str_Da1 },
> +    { "CPUID 0x00000007:0.ebx",     "7b0", str_7b0 },
> +    { "CPUID 0x00000007:0.ecx",     "7c0", str_7c0 },
> +    { "CPUID 0x80000007.edx",       "e7d", str_e7d },
> +    { "CPUID 0x80000008.ebx",       "e8b", str_e8b },
> +    { "CPUID 0x00000007:0.edx",     "7d0", str_7d0 },
> +    { "CPUID 0x00000007:1.eax",     "7a1", str_7a1 },
> +    { "CPUID 0x80000021.eax",      "e21a", str_e21a },
> +    { "CPUID 0x00000007:1.ebx",     "7b1", str_7b1 },
> +    { "CPUID 0x00000007:2.edx",     "7d2", str_7d2 },
> +    { "CPUID 0x00000007:1.ecx",     "7c1", str_7c1 },
> +    { "CPUID 0x00000007:1.edx",     "7d1", str_7d1 },

... I'm not really happy about this added verbosity. In a tool of this
name, I think it's pretty clear that unadorned names are CPUID stuff.

> +    { "MSR   0x0000010a.lo",      "m10Al", str_10Al },
> +    { "MSR   0x0000010a.hi",      "m10Ah", str_10Ah },

Once we gain a few more MSRs, I'm afraid the raw numbers aren't going
to be very useful. As vaguely suggested before, how about

    { "MSR_ARCH_CAPS.lo",      "m10Al", str_10Al },
    { "MSR_ARCH_CAPS.hi",      "m10Ah", str_10Ah },

?

Jan


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

* Re: [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names
  2023-05-24 11:25 ` [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names Andrew Cooper
@ 2023-05-24 14:56   ` Jan Beulich
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2023-05-24 14:56 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 24.05.2023 13:25, Andrew Cooper wrote:
> Seed the default visibility from the dom0 special case, which for the most
> part just exposes the *_NO bits.  EIBRS is the one non-*_NO bit, which is
> "just" a status bit to the guest indicating a change in implemention of IBRS
> which is already fully supported.
> 
> Insert a block dependency from the ARCH_CAPS CPUID bit to the entire content
> of the MSR.  This is because MSRs have no structure information similar to
> CPUID, and used by x86_cpu_policy_clear_out_of_range_leaves(), in order to
> bulk-clear inaccessable words.
> 
> The overall CPUID bit is still max-only, so all of MSR_ARCH_CAPS is hidden in
> the default policies.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>




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

* Re: [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies
  2023-05-24 11:25 ` [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies Andrew Cooper
@ 2023-05-24 15:02   ` Jan Beulich
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2023-05-24 15:02 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 24.05.2023 13:25, Andrew Cooper wrote:
> We already have common and default feature adjustment helpers.  Introduce one
> for max featuresets too.
> 
> Offer MSR_ARCH_CAPS unconditionally in the max policy, and stop clobbering the
> data inherited from the Host policy.  This will be necessary to level a VM
> safely for migration.  Annotate the ARCH_CAPS CPUID bit as special.  Note:
> ARCH_CAPS is still max-only for now, so will not be inhereted by the default
> policies.
> 
> With this done, the special case for dom0 can be shrunk to just resampling the
> Host policy (as ARCH_CAPS isn't visible by default yet).
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>




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

* Re: [PATCH v2 10/10] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check
  2023-05-24 11:25 ` [PATCH v2 10/10] x86/spec-ctrl: " Andrew Cooper
@ 2023-05-24 15:06   ` Jan Beulich
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2023-05-24 15:06 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 24.05.2023 13:25, Andrew Cooper wrote:
> MSR_ARCH_CAPS data is now included in featureset information.  Replace
> opencoded checks with regular feature ones.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>




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

* Re: [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS
  2023-05-24 14:53   ` Jan Beulich
@ 2023-05-24 18:02     ` Andrew Cooper
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2023-05-24 18:02 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 24/05/2023 3:53 pm, Jan Beulich wrote:
> On 24.05.2023 13:25, Andrew Cooper wrote:
>> Bits through 24 are already defined, meaning that we're not far off needing
>> the second word.  Put both in right away.
>>
>> As both halves are present now, the arch_caps field is full width.  Adjust the
>> unit test, which notices.
>>
>> The bool bitfield names in the arch_caps union are unused, and somewhat out of
>> date.  They'll shortly be automatically generated.
>>
>> Add CPUID and MSR prefixes to the ./xen-cpuid verbose output, now that there
>> are a mix of the two.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks,

> albeit ...
>
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -226,31 +226,41 @@ static const char *const str_7d2[32] =
>>      [ 4] = "bhi-ctrl",      [ 5] = "mcdt-no",
>>  };
>>  
>> +static const char *const str_10Al[32] =
>> +{
>> +};
>> +
>> +static const char *const str_10Ah[32] =
>> +{
>> +};
>> +
>>  static const struct {
>>      const char *name;
>>      const char *abbr;
>>      const char *const *strs;
>>  } decodes[] =
>>  {
>> -    { "0x00000001.edx",   "1d",  str_1d },
>> -    { "0x00000001.ecx",   "1c",  str_1c },
>> -    { "0x80000001.edx",   "e1d", str_e1d },
>> -    { "0x80000001.ecx",   "e1c", str_e1c },
>> -    { "0x0000000d:1.eax", "Da1", str_Da1 },
>> -    { "0x00000007:0.ebx", "7b0", str_7b0 },
>> -    { "0x00000007:0.ecx", "7c0", str_7c0 },
>> -    { "0x80000007.edx",   "e7d", str_e7d },
>> -    { "0x80000008.ebx",   "e8b", str_e8b },
>> -    { "0x00000007:0.edx", "7d0", str_7d0 },
>> -    { "0x00000007:1.eax", "7a1", str_7a1 },
>> -    { "0x80000021.eax",  "e21a", str_e21a },
>> -    { "0x00000007:1.ebx", "7b1", str_7b1 },
>> -    { "0x00000007:2.edx", "7d2", str_7d2 },
>> -    { "0x00000007:1.ecx", "7c1", str_7c1 },
>> -    { "0x00000007:1.edx", "7d1", str_7d1 },
>> +    { "CPUID 0x00000001.edx",        "1d", str_1d },
>> +    { "CPUID 0x00000001.ecx",        "1c", str_1c },
>> +    { "CPUID 0x80000001.edx",       "e1d", str_e1d },
>> +    { "CPUID 0x80000001.ecx",       "e1c", str_e1c },
>> +    { "CPUID 0x0000000d:1.eax",     "Da1", str_Da1 },
>> +    { "CPUID 0x00000007:0.ebx",     "7b0", str_7b0 },
>> +    { "CPUID 0x00000007:0.ecx",     "7c0", str_7c0 },
>> +    { "CPUID 0x80000007.edx",       "e7d", str_e7d },
>> +    { "CPUID 0x80000008.ebx",       "e8b", str_e8b },
>> +    { "CPUID 0x00000007:0.edx",     "7d0", str_7d0 },
>> +    { "CPUID 0x00000007:1.eax",     "7a1", str_7a1 },
>> +    { "CPUID 0x80000021.eax",      "e21a", str_e21a },
>> +    { "CPUID 0x00000007:1.ebx",     "7b1", str_7b1 },
>> +    { "CPUID 0x00000007:2.edx",     "7d2", str_7d2 },
>> +    { "CPUID 0x00000007:1.ecx",     "7c1", str_7c1 },
>> +    { "CPUID 0x00000007:1.edx",     "7d1", str_7d1 },
> ... I'm not really happy about this added verbosity. In a tool of this
> name, I think it's pretty clear that unadorned names are CPUID stuff.

You might make the connection, but it's unreasonable to expect the same
of everyone else.  This is used by end users.

If nothing else, the name of the binary is made stale by this change.

>> +    { "MSR   0x0000010a.lo",      "m10Al", str_10Al },
>> +    { "MSR   0x0000010a.hi",      "m10Ah", str_10Ah },
> Once we gain a few more MSRs, I'm afraid the raw numbers aren't going
> to be very useful. As vaguely suggested before, how about
>
>     { "MSR_ARCH_CAPS.lo",      "m10Al", str_10Al },
>     { "MSR_ARCH_CAPS.hi",      "m10Ah", str_10Ah },
>
> ?

I've done this.  I remain to be convinced, but it probably is nicer for
people who don't know the MSR indices like I do.

~Andrew


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

end of thread, other threads:[~2023-05-24 18:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-24 11:25 [PATCH v2 00/10] x86: Introduce MSR_ARCH_CAPS into featuresets Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 01/10] x86/boot: Rework dom0 feature configuration Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 02/10] x86/boot: Adjust MSR_ARCH_CAPS handling for the Host policy Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 03/10] x86/cpu-policy: Infrastructure for MSR_ARCH_CAPS Andrew Cooper
2023-05-24 14:53   ` Jan Beulich
2023-05-24 18:02     ` Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 04/10] x86/cpu-policy: MSR_ARCH_CAPS feature names Andrew Cooper
2023-05-24 14:56   ` Jan Beulich
2023-05-24 11:25 ` [PATCH v2 05/10] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy Andrew Cooper
2023-05-24 14:03   ` Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 06/10] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies Andrew Cooper
2023-05-24 15:02   ` Jan Beulich
2023-05-24 11:25 ` [PATCH v2 07/10] x86/cpufeature: Rework {boot_,}cpu_has() Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 08/10] x86/vtx: Remove opencoded MSR_ARCH_CAPS check Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 09/10] x86/tsx: " Andrew Cooper
2023-05-24 11:25 ` [PATCH v2 10/10] x86/spec-ctrl: " Andrew Cooper
2023-05-24 15:06   ` Jan Beulich

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.