Re: [PATCH 1/3] target/riscv: Set env->bins in gen_exception_illegal

[Alistair Francis]

On Sun, Jun 5, 2022 at 9:11 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> While we set env->bins when unwinding for ILLEGAL_INST,
> from e.g. csrrw, we weren't setting it for immediately
> illegal instructions.
>
> Add a testcase for mtval via both exception paths.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1060
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/riscv/translate.c                  |  2 +
>  tests/tcg/riscv64/Makefile.softmmu-target | 21 +++++++++
>  tests/tcg/riscv64/issue1060.S             | 53 +++++++++++++++++++++++
>  tests/tcg/riscv64/semihost.ld             | 21 +++++++++
>  4 files changed, 97 insertions(+)
>  create mode 100644 tests/tcg/riscv64/Makefile.softmmu-target
>  create mode 100644 tests/tcg/riscv64/issue1060.S
>  create mode 100644 tests/tcg/riscv64/semihost.ld
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 55a4713af2..9196aa71db 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -238,6 +238,8 @@ static void generate_exception_mtval(DisasContext *ctx, 
> int excp)
>
>  static void gen_exception_illegal(DisasContext *ctx)
>  {
> +    tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
> +                   offsetof(CPURISCVState, bins));
>      generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
>  }
>
> diff --git a/tests/tcg/riscv64/Makefile.softmmu-target 
> b/tests/tcg/riscv64/Makefile.softmmu-target
> new file mode 100644
> index 0000000000..d51ece7023
> --- /dev/null
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -0,0 +1,21 @@
> +#
> +# Aarch64 system tests

Copy and paste error

Otherwise:

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> +#
> +
> +TEST_SRC = $(SRC_PATH)/tests/tcg/riscv64
> +VPATH += $(TEST_SRC)
> +
> +LINK_SCRIPT = $(TEST_SRC)/semihost.ld
> +LDFLAGS = -T $(LINK_SCRIPT)
> +CFLAGS += -g -Og
> +
> +%.o: %.S
> +       $(CC) $(CFLAGS) $< -c -o $@
> +%: %.o $(LINK_SCRIPT)
> +       $(LD) $(LDFLAGS) $< -o $@
> +
> +QEMU_OPTS += -M virt -display none -semihosting -device loader,file=
> +
> +EXTRA_RUNS += run-issue1060
> +run-issue1060: issue1060
> +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> diff --git a/tests/tcg/riscv64/issue1060.S b/tests/tcg/riscv64/issue1060.S
> new file mode 100644
> index 0000000000..17b7fe1be2
> --- /dev/null
> +++ b/tests/tcg/riscv64/issue1060.S
> @@ -0,0 +1,53 @@
> +       .option norvc
> +
> +       .text
> +       .global _start
> +_start:
> +       lla     t0, trap
> +       csrw    mtvec, t0
> +
> +       # These are all illegal instructions
> +       csrw    time, x0
> +       .insn   i CUSTOM_0, 0, x0, x0, 0x321
> +       csrw    time, x0
> +       .insn   i CUSTOM_0, 0, x0, x0, 0x123
> +       csrw    cycle, x0
> +
> +       # Success!
> +       li      a0, 0
> +       j       _exit
> +
> +trap:
> +       # When an instruction traps, compare it to the insn in memory.
> +       csrr    t0, mepc
> +       csrr    t1, mtval
> +       lwu     t2, 0(t0)
> +       bne     t1, t2, fail
> +
> +       # Skip the insn and continue.
> +       addi    t0, t0, 4
> +       csrw    mepc, t0
> +       mret
> +
> +fail:
> +       li      a0, 1
> +
> +# Exit code in a0
> +_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       .
> +
> +       .data
> +       .balign 16
> +semiargs:
> +       .space  16
> diff --git a/tests/tcg/riscv64/semihost.ld b/tests/tcg/riscv64/semihost.ld
> new file mode 100644
> index 0000000000..a59cc56b28
> --- /dev/null
> +++ b/tests/tcg/riscv64/semihost.ld
> @@ -0,0 +1,21 @@
> +ENTRY(_start)
> +
> +SECTIONS
> +{
> +    /* virt machine, RAM starts at 2gb */
> +    . = 0x80000000;
> +    .text : {
> +        *(.text)
> +    }
> +    .rodata : {
> +        *(.rodata)
> +    }
> +    /* align r/w section to next 2mb */
> +    . = ALIGN(1 << 21);
> +    .data : {
> +        *(.data)
> +    }
> +    .bss : {
> +        *(.bss)
> +    }
> +}
> --
> 2.34.1
>
>