Re: [PATCH v4 3/3] target/riscv: Implement the stval/mtval illegal instruction

[Bin Meng]

On Mon, Dec 20, 2021 at 2:49 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> The stval and mtval registers can optionally contain the faulting
> instruction on an illegal instruction exception. This patch adds support
> for setting the stval and mtval registers.
>
> The RISC-V spec states that "The stval register can optionally also be
> used to return the faulting instruction bits on an illegal instruction
> exception...". In this case we are always writing the value on an
> illegal instruction.
>
> This doesn't match all CPUs (some CPUs won't write the data), but in
> QEMU let's just populate the value on illegal instructions. This won't
> break any guest software, but will provide more information to guests.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h        | 2 ++
>  target/riscv/cpu_helper.c | 3 +++
>  target/riscv/translate.c  | 3 +++
>  3 files changed, 8 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0760c0af93..3a163b57ed 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -127,6 +127,8 @@ struct CPURISCVState {
>      target_ulong frm;
>
>      target_ulong badaddr;
> +    uint32_t bins;

nits: does "badins" sound a better name? I took some time to figure
out what "bins" means :)

> +
>      target_ulong guest_phys_fault_addr;
>
>      target_ulong priv_ver;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9e1f5ee177..f76ba834e6 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1007,6 +1007,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>              write_gva = true;
>              tval = env->badaddr;
>              break;
> +        case RISCV_EXCP_ILLEGAL_INST:
> +            tval = env->bins;
> +            break;
>          default:
>              break;
>          }
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 24251bc8cc..921ca06bf9 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -167,6 +167,9 @@ 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);
>  }
>

Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>