Re: [PATCH] x86/split_lock: Enumerate architectural split lock disable bit

[Fenghua Yu <fenghua.yu@intel.com>]

Hi, Dear X86 Maintainers,

On 3/1/23 17:19, Fenghua Yu wrote:
> The December 2022 edition of the Intel Instruction Set Extensions manual
> defined that the split lock disable bit in the IA32_CORE_CAPABILITIES MSR
> is (and retrospectively always has been) architectural.
> 
> Remove all the model specific checks except for Ice Lake variants which are
> still needed because these CPU models do not enumerate presence of the
> IA32_CORE_CAPABILITIES MSR.
> 
> Link: https://lore.kernel.org/lkml/20220701131958.687066-1-fenghua.yu@intel.com/t/#mada243bee0915532a6adef6a9e32d244d1a9aef4
> Originally-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>   arch/x86/kernel/cpu/intel.c | 59 ++++++++++++++-----------------------
>   1 file changed, 22 insertions(+), 37 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 2d7ea5480ec3..d27f8973b58f 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -1266,31 +1266,13 @@ void handle_bus_lock(struct pt_regs *regs)
>   }
>   
>   /*
> - * Bits in the IA32_CORE_CAPABILITIES are not architectural, so they should
> - * only be trusted if it is confirmed that a CPU model implements a
> - * specific feature at a particular bit position.
> - *
> - * The possible driver data field values:
> - *
> - * - 0: CPU models that are known to have the per-core split-lock detection
> - *	feature even though they do not enumerate IA32_CORE_CAPABILITIES.
> - *
> - * - 1: CPU models which may enumerate IA32_CORE_CAPABILITIES and if so use
> - *      bit 5 to enumerate the per-core split-lock detection feature.
> + * CPU models that are known to have the per-core split-lock detection
> + * feature even though they do not enumerate IA32_CORE_CAPABILITIES.
>    */
>   static const struct x86_cpu_id split_lock_cpu_ids[] __initconst = {
> -	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		0),
> -	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_L,		0),
> -	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		0),
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT,	1),
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	1),
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_L,	1),
> -	X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE_L,		1),
> -	X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE,		1),
> -	X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,	1),
> -	X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,		1),
> -	X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L,		1),
> -	X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE,		1),
> +	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,	0),
> +	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_L,	0),
> +	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,	0),
>   	{}
>   };
>   
> @@ -1302,24 +1284,27 @@ static void __init split_lock_setup(struct cpuinfo_x86 *c)
>   	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
>   		return;
>   
> +	/* Check for CPUs that have support but do not enumerate it: */
>   	m = x86_match_cpu(split_lock_cpu_ids);
> -	if (!m)
> -		return;
> +	if (m)
> +		goto supported;
>   
> -	switch (m->driver_data) {
> -	case 0:
> -		break;
> -	case 1:
> -		if (!cpu_has(c, X86_FEATURE_CORE_CAPABILITIES))
> -			return;
> -		rdmsrl(MSR_IA32_CORE_CAPS, ia32_core_caps);
> -		if (!(ia32_core_caps & MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT))
> -			return;
> -		break;
> -	default:
> +	if (!cpu_has(c, X86_FEATURE_CORE_CAPABILITIES))
>   		return;
> -	}
>   
> +	/*
> +	 * Not all bits in MSR_IA32_CORE_CAPS are architectural, but
> +	 * MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT is.  All CPUs that set
> +	 * it have split lock detection.
> +	 */
> +	rdmsrl(MSR_IA32_CORE_CAPS, ia32_core_caps);
> +	if (ia32_core_caps & MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT)
> +		goto supported;
> +
> +	/* CPU is not in the model list and does not have the MSR bit: */
> +	return;
> +
> +supported:
>   	cpu_model_supports_sld = true;
>   	__split_lock_setup();
>   }

Any comment on this patch?

If this patch is in upstream, we won't continuously send model specific 
enumeration patches for future processors.

Thanks.

-Fenghua