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