* [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:50 ` Michael S. Tsirkin
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs Marcel Apfelbaum
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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..f560dea 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -30,6 +30,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
*/
qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n);
+/*
+ * Allocates a single IRQ. The irq is assigned with a handler, an opaque
+ * data and the interrupt number
+ */
+qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
+
/* Extends an Array of IRQs. Old IRQs have their handlers and opaque data
* preserved. New IRQs are assigned the argument handler and opaque data.
*/
@@ -37,6 +43,7 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
void *opaque, int n);
void qemu_free_irqs(qemu_irq *s);
+void qemu_free_irq(qemu_irq irq);
/* Returns a new IRQ with opposite polarity. */
qemu_irq qemu_irq_invert(qemu_irq irq);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ Marcel Apfelbaum
@ 2013-10-02 12:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2013-10-02 12:50 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 Wed, Oct 02, 2013 at 03:41:26PM +0300, Marcel Apfelbaum wrote:
> qemu_allocate_irq returns a single qemu_irq.
> The interface allows to specify an interrupt number.
>
> qemu_free_irq frees it.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> hw/core/irq.c | 16 ++++++++++++++++
> include/hw/irq.h | 7 +++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 2078542..03c8cb3 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -68,6 +68,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n)
> return qemu_extend_irqs(NULL, 0, handler, opaque, n);
> }
>
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
> +{
> + struct IRQState *irq;
> +
> + irq = g_new(struct IRQState, 1);
> + irq->handler = handler;
> + irq->opaque = opaque;
> + irq->n = n;
> +
> + return irq;
> +}
>
> void qemu_free_irqs(qemu_irq *s)
> {
> @@ -75,6 +86,11 @@ void qemu_free_irqs(qemu_irq *s)
> g_free(s);
> }
>
> +void qemu_free_irq(qemu_irq irq)
> +{
> + g_free(irq);
> +}
> +
> static void qemu_notirq(void *opaque, int line, int level)
> {
> struct IRQState *irq = opaque;
> diff --git a/include/hw/irq.h b/include/hw/irq.h
> index 610e6b7..f560dea 100644
> --- a/include/hw/irq.h
> +++ b/include/hw/irq.h
> @@ -30,6 +30,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
> */
> qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n);
>
> +/*
> + * Allocates a single IRQ. The irq is assigned with a handler, an opaque
> + * data and the interrupt number
Add period at end of line :)
no need to repost for this.
> + */
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
> +
> /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data
> * preserved. New IRQs are assigned the argument handler and opaque data.
> */
> @@ -37,6 +43,7 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
> void *opaque, int n);
>
> void qemu_free_irqs(qemu_irq *s);
> +void qemu_free_irq(qemu_irq irq);
>
> /* Returns a new IRQ with opposite polarity. */
> qemu_irq qemu_irq_invert(qemu_irq irq);
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:54 ` Michael S. Tsirkin
2013-10-02 15:21 ` Paolo Bonzini
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 3/9] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init Marcel Apfelbaum
` (7 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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 v1:
- Added pci irq wrappers for all qemu methods
setting irq and not only qemu_set_irq
- Added pci wrappers to allocate and pci irq
hw/pci/pci.c | 26 ++++++++++++++++++++++----
include/hw/pci/pci.h | 19 +++++++++++++++++++
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 00554a0..fbfd8f7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
static void pci_update_mappings(PCIDevice *d);
-static void pci_set_irq(void *opaque, int irq_num, int level);
+static void pci_irq_handler(void *opaque, int irq_num, int level);
static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
static void pci_del_option_rom(PCIDevice *pdev);
@@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
{
int i;
for (i = 0; i < PCI_NUM_PINS; ++i) {
- qemu_set_irq(dev->irq[i], 0);
+ pci_irq_handler(dev, i, 0);
}
}
@@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
pci_dev->config_read = config_read;
pci_dev->config_write = config_write;
bus->devices[devfn] = pci_dev;
- pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
+ pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
pci_dev->version_id = 2; /* Current pci device vmstate version */
return pci_dev;
}
@@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
/* generic PCI irq support */
/* 0 <= irq_num <= 3. level must be 0 or 1 */
-static void pci_set_irq(void *opaque, int irq_num, int level)
+static void pci_irq_handler(void *opaque, int irq_num, int level)
{
PCIDevice *pci_dev = opaque;
int change;
@@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
pci_change_irq_level(pci_dev, irq_num, change);
}
+static inline int pci_intx(PCIDevice *pci_dev)
+{
+ return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
+}
+
+qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
+{
+ int intx = pci_intx(pci_dev);
+
+ return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
+}
+
+void pci_set_irq(PCIDevice *pci_dev, int level)
+{
+ int intx = pci_intx(pci_dev);
+ pci_irq_handler(pci_dev, intx, level);
+}
+
/* Special hooks used by device assignment */
void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
{
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 4b90e5d..df7d316 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
+qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
+void pci_set_irq(PCIDevice *pci_dev, int level);
+
+static inline void pci_irq_raise(PCIDevice *pci_dev)
+{
+ pci_set_irq(pci_dev, 1);
+}
+
+static inline void pci_irq_lower(PCIDevice *pci_dev)
+{
+ pci_set_irq(pci_dev, 0);
+}
+
+static inline void pci_irq_pulse(PCIDevice *pci_dev)
+{
+ pci_irq_lower(pci_dev);
+ pci_irq_raise(pci_dev);
+}
+
static inline int pci_is_express(const PCIDevice *d)
{
return d->cap_present & QEMU_PCI_CAP_EXPRESS;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs Marcel Apfelbaum
@ 2013-10-02 12:54 ` Michael S. Tsirkin
2013-10-02 12:56 ` Marcel Apfelbaum
2013-10-02 15:21 ` Paolo Bonzini
1 sibling, 1 reply; 22+ messages in thread
From: Michael S. Tsirkin @ 2013-10-02 12:54 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 Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> register during device initialization. Devices should not call
> directly qemu_set_irq and specify the INTx pin on each call.
>
> Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> qemu_irq_lower and qemu_irq_pulse, setting the irq
> based on PCI_INTERRUPT_PIN.
>
> Added pci_allocate_irq wrapper to be used by devices that
> still need PCIDevice infrastructure to assert irqs.
>
> Renamed a static method which was named already pci_set_irq.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> Changes from v1:
> - Added pci irq wrappers for all qemu methods
> setting irq and not only qemu_set_irq
> - Added pci wrappers to allocate and pci irq
>
> hw/pci/pci.c | 26 ++++++++++++++++++++++----
> include/hw/pci/pci.h | 19 +++++++++++++++++++
> 2 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 00554a0..fbfd8f7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
>
> static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> static void pci_update_mappings(PCIDevice *d);
> -static void pci_set_irq(void *opaque, int irq_num, int level);
> +static void pci_irq_handler(void *opaque, int irq_num, int level);
> static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> static void pci_del_option_rom(PCIDevice *pdev);
>
> @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
> {
> int i;
> for (i = 0; i < PCI_NUM_PINS; ++i) {
> - qemu_set_irq(dev->irq[i], 0);
> + pci_irq_handler(dev, i, 0);
> }
> }
>
> @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> pci_dev->config_read = config_read;
> pci_dev->config_write = config_write;
> bus->devices[devfn] = pci_dev;
> - pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> + pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
> pci_dev->version_id = 2; /* Current pci device vmstate version */
> return pci_dev;
> }
> @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
> /* generic PCI irq support */
>
> /* 0 <= irq_num <= 3. level must be 0 or 1 */
> -static void pci_set_irq(void *opaque, int irq_num, int level)
> +static void pci_irq_handler(void *opaque, int irq_num, int level)
> {
> PCIDevice *pci_dev = opaque;
> int change;
> @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
> pci_change_irq_level(pci_dev, irq_num, change);
> }
>
> +static inline int pci_intx(PCIDevice *pci_dev)
> +{
> + return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> +}
> +
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> +{
> + int intx = pci_intx(pci_dev);
> +
> + return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> +}
> +
> +void pci_set_irq(PCIDevice *pci_dev, int level)
> +{
> + int intx = pci_intx(pci_dev);
> + pci_irq_handler(pci_dev, intx, level);
> +}
> +
> /* Special hooks used by device assignment */
> void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
> {
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4b90e5d..df7d316 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
>
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> +void pci_set_irq(PCIDevice *pci_dev, int level);
> +
> +static inline void pci_irq_raise(PCIDevice *pci_dev)
> +{
> + pci_set_irq(pci_dev, 1);
> +}
> +
> +static inline void pci_irq_lower(PCIDevice *pci_dev)
> +{
> + pci_set_irq(pci_dev, 0);
> +}
> +
I'd like to add that any users of pci_irq_pulse
are immediately suspect. PCI does not work this way.
If you resend this, maybe add a comment that all users
of this should be fixed.
> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> + pci_irq_lower(pci_dev);
> + pci_irq_raise(pci_dev);
> +}
> +
> static inline int pci_is_express(const PCIDevice *d)
> {
> return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
2013-10-02 12:54 ` Michael S. Tsirkin
@ 2013-10-02 12:56 ` Marcel Apfelbaum
0 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:56 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
On Wed, 2013-10-02 at 15:54 +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > register during device initialization. Devices should not call
> > directly qemu_set_irq and specify the INTx pin on each call.
> >
> > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > based on PCI_INTERRUPT_PIN.
> >
> > Added pci_allocate_irq wrapper to be used by devices that
> > still need PCIDevice infrastructure to assert irqs.
> >
> > Renamed a static method which was named already pci_set_irq.
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> > Changes from v1:
> > - Added pci irq wrappers for all qemu methods
> > setting irq and not only qemu_set_irq
> > - Added pci wrappers to allocate and pci irq
> >
> > hw/pci/pci.c | 26 ++++++++++++++++++++++----
> > include/hw/pci/pci.h | 19 +++++++++++++++++++
> > 2 files changed, 41 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 00554a0..fbfd8f7 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
> >
> > static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> > static void pci_update_mappings(PCIDevice *d);
> > -static void pci_set_irq(void *opaque, int irq_num, int level);
> > +static void pci_irq_handler(void *opaque, int irq_num, int level);
> > static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> > static void pci_del_option_rom(PCIDevice *pdev);
> >
> > @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
> > {
> > int i;
> > for (i = 0; i < PCI_NUM_PINS; ++i) {
> > - qemu_set_irq(dev->irq[i], 0);
> > + pci_irq_handler(dev, i, 0);
> > }
> > }
> >
> > @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > pci_dev->config_read = config_read;
> > pci_dev->config_write = config_write;
> > bus->devices[devfn] = pci_dev;
> > - pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> > + pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
> > pci_dev->version_id = 2; /* Current pci device vmstate version */
> > return pci_dev;
> > }
> > @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
> > /* generic PCI irq support */
> >
> > /* 0 <= irq_num <= 3. level must be 0 or 1 */
> > -static void pci_set_irq(void *opaque, int irq_num, int level)
> > +static void pci_irq_handler(void *opaque, int irq_num, int level)
> > {
> > PCIDevice *pci_dev = opaque;
> > int change;
> > @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int level)
> > pci_change_irq_level(pci_dev, irq_num, change);
> > }
> >
> > +static inline int pci_intx(PCIDevice *pci_dev)
> > +{
> > + return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> > +}
> > +
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> > +{
> > + int intx = pci_intx(pci_dev);
> > +
> > + return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> > +}
> > +
> > +void pci_set_irq(PCIDevice *pci_dev, int level)
> > +{
> > + int intx = pci_intx(pci_dev);
> > + pci_irq_handler(pci_dev, intx, level);
> > +}
> > +
> > /* Special hooks used by device assignment */
> > void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn route_intx_to_irq)
> > {
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 4b90e5d..df7d316 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> > PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> > PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
> >
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> > +void pci_set_irq(PCIDevice *pci_dev, int level);
> > +
> > +static inline void pci_irq_raise(PCIDevice *pci_dev)
> > +{
> > + pci_set_irq(pci_dev, 1);
> > +}
> > +
> > +static inline void pci_irq_lower(PCIDevice *pci_dev)
> > +{
> > + pci_set_irq(pci_dev, 0);
> > +}
> > +
>
> I'd like to add that any users of pci_irq_pulse
> are immediately suspect. PCI does not work this way.
> If you resend this, maybe add a comment that all users
> of this should be fixed.
Sure, thanks!
Marcel
>
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > + pci_irq_lower(pci_dev);
> > + pci_irq_raise(pci_dev);
> > +}
> > +
> > static inline int pci_is_express(const PCIDevice *d)
> > {
> > return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> > --
> > 1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs Marcel Apfelbaum
2013-10-02 12:54 ` Michael S. Tsirkin
@ 2013-10-02 15:21 ` Paolo Bonzini
2013-10-02 22:03 ` Marcel Apfelbaum
1 sibling, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2013-10-02 15:21 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
stefanha, dmitry, afaerber, ehabkost
Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> + pci_irq_lower(pci_dev);
> + pci_irq_raise(pci_dev);
> +}
> +
Why is this in the opposite order, compared to qemu_irq_pulse?
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
2013-10-02 15:21 ` Paolo Bonzini
@ 2013-10-02 22:03 ` Marcel Apfelbaum
0 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 22:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, alex.williamson, kraxel,
stefanha, dmitry, afaerber, ehabkost
On Wed, 2013-10-02 at 17:21 +0200, Paolo Bonzini wrote:
> Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > + pci_irq_lower(pci_dev);
> > + pci_irq_raise(pci_dev);
> > +}
> > +
>
> Why is this in the opposite order, compared to qemu_irq_pulse?
It is a bug, thank you for catching it!
Marcel
>
> Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH RFC v2 3/9] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 4/9] hw/vmxnet3: set interrupts using pci irq wrappers Marcel Apfelbaum
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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] 22+ messages in thread* [Qemu-devel] [PATCH RFC v2 4/9] hw/vmxnet3: set interrupts using pci irq wrappers
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (2 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 3/9] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: " Marcel Apfelbaum
` (5 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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 set.
An assert is used to ensure that intx received
from the quest OS corresponds to PCI_INTERRUPT_PIN.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/net/vmxnet3.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 49c2466..674ee7a 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -336,7 +336,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State *s, uint32_t int_idx)
}
VMW_IRPRN("Asserting line for interrupt %u", int_idx);
- qemu_set_irq(d->irq[int_idx], 1);
+ pci_set_irq(d, 1);
return true;
}
@@ -356,7 +356,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State *s, int lidx)
assert(!s->msi_used || !msi_enabled(d));
VMW_IRPRN("Deasserting line for interrupt %u", lidx);
- qemu_set_irq(d->irq[lidx], 0);
+ pci_set_irq(d, 1);
}
static void vmxnet3_update_interrupt_line_state(VMXNET3State *s, int lidx)
@@ -1299,6 +1299,12 @@ static void vmxnet3_update_features(VMXNET3State *s)
}
}
+static bool vmxnet3_verify_intx(VMXNET3State *s, int intx)
+{
+ return s->msix_used || s->msi_used || (intx ==
+ (pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1));
+}
+
static void vmxnet3_activate_device(VMXNET3State *s)
{
int i;
@@ -1332,6 +1338,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
s->event_int_idx =
VMXNET3_READ_DRV_SHARED8(s->drv_shmem, devRead.intrConf.eventIntrIdx);
+ assert(vmxnet3_verify_intx(s, s->event_int_idx));
VMW_CFPRN("Events interrupt line is %u", s->event_int_idx);
s->auto_int_masking =
@@ -1364,6 +1371,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
/* Read interrupt number for this TX queue */
s->txq_descr[i].intr_idx =
VMXNET3_READ_TX_QUEUE_DESCR8(qdescr_pa, conf.intrIdx);
+ assert(vmxnet3_verify_intx(s, s->txq_descr[i].intr_idx));
VMW_CFPRN("TX Queue %d interrupt: %d", i, s->txq_descr[i].intr_idx);
@@ -1411,6 +1419,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
/* Read interrupt number for this RX queue */
s->rxq_descr[i].intr_idx =
VMXNET3_READ_TX_QUEUE_DESCR8(qd_pa, conf.intrIdx);
+ assert(vmxnet3_verify_intx(s, s->rxq_descr[i].intr_idx));
VMW_CFPRN("RX Queue %d interrupt: %d", i, s->rxq_descr[i].intr_idx);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (3 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 4/9] hw/vmxnet3: set interrupts using pci irq wrappers Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 15:58 ` Alex Williamson
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 6/9] hw/xhci: " Marcel Apfelbaum
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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>
---
hw/misc/vfio.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index a1c08fb..3d7297c 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
'A' + vdev->intx.pin);
vdev->intx.pending = true;
- qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
+ pci_set_irq(&vdev->pdev, 1);
vfio_mmap_set_enabled(vdev, false);
if (vdev->intx.mmap_timeout) {
timer_mod(vdev->intx.mmap_timer,
@@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
vdev->host.bus, vdev->host.slot, vdev->host.function);
vdev->intx.pending = false;
- qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+ pci_set_irq(&vdev->pdev, 0);
vfio_unmask_intx(vdev);
}
@@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
vfio_mask_intx(vdev);
vdev->intx.pending = false;
- qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+ pci_set_irq(&vdev->pdev, 0);
/* Get an eventfd for resample/unmask */
if (event_notifier_init(&vdev->intx.unmask, 0)) {
@@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
*/
vfio_mask_intx(vdev);
vdev->intx.pending = false;
- qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+ pci_set_irq(&vdev->pdev, 0);
/* Tell KVM to stop listening for an INTx irqfd */
if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
@@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
vfio_disable_interrupts(vdev);
vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
+ pci_config_set_interrupt_pin(vdev->pdev.config, pin);
#ifdef CONFIG_KVM
/*
@@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
vfio_disable_intx_kvm(vdev);
vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
vdev->intx.pending = false;
- qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+ pci_set_irq(&vdev->pdev, 0);
vfio_mmap_set_enabled(vdev, true);
fd = event_notifier_get_fd(&vdev->intx.interrupt);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: " Marcel Apfelbaum
@ 2013-10-02 15:58 ` Alex Williamson
2013-10-02 22:16 ` Marcel Apfelbaum
0 siblings, 1 reply; 22+ messages in thread
From: Alex Williamson @ 2013-10-02 15:58 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, kraxel, stefanha,
dmitry, pbonzini, afaerber, ehabkost
On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> pci_set_irq and the other pci irq wrappers use
> PCI_INTERRUPT_PIN config register to compute device
> INTx pin to assert/deassert.
>
> Save INTx pin into the config register before calling
> pci_set_irq
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> hw/misc/vfio.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Seems ok, but why not take advantage of the pci_irq_raise/lower()
wrappers? BTW, with PCI being active low, should those be
assert/deassert to avoid confusion confusion with the actual signal
level? Thanks,
Alex
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index a1c08fb..3d7297c 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
> 'A' + vdev->intx.pin);
>
> vdev->intx.pending = true;
> - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> + pci_set_irq(&vdev->pdev, 1);
> vfio_mmap_set_enabled(vdev, false);
> if (vdev->intx.mmap_timeout) {
> timer_mod(vdev->intx.mmap_timer,
> @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
> vdev->host.bus, vdev->host.slot, vdev->host.function);
>
> vdev->intx.pending = false;
> - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> + pci_set_irq(&vdev->pdev, 0);
> vfio_unmask_intx(vdev);
> }
>
> @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
> qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> vfio_mask_intx(vdev);
> vdev->intx.pending = false;
> - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> + pci_set_irq(&vdev->pdev, 0);
>
> /* Get an eventfd for resample/unmask */
> if (event_notifier_init(&vdev->intx.unmask, 0)) {
> @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
> */
> vfio_mask_intx(vdev);
> vdev->intx.pending = false;
> - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> + pci_set_irq(&vdev->pdev, 0);
>
> /* Tell KVM to stop listening for an INTx irqfd */
> if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
> vfio_disable_interrupts(vdev);
>
> vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> + pci_config_set_interrupt_pin(vdev->pdev.config, pin);
>
> #ifdef CONFIG_KVM
> /*
> @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
> vfio_disable_intx_kvm(vdev);
> vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
> vdev->intx.pending = false;
> - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> + pci_set_irq(&vdev->pdev, 0);
> vfio_mmap_set_enabled(vdev, true);
>
> fd = event_notifier_get_fd(&vdev->intx.interrupt);
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
2013-10-02 15:58 ` Alex Williamson
@ 2013-10-02 22:16 ` Marcel Apfelbaum
0 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 22:16 UTC (permalink / raw)
To: Alex Williamson
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, kraxel, stefanha,
dmitry, pbonzini, afaerber, ehabkost
On Wed, 2013-10-02 at 09:58 -0600, Alex Williamson wrote:
> On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> > pci_set_irq and the other pci irq wrappers use
> > PCI_INTERRUPT_PIN config register to compute device
> > INTx pin to assert/deassert.
> >
> > Save INTx pin into the config register before calling
> > pci_set_irq
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> > hw/misc/vfio.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
>
> Seems ok, but why not take advantage of the pci_irq_raise/lower()
> wrappers? BTW, with PCI being active low, should those be
> assert/deassert to avoid confusion confusion with the actual signal
> level? Thanks,
Thanks for the review!
I can use pci_irq_raise/lower(), but I wanted to preserve
the current form, re-factoring:
qemu_set_irq -> pci_set_irq,
qemu_irq_lower -> pci_irq_lower
...
If you think is worth it, I'll change it. (in all the places)
About assert/deassert instead of lower/raise, I am afraid
it will confuse users having two different set of naming
for interrupts usage.
Is easier to understand that pci_irq_lower behaves the same
as qemu_pci_lower, then pci_irq_deassert.
What do you think?
Thanks,
Marcel
>
> Alex
>
> >
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index a1c08fb..3d7297c 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
> > 'A' + vdev->intx.pin);
> >
> > vdev->intx.pending = true;
> > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> > + pci_set_irq(&vdev->pdev, 1);
> > vfio_mmap_set_enabled(vdev, false);
> > if (vdev->intx.mmap_timeout) {
> > timer_mod(vdev->intx.mmap_timer,
> > @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
> > vdev->host.bus, vdev->host.slot, vdev->host.function);
> >
> > vdev->intx.pending = false;
> > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > + pci_set_irq(&vdev->pdev, 0);
> > vfio_unmask_intx(vdev);
> > }
> >
> > @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
> > qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> > vfio_mask_intx(vdev);
> > vdev->intx.pending = false;
> > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > + pci_set_irq(&vdev->pdev, 0);
> >
> > /* Get an eventfd for resample/unmask */
> > if (event_notifier_init(&vdev->intx.unmask, 0)) {
> > @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
> > */
> > vfio_mask_intx(vdev);
> > vdev->intx.pending = false;
> > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > + pci_set_irq(&vdev->pdev, 0);
> >
> > /* Tell KVM to stop listening for an INTx irqfd */
> > if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> > @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
> > vfio_disable_interrupts(vdev);
> >
> > vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> > + pci_config_set_interrupt_pin(vdev->pdev.config, pin);
> >
> > #ifdef CONFIG_KVM
> > /*
> > @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
> > vfio_disable_intx_kvm(vdev);
> > vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
> > vdev->intx.pending = false;
> > - qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > + pci_set_irq(&vdev->pdev, 0);
> > vfio_mmap_set_enabled(vdev, true);
> >
> > fd = event_notifier_get_fd(&vdev->intx.interrupt);
>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH RFC v2 6/9] hw/xhci: set interrupts using pci irq wrappers
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (4 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: " Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 7/9] hw: " Marcel Apfelbaum
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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.
Removed irq field from xhci state.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/usb/hcd-xhci.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 469c24d..54d6842 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -449,7 +449,6 @@ struct XHCIState {
/*< public >*/
USBBus bus;
- qemu_irq irq;
MemoryRegion mem;
MemoryRegion mem_cap;
MemoryRegion mem_oper;
@@ -739,7 +738,7 @@ static void xhci_intx_update(XHCIState *xhci)
}
trace_usb_xhci_irq_intx(level);
- qemu_set_irq(xhci->irq, level);
+ pci_set_irq(pci_dev, level);
}
static void xhci_msix_update(XHCIState *xhci, int v)
@@ -797,7 +796,7 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
if (v == 0) {
trace_usb_xhci_irq_intx(1);
- qemu_set_irq(xhci->irq, 1);
+ pci_set_irq(pci_dev, 1);
}
}
@@ -3433,8 +3432,6 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
- xhci->irq = dev->irq[0];
-
memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
"capabilities", LEN_CAP);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (5 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 6/9] hw/xhci: " Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-07 7:02 ` Gerd Hoffmann
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 8/9] hw/pcie: AER and hot-plug events must use device's interrupt Marcel Apfelbaum
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/audio/ac97.c | 4 ++--
hw/audio/es1370.c | 4 ++--
hw/audio/intel-hda.c | 2 +-
hw/block/nvme.c | 2 +-
hw/char/serial-pci.c | 5 +++--
hw/char/tpci200.c | 8 ++++----
hw/display/qxl.c | 2 +-
hw/ide/cmd646.c | 2 +-
hw/ide/ich.c | 3 ++-
hw/isa/vt82c686.c | 2 +-
hw/misc/ivshmem.c | 2 +-
hw/net/e1000.c | 2 +-
hw/net/eepro100.c | 4 ++--
hw/net/ne2000.c | 3 ++-
hw/net/pcnet-pci.c | 3 ++-
hw/net/rtl8139.c | 2 +-
hw/pci/shpc.c | 2 +-
hw/scsi/esp-pci.c | 3 ++-
hw/scsi/lsi53c895a.c | 2 +-
hw/scsi/megasas.c | 6 +++---
hw/scsi/vmw_pvscsi.c | 2 +-
hw/usb/hcd-ehci-pci.c | 2 +-
hw/usb/hcd-ohci.c | 2 +-
hw/usb/hcd-uhci.c | 2 +-
hw/virtio/virtio-pci.c | 4 ++--
25 files changed, 40 insertions(+), 35 deletions(-)
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 01b4dfb..1f94b19 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -280,12 +280,12 @@ static void update_sr (AC97LinkState *s, AC97BusMasterRegs *r, uint32_t new_sr)
if (level) {
s->glob_sta |= masks[r - s->bm_regs];
dolog ("set irq level=1\n");
- qemu_set_irq (s->dev.irq[0], 1);
+ pci_set_irq(&s->dev, 1);
}
else {
s->glob_sta &= ~masks[r - s->bm_regs];
dolog ("set irq level=0\n");
- qemu_set_irq (s->dev.irq[0], 0);
+ pci_set_irq(&s->dev, 0);
}
}
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index adb66ce..3dbd12b 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -323,7 +323,7 @@ static void es1370_update_status (ES1370State *s, uint32_t new_status)
else {
s->status = new_status & ~STAT_INTR;
}
- qemu_set_irq (s->dev.irq[0], !!level);
+ pci_set_irq(&s->dev, !!level);
}
static void es1370_reset (ES1370State *s)
@@ -349,7 +349,7 @@ static void es1370_reset (ES1370State *s)
s->dac_voice[i] = NULL;
}
}
- qemu_irq_lower (s->dev.irq[0]);
+ pci_irq_lower(&s->dev);
}
static void es1370_maybe_lower_irq (ES1370State *s, uint32_t sctl)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index a6666c6..4327264 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -269,7 +269,7 @@ static void intel_hda_update_irq(IntelHDAState *d)
msi_notify(&d->pci, 0);
}
} else {
- qemu_set_irq(d->pci.irq[0], level);
+ pci_set_irq(&d->pci, level);
}
}
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5dee229..2882ffe 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -69,7 +69,7 @@ static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
if (msix_enabled(&(n->parent_obj))) {
msix_notify(&(n->parent_obj), cq->vector);
} else {
- qemu_irq_pulse(n->parent_obj.irq[0]);
+ pci_irq_pulse(&n->parent_obj);
}
}
}
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index aec6705..991c99f 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -61,7 +61,7 @@ static int serial_pci_init(PCIDevice *dev)
}
pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
- s->irq = pci->dev.irq[0];
+ s->irq = pci_allocate_irq(&pci->dev);
memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s, "serial", 8);
pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
@@ -79,7 +79,7 @@ static void multi_serial_irq_mux(void *opaque, int n, int level)
pending = 1;
}
}
- qemu_set_irq(pci->dev.irq[0], pending);
+ pci_set_irq(&pci->dev, pending);
}
static int multi_serial_pci_init(PCIDevice *dev)
@@ -132,6 +132,7 @@ static void serial_pci_exit(PCIDevice *dev)
serial_exit_core(s);
memory_region_destroy(&s->io);
+ qemu_free_irq(s->irq);
}
static void multi_serial_pci_exit(PCIDevice *dev)
diff --git a/hw/char/tpci200.c b/hw/char/tpci200.c
index e04ff26..84831c1 100644
--- a/hw/char/tpci200.c
+++ b/hw/char/tpci200.c
@@ -134,8 +134,8 @@ static void tpci200_set_irq(void *opaque, int intno, int level)
/* Check if the interrupt is edge sensitive */
if (dev->ctrl[ip_n] & CTRL_INT_EDGE(intno)) {
if (level) {
- qemu_set_irq(dev->dev.irq[0], !dev->int_set);
- qemu_set_irq(dev->dev.irq[0], dev->int_set);
+ pci_set_irq(&dev->dev, !dev->int_set);
+ pci_set_irq(&dev->dev, dev->int_set);
}
} else {
unsigned i, j;
@@ -153,10 +153,10 @@ static void tpci200_set_irq(void *opaque, int intno, int level)
}
if (level_status && !dev->int_set) {
- qemu_irq_raise(dev->dev.irq[0]);
+ pci_irq_raise(&dev->dev);
dev->int_set = 1;
} else if (!level_status && dev->int_set) {
- qemu_irq_lower(dev->dev.irq[0]);
+ pci_irq_lower(&dev->dev);
dev->int_set = 0;
}
}
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ee2db0d..678ad38 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1101,7 +1101,7 @@ static void qxl_update_irq(PCIQXLDevice *d)
uint32_t pending = le32_to_cpu(d->ram->int_pending);
uint32_t mask = le32_to_cpu(d->ram->int_mask);
int level = !!(pending & mask);
- qemu_set_irq(d->pci.irq[0], level);
+ pci_set_irq(&d->pci, level);
qxl_ring_set_dirty(d);
}
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 0500a7a..a8e35fe 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -230,7 +230,7 @@ static void cmd646_update_irq(PCIIDEState *d)
!(pd->config[MRDMODE] & MRDMODE_BLK_CH0)) ||
((pd->config[MRDMODE] & MRDMODE_INTR_CH1) &&
!(pd->config[MRDMODE] & MRDMODE_BLK_CH1));
- qemu_set_irq(pd->irq[0], pci_level);
+ pci_set_irq(pd, pci_level);
}
/* the PCI irq level is the logical OR of the two channels */
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index bff952b..1c7c058 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -116,7 +116,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
dev->config[0x90] = 1 << 6; /* Address Map Register - AHCI mode */
msi_init(dev, 0x50, 1, true, false);
- d->ahci.irq = dev->irq[0];
+ d->ahci.irq = pci_allocate_irq(dev);
pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
&d->ahci.idp);
@@ -145,6 +145,7 @@ static void pci_ich9_uninit(PCIDevice *dev)
msi_uninit(dev);
ahci_uninit(&d->ahci);
+ qemu_free_irq(d->ahci.irq);
}
static void ich_ahci_class_init(ObjectClass *klass, void *data)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8fe4fcb..5fb8086 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -185,7 +185,7 @@ static void pm_update_sci(VT686PMState *s)
ACPI_BITMASK_POWER_BUTTON_ENABLE |
ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
ACPI_BITMASK_TIMER_ENABLE)) != 0);
- qemu_set_irq(s->dev.irq[0], sci_level);
+ pci_set_irq(&s->dev, sci_level);
/* schedule a timer interruption if needed */
acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) &&
!(pmsts & ACPI_BITMASK_TIMER_STATUS));
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 2838866..8d144ba 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -133,7 +133,7 @@ static void ivshmem_update_irq(IVShmemState *s, int val)
isr ? 1 : 0, s->intrstatus, s->intrmask);
}
- qemu_set_irq(d->irq[0], (isr != 0));
+ pci_set_irq(d, (isr != 0));
}
static void ivshmem_IntrMask_write(IVShmemState *s, uint32_t val)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 151d25e..c497bad 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -328,7 +328,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
}
s->mit_irq_level = (pending_ints != 0);
- qemu_set_irq(d->irq[0], s->mit_irq_level);
+ pci_set_irq(d, s->mit_irq_level);
}
static void
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index ffa60d5..0eb3f62 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -409,7 +409,7 @@ static void disable_interrupt(EEPRO100State * s)
{
if (s->int_stat) {
TRACE(INT, logout("interrupt disabled\n"));
- qemu_irq_lower(s->dev.irq[0]);
+ pci_irq_lower(&s->dev);
s->int_stat = 0;
}
}
@@ -418,7 +418,7 @@ static void enable_interrupt(EEPRO100State * s)
{
if (!s->int_stat) {
TRACE(INT, logout("interrupt enabled\n"));
- qemu_irq_raise(s->dev.irq[0]);
+ pci_irq_raise(&s->dev);
s->int_stat = 1;
}
}
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index c961258..a94cf74 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -731,7 +731,7 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
s = &d->ne2000;
ne2000_setup_io(s, DEVICE(pci_dev), 0x100);
pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
- s->irq = d->dev.irq[0];
+ s->irq = pci_allocate_irq(&d->dev);
qemu_macaddr_default_if_unset(&s->c.macaddr);
ne2000_reset(s);
@@ -752,6 +752,7 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
memory_region_destroy(&s->io);
qemu_del_nic(s->nic);
+ qemu_free_irq(s->irq);
}
static Property ne2000_properties[] = {
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index 865f2f0..6a5d806 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -282,6 +282,7 @@ static void pci_pcnet_uninit(PCIDevice *dev)
{
PCIPCNetState *d = PCI_PCNET(dev);
+ qemu_free_irq(d->state.irq);
memory_region_destroy(&d->state.mmio);
memory_region_destroy(&d->io_bar);
timer_del(d->state.poll_timer);
@@ -331,7 +332,7 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
pci_register_bar(pci_dev, 1, 0, &s->mmio);
- s->irq = pci_dev->irq[0];
+ s->irq = pci_allocate_irq(pci_dev);
s->phys_mem_read = pci_physical_memory_read;
s->phys_mem_write = pci_physical_memory_write;
s->dma_opaque = pci_dev;
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index c31199f..7d72b21 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -716,7 +716,7 @@ static void rtl8139_update_irq(RTL8139State *s)
DPRINTF("Set IRQ to %d (%04x %04x)\n", isr ? 1 : 0, s->IntrStatus,
s->IntrMask);
- qemu_set_irq(d->irq[0], (isr != 0));
+ pci_set_irq(d, (isr != 0));
}
static int rtl8139_RxWrap(RTL8139State *s)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index eb092fd..0bbd36e 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -172,7 +172,7 @@ static void shpc_interrupt_update(PCIDevice *d)
if (msi_enabled(d) && shpc->msi_requested != level)
msi_notify(d, 0);
else
- qemu_set_irq(d->irq[0], level);
+ pci_set_irq(d, level);
shpc->msi_requested = level;
}
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 99bf8ec..48c8b82 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -361,7 +361,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
"esp-io", 0x80);
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
- s->irq = dev->irq[0];
+ s->irq = pci_allocate_irq(dev);
scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
if (!d->hotplugged) {
@@ -378,6 +378,7 @@ static void esp_pci_scsi_uninit(PCIDevice *d)
{
PCIESPState *pci = PCI_ESP(d);
+ qemu_free_irq(pci->esp.irq);
memory_region_destroy(&pci->io);
}
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 36e5f50..cb30414 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -437,7 +437,7 @@ static void lsi_update_irq(LSIState *s)
level, s->dstat, s->sist1, s->sist0);
last_level = level;
}
- qemu_set_irq(d->irq[0], level);
+ pci_set_irq(d, level);
if (!level && lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON)) {
DPRINTF("Handled IRQs & disconnected, looking for pending "
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 09b51b3..699ca53 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -535,7 +535,7 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context)
msix_notify(pci_dev, 0);
} else {
trace_megasas_irq_raise();
- qemu_irq_raise(pci_dev->irq[0]);
+ pci_irq_raise(pci_dev);
}
}
} else {
@@ -1936,7 +1936,7 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
s->intr_mask = val;
if (!megasas_intr_enabled(s) && !msix_enabled(pci_dev)) {
trace_megasas_irq_lower();
- qemu_irq_lower(pci_dev->irq[0]);
+ pci_irq_lower(pci_dev);
}
if (megasas_intr_enabled(s)) {
trace_megasas_intr_enabled();
@@ -1952,7 +1952,7 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
stl_le_phys(s->producer_pa, s->reply_queue_head);
if (!msix_enabled(pci_dev)) {
trace_megasas_irq_lower();
- qemu_irq_lower(pci_dev->irq[0]);
+ pci_irq_lower(pci_dev);
}
}
break;
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 819d671..94b328f 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -330,7 +330,7 @@ pvscsi_update_irq_status(PVSCSIState *s)
return;
}
- qemu_set_irq(d->irq[0], !!should_raise);
+ pci_set_irq(d, !!should_raise);
}
static void
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 4d21a0b..0c98594 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -60,7 +60,7 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
pci_conf[0x6e] = 0x00;
pci_conf[0x6f] = 0xc0; /* USBLEFCTLSTS */
- s->irq = dev->irq[3];
+ s->irq = pci_allocate_irq(dev);
s->as = pci_get_address_space(dev);
usb_ehci_realize(s, DEVICE(dev), NULL);
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 35f0878..2b36ee5 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1944,7 +1944,7 @@ static int usb_ohci_initfn_pci(PCIDevice *dev)
pci_get_address_space(dev)) != 0) {
return -1;
}
- ohci->state.irq = dev->irq[0];
+ ohci->state.irq = pci_allocate_irq(dev);
pci_register_bar(dev, 0, 0, &ohci->state.mem);
return 0;
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index becc7fa..075046e 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -381,7 +381,7 @@ static void uhci_update_irq(UHCIState *s)
} else {
level = 0;
}
- qemu_set_irq(s->dev.irq[s->irq_pin], level);
+ pci_set_irq(&s->dev, level);
}
static void uhci_reset(void *opaque)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4825802..6b84c4a 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -116,7 +116,7 @@ static void virtio_pci_notify(DeviceState *d, uint16_t vector)
if (msix_enabled(&proxy->pci_dev))
msix_notify(&proxy->pci_dev, vector);
else
- qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1);
+ pci_set_irq(&proxy->pci_dev, proxy->vdev->isr & 1);
}
static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
@@ -362,7 +362,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
/* reading from the ISR also clears it. */
ret = vdev->isr;
vdev->isr = 0;
- qemu_set_irq(proxy->pci_dev.irq[0], 0);
+ pci_set_irq(&proxy->pci_dev, 0);
break;
case VIRTIO_MSI_CONFIG_VECTOR:
ret = vdev->config_vector;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 7/9] hw: " Marcel Apfelbaum
@ 2013-10-07 7:02 ` Gerd Hoffmann
2013-10-07 7:13 ` Marcel Apfelbaum
0 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2013-10-07 7:02 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, alex.williamson,
stefanha, dmitry, pbonzini, afaerber, ehabkost
On Mi, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> --- a/hw/usb/hcd-uhci.c
> +++ b/hw/usb/hcd-uhci.c
> @@ -381,7 +381,7 @@ static void uhci_update_irq(UHCIState *s)
> } else {
> level = 0;
> }
> - qemu_set_irq(s->dev.irq[s->irq_pin], level);
> + pci_set_irq(&s->dev, level);
> }
>
> static void uhci_reset(void *opaque)
That renders s->irq_pin unused. You can drop the struct member and the
initialization code for it.
cheers,
Gerd
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 7/9] hw: set interrupts using pci irq wrappers
2013-10-07 7:02 ` Gerd Hoffmann
@ 2013-10-07 7:13 ` Marcel Apfelbaum
0 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-07 7:13 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony, mst, sw,
jasowang, qemu-devel, dkoch, keith.busch, alex.williamson,
stefanha, dmitry, pbonzini, afaerber, ehabkost
On Mon, 2013-10-07 at 09:02 +0200, Gerd Hoffmann wrote:
> On Mi, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> > --- a/hw/usb/hcd-uhci.c
> > +++ b/hw/usb/hcd-uhci.c
> > @@ -381,7 +381,7 @@ static void uhci_update_irq(UHCIState *s)
> > } else {
> > level = 0;
> > }
> > - qemu_set_irq(s->dev.irq[s->irq_pin], level);
> > + pci_set_irq(&s->dev, level);
> > }
> >
> > static void uhci_reset(void *opaque)
>
> That renders s->irq_pin unused. You can drop the struct member and the
> initialization code for it.
Got it, thanks!
Marcel
>
> cheers,
> Gerd
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH RFC v2 8/9] hw/pcie: AER and hot-plug events must use device's interrupt
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (6 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 7/9] hw: " Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 9/9] hw/pci: removed irq field from PCIDevice Marcel Apfelbaum
2013-10-02 12:58 ` [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Michael S. Tsirkin
9 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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_set_irq wrapper instead.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/pci/pcie.c | 4 ++--
hw/pci/pcie_aer.c | 4 ++--
include/hw/pci/pcie.h | 18 ------------------
3 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 50af3c1..d4dd005 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -187,7 +187,7 @@ static void hotplug_event_notify(PCIDevice *dev)
} else if (msi_enabled(dev)) {
msi_notify(dev, pcie_cap_flags_get_vector(dev));
} else {
- qemu_set_irq(dev->irq[dev->exp.hpev_intx], dev->exp.hpev_notified);
+ pci_set_irq(dev, dev->exp.hpev_notified);
}
}
@@ -195,7 +195,7 @@ static void hotplug_event_clear(PCIDevice *dev)
{
hotplug_event_update_event_status(dev);
if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
- qemu_set_irq(dev->irq[dev->exp.hpev_intx], 0);
+ pci_set_irq(dev, 0);
}
}
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ca762ab..03b1e1f 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -285,7 +285,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
} else if (msi_enabled(dev)) {
msi_notify(dev, pcie_aer_root_get_vector(dev));
} else {
- qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
+ pci_set_irq(dev, 1);
}
}
@@ -768,7 +768,7 @@ void pcie_aer_root_write_config(PCIDevice *dev,
uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
/* 6.2.4.1.2 Interrupt Generation */
if (!msix_enabled(dev) && !msi_enabled(dev)) {
- qemu_set_irq(dev->irq[dev->exp.aer_intx], !!(root_cmd & enabled_cmd));
+ pci_set_irq(dev, !!(root_cmd & enabled_cmd));
return;
}
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index c010007..1966169 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -64,15 +64,6 @@ struct PCIExpressDevice {
uint8_t exp_cap;
/* SLOT */
- unsigned int hpev_intx; /* INTx for hot plug event (0-3:INT[A-D]#)
- * default is 0 = INTA#
- * If the chip wants to use other interrupt
- * line, initialize this member with the
- * desired number.
- * If the chip dynamically changes this member,
- * also initialize it when loaded as
- * appropreately.
- */
bool hpev_notified; /* Logical AND of conditions for hot plug event.
Following 6.7.3.4:
Software Notification of Hot-Plug Events, an interrupt
@@ -82,15 +73,6 @@ struct PCIExpressDevice {
/* AER */
uint16_t aer_cap;
PCIEAERLog aer_log;
- unsigned int aer_intx; /* INTx for error reporting
- * default is 0 = INTA#
- * If the chip wants to use other interrupt
- * line, initialize this member with the
- * desired number.
- * If the chip dynamically changes this member,
- * also initialize it when loaded as
- * appropreately.
- */
};
/* PCI express capability helper functions */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH RFC v2 9/9] hw/pci: removed irq field from PCIDevice
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (7 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 8/9] hw/pcie: AER and hot-plug events must use device's interrupt Marcel Apfelbaum
@ 2013-10-02 12:41 ` Marcel Apfelbaum
2013-10-02 12:58 ` [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Michael S. Tsirkin
9 siblings, 0 replies; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 12:41 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 df7d316..e9346bd 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -247,9 +247,6 @@ struct PCIDevice {
PCIConfigReadFunc *config_read;
PCIConfigWriteFunc *config_write;
- /* IRQ objects for the INTA-INTD pins. */
- qemu_irq *irq;
-
/* Legacy PCI VGA regions */
MemoryRegion *vga_regions[QEMU_PCI_VGA_NUM_REGIONS];
bool has_vga;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
2013-10-02 12:41 [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Marcel Apfelbaum
` (8 preceding siblings ...)
2013-10-02 12:41 ` [Qemu-devel] [PATCH RFC v2 9/9] hw/pci: removed irq field from PCIDevice Marcel Apfelbaum
@ 2013-10-02 12:58 ` Michael S. Tsirkin
2013-10-02 13:05 ` Marcel Apfelbaum
9 siblings, 1 reply; 22+ messages in thread
From: Michael S. Tsirkin @ 2013-10-02 12:58 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 Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> Note: Added RFC because not all affected devices were
> checked yet.
What do you have in mind exactly?
> Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> register during device initialization. Devices should not call
> directly qemu_set_irq and specify the INTx pin.
>
> Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> qemu_irq_lower and qemu_irq_pulse, setting the irq
> based on PCI_INTERRUPT_PIN.
>
> Added interface to allocate and free single irq.
> Added pci_allocate_irq wrapper to be used by devices that
> still need PCIDevice infrastructure to assert irqs.
>
> Removed irq field from PCIDevice, not needed anymore.
>
> Special cases of replacements were done in separate patches,
> all others in one patch "hw: set interrupts using pci irq wrappers"
Looks good to me overall.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Changes from v1:
> - Addressed Michael S. Tsirkin's comments:
> - pci_set_irq directly calls pci_irq handler
> - removed irq field from PCIDevice
> - Added qemu interface to allocate single irq
> - Added pci wrappers to allocate and free pci irq
> - Added pci irq wrappers for all qemu methods
> setting irq and not only qemu_set_irq
> - Replace all qemu irq setters with pci
> wrappers
>
> Marcel Apfelbaum (9):
> hw/core: Add interface to allocate and free a single IRQ
> hw/pci: add pci wrappers for allocating and asserting irqs
> hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> hw/vmxnet3: set interrupts using pci irq wrappers
> hw/vfio: set interrupts using pci irq wrappers
> hw/xhci: set interrupts using pci irq wrappers
> hw: set interrupts using pci irq wrappers
> hw/pcie: AER and hot-plug events must use device's interrupt
> hw/pci: removed irq field from PCIDevice
>
> hw/audio/ac97.c | 4 ++--
> hw/audio/es1370.c | 4 ++--
> hw/audio/intel-hda.c | 2 +-
> hw/block/nvme.c | 2 +-
> hw/char/serial-pci.c | 5 +++--
> hw/char/tpci200.c | 8 ++++----
> hw/core/irq.c | 16 ++++++++++++++++
> hw/display/qxl.c | 2 +-
> hw/ide/cmd646.c | 2 +-
> hw/ide/ich.c | 3 ++-
> hw/isa/vt82c686.c | 2 +-
> hw/misc/ivshmem.c | 2 +-
> hw/misc/vfio.c | 11 ++++++-----
> hw/net/e1000.c | 2 +-
> hw/net/eepro100.c | 4 ++--
> hw/net/ne2000.c | 3 ++-
> hw/net/pcnet-pci.c | 3 ++-
> hw/net/rtl8139.c | 2 +-
> hw/net/vmxnet3.c | 13 +++++++++++--
> hw/pci-bridge/pci_bridge_dev.c | 2 +-
> hw/pci/pci.c | 26 +++++++++++++++++++++-----
> hw/pci/pcie.c | 4 ++--
> hw/pci/pcie_aer.c | 4 ++--
> hw/pci/shpc.c | 2 +-
> hw/scsi/esp-pci.c | 3 ++-
> hw/scsi/lsi53c895a.c | 2 +-
> hw/scsi/megasas.c | 6 +++---
> hw/scsi/vmw_pvscsi.c | 2 +-
> hw/usb/hcd-ehci-pci.c | 2 +-
> hw/usb/hcd-ohci.c | 2 +-
> hw/usb/hcd-uhci.c | 2 +-
> hw/usb/hcd-xhci.c | 7 ++-----
> hw/virtio/virtio-pci.c | 4 ++--
> include/hw/irq.h | 7 +++++++
> include/hw/pci/pci.h | 22 +++++++++++++++++++---
> include/hw/pci/pcie.h | 18 ------------------
> 36 files changed, 127 insertions(+), 78 deletions(-)
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
2013-10-02 12:58 ` [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin Michael S. Tsirkin
@ 2013-10-02 13:05 ` Marcel Apfelbaum
2013-10-02 16:03 ` Alex Williamson
0 siblings, 1 reply; 22+ messages in thread
From: Marcel Apfelbaum @ 2013-10-02 13:05 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
On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> > Note: Added RFC because not all affected devices were
> > checked yet.
>
> What do you have in mind exactly?
Sanity test for *all* modified devices.
Any other thoughts?
>From self code review, I can say that besides 2 devices
(vmxnet3 and vfio), the code is re-factored with
no side effects.
vmxnet3: I reviewed linux driver code and when using legacy interrupts
only INTx 0 is used.
vfio: Also here only INTx 0 is used, so the interface selecting
the interrupt line seems to be unnecessary at least for
legacy interrupts.
>
> > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > register during device initialization. Devices should not call
> > directly qemu_set_irq and specify the INTx pin.
> >
> > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > based on PCI_INTERRUPT_PIN.
> >
> > Added interface to allocate and free single irq.
> > Added pci_allocate_irq wrapper to be used by devices that
> > still need PCIDevice infrastructure to assert irqs.
> >
> > Removed irq field from PCIDevice, not needed anymore.
> >
> > Special cases of replacements were done in separate patches,
> > all others in one patch "hw: set interrupts using pci irq wrappers"
>
> Looks good to me overall.
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
>
> > Changes from v1:
> > - Addressed Michael S. Tsirkin's comments:
> > - pci_set_irq directly calls pci_irq handler
> > - removed irq field from PCIDevice
> > - Added qemu interface to allocate single irq
> > - Added pci wrappers to allocate and free pci irq
> > - Added pci irq wrappers for all qemu methods
> > setting irq and not only qemu_set_irq
> > - Replace all qemu irq setters with pci
> > wrappers
> >
> > Marcel Apfelbaum (9):
> > hw/core: Add interface to allocate and free a single IRQ
> > hw/pci: add pci wrappers for allocating and asserting irqs
> > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> > hw/vmxnet3: set interrupts using pci irq wrappers
> > hw/vfio: set interrupts using pci irq wrappers
> > hw/xhci: set interrupts using pci irq wrappers
> > hw: set interrupts using pci irq wrappers
> > hw/pcie: AER and hot-plug events must use device's interrupt
> > hw/pci: removed irq field from PCIDevice
> >
> > hw/audio/ac97.c | 4 ++--
> > hw/audio/es1370.c | 4 ++--
> > hw/audio/intel-hda.c | 2 +-
> > hw/block/nvme.c | 2 +-
> > hw/char/serial-pci.c | 5 +++--
> > hw/char/tpci200.c | 8 ++++----
> > hw/core/irq.c | 16 ++++++++++++++++
> > hw/display/qxl.c | 2 +-
> > hw/ide/cmd646.c | 2 +-
> > hw/ide/ich.c | 3 ++-
> > hw/isa/vt82c686.c | 2 +-
> > hw/misc/ivshmem.c | 2 +-
> > hw/misc/vfio.c | 11 ++++++-----
> > hw/net/e1000.c | 2 +-
> > hw/net/eepro100.c | 4 ++--
> > hw/net/ne2000.c | 3 ++-
> > hw/net/pcnet-pci.c | 3 ++-
> > hw/net/rtl8139.c | 2 +-
> > hw/net/vmxnet3.c | 13 +++++++++++--
> > hw/pci-bridge/pci_bridge_dev.c | 2 +-
> > hw/pci/pci.c | 26 +++++++++++++++++++++-----
> > hw/pci/pcie.c | 4 ++--
> > hw/pci/pcie_aer.c | 4 ++--
> > hw/pci/shpc.c | 2 +-
> > hw/scsi/esp-pci.c | 3 ++-
> > hw/scsi/lsi53c895a.c | 2 +-
> > hw/scsi/megasas.c | 6 +++---
> > hw/scsi/vmw_pvscsi.c | 2 +-
> > hw/usb/hcd-ehci-pci.c | 2 +-
> > hw/usb/hcd-ohci.c | 2 +-
> > hw/usb/hcd-uhci.c | 2 +-
> > hw/usb/hcd-xhci.c | 7 ++-----
> > hw/virtio/virtio-pci.c | 4 ++--
> > include/hw/irq.h | 7 +++++++
> > include/hw/pci/pci.h | 22 +++++++++++++++++++---
> > include/hw/pci/pcie.h | 18 ------------------
> > 36 files changed, 127 insertions(+), 78 deletions(-)
> >
> > --
> > 1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
2013-10-02 13:05 ` Marcel Apfelbaum
@ 2013-10-02 16:03 ` Alex Williamson
0 siblings, 0 replies; 22+ messages in thread
From: Alex Williamson @ 2013-10-02 16:03 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: kwolf, peter.maydell, peter.crosthwaite, anthony,
Michael S. Tsirkin, sw, jasowang, qemu-devel, dkoch, keith.busch,
kraxel, stefanha, dmitry, pbonzini, afaerber, ehabkost
On Wed, 2013-10-02 at 16:05 +0300, Marcel Apfelbaum wrote:
> On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote:
> > > Note: Added RFC because not all affected devices were
> > > checked yet.
> >
> > What do you have in mind exactly?
> Sanity test for *all* modified devices.
> Any other thoughts?
>
> From self code review, I can say that besides 2 devices
> (vmxnet3 and vfio), the code is re-factored with
> no side effects.
>
> vmxnet3: I reviewed linux driver code and when using legacy interrupts
> only INTx 0 is used.
> vfio: Also here only INTx 0 is used, so the interface selecting
> the interrupt line seems to be unnecessary at least for
> legacy interrupts.
vfio uses the same pin as the physical device, so you can't guarantee
that's always INTA. Thanks,
Alex
> >
> > > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > > register during device initialization. Devices should not call
> > > directly qemu_set_irq and specify the INTx pin.
> > >
> > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > > based on PCI_INTERRUPT_PIN.
> > >
> > > Added interface to allocate and free single irq.
> > > Added pci_allocate_irq wrapper to be used by devices that
> > > still need PCIDevice infrastructure to assert irqs.
> > >
> > > Removed irq field from PCIDevice, not needed anymore.
> > >
> > > Special cases of replacements were done in separate patches,
> > > all others in one patch "hw: set interrupts using pci irq wrappers"
> >
> > Looks good to me overall.
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> >
> > > Changes from v1:
> > > - Addressed Michael S. Tsirkin's comments:
> > > - pci_set_irq directly calls pci_irq handler
> > > - removed irq field from PCIDevice
> > > - Added qemu interface to allocate single irq
> > > - Added pci wrappers to allocate and free pci irq
> > > - Added pci irq wrappers for all qemu methods
> > > setting irq and not only qemu_set_irq
> > > - Replace all qemu irq setters with pci
> > > wrappers
> > >
> > > Marcel Apfelbaum (9):
> > > hw/core: Add interface to allocate and free a single IRQ
> > > hw/pci: add pci wrappers for allocating and asserting irqs
> > > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init
> > > hw/vmxnet3: set interrupts using pci irq wrappers
> > > hw/vfio: set interrupts using pci irq wrappers
> > > hw/xhci: set interrupts using pci irq wrappers
> > > hw: set interrupts using pci irq wrappers
> > > hw/pcie: AER and hot-plug events must use device's interrupt
> > > hw/pci: removed irq field from PCIDevice
> > >
> > > hw/audio/ac97.c | 4 ++--
> > > hw/audio/es1370.c | 4 ++--
> > > hw/audio/intel-hda.c | 2 +-
> > > hw/block/nvme.c | 2 +-
> > > hw/char/serial-pci.c | 5 +++--
> > > hw/char/tpci200.c | 8 ++++----
> > > hw/core/irq.c | 16 ++++++++++++++++
> > > hw/display/qxl.c | 2 +-
> > > hw/ide/cmd646.c | 2 +-
> > > hw/ide/ich.c | 3 ++-
> > > hw/isa/vt82c686.c | 2 +-
> > > hw/misc/ivshmem.c | 2 +-
> > > hw/misc/vfio.c | 11 ++++++-----
> > > hw/net/e1000.c | 2 +-
> > > hw/net/eepro100.c | 4 ++--
> > > hw/net/ne2000.c | 3 ++-
> > > hw/net/pcnet-pci.c | 3 ++-
> > > hw/net/rtl8139.c | 2 +-
> > > hw/net/vmxnet3.c | 13 +++++++++++--
> > > hw/pci-bridge/pci_bridge_dev.c | 2 +-
> > > hw/pci/pci.c | 26 +++++++++++++++++++++-----
> > > hw/pci/pcie.c | 4 ++--
> > > hw/pci/pcie_aer.c | 4 ++--
> > > hw/pci/shpc.c | 2 +-
> > > hw/scsi/esp-pci.c | 3 ++-
> > > hw/scsi/lsi53c895a.c | 2 +-
> > > hw/scsi/megasas.c | 6 +++---
> > > hw/scsi/vmw_pvscsi.c | 2 +-
> > > hw/usb/hcd-ehci-pci.c | 2 +-
> > > hw/usb/hcd-ohci.c | 2 +-
> > > hw/usb/hcd-uhci.c | 2 +-
> > > hw/usb/hcd-xhci.c | 7 ++-----
> > > hw/virtio/virtio-pci.c | 4 ++--
> > > include/hw/irq.h | 7 +++++++
> > > include/hw/pci/pci.h | 22 +++++++++++++++++++---
> > > include/hw/pci/pcie.h | 18 ------------------
> > > 36 files changed, 127 insertions(+), 78 deletions(-)
> > >
> > > --
> > > 1.8.3.1
>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread