[MODERATED] Re: [PATCH 3/4] V7 more sampling fun 3

On Tue, Apr 14, 2020 at 06:23:38PM +0200, speck for Thomas Gleixner wrote:
> Mark,
> 
> speck for mark gross <[email protected]> writes:
> > On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote:
> >> +	/*
> >> +	 * Check to see if this is one of the MDS_NO systems supporting
> >> +	 * TSX that are only exposed to SRBDS when TSX is enabled.
> >> +	 */
> >> +	ia32_cap = x86_read_arch_cap_msr();
> >> +	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {
> >> +		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
> >> +		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) {
> >> +		srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
> >> +		goto out;
> >> +	}
> >> +
> >> +	if (cpu_mitigations_off() || srbds_off) {
> >> +		if (srbds_mitigation != SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF)
> >> +			srbds_mitigation = SRBDS_MITIGATION_OFF;
> >> +	}
> >> +out:
> > The test for cpu_mitigations_off or srbds off needs to be after out:
> > Otherwise when TSX is off and srbds=off will report the wrong answer.
> 
> If the CPU has SRBDS_IF_TSX and TSX is disabled then the correct
> answer is: Not affected (TSX off)
correct.

> That's what we do with other issues as well. If the CPU is not affected
> then we print this even with mitigation disabled (all or particular).

That is what I'm working toward.  Now that I look at the code the right answer
is to not move the out: lable and nuke the nested if int eht srbds_off branch.
That is whats wrong.

That said I plan to us your if/else construct on the next version and forget
about the goto's.

--mark