On Thu, 8 Feb 2018 15:19:13 +1300
Michael Clark <m...@sifive.com> wrote:
> On Wed, Feb 7, 2018 at 4:03 AM, Igor Mammedov <imamm...@redhat.com> wrote:
>
> > On Tue, 6 Feb 2018 11:09:56 +1300
> > Michael Clark <m...@sifive.com> wrote:
> >
> > > On Tue, Feb 6, 2018 at 4:04 AM, Igor Mammedov <imamm...@redhat.com>
> > wrote:
> > >
> > > > On Mon, 5 Feb 2018 19:22:28 +1300
> > > > Michael Clark <m...@sifive.com> wrote:
> > > >
> > > > > Add CPU state header, CPU definitions and initialization routines
> > > > >
> > > > > Signed-off-by: Michael Clark <m...@sifive.com>
> > > > > ---
> > > > > 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"
Also you can use RISCV_CPU_TYPE_NAME() from blow to form above names,
like:
#define TYPE_RISCV_CPU "riscv-cpu"
#define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
...
then whatever naming format is required, you'd be able to
change it just in RISCV_CPU_TYPE_NAME() without touching
the rest.
> > > > > +
> > > > > +#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...
> > pls, do so + -cpu suffix.
> >
> > > 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.
> > That's what we have in x86 land, code names + _suffix corresponding to
> > real cpus (mostly) with implicit feature set per each.
> > Features could be properties so user would be able to enumerate/query/set
> > them.
> >
>
> I just realized I neglected the change in the current patch-set to use the
> suffix scheme i.e. "<extension>-<version>-riscv-cpu" versus the
> "riscv-<extension>-<version>" scheme that we have presently.
Yep, just saw v5 still has old variant,
You also forgot about comment:
"
> +type_init(riscv_cpu_register_types)
For simplistic type definitions like that,
above parts should use DEFINE_TYPES(), see c6678108 for reference.
"
> I would like to discuss with RISC-V folk as how we should name these
> "generic" CPUs and how we should handle extensions and versioning. This is
> certainly going to be an area that is going to be worked on in the future
> so whether we evolve this in-tree or out-of-tree is up for discussion. We
> also have to decide whether we will add CPU models for all typical
> combinations of extensions or if this is handled some other way. There will
> also be actual CPU modules now silicon is becoming more widely available.
> The version number we have currently is essentially the supervisor
> specification version. That won't be necessary once we have real cpu models.
>
> There is also a future patch that will allow 'misa' extensions to be
> changed on the fly, which will require the extensions to be exposed in
> cpu_get_tb_cpu_state flags.
>
> One thing is for certain, is that there is no direct comparison to x86 land
> as the RISC-V extension scheme is presently quite different.
See, following thread which might be of interest to you:
"how to handle QOM 'container' objects whose contents depend on QOM
properties?"
