Re: [PATCH v1 RFC Zisslpcfi 4/9] target/riscv: helper functions for forward and backward cfi

[Deepak Gupta]

On Tue, Feb 14, 2023 at 10:26 PM LIU Zhiwei
<zhiwei_liu@linux.alibaba.com> wrote:
>
>
> On 2023/2/9 14:23, Deepak Gupta wrote:
> > Implementation for forward cfi and backward cfi needs helper function
> > to determine if currently fcfi and bcfi are enabled. Enable depends on
> > privilege mode and settings in sstatus/menvcfg/henvcfg/mseccfg CSRs.
> >
> > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > Signed-off-by: Kip Walker  <kip@rivosinc.com>
> > ---
> >   target/riscv/cpu.h        |  2 ++
> >   target/riscv/cpu_helper.c | 51 +++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 53 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 18db61a06a..d14ea4f91d 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -568,6 +568,8 @@ bool riscv_cpu_virt_enabled(CPURISCVState *env);
> >   void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
> >   bool riscv_cpu_two_stage_lookup(int mmu_idx);
> >   int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
> > +bool cpu_get_fcfien(CPURISCVState *env);
> > +bool cpu_get_bcfien(CPURISCVState *env);
> >   hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> >   G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> >                                                  MMUAccessType access_type, 
> > int mmu_idx,
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 9a28816521..a397023840 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -30,6 +30,7 @@
> >   #include "sysemu/cpu-timers.h"
> >   #include "cpu_bits.h"
> >   #include "debug.h"
> > +#include "pmp.h"
> >
> >   int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> >   {
> > @@ -40,6 +41,56 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> >   #endif
> >   }
> >
> > +bool cpu_get_fcfien(CPURISCVState *env)
> > +{
> > +#ifdef CONFIG_USER_ONLY
> > +    return false;
> > +#else
> > +    /* no cfi extension, return false */
> > +    if (!env_archcpu(env)->cfg.ext_cfi) {
> > +        return false;
> > +    }
> > +
> > +    switch (env->priv) {
> > +    case PRV_U:
> > +        return (env->mstatus & MSTATUS_UFCFIEN) ? true : false;
>
> It's not right. We should also check for menvcfg.cfie. The same to other
> checks in S mode or U mode.
>

Yes you're right. I lost that check in one of the re-factoring.
Thanks. Will fix it.

> Zhiwei
>
> > +    case PRV_S:
> > +        return (env->menvcfg & MENVCFG_SFCFIEN) ? true : false;
> > +    case PRV_M:
> > +        return (env->mseccfg & MSECCFG_MFCFIEN) ? true : false;
> > +    default:
> > +        g_assert_not_reached();
> > +    }
> > +#endif
> > +}
> > +
> > +bool cpu_get_bcfien(CPURISCVState *env)
> > +{
> > +#ifdef CONFIG_USER_ONLY
> > +    return false;
> > +#else
> > +    /* no cfi extension, return false */
> > +    if (!env_archcpu(env)->cfg.ext_cfi) {
> > +        return false;
> > +    }
> > +
> > +    switch (env->priv) {
> > +    case PRV_U:
> > +        return (env->mstatus & MSTATUS_UBCFIEN) ? true : false;
> > +
> > +        /*
> > +         * no gating for back cfi in M/S mode. back cfi is always on for
> > +         * M/S mode
> > +         */
> > +    case PRV_S:
> > +    case PRV_M:
> > +        return true;
> > +    default:
> > +        g_assert_not_reached();
> > +    }
> > +#endif
> > +}
> > +
> >   void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> >                             target_ulong *cs_base, uint32_t *pflags)
> >   {