Re: [PATCH 3/5] hw/arm/imx8mp-evk: Fix reference count of SoC object

Am 30. April 2026 15:39:48 UTC schrieb Peter Maydell <[email protected]>:
>On Sun, 8 Mar 2026 at 20:35, Bernhard Beschow <[email protected]> wrote:
>>
>> object_new() leaves a reference count of one, and object_property_add_child()
>> then leaves it at two. This would leak the SoC object if the machine object was
>> destroyed since the machine is the SoC's sole owner. Fix this by converting to
>> object_new_with_props() which gets the parenting and reference count right in
>> one go.
>>
>> Fixes: a4eefc69b237 ("hw/arm: Add i.MX 8M Plus EVK board")
>> cc: Gaurav Sharma <[email protected]>
>> Signed-off-by: Bernhard Beschow <[email protected]>
>> ---
>>  hw/arm/imx8mp-evk.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/imx8mp-evk.c b/hw/arm/imx8mp-evk.c
>> index b84ac91a17..cfc3038c36 100644
>> --- a/hw/arm/imx8mp-evk.c
>> +++ b/hw/arm/imx8mp-evk.c
>> @@ -74,8 +74,8 @@ static void imx8mp_evk_init(MachineState *machine)
>>          .modify_dtb = imx8mp_evk_modify_dtb,
>>      };
>>
>> -    s = FSL_IMX8MP(object_new(TYPE_FSL_IMX8MP));
>> -    object_property_add_child(OBJECT(machine), "soc", OBJECT(s));
>> +    s = FSL_IMX8MP(object_new_with_props(TYPE_FSL_IMX8MP, OBJECT(machine),
>> +                                         "soc", &error_fatal, NULL));
>>      object_property_set_uint(OBJECT(s), "fec1-phy-num", 1, &error_fatal);
>>      sysbus_realize_and_unref(SYS_BUS_DEVICE(s), &error_fatal);
>
>Looking again at this, is this really right?
>object_property_add_child(...) creates the object (refcount 1),
>adds it as a child of the specified parent (refcount 2), then
>does object_unref() (refcount 1). Then in this code we call
>sysbus_realize_and_unref(), which calls qdev_realize_and_unref().
>That function:
> * (indirectly) calls bus_add_child(), which takes a reference and
>   hands it to the link property on the parent (refcount 2)
> * calls object_unref(), as its name suggests (refcount 1)
>
>So now we have the object with refcount 1, so it doesn't just
>vanish, but on the other hand both the QOM tree parent and the
>qbus tree parent think they own a reference to the object,
>so our refcount is wrong.
>
>I think the existing code here was correct: we get a reference
>because we call object_new(), and then we drop it by calling
>the realize_and_unref function.
>
>I'm going to drop this patch from target-arm.next.

Yeah, makes sense. Thanks foe the clarification.

We'll need to fix imx8mm-evk now since it uses the same broken pattern (I cc'd Gurav who incorporated this patch into his series). I'll send a fix.

Best regards,
Bernhard