[Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Marcel Apfelbaum]

Note: Added RFC because not all affected devices were
      checked yet. 

Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
register during device initialization. Devices should not call
directly qemu_set_irq and specify the INTx pin.

Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
qemu_irq_lower and qemu_irq_pulse, setting the irq
based on PCI_INTERRUPT_PIN.

Added interface to allocate and free single irq.
Added pci_allocate_irq wrapper to be used by devices that
still need PCIDevice infrastructure to assert irqs.

Removed irq field from PCIDevice, not needed anymore.

Special cases of replacements were done in separate patches,
all others in one patch "hw: set interrupts using pci irq wrappers"

Changes from v1:
 - Addressed Michael S. Tsirkin's comments:
   - pci_set_irq directly calls pci_irq handler
   - removed irq field from PCIDevice
 - Added qemu interface to allocate single irq
 - Added pci wrappers to allocate and free pci irq
 - Added pci irq wrappers for all qemu methods
   setting irq and not only qemu_set_irq 
 - Replace all qemu irq setters with pci
   wrappers

Marcel Apfelbaum (9):
  hw/core: Add interface to allocate and free a single IRQ
  hw/pci: add pci wrappers for allocating and asserting irqs
  hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
  hw/vmxnet3: set interrupts using pci irq wrappers
  hw/vfio: set interrupts using pci irq wrappers
  hw/xhci: set interrupts using pci irq wrappers
  hw: set interrupts using pci irq wrappers
  hw/pcie: AER and hot-plug events must use device's interrupt
  hw/pci: removed irq field from PCIDevice

 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/core/irq.c                  | 16 ++++++++++++++++
 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/misc/vfio.c                 | 11 ++++++-----
 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/net/vmxnet3.c               | 13 +++++++++++--
 hw/pci-bridge/pci_bridge_dev.c |  2 +-
 hw/pci/pci.c                   | 26 +++++++++++++++++++++-----
 hw/pci/pcie.c                  |  4 ++--
 hw/pci/pcie_aer.c              |  4 ++--
 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              |  2 +-
 hw/usb/hcd-xhci.c              |  7 ++-----
 hw/virtio/virtio-pci.c         |  4 ++--
 include/hw/irq.h               |  7 +++++++
 include/hw/pci/pci.h           | 22 +++++++++++++++++++---
 include/hw/pci/pcie.h          | 18 ------------------
 36 files changed, 127 insertions(+), 78 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ

[Marcel Apfelbaum]

qemu_allocate_irq returns a single qemu_irq.
The interface allows to specify an interrupt number.

qemu_free_irq frees it.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/core/irq.c    | 16 ++++++++++++++++
 include/hw/irq.h |  7 +++++++
 2 files changed, 23 insertions(+)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 2078542..03c8cb3 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -68,6 +68,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n)
     return qemu_extend_irqs(NULL, 0, handler, opaque, n);
 }
 
+qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
+{
+    struct IRQState *irq;
+
+    irq = g_new(struct IRQState, 1);
+    irq->handler = handler;
+    irq->opaque = opaque;
+    irq->n = n;
+
+    return irq;
+}
 
 void qemu_free_irqs(qemu_irq *s)
 {
@@ -75,6 +86,11 @@ void qemu_free_irqs(qemu_irq *s)
     g_free(s);
 }
 
+void qemu_free_irq(qemu_irq irq)
+{
+    g_free(irq);
+}
+
 static void qemu_notirq(void *opaque, int line, int level)
 {
     struct IRQState *irq = opaque;
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 610e6b7..f560dea 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -30,6 +30,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
  */
 qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n);
 
+/*
+ * Allocates a single IRQ. The irq is assigned with a handler, an opaque
+ * data and the interrupt number
+ */
+qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
+
 /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data
  * preserved. New IRQs are assigned the argument handler and opaque data.
  */
@@ -37,6 +43,7 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                                 void *opaque, int n);
 
 void qemu_free_irqs(qemu_irq *s);
+void qemu_free_irq(qemu_irq irq);
 
 /* Returns a new IRQ with opposite polarity.  */
 qemu_irq qemu_irq_invert(qemu_irq irq);
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs

[Marcel Apfelbaum]

Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
register during device initialization. Devices should not call
directly qemu_set_irq and specify the INTx pin on each call.

Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
qemu_irq_lower and qemu_irq_pulse, setting the irq
based on PCI_INTERRUPT_PIN.

Added pci_allocate_irq wrapper to be used by devices that
still need PCIDevice infrastructure to assert irqs.

Renamed a static method which was named already pci_set_irq.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
Changes from v1:
 - Added pci irq wrappers for all qemu methods
   setting irq and not only qemu_set_irq 
 - Added pci wrappers to allocate and pci irq

 hw/pci/pci.c         | 26 ++++++++++++++++++++++----
 include/hw/pci/pci.h | 19 +++++++++++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 00554a0..fbfd8f7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
 
 static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
 static void pci_update_mappings(PCIDevice *d);
-static void pci_set_irq(void *opaque, int irq_num, int level);
+static void pci_irq_handler(void *opaque, int irq_num, int level);
 static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
 static void pci_del_option_rom(PCIDevice *pdev);
 
@@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
 {
     int i;
     for (i = 0; i < PCI_NUM_PINS; ++i) {
-        qemu_set_irq(dev->irq[i], 0);
+        pci_irq_handler(dev, i, 0);
     }
 }
 
@@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_dev->config_read = config_read;
     pci_dev->config_write = config_write;
     bus->devices[devfn] = pci_dev;
-    pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
+    pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
     pci_dev->version_id = 2; /* Current pci device vmstate version */
     return pci_dev;
 }
@@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
 /* generic PCI irq support */
 
 /* 0 <= irq_num <= 3. level must be 0 or 1 */
-static void pci_set_irq(void *opaque, int irq_num, int level)
+static void pci_irq_handler(void *opaque, int irq_num, int level)
 {
     PCIDevice *pci_dev = opaque;
     int change;
@@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
     pci_change_irq_level(pci_dev, irq_num, change);
 }
 
+static inline int pci_intx(PCIDevice *pci_dev)
+{
+    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
+}
+
+qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
+{
+    int intx = pci_intx(pci_dev);
+
+    return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
+}
+
+void pci_set_irq(PCIDevice *pci_dev, int level)
+{
+    int intx = pci_intx(pci_dev);
+    pci_irq_handler(pci_dev, intx, level);
+}
+
 /* Special hooks used by device assignment */
 void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
 {
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 4b90e5d..df7d316 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 
+qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
+void pci_set_irq(PCIDevice *pci_dev, int level);
+
+static inline void pci_irq_raise(PCIDevice *pci_dev)
+{
+    pci_set_irq(pci_dev, 1);
+}
+
+static inline void pci_irq_lower(PCIDevice *pci_dev)
+{
+    pci_set_irq(pci_dev, 0);
+}
+
+static inline void pci_irq_pulse(PCIDevice *pci_dev)
+{
+    pci_irq_lower(pci_dev);
+    pci_irq_raise(pci_dev);
+}
+
 static inline int pci_is_express(const PCIDevice *d)
 {
     return d->cap_present & QEMU_PCI_CAP_EXPRESS;
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 3/9] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init

[Marcel Apfelbaum]

The PCI_INTERRUPT_PIN will be used by shpc init, so
was moved before the call to shpc_init.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci-bridge/pci_bridge_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index a9392c7..440e187 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -53,6 +53,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
     if (err) {
         goto bridge_error;
     }
+    dev->config[PCI_INTERRUPT_PIN] = 0x1;
     memory_region_init(&bridge_dev->bar, OBJECT(dev), "shpc-bar", shpc_bar_size(dev));
     err = shpc_init(dev, &br->sec_bus, &bridge_dev->bar, 0);
     if (err) {
@@ -73,7 +74,6 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
      * Check whether that works well. */
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
 		     PCI_BASE_ADDRESS_MEM_TYPE_64, &bridge_dev->bar);
-    dev->config[PCI_INTERRUPT_PIN] = 0x1;
     return 0;
 msi_error:
     slotid_cap_cleanup(dev);
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 4/9] hw/vmxnet3: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

pci_set_irq uses PCI_INTERRUPT_PIN config register
to compute device INTx pin to set.

An assert is used to ensure that intx received
from the quest OS corresponds to PCI_INTERRUPT_PIN.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/net/vmxnet3.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 49c2466..674ee7a 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -336,7 +336,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State *s, uint32_t int_idx)
     }
 
     VMW_IRPRN("Asserting line for interrupt %u", int_idx);
-    qemu_set_irq(d->irq[int_idx], 1);
+    pci_set_irq(d, 1);
     return true;
 }
 
@@ -356,7 +356,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State *s, int lidx)
     assert(!s->msi_used || !msi_enabled(d));
 
     VMW_IRPRN("Deasserting line for interrupt %u", lidx);
-    qemu_set_irq(d->irq[lidx], 0);
+    pci_set_irq(d, 1);
 }
 
 static void vmxnet3_update_interrupt_line_state(VMXNET3State *s, int lidx)
@@ -1299,6 +1299,12 @@ static void vmxnet3_update_features(VMXNET3State *s)
     }
 }
 
+static bool vmxnet3_verify_intx(VMXNET3State *s, int intx)
+{
+    return s->msix_used || s->msi_used || (intx ==
+           (pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1));
+}
+
 static void vmxnet3_activate_device(VMXNET3State *s)
 {
     int i;
@@ -1332,6 +1338,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
 
     s->event_int_idx =
         VMXNET3_READ_DRV_SHARED8(s->drv_shmem, devRead.intrConf.eventIntrIdx);
+    assert(vmxnet3_verify_intx(s, s->event_int_idx));
     VMW_CFPRN("Events interrupt line is %u", s->event_int_idx);
 
     s->auto_int_masking =
@@ -1364,6 +1371,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
         /* Read interrupt number for this TX queue */
         s->txq_descr[i].intr_idx =
             VMXNET3_READ_TX_QUEUE_DESCR8(qdescr_pa, conf.intrIdx);
+        assert(vmxnet3_verify_intx(s, s->txq_descr[i].intr_idx));
 
         VMW_CFPRN("TX Queue %d interrupt: %d", i, s->txq_descr[i].intr_idx);
 
@@ -1411,6 +1419,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
         /* Read interrupt number for this RX queue */
         s->rxq_descr[i].intr_idx =
             VMXNET3_READ_TX_QUEUE_DESCR8(qd_pa, conf.intrIdx);
+        assert(vmxnet3_verify_intx(s, s->rxq_descr[i].intr_idx));
 
         VMW_CFPRN("RX Queue %d interrupt: %d", i, s->rxq_descr[i].intr_idx);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

pci_set_irq and the other pci irq wrappers use
PCI_INTERRUPT_PIN config register to compute device
INTx pin to assert/deassert.

Save INTx pin into the config register before calling
pci_set_irq

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/misc/vfio.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index a1c08fb..3d7297c 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
             'A' + vdev->intx.pin);
 
     vdev->intx.pending = true;
-    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
+    pci_set_irq(&vdev->pdev, 1);
     vfio_mmap_set_enabled(vdev, false);
     if (vdev->intx.mmap_timeout) {
         timer_mod(vdev->intx.mmap_timer,
@@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
             vdev->host.bus, vdev->host.slot, vdev->host.function);
 
     vdev->intx.pending = false;
-    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+    pci_set_irq(&vdev->pdev, 0);
     vfio_unmask_intx(vdev);
 }
 
@@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
     qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
     vfio_mask_intx(vdev);
     vdev->intx.pending = false;
-    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+    pci_set_irq(&vdev->pdev, 0);
 
     /* Get an eventfd for resample/unmask */
     if (event_notifier_init(&vdev->intx.unmask, 0)) {
@@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
      */
     vfio_mask_intx(vdev);
     vdev->intx.pending = false;
-    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+    pci_set_irq(&vdev->pdev, 0);
 
     /* Tell KVM to stop listening for an INTx irqfd */
     if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
@@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
     vfio_disable_interrupts(vdev);
 
     vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
+    pci_config_set_interrupt_pin(vdev->pdev.config, pin);
 
 #ifdef CONFIG_KVM
     /*
@@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
     vfio_disable_intx_kvm(vdev);
     vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
     vdev->intx.pending = false;
-    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+    pci_set_irq(&vdev->pdev, 0);
     vfio_mmap_set_enabled(vdev, true);
 
     fd = event_notifier_get_fd(&vdev->intx.interrupt);
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 6/9] hw/xhci: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

pci_set_irq uses PCI_INTERRUPT_PIN config register
to compute device INTx pin to assert/deassert.

Removed irq field from xhci state.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/usb/hcd-xhci.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 469c24d..54d6842 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_set_irq(pci_dev, 1);
     }
 }
 
@@ -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);
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

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.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 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      | 2 +-
 hw/virtio/virtio-pci.c | 4 ++--
 25 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 01b4dfb..1f94b19 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_set_irq(&s->dev, 1);
     }
     else {
         s->glob_sta &= ~masks[r - s->bm_regs];
         dolog ("set irq level=0\n");
-        qemu_set_irq (s->dev.irq[0], 0);
+        pci_set_irq(&s->dev, 0);
     }
 }
 
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index adb66ce..3dbd12b 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_lower(&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..84831c1 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_raise(&dev->dev);
             dev->int_set = 1;
         } else if (!level_status && dev->int_set) {
-            qemu_irq_lower(dev->dev.irq[0]);
+            pci_irq_lower(&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..0eb3f62 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_lower(&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_raise(&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..699ca53 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_raise(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_lower(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_lower(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..075046e 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -381,7 +381,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)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4825802..6b84c4a 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_set_irq(&proxy->pci_dev, 0);
         break;
     case VIRTIO_MSI_CONFIG_VECTOR:
         ret = vdev->config_vector;
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 8/9] hw/pcie: AER and hot-plug events must use device's interrupt

[Marcel Apfelbaum]

The fields hpev_intx and aer_intx were removed because
both AER and hot-plug events must use device's interrupt.
Assert/deassert interrupts using pci_set_irq wrapper instead.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci/pcie.c         |  4 ++--
 hw/pci/pcie_aer.c     |  4 ++--
 include/hw/pci/pcie.h | 18 ------------------
 3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 50af3c1..d4dd005 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -187,7 +187,7 @@ static void hotplug_event_notify(PCIDevice *dev)
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_cap_flags_get_vector(dev));
     } else {
-        qemu_set_irq(dev->irq[dev->exp.hpev_intx], dev->exp.hpev_notified);
+        pci_set_irq(dev, dev->exp.hpev_notified);
     }
 }
 
@@ -195,7 +195,7 @@ static void hotplug_event_clear(PCIDevice *dev)
 {
     hotplug_event_update_event_status(dev);
     if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
-        qemu_set_irq(dev->irq[dev->exp.hpev_intx], 0);
+        pci_set_irq(dev, 0);
     }
 }
 
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ca762ab..03b1e1f 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -285,7 +285,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_aer_root_get_vector(dev));
     } else {
-        qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
+        pci_set_irq(dev, 1);
     }
 }
 
@@ -768,7 +768,7 @@ void pcie_aer_root_write_config(PCIDevice *dev,
     uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
     /* 6.2.4.1.2 Interrupt Generation */
     if (!msix_enabled(dev) && !msi_enabled(dev)) {
-        qemu_set_irq(dev->irq[dev->exp.aer_intx], !!(root_cmd & enabled_cmd));
+        pci_set_irq(dev, !!(root_cmd & enabled_cmd));
         return;
     }
 
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index c010007..1966169 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -64,15 +64,6 @@ struct PCIExpressDevice {
     uint8_t exp_cap;
 
     /* SLOT */
-    unsigned int hpev_intx;     /* INTx for hot plug event (0-3:INT[A-D]#)
-                                 * default is 0 = INTA#
-                                 * If the chip wants to use other interrupt
-                                 * line, initialize this member with the
-                                 * desired number.
-                                 * If the chip dynamically changes this member,
-                                 * also initialize it when loaded as
-                                 * appropreately.
-                                 */
     bool hpev_notified; /* Logical AND of conditions for hot plug event.
                          Following 6.7.3.4:
                          Software Notification of Hot-Plug Events, an interrupt
@@ -82,15 +73,6 @@ struct PCIExpressDevice {
     /* AER */
     uint16_t aer_cap;
     PCIEAERLog aer_log;
-    unsigned int aer_intx;      /* INTx for error reporting
-                                 * default is 0 = INTA#
-                                 * If the chip wants to use other interrupt
-                                 * line, initialize this member with the
-                                 * desired number.
-                                 * If the chip dynamically changes this member,
-                                 * also initialize it when loaded as
-                                 * appropreately.
-                                 */
 };
 
 /* PCI express capability helper functions */
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC v2 9/9] hw/pci: removed irq field from PCIDevice

[Marcel Apfelbaum]

Instead of exposing the the irq field,
pci wrappers to qemu_set_irq or qemu_irq_*
can be used.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci/pci.c         | 2 --
 include/hw/pci/pci.h | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fbfd8f7..f8d0b81 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -863,14 +863,12 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_dev->config_read = config_read;
     pci_dev->config_write = config_write;
     bus->devices[devfn] = pci_dev;
-    pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
     pci_dev->version_id = 2; /* Current pci device vmstate version */
     return pci_dev;
 }
 
 static void do_pci_unregister_device(PCIDevice *pci_dev)
 {
-    qemu_free_irqs(pci_dev->irq);
     pci_dev->bus->devices[pci_dev->devfn] = NULL;
     pci_config_free(pci_dev);
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index df7d316..e9346bd 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -247,9 +247,6 @@ struct PCIDevice {
     PCIConfigReadFunc *config_read;
     PCIConfigWriteFunc *config_write;
 
-    /* IRQ objects for the INTA-INTD pins.  */
-    qemu_irq *irq;
-
     /* Legacy PCI VGA regions */
     MemoryRegion *vga_regions[QEMU_PCI_VGA_NUM_REGIONS];
     bool has_vga;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ

[Michael S. Tsirkin]

On Wed, Oct 02, 2013 at 03:41:26PM +0300, Marcel Apfelbaum wrote:
> qemu_allocate_irq returns a single qemu_irq.
> The interface allows to specify an interrupt number.
> 
> qemu_free_irq frees it.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  hw/core/irq.c    | 16 ++++++++++++++++
>  include/hw/irq.h |  7 +++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 2078542..03c8cb3 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -68,6 +68,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n)
>      return qemu_extend_irqs(NULL, 0, handler, opaque, n);
>  }
>  
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
> +{
> +    struct IRQState *irq;
> +
> +    irq = g_new(struct IRQState, 1);
> +    irq->handler = handler;
> +    irq->opaque = opaque;
> +    irq->n = n;
> +
> +    return irq;
> +}
>  
>  void qemu_free_irqs(qemu_irq *s)
>  {
> @@ -75,6 +86,11 @@ void qemu_free_irqs(qemu_irq *s)
>      g_free(s);
>  }
>  
> +void qemu_free_irq(qemu_irq irq)
> +{
> +    g_free(irq);
> +}
> +
>  static void qemu_notirq(void *opaque, int line, int level)
>  {
>      struct IRQState *irq = opaque;
> diff --git a/include/hw/irq.h b/include/hw/irq.h
> index 610e6b7..f560dea 100644
> --- a/include/hw/irq.h
> +++ b/include/hw/irq.h
> @@ -30,6 +30,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
>   */
>  qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n);
>  
> +/*
> + * Allocates a single IRQ. The irq is assigned with a handler, an opaque
> + * data and the interrupt number

Add period at end of line :)
no need to repost for this.

> + */
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
> +
>  /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data
>   * preserved. New IRQs are assigned the argument handler and opaque data.
>   */
> @@ -37,6 +43,7 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>                                  void *opaque, int n);
>  
>  void qemu_free_irqs(qemu_irq *s);
> +void qemu_free_irq(qemu_irq irq);
>  
>  /* Returns a new IRQ with opposite polarity.  */
>  qemu_irq qemu_irq_invert(qemu_irq irq);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs

[Michael S. Tsirkin]

On Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> register during device initialization. Devices should not call
> directly qemu_set_irq and specify the INTx pin on each call.
> 
> Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> qemu_irq_lower and qemu_irq_pulse, setting the irq
> based on PCI_INTERRUPT_PIN.
> 
> Added pci_allocate_irq wrapper to be used by devices that
> still need PCIDevice infrastructure to assert irqs.
> 
> Renamed a static method which was named already pci_set_irq.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> Changes from v1:
>  - Added pci irq wrappers for all qemu methods
>    setting irq and not only qemu_set_irq 
>  - Added pci wrappers to allocate and pci irq
> 
>  hw/pci/pci.c         | 26 ++++++++++++++++++++++----
>  include/hw/pci/pci.h | 19 +++++++++++++++++++
>  2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 00554a0..fbfd8f7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
>  
>  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>  static void pci_update_mappings(PCIDevice *d);
> -static void pci_set_irq(void *opaque, int irq_num, int level);
> +static void pci_irq_handler(void *opaque, int irq_num, int level);
>  static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
>  static void pci_del_option_rom(PCIDevice *pdev);
>  
> @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
>  {
>      int i;
>      for (i = 0; i < PCI_NUM_PINS; ++i) {
> -        qemu_set_irq(dev->irq[i], 0);
> +        pci_irq_handler(dev, i, 0);
>      }
>  }
>  
> @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      pci_dev->config_read = config_read;
>      pci_dev->config_write = config_write;
>      bus->devices[devfn] = pci_dev;
> -    pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> +    pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
>      pci_dev->version_id = 2; /* Current pci device vmstate version */
>      return pci_dev;
>  }
> @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
>  /* generic PCI irq support */
>  
>  /* 0 <= irq_num <= 3. level must be 0 or 1 */
> -static void pci_set_irq(void *opaque, int irq_num, int level)
> +static void pci_irq_handler(void *opaque, int irq_num, int level)
>  {
>      PCIDevice *pci_dev = opaque;
>      int change;
> @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
>      pci_change_irq_level(pci_dev, irq_num, change);
>  }
>  
> +static inline int pci_intx(PCIDevice *pci_dev)
> +{
> +    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> +}
> +
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> +{
> +    int intx = pci_intx(pci_dev);
> +
> +    return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> +}
> +
> +void pci_set_irq(PCIDevice *pci_dev, int level)
> +{
> +    int intx = pci_intx(pci_dev);
> +    pci_irq_handler(pci_dev, intx, level);
> +}
> +
>  /* Special hooks used by device assignment */
>  void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
>  {
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4b90e5d..df7d316 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
>  PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
>  PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
>  
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> +void pci_set_irq(PCIDevice *pci_dev, int level);
> +
> +static inline void pci_irq_raise(PCIDevice *pci_dev)
> +{
> +    pci_set_irq(pci_dev, 1);
> +}
> +
> +static inline void pci_irq_lower(PCIDevice *pci_dev)
> +{
> +    pci_set_irq(pci_dev, 0);
> +}
> +

I'd like to add that any users of pci_irq_pulse
are immediately suspect. PCI does not work this way.
If you resend this, maybe add a comment that all users
of this should be fixed.

> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> +    pci_irq_lower(pci_dev);
> +    pci_irq_raise(pci_dev);
> +}
> +
>  static inline int pci_is_express(const PCIDevice *d)
>  {
>      return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs

[Marcel Apfelbaum]

On Wed, 2013-10-02 at 15:54 +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > register during device initialization. Devices should not call
> > directly qemu_set_irq and specify the INTx pin on each call.
> > 
> > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > based on PCI_INTERRUPT_PIN.
> > 
> > Added pci_allocate_irq wrapper to be used by devices that
> > still need PCIDevice infrastructure to assert irqs.
> > 
> > Renamed a static method which was named already pci_set_irq.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> > Changes from v1:
> >  - Added pci irq wrappers for all qemu methods
> >    setting irq and not only qemu_set_irq 
> >  - Added pci wrappers to allocate and pci irq
> > 
> >  hw/pci/pci.c         | 26 ++++++++++++++++++++++----
> >  include/hw/pci/pci.h | 19 +++++++++++++++++++
> >  2 files changed, 41 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 00554a0..fbfd8f7 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
> >  
> >  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> >  static void pci_update_mappings(PCIDevice *d);
> > -static void pci_set_irq(void *opaque, int irq_num, int level);
> > +static void pci_irq_handler(void *opaque, int irq_num, int level);
> >  static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> >  static void pci_del_option_rom(PCIDevice *pdev);
> >  
> > @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
> >  {
> >      int i;
> >      for (i = 0; i < PCI_NUM_PINS; ++i) {
> > -        qemu_set_irq(dev->irq[i], 0);
> > +        pci_irq_handler(dev, i, 0);
> >      }
> >  }
> >  
> > @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >      pci_dev->config_read = config_read;
> >      pci_dev->config_write = config_write;
> >      bus->devices[devfn] = pci_dev;
> > -    pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> > +    pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
> >      pci_dev->version_id = 2; /* Current pci device vmstate version */
> >      return pci_dev;
> >  }
> > @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
> >  /* generic PCI irq support */
> >  
> >  /* 0 <= irq_num <= 3. level must be 0 or 1 */
> > -static void pci_set_irq(void *opaque, int irq_num, int level)
> > +static void pci_irq_handler(void *opaque, int irq_num, int level)
> >  {
> >      PCIDevice *pci_dev = opaque;
> >      int change;
> > @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
> >      pci_change_irq_level(pci_dev, irq_num, change);
> >  }
> >  
> > +static inline int pci_intx(PCIDevice *pci_dev)
> > +{
> > +    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> > +}
> > +
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> > +{
> > +    int intx = pci_intx(pci_dev);
> > +
> > +    return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> > +}
> > +
> > +void pci_set_irq(PCIDevice *pci_dev, int level)
> > +{
> > +    int intx = pci_intx(pci_dev);
> > +    pci_irq_handler(pci_dev, intx, level);
> > +}
> > +
> >  /* Special hooks used by device assignment */
> >  void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
> >  {
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 4b90e5d..df7d316 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> >  PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> >  PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
> >  
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> > +void pci_set_irq(PCIDevice *pci_dev, int level);
> > +
> > +static inline void pci_irq_raise(PCIDevice *pci_dev)
> > +{
> > +    pci_set_irq(pci_dev, 1);
> > +}
> > +
> > +static inline void pci_irq_lower(PCIDevice *pci_dev)
> > +{
> > +    pci_set_irq(pci_dev, 0);
> > +}
> > +
> 
> I'd like to add that any users of pci_irq_pulse
> are immediately suspect. PCI does not work this way.
> If you resend this, maybe add a comment that all users
> of this should be fixed.
Sure, thanks!
Marcel

> 
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > +    pci_irq_lower(pci_dev);
> > +    pci_irq_raise(pci_dev);
> > +}
> > +
> >  static inline int pci_is_express(const PCIDevice *d)
> >  {
> >      return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> > -- 
> > 1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Michael S. Tsirkin]

On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> Note: Added RFC because not all affected devices were
>       checked yet. 

What do you have in mind exactly?

> Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> register during device initialization. Devices should not call
> directly qemu_set_irq and specify the INTx pin.
> 
> Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> qemu_irq_lower and qemu_irq_pulse, setting the irq
> based on PCI_INTERRUPT_PIN.
> 
> Added interface to allocate and free single irq.
> Added pci_allocate_irq wrapper to be used by devices that
> still need PCIDevice infrastructure to assert irqs.
> 
> Removed irq field from PCIDevice, not needed anymore.
> 
> Special cases of replacements were done in separate patches,
> all others in one patch "hw: set interrupts using pci irq wrappers"

Looks good to me overall.
Acked-by: Michael S. Tsirkin <mst@redhat.com>


> Changes from v1:
>  - Addressed Michael S. Tsirkin's comments:
>    - pci_set_irq directly calls pci_irq handler
>    - removed irq field from PCIDevice
>  - Added qemu interface to allocate single irq
>  - Added pci wrappers to allocate and free pci irq
>  - Added pci irq wrappers for all qemu methods
>    setting irq and not only qemu_set_irq 
>  - Replace all qemu irq setters with pci
>    wrappers
> 
> Marcel Apfelbaum (9):
>   hw/core: Add interface to allocate and free a single IRQ
>   hw/pci: add pci wrappers for allocating and asserting irqs
>   hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
>   hw/vmxnet3: set interrupts using pci irq wrappers
>   hw/vfio: set interrupts using pci irq wrappers
>   hw/xhci: set interrupts using pci irq wrappers
>   hw: set interrupts using pci irq wrappers
>   hw/pcie: AER and hot-plug events must use device's interrupt
>   hw/pci: removed irq field from PCIDevice
> 
>  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/core/irq.c                  | 16 ++++++++++++++++
>  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/misc/vfio.c                 | 11 ++++++-----
>  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/net/vmxnet3.c               | 13 +++++++++++--
>  hw/pci-bridge/pci_bridge_dev.c |  2 +-
>  hw/pci/pci.c                   | 26 +++++++++++++++++++++-----
>  hw/pci/pcie.c                  |  4 ++--
>  hw/pci/pcie_aer.c              |  4 ++--
>  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              |  2 +-
>  hw/usb/hcd-xhci.c              |  7 ++-----
>  hw/virtio/virtio-pci.c         |  4 ++--
>  include/hw/irq.h               |  7 +++++++
>  include/hw/pci/pci.h           | 22 +++++++++++++++++++---
>  include/hw/pci/pcie.h          | 18 ------------------
>  36 files changed, 127 insertions(+), 78 deletions(-)
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Marcel Apfelbaum]

On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> > Note: Added RFC because not all affected devices were
> >       checked yet. 
> 
> What do you have in mind exactly?
Sanity test for *all* modified devices.
Any other thoughts?

>From self code review, I can say that besides 2 devices
(vmxnet3 and vfio), the code is re-factored with
no side effects.

vmxnet3: I reviewed linux driver code and when using legacy interrupts
         only INTx 0 is used.
vfio: Also here only INTx 0 is used, so the interface selecting
      the interrupt line seems to be unnecessary at least for
      legacy interrupts.

> 
> > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > register during device initialization. Devices should not call
> > directly qemu_set_irq and specify the INTx pin.
> > 
> > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > based on PCI_INTERRUPT_PIN.
> > 
> > Added interface to allocate and free single irq.
> > Added pci_allocate_irq wrapper to be used by devices that
> > still need PCIDevice infrastructure to assert irqs.
> > 
> > Removed irq field from PCIDevice, not needed anymore.
> > 
> > Special cases of replacements were done in separate patches,
> > all others in one patch "hw: set interrupts using pci irq wrappers"
> 
> Looks good to me overall.
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> 
> > Changes from v1:
> >  - Addressed Michael S. Tsirkin's comments:
> >    - pci_set_irq directly calls pci_irq handler
> >    - removed irq field from PCIDevice
> >  - Added qemu interface to allocate single irq
> >  - Added pci wrappers to allocate and free pci irq
> >  - Added pci irq wrappers for all qemu methods
> >    setting irq and not only qemu_set_irq 
> >  - Replace all qemu irq setters with pci
> >    wrappers
> > 
> > Marcel Apfelbaum (9):
> >   hw/core: Add interface to allocate and free a single IRQ
> >   hw/pci: add pci wrappers for allocating and asserting irqs
> >   hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> >   hw/vmxnet3: set interrupts using pci irq wrappers
> >   hw/vfio: set interrupts using pci irq wrappers
> >   hw/xhci: set interrupts using pci irq wrappers
> >   hw: set interrupts using pci irq wrappers
> >   hw/pcie: AER and hot-plug events must use device's interrupt
> >   hw/pci: removed irq field from PCIDevice
> > 
> >  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/core/irq.c                  | 16 ++++++++++++++++
> >  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/misc/vfio.c                 | 11 ++++++-----
> >  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/net/vmxnet3.c               | 13 +++++++++++--
> >  hw/pci-bridge/pci_bridge_dev.c |  2 +-
> >  hw/pci/pci.c                   | 26 +++++++++++++++++++++-----
> >  hw/pci/pcie.c                  |  4 ++--
> >  hw/pci/pcie_aer.c              |  4 ++--
> >  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              |  2 +-
> >  hw/usb/hcd-xhci.c              |  7 ++-----
> >  hw/virtio/virtio-pci.c         |  4 ++--
> >  include/hw/irq.h               |  7 +++++++
> >  include/hw/pci/pci.h           | 22 +++++++++++++++++++---
> >  include/hw/pci/pcie.h          | 18 ------------------
> >  36 files changed, 127 insertions(+), 78 deletions(-)
> > 
> > -- 
> > 1.8.3.1

Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs

[Paolo Bonzini]

Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> +    pci_irq_lower(pci_dev);
> +    pci_irq_raise(pci_dev);
> +}
> +

Why is this in the opposite order, compared to qemu_irq_pulse?

Paolo

Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers

[Alex Williamson]

On Wed, 2013-10-02 at 15:41 +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.
> 
> Save INTx pin into the config register before calling
> pci_set_irq
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  hw/misc/vfio.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

Seems ok, but why not take advantage of the pci_irq_raise/lower()
wrappers?  BTW, with PCI being active low, should those be
assert/deassert to avoid confusion confusion with the actual signal
level?  Thanks,

Alex

> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index a1c08fb..3d7297c 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
>              'A' + vdev->intx.pin);
>  
>      vdev->intx.pending = true;
> -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> +    pci_set_irq(&vdev->pdev, 1);
>      vfio_mmap_set_enabled(vdev, false);
>      if (vdev->intx.mmap_timeout) {
>          timer_mod(vdev->intx.mmap_timer,
> @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
>              vdev->host.bus, vdev->host.slot, vdev->host.function);
>  
>      vdev->intx.pending = false;
> -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +    pci_set_irq(&vdev->pdev, 0);
>      vfio_unmask_intx(vdev);
>  }
>  
> @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
>      qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
>      vfio_mask_intx(vdev);
>      vdev->intx.pending = false;
> -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +    pci_set_irq(&vdev->pdev, 0);
>  
>      /* Get an eventfd for resample/unmask */
>      if (event_notifier_init(&vdev->intx.unmask, 0)) {
> @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
>       */
>      vfio_mask_intx(vdev);
>      vdev->intx.pending = false;
> -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +    pci_set_irq(&vdev->pdev, 0);
>  
>      /* Tell KVM to stop listening for an INTx irqfd */
>      if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
>      vfio_disable_interrupts(vdev);
>  
>      vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> +    pci_config_set_interrupt_pin(vdev->pdev.config, pin);
>  
>  #ifdef CONFIG_KVM
>      /*
> @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
>      vfio_disable_intx_kvm(vdev);
>      vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
>      vdev->intx.pending = false;
> -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +    pci_set_irq(&vdev->pdev, 0);
>      vfio_mmap_set_enabled(vdev, true);
>  
>      fd = event_notifier_get_fd(&vdev->intx.interrupt);

Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin

[Alex Williamson]

On Wed, 2013-10-02 at 16:05 +0300, Marcel Apfelbaum wrote:
> On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> > > Note: Added RFC because not all affected devices were
> > >       checked yet. 
> > 
> > What do you have in mind exactly?
> Sanity test for *all* modified devices.
> Any other thoughts?
> 
> From self code review, I can say that besides 2 devices
> (vmxnet3 and vfio), the code is re-factored with
> no side effects.
> 
> vmxnet3: I reviewed linux driver code and when using legacy interrupts
>          only INTx 0 is used.
> vfio: Also here only INTx 0 is used, so the interface selecting
>       the interrupt line seems to be unnecessary at least for
>       legacy interrupts.

vfio uses the same pin as the physical device, so you can't guarantee
that's always INTA.  Thanks,

Alex

> > 
> > > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > > register during device initialization. Devices should not call
> > > directly qemu_set_irq and specify the INTx pin.
> > > 
> > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > > based on PCI_INTERRUPT_PIN.
> > > 
> > > Added interface to allocate and free single irq.
> > > Added pci_allocate_irq wrapper to be used by devices that
> > > still need PCIDevice infrastructure to assert irqs.
> > > 
> > > Removed irq field from PCIDevice, not needed anymore.
> > > 
> > > Special cases of replacements were done in separate patches,
> > > all others in one patch "hw: set interrupts using pci irq wrappers"
> > 
> > Looks good to me overall.
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > 
> > > Changes from v1:
> > >  - Addressed Michael S. Tsirkin's comments:
> > >    - pci_set_irq directly calls pci_irq handler
> > >    - removed irq field from PCIDevice
> > >  - Added qemu interface to allocate single irq
> > >  - Added pci wrappers to allocate and free pci irq
> > >  - Added pci irq wrappers for all qemu methods
> > >    setting irq and not only qemu_set_irq 
> > >  - Replace all qemu irq setters with pci
> > >    wrappers
> > > 
> > > Marcel Apfelbaum (9):
> > >   hw/core: Add interface to allocate and free a single IRQ
> > >   hw/pci: add pci wrappers for allocating and asserting irqs
> > >   hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> > >   hw/vmxnet3: set interrupts using pci irq wrappers
> > >   hw/vfio: set interrupts using pci irq wrappers
> > >   hw/xhci: set interrupts using pci irq wrappers
> > >   hw: set interrupts using pci irq wrappers
> > >   hw/pcie: AER and hot-plug events must use device's interrupt
> > >   hw/pci: removed irq field from PCIDevice
> > > 
> > >  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/core/irq.c                  | 16 ++++++++++++++++
> > >  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/misc/vfio.c                 | 11 ++++++-----
> > >  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/net/vmxnet3.c               | 13 +++++++++++--
> > >  hw/pci-bridge/pci_bridge_dev.c |  2 +-
> > >  hw/pci/pci.c                   | 26 +++++++++++++++++++++-----
> > >  hw/pci/pcie.c                  |  4 ++--
> > >  hw/pci/pcie_aer.c              |  4 ++--
> > >  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              |  2 +-
> > >  hw/usb/hcd-xhci.c              |  7 ++-----
> > >  hw/virtio/virtio-pci.c         |  4 ++--
> > >  include/hw/irq.h               |  7 +++++++
> > >  include/hw/pci/pci.h           | 22 +++++++++++++++++++---
> > >  include/hw/pci/pcie.h          | 18 ------------------
> > >  36 files changed, 127 insertions(+), 78 deletions(-)
> > > 
> > > -- 
> > > 1.8.3.1
> 
> 
> 

Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs

[Marcel Apfelbaum]

On Wed, 2013-10-02 at 17:21 +0200, Paolo Bonzini wrote:
> Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > +    pci_irq_lower(pci_dev);
> > +    pci_irq_raise(pci_dev);
> > +}
> > +
> 
> Why is this in the opposite order, compared to qemu_irq_pulse?
It is a bug, thank you for catching it!
Marcel

> 
> Paolo

Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

On Wed, 2013-10-02 at 09:58 -0600, Alex Williamson wrote:
> On Wed, 2013-10-02 at 15:41 +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.
> > 
> > Save INTx pin into the config register before calling
> > pci_set_irq
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  hw/misc/vfio.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> Seems ok, but why not take advantage of the pci_irq_raise/lower()
> wrappers?  BTW, with PCI being active low, should those be
> assert/deassert to avoid confusion confusion with the actual signal
> level?  Thanks,

Thanks for the review!

I can use pci_irq_raise/lower(), but I wanted to preserve
the current form, re-factoring:
    qemu_set_irq -> pci_set_irq,
    qemu_irq_lower -> pci_irq_lower
    ...
If you think is worth it, I'll change it. (in all the places)

About assert/deassert instead of lower/raise, I am afraid
it will confuse users having two different set of naming
for interrupts usage.
Is easier to understand that pci_irq_lower behaves the same
as qemu_pci_lower, then pci_irq_deassert.

What do you think?

Thanks,
Marcel

> 
> Alex
> 
> > 
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index a1c08fb..3d7297c 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
> >              'A' + vdev->intx.pin);
> >  
> >      vdev->intx.pending = true;
> > -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> > +    pci_set_irq(&vdev->pdev, 1);
> >      vfio_mmap_set_enabled(vdev, false);
> >      if (vdev->intx.mmap_timeout) {
> >          timer_mod(vdev->intx.mmap_timer,
> > @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
> >              vdev->host.bus, vdev->host.slot, vdev->host.function);
> >  
> >      vdev->intx.pending = false;
> > -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +    pci_set_irq(&vdev->pdev, 0);
> >      vfio_unmask_intx(vdev);
> >  }
> >  
> > @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
> >      qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> >      vfio_mask_intx(vdev);
> >      vdev->intx.pending = false;
> > -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +    pci_set_irq(&vdev->pdev, 0);
> >  
> >      /* Get an eventfd for resample/unmask */
> >      if (event_notifier_init(&vdev->intx.unmask, 0)) {
> > @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
> >       */
> >      vfio_mask_intx(vdev);
> >      vdev->intx.pending = false;
> > -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +    pci_set_irq(&vdev->pdev, 0);
> >  
> >      /* Tell KVM to stop listening for an INTx irqfd */
> >      if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> > @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
> >      vfio_disable_interrupts(vdev);
> >  
> >      vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> > +    pci_config_set_interrupt_pin(vdev->pdev.config, pin);
> >  
> >  #ifdef CONFIG_KVM
> >      /*
> > @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
> >      vfio_disable_intx_kvm(vdev);
> >      vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
> >      vdev->intx.pending = false;
> > -    qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +    pci_set_irq(&vdev->pdev, 0);
> >      vfio_mmap_set_enabled(vdev, true);
> >  
> >      fd = event_notifier_get_fd(&vdev->intx.interrupt);
> 
> 
> 

Re: [Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers

[Gerd Hoffmann]

On Mi, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> --- a/hw/usb/hcd-uhci.c
> +++ b/hw/usb/hcd-uhci.c
> @@ -381,7 +381,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)

That renders s->irq_pin unused.  You can drop the struct member and the
initialization code for it.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers

[Marcel Apfelbaum]

On Mon, 2013-10-07 at 09:02 +0200, Gerd Hoffmann wrote:
> On Mi, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> > --- a/hw/usb/hcd-uhci.c
> > +++ b/hw/usb/hcd-uhci.c
> > @@ -381,7 +381,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)
> 
> That renders s->irq_pin unused.  You can drop the struct member and the
> initialization code for it.
Got it, thanks!
Marcel

> 
> cheers,
>   Gerd
> 
>