Re: [PATCH v1 00/26] x86: Introduce centralized CPUID model

"Ahmed S. Darwish" <[email protected]>
Newsgroups dev.linux.lists.x86-cpuid,org.kernel.vger.linux-kernel
Message-ID <aBsor3iOb0SJsLSQ@lx-t490>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.