Re: [PATCH v1 04/26] x86/cpuid: Introduce centralized CPUID data

On 5/5/2025 10:04 PM, Ahmed S. Darwish wrote:
> Define the CPUID_LEAF() macro for building up centralized CPUID data.
> It automates defining a CPUID data repository in the form:
> 
>     struct cpuid_leaves {
> 	struct leaf_0x0_0	leaf_0x0_0[1];
> 	struct leaf_query_info	leaf_0x0_0_info;
> 	struct leaf_0x1_0	leaf_0x1_0[1];
> 	struct leaf_query_info	leaf_0x0_0_info;
> 
> 	struct leaf_0x4_0	leaf_0x4_0[8];
> 	struct leaf_query_info	leaf_0x4_0_info;
> 	...
>     };
> 
> where for each 'struct leaf_0xN_M', N is the leaf number and M is the
> subleaf.
> 

I am finding the structure names a bit confusing. Can we make it
slightly more descriptive since they are directly used in common code?

How about struct leaf_0xN_sl_M or struct leaf_0xN_subl_M?

The actual struct names would be:
leaf_0x1_sl_0 or leaf_0x1_subl_0
leaf_0x4_sl_0 or leaf_0x4_subl_0

The variable names can obviously be simpler based on usage and context.

> The complete C99 bitfield listings of the 'leaf_0xN_M' structures is auto
> generated by the x86-cpuid-db project and is merged in parent commits at
> <asm/cpuid/leaves.h>.  This avoids using ugly bitwise operations on CPUID
> register output.
> 
> Let the CPUID_LEAF() macro generate an array of output storage entries
> for each leaf/subleaf combination.  An array is used to accommodate
> leaves which produce the same output format for a large subleaf range,
> which is typical for CPUID leaves enumerating hierarchical objects;
> e.g. leaf 0x4 cache topology enumeration, leaf 0xd XSAVE enumeration, and
> leaf 0x12 SGX Enclave Page Cache enumeration.  In the CPUID table snippet
> above, leaf 0x4 has 8 storage entries.
> 
> For each of the leaf/subleaf entries in the CPUID table, attach a
> 'leaf_query_info' leaf_0xN_M_info structure.  It is to be filled by the
> generic scanning logic filling the CPUID table.  For now, that info
> structure has one element: the number of filled slots in the leaf/subleaf
> storage array.
> 
> Define 'struct cpuid_table' for representing the actual CPUID table, and
> embed in it a 'struct cpuid_leaves' instance.  This way, global table
> data can be later added.
> 
> Embed an instance of that 'struct cpuid_table' in the CPU capability
> structure 'struct cpuinfo_x86'.  This way, centralized CPUID data can be
> accessed on early boot (through boot_cpu_data) and later on a per-CPU
> basis through the 'cpu_info' per-CPU capability structures.  Since
> multiple code paths dealing with 'struct cpuinfo_x86' assume that it has
> no embedded pointers, embedding the cpuid_table instance avoids creating
> special cases for no apparent benefit.
> 
> Define entries for leaf 0x0 and leaf 0x1 in the CPUID table.  Next
> commits will add generic scanning logic for filling the CPUID data.
> 

Avoid using "next commits". How about:

Generic scanning logic for filling the CPUID data will be added later.

> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Ahmed S. Darwish <[email protected]>