| Message ID | 20240229181354.2560819-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/cpu-policy: Allow for levelling of VERW side effects | expand |
On Thu, Feb 29, 2024 at 06:13:54PM +0000, Andrew Cooper wrote: > MD_CLEAR and FB_CLEAR need OR-ing across a migrate pool. Allow this, by > having them unconditinally set in max, with the host values reflected in > default. Annotate the bits as having special properies. > > 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/cpu-policy.c | 24 +++++++++++++++++++++ > xen/include/public/arch-x86/cpufeatureset.h | 4 ++-- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c > index f3ed2d3a3227..41123e6cf778 100644 > --- a/xen/arch/x86/cpu-policy.c > +++ b/xen/arch/x86/cpu-policy.c > @@ -442,6 +442,16 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs) > __set_bit(X86_FEATURE_RSBA, fs); > __set_bit(X86_FEATURE_RRSBA, fs); > > + /* > + * These bits indicate that the VERW instruction may have gained > + * scrubbing side effects. With pooling, they mean "you might migrate > + * somewhere where scrubbing is necessary", and may need exposing on > + * unaffected hardware. This is fine, because the VERW instruction > + * has been around since the 286. > + */ > + __set_bit(X86_FEATURE_MD_CLEAR, fs); > + __set_bit(X86_FEATURE_FB_CLEAR, fs); > + > /* > * The Gather Data Sampling microcode mitigation (August 2023) has an > * adverse performance impact on the CLWB instruction on SKX/CLX/CPX. > @@ -476,6 +486,20 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs) > cpu_has_rdrand && !is_forced_cpu_cap(X86_FEATURE_RDRAND) ) > __clear_bit(X86_FEATURE_RDRAND, fs); > > + /* > + * These bits indicate that the VERW instruction may have gained > + * scrubbing side effects. The max policy has them set for migration > + * reasons, so reset the default policy back to the host values in > + * case we're unaffected. > + */ > + fs[FEATURESET_7d0] &= ~cpufeat_mask(X86_FEATURE_MD_CLEAR); > + fs[FEATURESET_m10Al] &= ~cpufeat_mask(X86_FEATURE_FB_CLEAR); > + > + fs[FEATURESET_7d0] |= (boot_cpu_data.x86_capability[FEATURESET_7d0] & > + cpufeat_mask(X86_FEATURE_MD_CLEAR)); > + fs[FEATURESET_m10Al] |= (boot_cpu_data.x86_capability[FEATURESET_m10Al] & > + cpufeat_mask(X86_FEATURE_FB_CLEAR)); This seems quite convoluted, why not use: __clear_bit(X86_FEATURE_MD_CLEAR, fs); if ( boot_cpu_has(X86_FEATURE_MD_CLEAR) ) __set_bit(X86_FEATURE_MD_CLEAR, fs); And the same for FB_CLEAR. I think that's quite easier to read. Thanks, Roger.
On 01/03/2024 1:28 pm, Roger Pau Monné wrote: > On Thu, Feb 29, 2024 at 06:13:54PM +0000, Andrew Cooper wrote: >> MD_CLEAR and FB_CLEAR need OR-ing across a migrate pool. Allow this, by >> having them unconditinally set in max, with the host values reflected in >> default. Annotate the bits as having special properies. >> >> 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/cpu-policy.c | 24 +++++++++++++++++++++ >> xen/include/public/arch-x86/cpufeatureset.h | 4 ++-- >> 2 files changed, 26 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c >> index f3ed2d3a3227..41123e6cf778 100644 >> --- a/xen/arch/x86/cpu-policy.c >> +++ b/xen/arch/x86/cpu-policy.c >> @@ -442,6 +442,16 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs) >> __set_bit(X86_FEATURE_RSBA, fs); >> __set_bit(X86_FEATURE_RRSBA, fs); >> >> + /* >> + * These bits indicate that the VERW instruction may have gained >> + * scrubbing side effects. With pooling, they mean "you might migrate >> + * somewhere where scrubbing is necessary", and may need exposing on >> + * unaffected hardware. This is fine, because the VERW instruction >> + * has been around since the 286. >> + */ >> + __set_bit(X86_FEATURE_MD_CLEAR, fs); >> + __set_bit(X86_FEATURE_FB_CLEAR, fs); >> + >> /* >> * The Gather Data Sampling microcode mitigation (August 2023) has an >> * adverse performance impact on the CLWB instruction on SKX/CLX/CPX. >> @@ -476,6 +486,20 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs) >> cpu_has_rdrand && !is_forced_cpu_cap(X86_FEATURE_RDRAND) ) >> __clear_bit(X86_FEATURE_RDRAND, fs); >> >> + /* >> + * These bits indicate that the VERW instruction may have gained >> + * scrubbing side effects. The max policy has them set for migration >> + * reasons, so reset the default policy back to the host values in >> + * case we're unaffected. >> + */ >> + fs[FEATURESET_7d0] &= ~cpufeat_mask(X86_FEATURE_MD_CLEAR); >> + fs[FEATURESET_m10Al] &= ~cpufeat_mask(X86_FEATURE_FB_CLEAR); >> + >> + fs[FEATURESET_7d0] |= (boot_cpu_data.x86_capability[FEATURESET_7d0] & >> + cpufeat_mask(X86_FEATURE_MD_CLEAR)); >> + fs[FEATURESET_m10Al] |= (boot_cpu_data.x86_capability[FEATURESET_m10Al] & >> + cpufeat_mask(X86_FEATURE_FB_CLEAR)); > This seems quite convoluted, why not use: > > __clear_bit(X86_FEATURE_MD_CLEAR, fs); > if ( boot_cpu_has(X86_FEATURE_MD_CLEAR) ) > __set_bit(X86_FEATURE_MD_CLEAR, fs); > > And the same for FB_CLEAR. I think that's quite easier to read. This better? + /* + * These bits indicate that the VERW instruction may have gained + * scrubbing side effects. The max policy has them set for migration + * reasons, so reset the default policy back to the host values in + * case we're unaffected. + */ + __clear_bit(X86_FEATURE_MD_CLEAR); + if ( cpu_has_md_clear ) + __set_bit(X86_FEATURE_MD_CLEAR); + + __clear_bit(X86_FEATURE_FB_CLEAR); + if ( cpu_has_fb_clear ) + __set_bit(X86_FEATURE_FB_CLEAR); ~Andrew
On Fri, Mar 01, 2024 at 03:01:36PM +0000, Andrew Cooper wrote: > On 01/03/2024 1:28 pm, Roger Pau Monné wrote: > > On Thu, Feb 29, 2024 at 06:13:54PM +0000, Andrew Cooper wrote: > >> MD_CLEAR and FB_CLEAR need OR-ing across a migrate pool. Allow this, by > >> having them unconditinally set in max, with the host values reflected in > >> default. Annotate the bits as having special properies. > >> > >> 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/cpu-policy.c | 24 +++++++++++++++++++++ > >> xen/include/public/arch-x86/cpufeatureset.h | 4 ++-- > >> 2 files changed, 26 insertions(+), 2 deletions(-) > >> > >> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c > >> index f3ed2d3a3227..41123e6cf778 100644 > >> --- a/xen/arch/x86/cpu-policy.c > >> +++ b/xen/arch/x86/cpu-policy.c > >> @@ -442,6 +442,16 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs) > >> __set_bit(X86_FEATURE_RSBA, fs); > >> __set_bit(X86_FEATURE_RRSBA, fs); > >> > >> + /* > >> + * These bits indicate that the VERW instruction may have gained > >> + * scrubbing side effects. With pooling, they mean "you might migrate > >> + * somewhere where scrubbing is necessary", and may need exposing on > >> + * unaffected hardware. This is fine, because the VERW instruction > >> + * has been around since the 286. > >> + */ > >> + __set_bit(X86_FEATURE_MD_CLEAR, fs); > >> + __set_bit(X86_FEATURE_FB_CLEAR, fs); > >> + > >> /* > >> * The Gather Data Sampling microcode mitigation (August 2023) has an > >> * adverse performance impact on the CLWB instruction on SKX/CLX/CPX. > >> @@ -476,6 +486,20 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs) > >> cpu_has_rdrand && !is_forced_cpu_cap(X86_FEATURE_RDRAND) ) > >> __clear_bit(X86_FEATURE_RDRAND, fs); > >> > >> + /* > >> + * These bits indicate that the VERW instruction may have gained > >> + * scrubbing side effects. The max policy has them set for migration > >> + * reasons, so reset the default policy back to the host values in > >> + * case we're unaffected. > >> + */ > >> + fs[FEATURESET_7d0] &= ~cpufeat_mask(X86_FEATURE_MD_CLEAR); > >> + fs[FEATURESET_m10Al] &= ~cpufeat_mask(X86_FEATURE_FB_CLEAR); > >> + > >> + fs[FEATURESET_7d0] |= (boot_cpu_data.x86_capability[FEATURESET_7d0] & > >> + cpufeat_mask(X86_FEATURE_MD_CLEAR)); > >> + fs[FEATURESET_m10Al] |= (boot_cpu_data.x86_capability[FEATURESET_m10Al] & > >> + cpufeat_mask(X86_FEATURE_FB_CLEAR)); > > This seems quite convoluted, why not use: > > > > __clear_bit(X86_FEATURE_MD_CLEAR, fs); > > if ( boot_cpu_has(X86_FEATURE_MD_CLEAR) ) > > __set_bit(X86_FEATURE_MD_CLEAR, fs); > > > > And the same for FB_CLEAR. I think that's quite easier to read. > > This better? > > + /* > + * These bits indicate that the VERW instruction may have gained > + * scrubbing side effects. The max policy has them set for > migration > + * reasons, so reset the default policy back to the host values in > + * case we're unaffected. > + */ > + __clear_bit(X86_FEATURE_MD_CLEAR); > + if ( cpu_has_md_clear ) > + __set_bit(X86_FEATURE_MD_CLEAR); > + > + __clear_bit(X86_FEATURE_FB_CLEAR); > + if ( cpu_has_fb_clear ) > + __set_bit(X86_FEATURE_FB_CLEAR); Sure, with that please add my: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c index f3ed2d3a3227..41123e6cf778 100644 --- a/xen/arch/x86/cpu-policy.c +++ b/xen/arch/x86/cpu-policy.c @@ -442,6 +442,16 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs) __set_bit(X86_FEATURE_RSBA, fs); __set_bit(X86_FEATURE_RRSBA, fs); + /* + * These bits indicate that the VERW instruction may have gained + * scrubbing side effects. With pooling, they mean "you might migrate + * somewhere where scrubbing is necessary", and may need exposing on + * unaffected hardware. This is fine, because the VERW instruction + * has been around since the 286. + */ + __set_bit(X86_FEATURE_MD_CLEAR, fs); + __set_bit(X86_FEATURE_FB_CLEAR, fs); + /* * The Gather Data Sampling microcode mitigation (August 2023) has an * adverse performance impact on the CLWB instruction on SKX/CLX/CPX. @@ -476,6 +486,20 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs) cpu_has_rdrand && !is_forced_cpu_cap(X86_FEATURE_RDRAND) ) __clear_bit(X86_FEATURE_RDRAND, fs); + /* + * These bits indicate that the VERW instruction may have gained + * scrubbing side effects. The max policy has them set for migration + * reasons, so reset the default policy back to the host values in + * case we're unaffected. + */ + fs[FEATURESET_7d0] &= ~cpufeat_mask(X86_FEATURE_MD_CLEAR); + fs[FEATURESET_m10Al] &= ~cpufeat_mask(X86_FEATURE_FB_CLEAR); + + fs[FEATURESET_7d0] |= (boot_cpu_data.x86_capability[FEATURESET_7d0] & + cpufeat_mask(X86_FEATURE_MD_CLEAR)); + fs[FEATURESET_m10Al] |= (boot_cpu_data.x86_capability[FEATURESET_m10Al] & + cpufeat_mask(X86_FEATURE_FB_CLEAR)); + /* * The Gather Data Sampling microcode mitigation (August 2023) has an * adverse performance impact on the CLWB instruction on SKX/CLX/CPX. diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index b230d3a6907d..0374cec3a2af 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -262,7 +262,7 @@ XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A AVX512 Multiply Accumulation Single XEN_CPUFEATURE(FSRM, 9*32+ 4) /*A Fast Short REP MOVS */ XEN_CPUFEATURE(AVX512_VP2INTERSECT, 9*32+8) /*a VP2INTERSECT{D,Q} insns */ XEN_CPUFEATURE(SRBDS_CTRL, 9*32+ 9) /* MSR_MCU_OPT_CTRL and RNGDS_MITG_DIS. */ -XEN_CPUFEATURE(MD_CLEAR, 9*32+10) /*A VERW clears microarchitectural buffers */ +XEN_CPUFEATURE(MD_CLEAR, 9*32+10) /*!A VERW clears microarchitectural buffers */ XEN_CPUFEATURE(RTM_ALWAYS_ABORT, 9*32+11) /*! June 2021 TSX defeaturing in microcode. */ XEN_CPUFEATURE(TSX_FORCE_ABORT, 9*32+13) /* MSR_TSX_FORCE_ABORT.RTM_ABORT */ XEN_CPUFEATURE(SERIALIZE, 9*32+14) /*A SERIALIZE insn */ @@ -334,7 +334,7 @@ XEN_CPUFEATURE(DOITM, 16*32+12) /* Data Operand Invariant Timing 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, 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) /*! Restricted RSB Alternative */ XEN_CPUFEATURE(BHI_NO, 16*32+20) /*A No Branch History Injection */
MD_CLEAR and FB_CLEAR need OR-ing across a migrate pool. Allow this, by having them unconditinally set in max, with the host values reflected in default. Annotate the bits as having special properies. 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/cpu-policy.c | 24 +++++++++++++++++++++ xen/include/public/arch-x86/cpufeatureset.h | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) base-commit: 54fd7b997470e6686667ca8e18f9ba6139efcdea prerequisite-patch-id: d2cbc8f341e98ccfd66016f19532df3ddbfc68a4 prerequisite-patch-id: 4b4799fae62b5f41b9b0d2078e8b081605341a0a