2023-03-27 12:04:05

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH 3/3] cacheinfo: Add use_arch[|_cache]_info field/function

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]>
---
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;
+}
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);
--
2.25.1


2023-03-27 12:20:46

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 3/3] cacheinfo: Add use_arch[|_cache]_info field/function

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 <[email protected]>
> ---
> 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

2023-04-06 07:39:04

by Pierre Gondois

[permalink] [raw]
Subject: Re: [PATCH 3/3] cacheinfo: Add use_arch[|_cache]_info field/function

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 <[email protected]>
>> ---
>> 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