| Message ID | 1392804931-30671-10-git-send-email-agordeev@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 02/19/2014 05:15 AM, Alexander Gordeev wrote: > As result of deprecation of MSI-X/MSI enablement functions > pci_enable_msix() and pci_enable_msi_block() all drivers > using these two interfaces need to be updated to use the > new pci_enable_msi_range() and pci_enable_msix_range() > interfaces. > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: David Vrabel <david.vrabel@citrix.com> > Cc: linux-pci@vger.kernel.org > --- > drivers/xen/xen-pciback/pciback_ops.c | 16 ++++++++++------ > 1 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c > index 64eb0cd..f5b4c3e 100644 > --- a/drivers/xen/xen-pciback/pciback_ops.c > +++ b/drivers/xen/xen-pciback/pciback_ops.c > @@ -213,9 +213,15 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, > entries[i].vector = op->msix_entries[i].vector; > } > > - result = pci_enable_msix(dev, entries, op->value); > + result = pci_enable_msix_range(dev, entries, op->value, op->value); > + if (result < op->value) { I think it would be better to have 'if (result != op->value)', in case op->value is negative (which presumably it should never be). -boris > + if (result > 0) > + pci_disable_msix(dev); > > - if (result == 0) { > + pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: err %d!\n", > + pci_name(dev), pdev->xdev->otherend_id, > + result); > + } else { > for (i = 0; i < op->value; i++) { > op->msix_entries[i].entry = entries[i].entry; > if (entries[i].vector) > @@ -227,10 +233,8 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, > pci_name(dev), i, > op->msix_entries[i].vector); > } > - } else > - pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: err %d!\n", > - pci_name(dev), pdev->xdev->otherend_id, > - result); > + } > + > kfree(entries); > > op->value = result; -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/19/2014 10:22 AM, Boris Ostrovsky wrote: > On 02/19/2014 05:15 AM, Alexander Gordeev wrote: >> As result of deprecation of MSI-X/MSI enablement functions >> pci_enable_msix() and pci_enable_msi_block() all drivers >> using these two interfaces need to be updated to use the >> new pci_enable_msi_range() and pci_enable_msix_range() >> interfaces. >> >> Signed-off-by: Alexander Gordeev <agordeev@redhat.com> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> Cc: David Vrabel <david.vrabel@citrix.com> >> Cc: linux-pci@vger.kernel.org >> --- >> drivers/xen/xen-pciback/pciback_ops.c | 16 ++++++++++------ >> 1 files changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/xen/xen-pciback/pciback_ops.c >> b/drivers/xen/xen-pciback/pciback_ops.c >> index 64eb0cd..f5b4c3e 100644 >> --- a/drivers/xen/xen-pciback/pciback_ops.c >> +++ b/drivers/xen/xen-pciback/pciback_ops.c >> @@ -213,9 +213,15 @@ int xen_pcibk_enable_msix(struct >> xen_pcibk_device *pdev, >> entries[i].vector = op->msix_entries[i].vector; >> } >> - result = pci_enable_msix(dev, entries, op->value); >> + result = pci_enable_msix_range(dev, entries, op->value, op->value); >> + if (result < op->value) { > > > I think it would be better to have 'if (result != op->value)', in case > op->value is negative (which presumably it should never be). Better yet, at the top of the routine we check 'if (op->value > SH_INFO_MAX_VEC)'. If you add '|| op->value < 0' we'd be all set. -boris > > > -boris > > >> + if (result > 0) >> + pci_disable_msix(dev); >> - if (result == 0) { >> + pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: >> err %d!\n", >> + pci_name(dev), pdev->xdev->otherend_id, >> + result); >> + } else { >> for (i = 0; i < op->value; i++) { >> op->msix_entries[i].entry = entries[i].entry; >> if (entries[i].vector) >> @@ -227,10 +233,8 @@ int xen_pcibk_enable_msix(struct >> xen_pcibk_device *pdev, >> pci_name(dev), i, >> op->msix_entries[i].vector); >> } >> - } else >> - pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: >> err %d!\n", >> - pci_name(dev), pdev->xdev->otherend_id, >> - result); >> + } >> + >> kfree(entries); >> op->value = result; > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 19, 2014 at 10:40:19AM -0500, Boris Ostrovsky wrote: > >>diff --git a/drivers/xen/xen-pciback/pciback_ops.c > >>b/drivers/xen/xen-pciback/pciback_ops.c > >>index 64eb0cd..f5b4c3e 100644 > >>--- a/drivers/xen/xen-pciback/pciback_ops.c > >>+++ b/drivers/xen/xen-pciback/pciback_ops.c > >>@@ -213,9 +213,15 @@ int xen_pcibk_enable_msix(struct > >>xen_pcibk_device *pdev, > >> entries[i].vector = op->msix_entries[i].vector; > >> } > >> - result = pci_enable_msix(dev, entries, op->value); > >>+ result = pci_enable_msix_range(dev, entries, op->value, op->value); > >>+ if (result < op->value) { > > > > > >I think it would be better to have 'if (result != op->value)', in > >case op->value is negative (which presumably it should never be). > > > Better yet, at the top of the routine we check 'if (op->value > > SH_INFO_MAX_VEC)'. If you add '|| op->value < 0' we'd be all set. xen_pci_op::value is uint32_t > -boris
On 02/19/2014 11:05 AM, Alexander Gordeev wrote: > On Wed, Feb 19, 2014 at 10:40:19AM -0500, Boris Ostrovsky wrote: >>>> diff --git a/drivers/xen/xen-pciback/pciback_ops.c >>>> b/drivers/xen/xen-pciback/pciback_ops.c >>>> index 64eb0cd..f5b4c3e 100644 >>>> --- a/drivers/xen/xen-pciback/pciback_ops.c >>>> +++ b/drivers/xen/xen-pciback/pciback_ops.c >>>> @@ -213,9 +213,15 @@ int xen_pcibk_enable_msix(struct >>>> xen_pcibk_device *pdev, >>>> entries[i].vector = op->msix_entries[i].vector; >>>> } >>>> - result = pci_enable_msix(dev, entries, op->value); >>>> + result = pci_enable_msix_range(dev, entries, op->value, op->value); >>>> + if (result < op->value) { >>> >>> I think it would be better to have 'if (result != op->value)', in >>> case op->value is negative (which presumably it should never be). >> >> Better yet, at the top of the routine we check 'if (op->value > >> SH_INFO_MAX_VEC)'. If you add '|| op->value < 0' we'd be all set. > xen_pci_op::value is uint32_t Ah, OK --- then 'if (op->value > SH_INFO_MAX_VEC)' alone will catch this (hopefully its' not in billions). Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 19, 2014 at 11:24:29AM -0500, Boris Ostrovsky wrote: Hi Boris et al, Based on recently accepted to the mainline pci_enable_msix_exact() function, I am sending an updated version of the patch.
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c index 64eb0cd..f5b4c3e 100644 --- a/drivers/xen/xen-pciback/pciback_ops.c +++ b/drivers/xen/xen-pciback/pciback_ops.c @@ -213,9 +213,15 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, entries[i].vector = op->msix_entries[i].vector; } - result = pci_enable_msix(dev, entries, op->value); + result = pci_enable_msix_range(dev, entries, op->value, op->value); + if (result < op->value) { + if (result > 0) + pci_disable_msix(dev); - if (result == 0) { + pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: err %d!\n", + pci_name(dev), pdev->xdev->otherend_id, + result); + } else { for (i = 0; i < op->value; i++) { op->msix_entries[i].entry = entries[i].entry; if (entries[i].vector) @@ -227,10 +233,8 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, pci_name(dev), i, op->msix_entries[i].vector); } - } else - pr_warn_ratelimited("%s: error enabling MSI-X for guest %u: err %d!\n", - pci_name(dev), pdev->xdev->otherend_id, - result); + } + kfree(entries); op->value = result;
As result of deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces. Signed-off-by: Alexander Gordeev <agordeev@redhat.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: David Vrabel <david.vrabel@citrix.com> Cc: linux-pci@vger.kernel.org --- drivers/xen/xen-pciback/pciback_ops.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)