Re: [PATCH v6 05/16] target/riscv: tracking indirect branches (fcfi) for zicfilp

[Richard Henderson]

On 8/22/24 07:50, Deepak Gupta wrote:
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9f08a67a9e..3a56bea8b9 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -133,6 +133,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>           flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
>       }
>   
> +    if (cpu_get_fcfien(env)) {
> +        /*
> +         * For Forward CFI, only the expectation of a lpcll at

s/lpcll/lpad/, though perhaps the whole paragraph could be simplified.
We don't need to describe the mechanics of the extension, only what data defined by the 
spec are being copied into tb_flags.


> @@ -1270,6 +1275,21 @@ static void riscv_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cpu)
>       decode_opc(env, ctx, opcode16);
>       ctx->base.pc_next += ctx->cur_insn_len;
>   
> +    /*
> +     * instr decode of `lpad` should have lowered fcfi_lp_expected.
> +     * If not `lpad`, raise sw check exception.
> +     */

Perhaps better as

  If 'fcfi_lp_expected' is still true after processing the instruction,
  then we did not see an 'lpad' instruction, and must raise an exception.
  Insert code to raise the exception at the start of the insn; any other
  code the insn may have emitted will be deleted as dead code following
  the noreturn exception.


r~