diff mbox series

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

Message ID 20211220064916.107241-4-alistair.francis@opensource.wdc.com
State New
Headers show
Series RISC-V: Populate mtval and stval | expand

Commit Message

Alistair Francis Dec. 20, 2021, 6:49 a.m. UTC 
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(+)

Comments

Richard Henderson Dec. 20, 2021, 7:39 p.m. UTC | #1 
On 12/19/21 10:49 PM, Alistair Francis 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(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Bin Meng Dec. 21, 2021, 7:46 a.m. UTC | #2 
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>
diff mbox series

Patch

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;
+
     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);
 }