[PATCH 3/3] target/riscv/translate.c: set vstart_eq_zero in mark_vs_dirty()

Daniel Henrique Barboza posted 3 patches 8 months, 2 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH 3/3] target/riscv/translate.c: set vstart_eq_zero in mark_vs_dirty()
Posted by Daniel Henrique Barboza 8 months, 2 weeks ago
The 'vstart_eq_zero' flag which is used to determine if some insns, like
vector reductor operations, should SIGILL. At this moment the flag is
being updated only during cpu_get_tb_cpu_state(), at the start of each
translation block.

This cadence isn't enough and we're facing situations where a vector
instruction successfully updated 'vstart' to zero, but the flag was
still marked as 'false', resulting in a SIGILL because instructions are
checking the flag.

mark_vs_dirty() is called after any instruction changes Vector CSR
state, making it a good place to update 'vstart_eq_zero'.

Fixes: 8e1ee1fb57 ("target/riscv: rvv-1.0: add translation-time vector context status")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/translate.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 177418b2b9..9943538894 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -652,9 +652,13 @@ static inline void mark_fs_dirty(DisasContext *ctx) { }
  */
 static void mark_vs_dirty(DisasContext *ctx)
 {
+    TCGLabel *vstart_zero, *done;
     TCGv tmp;
 
     if (ctx->mstatus_vs != EXT_STATUS_DIRTY) {
+        vstart_zero = gen_new_label();
+        done = gen_new_label();
+
         /* Remember the state change for the rest of the TB.  */
         ctx->mstatus_vs = EXT_STATUS_DIRTY;
 
@@ -668,6 +672,24 @@ static void mark_vs_dirty(DisasContext *ctx)
             tcg_gen_ori_tl(tmp, tmp, MSTATUS_VS);
             tcg_gen_st_tl(tmp, tcg_env, offsetof(CPURISCVState, mstatus_hs));
         }
+
+        /*
+         * We can safely make 'vl_eq_vlmax = false' if we marked
+         * VS as dirty with non-zero 'vstart', i.e. there's a fault
+         * to be handled. If 'vstart' is zero then we should retain
+         * the existing 'vl_eq_vlmax' - it'll be recalculated on the
+         * start of the next TB or during vset{i}vl{i} (that forces a
+         * TB end).
+         */
+        tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vstart, 0, vstart_zero);
+        ctx->vstart_eq_zero = false;
+        ctx->vl_eq_vlmax = false;
+        tcg_gen_br(done);
+
+        gen_set_label(vstart_zero);
+        ctx->vstart_eq_zero = true;
+
+        gen_set_label(done);
     }
 }
 #else
-- 
2.43.0