Re: [PATCH v2 3/4] target/riscv: add 'static' attribute of query-cpu-definitions

[Daniel Henrique Barboza]

On 4/10/23 23:03, Richard Henderson wrote:
> On 4/10/23 09:52, Daniel Henrique Barboza wrote:
> > 'static' is defined in the QMP doc as:
> >
> > "whether a CPU definition is static and will not change depending on
> > QEMU version, machine type, machine options and accelerator options. A
> > static model is always migration-safe."
> >
> > For RISC-V we'll consider all named CPUs as static since their
> > extensions can't be changed by user input. Generic CPUs will be
> > considered non-static.
> >
> > We aren't ready to make the change for generic CPUs yet because we're
> > using the same class init for every CPU. We'll deal with it next.
> >
> > Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> > ---
> >   target/riscv/cpu-qom.h        | 3 +++
> >   target/riscv/cpu.c            | 6 ++++++
> >   target/riscv/riscv-qmp-cmds.c | 2 ++
> >   3 files changed, 11 insertions(+)
>
> Is 'static = true' really what you want as default?
> Perhaps 'dynamic = false' (considering zero initialization) would be better?
> Do you want an attribute that can be changed at all?

I don't think there's a precedence in QEMU of a CPU that starts as static and 
then
becomes non-static during runtime. If the CPU has the capability of changing
its attributes/extensions during runtime then I'd call it non-static all
the time.


> You could plausibly implement this via class inheritance instead.
> E.g.
>
> static const TypeInfo dynamic_cpu_type_info = {
>      .name = TYPE_RISCV_DYN_CPU,
>      .parent = TYPE_RISCV_CPU,
>      .abstract = true,
>      ...
> };
>
> and then the dynamic cpus inherit from that.  Your dynamic attribute becomes 
> object_dynamic_cast(OBJECT(cpu), TYPE_RISCV_DYN_CPU) != NULL.

Sounds good. We'll avoid adding an extra flag in RISCVCPUClass too.


Thanks,


Daniel


> r~