All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Marcel Apfelbaum <marcel.a@redhat.com>
Cc: kwolf@redhat.com, peter.maydell@linaro.org,
	peter.crosthwaite@xilinx.com, anthony@codemonkey.ws,
	sw@weilnetz.de, jasowang@redhat.com, qemu-devel@nongnu.org,
	dkoch@verizon.com, keith.busch@intel.com,
	alex.williamson@redhat.com, kraxel@redhat.com,
	stefanha@redhat.com, dmitry@daynix.com, pbonzini@redhat.com,
	afaerber@suse.de, ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH v3 6/8] hw: set interrupts using pci irq wrappers
Date: Mon, 7 Oct 2013 11:04:23 +0300	[thread overview]
Message-ID: <20131007080423.GA1960@redhat.com> (raw)
In-Reply-To: <20131007080347.GB647@redhat.com>

On Mon, Oct 07, 2013 at 11:03:47AM +0300, Michael S. Tsirkin wrote:
> On Mon, Oct 07, 2013 at 10:36:39AM +0300, Marcel Apfelbaum wrote:
> > pci_set_irq and the other pci irq wrappers use
> > PCI_INTERRUPT_PIN config register to compute device
> > INTx pin to assert/deassert.
> > 
> > An irq is allocated using pci_allocate_irq wrapper
> > only if is needed by non pci devices.
> > 
> > Removed irq related fields from state if not used anymore.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> > Changes from v2:
> >  - Addressed Alex Williamson's comments
> >    - replaced calls to pci_set_irq with
> >      pci_irq_assert/deassert when possible
> >  - Addressed Gerd Hoffmann's comment
> >    - removed irq_pin from UHCIState state
> >      because it is not used anymore
> >  
> >  hw/audio/ac97.c        | 4 ++--
> >  hw/audio/es1370.c      | 4 ++--
> >  hw/audio/intel-hda.c   | 2 +-
> >  hw/block/nvme.c        | 2 +-
> >  hw/char/serial-pci.c   | 5 +++--
> >  hw/char/tpci200.c      | 8 ++++----
> >  hw/display/qxl.c       | 2 +-
> >  hw/ide/cmd646.c        | 2 +-
> >  hw/ide/ich.c           | 3 ++-
> >  hw/isa/vt82c686.c      | 2 +-
> >  hw/misc/ivshmem.c      | 2 +-
> >  hw/net/e1000.c         | 2 +-
> >  hw/net/eepro100.c      | 4 ++--
> >  hw/net/ne2000.c        | 3 ++-
> >  hw/net/pcnet-pci.c     | 3 ++-
> >  hw/net/rtl8139.c       | 2 +-
> >  hw/pci/shpc.c          | 2 +-
> >  hw/scsi/esp-pci.c      | 3 ++-
> >  hw/scsi/lsi53c895a.c   | 2 +-
> >  hw/scsi/megasas.c      | 6 +++---
> >  hw/scsi/vmw_pvscsi.c   | 2 +-
> >  hw/usb/hcd-ehci-pci.c  | 2 +-
> >  hw/usb/hcd-ohci.c      | 2 +-
> >  hw/usb/hcd-uhci.c      | 6 ++----
> >  hw/usb/hcd-xhci.c      | 7 ++-----
> >  hw/virtio/virtio-pci.c | 4 ++--
> >  26 files changed, 43 insertions(+), 43 deletions(-)
> > 
> > diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> > index 01b4dfb..03f4846 100644
> > --- a/hw/audio/ac97.c
> > +++ b/hw/audio/ac97.c
> > @@ -280,12 +280,12 @@ static void update_sr (AC97LinkState *s, AC97BusMasterRegs *r, uint32_t new_sr)
> >      if (level) {
> >          s->glob_sta |= masks[r - s->bm_regs];
> >          dolog ("set irq level=1\n");
> > -        qemu_set_irq (s->dev.irq[0], 1);
> > +        pci_irq_assert(&s->dev);
> >      }
> >      else {
> >          s->glob_sta &= ~masks[r - s->bm_regs];
> >          dolog ("set irq level=0\n");
> > -        qemu_set_irq (s->dev.irq[0], 0);
> > +        pci_irq_deassert(&s->dev);
> >      }
> >  }
> >  
> > diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
> > index adb66ce..1ec7ace 100644
> > --- a/hw/audio/es1370.c
> > +++ b/hw/audio/es1370.c
> > @@ -323,7 +323,7 @@ static void es1370_update_status (ES1370State *s, uint32_t new_status)
> >      else {
> >          s->status = new_status & ~STAT_INTR;
> >      }
> > -    qemu_set_irq (s->dev.irq[0], !!level);
> > +    pci_set_irq(&s->dev, !!level);
> >  }
> >  
> >  static void es1370_reset (ES1370State *s)
> > @@ -349,7 +349,7 @@ static void es1370_reset (ES1370State *s)
> >              s->dac_voice[i] = NULL;
> >          }
> >      }
> > -    qemu_irq_lower (s->dev.irq[0]);
> > +    pci_irq_deassert(&s->dev);
> >  }
> >  
> >  static void es1370_maybe_lower_irq (ES1370State *s, uint32_t sctl)
> > diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> > index a6666c6..4327264 100644
> > --- a/hw/audio/intel-hda.c
> > +++ b/hw/audio/intel-hda.c
> > @@ -269,7 +269,7 @@ static void intel_hda_update_irq(IntelHDAState *d)
> >              msi_notify(&d->pci, 0);
> >          }
> >      } else {
> > -        qemu_set_irq(d->pci.irq[0], level);
> > +        pci_set_irq(&d->pci, level);
> >      }
> >  }
> >  
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 5dee229..2882ffe 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -69,7 +69,7 @@ static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
> >          if (msix_enabled(&(n->parent_obj))) {
> >              msix_notify(&(n->parent_obj), cq->vector);
> >          } else {
> > -            qemu_irq_pulse(n->parent_obj.irq[0]);
> > +            pci_irq_pulse(&n->parent_obj);
> >          }
> >      }
> >  }
> > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> > index aec6705..991c99f 100644
> > --- a/hw/char/serial-pci.c
> > +++ b/hw/char/serial-pci.c
> > @@ -61,7 +61,7 @@ static int serial_pci_init(PCIDevice *dev)
> >      }
> >  
> >      pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
> > -    s->irq = pci->dev.irq[0];
> > +    s->irq = pci_allocate_irq(&pci->dev);
> >  
> >      memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s, "serial", 8);
> >      pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
> > @@ -79,7 +79,7 @@ static void multi_serial_irq_mux(void *opaque, int n, int level)
> >              pending = 1;
> >          }
> >      }
> > -    qemu_set_irq(pci->dev.irq[0], pending);
> > +    pci_set_irq(&pci->dev, pending);
> >  }
> >  
> >  static int multi_serial_pci_init(PCIDevice *dev)
> > @@ -132,6 +132,7 @@ static void serial_pci_exit(PCIDevice *dev)
> >  
> >      serial_exit_core(s);
> >      memory_region_destroy(&s->io);
> > +    qemu_free_irq(s->irq);
> >  }
> >  
> >  static void multi_serial_pci_exit(PCIDevice *dev)
> > diff --git a/hw/char/tpci200.c b/hw/char/tpci200.c
> > index e04ff26..a49d2ed 100644
> > --- a/hw/char/tpci200.c
> > +++ b/hw/char/tpci200.c
> > @@ -134,8 +134,8 @@ static void tpci200_set_irq(void *opaque, int intno, int level)
> >      /* Check if the interrupt is edge sensitive */
> >      if (dev->ctrl[ip_n] & CTRL_INT_EDGE(intno)) {
> >          if (level) {
> > -            qemu_set_irq(dev->dev.irq[0], !dev->int_set);
> > -            qemu_set_irq(dev->dev.irq[0],  dev->int_set);
> > +            pci_set_irq(&dev->dev, !dev->int_set);
> > +            pci_set_irq(&dev->dev,  dev->int_set);
> >          }
> >      } else {
> >          unsigned i, j;
> > @@ -153,10 +153,10 @@ static void tpci200_set_irq(void *opaque, int intno, int level)
> >          }
> >  
> >          if (level_status && !dev->int_set) {
> > -            qemu_irq_raise(dev->dev.irq[0]);
> > +            pci_irq_assert(&dev->dev);
> >              dev->int_set = 1;
> >          } else if (!level_status && dev->int_set) {
> > -            qemu_irq_lower(dev->dev.irq[0]);
> > +            pci_irq_deassert(&dev->dev);
> >              dev->int_set = 0;
> >          }
> >      }
> > diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> > index ee2db0d..678ad38 100644
> > --- a/hw/display/qxl.c
> > +++ b/hw/display/qxl.c
> > @@ -1101,7 +1101,7 @@ static void qxl_update_irq(PCIQXLDevice *d)
> >      uint32_t pending = le32_to_cpu(d->ram->int_pending);
> >      uint32_t mask    = le32_to_cpu(d->ram->int_mask);
> >      int level = !!(pending & mask);
> > -    qemu_set_irq(d->pci.irq[0], level);
> > +    pci_set_irq(&d->pci, level);
> >      qxl_ring_set_dirty(d);
> >  }
> >  
> > diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> > index 0500a7a..a8e35fe 100644
> > --- a/hw/ide/cmd646.c
> > +++ b/hw/ide/cmd646.c
> > @@ -230,7 +230,7 @@ static void cmd646_update_irq(PCIIDEState *d)
> >                   !(pd->config[MRDMODE] & MRDMODE_BLK_CH0)) ||
> >          ((pd->config[MRDMODE] & MRDMODE_INTR_CH1) &&
> >           !(pd->config[MRDMODE] & MRDMODE_BLK_CH1));
> > -    qemu_set_irq(pd->irq[0], pci_level);
> > +    pci_set_irq(pd, pci_level);
> >  }
> >  
> >  /* the PCI irq level is the logical OR of the two channels */
> > diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> > index bff952b..1c7c058 100644
> > --- a/hw/ide/ich.c
> > +++ b/hw/ide/ich.c
> > @@ -116,7 +116,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
> >      dev->config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
> >  
> >      msi_init(dev, 0x50, 1, true, false);
> > -    d->ahci.irq = dev->irq[0];
> > +    d->ahci.irq = pci_allocate_irq(dev);
> >  
> >      pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> >                       &d->ahci.idp);
> > @@ -145,6 +145,7 @@ static void pci_ich9_uninit(PCIDevice *dev)
> >  
> >      msi_uninit(dev);
> >      ahci_uninit(&d->ahci);
> > +    qemu_free_irq(d->ahci.irq);
> >  }
> >  
> >  static void ich_ahci_class_init(ObjectClass *klass, void *data)
> > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> > index 8fe4fcb..5fb8086 100644
> > --- a/hw/isa/vt82c686.c
> > +++ b/hw/isa/vt82c686.c
> > @@ -185,7 +185,7 @@ static void pm_update_sci(VT686PMState *s)
> >                     ACPI_BITMASK_POWER_BUTTON_ENABLE |
> >                     ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
> >                     ACPI_BITMASK_TIMER_ENABLE)) != 0);
> > -    qemu_set_irq(s->dev.irq[0], sci_level);
> > +    pci_set_irq(&s->dev, sci_level);
> >      /* schedule a timer interruption if needed */
> >      acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) &&
> >                         !(pmsts & ACPI_BITMASK_TIMER_STATUS));
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index 2838866..8d144ba 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -133,7 +133,7 @@ static void ivshmem_update_irq(IVShmemState *s, int val)
> >             isr ? 1 : 0, s->intrstatus, s->intrmask);
> >      }
> >  
> > -    qemu_set_irq(d->irq[0], (isr != 0));
> > +    pci_set_irq(d, (isr != 0));
> >  }
> >  
> >  static void ivshmem_IntrMask_write(IVShmemState *s, uint32_t val)
> > diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> > index 151d25e..c497bad 100644
> > --- a/hw/net/e1000.c
> > +++ b/hw/net/e1000.c
> > @@ -328,7 +328,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
> >      }
> >  
> >      s->mit_irq_level = (pending_ints != 0);
> > -    qemu_set_irq(d->irq[0], s->mit_irq_level);
> > +    pci_set_irq(d, s->mit_irq_level);
> >  }
> >  
> >  static void
> > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> > index ffa60d5..3b891ca 100644
> > --- a/hw/net/eepro100.c
> > +++ b/hw/net/eepro100.c
> > @@ -409,7 +409,7 @@ static void disable_interrupt(EEPRO100State * s)
> >  {
> >      if (s->int_stat) {
> >          TRACE(INT, logout("interrupt disabled\n"));
> > -        qemu_irq_lower(s->dev.irq[0]);
> > +        pci_irq_deassert(&s->dev);
> >          s->int_stat = 0;
> >      }
> >  }
> > @@ -418,7 +418,7 @@ static void enable_interrupt(EEPRO100State * s)
> >  {
> >      if (!s->int_stat) {
> >          TRACE(INT, logout("interrupt enabled\n"));
> > -        qemu_irq_raise(s->dev.irq[0]);
> > +        pci_irq_assert(&s->dev);
> >          s->int_stat = 1;
> >      }
> >  }
> > diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
> > index c961258..a94cf74 100644
> > --- a/hw/net/ne2000.c
> > +++ b/hw/net/ne2000.c
> > @@ -731,7 +731,7 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
> >      s = &d->ne2000;
> >      ne2000_setup_io(s, DEVICE(pci_dev), 0x100);
> >      pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
> > -    s->irq = d->dev.irq[0];
> > +    s->irq = pci_allocate_irq(&d->dev);
> >  
> >      qemu_macaddr_default_if_unset(&s->c.macaddr);
> >      ne2000_reset(s);
> > @@ -752,6 +752,7 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
> >  
> >      memory_region_destroy(&s->io);
> >      qemu_del_nic(s->nic);
> > +    qemu_free_irq(s->irq);
> >  }
> >  
> >  static Property ne2000_properties[] = {
> > diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
> > index 865f2f0..6a5d806 100644
> > --- a/hw/net/pcnet-pci.c
> > +++ b/hw/net/pcnet-pci.c
> > @@ -282,6 +282,7 @@ static void pci_pcnet_uninit(PCIDevice *dev)
> >  {
> >      PCIPCNetState *d = PCI_PCNET(dev);
> >  
> > +    qemu_free_irq(d->state.irq);
> >      memory_region_destroy(&d->state.mmio);
> >      memory_region_destroy(&d->io_bar);
> >      timer_del(d->state.poll_timer);
> > @@ -331,7 +332,7 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
> >  
> >      pci_register_bar(pci_dev, 1, 0, &s->mmio);
> >  
> > -    s->irq = pci_dev->irq[0];
> > +    s->irq = pci_allocate_irq(pci_dev);
> >      s->phys_mem_read = pci_physical_memory_read;
> >      s->phys_mem_write = pci_physical_memory_write;
> >      s->dma_opaque = pci_dev;
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index c31199f..7d72b21 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -716,7 +716,7 @@ static void rtl8139_update_irq(RTL8139State *s)
> >      DPRINTF("Set IRQ to %d (%04x %04x)\n", isr ? 1 : 0, s->IntrStatus,
> >          s->IntrMask);
> >  
> > -    qemu_set_irq(d->irq[0], (isr != 0));
> > +    pci_set_irq(d, (isr != 0));
> >  }
> >  
> >  static int rtl8139_RxWrap(RTL8139State *s)
> > diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> > index eb092fd..0bbd36e 100644
> > --- a/hw/pci/shpc.c
> > +++ b/hw/pci/shpc.c
> > @@ -172,7 +172,7 @@ static void shpc_interrupt_update(PCIDevice *d)
> >      if (msi_enabled(d) && shpc->msi_requested != level)
> >          msi_notify(d, 0);
> >      else
> > -        qemu_set_irq(d->irq[0], level);
> > +        pci_set_irq(d, level);
> >      shpc->msi_requested = level;
> >  }
> >  
> > diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> > index 99bf8ec..48c8b82 100644
> > --- a/hw/scsi/esp-pci.c
> > +++ b/hw/scsi/esp-pci.c
> > @@ -361,7 +361,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
> >                            "esp-io", 0x80);
> >  
> >      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
> > -    s->irq = dev->irq[0];
> > +    s->irq = pci_allocate_irq(dev);
> >  
> >      scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
> >      if (!d->hotplugged) {
> > @@ -378,6 +378,7 @@ static void esp_pci_scsi_uninit(PCIDevice *d)
> >  {
> >      PCIESPState *pci = PCI_ESP(d);
> >  
> > +    qemu_free_irq(pci->esp.irq);
> >      memory_region_destroy(&pci->io);
> >  }
> >  
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index 36e5f50..cb30414 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -437,7 +437,7 @@ static void lsi_update_irq(LSIState *s)
> >                  level, s->dstat, s->sist1, s->sist0);
> >          last_level = level;
> >      }
> > -    qemu_set_irq(d->irq[0], level);
> > +    pci_set_irq(d, level);
> >  
> >      if (!level && lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON)) {
> >          DPRINTF("Handled IRQs & disconnected, looking for pending "
> > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > index 09b51b3..7c5a1a2 100644
> > --- a/hw/scsi/megasas.c
> > +++ b/hw/scsi/megasas.c
> > @@ -535,7 +535,7 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context)
> >                  msix_notify(pci_dev, 0);
> >              } else {
> >                  trace_megasas_irq_raise();
> > -                qemu_irq_raise(pci_dev->irq[0]);
> > +                pci_irq_assert(pci_dev);
> >              }
> >          }
> >      } else {
> > @@ -1936,7 +1936,7 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
> >          s->intr_mask = val;
> >          if (!megasas_intr_enabled(s) && !msix_enabled(pci_dev)) {
> >              trace_megasas_irq_lower();
> > -            qemu_irq_lower(pci_dev->irq[0]);
> > +            pci_irq_deassert(pci_dev);
> >          }
> >          if (megasas_intr_enabled(s)) {
> >              trace_megasas_intr_enabled();
> > @@ -1952,7 +1952,7 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
> >              stl_le_phys(s->producer_pa, s->reply_queue_head);
> >              if (!msix_enabled(pci_dev)) {
> >                  trace_megasas_irq_lower();
> > -                qemu_irq_lower(pci_dev->irq[0]);
> > +                pci_irq_deassert(pci_dev);
> >              }
> >          }
> >          break;
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index 819d671..94b328f 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -330,7 +330,7 @@ pvscsi_update_irq_status(PVSCSIState *s)
> >          return;
> >      }
> >  
> > -    qemu_set_irq(d->irq[0], !!should_raise);
> > +    pci_set_irq(d, !!should_raise);
> >  }
> >  
> >  static void
> > diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
> > index 4d21a0b..0c98594 100644
> > --- a/hw/usb/hcd-ehci-pci.c
> > +++ b/hw/usb/hcd-ehci-pci.c
> > @@ -60,7 +60,7 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
> >      pci_conf[0x6e] = 0x00;
> >      pci_conf[0x6f] = 0xc0;  /* USBLEFCTLSTS */
> >  
> > -    s->irq = dev->irq[3];
> > +    s->irq = pci_allocate_irq(dev);
> >      s->as = pci_get_address_space(dev);
> >  
> >      usb_ehci_realize(s, DEVICE(dev), NULL);
> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> > index 35f0878..2b36ee5 100644
> > --- a/hw/usb/hcd-ohci.c
> > +++ b/hw/usb/hcd-ohci.c
> > @@ -1944,7 +1944,7 @@ static int usb_ohci_initfn_pci(PCIDevice *dev)
> >                        pci_get_address_space(dev)) != 0) {
> >          return -1;
> >      }
> > -    ohci->state.irq = dev->irq[0];
> > +    ohci->state.irq = pci_allocate_irq(dev);
> >  
> >      pci_register_bar(dev, 0, 0, &ohci->state.mem);
> >      return 0;
> > diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
> > index becc7fa..238d1d2 100644
> > --- a/hw/usb/hcd-uhci.c
> > +++ b/hw/usb/hcd-uhci.c
> > @@ -164,7 +164,6 @@ struct UHCIState {
> >  
> >      /* Interrupts that should be raised at the end of the current frame.  */
> >      uint32_t pending_int_mask;
> > -    int irq_pin;
> >  
> >      /* Active packets */
> >      QTAILQ_HEAD(, UHCIQueue) queues;
> > @@ -381,7 +380,7 @@ static void uhci_update_irq(UHCIState *s)
> >      } else {
> >          level = 0;
> >      }
> > -    qemu_set_irq(s->dev.irq[s->irq_pin], level);
> > +    pci_set_irq(&s->dev, level);
> >  }
> >  
> >  static void uhci_reset(void *opaque)
> > @@ -1240,8 +1239,7 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
> >      /* TODO: reset value should be 0. */
> >      pci_conf[USB_SBRN] = USB_RELEASE_1; // release number
> >  
> > -    s->irq_pin = u->info.irq_pin;
> > -    pci_config_set_interrupt_pin(pci_conf, s->irq_pin + 1);
> > +    pci_config_set_interrupt_pin(pci_conf, u->info.irq_pin + 1);
> 
> So everyone does this + 1/ - 1 logic.
> 
> We get comments like:
> 	pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
> which just shows the API is confusing.
> How about we change pci_config_set_interrupt_pin to do + 1
> internally?
> Then add pci_config_get_interrupt_pin to do - 1.
> 
> Add a comment that this does not support devices without interrupts,
> for that - use get_byte directly.


Note: this is not a problem with your patch, it can
be a cleanup on top.

> >  
> >      if (s->masterbus) {
> >          USBPort *ports[NB_PORTS];
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 469c24d..4f0bbb7 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -449,7 +449,6 @@ struct XHCIState {
> >      /*< public >*/
> >  
> >      USBBus bus;
> > -    qemu_irq irq;
> >      MemoryRegion mem;
> >      MemoryRegion mem_cap;
> >      MemoryRegion mem_oper;
> > @@ -739,7 +738,7 @@ static void xhci_intx_update(XHCIState *xhci)
> >      }
> >  
> >      trace_usb_xhci_irq_intx(level);
> > -    qemu_set_irq(xhci->irq, level);
> > +    pci_set_irq(pci_dev, level);
> >  }
> >  
> >  static void xhci_msix_update(XHCIState *xhci, int v)
> > @@ -797,7 +796,7 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
> >  
> >      if (v == 0) {
> >          trace_usb_xhci_irq_intx(1);
> > -        qemu_set_irq(xhci->irq, 1);
> > +        pci_irq_assert(pci_dev);
> >      }
> >  }
> >  
> > @@ -3433,8 +3432,6 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
> >  
> >      xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
> >  
> > -    xhci->irq = dev->irq[0];
> > -
> >      memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
> >      memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
> >                            "capabilities", LEN_CAP);
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 4825802..7647be8 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -116,7 +116,7 @@ static void virtio_pci_notify(DeviceState *d, uint16_t vector)
> >      if (msix_enabled(&proxy->pci_dev))
> >          msix_notify(&proxy->pci_dev, vector);
> >      else
> > -        qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1);
> > +        pci_set_irq(&proxy->pci_dev, proxy->vdev->isr & 1);
> >  }
> >  
> >  static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
> > @@ -362,7 +362,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
> >          /* reading from the ISR also clears it. */
> >          ret = vdev->isr;
> >          vdev->isr = 0;
> > -        qemu_set_irq(proxy->pci_dev.irq[0], 0);
> > +        pci_irq_deassert(&proxy->pci_dev);
> >          break;
> >      case VIRTIO_MSI_CONFIG_VECTOR:
> >          ret = vdev->config_vector;
> > -- 
> > 1.8.3.1

  reply	other threads:[~2013-10-07  8:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 1/8] hw/core: Add interface to allocate and free a single IRQ Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 2/8] hw/pci: add pci wrappers for allocating and asserting irqs Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 3/8] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 4/8] hw/vmxnet3: set interrupts using pci irq wrappers Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 5/8] hw/vfio: " Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 6/8] hw: " Marcel Apfelbaum
2013-10-07  8:03   ` Michael S. Tsirkin
2013-10-07  8:04     ` Michael S. Tsirkin [this message]
2013-10-07  8:14       ` Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 7/8] hw/pcie: AER and hot-plug events must use device's interrupt Marcel Apfelbaum
2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 8/8] hw/pci: removed irq field from PCIDevice Marcel Apfelbaum
2013-10-07 10:05 ` [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Michael S. Tsirkin
2013-10-08 14:33 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131007080423.GA1960@redhat.com \
    --to=mst@redhat.com \
    --cc=afaerber@suse.de \
    --cc=alex.williamson@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=dkoch@verizon.com \
    --cc=dmitry@daynix.com \
    --cc=ehabkost@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=keith.busch@intel.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.