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: <aKN0debsio7ocitW@google.com>
Date: Mon, 18 Aug 2025 11:44: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 Mon, Aug 18, 2025, Ahmed S. Darwish wrote:
> > 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);
> Hard case: Subleaves start repeating from subleaf > 0
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This would be the CPUID leaves:
> 
>     x86-cpuid-db/db/xml (tip)> git grep 'id="[1-9][0-9]*" array='
> 
>     leaf_0d.xml:    <subleaf id="2" array="62">
>     leaf_10.xml:    <subleaf id="1" array="2">
>     leaf_12.xml:    <subleaf id="2" array="30">
>     leaf_17.xml:    <subleaf id="1" array="3">
> 
> For something like CPUID(0xd), I cannot just blindly define a 'struct
> cpuid_0xd_n' data type.

Why not?  Like C structs, there can only be one variable sized array, i.e. there
can't be multiple "n" subleafs.  If the concern is calling __cpuid_subleaf_n()
with i < start, then I don't see how embedding start in the structure name helps
in any way, since 'i' isn't a compile-time constant and so needs to be checked at
runtime no matter what.

> We already have:
> 
>     struct leaf_0xd_0 { ... };
>     struct leaf_0xd_1 { ... };
>     struct leaf_0xd_2 { ... };
> 
> and they all have different bitfields.  A similar case exist for
> CPUID(0x10), CPUID(0x12), and CPUID(0x17).
> 
> But, we can still have:
> 
>     struct leaf_0xd_0	{ ... };
>     struct leaf_0xd_1	{ ... };
>     struct leaf_0xd_2_n	{ ... };
> 

...

> And the aforementioned KVM snippet would be:
> 
>     const struct leaf_0xd_2_n *l;
> 
>     for (int i = 0; i < ARRAY_SIZE(xstate_sizes) - XFEATURE_YMM; i++) {
>         l = __cpuid_subleaf_n(c, 0xd, 2, i);

IMO, this is still ugly and confusing.