[PATCH v1 25/40] x86/cacheinfo: Separate Intel and AMD leaf 0x4 code paths

Ahmed S. Darwish
2025-03-04T08:53:30Z

The leaf 0x4 parsing code at cpuid4_cache_lookup_regs() is ugly and
convoluted. It's tangled with multiple nested conditions to handle:

- AMD with TOPEXT, or Hygon CPUs via leaf 0x8000001d
- Legacy AMD fallback via leaf 0x4 emulation
- Intel CPUs via the actual cpuid leaf 0x4

AMD L3 northbridge initialization is also awkwardly placed alongside the
cpuid calls of the first two scenarios.

Refactor all of that as follows:

- Update AMD's leaf 0x4 emulation comment to represent current state.
- Clearly label the AMD leaf 0x4 emulation function as a fallback.
- Split AMD/Hygon and Intel code paths into separate functions.
- Move AMD L3 northbridge initialization out of leaf 0x4 code, and
into populate_cache_leaves() where it belongs. There,
ci_info_init() can directly store the initialized object in the
priv pointer of the linux/cacheinfo.h API.

Signed-off-by: Ahmed S. Darwish <darwi@linutronix.de>
---
arch/x86/kernel/cpu/cacheinfo.c | 93 ++++++++++++++++++---------------
1 file changed, 51 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index eadd117c05e1..cc320817cfc3 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -167,12 +167,11 @@ struct _cpuid4_info_regs {
unsigned long size;
};

-/* AMD doesn't have CPUID4. Emulate it here to report the same
- information to the user. This makes some assumptions about the machine:
- L2 not shared, no SMT etc. that is currently true on AMD CPUs.
+/*
+ * Fallback AMD CPUID(4) emulation
+ * AMD CPUs with TOPOEXT can just use CPUID(0x8000001d)
+ */

- In theory the TLBs could be reported as fake type (they are in "dummy").
- Maybe later */
union l1_cache {
struct {
unsigned line_size:8;
@@ -228,9 +227,8 @@ static const enum cache_type cache_type_map[] = {
[CTYPE_UNIFIED] = CACHE_TYPE_UNIFIED,
};

-static void
-amd_cpuid4(int index, union _cpuid4_leaf_eax *eax,
- union _cpuid4_leaf_ebx *ebx, union _cpuid4_leaf_ecx *ecx)
+static void legacy_amd_cpuid4(int index, union _cpuid4_leaf_eax *eax,
+ union _cpuid4_leaf_ebx *ebx, union _cpuid4_leaf_ecx *ecx)
{
unsigned int dummy, line_size, lines_per_tag, assoc, size_in_kb;
union l1_cache l1i, l1d;
@@ -297,37 +295,9 @@ amd_cpuid4(int index, union _cpuid4_leaf_eax *eax,
(ebx->split.ways_of_associativity + 1) - 1;
}

-/*
- * Fill passed _cpuid4_info_regs structure.
- * Intel-only code paths should pass NULL for the amd_northbridge
- * return pointer.
- */
-static int cpuid4_cache_lookup_regs(int index, struct _cpuid4_info_regs *id4,
- struct amd_northbridge **nb)
+static int cpuid4_info_fill_done(struct _cpuid4_info_regs *id4, union _cpuid4_leaf_eax eax,
+ union _cpuid4_leaf_ebx ebx, union _cpuid4_leaf_ecx ecx)
{
- union _cpuid4_leaf_eax eax;
- union _cpuid4_leaf_ebx ebx;
- union _cpuid4_leaf_ecx ecx;
- unsigned edx;
-
- if (x86_vendor_amd_or_hygon(boot_cpu_data.x86_vendor)) {
- if (boot_cpu_has(X86_FEATURE_TOPOEXT) ||
- boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
- /* AMD with TOPOEXT, or HYGON */
- cpuid_count(0x8000001d, index, &eax.full,
- &ebx.full, &ecx.full, &edx);
- } else {
- /* Legacy AMD fallback */
- amd_cpuid4(index, &eax, &ebx, &ecx);
- }
-
- if (nb)
- *nb = amd_init_l3_cache(index);
- } else {
- /* Intel */
- cpuid_count(4, index, &eax.full, &ebx.full, &ecx.full, &edx);
- }
-
if (eax.split.type == CTYPE_NULL)
return -EIO;

@@ -342,6 +312,40 @@ static int cpuid4_cache_lookup_regs(int index, struct _cpuid4_info_regs *id4,
return 0;
}

+static int amd_fill_cpuid4_info(int index, struct _cpuid4_info_regs *id4)
+{
+ union _cpuid4_leaf_eax eax;
+ union _cpuid4_leaf_ebx ebx;
+ union _cpuid4_leaf_ecx ecx;
+ unsigned int ignored;
+
+ if (boot_cpu_has(X86_FEATURE_TOPOEXT) || boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
+ cpuid_count(0x8000001d, index, &eax.full, &ebx.full, &ecx.full, &ignored);
+ else
+ legacy_amd_cpuid4(index, &eax, &ebx, &ecx);
+
+ return cpuid4_info_fill_done(id4, eax, ebx, ecx);
+}
+
+static int intel_fill_cpuid4_info(int index, struct _cpuid4_info_regs *id4)
+{
+ union _cpuid4_leaf_eax eax;
+ union _cpuid4_leaf_ebx ebx;
+ union _cpuid4_leaf_ecx ecx;
+ unsigned int ignored;
+
+ cpuid_count(4, index, &eax.full, &ebx.full, &ecx.full, &ignored);
+
+ return cpuid4_info_fill_done(id4, eax, ebx, ecx);
+}
+
+static int fill_cpuid4_info(int index, struct _cpuid4_info_regs *id4)
+{
+ return x86_vendor_amd_or_hygon(boot_cpu_data.x86_vendor) ?
+ amd_fill_cpuid4_info(index, id4) :
+ intel_fill_cpuid4_info(index, id4);
+}
+
static int find_num_cache_leaves(struct cpuinfo_x86 *c)
{
unsigned int eax, ebx, ecx, edx, op;
@@ -472,7 +476,7 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
struct _cpuid4_info_regs id4 = {};
int retval;

- retval = cpuid4_cache_lookup_regs(i, &id4, NULL);
+ retval = intel_fill_cpuid4_info(i, &id4);
if (retval < 0)
continue;

@@ -690,17 +694,22 @@ static void get_cache_id(int cpu, struct _cpuid4_info_regs *id4)

int populate_cache_leaves(unsigned int cpu)
{
- unsigned int idx, ret;
struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
struct cacheinfo *ci = this_cpu_ci->info_list;
struct _cpuid4_info_regs id4 = {};
- struct amd_northbridge *nb;
+ struct amd_northbridge *nb = NULL;
+ int idx, ret;

for (idx = 0; idx < this_cpu_ci->num_leaves; idx++) {
- ret = cpuid4_cache_lookup_regs(idx, &id4, &nb);
+ ret = fill_cpuid4_info(idx, &id4);
if (ret)
return ret;
+
get_cache_id(cpu, &id4);
+
+ if (x86_vendor_amd_or_hygon(boot_cpu_data.x86_vendor))
+ nb = amd_init_l3_cache(idx);
+
ci_info_init(ci++, &id4, nb);
__cache_cpumap_setup(cpu, idx, &id4);
}
--
2.48.1