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

["Ahmed S. Darwish" <darwi@linutronix.de>]

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