[PATCH v2 18/29] x86/cacheinfo: Use enums for cache descriptor types

Ahmed S. Darwish posted 29 patches 9 months ago
There is a newer version of this series
[PATCH v2 18/29] x86/cacheinfo: Use enums for cache descriptor types
Posted by Ahmed S. Darwish 9 months ago
The leaf 0x2 one-byte cache descriptor types:

	CACHE_L1_INST
	CACHE_L1_DATA
	CACHE_L2
	CACHE_L3

are just discriminators to be used within the cache_table[] mapping.
Their specific values are irrelevant.

Use enums for such types.

Make the enum packed and static assert that its values remain within a
single byte so that the cache_table[] array size do not go out of hand.

Signed-off-by: Ahmed S. Darwish <darwi@linutronix.de>
---
 arch/x86/include/asm/cpuid/types.h | 13 +++++++++++++
 arch/x86/kernel/cpu/cacheinfo.c    |  9 ++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/cpuid/types.h b/arch/x86/include/asm/cpuid/types.h
index 864047113e37..a66192f9df4c 100644
--- a/arch/x86/include/asm/cpuid/types.h
+++ b/arch/x86/include/asm/cpuid/types.h
@@ -2,6 +2,7 @@
 #ifndef _ASM_X86_CPUID_TYPES_H
 #define _ASM_X86_CPUID_TYPES_H
 
+#include <linux/build_bug.h>
 #include <linux/types.h>
 
 /*
@@ -42,4 +43,16 @@ union leaf_0x2_regs {
 	u8			desc[16];
 };
 
+/*
+ * Leaf 0x2 1-byte descriptors' cache types
+ * To be used for their mappings at cache_table[]
+ */
+enum _cache_table_type {
+	CACHE_L1_INST,
+	CACHE_L1_DATA,
+	CACHE_L2,
+	CACHE_L3
+} __packed;
+static_assert(sizeof(enum _cache_table_type) == 1);
+
 #endif /* _ASM_X86_CPUID_TYPES_H */
diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 777f95c86e03..9f5bf57fd4fc 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -23,11 +23,6 @@
 
 #include "cpu.h"
 
-#define CACHE_L1_INST	1
-#define CACHE_L1_DATA	2
-#define CACHE_L2	3
-#define CACHE_L3	4
-
 /* Shared last level cache maps */
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_llc_shared_map);
 
@@ -41,7 +36,7 @@ unsigned int memory_caching_control __ro_after_init;
 
 struct _cache_table {
 	unsigned char descriptor;
-	char cache_type;
+	enum _cache_table_type type;
 	short size;
 };
 
@@ -517,7 +512,7 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 			if (!entry)
 				continue;
 
-			switch (entry->cache_type) {
+			switch (entry->type) {
 			case CACHE_L1_INST:	l1i += entry->size; break;
 			case CACHE_L1_DATA:	l1d += entry->size; break;
 			case CACHE_L2:		l2  += entry->size; break;
-- 
2.48.1
Re: [PATCH v2 18/29] x86/cacheinfo: Use enums for cache descriptor types
Posted by kernel test robot 9 months ago
Hi Ahmed,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6d536cad0d55e71442b6d65500f74eb85544269e]

url:    https://github.com/intel-lab-lkp/linux/commits/Ahmed-S-Darwish/x86-treewide-Introduce-x86_vendor_amd_or_hygon/20250318-011153
base:   6d536cad0d55e71442b6d65500f74eb85544269e
patch link:    https://lore.kernel.org/r/20250317164745.4754-19-darwi%40linutronix.de
patch subject: [PATCH v2 18/29] x86/cacheinfo: Use enums for cache descriptor types
config: i386-randconfig-061-20250319 (https://download.01.org/0day-ci/archive/20250319/202503192150.Vhannmnp-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250319/202503192150.Vhannmnp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503192150.Vhannmnp-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/cpuidle/poll_state.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/cpuidle-haltpoll.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/sysfs.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/governor.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/driver.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/cpuidle.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/governors/menu.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"
--
   drivers/cpuidle/governors/haltpoll.c: note: in included file (through arch/x86/include/asm/cpuid/api.h, arch/x86/include/asm/cpuid.h, arch/x86/include/asm/processor.h, ...):
>> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse: static assertion failed: "sizeof(enum _cache_table_type) == 1"

vim +56 arch/x86/include/asm/cpuid/types.h

    45	
    46	/*
    47	 * Leaf 0x2 1-byte descriptors' cache types
    48	 * To be used for their mappings at cache_table[]
    49	 */
    50	enum _cache_table_type {
    51		CACHE_L1_INST,
    52		CACHE_L1_DATA,
    53		CACHE_L2,
    54		CACHE_L3
    55	} __packed;
  > 56	static_assert(sizeof(enum _cache_table_type) == 1);
    57	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 18/29] x86/cacheinfo: Use enums for cache descriptor types
Posted by Ahmed S. Darwish 9 months ago
Hi,

On Wed, 19 Mar 2025, kernel test robot wrote:
>
> arch/x86/include/asm/cpuid/types.h:56:1: sparse: sparse:
>   static assertion failed: "sizeof(enum _cache_table_type) == 1"
>

I've checked the error report and reproduced it on my machine:

    https://download.01.org/0day-ci/archive/20250319/202503192150.Vhannmnp-lkp@intel.com/reproduce

After cloning sparse git repo and playing around with its validation test
suite, I've realized that it does not understand __attribiute__((packed))
on enums.

Namely, applying below diff on top of sparse's git repo:

    |  diff --git a/validation/enum-min-size.c b/validation/enum-min-size.c
    |  index e8bd9fb1..e691b332 100644
    |  --- a/validation/enum-min-size.c
    |  +++ b/validation/enum-min-size.c
    |  @@ -1,5 +1,9 @@
    |   enum i { I = 1 };
    |   _Static_assert(sizeof(enum i) == sizeof(int), "int");
    |  +
    |  +enum k { X = 0} __attribute__((packed));
    |  +_Static_assert(sizeof(enum k) == sizeof(char), "char");
    |  +
    |   enum u { U = 1U };
    |   _Static_assert(sizeof(enum u) == sizeof(int), "uint");

Then running the modified test:

    sparse/$ cd validation
    sparse/validation$ ./test-suite enum-min-size.c

leads to the same error:

    TEST    enum-min-size (enum-min-size.c)
    +enum-min-size.c:5:31: error: static assertion failed: "char"
    error: FAIL: test 'enum-min-size.c' failed

After checking other kernel code which have __packed on enums, I found
this at <linux/rw_hint.h>, which is included by core SCSI code:

    /* Sparse ignores __packed annotations on enums, hence the #ifndef below. */
    #ifndef __CHECKER__
    static_assert(sizeof(enum rw_hint) == 1);
    #endif

So I'll add a similar CPP guard (in v4, since I've already posted v3.)

Thanks!

--
Ahmed S. Darwish
Linutronix GmbH