Re: [PATCH v3 01/18] hw/ide/piix: Expose output IRQ as properties for late object population

[Mark Cave-Ayland]

On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ide/piix.c         | 14 ++++++++++++--
>   include/hw/ide/piix.h |  4 ++++
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 41d60921e3..a36dac8469 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -121,6 +121,13 @@ static void piix_ide_reset(DeviceState *dev)
>       pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
>   }
>   
> +static void piix_ide_initfn(Object *obj)
> +{
> +    PCIIDEState *dev = PCI_IDE(obj);
> +
> +    qdev_init_gpio_out_named(DEVICE(obj), dev->isa_irq, "ide-irq", 2);
> +}
> +
>   static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
>   {
>       static const struct {
> @@ -133,6 +140,7 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, 
> Error **errp)
>       };
>       int ret;
>   
> +    qemu_irq irq_out = d->isa_irq[i] ? : isa_get_irq(NULL, port_info[i].isairq);
>       ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>       ret = ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
>                             port_info[i].iobase2);
> @@ -141,7 +149,7 @@ static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, 
> Error **errp)
>                            object_get_typename(OBJECT(d)), i);
>           return false;
>       }
> -    ide_bus_init_output_irq(&d->bus[i], isa_get_irq(NULL, 
> port_info[i].isairq));
> +    ide_bus_init_output_irq(&d->bus[i], irq_out);
>   
>       bmdma_init(&d->bus[i], &d->bmdma[i], d);
>       d->bmdma[i].bus = &d->bus[i];
> @@ -162,7 +170,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
> **errp)
>   
>       vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
>   
> -    for (unsigned i = 0; i < 2; i++) {
> +    for (unsigned i = 0; i < ARRAY_SIZE(d->isa_irq); i++) {
>           if (!pci_piix_init_bus(d, i, errp)) {
>               return;
>           }
> @@ -199,6 +207,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
> *data)
>   static const TypeInfo piix3_ide_info = {
>       .name          = TYPE_PIIX3_IDE,
>       .parent        = TYPE_PCI_IDE,
> +    .instance_init = piix_ide_initfn,
>       .class_init    = piix3_ide_class_init,
>   };
>   
> @@ -221,6 +230,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
>   static const TypeInfo piix4_ide_info = {
>       .name          = TYPE_PIIX4_IDE,
>       .parent        = TYPE_PCI_IDE,
> +    .instance_init = piix_ide_initfn,
>       .class_init    = piix4_ide_class_init,
>   };
>   
> diff --git a/include/hw/ide/piix.h b/include/hw/ide/piix.h
> index ef3ef3d62d..533d24d408 100644
> --- a/include/hw/ide/piix.h
> +++ b/include/hw/ide/piix.h
> @@ -1,6 +1,10 @@
>   #ifndef HW_IDE_PIIX_H
>   #define HW_IDE_PIIX_H
>   
> +/*
> + * QEMU interface:
> + *  + named GPIO outputs "ide-irq": asserted by each IDE channel
> + */
>   #define TYPE_PIIX3_IDE "piix3-ide"
>   #define TYPE_PIIX4_IDE "piix4-ide"

Comparing this with Bernhard's latest series, Bernhard's patch at 
20230422150728.176512-1-shentey@gmail.com/20230422150728.176512-2-shentey@gmail.com/">https://patchew.org/QEMU/20230422150728.176512-1-shentey@gmail.com/20230422150728.176512-2-shentey@gmail.com/ 
(with a small change) is the version we should use, since legacy IRQs are a feature 
of all PCI IDE controllers and not just the PIIX controllers.

If we do it this way then it is possible for all PCI IDE controllers to share the 
same logic for BDMA and legacy/native mode switching moving forward: if a PCI IDE 
controller doesn't implement legacy IRQ routing then the board can leave the IRQs 
disconnected.


ATB,

Mark.