[MODERATED] Re: [PATCH 2/3] V5 more sampling fun 2

On Mon, Apr 06, 2020 at 05:34:56PM -0700, speck for mark gross wrote:
> On Mon, Apr 06, 2020 at 05:07:14PM -0500, speck for Josh Poimboeuf 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) {
> > > +		if (!boot_cpu_has(X86_FEATURE_RTM))
> > > +			srbds_mitigation = SRBDS_NOT_AFFECTED_TSX_OFF;
> > > +	}
> > 
> > A 'goto out' would be helpful here; then the TSX_OFF checks below aren't
> > needed and the flow is simplified.
> a goto out would mess up the hypervisor check but, I'll add the goto for the
> mid function returns that set the srbds_mitigation value.

Just to clarify, I was thinking something like:

	ia32_cap = x86_read_arch_cap_msr();
	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {
		srbds_mitigation = SRBDS_NOT_AFFECTED_TSX_OFF;
		goto out;
	}

As far as I can tell, that doesn't mess up the hypervisor check, since
it only sets SRBDS_HYPERVISOR if TSX_OFF isn't set.

> 
> > > +
> > > +	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
> > > +		if (srbds_mitigation != SRBDS_NOT_AFFECTED_TSX_OFF)
> > > +			srbds_mitigation = SRBDS_HYPERVISOR;
> > > +		return;
> > > +	}
> > > +
> > > +/*
> > > + * List affected CPU's for issues that cannot be enumerated.
> > > + */
> > 
> > I don't understand the comment, SRBDS seems to be enumerated above.
> hmm, how about I remove the comment? 

Sounds good to me.

-- 
Josh