[MODERATED] Re: [PATCH v7 04/10] TAAv7 4

On Wed, Oct 23, 2019 at 12:05:55AM +0200, speck for Borislav Petkov wrote:
> On Tue, Oct 22, 2019 at 11:53:21PM +0200, Borislav Petkov wrote:
> > On Tue, Oct 22, 2019 at 02:29:02PM -0700, speck for Pawan Gupta wrote:
> > > Side effect of RTM check ahead of X86_BUG_TAA will be a dmesg print
> > > "Mitigation: TSX disabled" when X86_BUG_TAA is not set.
> > 
> > So what you're trying to tell me is that you don't want to print that
> > message at all?
> > 
> > 	if (!boot_cpu_has(X86_FEATURE_RTM)) {
> > 		taa_mitigation = TAA_MITIGATION_TSX_DISABLE;
> > 		return;
> > 	}
> > 
> > How's that?
> 
> And to perhaps answer your other question from below maybe - I'm
> assuming you want something like this:
> 
> 	if (!boot_cpu_has(X86_FEATURE_RTM)) {
> 		taa_mitigation = TAA_MITIGATION_TSX_DISABLE;
> 		setup_clear_cpu_bug(X86_BUG_TAA);
> 		return;
> 	}
> 
> This is assuming X86_FEATURE_RTM really mirrors the CPUID bit. And that
> should be the case since if you do tsx_disable(), it will clear the
> CPUID bit through the MSR. IOW, when TSX is disabled, you're basically
> not affected by TAA, of course.

Sysfs mitigations are shown by a common function cpu_show_common() which
checks the X86_BUG_TAA bit. If we clear X86_BUG_TAA, cpu_show_common()
will show "Not affected" in sysfs. This would be when X86_BUG_TAA exists
in the hardware, but was mitigated by disabling TSX. In my opinion
correct output in this case should be "Mitigation: TSX disabled", which
is what it shows now.

I am sorry, I am failing to understand what problem are you trying to
solve by changing the order of X86_FEATURE_RTM and X86_BUG_TAA checks?

> 
> > > > Also, from all the possible settings:
> > > > 
> > > >         [TAA_MITIGATION_OFF]            = "Vulnerable",
> > > >         [TAA_MITIGATION_UCODE_NEEDED]   = "Vulnerable: Clear CPU buffers attempted, no microcode",
> > > >         [TAA_MITIGATION_VERW]           = "Mitigation: Clear CPU buffers",
> > > >         [TAA_MITIGATION_TSX_DISABLE]    = "Mitigation: TSX disabled",
> > > > 
> > > > TAA_MITIGATION_TSX_DISABLE is the one that fits best for the !RTM case,
> > > > no?
> > > 
> > > When X86_BUG_TAA is not set sysfs shows "Not affected" irrespective of
> > > value of taa_mitigation.
> > > 
> > > 	cpu_show_common()
> > > 	{
> > > 		if (!boot_cpu_has_bug(bug))
> > > 			return sprintf(buf, "Not affected\n");
> > > 		[...]
> > 
> > Sorry, I can't follow what you're trying to tell me here.

That when X86_BUG_TAA is not set, no matter what the value
taa_mitigation has, sysfs is always going to say "Not affected". So
the first check taa_select_mitigation() does is whether X86_BUG_TAA is
set. If no, do nothing and return. If yes set the  appropriate
taa_mitigation.

When the user reads the sysfs file for vulnerabilities,
cpu_show_common() checks if the bug is present, if yes display the
mitigation string as per taa_mitigation, if no simply say "Not affected".

Thanks,
Pawan