diff mbox series

[v2,01/15] RISC-V: Adding XTheadCmo ISA extension

Message ID 20221223180016.2068508-2-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 XTheadCmo ISA extension.
To avoid interfering with standard extensions, decoder and translation
are in its own xthead* specific files.
Future patches should be able to easily add additional T-Head extension.

The implementation does not have much functionality (besides accepting
the instructions and not qualifying them as illegal instructions if
the hart executes in the required privilege level for the instruction),
as QEMU does not model CPU caches and instructions are documented
to not raise any exceptions.

Changes in v2:
- Add ISA_EXT_DATA_ENTRY()
- Explicit test for PRV_U
- Encapsule access to env-priv in inline function
- Use single decoder for XThead extensions

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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
 target/riscv/meson.build                   |  1 +
 target/riscv/translate.c                   | 15 +++-
 target/riscv/xthead.decode                 | 38 +++++++++
 6 files changed, 143 insertions(+), 3 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
 create mode 100644 target/riscv/xthead.decode

Comments

Alistair Francis Jan. 23, 2023, 10:49 p.m. UTC | #1 
On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> This patch adds support for the XTheadCmo ISA extension.
> To avoid interfering with standard extensions, decoder and translation
> are in its own xthead* specific files.
> Future patches should be able to easily add additional T-Head extension.
>
> The implementation does not have much functionality (besides accepting
> the instructions and not qualifying them as illegal instructions if
> the hart executes in the required privilege level for the instruction),
> as QEMU does not model CPU caches and instructions are documented
> to not raise any exceptions.
>
> Changes in v2:
> - Add ISA_EXT_DATA_ENTRY()
> - Explicit test for PRV_U
> - Encapsule access to env-priv in inline function
> - Use single decoder for XThead extensions
>
> 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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
>  target/riscv/meson.build                   |  1 +
>  target/riscv/translate.c                   | 15 +++-
>  target/riscv/xthead.decode                 | 38 +++++++++
>  6 files changed, 143 insertions(+), 3 deletions(-)
>  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
>  create mode 100644 target/riscv/xthead.decode
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6fe176e483..a90b82c5c5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
>      ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
>      ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> +    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0, ext_xtheadcmo),
>      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
>  };
>
> @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
>      DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>
>      /* Vendor-specific custom extensions */
> +    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
>      uint64_t mimpid;
>
>      /* Vendor-specific custom extensions */
> +    bool ext_xtheadcmo;
>      bool ext_XVentanaCondOps;
>
>      uint8_t pmu_num;
> diff --git a/target/riscv/insn_trans/trans_xthead.c.inc b/target/riscv/insn_trans/trans_xthead.c.inc
> new file mode 100644
> index 0000000000..00e75c7dca
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_xthead.c.inc
> @@ -0,0 +1,89 @@
> +/*
> + * RISC-V translation routines for the T-Head vendor extensions (xthead*).
> + *
> + * Copyright (c) 2022 VRULL GmbH.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define REQUIRE_XTHEADCMO(ctx) do {              \
> +    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
> +        return false;                            \
> +    }                                            \
> +} while (0)
> +
> +/* XTheadCmo */
> +
> +static inline int priv_level(DisasContext *ctx)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return PRV_U;
> +#else
> +     /* Priv level equals mem_idx -- see cpu_mmu_index. */
> +    return ctx->mem_idx;

This should be ANDed with TB_FLAGS_PRIV_MMU_MASK as sometimes this can
include hypervisor priv access information

> +#endif
> +}
> +
> +#define REQUIRE_PRIV_MHSU(ctx)                                  \
> +do {                                                            \
> +    int priv = priv_level(ctx);                                 \
> +    if (!(priv == PRV_M ||                                      \
> +          priv == PRV_H ||                                      \

PRV_H isn't used

> +          priv == PRV_S ||                                      \
> +          priv == PRV_U)) {                                     \
> +        return false;                                           \

When would this not be the case?

> +    }                                                           \
> +} while (0)
> +
> +#define REQUIRE_PRIV_MHS(ctx)                                   \
> +do {                                                            \
> +    int priv = priv_level(ctx);                                 \
> +    if (!(priv == PRV_M ||                                      \
> +          priv == PRV_H ||                                      \

Also not used

> +          priv == PRV_S)) {                                     \
> +        return false;                                           \
> +    }                                                           \
> +} while (0)
> +
> +#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
> +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
> +{                                                               \
> +    (void) a;                                                   \
> +    extcheck(ctx);                                              \
> +    privcheck(ctx);                                             \
> +    return true;                                                \
> +}
> +
> +NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> +NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> +NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> +NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +
> +NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> +
> +NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> +NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index ba25164d74..5dee37a242 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -2,6 +2,7 @@
>  gen = [
>    decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
>    decodetree.process('insn32.decode', extra_args: '--static-decode=decode_insn32'),
> +  decodetree.process('xthead.decode', extra_args: '--static-decode=decode_xthead'),
>    decodetree.process('XVentanaCondOps.decode', extra_args: '--static-decode=decode_XVentanaCodeOps'),
>  ]
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index db123da5ec..14d9116975 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -125,13 +125,18 @@ static bool always_true_p(DisasContext *ctx  __attribute__((__unused__)))
>      return true;
>  }
>
> +static bool has_xthead_p(DisasContext *ctx  __attribute__((__unused__)))
> +{
> +    return ctx->cfg_ptr->ext_xtheadcmo;
> +}
> +
>  #define MATERIALISE_EXT_PREDICATE(ext)  \
>      static bool has_ ## ext ## _p(DisasContext *ctx)    \
>      { \
>          return ctx->cfg_ptr->ext_ ## ext ; \
>      }
>
> -MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
> +MATERIALISE_EXT_PREDICATE(XVentanaCondOps)

Do we need this change?

>
>  #ifdef TARGET_RISCV32
>  #define get_xl(ctx)    MXL_RV32
> @@ -732,6 +737,10 @@ static int ex_rvc_shiftri(DisasContext *ctx, int imm)
>  /* Include the auto-generated decoder for 32 bit insn */
>  #include "decode-insn32.c.inc"
>
> +/* Include decoders for factored-out extensions */
> +#include "decode-xthead.c.inc"
> +#include "decode-XVentanaCondOps.c.inc"
> +
>  static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
>                               void (*func)(TCGv, TCGv, target_long))
>  {
> @@ -1033,12 +1042,11 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>  #include "insn_trans/trans_rvk.c.inc"
>  #include "insn_trans/trans_privileged.c.inc"
>  #include "insn_trans/trans_svinval.c.inc"
> +#include "insn_trans/trans_xthead.c.inc"
>  #include "insn_trans/trans_xventanacondops.c.inc"
>
>  /* Include the auto-generated decoder for 16 bit insn */
>  #include "decode-insn16.c.inc"
> -/* Include decoders for factored-out extensions */
> -#include "decode-XVentanaCondOps.c.inc"

Can we not leave these at the bottom?

Alistair

>
>  /* The specification allows for longer insns, but not supported by qemu. */
>  #define MAX_INSN_LEN  4
> @@ -1059,6 +1067,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>          bool (*decode_func)(DisasContext *, uint32_t);
>      } decoders[] = {
>          { always_true_p,  decode_insn32 },
> +        { has_xthead_p, decode_xthead },
>          { has_XVentanaCondOps_p,  decode_XVentanaCodeOps },
>      };
>
> diff --git a/target/riscv/xthead.decode b/target/riscv/xthead.decode
> new file mode 100644
> index 0000000000..30533a66f5
> --- /dev/null
> +++ b/target/riscv/xthead.decode
> @@ -0,0 +1,38 @@
> +#
> +# Translation routines for the instructions of the XThead* ISA extensions
> +#
> +# Copyright (c) 2022 Christoph Muellner, christoph.muellner@vrull.eu
> +#
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# The documentation of the ISA extensions can be found here:
> +#   https://github.com/T-head-Semi/thead-extension-spec/releases/latest
> +
> +# Fields:
> +%rs1       15:5
> +
> +# Formats
> +@sfence_vm  ....... ..... .....   ... ..... ....... %rs1
> +
> +# XTheadCmo
> +th_dcache_call   0000000 00001 00000 000 00000 0001011
> +th_dcache_ciall  0000000 00011 00000 000 00000 0001011
> +th_dcache_iall   0000000 00010 00000 000 00000 0001011
> +th_dcache_cpa    0000001 01001 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_cipa   0000001 01011 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_ipa    0000001 01010 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_cva    0000001 00101 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_civa   0000001 00111 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_iva    0000001 00110 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_csw    0000001 00001 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_cisw   0000001 00011 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_isw    0000001 00010 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_cpal1  0000001 01000 ..... 000 00000 0001011 @sfence_vm
> +th_dcache_cval1  0000001 00100 ..... 000 00000 0001011 @sfence_vm
> +th_icache_iall   0000000 10000 00000 000 00000 0001011
> +th_icache_ialls  0000000 10001 00000 000 00000 0001011
> +th_icache_ipa    0000001 11000 ..... 000 00000 0001011 @sfence_vm
> +th_icache_iva    0000001 10000 ..... 000 00000 0001011 @sfence_vm
> +th_l2cache_call  0000000 10101 00000 000 00000 0001011
> +th_l2cache_ciall 0000000 10111 00000 000 00000 0001011
> +th_l2cache_iall  0000000 10110 00000 000 00000 0001011
> --
> 2.38.1
>
>
Christoph Müllner Jan. 24, 2023, 5:31 p.m. UTC | #2 
On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis <alistair23@gmail.com>
wrote:

> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
> <christoph.muellner@vrull.eu> wrote:
> >
> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > This patch adds support for the XTheadCmo ISA extension.
> > To avoid interfering with standard extensions, decoder and translation
> > are in its own xthead* specific files.
> > Future patches should be able to easily add additional T-Head extension.
> >
> > The implementation does not have much functionality (besides accepting
> > the instructions and not qualifying them as illegal instructions if
> > the hart executes in the required privilege level for the instruction),
> > as QEMU does not model CPU caches and instructions are documented
> > to not raise any exceptions.
> >
> > Changes in v2:
> > - Add ISA_EXT_DATA_ENTRY()
> > - Explicit test for PRV_U
> > - Encapsule access to env-priv in inline function
> > - Use single decoder for XThead extensions
> >
> > 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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
> >  target/riscv/meson.build                   |  1 +
> >  target/riscv/translate.c                   | 15 +++-
> >  target/riscv/xthead.decode                 | 38 +++++++++
> >  6 files changed, 143 insertions(+), 3 deletions(-)
> >  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
> >  create mode 100644 target/riscv/xthead.decode
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 6fe176e483..a90b82c5c5 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> >      ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
> >      ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
> >      ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> > +    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0,
> ext_xtheadcmo),
> >      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
> ext_XVentanaCondOps),
> >  };
> >
> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
> >      DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> >
> >      /* Vendor-specific custom extensions */
> > +    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
> >      uint64_t mimpid;
> >
> >      /* Vendor-specific custom extensions */
> > +    bool ext_xtheadcmo;
> >      bool ext_XVentanaCondOps;
> >
> >      uint8_t pmu_num;
> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc
> b/target/riscv/insn_trans/trans_xthead.c.inc
> > new file mode 100644
> > index 0000000000..00e75c7dca
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
> > @@ -0,0 +1,89 @@
> > +/*
> > + * RISC-V translation routines for the T-Head vendor extensions
> (xthead*).
> > + *
> > + * Copyright (c) 2022 VRULL GmbH.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#define REQUIRE_XTHEADCMO(ctx) do {              \
> > +    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
> > +        return false;                            \
> > +    }                                            \
> > +} while (0)
> > +
> > +/* XTheadCmo */
> > +
> > +static inline int priv_level(DisasContext *ctx)
> > +{
> > +#ifdef CONFIG_USER_ONLY
> > +    return PRV_U;
> > +#else
> > +     /* Priv level equals mem_idx -- see cpu_mmu_index. */
> > +    return ctx->mem_idx;
>
> This should be ANDed with TB_FLAGS_PRIV_MMU_MASK as sometimes this can
> include hypervisor priv access information
>

Ok.


>
> > +#endif
> > +}
> > +
> > +#define REQUIRE_PRIV_MHSU(ctx)                                  \
> > +do {                                                            \
> > +    int priv = priv_level(ctx);                                 \
> > +    if (!(priv == PRV_M ||                                      \
> > +          priv == PRV_H ||                                      \
>
> PRV_H isn't used
>
> > +          priv == PRV_S ||                                      \
> > +          priv == PRV_U)) {                                     \
> > +        return false;                                           \
>
> When would this not be the case?
>

Ok, I will make this a macro that expands to nothing (and a comment).


>
> > +    }                                                           \
> > +} while (0)
> > +
> > +#define REQUIRE_PRIV_MHS(ctx)                                   \
> > +do {                                                            \
> > +    int priv = priv_level(ctx);                                 \
> > +    if (!(priv == PRV_M ||                                      \
> > +          priv == PRV_H ||                                      \
>
> Also not used
>

Ok, I will remove the PRV_H.


>
> > +          priv == PRV_S)) {                                     \
> > +        return false;                                           \
> > +    }                                                           \
> > +} while (0)
> > +
> > +#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
> > +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
> > +{                                                               \
> > +    (void) a;                                                   \
> > +    extcheck(ctx);                                              \
> > +    privcheck(ctx);                                             \
> > +    return true;                                                \
> > +}
> > +
> > +NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +
> > +NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> > +
> > +NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> > index ba25164d74..5dee37a242 100644
> > --- a/target/riscv/meson.build
> > +++ b/target/riscv/meson.build
> > @@ -2,6 +2,7 @@
> >  gen = [
> >    decodetree.process('insn16.decode', extra_args:
> ['--static-decode=decode_insn16', '--insnwidth=16']),
> >    decodetree.process('insn32.decode', extra_args:
> '--static-decode=decode_insn32'),
> > +  decodetree.process('xthead.decode', extra_args:
> '--static-decode=decode_xthead'),
> >    decodetree.process('XVentanaCondOps.decode', extra_args:
> '--static-decode=decode_XVentanaCodeOps'),
> >  ]
> >
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index db123da5ec..14d9116975 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -125,13 +125,18 @@ static bool always_true_p(DisasContext *ctx
> __attribute__((__unused__)))
> >      return true;
> >  }
> >
> > +static bool has_xthead_p(DisasContext *ctx  __attribute__((__unused__)))
> > +{
> > +    return ctx->cfg_ptr->ext_xtheadcmo;
> > +}
> > +
> >  #define MATERIALISE_EXT_PREDICATE(ext)  \
> >      static bool has_ ## ext ## _p(DisasContext *ctx)    \
> >      { \
> >          return ctx->cfg_ptr->ext_ ## ext ; \
> >      }
> >
> > -MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
> > +MATERIALISE_EXT_PREDICATE(XVentanaCondOps)
>
> Do we need this change?
>

It is indeed a drive-by cleanup, that is not necessary.
In v1 we were using this macro, therefore it made sense back then.
Will be dropped.


>
> >
> >  #ifdef TARGET_RISCV32
> >  #define get_xl(ctx)    MXL_RV32
> > @@ -732,6 +737,10 @@ static int ex_rvc_shiftri(DisasContext *ctx, int
> imm)
> >  /* Include the auto-generated decoder for 32 bit insn */
> >  #include "decode-insn32.c.inc"
> >
> > +/* Include decoders for factored-out extensions */
> > +#include "decode-xthead.c.inc"
> > +#include "decode-XVentanaCondOps.c.inc"
> > +
> >  static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
> >                               void (*func)(TCGv, TCGv, target_long))
> >  {
> > @@ -1033,12 +1042,11 @@ static uint32_t opcode_at(DisasContextBase
> *dcbase, target_ulong pc)
> >  #include "insn_trans/trans_rvk.c.inc"
> >  #include "insn_trans/trans_privileged.c.inc"
> >  #include "insn_trans/trans_svinval.c.inc"
> > +#include "insn_trans/trans_xthead.c.inc"
> >  #include "insn_trans/trans_xventanacondops.c.inc"
> >
> >  /* Include the auto-generated decoder for 16 bit insn */
> >  #include "decode-insn16.c.inc"
> > -/* Include decoders for factored-out extensions */
> > -#include "decode-XVentanaCondOps.c.inc"
>
> Can we not leave these at the bottom?
>

Ok.


> Alistair
>
> >
> >  /* The specification allows for longer insns, but not supported by
> qemu. */
> >  #define MAX_INSN_LEN  4
> > @@ -1059,6 +1067,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
> >          bool (*decode_func)(DisasContext *, uint32_t);
> >      } decoders[] = {
> >          { always_true_p,  decode_insn32 },
> > +        { has_xthead_p, decode_xthead },
> >          { has_XVentanaCondOps_p,  decode_XVentanaCodeOps },
> >      };
> >
> > diff --git a/target/riscv/xthead.decode b/target/riscv/xthead.decode
> > new file mode 100644
> > index 0000000000..30533a66f5
> > --- /dev/null
> > +++ b/target/riscv/xthead.decode
> > @@ -0,0 +1,38 @@
> > +#
> > +# Translation routines for the instructions of the XThead* ISA
> extensions
> > +#
> > +# Copyright (c) 2022 Christoph Muellner, christoph.muellner@vrull.eu
> > +#
> > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > +#
> > +# The documentation of the ISA extensions can be found here:
> > +#   https://github.com/T-head-Semi/thead-extension-spec/releases/latest
> > +
> > +# Fields:
> > +%rs1       15:5
> > +
> > +# Formats
> > +@sfence_vm  ....... ..... .....   ... ..... ....... %rs1
> > +
> > +# XTheadCmo
> > +th_dcache_call   0000000 00001 00000 000 00000 0001011
> > +th_dcache_ciall  0000000 00011 00000 000 00000 0001011
> > +th_dcache_iall   0000000 00010 00000 000 00000 0001011
> > +th_dcache_cpa    0000001 01001 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_cipa   0000001 01011 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_ipa    0000001 01010 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_cva    0000001 00101 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_civa   0000001 00111 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_iva    0000001 00110 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_csw    0000001 00001 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_cisw   0000001 00011 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_isw    0000001 00010 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_cpal1  0000001 01000 ..... 000 00000 0001011 @sfence_vm
> > +th_dcache_cval1  0000001 00100 ..... 000 00000 0001011 @sfence_vm
> > +th_icache_iall   0000000 10000 00000 000 00000 0001011
> > +th_icache_ialls  0000000 10001 00000 000 00000 0001011
> > +th_icache_ipa    0000001 11000 ..... 000 00000 0001011 @sfence_vm
> > +th_icache_iva    0000001 10000 ..... 000 00000 0001011 @sfence_vm
> > +th_l2cache_call  0000000 10101 00000 000 00000 0001011
> > +th_l2cache_ciall 0000000 10111 00000 000 00000 0001011
> > +th_l2cache_iall  0000000 10110 00000 000 00000 0001011
> > --
> > 2.38.1
> >
> >
>
Christoph Müllner Jan. 24, 2023, 7:51 p.m. UTC | #3 
On Tue, Jan 24, 2023 at 6:31 PM Christoph Müllner <
christoph.muellner@vrull.eu> wrote:

>
>
> On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis <alistair23@gmail.com>
> wrote:
>
>> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
>> <christoph.muellner@vrull.eu> wrote:
>> >
>> > From: Christoph Müllner <christoph.muellner@vrull.eu>
>> >
>> > This patch adds support for the XTheadCmo ISA extension.
>> > To avoid interfering with standard extensions, decoder and translation
>> > are in its own xthead* specific files.
>> > Future patches should be able to easily add additional T-Head extension.
>> >
>> > The implementation does not have much functionality (besides accepting
>> > the instructions and not qualifying them as illegal instructions if
>> > the hart executes in the required privilege level for the instruction),
>> > as QEMU does not model CPU caches and instructions are documented
>> > to not raise any exceptions.
>> >
>> > Changes in v2:
>> > - Add ISA_EXT_DATA_ENTRY()
>> > - Explicit test for PRV_U
>> > - Encapsule access to env-priv in inline function
>> > - Use single decoder for XThead extensions
>> >
>> > 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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
>> >  target/riscv/meson.build                   |  1 +
>> >  target/riscv/translate.c                   | 15 +++-
>> >  target/riscv/xthead.decode                 | 38 +++++++++
>> >  6 files changed, 143 insertions(+), 3 deletions(-)
>> >  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
>> >  create mode 100644 target/riscv/xthead.decode
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 6fe176e483..a90b82c5c5 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>> >      ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0,
>> ext_svinval),
>> >      ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0,
>> ext_svnapot),
>> >      ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
>> > +    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0,
>> ext_xtheadcmo),
>> >      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
>> ext_XVentanaCondOps),
>> >  };
>> >
>> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
>> >      DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>> >
>> >      /* Vendor-specific custom extensions */
>> > +    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
>> >      uint64_t mimpid;
>> >
>> >      /* Vendor-specific custom extensions */
>> > +    bool ext_xtheadcmo;
>> >      bool ext_XVentanaCondOps;
>> >
>> >      uint8_t pmu_num;
>> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc
>> b/target/riscv/insn_trans/trans_xthead.c.inc
>> > new file mode 100644
>> > index 0000000000..00e75c7dca
>> > --- /dev/null
>> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
>> > @@ -0,0 +1,89 @@
>> > +/*
>> > + * RISC-V translation routines for the T-Head vendor extensions
>> (xthead*).
>> > + *
>> > + * Copyright (c) 2022 VRULL GmbH.
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> modify it
>> > + * under the terms and conditions of the GNU General Public License,
>> > + * version 2 or later, as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope it will be useful, but
>> WITHOUT
>> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> or
>> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
>> License for
>> > + * more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> along with
>> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> > + */
>> > +
>> > +#define REQUIRE_XTHEADCMO(ctx) do {              \
>> > +    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
>> > +        return false;                            \
>> > +    }                                            \
>> > +} while (0)
>> > +
>> > +/* XTheadCmo */
>> > +
>> > +static inline int priv_level(DisasContext *ctx)
>> > +{
>> > +#ifdef CONFIG_USER_ONLY
>> > +    return PRV_U;
>> > +#else
>> > +     /* Priv level equals mem_idx -- see cpu_mmu_index. */
>> > +    return ctx->mem_idx;
>>
>> This should be ANDed with TB_FLAGS_PRIV_MMU_MASK as sometimes this can
>> include hypervisor priv access information
>>
>
> Ok.
>
>
>>
>> > +#endif
>> > +}
>> > +
>> > +#define REQUIRE_PRIV_MHSU(ctx)                                  \
>> > +do {                                                            \
>> > +    int priv = priv_level(ctx);                                 \
>> > +    if (!(priv == PRV_M ||                                      \
>> > +          priv == PRV_H ||                                      \
>>
>> PRV_H isn't used
>>
>> > +          priv == PRV_S ||                                      \
>> > +          priv == PRV_U)) {                                     \
>> > +        return false;                                           \
>>
>> When would this not be the case?
>>
>
> Ok, I will make this a macro that expands to nothing (and a comment).
>
>
>>
>> > +    }                                                           \
>> > +} while (0)
>> > +
>> > +#define REQUIRE_PRIV_MHS(ctx)                                   \
>> > +do {                                                            \
>> > +    int priv = priv_level(ctx);                                 \
>> > +    if (!(priv == PRV_M ||                                      \
>> > +          priv == PRV_H ||                                      \
>>
>> Also not used
>>
>
> Ok, I will remove the PRV_H.
>
>
>>
>> > +          priv == PRV_S)) {                                     \
>> > +        return false;                                           \
>> > +    }                                                           \
>> > +} while (0)
>> > +
>> > +#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
>> > +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
>> > +{                                                               \
>> > +    (void) a;                                                   \
>> > +    extcheck(ctx);                                              \
>> > +    privcheck(ctx);                                             \
>> > +    return true;                                                \
>> > +}
>> > +
>> > +NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>> > +NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>> > +NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>> > +NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +
>> > +NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>> > +
>> > +NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > +NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
>> > index ba25164d74..5dee37a242 100644
>> > --- a/target/riscv/meson.build
>> > +++ b/target/riscv/meson.build
>> > @@ -2,6 +2,7 @@
>> >  gen = [
>> >    decodetree.process('insn16.decode', extra_args:
>> ['--static-decode=decode_insn16', '--insnwidth=16']),
>> >    decodetree.process('insn32.decode', extra_args:
>> '--static-decode=decode_insn32'),
>> > +  decodetree.process('xthead.decode', extra_args:
>> '--static-decode=decode_xthead'),
>> >    decodetree.process('XVentanaCondOps.decode', extra_args:
>> '--static-decode=decode_XVentanaCodeOps'),
>> >  ]
>> >
>> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> > index db123da5ec..14d9116975 100644
>> > --- a/target/riscv/translate.c
>> > +++ b/target/riscv/translate.c
>> > @@ -125,13 +125,18 @@ static bool always_true_p(DisasContext *ctx
>> __attribute__((__unused__)))
>> >      return true;
>> >  }
>> >
>> > +static bool has_xthead_p(DisasContext *ctx
>> __attribute__((__unused__)))
>> > +{
>> > +    return ctx->cfg_ptr->ext_xtheadcmo;
>> > +}
>> > +
>> >  #define MATERIALISE_EXT_PREDICATE(ext)  \
>> >      static bool has_ ## ext ## _p(DisasContext *ctx)    \
>> >      { \
>> >          return ctx->cfg_ptr->ext_ ## ext ; \
>> >      }
>> >
>> > -MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
>> > +MATERIALISE_EXT_PREDICATE(XVentanaCondOps)
>>
>> Do we need this change?
>>
>
> It is indeed a drive-by cleanup, that is not necessary.
> In v1 we were using this macro, therefore it made sense back then.
> Will be dropped.
>
>
>>
>> >
>> >  #ifdef TARGET_RISCV32
>> >  #define get_xl(ctx)    MXL_RV32
>> > @@ -732,6 +737,10 @@ static int ex_rvc_shiftri(DisasContext *ctx, int
>> imm)
>> >  /* Include the auto-generated decoder for 32 bit insn */
>> >  #include "decode-insn32.c.inc"
>> >
>> > +/* Include decoders for factored-out extensions */
>> > +#include "decode-xthead.c.inc"
>> > +#include "decode-XVentanaCondOps.c.inc"
>> > +
>> >  static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
>> >                               void (*func)(TCGv, TCGv, target_long))
>> >  {
>> > @@ -1033,12 +1042,11 @@ static uint32_t opcode_at(DisasContextBase
>> *dcbase, target_ulong pc)
>> >  #include "insn_trans/trans_rvk.c.inc"
>> >  #include "insn_trans/trans_privileged.c.inc"
>> >  #include "insn_trans/trans_svinval.c.inc"
>> > +#include "insn_trans/trans_xthead.c.inc"
>> >  #include "insn_trans/trans_xventanacondops.c.inc"
>> >
>> >  /* Include the auto-generated decoder for 16 bit insn */
>> >  #include "decode-insn16.c.inc"
>> > -/* Include decoders for factored-out extensions */
>> > -#include "decode-XVentanaCondOps.c.inc"
>>
>> Can we not leave these at the bottom?
>>
>
> Ok.
>

I got reminded again, why this is like it is:
The decoder code needs to be included before the translation functions,
because the translation functions use types that are defined in the
generated decoder code.
And I wanted to keep all vendor extensions together.
I think your concern is about touching other code. Therefore, I will not
touch the VT decoder position in the v3.
Let me know if you prefer another solution.

BR
Christoph



>
>
>> Alistair
>>
>> >
>> >  /* The specification allows for longer insns, but not supported by
>> qemu. */
>> >  #define MAX_INSN_LEN  4
>> > @@ -1059,6 +1067,7 @@ static void decode_opc(CPURISCVState *env,
>> DisasContext *ctx, uint16_t opcode)
>> >          bool (*decode_func)(DisasContext *, uint32_t);
>> >      } decoders[] = {
>> >          { always_true_p,  decode_insn32 },
>> > +        { has_xthead_p, decode_xthead },
>> >          { has_XVentanaCondOps_p,  decode_XVentanaCodeOps },
>> >      };
>> >
>> > diff --git a/target/riscv/xthead.decode b/target/riscv/xthead.decode
>> > new file mode 100644
>> > index 0000000000..30533a66f5
>> > --- /dev/null
>> > +++ b/target/riscv/xthead.decode
>> > @@ -0,0 +1,38 @@
>> > +#
>> > +# Translation routines for the instructions of the XThead* ISA
>> extensions
>> > +#
>> > +# Copyright (c) 2022 Christoph Muellner, christoph.muellner@vrull.eu
>> > +#
>> > +# SPDX-License-Identifier: LGPL-2.1-or-later
>> > +#
>> > +# The documentation of the ISA extensions can be found here:
>> > +#
>> https://github.com/T-head-Semi/thead-extension-spec/releases/latest
>> > +
>> > +# Fields:
>> > +%rs1       15:5
>> > +
>> > +# Formats
>> > +@sfence_vm  ....... ..... .....   ... ..... ....... %rs1
>> > +
>> > +# XTheadCmo
>> > +th_dcache_call   0000000 00001 00000 000 00000 0001011
>> > +th_dcache_ciall  0000000 00011 00000 000 00000 0001011
>> > +th_dcache_iall   0000000 00010 00000 000 00000 0001011
>> > +th_dcache_cpa    0000001 01001 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_cipa   0000001 01011 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_ipa    0000001 01010 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_cva    0000001 00101 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_civa   0000001 00111 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_iva    0000001 00110 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_csw    0000001 00001 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_cisw   0000001 00011 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_isw    0000001 00010 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_cpal1  0000001 01000 ..... 000 00000 0001011 @sfence_vm
>> > +th_dcache_cval1  0000001 00100 ..... 000 00000 0001011 @sfence_vm
>> > +th_icache_iall   0000000 10000 00000 000 00000 0001011
>> > +th_icache_ialls  0000000 10001 00000 000 00000 0001011
>> > +th_icache_ipa    0000001 11000 ..... 000 00000 0001011 @sfence_vm
>> > +th_icache_iva    0000001 10000 ..... 000 00000 0001011 @sfence_vm
>> > +th_l2cache_call  0000000 10101 00000 000 00000 0001011
>> > +th_l2cache_ciall 0000000 10111 00000 000 00000 0001011
>> > +th_l2cache_iall  0000000 10110 00000 000 00000 0001011
>> > --
>> > 2.38.1
>> >
>> >
>>
>
Alistair Francis Jan. 29, 2023, 10:40 p.m. UTC | #4 
On Wed, Jan 25, 2023 at 5:51 AM Christoph Müllner
<christoph.muellner@vrull.eu> wrote:
>
>
>
> On Tue, Jan 24, 2023 at 6:31 PM Christoph Müllner <christoph.muellner@vrull.eu> wrote:
>>
>>
>>
>> On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>
>>> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
>>> <christoph.muellner@vrull.eu> wrote:
>>> >
>>> > From: Christoph Müllner <christoph.muellner@vrull.eu>
>>> >
>>> > This patch adds support for the XTheadCmo ISA extension.
>>> > To avoid interfering with standard extensions, decoder and translation
>>> > are in its own xthead* specific files.
>>> > Future patches should be able to easily add additional T-Head extension.
>>> >
>>> > The implementation does not have much functionality (besides accepting
>>> > the instructions and not qualifying them as illegal instructions if
>>> > the hart executes in the required privilege level for the instruction),
>>> > as QEMU does not model CPU caches and instructions are documented
>>> > to not raise any exceptions.
>>> >
>>> > Changes in v2:
>>> > - Add ISA_EXT_DATA_ENTRY()
>>> > - Explicit test for PRV_U
>>> > - Encapsule access to env-priv in inline function
>>> > - Use single decoder for XThead extensions
>>> >
>>> > 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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
>>> >  target/riscv/meson.build                   |  1 +
>>> >  target/riscv/translate.c                   | 15 +++-
>>> >  target/riscv/xthead.decode                 | 38 +++++++++
>>> >  6 files changed, 143 insertions(+), 3 deletions(-)
>>> >  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
>>> >  create mode 100644 target/riscv/xthead.decode
>>> >
>>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> > index 6fe176e483..a90b82c5c5 100644
>>> > --- a/target/riscv/cpu.c
>>> > +++ b/target/riscv/cpu.c
>>> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>>> >      ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
>>> >      ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
>>> >      ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
>>> > +    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0, ext_xtheadcmo),
>>> >      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
>>> >  };
>>> >
>>> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
>>> >      DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>>> >
>>> >      /* Vendor-specific custom extensions */
>>> > +    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
>>> > --- a/target/riscv/cpu.h
>>> > +++ b/target/riscv/cpu.h
>>> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
>>> >      uint64_t mimpid;
>>> >
>>> >      /* Vendor-specific custom extensions */
>>> > +    bool ext_xtheadcmo;
>>> >      bool ext_XVentanaCondOps;
>>> >
>>> >      uint8_t pmu_num;
>>> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc b/target/riscv/insn_trans/trans_xthead.c.inc
>>> > new file mode 100644
>>> > index 0000000000..00e75c7dca
>>> > --- /dev/null
>>> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
>>> > @@ -0,0 +1,89 @@
>>> > +/*
>>> > + * RISC-V translation routines for the T-Head vendor extensions (xthead*).
>>> > + *
>>> > + * Copyright (c) 2022 VRULL GmbH.
>>> > + *
>>> > + * This program is free software; you can redistribute it and/or modify it
>>> > + * under the terms and conditions of the GNU General Public License,
>>> > + * version 2 or later, as published by the Free Software Foundation.
>>> > + *
>>> > + * This program is distributed in the hope it will be useful, but WITHOUT
>>> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>> > + * more details.
>>> > + *
>>> > + * You should have received a copy of the GNU General Public License along with
>>> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
>>> > + */
>>> > +
>>> > +#define REQUIRE_XTHEADCMO(ctx) do {              \
>>> > +    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
>>> > +        return false;                            \
>>> > +    }                                            \
>>> > +} while (0)
>>> > +
>>> > +/* XTheadCmo */
>>> > +
>>> > +static inline int priv_level(DisasContext *ctx)
>>> > +{
>>> > +#ifdef CONFIG_USER_ONLY
>>> > +    return PRV_U;
>>> > +#else
>>> > +     /* Priv level equals mem_idx -- see cpu_mmu_index. */
>>> > +    return ctx->mem_idx;
>>>
>>> This should be ANDed with TB_FLAGS_PRIV_MMU_MASK as sometimes this can
>>> include hypervisor priv access information
>>
>>
>> Ok.
>>
>>>
>>>
>>> > +#endif
>>> > +}
>>> > +
>>> > +#define REQUIRE_PRIV_MHSU(ctx)                                  \
>>> > +do {                                                            \
>>> > +    int priv = priv_level(ctx);                                 \
>>> > +    if (!(priv == PRV_M ||                                      \
>>> > +          priv == PRV_H ||                                      \
>>>
>>> PRV_H isn't used
>>>
>>> > +          priv == PRV_S ||                                      \
>>> > +          priv == PRV_U)) {                                     \
>>> > +        return false;                                           \
>>>
>>> When would this not be the case?
>>
>>
>> Ok, I will make this a macro that expands to nothing (and a comment).
>>
>>>
>>>
>>> > +    }                                                           \
>>> > +} while (0)
>>> > +
>>> > +#define REQUIRE_PRIV_MHS(ctx)                                   \
>>> > +do {                                                            \
>>> > +    int priv = priv_level(ctx);                                 \
>>> > +    if (!(priv == PRV_M ||                                      \
>>> > +          priv == PRV_H ||                                      \
>>>
>>> Also not used
>>
>>
>> Ok, I will remove the PRV_H.
>>
>>>
>>>
>>> > +          priv == PRV_S)) {                                     \
>>> > +        return false;                                           \
>>> > +    }                                                           \
>>> > +} while (0)
>>> > +
>>> > +#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
>>> > +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
>>> > +{                                                               \
>>> > +    (void) a;                                                   \
>>> > +    extcheck(ctx);                                              \
>>> > +    privcheck(ctx);                                             \
>>> > +    return true;                                                \
>>> > +}
>>> > +
>>> > +NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>>> > +NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>>> > +NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>>> > +NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +
>>> > +NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
>>> > +
>>> > +NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > +NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
>>> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
>>> > index ba25164d74..5dee37a242 100644
>>> > --- a/target/riscv/meson.build
>>> > +++ b/target/riscv/meson.build
>>> > @@ -2,6 +2,7 @@
>>> >  gen = [
>>> >    decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
>>> >    decodetree.process('insn32.decode', extra_args: '--static-decode=decode_insn32'),
>>> > +  decodetree.process('xthead.decode', extra_args: '--static-decode=decode_xthead'),
>>> >    decodetree.process('XVentanaCondOps.decode', extra_args: '--static-decode=decode_XVentanaCodeOps'),
>>> >  ]
>>> >
>>> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>> > index db123da5ec..14d9116975 100644
>>> > --- a/target/riscv/translate.c
>>> > +++ b/target/riscv/translate.c
>>> > @@ -125,13 +125,18 @@ static bool always_true_p(DisasContext *ctx  __attribute__((__unused__)))
>>> >      return true;
>>> >  }
>>> >
>>> > +static bool has_xthead_p(DisasContext *ctx  __attribute__((__unused__)))
>>> > +{
>>> > +    return ctx->cfg_ptr->ext_xtheadcmo;
>>> > +}
>>> > +
>>> >  #define MATERIALISE_EXT_PREDICATE(ext)  \
>>> >      static bool has_ ## ext ## _p(DisasContext *ctx)    \
>>> >      { \
>>> >          return ctx->cfg_ptr->ext_ ## ext ; \
>>> >      }
>>> >
>>> > -MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
>>> > +MATERIALISE_EXT_PREDICATE(XVentanaCondOps)
>>>
>>> Do we need this change?
>>
>>
>> It is indeed a drive-by cleanup, that is not necessary.
>> In v1 we were using this macro, therefore it made sense back then.
>> Will be dropped.
>>
>>>
>>>
>>> >
>>> >  #ifdef TARGET_RISCV32
>>> >  #define get_xl(ctx)    MXL_RV32
>>> > @@ -732,6 +737,10 @@ static int ex_rvc_shiftri(DisasContext *ctx, int imm)
>>> >  /* Include the auto-generated decoder for 32 bit insn */
>>> >  #include "decode-insn32.c.inc"
>>> >
>>> > +/* Include decoders for factored-out extensions */
>>> > +#include "decode-xthead.c.inc"
>>> > +#include "decode-XVentanaCondOps.c.inc"
>>> > +
>>> >  static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
>>> >                               void (*func)(TCGv, TCGv, target_long))
>>> >  {
>>> > @@ -1033,12 +1042,11 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>>> >  #include "insn_trans/trans_rvk.c.inc"
>>> >  #include "insn_trans/trans_privileged.c.inc"
>>> >  #include "insn_trans/trans_svinval.c.inc"
>>> > +#include "insn_trans/trans_xthead.c.inc"
>>> >  #include "insn_trans/trans_xventanacondops.c.inc"
>>> >
>>> >  /* Include the auto-generated decoder for 16 bit insn */
>>> >  #include "decode-insn16.c.inc"
>>> > -/* Include decoders for factored-out extensions */
>>> > -#include "decode-XVentanaCondOps.c.inc"
>>>
>>> Can we not leave these at the bottom?
>>
>>
>> Ok.
>
>
> I got reminded again, why this is like it is:
> The decoder code needs to be included before the translation functions,
> because the translation functions use types that are defined in the generated decoder code.
> And I wanted to keep all vendor extensions together.

Ah ok, why not keep each extension together instead, like this:

#include "decode-xthead.c.inc"
#include "insn_trans/trans_xthead.c.inc"

#include "decode-XVentanaCondOps.c.inc"

Alistair
Christoph Müllner Jan. 30, 2023, 2:04 p.m. UTC | #5 
On Sun, Jan 29, 2023 at 11:40 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 25, 2023 at 5:51 AM Christoph Müllner
> <christoph.muellner@vrull.eu> wrote:
> >
> >
> >
> > On Tue, Jan 24, 2023 at 6:31 PM Christoph Müllner <christoph.muellner@vrull.eu> wrote:
> >>
> >>
> >>
> >> On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>>
> >>> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
> >>> <christoph.muellner@vrull.eu> wrote:
> >>> >
> >>> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >>> >
> >>> > This patch adds support for the XTheadCmo ISA extension.
> >>> > To avoid interfering with standard extensions, decoder and translation
> >>> > are in its own xthead* specific files.
> >>> > Future patches should be able to easily add additional T-Head extension.
> >>> >
> >>> > The implementation does not have much functionality (besides accepting
> >>> > the instructions and not qualifying them as illegal instructions if
> >>> > the hart executes in the required privilege level for the instruction),
> >>> > as QEMU does not model CPU caches and instructions are documented
> >>> > to not raise any exceptions.
> >>> >
> >>> > Changes in v2:
> >>> > - Add ISA_EXT_DATA_ENTRY()
> >>> > - Explicit test for PRV_U
> >>> > - Encapsule access to env-priv in inline function
> >>> > - Use single decoder for XThead extensions
> >>> >
> >>> > 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/insn_trans/trans_xthead.c.inc | 89 ++++++++++++++++++++++
> >>> >  target/riscv/meson.build                   |  1 +
> >>> >  target/riscv/translate.c                   | 15 +++-
> >>> >  target/riscv/xthead.decode                 | 38 +++++++++
> >>> >  6 files changed, 143 insertions(+), 3 deletions(-)
> >>> >  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
> >>> >  create mode 100644 target/riscv/xthead.decode
> >>> >
> >>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >>> > index 6fe176e483..a90b82c5c5 100644
> >>> > --- a/target/riscv/cpu.c
> >>> > +++ b/target/riscv/cpu.c
> >>> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> >>> >      ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
> >>> >      ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
> >>> >      ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> >>> > +    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0, ext_xtheadcmo),
> >>> >      ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
> >>> >  };
> >>> >
> >>> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
> >>> >      DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> >>> >
> >>> >      /* Vendor-specific custom extensions */
> >>> > +    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
> >>> > --- a/target/riscv/cpu.h
> >>> > +++ b/target/riscv/cpu.h
> >>> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
> >>> >      uint64_t mimpid;
> >>> >
> >>> >      /* Vendor-specific custom extensions */
> >>> > +    bool ext_xtheadcmo;
> >>> >      bool ext_XVentanaCondOps;
> >>> >
> >>> >      uint8_t pmu_num;
> >>> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc b/target/riscv/insn_trans/trans_xthead.c.inc
> >>> > new file mode 100644
> >>> > index 0000000000..00e75c7dca
> >>> > --- /dev/null
> >>> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
> >>> > @@ -0,0 +1,89 @@
> >>> > +/*
> >>> > + * RISC-V translation routines for the T-Head vendor extensions (xthead*).
> >>> > + *
> >>> > + * Copyright (c) 2022 VRULL GmbH.
> >>> > + *
> >>> > + * This program is free software; you can redistribute it and/or modify it
> >>> > + * under the terms and conditions of the GNU General Public License,
> >>> > + * version 2 or later, as published by the Free Software Foundation.
> >>> > + *
> >>> > + * This program is distributed in the hope it will be useful, but WITHOUT
> >>> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >>> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> >>> > + * more details.
> >>> > + *
> >>> > + * You should have received a copy of the GNU General Public License along with
> >>> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> >>> > + */
> >>> > +
> >>> > +#define REQUIRE_XTHEADCMO(ctx) do {              \
> >>> > +    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
> >>> > +        return false;                            \
> >>> > +    }                                            \
> >>> > +} while (0)
> >>> > +
> >>> > +/* XTheadCmo */
> >>> > +
> >>> > +static inline int priv_level(DisasContext *ctx)
> >>> > +{
> >>> > +#ifdef CONFIG_USER_ONLY
> >>> > +    return PRV_U;
> >>> > +#else
> >>> > +     /* Priv level equals mem_idx -- see cpu_mmu_index. */
> >>> > +    return ctx->mem_idx;
> >>>
> >>> This should be ANDed with TB_FLAGS_PRIV_MMU_MASK as sometimes this can
> >>> include hypervisor priv access information
> >>
> >>
> >> Ok.
> >>
> >>>
> >>>
> >>> > +#endif
> >>> > +}
> >>> > +
> >>> > +#define REQUIRE_PRIV_MHSU(ctx)                                  \
> >>> > +do {                                                            \
> >>> > +    int priv = priv_level(ctx);                                 \
> >>> > +    if (!(priv == PRV_M ||                                      \
> >>> > +          priv == PRV_H ||                                      \
> >>>
> >>> PRV_H isn't used
> >>>
> >>> > +          priv == PRV_S ||                                      \
> >>> > +          priv == PRV_U)) {                                     \
> >>> > +        return false;                                           \
> >>>
> >>> When would this not be the case?
> >>
> >>
> >> Ok, I will make this a macro that expands to nothing (and a comment).
> >>
> >>>
> >>>
> >>> > +    }                                                           \
> >>> > +} while (0)
> >>> > +
> >>> > +#define REQUIRE_PRIV_MHS(ctx)                                   \
> >>> > +do {                                                            \
> >>> > +    int priv = priv_level(ctx);                                 \
> >>> > +    if (!(priv == PRV_M ||                                      \
> >>> > +          priv == PRV_H ||                                      \
> >>>
> >>> Also not used
> >>
> >>
> >> Ok, I will remove the PRV_H.
> >>
> >>>
> >>>
> >>> > +          priv == PRV_S)) {                                     \
> >>> > +        return false;                                           \
> >>> > +    }                                                           \
> >>> > +} while (0)
> >>> > +
> >>> > +#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
> >>> > +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
> >>> > +{                                                               \
> >>> > +    (void) a;                                                   \
> >>> > +    extcheck(ctx);                                              \
> >>> > +    privcheck(ctx);                                             \
> >>> > +    return true;                                                \
> >>> > +}
> >>> > +
> >>> > +NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> >>> > +NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> >>> > +NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> >>> > +NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +
> >>> > +NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
> >>> > +
> >>> > +NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > +NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
> >>> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> >>> > index ba25164d74..5dee37a242 100644
> >>> > --- a/target/riscv/meson.build
> >>> > +++ b/target/riscv/meson.build
> >>> > @@ -2,6 +2,7 @@
> >>> >  gen = [
> >>> >    decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
> >>> >    decodetree.process('insn32.decode', extra_args: '--static-decode=decode_insn32'),
> >>> > +  decodetree.process('xthead.decode', extra_args: '--static-decode=decode_xthead'),
> >>> >    decodetree.process('XVentanaCondOps.decode', extra_args: '--static-decode=decode_XVentanaCodeOps'),
> >>> >  ]
> >>> >
> >>> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> >>> > index db123da5ec..14d9116975 100644
> >>> > --- a/target/riscv/translate.c
> >>> > +++ b/target/riscv/translate.c
> >>> > @@ -125,13 +125,18 @@ static bool always_true_p(DisasContext *ctx  __attribute__((__unused__)))
> >>> >      return true;
> >>> >  }
> >>> >
> >>> > +static bool has_xthead_p(DisasContext *ctx  __attribute__((__unused__)))
> >>> > +{
> >>> > +    return ctx->cfg_ptr->ext_xtheadcmo;
> >>> > +}
> >>> > +
> >>> >  #define MATERIALISE_EXT_PREDICATE(ext)  \
> >>> >      static bool has_ ## ext ## _p(DisasContext *ctx)    \
> >>> >      { \
> >>> >          return ctx->cfg_ptr->ext_ ## ext ; \
> >>> >      }
> >>> >
> >>> > -MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
> >>> > +MATERIALISE_EXT_PREDICATE(XVentanaCondOps)
> >>>
> >>> Do we need this change?
> >>
> >>
> >> It is indeed a drive-by cleanup, that is not necessary.
> >> In v1 we were using this macro, therefore it made sense back then.
> >> Will be dropped.
> >>
> >>>
> >>>
> >>> >
> >>> >  #ifdef TARGET_RISCV32
> >>> >  #define get_xl(ctx)    MXL_RV32
> >>> > @@ -732,6 +737,10 @@ static int ex_rvc_shiftri(DisasContext *ctx, int imm)
> >>> >  /* Include the auto-generated decoder for 32 bit insn */
> >>> >  #include "decode-insn32.c.inc"
> >>> >
> >>> > +/* Include decoders for factored-out extensions */
> >>> > +#include "decode-xthead.c.inc"
> >>> > +#include "decode-XVentanaCondOps.c.inc"
> >>> > +
> >>> >  static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
> >>> >                               void (*func)(TCGv, TCGv, target_long))
> >>> >  {
> >>> > @@ -1033,12 +1042,11 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> >>> >  #include "insn_trans/trans_rvk.c.inc"
> >>> >  #include "insn_trans/trans_privileged.c.inc"
> >>> >  #include "insn_trans/trans_svinval.c.inc"
> >>> > +#include "insn_trans/trans_xthead.c.inc"
> >>> >  #include "insn_trans/trans_xventanacondops.c.inc"
> >>> >
> >>> >  /* Include the auto-generated decoder for 16 bit insn */
> >>> >  #include "decode-insn16.c.inc"
> >>> > -/* Include decoders for factored-out extensions */
> >>> > -#include "decode-XVentanaCondOps.c.inc"
> >>>
> >>> Can we not leave these at the bottom?
> >>
> >>
> >> Ok.
> >
> >
> > I got reminded again, why this is like it is:
> > The decoder code needs to be included before the translation functions,
> > because the translation functions use types that are defined in the generated decoder code.
> > And I wanted to keep all vendor extensions together.
>
> Ah ok, why not keep each extension together instead, like this:
>
> #include "decode-xthead.c.inc"
> #include "insn_trans/trans_xthead.c.inc"
>
> #include "decode-XVentanaCondOps.c.inc"

In v3 I had the decode included below decode-insn32.c.inc.
And the trans_xthead.c.inc was at the end of the trans* includes.
For v4 I will move the decode down to the inclusion of trans_xthead.c.inc.

v4 will be sent out, once we have addressed the review comments from Richard.

Thanks,
Christoph



>
>
> Alistair
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6fe176e483..a90b82c5c5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -108,6 +108,7 @@  static const struct isa_ext_data isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
     ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
     ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
+    ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0, ext_xtheadcmo),
     ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, ext_XVentanaCondOps),
 };
 
@@ -1060,6 +1061,7 @@  static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
 
     /* Vendor-specific custom extensions */
+    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, 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 443d15a47c..ad1c19f870 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -465,6 +465,7 @@  struct RISCVCPUConfig {
     uint64_t mimpid;
 
     /* Vendor-specific custom extensions */
+    bool ext_xtheadcmo;
     bool ext_XVentanaCondOps;
 
     uint8_t pmu_num;
diff --git a/target/riscv/insn_trans/trans_xthead.c.inc b/target/riscv/insn_trans/trans_xthead.c.inc
new file mode 100644
index 0000000000..00e75c7dca
--- /dev/null
+++ b/target/riscv/insn_trans/trans_xthead.c.inc
@@ -0,0 +1,89 @@ 
+/*
+ * RISC-V translation routines for the T-Head vendor extensions (xthead*).
+ *
+ * Copyright (c) 2022 VRULL GmbH.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define REQUIRE_XTHEADCMO(ctx) do {              \
+    if (!ctx->cfg_ptr->ext_xtheadcmo) {          \
+        return false;                            \
+    }                                            \
+} while (0)
+
+/* XTheadCmo */
+
+static inline int priv_level(DisasContext *ctx)
+{
+#ifdef CONFIG_USER_ONLY
+    return PRV_U;
+#else
+     /* Priv level equals mem_idx -- see cpu_mmu_index. */
+    return ctx->mem_idx;
+#endif
+}
+
+#define REQUIRE_PRIV_MHSU(ctx)                                  \
+do {                                                            \
+    int priv = priv_level(ctx);                                 \
+    if (!(priv == PRV_M ||                                      \
+          priv == PRV_H ||                                      \
+          priv == PRV_S ||                                      \
+          priv == PRV_U)) {                                     \
+        return false;                                           \
+    }                                                           \
+} while (0)
+
+#define REQUIRE_PRIV_MHS(ctx)                                   \
+do {                                                            \
+    int priv = priv_level(ctx);                                 \
+    if (!(priv == PRV_M ||                                      \
+          priv == PRV_H ||                                      \
+          priv == PRV_S)) {                                     \
+        return false;                                           \
+    }                                                           \
+} while (0)
+
+#define NOP_PRIVCHECK(insn, extcheck, privcheck)                \
+static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn * a) \
+{                                                               \
+    (void) a;                                                   \
+    extcheck(ctx);                                              \
+    privcheck(ctx);                                             \
+    return true;                                                \
+}
+
+NOP_PRIVCHECK(th_dcache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+
+NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_icache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_icache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHSU)
+
+NOP_PRIVCHECK(th_l2cache_call, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_l2cache_ciall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_l2cache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MHS)
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index ba25164d74..5dee37a242 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -2,6 +2,7 @@ 
 gen = [
   decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
   decodetree.process('insn32.decode', extra_args: '--static-decode=decode_insn32'),
+  decodetree.process('xthead.decode', extra_args: '--static-decode=decode_xthead'),
   decodetree.process('XVentanaCondOps.decode', extra_args: '--static-decode=decode_XVentanaCodeOps'),
 ]
 
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index db123da5ec..14d9116975 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -125,13 +125,18 @@  static bool always_true_p(DisasContext *ctx  __attribute__((__unused__)))
     return true;
 }
 
+static bool has_xthead_p(DisasContext *ctx  __attribute__((__unused__)))
+{
+    return ctx->cfg_ptr->ext_xtheadcmo;
+}
+
 #define MATERIALISE_EXT_PREDICATE(ext)  \
     static bool has_ ## ext ## _p(DisasContext *ctx)    \
     { \
         return ctx->cfg_ptr->ext_ ## ext ; \
     }
 
-MATERIALISE_EXT_PREDICATE(XVentanaCondOps);
+MATERIALISE_EXT_PREDICATE(XVentanaCondOps)
 
 #ifdef TARGET_RISCV32
 #define get_xl(ctx)    MXL_RV32
@@ -732,6 +737,10 @@  static int ex_rvc_shiftri(DisasContext *ctx, int imm)
 /* Include the auto-generated decoder for 32 bit insn */
 #include "decode-insn32.c.inc"
 
+/* Include decoders for factored-out extensions */
+#include "decode-xthead.c.inc"
+#include "decode-XVentanaCondOps.c.inc"
+
 static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a,
                              void (*func)(TCGv, TCGv, target_long))
 {
@@ -1033,12 +1042,11 @@  static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 #include "insn_trans/trans_rvk.c.inc"
 #include "insn_trans/trans_privileged.c.inc"
 #include "insn_trans/trans_svinval.c.inc"
+#include "insn_trans/trans_xthead.c.inc"
 #include "insn_trans/trans_xventanacondops.c.inc"
 
 /* Include the auto-generated decoder for 16 bit insn */
 #include "decode-insn16.c.inc"
-/* Include decoders for factored-out extensions */
-#include "decode-XVentanaCondOps.c.inc"
 
 /* The specification allows for longer insns, but not supported by qemu. */
 #define MAX_INSN_LEN  4
@@ -1059,6 +1067,7 @@  static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
         bool (*decode_func)(DisasContext *, uint32_t);
     } decoders[] = {
         { always_true_p,  decode_insn32 },
+        { has_xthead_p, decode_xthead },
         { has_XVentanaCondOps_p,  decode_XVentanaCodeOps },
     };
 
diff --git a/target/riscv/xthead.decode b/target/riscv/xthead.decode
new file mode 100644
index 0000000000..30533a66f5
--- /dev/null
+++ b/target/riscv/xthead.decode
@@ -0,0 +1,38 @@ 
+#
+# Translation routines for the instructions of the XThead* ISA extensions
+#
+# Copyright (c) 2022 Christoph Muellner, christoph.muellner@vrull.eu
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# The documentation of the ISA extensions can be found here:
+#   https://github.com/T-head-Semi/thead-extension-spec/releases/latest
+
+# Fields:
+%rs1       15:5
+
+# Formats
+@sfence_vm  ....... ..... .....   ... ..... ....... %rs1
+
+# XTheadCmo
+th_dcache_call   0000000 00001 00000 000 00000 0001011
+th_dcache_ciall  0000000 00011 00000 000 00000 0001011
+th_dcache_iall   0000000 00010 00000 000 00000 0001011
+th_dcache_cpa    0000001 01001 ..... 000 00000 0001011 @sfence_vm
+th_dcache_cipa   0000001 01011 ..... 000 00000 0001011 @sfence_vm
+th_dcache_ipa    0000001 01010 ..... 000 00000 0001011 @sfence_vm
+th_dcache_cva    0000001 00101 ..... 000 00000 0001011 @sfence_vm
+th_dcache_civa   0000001 00111 ..... 000 00000 0001011 @sfence_vm
+th_dcache_iva    0000001 00110 ..... 000 00000 0001011 @sfence_vm
+th_dcache_csw    0000001 00001 ..... 000 00000 0001011 @sfence_vm
+th_dcache_cisw   0000001 00011 ..... 000 00000 0001011 @sfence_vm
+th_dcache_isw    0000001 00010 ..... 000 00000 0001011 @sfence_vm
+th_dcache_cpal1  0000001 01000 ..... 000 00000 0001011 @sfence_vm
+th_dcache_cval1  0000001 00100 ..... 000 00000 0001011 @sfence_vm
+th_icache_iall   0000000 10000 00000 000 00000 0001011
+th_icache_ialls  0000000 10001 00000 000 00000 0001011
+th_icache_ipa    0000001 11000 ..... 000 00000 0001011 @sfence_vm
+th_icache_iva    0000001 10000 ..... 000 00000 0001011 @sfence_vm
+th_l2cache_call  0000000 10101 00000 000 00000 0001011
+th_l2cache_ciall 0000000 10111 00000 000 00000 0001011
+th_l2cache_iall  0000000 10110 00000 000 00000 0001011