On Fri, Jun 24, 2022 at 1:31 AM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> This patch adds support for the Zawrs ISA extension.
> Given the current (incomplete) implementation of reservation sets
> there seems to be no way to provide a full emulation of the WRS
> instruction (wake on reservation set invalidation or timeout or
> interrupt). Therefore, we just pretend that an interrupt occured,
> exit the execution loop and finally continue execution.
>
> The specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
>
> Changes since v2:
> * Adjustments according to a specification change
> * Inline REQUIRE_ZAWRS() since it has only one user
>
> Changes since v1:
> * Adding zawrs to the ISA string that is passed to the kernel
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
> target/riscv/cpu.c | 2 +
> target/riscv/cpu.h | 1 +
> target/riscv/insn32.decode | 4 ++
> target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +++++++++++++++++++++
> target/riscv/translate.c | 1 +
> 5 files changed, 62 insertions(+)
> create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 05e6521351..6cb00fadff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
> DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> + DEFINE_PROP_BOOL("zawrs", RISCVCPU, cfg.ext_zawrs, true),
Would this be enabled by default?
The "true" was a personal preference (I prefer to keep the argument list for QEMU short)
and I did not see any conflicts with existing behavior (no code should break).
If you prefer otherwise or if I missed a policy I will change it.
> DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
> ISA_EDATA_ENTRY(zicsr, ext_icsr),
> ISA_EDATA_ENTRY(zifencei, ext_ifencei),
> ISA_EDATA_ENTRY(zmmul, ext_zmmul),
> + ISA_EDATA_ENTRY(zawrs, ext_zawrs),
> ISA_EDATA_ENTRY(zfh, ext_zfh),
> ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7d6397acdf..a22bc0fa09 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
> bool ext_h;
> bool ext_j;
> bool ext_v;
> + bool ext_zawrs;
> bool ext_zba;
> bool ext_zbb;
> bool ext_zbc;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 4033565393..513ea227fe 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -711,6 +711,10 @@ vsetvli 0 ........... ..... 111 ..... 1010111 @r2_zimm11
> vsetivli 11 .......... ..... 111 ..... 1010111 @r2_zimm10
> vsetvl 1000000 ..... ..... 111 ..... 1010111 @r
>
> +# *** Zawrs Standard Extension ***
> +wrs_nto 000000001101 00000 000 00000 1110011
> +wrs_sto 000000011101 00000 000 00000 1110011
> +
> # *** RV32 Zba Standard Extension ***
> sh1add 0010000 .......... 010 ..... 0110011 @r
> sh2add 0010000 .......... 100 ..... 0110011 @r
> diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> new file mode 100644
> index 0000000000..d0df56378e
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> @@ -0,0 +1,54 @@
> +/*
> + * RISC-V translation routines for the RISC-V Zawrs Extension.
> + *
> + * Copyright (c) 2022 Christoph Muellner, christoph.muellner@vrull.io
> + *
> + * 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/>.
> + */
> +
> +static bool trans_wrs(DisasContext *ctx)
> +{
> + if (!ctx->cfg_ptr->ext_zawrs) {
> + return false;
> + }
> +
> + /*
> + * We may continue if one or more of the following conditions are met:
> + * a) The reservation set is invalid
Shouldn't this be valid?
The CPU is supposed to continue (stop waiting) when the reservation set becomes invalid.
An earlier LR instruction registers a reservation set and the WRS.* instructions wait until
this reservation set becomes invalided by a store from another hart to the same reservation set.
So I think the description is correct.
> + * b) If WRS.STO, a short time since start of stall has elapsed
> + * c) An interrupt is observed
> + *
> + * A reservation set can be invalidated by any store to a reserved
> + * memory location. However, that's currently not implemented in QEMU.
> + * So let's just exit the CPU loop and pretend that an interrupt occured.
We don't actually pretend an interrupt occurs though. It seems like
it's valid to terminate the stall early, so we should just be able to
do that.
The specification allows stopping the CPU stall if an interrupt occurs that is disabled.
I think that would match the implemented behavior.
The latest spec update introduced the following sentence:
"While stalled, an implementation is permitted to occasionally terminate the stall and complete execution for any reason."
I did not want to use this justification for the implementation, because of the word "occasionally" (the correct word would
be "always" in the implementation). Do you prefer to use this sentence instead?
Thanks,
Christoph
Alistair
> + */
> +
> + /* Clear the load reservation (if any). */
> + tcg_gen_movi_tl(load_res, -1);
> +
> + gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> + tcg_gen_exit_tb(NULL, 0);
> + ctx->base.is_jmp = DISAS_NORETURN;
> +
> + return true;
> +}
> +
> +#define GEN_TRANS_WRS(insn) \
> +static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn *a) \
> +{ \
> + (void)a; \
> + return trans_wrs(ctx); \
> +}
> +
> +GEN_TRANS_WRS(wrs_nto)
> +GEN_TRANS_WRS(wrs_sto)
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index b151c20674..a4f07d5166 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1007,6 +1007,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> #include "insn_trans/trans_rvh.c.inc"
> #include "insn_trans/trans_rvv.c.inc"
> #include "insn_trans/trans_rvb.c.inc"
> +#include "insn_trans/trans_rvzawrs.c.inc"
> #include "insn_trans/trans_rvzfh.c.inc"
> #include "insn_trans/trans_rvk.c.inc"
> #include "insn_trans/trans_privileged.c.inc"
> --
> 2.35.3
>
>