[MODERATED] Re: [PATCH v4 03/10] TAAv4 3

On Thu, Sep 26, 2019 at 08:54:09AM -0500, speck for Josh Poimboeuf wrote:
> On Thu, Sep 26, 2019 at 12:15:55AM -0700, speck for Pawan Gupta wrote:
> > > Regardless, shouldn't tsx_init() at least be called before
> > > cpu_set_bug_bits(), so that X86_BUG_TAA can get set appropriately?
> > 
> > X86_BUG_TAA should be set even when tsx_init() disables TSX. This is to
> > indicate that the CPU has the bug but was mitigated by disabling TSX.
> > That is why we are calling cpu_set_bug_bits() first to set X86_BUG_TAA
> > and then disabling TSX in tsx_init(). Moreover cpu_set_bug_bits() is
> > called only by the boot-cpu, and tsx_init() is called per-cpu to disable
> > TSX on each CPU.
> 
> Hm, I guess that makes sense.  But what if TSX enumeration is disabled
> due to a previous kexec?  Then the CPUID feature bit (X86_FEATURE_RTM)
> won't be set, and thus X86_BUG_TAA won't get set, right?

That is correct.

> > > If early_init_intel() isn't the right spot, then maybe
> > > early_identify_cpu().
> > 
> > early_identify_cpu() is called for other vendors as well, I think
> > init_intel() is the ideal place for calling an Intel specific function.
> > Why do we want to move tsx_init() from init_intel()?
> 
> IIUC, the fact that the X86_FEATURE_RTM enumeration can be removed
> before kexec means that tsx_init() needs to run early, so that it can
> properly detect the enumeration so that X86_BUG_TAA can get set
> properly.
> 
> Since tsx_init() can clear X86_FEATURE_RTM even though the CPU supports
> it, cpu_set_bug_bits() could check TSX_CTRL_NOT_SUPPORTED instead of
> X86_FEATURE_RTM, like:
> 
> 	if (cpu_has_tsx() && !(ia32_cap & ARCH_CAP_TAA_NO))
> 		setup_force_cpu_bug(X86_BUG_TAA);
> 
> 
> where cpu_has_tsx() is just !TSX_CTRL_NOT_SUPPORTED.

Yes we need something like this to address kexec like cases where TSX is
disabled before the kernel boot. We can probably add a check for
TSX_CTRL in cpu_set_bug_bits() as well.

cpu_set_bug_bits()
{
[...]
	/*
	 * When processor is not mitigated for TAA (TAA_NO=0) set TAA bug when:
	 *	- TSX is supported or
	 *	- TSX_CTRL is supported
	 *
	 * TSX_CTRL check is needed for cases when TSX could be disabled before
	 * the kernel boot e.g. kexec
	 * TSX_CTRL check alone is not sufficient for cases when the microcode
	 * update is not present; running as guest that don't get TSX_CTRL.
	 */
	if ((boot_cpu_has(X86_FEATURE_RTM) || (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)) &&
	    !(ia32_cap & ARCH_CAP_TAA_NO))
		setup_force_cpu_bug(X86_BUG_TAA);

Thanks,
Pawan