linux-kernel - Re: [PATCH 11/11] x86/cpu: Make all all CPUID leaf names consistent

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: <Z1cajm9oqRZWDp_4@google.com>
Date: Mon, 9 Dec 2024 08:27:58 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, 
	linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de, 
	rafael@...nel.org, lenb@...nel.org
Subject: Re: [PATCH 11/11] x86/cpu: Make all all CPUID leaf names consistent

On Fri, Dec 06, 2024, Dave Hansen wrote:
> On 11/29/24 10:27, Borislav Petkov wrote:
> > Well, enum cpuid_leafs as it is now is the *indices* into the cap flags array:
> > 
> > struct cpuinfo_x86 {
> > 
> > 	...
> > 
> > 	__u32           x86_capability[NCAPINTS + NBUGINTS];
> > 
> > And having a "CPUID_" prefixed thing and a "CPUID_LEAF_" prefixed other thing
> > is going to cause confusion.

+1.

What about CPUID_FN_xxx for thing architectural leaf function number?  E.g.
CPUID_FN_80000007 or maybe even CPUID_FN_0x80000007.  CPUID_LEAF_xxx is arguably
wrong anyways for entries with sub-leaves.

There's still potential for confusion, but I think it would be clear enough to
be offset by the niceness of replacing all the open coded CPUID function literals.

> > And renaming enum cpuid_leafs is going to cause a massive churn...
> 
> Wait a sec though:
> 
> $ git grep 'enum cpuid_leafs' arch/x86/
> arch/x86/include/asm/cpufeature.h:enum cpuid_leafs
> arch/x86/kvm/cpuid.c:static __always_inline void kvm_cpu_cap_mask(enum
> cpuid_leafs leaf, u32 mask)
> 
> So there is only one direct reference to the type.
> 
> I think all it will take to rename the _type_ is something like the
> attached. Also, I think the new name 'x86_capability_words' and variable
> 'cap_nr' make the KVM site a lot more readable.

KVM's usage of the type will be gone in 6.14[*] (not yet applied, but it will be,
soon).  Unless renaming the enum type is central to your plans, maybe just wait
until after 6.14 to clean that up?  Note, we should also rename KVM's enum
kvm_only_cpuid_leafs to align with whatever the new enum name ends up being.

As for "cap_nr", IMO that is a net negative relative to "leaf".  For all CPUID
leaves that KVM cares about, the array entry is guaranteed to correspond to a
single CPUID leaf, albeit for only one output register.  KVM has definitely
bastardized "leaf", but I do think it helps convey that the "word" being modified
corresponds 1:1 with a specific CPUID leaf output.

[*] https://lore.kernel.org/all/20241128013424.4096668-27-seanjc@google.com