From: "Christoph Müllner" <christoph.muellner@vrull.eu>
To: Alistair Francis <alistair23@gmail.com>
Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
"Alistair Francis" <alistair.francis@wdc.com>,
"Bin Meng" <bin.meng@windriver.com>,
"Philipp Tomsich" <philipp.tomsich@vrull.eu>,
"Heiko Stübner" <heiko.stuebner@vrull.eu>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Nelson Chu" <nelson@rivosinc.com>,
"Kito Cheng" <kito.cheng@sifive.com>,
"Cooper Qu" <cooper.qu@linux.alibaba.com>,
"Lifang Xia" <lifang_xia@linux.alibaba.com>,
"Yunhai Shang" <yunhai@linux.alibaba.com>,
"Zhiwei Liu" <zhiwei_liu@linux.alibaba.com>
Subject: Re: [PATCH v2 01/15] RISC-V: Adding XTheadCmo ISA extension
Date: Tue, 24 Jan 2023 18:31:07 +0100 [thread overview]
Message-ID: <CAEg0e7jmWzOcTNOyKoBD19fsge_aCzfgizEg0WSto-a5SgzNeA@mail.gmail.com> (raw)
In-Reply-To: <CAKmqyKOXM2poHz5c1CbddvUBC-wQxQE3o_bm7bqrn+XTkqASrg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 14033 bytes --]
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
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 18205 bytes --]
next prev parent reply other threads:[~2023-01-24 17:32 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-23 18:00 [PATCH v2 00/15] Add support for the T-Head vendor extensions Christoph Muellner
2022-12-23 18:00 ` [PATCH v2 01/15] RISC-V: Adding XTheadCmo ISA extension Christoph Muellner
2023-01-23 22:49 ` Alistair Francis
2023-01-24 17:31 ` Christoph Müllner [this message]
2023-01-24 19:51 ` Christoph Müllner
2023-01-29 22:40 ` Alistair Francis
2023-01-30 14:04 ` Christoph Müllner
2022-12-23 18:00 ` [PATCH v2 02/15] RISC-V: Adding XTheadSync " Christoph Muellner
2023-01-23 22:54 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 03/15] RISC-V: Adding XTheadBa " Christoph Muellner
2023-01-23 22:55 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 04/15] RISC-V: Adding XTheadBb " Christoph Muellner
2023-01-23 22:57 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 05/15] RISC-V: Adding XTheadBs " Christoph Muellner
2023-01-23 22:58 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 06/15] RISC-V: Adding XTheadCondMov " Christoph Muellner
2023-01-23 22:59 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 07/15] RISC-V: Adding T-Head multiply-accumulate instructions Christoph Muellner
2023-01-23 23:00 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 08/15] RISC-V: Adding T-Head MemPair extension Christoph Muellner
2023-01-23 23:03 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 09/15] RISC-V: Adding T-Head MemIdx extension Christoph Muellner
2023-01-23 23:04 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 10/15] RISC-V: Adding T-Head FMemIdx extension Christoph Muellner
2023-01-23 23:38 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 11/15] RISC-V: Adding T-Head XMAE support Christoph Muellner
2023-01-23 23:49 ` Alistair Francis
2023-01-24 17:31 ` Christoph Müllner
2022-12-23 18:00 ` [PATCH v2 12/15] RISC-V: Set minimum priv version for Zfh to 1.11 Christoph Muellner
2023-01-23 23:39 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 13/15] RISC-V: Add initial support for T-Head C906 Christoph Muellner
2023-01-23 23:43 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 14/15] RISC-V: Adding XTheadFmv ISA extension Christoph Muellner
2023-01-23 23:44 ` Alistair Francis
2022-12-23 18:00 ` [PATCH v2 15/15] target/riscv: add a MAINTAINERS entry for XThead* extension support Christoph Muellner
2023-01-23 23:40 ` Alistair Francis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEg0e7jmWzOcTNOyKoBD19fsge_aCzfgizEg0WSto-a5SgzNeA@mail.gmail.com \
--to=christoph.muellner@vrull.eu \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=bin.meng@windriver.com \
--cc=cooper.qu@linux.alibaba.com \
--cc=heiko.stuebner@vrull.eu \
--cc=kito.cheng@sifive.com \
--cc=lifang_xia@linux.alibaba.com \
--cc=nelson@rivosinc.com \
--cc=palmer@dabbelt.com \
--cc=philipp.tomsich@vrull.eu \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=yunhai@linux.alibaba.com \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).