Re: [PATCH v3 06/35] x86/bugs: Restructure mmio mitigation

Josh Poimboeuf
2025-02-11T23:18:41Z

On Tue, Feb 11, 2025 at 08:35:27PM +0000, Kaplan, David wrote:
> > > @@ -538,21 +582,6 @@ static void __init mmio_select_mitigation(void)
> > > if (!(x86_arch_cap_msr & ARCH_CAP_FBSDP_NO))
> > > static_branch_enable(&mds_idle_clear);
> >
> > Right here it does the following:
> >
> > /*
> > * Enable CPU buffer clear mitigation for host and VMM, if also affected
> > * by MDS or TAA. Otherwise, enable mitigation for VMM only.
> > */
> > if (boot_cpu_has_bug(X86_BUG_MDS) ||
> > (boot_cpu_has_bug(X86_BUG_TAA) &&
> > boot_cpu_has(X86_FEATURE_RTM)))
> > setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF);
> >
> > Isn't that a cross-mitigation dependency? i.e. if
> > X86_FEATURE_CLEAR_CPU_BUF gets enabled here then the other mitigations
> > would need to update their mitigation reporting?
>
> I don't think so, nobody should be looking at
> X86_FEATURE_CLEAR_CPU_BUF to determine their mitigation selection,
> they should only be looking at the other variables like taa_mitigation
> as done in the verw_mitigation_enabled() function.

But isn't that a bug in the reporting? AFAICT one of the main
motivations for the cross dependencies (and the *_update_mitigation()
functions) is to fix the reporting if something actually ends up getting
mitigated by something else.

For example, "mds=off tsx_async_abort=full" results in both MDS and TAA
being reported "Mitigated", because they share the same VERW mitigation.

But in the above case, with X86_BUG_MDS, "mds=off mmio_stale_data=full"
shows MDS as vulnerable despite it actually being mitigated by VERW.

> /*
> * Enable CPU buffer clear mitigation for host and VMM, if also affected
> * by MDS or TAA.
> *
> * Only enable the VMM mitigation if the CPU buffer clear mitigation is
> * not being used.
> */
> if (rfds_mitigation == RFDS_MITIGATION_VERW ||
> boot_cpu_has_bug(X86_BUG_MDS) ||
> (boot_cpu_has_bug(X86_BUG_TAA) &&
> boot_cpu_has(X86_FEATURE_RTM))) {
> setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF);
> static_branch_disable(&mmio_stale_data_clear);
> } else
> static_branch_enable(&mmio_stale_data_clear);
>
> Does that sound right?

I *think* that's correct, but this still has the same issue that MDS/TAA
are now getting mitigated but not reported as such.

--
Josh