Re: [PATCH v5 10/35] x86/lib: Add CPUID(0x1) CPU family and model calculation

On Fri, Sep 05, 2025 at 02:14:50PM +0200, Ahmed S. Darwish wrote:
> The x86 library code provides x86_family() and x86_model().  They take
> raw CPUID(0x1) register output, extract the necessary fields with bitwise
> operations, then calculate the CPU family and model out of that.
> 
> In follow-up work, the x86 subystem will use parsed CPUID access, along

Unknown word [subystem] in commit message.
Suggestions: ['subsystem'...

Please introduce a spellchecker into your patch creation workflow.

> with its auto-generated <asm/cpuid/leaf_types.h> CPUID leaf data types,
> instead of direct CPUID access.
> 
> Introduce x86 family and model calculation functions to x86/lib that take
> such auto-generateds 'struct leaf_0x1_0' data types.  Refactor the

Unknown word [auto-generateds] in commit message.

> original bitwise operations code so that no logic is duplicated.
> 
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> ---
>  arch/x86/include/asm/cpu.h |  6 ++++++
>  arch/x86/lib/cpu.c         | 41 ++++++++++++++++++++++----------------
>  2 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index ad235dda1ded..90902cd91335 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -7,7 +7,9 @@
>  #include <linux/topology.h>
>  #include <linux/nodemask.h>
>  #include <linux/percpu.h>
> +
>  #include <asm/ibt.h>
> +#include <asm/cpuid/leaf_types.h>
>  
>  #ifndef CONFIG_SMP
>  #define cpu_physical_id(cpu)			boot_cpu_physical_apicid
> @@ -25,6 +27,10 @@ int mwait_usable(const struct cpuinfo_x86 *);
>  unsigned int x86_family(unsigned int sig);
>  unsigned int x86_model(unsigned int sig);
>  unsigned int x86_stepping(unsigned int sig);
> +
> +unsigned int cpuid_family(const struct leaf_0x1_0 *l);
> +unsigned int cpuid_model(const struct leaf_0x1_0 *l);
> +
>  #ifdef CONFIG_X86_BUS_LOCK_DETECT
>  extern void __init sld_setup(struct cpuinfo_x86 *c);
>  extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
> diff --git a/arch/x86/lib/cpu.c b/arch/x86/lib/cpu.c
> index 7ad68917a51e..eac217d637ac 100644
> --- a/arch/x86/lib/cpu.c
> +++ b/arch/x86/lib/cpu.c
> @@ -1,36 +1,43 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  #include <linux/types.h>
>  #include <linux/export.h>
> +
>  #include <asm/cpu.h>
> +#include <asm/cpuid/leaf_types.h>
>  
> -unsigned int x86_family(unsigned int sig)
> +static unsigned int __x86_family(unsigned int base_fam, unsigned int ext_fam)
>  {
> -	unsigned int x86;
> -
> -	x86 = (sig >> 8) & 0xf;
> +	return (base_fam == 0xf) ? base_fam + ext_fam : base_fam;

Please keep the old simpler code:

	if (base_fam == 0xf)
		base_fam += ext_fam;

	return base_fam;


This is way easier to parse than a ternary expression oneliner.

Ditto below.

> +}
>  
> -	if (x86 == 0xf)
> -		x86 += (sig >> 20) & 0xff;
> +static unsigned int
> +__x86_model(unsigned int family, unsigned int base_model, unsigned int ext_model)
> +{
> +	return (family >= 0x6) ? base_model | ext_model << 4 : base_model;
> +}

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette