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

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Sent: Tuesday, February 11, 2025 5:19 PM
> To: Kaplan, David <David.Kaplan@xxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Borislav Petkov <bp@xxxxxxxxx>; Peter
> Zijlstra <peterz@xxxxxxxxxxxxx>; Pawan Gupta
> <pawan.kumar.gupta@xxxxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>; Dave
> Hansen <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx; H . Peter Anvin
> <hpa@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 06/35] x86/bugs: Restructure mmio mitigation
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> 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.

Does it?  In that case, mds_update_mitigation() will see that verw_mitigation_enabled() is true (because mmio_mitigation!=MMIO_MITIGATION_OFF) and then enable the mds mitigation.

>
> >         /*
> >          * 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.
>

I think they are getting reported as mitigated because the mmio mitigation was enabled.

--David Kaplan