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

Philippe Mathieu-Daudé posted 18 patches 2 years, 7 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, John Snow <jsnow@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH v3 01/18] hw/ide/piix: Expose output IRQ as properties for late object population
Posted by Philippe Mathieu-Daudé 2 years, 7 months ago
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"
 
-- 
2.38.1


Re: [PATCH v3 01/18] hw/ide/piix: Expose output IRQ as properties for late object population
Posted by Mark Cave-Ayland 2 years, 5 months ago
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 
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.