On Mon, Mar 04, 2019 at 10:58:01AM -0800, Dave Hansen wrote:
> > * Clear/Set all flags overridden by options, after probe.
> > diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
> > b/arch/x86/kernel/cpu/cpuid-deps.c
> > index 2c0bd38a44ab..5ba11ce99f92 100644
> > --- a/arch/x86/kernel/cpu/cpuid-deps.c
> > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > @@ -59,6 +59,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> > { X86_FEATURE_AVX512_4VNNIW, X86_FEATURE_AVX512F },
> > { X86_FEATURE_AVX512_4FMAPS, X86_FEATURE_AVX512F },
> > { X86_FEATURE_AVX512_VPOPCNTDQ, X86_FEATURE_AVX512F },
> > + { X86_FEATURE_SPLIT_LOCK_DETECT, X86_FEATURE_CORE_CAPABILITY},
> > {}
> > };
>
> Please reindent the rest of the structure whenever you break the record
> for name length.
Ok. Will do it.
>
> > diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> > index fc3c07fe7df5..0c44c49f6005 100644
> > --- a/arch/x86/kernel/cpu/intel.c
> > +++ b/arch/x86/kernel/cpu/intel.c
> > @@ -1029,3 +1029,24 @@ static const struct cpu_dev intel_cpu_dev = {
> >
> > cpu_dev_register(intel_cpu_dev);
> >
> > +/**
> > + * init_core_capability - enumerate features supported in
> > IA32_CORE_CAPABILITY
> > + * @c: pointer to cpuinfo_x86
> > + *
> > + * Return: void
> > + */
> > +void init_core_capability(struct cpuinfo_x86 *c)
> > +{
> > + /*
> > + * If MSR_IA32_CORE_CAPABILITY exists, enumerate features that are
> > + * reported in the MSR.
> > + */
> > + if (c == &boot_cpu_data && cpu_has(c, X86_FEATURE_CORE_CAPABILITY)) {
>
> First of all, please endeavor to leave the main flow of the function at
> the first indent.
>
> *If* you were to do this, it would look like:
>
>
> if (c != &boot_cpu_data)
> return;
> if (!cpu_has(c, X86_FEATURE_CORE_CAPABILITY))
> return;
>
> rdmsrl(...);
Ok. Will do it.
>
> But, it goes unmentioned why you do the boot-cpu-only restriction here.
> It doesn't match the behavior of the two functions called before
> init_core_capability():
>
> init_scattered_cpuid_features(c);
> init_speculation_control(c);
>
> So why is this new function special?
The function sets the split_lock_detect feature which needs to be
applied before BSP calls apply_enforced_caps() in get_cpu_cap(). And I
want to enable split lock detection in earlier phase to detect split
lock earlier.
Thanks.
-Fenghua
