Re: [PATCH v2 1/4] x86/split_lock: Enumerate #AC exception for split locked access feature

[Eduardo Habkost]

On Fri, Jun 29, 2018 at 06:23:35PM +0200, Thomas Gleixner wrote:
> On Fri, 29 Jun 2018, Dave Hansen wrote:
> > On 06/29/2018 07:33 AM, Fenghua Yu wrote:
> > > +/* Detect feature of #AC for split lock by probing bit 29 in MSR_TEST_CTL. */
> > > +void detect_ac_split_lock(void)
> > > +{
> > > +	u64 val, orig_val;
> > > +	int ret;
> > > +
> > > +	/* Attempt to read the MSR. If the MSR doesn't exist, reading fails. */
> > > +	ret = rdmsrl_safe(MSR_TEST_CTL, &val);
> > > +	if (ret)
> > > +		return;
> > 
> > This is a bit fast and loose with how the feature is detected, which
> > might be OK, but can we call out why we are doing this, please?
> > 
> > Is this MSR not really model-specific?  Is it OK to go poking at it on
> > all x86 variants?  Or, do we at _least_ need a check for Intel cpus in here?
> 
> That definitely needs a vendor check. Also the whole code needs to be
> compiled out if CONFIG_INTEL=n.
> 
> Aside of that this wants to be enumerated. CPUID or MISC_FEATURES and not
> this guess work detection logic. Why do I have to ask for that for every
> other new feature thingy?

Yes, please.  KVM hosts normally expect guests to not touch MSRs
unless we explicitly tell them the MSR is available (normally
through CPUID).  This is important to ensure live migration
between different host kernel versions works reliably.

-- 
Eduardo