* [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects
@ 2024-02-29 18:13 Andrew Cooper
2024-03-01 13:28 ` Roger Pau Monné
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2024-02-29 18:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
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));
+
/*
* 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 */
base-commit: 54fd7b997470e6686667ca8e18f9ba6139efcdea
prerequisite-patch-id: d2cbc8f341e98ccfd66016f19532df3ddbfc68a4
prerequisite-patch-id: 4b4799fae62b5f41b9b0d2078e8b081605341a0a
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects
2024-02-29 18:13 [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects Andrew Cooper
@ 2024-03-01 13:28 ` Roger Pau Monné
2024-03-01 15:01 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Roger Pau Monné @ 2024-03-01 13:28 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Wei Liu
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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects
2024-03-01 13:28 ` Roger Pau Monné
@ 2024-03-01 15:01 ` Andrew Cooper
2024-03-01 19:34 ` Roger Pau Monné
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2024-03-01 15:01 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Xen-devel, Jan Beulich, Wei Liu
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects
2024-03-01 15:01 ` Andrew Cooper
@ 2024-03-01 19:34 ` Roger Pau Monné
0 siblings, 0 replies; 4+ messages in thread
From: Roger Pau Monné @ 2024-03-01 19:34 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Wei Liu
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-01 19:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-29 18:13 [PATCH] x86/cpu-policy: Allow for levelling of VERW side effects Andrew Cooper
2024-03-01 13:28 ` Roger Pau Monné
2024-03-01 15:01 ` Andrew Cooper
2024-03-01 19:34 ` Roger Pau Monné
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.