Re: [PATCH 12/18] hw/arm/digic: Replace object_initialize(ARMCPU) by object_new(ARMCPU)

[Peter Maydell]

On Wed, 25 Jan 2023 at 12:14, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > I'm not really a fan of this, because it moves away
> > from a standard coding pattern we've been using for
> > new QOM 'container' devices, where all the sub-components
> > of the device are structs embedded in the device's own
> > struct. This is as opposed to the old style which tended
> > to use "allocate memory for the sub-component and have
> > pointers to it". It means the CPU object is now being
> > treated differently from all the timers, UARTs, etc.
>
> I think you can certainly make the argument that CPU's have always been
> treated separately because we pass it around as an anonymous pointer all
> the time. We currently can't support two concrete CPU types in the same
> structure. For example zyncmp has:
>
>   struct XlnxZynqMPState {
>       /*< private >*/
>       DeviceState parent_obj;
>
>       /*< public >*/
>       CPUClusterState apu_cluster;
>       CPUClusterState rpu_cluster;
>       ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
>       ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
>
> which only works because A/R cpus are the same underlying type. However
> when we want to add Microblaze how would we do it?

You'd add fields
    MicroBlazeCPU other_cpu;

As you note, at the moment that doesn't work because cpu.h
is magic and embeds an assumption that it's only included
in compile-per-target objects and therefore any given source
file only includes one of them at once. I think we should be
aiming to remove those assumptions, not work around them.

thanks
-- PMM