Re: [Qemu-devel] [STABLE][PATCH] isa-bus: Remove bogus IRQ sharing check

[Aurelien Jarno]

On Sat, Feb 19, 2011 at 06:56:22PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <address@hidden>
> 
> Nothing prevented IRQ sharing on the ISA bus in principle. Not all
> boards supported this, neither each and every card nor driver and OS.
> Still, there existed valid IRQ sharing scenarios, (at least) two of them
> can also be found in QEMU: >2 PC UARTs and the PREP IDE buses.
> 
> So remove this artificial restriction from our ISA model and reenable
> both PREP IDE buses.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
> 
>  hw/hpet.c          |    1 -
>  hw/ide/piix.c      |    2 +-
>  hw/ide/via.c       |    2 +-
>  hw/isa-bus.c       |   16 +++-------------
>  hw/isa.h           |    2 +-
>  hw/mips_fulong2e.c |    2 +-
>  hw/mips_malta.c    |    4 ++--
>  hw/pc.c            |    2 +-
>  hw/pc_piix.c       |    4 ++--
>  hw/ppc_prep.c      |    2 +-
>  10 files changed, 13 insertions(+), 24 deletions(-)

I have just applied this patch to HEAD, with minor modifications:
- rebased to HEAD (due to 64d7e9a421fea0ac50b44541f5521de455e7cd5d)
- removed the ppc_prep.c part, did a git revert instead.

If no problem arises in the next days, I'll cherry-pick that into
stable-0.14.

> diff --git a/hw/hpet.c b/hw/hpet.c
> index 82a9a21..91ebb75 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -713,7 +713,6 @@ static int hpet_init(SysBusDevice *dev)
>      s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
>      s->capability |= ((HPET_CLK_PERIOD) << 32);
>  
> -    isa_reserve_irq(RTC_ISA_IRQ);
>      qdev_init_gpio_in(&dev->qdev, hpet_handle_rtc_irq, 1);
>  
>      /* HPET Area */
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index d4289af..c349644 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -122,7 +122,7 @@ 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_reserve_irq(port_info[i].isairq));
> +        ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
>  
>          bmdma_init(&d->bus[i], &d->bmdma[i]);
>          d->bmdma[i].bus = &d->bus[i];
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 0e90679..04f3290 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -145,7 +145,7 @@ 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_reserve_irq(port_info[i].isairq));
> +        ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
>  
>          bmdma_init(&d->bus[i], &d->bmdma[i]);
>          d->bmdma[i].bus = &d->bus[i];
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 6f349a5..d07aa41 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -25,7 +25,6 @@
>  struct ISABus {
>      BusState qbus;
>      qemu_irq *irqs;
> -    uint32_t assigned;
>  };
>  static ISABus *isabus;
>  target_phys_addr_t isa_mem_base = 0;
> @@ -61,33 +60,24 @@ void isa_bus_irqs(qemu_irq *irqs)
>  }
>  
>  /*
> - * isa_reserve_irq() reserves the ISA irq and returns the corresponding
> - * qemu_irq entry for the i8259.
> + * isa_get_irq() returns the corresponding qemu_irq entry for the i8259.
>   *
>   * 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_reserve_irq(int isairq)
> +qemu_irq isa_get_irq(int isairq)
>  {
>      if (isairq < 0 || isairq > 15) {
>          hw_error("isa irq %d invalid", isairq);
>      }
> -    if (isabus->assigned & (1 << isairq)) {
> -        hw_error("isa irq %d already assigned", isairq);
> -    }
> -    isabus->assigned |= (1 << isairq);
>      return isabus->irqs[isairq];
>  }
>  
>  void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
>  {
>      assert(dev->nirqs < ARRAY_SIZE(dev->isairq));
> -    if (isabus->assigned & (1 << isairq)) {
> -        hw_error("isa irq %d already assigned", isairq);
> -    }
> -    isabus->assigned |= (1 << isairq);
>      dev->isairq[dev->nirqs] = isairq;
> -    *p = isabus->irqs[isairq];
> +    *p = isa_get_irq(isairq);
>      dev->nirqs++;
>  }
>  
> diff --git a/hw/isa.h b/hw/isa.h
> index e26abfa..d2b6126 100644
> --- a/hw/isa.h
> +++ b/hw/isa.h
> @@ -26,7 +26,7 @@ struct ISADeviceInfo {
>  
>  ISABus *isa_bus_new(DeviceState *dev);
>  void isa_bus_irqs(qemu_irq *irqs);
> -qemu_irq isa_reserve_irq(int isairq);
> +qemu_irq isa_get_irq(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);
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index 2783ed5..3b0abdb 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -369,7 +369,7 @@ static void mips_fulong2e_init(ram_addr_t ram_size, const 
> char *boot_device,
>      qdev_init_nofail(eeprom);
>  
>      /* init other devices */
> -    pit = pit_init(0x40, isa_reserve_irq(0));
> +    pit = pit_init(0x40, isa_get_irq(0));
>      cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
>      DMA_init(0, cpu_exit_irq);
>  
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 930c51c..f86bcff 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -919,7 +919,7 @@ void mips_malta_init (ram_addr_t ram_size,
>      isa_bus_irqs(i8259);
>      pci_piix4_ide_init(pci_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_reserve_irq(9),
> +    smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, isa_get_irq(9),
>                            NULL, NULL, 0);
>      eeprom_buf = qemu_mallocz(8 * 256); /* XXX: make this persistent */
>      for (i = 0; i < 8; i++) {
> @@ -930,7 +930,7 @@ void mips_malta_init (ram_addr_t ram_size,
>          qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
>          qdev_init_nofail(eeprom);
>      }
> -    pit = pit_init(0x40, isa_reserve_irq(0));
> +    pit = pit_init(0x40, isa_get_irq(0));
>      cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
>      DMA_init(0, cpu_exit_irq);
>  
> diff --git a/hw/pc.c b/hw/pc.c
> index 56bf1d6..81d7a87 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1118,7 +1118,7 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>  
>      qemu_register_boot_set(pc_boot_set, *rtc_state);
>  
> -    pit = pit_init(0x40, isa_reserve_irq(0));
> +    pit = pit_init(0x40, isa_get_irq(0));
>      pcspk_init(pit);
>  
>      for(i = 0; i < MAX_SERIAL_PORTS; i++) {
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 2918454..97c5754 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -114,7 +114,7 @@ static void pc_init1(ram_addr_t ram_size,
>      }
>      isa_bus_irqs(isa_irq);
>  
> -    pc_register_ferr_irq(isa_reserve_irq(13));
> +    pc_register_ferr_irq(isa_get_irq(13));
>  
>      pc_vga_init(pci_enabled? pci_bus: NULL);
>  
> @@ -170,7 +170,7 @@ static void pc_init1(ram_addr_t ram_size,
>          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_reserve_irq(9), *cmos_s3, *smi_irq,
> +                              isa_get_irq(9), *cmos_s3, *smi_irq,
>                                kvm_enabled());
>          for (i = 0; i < 8; i++) {
>              DeviceState *eeprom;
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 6c1499a..6b22122 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -690,7 +690,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>          hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS);
>      }
>  
> -    for(i = 0; i < 1/*MAX_IDE_BUS*/; i++) {
> +    for(i = 0; i < MAX_IDE_BUS; i++) {
>          isa_ide_init(ide_iobase[i], ide_iobase2[i], ide_irq[i],
>                       hd[2 * i],
>                    hd[2 * i + 1]);
> -- 
> 1.7.1
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net