All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.