Re: [Qemu-devel] [PATCH v4 03/22] RISC-V CPU Core Definition

[Michael Clark]

On Tue, Feb 6, 2018 at 4:04 AM, Igor Mammedov <address@hidden> wrote:

> On Mon,  5 Feb 2018 19:22:28 +1300
> Michael Clark <address@hidden> wrote:
>
> > Add CPU state header, CPU definitions and initialization routines
> >
> > Signed-off-by: Michael Clark <address@hidden>
> > ---
> >  target/riscv/cpu.c      | 385 ++++++++++++++++++++++++++++++
> ++++++++++++++
> >  target/riscv/cpu.h      | 256 +++++++++++++++++++++++++++++
> >  target/riscv/cpu_bits.h | 417 ++++++++++++++++++++++++++++++
> ++++++++++++++++++
> >  3 files changed, 1058 insertions(+)
> >  create mode 100644 target/riscv/cpu.c
> >  create mode 100644 target/riscv/cpu.h
> >  create mode 100644 target/riscv/cpu_bits.h
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > new file mode 100644
> > index 0000000..684b78b
> > --- /dev/null
> > +++ b/target/riscv/cpu.c
> [...]
> > +
> > +static const RISCVCPUInfo riscv_cpus[] = {
> > +#ifdef CONFIG_USER_ONLY
> > +    { TYPE_RISCV_CPU_ANY,                riscv_any_cpu_init },
> > +#else
> > +    { TYPE_RISCV_CPU_IMAFDCSU_PRIV_1_09, riscv_imafdcsu_priv1_9_cpu_init
> },
> > +    { TYPE_RISCV_CPU_IMAFDCSU_PRIV_1_10, riscv_imafdcsu_priv1_10_cpu_init
> },
> > +    { TYPE_RISCV_CPU_IMACU_PRIV_1_10,    riscv_imacu_priv1_10_cpu_init
> },
> > +    { TYPE_RISCV_CPU_IMAC_PRIV_1_10,     riscv_imac_priv1_10_cpu_init },
> > +#endif
> > +    { NULL, NULL }
> > +};
> > +
> [...]
> > +static void cpu_register(const RISCVCPUInfo *info)
> > +{
> > +    TypeInfo type_info = {
> > +        .name = info->name,
> > +        .parent = TYPE_RISCV_CPU,
> > +        .instance_size = sizeof(RISCVCPU),
> > +        .instance_init = info->initfn,
> > +    };
> > +
> > +    type_register(&type_info);
> > +}
> > +
> > +static const TypeInfo riscv_cpu_type_info = {
> > +    .name = TYPE_RISCV_CPU,
> > +    .parent = TYPE_CPU,
> > +    .instance_size = sizeof(RISCVCPU),
> > +    .instance_init = riscv_cpu_init,
> > +    .abstract = false,
> > +    .class_size = sizeof(RISCVCPUClass),
> > +    .class_init = riscv_cpu_class_init,
> > +};
> [...]
>
> > +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)
> For simplistic type definitions like that,
> above parts should use DEFINE_TYPES(), see c6678108 for reference.
>
>
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > new file mode 100644
> > index 0000000..8b816ae
> > --- /dev/null
> > +++ b/target/riscv/cpu.h
> [...]
> > +#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)
> it still uses prefix notation versus commonly used suffix in form of
>  "targetFOO-cpu"
> this prefix approach would get in the way if we try to generalize
> naming <-> type conversion later[*].
> So it would better to be consistent with approach qemu uses for cpu types
> (I believe power had prefix based pnv types but it has been fixed
> to common suffix based pattern later).
>
> * discussion on thread "[PATCH v5 0/6]  Add a valid_cpu_types property"
>

I can reverse them if needed, just it seems a little odd to have riscv on
the right-hand side of the extensions. I can do this in the v5 spin...

It may make more sense once we have actual CPU models. Currently, we have
sets of extensions and privilege ISA versions. I guess these will become
implicit properties of a specific CPU model and the extensions will be
visible in the initialization function.