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

[Daniel Henrique Barboza]

On 1/14/25 12:25 PM, Andrew Jones wrote:
> On Tue, Jan 14, 2025 at 10:20:12AM -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_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     | 43 ++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 44 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 e10ecc4ece..14af141349 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -2419,10 +2419,45 @@ 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 */
> > +        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),
> > +
> > +        /* Named features: Sha, ssu64xl, ssnpm */
> > +        CPU_CFG_OFFSET(ext_sha),
> > +        CPU_CFG_OFFSET(ext_ssnpm),
>
> Why are we calling ssnpm a named feature? And same question for supm in
> the last patch.

They're both defined in RVA23 as follows:

- Supm Pointer masking, with the execution environment providing a means to
select PMLEN=0 and PMLEN=7 at minimum.

- Ssnpm Pointer masking, with senvcfg.PMM and henvcfg.PMM supporting, at 
minimum,
settings PMLEN=0 and PMLEN=7.


This is consistent to what we usually call a named feature, i.e. a special name
for a particular setting for an existing extension.

Granted, both are working as actual extensions in QEMU, so I guess we can treat
them as such regardless of what the spec says.


Thanks,

Daniel


> Thanks,
> drew
>
> > +
> > +        RISCV_PROFILE_EXT_LIST_END
> > +    }
> > +};
> > +
> >   RISCVCPUProfile *riscv_profiles[] = {
> >       &RVA22U64,
> >       &RVA22S64,
> >       &RVA23U64,
> > +    &RVA23S64,
> >       NULL,
> >   };
> >   
> > @@ -2916,6 +2951,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)
> > @@ -3196,6 +3238,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