On Fri, 12 Aug 2022 at 14:17, Philipp Tomsich <philipp.toms...@vrull.eu> wrote: > > Our decoding of fence-instructions is problematic in respect to the > RISC-V ISA specification: > - rs and rd are ignored, but need to be 0 > - fm is ignored > > This change adjusts the decode pattern to enfore rs and rd being 0, > and validates the fm-field (together with pred/succ for FENCE.TSO) to > determine whether a reserved instruction is specified. > > While the specification allows UNSPECIFIED behaviour for reserved > instructions, we now always raise an illegal instruction exception. > > Signed-off-by: Philipp Tomsich <philipp.toms...@vrull.eu> > > --- > > target/riscv/insn32.decode | 2 +- > target/riscv/insn_trans/trans_rvi.c.inc | 19 ++++++++++++++++++- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > index 089128c3dc..4e53df1b62 100644 > --- a/target/riscv/insn32.decode > +++ b/target/riscv/insn32.decode > @@ -150,7 +150,7 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > sra 0100000 ..... ..... 101 ..... 0110011 @r > or 0000000 ..... ..... 110 ..... 0110011 @r > and 0000000 ..... ..... 111 ..... 0110011 @r > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > +fence fm:4 pred:4 succ:4 00000 000 00000 0001111 > fence_i 000000000000 00000 001 00000 0001111 > csrrw ............ ..... 001 ..... 1110011 @csr > csrrs ............ ..... 010 ..... 1110011 @csr > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc > b/target/riscv/insn_trans/trans_rvi.c.inc > index ca8e3d1ea1..515bb3b22a 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -795,7 +795,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > { > - /* FENCE is a full memory barrier. */ > + switch (a->fm) { > + case 0b0000: > + /* normal fence */ > + break; > + > + case 0b0001: > + /* FENCE.TSO requires PRED and SUCC to be RW */ > + if (a->pred != 0xb0011 || a->succ != 0b0011) { > + return false; > + } > + break; > + > + default: > + /* reserved for future use */ > + return false; > + }
I think it would be neater to do this decode in the .decode file, rather than by hand in the trans function. thanks -- PMM