target/riscv: write mstatus.GVA on every trap into M-mode
Subsystems (2)
cover target/riscv: write mstatus.GVA on every trap into M-mode
Hi,
Based-on: <[email protected]> ("target/riscv: fix mtinst/htinst for G-stage faults on VS-stage A/D updates", whose hyp-guest.h and hyp-guest.S this test reuses)
Patch 1 fixes mstatus.GVA. On every trap into M-mode, GVA should be set to 1 if mtval holds a guest virtual address and to 0 otherwise [1]. QEMU does not clear gva or set GVA for particular faults.
Both date back to 9034e90ad9. The fix sets mstatus.GVA from write_gva, the value QEMU already computes for hstatus.GVA.
Patch 2 adds a test, all 4 pass on upstream Spike, all fail on master.
This bug was found while examining <[email protected]> and writing a generalized test. Claude was used to diagnose the issue. All commits were human written.
[1] Privileged ISA v1.12, 8.2.1, the description of GVA and the note under hstatus.GVA
https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12
tewaro (2):
target/riscv: write mstatus.GVA on every trap into M-mode tests/tcg/riscv64: test mstatus.GVA on traps into M-mode
target/riscv/tcg/cpu_helper.c | 4 +- tests/tcg/riscv64/system/meson.build | 21 ++++++++ tests/tcg/riscv64/test-mstatus-gva.S | 75 ++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/tcg/riscv64/test-mstatus-gva.S
Signature · 1 line
2.54.0 (Apple Git-157)
PATCH 1/2 target/riscv: write mstatus.GVA on every trap into M-mode
On every trap into M-mode, mstatus.GVA should be set to 1 if mtval holds a guest virtual address and to 0 otherwise [1]. riscv_cpu_do_interrupt() only ever sets it, and only when V = 1 and tval is nonzero. That has two effects:
- GVA is never cleared. After one guest-page fault, every later trap into M-mode, including an ecall, reports GVA = 1.
- GVA stays 0 when HLV, HLVX or HSV faults, because those run with V = 0. The spec names this case ("SPV=0 but GVA=1").
The HS-mode path already computes write_gva for hstatus.GVA. Use it for mstatus.GVA too.
[1] Privileged ISA v1.12, 8.2.1
https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4594
--- target/riscv/tcg/cpu_helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
target/riscv/tcg/cpu_helper.c +1 -3
@@ -2416,9 +2416,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) } env->mstatus =set_field (env->mstatus,MSTATUS_MPV , env->virt_enabled); -if (env->virt_enabled && tval) { - env->mstatus =set_field (env->mstatus,MSTATUS_GVA ,1 ); - } + env->mstatus =set_field (env->mstatus,MSTATUS_GVA , write_gva); mtval2 = env->guest_phys_fault_addr;
Signature · 1 line
2.54.0 (Apple Git-157)
Quoted 13 lines · tewaro
On every trap into M-mode, mstatus.GVA should be set to 1 if mtval holds a guest virtual address and to 0 otherwise [1].
riscv_cpu_do_interrupt()only ever sets it, and only when V = 1 and tval is nonzero. That has two effects:- GVA is never cleared. After one guest-page fault, every later trap into M-mode, including an ecall, reports GVA = 1.
- GVA stays 0 when HLV, HLVX or HSV faults, because those run with V = 0. The spec names this case ("SPV=0 but GVA=1").The HS-mode path already computes
write_gvafor hstatus.GVA. Use it for mstatus.GVA too.
Quoted 6 lines
[1] Privileged ISA v1.12, 8.2.1
https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4594
Signed-off-by tewaro <[email protected]>---
Quoted 18 lines
target/riscv/tcg/cpu_helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)target/riscv/tcg/cpu_helper.c +1
-3@@ -2416,9 +2416,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) } env->mstatus =set_field (env->mstatus,MSTATUS_MPV , env->virt_enabled); -if (env->virt_enabled && tval) { - env->mstatus =set_field (env->mstatus,MSTATUS_GVA ,1 ); - } + env->mstatus =set_field (env->mstatus,MSTATUS_GVA , write_gva); mtval2 = env->guest_phys_fault_addr;
PATCH 2/2 tests/tcg/riscv64: test mstatus.GVA on traps into M-mode
In reply to tewaroEvery trap into M-mode writes mstatus.GVA. If mtval holds a guest virtual address then mstatus.GVA should be 1 otherwise it should be 0.
test-mstatus-gva checks 4 cases:
case access from ------- ------ ---- load ld VS load-vu ld VU hlv hlv.d HS hsv hsv.d HS
Each case makes a guest-page fault and checks that GVA is 1. It then runs an ecall in M-mode, whose trap has no guest address, and checks that GVA is 0. Without the previous patch QEMU fails all 4:
- load and load-vu: GVA stays 1 after the ecall
- hlv and hsv: GVA is 0 after the fault
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. It uses hyp-guest.h and hyp-guest.S from the previous series.
--- tests/tcg/riscv64/system/meson.build | 21 ++++++++ tests/tcg/riscv64/test-mstatus-gva.S | 75 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 tests/tcg/riscv64/test-mstatus-gva.S
tests/tcg/riscv64/system/meson.build +21 -0
@@ -130,6 +130,27 @@ foreach name, flags : gstage_tinst } endforeach +# test-mstatus-gva.S: one build per case; the flags are documented there +mstatus_gva = { + 'load': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=3', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'load-vu': ['-DMODE=VU', '-DACCESS=ld t0, 0(a0)', '-DREG=4', + '-DFLAGS=(PTE_R|PTE_U)', '-DCAUSE=LOAD_GPF'], + 'hlv': ['-DMODE=HS', '-DACCESS=hlv.d t0, (a0)', '-DREG=5', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'hsv': ['-DMODE=HS', '-DACCESS=hsv.d zero, (a0)', '-DREG=6', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', '-DCAUSE=STORE_GPF'], +} +foreach name, flags : mstatus_gva + tests += { + 'test-mstatus-gva.S': { + 'exe_name': 'test-mstatus-gva-' + 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'],
tests/tcg/riscv64/test-mstatus-gva.S new file +75 -0
75 lines not on this page · raw message
Signature · 1 line
2.54.0 (Apple Git-157)
Re: [PATCH 2/2] tests/tcg/riscv64: test mstatus.GVA on traps into M-mode
Quoted 19 lines · tewaro
Every trap into M-mode writes mstatus.GVA. If mtval holds a guest virtual address then mstatus.GVA should be 1 otherwise it should be 0.
test-mstatus-gva checks 4 cases:
case access from ------- ------ ---- load ld VS load-vu ld VU hlv hlv.d HS hsv hsv.d HSEach case makes a guest-page fault and checks that GVA is 1. It then runs an ecall in M-mode, whose trap has no guest address, and checks that GVA is 0. Without the previous patch QEMU fails all 4:
- load and load-vu: GVA stays 1 after the ecall
- hlv and hsv: GVA is 0 after the fault
Quoted 6 lines
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. It uses
hyp-guest.handhyp-guest.Sfrom the previous series.Signed-off-by tewaro <[email protected]>---
Quoted 118 lines
tests/tcg/riscv64/system/meson.build | 21 ++++++++ tests/tcg/riscv64/test-mstatus-gva.S | 75 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+)create mode 100644
tests/tcg/riscv64/test-mstatus-gva.Stests/tcg/riscv64/system/meson.build +21
-0@@ -130,6 +130,27 @@ foreach name, flags : gstage_tinst } endforeach +# test-mstatus-gva.S: one build per case; the flags are documented there +mstatus_gva = { + 'load': ['-DMODE=VS', '-DACCESS=ld t0, 0(a0)', '-DREG=3', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'load-vu': ['-DMODE=VU', '-DACCESS=ld t0, 0(a0)', '-DREG=4', + '-DFLAGS=(PTE_R|PTE_U)', '-DCAUSE=LOAD_GPF'], + 'hlv': ['-DMODE=HS', '-DACCESS=hlv.d t0, (a0)', '-DREG=5', + '-DFLAGS=PTE_R', '-DCAUSE=LOAD_GPF'], + 'hsv': ['-DMODE=HS', '-DACCESS=hsv.d zero, (a0)', '-DREG=6', + '-DFLAGS=(PTE_R|PTE_W|PTE_A)', '-DCAUSE=STORE_GPF'], +} +foreach name, flags : mstatus_gva + tests += { + 'test-mstatus-gva.S': { + 'exe_name': 'test-mstatus-gva-' + 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'],tests/tcg/riscv64/test-mstatus-gva.S new file +75
-0@@ -0,0 +1,75 @@ +/* + * Test that every trap into M-mode writes mstatus.GVA: 1 if mtval holds + * a guest virtual address, 0 otherwise. + * + * 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 the start of region REG + * - REG: the region accessed + * - FLAGS: the guest's page-table entry for REG, with A or D clear, so + * that the walk's A/D write faults on the read-only G-stage + * - CAUSE: the fault expected + * + * The trap handler checks, and the exit status is the first that fails: + * 1 mcause is CAUSE + * 2 GVA is 1, since mtval holds the guest address + * 3 after an ecall from M-mode, whose trap has no guest address, GVA + * is 0 + * + * `trap` makes checks 1 and 2, then runs the ecall, whose trap goes to + * `ecall_trap` for check 3. + * + * Privileged ISA v1.12: 8.2.1. + * + * 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 + +/* read mstatus.GVA into t5 */ +#define MSTATUS_GVA_SHIFT 38 +#define GVA csrr t5, mstatus; srli t5, t5, MSTATUS_GVA_SHIFT; andi t5, t5, 1 + + .option norvc + .text + .global test, trap, hs_trap +test: + li t0, VS_L1 + (REG) * 8 /* the guest's entry for REG */ + li t1, leaf(REGION(REG), FLAGS) + sd t1, 0(t0) + li a0, G_RO + call set_tables_perm + + li a0, REGION(REG) + lla a1, probe + li a2, MODE + call run_guest + +probe: + ACCESS + ecall + + .balign 4 +trap: + csrr t5, mcause + CHECK(1, t5, CAUSE) + GVA + CHECK(2, t5, 1) + lla t0, ecall_trap /* the ecall below traps there */ + csrw mtvec, t0 + ecall + + .balign 4 +ecall_trap: + GVA + CHECK(3, t5, 0) + li a0, 0 + j exit + +/* nothing is delegated to HS-mode, so no trap should come here */ + .balign 4 +hs_trap: + li a0, 1 + j exit