[PATCH v8 0/2] x86/cacheinfo: Set the number of leaves per CPU

[PATCH v8 0/2] x86/cacheinfo: Set the number of leaves per CPU

[Ricardo Neri]

Hi,

This is v8 of a patchset to fix the cache sysfs interface by setting the
number of cache leaves independently for each CPU. This version merges
patches 2 and 3 from v7 into one as Borislav suggested. I also dropped
the wrappers functions I had added to access per-CPU cache info
(ci_cpu_cacheinfo).

It looked OK to me to keep the Reviewed-by and Tested-by tags collected
so far as all feedback still applies and changes were minor. I hope
reviewers are OK!

Previous versions can be found in [1], [2], [3], [4], [5], [6], and [7].

Below is the (updated) cover letter from v6 for reference:

The interface /sys/devices/system/cpu/cpuX/cache is broken (not populated)
if CPUs have different numbers of subleaves in CPUID 4. This is the case
of Intel Meteor Lake, which now is out in the world. Tools that rely on
sysfs (e.g., lstopo) fail.

Patch 2 fixes the described issue on Meteor Lake. Patch 1 deals with
prework in the cacheinfo base driver to fix issues uncovered while updating
cacheinfo for x86.

All the tests described in detail in [8] and [9] passed. This is the
summary:

  * /sys/devices/system/cpu/cpuX/cache is populated in Meteor Lake.
  * No inconsistencies are found in /sys/devices/system/cpu/cpuX/cache
    and the tools x86info, lstopo, and lscpu.
  * No splat is observed with and without CONFIG_PREEMPT_RT.
  * No new warnings/errors are seen the kernel log.
  * Tests done on assorted Intel and AMD client and server parts.

Changes since v7:
 * Merged patches 2/3 into one. (Borislav)
 * Dropped wrapper functions for ci_cpu_cacheinfo. (Borislav)
 * Check for zero cache leaves in init_cache_level() for x86.
   (Borislav)
 * Removed an ugly line break. (Borislav)

Changes since v6:
  * Merged patches 1 and 2 into one. (Borislav)
  * Fixed an formatting issue in allocate_cache_info(). (Borislav)

Changes since v5:
  * Reordered the arguments of set_num_cache_leaves().
  * Fixed wording on the subject of patch 2.
  * Added Reviewed-by tags from Andreas and Nikolay. Thanks!
  * Added Tested-by tags from Andreas. Thanks!

Changes since v4:
  * Combined two condition checks into one line. (Sudeep)
  * Added one more Reviewed-by tag from Sudeep. Thanks!

Changes since v3:
  * Fixed another NULL-pointer dereference when checking the validity of
    the last-level cache info.
  * Added the Reviewed-by tags from Radu and Sudeep. Thanks!
  * Rebased on v6.7-rc5.

Changes since v2:
  * This version uncovered a NULL-pointer dereference in recent changes to
    cacheinfo[10]. This dereference is observed when the system does not
    configure cacheinfo early during boot nor makes corrections later
    during CPU hotplug; as is the case in x86. Patch 1 fixes this issue.

Changes since v1:
  * Dave Hansen suggested to use the existing per-CPU ci_cpu_cacheinfo
    variable. Now the global variable num_cache_leaves became useless.
  * While here, I noticed that init_cache_level() also became useless:
    x86 does not need ci_cpu_cacheinfo::num_levels.

Thanks and BR,
Ricardo

[1]. https://lore.kernel.org/lkml/20230314231658.30169-1-ricardo.neri-calderon@linux.intel.com/
[2]. https://lore.kernel.org/all/20230424001956.21434-1-ricardo.neri-calderon@linux.intel.com/
[3]. https://lore.kernel.org/lkml/20230805012421.7002-1-ricardo.neri-calderon@linux.intel.com/
[4]. https://lore.kernel.org/all/20231212222519.12834-1-ricardo.neri-calderon@linux.intel.com/
[5]. https://lore.kernel.org/all/20240827051635.9114-1-ricardo.neri-calderon@linux.intel.com/
[6]. https://lore.kernel.org/all/20240905060036.5655-1-ricardo.neri-calderon@linux.intel.com/
[7]. https://lore.kernel.org/all/20240913083155.9783-1-ricardo.neri-calderon@linux.intel.com/
[8]. https://lore.kernel.org/lkml/20230912032350.GA17008@ranerica-svr.sc.intel.com/
[9]. https://lore.kernel.org/all/20240902074140.GA4179@alberich/
[10]. https://lore.kernel.org/all/20230412185759.755408-1-rrendec@redhat.com/

Ricardo Neri (2):
  cacheinfo: Allocate memory during CPU hotplug if not done from the
    primary CPU
  x86/cacheinfo: Delete global num_cache_leaves

 arch/x86/kernel/cpu/cacheinfo.c | 41 +++++++++++++++------------------
 drivers/base/cacheinfo.c        | 11 +++++----
 2 files changed, 25 insertions(+), 27 deletions(-)

-- 
2.34.1

[PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Ricardo Neri]

Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
adds functionality that architectures can use to optionally allocate and
build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
arch specific early level initializer") lets secondary CPUs correct (and
reallocate memory) cacheinfo data if needed.

If the early build functionality is not used and cacheinfo does not need
correction, memory for cacheinfo is never allocated. x86 does not use the
early build functionality. Consequently, during the cacheinfo CPU hotplug
callback, last_level_cache_is_valid() attempts to dereference a NULL
pointer:

     BUG: kernel NULL pointer dereference, address: 0000000000000100
     #PF: supervisor read access in kernel mode
     #PF: error_code(0x0000) - not present page
     PGD 0 P4D 0
     Oops: 0000 [#1] PREEPMT SMP NOPTI
     CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
     RIP: 0010: last_level_cache_is_valid+0x95/0xe0a

Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
not done earlier.

Moreover, before determining the validity of the last-level cache info,
ensure that it has been allocated. Simply checking for non-zero
cache_leaves() is not sufficient, as some architectures (e.g., Intel
processors) have non-zero cache_leaves() before allocation.

Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
This function iterates over all online CPUs. However, a CPU may have come
online recently, but its cacheinfo may not have been allocated yet.

While here, remove an unnecessary indentation in allocate_cache_info().

Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Reviewed-by: Radu Rendec <rrendec@redhat.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Tested-by: Andreas Herrmann <aherrmann@suse.de>
Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Andreas Herrmann <aherrmann@suse.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Nikolay Borisov <nik.borisov@suse.com>
Cc: Radu Rendec <rrendec@redhat.com>
Cc: Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Pu Wen <puwen@hygon.cn>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Will Deacon <will@kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: stable@vger.kernel.org # 6.3+
---
Change since v7:
 * None

Changes since v6:
 * Merged patches 1 and 2 of v6 into one. (Borislav)
 * Merged the history of patches 1 and 2ino this patch.
 * Kept the Reviewed-by and Tested-by tags from the two merged patches.
 * Fixed a formatting issue in allocate_cache_info(). (Borislav)

Changes since v5:
 * Fixed nonsensical subject (Nikolay).
 * Added Reviewed-by and Tested-by tags from Andreas. Thanks!
 * Added Reviewed-by tag from Nikolay. Thanks!

Changes since v4:
 * Combined checks for per_cpu_cacheinfo() and cache_leaves() in a single
   line. (Sudeep)
 * Added Reviewed-by tag from Sudeep. Thanks!

Changes since v3:
 * Added Reviewed-by tag from Radu and Sudeep. Thanks!

Changes since v2:
 * Introduced this patch.

Changes since v1:
 * N/A

---
The motivation for commit 5944ce092b97 was to prevent a BUG splat in
PREEMPT_RT kernels during memory allocation. This splat is not observed on
x86 because the memory allocation for cacheinfo happens in
detect_cache_attributes() from the cacheinfo CPU hotplug callback.

The dereference of a NULL cacheinfo is not observed today because
cache_leaves(cpu) is zero until after init_cache_level() is called
(during the CPU hotplug callback). A subsequent changeset will set
the number of cache leaves earlier and the NULL-pointer dereference
will be observed.
---
 drivers/base/cacheinfo.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 7a7609298e18..a1afc478e0e8 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -58,7 +58,7 @@ bool last_level_cache_is_valid(unsigned int cpu)
 {
 	struct cacheinfo *llc;
 
-	if (!cache_leaves(cpu))
+	if (!cache_leaves(cpu) || !per_cpu_cacheinfo(cpu))
 		return false;
 
 	llc = per_cpu_cacheinfo_idx(cpu, cache_leaves(cpu) - 1);
@@ -466,8 +466,7 @@ int __weak populate_cache_leaves(unsigned int cpu)
 static inline
 int allocate_cache_info(int cpu)
 {
-	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
-					 sizeof(struct cacheinfo), GFP_ATOMIC);
+	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu), sizeof(struct cacheinfo), GFP_ATOMIC);
 	if (!per_cpu_cacheinfo(cpu)) {
 		cache_leaves(cpu) = 0;
 		return -ENOMEM;
@@ -539,7 +538,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
 	 */
 	ci_cacheinfo(cpu)->early_ci_levels = false;
 
-	if (cache_leaves(cpu) <= early_leaves)
+	/*
+	 * Some architectures (e.g., x86) do not use early initialization.
+	 * Allocate memory now in such case.
+	 */
+	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
 		return 0;
 
 	kfree(per_cpu_cacheinfo(cpu));
-- 
2.34.1

[PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Ricardo Neri]

Linux remembers cpu_cachinfo::num_leaves per CPU, but x86 initializes all
CPUs from the same global "num_cache_leaves".

This is erroneous on systems such as Meteor Lake, where each CPU has a
distinct num_leaves value. Delete the global "num_cache_leaves" and
initialize num_leaves on each CPU.

init_cache_level() no longer needs to set num_leaves. Also, it never had to
set num_levels as it is unnecessary in x86. Keep checking for zero cache
leaves. Such condition indicates a bug.

Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
Reviewed-by: Len Brown <len.brown@intel.com>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Tested-by: Andreas Herrmann <aherrmann@suse.de>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Andreas Herrmann <aherrmann@suse.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Nikolay Borisov <nik.borisov@suse.com>
Cc: Radu Rendec <rrendec@redhat.com>
Cc: Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Pu Wen <puwen@hygon.cn>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Will Deacon <will@kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: stable@vger.kernel.org # 6.3+
---
After this change, all CPUs will traverse CPUID leaf 0x4 when booted for
the first time. On systems with symmetric cache topologies this is
useless work.

Creating a list of processor models that have asymmetric cache topologies
was considered. The burden of maintaining such list would outweigh the
performance benefit of skipping this extra step.
---
Changes since v7:
 * Removed an ugly linebreak. (Boris)
 * Folded patch 3/3 into 2/3 as both patches deal with init_cache_level().
   (Boris)
 * Removed the [set,get]_num_cache_leaves() wrappers. Instead, use the
   existing get_cpu_cacheinfo(). (Boris)
 * Future-proof init_cache_level() for cases in which cpu_cacheinfo::
   num_leaves is still zero afer cache info initialization.

Changes since v6:
 * None

Changes since v5:
 * Reordered the arguments of set_num_cache_leaves() for readability.
   (Nikolay)
 * Added Reviewed-by tag from Nikolay and Andreas. Thanks!
 * Added Tested-by tag from Andreas. Thanks!

Changes since v4:
 * None

Changes since v3:
 * Rebased on v6.7-rc5.

Changes since v2:
 * None

Changes since v1:
 * Do not make num_cache_leaves a per-CPU variable. Instead, reuse the
   existing per-CPU ci_cpu_cacheinfo variable. (Dave Hansen)
---
 arch/x86/kernel/cpu/cacheinfo.c | 41 +++++++++++++++------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 392d09c936d6..95e38ab98a72 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -178,8 +178,6 @@ struct _cpuid4_info_regs {
 	struct amd_northbridge *nb;
 };
 
-static unsigned short num_cache_leaves;
-
 /* 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.
@@ -718,19 +716,21 @@ void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c)
 void init_amd_cacheinfo(struct cpuinfo_x86 *c)
 {
 
+	unsigned int cpu = c->cpu_index;
+
 	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
-		num_cache_leaves = find_num_cache_leaves(c);
+		get_cpu_cacheinfo(cpu)->num_leaves = find_num_cache_leaves(c);
 	} else if (c->extended_cpuid_level >= 0x80000006) {
 		if (cpuid_edx(0x80000006) & 0xf000)
-			num_cache_leaves = 4;
+			get_cpu_cacheinfo(cpu)->num_leaves = 4;
 		else
-			num_cache_leaves = 3;
+			get_cpu_cacheinfo(cpu)->num_leaves = 3;
 	}
 }
 
 void init_hygon_cacheinfo(struct cpuinfo_x86 *c)
 {
-	num_cache_leaves = find_num_cache_leaves(c);
+	get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
 }
 
 void init_intel_cacheinfo(struct cpuinfo_x86 *c)
@@ -742,19 +742,18 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
 
 	if (c->cpuid_level > 3) {
-		static int is_initialized;
-
-		if (is_initialized == 0) {
-			/* Init num_cache_leaves from boot CPU */
-			num_cache_leaves = find_num_cache_leaves(c);
-			is_initialized++;
-		}
+		/*
+		 * There should be at least one leaf. A non-zero value means
+		 * that the number of leaves has been initialized.
+		 */
+		if (!get_cpu_cacheinfo(c->cpu_index)->num_leaves)
+			get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
 
 		/*
 		 * Whenever possible use cpuid(4), deterministic cache
 		 * parameters cpuid leaf to find the cache details
 		 */
-		for (i = 0; i < num_cache_leaves; i++) {
+		for (i = 0; i < get_cpu_cacheinfo(c->cpu_index)->num_leaves; i++) {
 			struct _cpuid4_info_regs this_leaf = {};
 			int retval;
 
@@ -790,14 +789,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	 * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
 	 * trace cache
 	 */
-	if ((num_cache_leaves == 0 || c->x86 == 15) && c->cpuid_level > 1) {
+	if ((!get_cpu_cacheinfo(c->cpu_index)->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
 		/* supports eax=2  call */
 		int j, n;
 		unsigned int regs[4];
 		unsigned char *dp = (unsigned char *)regs;
 		int only_trace = 0;
 
-		if (num_cache_leaves != 0 && c->x86 == 15)
+		if (get_cpu_cacheinfo(c->cpu_index)->num_leaves && c->x86 == 15)
 			only_trace = 1;
 
 		/* Number of times to iterate */
@@ -991,14 +990,10 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 
 int init_cache_level(unsigned int cpu)
 {
-	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
-
-	if (!num_cache_leaves)
+	/* There should be at least one leaf. */
+	if (!get_cpu_cacheinfo(cpu)->num_leaves)
 		return -ENOENT;
-	if (!this_cpu_ci)
-		return -EINVAL;
-	this_cpu_ci->num_levels = 3;
-	this_cpu_ci->num_leaves = num_cache_leaves;
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v8 0/2] x86/cacheinfo: Set the number of leaves per CPU

[Borislav Petkov]

On November 28, 2024 1:22:45 AM GMT+01:00, Ricardo Neri <ricardo.neri-calderon@linux.intel.com> wrote:
>Changes since v7:
> * Merged patches 2/3 into one. (Borislav)
> * Dropped wrapper functions for ci_cpu_cacheinfo. (Borislav)
> * Check for zero cache leaves in init_cache_level() for x86.
>   (Borislav)
> * Removed an ugly line break. (Borislav)
>
>Changes since v6:
>  * Merged patches 1 and 2 into one. (Borislav)
>  * Fixed an formatting issue in allocate_cache_info(). (Borislav)

I don't think you should keep the tags after those changes...

-- 
Sent from a small device: formatting sucks and brevity is inevitable.

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Radu Rendec]

On Wed, 2024-11-27 at 16:22 -0800, Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>      BUG: kernel NULL pointer dereference, address: 0000000000000100
>      #PF: supervisor read access in kernel mode
>      #PF: error_code(0x0000) - not present page
>      PGD 0 P4D 0
>      Oops: 0000 [#1] PREEPMT SMP NOPTI
>      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.
> 
> Moreover, before determining the validity of the last-level cache info,
> ensure that it has been allocated. Simply checking for non-zero
> cache_leaves() is not sufficient, as some architectures (e.g., Intel
> processors) have non-zero cache_leaves() before allocation.
> 
> Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> This function iterates over all online CPUs. However, a CPU may have come
> online recently, but its cacheinfo may not have been allocated yet.
> 
> While here, remove an unnecessary indentation in allocate_cache_info().
> 
> Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Andreas Herrmann <aherrmann@suse.de>
> Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Cc: Andreas Herrmann <aherrmann@suse.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Nikolay Borisov <nik.borisov@suse.com>
> Cc: Radu Rendec <rrendec@redhat.com>
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>
> Cc: Pu Wen <puwen@hygon.cn>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: stable@vger.kernel.org # 6.3+
> ---
> Change since v7:
>  * None
> 
> Changes since v6:
>  * Merged patches 1 and 2 of v6 into one. (Borislav)
>  * Merged the history of patches 1 and 2ino this patch.
>  * Kept the Reviewed-by and Tested-by tags from the two merged patches.
>  * Fixed a formatting issue in allocate_cache_info(). (Borislav)
> 
> Changes since v5:
>  * Fixed nonsensical subject (Nikolay).
>  * Added Reviewed-by and Tested-by tags from Andreas. Thanks!
>  * Added Reviewed-by tag from Nikolay. Thanks!
> 
> Changes since v4:
>  * Combined checks for per_cpu_cacheinfo() and cache_leaves() in a single
>    line. (Sudeep)
>  * Added Reviewed-by tag from Sudeep. Thanks!
> 
> Changes since v3:
>  * Added Reviewed-by tag from Radu and Sudeep. Thanks!
> 
> Changes since v2:
>  * Introduced this patch.
> 
> Changes since v1:
>  * N/A
> 
> ---
> The motivation for commit 5944ce092b97 was to prevent a BUG splat in
> PREEMPT_RT kernels during memory allocation. This splat is not observed on
> x86 because the memory allocation for cacheinfo happens in
> detect_cache_attributes() from the cacheinfo CPU hotplug callback.
> 
> The dereference of a NULL cacheinfo is not observed today because
> cache_leaves(cpu) is zero until after init_cache_level() is called
> (during the CPU hotplug callback). A subsequent changeset will set
> the number of cache leaves earlier and the NULL-pointer dereference
> will be observed.
> ---
>  drivers/base/cacheinfo.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index 7a7609298e18..a1afc478e0e8 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -58,7 +58,7 @@ bool last_level_cache_is_valid(unsigned int cpu)
>  {
>  	struct cacheinfo *llc;
>  
> -	if (!cache_leaves(cpu))
> +	if (!cache_leaves(cpu) || !per_cpu_cacheinfo(cpu))
>  		return false;
>  
>  	llc = per_cpu_cacheinfo_idx(cpu, cache_leaves(cpu) - 1);
> @@ -466,8 +466,7 @@ int __weak populate_cache_leaves(unsigned int cpu)
>  static inline
>  int allocate_cache_info(int cpu)
>  {
> -	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
> -					 sizeof(struct cacheinfo), GFP_ATOMIC);
> +	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu), sizeof(struct cacheinfo), GFP_ATOMIC);
>  	if (!per_cpu_cacheinfo(cpu)) {
>  		cache_leaves(cpu) = 0;
>  		return -ENOMEM;
> @@ -539,7 +538,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
>  	 */
>  	ci_cacheinfo(cpu)->early_ci_levels = false;
>  
> -	if (cache_leaves(cpu) <= early_leaves)
> +	/*
> +	 * Some architectures (e.g., x86) do not use early initialization.
> +	 * Allocate memory now in such case.
> +	 */
> +	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
>  		return 0;
>  
>  	kfree(per_cpu_cacheinfo(cpu));

Since Borislav explicitly said you were not supposed to keep the tags,
I reviewed it again, and I'm OK to keep mine. Thanks!

Reviewed-by: Radu Rendec <rrendec@redhat.com>

--
Best regards,
Radu

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Ricardo Neri]

On Fri, Nov 29, 2024 at 11:12:52AM -0500, Radu Rendec wrote:
> On Wed, 2024-11-27 at 16:22 -0800, Ricardo Neri wrote:
> > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > adds functionality that architectures can use to optionally allocate and
> > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > arch specific early level initializer") lets secondary CPUs correct (and
> > reallocate memory) cacheinfo data if needed.
> > 
> > If the early build functionality is not used and cacheinfo does not need
> > correction, memory for cacheinfo is never allocated. x86 does not use the
> > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > pointer:
> > 
> >      BUG: kernel NULL pointer dereference, address: 0000000000000100
> >      #PF: supervisor read access in kernel mode
> >      #PF: error_code(0x0000) - not present page
> >      PGD 0 P4D 0
> >      Oops: 0000 [#1] PREEPMT SMP NOPTI
> >      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> >      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > 
> > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > not done earlier.
> > 
> > Moreover, before determining the validity of the last-level cache info,
> > ensure that it has been allocated. Simply checking for non-zero
> > cache_leaves() is not sufficient, as some architectures (e.g., Intel
> > processors) have non-zero cache_leaves() before allocation.
> > 
> > Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> > This function iterates over all online CPUs. However, a CPU may have come
> > online recently, but its cacheinfo may not have been allocated yet.
> > 
> > While here, remove an unnecessary indentation in allocate_cache_info().
> > 
> > Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > Reviewed-by: Radu Rendec <rrendec@redhat.com>
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > Tested-by: Andreas Herrmann <aherrmann@suse.de>
> > Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Cc: Andreas Herrmann <aherrmann@suse.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Chen Yu <yu.c.chen@intel.com>
> > Cc: Huang Ying <ying.huang@intel.com>
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: Nikolay Borisov <nik.borisov@suse.com>
> > Cc: Radu Rendec <rrendec@redhat.com>
> > Cc: Pierre Gondois <Pierre.Gondois@arm.com>
> > Cc: Pu Wen <puwen@hygon.cn>
> > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: stable@vger.kernel.org # 6.3+
> > ---
> > Change since v7:
> >  * None
> > 
> > Changes since v6:
> >  * Merged patches 1 and 2 of v6 into one. (Borislav)
> >  * Merged the history of patches 1 and 2ino this patch.
> >  * Kept the Reviewed-by and Tested-by tags from the two merged patches.
> >  * Fixed a formatting issue in allocate_cache_info(). (Borislav)
> > 
> > Changes since v5:
> >  * Fixed nonsensical subject (Nikolay).
> >  * Added Reviewed-by and Tested-by tags from Andreas. Thanks!
> >  * Added Reviewed-by tag from Nikolay. Thanks!
> > 
> > Changes since v4:
> >  * Combined checks for per_cpu_cacheinfo() and cache_leaves() in a single
> >    line. (Sudeep)
> >  * Added Reviewed-by tag from Sudeep. Thanks!
> > 
> > Changes since v3:
> >  * Added Reviewed-by tag from Radu and Sudeep. Thanks!
> > 
> > Changes since v2:
> >  * Introduced this patch.
> > 
> > Changes since v1:
> >  * N/A
> > 
> > ---
> > The motivation for commit 5944ce092b97 was to prevent a BUG splat in
> > PREEMPT_RT kernels during memory allocation. This splat is not observed on
> > x86 because the memory allocation for cacheinfo happens in
> > detect_cache_attributes() from the cacheinfo CPU hotplug callback.
> > 
> > The dereference of a NULL cacheinfo is not observed today because
> > cache_leaves(cpu) is zero until after init_cache_level() is called
> > (during the CPU hotplug callback). A subsequent changeset will set
> > the number of cache leaves earlier and the NULL-pointer dereference
> > will be observed.
> > ---
> >  drivers/base/cacheinfo.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> > index 7a7609298e18..a1afc478e0e8 100644
> > --- a/drivers/base/cacheinfo.c
> > +++ b/drivers/base/cacheinfo.c
> > @@ -58,7 +58,7 @@ bool last_level_cache_is_valid(unsigned int cpu)
> >  {
> >  	struct cacheinfo *llc;
> >  
> > -	if (!cache_leaves(cpu))
> > +	if (!cache_leaves(cpu) || !per_cpu_cacheinfo(cpu))
> >  		return false;
> >  
> >  	llc = per_cpu_cacheinfo_idx(cpu, cache_leaves(cpu) - 1);
> > @@ -466,8 +466,7 @@ int __weak populate_cache_leaves(unsigned int cpu)
> >  static inline
> >  int allocate_cache_info(int cpu)
> >  {
> > -	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
> > -					 sizeof(struct cacheinfo), GFP_ATOMIC);
> > +	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu), sizeof(struct cacheinfo), GFP_ATOMIC);
> >  	if (!per_cpu_cacheinfo(cpu)) {
> >  		cache_leaves(cpu) = 0;
> >  		return -ENOMEM;
> > @@ -539,7 +538,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
> >  	 */
> >  	ci_cacheinfo(cpu)->early_ci_levels = false;
> >  
> > -	if (cache_leaves(cpu) <= early_leaves)
> > +	/*
> > +	 * Some architectures (e.g., x86) do not use early initialization.
> > +	 * Allocate memory now in such case.
> > +	 */
> > +	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
> >  		return 0;
> >  
> >  	kfree(per_cpu_cacheinfo(cpu));
> 
> Since Borislav explicitly said you were not supposed to keep the tags,
> I reviewed it again, and I'm OK to keep mine. Thanks!
> 
> Reviewed-by: Radu Rendec <rrendec@redhat.com>

Thank you!

Re: [PATCH v8 0/2] x86/cacheinfo: Set the number of leaves per CPU

[Ricardo Neri]

On Thu, Nov 28, 2024 at 10:30:46AM +0100, Borislav Petkov wrote:
> On November 28, 2024 1:22:45 AM GMT+01:00, Ricardo Neri <ricardo.neri-calderon@linux.intel.com> wrote:
> >Changes since v7:
> > * Merged patches 2/3 into one. (Borislav)
> > * Dropped wrapper functions for ci_cpu_cacheinfo. (Borislav)
> > * Check for zero cache leaves in init_cache_level() for x86.
> >   (Borislav)
> > * Removed an ugly line break. (Borislav)
> >
> >Changes since v6:
> >  * Merged patches 1 and 2 into one. (Borislav)
> >  * Fixed an formatting issue in allocate_cache_info(). (Borislav)
> 
> I don't think you should keep the tags after those changes...


OK. Should I post a v9 without the tags? Perhaps I could wait a few days
in case reviewers and testers want to chime in.

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Nikolay Borisov]

On 28.11.24 г. 2:22 ч., Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>       BUG: kernel NULL pointer dereference, address: 0000000000000100
>       #PF: supervisor read access in kernel mode
>       #PF: error_code(0x0000) - not present page
>       PGD 0 P4D 0
>       Oops: 0000 [#1] PREEPMT SMP NOPTI
>       CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>       RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.
> 
> Moreover, before determining the validity of the last-level cache info,
> ensure that it has been allocated. Simply checking for non-zero
> cache_leaves() is not sufficient, as some architectures (e.g., Intel
> processors) have non-zero cache_leaves() before allocation.
> 
> Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> This function iterates over all online CPUs. However, a CPU may have come
> online recently, but its cacheinfo may not have been allocated yet.
> 
> While here, remove an unnecessary indentation in allocate_cache_info().
> 
> Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Andreas Herrmann <aherrmann@suse.de>
> Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>


Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Andreas Herrmann]

On Wed, Nov 27, 2024 at 04:22:46PM -0800, Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>      BUG: kernel NULL pointer dereference, address: 0000000000000100
>      #PF: supervisor read access in kernel mode
>      #PF: error_code(0x0000) - not present page
>      PGD 0 P4D 0
>      Oops: 0000 [#1] PREEPMT SMP NOPTI
>      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.
> 
> Moreover, before determining the validity of the last-level cache info,
> ensure that it has been allocated. Simply checking for non-zero
> cache_leaves() is not sufficient, as some architectures (e.g., Intel
> processors) have non-zero cache_leaves() before allocation.
> 
> Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> This function iterates over all online CPUs. However, a CPU may have come
> online recently, but its cacheinfo may not have been allocated yet.
> 
> While here, remove an unnecessary indentation in allocate_cache_info().
> 
> Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Andreas Herrmann <aherrmann@suse.de>
> Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---

Reviewed-by: Andreas Herrmann <aherrmann@suse.de>

-- 
Regards,
Andreas

Re: [PATCH v8 0/2] x86/cacheinfo: Set the number of leaves per CPU

[Borislav Petkov]

On Sun, Dec 01, 2024 at 02:41:26PM -0800, Ricardo Neri wrote:
> OK. Should I post a v9 without the tags? Perhaps I could wait a few days
> in case reviewers and testers want to chime in.

Yap, and then I can collect them all so you don't need to repost.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Sudeep Holla]

On Wed, Nov 27, 2024 at 04:22:46PM -0800, Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>      BUG: kernel NULL pointer dereference, address: 0000000000000100
>      #PF: supervisor read access in kernel mode
>      #PF: error_code(0x0000) - not present page
>      PGD 0 P4D 0
>      Oops: 0000 [#1] PREEPMT SMP NOPTI
>      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.
> 
> Moreover, before determining the validity of the last-level cache info,
> ensure that it has been allocated. Simply checking for non-zero
> cache_leaves() is not sufficient, as some architectures (e.g., Intel
> processors) have non-zero cache_leaves() before allocation.
> 
> Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> This function iterates over all online CPUs. However, a CPU may have come
> online recently, but its cacheinfo may not have been allocated yet.
> 
> While here, remove an unnecessary indentation in allocate_cache_info().
> 
> Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

In case old tags are deemed null 😄,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Ricardo Neri]

On Mon, Dec 02, 2024 at 11:03:49AM +0000, Sudeep Holla wrote:
> On Wed, Nov 27, 2024 at 04:22:46PM -0800, Ricardo Neri wrote:
> > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > adds functionality that architectures can use to optionally allocate and
> > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > arch specific early level initializer") lets secondary CPUs correct (and
> > reallocate memory) cacheinfo data if needed.
> > 
> > If the early build functionality is not used and cacheinfo does not need
> > correction, memory for cacheinfo is never allocated. x86 does not use the
> > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > pointer:
> > 
> >      BUG: kernel NULL pointer dereference, address: 0000000000000100
> >      #PF: supervisor read access in kernel mode
> >      #PF: error_code(0x0000) - not present page
> >      PGD 0 P4D 0
> >      Oops: 0000 [#1] PREEPMT SMP NOPTI
> >      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> >      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > 
> > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > not done earlier.
> > 
> > Moreover, before determining the validity of the last-level cache info,
> > ensure that it has been allocated. Simply checking for non-zero
> > cache_leaves() is not sufficient, as some architectures (e.g., Intel
> > processors) have non-zero cache_leaves() before allocation.
> > 
> > Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> > This function iterates over all online CPUs. However, a CPU may have come
> > online recently, but its cacheinfo may not have been allocated yet.
> > 
> > While here, remove an unnecessary indentation in allocate_cache_info().
> > 
> > Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > Reviewed-by: Radu Rendec <rrendec@redhat.com>
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> In case old tags are deemed null 😄,
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

Thank you!

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Ricardo Neri]

On Mon, Dec 02, 2024 at 09:02:03AM +0100, Andreas Herrmann wrote:
> On Wed, Nov 27, 2024 at 04:22:46PM -0800, Ricardo Neri wrote:
> > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > adds functionality that architectures can use to optionally allocate and
> > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > arch specific early level initializer") lets secondary CPUs correct (and
> > reallocate memory) cacheinfo data if needed.
> > 
> > If the early build functionality is not used and cacheinfo does not need
> > correction, memory for cacheinfo is never allocated. x86 does not use the
> > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > pointer:
> > 
> >      BUG: kernel NULL pointer dereference, address: 0000000000000100
> >      #PF: supervisor read access in kernel mode
> >      #PF: error_code(0x0000) - not present page
> >      PGD 0 P4D 0
> >      Oops: 0000 [#1] PREEPMT SMP NOPTI
> >      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> >      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > 
> > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > not done earlier.
> > 
> > Moreover, before determining the validity of the last-level cache info,
> > ensure that it has been allocated. Simply checking for non-zero
> > cache_leaves() is not sufficient, as some architectures (e.g., Intel
> > processors) have non-zero cache_leaves() before allocation.
> > 
> > Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> > This function iterates over all online CPUs. However, a CPU may have come
> > online recently, but its cacheinfo may not have been allocated yet.
> > 
> > While here, remove an unnecessary indentation in allocate_cache_info().
> > 
> > Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > Reviewed-by: Radu Rendec <rrendec@redhat.com>
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > Tested-by: Andreas Herrmann <aherrmann@suse.de>
> > Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> 
> Reviewed-by: Andreas Herrmann <aherrmann@suse.de>

Thank you!

Re: [PATCH v8 1/2] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[Ricardo Neri]

On Mon, Dec 02, 2024 at 08:53:44AM +0200, Nikolay Borisov wrote:
> 
> 
> On 28.11.24 г. 2:22 ч., Ricardo Neri wrote:
> > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > adds functionality that architectures can use to optionally allocate and
> > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > arch specific early level initializer") lets secondary CPUs correct (and
> > reallocate memory) cacheinfo data if needed.
> > 
> > If the early build functionality is not used and cacheinfo does not need
> > correction, memory for cacheinfo is never allocated. x86 does not use the
> > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > pointer:
> > 
> >       BUG: kernel NULL pointer dereference, address: 0000000000000100
> >       #PF: supervisor read access in kernel mode
> >       #PF: error_code(0x0000) - not present page
> >       PGD 0 P4D 0
> >       Oops: 0000 [#1] PREEPMT SMP NOPTI
> >       CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> >       RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > 
> > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > not done earlier.
> > 
> > Moreover, before determining the validity of the last-level cache info,
> > ensure that it has been allocated. Simply checking for non-zero
> > cache_leaves() is not sufficient, as some architectures (e.g., Intel
> > processors) have non-zero cache_leaves() before allocation.
> > 
> > Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
> > This function iterates over all online CPUs. However, a CPU may have come
> > online recently, but its cacheinfo may not have been allocated yet.
> > 
> > While here, remove an unnecessary indentation in allocate_cache_info().
> > 
> > Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > Reviewed-by: Radu Rendec <rrendec@redhat.com>
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > Tested-by: Andreas Herrmann <aherrmann@suse.de>
> > Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> 
> 
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

Thank you!

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Borislav Petkov]

On Wed, Nov 27, 2024 at 04:22:47PM -0800, Ricardo Neri wrote:
>  arch/x86/kernel/cpu/cacheinfo.c | 41 +++++++++++++++------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)

Does that work too?

---
diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 95e38ab98a72..e6fa03ed9172 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -715,22 +715,23 @@ void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c)
 
 void init_amd_cacheinfo(struct cpuinfo_x86 *c)
 {
-
-	unsigned int cpu = c->cpu_index;
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
 
 	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
-		get_cpu_cacheinfo(cpu)->num_leaves = find_num_cache_leaves(c);
+		ci->num_leaves = find_num_cache_leaves(c);
 	} else if (c->extended_cpuid_level >= 0x80000006) {
 		if (cpuid_edx(0x80000006) & 0xf000)
-			get_cpu_cacheinfo(cpu)->num_leaves = 4;
+			ci->num_leaves = 4;
 		else
-			get_cpu_cacheinfo(cpu)->num_leaves = 3;
+			ci->num_leaves = 3;
 	}
 }
 
 void init_hygon_cacheinfo(struct cpuinfo_x86 *c)
 {
-	get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
+
+	ci->num_leaves = find_num_cache_leaves(c);
 }
 
 void init_intel_cacheinfo(struct cpuinfo_x86 *c)
@@ -740,20 +741,21 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	unsigned int new_l1d = 0, new_l1i = 0; /* Cache sizes from cpuid(4) */
 	unsigned int new_l2 = 0, new_l3 = 0, i; /* Cache sizes from cpuid(4) */
 	unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
 
 	if (c->cpuid_level > 3) {
 		/*
 		 * There should be at least one leaf. A non-zero value means
 		 * that the number of leaves has been initialized.
 		 */
-		if (!get_cpu_cacheinfo(c->cpu_index)->num_leaves)
-			get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
+		if (!ci->num_leaves)
+			ci->num_leaves = find_num_cache_leaves(c);
 
 		/*
 		 * Whenever possible use cpuid(4), deterministic cache
 		 * parameters cpuid leaf to find the cache details
 		 */
-		for (i = 0; i < get_cpu_cacheinfo(c->cpu_index)->num_leaves; i++) {
+		for (i = 0; i < ci->num_leaves; i++) {
 			struct _cpuid4_info_regs this_leaf = {};
 			int retval;
 
@@ -789,14 +791,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	 * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
 	 * trace cache
 	 */
-	if ((!get_cpu_cacheinfo(c->cpu_index)->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
+	if ((!ci->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
 		/* supports eax=2  call */
 		int j, n;
 		unsigned int regs[4];
 		unsigned char *dp = (unsigned char *)regs;
 		int only_trace = 0;
 
-		if (get_cpu_cacheinfo(c->cpu_index)->num_leaves && c->x86 == 15)
+		if (ci->num_leaves && c->x86 == 15)
 			only_trace = 1;
 
 		/* Number of times to iterate */
@@ -990,8 +992,10 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 
 int init_cache_level(unsigned int cpu)
 {
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
+
 	/* There should be at least one leaf. */
-	if (!get_cpu_cacheinfo(cpu)->num_leaves)
+	if (!ci->num_leaves)
 		return -ENOENT;
 
 	return 0;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Ricardo Neri]

On Wed, Dec 04, 2024 at 03:32:06PM +0100, Borislav Petkov wrote:
> On Wed, Nov 27, 2024 at 04:22:47PM -0800, Ricardo Neri wrote:
> >  arch/x86/kernel/cpu/cacheinfo.c | 41 +++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> Does that work too?
> 
> ---
> diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
> index 95e38ab98a72..e6fa03ed9172 100644
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> @@ -715,22 +715,23 @@ void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c)
>  
>  void init_amd_cacheinfo(struct cpuinfo_x86 *c)
>  {
> -
> -	unsigned int cpu = c->cpu_index;
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
>  
>  	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
> -		get_cpu_cacheinfo(cpu)->num_leaves = find_num_cache_leaves(c);
> +		ci->num_leaves = find_num_cache_leaves(c);
>  	} else if (c->extended_cpuid_level >= 0x80000006) {
>  		if (cpuid_edx(0x80000006) & 0xf000)
> -			get_cpu_cacheinfo(cpu)->num_leaves = 4;
> +			ci->num_leaves = 4;
>  		else
> -			get_cpu_cacheinfo(cpu)->num_leaves = 3;
> +			ci->num_leaves = 3;
>  	}
>  }
>  
>  void init_hygon_cacheinfo(struct cpuinfo_x86 *c)
>  {
> -	get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
> +
> +	ci->num_leaves = find_num_cache_leaves(c);
>  }
>  
>  void init_intel_cacheinfo(struct cpuinfo_x86 *c)
> @@ -740,20 +741,21 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
>  	unsigned int new_l1d = 0, new_l1i = 0; /* Cache sizes from cpuid(4) */
>  	unsigned int new_l2 = 0, new_l3 = 0, i; /* Cache sizes from cpuid(4) */
>  	unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
>  
>  	if (c->cpuid_level > 3) {
>  		/*
>  		 * There should be at least one leaf. A non-zero value means
>  		 * that the number of leaves has been initialized.
>  		 */
> -		if (!get_cpu_cacheinfo(c->cpu_index)->num_leaves)
> -			get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
> +		if (!ci->num_leaves)
> +			ci->num_leaves = find_num_cache_leaves(c);
>  
>  		/*
>  		 * Whenever possible use cpuid(4), deterministic cache
>  		 * parameters cpuid leaf to find the cache details
>  		 */
> -		for (i = 0; i < get_cpu_cacheinfo(c->cpu_index)->num_leaves; i++) {
> +		for (i = 0; i < ci->num_leaves; i++) {
>  			struct _cpuid4_info_regs this_leaf = {};
>  			int retval;
>  
> @@ -789,14 +791,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
>  	 * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
>  	 * trace cache
>  	 */
> -	if ((!get_cpu_cacheinfo(c->cpu_index)->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
> +	if ((!ci->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
>  		/* supports eax=2  call */
>  		int j, n;
>  		unsigned int regs[4];
>  		unsigned char *dp = (unsigned char *)regs;
>  		int only_trace = 0;
>  
> -		if (get_cpu_cacheinfo(c->cpu_index)->num_leaves && c->x86 == 15)
> +		if (ci->num_leaves && c->x86 == 15)
>  			only_trace = 1;
>  
>  		/* Number of times to iterate */
> @@ -990,8 +992,10 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
>  
>  int init_cache_level(unsigned int cpu)
>  {
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> +
>  	/* There should be at least one leaf. */
> -	if (!get_cpu_cacheinfo(cpu)->num_leaves)
> +	if (!ci->num_leaves)
>  		return -ENOENT;
>  
>  	return 0;

Yes, this looks OK to me and is an improvement.

I can post a v9 with these changes.

I think I can keep the Reviewed-by tags as they were given to Patch 1.

Thanks and BR,
Ricardo

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Borislav Petkov]

On Wed, Dec 04, 2024 at 08:39:11AM -0800, Ricardo Neri wrote:
> Yes, this looks OK to me and is an improvement.
> 
> I can post a v9 with these changes.

No need - I have everything here. I can give you a branch to test one last
time before I queue.

> I think I can keep the Reviewed-by tags as they were given to Patch 1.

I'll take care of that too.

Just sit tight and wait for a note from me. :)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Ricardo Neri]

On Wed, Dec 04, 2024 at 08:32:05PM +0100, Borislav Petkov wrote:
> On Wed, Dec 04, 2024 at 08:39:11AM -0800, Ricardo Neri wrote:
> > Yes, this looks OK to me and is an improvement.
> > 
> > I can post a v9 with these changes.
> 
> No need - I have everything here. I can give you a branch to test one last
> time before I queue.

Sure! Thanks! I can test it on Meteor Lake and non-Meteor Lake.

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Borislav Petkov]

On Wed, Dec 04, 2024 at 02:22:38PM -0800, Ricardo Neri wrote:
> Sure! Thanks! I can test it on Meteor Lake and non-Meteor Lake.

Ok, pls run this:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-urgent-wip

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Ricardo Neri]

On Thu, Dec 05, 2024 at 04:14:45PM +0100, Borislav Petkov wrote:
> On Wed, Dec 04, 2024 at 02:22:38PM -0800, Ricardo Neri wrote:
> > Sure! Thanks! I can test it on Meteor Lake and non-Meteor Lake.
> 
> Ok, pls run this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-urgent-wip

I tested this branch on Alder Lake, Meteor Lake, Sapphire Rapids,
Broadwell server, Granite Rapids, Icelake server, Sierra Forest, and Rome.
/sys/devices/system/cpu/cpu*/cache is populated correctly.

I ran the tests documented in

https://lore.kernel.org/lkml/20230912032350.GA17008@ranerica-svr.sc.intel.com/

Thanks and BR,
Ricardo

Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Borislav Petkov]

On Thu, Dec 05, 2024 at 05:47:29PM -0800, Ricardo Neri wrote:
> I tested this branch on Alder Lake, Meteor Lake, Sapphire Rapids,
> Broadwell server, Granite Rapids, Icelake server, Sierra Forest, and Rome.
> /sys/devices/system/cpu/cpu*/cache is populated correctly.

Thanks!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[tip: x86/urgent] x86/cacheinfo: Delete global num_cache_leaves

[tip-bot2 for Ricardo Neri]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     9677be09e5e4fbe48aeccb06ae3063c5eba331c3
Gitweb:        https://git.kernel.org/tip/9677be09e5e4fbe48aeccb06ae3063c5eba331c3
Author:        Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
AuthorDate:    Wed, 27 Nov 2024 16:22:47 -08:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Fri, 06 Dec 2024 13:13:36 +01:00

x86/cacheinfo: Delete global num_cache_leaves

Linux remembers cpu_cachinfo::num_leaves per CPU, but x86 initializes all
CPUs from the same global "num_cache_leaves".

This is erroneous on systems such as Meteor Lake, where each CPU has a
distinct num_leaves value. Delete the global "num_cache_leaves" and
initialize num_leaves on each CPU.

init_cache_level() no longer needs to set num_leaves. Also, it never had to
set num_levels as it is unnecessary in x86. Keep checking for zero cache
leaves. Such condition indicates a bug.

  [ bp: Cleanup. ]

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@vger.kernel.org # 6.3+
Link: https://lore.kernel.org/r/20241128002247.26726-3-ricardo.neri-calderon@linux.intel.com
---
 arch/x86/kernel/cpu/cacheinfo.c | 43 +++++++++++++++-----------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 392d09c..e6fa03e 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -178,8 +178,6 @@ struct _cpuid4_info_regs {
 	struct amd_northbridge *nb;
 };
 
-static unsigned short num_cache_leaves;
-
 /* 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.
@@ -717,20 +715,23 @@ void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c)
 
 void init_amd_cacheinfo(struct cpuinfo_x86 *c)
 {
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
 
 	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
-		num_cache_leaves = find_num_cache_leaves(c);
+		ci->num_leaves = find_num_cache_leaves(c);
 	} else if (c->extended_cpuid_level >= 0x80000006) {
 		if (cpuid_edx(0x80000006) & 0xf000)
-			num_cache_leaves = 4;
+			ci->num_leaves = 4;
 		else
-			num_cache_leaves = 3;
+			ci->num_leaves = 3;
 	}
 }
 
 void init_hygon_cacheinfo(struct cpuinfo_x86 *c)
 {
-	num_cache_leaves = find_num_cache_leaves(c);
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
+
+	ci->num_leaves = find_num_cache_leaves(c);
 }
 
 void init_intel_cacheinfo(struct cpuinfo_x86 *c)
@@ -740,21 +741,21 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	unsigned int new_l1d = 0, new_l1i = 0; /* Cache sizes from cpuid(4) */
 	unsigned int new_l2 = 0, new_l3 = 0, i; /* Cache sizes from cpuid(4) */
 	unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
 
 	if (c->cpuid_level > 3) {
-		static int is_initialized;
-
-		if (is_initialized == 0) {
-			/* Init num_cache_leaves from boot CPU */
-			num_cache_leaves = find_num_cache_leaves(c);
-			is_initialized++;
-		}
+		/*
+		 * There should be at least one leaf. A non-zero value means
+		 * that the number of leaves has been initialized.
+		 */
+		if (!ci->num_leaves)
+			ci->num_leaves = find_num_cache_leaves(c);
 
 		/*
 		 * Whenever possible use cpuid(4), deterministic cache
 		 * parameters cpuid leaf to find the cache details
 		 */
-		for (i = 0; i < num_cache_leaves; i++) {
+		for (i = 0; i < ci->num_leaves; i++) {
 			struct _cpuid4_info_regs this_leaf = {};
 			int retval;
 
@@ -790,14 +791,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	 * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
 	 * trace cache
 	 */
-	if ((num_cache_leaves == 0 || c->x86 == 15) && c->cpuid_level > 1) {
+	if ((!ci->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
 		/* supports eax=2  call */
 		int j, n;
 		unsigned int regs[4];
 		unsigned char *dp = (unsigned char *)regs;
 		int only_trace = 0;
 
-		if (num_cache_leaves != 0 && c->x86 == 15)
+		if (ci->num_leaves && c->x86 == 15)
 			only_trace = 1;
 
 		/* Number of times to iterate */
@@ -991,14 +992,12 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 
 int init_cache_level(unsigned int cpu)
 {
-	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
 
-	if (!num_cache_leaves)
+	/* There should be at least one leaf. */
+	if (!ci->num_leaves)
 		return -ENOENT;
-	if (!this_cpu_ci)
-		return -EINVAL;
-	this_cpu_ci->num_levels = 3;
-	this_cpu_ci->num_leaves = num_cache_leaves;
+
 	return 0;
 }
 

[tip: x86/urgent] cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

[tip-bot2 for Ricardo Neri]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     b3fce429a1e030b50c1c91351d69b8667eef627b
Gitweb:        https://git.kernel.org/tip/b3fce429a1e030b50c1c91351d69b8667eef627b
Author:        Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
AuthorDate:    Wed, 27 Nov 2024 16:22:46 -08:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Fri, 06 Dec 2024 13:07:47 +01:00

cacheinfo: Allocate memory during CPU hotplug if not done from the primary CPU

Commit

  5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")

adds functionality that architectures can use to optionally allocate and
build cacheinfo early during boot. Commit

  6539cffa9495 ("cacheinfo: Add arch specific early level initializer")

lets secondary CPUs correct (and reallocate memory) cacheinfo data if
needed.

If the early build functionality is not used and cacheinfo does not need
correction, memory for cacheinfo is never allocated. x86 does not use
the early build functionality. Consequently, during the cacheinfo CPU
hotplug callback, last_level_cache_is_valid() attempts to dereference
a NULL pointer:

  BUG: kernel NULL pointer dereference, address: 0000000000000100
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not present page
  PGD 0 P4D 0
  Oops: 0000 [#1] PREEPMT SMP NOPTI
  CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
  RIP: 0010: last_level_cache_is_valid+0x95/0xe0a

Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback
if not done earlier.

Moreover, before determining the validity of the last-level cache info,
ensure that it has been allocated. Simply checking for non-zero
cache_leaves() is not sufficient, as some architectures (e.g., Intel
processors) have non-zero cache_leaves() before allocation.

Dereferencing NULL cacheinfo can occur in update_per_cpu_data_slice_size().
This function iterates over all online CPUs. However, a CPU may have come
online recently, but its cacheinfo may not have been allocated yet.

While here, remove an unnecessary indentation in allocate_cache_info().

  [ bp: Massage. ]

Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Radu Rendec <rrendec@redhat.com>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: stable@vger.kernel.org # 6.3+
Link: https://lore.kernel.org/r/20241128002247.26726-2-ricardo.neri-calderon@linux.intel.com
---
 drivers/base/cacheinfo.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 609935a..cf0d455 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -58,7 +58,7 @@ bool last_level_cache_is_valid(unsigned int cpu)
 {
 	struct cacheinfo *llc;
 
-	if (!cache_leaves(cpu))
+	if (!cache_leaves(cpu) || !per_cpu_cacheinfo(cpu))
 		return false;
 
 	llc = per_cpu_cacheinfo_idx(cpu, cache_leaves(cpu) - 1);
@@ -458,11 +458,9 @@ int __weak populate_cache_leaves(unsigned int cpu)
 	return -ENOENT;
 }
 
-static inline
-int allocate_cache_info(int cpu)
+static inline int allocate_cache_info(int cpu)
 {
-	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
-					 sizeof(struct cacheinfo), GFP_ATOMIC);
+	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu), sizeof(struct cacheinfo), GFP_ATOMIC);
 	if (!per_cpu_cacheinfo(cpu)) {
 		cache_leaves(cpu) = 0;
 		return -ENOMEM;
@@ -534,7 +532,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
 	 */
 	ci_cacheinfo(cpu)->early_ci_levels = false;
 
-	if (cache_leaves(cpu) <= early_leaves)
+	/*
+	 * Some architectures (e.g., x86) do not use early initialization.
+	 * Allocate memory now in such case.
+	 */
+	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
 		return 0;
 
 	kfree(per_cpu_cacheinfo(cpu));