Re: [PATCH v3 05/35] x86/bugs: Restructure taa mitigation

Josh Poimboeuf
2025-02-10T22:50:18Z

On Wed, Jan 08, 2025 at 02:24:45PM -0600, David Kaplan wrote:
> @@ -400,48 +402,71 @@ static void __init taa_select_mitigation(void)
> return;
> }
>
> - if (cpu_mitigations_off()) {
> + if (cpu_mitigations_off())
> taa_mitigation = TAA_MITIGATION_OFF;
> - return;
> - }
>
> /*
> * TAA mitigation via VERW is turned off if both
> * tsx_async_abort=off and mds=off are specified.
> + *
> + * MDS mitigation will be checked in taa_update_mitigation().
> */
> - if (taa_mitigation == TAA_MITIGATION_OFF &&
> - mds_mitigation == MDS_MITIGATION_OFF)
> + if (taa_mitigation == TAA_MITIGATION_OFF)
> return;

This check seems rather pointless, the only thing after this is the
TAA_MITIGATION_AUTO check.

>
> - if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
> + /* Microcode will be checked in taa_update_mitigation(). */
> + if (taa_mitigation == TAA_MITIGATION_AUTO)
> taa_mitigation = TAA_MITIGATION_VERW;
> - else
> - taa_mitigation = TAA_MITIGATION_UCODE_NEEDED;

In the previous patch, MDS checks for ucode in both select and update,
which is overkill. That should probably be done only in
mds_update_mitigation() to be consistent with how TAA does it here?

>
> - /*
> - * VERW doesn't clear the CPU buffers when MD_CLEAR=1 and MDS_NO=1.
> - * A microcode update fixes this behavior to clear CPU buffers. It also
> - * adds support for MSR_IA32_TSX_CTRL which is enumerated by the
> - * ARCH_CAP_TSX_CTRL_MSR bit.
> - *
> - * On MDS_NO=1 CPUs if ARCH_CAP_TSX_CTRL_MSR is not set, microcode
> - * update is required.
> - */
> - if ( (x86_arch_cap_msr & ARCH_CAP_MDS_NO) &&
> - !(x86_arch_cap_msr & ARCH_CAP_TSX_CTRL_MSR))
> - taa_mitigation = TAA_MITIGATION_UCODE_NEEDED;
> +}

Extra whitespace here at the end of the function.

> +static void __init taa_update_mitigation(void)
> +{
> + if (!boot_cpu_has_bug(X86_BUG_TAA) || cpu_mitigations_off())
> + return;
> +
> + if (verw_mitigation_enabled())
> + taa_mitigation = TAA_MITIGATION_VERW;

This overwrites TAA_MITIGATION_TSX_DISABLED?

I think reporting TSX disabled here is more accurate than reporting
VERW, since the VERW is only done to mitigate the other vulns.

> +static void __init taa_apply_mitigation(void)
> +{
> + if (taa_mitigation == TAA_MITIGATION_VERW ||
> + taa_mitigation == TAA_MITIGATION_UCODE_NEEDED) {
> + /*
> + * TSX is enabled, select alternate mitigation for TAA which is
> + * the same as MDS. Enable MDS static branch to clear CPU buffers.
> + *
> + * For guests that can't determine whether the correct microcode is
> + * present on host, enable the mitigation for UCODE_NEEDED as well.
> + */
> + setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF);
> +
> + if (taa_nosmt || cpu_mitigations_auto_nosmt())
> + cpu_smt_disable(false);
> + }
>
> - if (taa_nosmt || cpu_mitigations_auto_nosmt())
> - cpu_smt_disable(false);
> }

Another extra whitespace here at the end of the function.

--
Josh