linux-kernel - Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 31 Aug 2020 14:15:38 +0800
From:   Zong Li <zong.li@...ive.com>
To:     Pekka Enberg <penberg@...il.com>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        David Abdurachmanov <david.abdurachmanov@...ive.com>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo

On Sun, Aug 30, 2020 at 3:54 PM Pekka Enberg <penberg@...il.com> wrote:
>
> Hi,
>
> On Fri, Aug 28, 2020 at 10:09 AM Zong Li <zong.li@...ive.com> 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 <zong.li@...ive.com>
> > ---
> >  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;
> > +}