Re: [PATCH v2 1/2] hw/arm/fsl-imx8mp: Derive struct FslImx8mpState from TYPE_SYS_BUS_DEVICE

[Bernhard Beschow <shentey@gmail.com>]

Am 18. März 2025 15:29:17 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Tue, 18 Mar 2025 at 15:27, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Fri, 14 Mar 2025 at 18:32, Bernhard Beschow <shentey@gmail.com> wrote:
>> >
>> > Deriving from TYPE_SYS_BUS_DEVICE fixes the SoC object to be reset upon machine
>> > reset. It also makes the SoC implementation not user-creatable which can trigger
>> > the following crash:
>> >
>> >   $ ./qemu-system-aarch64  -M virt -device fsl-imx8mp
>> >   **
>> >   ERROR:../../devel/qemu/tcg/tcg.c:1006:tcg_register_thread: assertion failed:
>> >   (n < tcg_max_ctxs)
>> >   Bail out! ERROR:../../devel/qemu/tcg/tcg.c:1006:tcg_register_thread:
>> >   assertion failed: (n < tcg_max_ctxs)
>> >   Aborted (core dumped)
>>
>> > diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
>> > index c3f6da6322..82edf61082 100644
>> > --- a/hw/arm/fsl-imx8mp.c
>> > +++ b/hw/arm/fsl-imx8mp.c
>> > @@ -702,7 +702,7 @@ static void fsl_imx8mp_class_init(ObjectClass *oc, void *data)
>> >  static const TypeInfo fsl_imx8mp_types[] = {
>> >      {
>> >          .name = TYPE_FSL_IMX8MP,
>> > -        .parent = TYPE_DEVICE,
>> > +        .parent = TYPE_SYS_BUS_DEVICE,
>> >          .instance_size = sizeof(FslImx8mpState),
>> >          .instance_init = fsl_imx8mp_init,
>> >          .class_init = fsl_imx8mp_class_init,
>> > diff --git a/hw/arm/imx8mp-evk.c b/hw/arm/imx8mp-evk.c
>> > index e1a7892fd7..f17d5db466 100644
>> > --- a/hw/arm/imx8mp-evk.c
>> > +++ b/hw/arm/imx8mp-evk.c
>> > @@ -37,7 +37,7 @@ static void imx8mp_evk_init(MachineState *machine)
>> >      s = FSL_IMX8MP(object_new(TYPE_FSL_IMX8MP));
>> >      object_property_add_child(OBJECT(machine), "soc", OBJECT(s));
>> >      object_property_set_uint(OBJECT(s), "fec1-phy-num", 1, &error_fatal);
>> > -    qdev_realize(DEVICE(s), NULL, &error_fatal);
>> > +    sysbus_realize_and_unref(SYS_BUS_DEVICE(s), &error_fatal);
>>
>> You want sysbus_realize() here, not the _and_unref() variant,
>> because the device was created with object_initialize_child().
>
>No, that's wrong, we create it with object_new(). So
>the _and_unref() *is* correct, but this is a separate bug fix
>from the "should be sysbus, not qdev" bug this patch says
>it is fixing. Can it be in a separate patch, please?

Sure, fixed in v3.

Best regards,
Bernhard

>
>thanks
>-- PMM