* [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
@ 2017-11-16 12:58 Tan Xiaojun
2017-11-16 15:23 ` Sudeep Holla
0 siblings, 1 reply; 7+ messages in thread
From: Tan Xiaojun @ 2017-11-16 12:58 UTC (permalink / raw)
To: gregkh, sudeep.holla; +Cc: linux-kernel
Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
overrides for cache properties"), we can set the correct cacheinfo
via DT. But the cache type can't be set in the same way.
I found this may be a problem in recent tests. I tested L3 cache
node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
via sysfs below:
$ cat /sys/devices/system/cpu/cpu*/cache/index3/
allocation_policy level power/
shared_cpu_map uevent write_policy
coherency_line_size number_of_sets shared_cpu_list
size ways_of_associativity
This is incomplete, no type file to display type info. Because L3
cache is uncore, we can't get correct type info from system
register, and will get a default type "CACHE_TYPE_NOCACHE". Then
use "lscpu" will print an error like below:
$ lscpu
lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
No such file or directory
So I think maybe we can set correct cache type via DT too.
Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
---
drivers/base/cacheinfo.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index eb3af27..3e650dc 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
return type;
}
+static void cache_type(struct cacheinfo *this_leaf)
+{
+ const __be32 *cache_type;
+
+ cache_type = of_get_property(this_leaf->of_node, "type", NULL);
+ if (cache_type)
+ this_leaf->type = of_read_number(cache_type, 1);
+}
+
static void cache_size(struct cacheinfo *this_leaf)
{
const char *propname;
@@ -194,6 +203,7 @@ static void cache_of_override_properties(unsigned int cpu)
for (index = 0; index < cache_leaves(cpu); index++) {
this_leaf = this_cpu_ci->info_list + index;
+ cache_type(this_leaf);
cache_size(this_leaf);
cache_get_line_size(this_leaf);
cache_nr_sets(this_leaf);
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-16 12:58 [PATCH] drivers: base: cacheinfo: support DT overrides for cache type Tan Xiaojun
@ 2017-11-16 15:23 ` Sudeep Holla
2017-11-17 2:13 ` Tan Xiaojun
0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2017-11-16 15:23 UTC (permalink / raw)
To: Tan Xiaojun; +Cc: gregkh, Sudeep Holla, linux-kernel
On 16/11/17 12:58, Tan Xiaojun wrote:
> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
> overrides for cache properties"), we can set the correct cacheinfo
> via DT. But the cache type can't be set in the same way.
>
> I found this may be a problem in recent tests. I tested L3 cache
> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
> via sysfs below:
I assume L3 is outer non-architected system cache.
>
> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
> allocation_policy level power/
> shared_cpu_map uevent write_policy
> coherency_line_size number_of_sets shared_cpu_list
> size ways_of_associativity
>
> This is incomplete, no type file to display type info. Because L3
> cache is uncore, we can't get correct type info from system
> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
> use "lscpu" will print an error like below:
>
> $ lscpu
> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
> No such file or directory
>
> So I think maybe we can set correct cache type via DT too.
>
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
> ---
> drivers/base/cacheinfo.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index eb3af27..3e650dc 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
> return type;
> }
>
> +static void cache_type(struct cacheinfo *this_leaf)
> +{
> + const __be32 *cache_type;
> +
> + cache_type = of_get_property(this_leaf->of_node, "type", NULL);
NACK for this:
1. We don't have any DT binding for the "type"
2. Even if we had, we will never have such a binding that magical
returns the enum used in Linux implementation. That's not how DT
bindings are designed.
Please refer ePAPR or Devicetree Specification Release 0.1 from
devicetree.org, we have cache-unified boolean for type.
Let me know if the below patch works for you, I will submit it
preferably with your tested-by.
Regards,
Sudeep
-->8
diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
index eb3af2739537..07532d83be0b 100644
--- i/drivers/base/cacheinfo.c
+++ w/drivers/base/cacheinfo.c
@@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
*this_leaf)
this_leaf->ways_of_associativity = (size / nr_sets) /
line_size;
}
+static bool cache_node_is_unified(struct cacheinfo *this_leaf)
+{
+ return of_property_read_bool(this_leaf->of_node, "cache-unified");
+}
+
static void cache_of_override_properties(unsigned int cpu)
{
int index;
@@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
int cpu)
for (index = 0; index < cache_leaves(cpu); index++) {
this_leaf = this_cpu_ci->info_list + index;
+ /*
+ * init_cache_level must setup the cache level correctly
+ * overriding the architecturally specified levels, so
+ * if type is NONE at this stage, it should be unified
+ */
+ if (this_leaf->type == CACHE_TYPE_NOCACHE &&
+ cache_node_is_unified(this_leaf))
+ this_leaf->type = CACHE_TYPE_UNIFIED;
cache_size(this_leaf);
cache_get_line_size(this_leaf);
cache_nr_sets(this_leaf);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-16 15:23 ` Sudeep Holla
@ 2017-11-17 2:13 ` Tan Xiaojun
2017-11-17 5:37 ` Tan Xiaojun
2017-11-17 11:13 ` Sudeep Holla
0 siblings, 2 replies; 7+ messages in thread
From: Tan Xiaojun @ 2017-11-17 2:13 UTC (permalink / raw)
To: Sudeep Holla; +Cc: gregkh, linux-kernel
On 2017/11/16 23:23, Sudeep Holla wrote:
>
>
> On 16/11/17 12:58, Tan Xiaojun wrote:
>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>> overrides for cache properties"), we can set the correct cacheinfo
>> via DT. But the cache type can't be set in the same way.
>>
>> I found this may be a problem in recent tests. I tested L3 cache
>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>> via sysfs below:
>
> I assume L3 is outer non-architected system cache.
>
Yes.
>>
>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>> allocation_policy level power/
>> shared_cpu_map uevent write_policy
>> coherency_line_size number_of_sets shared_cpu_list
>> size ways_of_associativity
>>
>> This is incomplete, no type file to display type info. Because L3
>> cache is uncore, we can't get correct type info from system
>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>> use "lscpu" will print an error like below:
>>
>> $ lscpu
>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>> No such file or directory
>>
>> So I think maybe we can set correct cache type via DT too.
>>
>> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
>> ---
>> drivers/base/cacheinfo.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>> index eb3af27..3e650dc 100644
>> --- a/drivers/base/cacheinfo.c
>> +++ b/drivers/base/cacheinfo.c
>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
>> return type;
>> }
>>
>> +static void cache_type(struct cacheinfo *this_leaf)
>> +{
>> + const __be32 *cache_type;
>> +
>> + cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>
> NACK for this:
>
> 1. We don't have any DT binding for the "type"
> 2. Even if we had, we will never have such a binding that magical
> returns the enum used in Linux implementation. That's not how DT
> bindings are designed.
>
> Please refer ePAPR or Devicetree Specification Release 0.1 from
> devicetree.org, we have cache-unified boolean for type.
>
> Let me know if the below patch works for you, I will submit it
> preferably with your tested-by.
>
> Regards,
> Sudeep
>
OK. That's fine. I will test this.
By the way, Arm64 tend to use acpi way to boot now. Is there some
suitable solution for acpi?
Thanks.
Xiaojun.
> -->8
>
> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
> index eb3af2739537..07532d83be0b 100644
> --- i/drivers/base/cacheinfo.c
> +++ w/drivers/base/cacheinfo.c
> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
> *this_leaf)
> this_leaf->ways_of_associativity = (size / nr_sets) /
> line_size;
> }
>
> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
> +{
> + return of_property_read_bool(this_leaf->of_node, "cache-unified");
> +}
> +
> static void cache_of_override_properties(unsigned int cpu)
> {
> int index;
> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
> int cpu)
>
> for (index = 0; index < cache_leaves(cpu); index++) {
> this_leaf = this_cpu_ci->info_list + index;
> + /*
> + * init_cache_level must setup the cache level correctly
> + * overriding the architecturally specified levels, so
> + * if type is NONE at this stage, it should be unified
> + */
> + if (this_leaf->type == CACHE_TYPE_NOCACHE &&
> + cache_node_is_unified(this_leaf))
> + this_leaf->type = CACHE_TYPE_UNIFIED;
> cache_size(this_leaf);
> cache_get_line_size(this_leaf);
> cache_nr_sets(this_leaf);
>
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-17 2:13 ` Tan Xiaojun
@ 2017-11-17 5:37 ` Tan Xiaojun
2017-11-17 11:16 ` Sudeep Holla
2017-11-17 11:13 ` Sudeep Holla
1 sibling, 1 reply; 7+ messages in thread
From: Tan Xiaojun @ 2017-11-17 5:37 UTC (permalink / raw)
To: Sudeep Holla; +Cc: gregkh, linux-kernel
On 2017/11/17 10:13, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
>>
>>
>> On 16/11/17 12:58, Tan Xiaojun wrote:
>>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>>> overrides for cache properties"), we can set the correct cacheinfo
>>> via DT. But the cache type can't be set in the same way.
>>>
>>> I found this may be a problem in recent tests. I tested L3 cache
>>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>>> via sysfs below:
>>
>> I assume L3 is outer non-architected system cache.
>>
>
> Yes.
>
>>>
>>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>>> allocation_policy level power/
>>> shared_cpu_map uevent write_policy
>>> coherency_line_size number_of_sets shared_cpu_list
>>> size ways_of_associativity
>>>
>>> This is incomplete, no type file to display type info. Because L3
>>> cache is uncore, we can't get correct type info from system
>>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>>> use "lscpu" will print an error like below:
>>>
>>> $ lscpu
>>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>>> No such file or directory
>>>
>>> So I think maybe we can set correct cache type via DT too.
>>>
>>> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
>>> ---
>>> drivers/base/cacheinfo.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index eb3af27..3e650dc 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
>>> return type;
>>> }
>>>
>>> +static void cache_type(struct cacheinfo *this_leaf)
>>> +{
>>> + const __be32 *cache_type;
>>> +
>>> + cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>>
>> NACK for this:
>>
>> 1. We don't have any DT binding for the "type"
>> 2. Even if we had, we will never have such a binding that magical
>> returns the enum used in Linux implementation. That's not how DT
>> bindings are designed.
>>
>> Please refer ePAPR or Devicetree Specification Release 0.1 from
>> devicetree.org, we have cache-unified boolean for type.
>>
>> Let me know if the below patch works for you, I will submit it
>> preferably with your tested-by.
>>
>> Regards,
>> Sudeep
>>
>
> OK. That's fine. I will test this.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
>
> Thanks.
> Xiaojun.
>
Test passed, this is indeed a better solution.
Thanks.
Xiaojun.
>> -->8
>>
>> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
>> index eb3af2739537..07532d83be0b 100644
>> --- i/drivers/base/cacheinfo.c
>> +++ w/drivers/base/cacheinfo.c
>> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
>> *this_leaf)
>> this_leaf->ways_of_associativity = (size / nr_sets) /
>> line_size;
>> }
>>
>> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
>> +{
>> + return of_property_read_bool(this_leaf->of_node, "cache-unified");
>> +}
>> +
>> static void cache_of_override_properties(unsigned int cpu)
>> {
>> int index;
>> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
>> int cpu)
>>
>> for (index = 0; index < cache_leaves(cpu); index++) {
>> this_leaf = this_cpu_ci->info_list + index;
>> + /*
>> + * init_cache_level must setup the cache level correctly
>> + * overriding the architecturally specified levels, so
>> + * if type is NONE at this stage, it should be unified
>> + */
>> + if (this_leaf->type == CACHE_TYPE_NOCACHE &&
>> + cache_node_is_unified(this_leaf))
>> + this_leaf->type = CACHE_TYPE_UNIFIED;
>> cache_size(this_leaf);
>> cache_get_line_size(this_leaf);
>> cache_nr_sets(this_leaf);
>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-17 2:13 ` Tan Xiaojun
2017-11-17 5:37 ` Tan Xiaojun
@ 2017-11-17 11:13 ` Sudeep Holla
2017-11-20 1:18 ` Tan Xiaojun
1 sibling, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2017-11-17 11:13 UTC (permalink / raw)
To: Tan Xiaojun; +Cc: gregkh, linux-kernel, Sudeep Holla
On Fri, Nov 17, 2017 at 10:13:39AM +0800, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
> >
> > I assume L3 is outer non-architected system cache.
> >
>
> Yes.
>
Good.
> OK. That's fine. I will test this.
Thanks.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
>
Yes Jeremy is currently working on it[1] and IIRC Xiongfeng Wang from Huawei
was testing them[2]. You need to talk to him ;)
--
Regards,
Sudeep
[1] https://marc.info/?l=linux-pm&m=151026155606677&w=2
[2] https://marc.info/?l=linux-pm&m=151073816218307&w=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-17 5:37 ` Tan Xiaojun
@ 2017-11-17 11:16 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2017-11-17 11:16 UTC (permalink / raw)
To: Tan Xiaojun; +Cc: gregkh, linux-kernel, Sudeep Holla
On Fri, Nov 17, 2017 at 01:37:41PM +0800, Tan Xiaojun wrote:
> On 2017/11/17 10:13, Tan Xiaojun wrote:
> > On 2017/11/16 23:23, Sudeep Holla wrote:
[..]
> >>
> >> Let me know if the below patch works for you, I will submit it
> >> preferably with your tested-by.
> >>
> >> Regards,
> >> Sudeep
> >>
> >
> > OK. That's fine. I will test this.
> > By the way, Arm64 tend to use acpi way to boot now. Is there some
> > suitable solution for acpi?
> >
> > Thanks.
> > Xiaojun.
> >
>
> Test passed, this is indeed a better solution.
Thanks, will post the patch soon sticking you tested-by.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type
2017-11-17 11:13 ` Sudeep Holla
@ 2017-11-20 1:18 ` Tan Xiaojun
0 siblings, 0 replies; 7+ messages in thread
From: Tan Xiaojun @ 2017-11-20 1:18 UTC (permalink / raw)
To: Sudeep Holla; +Cc: gregkh, linux-kernel
On 2017/11/17 19:13, Sudeep Holla wrote:
> On Fri, Nov 17, 2017 at 10:13:39AM +0800, Tan Xiaojun wrote:
>> On 2017/11/16 23:23, Sudeep Holla wrote:
>>>
>>> I assume L3 is outer non-architected system cache.
>>>
>>
>> Yes.
>>
>
> Good.
>
>> OK. That's fine. I will test this.
>
> Thanks.
>
>> By the way, Arm64 tend to use acpi way to boot now. Is there some
>> suitable solution for acpi?
>>
>
> Yes Jeremy is currently working on it[1] and IIRC Xiongfeng Wang from Huawei
> was testing them[2]. You need to talk to him ;)
>
> --
> Regards,
> Sudeep
>
> [1] https://marc.info/?l=linux-pm&m=151026155606677&w=2
> [2] https://marc.info/?l=linux-pm&m=151073816218307&w=2
>
>
> .
>
OK. Thank you for telling me this.
Xiaojun.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-20 1:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-16 12:58 [PATCH] drivers: base: cacheinfo: support DT overrides for cache type Tan Xiaojun
2017-11-16 15:23 ` Sudeep Holla
2017-11-17 2:13 ` Tan Xiaojun
2017-11-17 5:37 ` Tan Xiaojun
2017-11-17 11:16 ` Sudeep Holla
2017-11-17 11:13 ` Sudeep Holla
2017-11-20 1:18 ` Tan Xiaojun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).