Re: [PATCH v3 08/10] x86/setcpuid: Add kernel option setcpuid

From: Fenghua Yu
Date: Mon Feb 11 2019 - 14:23:09 EST


On Sun, Feb 10, 2019 at 08:20:20PM +0100, Thomas Gleixner wrote:
> On Tue, 5 Feb 2019, Peter Zijlstra wrote:
> > On Tue, Feb 05, 2019 at 07:19:16AM -0800, Dave Hansen wrote:
> > > On 2/5/19 12:48 AM, Peter Zijlstra wrote:
> > > This isn't something we want everybody and their grandma to turn on;
> > > it's a rather specialized feature. It's really only for folks that care
> > > about the latency incurred across the entire system on split lock
> > > operations.
> >
> > That really should be everyone. That split lock stuff is horrible. There
> > is no real down-side to having it always enabled. Code that breaks is
> > bad code you want fixed anyway.
> >
> > Like I said elsewhere, I wish it would #AC for any unaligned LOCK
> > prefix, not just cross-line. I see why we'd not want to traditional RISC
> > #AC for every load/store, but atomics really had better be aligned.
>
> Right, we should really make this default enabled.

Yes, I agree.

>
> > > > Is this some transient state; where a few (early) models will not have
> > > > the enumeration sorted but all later models will have it all neat and
> > > > tidy?
> > >
> > > From my understanding, it's not just an early stepping. It's a
> > > generational thing. The current generation lacks the enumeration and
> > > the next generation will get it. Both have the silicon to implement the
> > > feature itself.
> >
> > I never said stepping, in fact I explicitly said model.
> >
> > > > If so, we can easily do the FMS solution for this.
> > >
> > > Yeah, we can. I honestly forget why we didn't do FMS. :)
> >
> > Right so FMS is fairly horrible; but when it is a stop-gap for a limited
> > number of models it's waaay better than dodgy cmdline things.
>
> One or two is fine. And _IF_ we get the enumeration sorted before we merge
> that, then we can declare the FM list as immutable :)

There will be about 8 models that have the split lock feature but
don't have the IA32_CORE_CAPABILITY enumeration. The models will NOT
have the IA32_CORE_CAPABILITY enumerate in the future as planned. All
of other models than these ones will have the IA32_CORE_CAPABILITY
enumeration.

I don't see any of the model numbers are public yet as of now.

In the next version of patches, I will do the following changes from
v3:

1. Remove patch #11 that implements "setcpuid=".
2. But I will not implement the patch that enables the split lock feature
based on FMS because there is no public FMS numbers of those models. The
patch/patches will be implemented only after the FMS numbers are public.
3. The new patches enables the feature once it enumerates the feature
by bit 5 in IA32_CORE_CAPABILITY.
4. The feature can be disabled by kernel option
"clearcpuid=split_lock_detection" during early boot time.
5. The feature can be disabled/enabled during run time by
sysfs interface "/sys/kernel/split_lock_detection"

Does that make sense?

Thanks.

-Fenghua