Received-SPF: pass (google.com: domain of linux-kernel+bounces-134263-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240329121414.688391-1-christoph.muellner@vrull.eu>
 <20240329121414.688391-2-christoph.muellner@vrull.eu> <0b817fa8-2a1c-4da4-b5d4-f36ac93dbfd9@sifive.com>
In-Reply-To: <0b817fa8-2a1c-4da4-b5d4-f36ac93dbfd9@sifive.com>
From: =?UTF-8?Q?Christoph_M=C3=BCllner?= <christoph.muellner@vrull.eu>
Date: Sun, 7 Apr 2024 10:47:35 +0200
Message-ID: <CAEg0e7g+tRSG35pjQXemyao5OQtgiio5=nq1FGHE_5On_HZejQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] riscv: thead: Rename T-Head PBMT to MAEE
To: Samuel Holland <samuel.holland@sifive.com>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, 
	Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, 
	Albert Ou <aou@eecs.berkeley.edu>, Philipp Tomsich <philipp.tomsich@vrull.eu>, 
	=?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>, 
	Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Heiko Stuebner <heiko@sntech.de>, 
	Cooper Qu <cooper.qu@linux.alibaba.com>, Zhiwei Liu <zhiwei_liu@linux.alibaba.com>, 
	Huang Tao <eric.huang@linux.alibaba.com>, Alistair Francis <alistair.francis@wdc.com>, 
	Andrew Jones <ajones@ventanamicro.com>, Conor Dooley <conor@kernel.org>, 
	Qingfang Deng <dqfext@gmail.com>, Alexandre Ghiti <alex@ghiti.fr>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 4, 2024 at 8:15=E2=80=AFPM Samuel Holland <samuel.holland@sifiv=
e.com> wrote:
>
> Hi Christoph,
>
> On 2024-03-29 7:14 AM, Christoph M=C3=BCllner wrote:
> > T-Head's vendor extension to set page attributes has the name
> > MAEE (MMU address attribute extension).
> > Let's rename it, so it is clear what this referes to.
> >
> > See also:
> >   https://github.com/T-head-Semi/thead-extension-spec/blob/master/xthea=
dmaee.adoc
>
> My understanding is that MAEE is the name of the CSR bit and stands for "=
MMU (or
> Memory) Attribute Extension Enable", so the name for the extension itself=
 would
> be "MAE" (just one E). This is similar to THEADISAEE =3D> T-HEAD ISA Exte=
nsion
> Enable. Does that sound right?

Yes, you are right.
I noticed that before as well but did not care too much about it.
Now that I'm not the only one who sees it, I've created a PR for the spec
to rename the extension to XTheadMae:
  https://github.com/T-head-Semi/thead-extension-spec/pull/48
The PR has already been merged, so I'll provide a revised patch.

Thanks!

>
> Regards,
> Samuel
>
> > Signed-off-by: Christoph M=C3=BCllner <christoph.muellner@vrull.eu>
> > ---
> >  arch/riscv/Kconfig.errata            |  8 ++++----
> >  arch/riscv/errata/thead/errata.c     |  8 ++++----
> >  arch/riscv/include/asm/errata_list.h | 20 ++++++++++----------
> >  3 files changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> > index 910ba8837add..2c24bef7e112 100644
> > --- a/arch/riscv/Kconfig.errata
> > +++ b/arch/riscv/Kconfig.errata
> > @@ -82,14 +82,14 @@ config ERRATA_THEAD
> >
> >         Otherwise, please say "N" here to avoid unnecessary overhead.
> >
> > -config ERRATA_THEAD_PBMT
> > -     bool "Apply T-Head memory type errata"
> > +config ERRATA_THEAD_MAEE
> > +     bool "Apply T-Head's MMU address attribute (MAEE)"
> >       depends on ERRATA_THEAD && 64BIT && MMU
> >       select RISCV_ALTERNATIVE_EARLY
> >       default y
> >       help
> > -       This will apply the memory type errata to handle the non-standa=
rd
> > -       memory type bits in page-table-entries on T-Head SoCs.
> > +       This will apply the memory type errata to handle T-Head's MMU a=
ddress
> > +       attribute extension (MAEE).
> >
> >         If you don't know what to do here, say "Y".
> >
> > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead=
/errata.c
> > index b1c410bbc1ae..8c8a8a4b0421 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -19,10 +19,10 @@
> >  #include <asm/patch.h>
> >  #include <asm/vendorid_list.h>
> >
> > -static bool errata_probe_pbmt(unsigned int stage,
> > +static bool errata_probe_maee(unsigned int stage,
> >                             unsigned long arch_id, unsigned long impid)
> >  {
> > -     if (!IS_ENABLED(CONFIG_ERRATA_THEAD_PBMT))
> > +     if (!IS_ENABLED(CONFIG_ERRATA_THEAD_MAEE))
> >               return false;
> >
> >       if (arch_id !=3D 0 || impid !=3D 0)
> > @@ -140,8 +140,8 @@ static u32 thead_errata_probe(unsigned int stage,
> >  {
> >       u32 cpu_req_errata =3D 0;
> >
> > -     if (errata_probe_pbmt(stage, archid, impid))
> > -             cpu_req_errata |=3D BIT(ERRATA_THEAD_PBMT);
> > +     if (errata_probe_maee(stage, archid, impid))
> > +             cpu_req_errata |=3D BIT(ERRATA_THEAD_MAEE);
> >
> >       errata_probe_cmo(stage, archid, impid);
> >
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/=
asm/errata_list.h
> > index ea33288f8a25..7c377e137b41 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -23,7 +23,7 @@
> >  #endif
> >
> >  #ifdef CONFIG_ERRATA_THEAD
> > -#define      ERRATA_THEAD_PBMT 0
> > +#define      ERRATA_THEAD_MAEE 0
> >  #define      ERRATA_THEAD_PMU 1
> >  #define      ERRATA_THEAD_NUMBER 2
> >  #endif
> > @@ -53,20 +53,20 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFI=
VE_VENDOR_ID,  \
> >   * in the default case.
> >   */
> >  #define ALT_SVPBMT_SHIFT 61
> > -#define ALT_THEAD_PBMT_SHIFT 59
> > +#define ALT_THEAD_MAEE_SHIFT 59
> >  #define ALT_SVPBMT(_val, prot)                                        =
       \
> >  asm(ALTERNATIVE_2("li %0, 0\t\nnop",                                 \
> >                 "li %0, %1\t\nslli %0,%0,%3", 0,                      \
> >                       RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,  \
> >                 "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID,        \
> > -                     ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT)    \
> > +                     ERRATA_THEAD_MAEE, CONFIG_ERRATA_THEAD_MAEE)    \
> >               : "=3Dr"(_val)                                           =
 \
> >               : "I"(prot##_SVPBMT >> ALT_SVPBMT_SHIFT),               \
> > -               "I"(prot##_THEAD >> ALT_THEAD_PBMT_SHIFT),            \
> > +               "I"(prot##_THEAD >> ALT_THEAD_MAEE_SHIFT),            \
> >                 "I"(ALT_SVPBMT_SHIFT),                                \
> > -               "I"(ALT_THEAD_PBMT_SHIFT))
> > +               "I"(ALT_THEAD_MAEE_SHIFT))
> >
> > -#ifdef CONFIG_ERRATA_THEAD_PBMT
> > +#ifdef CONFIG_ERRATA_THEAD_MAEE
> >  /*
> >   * IO/NOCACHE memory types are handled together with svpbmt,
> >   * so on T-Head chips, check if no other memory type is set,
> > @@ -83,11 +83,11 @@ asm volatile(ALTERNATIVE(                          =
               \
> >       "slli    t3, t3, %3\n\t"                                        \
> >       "or      %0, %0, t3\n\t"                                        \
> >       "2:",  THEAD_VENDOR_ID,                                         \
> > -             ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT)            \
> > +             ERRATA_THEAD_MAEE, CONFIG_ERRATA_THEAD_MAEE)            \
> >       : "+r"(_val)                                                    \
> > -     : "I"(_PAGE_MTMASK_THEAD >> ALT_THEAD_PBMT_SHIFT),              \
> > -       "I"(_PAGE_PMA_THEAD >> ALT_THEAD_PBMT_SHIFT),                 \
> > -       "I"(ALT_THEAD_PBMT_SHIFT)                                     \
> > +     : "I"(_PAGE_MTMASK_THEAD >> ALT_THEAD_MAEE_SHIFT),              \
> > +       "I"(_PAGE_PMA_THEAD >> ALT_THEAD_MAEE_SHIFT),                 \
> > +       "I"(ALT_THEAD_MAEE_SHIFT)                                     \
> >       : "t3")
> >  #else
> >  #define ALT_THEAD_PMA(_val)
>