[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d5aa62d8-1f15-459c-bbd1-4b6e5775e639@intel.com>
Date: Thu, 15 May 2025 15:12:55 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: "Ahmed S. Darwish" <darwi@...utronix.de>
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 04/26] x86/cpuid: Introduce centralized CPUID data
On 5/15/2025 2:23 PM, Ahmed S. Darwish wrote:
> On Tue, 13 May 2025, Sohil Mehta wrote:
>>
>> 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 problem is that at the call sites, even with abbreviated variable
> names, the lines are already wide. Adding "sl_" makes things worse.
>
> For example, at patch 23/26 ("x86/cacheinfo: Use scanned
> CPUID(0x80000005) and CPUID(0x80000006)"), we have:
>
> const struct leaf_0x80000005_0 *el5 = cpudata_cpuid_index(c, 0x80000005, index);
> const struct leaf_0x80000006_0 *el6 = cpudata_cpuid_index(c, 0x80000006, index);
>
> Making that even wider with an "sl_":
>
> const struct leaf_0x80000005_sl_0 *el5 = cpudata_cpuid_index(c, 0x80000005, index);
> const struct leaf_0x80000006_sl_0 *el6 = cpudata_cpuid_index(c, 0x80000006, index);
>
> or "subl_":
>
> const struct leaf_0x80000005_subl_0 *el5 = cpudata_cpuid_index(c, 0x80000005, index);
> const struct leaf_0x80000006_subl_0 *el6 = cpudata_cpuid_index(c, 0x80000006, index);
>
> makes everything overly verbose, without IMHO much benefit.
>
It does make it more verbose but things can get harder to read once the
subleaf numbers start going higher. Example (actual cpuid values):
leaf_0x4_3
leaf_0x10_1
leaf_0xd_11
leaf_0x1d_1
I don't have a better alternative, so I'll leave it up to you.
> I'll sleep over this a bit before sending v2.
>
Another thing to ponder over would be the combination of hexadecimal and
decimal in the 0xN_M naming scheme. The "cpuid" tool uses hex for
printing the subleaf, but the Intel spec describes CPUID using decimals.
Leaving it as decimal is probably fine.
Sohil
Powered by blists - more mailing lists