From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: qemu-devel@nongnu.org
Cc: qemu-riscv@nongnu.org, alistair.francis@wdc.com,
bmeng@tinylab.org, liwei1518@gmail.com,
zhiwei_liu@linux.alibaba.com, palmer@rivosinc.com,
richard.henderson@linaro.org, max.chou@sifive.com,
Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Subject: [PATCH v6 1/9] trans_rvv.c.inc: mark_vs_dirty() before loads and stores
Date: Wed, 21 Feb 2024 18:31:32 -0300 [thread overview]
Message-ID: <20240221213140.365232-2-dbarboza@ventanamicro.com> (raw)
In-Reply-To: <20240221213140.365232-1-dbarboza@ventanamicro.com>
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>
---
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
next prev parent reply other threads:[~2024-02-21 21:32 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-21 21:31 [PATCH v6 0/9] riscv: set vstart_eq_zero on mark_vs_dirty Daniel Henrique Barboza
2024-02-21 21:31 ` Daniel Henrique Barboza [this message]
2024-03-06 1:22 ` [PATCH v6 1/9] trans_rvv.c.inc: mark_vs_dirty() before loads and stores Alistair Francis
2024-02-21 21:31 ` [PATCH v6 2/9] trans_rvv.c.inc: remove 'is_store' bool from load/store fns Daniel Henrique Barboza
2024-03-06 1:25 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 3/9] target/riscv: remove 'over' brconds from vector trans Daniel Henrique Barboza
2024-03-06 1:27 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 4/9] target/riscv/translate.c: remove 'cpu_vstart' global Daniel Henrique Barboza
2024-02-22 7:59 ` Philippe Mathieu-Daudé
2024-03-06 1:28 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 5/9] target/riscv: remove 'cpu_vl' global Daniel Henrique Barboza
2024-02-21 22:01 ` Richard Henderson
2024-02-22 7:59 ` Philippe Mathieu-Daudé
2024-03-06 1:32 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 6/9] target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX() Daniel Henrique Barboza
2024-03-06 1:35 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 7/9] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls Daniel Henrique Barboza
2024-03-06 1:36 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 8/9] target/riscv: Clear vstart_qe_zero flag Daniel Henrique Barboza
2024-03-06 1:38 ` Alistair Francis
2024-02-21 21:31 ` [PATCH v6 9/9] target/riscv/vector_helper.c: optimize loops in ldst helpers Daniel Henrique Barboza
2024-02-21 22:03 ` Richard Henderson
2024-03-06 1:39 ` Alistair Francis
2024-03-06 1:58 ` [PATCH v6 0/9] riscv: set vstart_eq_zero on mark_vs_dirty 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=20240221213140.365232-2-dbarboza@ventanamicro.com \
--to=dbarboza@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=liwei1518@gmail.com \
--cc=max.chou@sifive.com \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--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 read-only IMAP folder(s) and NNTP newsgroup(s).