lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260116203117.GAaWqgFYTv1bHLxKV_@fat_crate.local>
Date: Fri, 16 Jan 2026 21:31:17 +0100
From: Borislav Petkov <bp@...en8.de>
To: "Ahmed S. Darwish" <darwi@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Sean Christopherson <seanjc@...gle.com>,
	David Woodhouse <dwmw2@...radead.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Sohil Mehta <sohil.mehta@...el.com>,
	John Ogness <john.ogness@...utronix.de>, x86@...nel.org,
	x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 07/35] x86: Introduce a centralized CPUID data model

On Fri, Sep 05, 2025 at 02:14:47PM +0200, Ahmed S. Darwish wrote:
>     const struct leaf_0x4_n *l4_0, *l4_1, l4_2;
> 
>     l4_0 = cpuid_subleaf_n(c, 0x4, 0);
>                            |   |   └──────────┐
>                            |   └─────────┐    |
>                            *             *    v
>                           &c.cpuid.leaf_0x4_n[0]
> 
>     l4_1 = cpuid_subleaf_n(c, 0x4, 1);
>                            |   |   └──────────┐
>                            |   └─────────┐    |
>                            *             *    v
>                           &c.cpuid.leaf_0x4_n[1]
> 
>     l4_2 = cpuid_subleaf_n(c, 0x4, 2);
>                            |   |   └──────────┐
>                            |   └─────────┐    |
>                            *             *    v
>                           &c.cpuid.leaf_0x4_n[2]
> 
> where dynamic leaf types are marked by their "_n" suffix and the indices

Why isn't the suffix "_dyn" for dynamic leaf types? Then it'll be right there
in the name what it is...

Btw, why are we calling them dynamic?

This is confusing. Those leafs simply have multiple subleafs specified in ECX.
Let's please not invent our own things here but simply stick to the
nomenclature in the vendor docs.

This is a very simple explanation IMO:

"The information is accessed by (1) selecting the CPUID function setting EAX
and optionally ECX for some functions,"

and there's no talk about dynamic and whatnot.

> 0, 1, 2 above can be passed dynamically.  This is by design: hierarchical
> CPUID enumeration usually passes the CPUID subleaf dynamically; e.g.,
> within a for loop.
> 
> For each of the CPUID leaf/subleaf output storage entries, attach a
> 'struct leaf_query_info' instance.  It is to be set by the CPUID parser
> while filling the CPUID tables.  For now, this info structure has one
> element: the number of filled slots at the respective output storage
> array.
> 
> ** Call-site APIs
> 
> Introduce below APIs for CPUID leaves with static subleaves:
> 
>     cpuid_leaf(_cpuinfo, _leaf)
>     cpuid_leaf_raw(_cpuinfo, _leaf)
>     cpuid_subleaf(_cpuinfo, _leaf, _subleaf)
> 
> and below APIs for CPUID leaves with dynamic subleaves:
> 
>     cpuid_subleaf_n(_cpuinfo, _leaf, _idx)
>     cpuid_subleaf_n_raw(_cpuinfo, _leaf, _idx)
>     cpuid_subleaf_count(_cpuinfo, _leaf)
>
> At <asm/cpuid/api.h>, add a clear rationale for why call sites should use
> the above APIs instead of directly invoking CPUID queries.

I appreciate the long and detailed explanation but pls refrain from explaining
the patch. Explaining the "why" is perfectly fine.

> ** Next steps
> 
> For now, define entries for CPUID(0x0) and CPUID(0x1) in the CPUID table.
> 
> Generic CPUID parser logic to fill the CPUID tables, along with more
> CPUID leaves support, will be added next.
> 
> Suggested-by: Thomas Gleixner <tglx@...utronix.de>	# CPUID data model
> Suggested-by: Andrew Cooper <andrew.cooper3@...rix.com>	# x86-cpuid-db schema
> Suggested-by: Borislav Petkov <bp@...en8.de>		# Early CPUID centralization drafts
> Suggested-by: Ingo Molnar <mingo@...nel.org>		# CPUID APIs restructuring
> Suggested-by: Sean Christopherson <seanjc@...gle.com>	# Dynamic leaves CPUID API
> Signed-off-by: Ahmed S. Darwish <darwi@...utronix.de>
> Link: https://lore.kernel.org/lkml/874ixernra.ffs@tglx
> Link: https://gitlab.com/x86-cpuid.org/x86-cpuid-db
> Link: https://lore.kernel.org/lkml/aBnSgu_JyEi8fvog@gmail.com
> Link: https://lore.kernel.org/lkml/aJ9TbaNMgaplKSbH@google.com
> ---
>  arch/x86/include/asm/cpuid/api.h   | 273 +++++++++++++++++++++++++++++
>  arch/x86/include/asm/cpuid/types.h | 125 +++++++++++++
>  arch/x86/include/asm/processor.h   |   2 +
>  3 files changed, 400 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
> index 2b9750cc8a75..dbe94c8c4900 100644
> --- a/arch/x86/include/asm/cpuid/api.h
> +++ b/arch/x86/include/asm/cpuid/api.h
> @@ -289,4 +289,277 @@ static inline bool cpuid_amd_hygon_has_l3_cache(void)
>  	return cpuid_edx(0x80000006);
>  }
>  
> +/*
> + * 'struct cpuid_leaves' accessors (without sanity checks):
> + *
> + * For internal use by the CPUID parser.
> + */
> +
> +/*
> + * Return constified pointers for all call-site APIs
> + */
> +#define __const_ptr(_ptr)							\
> +	((const __typeof__(*(_ptr)) *)(_ptr))
> +
> +/*
> + * __cpuid_leaves_subleaf() - Get parsed CPUID output (without sanity checks)

Above says already "without sanity checks".

> + * @_leaves:	&struct cpuid_leaves instance
> + * @_leaf:	CPUID leaf, in compile-time 0xN format
> + * @_subleaf:	CPUID subleaf, in compile-time decimal format
> + */
> +#define __cpuid_leaves_subleaf(_leaves, _leaf, _subleaf)			\
> +	__const_ptr(&((_leaves)->leaf_ ## _leaf ## _ ## _subleaf)[0])
> +
> +/*
> + * __cpuid_leaves_subleaf_n() - Get parsed CPUID output for dynamic subleaf (without checks)

Ditto.

> + * @_leaves:	&struct cpuid_leaves instance
> + * @_leaf:	CPUID leaf, in compile-time 0xN format
> + * @_index:	Index within the dynamic subleaf storage array
> + */
> +#define __cpuid_leaves_subleaf_n(_leaves, _leaf, _index)			\
> +	__const_ptr(&((_leaves)->leaf_ ## _leaf ## _ ## n)[_index])
> +
> +/*
> + * __cpuid_leaves_subleaf_info() - Get CPUID query info for @_leaf/@...bleaf
> + * @_leaves:	&struct cpuid_leaves instance
> + * @_leaf:	CPUID leaf, in compile-time 0xN format
> + * @_subleaf:	CPUID subleaf, in compile-time decimal format, or just 'n' for
> + *		leaves with a dynamic subleaf range.
> + */
> +#define __cpuid_leaves_subleaf_info(_leaves, _leaf, _subleaf)			\
> +	__const_ptr(&((_leaves)->leaf_ ## _leaf ## _ ## _subleaf ## _ ## info))
> +
> +/*
> + * 'struct cpuid_table' accessors (with sanity checks):
> + *
> + * For internal use by the CPUID parser.
> + */
> +
> +#define __cpuid_table_nr_filled_subleaves(_table, _leaf, _subleaf)				\
> +	__cpuid_leaves_subleaf_info(&((_table)->leaves), _leaf, _subleaf)->nr_entries
> +
> +#define __cpuid_table_dynamic_subleaf_storage(_table, _leaf)					\

That "storage" thing reads weird. We usually call those "size".

> +	ARRAY_SIZE((_table)->leaves.leaf_ ## _leaf ## _n)
> +
> +#define __cpuid_table_invalid_dynamic_subleaf(_table, _leaf, _subleaf)				\
> +	(((_subleaf) < (__cpuid_leaf_first_dynamic_subleaf(_leaf))) ||				\
> +	 ((_subleaf) > (__cpuid_leaf_first_dynamic_subleaf(_leaf) +				\
> +			__cpuid_table_dynamic_subleaf_storage(_table, _leaf) - 1)))

...

> +/**
> + * cpuid_subleaf() - Access parsed CPUID
> + * @_cpuinfo:	CPU capability structure reference ('struct cpuinfo_x86')
> + * @_leaf:	CPUID leaf, in compile-time 0xN format; e.g. 0x7, 0xf
> + * @_subleaf:	CPUID subleaf, in compile-time decimal format; e.g. 0, 1, 3

"compile-time" ... format?

> + *
> + * Returns a pointer to parsed CPUID output, from the CPUID table inside
> + * @_cpuinfo, as a <cpuid/leaf_types.h> data type: 'struct leaf_0xM_N', where
> + * 0xM is the token provided at @_leaf, and N is the token provided at
> + * @_subleaf; e.g. struct leaf_0x7_0.
> + *
> + * Returns NULL if the requested CPUID @_leaf/@...bleaf query output is not
> + * present at the parsed CPUID table inside @_cpuinfo.  This can happen if:
> + *
> + * - The CPUID table inside @_cpuinfo has not yet been populated.
> + * - The CPUID table inside @_cpuinfo was populated, but the CPU does not
> + *   implement the requested CPUID @_leaf/@...bleaf combination.
> + * - The CPUID table inside @_cpuinfo was populated, but the kernel's CPUID
> + *   parser has predetermined that the requested CPUID @_leaf/@...bleaf

"determined" is just fine.

> + *   hardware output is invalid or unsupported.
> + *
> + * Example usage::
> + *
> + *	const struct leaf_0x7_0 *l7_0 = cpuid_subleaf(c, 0x7, 0);
> + *	if (!l7_0) {
> + *		// Handle error
> + *	}
> + *
> + *	const struct leaf_0x7_1 *l7_1 = cpuid_subleaf(c, 0x7, 1);
> + *	if (!l7_1) {
> + *		// Handle error
> + *	}

Good.

> + */
> +#define cpuid_subleaf(_cpuinfo, _leaf, _subleaf)				\
> +	__cpuid_table_subleaf(&(_cpuinfo)->cpuid, _leaf, _subleaf)		\
> +

...

> +/*
> + * Types for centralized CPUID tables:
> + *
> + * For internal use by the CPUID parser.
> + */
> +
> +/**
> + * struct leaf_query_info - Parse info for a CPUID leaf/subleaf query

Why not simply "leaf_info"? "query" is superfluous here.

> + * @nr_entries:	Number of valid output storage entries filled by the CPUID parser
> + *
> + * In a CPUID table (struct cpuid_leaves), each CPUID leaf/subleaf query output
> + * storage entry from <cpuid/leaf_types.h> is paired with a unique instance of

Oh boy, a "CPUID leaf query output storage entry".

Or simply a CPUID leaf. No?

Let's tone down on the unnecessary words in that whole area pls. Let's keep it
simple first and then we'll make it more complicated when needed.

> + * this type.
> + */
> +struct leaf_query_info {
> +	unsigned int		nr_entries;
> +};
> +
> +/**
> + * __CPUID_LEAF() - Define a CPUID output storage and query info entry
> + * @_name:	Struct type name of the CPUID leaf/subleaf (e.g. 'leaf_0x7_0'). Such
> + *		types are defined at <cpuid/leaf_types.h> and follow the leaf_0xM_N
> + *		format, where 0xM is the leaf and N is the subleaf.  If N is 'n' instead
> + *		of a decimal literal, then this storage entry is for a "dynamic" leaf.
> + * @_count:	Number of storage entries to allocate for this leaf/subleaf.  Static
> + *		leaves need only one entry, while dynamic leaves require more.

This is where the problem lies: you're calling static leaves those which have
one subleaf and dynamic those which have multiple.

But if one "static" leaf starts adding subleafs, the "static" one becomes
"dynamic". And that's confusing. Nothing dynamic about it. You simply have
CPUID leafs with 1 or more subleafs. And that should be the nomenclature we
use.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ