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: <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

Powered by Openwall GNU/*/Linux Powered by OpenVZ