* [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.