Re: [Qemu-devel] [PATCH v2 11/12] isa: give bus to isa_get_irq() and isa_bus_irqs()

[Anthony Liguori]

On 09/20/2011 12:24 AM, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau<address@hidden>

This patch breaks booting a 64-bit Ubuntu guest under KVM (probably under TCG 
too).

It hangs during startup.

Regards,

Anthony Liguori

> ---
>   hw/i8254.c         |    2 +-
>   hw/ide.h           |   12 ++++++++----
>   hw/ide/piix.c      |   30 ++++++++++++++++++++++--------
>   hw/ide/via.c       |   18 ++++++++++++++----
>   hw/isa-bus.c       |   14 +++++++-------
>   hw/isa.h           |    9 +++++++--
>   hw/mips_fulong2e.c |    2 +-
>   hw/mips_jazz.c     |    2 +-
>   hw/mips_malta.c    |    6 +++---
>   hw/mips_r4k.c      |    2 +-
>   hw/pc_piix.c       |   11 ++++++-----
>   hw/ppc_prep.c      |    2 +-
>   12 files changed, 72 insertions(+), 38 deletions(-)
>
> diff --git a/hw/i8254.c b/hw/i8254.c
> index a9ca9f6..35a0d01 100644
> --- a/hw/i8254.c
> +++ b/hw/i8254.c
> @@ -514,7 +514,7 @@ static int pit_initfn(ISADevice *dev)
>       s =&pit->channels[0];
>       /* the timer 0 is connected to an IRQ */
>       s->irq_timer = qemu_new_timer_ns(vm_clock, pit_irq_timer, s);
> -    s->irq = isa_get_irq(pit->irq);
> +    s->irq = isa_get_irq(isa_bus_from_device(dev), pit->irq);
>
>       register_ioport_write(pit->iobase, 4, 1, pit_ioport_write, pit);
>       register_ioport_read(pit->iobase, 3, 1, pit_ioport_read, pit);
> diff --git a/hw/ide.h b/hw/ide.h
> index 7075170..f1cfeb1 100644
> --- a/hw/ide.h
> +++ b/hw/ide.h
> @@ -13,10 +13,14 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int 
> iobase2, int isairq,
>   /* ide-pci.c */
>   void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
>                            int secondary_ide_enabled);
> -PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int 
> devfn);
> -PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> -void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> +PCIDevice *pci_piix3_xen_ide_init(PCIBus *pci_bus, ISABus *isa_bus,
> +                                  DriveInfo **hd_table, int devfn);
> +PCIDevice *pci_piix3_ide_init(PCIBus *bus, ISABus *isa_bus,
> +                              DriveInfo **hd_table, int devfn);
> +PCIDevice *pci_piix4_ide_init(PCIBus *bus, ISABus *isa_bus,
> +                              DriveInfo **hd_table, int devfn);
> +void vt82c686b_ide_init(PCIBus *pci_bus, ISABus *isa_bus, DriveInfo **hd_table,
> +                        int devfn);
>
>   /* ide-macio.c */
>   MemoryRegion *pmac_ide_init (DriveInfo **hd_table, qemu_irq irq,
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 88d3181..70b7835 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -121,7 +121,8 @@ static void piix3_reset(void *opaque)
>       pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
>   }
>
> -static void pci_piix_init_ports(PCIIDEState *d) {
> +static void pci_piix_init_legacy_ports(ISABus *isa_bus, PCIIDEState *d)
> +{
>       int i;
>       struct {
>           int iobase;
> @@ -135,8 +136,15 @@ static void pci_piix_init_ports(PCIIDEState *d) {
>       for (i = 0; i<  2; i++) {
>           ide_bus_new(&d->bus[i],&d->dev.qdev, i);
>           ide_init_ioport(&d->bus[i], port_info[i].iobase, 
> port_info[i].iobase2);
> -        ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
> +        ide_init2(&d->bus[i], isa_get_irq(isa_bus, port_info[i].isairq));
> +    }
> +}
>
> +static void pci_piix_init_ports(PCIIDEState *d)
> +{
> +    int i;
> +
> +    for (i = 0; i<  2; i++) {
>           bmdma_init(&d->bus[i],&d->bmdma[i], d);
>           d->bmdma[i].bus =&d->bus[i];
>           qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb,
> @@ -189,11 +197,13 @@ static int pci_piix3_xen_ide_unplug(DeviceState *dev)
>       return 0;
>   }
>
> -PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +PCIDevice *pci_piix3_xen_ide_init(PCIBus *pci_bus, ISABus *isa_bus,
> +                                  DriveInfo **hd_table, int devfn)
>   {
>       PCIDevice *dev;
>
> -    dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
> +    dev = pci_create_simple(pci_bus, devfn, "piix3-ide-xen");
> +    pci_piix_init_legacy_ports(isa_bus, DO_UPCAST(PCIIDEState, dev, dev));
>       dev->qdev.info->unplug = pci_piix3_xen_ide_unplug;
>       pci_ide_create_devs(dev, hd_table);
>       return dev;
> @@ -217,22 +227,26 @@ static int pci_piix_ide_exitfn(PCIDevice *dev)
>
>   /* hd_table must contain 4 block drivers */
>   /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
> -PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +PCIDevice *pci_piix3_ide_init(PCIBus *pci_bus, ISABus *isa_bus,
> +                              DriveInfo **hd_table, int devfn)
>   {
>       PCIDevice *dev;
>
> -    dev = pci_create_simple(bus, devfn, "piix3-ide");
> +    dev = pci_create_simple(pci_bus, devfn, "piix3-ide");
> +    pci_piix_init_legacy_ports(isa_bus, DO_UPCAST(PCIIDEState, dev, dev));
>       pci_ide_create_devs(dev, hd_table);
>       return dev;
>   }
>
>   /* hd_table must contain 4 block drivers */
>   /* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +PCIDevice *pci_piix4_ide_init(PCIBus *pci_bus, ISABus *isa_bus,
> +                              DriveInfo **hd_table, int devfn)
>   {
>       PCIDevice *dev;
>
> -    dev = pci_create_simple(bus, devfn, "piix4-ide");
> +    dev = pci_create_simple(pci_bus, devfn, "piix4-ide");
> +    pci_piix_init_legacy_ports(isa_bus, DO_UPCAST(PCIIDEState, dev, dev));
>       pci_ide_create_devs(dev, hd_table);
>       return dev;
>   }
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index dab8a39..6af4e79 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -145,7 +145,8 @@ static void via_reset(void *opaque)
>       pci_set_long(pci_conf + 0xc0, 0x00020001);
>   }
>
> -static void vt82c686b_init_ports(PCIIDEState *d) {
> +static void vt82c686b_init_legacy_ports(ISABus *isa_bus, PCIIDEState *d)
> +{
>       int i;
>       struct {
>           int iobase;
> @@ -159,8 +160,15 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
>       for (i = 0; i<  2; i++) {
>           ide_bus_new(&d->bus[i],&d->dev.qdev, i);
>           ide_init_ioport(&d->bus[i], port_info[i].iobase, 
> port_info[i].iobase2);
> -        ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
> +        ide_init2(&d->bus[i], isa_get_irq(isa_bus, port_info[i].isairq));
> +    }
> +}
>
> +static void vt82c686b_init_ports(PCIIDEState *d)
> +{
> +    int i;
> +
> +    for (i = 0; i<  2; i++) {
>           bmdma_init(&d->bus[i],&d->bmdma[i], d);
>           d->bmdma[i].bus =&d->bus[i];
>           qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb,
> @@ -204,11 +212,13 @@ static int vt82c686b_ide_exitfn(PCIDevice *dev)
>       return 0;
>   }
>
> -void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +void vt82c686b_ide_init(PCIBus *pci_bus, ISABus *isa_bus,
> +                        DriveInfo **hd_table, int devfn)
>   {
>       PCIDevice *dev;
>
> -    dev = pci_create_simple(bus, devfn, "via-ide");
> +    dev = pci_create_simple(pci_bus, devfn, "via-ide");
> +    vt82c686b_init_legacy_ports(isa_bus, DO_UPCAST(PCIIDEState, dev, dev));
>       pci_ide_create_devs(dev, hd_table);
>   }
>
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 773a424..25c7191 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -53,12 +53,12 @@ void isa_bus_new(ISABus *bus, ISABusOps *ops, DeviceState 
> *host)
>       isabus = bus;
>   }
>
> -void isa_bus_irqs(qemu_irq *irqs)
> +void isa_bus_irqs(ISABus *bus, qemu_irq *irqs)
>   {
> -    if (!isabus || !isabus->ops->set_irqs) {
> +    if (!bus || !bus->ops->set_irqs) {
>           hw_error("Tried to set isa irqs with no isa bus present.");
>       }
> -    isabus->ops->set_irqs(isabus, irqs);
> +    bus->ops->set_irqs(bus, irqs);
>   }
>
>   /*
> @@ -67,19 +67,19 @@ void isa_bus_irqs(qemu_irq *irqs)
>    * This function is only for special cases such as the 'ferr', and
>    * temporary use for normal devices until they are converted to qdev.
>    */
> -qemu_irq isa_get_irq(int isairq)
> +qemu_irq isa_get_irq(ISABus *bus, int isairq)
>   {
> -    if (!isabus || !isabus->ops->get_irq) {
> +    if (!bus || !bus->ops->get_irq) {
>           hw_error("ISA bus invalid");
>       }
> -    return isabus->ops->get_irq(isabus, isairq);
> +    return bus->ops->get_irq(bus, isairq);
>   }
>
>   void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
>   {
>       assert(dev->nirqs<  ARRAY_SIZE(dev->isairq));
>       dev->isairq[dev->nirqs] = isairq;
> -    *p = isa_get_irq(isairq);
> +    *p = isa_get_irq(isa_bus_from_device(dev), isairq);
>       dev->nirqs++;
>   }
>
> diff --git a/hw/isa.h b/hw/isa.h
> index 03ecd55..300c8c8 100644
> --- a/hw/isa.h
> +++ b/hw/isa.h
> @@ -41,8 +41,8 @@ struct ISABusOps {
>   ISABus *isa_bus_bridge_init(void);
>
>   void isa_bus_new(ISABus *bus, ISABusOps *ops, DeviceState *host);
> -void isa_bus_irqs(qemu_irq *irqs);
> -qemu_irq isa_get_irq(int isairq);
> +void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
> +qemu_irq isa_get_irq(ISABus *bus, int isairq);
>   void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
>   void isa_init_ioport(ISADevice *dev, uint16_t ioport);
>   void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
> @@ -52,6 +52,11 @@ ISADevice *isa_create(ISABus *bus, const char *name);
>   ISADevice *isa_try_create(ISABus *bus, const char *name);
>   ISADevice *isa_create_simple(ISABus *bus, const char *name);
>
> +static inline ISABus *isa_bus_from_device(ISADevice *d)
> +{
> +    return DO_UPCAST(ISABus, qbus, d->qdev.parent_bus);
> +}
> +
>   extern target_phys_addr_t isa_mem_base;
>
>   void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size);
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index f509b67..0b98402 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -343,7 +343,7 @@ static void mips_fulong2e_init(ram_addr_t ram_size, const 
> char *boot_device,
>           exit(1);
>       }
>
> -    vt82c686b_ide_init(pci_bus, hd, PCI_DEVFN(FULONG2E_VIA_SLOT, 1));
> +    vt82c686b_ide_init(pci_bus, isa_bus, hd, PCI_DEVFN(FULONG2E_VIA_SLOT, 1));
>       usb_uhci_vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 2));
>       usb_uhci_vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 3));
>
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index 24a8198..d3f97bb 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -183,7 +183,7 @@ void mips_jazz_init (ram_addr_t ram_size,
>       /* ISA devices */
>       i8259 = i8259_init(env->irq[4]);
>       isa_bus = isa_bus_bridge_init();
> -    isa_bus_irqs(i8259);
> +    isa_bus_irqs(isa_bus, i8259);
>       cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
>       DMA_init(0, cpu_exit_irq);
>       pit = pit_init(isa_bus, 0x40, 0);
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 7690822..829b602 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -941,10 +941,10 @@ void mips_malta_init (ram_addr_t ram_size,
>       ide_drive_get(hd, MAX_IDE_BUS);
>
>       isa_bus = piix4_init(pci_bus, 80, i8259,&piix4_devfn);
> -    pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
> +    pci_piix4_ide_init(pci_bus, isa_bus, hd, piix4_devfn + 1);
>       usb_uhci_piix4_init(pci_bus, piix4_devfn + 2);
> -    smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, isa_get_irq(9),
> -                          NULL, NULL, 0);
> +    smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
> +                          isa_get_irq(isa_bus, 9), NULL, NULL, 0);
>       /* TODO: Populate SPD eeprom data.  */
>       smbus_eeprom_init(smbus, 8, NULL, 0);
>       pit = pit_init(isa_bus, 0x40, 0);
> diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
> index 78eef37..c8f9877 100644
> --- a/hw/mips_r4k.c
> +++ b/hw/mips_r4k.c
> @@ -268,7 +268,7 @@ void mips_r4k_init (ram_addr_t ram_size,
>       /* The PIC is attached to the MIPS CPU INT0 pin */
>       i8259 = i8259_init(env->irq[2]);
>       isa_bus = isa_bus_bridge_init();
> -    isa_bus_irqs(i8259);
> +    isa_bus_irqs(isa_bus, i8259);
>
>       rtc_init(isa_bus, 2000, NULL);
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index ed8fc0d..47475a0 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -152,9 +152,9 @@ static void pc_init1(MemoryRegion *system_memory,
>           i440fx_state = NULL;
>           isa_bus = isa_bus_bridge_init();
>       }
> -    isa_bus_irqs(isa_irq);
> +    isa_bus_irqs(isa_bus, isa_irq);
>
> -    pc_register_ferr_irq(isa_get_irq(13));
> +    pc_register_ferr_irq(isa_get_irq(isa_bus, 13));
>
>       pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
>
> @@ -178,9 +178,10 @@ static void pc_init1(MemoryRegion *system_memory,
>       if (pci_enabled) {
>           PCIDevice *dev;
>           if (xen_enabled()) {
> -            dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
> +            dev = pci_piix3_xen_ide_init(pci_bus, isa_bus, hd,
> +                                         piix3_devfn + 1);
>           } else {
> -            dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
> +            dev = pci_piix3_ide_init(pci_bus, isa_bus, hd, piix3_devfn + 1);
>           }
>           idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
>           idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> @@ -214,7 +215,7 @@ static void pc_init1(MemoryRegion *system_memory,
>           smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
>           /* TODO: Populate SPD eeprom data.  */
>           smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> -                              isa_get_irq(9), *cmos_s3, *smi_irq,
> +                              isa_get_irq(isa_bus, 9), *cmos_s3, *smi_irq,
>                                 kvm_enabled());
>           smbus_eeprom_init(smbus, 8, NULL, 0);
>       }
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index d8d3fae..053254d 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -653,7 +653,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>       pci_bus = pci_prep_init(i8259, get_system_memory(), get_system_io());
>       /* Hmm, prep has no pci-isa bridge ??? */
>       isa_bus = isa_bus_bridge_init();
> -    isa_bus_irqs(i8259);
> +    isa_bus_irqs(isa_bus, i8259);
>       //    pci_bus = i440fx_init();
>       /* Register 8 MB of ISA IO space (needed for non-contiguous map) */
>       PPC_io_memory = cpu_register_io_memory(PPC_prep_io_read,