2013-10-19 07:04:59

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 00/29] Introduce pcim_enable_msi*() family helpers

This series is against "next" branch in Bjorn's repo:
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git

Currently many device drivers need contiguously call functions
pci_enable_msix() for MSI-X or pci_enable_msi_block() for MSI
in a loop until success or failure. This update generalizes
this usage pattern and introduces pcim_enable_msi*() family
helpers.

As result, device drivers do not have to deal with tri-state
return values from pci_enable_msix() and pci_enable_msi_block()
functions directly and expected to have more clearer and straight
code.

So i.e. the request loop described in the documentation...

int foo_driver_enable_msix(struct foo_adapter *adapter,
int nvec)
{
while (nvec >= FOO_DRIVER_MINIMUM_NVEC) {
rc = pci_enable_msix(adapter->pdev,
adapter->msix_entries,
nvec);
if (rc > 0)
nvec = rc;
else
return rc;
}

return -ENOSPC;
}

...would turn into a single helper call....

rc = pcim_enable_msix_range(adapter->pdev,
adapter->msix_entries,
nvec,
FOO_DRIVER_MINIMUM_NVEC);


Device drivers with more specific requirements (i.e. a number of
MSI-Xs which is a multiple of a certain number within a specified
range) would still need to implement the loop using the two old
functions.

It is fair to say the proposal in v1 ("Re-design MSI/MSI-X interrupts
enablement pattern")is scrapped. Thus, I do not clarify what are the
changes since v1 - the whole v2 series could be
considered as completely new.

Device driver updates (patches 12-29) are provided to clarify the
change and expected to get reviews in follow-up post(s) when/if
patch 12 ("PCI/MSI: Introduce pcim_enable_msi*() family helpers")
is accepted.

Patches 1,2 - ACK'ed tweaks for s390 architecture
Patches 3,4 - fixes for PowerPC pSeries platform
Patches 5-12 - fixes, tweaks and changes of the generic MSI code
Patches 12-29 - example updates of few device drivers

The tree could be found in "pci-next-msi-v2" branch in repo:
https://github.com/a-gordeev/linux.git

Alexander Gordeev (29):
PCI/MSI/s390: Fix single MSI only check
PCI/MSI/s390: Remove superfluous check of MSI type
PCI/MSI/pSeries: Fix wrong error code reporting
PCI/MSI/pSeries: Make quota traversing and requesting race-safe
PCI/MSI: Fix return value when populate_msi_sysfs() failed
PCI/MSI: Get rid of useless count of msi_desc leftovers
PCI/MSI: Return -ENOSYS for unimplemented interfaces, not -1
PCI/MSI: Make pci_enable_msix() 'nvec' argument unsigned int
PCI/MSI: Factor out pci_get_msi_cap() interface
PCI/MSI: Get rid of pci_enable_msi_block_auto() interface
PCI/MSI: Convert pci_msix_table_size() to a public interface
PCI/MSI: Introduce pcim_enable_msi*() family helpers
ipr: Do not call pci_disable_msi/msix() if pci_enable_msi/msix()
failed
ipr: Make use of pcim_enable_msi/msix() interfaces
ixgbe: Make use of pcim_enable_msix_range() interface
ixgbevf: Make use of pcim_enable_msix_range() interface
megaraid: Make use of pcim_enable_msix() interface
ntb: Fix missed call to pci_enable_msix()
ntb: Make use of pcim_enable_msix/msix_exact() interfaces
qib: Make use of pcim_enable_msix() and pci_msix_table_size()
interfaces
qla2xxx: Make use of pcim_enable_msix_range() interface
qlge: Get rid of an redundant assignment
qlge: Make use of pcim_enable_msix() interface
tg3: Make use of pcim_enable_msix() interface
vmxnet3: Return -EINVAL if number of requested MSI-Xs is not enough
vmxnet3: Fixup a weird loop exit
vmxnet3: Return -ENOSPC when not enough MSI-X vectors available
vmxnet3: Limit number of rx queues to 1 if per-queue MSI-Xs failed
vmxnet3: Make use of pcim_enable_msix_range() interface

Documentation/PCI/MSI-HOWTO.txt | 169 ++++++++++++++++++---
arch/powerpc/platforms/pseries/msi.c | 26 +++-
arch/s390/pci/pci.c | 4 +-
drivers/ata/ahci.c | 56 +++++---
drivers/infiniband/hw/qib/qib_pcie.c | 48 +++---
drivers/net/ethernet/broadcom/tg3.c | 6 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 16 +--
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 33 ++---
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 16 +--
drivers/net/vmxnet3/vmxnet3_drv.c | 58 +++-----
drivers/ntb/ntb_hw.c | 34 ++---
drivers/pci/msi.c | 129 +++++++++-------
drivers/pci/pcie/portdrv_core.c | 5 +-
drivers/scsi/ipr.c | 51 +++----
drivers/scsi/megaraid/megaraid_sas_base.c | 16 +--
drivers/scsi/qla2xxx/qla_isr.c | 28 ++--
include/linux/pci.h | 79 +++++++++--
17 files changed, 466 insertions(+), 308 deletions(-)

--
1.7.7.6


2013-10-19 06:03:33

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 26/29] vmxnet3: Fixup a weird loop exit

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 5b8ea71..3518173 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2750,7 +2750,7 @@ vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
} else if (err < 0) {
dev_err(&adapter->netdev->dev,
"Failed to enable MSI-X, error: %d\n", err);
- vectors = 0;
+ return err;
} else if (err < vector_threshold) {
break;
} else {
--
1.7.7.6

2013-10-19 06:05:08

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 03/29] PCI/MSI/pSeries: Fix wrong error code reporting

Signed-off-by: Alexander Gordeev <[email protected]>
---
arch/powerpc/platforms/pseries/msi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 6d2f0ab..009ec73 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -467,7 +467,7 @@ again:
list_for_each_entry(entry, &pdev->msi_list, list) {
hwirq = rtas_query_irq_number(pdn, i++);
if (hwirq < 0) {
- pr_debug("rtas_msi: error (%d) getting hwirq\n", rc);
+ pr_debug("rtas_msi: error (%d) getting hwirq\n", hwirq);
return hwirq;
}

--
1.7.7.6

2013-10-19 06:06:54

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 25/29] vmxnet3: Return -EINVAL if number of requested MSI-Xs is not enough

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 7e2788c..5b8ea71 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2738,7 +2738,7 @@ static int
vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
int vectors)
{
- int err = 0, vector_threshold;
+ int err = -EINVAL, vector_threshold;
vector_threshold = VMXNET3_LINUX_MIN_MSIX_VECT;

while (vectors >= vector_threshold) {
--
1.7.7.6

2013-10-19 06:08:40

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 22/29] qlge: Get rid of an redundant assignment

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 2553cf4..ac54cb0 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -3287,7 +3287,6 @@ static void ql_enable_msix(struct ql_adapter *qdev)
qdev->msi_x_entry = NULL;
netif_warn(qdev, ifup, qdev->ndev,
"MSI-X Enable failed, trying MSI.\n");
- qdev->intr_count = 1;
qlge_irq_type = MSI_IRQ;
} else if (err == 0) {
set_bit(QL_MSIX_ENABLED, &qdev->flags);
--
1.7.7.6

2013-10-19 06:10:31

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 02/29] PCI/MSI/s390: Remove superfluous check of MSI type

arch_setup_msi_irqs() hook can only be called from the generic
MSI code which ensures correct MSI type parameter.

Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Martin Schwidefsky <[email protected]>
---
arch/s390/pci/pci.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index c79c6e4..61a3c2c 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -425,8 +425,6 @@ int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
int rc;

pr_debug("%s: requesting %d MSI-X interrupts...", __func__, nvec);
- if (type != PCI_CAP_ID_MSIX && type != PCI_CAP_ID_MSI)
- return -EINVAL;
if (type == PCI_CAP_ID_MSI && nvec > 1)
return 1;
msi_vecs = min(nvec, ZPCI_MSI_VEC_MAX);
--
1.7.7.6

2013-10-19 06:12:21

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 01/29] PCI/MSI/s390: Fix single MSI only check

Multiple MSIs have never been supported on s390 architecture,
but the platform code fails to report single MSI only.

Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Martin Schwidefsky <[email protected]>
---
arch/s390/pci/pci.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index f17a834..c79c6e4 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -427,6 +427,8 @@ int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
pr_debug("%s: requesting %d MSI-X interrupts...", __func__, nvec);
if (type != PCI_CAP_ID_MSIX && type != PCI_CAP_ID_MSI)
return -EINVAL;
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
msi_vecs = min(nvec, ZPCI_MSI_VEC_MAX);
msi_vecs = min_t(unsigned int, msi_vecs, CONFIG_PCI_NR_MSI);

--
1.7.7.6

2013-10-19 06:14:10

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 24/29] tg3: Make use of pcim_enable_msix() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/ethernet/broadcom/tg3.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 12d961c..026c371 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -11262,12 +11262,10 @@ static bool tg3_enable_msix(struct tg3 *tp)
msix_ent[i].vector = 0;
}

- rc = pci_enable_msix(tp->pdev, msix_ent, tp->irq_cnt);
+ rc = pcim_enable_msix(tp->pdev, msix_ent, tp->irq_cnt);
if (rc < 0) {
return false;
- } else if (rc != 0) {
- if (pci_enable_msix(tp->pdev, msix_ent, rc))
- return false;
+ } else if (rc < tp->irq_cnt) {
netdev_notice(tp->dev, "Requested %d MSI-X vectors, received %d\n",
tp->irq_cnt, rc);
tp->irq_cnt = rc;
--
1.7.7.6

2013-10-19 06:16:00

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 18/29] ntb: Fix missed call to pci_enable_msix()

Current MSI-X enablement code assumes MSI-Xs were successfully
allocated in case less than requested vectors were available.
That assumption is wrong, since MSI-Xs should be enabled with
a repeated call to pci_enable_msix(). This update fixes this.

Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Jon Mason <[email protected]>
---
drivers/ntb/ntb_hw.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 1cb6e51..de2062c 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1075,6 +1075,10 @@ static int ntb_setup_msix(struct ntb_device *ndev)
"Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
rc);
msix_entries = rc;
+
+ rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc)
+ goto err1;
}

for (i = 0; i < msix_entries; i++) {
--
1.7.7.6

2013-10-19 06:17:50

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 13/29] ipr: Do not call pci_disable_msi/msix() if pci_enable_msi/msix() failed

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/scsi/ipr.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 36ac1c3..fb57e21 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -9255,10 +9255,8 @@ static int ipr_enable_msix(struct ipr_ioa_cfg *ioa_cfg)
while ((err = pci_enable_msix(ioa_cfg->pdev, entries, vectors)) > 0)
vectors = err;

- if (err < 0) {
- pci_disable_msix(ioa_cfg->pdev);
+ if (err < 0)
return err;
- }

if (!err) {
for (i = 0; i < vectors; i++)
@@ -9278,10 +9276,8 @@ static int ipr_enable_msi(struct ipr_ioa_cfg *ioa_cfg)
while ((err = pci_enable_msi_block(ioa_cfg->pdev, vectors)) > 0)
vectors = err;

- if (err < 0) {
- pci_disable_msi(ioa_cfg->pdev);
+ if (err < 0)
return err;
- }

if (!err) {
for (i = 0; i < vectors; i++)
--
1.7.7.6

2013-10-19 06:19:40

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 07/29] PCI/MSI: Return -ENOSYS for unimplemented interfaces, not -1

Signed-off-by: Alexander Gordeev <[email protected]>
Suggested-by: Ben Hutchings <[email protected]>
---
include/linux/pci.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index d3a888a..5cfe54c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1148,13 +1148,13 @@ struct msix_entry {
#ifndef CONFIG_PCI_MSI
static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
{
- return -1;
+ return -ENOSYS;
}

static inline int
pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec)
{
- return -1;
+ return -ENOSYS;
}

static inline void pci_msi_shutdown(struct pci_dev *dev)
@@ -1169,7 +1169,7 @@ static inline int pci_msix_table_size(struct pci_dev *dev)
static inline int pci_enable_msix(struct pci_dev *dev,
struct msix_entry *entries, int nvec)
{
- return -1;
+ return -ENOSYS;
}

static inline void pci_msix_shutdown(struct pci_dev *dev)
--
1.7.7.6

2013-10-19 06:21:40

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 27/29] vmxnet3: Return -ENOSPC when not enough MSI-X vectors available

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 3518173..3df7f32 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2752,7 +2752,10 @@ vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
"Failed to enable MSI-X, error: %d\n", err);
return err;
} else if (err < vector_threshold) {
- break;
+ dev_info(&adapter->pdev->dev,
+ "Number of MSI-Xs which can be allocated "
+ "is lower than min threshold required.\n");
+ return -ENOSPC;
} else {
/* If fails to enable required number of MSI-x vectors
* try enabling minimum number of vectors required.
@@ -2764,9 +2767,6 @@ vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
}
}

- dev_info(&adapter->pdev->dev,
- "Number of MSI-X interrupts which can be allocated "
- "is lower than min threshold required.\n");
return err;
}

--
1.7.7.6

2013-10-19 06:23:28

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 17/29] megaraid: Make use of pcim_enable_msix() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/scsi/megaraid/megaraid_sas_base.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 3020921..e54c2e7 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3727,18 +3727,12 @@ static int megasas_init_fw(struct megasas_instance *instance)
(unsigned int)num_online_cpus());
for (i = 0; i < instance->msix_vectors; i++)
instance->msixentry[i].entry = i;
- i = pci_enable_msix(instance->pdev, instance->msixentry,
- instance->msix_vectors);
- if (i >= 0) {
- if (i) {
- if (!pci_enable_msix(instance->pdev,
- instance->msixentry, i))
- instance->msix_vectors = i;
- else
- instance->msix_vectors = 0;
- }
- } else
+ i = pcim_enable_msix(instance->pdev, instance->msixentry,
+ instance->msix_vectors);
+ if (i < 0)
instance->msix_vectors = 0;
+ else
+ instance->msix_vectors = i;

dev_info(&instance->pdev->dev, "[scsi%d]: FW supports"
"<%d> MSIX vector,Online CPUs: <%d>,"
--
1.7.7.6

2013-10-19 06:25:20

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 06/29] PCI/MSI: Get rid of useless count of msi_desc leftovers

If an architecture failed to allocate requested number of
MSI-Xs we should not mislead device drivers and make them
retry based on how many MSI-Xs were possibly allocated by
the architecture in the process, before the failure. It is
rude for an architecture to leave any leftovers anyway.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pci/msi.c | 19 +------------------
1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index b43f391..bbe3d3d 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -719,7 +719,7 @@ static int msix_capability_init(struct pci_dev *dev,

ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
if (ret)
- goto out_avail;
+ goto out_free;

/*
* Some devices require MSI-X to be enabled before we can touch the
@@ -744,23 +744,6 @@ static int msix_capability_init(struct pci_dev *dev,

return 0;

-out_avail:
- if (ret < 0) {
- /*
- * If we had some success, report the number of irqs
- * we succeeded in setting up.
- */
- struct msi_desc *entry;
- int avail = 0;
-
- list_for_each_entry(entry, &dev->msi_list, list) {
- if (entry->irq != 0)
- avail++;
- }
- if (avail != 0)
- ret = avail;
- }
-
out_free:
free_msi_irqs(dev);

--
1.7.7.6

2013-10-19 06:36:38

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 05/29] PCI/MSI: Fix return value when populate_msi_sysfs() failed

If populate_msi_sysfs() function failed msix_capability_init()
must return the error code, but it returns the success instead.
This update fixes the described misbehaviour.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pci/msi.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d5f90d6..b43f391 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -719,7 +719,7 @@ static int msix_capability_init(struct pci_dev *dev,

ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
if (ret)
- goto error;
+ goto out_avail;

/*
* Some devices require MSI-X to be enabled before we can touch the
@@ -732,10 +732,8 @@ static int msix_capability_init(struct pci_dev *dev,
msix_program_entries(dev, entries);

ret = populate_msi_sysfs(dev);
- if (ret) {
- ret = 0;
- goto error;
- }
+ if (ret)
+ goto out_free;

/* Set MSI-X enabled bits and unmask the function */
pci_intx_for_msi(dev, 0);
@@ -746,7 +744,7 @@ static int msix_capability_init(struct pci_dev *dev,

return 0;

-error:
+out_avail:
if (ret < 0) {
/*
* If we had some success, report the number of irqs
@@ -763,6 +761,7 @@ error:
ret = avail;
}

+out_free:
free_msi_irqs(dev);

return ret;
--
1.7.7.6

2013-10-19 06:38:27

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 23/29] qlge: Make use of pcim_enable_msix() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index ac54cb0..b142902 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -3272,24 +3272,17 @@ static void ql_enable_msix(struct ql_adapter *qdev)
for (i = 0; i < qdev->intr_count; i++)
qdev->msi_x_entry[i].entry = i;

- /* Loop to get our vectors. We start with
- * what we want and settle for what we get.
- */
- do {
- err = pci_enable_msix(qdev->pdev,
- qdev->msi_x_entry, qdev->intr_count);
- if (err > 0)
- qdev->intr_count = err;
- } while (err > 0);
-
+ err = pcim_enable_msix(qdev->pdev,
+ qdev->msi_x_entry, qdev->intr_count);
if (err < 0) {
kfree(qdev->msi_x_entry);
qdev->msi_x_entry = NULL;
netif_warn(qdev, ifup, qdev->ndev,
"MSI-X Enable failed, trying MSI.\n");
qlge_irq_type = MSI_IRQ;
- } else if (err == 0) {
+ } else {
set_bit(QL_MSIX_ENABLED, &qdev->flags);
+ qdev->intr_count = err;
netif_info(qdev, ifup, qdev->ndev,
"MSI-X Enabled, got %d vectors.\n",
qdev->intr_count);
--
1.7.7.6

2013-10-19 06:40:14

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 28/29] vmxnet3: Limit number of rx queues to 1 if per-queue MSI-Xs failed

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 3df7f32..d33802c 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2814,23 +2814,21 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)

err = vmxnet3_acquire_msix_vectors(adapter,
adapter->intr.num_intrs);
- /* If we cannot allocate one MSIx vector per queue
- * then limit the number of rx queues to 1
- */
- if (err == VMXNET3_LINUX_MIN_MSIX_VECT) {
- if (adapter->share_intr != VMXNET3_INTR_BUDDYSHARE
- || adapter->num_rx_queues != 1) {
+ if (!err) {
+ /* If we cannot allocate one MSIx vector per queue
+ * then limit the number of rx queues to 1
+ */
+ if ((adapter->intr.num_intrs ==
+ VMXNET3_LINUX_MIN_MSIX_VECT) &&
+ ((adapter->share_intr != VMXNET3_INTR_BUDDYSHARE) ||
+ (adapter->num_rx_queues != 1))) {
adapter->share_intr = VMXNET3_INTR_TXSHARE;
netdev_err(adapter->netdev,
"Number of rx queues : 1\n");
adapter->num_rx_queues = 1;
- adapter->intr.num_intrs =
- VMXNET3_LINUX_MIN_MSIX_VECT;
}
return;
}
- if (!err)
- return;

/* If we cannot allocate MSIx vectors use only one rx queue */
dev_info(&adapter->pdev->dev,
--
1.7.7.6

2013-10-19 06:42:12

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 15/29] ixgbe: Make use of pcim_enable_msix_range() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 16 ++++------------
1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 90b4e10..9954b7a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -697,7 +697,7 @@ static void ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
int vectors)
{
- int err, vector_threshold;
+ int vector_threshold;

/* We'll want at least 2 (vector_threshold):
* 1) TxQ[0] + RxQ[0] handler
@@ -711,18 +711,10 @@ static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
* Right now, we simply care about how many we'll get; we'll
* set them up later while requesting irq's.
*/
- while (vectors >= vector_threshold) {
- err = pci_enable_msix(adapter->pdev, adapter->msix_entries,
- vectors);
- if (!err) /* Success in acquiring all requested vectors. */
- break;
- else if (err < 0)
- vectors = 0; /* Nasty failure, quit now */
- else /* err == number of vectors we should try again with */
- vectors = err;
- }
+ vectors = pcim_enable_msix_range(adapter->pdev, adapter->msix_entries,
+ vectors, vector_threshold);

- if (vectors < vector_threshold) {
+ if (vectors < 0) {
/* Can't allocate enough MSI-X interrupts? Oh well.
* This just means we'll go with either a single MSI
* vector or fall back to legacy interrupts.
--
1.7.7.6

2013-10-19 06:43:59

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 21/29] qla2xxx: Make use of pcim_enable_msix_range() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/scsi/qla2xxx/qla_isr.c | 28 ++++++++++++----------------
1 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index df1b30b..3238a0ac 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2836,27 +2836,23 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
for (i = 0; i < ha->msix_count; i++)
entries[i].entry = i;

- ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
- if (ret) {
- if (ret < MIN_MSIX_COUNT)
- goto msix_failed;
-
+ ret = pcim_enable_msix_range(ha->pdev,
+ entries, ha->msix_count, MIN_MSIX_COUNT);
+ if (ret < 0) {
+ ql_log(ql_log_fatal, vha, 0x00c7,
+ "MSI-X: Failed to enable support, "
+ "giving up -- %d/%d.\n",
+ ha->msix_count, ret);
+ goto msix_out;
+ }
+ if (ret < ha->msix_count) {
ql_log(ql_log_warn, vha, 0x00c6,
"MSI-X: Failed to enable support "
"-- %d/%d\n Retry with %d vectors.\n",
ha->msix_count, ret, ret);
- ha->msix_count = ret;
- ret = pci_enable_msix(ha->pdev, entries, ha->msix_count);
- if (ret) {
-msix_failed:
- ql_log(ql_log_fatal, vha, 0x00c7,
- "MSI-X: Failed to enable support, "
- "giving up -- %d/%d.\n",
- ha->msix_count, ret);
- goto msix_out;
- }
- ha->max_rsp_queues = ha->msix_count - 1;
}
+ ha->msix_count = ret;
+ ha->max_rsp_queues = ha->msix_count - 1;
ha->msix_entries = kzalloc(sizeof(struct qla_msix_entry) *
ha->msix_count, GFP_KERNEL);
if (!ha->msix_entries) {
--
1.7.7.6

2013-10-19 06:45:51

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 40 +++++++++---------------------------
1 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index d33802c..e552d2b 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2735,39 +2735,19 @@ vmxnet3_read_mac_addr(struct vmxnet3_adapter *adapter, u8 *mac)
*/

static int
-vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
- int vectors)
+vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter, int vectors)
{
- int err = -EINVAL, vector_threshold;
- vector_threshold = VMXNET3_LINUX_MIN_MSIX_VECT;
-
- while (vectors >= vector_threshold) {
- err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
- vectors);
- if (!err) {
- adapter->intr.num_intrs = vectors;
- return 0;
- } else if (err < 0) {
- dev_err(&adapter->netdev->dev,
- "Failed to enable MSI-X, error: %d\n", err);
- return err;
- } else if (err < vector_threshold) {
- dev_info(&adapter->pdev->dev,
- "Number of MSI-Xs which can be allocated "
- "is lower than min threshold required.\n");
- return -ENOSPC;
- } else {
- /* If fails to enable required number of MSI-x vectors
- * try enabling minimum number of vectors required.
- */
- dev_err(&adapter->netdev->dev,
- "Failed to enable %d MSI-X, trying %d instead\n",
- vectors, vector_threshold);
- vectors = vector_threshold;
- }
+ vectors = pcim_enable_msix_range(adapter->pdev,
+ adapter->intr.msix_entries, vectors,
+ VMXNET3_LINUX_MIN_MSIX_VECT);
+ if (vectors < 0) {
+ dev_err(&adapter->netdev->dev,
+ "Failed to enable MSI-X, error: %d\n", vectors);
+ return vectors;
}

- return err;
+ adapter->intr.num_intrs = vectors;
+ return 0;
}


--
1.7.7.6

2013-10-19 06:47:39

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 19/29] ntb: Make use of pcim_enable_msix/msix_exact() interfaces

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/ntb/ntb_hw.c | 38 +++++++++++++-------------------------
1 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index de2062c..b7d6eed 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1032,22 +1032,15 @@ static int ntb_setup_msix(struct ntb_device *ndev)
struct msix_entry *msix;
int msix_entries;
int rc, i;
- u16 val;

- if (!pdev->msix_cap) {
- rc = -EIO;
- goto err;
- }
-
- rc = pci_read_config_word(pdev, pdev->msix_cap + PCI_MSIX_FLAGS, &val);
- if (rc)
+ rc = pci_msix_table_size(pdev);
+ if (rc < 0)
goto err;
-
- msix_entries = msix_table_size(val);
- if (msix_entries > ndev->limits.msix_cnt) {
+ else if (rc > ndev->limits.msix_cnt) {
rc = -EINVAL;
goto err;
- }
+ } else
+ msix_entries = rc;

ndev->msix_entries = kmalloc(sizeof(struct msix_entry) * msix_entries,
GFP_KERNEL);
@@ -1059,26 +1052,21 @@ static int ntb_setup_msix(struct ntb_device *ndev)
for (i = 0; i < msix_entries; i++)
ndev->msix_entries[i].entry = i;

- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc < 0)
- goto err1;
- if (rc > 0) {
+ if (ndev->hw_type != BWD_HW)
/* On SNB, the link interrupt is always tied to 4th vector. If
* we can't get all 4, then we can't use MSI-X.
*/
- if (ndev->hw_type != BWD_HW) {
- rc = -EIO;
- goto err1;
- }
-
+ rc = pcim_enable_msix_exact(pdev,
+ ndev->msix_entries, msix_entries);
+ else
+ rc = pcim_enable_msix(pdev, ndev->msix_entries, msix_entries);
+ if (rc < 0)
+ goto err1;
+ else if (rc < msix_entries) {
dev_warn(&pdev->dev,
"Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
rc);
msix_entries = rc;
-
- rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
- if (rc)
- goto err1;
}

for (i = 0; i < msix_entries; i++) {
--
1.7.7.6

2013-10-19 06:49:37

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 14/29] ipr: Make use of pcim_enable_msi/msix() interfaces

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/scsi/ipr.c | 47 ++++++++++++++++++-----------------------------
1 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index fb57e21..d7c94d5 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -9245,47 +9245,36 @@ ipr_get_chip_info(const struct pci_device_id *dev_id)
static int ipr_enable_msix(struct ipr_ioa_cfg *ioa_cfg)
{
struct msix_entry entries[IPR_MAX_MSIX_VECTORS];
- int i, err, vectors;
+ int i, vectors;

- for (i = 0; i < ARRAY_SIZE(entries); ++i)
+ vectors = min_t(int, ARRAY_SIZE(entries), ipr_number_of_msix);
+ for (i = 0; i < vectors; ++i)
entries[i].entry = i;

- vectors = ipr_number_of_msix;
+ vectors = pcim_enable_msix(ioa_cfg->pdev, entries, vectors);
+ if (vectors < 0)
+ return vectors;

- while ((err = pci_enable_msix(ioa_cfg->pdev, entries, vectors)) > 0)
- vectors = err;
+ for (i = 0; i < vectors; i++)
+ ioa_cfg->vectors_info[i].vec = entries[i].vector;
+ ioa_cfg->nvectors = vectors;

- if (err < 0)
- return err;
-
- if (!err) {
- for (i = 0; i < vectors; i++)
- ioa_cfg->vectors_info[i].vec = entries[i].vector;
- ioa_cfg->nvectors = vectors;
- }
-
- return err;
+ return 0;
}

static int ipr_enable_msi(struct ipr_ioa_cfg *ioa_cfg)
{
- int i, err, vectors;
+ int i, vectors;

- vectors = ipr_number_of_msix;
+ vectors = pcim_enable_msi(ioa_cfg->pdev, ipr_number_of_msix);
+ if (vectors < 0)
+ return vectors;

- while ((err = pci_enable_msi_block(ioa_cfg->pdev, vectors)) > 0)
- vectors = err;
+ for (i = 0; i < vectors; i++)
+ ioa_cfg->vectors_info[i].vec = ioa_cfg->pdev->irq + i;
+ ioa_cfg->nvectors = vectors;

- if (err < 0)
- return err;
-
- if (!err) {
- for (i = 0; i < vectors; i++)
- ioa_cfg->vectors_info[i].vec = ioa_cfg->pdev->irq + i;
- ioa_cfg->nvectors = vectors;
- }
-
- return err;
+ return 0;
}

static void name_msi_vectors(struct ipr_ioa_cfg *ioa_cfg)
--
1.7.7.6

2013-10-19 06:51:23

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 16/29] ixgbevf: Make use of pcim_enable_msix_range() interface

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 33 +++++++-------------
1 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 59a62bb..ced3b24 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1749,7 +1749,6 @@ void ixgbevf_reset(struct ixgbevf_adapter *adapter)
static int ixgbevf_acquire_msix_vectors(struct ixgbevf_adapter *adapter,
int vectors)
{
- int err = 0;
int vector_threshold;

/* We'll want at least 2 (vector_threshold):
@@ -1763,33 +1762,25 @@ static int ixgbevf_acquire_msix_vectors(struct ixgbevf_adapter *adapter,
* Right now, we simply care about how many we'll get; we'll
* set them up later while requesting irq's.
*/
- while (vectors >= vector_threshold) {
- err = pci_enable_msix(adapter->pdev, adapter->msix_entries,
- vectors);
- if (!err || err < 0) /* Success or a nasty failure. */
- break;
- else /* err == number of vectors we should try again with */
- vectors = err;
- }
+ vectors = pcim_enable_msix_range(adapter->pdev, adapter->msix_entries,
+ vectors, vector_threshold);

- if (vectors < vector_threshold)
- err = -ENOMEM;
-
- if (err) {
+ if (vectors < 0) {
dev_err(&adapter->pdev->dev,
"Unable to allocate MSI-X interrupts\n");
kfree(adapter->msix_entries);
adapter->msix_entries = NULL;
- } else {
- /*
- * Adjust for only the vectors we'll use, which is minimum
- * of max_msix_q_vectors + NON_Q_VECTORS, or the number of
- * vectors we were allocated.
- */
- adapter->num_msix_vectors = vectors;
+ return vectors;
}

- return err;
+ /*
+ * Adjust for only the vectors we'll use, which is minimum
+ * of max_msix_q_vectors + NON_Q_VECTORS, or the number of
+ * vectors we were allocated.
+ */
+ adapter->num_msix_vectors = vectors;
+
+ return 0;
}

/**
--
1.7.7.6

2013-10-19 06:53:09

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 20/29] qib: Make use of pcim_enable_msix() and pci_msix_table_size() interfaces

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/infiniband/hw/qib/qib_pcie.c | 48 +++++++++++++++++-----------------
1 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c
index 3f14009..bad0ca3 100644
--- a/drivers/infiniband/hw/qib/qib_pcie.c
+++ b/drivers/infiniband/hw/qib/qib_pcie.c
@@ -197,46 +197,46 @@ static void qib_msix_setup(struct qib_devdata *dd, int pos, u32 *msixcnt,
struct qib_msix_entry *qib_msix_entry)
{
int ret;
- u32 tabsize = 0;
- u16 msix_flags;
+ int tabsize = *msixcnt;
struct msix_entry *msix_entry;
int i;

+ ret = pci_msix_table_size(dd->pcidev);
+ if (ret < 0)
+ goto err;
+
+ tabsize = min(tabsize, ret);
+
/* We can't pass qib_msix_entry array to qib_msix_setup
* so use a dummy msix_entry array and copy the allocated
* irq back to the qib_msix_entry array. */
- msix_entry = kmalloc(*msixcnt * sizeof(*msix_entry), GFP_KERNEL);
+ msix_entry = kmalloc(tabsize * sizeof(*msix_entry), GFP_KERNEL);
if (!msix_entry) {
ret = -ENOMEM;
- goto do_intx;
+ goto err;
}
- for (i = 0; i < *msixcnt; i++)
+ for (i = 0; i < tabsize; i++)
msix_entry[i] = qib_msix_entry[i].msix;

- pci_read_config_word(dd->pcidev, pos + PCI_MSIX_FLAGS, &msix_flags);
- tabsize = 1 + (msix_flags & PCI_MSIX_FLAGS_QSIZE);
- if (tabsize > *msixcnt)
- tabsize = *msixcnt;
- ret = pci_enable_msix(dd->pcidev, msix_entry, tabsize);
- if (ret > 0) {
+ ret = pcim_enable_msix(dd->pcidev, msix_entry, tabsize);
+ if (ret < 0)
+ goto err;
+ else
tabsize = ret;
- ret = pci_enable_msix(dd->pcidev, msix_entry, tabsize);
- }
-do_intx:
- if (ret) {
- qib_dev_err(dd,
- "pci_enable_msix %d vectors failed: %d, falling back to INTx\n",
- tabsize, ret);
- tabsize = 0;
- }
+
for (i = 0; i < tabsize; i++)
qib_msix_entry[i].msix = msix_entry[i];
+
kfree(msix_entry);
*msixcnt = tabsize;
-
- if (ret)
- qib_enable_intx(dd->pcidev);
-
+ return;
+
+err:
+ qib_dev_err(dd,
+ "pci_enable_msix %d vectors failed: %d, falling back to INTx\n",
+ tabsize, ret);
+ *msixcnt = 0;
+ qib_enable_intx(dd->pcidev);
}

/**
--
1.7.7.6

2013-10-19 06:55:00

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 08/29] PCI/MSI: Make pci_enable_msix() 'nvec' argument unsigned int

Make pci_enable_msix() and pci_enable_msi_block() consistent
with regard to the type of 'nvec' argument. Indeed, a number
of vectors to allocate is a natural value, so make it unsigned.

Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/PCI/MSI-HOWTO.txt | 3 ++-
drivers/pci/msi.c | 3 ++-
include/linux/pci.h | 5 +++--
3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index a091780..845edb5 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -190,7 +190,8 @@ same number.

4.3.1 pci_enable_msix

-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
+int pci_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)

Calling this function asks the PCI subsystem to allocate 'nvec' MSIs.
The 'entries' argument is a pointer to an array of msix_entry structs
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index bbe3d3d..fdae3a4 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -932,7 +932,8 @@ int pci_msix_table_size(struct pci_dev *dev)
* of irqs or MSI-X vectors available. Driver should use the returned value to
* re-send its request.
**/
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
+int pci_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)
{
int status, nr_entries;
int i, j;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5cfe54c..d6832de 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1167,7 +1167,7 @@ static inline int pci_msix_table_size(struct pci_dev *dev)
return 0;
}
static inline int pci_enable_msix(struct pci_dev *dev,
- struct msix_entry *entries, int nvec)
+ struct msix_entry *entries, int unsigned nvec)
{
return -ENOSYS;
}
@@ -1192,7 +1192,8 @@ int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
void pci_msi_shutdown(struct pci_dev *dev);
void pci_disable_msi(struct pci_dev *dev);
int pci_msix_table_size(struct pci_dev *dev);
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec);
+int pci_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec);
void pci_msix_shutdown(struct pci_dev *dev);
void pci_disable_msix(struct pci_dev *dev);
void msi_remove_pci_irq_vectors(struct pci_dev *dev);
--
1.7.7.6

2013-10-19 06:56:48

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 04/29] PCI/MSI/pSeries: Make quota traversing and requesting race-safe

Signed-off-by: Alexander Gordeev <[email protected]>
---
arch/powerpc/platforms/pseries/msi.c | 24 ++++++++++++++++++++++--
1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 009ec73..0e1d288 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -26,6 +26,8 @@ static int query_token, change_token;
#define RTAS_CHANGE_MSIX_FN 4
#define RTAS_CHANGE_32MSI_FN 5

+static DEFINE_MUTEX(rtas_quota_mutex);
+
/* RTAS Helpers */

static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
@@ -345,7 +347,9 @@ static int rtas_msi_check_device(struct pci_dev *pdev, int nvec, int type)
if (rc)
return rc;

+ mutex_lock(&rtas_quota_mutex);
quota = msi_quota_for_device(pdev, nvec);
+ mutex_unlock(&rtas_quota_mutex);

if (quota && quota < nvec)
return quota;
@@ -399,6 +403,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
struct msi_msg msg;
int nvec = nvec_in;
int use_32bit_msi_hack = 0;
+ int quota;

pdn = pci_get_pdn(pdev);
if (!pdn)
@@ -407,13 +412,21 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
if (type == PCI_CAP_ID_MSIX && check_msix_entries(pdev))
return -EINVAL;

+ mutex_lock(&rtas_quota_mutex);
+
+ quota = msi_quota_for_device(pdev, nvec);
+ if (quota && quota < nvec) {
+ mutex_unlock(&rtas_quota_mutex);
+ return quota;
+ }
+
/*
* Firmware currently refuse any non power of two allocation
* so we round up if the quota will allow it.
*/
if (type == PCI_CAP_ID_MSIX) {
int m = roundup_pow_of_two(nvec);
- int quota = msi_quota_for_device(pdev, m);
+ quota = msi_quota_for_device(pdev, m);

if (quota >= m)
nvec = m;
@@ -433,8 +446,11 @@ again:
* We only want to run the 32 bit MSI hack below if
* the max bus speed is Gen2 speed
*/
- if (pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT)
+ if (pdev->bus->max_bus_speed !=
+ PCIE_SPEED_5_0GT) {
+ mutex_unlock(&rtas_quota_mutex);
return rc;
+ }

use_32bit_msi_hack = 1;
}
@@ -459,6 +475,7 @@ again:
nvec = nvec_in;
goto again;
}
+ mutex_unlock(&rtas_quota_mutex);
pr_debug("rtas_msi: rtas_change_msi() failed\n");
return rc;
}
@@ -467,6 +484,7 @@ again:
list_for_each_entry(entry, &pdev->msi_list, list) {
hwirq = rtas_query_irq_number(pdn, i++);
if (hwirq < 0) {
+ mutex_unlock(&rtas_quota_mutex);
pr_debug("rtas_msi: error (%d) getting hwirq\n", hwirq);
return hwirq;
}
@@ -474,6 +492,7 @@ again:
virq = irq_create_mapping(NULL, hwirq);

if (virq == NO_IRQ) {
+ mutex_unlock(&rtas_quota_mutex);
pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
return -ENOSPC;
}
@@ -486,6 +505,7 @@ again:
entry->msg = msg;
}

+ mutex_unlock(&rtas_quota_mutex);
return 0;
}

--
1.7.7.6

2013-10-19 07:03:10

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 09/29] PCI/MSI: Factor out pci_get_msi_cap() interface

Device drivers can use this interface to obtain maximum number
of MSI interrupts the device supports and use that number i.e.
in a following call to pci_enable_msi_block() interface.

Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/PCI/MSI-HOWTO.txt | 15 +++++++++++++++
drivers/pci/msi.c | 33 +++++++++++++++++++++------------
include/linux/pci.h | 6 ++++++
3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index 845edb5..e7e3b20 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -169,6 +169,21 @@ on any interrupt for which it previously called request_irq().
Failure to do so results in a BUG_ON(), leaving the device with
MSI enabled and thus leaking its vector.

+4.2.5 pci_get_msi_cap
+
+int pci_get_msi_cap(struct pci_dev *dev)
+
+This function could be used to retrieve the number of MSI vectors the
+device requested (via the Multiple Message Capable register). The MSI
+specification only allows the returned value to be a power of two,
+up to a maximum of 2^5 (32).
+
+If this function returns a negative number, it indicates the device is
+not capable of sending MSIs.
+
+If this function returns a positive number, it indicates the maximum
+number of MSI interrupt vectors that could be allocated.
+
4.3 Using MSI-X

The MSI-X capability is much more flexible than the MSI capability.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index fdae3a4..14dc4d1 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -795,6 +795,21 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
return 0;
}

+int pci_get_msi_cap(struct pci_dev *dev)
+{
+ int ret;
+ u16 msgctl;
+
+ if (!dev->msi_cap)
+ return -EINVAL;
+
+ pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
+ ret = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+
+ return ret;
+}
+EXPORT_SYMBOL(pci_get_msi_cap);
+
/**
* pci_enable_msi_block - configure device's MSI capability structure
* @dev: device to configure
@@ -811,13 +826,10 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
{
int status, maxvec;
- u16 msgctl;

- if (!dev->msi_cap)
- return -EINVAL;
-
- pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
- maxvec = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+ maxvec = pci_get_msi_cap(dev);
+ if (maxvec < 0)
+ return maxvec;
if (nvec > maxvec)
return maxvec;

@@ -842,13 +854,10 @@ EXPORT_SYMBOL(pci_enable_msi_block);
int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec)
{
int ret, nvec;
- u16 msgctl;

- if (!dev->msi_cap)
- return -EINVAL;
-
- pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
- ret = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+ ret = pci_get_msi_cap(dev);
+ if (ret < 0)
+ return ret;

if (maxvec)
*maxvec = ret;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d6832de..df72084 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1146,6 +1146,11 @@ struct msix_entry {


#ifndef CONFIG_PCI_MSI
+static inline int pci_get_msi_cap(struct pci_dev *dev)
+{
+ return -ENOSYS;
+}
+
static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
{
return -ENOSYS;
@@ -1187,6 +1192,7 @@ static inline int pci_msi_enabled(void)
return 0;
}
#else
+int pci_get_msi_cap(struct pci_dev *dev);
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
void pci_msi_shutdown(struct pci_dev *dev);
--
1.7.7.6

2013-10-19 07:06:27

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 11/29] PCI/MSI: Convert pci_msix_table_size() to a public interface

Make pci_msix_table_size() return error code if the device
does not support MSI-X. This update is needed to create a
consistent MSI-X counterpart for pci_get_msi_cap() MSI
interface.

Device drivers can use this interface to obtain maximum number
of MSI-X interrupts the device supports and i.e. use that number
in a following call to pci_enable_msix() interface.

Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/PCI/MSI-HOWTO.txt | 13 +++++++++++++
drivers/pci/msi.c | 5 ++++-
drivers/pci/pcie/portdrv_core.c | 5 +++--
include/linux/pci.h | 2 +-
4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index 47118f3..fdf3ae3 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -245,6 +245,19 @@ MSI-X Table. This address is mapped by the PCI subsystem, and should not
be accessed directly by the device driver. If the driver wishes to
mask or unmask an interrupt, it should call disable_irq() / enable_irq().

+4.3.4 pci_msix_table_size
+
+int pci_msix_table_size(struct pci_dev *dev)
+
+This function could be used to retrieve number of entries in the device
+MSI-X table.
+
+If this function returns a negative number, it indicates the device is
+not capable of sending MSI-Xs.
+
+If this function returns a positive number, it indicates the maximum
+number of MSI-X interrupt vectors that could be allocated.
+
4.4 Handling devices implementing both MSI and MSI-X capabilities

If a device implements both MSI and MSI-X capabilities, it can
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 13bd901..96f51d0 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -898,11 +898,12 @@ int pci_msix_table_size(struct pci_dev *dev)
u16 control;

if (!dev->msix_cap)
- return 0;
+ return -EINVAL;

pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
return msix_table_size(control);
}
+EXPORT_SYMBOL(pci_msix_table_size);

/**
* pci_enable_msix - configure device's MSI-X capability structure
@@ -933,6 +934,8 @@ int pci_enable_msix(struct pci_dev *dev,
return status;

nr_entries = pci_msix_table_size(dev);
+ if (nr_entries < 0)
+ return nr_entries;
if (nvec > nr_entries)
return nr_entries;

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 31063ac..242f9ff 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -80,8 +80,9 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
u32 reg32;

nr_entries = pci_msix_table_size(dev);
- if (!nr_entries)
- return -EINVAL;
+ if (nr_entries < 0)
+ return nr_entries;
+ BUG_ON(!nr_entries);
if (nr_entries > PCIE_PORT_MAX_MSIX_ENTRIES)
nr_entries = PCIE_PORT_MAX_MSIX_ENTRIES;

diff --git a/include/linux/pci.h b/include/linux/pci.h
index a04f5da..bef5775 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1163,7 +1163,7 @@ static inline void pci_disable_msi(struct pci_dev *dev)

static inline int pci_msix_table_size(struct pci_dev *dev)
{
- return 0;
+ return -ENOSYS;
}
static inline int pci_enable_msix(struct pci_dev *dev,
struct msix_entry *entries, int unsigned nvec)
--
1.7.7.6

2013-10-19 07:06:49

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 10/29] PCI/MSI: Get rid of pci_enable_msi_block_auto() interface

As result of introduction of pci_get_msi_cap() interface
pci_enable_msi_block_auto() function became superflous.

To enable maximum possible number of MSIs drivers will
first obtain that number from pci_get_msi_cap() function
and then call pci_enable_msi_block() interface.

Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/PCI/MSI-HOWTO.txt | 30 +-------------------
drivers/ata/ahci.c | 56 ++++++++++++++++++++++++--------------
drivers/pci/msi.c | 22 ---------------
include/linux/pci.h | 7 -----
4 files changed, 37 insertions(+), 78 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index e7e3b20..47118f3 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -127,33 +127,7 @@ on the number of vectors that can be allocated; pci_enable_msi_block()
returns as soon as it finds any constraint that doesn't allow the
call to succeed.

-4.2.3 pci_enable_msi_block_auto
-
-int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *count)
-
-This variation on pci_enable_msi() call allows a device driver to request
-the maximum possible number of MSIs. The MSI specification only allows
-interrupts to be allocated in powers of two, up to a maximum of 2^5 (32).
-
-If this function returns a positive number, it indicates that it has
-succeeded and the returned value is the number of allocated interrupts. In
-this case, the function enables MSI on this device and updates dev->irq to
-be the lowest of the new interrupts assigned to it. The other interrupts
-assigned to the device are in the range dev->irq to dev->irq + returned
-value - 1.
-
-If this function returns a negative number, it indicates an error and
-the driver should not attempt to request any more MSI interrupts for
-this device.
-
-If the device driver needs to know the number of interrupts the device
-supports it can pass the pointer count where that number is stored. The
-device driver must decide what action to take if pci_enable_msi_block_auto()
-succeeds, but returns a value less than the number of interrupts supported.
-If the device driver does not need to know the number of interrupts
-supported, it can set the pointer count to NULL.
-
-4.2.4 pci_disable_msi
+4.2.3 pci_disable_msi

void pci_disable_msi(struct pci_dev *dev)

@@ -169,7 +143,7 @@ on any interrupt for which it previously called request_irq().
Failure to do so results in a BUG_ON(), leaving the device with
MSI enabled and thus leaking its vector.

-4.2.5 pci_get_msi_cap
+4.2.4 pci_get_msi_cap

int pci_get_msi_cap(struct pci_dev *dev)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 9d715ae..f1c8838 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1091,26 +1091,40 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host)
{}
#endif

-int ahci_init_interrupts(struct pci_dev *pdev, struct ahci_host_priv *hpriv)
+int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports,
+ struct ahci_host_priv *hpriv)
{
- int rc;
- unsigned int maxvec;
+ int rc, nvec;

- if (!(hpriv->flags & AHCI_HFLAG_NO_MSI)) {
- rc = pci_enable_msi_block_auto(pdev, &maxvec);
- if (rc > 0) {
- if ((rc == maxvec) || (rc == 1))
- return rc;
- /*
- * Assume that advantage of multipe MSIs is negated,
- * so fallback to single MSI mode to save resources
- */
- pci_disable_msi(pdev);
- if (!pci_enable_msi(pdev))
- return 1;
- }
- }
+ if (hpriv->flags & AHCI_HFLAG_NO_MSI)
+ goto intx;
+
+ rc = pci_get_msi_cap(pdev);
+ if (rc < 0)
+ goto intx;
+
+ /*
+ * If number of MSIs is less than number of ports then Sharing Last
+ * Message mode could be enforced. In this case assume that advantage
+ * of multipe MSIs is negated and use single MSI mode instead.
+ */
+ if (rc < n_ports)
+ goto single_msi;
+
+ nvec = rc;
+ rc = pci_enable_msi_block(pdev, nvec);
+ if (rc)
+ goto intx;

+ return nvec;
+
+single_msi:
+ rc = pci_enable_msi(pdev);
+ if (rc)
+ goto intx;
+ return 1;
+
+intx:
pci_intx(pdev, 1);
return 0;
}
@@ -1277,10 +1291,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)

hpriv->mmio = pcim_iomap_table(pdev)[ahci_pci_bar];

- n_msis = ahci_init_interrupts(pdev, hpriv);
- if (n_msis > 1)
- hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
-
/* save initial config */
ahci_pci_save_initial_config(pdev, hpriv);

@@ -1335,6 +1345,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));

+ n_msis = ahci_init_interrupts(pdev, n_ports, hpriv);
+ if (n_msis > 1)
+ hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
+
host = ata_host_alloc_pinfo(&pdev->dev, ppi, n_ports);
if (!host)
return -ENOMEM;
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 14dc4d1..13bd901 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -851,28 +851,6 @@ int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
}
EXPORT_SYMBOL(pci_enable_msi_block);

-int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec)
-{
- int ret, nvec;
-
- ret = pci_get_msi_cap(dev);
- if (ret < 0)
- return ret;
-
- if (maxvec)
- *maxvec = ret;
-
- do {
- nvec = ret;
- ret = pci_enable_msi_block(dev, nvec);
- } while (ret > 0);
-
- if (ret < 0)
- return ret;
- return nvec;
-}
-EXPORT_SYMBOL(pci_enable_msi_block_auto);
-
void pci_msi_shutdown(struct pci_dev *dev)
{
struct msi_desc *desc;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index df72084..a04f5da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1156,12 +1156,6 @@ static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
return -ENOSYS;
}

-static inline int
-pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec)
-{
- return -ENOSYS;
-}
-
static inline void pci_msi_shutdown(struct pci_dev *dev)
{ }
static inline void pci_disable_msi(struct pci_dev *dev)
@@ -1194,7 +1188,6 @@ static inline int pci_msi_enabled(void)
#else
int pci_get_msi_cap(struct pci_dev *dev);
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
-int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
void pci_msi_shutdown(struct pci_dev *dev);
void pci_disable_msi(struct pci_dev *dev);
int pci_msix_table_size(struct pci_dev *dev);
--
1.7.7.6

2013-10-19 07:08:35

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

Currently many device drivers need contiguously call functions
pci_enable_msix() for MSI-X or pci_enable_msi_block() for MSI
in a loop until success or failure. This update generalizes
this usage pattern and introduces pcim_enable_msi*() family
helpers.

As result, device drivers do not have to deal with tri-state
return values from pci_enable_msix() and pci_enable_msi_block()
functions directly and expected to have more clearer and straight
code.

So i.e. the request loop described in the documentation...

int foo_driver_enable_msix(struct foo_adapter *adapter,
int nvec)
{
while (nvec >= FOO_DRIVER_MINIMUM_NVEC) {
rc = pci_enable_msix(adapter->pdev,
adapter->msix_entries,
nvec);
if (rc > 0)
nvec = rc;
else
return rc;
}

return -ENOSPC;
}

...would turn into a single helper call....

rc = pcim_enable_msix_range(adapter->pdev,
adapter->msix_entries,
nvec,
FOO_DRIVER_MINIMUM_NVEC);

Device drivers with more specific requirements (i.e. a number of
MSI-Xs which is a multiple of a certain number within a specified
range) would still need to implement the loop using the two old
functions.

Signed-off-by: Alexander Gordeev <[email protected]>
Suggested-by: Ben Hutchings <[email protected]>
---
Documentation/PCI/MSI-HOWTO.txt | 134 +++++++++++++++++++++++++++++++++++++--
drivers/pci/msi.c | 46 +++++++++++++
include/linux/pci.h | 59 +++++++++++++++++
3 files changed, 234 insertions(+), 5 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index fdf3ae3..f348b6f 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -127,7 +127,62 @@ on the number of vectors that can be allocated; pci_enable_msi_block()
returns as soon as it finds any constraint that doesn't allow the
call to succeed.

-4.2.3 pci_disable_msi
+4.2.3 pcim_enable_msi_range
+
+int pcim_enable_msi_range(struct pci_dev *dev, struct msix_entry *entries,
+ unsigned int nvec, unsigned int minvec)
+
+This variation on pci_enable_msi_block() call allows a device driver to
+request any number of MSIs within specified range minvec to nvec. Whenever
+possible device drivers are encouraged to use this function rather than
+explicit request loop calling pci_enable_msi_block().
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to request any more MSI interrupts for
+this device.
+
+If this function returns a positive number it indicates at least the
+returned number of MSI interrupts have been successfully allocated (it may
+have allocated more in order to satisfy the power-of-two requirement).
+Device drivers can use this number to further initialize devices.
+
+4.2.4 pcim_enable_msi
+
+int pcim_enable_msi(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int maxvec)
+
+This variation on pci_enable_msi_block() call allows a device driver to
+request any number of MSIs up to maxvec. Whenever possible device drivers
+are encouraged to use this function rather than explicit request loop
+calling pci_enable_msi_block().
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to request any more MSI interrupts for
+this device.
+
+If this function returns a positive number it indicates at least the
+returned number of MSI interrupts have been successfully allocated (it may
+have allocated more in order to satisfy the power-of-two requirement).
+Device drivers can use this number to further initialize devices.
+
+4.2.5 pcim_enable_msi_exact
+
+int pcim_enable_msi_exact(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)
+
+This variation on pci_enable_msi_block() call allows a device driver to
+request exactly nvec MSIs.
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to request any more MSI interrupts for
+this device.
+
+If this function returns the value of nvec it indicates MSI interrupts
+have been successfully allocated. No other value in case of success could
+be returned. Device drivers can use this value to further allocate and
+initialize device resources.
+
+4.2.6 pci_disable_msi

void pci_disable_msi(struct pci_dev *dev)

@@ -143,7 +198,7 @@ on any interrupt for which it previously called request_irq().
Failure to do so results in a BUG_ON(), leaving the device with
MSI enabled and thus leaking its vector.

-4.2.4 pci_get_msi_cap
+4.2.7 pci_get_msi_cap

int pci_get_msi_cap(struct pci_dev *dev)

@@ -224,7 +279,76 @@ static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec)
return -ENOSPC;
}

-4.3.2 pci_disable_msix
+4.3.2 pcim_enable_msix_range
+
+int pcim_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+ unsigned int nvec, unsigned int minvec)
+
+This variation on pci_enable_msix() call allows a device driver to request
+any number of MSI-Xs within specified range minvec to nvec. Whenever possible
+device drivers are encouraged to use this function rather than explicit
+request loop calling pci_enable_msix().
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to allocate any more MSI-X interrupts for
+this device.
+
+If this function returns a positive number it indicates the number of
+MSI-X interrupts that have been successfully allocated. Device drivers
+can use this number to further allocate and initialize device resources.
+
+A modified function calling pci_enable_msix() in a loop might look like:
+
+static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec)
+{
+ rc = pcim_enable_msix_range(adapter->pdev, adapter->msix_entries,
+ nvec, FOO_DRIVER_MINIMUM_NVEC);
+ if (rc < 0)
+ return rc;
+
+ rc = foo_driver_init_other(adapter, rc);
+ if (rc < 0)
+ pci_disable_msix(adapter->pdev);
+
+ return rc;
+}
+
+4.3.3 pcim_enable_msix
+
+int pcim_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int maxvec)
+
+This variation on pci_enable_msix() call allows a device driver to request
+any number of MSI-Xs up to maxvec. Whenever possible device drivers are
+encouraged to use this function rather than explicit request loop calling
+pci_enable_msix().
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to allocate any more MSI-X interrupts for
+this device.
+
+If this function returns a positive number it indicates the number of
+MSI-X interrupts that have been successfully allocated. Device drivers
+can use this number to further allocate and initialize device resources.
+
+4.3.4 pcim_enable_msix_exact
+
+int pcim_enable_msix_exact(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)
+
+This variation on pci_enable_msix() call allows a device driver to request
+exactly nvec MSI-Xs.
+
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to allocate any more MSI-X interrupts for
+this device.
+
+If this function returns the value of nvec it indicates MSI-X interrupts
+have been successfully allocated. No other value in case of success could
+be returned. Device drivers can use this value to further allocate and
+initialize device resources.
+
+4.3.5 pci_disable_msix

void pci_disable_msix(struct pci_dev *dev)

@@ -238,14 +362,14 @@ on any interrupt for which it previously called request_irq().
Failure to do so results in a BUG_ON(), leaving the device with
MSI-X enabled and thus leaking its vector.

-4.3.3 The MSI-X Table
+4.3.6 The MSI-X Table

The MSI-X capability specifies a BAR and offset within that BAR for the
MSI-X Table. This address is mapped by the PCI subsystem, and should not
be accessed directly by the device driver. If the driver wishes to
mask or unmask an interrupt, it should call disable_irq() / enable_irq().

-4.3.4 pci_msix_table_size
+4.3.7 pci_msix_table_size

int pci_msix_table_size(struct pci_dev *dev)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 96f51d0..91acd8a 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1042,3 +1042,49 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
if (dev->msix_cap)
msix_set_enable(dev, 0);
}
+
+int pcim_enable_msi_range(struct pci_dev *dev,
+ unsigned int nvec, unsigned int minvec)
+{
+ int rc;
+
+ if (nvec < minvec)
+ return -ERANGE;
+
+ do {
+ rc = pci_enable_msi_block(dev, nvec);
+ if (rc < 0) {
+ return rc;
+ } else if (rc > 0) {
+ if (rc < minvec)
+ return -ENOSPC;
+ nvec = rc;
+ }
+ } while (rc);
+
+ return nvec;
+}
+EXPORT_SYMBOL(pcim_enable_msi_range);
+
+int pcim_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+ unsigned int nvec, unsigned int minvec)
+{
+ int rc;
+
+ if (nvec < minvec)
+ return -ERANGE;
+
+ do {
+ rc = pci_enable_msix(dev, entries, nvec);
+ if (rc < 0) {
+ return rc;
+ } else if (rc > 0) {
+ if (rc < minvec)
+ return -ENOSPC;
+ nvec = rc;
+ }
+ } while (rc);
+
+ return nvec;
+}
+EXPORT_SYMBOL(pcim_enable_msix_range);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index bef5775..3c18a8f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1185,6 +1185,39 @@ static inline int pci_msi_enabled(void)
{
return 0;
}
+
+int pcim_enable_msi_range(struct pci_dev *dev,
+ unsigned int nvec, unsigned int minvec)
+{
+ return -ENOSYS;
+}
+static inline int pcim_enable_msi(struct pci_dev *dev, unsigned int maxvec)
+{
+ return -ENOSYS;
+}
+static inline int pcim_enable_msi_exact(struct pci_dev *dev, unsigned int nvec)
+{
+ return -ENOSYS;
+}
+
+static inline int
+pcim_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+ unsigned int nvec, unsigned int maxvec)
+{
+ return -ENOSYS;
+}
+static inline int
+pcim_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int maxvec)
+{
+ return -ENOSYS;
+}
+static inline int
+pcim_enable_msix_exact(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)
+{
+ return -ENOSYS;
+}
#else
int pci_get_msi_cap(struct pci_dev *dev);
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
@@ -1198,6 +1231,32 @@ void pci_disable_msix(struct pci_dev *dev);
void msi_remove_pci_irq_vectors(struct pci_dev *dev);
void pci_restore_msi_state(struct pci_dev *dev);
int pci_msi_enabled(void);
+
+int pcim_enable_msi_range(struct pci_dev *dev,
+ unsigned int nvec, unsigned int minvec);
+static inline int pcim_enable_msi(struct pci_dev *dev, unsigned int maxvec)
+{
+ return pcim_enable_msi_range(dev, maxvec, 1);
+}
+static inline int pcim_enable_msi_exact(struct pci_dev *dev, unsigned int nvec)
+{
+ return pcim_enable_msi_range(dev, nvec, nvec);
+}
+
+int pcim_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+ unsigned int nvec, unsigned int minvec);
+static inline int
+pcim_enable_msix(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int maxvec)
+{
+ return pcim_enable_msix_range(dev, entries, maxvec, 1);
+}
+static inline int
+pcim_enable_msix_exact(struct pci_dev *dev,
+ struct msix_entry *entries, unsigned int nvec)
+{
+ return pcim_enable_msix_range(dev, entries, nvec, nvec);
+}
#endif

#ifdef CONFIG_PCIEPORTBUS
--
1.7.7.6

2013-10-21 09:00:42

by David Laight

[permalink] [raw]
Subject: RE: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

> Subject: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface
...
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index d33802c..e552d2b 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -2735,39 +2735,19 @@ vmxnet3_read_mac_addr(struct vmxnet3_adapter *adapter, u8 *mac)
> */
>
> static int
> -vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
> - int vectors)
> +vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter, int vectors)
> {
> - int err = -EINVAL, vector_threshold;
> - vector_threshold = VMXNET3_LINUX_MIN_MSIX_VECT;
> -
> - while (vectors >= vector_threshold) {
> - err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
> - vectors);
> - if (!err) {
> - adapter->intr.num_intrs = vectors;
> - return 0;
> - } else if (err < 0) {
> - dev_err(&adapter->netdev->dev,
> - "Failed to enable MSI-X, error: %d\n", err);
> - return err;
> - } else if (err < vector_threshold) {
> - dev_info(&adapter->pdev->dev,
> - "Number of MSI-Xs which can be allocated "
> - "is lower than min threshold required.\n");
> - return -ENOSPC;
> - } else {
> - /* If fails to enable required number of MSI-x vectors
> - * try enabling minimum number of vectors required.
> - */
> - dev_err(&adapter->netdev->dev,
> - "Failed to enable %d MSI-X, trying %d instead\n",
> - vectors, vector_threshold);
> - vectors = vector_threshold;
> - }
> + vectors = pcim_enable_msix_range(adapter->pdev,
> + adapter->intr.msix_entries, vectors,
> + VMXNET3_LINUX_MIN_MSIX_VECT);
> + if (vectors < 0) {
> + dev_err(&adapter->netdev->dev,
> + "Failed to enable MSI-X, error: %d\n", vectors);
> + return vectors;
> }
>
> - return err;
> + adapter->intr.num_intrs = vectors;
> + return 0;
> }

AFAICT the old code either used the requested number or the minimum number.
The new code seems to claim an intermediate number of interrupts - but probably
only uses the minimum number.
This wastes the last few MSI-X interrupts.
The code (especially the calling code) would be easier to read if the 'vectors'
value wasn't explicitly passed.

David


2013-10-21 10:18:12

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

On Mon, Oct 21, 2013 at 09:51:32AM +0100, David Laight wrote:
> > Subject: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface
> AFAICT the old code either used the requested number or the minimum number.
> The new code seems to claim an intermediate number of interrupts - but probably
> only uses the minimum number.

Yes, you are right. I missed the vectors = vector_threshold assignment.
This driver is inapplicable as an example.


--
Regards,
Alexander Gordeev
[email protected]

2013-10-24 10:52:07

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

Hello, Alexander.

On Fri, Oct 18, 2013 at 07:12:12PM +0200, Alexander Gordeev wrote:
> So i.e. the request loop described in the documentation...
>
> int foo_driver_enable_msix(struct foo_adapter *adapter,
> int nvec)
> {
> while (nvec >= FOO_DRIVER_MINIMUM_NVEC) {
> rc = pci_enable_msix(adapter->pdev,
> adapter->msix_entries,
> nvec);
> if (rc > 0)
> nvec = rc;
> else
> return rc;
> }
>
> return -ENOSPC;
> }
>
> ...would turn into a single helper call....
>
> rc = pcim_enable_msix_range(adapter->pdev,
> adapter->msix_entries,
> nvec,
> FOO_DRIVER_MINIMUM_NVEC);

I haven't looked into any details but, if the above works for most use
cases, it looks really good to me.

Thanks!

--
tejun

2013-10-24 10:59:44

by David Laight

[permalink] [raw]
Subject: RE: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

> > ...would turn into a single helper call....
> >
> > rc = pcim_enable_msix_range(adapter->pdev,
> > adapter->msix_entries,
> > nvec,
> > FOO_DRIVER_MINIMUM_NVEC);
>
> I haven't looked into any details but, if the above works for most use
> cases, it looks really good to me.

The one case it doesn't work is where the driver either
wants the full number or the minimum number - but not
a value in between.

Might be worth adding an extra parameter so that this
(and maybe other) odd requirements can be met.

Some static inline functions could be used for the common cases.

David


2013-10-24 11:01:48

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

Hello,

On Thu, Oct 24, 2013 at 11:57:40AM +0100, David Laight wrote:
> > > ...would turn into a single helper call....
> > >
> > > rc = pcim_enable_msix_range(adapter->pdev,
> > > adapter->msix_entries,
> > > nvec,
> > > FOO_DRIVER_MINIMUM_NVEC);
> >
> > I haven't looked into any details but, if the above works for most use
> > cases, it looks really good to me.
>
> The one case it doesn't work is where the driver either
> wants the full number or the minimum number - but not
> a value in between.
>
> Might be worth adding an extra parameter so that this
> (and maybe other) odd requirements can be met.
>
> Some static inline functions could be used for the common cases.

If those are edge cases, I don't think it's a big deal no matter what
we do.

Thanks.

--
tejun

2013-10-24 11:39:44

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On Thu, Oct 24, 2013 at 11:57:40AM +0100, David Laight wrote:
> The one case it doesn't work is where the driver either
> wants the full number or the minimum number - but not
> a value in between.
>
> Might be worth adding an extra parameter so that this
> (and maybe other) odd requirements can be met.

IMHO its not worth it, since it is not possible to generalize
all odd requirements out there. I do not think we should blow
the API in this case.

Having said that, the min-or-max interface is probably the only
worth considering. But again, I would prefer to put its semantics
to function name rather than to extra parameters, i.e.

pcim_enable_msix_min_max(struct pci_dev *dev, struct msix_entry *entries,
unsigned int minvec, unsigned int maxvec);

> Some static inline functions could be used for the common cases.
>
> David

--
Regards,
Alexander Gordeev
[email protected]

2013-10-24 14:29:25

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On Thu, Oct 24, 2013 at 11:51:58AM +0100, Tejun Heo wrote:
> I haven't looked into any details but, if the above works for most use
> cases, it looks really good to me.

Well, if we reuse Michael's statistics:

- 58 drivers call pci_enable_msix()
- 24 try a single allocation and then fallback to MSI/LSI
- 19 use the loop style allocation
- 14 try an allocation, and if it fails retry once

...then I expect most of 19/58 (loop style) could be converted to
pcim_enable_msix() and pcim_enable_msix_range() and all of 14/58
(single fallback) should be converted to pcim_enable_msix() users.

> tejun

--
Regards,
Alexander Gordeev
[email protected]

2013-10-25 02:22:35

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On 13-10-24 07:41 AM, Alexander Gordeev wrote:
> On Thu, Oct 24, 2013 at 11:57:40AM +0100, David Laight wrote:
>> The one case it doesn't work is where the driver either
>> wants the full number or the minimum number - but not
>> a value in between.
>>
>> Might be worth adding an extra parameter so that this
>> (and maybe other) odd requirements can be met.
>
> IMHO its not worth it, since it is not possible to generalize
> all odd requirements out there. I do not think we should blow
> the API in this case.
>
> Having said that, the min-or-max interface is probably the only
> worth considering. But again, I would prefer to put its semantics
> to function name rather than to extra parameters, i.e.
>
> pcim_enable_msix_min_max(struct pci_dev *dev, struct msix_entry *entries,
> unsigned int minvec, unsigned int maxvec);

The hardware I have in mind here works only for powers of two.
Eg. 16, 8, 4, 2, or 1 MSI-X vector. Not the odd values in between.

But it appears I can still just use a loop for that case,
calling the new function above instead of the old functions.

Cheers

2013-10-25 02:23:35

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On 13-10-24 10:31 AM, Alexander Gordeev wrote:
> On Thu, Oct 24, 2013 at 11:51:58AM +0100, Tejun Heo wrote:
>> I haven't looked into any details but, if the above works for most use
>> cases, it looks really good to me.
>
> Well, if we reuse Michael's statistics:
>
> - 58 drivers call pci_enable_msix()
> - 24 try a single allocation and then fallback to MSI/LSI
> - 19 use the loop style allocation
> - 14 try an allocation, and if it fails retry once
>
> ...then I expect most of 19/58 (loop style) could be converted to
> pcim_enable_msix() and pcim_enable_msix_range() and all of 14/58
> (single fallback) should be converted to pcim_enable_msix() users.

Those are just the in-kernel drivers.
There are many, many more out-of-kernel drivers for embedded platforms,
hardware in-development, etc..

Let's not be overly presumptive about the size of the user base.

Cheers

2013-10-25 09:12:12

by David Laight

[permalink] [raw]
Subject: RE: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

> > pcim_enable_msix_min_max(struct pci_dev *dev, struct msix_entry *entries,
> > unsigned int minvec, unsigned int maxvec);
>
> The hardware I have in mind here works only for powers of two.
> Eg. 16, 8, 4, 2, or 1 MSI-X vector. Not the odd values in between.
>
> But it appears I can still just use a loop for that case,
> calling the new function above instead of the old functions.

You'd either have to loop with min and max the same (starting at 16),
or do a single call with min=1 and max=16 and the release the
unwanted ones.

The latter might be preferred because it might stop an annoying
trace about the system not having enough MSI interrupts.

What this doesn't resolve is a driver requesting a lot of interrupts
early on and leaving none for later drivers.

Really the system needs to allocate the minimum number to all
drivers before giving out any extra ones - I've NFI how this
would be arranged!

David

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-10-25 09:59:15

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On Fri, Oct 25, 2013 at 10:10:02AM +0100, David Laight wrote:
> What this doesn't resolve is a driver requesting a lot of interrupts
> early on and leaving none for later drivers.

If this problem really exists anywhere besides pSeries?

I can imagine x86 hitting lack of vectors in interrupt table when
number of CPUs exceeds hundreds, but do we have this problem now?

> Really the system needs to allocate the minimum number to all
> drivers before giving out any extra ones - I've NFI how this
> would be arranged!

Do not know. The pSeries quota approach seems more reasonable to me.

> David

--
Regards,
Alexander Gordeev
[email protected]

2013-10-27 22:27:58

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On Fri, 2013-10-25 at 12:01 +0200, Alexander Gordeev wrote:
> On Fri, Oct 25, 2013 at 10:10:02AM +0100, David Laight wrote:
> > What this doesn't resolve is a driver requesting a lot of interrupts
> > early on and leaving none for later drivers.
>
> If this problem really exists anywhere besides pSeries?
>
> I can imagine x86 hitting lack of vectors in interrupt table when
> number of CPUs exceeds hundreds, but do we have this problem now?
>
> > Really the system needs to allocate the minimum number to all
> > drivers before giving out any extra ones - I've NFI how this
> > would be arranged!
>
> Do not know. The pSeries quota approach seems more reasonable to me.

When the system boots each driver should get a fair share of the
available MSIs, the quota achieves this.

But ideally the sysadmin would then be able to override that, and give
more MSIs to one device, the quota doesn't allow that.

Hopefully we'll see the number of available MSIs grow faster than the
number required by devices (usually driven by NR_CPUs), and so this will
become a non-problem.

cheers

2013-10-28 16:31:11

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH RFC v2 12/29] PCI/MSI: Introduce pcim_enable_msi*() family helpers

On 13-10-25 06:01 AM, Alexander Gordeev wrote:
>
> If this problem really exists anywhere besides pSeries?
>
> I can imagine x86 hitting lack of vectors in interrupt table when
> number of CPUs exceeds hundreds, but do we have this problem now?

An awful lot of x86 hardware has a 256 (255?) vector limit for MSI/MSI-X.
Couple that with PCIe Virtual Functions, each wanting 16 vectors (for example),
and that limit is really simple to exceed today.

But this is more a problem for a sysadmin, and I am happy with the current
and the proposed methods.

Cheers