diff mbox series

[v2,11/15] RISC-V: Adding T-Head XMAE support

Message ID 20221223180016.2068508-12-christoph.muellner@vrull.eu (mailing list archive)
State New, archived
Headers show
Series Add support for the T-Head vendor extensions | expand

Commit Message

Christoph Müllner Dec. 23, 2022, 6 p.m. UTC 
From: Christoph Müllner <christoph.muellner@vrull.eu>

This patch adds support for the T-Head specific extended memory
attributes. Similar like Svpbmt, this support does not have much effect
as most behaviour is not modelled in QEMU.

We also don't set any EDATA information, because XMAE discovery is done
using the vendor ID in the Linux kernel.

Changes in v2:
- Add ISA_EXT_DATA_ENTRY()

Co-developed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
---
 target/riscv/cpu.c        | 2 ++
 target/riscv/cpu.h        | 1 +
 target/riscv/cpu_helper.c | 6 ++++--
 3 files changed, 7 insertions(+), 2 deletions(-)

Comments

Alistair Francis Jan. 23, 2023, 11:49 p.m. UTC | #1 
On Sat, Dec 24, 2022 at 4:04 AM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> This patch adds support for the T-Head specific extended memory
> attributes. Similar like Svpbmt, this support does not have much effect
> as most behaviour is not modelled in QEMU.
>
> We also don't set any EDATA information, because XMAE discovery is done
> using the vendor ID in the Linux kernel.
>
> Changes in v2:
> - Add ISA_EXT_DATA_ENTRY()
>
> Co-developed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  target/riscv/cpu.c        | 2 ++
>  target/riscv/cpu.h        | 1 +
>  target/riscv/cpu_helper.c | 6 ++++--
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9c31a50e90..bb310755b1 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -118,6 +118,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(xtheadmemidx, true, PRIV_VERSION_1_11_0, ext_xtheadmemidx),
>      ISA_EXT_DATA_ENTRY(xtheadmempair, true, PRIV_VERSION_1_11_0, ext_xtheadmempair),
>      ISA_EXT_DATA_ENTRY(xtheadsync, true, PRIV_VERSION_1_11_0, ext_xtheadsync),
> +    ISA_EXT_DATA_ENTRY(xtheadxmae, true, PRIV_VERSION_1_11_0, ext_xtheadxmae),
>      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
>  };
>
> @@ -1080,6 +1081,7 @@ static Property riscv_cpu_extensions[] = {
>      DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx, false),
>      DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair, false),
>      DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
> +    DEFINE_PROP_BOOL("xtheadxmae", RISCVCPU, cfg.ext_xtheadxmae, false),
>      DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, false),
>
>      /* These are experimental so mark with 'x-' */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c97c1c0af0..897962f107 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -475,6 +475,7 @@ struct RISCVCPUConfig {
>      bool ext_xtheadmemidx;
>      bool ext_xtheadmempair;
>      bool ext_xtheadsync;
> +    bool ext_xtheadxmae;
>      bool ext_XVentanaCondOps;
>
>      uint8_t pmu_num;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 278d163803..345bb69b79 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -938,7 +938,8 @@ restart:
>
>          if (riscv_cpu_sxl(env) == MXL_RV32) {
>              ppn = pte >> PTE_PPN_SHIFT;
> -        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> +        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot ||
> +                   cpu->cfg.ext_xtheadxmae) {

I don't like this. This is some pretty core code that is now getting
vendor extensions. I know this is very simple, but I'm worried we are
opening the doors to other vendors adding their MMU changes.

Can we just set ext_svpbmt instead?

Alistair

>              ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>          } else {
>              ppn = pte >> PTE_PPN_SHIFT;
> @@ -950,7 +951,8 @@ restart:
>          if (!(pte & PTE_V)) {
>              /* Invalid PTE */
>              return TRANSLATE_FAIL;
> -        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT)) {
> +        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT) &&
> +                   !cpu->cfg.ext_xtheadxmae) {
>              return TRANSLATE_FAIL;
>          } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>              /* Inner PTE, continue walking */
> --
> 2.38.1
>
>
Christoph Müllner Jan. 24, 2023, 5:31 p.m. UTC | #2 
On Tue, Jan 24, 2023 at 12:49 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Sat, Dec 24, 2022 at 4:04 AM Christoph Muellner
> <christoph.muellner@vrull.eu> wrote:
> >
> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > This patch adds support for the T-Head specific extended memory
> > attributes. Similar like Svpbmt, this support does not have much effect
> > as most behaviour is not modelled in QEMU.
> >
> > We also don't set any EDATA information, because XMAE discovery is done
> > using the vendor ID in the Linux kernel.
> >
> > Changes in v2:
> > - Add ISA_EXT_DATA_ENTRY()
> >
> > Co-developed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> > ---
> >  target/riscv/cpu.c        | 2 ++
> >  target/riscv/cpu.h        | 1 +
> >  target/riscv/cpu_helper.c | 6 ++++--
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 9c31a50e90..bb310755b1 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -118,6 +118,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> >      ISA_EXT_DATA_ENTRY(xtheadmemidx, true, PRIV_VERSION_1_11_0,
> ext_xtheadmemidx),
> >      ISA_EXT_DATA_ENTRY(xtheadmempair, true, PRIV_VERSION_1_11_0,
> ext_xtheadmempair),
> >      ISA_EXT_DATA_ENTRY(xtheadsync, true, PRIV_VERSION_1_11_0,
> ext_xtheadsync),
> > +    ISA_EXT_DATA_ENTRY(xtheadxmae, true, PRIV_VERSION_1_11_0,
> ext_xtheadxmae),
> >      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
> ext_XVentanaCondOps),
> >  };
> >
> > @@ -1080,6 +1081,7 @@ static Property riscv_cpu_extensions[] = {
> >      DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx,
> false),
> >      DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair,
> false),
> >      DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
> > +    DEFINE_PROP_BOOL("xtheadxmae", RISCVCPU, cfg.ext_xtheadxmae, false),
> >      DEFINE_PROP_BOOL("xventanacondops", RISCVCPU,
> cfg.ext_XVentanaCondOps, false),
> >
> >      /* These are experimental so mark with 'x-' */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c97c1c0af0..897962f107 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -475,6 +475,7 @@ struct RISCVCPUConfig {
> >      bool ext_xtheadmemidx;
> >      bool ext_xtheadmempair;
> >      bool ext_xtheadsync;
> > +    bool ext_xtheadxmae;
> >      bool ext_XVentanaCondOps;
> >
> >      uint8_t pmu_num;
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 278d163803..345bb69b79 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -938,7 +938,8 @@ restart:
> >
> >          if (riscv_cpu_sxl(env) == MXL_RV32) {
> >              ppn = pte >> PTE_PPN_SHIFT;
> > -        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> > +        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot ||
> > +                   cpu->cfg.ext_xtheadxmae) {
>
> I don't like this. This is some pretty core code that is now getting
> vendor extensions. I know this is very simple, but I'm worried we are
> opening the doors to other vendors adding their MMU changes.
>
> Can we just set ext_svpbmt instead?
>

Ok.
I will drop this patch.


>
> Alistair
>
> >              ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> >          } else {
> >              ppn = pte >> PTE_PPN_SHIFT;
> > @@ -950,7 +951,8 @@ restart:
> >          if (!(pte & PTE_V)) {
> >              /* Invalid PTE */
> >              return TRANSLATE_FAIL;
> > -        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT)) {
> > +        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT) &&
> > +                   !cpu->cfg.ext_xtheadxmae) {
> >              return TRANSLATE_FAIL;
> >          } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> >              /* Inner PTE, continue walking */
> > --
> > 2.38.1
> >
> >
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9c31a50e90..bb310755b1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -118,6 +118,7 @@  static const struct isa_ext_data isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(xtheadmemidx, true, PRIV_VERSION_1_11_0, ext_xtheadmemidx),
     ISA_EXT_DATA_ENTRY(xtheadmempair, true, PRIV_VERSION_1_11_0, ext_xtheadmempair),
     ISA_EXT_DATA_ENTRY(xtheadsync, true, PRIV_VERSION_1_11_0, ext_xtheadsync),
+    ISA_EXT_DATA_ENTRY(xtheadxmae, true, PRIV_VERSION_1_11_0, ext_xtheadxmae),
     ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
 };
 
@@ -1080,6 +1081,7 @@  static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx, false),
     DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair, false),
     DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
+    DEFINE_PROP_BOOL("xtheadxmae", RISCVCPU, cfg.ext_xtheadxmae, false),
     DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, false),
 
     /* These are experimental so mark with 'x-' */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c97c1c0af0..897962f107 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -475,6 +475,7 @@  struct RISCVCPUConfig {
     bool ext_xtheadmemidx;
     bool ext_xtheadmempair;
     bool ext_xtheadsync;
+    bool ext_xtheadxmae;
     bool ext_XVentanaCondOps;
 
     uint8_t pmu_num;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 278d163803..345bb69b79 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -938,7 +938,8 @@  restart:
 
         if (riscv_cpu_sxl(env) == MXL_RV32) {
             ppn = pte >> PTE_PPN_SHIFT;
-        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
+        } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot ||
+                   cpu->cfg.ext_xtheadxmae) {
             ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
         } else {
             ppn = pte >> PTE_PPN_SHIFT;
@@ -950,7 +951,8 @@  restart:
         if (!(pte & PTE_V)) {
             /* Invalid PTE */
             return TRANSLATE_FAIL;
-        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT)) {
+        } else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT) &&
+                   !cpu->cfg.ext_xtheadxmae) {
             return TRANSLATE_FAIL;
         } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
             /* Inner PTE, continue walking */