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

[Mark Cave-Ayland]

On 08/02/2024 18:12, 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);
> +    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)

I see that Zoltan has already commented about checking the success of qdev_realise() 
before wiring the sysbus IRQs, so with that fixed:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.