* [PATCH v2] target/riscv: check G-stage write permission for VS-stage A/D updates
[not found] <v1-Message-ID>
@ 2026-07-05 14:02 ` imaginos
2026-07-10 2:23 ` Alistair Francis
0 siblings, 1 reply; 2+ messages in thread
From: imaginos @ 2026-07-05 14:02 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis
Cc: Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Chao Liu,
qemu-riscv, qemu-devel, imaginos
During a two-stage (VS-stage + G-stage) page-table walk with hardware
A/D updating enabled (Svadu / menvcfg.ADUE), a store that reaches a
VS-stage leaf PTE whose accessed or dirty bit is clear triggers a
hardware write-back of those bits into the PTE. That write-back is an
implicit store to the PTE's guest-physical address, so it must be
permitted by G-stage.
Fix this by re-running the G-stage translation of the guest PTE's
address with store semantics. get_physical_address() then also checks
that G-stage permits the guest PTE to be written, and raises a
guest-page store fault when it does not.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3426
Signed-off-by: imaginos <imaginos32@gmail.com>
---
target/riscv/cpu_helper.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
v2:
Reword an incomplete sentence in the code comment.
Verified against Spike, with this patch QEMU's
reported values match Spike.
Spike (reference): QEMU, before this patch:
mcause 0x17 mcause 0x17
mtval 0x80000000 mtval 0x80000000
mtval2 0x20001004 mtval2 0x20000000
mtinst 0x3020 mtinst 0x6a704073
If I've misread any of the A/D-update semantics here, I'd appreciate the
correction.
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2db07f5dfb..eb34ba19e1 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1370,6 +1370,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
int ptshift;
target_ulong pte;
hwaddr pte_addr;
+ hwaddr pte_gpa = 0;
const hwaddr base_root = base;
const bool be = mo_endian_env(env) == MO_BE;
int i;
@@ -1407,6 +1408,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
}
pte_addr = vbase + idx * ptesize;
+ pte_gpa = base + idx * ptesize;
} else {
pte_addr = base + idx * ptesize;
}
@@ -1661,6 +1663,28 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
return TRANSLATE_FAIL;
}
+ /*
+ * The implicit store that writes updated A/D bits back to a VS-stage
+ * (first-stage) PTE must itself be permitted by G-stage. Re-run the
+ * second-stage translation of the PTE's guest-physical address with
+ * store semantics. If G-stage denies write, raise G-stage store fault
+ * against the PTE address.
+ */
+ if (two_stage && first_stage) {
+ int gpa_prot;
+ hwaddr gpa_paddr;
+ int gpa_ret = get_physical_address(env, &gpa_paddr, &gpa_prot,
+ pte_gpa, NULL, MMU_DATA_STORE,
+ MMUIdx_U, false, true,
+ is_debug, false);
+ if (gpa_ret != TRANSLATE_SUCCESS) {
+ if (fault_pte_addr) {
+ *fault_pte_addr = pte_gpa >> 2;
+ }
+ return TRANSLATE_G_STAGE_FAIL;
+ }
+ }
+
pmp_ret = get_physical_address_pmp(env, &pmp_prot, pte_addr,
sxlen_bytes, MMU_DATA_STORE, PRV_S);
if (pmp_ret != TRANSLATE_SUCCESS) {
@@ -2345,6 +2369,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
* doing VS-stage page table walk.
*/
tinst = (riscv_cpu_xlen(env) == 32) ? 0x00002000 : 0x00003000;
+
+ if (cause == RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT) {
+ tinst |= 0x20;
+ }
} else {
/*
* The "Addr. Offset" field in transformed instruction is
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] target/riscv: check G-stage write permission for VS-stage A/D updates
2026-07-05 14:02 ` [PATCH v2] target/riscv: check G-stage write permission for VS-stage A/D updates imaginos
@ 2026-07-10 2:23 ` Alistair Francis
0 siblings, 0 replies; 2+ messages in thread
From: Alistair Francis @ 2026-07-10 2:23 UTC (permalink / raw)
To: imaginos32@gmail.com, palmer@dabbelt.com
Cc: chao.liu.zevorn@gmail.com, qemu-riscv@nongnu.org,
daniel.barboza@oss.qualcomm.com, qemu-devel@nongnu.org,
liwei1518@gmail.com, zhiwei_liu@linux.alibaba.com
On Sun, 2026-07-05 at 16:02 +0200, imaginos wrote:
> During a two-stage (VS-stage + G-stage) page-table walk with hardware
> A/D updating enabled (Svadu / menvcfg.ADUE), a store that reaches a
> VS-stage leaf PTE whose accessed or dirty bit is clear triggers a
> hardware write-back of those bits into the PTE. That write-back is an
> implicit store to the PTE's guest-physical address, so it must be
> permitted by G-stage.
>
> Fix this by re-running the G-stage translation of the guest PTE's
> address with store semantics. get_physical_address() then also checks
> that G-stage permits the guest PTE to be written, and raises a
> guest-page store fault when it does not.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3426
> Signed-off-by: imaginos <imaginos32@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu_helper.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> v2:
> Reword an incomplete sentence in the code comment.
>
> Verified against Spike, with this patch QEMU's
> reported values match Spike.
>
> Spike (reference): QEMU, before this patch:
> mcause 0x17 mcause 0x17
> mtval 0x80000000 mtval 0x80000000
> mtval2 0x20001004 mtval2 0x20000000
> mtinst 0x3020 mtinst 0x6a704073
>
> If I've misread any of the A/D-update semantics here, I'd appreciate
> the
> correction.
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2db07f5dfb..eb34ba19e1 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1370,6 +1370,7 @@ static int get_physical_address(CPURISCVState
> *env, hwaddr *physical,
> int ptshift;
> target_ulong pte;
> hwaddr pte_addr;
> + hwaddr pte_gpa = 0;
> const hwaddr base_root = base;
> const bool be = mo_endian_env(env) == MO_BE;
> int i;
> @@ -1407,6 +1408,7 @@ static int get_physical_address(CPURISCVState
> *env, hwaddr *physical,
> }
>
> pte_addr = vbase + idx * ptesize;
> + pte_gpa = base + idx * ptesize;
> } else {
> pte_addr = base + idx * ptesize;
> }
> @@ -1661,6 +1663,28 @@ static int get_physical_address(CPURISCVState
> *env, hwaddr *physical,
> return TRANSLATE_FAIL;
> }
>
> + /*
> + * The implicit store that writes updated A/D bits back to a
> VS-stage
> + * (first-stage) PTE must itself be permitted by G-stage.
> Re-run the
> + * second-stage translation of the PTE's guest-physical
> address with
> + * store semantics. If G-stage denies write, raise G-stage
> store fault
> + * against the PTE address.
> + */
> + if (two_stage && first_stage) {
> + int gpa_prot;
> + hwaddr gpa_paddr;
> + int gpa_ret = get_physical_address(env, &gpa_paddr,
> &gpa_prot,
> + pte_gpa, NULL,
> MMU_DATA_STORE,
> + MMUIdx_U, false,
> true,
> + is_debug, false);
> + if (gpa_ret != TRANSLATE_SUCCESS) {
> + if (fault_pte_addr) {
> + *fault_pte_addr = pte_gpa >> 2;
> + }
> + return TRANSLATE_G_STAGE_FAIL;
> + }
> + }
> +
> pmp_ret = get_physical_address_pmp(env, &pmp_prot, pte_addr,
> sxlen_bytes,
> MMU_DATA_STORE, PRV_S);
> if (pmp_ret != TRANSLATE_SUCCESS) {
> @@ -2345,6 +2369,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> * doing VS-stage page table walk.
> */
> tinst = (riscv_cpu_xlen(env) == 32) ? 0x00002000 :
> 0x00003000;
> +
> + if (cause ==
> RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT) {
> + tinst |= 0x20;
> + }
> } else {
> /*
> * The "Addr. Offset" field in transformed
> instruction is
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-10 2:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <v1-Message-ID>
2026-07-05 14:02 ` [PATCH v2] target/riscv: check G-stage write permission for VS-stage A/D updates imaginos
2026-07-10 2:23 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.