Re: [PATCH v11 14/20] target/riscv: AMO operations always raise store/AMO fault

[Alistair Francis <alistair23@gmail.com>]

On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> This patch adds one more word for tcg compile which can be obtained during
> unwind time to determine fault type for original operation (example AMO).
> Depending on that, fault can be promoted to store/AMO fault.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/cpu.h         |  9 ++++++++-
>  target/riscv/cpu_helper.c  | 20 ++++++++++++++++++++
>  target/riscv/tcg/tcg-cpu.c |  1 +
>  target/riscv/translate.c   |  2 +-
>  4 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e758f4497e..0a13604e37 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -46,8 +46,13 @@ typedef struct CPUArchState CPURISCVState;
>  /*
>   * RISC-V-specific extra insn start words:
>   * 1: Original instruction opcode
> + * 2: more information about instruction
>   */
> -#define TARGET_INSN_START_EXTRA_WORDS 1
> +#define TARGET_INSN_START_EXTRA_WORDS 2
> +/*
> + * b0: Whether a instruction always raise a store AMO or not.
> + */
> +#define RISCV_UW2_ALWAYS_STORE_AMO 1
>
>  #define RV(x) ((target_ulong)1 << (x - 'A'))
>
> @@ -226,6 +231,8 @@ struct CPUArchState {
>      bool      elp;
>      /* shadow stack register for zicfiss extension */
>      target_ulong ssp;
> +    /* env place holder for extra word 2 during unwind */
> +    target_ulong excp_uw2;
>      /* sw check code for sw check exception */
>      target_ulong sw_check_code;
>  #ifdef CONFIG_USER_ONLY
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 39544cade6..8294279b01 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1741,6 +1741,22 @@ static target_ulong riscv_transformed_insn(CPURISCVState *env,
>      return xinsn;
>  }
>
> +static target_ulong promote_load_fault(target_ulong orig_cause)
> +{
> +    switch (orig_cause) {
> +    case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
> +        return RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
> +
> +    case RISCV_EXCP_LOAD_ACCESS_FAULT:
> +        return RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> +
> +    case RISCV_EXCP_LOAD_PAGE_FAULT:
> +        return RISCV_EXCP_STORE_PAGE_FAULT;
> +    }
> +
> +    /* if no promotion, return original cause */
> +    return orig_cause;
> +}
>  /*
>   * Handle Traps
>   *
> @@ -1752,6 +1768,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
>      bool write_gva = false;
> +    bool always_storeamo = (env->excp_uw2 & RISCV_UW2_ALWAYS_STORE_AMO);
>      uint64_t s;
>
>      /*
> @@ -1785,6 +1802,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          case RISCV_EXCP_STORE_AMO_ACCESS_FAULT:
>          case RISCV_EXCP_LOAD_PAGE_FAULT:
>          case RISCV_EXCP_STORE_PAGE_FAULT:
> +            if (always_storeamo) {
> +                cause = promote_load_fault(cause);
> +            }
>              write_gva = env->two_stage_lookup;
>              tval = env->badaddr;
>              if (env->two_stage_indirect_lookup) {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 4da26cb926..83771303a8 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -129,6 +129,7 @@ static void riscv_restore_state_to_opc(CPUState *cs,
>          env->pc = pc;
>      }
>      env->bins = data[1];
> +    env->excp_uw2 = data[2];
>  }
>
>  static const TCGCPUOps riscv_tcg_ops = {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index b1d251e893..16fff70dac 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1265,7 +1265,7 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>          pc_next &= ~TARGET_PAGE_MASK;
>      }
>
> -    tcg_gen_insn_start(pc_next, 0);
> +    tcg_gen_insn_start(pc_next, 0, 0);
>      ctx->insn_start_updated = false;
>  }
>
> --
> 2.44.0
>
>