Re: [PULL 04/49] hw: Add QOM parentship relation with CPUs

[Philippe Mathieu-Daudé]

On 13/1/25 13:28, Igor Mammedov wrote:
> On Sun, 12 Jan 2025 23:16:40 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> > QDev objects created with object_new() need to manually add
> > their parent relationship with object_property_add_child().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > Message-Id: <20240216110313.17039-22-philmd@linaro.org>
> > ---
> >   hw/i386/x86-common.c                     | 1 +
> >   hw/microblaze/petalogix_ml605_mmu.c      | 1 +
> >   hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
> >   hw/mips/cps.c                            | 1 +
> >   hw/ppc/e500.c                            | 1 +
> >   hw/ppc/spapr.c                           | 1 +
> >   6 files changed, 6 insertions(+)
> >
> > diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> > index 97b4f7d4a0d..9c9ffb3484a 100644
> > --- a/hw/i386/x86-common.c
> > +++ b/hw/i386/x86-common.c
> > @@ -60,6 +60,7 @@ static void x86_cpu_new(X86MachineState *x86ms, int64_t 
> > apic_id, Error **errp)
> >       if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
> >           goto out;
> >       }
> > +    object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
>
> I might  be missing something but why it needs to be done manually?
>
> device_set_realized() will place any parent-less device under (1) 
> /machine/unattached

This is exactly what we want to avoid, to eventually remove
the "/machine/unattached" container for good.

See "= Problem 4: The /machine/unattached/ orphanage =" in:
https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/

> while devices created with device_add() are be placed under 
> /machine/peripheral[-anon]
>
> The commit message unfortunately doesn't explain why [1] shall be replaced
> by direct cpu[*] array property directly under machine.

Right. I'll drop for now and respin once reworded.

> Granted, those paths aren't any kind of ABI and wrt x86 cpus
> nothing should break (or I'd say it shouldn't break our promises)
> But I'd rather not do this without a good reason/explanation.
>
> >       qdev_realize(DEVICE(cpu), NULL, errp);
> >   
> >   out: