Re: [PATCH v2 4/4] target/riscv: add RVA23S64 profile

[Daniel Henrique Barboza]

On 1/15/25 5:24 AM, Andrew Jones wrote:
> On Tue, Jan 14, 2025 at 04:00:01PM -0300, Daniel Henrique Barboza wrote:
> > Add RVA23S64 as described in [1]. This profile inherits all mandatory
> > extensions of RVA23U64, making it a child of the U64 profile.
> >
> > A new "rva23s64" profile CPU is also added. This is the generated
> > riscv,isa for it (taken via -M dumpdtb):
> >
> > rv64imafdcbvh_zic64b_zicbom_zicbop_zicboz_ziccamoa_ziccif_zicclsm_
> > ziccrse_zicond_zicntr_zicsr_zifencei_zihintntl_zihintpause_zihpm_zimop_
> > zmmul_za64rs_zaamo_zalrsc_zawrs_zfa_zfhmin_zca_zcb_zcd_zcmop_zba_zbb_zbs_
> > zkt_zvbb_zve32f_zve32x_zve64f_zve64d_zve64x_zvfhmin_zvkb_zvkt_shcounterenw_
> > sha_shgatpa_shtvala_shvsatpa_shvstvala_shvstvecd_smnpm_smstateen_ssccptr_
> > sscofpmf_sscounterenw_ssnpm_ssstateen_sstc_sstvala_sstvecd_ssu64xl_
> > supm_svade_svinval_svnapot_svpbmt
> >
> > [1] https://github.com/riscv/riscv-profiles/blob/main/src/rva23-profile.adoc
> >
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> >   target/riscv/cpu-qom.h |  1 +
> >   target/riscv/cpu.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 43 insertions(+)
> >
> > diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
> > index 53ead481a9..4cfdb74891 100644
> > --- a/target/riscv/cpu-qom.h
> > +++ b/target/riscv/cpu-qom.h
> > @@ -41,6 +41,7 @@
> >   #define TYPE_RISCV_CPU_RVA22U64         RISCV_CPU_TYPE_NAME("rva22u64")
> >   #define TYPE_RISCV_CPU_RVA22S64         RISCV_CPU_TYPE_NAME("rva22s64")
> >   #define TYPE_RISCV_CPU_RVA23U64         RISCV_CPU_TYPE_NAME("rva23u64")
> > +#define TYPE_RISCV_CPU_RVA23S64         RISCV_CPU_TYPE_NAME("rva23s64")
> >   #define TYPE_RISCV_CPU_IBEX             RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> >   #define TYPE_RISCV_CPU_SHAKTI_C         RISCV_CPU_TYPE_NAME("shakti-c")
> >   #define TYPE_RISCV_CPU_SIFIVE_E31       RISCV_CPU_TYPE_NAME("sifive-e31")
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 371a7d63fa..9225459bf8 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -2418,10 +2418,44 @@ static RISCVCPUProfile RVA23U64 = {
> >       }
> >   };
> >   
> > +/*
> > + * As with RVA23U64, RVA23S64 also defines 'named features'.
> > + *
> > + * Cache related features that we consider enabled since we don't
> > + * implement cache: Ssccptr
> > + *
> > + * Other named features that we already implement: Sstvecd, Sstvala,
> > + * Sscounterenw, Ssu64xl
> > + *
> > + * The remaining features/extensions comes from RVA23U64.
> > + */
> > +static RISCVCPUProfile RVA23S64 = {
> > +    .parent = &RVA23U64,
> > +    .name = "rva23s64",
> > +    .misa_ext = RVS,
> > +    .priv_spec = PRIV_VERSION_1_13_0,
> > +    .satp_mode = VM_1_10_SV39,
> > +    .ext_offsets = {
> > +        /* These were present in RVA22S64 */
>
> I'd prefer we come up with a way to inherit rva22s64, as it'll
> make the fact that later profiles should expand earlier profiles
> more clear and also avoid issues like missing zifencei. I guess
> it should be as easy as renaming parent to uparent and adding an
> sparent, right?

I think it's worth a shot. I'll see what I can do in v3. Thanks,


Daniel


> Thanks,
> drew
>
> > +        CPU_CFG_OFFSET(ext_zifencei), CPU_CFG_OFFSET(ext_svade),
> > +        CPU_CFG_OFFSET(ext_svpbmt), CPU_CFG_OFFSET(ext_svinval),
> > +
> > +        /* New in RVA23S64 */
> > +        CPU_CFG_OFFSET(ext_svnapot), CPU_CFG_OFFSET(ext_sstc),
> > +        CPU_CFG_OFFSET(ext_sscofpmf), CPU_CFG_OFFSET(ext_ssnpm),
> > +
> > +        /* Named features: Sha */
> > +        CPU_CFG_OFFSET(ext_sha),
> > +
> > +        RISCV_PROFILE_EXT_LIST_END
> > +    }
> > +};
> > +
> >   RISCVCPUProfile *riscv_profiles[] = {
> >       &RVA22U64,
> >       &RVA22S64,
> >       &RVA23U64,
> > +    &RVA23S64,
> >       NULL,
> >   };
> >   
> > @@ -2915,6 +2949,13 @@ static void rva23u64_profile_cpu_init(Object *obj)
> >   
> >       RVA23U64.enabled = true;
> >   }
> > +
> > +static void rva23s64_profile_cpu_init(Object *obj)
> > +{
> > +    rv64i_bare_cpu_init(obj);
> > +
> > +    RVA23S64.enabled = true;
> > +}
> >   #endif
> >   
> >   static const gchar *riscv_gdb_arch_name(CPUState *cs)
> > @@ -3195,6 +3236,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> >       DEFINE_PROFILE_CPU(TYPE_RISCV_CPU_RVA22U64,  MXL_RV64,  
> > rva22u64_profile_cpu_init),
> >       DEFINE_PROFILE_CPU(TYPE_RISCV_CPU_RVA22S64,  MXL_RV64,  
> > rva22s64_profile_cpu_init),
> >       DEFINE_PROFILE_CPU(TYPE_RISCV_CPU_RVA23U64,  MXL_RV64,  
> > rva23u64_profile_cpu_init),
> > +    DEFINE_PROFILE_CPU(TYPE_RISCV_CPU_RVA23S64,  MXL_RV64,  
> > rva23s64_profile_cpu_init),
> >   #endif /* TARGET_RISCV64 */
> >   };
> >   
> > --
> > 2.47.1