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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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: