* [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init()
@ 2024-04-14 2:58 Yunhui Cui
2024-04-14 2:58 ` [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT Yunhui Cui
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yunhui Cui @ 2024-04-14 2:58 UTC (permalink / raw
To: rafael, lenb, linux-acpi, linux-kernel, paul.walmsley, palmer,
aou, linux-riscv, bhelgaas, james.morse, jhugo, jeremy.linton,
john.garry, Jonathan.Cameron, pierre.gondois, sudeep.holla,
tiantao6
Cc: Yunhui Cui
The implementation of the ci_leaf_init() function body and the caller
do not use the input parameter (struct device_node *node), so remove it.
Fixes: 6a24915145c9 ("Revert "riscv: Set more data to cacheinfo"")
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
arch/riscv/kernel/cacheinfo.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 09e9b88110d1..30a6878287ad 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -64,7 +64,6 @@ uintptr_t get_cache_geometry(u32 level, enum cache_type type)
}
static void ci_leaf_init(struct cacheinfo *this_leaf,
- struct device_node *node,
enum cache_type type, unsigned int level)
{
this_leaf->level = level;
@@ -80,11 +79,11 @@ int populate_cache_leaves(unsigned int cpu)
int levels = 1, level = 1;
if (of_property_read_bool(np, "cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
if (of_property_read_bool(np, "i-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
if (of_property_read_bool(np, "d-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
prev = np;
while ((np = of_find_next_cache_node(np))) {
@@ -97,11 +96,11 @@ int populate_cache_leaves(unsigned int cpu)
if (level <= levels)
break;
if (of_property_read_bool(np, "cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
if (of_property_read_bool(np, "i-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
if (of_property_read_bool(np, "d-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
levels = level;
}
of_node_put(np);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT
2024-04-14 2:58 [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Yunhui Cui
@ 2024-04-14 2:58 ` Yunhui Cui
2024-04-15 8:44 ` Sudeep Holla
2024-04-14 2:58 ` [PATCH v2 3/3] RISC-V: Select ACPI PPTT drivers Yunhui Cui
2024-04-15 8:47 ` [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Sudeep Holla
2 siblings, 1 reply; 7+ messages in thread
From: Yunhui Cui @ 2024-04-14 2:58 UTC (permalink / raw
To: rafael, lenb, linux-acpi, linux-kernel, paul.walmsley, palmer,
aou, linux-riscv, bhelgaas, james.morse, jhugo, jeremy.linton,
john.garry, Jonathan.Cameron, pierre.gondois, sudeep.holla,
tiantao6
Cc: Yunhui Cui
Before cacheinfo can be built correctly, we need to initialize level
and type. Since RSIC-V currently does not have a register group that
describes cache-related attributes like ARM64, we cannot obtain them
directly, so now we obtain cache leaves from the ACPI PPTT table
(acpi_get_cache_info()) and set the cache type through split_levels.
Suggested-by: Jeremy Linton <jeremy.linton@arm.com>
Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
arch/riscv/kernel/cacheinfo.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 30a6878287ad..ece92aa404e3 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -6,6 +6,7 @@
#include <linux/cpu.h>
#include <linux/of.h>
#include <asm/cacheinfo.h>
+#include <linux/acpi.h>
static struct riscv_cacheinfo_ops *rv_cache_ops;
@@ -78,6 +79,28 @@ int populate_cache_leaves(unsigned int cpu)
struct device_node *prev = NULL;
int levels = 1, level = 1;
+ if (!acpi_disabled) {
+ int ret, idx, fw_levels, split_levels;
+
+ ret = acpi_get_cache_info(cpu, &fw_levels, &split_levels);
+ if (ret)
+ return ret;
+
+ /* must be set, so we can drop num_leaves assignment below */
+ this_cpu_ci->num_leaves = fw_levels + split_levels;
+
+ for (idx = 0; level <= this_cpu_ci->num_levels &&
+ idx < this_cpu_ci->num_leaves; idx++, level++) {
+ if (level <= split_levels) {
+ ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
+ } else {
+ ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
+ }
+ }
+ return 0;
+ }
+
if (of_property_read_bool(np, "cache-size"))
ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
if (of_property_read_bool(np, "i-cache-size"))
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] RISC-V: Select ACPI PPTT drivers
2024-04-14 2:58 [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Yunhui Cui
2024-04-14 2:58 ` [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT Yunhui Cui
@ 2024-04-14 2:58 ` Yunhui Cui
2024-04-15 8:47 ` [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Sudeep Holla
2 siblings, 0 replies; 7+ messages in thread
From: Yunhui Cui @ 2024-04-14 2:58 UTC (permalink / raw
To: rafael, lenb, linux-acpi, linux-kernel, paul.walmsley, palmer,
aou, linux-riscv, bhelgaas, james.morse, jhugo, jeremy.linton,
john.garry, Jonathan.Cameron, pierre.gondois, sudeep.holla,
tiantao6
Cc: Yunhui Cui
After adding ACPI support to populate_cache_leaves(), RISC-V can build
cacheinfo through the ACPI PPTT table, thus enabling the ACPI_PPTT
configuration.
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
arch/riscv/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 6d64888134ba..5d73fcaf9136 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -14,6 +14,7 @@ config RISCV
def_bool y
select ACPI_GENERIC_GSI if ACPI
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+ select ACPI_PPTT if ACPI
select ARCH_DMA_DEFAULT_COHERENT
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT
2024-04-14 2:58 ` [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT Yunhui Cui
@ 2024-04-15 8:44 ` Sudeep Holla
2024-04-15 12:03 ` [External] " yunhui cui
0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2024-04-15 8:44 UTC (permalink / raw
To: Yunhui Cui
Cc: rafael, lenb, linux-acpi, linux-kernel, paul.walmsley, palmer,
aou, linux-riscv, bhelgaas, james.morse, jhugo, jeremy.linton,
john.garry, Jonathan.Cameron, pierre.gondois, tiantao6
On Sun, Apr 14, 2024 at 10:58:25AM +0800, Yunhui Cui wrote:
> Before cacheinfo can be built correctly, we need to initialize level
> and type. Since RSIC-V currently does not have a register group that
> describes cache-related attributes like ARM64, we cannot obtain them
> directly, so now we obtain cache leaves from the ACPI PPTT table
> (acpi_get_cache_info()) and set the cache type through split_levels.
>
> Suggested-by: Jeremy Linton <jeremy.linton@arm.com>
> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
> arch/riscv/kernel/cacheinfo.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> index 30a6878287ad..ece92aa404e3 100644
> --- a/arch/riscv/kernel/cacheinfo.c
> +++ b/arch/riscv/kernel/cacheinfo.c
> @@ -6,6 +6,7 @@
> #include <linux/cpu.h>
> #include <linux/of.h>
> #include <asm/cacheinfo.h>
> +#include <linux/acpi.h>
>
> static struct riscv_cacheinfo_ops *rv_cache_ops;
>
> @@ -78,6 +79,28 @@ int populate_cache_leaves(unsigned int cpu)
> struct device_node *prev = NULL;
> int levels = 1, level = 1;
>
> + if (!acpi_disabled) {
> + int ret, idx, fw_levels, split_levels;
> +
> + ret = acpi_get_cache_info(cpu, &fw_levels, &split_levels);
> + if (ret)
> + return ret;
> +
> + /* must be set, so we can drop num_leaves assignment below */
I intentionally added this above comment to check and drop the below statement
if it is already set. Please check if the value is already set when we call
into this function(which I think is the case).
> + this_cpu_ci->num_leaves = fw_levels + split_levels;
> +
> + for (idx = 0; level <= this_cpu_ci->num_levels &&
> + idx < this_cpu_ci->num_leaves; idx++, level++) {
> + if (level <= split_levels) {
> + ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
> + ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
> + } else {
> + ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
> + }
> + }
> + return 0;
> + }
> +
> if (of_property_read_bool(np, "cache-size"))
> ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
> if (of_property_read_bool(np, "i-cache-size"))
> --
> 2.20.1
>
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init()
2024-04-14 2:58 [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Yunhui Cui
2024-04-14 2:58 ` [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT Yunhui Cui
2024-04-14 2:58 ` [PATCH v2 3/3] RISC-V: Select ACPI PPTT drivers Yunhui Cui
@ 2024-04-15 8:47 ` Sudeep Holla
2 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2024-04-15 8:47 UTC (permalink / raw
To: Yunhui Cui
Cc: rafael, lenb, linux-acpi, Sudeep Holla, linux-kernel,
paul.walmsley, palmer, aou, linux-riscv, bhelgaas, james.morse,
jhugo, jeremy.linton, john.garry, Jonathan.Cameron,
pierre.gondois, tiantao6
On Sun, Apr 14, 2024 at 10:58:24AM +0800, Yunhui Cui wrote:
> The implementation of the ci_leaf_init() function body and the caller
> do not use the input parameter (struct device_node *node), so remove it.
>
> Fixes: 6a24915145c9 ("Revert "riscv: Set more data to cacheinfo"")
Not sure if this can be tagged as fix, but I leave that to RISC-V maintainers.
With the comment in PATCH 2/3 fixed based on your experiment, feel free
to add to the whole series,
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [External] Re: [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT
2024-04-15 8:44 ` Sudeep Holla
@ 2024-04-15 12:03 ` yunhui cui
2024-04-15 13:21 ` Sudeep Holla
0 siblings, 1 reply; 7+ messages in thread
From: yunhui cui @ 2024-04-15 12:03 UTC (permalink / raw
To: Sudeep Holla
Cc: rafael, lenb, linux-acpi, linux-kernel, paul.walmsley, palmer,
aou, linux-riscv, bhelgaas, james.morse, jhugo, jeremy.linton,
john.garry, Jonathan.Cameron, pierre.gondois, tiantao6
Hi Sudeep,
On Mon, Apr 15, 2024 at 4:45 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Sun, Apr 14, 2024 at 10:58:25AM +0800, Yunhui Cui wrote:
> > Before cacheinfo can be built correctly, we need to initialize level
> > and type. Since RSIC-V currently does not have a register group that
> > describes cache-related attributes like ARM64, we cannot obtain them
> > directly, so now we obtain cache leaves from the ACPI PPTT table
> > (acpi_get_cache_info()) and set the cache type through split_levels.
> >
> > Suggested-by: Jeremy Linton <jeremy.linton@arm.com>
> > Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> > arch/riscv/kernel/cacheinfo.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> > index 30a6878287ad..ece92aa404e3 100644
> > --- a/arch/riscv/kernel/cacheinfo.c
> > +++ b/arch/riscv/kernel/cacheinfo.c
> > @@ -6,6 +6,7 @@
> > #include <linux/cpu.h>
> > #include <linux/of.h>
> > #include <asm/cacheinfo.h>
> > +#include <linux/acpi.h>
> >
> > static struct riscv_cacheinfo_ops *rv_cache_ops;
> >
> > @@ -78,6 +79,28 @@ int populate_cache_leaves(unsigned int cpu)
> > struct device_node *prev = NULL;
> > int levels = 1, level = 1;
> >
> > + if (!acpi_disabled) {
> > + int ret, idx, fw_levels, split_levels;
> > +
> > + ret = acpi_get_cache_info(cpu, &fw_levels, &split_levels);
> > + if (ret)
> > + return ret;
> > +
> > + /* must be set, so we can drop num_leaves assignment below */
>
> I intentionally added this above comment to check and drop the below statement
> if it is already set. Please check if the value is already set when we call
> into this function(which I think is the case).
>
> > + this_cpu_ci->num_leaves = fw_levels + split_levels;
Uh,got it. I understand that there is no need to add this line:
"this_cpu_ci->num_leaves = fw_levels + split_levels; " , because in
the Master core first it will:
smp_prepare_cpus
->init_cpu_topology
->for_each_possible_cpu(cpu) {
fetch_cache_info(cpu); //num_leaves and num_levels will be set
Then store_cpu_topology->update_siblings_masks->detect_cache_attributes->populate_cache_leaves().
Slave core will follow the logic of smp_callin->store_cpu_topology().
It's the same after I tested it, so I plan to remove that line and
update V3, what do you think?
Thanks,
Yunhui
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [External] Re: [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT
2024-04-15 12:03 ` [External] " yunhui cui
@ 2024-04-15 13:21 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2024-04-15 13:21 UTC (permalink / raw
To: yunhui cui
Cc: rafael, lenb, linux-acpi, Sudeep Holla, linux-kernel,
paul.walmsley, palmer, aou, linux-riscv, bhelgaas, james.morse,
jhugo, jeremy.linton, john.garry, Jonathan.Cameron,
pierre.gondois, tiantao6
On Mon, Apr 15, 2024 at 08:03:38PM +0800, yunhui cui wrote:
> Hi Sudeep,
>
> On Mon, Apr 15, 2024 at 4:45 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Sun, Apr 14, 2024 at 10:58:25AM +0800, Yunhui Cui wrote:
> > > Before cacheinfo can be built correctly, we need to initialize level
> > > and type. Since RSIC-V currently does not have a register group that
> > > describes cache-related attributes like ARM64, we cannot obtain them
> > > directly, so now we obtain cache leaves from the ACPI PPTT table
> > > (acpi_get_cache_info()) and set the cache type through split_levels.
> > >
> > > Suggested-by: Jeremy Linton <jeremy.linton@arm.com>
> > > Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > ---
> > > arch/riscv/kernel/cacheinfo.c | 23 +++++++++++++++++++++++
> > > 1 file changed, 23 insertions(+)
> > >
> > > diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> > > index 30a6878287ad..ece92aa404e3 100644
> > > --- a/arch/riscv/kernel/cacheinfo.c
> > > +++ b/arch/riscv/kernel/cacheinfo.c
> > > @@ -6,6 +6,7 @@
> > > #include <linux/cpu.h>
> > > #include <linux/of.h>
> > > #include <asm/cacheinfo.h>
> > > +#include <linux/acpi.h>
> > >
> > > static struct riscv_cacheinfo_ops *rv_cache_ops;
> > >
> > > @@ -78,6 +79,28 @@ int populate_cache_leaves(unsigned int cpu)
> > > struct device_node *prev = NULL;
> > > int levels = 1, level = 1;
> > >
> > > + if (!acpi_disabled) {
> > > + int ret, idx, fw_levels, split_levels;
> > > +
> > > + ret = acpi_get_cache_info(cpu, &fw_levels, &split_levels);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* must be set, so we can drop num_leaves assignment below */
> >
> > I intentionally added this above comment to check and drop the below statement
> > if it is already set. Please check if the value is already set when we call
> > into this function(which I think is the case).
> >
> > > + this_cpu_ci->num_leaves = fw_levels + split_levels;
>
> Uh,got it. I understand that there is no need to add this line:
> "this_cpu_ci->num_leaves = fw_levels + split_levels; " , because in
> the Master core first it will:
> smp_prepare_cpus
> ->init_cpu_topology
> ->for_each_possible_cpu(cpu) {
> fetch_cache_info(cpu); //num_leaves and num_levels will be set
> Then store_cpu_topology->update_siblings_masks->detect_cache_attributes->populate_cache_leaves().
>
> Slave core will follow the logic of smp_callin->store_cpu_topology().
> It's the same after I tested it, so I plan to remove that line and
> update V3, what do you think?
>
Correct, just drop the statement updating "this_cpu_ci->num_leaves".
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-15 13:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-14 2:58 [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Yunhui Cui
2024-04-14 2:58 ` [PATCH v2 2/3] riscv: cacheinfo: initialize cacheinfo's level and type from ACPI PPTT Yunhui Cui
2024-04-15 8:44 ` Sudeep Holla
2024-04-15 12:03 ` [External] " yunhui cui
2024-04-15 13:21 ` Sudeep Holla
2024-04-14 2:58 ` [PATCH v2 3/3] RISC-V: Select ACPI PPTT drivers Yunhui Cui
2024-04-15 8:47 ` [PATCH v2 1/3] riscv: cacheinfo: remove the useless parameter (node) of ci_leaf_init() Sudeep Holla
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 read-only IMAP folder(s) and NNTP newsgroup(s).