[PATCH v3 0/5] arch_topology: Build cacheinfo from primary CPU

[PATCH v3 0/5] arch_topology: Build cacheinfo from primary CPU

[Pierre Gondois]

v2:
 - Applied renaming/formatting comments from v1.
 - Check CACHE_TYPE_VALID flag in pppt.c.
v3:
 - Applied Sudeep's suggestions (for patch 5/5):
   - Renaming allocate_cache_info() -> fecth_cache_info()
   - Updated error message
   - Extract an inline allocate_cache_info() function
 - Re-run checkpatch with --strict option

Note:
This patchset requires the following patch to be applied first in
order to avoid the same bug described in the commit message:
https://lore.kernel.org/all/20221116094958.2141072-1-pierre.gondois@arm.com/

[1] and [2] build the CPU topology from the cacheinfo information for
both DT/ACPI based systems and remove (struct cpu_topology).llc_id
which was used by ACPI only.

Creating the cacheinfo for secondary CPUs is done during early boot.
Preemption and interrupts are disabled at this stage. On PREEMPT_RT
kernels, allocating memory (and parsing the PPTT table for ACPI based
systems) triggers a:
  'BUG: sleeping function called from invalid context' [4]

To prevent this bug, allocate the cacheinfo from the primary CPU when
preemption and interrupts are enabled and before booting secondary
CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
only, without relying on the arm64 CLIDR_EL1 register.
If no cache information is found in the DT/ACPI PPTT, then fallback
to the current state, triggering [4] on PREEMPT_RT kernels.

Patches to update the arm64 device trees that have incomplete cacheinfo
(mostly for missing the 'cache-level' or 'cache-unified' property)
have been sent at [3].

Tested platforms:
- ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
  Kunpeng 920, Juno-r2
- DT: rb5, db845c, Juno-r2

[1] https://lore.kernel.org/all/20220704101605.1318280-1-sudeep.holla@arm.com/
[2] https://lore.kernel.org/all/20220720-arch_topo_fixes-v3-0-43d696288e84@arm.com/
[3] https://lore.kernel.org/all/20221107155825.1644604-1-pierre.gondois@arm.com/
[4] On an Ampere Altra, with PREEMPT_RT kernel based on v6.0.0-rc4:


[    7.560791] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
[    7.560794] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/111
[    7.560796] preempt_count: 1, expected: 0
[    7.560797] RCU nest depth: 1, expected: 1
[    7.560799] 3 locks held by swapper/111/0:
[    7.560800]  #0: ffff403e406cae98 (&pcp->lock){+.+.}-{3:3}, at: get_page_from_freelist+0x218/0x12c8
[    7.560811]  #1: ffffc5f8ed09f8e8 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x48/0xf0
[    7.560820]  #2: ffff403f400b4fd8 (&zone->lock){+.+.}-{3:3}, at: rmqueue_bulk+0x64/0xa80
[    7.560824] irq event stamp: 0
[    7.560825] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
[    7.560827] hardirqs last disabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[    7.560830] softirqs last  enabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[    7.560833] softirqs last disabled at (0): [<0000000000000000>] 0x0
[    7.560834] Preemption disabled at:
[    7.560835] [<ffffc5f8e9fd3c28>] migrate_enable+0x30/0x130
[    7.560838] CPU: 111 PID: 0 Comm: swapper/111 Tainted: G        W          6.0.0-rc4-[...]
[    7.560841] Call trace:
[...]
[    7.560870]  __kmalloc+0xbc/0x1e8
[    7.560873]  detect_cache_attributes+0x2d4/0x5f0
[    7.560876]  update_siblings_masks+0x30/0x368
[    7.560880]  store_cpu_topology+0x78/0xb8
[    7.560883]  secondary_start_kernel+0xd0/0x198
[    7.560885]  __secondary_switched+0xb0/0xb4

Pierre Gondois (5):
  cacheinfo: Use RISC-V's init_cache_level() as generic OF
    implementation
  cacheinfo: Return error code in init_of_cache_level()
  ACPI: PPTT: Remove acpi_find_cache_levels()
  ACPI: PPTT: Update acpi_find_last_cache_level() to
    acpi_get_cache_info()
  arch_topology: Build cacheinfo from primary CPU

 arch/arm64/kernel/cacheinfo.c |  11 ++--
 arch/riscv/kernel/cacheinfo.c |  39 +----------
 drivers/acpi/pptt.c           |  93 +++++++++++++++-----------
 drivers/base/arch_topology.c  |  12 +++-
 drivers/base/cacheinfo.c      | 119 +++++++++++++++++++++++++++++-----
 include/linux/cacheinfo.h     |  11 +++-
 6 files changed, 182 insertions(+), 103 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/5] cacheinfo: Use RISC-V's init_cache_level() as generic OF implementation

[Pierre Gondois]

RISC-V's implementation of init_of_cache_level() is following
the Devicetree Specification v0.3 regarding caches, cf.:
- s3.7.3 'Internal (L1) Cache Properties'
- s3.8 'Multi-level and Shared Cache Nodes'

Allow reusing the implementation by moving it.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 arch/riscv/kernel/cacheinfo.c | 39 +------------------------------
 drivers/base/cacheinfo.c      | 44 +++++++++++++++++++++++++++++++++++
 include/linux/cacheinfo.h     |  1 +
 3 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 90deabfe63ea..440a3df5944c 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -115,44 +115,7 @@ static void fill_cacheinfo(struct cacheinfo **this_leaf,
 
 int init_cache_level(unsigned int cpu)
 {
-	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
-	struct device_node *np = of_cpu_device_node_get(cpu);
-	struct device_node *prev = NULL;
-	int levels = 0, leaves = 0, level;
-
-	if (of_property_read_bool(np, "cache-size"))
-		++leaves;
-	if (of_property_read_bool(np, "i-cache-size"))
-		++leaves;
-	if (of_property_read_bool(np, "d-cache-size"))
-		++leaves;
-	if (leaves > 0)
-		levels = 1;
-
-	prev = np;
-	while ((np = of_find_next_cache_node(np))) {
-		of_node_put(prev);
-		prev = np;
-		if (!of_device_is_compatible(np, "cache"))
-			break;
-		if (of_property_read_u32(np, "cache-level", &level))
-			break;
-		if (level <= levels)
-			break;
-		if (of_property_read_bool(np, "cache-size"))
-			++leaves;
-		if (of_property_read_bool(np, "i-cache-size"))
-			++leaves;
-		if (of_property_read_bool(np, "d-cache-size"))
-			++leaves;
-		levels = level;
-	}
-
-	of_node_put(np);
-	this_cpu_ci->num_levels = levels;
-	this_cpu_ci->num_leaves = leaves;
-
-	return 0;
+	return init_of_cache_level(cpu);
 }
 
 int populate_cache_leaves(unsigned int cpu)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 4b5cd08c5a65..a4308b48dd3e 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -224,8 +224,52 @@ static int cache_setup_of_node(unsigned int cpu)
 
 	return 0;
 }
+
+int init_of_cache_level(unsigned int cpu)
+{
+	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+	struct device_node *np = of_cpu_device_node_get(cpu);
+	struct device_node *prev = NULL;
+	int levels = 0, leaves = 0, level;
+
+	if (of_property_read_bool(np, "cache-size"))
+		++leaves;
+	if (of_property_read_bool(np, "i-cache-size"))
+		++leaves;
+	if (of_property_read_bool(np, "d-cache-size"))
+		++leaves;
+	if (leaves > 0)
+		levels = 1;
+
+	prev = np;
+	while ((np = of_find_next_cache_node(np))) {
+		of_node_put(prev);
+		prev = np;
+		if (!of_device_is_compatible(np, "cache"))
+			break;
+		if (of_property_read_u32(np, "cache-level", &level))
+			break;
+		if (level <= levels)
+			break;
+		if (of_property_read_bool(np, "cache-size"))
+			++leaves;
+		if (of_property_read_bool(np, "i-cache-size"))
+			++leaves;
+		if (of_property_read_bool(np, "d-cache-size"))
+			++leaves;
+		levels = level;
+	}
+
+	of_node_put(np);
+	this_cpu_ci->num_levels = levels;
+	this_cpu_ci->num_leaves = leaves;
+
+	return 0;
+}
+
 #else
 static inline int cache_setup_of_node(unsigned int cpu) { return 0; }
+int init_of_cache_level(unsigned int cpu) { return 0; }
 #endif
 
 int __weak cache_setup_acpi(unsigned int cpu)
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index 00b7a6ae8617..ff0328f3fbb0 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -80,6 +80,7 @@ struct cpu_cacheinfo {
 
 struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu);
 int init_cache_level(unsigned int cpu);
+int init_of_cache_level(unsigned int cpu);
 int populate_cache_leaves(unsigned int cpu);
 int cache_setup_acpi(unsigned int cpu);
 bool last_level_cache_is_valid(unsigned int cpu);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/5] cacheinfo: Return error code in init_of_cache_level()

[Pierre Gondois]

Make init_of_cache_level() return an error code when the cache
information parsing fails to help detecting missing information.

init_of_cache_level() is only called for riscv. Returning an error
code instead of 0 will prevent detect_cache_attributes() to allocate
memory if an incomplete DT is parsed.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/base/cacheinfo.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index a4308b48dd3e..6f6cd120c4f1 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -246,11 +246,11 @@ int init_of_cache_level(unsigned int cpu)
 		of_node_put(prev);
 		prev = np;
 		if (!of_device_is_compatible(np, "cache"))
-			break;
+			goto err_out;
 		if (of_property_read_u32(np, "cache-level", &level))
-			break;
+			goto err_out;
 		if (level <= levels)
-			break;
+			goto err_out;
 		if (of_property_read_bool(np, "cache-size"))
 			++leaves;
 		if (of_property_read_bool(np, "i-cache-size"))
@@ -265,6 +265,10 @@ int init_of_cache_level(unsigned int cpu)
 	this_cpu_ci->num_leaves = leaves;
 
 	return 0;
+
+err_out:
+	of_node_put(np);
+	return -EINVAL;
 }
 
 #else
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/5] ACPI: PPTT: Remove acpi_find_cache_levels()

[Pierre Gondois]

acpi_find_cache_levels() is used at a single place and is short
enough to be merged into the calling function. The removal allows
an easier renaming of the calling function in the next patch.

Also reorder the local variables in the 'reversed Christmas tree'
order.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Rafael J. Wysocki  <rafael.j.wysocki@intel.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/acpi/pptt.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index c91342dcbcd6..97c1d33822d1 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -281,19 +281,6 @@ static struct acpi_pptt_processor *acpi_find_processor_node(struct acpi_table_he
 	return NULL;
 }
 
-static int acpi_find_cache_levels(struct acpi_table_header *table_hdr,
-				  u32 acpi_cpu_id)
-{
-	int number_of_levels = 0;
-	struct acpi_pptt_processor *cpu;
-
-	cpu = acpi_find_processor_node(table_hdr, acpi_cpu_id);
-	if (cpu)
-		number_of_levels = acpi_count_levels(table_hdr, cpu);
-
-	return number_of_levels;
-}
-
 static u8 acpi_cache_type(enum cache_type type)
 {
 	switch (type) {
@@ -613,9 +600,10 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
  */
 int acpi_find_last_cache_level(unsigned int cpu)
 {
-	u32 acpi_cpu_id;
+	struct acpi_pptt_processor *cpu_node;
 	struct acpi_table_header *table;
 	int number_of_levels = 0;
+	u32 acpi_cpu_id;
 
 	table = acpi_get_pptt();
 	if (!table)
@@ -624,7 +612,10 @@ int acpi_find_last_cache_level(unsigned int cpu)
 	pr_debug("Cache Setup find last level CPU=%d\n", cpu);
 
 	acpi_cpu_id = get_acpi_id_for_cpu(cpu);
-	number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
+	cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
+	if (cpu_node)
+		number_of_levels = acpi_count_levels(table, cpu_node);
+
 	pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
 
 	return number_of_levels;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/5] ACPI: PPTT: Update acpi_find_last_cache_level() to acpi_get_cache_info()

[Pierre Gondois]

acpi_find_last_cache_level() allows to find the last level of cache
for a given CPU. The function is only called on arm64 ACPI based
platforms to check for cache information that would be missing in
the CLIDR_EL1 register.
To allow populating (struct cpu_cacheinfo).num_leaves by only parsing
a PPTT, update acpi_find_last_cache_level() to get the 'split_levels',
i.e. the number of cache levels being split in data/instruction
caches.

It is assumed that there will not be data/instruction caches above a
unified cache.
If a split level consist of one data cache and no instruction cache
(or opposite), then the missing cache will still be populated
by default with minimal cache information, and maximal cpumask
(all non-existing caches have the same fw_token).

Suggested-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Rafael J. Wysocki  <rafael.j.wysocki@intel.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 arch/arm64/kernel/cacheinfo.c | 11 +++--
 drivers/acpi/pptt.c           | 76 +++++++++++++++++++++++------------
 include/linux/cacheinfo.h     |  9 +++--
 3 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 97c42be71338..36c3b07cdf2d 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -46,7 +46,7 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
 int init_cache_level(unsigned int cpu)
 {
 	unsigned int ctype, level, leaves;
-	int fw_level;
+	int fw_level, ret;
 	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
 
 	for (level = 1, leaves = 0; level <= MAX_CACHE_LEVEL; level++) {
@@ -59,10 +59,13 @@ int init_cache_level(unsigned int cpu)
 		leaves += (ctype == CACHE_TYPE_SEPARATE) ? 2 : 1;
 	}
 
-	if (acpi_disabled)
+	if (acpi_disabled) {
 		fw_level = of_find_last_cache_level(cpu);
-	else
-		fw_level = acpi_find_last_cache_level(cpu);
+	} else {
+		ret = acpi_get_cache_info(cpu, &fw_level, NULL);
+		if (ret < 0)
+			return ret;
+	}
 
 	if (fw_level < 0)
 		return fw_level;
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 97c1d33822d1..10975bb603fb 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -81,6 +81,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
  * acpi_pptt_walk_cache() - Attempt to find the requested acpi_pptt_cache
  * @table_hdr: Pointer to the head of the PPTT table
  * @local_level: passed res reflects this cache level
+ * @split_levels: Number of split cache levels (data/instruction).
  * @res: cache resource in the PPTT we want to walk
  * @found: returns a pointer to the requested level if found
  * @level: the requested cache level
@@ -100,6 +101,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
  */
 static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
 					 unsigned int local_level,
+					 unsigned int *split_levels,
 					 struct acpi_subtable_header *res,
 					 struct acpi_pptt_cache **found,
 					 unsigned int level, int type)
@@ -113,8 +115,17 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
 	while (cache) {
 		local_level++;
 
+		if (!(cache->flags & ACPI_PPTT_CACHE_TYPE_VALID)) {
+			cache = fetch_pptt_cache(table_hdr, cache->next_level_of_cache);
+			continue;
+		}
+
+		if (split_levels &&
+		    (acpi_pptt_match_type(cache->attributes, ACPI_PPTT_CACHE_TYPE_DATA) ||
+		     acpi_pptt_match_type(cache->attributes, ACPI_PPTT_CACHE_TYPE_INSTR)))
+			*split_levels = local_level;
+
 		if (local_level == level &&
-		    cache->flags & ACPI_PPTT_CACHE_TYPE_VALID &&
 		    acpi_pptt_match_type(cache->attributes, type)) {
 			if (*found != NULL && cache != *found)
 				pr_warn("Found duplicate cache level/type unable to determine uniqueness\n");
@@ -135,8 +146,8 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
 static struct acpi_pptt_cache *
 acpi_find_cache_level(struct acpi_table_header *table_hdr,
 		      struct acpi_pptt_processor *cpu_node,
-		      unsigned int *starting_level, unsigned int level,
-		      int type)
+		      unsigned int *starting_level, unsigned int *split_levels,
+		      unsigned int level, int type)
 {
 	struct acpi_subtable_header *res;
 	unsigned int number_of_levels = *starting_level;
@@ -149,7 +160,8 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
 		resource++;
 
 		local_level = acpi_pptt_walk_cache(table_hdr, *starting_level,
-						   res, &ret, level, type);
+						   split_levels, res, &ret,
+						   level, type);
 		/*
 		 * we are looking for the max depth. Since its potentially
 		 * possible for a given node to have resources with differing
@@ -165,29 +177,29 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
 }
 
 /**
- * acpi_count_levels() - Given a PPTT table, and a CPU node, count the caches
+ * acpi_count_levels() - Given a PPTT table, and a CPU node, count the cache
+ * levels and split cache levels (data/instruction).
  * @table_hdr: Pointer to the head of the PPTT table
  * @cpu_node: processor node we wish to count caches for
+ * @levels: Number of levels if success.
+ * @split_levels:	Number of split cache levels (data/instruction) if
+ *			success. Can by NULL.
  *
  * Given a processor node containing a processing unit, walk into it and count
  * how many levels exist solely for it, and then walk up each level until we hit
  * the root node (ignore the package level because it may be possible to have
- * caches that exist across packages). Count the number of cache levels that
- * exist at each level on the way up.
- *
- * Return: Total number of levels found.
+ * caches that exist across packages). Count the number of cache levels and
+ * split cache levels (data/instruction) that exist at each level on the way
+ * up.
  */
-static int acpi_count_levels(struct acpi_table_header *table_hdr,
-			     struct acpi_pptt_processor *cpu_node)
+static void acpi_count_levels(struct acpi_table_header *table_hdr,
+			      struct acpi_pptt_processor *cpu_node,
+			      unsigned int *levels, unsigned int *split_levels)
 {
-	int total_levels = 0;
-
 	do {
-		acpi_find_cache_level(table_hdr, cpu_node, &total_levels, 0, 0);
+		acpi_find_cache_level(table_hdr, cpu_node, levels, split_levels, 0, 0);
 		cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
 	} while (cpu_node);
-
-	return total_levels;
 }
 
 /**
@@ -321,7 +333,7 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
 
 	while (cpu_node && !found) {
 		found = acpi_find_cache_level(table_hdr, cpu_node,
-					      &total_levels, level, acpi_type);
+					      &total_levels, NULL, level, acpi_type);
 		*node = cpu_node;
 		cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
 	}
@@ -589,36 +601,48 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
 }
 
 /**
- * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
+ * acpi_get_cache_info() - Determine the number of cache levels and
+ * split cache levels (data/instruction) and for a PE.
  * @cpu: Kernel logical CPU number
+ * @levels: Number of levels if success.
+ * @split_levels:	Number of levels being split (i.e. data/instruction)
+ *			if success. Can by NULL.
  *
  * Given a logical CPU number, returns the number of levels of cache represented
  * in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
  * indicating we didn't find any cache levels.
  *
- * Return: Cache levels visible to this core.
+ * Return: -ENOENT if no PPTT table or no PPTT processor struct found.
+ *	   0 on success.
  */
-int acpi_find_last_cache_level(unsigned int cpu)
+int acpi_get_cache_info(unsigned int cpu, unsigned int *levels,
+			unsigned int *split_levels)
 {
 	struct acpi_pptt_processor *cpu_node;
 	struct acpi_table_header *table;
-	int number_of_levels = 0;
 	u32 acpi_cpu_id;
 
+	*levels = 0;
+	if (split_levels)
+		*split_levels = 0;
+
 	table = acpi_get_pptt();
 	if (!table)
 		return -ENOENT;
 
-	pr_debug("Cache Setup find last level CPU=%d\n", cpu);
+	pr_debug("Cache Setup: find cache levels for CPU=%d\n", cpu);
 
 	acpi_cpu_id = get_acpi_id_for_cpu(cpu);
 	cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
-	if (cpu_node)
-		number_of_levels = acpi_count_levels(table, cpu_node);
+	if (!cpu_node)
+		return -ENOENT;
 
-	pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
+	acpi_count_levels(table, cpu_node, levels, split_levels);
 
-	return number_of_levels;
+	pr_debug("Cache Setup: last_level=%d split_levels=%d\n",
+		 *levels, split_levels ? *split_levels : -1);
+
+	return 0;
 }
 
 /**
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index ff0328f3fbb0..00d8e7f9d1c6 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -88,19 +88,22 @@ bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y);
 int detect_cache_attributes(unsigned int cpu);
 #ifndef CONFIG_ACPI_PPTT
 /*
- * acpi_find_last_cache_level is only called on ACPI enabled
+ * acpi_get_cache_info() is only called on ACPI enabled
  * platforms using the PPTT for topology. This means that if
  * the platform supports other firmware configuration methods
  * we need to stub out the call when ACPI is disabled.
  * ACPI enabled platforms not using PPTT won't be making calls
  * to this function so we need not worry about them.
  */
-static inline int acpi_find_last_cache_level(unsigned int cpu)
+static inline
+int acpi_get_cache_info(unsigned int cpu,
+			unsigned int *levels, unsigned int *split_levels)
 {
 	return 0;
 }
 #else
-int acpi_find_last_cache_level(unsigned int cpu);
+int acpi_get_cache_info(unsigned int cpu,
+			unsigned int *levels, unsigned int *split_levels);
 #endif
 
 const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 5/5] arch_topology: Build cacheinfo from primary CPU

[Pierre Gondois]

commit 3fcbf1c77d08 ("arch_topology: Fix cache attributes detection
in the CPU hotplug path")
adds a call to detect_cache_attributes() to populate the cacheinfo
before updating the siblings mask. detect_cache_attributes() allocates
memory and can take the PPTT mutex (on ACPI platforms). On PREEMPT_RT
kernels, on secondary CPUs, this triggers a:
  'BUG: sleeping function called from invalid context' [1]
as the code is executed with preemption and interrupts disabled.

The primary CPU was previously storing the cache information using
the now removed (struct cpu_topology).llc_id:
commit 5b8dc787ce4a ("arch_topology: Drop LLC identifier stash from
the CPU topology")

allocate_cache_info() tries to build the cacheinfo from the primary
CPU prior secondary CPUs boot, if the DT/ACPI description
contains cache information.
If allocate_cache_info() fails, then fallback to the current state
for the cacheinfo allocation. [1] will be triggered in such case.

When unplugging a CPU, the cacheinfo memory cannot be freed. If it
was, then the memory would be allocated early by the re-plugged
CPU and would trigger [1].

Note that populate_cache_leaves() might be called multiple times
due to populate_leaves being moved up. This is required since
detect_cache_attributes() might be called with per_cpu_cacheinfo(cpu)
being allocated but not populated.

[1]:
[    7.560791] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
[    7.560794] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/111
[    7.560796] preempt_count: 1, expected: 0
[    7.560797] RCU nest depth: 1, expected: 1
[    7.560799] 3 locks held by swapper/111/0:
[    7.560800]  #0: ffff403e406cae98 (&pcp->lock){+.+.}-{3:3}, at: get_page_from_freelist+0x218/0x12c8
[    7.560811]  #1: ffffc5f8ed09f8e8 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x48/0xf0
[    7.560820]  #2: ffff403f400b4fd8 (&zone->lock){+.+.}-{3:3}, at: rmqueue_bulk+0x64/0xa80
[    7.560824] irq event stamp: 0
[    7.560825] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
[    7.560827] hardirqs last disabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[    7.560830] softirqs last  enabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[    7.560833] softirqs last disabled at (0): [<0000000000000000>] 0x0
[    7.560834] Preemption disabled at:
[    7.560835] [<ffffc5f8e9fd3c28>] migrate_enable+0x30/0x130
[    7.560838] CPU: 111 PID: 0 Comm: swapper/111 Tainted: G        W          6.0.0-rc4-rt6-[...]
[    7.560841] Call trace:
[...]
[    7.560870]  __kmalloc+0xbc/0x1e8
[    7.560873]  detect_cache_attributes+0x2d4/0x5f0
[    7.560876]  update_siblings_masks+0x30/0x368
[    7.560880]  store_cpu_topology+0x78/0xb8
[    7.560883]  secondary_start_kernel+0xd0/0x198
[    7.560885]  __secondary_switched+0xb0/0xb4

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/base/arch_topology.c | 12 +++++-
 drivers/base/cacheinfo.c     | 71 +++++++++++++++++++++++++++---------
 include/linux/cacheinfo.h    |  1 +
 3 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index e7d6e6657ffa..b1c1dd38ab01 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -736,7 +736,7 @@ void update_siblings_masks(unsigned int cpuid)
 
 	ret = detect_cache_attributes(cpuid);
 	if (ret && ret != -ENOENT)
-		pr_info("Early cacheinfo failed, ret = %d\n", ret);
+		pr_info("Early cacheinfo allocation failed, ret = %d\n", ret);
 
 	/* update core and thread sibling masks */
 	for_each_online_cpu(cpu) {
@@ -825,7 +825,7 @@ __weak int __init parse_acpi_topology(void)
 #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
 void __init init_cpu_topology(void)
 {
-	int ret;
+	int cpu, ret;
 
 	reset_cpu_topology();
 	ret = parse_acpi_topology();
@@ -840,6 +840,14 @@ void __init init_cpu_topology(void)
 		reset_cpu_topology();
 		return;
 	}
+
+	for_each_possible_cpu(cpu) {
+		ret = fetch_cache_info(cpu);
+		if (ret) {
+			pr_err("Early cacheinfo failed, ret = %d\n", ret);
+			break;
+		}
+	}
 }
 
 void store_cpu_topology(unsigned int cpuid)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 6f6cd120c4f1..d8f71ee859d2 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -371,10 +371,6 @@ static void free_cache_attributes(unsigned int cpu)
 		return;
 
 	cache_shared_cpu_map_remove(cpu);
-
-	kfree(per_cpu_cacheinfo(cpu));
-	per_cpu_cacheinfo(cpu) = NULL;
-	cache_leaves(cpu) = 0;
 }
 
 int __weak init_cache_level(unsigned int cpu)
@@ -387,29 +383,71 @@ int __weak populate_cache_leaves(unsigned int cpu)
 	return -ENOENT;
 }
 
+static inline
+int allocate_cache_info(int cpu)
+{
+	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
+					 sizeof(struct cacheinfo), GFP_ATOMIC);
+	if (!per_cpu_cacheinfo(cpu)) {
+		cache_leaves(cpu) = 0;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+int fetch_cache_info(unsigned int cpu)
+{
+	struct cpu_cacheinfo *this_cpu_ci;
+	unsigned int levels, split_levels;
+	int ret;
+
+	if (acpi_disabled) {
+		ret = init_of_cache_level(cpu);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = acpi_get_cache_info(cpu, &levels, &split_levels);
+		if (ret < 0)
+			return ret;
+
+		this_cpu_ci = get_cpu_cacheinfo(cpu);
+		this_cpu_ci->num_levels = levels;
+		/*
+		 * This assumes that:
+		 * - there cannot be any split caches (data/instruction)
+		 *   above a unified cache
+		 * - data/instruction caches come by pair
+		 */
+		this_cpu_ci->num_leaves = levels + split_levels;
+	}
+	if (!cache_leaves(cpu))
+		return -ENOENT;
+
+	return allocate_cache_info(cpu);
+}
+
 int detect_cache_attributes(unsigned int cpu)
 {
 	int ret;
 
-	/* Since early detection of the cacheinfo is allowed via this
-	 * function and this also gets called as CPU hotplug callbacks via
-	 * cacheinfo_cpu_online, the initialisation can be skipped and only
-	 * CPU maps can be updated as the CPU online status would be update
-	 * if called via cacheinfo_cpu_online path.
+	/* Since early initialization/allocation of the cacheinfo is allowed
+	 * via fetch_cache_info() and this also gets called as CPU hotplug
+	 * callbacks via cacheinfo_cpu_online, the init/alloc can be skipped
+	 * as it will happen only once (the cacheinfo memory is never freed).
+	 * Just populate the cacheinfo.
 	 */
 	if (per_cpu_cacheinfo(cpu))
-		goto update_cpu_map;
+		goto populate_leaves;
 
 	if (init_cache_level(cpu) || !cache_leaves(cpu))
 		return -ENOENT;
 
-	per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
-					 sizeof(struct cacheinfo), GFP_ATOMIC);
-	if (per_cpu_cacheinfo(cpu) == NULL) {
-		cache_leaves(cpu) = 0;
-		return -ENOMEM;
-	}
+	ret = allocate_cache_info(cpu);
+	if (ret)
+		return ret;
 
+populate_leaves:
 	/*
 	 * populate_cache_leaves() may completely setup the cache leaves and
 	 * shared_cpu_map or it may leave it partially setup.
@@ -418,7 +456,6 @@ int detect_cache_attributes(unsigned int cpu)
 	if (ret)
 		goto free_ci;
 
-update_cpu_map:
 	/*
 	 * For systems using DT for cache hierarchy, fw_token
 	 * and shared_cpu_map will be set up here only if they are
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index 00d8e7f9d1c6..dfef57077cd0 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -85,6 +85,7 @@ int populate_cache_leaves(unsigned int cpu);
 int cache_setup_acpi(unsigned int cpu);
 bool last_level_cache_is_valid(unsigned int cpu);
 bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y);
+int fetch_cache_info(unsigned int cpu);
 int detect_cache_attributes(unsigned int cpu);
 #ifndef CONFIG_ACPI_PPTT
 /*
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/5] cacheinfo: Use RISC-V's init_cache_level() as generic OF implementation

[Rob Herring]

On Fri, Dec 09, 2022 at 11:31:23AM +0100, Pierre Gondois wrote:
> RISC-V's implementation of init_of_cache_level() is following
> the Devicetree Specification v0.3 regarding caches, cf.:
> - s3.7.3 'Internal (L1) Cache Properties'
> - s3.8 'Multi-level and Shared Cache Nodes'
> 
> Allow reusing the implementation by moving it.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> ---
>  arch/riscv/kernel/cacheinfo.c | 39 +------------------------------
>  drivers/base/cacheinfo.c      | 44 +++++++++++++++++++++++++++++++++++
>  include/linux/cacheinfo.h     |  1 +
>  3 files changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> index 90deabfe63ea..440a3df5944c 100644
> --- a/arch/riscv/kernel/cacheinfo.c
> +++ b/arch/riscv/kernel/cacheinfo.c
> @@ -115,44 +115,7 @@ static void fill_cacheinfo(struct cacheinfo **this_leaf,
>  
>  int init_cache_level(unsigned int cpu)
>  {
> -	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> -	struct device_node *np = of_cpu_device_node_get(cpu);
> -	struct device_node *prev = NULL;
> -	int levels = 0, leaves = 0, level;
> -
> -	if (of_property_read_bool(np, "cache-size"))
> -		++leaves;
> -	if (of_property_read_bool(np, "i-cache-size"))
> -		++leaves;
> -	if (of_property_read_bool(np, "d-cache-size"))
> -		++leaves;
> -	if (leaves > 0)
> -		levels = 1;
> -
> -	prev = np;
> -	while ((np = of_find_next_cache_node(np))) {
> -		of_node_put(prev);
> -		prev = np;
> -		if (!of_device_is_compatible(np, "cache"))
> -			break;
> -		if (of_property_read_u32(np, "cache-level", &level))
> -			break;
> -		if (level <= levels)
> -			break;
> -		if (of_property_read_bool(np, "cache-size"))
> -			++leaves;
> -		if (of_property_read_bool(np, "i-cache-size"))
> -			++leaves;
> -		if (of_property_read_bool(np, "d-cache-size"))
> -			++leaves;
> -		levels = level;
> -	}
> -
> -	of_node_put(np);
> -	this_cpu_ci->num_levels = levels;
> -	this_cpu_ci->num_leaves = leaves;
> -
> -	return 0;
> +	return init_of_cache_level(cpu);

Not in this patch, but in patch 5, shouldn't riscv init_cache_level() be 
removed? The topology code already called init_of_cache_level() and 
RiscV has nothing architectural to add/change. IOW, init_cache_level() 
should only do architecture defined init, and not anything DT or ACPI 
related (unless those are non-standard).

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/5] cacheinfo: Use RISC-V's init_cache_level() as generic OF implementation

[Pierre Gondois]

Hello Rob,

On 12/21/22 00:39, Rob Herring wrote:
> On Fri, Dec 09, 2022 at 11:31:23AM +0100, Pierre Gondois wrote:
>> RISC-V's implementation of init_of_cache_level() is following
>> the Devicetree Specification v0.3 regarding caches, cf.:
>> - s3.7.3 'Internal (L1) Cache Properties'
>> - s3.8 'Multi-level and Shared Cache Nodes'
>>
>> Allow reusing the implementation by moving it.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>> ---
>>   arch/riscv/kernel/cacheinfo.c | 39 +------------------------------
>>   drivers/base/cacheinfo.c      | 44 +++++++++++++++++++++++++++++++++++
>>   include/linux/cacheinfo.h     |  1 +
>>   3 files changed, 46 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
>> index 90deabfe63ea..440a3df5944c 100644
>> --- a/arch/riscv/kernel/cacheinfo.c
>> +++ b/arch/riscv/kernel/cacheinfo.c
>> @@ -115,44 +115,7 @@ static void fill_cacheinfo(struct cacheinfo **this_leaf,
>>   
>>   int init_cache_level(unsigned int cpu)
>>   {
>> -	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>> -	struct device_node *np = of_cpu_device_node_get(cpu);
>> -	struct device_node *prev = NULL;
>> -	int levels = 0, leaves = 0, level;
>> -
>> -	if (of_property_read_bool(np, "cache-size"))
>> -		++leaves;
>> -	if (of_property_read_bool(np, "i-cache-size"))
>> -		++leaves;
>> -	if (of_property_read_bool(np, "d-cache-size"))
>> -		++leaves;
>> -	if (leaves > 0)
>> -		levels = 1;
>> -
>> -	prev = np;
>> -	while ((np = of_find_next_cache_node(np))) {
>> -		of_node_put(prev);
>> -		prev = np;
>> -		if (!of_device_is_compatible(np, "cache"))
>> -			break;
>> -		if (of_property_read_u32(np, "cache-level", &level))
>> -			break;
>> -		if (level <= levels)
>> -			break;
>> -		if (of_property_read_bool(np, "cache-size"))
>> -			++leaves;
>> -		if (of_property_read_bool(np, "i-cache-size"))
>> -			++leaves;
>> -		if (of_property_read_bool(np, "d-cache-size"))
>> -			++leaves;
>> -		levels = level;
>> -	}
>> -
>> -	of_node_put(np);
>> -	this_cpu_ci->num_levels = levels;
>> -	this_cpu_ci->num_leaves = leaves;
>> -
>> -	return 0;
>> +	return init_of_cache_level(cpu);
> 
> Not in this patch, but in patch 5, shouldn't riscv init_cache_level() be
> removed? The topology code already called init_of_cache_level() and
> RiscV has nothing architectural to add/change. IOW, init_cache_level()
> should only do architecture defined init, and not anything DT or ACPI
> related (unless those are non-standard).
> 
> Rob

I think you are right. Just to re-phrase your point:
init_of_cache_level() is called through this path:
init_cpu_topology()
\-fetch_cache_info()
   \-init_of_cache_level()

If there is missing information in the DT and it's not possible to create the
cacheinfo, then the arch specific implementation i.e. init_cache_level() is
called through:
update_siblings_masks() | cacheinfo_cpu_online()
\-detect_cache_attributes()
   \-init_cache_level()

This is useful for arm to call init_cache_level() since it is possible
to extract some information from some registers. For RISC-V, if
init_of_cache_level() fails, then init_cache_level() will fail again.
So removing RISC-V's init_cache_level() makes sense.

Regards,
Pierre


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/5] arch_topology: Build cacheinfo from primary CPU

[Krzysztof Kozlowski]

[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]

On 09/12/2022 11:31, Pierre Gondois wrote:
> v2:
>  - Applied renaming/formatting comments from v1.
>  - Check CACHE_TYPE_VALID flag in pppt.c.
> v3:
>  - Applied Sudeep's suggestions (for patch 5/5):
>    - Renaming allocate_cache_info() -> fecth_cache_info()
>    - Updated error message
>    - Extract an inline allocate_cache_info() function
>  - Re-run checkpatch with --strict option
> 
> Note:
> This patchset requires the following patch to be applied first in
> order to avoid the same bug described in the commit message:
> https://lore.kernel.org/all/20221116094958.2141072-1-pierre.gondois@arm.com/
> 
> [1] and [2] build the CPU topology from the cacheinfo information for
> both DT/ACPI based systems and remove (struct cpu_topology).llc_id
> which was used by ACPI only.
> 
> Creating the cacheinfo for secondary CPUs is done during early boot.
> Preemption and interrupts are disabled at this stage. On PREEMPT_RT
> kernels, allocating memory (and parsing the PPTT table for ACPI based
> systems) triggers a:
>   'BUG: sleeping function called from invalid context' [4]
> 
> To prevent this bug, allocate the cacheinfo from the primary CPU when
> preemption and interrupts are enabled and before booting secondary
> CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
> only, without relying on the arm64 CLIDR_EL1 register.
> If no cache information is found in the DT/ACPI PPTT, then fallback
> to the current state, triggering [4] on PREEMPT_RT kernels.
> 
> Patches to update the arm64 device trees that have incomplete cacheinfo
> (mostly for missing the 'cache-level' or 'cache-unified' property)
> have been sent at [3].
> 
> Tested platforms:
> - ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
>   Kunpeng 920, Juno-r2
> - DT: rb5, db845c, Juno-r2
> 

I gave the patchset a try with DTS fixes for cache topology on Qualcomm
RB5 board (SM8250 SoC) and with KASAN it produces:

BUG: KASAN: slab-out-of-bounds in populate_cache_leaves+0x84/0x15c
[    0.633014]  dump_backtrace.part.0+0xe0/0xf0
[    0.633035]  show_stack+0x18/0x40
[    0.633050]  dump_stack_lvl+0x8c/0xb8
[    0.633085]  print_report+0x188/0x488
[    0.633106]  kasan_report+0xac/0xf0
[    0.633136]  __asan_store4+0x80/0xa4
[    0.633158]  populate_cache_leaves+0x84/0x15c
[    0.633181]  detect_cache_attributes+0xc0/0x8c4
[    0.633213]  update_siblings_masks+0x28/0x43c
[    0.633235]  store_cpu_topology+0x98/0xc0
[    0.633251]  smp_prepare_cpus+0x2c/0x15c
[    0.633281]  kernel_init_freeable+0x22c/0x424
[    0.633310]  kernel_init+0x24/0x13c
[    0.633328]  ret_from_fork+0x10/0x20
[    0.633388]
[    0.708729] Allocated by task 1:
[    0.712078]  kasan_save_stack+0x2c/0x60
[    0.716066]  kasan_set_track+0x2c/0x40
[    0.719959]  kasan_save_alloc_info+0x24/0x3c
[    0.724387]  __kasan_kmalloc+0xa0/0xbc
[    0.728278]  __kmalloc+0x74/0x110
[    0.731740]  fetch_cache_info+0x170/0x210
[    0.735902]  init_cpu_topology+0x254/0x2bc
[    0.740171]  smp_prepare_cpus+0x20/0x15c
[    0.744272]  kernel_init_freeable+0x22c/0x424
[    0.748791]  kernel_init+0x24/0x13c
[    0.752420]  ret_from_fork+0x10/0x20

Best regards,
Krzysztof

[-- Attachment #2: log.txt --]
[-- Type: text/plain, Size: 10327 bytes --]

[    0.000000] arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
[    0.000001] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
[    0.005051] Console: colour dummy device 80x25
[    0.478980] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.478992] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.479002] ... MAX_LOCK_DEPTH:          48
[    0.479011] ... MAX_LOCKDEP_KEYS:        8192
[    0.479019] ... CLASSHASH_SIZE:          4096
[    0.479027] ... MAX_LOCKDEP_ENTRIES:     32768
[    0.479035] ... MAX_LOCKDEP_CHAINS:      65536
[    0.479043] ... CHAINHASH_SIZE:          32768
[    0.479052]  memory used by lock dependency info: 6365 kB
[    0.479061]  memory used for stack traces: 4224 kB
[    0.479069]  per task-struct memory footprint: 1920 bytes
[    0.479976] Calibrating delay loop (skipped), value calculated using timer frequency.. 38.40 BogoMIPS (lpj=19200)
[    0.480007] pid_max: default: 32768 minimum: 301
[    0.482256] LSM: Security Framework initializing
[    0.484629] Mount-cache hash table entries: 16384 (order: 5, 131072 bytes, linear)
[    0.484692] Mountpoint-cache hash table entries: 16384 (order: 5, 131072 bytes, linear)
[    0.518295] ==================================================================
[    0.617001] BUG: KASAN: slab-out-of-bounds in populate_cache_leaves+0x84/0x15c
[    0.624489] Write of size 4 at addr ffff0d83400366c8 by task swapper/0/1
[    0.631400]
[    0.632973] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rt5-00372-ga6339d0b4e8e #45
[    0.632995] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
[    0.633006] Call trace:
[    0.633014]  dump_backtrace.part.0+0xe0/0xf0
[    0.633035]  show_stack+0x18/0x40
[    0.633050]  dump_stack_lvl+0x8c/0xb8
[    0.633085]  print_report+0x188/0x488
[    0.633106]  kasan_report+0xac/0xf0
[    0.633136]  __asan_store4+0x80/0xa4
[    0.633158]  populate_cache_leaves+0x84/0x15c
[    0.633181]  detect_cache_attributes+0xc0/0x8c4
[    0.633213]  update_siblings_masks+0x28/0x43c
[    0.633235]  store_cpu_topology+0x98/0xc0
[    0.633251]  smp_prepare_cpus+0x2c/0x15c
[    0.633281]  kernel_init_freeable+0x22c/0x424
[    0.633310]  kernel_init+0x24/0x13c
[    0.633328]  ret_from_fork+0x10/0x20
[    0.633388]
[    0.708729] Allocated by task 1:
[    0.712078]  kasan_save_stack+0x2c/0x60
[    0.716066]  kasan_set_track+0x2c/0x40
[    0.719959]  kasan_save_alloc_info+0x24/0x3c
[    0.724387]  __kasan_kmalloc+0xa0/0xbc
[    0.728278]  __kmalloc+0x74/0x110
[    0.731740]  fetch_cache_info+0x170/0x210
[    0.735902]  init_cpu_topology+0x254/0x2bc
[    0.740171]  smp_prepare_cpus+0x20/0x15c
[    0.744272]  kernel_init_freeable+0x22c/0x424
[    0.748791]  kernel_init+0x24/0x13c
[    0.752420]  ret_from_fork+0x10/0x20
[    0.756131]
[    0.757726] The buggy address belongs to the object at ffff0d8340036600
[    0.757726]  which belongs to the cache kmalloc-256 of size 256
[    0.770607] The buggy address is located 200 bytes inside of
[    0.770607]  256-byte region [ffff0d8340036600, ffff0d8340036700)
[    0.782690]
[    0.784256] The buggy address belongs to the physical page:
[    0.790008] page:(____ptrval____) refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x100034
[    0.799686] head:(____ptrval____) order:2 compound_mapcount:0 compound_pincount:0
[    0.807405] flags: 0x800000000010200(slab|head|node=0|zone=2)
[    0.813365] raw: 0800000000010200 0000000000000000 dead000000000122 ffff0d8340002480
[    0.821349] raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000
[    0.829364] page dumped because: kasan: bad access detected
[    0.835117]
[    0.836679] Memory state around the buggy address:
[    0.841639]  ffff0d8340036580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[    0.849085]  ffff0d8340036600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.856562] >ffff0d8340036680: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
[    0.864005]                                               ^
[    0.869760]  ffff0d8340036700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[    0.877206]  ffff0d8340036780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[    0.884650] ==================================================================
[    0.892100] Disabling lock debugging due to kernel taint
[    0.904200] cblist_init_generic: Setting adjustable number of callback queues.
[    0.904241] cblist_init_generic: Setting shift to 3 and lim to 1.
[    0.918718] cblist_init_generic: Setting shift to 3 and lim to 1.
[    0.925721] Running RCU-tasks wait API self tests
[    1.036108] rcu: Hierarchical SRCU implementation.
[    1.036118] rcu:     Max phase no-delay instances is 400.
[    1.036756] printk: bootconsole [qcom_geni0] printing thread started
[    1.048808] Callback from call_rcu_tasks_trace() invoked.
[    1.081711] EFI services will not be available.
[    1.091529] smp: Bringing up secondary CPUs ...
[    1.103585] Detected VIPT I-cache on CPU1
[    1.103747] GICv3: CPU1: found redistributor 100 region 0:0x0000000017a80000
[    1.103830] CPU1: Booted secondary processor 0x0000000100 [0x51df805e]
[    1.129767] Detected VIPT I-cache on CPU2
[    1.129911] GICv3: CPU2: found redistributor 200 region 0:0x0000000017aa0000
[    1.129979] CPU2: Booted secondary processor 0x0000000200 [0x51df805e]
[    1.155742] Detected VIPT I-cache on CPU3
[    1.155876] GICv3: CPU3: found redistributor 300 region 0:0x0000000017ac0000
[    1.155936] CPU3: Booted secondary processor 0x0000000300 [0x51df805e]
[    1.182791] CPU features: detected: Spectre-v4
[    1.182824] CPU features: detected: Spectre-BHB
[    1.182854] CPU features: detected: ARM erratum 1508412 (kernel portion)
[    1.182900] Detected PIPT I-cache on CPU4
[    1.183187] GICv3: CPU4: found redistributor 400 region 0:0x0000000017ae0000
[    1.183294] CPU4: Booted secondary processor 0x0000000400 [0x411fd0d0]
[    1.225640] Detected PIPT I-cache on CPU5
[    1.226000] GICv3: CPU5: found redistributor 500 region 0:0x0000000017b00000
[    1.226099] CPU5: Booted secondary processor 0x0000000500 [0x411fd0d0]
[    1.252358] Detected PIPT I-cache on CPU6
[    1.252722] GICv3: CPU6: found redistributor 600 region 0:0x0000000017b20000
[    1.252821] CPU6: Booted secondary processor 0x0000000600 [0x411fd0d0]
[    1.266024] Callback from call_rcu_tasks() invoked.
[    1.284303] Detected PIPT I-cache on CPU7
[    1.284475] GICv3: CPU7: found redistributor 700 region 0:0x0000000017b40000
[    1.284525] CPU7: Booted secondary processor 0x0000000700 [0x411fd0d0]
[    1.284926] smp: Brought up 1 node, 8 CPUs
[    1.284943] SMP: Total of 8 processors activated.
[    1.284954] CPU features: detected: 32-bit EL0 Support
[    1.284963] CPU features: detected: Data cache clean to the PoU not required for I/D coherence
[    1.284975] CPU features: detected: Common not Private translations
[    1.284985] CPU features: detected: CRC32 instructions
[    1.284999] CPU features: detected: RCpc load-acquire (LDAPR)
[    1.285009] CPU features: detected: LSE atomic instructions
[    1.285019] CPU features: detected: Privileged Access Never
[    1.285029] CPU features: detected: RAS Extension Support
[    1.285044] CPU features: detected: Speculative Store Bypassing Safe (SSBS)
[    1.296826] CPU: All CPU(s) started at EL1
[    1.296891] alternatives: applying system-wide alternatives
[    1.315641] devtmpfs: initialized
[    1.670123] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275000 ns
[    1.670327] futex hash table entries: 2048 (order: 6, 393216 bytes, linear)
[    1.673537] pinctrl core: initialized pinctrl subsystem
[    1.684021] DMI not present or invalid.
[    1.687165] NET: Registered PF_NETLINK/PF_ROUTE protocol family
[    1.695595] DMA: preallocated 1024 KiB GFP_KERNEL pool for atomic allocations
[    1.696373] DMA: preallocated 1024 KiB GFP_KERNEL|GFP_DMA pool for atomic allocations
[    1.698379] DMA: preallocated 1024 KiB GFP_KERNEL|GFP_DMA32 pool for atomic allocations
[    1.698742] audit: initializing netlink subsys (disabled)
[    1.699628] audit: type=2000 audit(1.628:1): state=initialized audit_enabled=0 res=1
[    1.708543] thermal_sys: Registered thermal governor 'step_wise'
[    1.708570] thermal_sys: Registered thermal governor 'power_allocator'
[    1.708961] cpuidle: using governor ladder
[    1.709035] cpuidle: using governor menu
[    1.709849] NET: Registered PF_QIPCRTR protocol family
[    1.711345] hw-breakpoint: found 6 breakpoint and 4 watchpoint registers.
[    1.719297] ASID allocator initialised with 32768 entries
[    1.733197] Serial: AMBA PL011 UART driver
[    1.855339] platform 1d87000.phy: Fixing up cyclic dependency with 1d84000.ufshc
[    2.041279] KASLR enabled
[    2.174612] HugeTLB: registered 1.00 GiB page size, pre-allocated 0 pages
[    2.174626] HugeTLB: 16380 KiB vmemmap can be freed for a 1.00 GiB page
[    2.174634] HugeTLB: registered 32.0 MiB page size, pre-allocated 0 pages
[    2.174640] HugeTLB: 508 KiB vmemmap can be freed for a 32.0 MiB page
[    2.174647] HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages
[    2.174652] HugeTLB: 28 KiB vmemmap can be freed for a 2.00 MiB page
[    2.174660] HugeTLB: registered 64.0 KiB page size, pre-allocated 0 pages
[    2.174665] HugeTLB: 0 KiB vmemmap can be freed for a 64.0 KiB page
[    2.188583] ACPI: Interpreter disabled.
[    2.306645] iommu: Default domain type: Translated
[    2.306660] iommu: DMA domain TLB invalidation policy: strict mode
[    2.310343] SCSI subsystem initialized
[    2.313611] usbcore: registered new interface driver usbfs
[    2.313987] usbcore: registered new interface driver hub
[    2.314314] usbcore: registered new device driver usb
[    2.321138] pps_core: LinuxPPS API ver. 1 registered
[    2.321144] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
[    2.321246] PTP clock support registered
[    2.321452] EDAC MC: Ver: 3.0.0
[    2.324853] CPUidle PSCI: Initialized CPU PM domain topology
[    2.327564] qcom_scm: convention: smc arm 64
[    2.335862] FPGA manager framework


[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/5] arch_topology: Build cacheinfo from primary CPU

[Pierre Gondois]

On 12/29/22 17:47, Krzysztof Kozlowski wrote:
> On 09/12/2022 11:31, Pierre Gondois wrote:
>> v2:
>>   - Applied renaming/formatting comments from v1.
>>   - Check CACHE_TYPE_VALID flag in pppt.c.
>> v3:
>>   - Applied Sudeep's suggestions (for patch 5/5):
>>     - Renaming allocate_cache_info() -> fecth_cache_info()
>>     - Updated error message
>>     - Extract an inline allocate_cache_info() function
>>   - Re-run checkpatch with --strict option
>>
>> Note:
>> This patchset requires the following patch to be applied first in
>> order to avoid the same bug described in the commit message:
>> https://lore.kernel.org/all/20221116094958.2141072-1-pierre.gondois@arm.com/
>>
>> [1] and [2] build the CPU topology from the cacheinfo information for
>> both DT/ACPI based systems and remove (struct cpu_topology).llc_id
>> which was used by ACPI only.
>>
>> Creating the cacheinfo for secondary CPUs is done during early boot.
>> Preemption and interrupts are disabled at this stage. On PREEMPT_RT
>> kernels, allocating memory (and parsing the PPTT table for ACPI based
>> systems) triggers a:
>>    'BUG: sleeping function called from invalid context' [4]
>>
>> To prevent this bug, allocate the cacheinfo from the primary CPU when
>> preemption and interrupts are enabled and before booting secondary
>> CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
>> only, without relying on the arm64 CLIDR_EL1 register.
>> If no cache information is found in the DT/ACPI PPTT, then fallback
>> to the current state, triggering [4] on PREEMPT_RT kernels.
>>
>> Patches to update the arm64 device trees that have incomplete cacheinfo
>> (mostly for missing the 'cache-level' or 'cache-unified' property)
>> have been sent at [3].
>>
>> Tested platforms:
>> - ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
>>    Kunpeng 920, Juno-r2
>> - DT: rb5, db845c, Juno-r2
>>
> 
> I gave the patchset a try with DTS fixes for cache topology on Qualcomm
> RB5 board (SM8250 SoC) and with KASAN it produces:
> 
> BUG: KASAN: slab-out-of-bounds in populate_cache_leaves+0x84/0x15c
> [    0.633014]  dump_backtrace.part.0+0xe0/0xf0
> [    0.633035]  show_stack+0x18/0x40
> [    0.633050]  dump_stack_lvl+0x8c/0xb8
> [    0.633085]  print_report+0x188/0x488
> [    0.633106]  kasan_report+0xac/0xf0
> [    0.633136]  __asan_store4+0x80/0xa4
> [    0.633158]  populate_cache_leaves+0x84/0x15c
> [    0.633181]  detect_cache_attributes+0xc0/0x8c4
> [    0.633213]  update_siblings_masks+0x28/0x43c
> [    0.633235]  store_cpu_topology+0x98/0xc0
> [    0.633251]  smp_prepare_cpus+0x2c/0x15c
> [    0.633281]  kernel_init_freeable+0x22c/0x424
> [    0.633310]  kernel_init+0x24/0x13c
> [    0.633328]  ret_from_fork+0x10/0x20
> [    0.633388]
> [    0.708729] Allocated by task 1:
> [    0.712078]  kasan_save_stack+0x2c/0x60
> [    0.716066]  kasan_set_track+0x2c/0x40
> [    0.719959]  kasan_save_alloc_info+0x24/0x3c
> [    0.724387]  __kasan_kmalloc+0xa0/0xbc
> [    0.728278]  __kmalloc+0x74/0x110
> [    0.731740]  fetch_cache_info+0x170/0x210
> [    0.735902]  init_cpu_topology+0x254/0x2bc
> [    0.740171]  smp_prepare_cpus+0x20/0x15c
> [    0.744272]  kernel_init_freeable+0x22c/0x424
> [    0.748791]  kernel_init+0x24/0x13c
> [    0.752420]  ret_from_fork+0x10/0x20
> 
> Best regards,
> Krzysztof

Hello Krzysztof,
Thanks for trying the patch-set and reporting the issue. Hopefully
the v4 should solve this:
https://lore.kernel.org/all/20230104183033.755668-1-pierre.gondois@arm.com/

I will also try to follow the corresponding dts modifications,

Regards,
Pierre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel