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 <[email protected]> >> --- >> CC: Jan Beulich <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: Wei Liu <[email protected]> >> --- >> 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
