linux-kernel - Re: [PATCH v1 00/26] x86: Introduce centralized CPUID 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] [day] [month] [year] [list]

Message-ID: <aBsor3iOb0SJsLSQ@lx-t490>
Date: Wed, 7 May 2025 11:32:31 +0200
From: "Ahmed S. Darwish" <darwi@...utronix.de>
To: Ingo Molnar <mingo@...nel.org>
Cc: Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	John Ogness <john.ogness@...utronix.de>, x86@...nel.org,
	x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 00/26] x86: Introduce centralized CPUID model

On Tue, 06 May, Ingo Molnar wrote:
>
> Wrt. <asm/cpuid/api.h>, we'll need a few followup cleanups there too
> I think, such as migrating to the cpuid_*() namespace:
>

Perfect, then I'll move ahead and do a "CPUID headers cleanup" patch
queue /before/ moving into a v2 of this series.

Any other wishes in that domain?

> Rename have_cpuid_p() to cpuid_feature() or so.

ACK.

> I find the cpudata_cpuid_ namespace a bit confusing:
>
>	__cpudata_cpuid_subleaf_idx(__table, __leaf, __subleaf, __idx)
>	__cpudata_cpuid_subleaf(__table, __leaf, __subleaf)
>	cpudata_cpuid_subleaf(_cpuinfo, _leaf, _subleaf)
>	cpudata_cpuid(_cpuinfo, _leaf)
>	cpudata_cpuid_nr_entries(_cpuinfo, _leaf)
>	cpudata_cpuid_index(_cpuinfo, _leaf, _idx)
>	cpudata_cpuid_regs(_cpuinfo, _leaf)
>	cpudata_cpuid_index_regs(_cpuinfo, _leaf, _idx)
>

> All of CPUID processing is related to 'data', and we don't
> really have any 'cpudata' primitives, so the cpudata_ prefix is
> confusing to me.
>
> It's particularly confusing for methods like cpudata_cpuid(),
> which sounds like a generic method, while in reality it accesses
> subleaf 0, right? Why not name it cpuid_subleaf_0() or so?
>
> My suggestion would be to use a structure like this:
>
>	__cpuid_subleaf_idx(__table, __leaf, __subleaf, __idx)
>	__cpuid_subleaf(__table, __leaf, __subleaf)
>	cpuid_subleaf(_cpuinfo, _leaf, _subleaf)
>	cpuid_subleaf_0(_cpuinfo, _leaf)
>	cpuid_leaf_nr_entries(_cpuinfo, _leaf)
>	cpuid_leaf_index(_cpuinfo, _leaf, _idx)
>	cpuid_leaf_regs(_cpuinfo, _leaf)
>	cpuid_leaf_index_regs(_cpuinfo, _leaf, _idx)
>
> Or so?

Yeah, that's honestly much much better.

(cpuid_subleaf() is already a raw CPUID OP at <asm/cpuid/api.h> now, but
 luckily it has one external call site, so it'll be easy to rename.)

> In my book it's a nice bonus that they thus become part of the overall
> cpuid_*() API family. Note how these accessors still are all still
> either cpuid_leaf_ or cpuid_subleaf_ prefixed.

Yeah, now a single <asm/cpuid/api.h> makes full sense as well :)

Thanks!
Ahmed