From: Zephyr Li <[email protected]> helper_wfi() never returns to translated code: it either raises an exception or exits the CPU loop after marking the CPU as halted. However, trans_wfi() leaves the translation state as DISAS_NEXT, so instructions following WFI can be included in the same TB.
With icount enabled, this makes the following instruction part of the TB's instruction count when WFI exits, causing minstret to be incremented once too many. Set DISAS_NORETURN after emitting the helper. Add a TCG test for both reported cases: WFI waking for a locally enabled pending interrupt with mstatus.MIE clear, and WFI taking a timer interrupt. Cc: [email protected] Fixes: 55c2a12cbcd3 ("RISC-V TCG Code Generation") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4230 Signed-off-by: Zephyr Li <[email protected]> Reviewed-by: Daniel Henrique Barboza <[email protected]> Message-ID: <[email protected]> Signed-off-by: Alistair Francis <[email protected]> --- .../tcg/insn_trans/trans_privileged.c.inc | 1 + tests/tcg/riscv64/system/meson.build | 1 + tests/tcg/riscv64/test-minstret-wfi.S | 100 ++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 tests/tcg/riscv64/test-minstret-wfi.S diff --git a/target/riscv/tcg/insn_trans/trans_privileged.c.inc b/target/riscv/tcg/insn_trans/trans_privileged.c.inc index a8eaccef67..ebbbb01179 100644 --- a/target/riscv/tcg/insn_trans/trans_privileged.c.inc +++ b/target/riscv/tcg/insn_trans/trans_privileged.c.inc @@ -145,6 +145,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a) decode_save_opc(ctx, 0); gen_update_pc(ctx, ctx->cur_insn_len); gen_helper_wfi(tcg_env); + ctx->base.is_jmp = DISAS_NORETURN; return true; #else return false; diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build index c2d7d09887..a632674832 100644 --- a/tests/tcg/riscv64/system/meson.build +++ b/tests/tcg/riscv64/system/meson.build @@ -23,6 +23,7 @@ tests += { 'test-mepc-masking.S': setup, 'test-crc32.S': setup + {'qemu_args': ['-cpu', 'rv64,xlrbr=true', qemu_args]}, 'test-minstret-ecall.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]}, + 'test-minstret-wfi.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]}, 'test-misa-w.S': setup + {'qemu_args': ['-cpu', 'rv64,x-misa-w=true,c=true,v=true', qemu_args]}, } diff --git a/tests/tcg/riscv64/test-minstret-wfi.S b/tests/tcg/riscv64/test-minstret-wfi.S new file mode 100644 index 0000000000..0fd0158d5b --- /dev/null +++ b/tests/tcg/riscv64/test-minstret-wfi.S @@ -0,0 +1,100 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + + .option norvc + + .text + .global _start +_start: + lla t0, trap + csrw mtvec, t0 + li s3, 0 + + /* + * A pending, locally enabled interrupt wakes WFI even with MIE + * clear. + */ + li t0, 0x8 /* MIE_MSIE */ + csrw mie, t0 + csrci mstatus, 0x8 /* MSTATUS_MIE */ + li t0, 0x2000000 /* MSIP0 */ + li t1, 1 + sw t1, 0(t0) + + /* + * The first CSR read and WFI retire before the second read obtains s1, + * so the expected difference is two. + */ + csrr s0, minstret + wfi + csrr s1, minstret + sub s1, s1, s0 + li t1, 2 + beq s1, t1, 1f + ori s3, s3, 1 +1: + + /* Clear the software interrupt before testing the trap path. */ + sw zero, 0(t0) + csrw mie, zero + + /* Schedule a timer interrupt far enough ahead to reach WFI first. */ + li t0, 0x200bff8 /* MTIME */ + ld t1, 0(t0) + addi t1, t1, 100 + li t0, 0x2004000 /* MTIMECMP0 */ + sd t1, 0(t0) + li t0, 0x80 /* MIE_MTIE */ + csrw mie, t0 + csrsi mstatus, 0x8 /* MSTATUS_MIE */ + + /* + * The first CSR read and WFI retire before the trap handler obtains s1, + * so the expected difference is again two. + */ + li s2, 1 + csrr s0, minstret + wfi + beqz s2, 1f + ori s3, s3, 2 +1: + bnez s3, fail + li a0, 0 + j _exit + +fail: + mv a0, s3 + +_exit: + lla a1, semiargs + li t0, 0x20026 /* ADP_Stopped_ApplicationExit */ + sd t0, 0(a1) + sd a0, 8(a1) + li a0, 0x20 /* TARGET_SYS_EXIT_EXTENDED */ + + /* Semihosting call sequence */ + .balign 16 + slli zero, zero, 0x1f + ebreak + srai zero, zero, 0x7 + j . + + .balign 4 +trap: + /* Read minstret before retiring an instruction in the handler. */ + csrr s1, minstret + sub s1, s1, s0 + addi s1, s1, -2 + snez s2, s1 + + /* Disable the timer interrupt before returning past WFI. */ + li t0, 0x2004000 /* MTIMECMP0 */ + li t1, -1 + sd t1, 0(t0) + li t0, 0x80 /* MIE_MTIE */ + csrc mie, t0 + mret + + .data + .balign 16 +semiargs: + .space 16 -- 2.55.0
