Set cacheinfo.{size,sets,line_size} for each cache node, then we can
get these information from userland through auxiliary vector.
Signed-off-by: Zong Li <[email protected]>
---
arch/riscv/kernel/cacheinfo.c | 59 ++++++++++++++++++++++++++---------
1 file changed, 44 insertions(+), 15 deletions(-)
diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index bd0f122965c3..8b85abfbd77a 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -25,12 +25,46 @@ cache_get_priv_group(struct cacheinfo *this_leaf)
return NULL;
}
-static void ci_leaf_init(struct cacheinfo *this_leaf,
- struct device_node *node,
- enum cache_type type, unsigned int level)
+static void ci_leaf_init(struct cacheinfo *this_leaf, enum cache_type type,
+ unsigned int level, unsigned int size,
+ unsigned int sets, unsigned int line_size)
{
this_leaf->level = level;
this_leaf->type = type;
+ this_leaf->size = size;
+ this_leaf->number_of_sets = sets;
+ this_leaf->coherency_line_size = line_size;
+
+ /*
+ * If the cache is fully associative, there is no need to
+ * check the other properties.
+ */
+ if (!(sets == 1) && (sets > 0 && size > 0 && line_size > 0))
+ this_leaf->ways_of_associativity = (size / sets) / line_size;
+}
+
+static void fill_cacheinfo(struct cacheinfo **this_leaf,
+ struct device_node *node, unsigned int level)
+{
+ unsigned int size, sets, line_size;
+
+ if (!of_property_read_u32(node, "cache-size", &size) &&
+ !of_property_read_u32(node, "cache-block-size", &line_size) &&
+ !of_property_read_u32(node, "cache-sets", &sets)) {
+ ci_leaf_init((*this_leaf)++, CACHE_TYPE_UNIFIED, level, size, sets, line_size);
+ }
+
+ if (!of_property_read_u32(node, "i-cache-size", &size) &&
+ !of_property_read_u32(node, "i-cache-sets", &sets) &&
+ !of_property_read_u32(node, "i-cache-block-size", &line_size)) {
+ ci_leaf_init((*this_leaf)++, CACHE_TYPE_INST, level, size, sets, line_size);
+ }
+
+ if (!of_property_read_u32(node, "d-cache-size", &size) &&
+ !of_property_read_u32(node, "d-cache-sets", &sets) &&
+ !of_property_read_u32(node, "d-cache-block-size", &line_size)) {
+ ci_leaf_init((*this_leaf)++, CACHE_TYPE_DATA, level, size, sets, line_size);
+ }
}
static int __init_cache_level(unsigned int cpu)
@@ -83,29 +117,24 @@ static int __populate_cache_leaves(unsigned int cpu)
struct device_node *prev = NULL;
int levels = 1, level = 1;
- if (of_property_read_bool(np, "cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
- if (of_property_read_bool(np, "i-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
- if (of_property_read_bool(np, "d-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
+ /* Level 1 caches in cpu node */
+ fill_cacheinfo(&this_leaf, np, level);
+ /* Next level caches in cache nodes */
prev = np;
while ((np = of_find_next_cache_node(np))) {
of_node_put(prev);
prev = np;
+
if (!of_device_is_compatible(np, "cache"))
break;
if (of_property_read_u32(np, "cache-level", &level))
break;
if (level <= levels)
break;
- if (of_property_read_bool(np, "cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
- if (of_property_read_bool(np, "i-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
- if (of_property_read_bool(np, "d-cache-size"))
- ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
+
+ fill_cacheinfo(&this_leaf, np, level);
+
levels = level;
}
of_node_put(np);
--
2.28.0
Hi,
On Fri, Aug 28, 2020 at 10:09 AM Zong Li <[email protected]> wrote:
>
> Set cacheinfo.{size,sets,line_size} for each cache node, then we can
> get these information from userland through auxiliary vector.
>
> Signed-off-by: Zong Li <[email protected]>
> ---
> arch/riscv/kernel/cacheinfo.c | 59 ++++++++++++++++++++++++++---------
> 1 file changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> index bd0f122965c3..8b85abfbd77a 100644
> --- a/arch/riscv/kernel/cacheinfo.c
> +++ b/arch/riscv/kernel/cacheinfo.c
> @@ -25,12 +25,46 @@ cache_get_priv_group(struct cacheinfo *this_leaf)
> return NULL;
> }
>
> -static void ci_leaf_init(struct cacheinfo *this_leaf,
> - struct device_node *node,
> - enum cache_type type, unsigned int level)
> +static void ci_leaf_init(struct cacheinfo *this_leaf, enum cache_type type,
> + unsigned int level, unsigned int size,
> + unsigned int sets, unsigned int line_size)
> {
> this_leaf->level = level;
> this_leaf->type = type;
> + this_leaf->size = size;
> + this_leaf->number_of_sets = sets;
> + this_leaf->coherency_line_size = line_size;
> +
> + /*
> + * If the cache is fully associative, there is no need to
> + * check the other properties.
> + */
> + if (!(sets == 1) && (sets > 0 && size > 0 && line_size > 0))
Can you explain what this is attempting to do? AFAICT, the if
expression can be reduced to "sets > 1 && size > 0 && size > 0", but
what do you mean with the comment about fully associative caches?
> + this_leaf->ways_of_associativity = (size / sets) / line_size;
> +}
On Sun, Aug 30, 2020 at 3:54 PM Pekka Enberg <[email protected]> wrote:
>
> Hi,
>
> On Fri, Aug 28, 2020 at 10:09 AM Zong Li <[email protected]> wrote:
> >
> > Set cacheinfo.{size,sets,line_size} for each cache node, then we can
> > get these information from userland through auxiliary vector.
> >
> > Signed-off-by: Zong Li <[email protected]>
> > ---
> > arch/riscv/kernel/cacheinfo.c | 59 ++++++++++++++++++++++++++---------
> > 1 file changed, 44 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> > index bd0f122965c3..8b85abfbd77a 100644
> > --- a/arch/riscv/kernel/cacheinfo.c
> > +++ b/arch/riscv/kernel/cacheinfo.c
> > @@ -25,12 +25,46 @@ cache_get_priv_group(struct cacheinfo *this_leaf)
> > return NULL;
> > }
> >
> > -static void ci_leaf_init(struct cacheinfo *this_leaf,
> > - struct device_node *node,
> > - enum cache_type type, unsigned int level)
> > +static void ci_leaf_init(struct cacheinfo *this_leaf, enum cache_type type,
> > + unsigned int level, unsigned int size,
> > + unsigned int sets, unsigned int line_size)
> > {
> > this_leaf->level = level;
> > this_leaf->type = type;
> > + this_leaf->size = size;
> > + this_leaf->number_of_sets = sets;
> > + this_leaf->coherency_line_size = line_size;
> > +
> > + /*
> > + * If the cache is fully associative, there is no need to
> > + * check the other properties.
> > + */
> > + if (!(sets == 1) && (sets > 0 && size > 0 && line_size > 0))
>
> Can you explain what this is attempting to do? AFAICT, the if
> expression can be reduced to "sets > 1 && size > 0 && size > 0", but
> what do you mean with the comment about fully associative caches?
If the sets is one, it means that the cache is fully associative, then
we don't need to fill the ways number, just keep way number as zero,
so here we want to find the fully associative case first and make the
if expression fail at the beginning. We might also rewrite it as
follow:
/* Fully associative */
if (sets == 1)
return;
/* n-ways associative, make sure all properties are big than zero, it
is meaningless when one of them are zero. Full associative case is
filtered by the condition above, so we don't need to consider sets ==
1 again. */
if (sets > 0 && size > 0 && line_size >0)
this_leaf->ways_of_associativity = (size / sets) / line_size;
>
> > + this_leaf->ways_of_associativity = (size / sets) / line_size;
> > +}
Hi,
On Mon, Aug 31, 2020 at 9:15 AM Zong Li <[email protected]> wrote:
> If the sets is one, it means that the cache is fully associative, then
> we don't need to fill the ways number, just keep way number as zero,
> so here we want to find the fully associative case first and make the
> if expression fail at the beginning. We might also rewrite it as
> follow:
[snip]
Thanks for the explanation! The rewritten version is much easier to
read, at least to me.
- Pekka
On Mon, Aug 31, 2020 at 3:00 PM Pekka Enberg <[email protected]> wrote:
>
> Hi,
>
> On Mon, Aug 31, 2020 at 9:15 AM Zong Li <[email protected]> wrote:
> > If the sets is one, it means that the cache is fully associative, then
> > we don't need to fill the ways number, just keep way number as zero,
> > so here we want to find the fully associative case first and make the
> > if expression fail at the beginning. We might also rewrite it as
> > follow:
>
> [snip]
>
> Thanks for the explanation! The rewritten version is much easier to
> read, at least to me.
>
Let me change it for readability in the next version, thanks.
> - Pekka