Re: [PATCH v3 06/11] hw/misc/macio: Realize IDE controller before accessing it

[BALATON Zoltan]

On Thu, 8 Feb 2024, Philippe Mathieu-Daudé wrote:
> We should not wire IRQs on unrealized device.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/misc/macio/macio.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index c9f22f8515..db662a2065 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -122,15 +122,17 @@ static bool macio_realize_ide(MacIOState *s, 
> MACIOIDEState *ide,
>                               Error **errp)
> {
>     SysBusDevice *sbd = SYS_BUS_DEVICE(ide);
> +    bool success;
>
> -    sysbus_connect_irq(sbd, 0, irq0);
> -    sysbus_connect_irq(sbd, 1, irq1);
>     qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
>     object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma),
>                              &error_abort);
>     macio_ide_register_dma(ide);
> +    success = qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);

If realize is unsuccessful can you connect irqs if device may be 
unrealized? So maybe either the next two lines should be in an if block or 
drop the success flag and return false here if (!qdev_realize) and true at 
end of func?

Regards,
BALATON Zoltan

> +    sysbus_connect_irq(sbd, 0, irq0);
> +    sysbus_connect_irq(sbd, 1, irq1);
>
> -    return qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
> +    return success;
> }
>
> static void macio_oldworld_realize(PCIDevice *d, Error **errp)