Re: [Qemu-devel] [PATCH v2 03/21] RISC-V CPU Core Definition

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 24 Jan 2018 10:21:01 -0800
Michael Clark <mjc@sifive.com> wrote:

> On Mon, Jan 15, 2018 at 5:44 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Wed, 10 Jan 2018 15:46:22 -0800
> > Michael Clark <mjc@sifive.com> wrote:
> >  
> > > Add CPU state header, CPU definitions and initialization routines
> > >
> > > Signed-off-by: Michael Clark <mjc@sifive.com>
> > > ---
> > >  target/riscv/cpu.c      | 391 ++++++++++++++++++++++++++++++  
> > +++++++++++++++  
> > >  target/riscv/cpu.h      | 271 +++++++++++++++++++++++++++++++
> > >  target/riscv/cpu_bits.h | 417 ++++++++++++++++++++++++++++++  
> > ++++++++++++++++++  
> > >  3 files changed, 1079 insertions(+)
> > >  create mode 100644 target/riscv/cpu.c
> > >  create mode 100644 target/riscv/cpu.h
> > >  create mode 100644 target/riscv/cpu_bits.h
> > >
[...]

> > > +
> > > +void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> > > +{
> > > +    const RISCVCPUInfo *info = riscv_cpus;
> > > +
> > > +    while (info->name) {
> > > +        (*cpu_fprintf)(f, "%s\n", info->name);
> > > +        info++;
> > > +    }
> > > +}
> > > +
> > > +static void riscv_cpu_register_types(void)
> > > +{
> > > +    const RISCVCPUInfo *info = riscv_cpus;
> > > +
> > > +    type_register_static(&riscv_cpu_type_info);
> > > +
> > > +    while (info->name) {
> > > +        cpu_register(info);
> > > +        info++;
> > > +    }
> > > +}
> > > +
> > > +type_init(riscv_cpu_register_types)
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > new file mode 100644
> > > index 0000000..9dd131b
> > > --- /dev/null
> > > +++ b/target/riscv/cpu.h
> > > @@ -0,0 +1,271 @@
> > > +#ifndef RISCV_CPU_H
> > > +#define RISCV_CPU_H
> > > +
> > > +/* QEMU addressing/paging config */
> > > +#define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
> > > +#if defined(TARGET_RISCV64)
> > > +#define TARGET_LONG_BITS 64
> > > +#define TARGET_PHYS_ADDR_SPACE_BITS 50
> > > +#define TARGET_VIRT_ADDR_SPACE_BITS 39
> > > +#elif defined(TARGET_RISCV32)
> > > +#define TARGET_LONG_BITS 32
> > > +#define TARGET_PHYS_ADDR_SPACE_BITS 34
> > > +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> > > +#endif
> > > +
> > > +#define ELF_MACHINE EM_RISCV
> > > +#define CPUArchState struct CPURISCVState
> > > +
> > > +#include "qemu-common.h"
> > > +#include "qom/cpu.h"
> > > +#include "exec/cpu-defs.h"
> > > +#include "fpu/softfloat.h"
> > > +
> > > +/* #define DEBUG_OP */
> > > +/* #define RISCV_DEBUG_PRINT */
> > > +
> > > +#define TYPE_RISCV_CPU                    "riscv"
> > > +#define TYPE_RISCV_CPU_ANY                "riscv-any"
> > > +#define TYPE_RISCV_CPU_IMAFDCSU_PRIV_1_09 "riscv-imafdcsu-priv1.9"
> > > +#define TYPE_RISCV_CPU_IMAFDCSU_PRIV_1_10 "riscv-imafdcsu-priv1.10"
> > > +#define TYPE_RISCV_CPU_IMACU_PRIV_1_10    "riscv-imacu-priv1.10"
> > > +#define TYPE_RISCV_CPU_IMAC_PRIV_1_10     "riscv-imac-priv1.10"
> > > +
> > > +#define RISCV_CPU_TYPE_PREFIX TYPE_RISCV_CPU "-"
> > > +#define RISCV_CPU_TYPE_NAME(name) (RISCV_CPU_TYPE_PREFIX name)  
> > typically we use suffix convention here, for base type it would be
> >
> >  #define TYPE_RISCV_CPU "riscv-cpu"
> >
> > then for leaf types it would be
> >
> > #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> > #define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX)  
> 
> 
> The choice to invert it for RISC-V was somewhat deliberate.
> 
> In RISC-V land we usually use rv, riscv and the xlen as a prefix to the ISA
> extensions, and spec version, so it made more sense to have riscv on the
> left.
> 
> e.g. rv64imafdc
> 
> At the moment we only have generic CPUs formed using the ISA extensions.
> 
> "imac-priv1.10-riscv-cpu" makes much less sense than "riscv-imac-priv1.10"
> 
> It may make more sense when we have vendor CPU models and their properties
> e.g. extensions and spec versions become intrinsic to the specific CPU
> model.
> 
> I can backwardize it, but the naming in a cpu list will make a little less
> sense.
I don't insist on it but being consistent with how all
other targets name their cpu types has its own benefits,
for example known grep-able suffixes and potential to unify
FOO_cpu_list() handling which currently is a mess.
So if name is not dictated by spec I'd use QEMU's naming convention.

[...]