diff mbox series

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

Message ID 20230220080044.4646-1-philmd@linaro.org
State Superseded
Headers show
Series None | expand

Commit Message

Philippe Mathieu-Daudé Feb. 20, 2023, 8 a.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>
---
 hw/ide/piix.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Gerd Hoffmann Feb. 20, 2023, 9:10 a.m. UTC | #1 
On Mon, Feb 20, 2023 at 09:00:44AM +0100, 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.

Well.  On physical hardware you have a config switch in the bios
setup which turns off sata and enables ide instead (i.e. the
chipset implements both and can be configured to expose the one
or the other).

If we want support ide for q35 we should IMHO do something simliar
instead of making piix-ide user-pluggable.  We already have -machine
q35,sata={on,off}.  We could extend that somehow, by adding
ide={on,off}, or by using storage={sata,ide,off} instead.

This has been discussed now and then in the past and the usual
conclusion was that there is little reason to implement that given
that you can just use the 'pc' machine type.  For guests that old
that they can't handle sata storage this is usually the better fit
anyway ...

take care,
  Gerd
diff mbox series

Patch

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 9d876dd4a7..50975a16b3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -170,6 +170,17 @@  static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
 
+    if (!d->irq[0] && !d->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 unique ISA bus");
+            return;
+        }
+        d->irq[0] = isa_bus_get_irq(ISA_BUS(isabus), 14);
+        d->irq[1] = isa_bus_get_irq(ISA_BUS(isabus), 15);
+    }
     for (unsigned i = 0; i < ARRAY_SIZE(d->irq); i++) {
         if (!pci_piix_init_bus(d, i, errp)) {
             return;
@@ -202,6 +213,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 +243,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 = {