From: Dave Hansen <dave.hansen@linux.intel.com>
The leaf names are not consistent. Give them all a CPUID_LEAF_ prefix
for consistency and vertical alignment.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---
b/arch/x86/events/intel/pt.c | 4 ++--
b/arch/x86/include/asm/cpuid.h | 12 ++++++------
b/arch/x86/kernel/acpi/cstate.c | 2 +-
b/arch/x86/kernel/cpu/common.c | 6 +++---
b/arch/x86/kernel/fpu/xstate.c | 20 ++++++++++----------
b/arch/x86/kernel/hpet.c | 2 +-
b/arch/x86/kernel/process.c | 2 +-
b/arch/x86/kernel/smpboot.c | 2 +-
b/arch/x86/kernel/tsc.c | 18 +++++++++---------
b/arch/x86/xen/enlighten_pv.c | 4 ++--
b/drivers/acpi/acpi_pad.c | 2 +-
b/drivers/dma/ioat/dca.c | 2 +-
b/drivers/idle/intel_idle.c | 2 +-
b/drivers/platform/x86/intel/pmc/core.c | 4 ++--
14 files changed, 41 insertions(+), 41 deletions(-)
diff -puN arch/x86/events/intel/pt.c~xsave-leaf-checks-3 arch/x86/events/intel/pt.c
--- a/arch/x86/events/intel/pt.c~xsave-leaf-checks-3 2024-10-30 12:26:59.218216325 -0700
+++ b/arch/x86/events/intel/pt.c 2024-10-30 12:26:59.238216364 -0700
@@ -202,10 +202,10 @@ static int __init pt_pmu_hw_init(void)
* otherwise, zero for numerator stands for "not enumerated"
* as per SDM
*/
- if (boot_cpu_data.cpuid_level >= CPUID_TSC_LEAF) {
+ if (boot_cpu_data.cpuid_level >= CPUID_LEAF_TSC) {
u32 eax, ebx, ecx, edx;
- cpuid(CPUID_TSC_LEAF, &eax, &ebx, &ecx, &edx);
+ cpuid(CPUID_LEAF_TSC, &eax, &ebx, &ecx, &edx);
pt_pmu.tsc_art_num = ebx;
pt_pmu.tsc_art_den = eax;
diff -puN arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 arch/x86/include/asm/cpuid.h
--- a/arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 2024-10-30 12:26:59.222216332 -0700
+++ b/arch/x86/include/asm/cpuid.h 2024-10-30 12:26:59.238216364 -0700
@@ -19,12 +19,12 @@ enum cpuid_regs_idx {
CPUID_EDX,
};
-#define CPUID_MWAIT_LEAF 0x5
-#define CPUID_DCA_LEAF 0x9
-#define XSTATE_CPUID 0x0d
-#define CPUID_TSC_LEAF 0x15
-#define CPUID_FREQ_LEAF 0x16
-#define TILE_CPUID 0x1d
+#define CPUID_LEAF_MWAIT 0x5
+#define CPUID_LEAF_DCA 0x9
+#define CPUID_LEAF_XSTATE 0x0d
+#define CPUID_LEAF_TSC 0x15
+#define CPUID_LEAF_FREQ 0x16
+#define CPUID_LEAF_TILE 0x1d
#ifdef CONFIG_X86_32
extern int have_cpuid_p(void);
diff -puN arch/x86/kernel/acpi/cstate.c~xsave-leaf-checks-3 arch/x86/kernel/acpi/cstate.c
--- a/arch/x86/kernel/acpi/cstate.c~xsave-leaf-checks-3 2024-10-30 12:26:59.222216332 -0700
+++ b/arch/x86/kernel/acpi/cstate.c 2024-10-30 12:26:59.238216364 -0700
@@ -129,7 +129,7 @@ static long acpi_processor_ffh_cstate_pr
unsigned int cstate_type; /* C-state type and not ACPI C-state type */
unsigned int num_cstate_subtype;
- cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
+ cpuid(CPUID_LEAF_MWAIT, &eax, &ebx, &ecx, &edx);
/* Check whether this particular cx_type (in CST) is supported or not */
cstate_type = (((cx->address >> MWAIT_SUBSTATE_SIZE) &
diff -puN arch/x86/kernel/cpu/common.c~xsave-leaf-checks-3 arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c~xsave-leaf-checks-3 2024-10-30 12:26:59.222216332 -0700
+++ b/arch/x86/kernel/cpu/common.c 2024-10-30 12:26:59.238216364 -0700
@@ -637,9 +637,9 @@ struct cpuid_dependent_feature {
static const struct cpuid_dependent_feature
cpuid_dependent_features[] = {
- { X86_FEATURE_MWAIT, CPUID_MWAIT_LEAF },
- { X86_FEATURE_DCA, CPUID_DCA_LEAF },
- { X86_FEATURE_XSAVE, XSTATE_CPUID },
+ { X86_FEATURE_MWAIT, CPUID_LEAF_MWAIT },
+ { X86_FEATURE_DCA, CPUID_LEAF_DCA },
+ { X86_FEATURE_XSAVE, CPUID_LEAF_XSTATE },
{ 0, 0 }
};
diff -puN arch/x86/kernel/fpu/xstate.c~xsave-leaf-checks-3 arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~xsave-leaf-checks-3 2024-10-30 12:26:59.226216341 -0700
+++ b/arch/x86/kernel/fpu/xstate.c 2024-10-30 12:26:59.238216364 -0700
@@ -233,7 +233,7 @@ static void __init setup_xstate_cache(vo
xmm_space);
for_each_extended_xfeature(i, fpu_kernel_cfg.max_features) {
- cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, i, &eax, &ebx, &ecx, &edx);
xstate_sizes[i] = eax;
xstate_flags[i] = ecx;
@@ -399,7 +399,7 @@ int xfeature_size(int xfeature_nr)
u32 eax, ebx, ecx, edx;
CHECK_XFEATURE(xfeature_nr);
- cpuid_count(XSTATE_CPUID, xfeature_nr, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, xfeature_nr, &eax, &ebx, &ecx, &edx);
return eax;
}
@@ -442,9 +442,9 @@ static void __init __xstate_dump_leaves(
* just in case there are some goodies up there
*/
for (i = 0; i < XFEATURE_MAX + 10; i++) {
- cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, i, &eax, &ebx, &ecx, &edx);
pr_warn("CPUID[%02x, %02x]: eax=%08x ebx=%08x ecx=%08x edx=%08x\n",
- XSTATE_CPUID, i, eax, ebx, ecx, edx);
+ CPUID_LEAF_XSTATE, i, eax, ebx, ecx, edx);
}
}
@@ -485,7 +485,7 @@ static int __init check_xtile_data_again
* Check the maximum palette id:
* eax: the highest numbered palette subleaf.
*/
- cpuid_count(TILE_CPUID, 0, &max_palid, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_TILE, 0, &max_palid, &ebx, &ecx, &edx);
/*
* Cross-check each tile size and find the maximum number of
@@ -499,7 +499,7 @@ static int __init check_xtile_data_again
* eax[31:16]: bytes per title
* ebx[31:16]: the max names (or max number of tiles)
*/
- cpuid_count(TILE_CPUID, palid, &eax, &ebx, &edx, &edx);
+ cpuid_count(CPUID_LEAF_TILE, palid, &eax, &ebx, &edx, &edx);
tile_size = eax >> 16;
max = ebx >> 16;
@@ -634,7 +634,7 @@ static unsigned int __init get_compacted
* are no supervisor states, but XSAVEC still uses compacted
* format.
*/
- cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, 1, &eax, &ebx, &ecx, &edx);
return ebx;
}
@@ -675,7 +675,7 @@ static unsigned int __init get_xsave_siz
* containing all the *user* state components
* corresponding to bits currently set in XCR0.
*/
- cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, 0, &eax, &ebx, &ecx, &edx);
return ebx;
}
@@ -767,13 +767,13 @@ void __init fpu__init_system_xstate(unsi
/*
* Find user xstates supported by the processor.
*/
- cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, 0, &eax, &ebx, &ecx, &edx);
fpu_kernel_cfg.max_features = eax + ((u64)edx << 32);
/*
* Find supervisor xstates supported by the processor.
*/
- cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
+ cpuid_count(CPUID_LEAF_XSTATE, 1, &eax, &ebx, &ecx, &edx);
fpu_kernel_cfg.max_features |= ecx + ((u64)edx << 32);
if ((fpu_kernel_cfg.max_features & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
diff -puN arch/x86/kernel/hpet.c~xsave-leaf-checks-3 arch/x86/kernel/hpet.c
--- a/arch/x86/kernel/hpet.c~xsave-leaf-checks-3 2024-10-30 12:26:59.226216341 -0700
+++ b/arch/x86/kernel/hpet.c 2024-10-30 12:26:59.238216364 -0700
@@ -928,7 +928,7 @@ static bool __init mwait_pc10_supported(
if (!cpu_feature_enabled(X86_FEATURE_MWAIT))
return false;
- cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
+ cpuid(CPUID_LEAF_MWAIT, &eax, &ebx, &ecx, &mwait_substates);
return (ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) &&
(ecx & CPUID5_ECX_INTERRUPT_BREAK) &&
diff -puN arch/x86/kernel/process.c~xsave-leaf-checks-3 arch/x86/kernel/process.c
--- a/arch/x86/kernel/process.c~xsave-leaf-checks-3 2024-10-30 12:26:59.226216341 -0700
+++ b/arch/x86/kernel/process.c 2024-10-30 12:26:59.238216364 -0700
@@ -878,7 +878,7 @@ static __init bool prefer_mwait_c1_over_
if (boot_cpu_has_bug(X86_BUG_MONITOR) || boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E))
return false;
- cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
+ cpuid(CPUID_LEAF_MWAIT, &eax, &ebx, &ecx, &edx);
/*
* If MWAIT extensions are not available, it is safe to use MWAIT
diff -puN arch/x86/kernel/smpboot.c~xsave-leaf-checks-3 arch/x86/kernel/smpboot.c
--- a/arch/x86/kernel/smpboot.c~xsave-leaf-checks-3 2024-10-30 12:26:59.226216341 -0700
+++ b/arch/x86/kernel/smpboot.c 2024-10-30 12:26:59.238216364 -0700
@@ -1292,7 +1292,7 @@ static inline void mwait_play_dead(void)
if (!this_cpu_has(X86_FEATURE_CLFLUSH))
return;
- eax = CPUID_MWAIT_LEAF;
+ eax = CPUID_LEAF_MWAIT;
ecx = 0;
native_cpuid(&eax, &ebx, &ecx, &edx);
diff -puN arch/x86/kernel/tsc.c~xsave-leaf-checks-3 arch/x86/kernel/tsc.c
--- a/arch/x86/kernel/tsc.c~xsave-leaf-checks-3 2024-10-30 12:26:59.230216349 -0700
+++ b/arch/x86/kernel/tsc.c 2024-10-30 12:26:59.238216364 -0700
@@ -665,13 +665,13 @@ unsigned long native_calibrate_tsc(void)
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return 0;
- if (boot_cpu_data.cpuid_level < CPUID_TSC_LEAF)
+ if (boot_cpu_data.cpuid_level < CPUID_LEAF_TSC)
return 0;
eax_denominator = ebx_numerator = ecx_hz = edx = 0;
/* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
- cpuid(CPUID_TSC_LEAF, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
+ cpuid(CPUID_LEAF_TSC, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
if (ebx_numerator == 0 || eax_denominator == 0)
return 0;
@@ -680,7 +680,7 @@ unsigned long native_calibrate_tsc(void)
/*
* Denverton SoCs don't report crystal clock, and also don't support
- * CPUID_FREQ_LEAF for the calculation below, so hardcode the 25MHz
+ * CPUID_LEAF_FREQ for the calculation below, so hardcode the 25MHz
* crystal clock.
*/
if (crystal_khz == 0 &&
@@ -700,10 +700,10 @@ unsigned long native_calibrate_tsc(void)
* clock, but we can easily calculate it to a high degree of accuracy
* by considering the crystal ratio and the CPU speed.
*/
- if (crystal_khz == 0 && boot_cpu_data.cpuid_level >= CPUID_FREQ_LEAF) {
+ if (crystal_khz == 0 && boot_cpu_data.cpuid_level >= CPUID_LEAF_FREQ) {
unsigned int eax_base_mhz, ebx, ecx, edx;
- cpuid(CPUID_FREQ_LEAF, &eax_base_mhz, &ebx, &ecx, &edx);
+ cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
crystal_khz = eax_base_mhz * 1000 *
eax_denominator / ebx_numerator;
}
@@ -738,12 +738,12 @@ static unsigned long cpu_khz_from_cpuid(
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return 0;
- if (boot_cpu_data.cpuid_level < CPUID_FREQ_LEAF)
+ if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
return 0;
eax_base_mhz = ebx_max_mhz = ecx_bus_mhz = edx = 0;
- cpuid(CPUID_FREQ_LEAF, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
+ cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
return eax_base_mhz * 1000;
}
@@ -1076,7 +1076,7 @@ static void __init detect_art(void)
{
unsigned int unused;
- if (boot_cpu_data.cpuid_level < CPUID_TSC_LEAF)
+ if (boot_cpu_data.cpuid_level < CPUID_LEAF_TSC)
return;
/*
@@ -1089,7 +1089,7 @@ static void __init detect_art(void)
tsc_async_resets)
return;
- cpuid(CPUID_TSC_LEAF, &art_base_clk.denominator,
+ cpuid(CPUID_LEAF_TSC, &art_base_clk.denominator,
&art_base_clk.numerator, &art_base_clk.freq_khz, &unused);
art_base_clk.freq_khz /= KHZ;
diff -puN arch/x86/xen/enlighten_pv.c~xsave-leaf-checks-3 arch/x86/xen/enlighten_pv.c
--- a/arch/x86/xen/enlighten_pv.c~xsave-leaf-checks-3 2024-10-30 12:26:59.230216349 -0700
+++ b/arch/x86/xen/enlighten_pv.c 2024-10-30 12:26:59.238216364 -0700
@@ -231,7 +231,7 @@ static void xen_cpuid(unsigned int *ax,
or_ebx = smp_processor_id() << 24;
break;
- case CPUID_MWAIT_LEAF:
+ case CPUID_LEAF_MWAIT:
/* Synthesize the values.. */
*ax = 0;
*bx = 0;
@@ -301,7 +301,7 @@ static bool __init xen_check_mwait(void)
* ecx and edx. The hypercall provides only partial information.
*/
- ax = CPUID_MWAIT_LEAF;
+ ax = CPUID_LEAF_MWAIT;
bx = 0;
cx = 0;
dx = 0;
diff -puN drivers/acpi/acpi_pad.c~xsave-leaf-checks-3 drivers/acpi/acpi_pad.c
--- a/drivers/acpi/acpi_pad.c~xsave-leaf-checks-3 2024-10-30 12:26:59.230216349 -0700
+++ b/drivers/acpi/acpi_pad.c 2024-10-30 12:26:59.238216364 -0700
@@ -48,7 +48,7 @@ static void power_saving_mwait_init(void
if (!boot_cpu_has(X86_FEATURE_MWAIT))
return;
- cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
+ cpuid(CPUID_LEAF_MWAIT, &eax, &ebx, &ecx, &edx);
if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
!(ecx & CPUID5_ECX_INTERRUPT_BREAK))
diff -puN drivers/dma/ioat/dca.c~xsave-leaf-checks-3 drivers/dma/ioat/dca.c
--- a/drivers/dma/ioat/dca.c~xsave-leaf-checks-3 2024-10-30 12:26:59.234216356 -0700
+++ b/drivers/dma/ioat/dca.c 2024-10-30 12:26:59.238216364 -0700
@@ -63,7 +63,7 @@ static int dca_enabled_in_bios(struct pc
u32 eax;
int res;
- eax = cpuid_eax(CPUID_DCA_LEAF);
+ eax = cpuid_eax(CPUID_LEAF_DCA);
res = eax & BIT(0);
if (!res)
dev_dbg(&pdev->dev, "DCA is disabled in BIOS\n");
diff -puN drivers/idle/intel_idle.c~xsave-leaf-checks-3 drivers/idle/intel_idle.c
--- a/drivers/idle/intel_idle.c~xsave-leaf-checks-3 2024-10-30 12:26:59.234216356 -0700
+++ b/drivers/idle/intel_idle.c 2024-10-30 12:26:59.242216372 -0700
@@ -2269,7 +2269,7 @@ static int __init intel_idle_init(void)
return -ENODEV;
}
- cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
+ cpuid(CPUID_LEAF_MWAIT, &eax, &ebx, &ecx, &mwait_substates);
if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
!(ecx & CPUID5_ECX_INTERRUPT_BREAK) ||
diff -puN drivers/platform/x86/intel/pmc/core.c~xsave-leaf-checks-3 drivers/platform/x86/intel/pmc/core.c
--- a/drivers/platform/x86/intel/pmc/core.c~xsave-leaf-checks-3 2024-10-30 12:26:59.234216356 -0700
+++ b/drivers/platform/x86/intel/pmc/core.c 2024-10-30 12:26:59.242216372 -0700
@@ -937,13 +937,13 @@ static unsigned int pmc_core_get_crystal
{
unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
- if (boot_cpu_data.cpuid_level < CPUID_TSC_LEAF)
+ if (boot_cpu_data.cpuid_level < CPUID_LEAF_TSC)
return 0;
eax_denominator = ebx_numerator = ecx_hz = edx = 0;
/* TSC/Crystal ratio, plus optionally Crystal Hz */
- cpuid(CPUID_TSC_LEAF, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
+ cpuid(CPUID_LEAF_TSC, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
if (ebx_numerator == 0 || eax_denominator == 0)
return 0;
_On Wed, Oct 30, 2024 at 02:33:29PM -0700, Dave Hansen wrote:
> diff -puN arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 arch/x86/include/asm/cpuid.h
> --- a/arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 2024-10-30 12:26:59.222216332 -0700
> +++ b/arch/x86/include/asm/cpuid.h 2024-10-30 12:26:59.238216364 -0700
> @@ -19,12 +19,12 @@ enum cpuid_regs_idx {
> CPUID_EDX,
> };
>
> -#define CPUID_MWAIT_LEAF 0x5
> -#define CPUID_DCA_LEAF 0x9
> -#define XSTATE_CPUID 0x0d
> -#define CPUID_TSC_LEAF 0x15
> -#define CPUID_FREQ_LEAF 0x16
> -#define TILE_CPUID 0x1d
> +#define CPUID_LEAF_MWAIT 0x5
> +#define CPUID_LEAF_DCA 0x9
> +#define CPUID_LEAF_XSTATE 0x0d
> +#define CPUID_LEAF_TSC 0x15
> +#define CPUID_LEAF_FREQ 0x16
> +#define CPUID_LEAF_TILE 0x1d
... and just to confuse things even more, there's enum cpuid_leafs too which
start with the "CPUID_" prefix too.
Pfff.
I'd like to unify them and I *think* kvm_cpu_cap_mask() should be able to
stomach that (or fixed if not)...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 10/31/24 03:18, Borislav Petkov wrote:
> On Wed, Oct 30, 2024 at 02:33:29PM -0700, Dave Hansen wrote:
>> diff -puN arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 arch/x86/include/asm/cpuid.h
>> --- a/arch/x86/include/asm/cpuid.h~xsave-leaf-checks-3 2024-10-30 12:26:59.222216332 -0700
>> +++ b/arch/x86/include/asm/cpuid.h 2024-10-30 12:26:59.238216364 -0700
>> @@ -19,12 +19,12 @@ enum cpuid_regs_idx {
>> CPUID_EDX,
>> };
>>
>> -#define CPUID_MWAIT_LEAF 0x5
>> -#define CPUID_DCA_LEAF 0x9
>> -#define XSTATE_CPUID 0x0d
>> -#define CPUID_TSC_LEAF 0x15
>> -#define CPUID_FREQ_LEAF 0x16
>> -#define TILE_CPUID 0x1d
>> +#define CPUID_LEAF_MWAIT 0x5
>> +#define CPUID_LEAF_DCA 0x9
>> +#define CPUID_LEAF_XSTATE 0x0d
>> +#define CPUID_LEAF_TSC 0x15
>> +#define CPUID_LEAF_FREQ 0x16
>> +#define CPUID_LEAF_TILE 0x1d
>
> ... and just to confuse things even more, there's enum cpuid_leafs too which
> start with the "CPUID_" prefix too.
>
> Pfff.
Yeah, lovely. 'enum cpuid_leafs' does appear misnamed though. It is a
list of *words*, not actual leaf numbers. There's also very little
overlap between those leafs and the newly-renamed ones in this series.
I think that's because most of the leaves we dump into the CPU caps have
random feature bits that aren't logically grouped and resist naming.
The one exception to that is the CPUID_D_1_EAX aka. CPUID_LEAF_XSTATE.
We could do something like the attached patch, but I don't think it
really helps much.
> I'd like to unify them and I *think* kvm_cpu_cap_mask() should be able to
> stomach that (or fixed if not)...
Do you mean we could unify the CPUID_8000_0001_EDX enum values and the
CPUID_LEAF_* defines from this series?
I'm not quite sure how that would look. I think we'd end up doing
something like:
#define CPUID_LEAF_C000_0001 0xC0000001
and we'd still need some macro magic to munge the word "number" into
there, like:
#define FOO(x,reg) ((x)<<2 | CPUID_##reg)
that could be used this way:
enum cpuid_leafs
{
...
FOO(CPUID_LEAF_C000_0001, EDX),
It would let us do stuff like this:
- case 0xC0000001:
+ case CPUID_LEAF_C000_0001:
cpuid_entry_override(entry, CPUID_C000_0001_EDX);
break;
But I'm not sure that makes things all that much more readable or greppable.
On Thu, Oct 31, 2024 at 10:19:37AM -0700, Dave Hansen wrote:
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index a5f221ea5688..b44dbb952d8c 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1001,8 +1001,8 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
> }
>
> /* Extended state features: level 0x0000000d */
> - if (c->cpuid_level >= 0x0000000d) {
> - cpuid_count(0x0000000d, 1, &eax, &ebx, &ecx, &edx);
> + if (c->cpuid_level >= CPUID_LEAF_XSTATE) {
> + cpuid_count(CPUID_LEAF_XSTATE, 1, &eax, &ebx, &ecx, &edx);
>
> c->x86_capability[CPUID_D_1_EAX] = eax;
Yah, I'll take whatever I can get. You should add this hunk to your set.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Oct 31, 2024 at 10:19:37AM -0700, Dave Hansen wrote:
> >> -#define CPUID_MWAIT_LEAF 0x5
> >> -#define CPUID_DCA_LEAF 0x9
> >> -#define XSTATE_CPUID 0x0d
> >> -#define CPUID_TSC_LEAF 0x15
> >> -#define CPUID_FREQ_LEAF 0x16
> >> -#define TILE_CPUID 0x1d
> >> +#define CPUID_LEAF_MWAIT 0x5
> >> +#define CPUID_LEAF_DCA 0x9
> >> +#define CPUID_LEAF_XSTATE 0x0d
> >> +#define CPUID_LEAF_TSC 0x15
> >> +#define CPUID_LEAF_FREQ 0x16
> >> +#define CPUID_LEAF_TILE 0x1d
> >
> > ... and just to confuse things even more, there's enum cpuid_leafs too which
> > start with the "CPUID_" prefix too.
> >
> > Pfff.
>
> Yeah, lovely. 'enum cpuid_leafs' does appear misnamed though. It is a
> list of *words*, not actual leaf numbers. There's also very little
> overlap between those leafs and the newly-renamed ones in this series.
> I think that's because most of the leaves we dump into the CPU caps have
> random feature bits that aren't logically grouped and resist naming.
>
> The one exception to that is the CPUID_D_1_EAX aka. CPUID_LEAF_XSTATE.
> We could do something like the attached patch, but I don't think it
> really helps much.
>
> > I'd like to unify them and I *think* kvm_cpu_cap_mask() should be able to
> > stomach that (or fixed if not)...
>
> Do you mean we could unify the CPUID_8000_0001_EDX enum values and the
> CPUID_LEAF_* defines from this series?
Well, enum cpuid_leafs as it is now is the *indices* into the cap flags array:
struct cpuinfo_x86 {
...
__u32 x86_capability[NCAPINTS + NBUGINTS];
And having a "CPUID_" prefixed thing and a "CPUID_LEAF_" prefixed other thing
is going to cause confusion.
And renaming enum cpuid_leafs is going to cause a massive churn...
IOW:
[ Rock ] <-- us --> [ Hard Place ]
:-\
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 11/29/24 10:27, Borislav Petkov wrote:
> Well, enum cpuid_leafs as it is now is the *indices* into the cap flags array:
>
> struct cpuinfo_x86 {
>
> ...
>
> __u32 x86_capability[NCAPINTS + NBUGINTS];
>
> And having a "CPUID_" prefixed thing and a "CPUID_LEAF_" prefixed other thing
> is going to cause confusion.
>
> And renaming enum cpuid_leafs is going to cause a massive churn...
Wait a sec though:
$ git grep 'enum cpuid_leafs' arch/x86/
arch/x86/include/asm/cpufeature.h:enum cpuid_leafs
arch/x86/kvm/cpuid.c:static __always_inline void kvm_cpu_cap_mask(enum
cpuid_leafs leaf, u32 mask)
So there is only one direct reference to the type.
I think all it will take to rename the _type_ is something like the
attached. Also, I think the new name 'x86_capability_words' and variable
'cap_nr' make the KVM site a lot more readable.
Thoughts?
On Fri, Dec 06, 2024, Dave Hansen wrote:
> On 11/29/24 10:27, Borislav Petkov wrote:
> > Well, enum cpuid_leafs as it is now is the *indices* into the cap flags array:
> >
> > struct cpuinfo_x86 {
> >
> > ...
> >
> > __u32 x86_capability[NCAPINTS + NBUGINTS];
> >
> > And having a "CPUID_" prefixed thing and a "CPUID_LEAF_" prefixed other thing
> > is going to cause confusion.
+1.
What about CPUID_FN_xxx for thing architectural leaf function number? E.g.
CPUID_FN_80000007 or maybe even CPUID_FN_0x80000007. CPUID_LEAF_xxx is arguably
wrong anyways for entries with sub-leaves.
There's still potential for confusion, but I think it would be clear enough to
be offset by the niceness of replacing all the open coded CPUID function literals.
> > And renaming enum cpuid_leafs is going to cause a massive churn...
>
> Wait a sec though:
>
> $ git grep 'enum cpuid_leafs' arch/x86/
> arch/x86/include/asm/cpufeature.h:enum cpuid_leafs
> arch/x86/kvm/cpuid.c:static __always_inline void kvm_cpu_cap_mask(enum
> cpuid_leafs leaf, u32 mask)
>
> So there is only one direct reference to the type.
>
> I think all it will take to rename the _type_ is something like the
> attached. Also, I think the new name 'x86_capability_words' and variable
> 'cap_nr' make the KVM site a lot more readable.
KVM's usage of the type will be gone in 6.14[*] (not yet applied, but it will be,
soon). Unless renaming the enum type is central to your plans, maybe just wait
until after 6.14 to clean that up? Note, we should also rename KVM's enum
kvm_only_cpuid_leafs to align with whatever the new enum name ends up being.
As for "cap_nr", IMO that is a net negative relative to "leaf". For all CPUID
leaves that KVM cares about, the array entry is guaranteed to correspond to a
single CPUID leaf, albeit for only one output register. KVM has definitely
bastardized "leaf", but I do think it helps convey that the "word" being modified
corresponds 1:1 with a specific CPUID leaf output.
[*] https://lore.kernel.org/all/20241128013424.4096668-27-seanjc@google.com
On Mon, Dec 09, 2024 at 08:27:58AM -0800, Sean Christopherson wrote:
> > > And having a "CPUID_" prefixed thing and a "CPUID_LEAF_" prefixed other thing
> > > is going to cause confusion.
>
> +1.
>
> What about CPUID_FN_xxx for thing architectural leaf function number? E.g.
> CPUID_FN_80000007 or maybe even CPUID_FN_0x80000007. CPUID_LEAF_xxx is arguably
> wrong anyways for entries with sub-leaves.
Makes sense to me.
Especially if the name already has the function number in it too, which is
self-documenting.
Vs.
CPUID_FN_XSTATE
which will make me go lookup what that "XSTATE" function number was...
In any case, I think having a clear distinction in the naming between
* arch/hw names: CPUID_FN
* our own: CPUID_LNX_...
should help.
And once we're clear on the nomenclature, the conversion will happen
"automatically". :)
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 12/9/24 08:27, Sean Christopherson wrote: > As for "cap_nr", IMO that is a net negative relative to "leaf". For all CPUID > leaves that KVM cares about, the array entry is guaranteed to correspond to a > single CPUID leaf, albeit for only one output register. KVM has definitely > bastardized "leaf", but I do think it helps convey that the "word" being modified > corresponds 1:1 with a specific CPUID leaf output. I'm having a little trouble parsing this. I think you're saying that, right now, if KVM cares about a CPUID leaf that it only cares about a single _word_, even if the core x86 code cares about multiple words. So the concept of a word is actually mostly changeable with a leaf ... for now. Is that right?
On Mon, Dec 09, 2024, Dave Hansen wrote: > On 12/9/24 08:27, Sean Christopherson wrote: > > As for "cap_nr", IMO that is a net negative relative to "leaf". For all CPUID > > leaves that KVM cares about, the array entry is guaranteed to correspond to a > > single CPUID leaf, albeit for only one output register. KVM has definitely > > bastardized "leaf", but I do think it helps convey that the "word" being modified > > corresponds 1:1 with a specific CPUID leaf output. > > I'm having a little trouble parsing this. Gah, sorry, too much implied KVM knowledge. > I think you're saying that, right now, if KVM cares about a CPUID leaf > that it only cares about a single _word_, even if the core x86 code > cares about multiple words. So the concept of a word is actually mostly > changeable with a leaf ... for now. > > Is that right? No. What I was trying to say is that KVM's CPUID code only ever manipulates words that are hardware-defined, i.e. that aren't any of the CPUID_LNX_x words. Because KVM doesn't care about the Linux-defined words, "leaf" can be used to refer to a specific CPUID leaf+register without being too misleading, i.e. doesn't incorrectly suggest that the Linux-defined words somehow correspond to CPUID leaves. KVM usage of "leaf" isn't perfectly aligned with the SDM's usage, but I can't recall anyone ever complaining that KVM's usage of "leaf" is confusing. On the plus side, "leaf" helps communicate to readers that the code is dealing with the data, as opposed to referring to the function+index values themselves. And IMO, "leaf" is much more self-documenting that "word" or "cap_nr".
© 2016 - 2026 Red Hat, Inc.