* [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 ` (2 more replies) 0 siblings, 3 replies; 8+ 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] 8+ 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 2026-09-03 21:44 ` Imaginos 2026-09-07 4:56 ` Alistair Francis 2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro 2 siblings, 1 reply; 8+ 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] 8+ messages in thread
* Re: [PATCH v2] target/riscv: check G-stage write permission for VS-stage A/D updates 2026-07-10 2:23 ` Alistair Francis @ 2026-09-03 21:44 ` Imaginos 0 siblings, 0 replies; 8+ messages in thread From: Imaginos @ 2026-09-03 21:44 UTC (permalink / raw) To: Alistair Francis Cc: palmer@dabbelt.com, 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 [-- Attachment #1: Type: text/plain, Size: 4630 bytes --] Just following up to ping the patch again пет, 10. јул 2026. у 04:23 Alistair Francis <Alistair.Francis@wdc.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 > [-- Attachment #2: Type: text/html, Size: 6502 bytes --] ^ permalink raw reply [flat|nested] 8+ 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 @ 2026-09-07 4:56 ` Alistair Francis 2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro 2 siblings, 0 replies; 8+ messages in thread From: Alistair Francis @ 2026-09-07 4:56 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> Thanks! Applied to riscv-to-apply.next 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] 8+ messages in thread
* [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on 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 2026-09-07 4:56 ` Alistair Francis @ 2026-11-03 7:29 ` tewaro 2026-11-03 7:29 ` [PATCH 1/2] target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update tewaro ` (2 more replies) 2 siblings, 3 replies; 8+ messages in thread From: tewaro @ 2026-11-03 7:29 UTC (permalink / raw) To: qemu-devel; +Cc: disclosures, tewaro, imaginos32 Hi, This series is about the traps taken when the G-stage refuses one of the implicit accesses that VS-stage address translation makes. It follows up on commit 366b4fba0fae ("target/riscv: check G-stage write permission for VS-stage A/D updates"), which made the A/D write respect G-stage permission, and fixes what is still reported wrong on such a trap. Patch 2 adds a test, test-gstage-tinst (19 cases), master fails 8. Built for Spike's HTIF console instead of semihosting, all 19 cases pass on upstream Spike. Patch 1 fixes the read/write pseudoinstruction in mtinst/htinst (0x3000 or 0x3020) [1]. 366b4fba0fae sets the write bit when the trap cause is a store guest-page fault, but the cause is the original access type [2], while the pseudoinstruction describes the implicit access that failed: original access, what failed Spike master load or fetch, A/D write 0x3020 0x3000 store, A/D write 0x3020 0x3020 store, read of a non-leaf PTE 0x3000 0x3020 The commit was checked against Spike for the store case, where both agree. Patch 1 fixes all 8 failures of test-gstage-tinst. Results on master (1c1f512972), with make check-tcg for riscv64-softmmu: applied test-gstage-tinst check-tcg patch 2 only 8 of 19 fail 29/37 patches 1-2 all pass 37/37 The test is last so that every commit passes check-tcg. This bug was found during formal verification of a sandbox on top of the xv6 OS where tests passed on qemu but verification against Sail failed. Claude was used to diagnose the issue. All commits were human written. [1] Privileged ISA v1.12, 8.6.3, Table 8.12 [2] Privileged ISA v1.12, 8.5.1 "Guest Physical Address Translation" https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12 tewaro (2): target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update tests/tcg/riscv64: test faults on implicit VS-stage page-table accesses target/riscv/cpu.h | 1 + target/riscv/tcg/cpu_helper.c | 8 +- tests/tcg/riscv64/hyp-guest.S | 110 +++++++++++++++++++++ tests/tcg/riscv64/hyp-guest.h | 71 ++++++++++++++ tests/tcg/riscv64/system/meson.build | 77 +++++++++++++++ tests/tcg/riscv64/test-gstage-tinst.S | 135 ++++++++++++++++++++++++++ 6 files changed, 401 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/riscv64/hyp-guest.S create mode 100644 tests/tcg/riscv64/hyp-guest.h create mode 100644 tests/tcg/riscv64/test-gstage-tinst.S -- 2.54.0 (Apple Git-157) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update 2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro @ 2026-11-03 7:29 ` tewaro 2026-11-03 7:29 ` [PATCH 2/2] tests/tcg/riscv64: test faults on implicit VS-stage page-table accesses tewaro 2026-11-04 0:22 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on VS-stage A/D updates Alistair 2 siblings, 0 replies; 8+ messages in thread From: tewaro @ 2026-11-03 7:29 UTC (permalink / raw) To: qemu-devel Cc: disclosures, tewaro, imaginos32, Palmer Dabbelt, Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Chao Liu, open list:RISC-V TCG CPUs When a G-stage fault is taken during a VS-stage page-table walk, riscv_cpu_do_interrupt() writes a pseudoinstruction to mtinst/htinst. This instruction says if the walk's access was a read (0x3000) or a write (0x3020) of the PTE [1]. Commit 366b4fba0fae made the A/D update check G-stage write permission, and sets the write bit when the trap cause is a store guest-page fault. The trap cause is the wrong signal. The cause follows the original access [2], but the pseudoinstruction describes the implicit access that failed. The two disagree in two cases: original access failed access Spike reports QEMU reports load or fetch A/D write 0x3020 0x3000 store PTE read 0x3000 0x3020 They agree only for a store that fails on the A/D write, which is the case 366b4fba0fae was tested with. Record in the CPU state that the G-stage fault came from the A/D write, and use that for both the data and the instruction tinst sites. Clear it on entry to riscv_cpu_tlb_fill() and after the trap. [1] Privileged ISA v1.12, 8.6.3, Table 8.12 [2] Privileged ISA v1.12, 8.5.1 "Guest Physical Address Translation" https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12 Fixes: 366b4fba0fae ("target/riscv: check G-stage write permission for VS-stage A/D updates") Signed-off-by: tewaro <tewaro@theorem.dev> --- target/riscv/cpu.h | 1 + target/riscv/tcg/cpu_helper.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 49a1186bdb..d56566d8c8 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -445,6 +445,7 @@ struct CPUArchState { * address translation for the VS-stage page table walk. */ bool two_stage_indirect_lookup; + bool two_stage_indirect_write; uint32_t scounteren; uint32_t mcounteren; diff --git a/target/riscv/tcg/cpu_helper.c b/target/riscv/tcg/cpu_helper.c index 58e98e975c..063525cfc5 100644 --- a/target/riscv/tcg/cpu_helper.c +++ b/target/riscv/tcg/cpu_helper.c @@ -1544,6 +1544,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, if (fault_pte_addr) { *fault_pte_addr = pte_gpa >> 2; } + env->two_stage_indirect_write = true; return TRANSLATE_G_STAGE_FAIL; } } @@ -1785,6 +1786,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, hwaddr tlb_size = TARGET_PAGE_SIZE; env->guest_phys_fault_addr = 0; + env->two_stage_indirect_write = false; qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n", __func__, address, access_type, mmu_idx); @@ -2233,7 +2235,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) */ tinst = (riscv_cpu_xlen(env) == 32) ? 0x00002000 : 0x00003000; - if (cause == RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT) { + if (env->two_stage_indirect_write) { tinst |= 0x20; } } else { @@ -2256,6 +2258,9 @@ void riscv_cpu_do_interrupt(CPUState *cs) * doing VS-stage page table walk. */ tinst = (riscv_cpu_xlen(env) == 32) ? 0x00002000 : 0x00003000; + if (env->two_stage_indirect_write) { + tinst |= 0x20; + } } break; case RISCV_EXCP_ILLEGAL_INST: @@ -2506,6 +2511,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) env->two_stage_lookup = false; env->two_stage_indirect_lookup = false; + env->two_stage_indirect_write = false; } #endif /* !CONFIG_USER_ONLY */ -- 2.54.0 (Apple Git-157) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] tests/tcg/riscv64: test faults on implicit VS-stage page-table accesses 2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro 2026-11-03 7:29 ` [PATCH 1/2] target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update tewaro @ 2026-11-03 7:29 ` tewaro 2026-11-04 0:22 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on VS-stage A/D updates Alistair 2 siblings, 0 replies; 8+ messages in thread From: tewaro @ 2026-11-03 7:29 UTC (permalink / raw) To: qemu-devel Cc: disclosures, tewaro, imaginos32, Palmer Dabbelt, Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Chao Liu, open list:RISC-V TCG CPUs When a guest accesses memory, the hardware walks the guest's page tables. The guest reads the entries and, with Svadu, writes the A/D bits. The G-stage checks these reads and writes like any other guest access. If it refuses one, the guest-page fault reports: - cause: the type of the guest's original access - mtval2/htval: the guest-physical address of the entry, >> 2 - mtinst/htinst: 0x3000 if the walk was reading, 0x3020 if writing test-gstage-tinst checks 19 cases: case access from G-stage refuses reports ------------ ------ ---- --------------- ----------------- load ld VS A write load GPF, 0x3020 fetch jr VS A write fetch GPF, 0x3020 store sd VS D write store GPF, 0x3020 store-ad sd VS A/D write store GPF, 0x3020 amo amoadd VS D write store GPF, 0x3020 load-4k ld VS A write load GPF, 0x3020 load-1g ld VS A write load GPF, 0x3020 load-vu ld VU A write load GPF, 0x3020 hlv hlv.d HS A write load GPF, 0x3020 hsv hsv.d HS D write store GPF, 0x3020 load-hs ld VS A write load GPF, 0x3020 store-hs sd VS D write store GPF, 0x3020 hlv-noread hlv.d HS root PTE read load GPF, 0x3000 hsv-noread hsv.d HS root PTE read store GPF, 0x3000 fetch-noread fetch VS root PTE read fetch GPF, 0x3000 noadue-h ld VS - load PF noadue-m ld VS - load PF noadue-store sd VS - store PF control sd VS the store store GPF - Leaves are 2 MiB, except in load-4k and load-1g. - Faults trap to M-mode, except in load-hs and store-hs. - The noadue cases turn ADUE off, so nothing is written and the fault is an ordinary page fault. - In control the guest itself stores into its read-only tables. That is not a walk, so tinst must be neither 0x3000 nor 0x3020. Booting, the page tables and entering the guest are in hyp-guest.h and hyp-guest.S, for reuse by other hypervisor tests. The trap handlers, which make the checks, are in the test. Each case is its own test, built with the -D flags listed in meson.build, and exits with 0 or the number of the first check that failed. Happy to test in a different way if that is desired. Signed-off-by: tewaro <tewaro@theorem.dev> --- tests/tcg/riscv64/hyp-guest.S | 110 +++++++++++++++++++++ tests/tcg/riscv64/hyp-guest.h | 71 ++++++++++++++ tests/tcg/riscv64/system/meson.build | 77 +++++++++++++++ tests/tcg/riscv64/test-gstage-tinst.S | 135 ++++++++++++++++++++++++++ 4 files changed, 393 insertions(+) create mode 100644 tests/tcg/riscv64/hyp-guest.S create mode 100644 tests/tcg/riscv64/hyp-guest.h create mode 100644 tests/tcg/riscv64/test-gstage-tinst.S diff --git a/tests/tcg/riscv64/hyp-guest.S b/tests/tcg/riscv64/hyp-guest.S new file mode 100644 index 0000000000..31dc37810c --- /dev/null +++ b/tests/tcg/riscv64/hyp-guest.S @@ -0,0 +1,110 @@ +/* + * Routines for the riscv64 hypervisor-extension system tests targeting + * page-table translations between the host and guest. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include "hyp-guest.h" + +#define FLUSH fence rw, rw; sfence.vma; hfence.vvma; hfence.gvma; fence.i + + .option norvc + .text + .global _start, set_tables_perm, run_guest, exit +_start: + lla sp, stack_top + lla t0, trap + csrw mtvec, t0 + lla t0, hs_trap + csrw stvec, t0 + li t0, -1 /* no PMP restrictions */ + csrw pmpaddr0, t0 + li t0, 0x1f + csrw pmpcfg0, t0 + li t0, ADUE /* hardware A/D updating */ + csrw menvcfg, t0 + csrw henvcfg, t0 + li t0, 0x100 /* hstatus.SPVP: hlv/hsv act as VS */ + csrs hstatus, t0 + + /* G-stage: regions 0-24, 1:1, all accessible */ + li t0, G_ROOT + 2 * 8 + li t1, table(G_L1) + sd t1, 0(t0) + li t0, G_L1 + li t1, leaf(REGION(0), G_RW) + li t2, 0x200000 >> 2 + li t3, 25 +1: sd t1, 0(t0) + add t1, t1, t2 + addi t0, t0, 8 + addi t3, t3, -1 + bnez t3, 1b + + /* guest: the code, the code again for VU, the 4 KiB table */ + li t0, VS_ROOT + 2 * 8 + li t1, table(VS_L1) + sd t1, 0(t0) + li t0, VS_L1 + li t1, leaf(REGION(0), G_RW & ~PTE_U) + sd t1, 0(t0) + li t1, leaf(REGION(0), G_RO & ~PTE_W) + sd t1, 24 * 8(t0) + li t1, table(VS_L0) + sd t1, 8 * 8(t0) + + li t0, (8 << 60) | (G_ROOT >> 12) + csrw hgatp, t0 + li t0, (8 << 60) | (VS_ROOT >> 12) + csrw vsatp, t0 + FLUSH + + call test + li a0, 1 /* test returned: no trap */ + +/* exit through semihosting with status a0 */ +exit: + lla a1, semiargs + li t0, 0x20026 /* ADP_Stopped_ApplicationExit */ + sd t0, 0(a1) + sd a0, 8(a1) + li a0, 0x20 /* SYS_EXIT_EXTENDED */ + .balign 16 + slli zero, zero, 0x1f + ebreak + srai zero, zero, 0x7 +1: j 1b + +set_tables_perm: + li t0, leaf(REGION(2), 0) + or t1, a0, t0 + li t0, G_L1 + 2 * 8 + sd t1, 0(t0) + FLUSH + ret + +/* mret into the mode: mstatus.MPV = 1 for a guest, MPP = S or U */ +run_guest: + li t0, (3 << 11) | (1 << 39) + csrc mstatus, t0 + li t0, HS + beq a2, t0, 2f + li t0, 1 << 39 /* MPV: a guest mode */ + csrs mstatus, t0 + li t0, VU + bne a2, t0, 2f + li t0, REGION(24) - REGION(0) /* VU: the user copy */ + add a1, a1, t0 + j 3f +2: li t0, 1 << 11 /* MPP = S */ + csrs mstatus, t0 +3: csrw mepc, a1 + mret + + .data + .balign 8 +semiargs: .dword 0, 0 + .bss + .balign 16 + .space 4096 +stack_top: diff --git a/tests/tcg/riscv64/hyp-guest.h b/tests/tcg/riscv64/hyp-guest.h new file mode 100644 index 0000000000..20ecef70a5 --- /dev/null +++ b/tests/tcg/riscv64/hyp-guest.h @@ -0,0 +1,71 @@ +/* + * Shared definitions for the riscv64 hypervisor-extension system tests, + * whose routines are in hyp-guest.S. hyp-guest.S: + * + * 1. Boots the machine in M-mode, sets up the stack and tells the CPU + * which trap handlers to use. + * 2. Builds the guest's page tables and the G-stage (host) page tables. + * 3. Calls `test`, which starts the test. + * + * The test file provides: + * - test: where the test starts, called by _start after setup + * - trap: the M-mode trap handler + * - hs_trap: the HS-mode trap handler + * + * Helpers a test can call: + * - set_tables_perm(a0): set what the G-stage lets the guest do to its + * own page tables. a0 is G_RW, G_RO or G_NOREAD. + * - run_guest(a0, a1, a2): run the code at address a1 in mode a2 (VS, VU + * or HS), with a0 passed on to it in a0. It does not return: the + * code's trap goes to the test's handler. + * - exit(a0): end the test with status a0, 0 for a pass, otherwise the + * number of the check that failed. + * + * Memory is split into 2 MiB regions; region n starts at + * REGION(n) = 0x80000000 + n * 2 MiB. Both stages map each region to + * itself. + * + * 0 code and the G-stage page tables + * 1 data and the stack + * 2 the guest's page tables + * 3-23 free for the test + * 24 region 0's code again, user-accessible, for VU-mode + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#define PTE_V 0x01 +#define PTE_R 0x02 +#define PTE_W 0x04 +#define PTE_X 0x08 +#define PTE_U 0x10 +#define PTE_A 0x40 +#define PTE_D 0x80 +#define leaf(pa, flags) (((pa) >> 2) | PTE_V | (flags)) +#define table(pa) (((pa) >> 2) | PTE_V) + +#define REGION(n) (0x80000000 + (n) * 0x200000) +#define G_ROOT 0x80104000 +#define G_L1 0x80108000 +#define VS_ROOT 0x80400000 +#define VS_L1 0x80401000 +#define VS_L0 0x80402000 /* 4 KiB table for region 8 */ + +/* G-stage permission of the guest's tables */ +#define G_RW (PTE_R | PTE_W | PTE_X | PTE_U | PTE_A | PTE_D) +#define G_RO (G_RW & ~PTE_W) +#define G_NOREAD (G_RO & ~PTE_R) + +/* run_guest modes */ +#define VU 0 +#define HS 1 +#define VS 2 + +#define LOAD_PF 13 +#define STORE_PF 15 +#define FETCH_GPF 20 +#define LOAD_GPF 21 +#define STORE_GPF 23 +#define TINST_READ 0x3000 /* walk was reading a page-table entry */ +#define TINST_WRITE 0x3020 /* walk was writing A/D */ +#define ADUE (1 << 61) diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build index 3c8b018d9e..05ed8b4c8f 100644 --- a/tests/tcg/riscv64/system/meson.build +++ b/tests/tcg/riscv64/system/meson.build @@ -53,6 +53,83 @@ tests += { } } +hyp_guest = files('../hyp-guest.S')[0] + +# test-gstage-tinst.S: one build per case; the flags are documented there +gstage_tinst = { + # the guest's tables are read-only, so the walk's A/D write is refused + 'load': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=3', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'fetch': ['-DMODE=VS', '-DACCESS=jr a0', '-DREG=4', + '-DFLAGS=(PTE_R|PTE_X)', '-DCAUSE=FETCH_GPF'], + 'store': ['-DMODE=VS', '-DACCESS=sd zero, 0(a0)', '-DREG=5', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', + '-DCAUSE=STORE_GPF'], + 'store-ad': ['-DMODE=VS', '-DACCESS=sd zero, 0(a0)', '-DREG=6', + '-DFLAGS=(PTE_R|PTE_W)', + '-DCAUSE=STORE_GPF'], + 'amo': ['-DMODE=VS', '-DACCESS=amoadd.d t0, zero, (a0)', '-DREG=7', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', + '-DCAUSE=STORE_GPF'], + 'load-4k': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=8', + '-DPTE_ADDR=VS_L0', '-DFLAGS=PTE_R', + '-DCAUSE=LOAD_GPF'], + 'load-1g': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', + '-DADDR=(0xC0000000+9*0x200000)', '-DPTE_ADDR=(VS_ROOT+3*8)', + '-DPTE_PA=REGION(0)', '-DFLAGS=PTE_R', + '-DCAUSE=LOAD_GPF'], + 'load-vu': ['-DMODE=VU', '-DACCESS=ld t0, 0(a0)', '-DREG=10', + '-DFLAGS=(PTE_R|PTE_U)', '-DCAUSE=LOAD_GPF'], + 'hlv': ['-DMODE=HS', '-DACCESS=hlv.d t0, (a0)', '-DREG=11', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'hsv': ['-DMODE=HS', '-DACCESS=hsv.d zero, (a0)', '-DREG=12', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', + '-DCAUSE=STORE_GPF'], + 'load-hs': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=13', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF', + '-DTO_HS'], + 'store-hs': ['-DMODE=VS', '-DACCESS=sd zero, 0(a0)', '-DREG=14', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', + '-DCAUSE=STORE_GPF', '-DTO_HS'], + # the tables are unreadable, so the walk fails reading root entry 2 + 'hsv-noread': ['-DMODE=HS', '-DACCESS=hsv.d zero, (a0)', '-DREG=15', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', '-DPERM=G_NOREAD', + '-DENTRY=(VS_ROOT+2*8)', '-DCAUSE=STORE_GPF', + '-DTINST=TINST_READ'], + 'hlv-noread': ['-DMODE=HS', '-DACCESS=hlv.d t0, (a0)', '-DREG=16', + '-DFLAGS=(PTE_R|PTE_A)', '-DPERM=G_NOREAD', + '-DENTRY=(VS_ROOT+2*8)', '-DCAUSE=LOAD_GPF', + '-DTINST=TINST_READ'], + 'fetch-noread': ['-DMODE=VS', '-DREG=17', '-DADDR=probe', + '-DFLAGS=(PTE_R|PTE_X|PTE_A)', '-DPERM=G_NOREAD', + '-DENTRY=(VS_ROOT+2*8)', '-DCAUSE=FETCH_GPF', + '-DTINST=TINST_READ'], + # A/D updating off: an ordinary page fault, nothing written + 'noadue-h': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=21', + '-DFLAGS=PTE_R', '-DHENVCFG=0', '-DCAUSE=LOAD_PF', + '-DENTRY=0', '-DTINST_NOT_WALK'], + 'noadue-m': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=22', + '-DFLAGS=PTE_R', '-DMENVCFG=0', '-DCAUSE=LOAD_PF', + '-DENTRY=0', '-DTINST_NOT_WALK'], + 'noadue-store': ['-DMODE=VS', '-DACCESS=sd zero, 0(a0)', '-DREG=23', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', '-DHENVCFG=0', + '-DCAUSE=STORE_PF', '-DENTRY=0', '-DTINST_NOT_WALK'], + # control: the guest itself stores into its read-only tables + 'control': ['-DMODE=VS', '-DACCESS=sd zero, 0(a0)', '-DREG=2', + '-DFLAGS=(PTE_R|PTE_W|PTE_A|PTE_D)', '-DADDR=(VS_ROOT+0x800)', + '-DENTRY=(VS_ROOT+0x800)', '-DCAUSE=STORE_GPF', + '-DTINST_NOT_WALK'], +} +foreach name, flags : gstage_tinst + tests += { + 'test-gstage-tinst.S': { + 'exe_name': 'test-gstage-tinst-' + name, + 'cflags': cflags + ['-march=rv64gch', hyp_guest] + flags, + 'qemu_args': ['-cpu', 'rv64,h=true,svadu=true', qemu_args], + } + } +endforeach + tests += { 'test-vle32ff.S': { 'cflags': cflags + ['-march=rv64gcv'], diff --git a/tests/tcg/riscv64/test-gstage-tinst.S b/tests/tcg/riscv64/test-gstage-tinst.S new file mode 100644 index 0000000000..8c9c7b831f --- /dev/null +++ b/tests/tcg/riscv64/test-gstage-tinst.S @@ -0,0 +1,135 @@ +/* + * Test what a fault reports when the G-stage refuses the guest's + * page-table walk: its read of a page-table entry, or its write of the + * A/D bits. + * + * Each build is one case, set by -D flags in meson.build: + * - MODE: VS, VU or HS (HS runs hlv/hsv on the guest's behalf) + * - ACCESS: the instruction run on ADDR + * - REG: the region accessed; ADDR defaults to its start + * - FLAGS: the guest's page-table entry for ADDR + * - PERM: the G-stage permission of the guest's tables (default G_RO) + * - TO_HS: take the fault in HS-mode instead of M-mode + * - MENVCFG, HENVCFG: turn ADUE off + * - PTE_ADDR, PTE_PA: where the entry is and where it points, for the + * 4 KiB and 1 GiB cases + * - CAUSE, ENTRY, TINST, TINST_NOT_WALK: what the fault must report + * + * The trap handler checks, and the exit status is the first that fails: + * 1 cause CAUSE + * 2 tval ADDR + * 3 tval2 ENTRY >> 2, the entry the walk failed on (default: the + * guest's entry for ADDR) + * 4 tinst TINST: 0x3020 if the walk was writing A/D (default), + * 0x3000 if reading; with TINST_NOT_WALK, neither + * 5 the entry is unchanged, so the refused A/D write did not happen + * + * `trap` checks a fault taken in M-mode. `hs_trap` checks one taken in + * HS-mode, using scause, stval, htval and htinst. + * + * Privileged ISA v1.12: 8.5.1, 8.6.3 Table 8.12. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include "hyp-guest.h" + +/* exit with status n unless reg == want */ +#define CHECK(n, reg, want) li t6, want; li a0, n; bne reg, t6, exit + +#ifdef REG +#ifndef ADDR +#define ADDR REGION(REG) +#endif +#ifndef PTE_ADDR +#define PTE_ADDR (VS_L1 + (REG) * 8) +#endif +#ifndef PTE_PA +#define PTE_PA REGION(REG) +#endif +#endif +#ifndef ENTRY +#define ENTRY PTE_ADDR +#endif +#ifndef PERM +#define PERM G_RO +#endif +#ifndef TINST +#define TINST TINST_WRITE +#endif + + .option norvc + .text + .global test, trap, hs_trap +test: +#ifdef MENVCFG + li t0, MENVCFG /* A/D updating off */ + csrw menvcfg, t0 +#endif +#ifdef HENVCFG + li t0, HENVCFG + csrw henvcfg, t0 +#endif +#ifdef TO_HS + li t0, 1 << CAUSE + csrw medeleg, t0 +#endif + li t0, PTE_ADDR /* the guest's entry for ADDR */ + li t1, leaf(PTE_PA, FLAGS) + sd t1, 0(t0) +#if REG >= 3 + li t0, REGION(REG) /* an ecall for a jump to land on */ + li t1, 0x00000073 + sw t1, 0(t0) +#endif + li a0, PERM + call set_tables_perm + + ld a0, addr + lla a1, probe + li a2, MODE + call run_guest + +probe: +#ifdef ACCESS + ACCESS +#endif + ecall + +/* the fault, as reported by CSRs cause, tval, tval2 and tinst */ +.macro check_fault cause, tval, tval2, tinst + csrr t5, \cause + CHECK(1, t5, CAUSE) + csrr t5, \tval + ld t6, addr + li a0, 2 + bne t5, t6, exit + csrr t5, \tval2 + CHECK(3, t5, (ENTRY) >> 2) + csrr t5, \tinst +#ifdef TINST_NOT_WALK + li a0, 4 + li t6, TINST_READ + beq t5, t6, exit + li t6, TINST_WRITE + beq t5, t6, exit +#else + CHECK(4, t5, TINST) +#endif + li t0, PTE_ADDR + ld t5, 0(t0) + CHECK(5, t5, leaf(PTE_PA, FLAGS)) + li a0, 0 + j exit +.endm + + .balign 4 +trap: + check_fault mcause, mtval, mtval2, mtinst + + .balign 4 +hs_trap: + check_fault scause, stval, htval, htinst + + .data + .balign 8 +addr: .dword ADDR -- 2.54.0 (Apple Git-157) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on VS-stage A/D updates 2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro 2026-11-03 7:29 ` [PATCH 1/2] target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update tewaro 2026-11-03 7:29 ` [PATCH 2/2] tests/tcg/riscv64: test faults on implicit VS-stage page-table accesses tewaro @ 2026-11-04 0:22 ` Alistair 2 siblings, 0 replies; 8+ messages in thread From: Alistair @ 2026-11-04 0:22 UTC (permalink / raw) To: tewaro, qemu-devel; +Cc: disclosures, tewaro, imaginos32 On Mon, 2026-11-02 at 23:29 -0800, tewaro wrote: > Hi, > > This series is about the traps taken when the G-stage refuses one of > the > implicit accesses that VS-stage address translation makes. It follows > up on > commit 366b4fba0fae ("target/riscv: check G-stage write permission > for > VS-stage A/D updates"), which made the A/D write respect G-stage > permission, > and fixes what is still reported wrong on such a trap. > > Patch 2 adds a test, test-gstage-tinst (19 cases), master fails 8. > Built for > Spike's HTIF console instead of semihosting, all 19 cases pass on > upstream > Spike. > > Patch 1 fixes the read/write pseudoinstruction in mtinst/htinst > (0x3000 or > 0x3020) [1]. 366b4fba0fae sets the write bit when the trap cause is a > store > guest-page fault, but the cause is the original access type [2], > while the > pseudoinstruction describes the implicit access that failed: > > original access, what failed Spike master > load or fetch, A/D write 0x3020 0x3000 > store, A/D write 0x3020 0x3020 > store, read of a non-leaf PTE 0x3000 0x3020 > > The commit was checked against Spike for the store case, where both > agree. > Patch 1 fixes all 8 failures of test-gstage-tinst. > > Results on master (1c1f512972), with make check-tcg for riscv64- > softmmu: > > applied test-gstage-tinst check-tcg > patch 2 only 8 of 19 fail 29/37 > patches 1-2 all pass 37/37 > > The test is last so that every commit passes check-tcg. > > This bug was found during formal verification of a sandbox on top of > the xv6 > OS where tests passed on qemu but verification against Sail failed. > Claude > was used to diagnose the issue. All commits were human written. > > [1] Privileged ISA v1.12, 8.6.3, Table 8.12 > [2] Privileged ISA v1.12, 8.5.1 "Guest Physical Address Translation" > https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12 > > tewaro (2): > target/riscv: report the write pseudoinstruction for a failed VS- > stage > A/D update > tests/tcg/riscv64: test faults on implicit VS-stage page-table > accesses Can you please use the `./scripts/get_maintainer.pl` script on your patches to find the complete list of email addresses to send this to Alistair > > target/riscv/cpu.h | 1 + > target/riscv/tcg/cpu_helper.c | 8 +- > tests/tcg/riscv64/hyp-guest.S | 110 +++++++++++++++++++++ > tests/tcg/riscv64/hyp-guest.h | 71 ++++++++++++++ > tests/tcg/riscv64/system/meson.build | 77 +++++++++++++++ > tests/tcg/riscv64/test-gstage-tinst.S | 135 > ++++++++++++++++++++++++++ > 6 files changed, 401 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/riscv64/hyp-guest.S > create mode 100644 tests/tcg/riscv64/hyp-guest.h > create mode 100644 tests/tcg/riscv64/test-gstage-tinst.S ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-04 0:23 UTC | newest]
Thread overview: 8+ 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
2026-09-03 21:44 ` Imaginos
2026-09-07 4:56 ` Alistair Francis
2026-11-03 7:29 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on " tewaro
2026-11-03 7:29 ` [PATCH 1/2] target/riscv: report the write pseudoinstruction for a failed VS-stage A/D update tewaro
2026-11-03 7:29 ` [PATCH 2/2] tests/tcg/riscv64: test faults on implicit VS-stage page-table accesses tewaro
2026-11-04 0:22 ` [PATCH 0/2] target/riscv: fix mtinst/htinst for G-stage faults on VS-stage A/D updates Alistair
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.