v4:
arch_topology: Remove early cacheinfo error message:
- Only remove the error message if the error code is -ENOENT
cacheinfo: Add use_arch[|_cache]_info field/function:
- Use a static variable instead of a per-leaf 'use_arch_info'
- Reformat the use_arch_cache_info() define
v3:
cacheinfo: Check sib_leaf in cache_leaves_are_shared():
- Reformulate commit message
- Fix rebase issue and move '&&' condition which was in the last patch
to this patch.
cacheinfo: Add use_arch[|_cache]_info field/function:
- Put the function declaration in one line.
arch_topology: Remove early cacheinfo error message:
- New patch.
v2:
cacheinfo: Check sib_leaf in cache_leaves_are_shared()
- Reformulate commit message
- Add 'Fixes: f16d1becf96f ("cacheinfo: Use cache identifiers [...]'
cacheinfo: Check cache properties are present in DT
- Use of_property_present()
- Add 'Reported-by: Alexandre Ghiti <[email protected]>'
cacheinfo: Add use_arch[|_cache]_info field/function:
- Make use_arch_cache_info() a static inline function
The cache information can be extracted from either a Device
Tree (DT), the PPTT ACPI table, or arch registers (clidr_el1
for arm64).
When the DT is used but no cache properties are advertised,
the current code doesn't correctly fallback to using arch information.
Correct this. Also use the assumption that L1 data/instruction caches
are private and L2/higher caches are shared when the cache information
is coming form clidr_el1.
As suggested by Alexandre, this serie should ideally go to 6.3 fixes.
Pierre Gondois (4):
cacheinfo: Check sib_leaf in cache_leaves_are_shared()
cacheinfo: Check cache properties are present in DT
arch_topology: Remove early cacheinfo error message if -ENOENT
cacheinfo: Add use_arch[|_cache]_info field/function
drivers/base/arch_topology.c | 7 +++---
drivers/base/cacheinfo.c | 49 ++++++++++++++++++++++++++++++++----
include/linux/cacheinfo.h | 6 +++++
3 files changed, 54 insertions(+), 8 deletions(-)
--
2.25.1
If a Device Tree (DT) is used, the presence of cache properties is
assumed. Not finding any is not considered. For arm64 platforms,
cache information can be fetched from the clidr_el1 register.
Checking whether cache information is available in the DT
allows to switch to using clidr_el1.
init_of_cache_level()
\-of_count_cache_leaves()
will assume there a 2 cache leaves (L1 data/instruction caches), which
can be different from clidr_el1 information.
cache_setup_of_node() tries to read cache properties in the DT.
If there are none, this is considered a success. Knowing no
information was available would allow to switch to using clidr_el1.
Fixes: de0df442ee49 ("cacheinfo: Check 'cache-unified' property to count cache leaves")
Reported-by: Alexandre Ghiti <[email protected]>
Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
Signed-off-by: Pierre Gondois <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
drivers/base/cacheinfo.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index c5d2293ac2a6..ea8f416852bd 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -78,6 +78,9 @@ bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y)
}
#ifdef CONFIG_OF
+
+static bool of_check_cache_nodes(struct device_node *np);
+
/* OF properties to query for a given cache type */
struct cache_type_info {
const char *size_prop;
@@ -205,6 +208,11 @@ static int cache_setup_of_node(unsigned int cpu)
return -ENOENT;
}
+ if (!of_check_cache_nodes(np)) {
+ of_node_put(np);
+ return -ENOENT;
+ }
+
prev = np;
while (index < cache_leaves(cpu)) {
@@ -229,6 +237,25 @@ static int cache_setup_of_node(unsigned int cpu)
return 0;
}
+static bool of_check_cache_nodes(struct device_node *np)
+{
+ struct device_node *next;
+
+ if (of_property_present(np, "cache-size") ||
+ of_property_present(np, "i-cache-size") ||
+ of_property_present(np, "d-cache-size") ||
+ of_property_present(np, "cache-unified"))
+ return true;
+
+ next = of_find_next_cache_node(np);
+ if (next) {
+ of_node_put(next);
+ return true;
+ }
+
+ return false;
+}
+
static int of_count_cache_leaves(struct device_node *np)
{
unsigned int leaves = 0;
@@ -260,6 +287,11 @@ int init_of_cache_level(unsigned int cpu)
struct device_node *prev = NULL;
unsigned int levels = 0, leaves, level;
+ if (!of_check_cache_nodes(np)) {
+ of_node_put(np);
+ return -ENOENT;
+ }
+
leaves = of_count_cache_leaves(np);
if (leaves > 0)
levels = 1;
--
2.25.1
fetch_cache_info() tries to get the number of cache leaves/levels
for each CPU in order to pre-allocate memory for cacheinfo struct.
Allocating this memory later triggers a:
'BUG: sleeping function called from invalid context'
in PREEMPT_RT kernels.
If there is no cache related information available in DT or ACPI,
fetch_cache_info() fails and an error message is printed:
'Early cacheinfo failed, ret = ...'
Not having cache information should be a valid configuration.
Remove the error message if fetch_cache_info() fails with -ENOENT.
Suggested-by: Conor Dooley <[email protected]>
Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
Signed-off-by: Pierre Gondois <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
drivers/base/arch_topology.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index b1c1dd38ab01..c4b6198d7461 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -843,10 +843,11 @@ void __init init_cpu_topology(void)
for_each_possible_cpu(cpu) {
ret = fetch_cache_info(cpu);
- if (ret) {
+ if (!ret)
+ continue;
+ else if (ret != -ENOENT)
pr_err("Early cacheinfo failed, ret = %d\n", ret);
- break;
- }
+ return;
}
}
--
2.25.1
The cache information can be extracted from either a Device
Tree (DT), the PPTT ACPI table, or arch registers (clidr_el1
for arm64).
The clidr_el1 register is used only if DT/ACPI information is not
available. It does not states how caches are shared among CPUs.
Add a use_arch_cache_info field/function to identify when the
DT/ACPI doesn't provide cache information. Use this information
to assume L1 caches are privates and L2 and higher are shared among
all CPUs.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/base/cacheinfo.c | 12 ++++++++++--
include/linux/cacheinfo.h | 6 ++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index ea8f416852bd..8120ac1ddbe4 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -28,6 +28,9 @@ static DEFINE_PER_CPU(struct cpu_cacheinfo, ci_cpu_cacheinfo);
#define per_cpu_cacheinfo_idx(cpu, idx) \
(per_cpu_cacheinfo(cpu) + (idx))
+/* Set if no cache information is found in DT/ACPI. */
+static bool use_arch_info;
+
struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu)
{
return ci_cacheinfo(cpu);
@@ -40,7 +43,8 @@ static inline bool cache_leaves_are_shared(struct cacheinfo *this_leaf,
* For non DT/ACPI systems, assume unique level 1 caches,
* system-wide shared caches for all other levels.
*/
- if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI)))
+ if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI)) ||
+ use_arch_info)
return (this_leaf->level != 1) && (sib_leaf->level != 1);
if ((sib_leaf->attributes & CACHE_ID) &&
@@ -343,6 +347,10 @@ static int cache_setup_properties(unsigned int cpu)
else if (!acpi_disabled)
ret = cache_setup_acpi(cpu);
+ // Assume there is no cache information available in DT/ACPI from now.
+ if (ret && use_arch_cache_info())
+ use_arch_info = true;
+
return ret;
}
@@ -361,7 +369,7 @@ static int cache_shared_cpu_map_setup(unsigned int cpu)
* to update the shared cpu_map if the cache attributes were
* populated early before all the cpus are brought online
*/
- if (!last_level_cache_is_valid(cpu)) {
+ if (!last_level_cache_is_valid(cpu) && !use_arch_info) {
ret = cache_setup_properties(cpu);
if (ret)
return ret;
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index 908e19d17f49..b91cc9991c7c 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -129,4 +129,10 @@ static inline int get_cpu_cacheinfo_id(int cpu, int level)
return -1;
}
+#ifdef CONFIG_ARM64
+#define use_arch_cache_info() (true)
+#else
+#define use_arch_cache_info() (false)
+#endif
+
#endif /* _LINUX_CACHEINFO_H */
--
2.25.1
On 4/14/23 01:14, Pierre Gondois wrote:
> v4:
> arch_topology: Remove early cacheinfo error message:
> - Only remove the error message if the error code is -ENOENT
> cacheinfo: Add use_arch[|_cache]_info field/function:
> - Use a static variable instead of a per-leaf 'use_arch_info'
> - Reformat the use_arch_cache_info() define
>
> v3:
> cacheinfo: Check sib_leaf in cache_leaves_are_shared():
> - Reformulate commit message
> - Fix rebase issue and move '&&' condition which was in the last patch
> to this patch.
> cacheinfo: Add use_arch[|_cache]_info field/function:
> - Put the function declaration in one line.
> arch_topology: Remove early cacheinfo error message:
> - New patch.
>
> v2:
> cacheinfo: Check sib_leaf in cache_leaves_are_shared()
> - Reformulate commit message
> - Add 'Fixes: f16d1becf96f ("cacheinfo: Use cache identifiers [...]'
> cacheinfo: Check cache properties are present in DT
> - Use of_property_present()
> - Add 'Reported-by: Alexandre Ghiti <[email protected]>'
> cacheinfo: Add use_arch[|_cache]_info field/function:
> - Make use_arch_cache_info() a static inline function
>
> The cache information can be extracted from either a Device
> Tree (DT), the PPTT ACPI table, or arch registers (clidr_el1
> for arm64).
>
> When the DT is used but no cache properties are advertised,
> the current code doesn't correctly fallback to using arch information.
>
> Correct this. Also use the assumption that L1 data/instruction caches
> are private and L2/higher caches are shared when the cache information
> is coming form clidr_el1.
>
> As suggested by Alexandre, this serie should ideally go to 6.3 fixes.
FWIW:
Tested-by: Florian Fainelli <[email protected]>
Thanks!
--
Florian
On Fri, 14 Apr 2023 10:14:48 +0200, Pierre Gondois wrote:
> v4:
> arch_topology: Remove early cacheinfo error message:
> - Only remove the error message if the error code is -ENOENT
> cacheinfo: Add use_arch[|_cache]_info field/function:
> - Use a static variable instead of a per-leaf 'use_arch_info'
> - Reformat the use_arch_cache_info() define
>
> [...]
Applied to sudeep.holla/linux (for-next/cacheinfo), thanks!
[1/4] cacheinfo: Check sib_leaf in cache_leaves_are_shared()
https://git.kernel.org/sudeep.holla/c/7a306e3eabf2
[2/4] cacheinfo: Check cache properties are present in DT
https://git.kernel.org/sudeep.holla/c/cde0fbff07ef
[3/4] arch_topology: Remove early cacheinfo error message if -ENOENT
https://git.kernel.org/sudeep.holla/c/3522340199cc
[4/4] cacheinfo: Add use_arch[|_cache]_info field/function
https://git.kernel.org/sudeep.holla/c/ef9f643a9f8b
--
Regards,
Sudeep