2019-11-21 01:48:24

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v10 4/6] x86/split_lock: Enumerate split lock detection if the IA32_CORE_CAPABILITIES MSR is not supported

Architecturally the split lock detection feature is enumerated by
IA32_CORE_CAPABILITIES MSR and future CPU models will indicate presence
of the feature by setting bit 5. But the feature is present in a few
older models where split lock detection is enumerated by the CPU models.

Use a "x86_cpu_id" table to list the older CPU models with the feature.

Signed-off-by: Fenghua Yu <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
---
arch/x86/kernel/cpu/intel.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index ce87e2c68767..2614616fb6d3 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -19,6 +19,7 @@
#include <asm/microcode_intel.h>
#include <asm/hwcap2.h>
#include <asm/elf.h>
+#include <asm/cpu_device_id.h>

#ifdef CONFIG_X86_64
#include <linux/topology.h>
@@ -1034,15 +1035,31 @@ static void __init split_lock_setup(void)
setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT);
}

+#define SPLIT_LOCK_CPU(model) {X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY}
+
+/*
+ * The following processors have split lock detection feature. But since they
+ * don't have MSR IA32_CORE_CAPABILITIES, the feature cannot be enumerated by
+ * the MSR. So enumerate the feature by family and model on these processors.
+ */
+static const struct x86_cpu_id split_lock_cpu_ids[] __initconst = {
+ SPLIT_LOCK_CPU(INTEL_FAM6_ICELAKE_X),
+ SPLIT_LOCK_CPU(INTEL_FAM6_ICELAKE_L),
+ {}
+};
+
void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
{
u64 ia32_core_caps = 0;

- if (!cpu_has(c, X86_FEATURE_CORE_CAPABILITIES))
- return;
-
- /* Enumerate features reported in IA32_CORE_CAPABILITIES MSR. */
- rdmsrl(MSR_IA32_CORE_CAPABILITIES, ia32_core_caps);
+ if (cpu_has(c, X86_FEATURE_CORE_CAPABILITIES)) {
+ /* Enumerate features reported in IA32_CORE_CAPABILITIES MSR. */
+ rdmsrl(MSR_IA32_CORE_CAPABILITIES, ia32_core_caps);
+ } else {
+ /* Enumerate split lock detection by family and model. */
+ if (x86_match_cpu(split_lock_cpu_ids))
+ ia32_core_caps |= MSR_IA32_CORE_CAPABILITIES_SPLIT_LOCK_DETECT;
+ }

if (ia32_core_caps & MSR_IA32_CORE_CAPABILITIES_SPLIT_LOCK_DETECT)
split_lock_setup();
--
2.19.1


2019-11-21 22:12:53

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v10 4/6] x86/split_lock: Enumerate split lock detection if the IA32_CORE_CAPABILITIES MSR is not supported



> On Nov 20, 2019, at 5:45 PM, Fenghua Yu <[email protected]> wrote:
>
> Architecturally the split lock detection feature is enumerated by
> IA32_CORE_CAPABILITIES MSR and future CPU models will indicate presence
> of the feature by setting bit 5. But the feature is present in a few
> older models where split lock detection is enumerated by the CPU models.
>
> Use a "x86_cpu_id" table to list the older CPU models with the feature.
>

This may need to be disabled if the HYPERVISOR bit is set.

2019-11-22 00:28:10

by Fenghua Yu

[permalink] [raw]
Subject: Re: [PATCH v10 4/6] x86/split_lock: Enumerate split lock detection if the IA32_CORE_CAPABILITIES MSR is not supported

On Thu, Nov 21, 2019 at 02:07:38PM -0800, Andy Lutomirski wrote:
>
>
> > On Nov 20, 2019, at 5:45 PM, Fenghua Yu <[email protected]> wrote:
> >
> > Architecturally the split lock detection feature is enumerated by
> > IA32_CORE_CAPABILITIES MSR and future CPU models will indicate presence
> > of the feature by setting bit 5. But the feature is present in a few
> > older models where split lock detection is enumerated by the CPU models.
> >
> > Use a "x86_cpu_id" table to list the older CPU models with the feature.
> >
>
> This may need to be disabled if the HYPERVISOR bit is set.

How about just keeping this patch set as basic enabling code and
keep HYPERVISOR out of scope as of now? KVM folks will have better
handling of split lock in KVM once this patch set is available in
the kernel.

Thanks.

-Fenghua

2019-11-22 02:15:49

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v10 4/6] x86/split_lock: Enumerate split lock detection if the IA32_CORE_CAPABILITIES MSR is not supported


> On Nov 21, 2019, at 4:25 PM, Fenghua Yu <[email protected]> wrote:
>
> On Thu, Nov 21, 2019 at 02:07:38PM -0800, Andy Lutomirski wrote:
>>
>>
>>>> On Nov 20, 2019, at 5:45 PM, Fenghua Yu <[email protected]> wrote:
>>>
>>> Architecturally the split lock detection feature is enumerated by
>>> IA32_CORE_CAPABILITIES MSR and future CPU models will indicate presence
>>> of the feature by setting bit 5. But the feature is present in a few
>>> older models where split lock detection is enumerated by the CPU models.
>>>
>>> Use a "x86_cpu_id" table to list the older CPU models with the feature.
>>>
>>
>> This may need to be disabled if the HYPERVISOR bit is set.
>
> How about just keeping this patch set as basic enabling code and
> keep HYPERVISOR out of scope as of now? KVM folks will have better
> handling of split lock in KVM once this patch set is available in
> the kernel.
>
>

You seem to be assuming that certain model CPUs have this feature even if not enumerated. You need to make sure you don’t try to use it in a VM without the hypervisor giving you an indication that it’s available and permitted. My suggestion is to disable model-based enumeration if HYPERVISOR is set. You should also consider probing the MSR to double check even if you don’t think you have a hypervisor.

2019-11-22 09:51:43

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v10 4/6] x86/split_lock: Enumerate split lock detection if the IA32_CORE_CAPABILITIES MSR is not supported

On Thu, Nov 21, 2019 at 06:13:18PM -0800, Andy Lutomirski wrote:

> You seem to be assuming that certain model CPUs have this feature even
> if not enumerated. You need to make sure you don’t try to use it in a
> VM without the hypervisor giving you an indication that it’s available
> and permitted. My suggestion is to disable model-based enumeration if
> HYPERVISOR is set. You should also consider probing the MSR to double
> check even if you don’t think you have a hypervisor.

Yep, in patch 6 this results in an unconditinoal WRMSR, which, when ran
under a HV, will explode most mighty.

He doesn't double check, doesn't use wrmsrl_safe()...