2014-02-19 10:15:07

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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 <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: David Vrabel <[email protected]>
Cc: [email protected]
---
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) {
+ 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;
--
1.7.7.6


2014-02-19 15:21:25

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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 <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: David Vrabel <[email protected]>
> Cc: [email protected]
> ---
> 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;

2014-02-19 15:38:47

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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 <[email protected]>
>> Cc: Konrad Rzeszutek Wilk <[email protected]>
>> Cc: Boris Ostrovsky <[email protected]>
>> Cc: David Vrabel <[email protected]>
>> Cc: [email protected]
>> ---
>> 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;
>

2014-02-19 16:03:08

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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

--
Regards,
Alexander Gordeev
[email protected]

2014-02-19 16:23:04

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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 <[email protected]>

2014-02-21 16:50:46

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH] xen-pciback: Use pci_enable_msix_range() instead of pci_enable_msix()

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.

--
Regards,
Alexander Gordeev
[email protected]

2014-02-21 16:51:45

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

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() or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Signed-off-by: Alexander Gordeev <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: David Vrabel <[email protected]>
Cc: [email protected]
---
drivers/xen/xen-pciback/pciback_ops.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 64eb0cd..929dd46 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -213,8 +213,7 @@ 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_exact(dev, entries, op->value);
if (result == 0) {
for (i = 0; i < op->value; i++) {
op->msix_entries[i].entry = entries[i].entry;
--
1.7.7.6

--
Regards,
Alexander Gordeev
[email protected]

2014-02-21 17:00:52

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

Hi Boris et al,

Based on recently accepted to the mainline pci_enable_msix_exact() function,
I am sending a updated version of the patch. Please, let me know if it does
not work for you and you need and incremental update from the previous version.

Thanks!

--
Regards,
Alexander Gordeev
[email protected]

2014-02-21 18:31:59

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On 02/21/2014 11:53 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() or pci_enable_msi_exact()
> and pci_enable_msix_range() or pci_enable_msix_exact()
> interfaces.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: David Vrabel <[email protected]>
> Cc: [email protected]

Reviewed-by: Boris Ostrovsky <[email protected]>

> ---
> drivers/xen/xen-pciback/pciback_ops.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
> index 64eb0cd..929dd46 100644
> --- a/drivers/xen/xen-pciback/pciback_ops.c
> +++ b/drivers/xen/xen-pciback/pciback_ops.c
> @@ -213,8 +213,7 @@ 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_exact(dev, entries, op->value);
> if (result == 0) {
> for (i = 0; i < op->value; i++) {
> op->msix_entries[i].entry = entries[i].entry;

2014-02-21 18:57:09

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On Fri, Feb 21, 2014 at 01:33:34PM -0500, Boris Ostrovsky wrote:
> Reviewed-by: Boris Ostrovsky <[email protected]>

Thank you, Boris!
Whom should I ask this patch for inclusion?

--
Regards,
Alexander Gordeev
[email protected]

2014-02-21 19:07:30

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On 02/21/2014 01:58 PM, Alexander Gordeev wrote:
> On Fri, Feb 21, 2014 at 01:33:34PM -0500, Boris Ostrovsky wrote:
>> Reviewed-by: Boris Ostrovsky <[email protected]>
> Thank you, Boris!
> Whom should I ask this patch for inclusion?
>

This should go into the Xen tree. Konrad is usually the one putting
together pull requests for Linus.

I don't think this needs to go in right away so presumably this will be
queued for the next merge window (unless Konrad has other things for
next RCs and decides to tack this onto other patches).


-boris

2014-02-21 21:15:19

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On Fri, Feb 21, 2014 at 02:09:02PM -0500, Boris Ostrovsky wrote:
> On 02/21/2014 01:58 PM, Alexander Gordeev wrote:
> >On Fri, Feb 21, 2014 at 01:33:34PM -0500, Boris Ostrovsky wrote:
> >>Reviewed-by: Boris Ostrovsky <[email protected]>
> >Thank you, Boris!
> >Whom should I ask this patch for inclusion?
> >
>
> This should go into the Xen tree. Konrad is usually the one putting
> together pull requests for Linus.
>
> I don't think this needs to go in right away so presumably this will
> be queued for the next merge window (unless Konrad has other things
> for next RCs and decides to tack this onto other patches).

I was thinking for next one. Will have a branch ready on Monday.
>
>
> -boris

2014-02-24 19:18:48

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On Fri, Feb 21, 2014 at 05:53:40PM +0100, 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() or pci_enable_msi_exact()
> and pci_enable_msix_range() or pci_enable_msix_exact()
> interfaces.

What is this based on? Thanks.

>
> Signed-off-by: Alexander Gordeev <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: David Vrabel <[email protected]>
> Cc: [email protected]
> ---
> drivers/xen/xen-pciback/pciback_ops.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
> index 64eb0cd..929dd46 100644
> --- a/drivers/xen/xen-pciback/pciback_ops.c
> +++ b/drivers/xen/xen-pciback/pciback_ops.c
> @@ -213,8 +213,7 @@ 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_exact(dev, entries, op->value);
> if (result == 0) {
> for (i = 0; i < op->value; i++) {
> op->msix_entries[i].entry = entries[i].entry;
> --
> 1.7.7.6
>
> --
> Regards,
> Alexander Gordeev
> [email protected]

2014-02-25 08:30:46

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH v2] xen-pciback: Use pci_enable_msix_exact() instead of pci_enable_msix()

On Mon, Feb 24, 2014 at 02:18:31PM -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Feb 21, 2014 at 05:53:40PM +0100, 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() or pci_enable_msi_exact()
> > and pci_enable_msix_range() or pci_enable_msix_exact()
> > interfaces.
>
> What is this based on? Thanks.

3.14-rc4

--
Regards,
Alexander Gordeev
[email protected]