Message ID | 20240221213140.365232-2-dbarboza@ventanamicro.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | riscv: set vstart_eq_zero on mark_vs_dirty | expand |
On Thu, Feb 22, 2024 at 7:34 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > While discussing a problem with how we're (not) setting vstart_eq_zero > Richard had the following to say w.r.t the conditional mark_vs_dirty() > calls on load/store functions [1]: > > "I think it's required to have stores set dirty unconditionally, before > the operation. > > Consider a store that traps on the 2nd element, leaving vstart = 2, and > exiting to the main loop via exception. The exception enters the kernel > page fault handler. The kernel may need to fault in the page for the > process, and in the meantime task switch. > > If vs dirty is not already set, the kernel won't know to save vector > state on task switch." > > Do a mark_vs_dirty() before both loads and stores. > > [1] https://lore.kernel.org/qemu-riscv/72c7503b-0f43-44b8-aa82-fbafed2aac0c@linaro.org/ > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/insn_trans/trans_rvv.c.inc | 23 ++++++++--------------- > 1 file changed, 8 insertions(+), 15 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc > index 9e101ab434..7a98f1caa6 100644 > --- a/target/riscv/insn_trans/trans_rvv.c.inc > +++ b/target/riscv/insn_trans/trans_rvv.c.inc > @@ -636,11 +636,9 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data, > tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); > tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); > > - fn(dest, mask, base, tcg_env, desc); > + mark_vs_dirty(s); > > - if (!is_store) { > - mark_vs_dirty(s); > - } > + fn(dest, mask, base, tcg_env, desc); > > gen_set_label(over); > return true; > @@ -797,11 +795,9 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2, > tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); > tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); > > - fn(dest, mask, base, stride, tcg_env, desc); > + mark_vs_dirty(s); > > - if (!is_store) { > - mark_vs_dirty(s); > - } > + fn(dest, mask, base, stride, tcg_env, desc); > > gen_set_label(over); > return true; > @@ -904,11 +900,9 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, > tcg_gen_addi_ptr(index, tcg_env, vreg_ofs(s, vs2)); > tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); > > - fn(dest, mask, base, index, tcg_env, desc); > + mark_vs_dirty(s); > > - if (!is_store) { > - mark_vs_dirty(s); > - } > + fn(dest, mask, base, index, tcg_env, desc); > > gen_set_label(over); > return true; > @@ -1102,11 +1096,10 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf, > base = get_gpr(s, rs1, EXT_NONE); > tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); > > + mark_vs_dirty(s); > + > fn(dest, base, tcg_env, desc); > > - if (!is_store) { > - mark_vs_dirty(s); > - } > gen_set_label(over); > > return true; > -- > 2.43.2 > >
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index 9e101ab434..7a98f1caa6 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -636,11 +636,9 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data, tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); - fn(dest, mask, base, tcg_env, desc); + mark_vs_dirty(s); - if (!is_store) { - mark_vs_dirty(s); - } + fn(dest, mask, base, tcg_env, desc); gen_set_label(over); return true; @@ -797,11 +795,9 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2, tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); - fn(dest, mask, base, stride, tcg_env, desc); + mark_vs_dirty(s); - if (!is_store) { - mark_vs_dirty(s); - } + fn(dest, mask, base, stride, tcg_env, desc); gen_set_label(over); return true; @@ -904,11 +900,9 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, tcg_gen_addi_ptr(index, tcg_env, vreg_ofs(s, vs2)); tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); - fn(dest, mask, base, index, tcg_env, desc); + mark_vs_dirty(s); - if (!is_store) { - mark_vs_dirty(s); - } + fn(dest, mask, base, index, tcg_env, desc); gen_set_label(over); return true; @@ -1102,11 +1096,10 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf, base = get_gpr(s, rs1, EXT_NONE); tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd)); + mark_vs_dirty(s); + fn(dest, base, tcg_env, desc); - if (!is_store) { - mark_vs_dirty(s); - } gen_set_label(over); return true;