diff mbox series

[v3,02/18] hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function

Message ID 20230302224058.43315-3-philmd@linaro.org
State New
Headers show
Series hw/ide: Untangle ISA/PCI abuses of ide_init_ioport() | expand

Commit Message

Philippe Mathieu-Daudé March 2, 2023, 10:40 p.m. UTC
In order to allow Frankenstein uses such plugging a PIIX3
IDE function on a ICH9 chipset (which already exposes AHCI
ports...) as:

  $ qemu-system-x86_64 -M q35 -device piix3-ide

add a kludge to automatically wires the IDE IRQs on an ISA
bus exposed by a PCI-to-ISA bridge (usually function #0).
Restrict this kludge to the PIIX3.

Reported-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: describe why this configuration is broken (multiple
output IRQs wired to the same input IRQ can lead to various
IRQ level changed in the iothread, thus missed by the vCPUs).
---
 hw/ide/piix.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Mark Cave-Ayland April 26, 2023, 12:48 p.m. UTC | #1
On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:

> In order to allow Frankenstein uses such plugging a PIIX3
> IDE function on a ICH9 chipset (which already exposes AHCI
> ports...) as:
> 
>    $ qemu-system-x86_64 -M q35 -device piix3-ide
> 
> add a kludge to automatically wires the IDE IRQs on an ISA
> bus exposed by a PCI-to-ISA bridge (usually function #0).
> Restrict this kludge to the PIIX3.
> 
> Reported-by: Bernhard Beschow <shentey@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: describe why this configuration is broken (multiple
> output IRQs wired to the same input IRQ can lead to various
> IRQ level changed in the iothread, thus missed by the vCPUs).
> ---
>   hw/ide/piix.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index a36dac8469..7cb96ef67f 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -170,6 +170,18 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>   
>       vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
>   
> +    if (!d->isa_irq[0] && !d->isa_irq[1]
> +                       && DEVICE_GET_CLASS(d)->user_creatable) {
> +        /* kludge specific to TYPE_PIIX3_IDE */
> +        Object *isabus = object_resolve_path_type("", TYPE_ISA_BUS, NULL);
> +
> +        if (!isabus) {
> +            error_setg(errp, "Unable to find a single ISA bus");
> +            return;
> +        }
> +        d->isa_irq[0] = isa_bus_get_irq(ISA_BUS(isabus), 14);
> +        d->isa_irq[1] = isa_bus_get_irq(ISA_BUS(isabus), 15);
> +    }
>       for (unsigned i = 0; i < ARRAY_SIZE(d->isa_irq); i++) {
>           if (!pci_piix_init_bus(d, i, errp)) {
>               return;
> @@ -202,6 +214,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
>       k->class_id = PCI_CLASS_STORAGE_IDE;
>       set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>       dc->hotpluggable = false;
> +    /*
> +     * This function is part of a Super I/O chip and shouldn't be user
> +     * creatable. However QEMU accepts impossible hardware setups such
> +     * plugging a PIIX IDE function on a ICH ISA bridge.
> +     * Keep this Frankenstein (ab)use working.
> +     */
> +    dc->user_creatable = true;
>   }
>   
>   static const TypeInfo piix3_ide_info = {
> @@ -225,6 +244,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
>       k->class_id = PCI_CLASS_STORAGE_IDE;
>       set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>       dc->hotpluggable = false;
> +    /* Reason: Part of a Super I/O chip */
> +    dc->user_creatable = false;
>   }
>   
>   static const TypeInfo piix4_ide_info = {

Heh. Do we really want to support this configuration? :O  If PIIX is hard-wired to 
use legacy IRQs then our options are limited, since the device will always require 
separate routing to an ISABus which can only be present in the PIIX's own PCI-ISA 
bridge (i.e. it cannot exist standalone).

Having said that, it should be possible to do this with the VIA IDE since that can 
simply be switched to native mode.


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a36dac8469..7cb96ef67f 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -170,6 +170,18 @@  static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
 
+    if (!d->isa_irq[0] && !d->isa_irq[1]
+                       && DEVICE_GET_CLASS(d)->user_creatable) {
+        /* kludge specific to TYPE_PIIX3_IDE */
+        Object *isabus = object_resolve_path_type("", TYPE_ISA_BUS, NULL);
+
+        if (!isabus) {
+            error_setg(errp, "Unable to find a single ISA bus");
+            return;
+        }
+        d->isa_irq[0] = isa_bus_get_irq(ISA_BUS(isabus), 14);
+        d->isa_irq[1] = isa_bus_get_irq(ISA_BUS(isabus), 15);
+    }
     for (unsigned i = 0; i < ARRAY_SIZE(d->isa_irq); i++) {
         if (!pci_piix_init_bus(d, i, errp)) {
             return;
@@ -202,6 +214,13 @@  static void piix3_ide_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->hotpluggable = false;
+    /*
+     * This function is part of a Super I/O chip and shouldn't be user
+     * creatable. However QEMU accepts impossible hardware setups such
+     * plugging a PIIX IDE function on a ICH ISA bridge.
+     * Keep this Frankenstein (ab)use working.
+     */
+    dc->user_creatable = true;
 }
 
 static const TypeInfo piix3_ide_info = {
@@ -225,6 +244,8 @@  static void piix4_ide_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->hotpluggable = false;
+    /* Reason: Part of a Super I/O chip */
+    dc->user_creatable = false;
 }
 
 static const TypeInfo piix4_ide_info = {