All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin
@ 2013-10-07  7:36 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
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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 v2:
 - Addressed Michael S. Tsirkin's comments:
   - Terminate comments by "."
   - Add "fixme" comment to pci_irq_pulse
 - Addressed Paolo Bonzini's comment:
   - fixed implementation of pci_irq_pulse
 - Addressed Alex Williamson's comments
   - replaced pci_irq_raise/lower with
     pci_irq_assert/deassert
   - 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
     
 - Fixed a bug in vmxnet3 (deassert was replaced by an assert)

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: 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              |  6 ++----
 hw/usb/hcd-xhci.c              |  7 ++-----
 hw/virtio/virtio-pci.c         |  4 ++--
 include/hw/irq.h               |  7 +++++++
 include/hw/pci/pci.h           | 26 +++++++++++++++++++++++---
 include/hw/pci/pcie.h          | 18 ------------------
 36 files changed, 132 insertions(+), 81 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 1/8] hw/core: Add interface to allocate and free a single IRQ
  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 ` 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
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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..d08bc02 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 2/8] hw/pci: add pci wrappers for allocating and asserting irqs
  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 ` 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
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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 v2:
 - Addressed Michael S. Tsirkin's comments:
   - Add "fixme" comment to pci_irq_pulse
 - Addressed Alex Williamson's comments
   - replaced pci_irq_raise/lower with
     pci_irq_assert/deassert
 - Addressed Paolo Bonzini's comment:
   - fixed implementation of pci_irq_pulse
 
 hw/pci/pci.c         | 26 ++++++++++++++++++++++----
 include/hw/pci/pci.h | 23 +++++++++++++++++++++++
 2 files changed, 45 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..990342c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -632,6 +632,29 @@ 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_assert(PCIDevice *pci_dev)
+{
+    pci_set_irq(pci_dev, 1);
+}
+
+static inline void pci_irq_deassert(PCIDevice *pci_dev)
+{
+    pci_set_irq(pci_dev, 0);
+}
+
+/*
+ * FIXME: PCI does not work this way.
+ * All the callers to this method should be fixed.
+ */
+static inline void pci_irq_pulse(PCIDevice *pci_dev)
+{
+    pci_irq_assert(pci_dev);
+    pci_irq_deassert(pci_dev);
+}
+
 static inline int pci_is_express(const PCIDevice *d)
 {
     return d->cap_present & QEMU_PCI_CAP_EXPRESS;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 3/8] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
  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 ` Marcel Apfelbaum
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 4/8] hw/vmxnet3: set interrupts using pci irq wrappers Marcel Apfelbaum
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 4/8] hw/vmxnet3: set interrupts using pci irq wrappers
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (2 preceding siblings ...)
  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 ` Marcel Apfelbaum
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 5/8] hw/vfio: " Marcel Apfelbaum
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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

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>
---
Changes from v2:
  - Addressed Alex Williamson's comments
   - replaced calls to pci_set_irq with
     pci_irq_assert/deassert when possible

  - fixed a deassert replaced by an assert by mistake

 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..19687aa 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_irq_assert(d);
     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_irq_deassert(d);
 }
 
 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 5/8] hw/vfio: set interrupts using pci irq wrappers
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (3 preceding siblings ...)
  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 ` Marcel Apfelbaum
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 6/8] hw: " Marcel Apfelbaum
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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>
---
Changes from v2:
 - Addressed Alex Williamson's comments
   - replaced calls to pci_set_irq with
     pci_irq_assert/deassert when possible

 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..9d02e49 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_irq_assert(&vdev->pdev);
     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_irq_deassert(&vdev->pdev);
     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_irq_deassert(&vdev->pdev);
 
     /* 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_irq_deassert(&vdev->pdev);
 
     /* 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_irq_deassert(&vdev->pdev);
     vfio_mmap_set_enabled(vdev, true);
 
     fd = event_notifier_get_fd(&vdev->intx.interrupt);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 6/8] hw: set interrupts using pci irq wrappers
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (4 preceding siblings ...)
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 5/8] hw/vfio: " Marcel Apfelbaum
@ 2013-10-07  7:36 ` Marcel Apfelbaum
  2013-10-07  8:03   ` Michael S. Tsirkin
  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
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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);
 
     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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 7/8] hw/pcie: AER and hot-plug events must use device's interrupt
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (5 preceding siblings ...)
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 6/8] hw: " Marcel Apfelbaum
@ 2013-10-07  7:36 ` Marcel Apfelbaum
  2013-10-07  7:36 ` [Qemu-devel] [PATCH v3 8/8] hw/pci: removed irq field from PCIDevice Marcel Apfelbaum
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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 irq wrappers instead.

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

 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..268a696 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_irq_deassert(dev);
     }
 }
 
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ca762ab..32aa0c6 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_irq_assert(dev);
     }
 }
 
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3 8/8] hw/pci: removed irq field from PCIDevice
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (6 preceding siblings ...)
  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 ` 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
  9 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  7:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
	jasowang, dkoch, keith.busch, alex.williamson, kraxel, stefanha,
	dmitry, pbonzini, afaerber, ehabkost

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 990342c..37ffa53 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/8] hw: set interrupts using pci irq wrappers
  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
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-07  8:03 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, sw, jasowang,
	qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
	stefanha, dmitry, pbonzini, afaerber, ehabkost

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.

>  
>      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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/8] hw: set interrupts using pci irq wrappers
  2013-10-07  8:03   ` Michael S. Tsirkin
@ 2013-10-07  8:04     ` Michael S. Tsirkin
  2013-10-07  8:14       ` Marcel Apfelbaum
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-07  8:04 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, sw, jasowang,
	qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
	stefanha, dmitry, pbonzini, afaerber, ehabkost

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/8] hw: set interrupts using pci irq wrappers
  2013-10-07  8:04     ` Michael S. Tsirkin
@ 2013-10-07  8:14       ` Marcel Apfelbaum
  0 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07  8:14 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, sw, jasowang,
	qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
	stefanha, dmitry, pbonzini, afaerber, ehabkost


> > >  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.
It seems a good idea.  
> > 
> > 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.
> 
I will make another patch(not part of this series)
to be applied on top of this series.

Thanks,
Marcel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (7 preceding siblings ...)
  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 ` Michael S. Tsirkin
  2013-10-08 14:33 ` Michael S. Tsirkin
  9 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-07 10:05 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, sw, jasowang,
	qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
	stefanha, dmitry, pbonzini, afaerber, ehabkost

On Mon, Oct 07, 2013 at 10:36:33AM +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.
> 
> 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"

OK so 
Acked-by: Michael S. Tsirkin <mst@redhat.com>

Let's wait for some acks, then I'll merge this.

> Changes from v2:
>  - Addressed Michael S. Tsirkin's comments:
>    - Terminate comments by "."
>    - Add "fixme" comment to pci_irq_pulse
>  - Addressed Paolo Bonzini's comment:
>    - fixed implementation of pci_irq_pulse
>  - Addressed Alex Williamson's comments
>    - replaced pci_irq_raise/lower with
>      pci_irq_assert/deassert
>    - 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
>      
>  - Fixed a bug in vmxnet3 (deassert was replaced by an assert)
> 
> 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: 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              |  6 ++----
>  hw/usb/hcd-xhci.c              |  7 ++-----
>  hw/virtio/virtio-pci.c         |  4 ++--
>  include/hw/irq.h               |  7 +++++++
>  include/hw/pci/pci.h           | 26 +++++++++++++++++++++++---
>  include/hw/pci/pcie.h          | 18 ------------------
>  36 files changed, 132 insertions(+), 81 deletions(-)
> 
> -- 
> 1.8.3.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin
  2013-10-07  7:36 [Qemu-devel] [PATCH v3 0/8] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
                   ` (8 preceding siblings ...)
  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
  9 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-08 14:33 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, sw, jasowang,
	qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
	stefanha, dmitry, pbonzini, afaerber, ehabkost

On Mon, Oct 07, 2013 at 10:36:33AM +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.
> 
> 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"

Applied, thanks.


> Changes from v2:
>  - Addressed Michael S. Tsirkin's comments:
>    - Terminate comments by "."
>    - Add "fixme" comment to pci_irq_pulse
>  - Addressed Paolo Bonzini's comment:
>    - fixed implementation of pci_irq_pulse
>  - Addressed Alex Williamson's comments
>    - replaced pci_irq_raise/lower with
>      pci_irq_assert/deassert
>    - 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
>      
>  - Fixed a bug in vmxnet3 (deassert was replaced by an assert)
> 
> 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: 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              |  6 ++----
>  hw/usb/hcd-xhci.c              |  7 ++-----
>  hw/virtio/virtio-pci.c         |  4 ++--
>  include/hw/irq.h               |  7 +++++++
>  include/hw/pci/pci.h           | 26 +++++++++++++++++++++++---
>  include/hw/pci/pcie.h          | 18 ------------------
>  36 files changed, 132 insertions(+), 81 deletions(-)
> 
> -- 
> 1.8.3.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-10-08 14:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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.