| Message ID | 20230515144259.1009245-5-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: Introduce MSR_ARCH_CAPS into featuresets | expand |
On 15.05.2023 16:42, Andrew Cooper wrote: > Seed the default visibility from the dom0 special case, which for the most > part just exposes the *_NO bits. EIBRS and SKIP_L1DFL are outliers here, in not presently being exposed to Dom0. If (latent) exposing of them wasn't an oversight, then this would imo want justifying here. They'll get exposed, after all, ... > Insert a block dependency from the ARCH_CAPS > CPUID bit to the entire content of the MSR. > > The overall CPUID bit is still max-only, so all of MSR_ARCH_CAPS is hidden in > the default policies. ... once this changes, as they're also not just 'a', but 'A'. > --- 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", Not "energy-filtering" or "energy-filtering-ctl" for the right one here? > + [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] = > --- 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) /*A 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 */ These last two aren't exactly in sync with the SDM; I assume that's intended? > +XEN_CPUFEATURE(DOITM, 16*32+12) /* Data Operand Invariant Timing Mode */ > +XEN_CPUFEATURE(SBDR_SSBD_NO, 16*32+13) /*A No Shared Buffer Data Read or Sideband Stale Data Propagation */ SBDR_SSDP_NO? > +XEN_CPUFEATURE(FBDSP_NO, 16*32+14) /*A No Fill Buffer Stale Data Propagation */ FBSDP_NO? Jan
On 16/05/2023 1:27 pm, Jan Beulich wrote: > On 15.05.2023 16:42, Andrew Cooper wrote: >> Seed the default visibility from the dom0 special case, which for the most >> part just exposes the *_NO bits. > EIBRS and SKIP_L1DFL are outliers here, in not presently being exposed > to Dom0. If (latent) exposing of them wasn't an oversight, then this would > imo want justifying here. They'll get exposed, after all, ... EIBRS is exposed to dom0. I've intentionally renamed it from ARCH_CAPS_IBRS_ALL because EIBRS is by far the more recognisable name now. SKIP_L1DFL is more complicated, but on yet more consideration I think it's probably wrong here. The confusion is regarding L1 Terminal Fault, where RDCL_NO was retroactively declared to mean "also fixes L1TF". SKIP_L1DFL means "you don't need to flush on vmentry", which is advertised by real hardware that also advertises RDCL_NO, but should also be advertised on vulnerable hardware by the L0 hypervisor to an L1 to say "don't worry, I'm already taking care of that for you". So on consideration, I think SKIP_L1DFL should not be visible by default, and when we start doing nested virt, should be reintroduced with a dependency on the VMX feature. >> + [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] = >> --- 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) /*A 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 */ > These last two aren't exactly in sync with the SDM; I assume that's > intended? Yeah. I'm bored of Intel failing with naming consistency. This one, I was assured that the draft was going to be fixed before publishing it, and yet... > >> +XEN_CPUFEATURE(DOITM, 16*32+12) /* Data Operand Invariant Timing Mode */ >> +XEN_CPUFEATURE(SBDR_SSBD_NO, 16*32+13) /*A No Shared Buffer Data Read or Sideband Stale Data Propagation */ > SBDR_SSDP_NO? > >> +XEN_CPUFEATURE(FBDSP_NO, 16*32+14) /*A No Fill Buffer Stale Data Propagation */ > FBSDP_NO? Oops. I hate the MMIO vulns especially because they're too similar to other vulns. Will fix. ~Andrew
On 16.05.2023 14:56, Andrew Cooper wrote: > On 16/05/2023 1:27 pm, Jan Beulich wrote: >> On 15.05.2023 16:42, Andrew Cooper wrote: >>> Seed the default visibility from the dom0 special case, which for the most >>> part just exposes the *_NO bits. >> EIBRS and SKIP_L1DFL are outliers here, in not presently being exposed >> to Dom0. If (latent) exposing of them wasn't an oversight, then this would >> imo want justifying here. They'll get exposed, after all, ... > > EIBRS is exposed to dom0. I've intentionally renamed it from > ARCH_CAPS_IBRS_ALL because EIBRS is by far the more recognisable name now. Oh, of course - I should have looked at more than just the names themselves. Hence the comment on a later patch then also is incorrectly saying "two bits", when referring back here. Jan
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c index 258584aafb9f..5b717f3f0091 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..3cfdc71df92b 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) /*A 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_SSBD_NO, 16*32+13) /*A No Shared Buffer Data Read or Sideband Stale Data Propagation */ +XEN_CPUFEATURE(FBDSP_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()))
Seed the default visibility from the dom0 special case, which for the most part just exposes the *_NO bits. Insert a block dependency from the ARCH_CAPS CPUID bit to the entire content of the MSR. 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. --- 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(+)