linux-kernel - Re: [PATCH v4 07/34] x86/cpuid: Introduce a centralized CPUID data model

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aJ9TbaNMgaplKSbH@google.com>
Date: Fri, 15 Aug 2025 08:34:05 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: "Ahmed S. Darwish" <darwi@...utronix.de>
Cc: Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Andrew Cooper <andrew.cooper3@...rix.com>, "H. Peter Anvin" <hpa@...or.com>, 
	David Woodhouse <dwmw2@...radead.org>, 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 v4 07/34] x86/cpuid: Introduce a centralized CPUID data model

On Fri, Aug 15, 2025, Ahmed S. Darwish wrote:
> + * For a given leaf/subleaf combination, the CPUID table inside @_cpuinfo
> + * contains an array of CPUID output storage entries.  An array of storage
> + * entries is used to accommodate CPUID leaves which produce the same output
> + * format for a large subleaf range.  This is common for CPUID hierarchical
> + * objects enumeration; e.g., CPUID(0x4) and CPUID(0xd).  Check CPUID_LEAF().

IMO, the end result is quite misleading for leaves with "repeated" entries.  0xd
in particular will be bizarre due to its array starting at ECX=2.  E.g.
cpuid_subleaf_index() straight up won't work (without lying) due to hardcoding
'0' as the subleaf.

#define cpuid_subleaf_index(_cpuinfo, _leaf, _idx)			\
({									\
	__cpuid_assert_leaf_has_dynamic_subleaves(_cpuinfo, _leaf);	\
	__cpuid_table_subleaf_idx(&(_cpuinfo)->cpuid, _leaf, 0, _idx);	\
                                                             ^
                                                             |

And then the usage would be similarly bizarre, e.g.

	for (i = XFEATURE_YMM; i < ARRAY_SIZE(xstate_sizes); i++) {
		struct cpuid_xstate_sizes *xs = &xstate_sizes[i];
		struct cpuid_0xd_2 *c = cpuid_subleaf_index(..., 0xD, i - 2);

		...
	}

Even the cases where the array starts at '0' look weird:

	const struct leaf_0x4_0 *regs = cpuid_subleaf_index(c, 0x4, index);

because the code is obviously not grabbing CPUID(0x4).0.

And the subleaf_index naming is also weird, because they're essentially the same
thing, e.g. the SDM refers to "sub-leaf index" for more than just the repeated
cases.

Rather than define the structures names using an explicit starting subleaf, what
if the structures and APIs explicitly reference 'n' as the subleaf?  That would
communicate that the struct represents a repeated subleaf, explicitly tie the API
to that structure, and would provide macro/function names that don't make the
reader tease out the subtle usage of "index".

And then instead of just the array size, capture the start:end of the repeated
subleaf so that the caller doesn't need to manually do the math.

E.g.

	const struct leaf_0x4_n *regs = cpuid_subleaf_n(c, 0x4, index);

	struct cpuid_0xd_n *c = cpuid_subleaf_n(..., 0xD, i);
and

Tangentially related, having to manually specific count=1 to CPUID_LEAF() for
the common case is also ugly.  If a dedicated CPUID_LEAF_N() macro is added to
specificy the start:end of the range, then CPUID_LEAF() can just hardcode the
count to '1'.  E.g.

struct cpuid_leaves {
	CPUID_LEAF(0x0,		 	0);
	CPUID_LEAF(0x1,			0);
	CPUID_LEAF(0x2,		 	0);
	CPUID_LEAF_N(0x4,		0,	7);
	CPUID_LEAF(0xd,			0);
	CPUID_LEAF(0xd,			1);
	CPUID_LEAF_N(0xd,		2,	63);
	CPUID_LEAF(0x16,	 	0);
	CPUID_LEAF(0x80000000,		0);
	CPUID_LEAF(0x80000005,		0);
	CPUID_LEAF(0x80000006,		0);
	CPUID_LEAF_N(0x8000001d,	0,	7);
};