Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1598598459.git.zong.li@sifive.com> <3f1f693147f0b62f19f4a44b8e8ef3d5bde23352.1598598459.git.zong.li@sifive.com>
 <CAOJsxLHOvvt8WGQUynGkLxZCA4OQQ+KgxxJJD7s_iP8Pxf-Omg@mail.gmail.com>
In-Reply-To: <CAOJsxLHOvvt8WGQUynGkLxZCA4OQQ+KgxxJJD7s_iP8Pxf-Omg@mail.gmail.com>
From:   Zong Li <zong.li@sifive.com>
Date:   Mon, 31 Aug 2020 14:15:38 +0800
Message-ID: <CANXhq0r2jg_fVZJE0shquGc25QNgenL+Qq6HRUBXEYPSf=QcFQ@mail.gmail.com>
Subject: Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo
To:     Pekka Enberg <penberg@gmail.com>
Cc:     Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        David Abdurachmanov <david.abdurachmanov@sifive.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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