Message ID | 20230327115953.788244-4-pierre.gondois@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | cacheinfo: Correctly fallback to using clidr_el1's information | expand |
On Mon, Mar 27, 2023 at 01:59:51PM +0200, Pierre Gondois wrote: > 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 <pierre.gondois@arm.com> > --- > arch/arm64/kernel/cacheinfo.c | 5 +++++ > drivers/base/cacheinfo.c | 20 ++++++++++++++++++-- > include/linux/cacheinfo.h | 2 ++ > 3 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c > index c307f69e9b55..b6306cda0fa7 100644 > --- a/arch/arm64/kernel/cacheinfo.c > +++ b/arch/arm64/kernel/cacheinfo.c > @@ -96,3 +96,8 @@ int populate_cache_leaves(unsigned int cpu) > } > return 0; > } > + > +bool use_arch_cache_info(unsigned int cpu) > +{ > + return true; > +} It would be a lot nicer if this was a static inline function in a header rather than a weak symbol. Will
Hello Will, On 3/27/23 14:17, Will Deacon wrote: > On Mon, Mar 27, 2023 at 01:59:51PM +0200, Pierre Gondois wrote: >> 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 <pierre.gondois@arm.com> >> --- >> arch/arm64/kernel/cacheinfo.c | 5 +++++ >> drivers/base/cacheinfo.c | 20 ++++++++++++++++++-- >> include/linux/cacheinfo.h | 2 ++ >> 3 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c >> index c307f69e9b55..b6306cda0fa7 100644 >> --- a/arch/arm64/kernel/cacheinfo.c >> +++ b/arch/arm64/kernel/cacheinfo.c >> @@ -96,3 +96,8 @@ int populate_cache_leaves(unsigned int cpu) >> } >> return 0; >> } >> + >> +bool use_arch_cache_info(unsigned int cpu) >> +{ >> + return true; >> +} > > It would be a lot nicer if this was a static inline function in a header > rather than a weak symbol. I am not sure I see where the static inline function should be added. Do you prefer to have function like the following in include/linux/cacheinfo.h ? static inline bool use_arch_cache_info(unsigned int cpu) { #if defined(CONFIG_ARM64) return true; #else return false; #endif } Regards, Pierre > > Will
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c index c307f69e9b55..b6306cda0fa7 100644 --- a/arch/arm64/kernel/cacheinfo.c +++ b/arch/arm64/kernel/cacheinfo.c @@ -96,3 +96,8 @@ int populate_cache_leaves(unsigned int cpu) } return 0; } + +bool use_arch_cache_info(unsigned int cpu) +{ + return true; +} diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c index 5b0edf2d5da8..c6266ccc2df5 100644 --- a/drivers/base/cacheinfo.c +++ b/drivers/base/cacheinfo.c @@ -40,8 +40,9 @@ 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))) - return (this_leaf->level != 1) || (sib_leaf->level != 1); + if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI)) || + this_leaf->use_arch_info) + return (this_leaf->level != 1) && (sib_leaf->level != 1); if ((sib_leaf->attributes & CACHE_ID) && (this_leaf->attributes & CACHE_ID)) @@ -330,6 +331,11 @@ int __weak cache_setup_acpi(unsigned int cpu) return -ENOTSUPP; } +bool __weak use_arch_cache_info(unsigned int cpu) +{ + return false; +} + unsigned int coherency_max_size; static int cache_setup_properties(unsigned int cpu) @@ -349,6 +355,7 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); struct cacheinfo *this_leaf, *sib_leaf; unsigned int index, sib_index; + bool use_arch_info = false; int ret = 0; if (this_cpu_ci->cpu_map_populated) @@ -361,6 +368,12 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) */ if (!last_level_cache_is_valid(cpu)) { ret = cache_setup_properties(cpu); + // Possibility to rely on arch specific information. + if (ret && use_arch_cache_info(cpu)) { + use_arch_info = true; + ret = 0; + } + if (ret) return ret; } @@ -370,6 +383,9 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) this_leaf = per_cpu_cacheinfo_idx(cpu, index); + if (use_arch_info) + this_leaf->use_arch_info = true; + cpumask_set_cpu(cpu, &this_leaf->shared_cpu_map); for_each_online_cpu(i) { struct cpu_cacheinfo *sib_cpu_ci = get_cpu_cacheinfo(i); diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h index 908e19d17f49..bcbb8bd5759a 100644 --- a/include/linux/cacheinfo.h +++ b/include/linux/cacheinfo.h @@ -66,6 +66,7 @@ struct cacheinfo { #define CACHE_ALLOCATE_POLICY_MASK \ (CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE) #define CACHE_ID BIT(4) + bool use_arch_info; void *fw_token; bool disable_sysfs; void *priv; @@ -82,6 +83,7 @@ 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); +bool use_arch_cache_info(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);
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 <pierre.gondois@arm.com> --- arch/arm64/kernel/cacheinfo.c | 5 +++++ drivers/base/cacheinfo.c | 20 ++++++++++++++++++-- include/linux/cacheinfo.h | 2 ++ 3 files changed, 25 insertions(+), 2 deletions(-)