I am proposing these three patches for 2.6.39.
The first is to take advantage of the new method of requesting
a Linux IRQ and providing the Xen PIRQ value. The second
makes it possible for a PV guest to bootup if the backend has provided
incorrect values. [*2]
Lastly, the third is to remove deprecated code.
Konrad Rzeszutek Wilk (2):
pci/xen: Use xen_allocate_pirq_msi
xen-pcifront: Sanity check the MSI/MSI-X values
Tejun Heo (1):
xen-pcifront: don't use flush_scheduled_work()
arch/x86/pci/xen.c | 6 +++---
drivers/pci/xen-pcifront.c | 20 +++++++++++++++-----
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 25cd4a0..6432f75 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
goto error;
i = 0;
list_for_each_entry(msidesc, &dev->msi_list, list) {
- irq = xen_allocate_pirq(v[i], 0, /* not sharable */
+ xen_allocate_pirq_msi(
(type == PCI_CAP_ID_MSIX) ?
- "pcifront-msi-x" : "pcifront-msi");
+ "pcifront-msi-x" : "pcifront-msi",
+ &irq, &v[i], XEN_ALLOC_IRQ);
if (irq < 0) {
ret = -1;
goto free;
}
-
ret = set_irq_msi(irq, msidesc);
if (ret)
goto error_while;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 3a5a6fc..dd615d9 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -277,13 +277,20 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
if (likely(!err)) {
if (likely(!op.value)) {
/* we get the result */
- for (i = 0; i < nvec; i++)
+ for (i = 0; i < nvec; i++) {
+ if (op.msix_entries[i].vector <= 0) {
+ dev_warn(&dev->dev, "MSI-X entry %d" \
+ " is zero!\n", i);
+ err = -EINVAL;
+ continue;
+ }
*(*vector+i) = op.msix_entries[i].vector;
- return 0;
+ }
+ return err;
} else {
printk(KERN_DEBUG "enable msix get value %x\n",
op.value);
- return op.value;
+ return err;
}
} else {
dev_err(&dev->dev, "enable msix get err %x\n", err);
@@ -325,6 +332,10 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
err = do_pci_op(pdev, &op);
if (likely(!err)) {
*(*vector) = op.value;
+ if (op.value <= 0) {
+ dev_warn(&dev->dev, "MSI entry is zero!\n");
+ err = -EINVAL;
+ }
} else {
dev_err(&dev->dev, "pci frontend enable msi failed for dev "
"%x:%x\n", op.bus, op.devfn);
@@ -733,8 +744,7 @@ static void free_pdev(struct pcifront_device *pdev)
pcifront_free_roots(pdev);
- /*For PCIE_AER error handling job*/
- flush_scheduled_work();
+ cancel_work_sync(&pdev->op_work);
if (pdev->irq >= 0)
unbind_from_irqhandler(pdev->irq, pdev);
[*2]: as so:
0.877864] e1000e: Intel(R) PRO/1000 Network Driver - 1.2.20-k2
[ 0.877874] e1000e: Copyright(c) 1999 - 2011 Intel Corporation.
[ 0.877957] e1000e 0000:00:00.0: enabling device (0000 -> 0002)
[ 0.878054] e1000e 0000:00:00.0: Xen PCI mapped GSI16 to IRQ27
[ 0.878056] e1000e 0000:00:00.0: setting latency timer to 64
[ 0.879924] e1000e 0000:00:00.0: MSI-X entry 0 is zero!
[ 0.879935] e1000e 0000:00:00.0: MSI-X entry 1 is zero!
[ 0.879940] e1000e 0000:00:00.0: MSI-X entry 2 is zero!
[ 0.880207] e1000e 0000:00:00.0: (unregistered net_device): Failed to initialize MSI-X interrupts. Falling back to MSI interrupts.
[ 0.880730] e1000e 0000:00:00.0: MSI entry is zero!
[ 0.880788] e1000e 0000:00:00.0: (unregistered net_device): Failed to initialize MSI interrupts. Falling back to legacy interrupts.
[ 0.880945] e1000e 0000:00:00.0: Disabling ASPM L0s
[ 0.977188] e1000e 0000:00:00.0: eth0: (PCI Express:2.5GB/s:Width x1) 00:1b:21:61:49:00
[ 0.977201] e1000e 0000:00:00.0: eth0: Intel(R) PRO/1000 Network Connection
[ 0.977217] e1000e 0000:00:00.0: eth0: MAC: 3, PHY: 8, PBA No: E46981-003
...
Check the returned vector values for any values that are
odd or plain incorrect (say vector value zero), and if so
print a warning. Also fixup the return values.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/pci/xen-pcifront.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 3a5a6fc..6acf6ae 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -277,13 +277,20 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
if (likely(!err)) {
if (likely(!op.value)) {
/* we get the result */
- for (i = 0; i < nvec; i++)
+ for (i = 0; i < nvec; i++) {
+ if (op.msix_entries[i].vector <= 0) {
+ dev_warn(&dev->dev, "MSI-X entry %d" \
+ " is zero!\n", i);
+ err = -EINVAL;
+ continue;
+ }
*(*vector+i) = op.msix_entries[i].vector;
- return 0;
+ }
+ return err;
} else {
printk(KERN_DEBUG "enable msix get value %x\n",
op.value);
- return op.value;
+ return err;
}
} else {
dev_err(&dev->dev, "enable msix get err %x\n", err);
@@ -325,6 +332,10 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
err = do_pci_op(pdev, &op);
if (likely(!err)) {
*(*vector) = op.value;
+ if (op.value <= 0) {
+ dev_warn(&dev->dev, "MSI entry is zero!\n");
+ err = -EINVAL;
+ }
} else {
dev_err(&dev->dev, "pci frontend enable msi failed for dev "
"%x:%x\n", op.bus, op.devfn);
--
1.7.1
There is no need to use the old interface.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
arch/x86/pci/xen.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 25cd4a0..6432f75 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
goto error;
i = 0;
list_for_each_entry(msidesc, &dev->msi_list, list) {
- irq = xen_allocate_pirq(v[i], 0, /* not sharable */
+ xen_allocate_pirq_msi(
(type == PCI_CAP_ID_MSIX) ?
- "pcifront-msi-x" : "pcifront-msi");
+ "pcifront-msi-x" : "pcifront-msi",
+ &irq, &v[i], XEN_ALLOC_IRQ);
if (irq < 0) {
ret = -1;
goto free;
}
-
ret = set_irq_msi(irq, msidesc);
if (ret)
goto error_while;
--
1.7.1
From: Tejun Heo <[email protected]>
flush_scheduled_work() is scheduled for deprecation. Cancel ->op_work
directly instead.
Signed-off-by: Tejun Heo <[email protected]>
Cc: Ryan Wilson <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Jesse Barnes <[email protected]>
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/pci/xen-pcifront.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 6acf6ae..dd615d9 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -744,8 +744,7 @@ static void free_pdev(struct pcifront_device *pdev)
pcifront_free_roots(pdev);
- /*For PCIE_AER error handling job*/
- flush_scheduled_work();
+ cancel_work_sync(&pdev->op_work);
if (pdev->irq >= 0)
unbind_from_irqhandler(pdev->irq, pdev);
--
1.7.1
On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> I am proposing these three patches for 2.6.39.
>
> The first is to take advantage of the new method of requesting
> a Linux IRQ and providing the Xen PIRQ value. The second
> makes it possible for a PV guest to bootup if the backend has provided
> incorrect values. [*2]
I approve of being liberal in what is accepted but do we also have a
handle on why the backend is providing incorrect values in the first
place?
>
> Lastly, the third is to remove deprecated code.
>
> Konrad Rzeszutek Wilk (2):
> pci/xen: Use xen_allocate_pirq_msi
> xen-pcifront: Sanity check the MSI/MSI-X values
>
> Tejun Heo (1):
> xen-pcifront: don't use flush_scheduled_work()
>
> arch/x86/pci/xen.c | 6 +++---
> drivers/pci/xen-pcifront.c | 20 +++++++++++++++-----
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 25cd4a0..6432f75 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> goto error;
> i = 0;
> list_for_each_entry(msidesc, &dev->msi_list, list) {
> - irq = xen_allocate_pirq(v[i], 0, /* not sharable */
> + xen_allocate_pirq_msi(
> (type == PCI_CAP_ID_MSIX) ?
> - "pcifront-msi-x" : "pcifront-msi");
> + "pcifront-msi-x" : "pcifront-msi",
> + &irq, &v[i], XEN_ALLOC_IRQ);
> if (irq < 0) {
> ret = -1;
> goto free;
> }
> -
> ret = set_irq_msi(irq, msidesc);
> if (ret)
> goto error_while;
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 3a5a6fc..dd615d9 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -277,13 +277,20 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
> if (likely(!err)) {
> if (likely(!op.value)) {
> /* we get the result */
> - for (i = 0; i < nvec; i++)
> + for (i = 0; i < nvec; i++) {
> + if (op.msix_entries[i].vector <= 0) {
> + dev_warn(&dev->dev, "MSI-X entry %d" \
> + " is zero!\n", i);
> + err = -EINVAL;
> + continue;
> + }
> *(*vector+i) = op.msix_entries[i].vector;
> - return 0;
> + }
> + return err;
> } else {
> printk(KERN_DEBUG "enable msix get value %x\n",
> op.value);
> - return op.value;
> + return err;
> }
> } else {
> dev_err(&dev->dev, "enable msix get err %x\n", err);
> @@ -325,6 +332,10 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
> err = do_pci_op(pdev, &op);
> if (likely(!err)) {
> *(*vector) = op.value;
> + if (op.value <= 0) {
> + dev_warn(&dev->dev, "MSI entry is zero!\n");
> + err = -EINVAL;
> + }
> } else {
> dev_err(&dev->dev, "pci frontend enable msi failed for dev "
> "%x:%x\n", op.bus, op.devfn);
> @@ -733,8 +744,7 @@ static void free_pdev(struct pcifront_device *pdev)
>
> pcifront_free_roots(pdev);
>
> - /*For PCIE_AER error handling job*/
> - flush_scheduled_work();
> + cancel_work_sync(&pdev->op_work);
>
> if (pdev->irq >= 0)
> unbind_from_irqhandler(pdev->irq, pdev);
>
>
> [*2]: as so:
>
> 0.877864] e1000e: Intel(R) PRO/1000 Network Driver - 1.2.20-k2
> [ 0.877874] e1000e: Copyright(c) 1999 - 2011 Intel Corporation.
> [ 0.877957] e1000e 0000:00:00.0: enabling device (0000 -> 0002)
> [ 0.878054] e1000e 0000:00:00.0: Xen PCI mapped GSI16 to IRQ27
> [ 0.878056] e1000e 0000:00:00.0: setting latency timer to 64
> [ 0.879924] e1000e 0000:00:00.0: MSI-X entry 0 is zero!
> [ 0.879935] e1000e 0000:00:00.0: MSI-X entry 1 is zero!
> [ 0.879940] e1000e 0000:00:00.0: MSI-X entry 2 is zero!
> [ 0.880207] e1000e 0000:00:00.0: (unregistered net_device): Failed to initialize MSI-X interrupts. Falling back to MSI interrupts.
> [ 0.880730] e1000e 0000:00:00.0: MSI entry is zero!
> [ 0.880788] e1000e 0000:00:00.0: (unregistered net_device): Failed to initialize MSI interrupts. Falling back to legacy interrupts.
> [ 0.880945] e1000e 0000:00:00.0: Disabling ASPM L0s
> [ 0.977188] e1000e 0000:00:00.0: eth0: (PCI Express:2.5GB/s:Width x1) 00:1b:21:61:49:00
> [ 0.977201] e1000e 0000:00:00.0: eth0: Intel(R) PRO/1000 Network Connection
> [ 0.977217] e1000e 0000:00:00.0: eth0: MAC: 3, PHY: 8, PBA No: E46981-003
> ...
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> http://lists.xensource.com/xen-devel
On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> There is no need to use the old interface.
xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
xen_initial_domain() in addition to the kernel side book-keeping side of
things (set chip and handler, update irq_info etc) whereas
xen_allocate_pirq_msi just does the kernel book keeping.
Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
So this change is actually a semantic change and not just a switch to a
new interface. I think the change is OK (because the caller is domU
only?) but a comment explaining this would be appreciated.
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> ---
> arch/x86/pci/xen.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 25cd4a0..6432f75 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> goto error;
> i = 0;
> list_for_each_entry(msidesc, &dev->msi_list, list) {
> - irq = xen_allocate_pirq(v[i], 0, /* not sharable */
> + xen_allocate_pirq_msi(
> (type == PCI_CAP_ID_MSIX) ?
> - "pcifront-msi-x" : "pcifront-msi");
> + "pcifront-msi-x" : "pcifront-msi",
> + &irq, &v[i], XEN_ALLOC_IRQ);
All callers have this (type == MSIX) ? "msi-x" : "msi" construct,
differing only in their use of a "pcifront-" prefix. Something which
could be hoisted into the function perhaps?
Another aside: I think there are other cleanups which could be made to
the various pirq allocation/mapping interfaces (of which there seem to
be too many, or at least their individual unique qualities are
indistinguishable by their names), I made a start on this at one point
since I wanted to get rid of the pirq_to_irq[nr_irqs] array, for obvious
reasons, but didn't finish it off. I'll hopefully revisit this soon.
Ian.
> if (irq < 0) {
> ret = -1;
> goto free;
> }
> -
> ret = set_irq_msi(irq, msidesc);
> if (ret)
> goto error_while;
On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> Check the returned vector values for any values that are
> odd or plain incorrect (say vector value zero), and if so
> print a warning. Also fixup the return values.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> ---
> drivers/pci/xen-pcifront.c | 17 ++++++++++++++---
> 1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 3a5a6fc..6acf6ae 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -277,13 +277,20 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
> if (likely(!err)) {
> if (likely(!op.value)) {
> /* we get the result */
> - for (i = 0; i < nvec; i++)
> + for (i = 0; i < nvec; i++) {
> + if (op.msix_entries[i].vector <= 0) {
> + dev_warn(&dev->dev, "MSI-X entry %d" \
> + " is zero!\n", i);
The test says "<= 0" but the text says "== 0". Perhaps
dev_warn(&dev->dev, "MSI-X entry %d has invalid vector %d\n",
i, op.msix_entries[i].vector);
>
> + err = -EINVAL;
> + continue;
Do we need / should we set *(*vector+i) to something to indicate its
invalidness rather than leave it potentially uninitialised?
> + }
> *(*vector+i) = op.msix_entries[i].vector;
BTW does the double indirection of the vector serve a purpose?
Everywhere I can see just updates *(*vector+i), I can't see any realloc
of the array itself etc.
Removing the extra level of indirection leads to vector[i] = foo instead
which is much easier on the eye.
> - return 0;
> + }
> + return err;
> } else {
> printk(KERN_DEBUG "enable msix get value %x\n",
> op.value);
> - return op.value;
> + return err;
I think all of these "return err"s can now be pulled out to the end of
the function? makes it clearer what the return is.
> }
> } else {
> dev_err(&dev->dev, "enable msix get err %x\n", err);
> @@ -325,6 +332,10 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
> err = do_pci_op(pdev, &op);
> if (likely(!err)) {
> *(*vector) = op.value;
> + if (op.value <= 0) {
> + dev_warn(&dev->dev, "MSI entry is zero!\n");
Same comment re <= vs == 0.
> + err = -EINVAL;
> + }
> } else {
> dev_err(&dev->dev, "pci frontend enable msi failed for dev "
> "%x:%x\n", op.bus, op.devfn);
On Thu, Feb 17, 2011 at 08:29:04AM +0000, Ian Campbell wrote:
> On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > I am proposing these three patches for 2.6.39.
> >
> > The first is to take advantage of the new method of requesting
> > a Linux IRQ and providing the Xen PIRQ value. The second
> > makes it possible for a PV guest to bootup if the backend has provided
> > incorrect values. [*2]
>
> I approve of being liberal in what is accepted but do we also have a
> handle on why the backend is providing incorrect values in the first
> place?
I got those fixed too - was using 'xen_irq_from_gsi' while
it should have used 'xen_irq_from_pirq'. This was the new mechanism
to obtain the vector values after .. 2.6.38-rc1 ish.. (as the
GSI values now have the correct value of zero, and the vector value
for MSI/MSI-X is written in the PIRQ entry).
But those patches are in a different branch (devel/xen-pciback-0.3).
On Thu, Feb 17, 2011 at 08:41:31AM +0000, Ian Campbell wrote:
> On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > There is no need to use the old interface.
>
> xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> xen_initial_domain() in addition to the kernel side book-keeping side of
> things (set chip and handler, update irq_info etc) whereas
> xen_allocate_pirq_msi just does the kernel book keeping.
>
> Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
Which is OK. These are MSIs.
>
> So this change is actually a semantic change and not just a switch to a
> new interface. I think the change is OK (because the caller is domU
Right.
> only?) but a comment explaining this would be appreciated.
Correct: "domU side".
Will fix it up.
>
> > Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> > ---
> > arch/x86/pci/xen.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> > index 25cd4a0..6432f75 100644
> > --- a/arch/x86/pci/xen.c
> > +++ b/arch/x86/pci/xen.c
> > @@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > goto error;
> > i = 0;
> > list_for_each_entry(msidesc, &dev->msi_list, list) {
> > - irq = xen_allocate_pirq(v[i], 0, /* not sharable */
> > + xen_allocate_pirq_msi(
> > (type == PCI_CAP_ID_MSIX) ?
> > - "pcifront-msi-x" : "pcifront-msi");
> > + "pcifront-msi-x" : "pcifront-msi",
> > + &irq, &v[i], XEN_ALLOC_IRQ);
>
> All callers have this (type == MSIX) ? "msi-x" : "msi" construct,
> differing only in their use of a "pcifront-" prefix. Something which
> could be hoisted into the function perhaps?
>
> Another aside: I think there are other cleanups which could be made to
> the various pirq allocation/mapping interfaces (of which there seem to
> be too many, or at least their individual unique qualities are
> indistinguishable by their names), I made a start on this at one point
> since I wanted to get rid of the pirq_to_irq[nr_irqs] array, for obvious
> reasons, but didn't finish it off. I'll hopefully revisit this soon.
>
> Ian.
>
> > if (irq < 0) {
> > ret = -1;
> > goto free;
> > }
> > -
> > ret = set_irq_msi(irq, msidesc);
> > if (ret)
> > goto error_while;
>
On Thu, 2011-02-17 at 14:28 +0000, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 17, 2011 at 08:29:04AM +0000, Ian Campbell wrote:
> > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > I am proposing these three patches for 2.6.39.
> > >
> > > The first is to take advantage of the new method of requesting
> > > a Linux IRQ and providing the Xen PIRQ value. The second
> > > makes it possible for a PV guest to bootup if the backend has provided
> > > incorrect values. [*2]
> >
> > I approve of being liberal in what is accepted but do we also have a
> > handle on why the backend is providing incorrect values in the first
> > place?
>
> I got those fixed too - was using 'xen_irq_from_gsi' while
> it should have used 'xen_irq_from_pirq'. This was the new mechanism
> to obtain the vector values after .. 2.6.38-rc1 ish.. (as the
> GSI values now have the correct value of zero, and the vector value
> for MSI/MSI-X is written in the PIRQ entry).
>
> But those patches are in a different branch (devel/xen-pciback-0.3).
Thanks, it may be worth co-referencing the front and backend fixes in
their respective commit messages.
Ian.
> All callers have this (type == MSIX) ? "msi-x" : "msi" construct,
> differing only in their use of a "pcifront-" prefix. Something which
> could be hoisted into the function perhaps?
I was thinking to defer this for a cleanup patch.
>
> Another aside: I think there are other cleanups which could be made to
> the various pirq allocation/mapping interfaces (of which there seem to
> be too many, or at least their individual unique qualities are
> indistinguishable by their names), I made a start on this at one point
> since I wanted to get rid of the pirq_to_irq[nr_irqs] array, for obvious
> reasons, but didn't finish it off. I'll hopefully revisit this soon.
Excellent. Looking forward to it.
On Thu, Feb 17, 2011 at 09:30:03AM -0500, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 17, 2011 at 08:41:31AM +0000, Ian Campbell wrote:
> > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > There is no need to use the old interface.
> >
> > xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> > xen_initial_domain() in addition to the kernel side book-keeping side of
> > things (set chip and handler, update irq_info etc) whereas
> > xen_allocate_pirq_msi just does the kernel book keeping.
> >
> > Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> > xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
>
> Which is OK. These are MSIs.
> >
> > So this change is actually a semantic change and not just a switch to a
> > new interface. I think the change is OK (because the caller is domU
>
> Right.
>
> > only?) but a comment explaining this would be appreciated.
>
> Correct: "domU side".
>
> Will fix it up.
How does this look to you?
>From eb832bece3131ecbdb509f7f2a9bc53f6692177c Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Wed, 16 Feb 2011 13:43:04 -0500
Subject: [PATCH 3/5] pci/xen: Use xen_allocate_pirq_msi instead of xen_allocate_pirq
xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
xen_initial_domain() in addition to the kernel side book-keeping side of
things (set chip and handler, update irq_info etc) whereas
xen_allocate_pirq_msi just does the kernel book keeping.
Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
All of this is uneccessary as this code path is only executed
when we run as a domU PV guest with an MSI/MSI-X PCI card passed in.
Hence we can jump straight to allocating an dynamic IRQ (and
binding it to the proper PIRQ) and skip the rest.
In short: this change is a cosmetic one.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
arch/x86/pci/xen.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 25cd4a0..6432f75 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
goto error;
i = 0;
list_for_each_entry(msidesc, &dev->msi_list, list) {
- irq = xen_allocate_pirq(v[i], 0, /* not sharable */
+ xen_allocate_pirq_msi(
(type == PCI_CAP_ID_MSIX) ?
- "pcifront-msi-x" : "pcifront-msi");
+ "pcifront-msi-x" : "pcifront-msi",
+ &irq, &v[i], XEN_ALLOC_IRQ);
if (irq < 0) {
ret = -1;
goto free;
}
-
ret = set_irq_msi(irq, msidesc);
if (ret)
goto error_while;
--
1.7.1
On Thu, Feb 17, 2011 at 08:53:40AM +0000, Ian Campbell wrote:
> On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > Check the returned vector values for any values that are
> > odd or plain incorrect (say vector value zero), and if so
> > print a warning. Also fixup the return values.
> >
How about this one (and there is a cleanup patch shortly following for the *(vector)...)
>From 631b743a5a587d195bf612112026eec8ea649344 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Wed, 16 Feb 2011 13:43:22 -0500
Subject: [PATCH 2/5] xen-pcifront: Sanity check the MSI/MSI-X values
Check the returned vector values for any values that are
odd or plain incorrect (say vector value zero), and if so
print a warning. Also fixup the return values.
This patch was precipiated by the Xen PCIBack returning the
incorrect values due to how it was retrieving PIRQ values.
This has been fixed in the xen-pciback by
"xen/pciback: Utilize 'xen_pirq_from_irq' to get PIRQ value"
patch.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/pci/xen-pcifront.c | 21 +++++++++++++++++----
1 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 030ce37..d9fd1e0 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -277,18 +277,25 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
if (likely(!err)) {
if (likely(!op.value)) {
/* we get the result */
- for (i = 0; i < nvec; i++)
+ for (i = 0; i < nvec; i++) {
+ if (op.msix_entries[i].vector <= 0) {
+ dev_warn(&dev->dev, "MSI-X entry %d" \
+ " is invalid: %d!\n", i,
+ op.msix_entries[i].vector);
+ err = -EINVAL;
+ *(*vector+i) = -1;
+ continue;
+ }
*(*vector+i) = op.msix_entries[i].vector;
- return 0;
+ }
} else {
printk(KERN_DEBUG "enable msix get value %x\n",
op.value);
- return op.value;
}
} else {
dev_err(&dev->dev, "enable msix get err %x\n", err);
- return err;
}
+ return err;
}
static void pci_frontend_disable_msix(struct pci_dev *dev)
@@ -325,6 +332,12 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
err = do_pci_op(pdev, &op);
if (likely(!err)) {
*(*vector) = op.value;
+ if (op.value <= 0) {
+ dev_warn(&dev->dev, "MSI entry is invalid: %d!\n",
+ op.value);
+ err = -EINVAL;
+ *(*vector) = -1;
+ }
} else {
dev_err(&dev->dev, "pci frontend enable msi failed for dev "
"%x:%x\n", op.bus, op.devfn);
--
1.7.1
On Fri, 2011-02-18 at 14:07 +0000, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 17, 2011 at 09:30:03AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Thu, Feb 17, 2011 at 08:41:31AM +0000, Ian Campbell wrote:
> > > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > > There is no need to use the old interface.
> > >
> > > xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> > > xen_initial_domain() in addition to the kernel side book-keeping side of
> > > things (set chip and handler, update irq_info etc) whereas
> > > xen_allocate_pirq_msi just does the kernel book keeping.
> > >
> > > Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> > > xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
> >
> > Which is OK. These are MSIs.
> > >
> > > So this change is actually a semantic change and not just a switch to a
> > > new interface. I think the change is OK (because the caller is domU
> >
> > Right.
> >
> > > only?) but a comment explaining this would be appreciated.
> >
> > Correct: "domU side".
> >
> > Will fix it up.
>
> How does this look to you?
>
> From eb832bece3131ecbdb509f7f2a9bc53f6692177c Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <[email protected]>
> Date: Wed, 16 Feb 2011 13:43:04 -0500
> Subject: [PATCH 3/5] pci/xen: Use xen_allocate_pirq_msi instead of xen_allocate_pirq
>
> xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> xen_initial_domain() in addition to the kernel side book-keeping side of
> things (set chip and handler, update irq_info etc) whereas
> xen_allocate_pirq_msi just does the kernel book keeping.
>
> Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
>
> All of this is uneccessary as this code path is only executed
> when we run as a domU PV guest with an MSI/MSI-X PCI card passed in.
> Hence we can jump straight to allocating an dynamic IRQ (and
> binding it to the proper PIRQ) and skip the rest.
>
> In short: this change is a cosmetic one.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
Looks good,
Reviewed-by: Ian Campbell <[email protected]>
BTW I'm currently testing a series of further cleanups, to the
xen_create_msi_irq and xen_allocate_pcirq_msi stuff in particular.
I hope to post it today, assuming my test boxes co-operate...
Ian.
> ---
> arch/x86/pci/xen.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 25cd4a0..6432f75 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -157,14 +157,14 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> goto error;
> i = 0;
> list_for_each_entry(msidesc, &dev->msi_list, list) {
> - irq = xen_allocate_pirq(v[i], 0, /* not sharable */
> + xen_allocate_pirq_msi(
> (type == PCI_CAP_ID_MSIX) ?
> - "pcifront-msi-x" : "pcifront-msi");
> + "pcifront-msi-x" : "pcifront-msi",
> + &irq, &v[i], XEN_ALLOC_IRQ);
> if (irq < 0) {
> ret = -1;
> goto free;
> }
> -
> ret = set_irq_msi(irq, msidesc);
> if (ret)
> goto error_while;
On Fri, 18 Feb 2011, Ian Campbell wrote:
> On Fri, 2011-02-18 at 14:07 +0000, Konrad Rzeszutek Wilk wrote:
> > On Thu, Feb 17, 2011 at 09:30:03AM -0500, Konrad Rzeszutek Wilk wrote:
> > > On Thu, Feb 17, 2011 at 08:41:31AM +0000, Ian Campbell wrote:
> > > > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > > > There is no need to use the old interface.
> > > >
> > > > xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> > > > xen_initial_domain() in addition to the kernel side book-keeping side of
> > > > things (set chip and handler, update irq_info etc) whereas
> > > > xen_allocate_pirq_msi just does the kernel book keeping.
> > > >
> > > > Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> > > > xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
> > >
> > > Which is OK. These are MSIs.
> > > >
> > > > So this change is actually a semantic change and not just a switch to a
> > > > new interface. I think the change is OK (because the caller is domU
> > >
> > > Right.
> > >
> > > > only?) but a comment explaining this would be appreciated.
> > >
> > > Correct: "domU side".
> > >
> > > Will fix it up.
> >
> > How does this look to you?
> >
> > From eb832bece3131ecbdb509f7f2a9bc53f6692177c Mon Sep 17 00:00:00 2001
> > From: Konrad Rzeszutek Wilk <[email protected]>
> > Date: Wed, 16 Feb 2011 13:43:04 -0500
> > Subject: [PATCH 3/5] pci/xen: Use xen_allocate_pirq_msi instead of xen_allocate_pirq
> >
> > xen_allocate_pirq -> xen_map_pirq_gsi -> PHYSDEVOP_alloc_irq_vector IFF
> > xen_initial_domain() in addition to the kernel side book-keeping side of
> > things (set chip and handler, update irq_info etc) whereas
> > xen_allocate_pirq_msi just does the kernel book keeping.
> >
> > Also xen_allocate_pirq allocates an IRQ in the 1-1 GSI space whereas
> > xen_allocate_pirq_msi allocates a dynamic one in the >GSI IRQ space.
> >
> > All of this is uneccessary as this code path is only executed
> > when we run as a domU PV guest with an MSI/MSI-X PCI card passed in.
> > Hence we can jump straight to allocating an dynamic IRQ (and
> > binding it to the proper PIRQ) and skip the rest.
> >
> > In short: this change is a cosmetic one.
> >
> > Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
>
> Looks good,
>
> Reviewed-by: Ian Campbell <[email protected]>
You can add my reviewed-by too.
On Fri, 2011-02-18 at 14:08 +0000, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 17, 2011 at 08:53:40AM +0000, Ian Campbell wrote:
> > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > Check the returned vector values for any values that are
> > > odd or plain incorrect (say vector value zero), and if so
> > > print a warning. Also fixup the return values.
> > >
>
> How about this one (and there is a cleanup patch shortly following for the *(vector)...)
>
> From 631b743a5a587d195bf612112026eec8ea649344 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <[email protected]>
> Date: Wed, 16 Feb 2011 13:43:22 -0500
> Subject: [PATCH 2/5] xen-pcifront: Sanity check the MSI/MSI-X values
>
> Check the returned vector values for any values that are
> odd or plain incorrect (say vector value zero), and if so
> print a warning. Also fixup the return values.
>
> This patch was precipiated by the Xen PCIBack returning the
> incorrect values due to how it was retrieving PIRQ values.
> This has been fixed in the xen-pciback by
> "xen/pciback: Utilize 'xen_pirq_from_irq' to get PIRQ value"
> patch.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> ---
> drivers/pci/xen-pcifront.c | 21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 030ce37..d9fd1e0 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -277,18 +277,25 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
> if (likely(!err)) {
> if (likely(!op.value)) {
> /* we get the result */
> - for (i = 0; i < nvec; i++)
> + for (i = 0; i < nvec; i++) {
> + if (op.msix_entries[i].vector <= 0) {
> + dev_warn(&dev->dev, "MSI-X entry %d" \
> + " is invalid: %d!\n", i,
String constants are not subject to the 80-column limit in these days
and the preference is to not split them to make them more greppable.
Otherwise:
Reviewed-by: Ian Campbell <[email protected]>
> + op.msix_entries[i].vector);
> + err = -EINVAL;
> + *(*vector+i) = -1;
> + continue;
> + }
> *(*vector+i) = op.msix_entries[i].vector;
> - return 0;
> + }
> } else {
> printk(KERN_DEBUG "enable msix get value %x\n",
> op.value);
> - return op.value;
> }
> } else {
> dev_err(&dev->dev, "enable msix get err %x\n", err);
> - return err;
> }
> + return err;
> }
>
> static void pci_frontend_disable_msix(struct pci_dev *dev)
> @@ -325,6 +332,12 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
> err = do_pci_op(pdev, &op);
> if (likely(!err)) {
> *(*vector) = op.value;
> + if (op.value <= 0) {
> + dev_warn(&dev->dev, "MSI entry is invalid: %d!\n",
> + op.value);
> + err = -EINVAL;
> + *(*vector) = -1;
> + }
> } else {
> dev_err(&dev->dev, "pci frontend enable msi failed for dev "
> "%x:%x\n", op.bus, op.devfn);
On Fri, Feb 18, 2011 at 09:08:45AM -0500, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 17, 2011 at 08:53:40AM +0000, Ian Campbell wrote:
> > On Wed, 2011-02-16 at 22:17 +0000, Konrad Rzeszutek Wilk wrote:
> > > Check the returned vector values for any values that are
> > > odd or plain incorrect (say vector value zero), and if so
> > > print a warning. Also fixup the return values.
> > >
>
> How about this one (and there is a cleanup patch shortly following for the *(vector)...)
>
The cleanup patch:
>From ee7eb70ab9c56a005b6023664c65da1305e3aa56 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Thu, 17 Feb 2011 12:02:23 -0500
Subject: [PATCH 4/5] pci/xen: Cleanup: convert int** to int[]
Cleanup code. Cosmetic change to make the code look easier
to read.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
arch/x86/include/asm/xen/pci.h | 8 ++++----
arch/x86/pci/xen.c | 4 ++--
drivers/pci/xen-pcifront.c | 12 ++++++------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/xen/pci.h b/arch/x86/include/asm/xen/pci.h
index 2329b3e..aa86209 100644
--- a/arch/x86/include/asm/xen/pci.h
+++ b/arch/x86/include/asm/xen/pci.h
@@ -27,16 +27,16 @@ static inline void __init xen_setup_pirqs(void)
* its own functions.
*/
struct xen_pci_frontend_ops {
- int (*enable_msi)(struct pci_dev *dev, int **vectors);
+ int (*enable_msi)(struct pci_dev *dev, int vectors[]);
void (*disable_msi)(struct pci_dev *dev);
- int (*enable_msix)(struct pci_dev *dev, int **vectors, int nvec);
+ int (*enable_msix)(struct pci_dev *dev, int vectors[], int nvec);
void (*disable_msix)(struct pci_dev *dev);
};
extern struct xen_pci_frontend_ops *xen_pci_frontend;
static inline int xen_pci_frontend_enable_msi(struct pci_dev *dev,
- int **vectors)
+ int vectors[])
{
if (xen_pci_frontend && xen_pci_frontend->enable_msi)
return xen_pci_frontend->enable_msi(dev, vectors);
@@ -48,7 +48,7 @@ static inline void xen_pci_frontend_disable_msi(struct pci_dev *dev)
xen_pci_frontend->disable_msi(dev);
}
static inline int xen_pci_frontend_enable_msix(struct pci_dev *dev,
- int **vectors, int nvec)
+ int vectors[], int nvec)
{
if (xen_pci_frontend && xen_pci_frontend->enable_msix)
return xen_pci_frontend->enable_msix(dev, vectors, nvec);
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 6432f75..30fdd09 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -150,9 +150,9 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
return -ENOMEM;
if (type == PCI_CAP_ID_MSIX)
- ret = xen_pci_frontend_enable_msix(dev, &v, nvec);
+ ret = xen_pci_frontend_enable_msix(dev, v, nvec);
else
- ret = xen_pci_frontend_enable_msi(dev, &v);
+ ret = xen_pci_frontend_enable_msi(dev, v);
if (ret)
goto error;
i = 0;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index d9fd1e0..27dae44 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -243,7 +243,7 @@ struct pci_ops pcifront_bus_ops = {
#ifdef CONFIG_PCI_MSI
static int pci_frontend_enable_msix(struct pci_dev *dev,
- int **vector, int nvec)
+ int vector[], int nvec)
{
int err;
int i;
@@ -283,10 +283,10 @@ static int pci_frontend_enable_msix(struct pci_dev *dev,
" is invalid: %d!\n", i,
op.msix_entries[i].vector);
err = -EINVAL;
- *(*vector+i) = -1;
+ vector[i] = -1;
continue;
}
- *(*vector+i) = op.msix_entries[i].vector;
+ vector[i] = op.msix_entries[i].vector;
}
} else {
printk(KERN_DEBUG "enable msix get value %x\n",
@@ -317,7 +317,7 @@ static void pci_frontend_disable_msix(struct pci_dev *dev)
dev_err(&dev->dev, "pci_disable_msix get err %x\n", err);
}
-static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
+static int pci_frontend_enable_msi(struct pci_dev *dev, int vector[])
{
int err;
struct xen_pci_op op = {
@@ -331,12 +331,12 @@ static int pci_frontend_enable_msi(struct pci_dev *dev, int **vector)
err = do_pci_op(pdev, &op);
if (likely(!err)) {
- *(*vector) = op.value;
+ vector[0] = op.value;
if (op.value <= 0) {
dev_warn(&dev->dev, "MSI entry is invalid: %d!\n",
op.value);
err = -EINVAL;
- *(*vector) = -1;
+ vector[0] = -1;
}
} else {
dev_err(&dev->dev, "pci frontend enable msi failed for dev "
--
1.7.1
On Wed, Feb 16, 2011 at 05:17:15PM -0500, Konrad Rzeszutek Wilk wrote:
> I am proposing these three patches for 2.6.39.
>
> The first is to take advantage of the new method of requesting
> a Linux IRQ and providing the Xen PIRQ value. The second
> makes it possible for a PV guest to bootup if the backend has provided
> incorrect values. [*2]
>
> Lastly, the third is to remove deprecated code.
And lets add one more:
>From e4e8523b1d374a3f4276c34e6d017b425ce0d1ae Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Thu, 17 Feb 2011 16:12:51 -0500
Subject: [PATCH 5/5] pci/xen: When free-ing MSI-X/MSI irq->desc also use generic code.
This code path is only run when an MSI/MSI-X PCI device is passed
in to PV DomU.
In 2.6.37 time-frame we over-wrote the default cleanup handler for
MSI/MSI-X irq->desc to be "xen_teardown_msi_irqs". That function
calls the the xen-pcifront driver which can tell the backend to
cleanup/take back the MSI/MSI-X device.
However, we forgot to continue the process of free-ing the MSI/MSI-X
device resources (irq->desc) in the PV domU side. Which is what
the default cleanup handler "default_teardown_msi_irqs" did.
Hence we would leak IRQ descriptors.
Without this patch, doing "rmmod igbvf;modprobe igbvf" multiple
times ends with abandoned IRQ descriptors:
28: 5 xen-pirq-pcifront-msi-x
29: 8 xen-pirq-pcifront-msi-x
...
130: 10 xen-pirq-pcifront-msi-x
with the end result of running out of IRQ descriptors.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
arch/x86/pci/xen.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 30fdd09..57afd1d 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -193,6 +193,9 @@ static void xen_teardown_msi_irqs(struct pci_dev *dev)
xen_pci_frontend_disable_msix(dev);
else
xen_pci_frontend_disable_msi(dev);
+
+ /* Free the IRQ's and the msidesc using the generic code. */
+ default_teardown_msi_irqs(dev);
}
static void xen_teardown_msi_irq(unsigned int irq)
--
1.7.1