Re: [PATCH v4 16/16] target/riscv: add trace-hooks for each case of sw-check exception

[Richard Henderson]

On 8/16/24 11:07, Deepak Gupta wrote:
> Violations to control flow rules setup by zicfilp and zicfiss lead to
> software check exceptions. To debug and fix such sw check issues in guest
> , add trace-hooks for each case.
>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>   target/riscv/helper.h                         |  3 +++
>   target/riscv/insn_trans/trans_rvi.c.inc       |  3 +++
>   target/riscv/insn_trans/trans_rvzicfiss.c.inc |  1 +
>   target/riscv/op_helper.c                      | 13 +++++++++++++
>   target/riscv/trace-events                     |  6 ++++++
>   target/riscv/translate.c                      |  2 ++
>   6 files changed, 28 insertions(+)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index e946ba61fd..6e90fbd225 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -123,6 +123,9 @@ DEF_HELPER_2(cbo_zero, void, env, tl)
>   
>   /* helper to raise sw check exception */
>   DEF_HELPER_2(raise_sw_check_excep, void, env, tl)
> +/* helper functions to trace riscv cfi violations */
> +DEF_HELPER_3(zicfilp_label_mismatch, void, env, tl, tl)
> +DEF_HELPER_3(zicfiss_ra_mismatch, void, env, tl, tl)
>   
>   /* Special functions */
>   DEF_HELPER_2(csrr, tl, env, int)
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 936b430282..7021f8d3da 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -54,6 +54,7 @@ static bool trans_lpad(DisasContext *ctx, arg_lpad *a)
>               /*
>                * misaligned, according to spec we should raise sw check 
> exception
>                */
> +            trace_zicfilp_unaligned_lpad_instr(ctx->base.pc_first);
>               gen_helper_raise_sw_check_excep(tcg_env,
>                   tcg_constant_tl(RISCV_EXCP_SW_CHECK_FCFI_TVAL));

Ah, no.

This performs the trace at translation time.
You want the trace at execution time.

    gen_update_pc(ctx, 0);
    gen_helper_zicfilp_unaligned_lpad(tcg_env);
    ctx->base.is_jmp = DISAS_NORETURN;


void HELPER(zicfilp_unaligned_lpad)(CPURISCVState *env)
{
    trace_zicfilp_unaligned_lpad(env->pc);
    env->sw_check_code = RISCV_EXCP_SW_CHECK_FCFI_TVAL;
    riscv_raise_exception(RISCV_EXCP_SW_CHECK, 0);
}

etc.

Nevermind the previous advice vs patch 5 saying you could inline everything; I had 
forgotten the desire for tracepoints.

You should probably add these helpers and tracepoints as you add the code.  Anything else 
is going to be a bit confusing.


r~