Re: [PATCH v4 03/36] x86/bugs: Restructure mmio mitigation

[Pawan Gupta]

On Thu, Mar 13, 2025 at 10:36:17AM +0100, Borislav Petkov wrote:
> I'd expect to see:
> 
> 	if (mmio_mitigation == MMIO_MITIGATION_AUTO) {
>                 mmio_mitigation = MMIO_MITIGATION_VERW;
> 		verw_mitigation_selected = true;
> 	}
> 
>         if (boot_cpu_has_bug(X86_BUG_MDS) || taa_vulnerable())
>                 verw_mitigation_selected = true;
> 
> because the above branch already selected MMIO_MITIGATION_VERW so we might as
> well set verw_mitigation_selected, right?

There is a subtle difference between setting verw_mitigation_selected and
MMIO_MITIGATION_VERW. The former is a system-wide switch that indicates
VERW is needed at both kernel-exit and VMenter. MMIO Stale Data is
different from other VERW based mitigations because it only requires VERW
at VMenter, when not affected by MDS/TAA. So, turning the system-wide knob
here would be wrong.

> > +static void __init mmio_update_mitigation(void)
> > +{
> > +	if (!boot_cpu_has_bug(X86_BUG_MMIO_STALE_DATA) || cpu_mitigations_off())
> > +		return;
> > +
> > +	if (verw_mitigation_selected)
> > +		mmio_mitigation = MMIO_MITIGATION_VERW;
[...]
> > +	if (boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN))
> 
> Btw, that UNKNOWN thing is just silly. Looking at git history:
> 
> 7df548840c49 ("x86/bugs: Add "unknown" reporting for MMIO Stale Data")
> 
> this was added just so that it doesn't say "Not affected" about those CPUs but
> "unknown."
> 
> But
> 
>   "Mitigation is not deployed when the status is unknown."
> 
> so if it is only about reporting, I think we can synthesize the logic of this:
> 
>         if (!arch_cap_mmio_immune(x86_arch_cap_msr)) {
>                 if (cpu_matches(cpu_vuln_blacklist, MMIO))
>                         setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
>                 else if (!cpu_matches(cpu_vuln_whitelist, NO_MMIO))
>                         setup_force_cpu_bug(X86_BUG_MMIO_UNKNOWN);
>         }
> 
> into a separate function and get rid of that X86_BUG_MMIO_UNKNOWN thing.

Hmm, that would not be straightforward, specially for sysfs status. The
above logic requires parsing the cpu_vuln_whitelist which is not available
after init. Moreover, sysfs reads would become slower if it has to read an
MSR and parse tables.

Also, cpu_show_common() by default shows "Not affected" in the absence of
bug bit. So setting X86_BUG_MMIO_UNKNOWN is simpler overall.

cpu_show_common(bug)
{
	if (!boot_cpu_has_bug(bug))
		return sysfs_emit(buf, "Not affected\n");