Re: [PATCH v3 03/18] hw/i386/pc_piix: Wire PIIX3 IDE ouput IRQs to ISA bus IRQs 14/15

[Mark Cave-Ayland]

On 27/04/2023 08:58, Bernhard Beschow wrote:

> Am 26. April 2023 12:50:08 UTC schrieb Mark Cave-Ayland 
> <mark.cave-ayland@ilande.co.uk>:
> > On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:
> >
> > > Since pc_init1() has access to the ISABus*, retrieve the
> > > ISA IRQs with isa_bus_get_irq().
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >    hw/i386/pc_piix.c | 8 +++++++-
> > >    1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index 126b6c11df..1e90b9ff0d 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -277,7 +277,13 @@ static void pc_init1(MachineState *machine,
> > >        if (pcmc->pci_enabled) {
> > >            PCIDevice *dev;
> > >    -        dev = pci_create_simple(pci_bus, piix3_devfn + 1, TYPE_PIIX3_IDE);
> > > +        dev = pci_new_multifunction(piix3_devfn + 1, false, TYPE_PIIX3_IDE);
> > > +        qdev_connect_gpio_out_named(DEVICE(dev), "ide-irq", 0,
> > > +                                    isa_bus_get_irq(isa_bus, 14));
> > > +        qdev_connect_gpio_out_named(DEVICE(dev), "ide-irq", 1,
> > > +                                    isa_bus_get_irq(isa_bus, 15));
> > > +        pci_realize_and_unref(dev, pci_bus, &error_fatal);
> > > +
> > >            pci_ide_create_devs(dev);
> > >            idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
> > >            idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> >
> > Another reason this probably isn't a good idea: you're having to call 
> > qdev_connect_gpio_*() before realizing the device :(
>
> Just curious: Resources like memory regions are assigned before realization, 
> see e.g. i440fx or q35. Interrupts are also resources. What makes them special?

I think I've covered the .instance_init() vs. realize() part in my reply to Zoltan, 
but in terms of qdev_connect_gpio_out_named() the reasoning here is that a device 
shouldn't change it's internal behaviour depending upon how it is wired. In other 
words a standalone device will behave exactly the same as a device connected into a 
machine.

> I'm asking since this issue seems to be the main blocker for the piix 
> consolidation to be merged.

I did have a brief catch-up with Phil at the start of the week, and he's tagged your 
series for review but he is really busy at the moment. As before I repeat my offer to 
help out if needed as I think it's a good series, but for now we're waiting for Phil 
to let us know what the next steps are...


ATB,

Mark.