[PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Piotr Stankiewicz]

When debugging an issue where I was asking the PCI machinery to enable a
set of MSI-X vectors, without falling back on MSI, I ran across a
behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
always return -ENOSPC on failure, when allocating MSI-X vectors only,
whereas with MSI fallback it will forward any error returned by
__pci_enable_msi_range(). This is a confusing behaviour, so have the
pci_alloc_irq_vectors_affinity() forward the error code from
__pci_enable_msix_range() when appropriate.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/pci/msi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 6b43a5455c7a..443cc324b196 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1231,8 +1231,9 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 		}
 	}
 
-	if (msix_vecs == -ENOSPC)
-		return -ENOSPC;
+	if (msix_vecs == -ENOSPC ||
+	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
+		return msix_vecs;
 	return msi_vecs;
 }
 EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
-- 
2.17.2

Re: [PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Logan Gunthorpe]

On 2020-06-03 5:44 a.m., Piotr Stankiewicz wrote:
> When debugging an issue where I was asking the PCI machinery to enable a
> set of MSI-X vectors, without falling back on MSI, I ran across a
> behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
> always return -ENOSPC on failure, when allocating MSI-X vectors only,
> whereas with MSI fallback it will forward any error returned by
> __pci_enable_msi_range(). This is a confusing behaviour, so have the
> pci_alloc_irq_vectors_affinity() forward the error code from
> __pci_enable_msix_range() when appropriate.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  drivers/pci/msi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 6b43a5455c7a..443cc324b196 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1231,8 +1231,9 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  		}
>  	}
>  
> -	if (msix_vecs == -ENOSPC)
> -		return -ENOSPC;
> +	if (msix_vecs == -ENOSPC ||
> +	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
> +		return msix_vecs;
>  	return msi_vecs;
>  }
>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
> 

It occurs to me that we could clean this function up a bit more... I
don't see any need to have two variables for msi_vecs and msix_vecs and
then have a complicated bit of logic at the end to decide which to return.

Why not instead just have one variable which is set by
__pci_enable_msix_range(), then __pci_enable_msi_range(), then returned
if they both fail?

Logan

RE: [PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Stankiewicz, Piotr]

> -----Original Message-----
> From: Logan Gunthorpe <logang@deltatee.com>
> Sent: Wednesday, June 3, 2020 5:48 PM
> 
> 
> 
> On 2020-06-03 5:44 a.m., Piotr Stankiewicz wrote:
> > When debugging an issue where I was asking the PCI machinery to enable a
> > set of MSI-X vectors, without falling back on MSI, I ran across a
> > behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
> > always return -ENOSPC on failure, when allocating MSI-X vectors only,
> > whereas with MSI fallback it will forward any error returned by
> > __pci_enable_msi_range(). This is a confusing behaviour, so have the
> > pci_alloc_irq_vectors_affinity() forward the error code from
> > __pci_enable_msix_range() when appropriate.
> >
> > Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > ---
> >  drivers/pci/msi.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index 6b43a5455c7a..443cc324b196 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -1231,8 +1231,9 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev
> *dev, unsigned int min_vecs,
> >  		}
> >  	}
> >
> > -	if (msix_vecs == -ENOSPC)
> > -		return -ENOSPC;
> > +	if (msix_vecs == -ENOSPC ||
> > +	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
> > +		return msix_vecs;
> >  	return msi_vecs;
> >  }
> >  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
> >
> 
> It occurs to me that we could clean this function up a bit more... I
> don't see any need to have two variables for msi_vecs and msix_vecs and
> then have a complicated bit of logic at the end to decide which to return.
> 
> Why not instead just have one variable which is set by
> __pci_enable_msix_range(), then __pci_enable_msi_range(), then returned
> if they both fail?
>

That wouldn't preserve the original bit of logic where -ENOSPC is returned
any time __pci_enable_msix_range() fails with -ENOSPC, irrespective of whether
MSI fallback was requested. Though I don't know if that is desired behaviour.
 
BR,
Piotr

Re: [PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Logan Gunthorpe]

On 2020-06-03 10:04 a.m., Stankiewicz, Piotr wrote:
>> -----Original Message-----
>> From: Logan Gunthorpe <logang@deltatee.com>
>> Sent: Wednesday, June 3, 2020 5:48 PM
>>
>>
>>
>> On 2020-06-03 5:44 a.m., Piotr Stankiewicz wrote:
>>> When debugging an issue where I was asking the PCI machinery to enable a
>>> set of MSI-X vectors, without falling back on MSI, I ran across a
>>> behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
>>> always return -ENOSPC on failure, when allocating MSI-X vectors only,
>>> whereas with MSI fallback it will forward any error returned by
>>> __pci_enable_msi_range(). This is a confusing behaviour, so have the
>>> pci_alloc_irq_vectors_affinity() forward the error code from
>>> __pci_enable_msix_range() when appropriate.
>>>
>>> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>>> ---
>>>  drivers/pci/msi.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>>> index 6b43a5455c7a..443cc324b196 100644
>>> --- a/drivers/pci/msi.c
>>> +++ b/drivers/pci/msi.c
>>> @@ -1231,8 +1231,9 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev
>> *dev, unsigned int min_vecs,
>>>  		}
>>>  	}
>>>
>>> -	if (msix_vecs == -ENOSPC)
>>> -		return -ENOSPC;
>>> +	if (msix_vecs == -ENOSPC ||
>>> +	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
>>> +		return msix_vecs;
>>>  	return msi_vecs;
>>>  }
>>>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
>>>
>>
>> It occurs to me that we could clean this function up a bit more... I
>> don't see any need to have two variables for msi_vecs and msix_vecs and
>> then have a complicated bit of logic at the end to decide which to return.
>>
>> Why not instead just have one variable which is set by
>> __pci_enable_msix_range(), then __pci_enable_msi_range(), then returned
>> if they both fail?
>>
> 
> That wouldn't preserve the original bit of logic where -ENOSPC is returned
> any time __pci_enable_msix_range() fails with -ENOSPC, irrespective of whether
> MSI fallback was requested. Though I don't know if that is desired behaviour.

That does look very odd, but ok... Then, couldn't we just set msi_vecs
to msix_vecs after calling __pci_enable_msix_range() such that if
__pci_enable_msi_range() doesn't get called we will return the same
error without needing the messy second conditional?

Logan

RE: [PATCH v2 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Stankiewicz, Piotr]

> -----Original Message-----
> From: Logan Gunthorpe <logang@deltatee.com>
> Sent: Wednesday, June 3, 2020 6:22 PM 
> 
> 
> On 2020-06-03 10:04 a.m., Stankiewicz, Piotr wrote:
> >> -----Original Message-----
> >> From: Logan Gunthorpe <logang@deltatee.com>
> >> Sent: Wednesday, June 3, 2020 5:48 PM
> >>
> >>
> >>
> >> On 2020-06-03 5:44 a.m., Piotr Stankiewicz wrote:
> >>> When debugging an issue where I was asking the PCI machinery to enable a
> >>> set of MSI-X vectors, without falling back on MSI, I ran across a
> >>> behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
> >>> always return -ENOSPC on failure, when allocating MSI-X vectors only,
> >>> whereas with MSI fallback it will forward any error returned by
> >>> __pci_enable_msi_range(). This is a confusing behaviour, so have the
> >>> pci_alloc_irq_vectors_affinity() forward the error code from
> >>> __pci_enable_msix_range() when appropriate.
> >>>
> >>> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> >>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >>> ---
> >>>  drivers/pci/msi.c | 5 +++--
> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> >>> index 6b43a5455c7a..443cc324b196 100644
> >>> --- a/drivers/pci/msi.c
> >>> +++ b/drivers/pci/msi.c
> >>> @@ -1231,8 +1231,9 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev
> >> *dev, unsigned int min_vecs,
> >>>  		}
> >>>  	}
> >>>
> >>> -	if (msix_vecs == -ENOSPC)
> >>> -		return -ENOSPC;
> >>> +	if (msix_vecs == -ENOSPC ||
> >>> +	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
> >>> +		return msix_vecs;
> >>>  	return msi_vecs;
> >>>  }
> >>>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
> >>>
> >>
> >> It occurs to me that we could clean this function up a bit more... I
> >> don't see any need to have two variables for msi_vecs and msix_vecs and
> >> then have a complicated bit of logic at the end to decide which to return.
> >>
> >> Why not instead just have one variable which is set by
> >> __pci_enable_msix_range(), then __pci_enable_msi_range(), then returned
> >> if they both fail?
> >>
> >
> > That wouldn't preserve the original bit of logic where -ENOSPC is returned
> > any time __pci_enable_msix_range() fails with -ENOSPC, irrespective of
> whether
> > MSI fallback was requested. Though I don't know if that is desired behaviour.
> 
> That does look very odd, but ok... Then, couldn't we just set msi_vecs
> to msix_vecs after calling __pci_enable_msix_range() such that if
> __pci_enable_msi_range() doesn't get called we will return the same
> error without needing the messy second conditional?

Having thought about it a bit more - the original behavior seems broken because
in case someone asked for MSI only and that errored we'd always return -ENOSPC.
So I went with your original suggestion of having a single return code (I just sent out
a v3).

Thanks,
Piotr

[PATCH v2 03/15] PCI: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/msi.c               | 3 +--
 drivers/pci/pcie/portdrv_core.c | 4 ++--
 drivers/pci/switch/switchtec.c  | 3 +--
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 443cc324b196..9db9ce5dddb3 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1231,8 +1231,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 		}
 	}
 
-	if (msix_vecs == -ENOSPC ||
-	    (flags & (PCI_IRQ_MSI | PCI_IRQ_MSIX)) == PCI_IRQ_MSIX)
+	if (msix_vecs == -ENOSPC || (flags & PCI_IRQ_MSI_TYPES) == PCI_IRQ_MSIX)
 		return msix_vecs;
 	return msi_vecs;
 }
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 50a9522ab07d..2a38a918ba12 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -105,7 +105,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 
 	/* Allocate the maximum possible number of MSI/MSI-X vectors */
 	nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES,
-			PCI_IRQ_MSIX | PCI_IRQ_MSI);
+			PCI_IRQ_MSI_TYPES);
 	if (nr_entries < 0)
 		return nr_entries;
 
@@ -131,7 +131,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 		pci_free_irq_vectors(dev);
 
 		nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI);
+				PCI_IRQ_MSI_TYPES);
 		if (nr_entries < 0)
 			return nr_entries;
 	}
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index e69cac84b605..11fbe9c6b201 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1442,8 +1442,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 		nirqs = 4;
 
 	nvecs = pci_alloc_irq_vectors(stdev->pdev, 1, nirqs,
-				      PCI_IRQ_MSIX | PCI_IRQ_MSI |
-				      PCI_IRQ_VIRTUAL);
+				      PCI_IRQ_MSI_TYPES | PCI_IRQ_VIRTUAL);
 	if (nvecs < 0)
 		return nvecs;
 
-- 
2.17.2

[PATCH v2 04/15] ahci: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/ata/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 0c0a736eb861..ca1bf4ef0f17 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1556,7 +1556,7 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports,
 	 */
 	if (n_ports > 1) {
 		nvec = pci_alloc_irq_vectors(pdev, n_ports, INT_MAX,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI);
+				PCI_IRQ_MSI_TYPES);
 		if (nvec > 0) {
 			if (!(readl(hpriv->mmio + HOST_CTL) & HOST_MRSM)) {
 				hpriv->get_irq_vector = ahci_get_irq_vector;
-- 
2.17.2

[PATCH v2 05/15] crypto: inside-secure - Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Antoine Tenart <antoine.tenart@bootlin.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 2cb53fbae841..1b2faa2a6ab0 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -1567,7 +1567,7 @@ static int safexcel_probe_generic(void *pdev,
 		ret = pci_alloc_irq_vectors(pci_pdev,
 					    priv->config.rings + 1,
 					    priv->config.rings + 1,
-					    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					    PCI_IRQ_MSI_TYPES);
 		if (ret < 0) {
 			dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
 			return ret;
-- 
2.17.2

Re: [PATCH v2 05/15] crypto: inside-secure - Use PCI_IRQ_MSI_TYPES where appropriate

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 18852 bytes --]

Hi Piotr,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on mkp-scsi/for-next scsi/for-next linus/master v5.7 next-20200603]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Piotr-Stankiewicz/Forward-MSI-X-vector-enable-error-code-in-pci_alloc_irq_vectors_affinity/20200603-195246
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/crypto/inside-secure/safexcel.c:649:11: note: in expansion of macro 'GENMASK'
649 |           GENMASK(priv->config.rings - 1, 0),
|           ^~~~~~~
include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                            ^
arch/sh/include/asm/io.h:32:77: note: in definition of macro '__raw_writel'
32 | #define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) = (v))
|                                                                             ^
arch/sh/include/asm/io.h:47:62: note: in expansion of macro 'ioswabl'
47 | #define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
|                                                              ^~~~~~~
arch/sh/include/asm/io.h:57:32: note: in expansion of macro 'writel_relaxed'
57 | #define writel(v,a)  ({ wmb(); writel_relaxed((v),(a)); })
|                                ^~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:757:3: note: in expansion of macro 'writel'
757 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|   ^~~~~~
include/linux/bits.h:25:3: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
25 |  (BUILD_BUG_ON_ZERO(__builtin_choose_expr(          |   ^~~~~~~~~~~~~~~~~
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:757:35: note: in expansion of macro 'GENMASK'
757 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|                                   ^~~~~~~
include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                                        ^
arch/sh/include/asm/io.h:32:77: note: in definition of macro '__raw_writel'
32 | #define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) = (v))
|                                                                             ^
arch/sh/include/asm/io.h:47:62: note: in expansion of macro 'ioswabl'
47 | #define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
|                                                              ^~~~~~~
arch/sh/include/asm/io.h:57:32: note: in expansion of macro 'writel_relaxed'
57 | #define writel(v,a)  ({ wmb(); writel_relaxed((v),(a)); })
|                                ^~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:757:3: note: in expansion of macro 'writel'
757 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|   ^~~~~~
include/linux/bits.h:25:3: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
25 |  (BUILD_BUG_ON_ZERO(__builtin_choose_expr(          |   ^~~~~~~~~~~~~~~~~
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:757:35: note: in expansion of macro 'GENMASK'
757 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|                                   ^~~~~~~
include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                            ^
arch/sh/include/asm/io.h:32:77: note: in definition of macro '__raw_writel'
32 | #define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) = (v))
|                                                                             ^
arch/sh/include/asm/io.h:47:62: note: in expansion of macro 'ioswabl'
47 | #define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
|                                                              ^~~~~~~
arch/sh/include/asm/io.h:57:32: note: in expansion of macro 'writel_relaxed'
57 | #define writel(v,a)  ({ wmb(); writel_relaxed((v),(a)); })
|                                ^~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:761:3: note: in expansion of macro 'writel'
761 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|   ^~~~~~
include/linux/bits.h:25:3: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
25 |  (BUILD_BUG_ON_ZERO(__builtin_choose_expr(          |   ^~~~~~~~~~~~~~~~~
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:761:35: note: in expansion of macro 'GENMASK'
761 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|                                   ^~~~~~~
include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
|                                        ^
arch/sh/include/asm/io.h:32:77: note: in definition of macro '__raw_writel'
32 | #define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) = (v))
|                                                                             ^
arch/sh/include/asm/io.h:47:62: note: in expansion of macro 'ioswabl'
47 | #define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
|                                                              ^~~~~~~
arch/sh/include/asm/io.h:57:32: note: in expansion of macro 'writel_relaxed'
57 | #define writel(v,a)  ({ wmb(); writel_relaxed((v),(a)); })
|                                ^~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:761:3: note: in expansion of macro 'writel'
761 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|   ^~~~~~
include/linux/bits.h:25:3: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
25 |  (BUILD_BUG_ON_ZERO(__builtin_choose_expr(          |   ^~~~~~~~~~~~~~~~~
include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|   ^~~~~~~~~~~~~~~~~~~
drivers/crypto/inside-secure/safexcel.c:761:35: note: in expansion of macro 'GENMASK'
761 |   writel(EIP197_DxE_THR_CTRL_EN | GENMASK(priv->config.rings - 1, 0),
|                                   ^~~~~~~
drivers/crypto/inside-secure/safexcel.c: In function 'safexcel_probe_generic':
<<                  from drivers/crypto/inside-secure/safexcel.c:10:
>> drivers/crypto/inside-secure/safexcel.c:1570:10: error: 'PCI_IRQ_MSI_TYPES' undeclared (first use in this function); did you mean 'PCI_IRQ_MSIX'?
1570 |          PCI_IRQ_MSI_TYPES);
|          ^~~~~~~~~~~~~~~~~
|          PCI_IRQ_MSIX
drivers/crypto/inside-secure/safexcel.c:1570:10: note: each undeclared identifier is reported only once for each function it appears in

vim +1570 drivers/crypto/inside-secure/safexcel.c

  1382	
  1383	/*
  1384	 * Generic part of probe routine, shared by platform and PCI driver
  1385	 *
  1386	 * Assumes IO resources have been mapped, private data mem has been allocated,
  1387	 * clocks have been enabled, device pointer has been assigned etc.
  1388	 *
  1389	 */
  1390	static int safexcel_probe_generic(void *pdev,
  1391					  struct safexcel_crypto_priv *priv,
  1392					  int is_pci_dev)
  1393	{
  1394		struct device *dev = priv->dev;
  1395		u32 peid, version, mask, val, hiaopt, hwopt, peopt;
  1396		int i, ret, hwctg;
  1397	
  1398		priv->context_pool = dmam_pool_create("safexcel-context", dev,
  1399						      sizeof(struct safexcel_context_record),
  1400						      1, 0);
  1401		if (!priv->context_pool)
  1402			return -ENOMEM;
  1403	
  1404		/*
  1405		 * First try the EIP97 HIA version regs
  1406		 * For the EIP197, this is guaranteed to NOT return any of the test
  1407		 * values
  1408		 */
  1409		version = readl(priv->base + EIP97_HIA_AIC_BASE + EIP197_HIA_VERSION);
  1410	
  1411		mask = 0;  /* do not swap */
  1412		if (EIP197_REG_LO16(version) == EIP197_HIA_VERSION_LE) {
  1413			priv->hwconfig.hiaver = EIP197_VERSION_MASK(version);
  1414		} else if (EIP197_REG_HI16(version) == EIP197_HIA_VERSION_BE) {
  1415			/* read back byte-swapped, so complement byte swap bits */
  1416			mask = EIP197_MST_CTRL_BYTE_SWAP_BITS;
  1417			priv->hwconfig.hiaver = EIP197_VERSION_SWAP(version);
  1418		} else {
  1419			/* So it wasn't an EIP97 ... maybe it's an EIP197? */
  1420			version = readl(priv->base + EIP197_HIA_AIC_BASE +
  1421					EIP197_HIA_VERSION);
  1422			if (EIP197_REG_LO16(version) == EIP197_HIA_VERSION_LE) {
  1423				priv->hwconfig.hiaver = EIP197_VERSION_MASK(version);
  1424				priv->flags |= SAFEXCEL_HW_EIP197;
  1425			} else if (EIP197_REG_HI16(version) ==
  1426				   EIP197_HIA_VERSION_BE) {
  1427				/* read back byte-swapped, so complement swap bits */
  1428				mask = EIP197_MST_CTRL_BYTE_SWAP_BITS;
  1429				priv->hwconfig.hiaver = EIP197_VERSION_SWAP(version);
  1430				priv->flags |= SAFEXCEL_HW_EIP197;
  1431			} else {
  1432				return -ENODEV;
  1433			}
  1434		}
  1435	
  1436		/* Now initialize the reg offsets based on the probing info so far */
  1437		safexcel_init_register_offsets(priv);
  1438	
  1439		/*
  1440		 * If the version was read byte-swapped, we need to flip the device
  1441		 * swapping Keep in mind here, though, that what we write will also be
  1442		 * byte-swapped ...
  1443		 */
  1444		if (mask) {
  1445			val = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
  1446			val = val ^ (mask >> 24); /* toggle byte swap bits */
  1447			writel(val, EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
  1448		}
  1449	
  1450		/*
  1451		 * We're not done probing yet! We may fall through to here if no HIA
  1452		 * was found at all. So, with the endianness presumably correct now and
  1453		 * the offsets setup, *really* probe for the EIP97/EIP197.
  1454		 */
  1455		version = readl(EIP197_GLOBAL(priv) + EIP197_VERSION);
  1456		if (((priv->flags & SAFEXCEL_HW_EIP197) &&
  1457		     (EIP197_REG_LO16(version) != EIP197_VERSION_LE) &&
  1458		     (EIP197_REG_LO16(version) != EIP196_VERSION_LE)) ||
  1459		    ((!(priv->flags & SAFEXCEL_HW_EIP197) &&
  1460		     (EIP197_REG_LO16(version) != EIP97_VERSION_LE)))) {
  1461			/*
  1462			 * We did not find the device that matched our initial probing
  1463			 * (or our initial probing failed) Report appropriate error.
  1464			 */
  1465			dev_err(priv->dev, "Probing for EIP97/EIP19x failed - no such device (read %08x)\n",
  1466				version);
  1467			return -ENODEV;
  1468		}
  1469	
  1470		priv->hwconfig.hwver = EIP197_VERSION_MASK(version);
  1471		hwctg = version >> 28;
  1472		peid = version & 255;
  1473	
  1474		/* Detect EIP206 processing pipe */
  1475		version = readl(EIP197_PE(priv) + + EIP197_PE_VERSION(0));
  1476		if (EIP197_REG_LO16(version) != EIP206_VERSION_LE) {
  1477			dev_err(priv->dev, "EIP%d: EIP206 not detected\n", peid);
  1478			return -ENODEV;
  1479		}
  1480		priv->hwconfig.ppver = EIP197_VERSION_MASK(version);
  1481	
  1482		/* Detect EIP96 packet engine and version */
  1483		version = readl(EIP197_PE(priv) + EIP197_PE_EIP96_VERSION(0));
  1484		if (EIP197_REG_LO16(version) != EIP96_VERSION_LE) {
  1485			dev_err(dev, "EIP%d: EIP96 not detected.\n", peid);
  1486			return -ENODEV;
  1487		}
  1488		priv->hwconfig.pever = EIP197_VERSION_MASK(version);
  1489	
  1490		hwopt = readl(EIP197_GLOBAL(priv) + EIP197_OPTIONS);
  1491		hiaopt = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_OPTIONS);
  1492	
  1493		if (priv->flags & SAFEXCEL_HW_EIP197) {
  1494			/* EIP197 */
  1495			peopt = readl(EIP197_PE(priv) + EIP197_PE_OPTIONS(0));
  1496	
  1497			priv->hwconfig.hwdataw  = (hiaopt >> EIP197_HWDATAW_OFFSET) &
  1498						  EIP197_HWDATAW_MASK;
  1499			priv->hwconfig.hwcfsize = ((hiaopt >> EIP197_CFSIZE_OFFSET) &
  1500						   EIP197_CFSIZE_MASK) +
  1501						  EIP197_CFSIZE_ADJUST;
  1502			priv->hwconfig.hwrfsize = ((hiaopt >> EIP197_RFSIZE_OFFSET) &
  1503						   EIP197_RFSIZE_MASK) +
  1504						  EIP197_RFSIZE_ADJUST;
  1505			priv->hwconfig.hwnumpes	= (hiaopt >> EIP197_N_PES_OFFSET) &
  1506						  EIP197_N_PES_MASK;
  1507			priv->hwconfig.hwnumrings = (hiaopt >> EIP197_N_RINGS_OFFSET) &
  1508						    EIP197_N_RINGS_MASK;
  1509			if (hiaopt & EIP197_HIA_OPT_HAS_PE_ARB)
  1510				priv->flags |= EIP197_PE_ARB;
  1511			if (EIP206_OPT_ICE_TYPE(peopt) == 1)
  1512				priv->flags |= EIP197_ICE;
  1513			/* If not a full TRC, then assume simple TRC */
  1514			if (!(hwopt & EIP197_OPT_HAS_TRC))
  1515				priv->flags |= EIP197_SIMPLE_TRC;
  1516			/* EIP197 always has SOME form of TRC */
  1517			priv->flags |= EIP197_TRC_CACHE;
  1518		} else {
  1519			/* EIP97 */
  1520			priv->hwconfig.hwdataw  = (hiaopt >> EIP197_HWDATAW_OFFSET) &
  1521						  EIP97_HWDATAW_MASK;
  1522			priv->hwconfig.hwcfsize = (hiaopt >> EIP97_CFSIZE_OFFSET) &
  1523						  EIP97_CFSIZE_MASK;
  1524			priv->hwconfig.hwrfsize = (hiaopt >> EIP97_RFSIZE_OFFSET) &
  1525						  EIP97_RFSIZE_MASK;
  1526			priv->hwconfig.hwnumpes	= 1; /* by definition */
  1527			priv->hwconfig.hwnumrings = (hiaopt >> EIP197_N_RINGS_OFFSET) &
  1528						    EIP197_N_RINGS_MASK;
  1529		}
  1530	
  1531		/* Scan for ring AIC's */
  1532		for (i = 0; i < EIP197_MAX_RING_AIC; i++) {
  1533			version = readl(EIP197_HIA_AIC_R(priv) +
  1534					EIP197_HIA_AIC_R_VERSION(i));
  1535			if (EIP197_REG_LO16(version) != EIP201_VERSION_LE)
  1536				break;
  1537		}
  1538		priv->hwconfig.hwnumraic = i;
  1539		/* Low-end EIP196 may not have any ring AIC's ... */
  1540		if (!priv->hwconfig.hwnumraic) {
  1541			dev_err(priv->dev, "No ring interrupt controller present!\n");
  1542			return -ENODEV;
  1543		}
  1544	
  1545		/* Get supported algorithms from EIP96 transform engine */
  1546		priv->hwconfig.algo_flags = readl(EIP197_PE(priv) +
  1547					    EIP197_PE_EIP96_OPTIONS(0));
  1548	
  1549		/* Print single info line describing what we just detected */
  1550		dev_info(priv->dev, "EIP%d:%x(%d,%d,%d,%d)-HIA:%x(%d,%d,%d),PE:%x/%x,alg:%08x\n",
  1551			 peid, priv->hwconfig.hwver, hwctg, priv->hwconfig.hwnumpes,
  1552			 priv->hwconfig.hwnumrings, priv->hwconfig.hwnumraic,
  1553			 priv->hwconfig.hiaver, priv->hwconfig.hwdataw,
  1554			 priv->hwconfig.hwcfsize, priv->hwconfig.hwrfsize,
  1555			 priv->hwconfig.ppver, priv->hwconfig.pever,
  1556			 priv->hwconfig.algo_flags);
  1557	
  1558		safexcel_configure(priv);
  1559	
  1560		if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) {
  1561			/*
  1562			 * Request MSI vectors for global + 1 per ring -
  1563			 * or just 1 for older dev images
  1564			 */
  1565			struct pci_dev *pci_pdev = pdev;
  1566	
  1567			ret = pci_alloc_irq_vectors(pci_pdev,
  1568						    priv->config.rings + 1,
  1569						    priv->config.rings + 1,
> 1570						    PCI_IRQ_MSI_TYPES);
  1571			if (ret < 0) {
  1572				dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
  1573				return ret;
  1574			}
  1575		}
  1576	
  1577		/* Register the ring IRQ handlers and configure the rings */
  1578		priv->ring = devm_kcalloc(dev, priv->config.rings,
  1579					  sizeof(*priv->ring),
  1580					  GFP_KERNEL);
  1581		if (!priv->ring)
  1582			return -ENOMEM;
  1583	
  1584		for (i = 0; i < priv->config.rings; i++) {
  1585			char wq_name[9] = {0};
  1586			int irq;
  1587			struct safexcel_ring_irq_data *ring_irq;
  1588	
  1589			ret = safexcel_init_ring_descriptors(priv,
  1590							     &priv->ring[i].cdr,
  1591							     &priv->ring[i].rdr);
  1592			if (ret) {
  1593				dev_err(dev, "Failed to initialize rings\n");
  1594				return ret;
  1595			}
  1596	
  1597			priv->ring[i].rdr_req = devm_kcalloc(dev,
  1598				EIP197_DEFAULT_RING_SIZE,
  1599				sizeof(priv->ring[i].rdr_req),
  1600				GFP_KERNEL);
  1601			if (!priv->ring[i].rdr_req)
  1602				return -ENOMEM;
  1603	
  1604			ring_irq = devm_kzalloc(dev, sizeof(*ring_irq), GFP_KERNEL);
  1605			if (!ring_irq)
  1606				return -ENOMEM;
  1607	
  1608			ring_irq->priv = priv;
  1609			ring_irq->ring = i;
  1610	
  1611			irq = safexcel_request_ring_irq(pdev,
  1612							EIP197_IRQ_NUMBER(i, is_pci_dev),
  1613							is_pci_dev,
  1614							safexcel_irq_ring,
  1615							safexcel_irq_ring_thread,
  1616							ring_irq);
  1617			if (irq < 0) {
  1618				dev_err(dev, "Failed to get IRQ ID for ring %d\n", i);
  1619				return irq;
  1620			}
  1621	
  1622			priv->ring[i].work_data.priv = priv;
  1623			priv->ring[i].work_data.ring = i;
  1624			INIT_WORK(&priv->ring[i].work_data.work,
  1625				  safexcel_dequeue_work);
  1626	
  1627			snprintf(wq_name, 9, "wq_ring%d", i);
  1628			priv->ring[i].workqueue =
  1629				create_singlethread_workqueue(wq_name);
  1630			if (!priv->ring[i].workqueue)
  1631				return -ENOMEM;
  1632	
  1633			priv->ring[i].requests = 0;
  1634			priv->ring[i].busy = false;
  1635	
  1636			crypto_init_queue(&priv->ring[i].queue,
  1637					  EIP197_DEFAULT_RING_SIZE);
  1638	
  1639			spin_lock_init(&priv->ring[i].lock);
  1640			spin_lock_init(&priv->ring[i].queue_lock);
  1641		}
  1642	
  1643		atomic_set(&priv->ring_used, 0);
  1644	
  1645		ret = safexcel_hw_init(priv);
  1646		if (ret) {
  1647			dev_err(dev, "HW init failed (%d)\n", ret);
  1648			return ret;
  1649		}
  1650	
  1651		ret = safexcel_register_algorithms(priv);
  1652		if (ret) {
  1653			dev_err(dev, "Failed to register algorithms (%d)\n", ret);
  1654			return ret;
  1655		}
  1656	
  1657		return 0;
  1658	}
  1659	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54649 bytes --]

[PATCH v2 06/15] dmaengine: dw-edma: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index dc85f55e1bb8..46defe30ac25 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -117,7 +117,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 
 	/* IRQs allocation */
 	nr_irqs = pci_alloc_irq_vectors(pdev, 1, pdata->irqs,
-					PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					PCI_IRQ_MSI_TYPES);
 	if (nr_irqs < 1) {
 		pci_err(pdev, "fail to alloc IRQ vector (number of IRQs=%u)\n",
 			nr_irqs);
-- 
2.17.2

RE: [PATCH v2 06/15] dmaengine: dw-edma: Use PCI_IRQ_MSI_TYPES where appropriate

[Gustavo Pimentel]

On Wed, Jun 3, 2020 at 12:47:42, Piotr Stankiewicz 
<piotr.stankiewicz@intel.com> wrote:

> Seeing as there is shorthand available to use when asking for any type
> of interrupt, or any type of message signalled interrupt, leverage it.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index dc85f55e1bb8..46defe30ac25 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -117,7 +117,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  
>  	/* IRQs allocation */
>  	nr_irqs = pci_alloc_irq_vectors(pdev, 1, pdata->irqs,
> -					PCI_IRQ_MSI | PCI_IRQ_MSIX);
> +					PCI_IRQ_MSI_TYPES);
>  	if (nr_irqs < 1) {
>  		pci_err(pdev, "fail to alloc IRQ vector (number of IRQs=%u)\n",
>  			nr_irqs);
> -- 
> 2.17.2


Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

[PATCH v2 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 5ed4227f304b..2588dd1015db 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -249,15 +249,10 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
 
 	if (amdgpu_msi_ok(adev)) {
 		int nvec = pci_msix_vec_count(adev->pdev);
-		unsigned int flags;
 
-		if (nvec <= 0) {
-			flags = PCI_IRQ_MSI;
-		} else {
-			flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
-		}
 		/* we only need one vector */
-		nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
+		nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1,
+					     PCI_IRQ_MSI_TYPES);
 		if (nvec > 0) {
 			adev->irq.msi_enabled = true;
 			dev_dbg(adev->dev, "amdgpu: using MSI/MSI-X.\n");
-- 
2.17.2

Re: [PATCH v2 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

[Alex Deucher]

On Wed, Jun 3, 2020 at 7:48 AM Piotr Stankiewicz
<piotr.stankiewicz@intel.com> wrote:
>
> Seeing as there is shorthand available to use when asking for any type
> of interrupt, or any type of message signalled interrupt, leverage it.
>
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 5ed4227f304b..2588dd1015db 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -249,15 +249,10 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>
>         if (amdgpu_msi_ok(adev)) {
>                 int nvec = pci_msix_vec_count(adev->pdev);

I think you can drop pci_msix_vec_count() here.  It's not used since
we always request 1 vector.

Alex

> -               unsigned int flags;
>
> -               if (nvec <= 0) {
> -                       flags = PCI_IRQ_MSI;
> -               } else {
> -                       flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
> -               }
>                 /* we only need one vector */
> -               nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
> +               nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1,
> +                                            PCI_IRQ_MSI_TYPES);
>                 if (nvec > 0) {
>                         adev->irq.msi_enabled = true;
>                         dev_dbg(adev->dev, "amdgpu: using MSI/MSI-X.\n");
> --
> 2.17.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 08/15] IB/qib: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 3dc6ce033319..caff44d2c12c 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -213,7 +213,7 @@ int qib_pcie_params(struct qib_devdata *dd, u32 minw, u32 *nent)
 	u16 linkstat, speed;
 	int nvec;
 	int maxvec;
-	unsigned int flags = PCI_IRQ_MSIX | PCI_IRQ_MSI;
+	unsigned int flags;
 
 	if (!pci_is_pcie(dd->pcidev)) {
 		qib_dev_err(dd, "Can't find PCI Express capability!\n");
@@ -225,7 +225,9 @@ int qib_pcie_params(struct qib_devdata *dd, u32 minw, u32 *nent)
 	}
 
 	if (dd->flags & QIB_HAS_INTX)
-		flags |= PCI_IRQ_LEGACY;
+		flags = PCI_IRQ_ALL_TYPES;
+	else
+		flags = PCI_IRQ_MSI_TYPES;
 	maxvec = (nent && *nent) ? *nent : 1;
 	nvec = pci_alloc_irq_vectors(dd->pcidev, 1, maxvec, flags);
 	if (nvec < 0)
-- 
2.17.2

[PATCH v2 09/15] media: ddbridge: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/media/pci/ddbridge/ddbridge-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c
index 03dc9924fa2c..ff5c33d7a736 100644
--- a/drivers/media/pci/ddbridge/ddbridge-main.c
+++ b/drivers/media/pci/ddbridge/ddbridge-main.c
@@ -106,7 +106,7 @@ static void ddb_irq_msi(struct ddb *dev, int nr)
 
 	if (msi && pci_msi_enabled()) {
 		stat = pci_alloc_irq_vectors(dev->pdev, 1, nr,
-					     PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					     PCI_IRQ_MSI_TYPES);
 		if (stat >= 1) {
 			dev->msi = stat;
 			dev_info(dev->dev, "using %d MSI interrupt(s)\n",
-- 
2.17.2

[PATCH v2 10/15] vmw_vmci: Use PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/misc/vmw_vmci/vmci_guest.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
index cc8eeb361fcd..ea300573f652 100644
--- a/drivers/misc/vmw_vmci/vmci_guest.c
+++ b/drivers/misc/vmw_vmci/vmci_guest.c
@@ -584,8 +584,7 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
 	error = pci_alloc_irq_vectors(pdev, VMCI_MAX_INTRS, VMCI_MAX_INTRS,
 			PCI_IRQ_MSIX);
 	if (error < 0) {
-		error = pci_alloc_irq_vectors(pdev, 1, 1,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_LEGACY);
+		error = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
 		if (error < 0)
 			goto err_remove_bitmap;
 	} else {
-- 
2.17.2

[PATCH v2 11/15] mmc: sdhci: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pci-gli.c     | 3 +--
 drivers/mmc/host/sdhci-pci-o2micro.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index fd76aa672e02..d14997f9cbf9 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -271,8 +271,7 @@ static void gli_pcie_enable_msi(struct sdhci_pci_slot *slot)
 {
 	int ret;
 
-	ret = pci_alloc_irq_vectors(slot->chip->pdev, 1, 1,
-				    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+	ret = pci_alloc_irq_vectors(slot->chip->pdev, 1, 1, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		pr_warn("%s: enable PCI MSI failed, error=%d\n",
 		       mmc_hostname(slot->host->mmc), ret);
diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c
index fa8105087d68..498c51207bfb 100644
--- a/drivers/mmc/host/sdhci-pci-o2micro.c
+++ b/drivers/mmc/host/sdhci-pci-o2micro.c
@@ -470,8 +470,7 @@ static void sdhci_pci_o2_enable_msi(struct sdhci_pci_chip *chip,
 		return;
 	}
 
-	ret = pci_alloc_irq_vectors(chip->pdev, 1, 1,
-				    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+	ret = pci_alloc_irq_vectors(chip->pdev, 1, 1, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		pr_err("%s: enable PCI MSI failed, err=%d\n",
 		       mmc_hostname(host->mmc), ret);
-- 
2.17.2

[PATCH v2 12/15] amd-xgbe: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index 7b86240ecd5f..903bc5ef2518 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -133,7 +133,7 @@ static int xgbe_config_multi_msi(struct xgbe_prv_data *pdata)
 			    pdata->tx_ring_count);
 
 	ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSI_MIN_COUNT,
-				    vector_count, PCI_IRQ_MSI | PCI_IRQ_MSIX);
+				    vector_count, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		dev_info(pdata->dev, "multi MSI/MSI-X enablement failed\n");
 		return ret;
-- 
2.17.2

[PATCH v2 13/15] aquantia: atlantic: Use PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 8a70ffe1d326..2a0ebf296478 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -277,9 +277,7 @@ static int aq_pci_probe(struct pci_dev *pdev,
 	numvecs += AQ_HW_SERVICE_IRQS;
 	/*enable interrupts */
 #if !AQ_CFG_FORCE_LEGACY_INT
-	err = pci_alloc_irq_vectors(self->pdev, 1, numvecs,
-				    PCI_IRQ_MSIX | PCI_IRQ_MSI |
-				    PCI_IRQ_LEGACY);
+	err = pci_alloc_irq_vectors(self->pdev, 1, numvecs, PCI_IRQ_ALL_TYPES);
 
 	if (err < 0)
 		goto err_hwinit;
-- 
2.17.2

[PATCH v2 14/15] net: hns3: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c   | 3 +--
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index a758f9ae32be..c49313e87170 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2334,8 +2334,7 @@ static int hclge_init_msi(struct hclge_dev *hdev)
 	int i;
 
 	vectors = pci_alloc_irq_vectors(pdev, HNAE3_MIN_VECTOR_NUM,
-					hdev->num_msi,
-					PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					hdev->num_msi, PCI_IRQ_MSI_TYPES);
 	if (vectors < 0) {
 		dev_err(&pdev->dev,
 			"failed(%d) to allocate MSI/MSI-X vectors\n",
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index e02d427131ee..2b02024bd00a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2341,8 +2341,7 @@ static int hclgevf_init_msi(struct hclgevf_dev *hdev)
 						PCI_IRQ_MSIX);
 	else
 		vectors = pci_alloc_irq_vectors(pdev, HNAE3_MIN_VECTOR_NUM,
-						hdev->num_msi,
-						PCI_IRQ_MSI | PCI_IRQ_MSIX);
+						hdev->num_msi, PCI_IRQ_MSI_TYPES);
 
 	if (vectors < 0) {
 		dev_err(&pdev->dev,
-- 
2.17.2

[PATCH v2 15/15] scsi: Use PCI_IRQ_MSI_TYPES and PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/scsi/ipr.c        | 5 +++--
 drivers/scsi/vmw_pvscsi.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index d48a8fa997b9..8e8d66daabea 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -10273,9 +10273,10 @@ static int ipr_probe_ioa(struct pci_dev *pdev,
 		ipr_number_of_msix = IPR_MAX_MSIX_VECTORS;
 	}
 
-	irq_flag = PCI_IRQ_LEGACY;
 	if (ioa_cfg->ipr_chip->has_msi)
-		irq_flag |= PCI_IRQ_MSI | PCI_IRQ_MSIX;
+		irq_flag = PCI_IRQ_ALL_TYPES;
+	else
+		irq_flag = PCI_IRQ_LEGACY;
 	rc = pci_alloc_irq_vectors(pdev, 1, ipr_number_of_msix, irq_flag);
 	if (rc < 0) {
 		ipr_wait_for_pci_err_recovery(ioa_cfg);
diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index c3f010df641e..825b7db9c713 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -1347,7 +1347,7 @@ static u32 pvscsi_get_max_targets(struct pvscsi_adapter *adapter)
 
 static int pvscsi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
-	unsigned int irq_flag = PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_LEGACY;
+	unsigned int irq_flag = PCI_IRQ_ALL_TYPES;
 	struct pvscsi_adapter *adapter;
 	struct pvscsi_adapter adapter_temp;
 	struct Scsi_Host *host = NULL;
-- 
2.17.2

[PATCH v3 00/15] Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Piotr Stankiewicz]

The primary objective of this patch series is to change the behaviour
of pci_alloc_irq_vectors_affinity() such that it forwards the MSI-X enable
error code when appropriate. In the process, though, it was pointed out
that there are multiple places in the kernel which check/ask for message
signalled interrupts (MSI or MSI-X), which spawned the first patch adding
PCI_IRQ_MSI_TYPES. Finally the rest of the chain converts all users to
take advantage of PCI_IRQ_MSI_TYPES or PCI_IRQ_ALL_TYPES, as
appropriate.

Piotr Stankiewicz (15):
  PCI/MSI: Forward MSI-X vector enable error code in
    pci_alloc_irq_vectors_affinity()
  PCI: Add macro for message signalled interrupt types
  PCI: Use PCI_IRQ_MSI_TYPES where appropriate
  ahci: Use PCI_IRQ_MSI_TYPES where appropriate
  crypto: inside-secure - Use PCI_IRQ_MSI_TYPES where appropriate
  dmaengine: dw-edma: Use PCI_IRQ_MSI_TYPES  where appropriate
  drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate
  IB/qib: Use PCI_IRQ_MSI_TYPES where appropriate
  media: ddbridge: Use PCI_IRQ_MSI_TYPES where appropriate
  vmw_vmci: Use PCI_IRQ_ALL_TYPES where appropriate
  mmc: sdhci: Use PCI_IRQ_MSI_TYPES where appropriate
  amd-xgbe: Use PCI_IRQ_MSI_TYPES where appropriate
  aquantia: atlantic: Use PCI_IRQ_ALL_TYPES where appropriate
  net: hns3: Use PCI_IRQ_MSI_TYPES where appropriate
  scsi: Use PCI_IRQ_MSI_TYPES and PCI_IRQ_ALL_TYPES where appropriate

 Documentation/PCI/msi-howto.rst               |  5 +++--
 drivers/ata/ahci.c                            |  2 +-
 drivers/crypto/inside-secure/safexcel.c       |  2 +-
 drivers/dma/dw-edma/dw-edma-pcie.c            |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c       | 11 +---------
 drivers/infiniband/hw/qib/qib_pcie.c          |  6 +++--
 drivers/media/pci/ddbridge/ddbridge-main.c    |  2 +-
 drivers/misc/vmw_vmci/vmci_guest.c            |  3 +--
 drivers/mmc/host/sdhci-pci-gli.c              |  3 +--
 drivers/mmc/host/sdhci-pci-o2micro.c          |  3 +--
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c      |  2 +-
 .../ethernet/aquantia/atlantic/aq_pci_func.c  |  4 +---
 .../hisilicon/hns3/hns3pf/hclge_main.c        |  3 +--
 .../hisilicon/hns3/hns3vf/hclgevf_main.c      |  3 +--
 drivers/pci/msi.c                             | 22 ++++++++-----------
 drivers/pci/pcie/portdrv_core.c               |  4 ++--
 drivers/pci/switch/switchtec.c                |  3 +--
 drivers/scsi/ipr.c                            |  5 +++--
 drivers/scsi/vmw_pvscsi.c                     |  2 +-
 include/linux/pci.h                           |  4 ++--
 20 files changed, 37 insertions(+), 54 deletions(-)

-- 
2.17.2

[PATCH v3 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Piotr Stankiewicz]

When debugging an issue where I was asking the PCI machinery to enable a
set of MSI-X vectors, without falling back on MSI, I ran across a
behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
always return -ENOSPC on failure, when allocating MSI-X vectors only,
whereas with MSI fallback it will forward any error returned by
__pci_enable_msi_range(). This is a confusing behaviour, so have the
pci_alloc_irq_vectors_affinity() forward the error code from
__pci_enable_msix_range() when appropriate.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
---
 drivers/pci/msi.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 6b43a5455c7a..cade9be68b09 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1191,8 +1191,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 				   struct irq_affinity *affd)
 {
 	struct irq_affinity msi_default_affd = {0};
-	int msix_vecs = -ENOSPC;
-	int msi_vecs = -ENOSPC;
+	int nvecs = -ENOSPC;
 
 	if (flags & PCI_IRQ_AFFINITY) {
 		if (!affd)
@@ -1203,17 +1202,16 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 	}
 
 	if (flags & PCI_IRQ_MSIX) {
-		msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,
-						    max_vecs, affd, flags);
-		if (msix_vecs > 0)
-			return msix_vecs;
+		nvecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
+						affd, flags);
+		if (nvecs > 0)
+			return nvecs;
 	}
 
 	if (flags & PCI_IRQ_MSI) {
-		msi_vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs,
-						  affd);
-		if (msi_vecs > 0)
-			return msi_vecs;
+		nvecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
+		if (nvecs > 0)
+			return nvecs;
 	}
 
 	/* use legacy IRQ if allowed */
@@ -1231,9 +1229,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 		}
 	}
 
-	if (msix_vecs == -ENOSPC)
-		return -ENOSPC;
-	return msi_vecs;
+	return nvecs;
 }
 EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
 
-- 
2.17.2

Re: [PATCH v3 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Logan Gunthorpe]

On 2020-06-09 3:14 a.m., Piotr Stankiewicz wrote:
> When debugging an issue where I was asking the PCI machinery to enable a
> set of MSI-X vectors, without falling back on MSI, I ran across a
> behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
> always return -ENOSPC on failure, when allocating MSI-X vectors only,
> whereas with MSI fallback it will forward any error returned by
> __pci_enable_msi_range(). This is a confusing behaviour, so have the
> pci_alloc_irq_vectors_affinity() forward the error code from
> __pci_enable_msix_range() when appropriate.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>

Looks fine to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks!

> ---
>  drivers/pci/msi.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 6b43a5455c7a..cade9be68b09 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1191,8 +1191,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  				   struct irq_affinity *affd)
>  {
>  	struct irq_affinity msi_default_affd = {0};
> -	int msix_vecs = -ENOSPC;
> -	int msi_vecs = -ENOSPC;
> +	int nvecs = -ENOSPC;
>  
>  	if (flags & PCI_IRQ_AFFINITY) {
>  		if (!affd)
> @@ -1203,17 +1202,16 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  	}
>  
>  	if (flags & PCI_IRQ_MSIX) {
> -		msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,
> -						    max_vecs, affd, flags);
> -		if (msix_vecs > 0)
> -			return msix_vecs;
> +		nvecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
> +						affd, flags);
> +		if (nvecs > 0)
> +			return nvecs;
>  	}
>  
>  	if (flags & PCI_IRQ_MSI) {
> -		msi_vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs,
> -						  affd);
> -		if (msi_vecs > 0)
> -			return msi_vecs;
> +		nvecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
> +		if (nvecs > 0)
> +			return nvecs;
>  	}
>  
>  	/* use legacy IRQ if allowed */
> @@ -1231,9 +1229,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  		}
>  	}
>  
> -	if (msix_vecs == -ENOSPC)
> -		return -ENOSPC;
> -	return msi_vecs;
> +	return nvecs;
>  }
>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
>  
> 

Re: [PATCH v3 01/15] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v3 03/15] PCI: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/pcie/portdrv_core.c | 4 ++--
 drivers/pci/switch/switchtec.c  | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 50a9522ab07d..2a38a918ba12 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -105,7 +105,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 
 	/* Allocate the maximum possible number of MSI/MSI-X vectors */
 	nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES,
-			PCI_IRQ_MSIX | PCI_IRQ_MSI);
+			PCI_IRQ_MSI_TYPES);
 	if (nr_entries < 0)
 		return nr_entries;
 
@@ -131,7 +131,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 		pci_free_irq_vectors(dev);
 
 		nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI);
+				PCI_IRQ_MSI_TYPES);
 		if (nr_entries < 0)
 			return nr_entries;
 	}
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 850cfeb74608..0ab17a71fe63 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1442,8 +1442,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 		nirqs = 4;
 
 	nvecs = pci_alloc_irq_vectors(stdev->pdev, 1, nirqs,
-				      PCI_IRQ_MSIX | PCI_IRQ_MSI |
-				      PCI_IRQ_VIRTUAL);
+				      PCI_IRQ_MSI_TYPES | PCI_IRQ_VIRTUAL);
 	if (nvecs < 0)
 		return nvecs;
 
-- 
2.17.2

Re: [PATCH v3 03/15] PCI: Use PCI_IRQ_MSI_TYPES where appropriate

[Christoph Hellwig]

On Tue, Jun 09, 2020 at 11:16:46AM +0200, Piotr Stankiewicz wrote:
> Seeing as there is shorthand available to use when asking for any type
> of interrupt, or any type of message signalled interrupt, leverage it.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

The patch actually adding PCI_IRQ_MSI_TYPES still didn't show up on
linux-pci.

But from the person who added PCI_IRQ_*:

NAK to the whole concept of PCI_IRQ_MSI_TYPES, I think this just makes
the code a lot more confusing.  Especially with the new IMS interrupt
type, which really is a MSI-like interrupt but certainly shouldn't be
added to this mask.

RE: [PATCH v3 03/15] PCI: Use PCI_IRQ_MSI_TYPES where appropriate

[Stankiewicz, Piotr]

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Tuesday, June 9, 2020 5:51 PM
> 
> On Tue, Jun 09, 2020 at 11:16:46AM +0200, Piotr Stankiewicz wrote:
> > Seeing as there is shorthand available to use when asking for any type
> > of interrupt, or any type of message signalled interrupt, leverage it.
> >
> > Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> 
> The patch actually adding PCI_IRQ_MSI_TYPES still didn't show up on linux-pci.
> 

I'm not sure why it didn't get delivered. I just resent it, for sake of completeness.

> But from the person who added PCI_IRQ_*:
> 
> NAK to the whole concept of PCI_IRQ_MSI_TYPES, I think this just makes the
> code a lot more confusing.  Especially with the new IMS interrupt type, which
> really is a MSI-like interrupt but certainly shouldn't be added to this mask.

Re: [PATCH v3 03/15] PCI: Use PCI_IRQ_MSI_TYPES where appropriate

[Logan Gunthorpe]

On 2020-06-09 9:51 a.m., Christoph Hellwig wrote:
> On Tue, Jun 09, 2020 at 11:16:46AM +0200, Piotr Stankiewicz wrote:
>> Seeing as there is shorthand available to use when asking for any type
>> of interrupt, or any type of message signalled interrupt, leverage it.
>>
>> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> 
> The patch actually adding PCI_IRQ_MSI_TYPES still didn't show up on
> linux-pci.
> 
> But from the person who added PCI_IRQ_*:
> 
> NAK to the whole concept of PCI_IRQ_MSI_TYPES, I think this just makes
> the code a lot more confusing.  Especially with the new IMS interrupt
> type, which really is a MSI-like interrupt but certainly shouldn't be
> added to this mask.

Yes, after cleaning up the first patch to not require the long messy
conditional, this change doesn't seems like a win. I agree we should
drop these patches.

Logan

[PATCH v3 04/15] ahci: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/ata/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 0c0a736eb861..ca1bf4ef0f17 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1556,7 +1556,7 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports,
 	 */
 	if (n_ports > 1) {
 		nvec = pci_alloc_irq_vectors(pdev, n_ports, INT_MAX,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI);
+				PCI_IRQ_MSI_TYPES);
 		if (nvec > 0) {
 			if (!(readl(hpriv->mmio + HOST_CTL) & HOST_MRSM)) {
 				hpriv->get_irq_vector = ahci_get_irq_vector;
-- 
2.17.2

[PATCH v3 05/15] crypto: inside-secure - Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Antoine Tenart <antoine.tenart@bootlin.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 2cb53fbae841..1b2faa2a6ab0 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -1567,7 +1567,7 @@ static int safexcel_probe_generic(void *pdev,
 		ret = pci_alloc_irq_vectors(pci_pdev,
 					    priv->config.rings + 1,
 					    priv->config.rings + 1,
-					    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					    PCI_IRQ_MSI_TYPES);
 		if (ret < 0) {
 			dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
 			return ret;
-- 
2.17.2

[PATCH v3 06/15] dmaengine: dw-edma: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index dc85f55e1bb8..46defe30ac25 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -117,7 +117,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 
 	/* IRQs allocation */
 	nr_irqs = pci_alloc_irq_vectors(pdev, 1, pdata->irqs,
-					PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					PCI_IRQ_MSI_TYPES);
 	if (nr_irqs < 1) {
 		pci_err(pdev, "fail to alloc IRQ vector (number of IRQs=%u)\n",
 			nr_irqs);
-- 
2.17.2

Re: [PATCH v3 06/15] dmaengine: dw-edma: Use PCI_IRQ_MSI_TYPES where appropriate

[Andy Shevchenko]

On Tue, Jun 09, 2020 at 11:17:47AM +0200, Piotr Stankiewicz wrote:
> Seeing as there is shorthand available to use when asking for any type
> of interrupt, or any type of message signalled interrupt, leverage it.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

I think I saw somebody gave you tag here...
Am I mistaken?

...

>  	nr_irqs = pci_alloc_irq_vectors(pdev, 1, pdata->irqs,
> -					PCI_IRQ_MSI | PCI_IRQ_MSIX);
> +					PCI_IRQ_MSI_TYPES);

Now one line?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 0cc4c67f95f7..97141aa81f32 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -248,17 +248,8 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
 	adev->irq.msi_enabled = false;
 
 	if (amdgpu_msi_ok(adev)) {
-		int nvec = pci_msix_vec_count(adev->pdev);
-		unsigned int flags;
-
-		if (nvec <= 0) {
-			flags = PCI_IRQ_MSI;
-		} else {
-			flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
-		}
 		/* we only need one vector */
-		nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
-		if (nvec > 0) {
+		if (pci_alloc_irq_vectors(adev->pdev, 1, 1, PCI_IRQ_MSI_TYPES) > 0) {
 			adev->irq.msi_enabled = true;
 			dev_dbg(adev->dev, "using MSI/MSI-X.\n");
 		}
-- 
2.17.2

Re: [PATCH v3 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

[Alex Deucher]

On Tue, Jun 9, 2020 at 5:18 AM Piotr Stankiewicz
<piotr.stankiewicz@intel.com> wrote:
>
> Seeing as there is shorthand available to use when asking for any type
> of interrupt, or any type of message signalled interrupt, leverage it.
>
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 0cc4c67f95f7..97141aa81f32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -248,17 +248,8 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>         adev->irq.msi_enabled = false;
>
>         if (amdgpu_msi_ok(adev)) {
> -               int nvec = pci_msix_vec_count(adev->pdev);
> -               unsigned int flags;
> -
> -               if (nvec <= 0) {
> -                       flags = PCI_IRQ_MSI;
> -               } else {
> -                       flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
> -               }
>                 /* we only need one vector */
> -               nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
> -               if (nvec > 0) {
> +               if (pci_alloc_irq_vectors(adev->pdev, 1, 1, PCI_IRQ_MSI_TYPES) > 0) {
>                         adev->irq.msi_enabled = true;
>                         dev_dbg(adev->dev, "using MSI/MSI-X.\n");
>                 }
> --
> 2.17.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH v3 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

[Stankiewicz, Piotr]

> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Tuesday, June 9, 2020 10:24 PM
> 
> On Tue, Jun 9, 2020 at 5:18 AM Piotr Stankiewicz
> <piotr.stankiewicz@intel.com> wrote:
> >
> > Seeing as there is shorthand available to use when asking for any type
> > of interrupt, or any type of message signalled interrupt, leverage it.
> >
> > Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 

Thanks. Adding PCI_IRQ_MSI_TYPES won't land upstream. But I'll send
a patch with the other simplifications, just now.

[PATCH] drm/amdgpu: Simplify IRQ vector request logic

[Piotr Stankiewicz]

pci_alloc_irq_vectors() will handle fallback from MSI-X to MSI
internally, if necessary. So remove checks which determine if we are
dealing with MSI or MSI-X and rely on pci_alloc_irq_vectors() to do the
right thing.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 0cc4c67f95f7..2d68ad7d45d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -248,17 +248,8 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
 	adev->irq.msi_enabled = false;
 
 	if (amdgpu_msi_ok(adev)) {
-		int nvec = pci_msix_vec_count(adev->pdev);
-		unsigned int flags;
-
-		if (nvec <= 0) {
-			flags = PCI_IRQ_MSI;
-		} else {
-			flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
-		}
 		/* we only need one vector */
-		nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
-		if (nvec > 0) {
+		if (pci_alloc_irq_vectors(adev->pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_MSIX) > 0) {
 			adev->irq.msi_enabled = true;
 			dev_dbg(adev->dev, "using MSI/MSI-X.\n");
 		}
-- 
2.17.2

[PATCH v3 08/15] IB/qib: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/infiniband/hw/qib/qib_pcie.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 3dc6ce033319..caff44d2c12c 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -213,7 +213,7 @@ int qib_pcie_params(struct qib_devdata *dd, u32 minw, u32 *nent)
 	u16 linkstat, speed;
 	int nvec;
 	int maxvec;
-	unsigned int flags = PCI_IRQ_MSIX | PCI_IRQ_MSI;
+	unsigned int flags;
 
 	if (!pci_is_pcie(dd->pcidev)) {
 		qib_dev_err(dd, "Can't find PCI Express capability!\n");
@@ -225,7 +225,9 @@ int qib_pcie_params(struct qib_devdata *dd, u32 minw, u32 *nent)
 	}
 
 	if (dd->flags & QIB_HAS_INTX)
-		flags |= PCI_IRQ_LEGACY;
+		flags = PCI_IRQ_ALL_TYPES;
+	else
+		flags = PCI_IRQ_MSI_TYPES;
 	maxvec = (nent && *nent) ? *nent : 1;
 	nvec = pci_alloc_irq_vectors(dd->pcidev, 1, maxvec, flags);
 	if (nvec < 0)
-- 
2.17.2

RE: [PATCH v3 08/15] IB/qib: Use PCI_IRQ_MSI_TYPES where appropriate

[Ruhl, Michael J]

>-----Original Message-----
>From: linux-rdma-owner@vger.kernel.org <linux-rdma-
>owner@vger.kernel.org> On Behalf Of Piotr Stankiewicz
>Sent: Tuesday, June 9, 2020 5:18 AM
>To: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org
>Cc: Stankiewicz, Piotr <piotr.stankiewicz@intel.com>; Dalessandro, Dennis
><dennis.dalessandro@intel.com>; Marciniszyn, Mike
><mike.marciniszyn@intel.com>; Doug Ledford <dledford@redhat.com>;
>Jason Gunthorpe <jgg@ziepe.ca>; Arnd Bergmann <arnd@arndb.de>;
>Shevchenko, Andriy <andriy.shevchenko@intel.com>; linux-
>rdma@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: [PATCH v3 08/15] IB/qib: Use PCI_IRQ_MSI_TYPES where appropriate
>
>Seeing as there is shorthand available to use when asking for any type
>of interrupt, or any type of message signalled interrupt, leverage it.
>
>Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
>Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>---
> drivers/infiniband/hw/qib/qib_pcie.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/infiniband/hw/qib/qib_pcie.c
>b/drivers/infiniband/hw/qib/qib_pcie.c
>index 3dc6ce033319..caff44d2c12c 100644
>--- a/drivers/infiniband/hw/qib/qib_pcie.c
>+++ b/drivers/infiniband/hw/qib/qib_pcie.c
>@@ -213,7 +213,7 @@ int qib_pcie_params(struct qib_devdata *dd, u32
>minw, u32 *nent)
> 	u16 linkstat, speed;
> 	int nvec;
> 	int maxvec;
>-	unsigned int flags = PCI_IRQ_MSIX | PCI_IRQ_MSI;
>+	unsigned int flags;
>
> 	if (!pci_is_pcie(dd->pcidev)) {
> 		qib_dev_err(dd, "Can't find PCI Express capability!\n");
>@@ -225,7 +225,9 @@ int qib_pcie_params(struct qib_devdata *dd, u32
>minw, u32 *nent)
> 	}
>
> 	if (dd->flags & QIB_HAS_INTX)
>-		flags |= PCI_IRQ_LEGACY;
>+		flags = PCI_IRQ_ALL_TYPES;
>+	else
>+		flags = PCI_IRQ_MSI_TYPES;

Thinking about lines of code, this patch could probably just be:

-	unsigned int flags = PCI_IRQ_MSIX | PCI_IRQ_MSI;
+	unsigned int flags = PCI_IRQ_MSI_TYPES;

Or maybe even:

-	unsigned int flags = PCI_IRQ_MSIX | PCI_IRQ_MSI;
+	unsigned int flags = PCI_IRQ_ALL_TYPES;

- 	if (dd->flags & QIB_HAS_INTX)
-		flags |= PCI_IRQ_LEGACY;

?

M

> 	maxvec = (nent && *nent) ? *nent : 1;
> 	nvec = pci_alloc_irq_vectors(dd->pcidev, 1, maxvec, flags);
> 	if (nvec < 0)
>--
>2.17.2

[PATCH v3 09/15] media: ddbridge: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/media/pci/ddbridge/ddbridge-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-main.c b/drivers/media/pci/ddbridge/ddbridge-main.c
index 03dc9924fa2c..ff5c33d7a736 100644
--- a/drivers/media/pci/ddbridge/ddbridge-main.c
+++ b/drivers/media/pci/ddbridge/ddbridge-main.c
@@ -106,7 +106,7 @@ static void ddb_irq_msi(struct ddb *dev, int nr)
 
 	if (msi && pci_msi_enabled()) {
 		stat = pci_alloc_irq_vectors(dev->pdev, 1, nr,
-					     PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					     PCI_IRQ_MSI_TYPES);
 		if (stat >= 1) {
 			dev->msi = stat;
 			dev_info(dev->dev, "using %d MSI interrupt(s)\n",
-- 
2.17.2

[PATCH v3 10/15] vmw_vmci: Use PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/misc/vmw_vmci/vmci_guest.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
index cc8eeb361fcd..ea300573f652 100644
--- a/drivers/misc/vmw_vmci/vmci_guest.c
+++ b/drivers/misc/vmw_vmci/vmci_guest.c
@@ -584,8 +584,7 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
 	error = pci_alloc_irq_vectors(pdev, VMCI_MAX_INTRS, VMCI_MAX_INTRS,
 			PCI_IRQ_MSIX);
 	if (error < 0) {
-		error = pci_alloc_irq_vectors(pdev, 1, 1,
-				PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_LEGACY);
+		error = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
 		if (error < 0)
 			goto err_remove_bitmap;
 	} else {
-- 
2.17.2

[PATCH v3 11/15] mmc: sdhci: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pci-gli.c     | 3 +--
 drivers/mmc/host/sdhci-pci-o2micro.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index ca0166d9bf82..aa7eef201ada 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -284,8 +284,7 @@ static void gli_pcie_enable_msi(struct sdhci_pci_slot *slot)
 {
 	int ret;
 
-	ret = pci_alloc_irq_vectors(slot->chip->pdev, 1, 1,
-				    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+	ret = pci_alloc_irq_vectors(slot->chip->pdev, 1, 1, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		pr_warn("%s: enable PCI MSI failed, error=%d\n",
 		       mmc_hostname(slot->host->mmc), ret);
diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c
index e2a846885902..c81a5c328a81 100644
--- a/drivers/mmc/host/sdhci-pci-o2micro.c
+++ b/drivers/mmc/host/sdhci-pci-o2micro.c
@@ -470,8 +470,7 @@ static void sdhci_pci_o2_enable_msi(struct sdhci_pci_chip *chip,
 		return;
 	}
 
-	ret = pci_alloc_irq_vectors(chip->pdev, 1, 1,
-				    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+	ret = pci_alloc_irq_vectors(chip->pdev, 1, 1, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		pr_err("%s: enable PCI MSI failed, err=%d\n",
 		       mmc_hostname(host->mmc), ret);
-- 
2.17.2

[PATCH v3 12/15] amd-xgbe: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index 7b86240ecd5f..903bc5ef2518 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -133,7 +133,7 @@ static int xgbe_config_multi_msi(struct xgbe_prv_data *pdata)
 			    pdata->tx_ring_count);
 
 	ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSI_MIN_COUNT,
-				    vector_count, PCI_IRQ_MSI | PCI_IRQ_MSIX);
+				    vector_count, PCI_IRQ_MSI_TYPES);
 	if (ret < 0) {
 		dev_info(pdata->dev, "multi MSI/MSI-X enablement failed\n");
 		return ret;
-- 
2.17.2

[PATCH v3 13/15] aquantia: atlantic: Use PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 41c0f560f95b..bc228be9f0a3 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -299,9 +299,7 @@ static int aq_pci_probe(struct pci_dev *pdev,
 	numvecs += AQ_HW_SERVICE_IRQS;
 	/*enable interrupts */
 #if !AQ_CFG_FORCE_LEGACY_INT
-	err = pci_alloc_irq_vectors(self->pdev, 1, numvecs,
-				    PCI_IRQ_MSIX | PCI_IRQ_MSI |
-				    PCI_IRQ_LEGACY);
+	err = pci_alloc_irq_vectors(self->pdev, 1, numvecs, PCI_IRQ_ALL_TYPES);
 
 	if (err < 0)
 		goto err_hwinit;
-- 
2.17.2

[PATCH v3 14/15] net: hns3: Use PCI_IRQ_MSI_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c   | 3 +--
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 96bfad52630d..535d9f4a31f5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2327,8 +2327,7 @@ static int hclge_init_msi(struct hclge_dev *hdev)
 	int i;
 
 	vectors = pci_alloc_irq_vectors(pdev, HNAE3_MIN_VECTOR_NUM,
-					hdev->num_msi,
-					PCI_IRQ_MSI | PCI_IRQ_MSIX);
+					hdev->num_msi, PCI_IRQ_MSI_TYPES);
 	if (vectors < 0) {
 		dev_err(&pdev->dev,
 			"failed(%d) to allocate MSI/MSI-X vectors\n",
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 1b9578d0bd80..fbfd4e7b2817 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2608,8 +2608,7 @@ static int hclgevf_init_msi(struct hclgevf_dev *hdev)
 						PCI_IRQ_MSIX);
 	else
 		vectors = pci_alloc_irq_vectors(pdev, HNAE3_MIN_VECTOR_NUM,
-						hdev->num_msi,
-						PCI_IRQ_MSI | PCI_IRQ_MSIX);
+						hdev->num_msi, PCI_IRQ_MSI_TYPES);
 
 	if (vectors < 0) {
 		dev_err(&pdev->dev,
-- 
2.17.2

[PATCH v3 15/15] scsi: Use PCI_IRQ_MSI_TYPES and PCI_IRQ_ALL_TYPES where appropriate

[Piotr Stankiewicz]

Seeing as there is shorthand available to use when asking for any type
of interrupt, or any type of message signalled interrupt, leverage it.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/scsi/ipr.c        | 5 +++--
 drivers/scsi/vmw_pvscsi.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 7d77997d26d4..b320fc765a57 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -10272,9 +10272,10 @@ static int ipr_probe_ioa(struct pci_dev *pdev,
 		ipr_number_of_msix = IPR_MAX_MSIX_VECTORS;
 	}
 
-	irq_flag = PCI_IRQ_LEGACY;
 	if (ioa_cfg->ipr_chip->has_msi)
-		irq_flag |= PCI_IRQ_MSI | PCI_IRQ_MSIX;
+		irq_flag = PCI_IRQ_ALL_TYPES;
+	else
+		irq_flag = PCI_IRQ_LEGACY;
 	rc = pci_alloc_irq_vectors(pdev, 1, ipr_number_of_msix, irq_flag);
 	if (rc < 0) {
 		ipr_wait_for_pci_err_recovery(ioa_cfg);
diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index 8dbb4db6831a..4aa7166d13fb 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -1347,7 +1347,7 @@ static u32 pvscsi_get_max_targets(struct pvscsi_adapter *adapter)
 
 static int pvscsi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
-	unsigned int irq_flag = PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_LEGACY;
+	unsigned int irq_flag = PCI_IRQ_ALL_TYPES;
 	struct pvscsi_adapter *adapter;
 	struct pvscsi_adapter adapter_temp;
 	struct Scsi_Host *host = NULL;
-- 
2.17.2

[PATCH v3 02/15] PCI: Add macro for message signalled interrupt types

[Piotr Stankiewicz]

There are several places in the kernel which check/ask for MSI or MSI-X
interrupts. It would make sense to have a macro which defines all types
of message signalled interrupts, to use in such situations. Add
PCI_IRQ_MSI_TYPES, for this purpose.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 Documentation/PCI/msi-howto.rst | 5 +++--
 include/linux/pci.h             | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/PCI/msi-howto.rst b/Documentation/PCI/msi-howto.rst
index aa2046af69f7..2800ff5aa395 100644
--- a/Documentation/PCI/msi-howto.rst
+++ b/Documentation/PCI/msi-howto.rst
@@ -105,7 +105,8 @@ if it can't meet the minimum number of vectors.
 The flags argument is used to specify which type of interrupt can be used
 by the device and the driver (PCI_IRQ_LEGACY, PCI_IRQ_MSI, PCI_IRQ_MSIX).
 A convenient short-hand (PCI_IRQ_ALL_TYPES) is also available to ask for
-any possible kind of interrupt.  If the PCI_IRQ_AFFINITY flag is set,
+any possible kind of interrupt, and (PCI_IRQ_MSI_TYPES) to ask for message
+signalled interrupts (MSI or MSI-X).  If the PCI_IRQ_AFFINITY flag is set,
 pci_alloc_irq_vectors() will spread the interrupts around the available CPUs.
 
 To get the Linux IRQ numbers passed to request_irq() and free_irq() and the
@@ -160,7 +161,7 @@ the single MSI mode for a device.  It could be done by passing two 1s as
 Some devices might not support using legacy line interrupts, in which case
 the driver can specify that only MSI or MSI-X is acceptable::
 
-	nvec = pci_alloc_irq_vectors(pdev, 1, nvec, PCI_IRQ_MSI | PCI_IRQ_MSIX);
+	nvec = pci_alloc_irq_vectors(pdev, 1, nvec, PCI_IRQ_MSI_TYPES);
 	if (nvec < 0)
 		goto out_err;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c79d83304e52..a99094f17b21 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1431,8 +1431,8 @@ int pci_set_vga_state(struct pci_dev *pdev, bool decode,
  */
 #define PCI_IRQ_VIRTUAL		(1 << 4)
 
-#define PCI_IRQ_ALL_TYPES \
-	(PCI_IRQ_LEGACY | PCI_IRQ_MSI | PCI_IRQ_MSIX)
+#define PCI_IRQ_MSI_TYPES	(PCI_IRQ_MSI | PCI_IRQ_MSIX)
+#define PCI_IRQ_ALL_TYPES	(PCI_IRQ_LEGACY | PCI_IRQ_MSI_TYPES)
 
 /* kmem_cache style wrapper around pci_alloc_consistent() */
 
-- 
2.17.2

Re: [PATCH v3 02/15] PCI: Add macro for message signalled interrupt types

[Christoph Hellwig]

On Tue, Jun 09, 2020 at 06:22:40PM +0200, Piotr Stankiewicz wrote:
> There are several places in the kernel which check/ask for MSI or MSI-X
> interrupts. It would make sense to have a macro which defines all types
> of message signalled interrupts, to use in such situations. Add
> PCI_IRQ_MSI_TYPES, for this purpose.

To state my objection voices in patch 3 here again:

I think this is a very bad idea.  Everyone knows what MSI and MSI-X
mean and that directly maps to specification.  The new IMS interrupt
scheme from the Intel SIOV spec for example is a message signalled
interrupt type as well and should not be picked up automatically.

If we want to change anything in this area we should probably remove
PCI_IRQ_ALL_TYPES instead..

RE: [PATCH v3 02/15] PCI: Add macro for message signalled interrupt types

[Stankiewicz, Piotr]

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Wednesday, June 10, 2020 9:08 AM
> 
> On Tue, Jun 09, 2020 at 06:22:40PM +0200, Piotr Stankiewicz wrote:
> > There are several places in the kernel which check/ask for MSI or
> > MSI-X interrupts. It would make sense to have a macro which defines
> > all types of message signalled interrupts, to use in such situations.
> > Add PCI_IRQ_MSI_TYPES, for this purpose.
> 
> To state my objection voices in patch 3 here again:
> 
> I think this is a very bad idea.  Everyone knows what MSI and MSI-X mean and
> that directly maps to specification.  The new IMS interrupt scheme from the Intel
> SIOV spec for example is a message signalled interrupt type as well and should
> not be picked up automatically.
> 
> If we want to change anything in this area we should probably remove
> PCI_IRQ_ALL_TYPES instead..

Thanks, I've resent only the first patch as v4 of the chain, and dropped the rest.

[PATCH v4] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Piotr Stankiewicz]

When debugging an issue where I was asking the PCI machinery to enable a
set of MSI-X vectors, without falling back on MSI, I ran across a
behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
always return -ENOSPC on failure, when allocating MSI-X vectors only,
whereas with MSI fallback it will forward any error returned by
__pci_enable_msi_range(). This is a confusing behaviour, so have the
pci_alloc_irq_vectors_affinity() forward the error code from
__pci_enable_msix_range() when appropriate.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/msi.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 6b43a5455c7a..cade9be68b09 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1191,8 +1191,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 				   struct irq_affinity *affd)
 {
 	struct irq_affinity msi_default_affd = {0};
-	int msix_vecs = -ENOSPC;
-	int msi_vecs = -ENOSPC;
+	int nvecs = -ENOSPC;
 
 	if (flags & PCI_IRQ_AFFINITY) {
 		if (!affd)
@@ -1203,17 +1202,16 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 	}
 
 	if (flags & PCI_IRQ_MSIX) {
-		msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,
-						    max_vecs, affd, flags);
-		if (msix_vecs > 0)
-			return msix_vecs;
+		nvecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
+						affd, flags);
+		if (nvecs > 0)
+			return nvecs;
 	}
 
 	if (flags & PCI_IRQ_MSI) {
-		msi_vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs,
-						  affd);
-		if (msi_vecs > 0)
-			return msi_vecs;
+		nvecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
+		if (nvecs > 0)
+			return nvecs;
 	}
 
 	/* use legacy IRQ if allowed */
@@ -1231,9 +1229,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 		}
 	}
 
-	if (msix_vecs == -ENOSPC)
-		return -ENOSPC;
-	return msi_vecs;
+	return nvecs;
 }
 EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
 
-- 
2.17.2

Re: [PATCH v4] PCI/MSI: Forward MSI-X vector enable error code in pci_alloc_irq_vectors_affinity()

[Bjorn Helgaas]

On Tue, Jun 16, 2020 at 09:33:16AM +0200, Piotr Stankiewicz wrote:
> When debugging an issue where I was asking the PCI machinery to enable a
> set of MSI-X vectors, without falling back on MSI, I ran across a
> behaviour which seems odd. The pci_alloc_irq_vectors_affinity() will
> always return -ENOSPC on failure, when allocating MSI-X vectors only,
> whereas with MSI fallback it will forward any error returned by
> __pci_enable_msi_range(). This is a confusing behaviour, so have the
> pci_alloc_irq_vectors_affinity() forward the error code from
> __pci_enable_msix_range() when appropriate.
> 
> Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Applied to pci/msi for v5.8, thanks!

> ---
>  drivers/pci/msi.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 6b43a5455c7a..cade9be68b09 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1191,8 +1191,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  				   struct irq_affinity *affd)
>  {
>  	struct irq_affinity msi_default_affd = {0};
> -	int msix_vecs = -ENOSPC;
> -	int msi_vecs = -ENOSPC;
> +	int nvecs = -ENOSPC;
>  
>  	if (flags & PCI_IRQ_AFFINITY) {
>  		if (!affd)
> @@ -1203,17 +1202,16 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  	}
>  
>  	if (flags & PCI_IRQ_MSIX) {
> -		msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,
> -						    max_vecs, affd, flags);
> -		if (msix_vecs > 0)
> -			return msix_vecs;
> +		nvecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
> +						affd, flags);
> +		if (nvecs > 0)
> +			return nvecs;
>  	}
>  
>  	if (flags & PCI_IRQ_MSI) {
> -		msi_vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs,
> -						  affd);
> -		if (msi_vecs > 0)
> -			return msi_vecs;
> +		nvecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
> +		if (nvecs > 0)
> +			return nvecs;
>  	}
>  
>  	/* use legacy IRQ if allowed */
> @@ -1231,9 +1229,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>  		}
>  	}
>  
> -	if (msix_vecs == -ENOSPC)
> -		return -ENOSPC;
> -	return msi_vecs;
> +	return nvecs;
>  }
>  EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
>  
> -- 
> 2.17.2
>