From b3bac8e57c82e8d3e05f4abcb18c4f0a40656655 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 23 Jan 2014 11:30:37 -0800 Subject: [PATCH 1/9] PCI/MSI: Check kmalloc() return value, fix leak of name Coverity reported that I forgot to check the return value of kmalloc() when creating the MSI attribute name, so fix that up and properly free it if there is an error when allocating the msi_dev_attr variable. Found by Coverity (CID 1163315 and 1163316). Fixes: 1c51b50c2995 ("PCI/MSI: Export MSI mode using attributes, not kobjects") Signed-off-by: Greg Kroah-Hartman Signed-off-by: Bjorn Helgaas --- drivers/pci/msi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 7a0fec6ce571..39dff3fe57af 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -545,9 +545,15 @@ static int populate_msi_sysfs(struct pci_dev *pdev) return -ENOMEM; list_for_each_entry(entry, &pdev->msi_list, list) { char *name = kmalloc(20, GFP_KERNEL); + if (!name) + goto error_attrs; + msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL); - if (!msi_dev_attr) + if (!msi_dev_attr) { + kfree(name); goto error_attrs; + } + sprintf(name, "%d", entry->irq); sysfs_attr_init(&msi_dev_attr->attr); msi_dev_attr->attr.name = name; From 13f81c099bee8141f764fd41a1f4b68b93be3296 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 23 Jan 2014 12:01:12 -0800 Subject: [PATCH 2/9] PCI/MSI: Fix leak of msi_attrs Coverity reported that I forgot to clean up some allocated memory on the error path in populate_msi_sysfs(), so this patch fixes that. Thanks to Dave Jones for pointing out where the error was, I obviously can't read code this morning... Found by Coverity (CID 1163317). Fixes: 1c51b50c2995 ("PCI/MSI: Export MSI mode using attributes, not kobjects") Signed-off-by: Greg Kroah-Hartman Signed-off-by: Bjorn Helgaas Cc: Dave Jones --- drivers/pci/msi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 39dff3fe57af..6f0474ebe420 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -595,6 +595,7 @@ static int populate_msi_sysfs(struct pci_dev *pdev) ++count; msi_attr = msi_attrs[count]; } + kfree(msi_attrs); return ret; } From 39af663b5fd25c9dd427e12e106763b6b4f9572c Mon Sep 17 00:00:00 2001 From: Masanari Iida Date: Tue, 21 Jan 2014 00:42:55 +0900 Subject: [PATCH 3/9] PCI/MSI: Fix pci_msix_vec_count() htmldocs failure An empty line in msi.c caused "make htmldocs" failure: Warning(/home/iida/Repo/linux-next//drivers/pci/msi.c:962): bad line: Fixes: ff1aa430a2fa ("PCI/MSI: Add pci_msix_vec_count()") Signed-off-by: Masanari Iida Signed-off-by: Bjorn Helgaas --- drivers/pci/msi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 6f0474ebe420..955ab7990c5b 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -966,7 +966,6 @@ EXPORT_SYMBOL(pci_disable_msi); /** * pci_msix_vec_count - return the number of device's MSI-X table entries * @dev: pointer to the pci_dev data structure of MSI-X device function - * This function returns the number of device's MSI-X table entries and * therefore the number of MSI-X vectors device is capable of sending. * It returns a negative errno if the device is not capable of sending MSI-X From 9a8b0af9b1dacf1d865d4b8bff7360e5b318481a Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 17 Jan 2014 10:26:20 +0100 Subject: [PATCH 4/9] PCI/MSI: Add pci_enable_msi() documentation back We deprecated pci_enable_msi() in 302a2523c277 ("PCI/MSI: Add pci_enable_msi_range() and pci_enable_msix_range()"). But we changed our minds after noticing that: - pci_enable_msi() doesn't have confusing return values like pci_enable_msi_block() and pci_enable_msix() did, and - pci_enable_msi() has a hundred or so callers that we don't want to change. This adds back the pci_enable_msi() documentation. [bhelgaas: changelog] Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas --- Documentation/PCI/MSI-HOWTO.txt | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt index a8d01005f480..3c3080e9afef 100644 --- a/Documentation/PCI/MSI-HOWTO.txt +++ b/Documentation/PCI/MSI-HOWTO.txt @@ -82,7 +82,19 @@ Most of the hard work is done for the driver in the PCI layer. It simply has to request that the PCI layer set up the MSI capability for this device. -4.2.1 pci_enable_msi_range +4.2.1 pci_enable_msi + +int pci_enable_msi(struct pci_dev *dev) + +A successful call allocates ONE interrupt to the device, regardless +of how many MSIs the device supports. The device is switched from +pin-based interrupt mode to MSI mode. The dev->irq number is changed +to a new number which represents the message signaled interrupt; +consequently, this function should be called before the driver calls +request_irq(), because an MSI is delivered via a vector that is +different from the vector of a pin-based interrupt. + +4.2.2 pci_enable_msi_range int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec) @@ -158,7 +170,12 @@ static int foo_driver_enable_single_msi(struct pci_dev *pdev) return pci_enable_msi_range(pdev, 1, 1); } -4.2.2 pci_disable_msi +Note, unlike pci_enable_msi() function, which could be also used to +enable the single MSI mode, pci_enable_msi_range() returns either a +negative errno or 1 (not negative errno or 0 - as pci_enable_msi() +does). + +4.2.3 pci_disable_msi void pci_disable_msi(struct pci_dev *dev) @@ -172,7 +189,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.3 pci_msi_vec_count +4.2.4 pci_msi_vec_count int pci_msi_vec_count(struct pci_dev *dev) From fef07277f87735d077c78e12e25639a28d9913ab Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Wed, 12 Feb 2014 11:35:25 +0100 Subject: [PATCH 5/9] PCI/MSI: Fix cut-and-paste errors in documentation Function pci_enable_msi_range() is used in examples where pci_enable_msix_range() should have been used instead. Reported-by: Bjorn Helgaas Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas --- Documentation/PCI/MSI-HOWTO.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt index 3c3080e9afef..96ee5eb9d4d4 100644 --- a/Documentation/PCI/MSI-HOWTO.txt +++ b/Documentation/PCI/MSI-HOWTO.txt @@ -274,8 +274,8 @@ possible, likely up to the limit returned by pci_msix_vec_count() function: static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec) { - return pci_enable_msi_range(adapter->pdev, adapter->msix_entries, - 1, nvec); + return pci_enable_msix_range(adapter->pdev, adapter->msix_entries, + 1, nvec); } Note the value of 'minvec' parameter is 1. As 'minvec' is inclusive, @@ -286,8 +286,8 @@ In this case the function could look like this: static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec) { - return pci_enable_msi_range(adapter->pdev, adapter->msix_entries, - FOO_DRIVER_MINIMUM_NVEC, nvec); + return pci_enable_msix_range(adapter->pdev, adapter->msix_entries, + FOO_DRIVER_MINIMUM_NVEC, nvec); } 4.3.1.2 Exact number of MSI-X interrupts @@ -299,8 +299,8 @@ parameters: static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec) { - return pci_enable_msi_range(adapter->pdev, adapter->msix_entries, - nvec, nvec); + return pci_enable_msix_range(adapter->pdev, adapter->msix_entries, + nvec, nvec); } 4.3.1.3 Specific requirements to the number of MSI-X interrupts From f7fc32cb2073902b3ee67d036656889ea75ffcb7 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Thu, 13 Feb 2014 15:25:02 +0100 Subject: [PATCH 6/9] PCI/MSI: Add pci_enable_msi_exact() and pci_enable_msix_exact() The new functions are special cases for pci_enable_msi_range() and pci_enable_msix_range() when a particular number of MSI or MSI-X is needed. By contrast with pci_enable_msi_range() and pci_enable_msix_range() functions, pci_enable_msi_exact() and pci_enable_msix_exact() return zero in case of success, which indicates MSI or MSI-X interrupts have been successfully allocated. Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas --- Documentation/PCI/MSI-HOWTO.txt | 86 ++++++++++++++++++++++++++++++++- include/linux/pci.h | 20 ++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt index 96ee5eb9d4d4..10a93696e55a 100644 --- a/Documentation/PCI/MSI-HOWTO.txt +++ b/Documentation/PCI/MSI-HOWTO.txt @@ -159,6 +159,11 @@ static int foo_driver_enable_msi(struct pci_dev *pdev, int nvec) return pci_enable_msi_range(pdev, nvec, nvec); } +Note, unlike pci_enable_msi_exact() function, which could be also used to +enable a particular number of MSI-X interrupts, pci_enable_msi_range() +returns either a negative errno or 'nvec' (not negative errno or 0 - as +pci_enable_msi_exact() does). + 4.2.1.3 Single MSI mode The most notorious example of the request type described above is @@ -175,7 +180,22 @@ enable the single MSI mode, pci_enable_msi_range() returns either a negative errno or 1 (not negative errno or 0 - as pci_enable_msi() does). -4.2.3 pci_disable_msi +4.2.3 pci_enable_msi_exact + +int pci_enable_msi_exact(struct pci_dev *dev, int nvec) + +This variation on pci_enable_msi_range() 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. + +By contrast with pci_enable_msi_range() function, pci_enable_msi_exact() +returns zero in case of success, which indicates MSI interrupts have been +successfully allocated. + +4.2.4 pci_disable_msi void pci_disable_msi(struct pci_dev *dev) @@ -303,6 +323,11 @@ static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec) nvec, nvec); } +Note, unlike pci_enable_msix_exact() function, which could be also used to +enable a particular number of MSI-X interrupts, pci_enable_msix_range() +returns either a negative errno or 'nvec' (not negative errno or 0 - as +pci_enable_msix_exact() does). + 4.3.1.3 Specific requirements to the number of MSI-X interrupts As noted above, there could be devices that can not operate with just any @@ -349,7 +374,64 @@ Note how pci_enable_msix_range() return value is analized for a fallback - any error code other than -ENOSPC indicates a fatal error and should not be retried. -4.3.2 pci_disable_msix +4.3.2 pci_enable_msix_exact + +int pci_enable_msix_exact(struct pci_dev *dev, + struct msix_entry *entries, int nvec) + +This variation on pci_enable_msix_range() 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. + +By contrast with pci_enable_msix_range() function, pci_enable_msix_exact() +returns zero in case of success, which indicates MSI-X interrupts have been +successfully allocated. + +Another version of a routine that enables MSI-X mode for a device with +specific requirements described in chapter 4.3.1.3 might look like this: + +/* + * Assume 'minvec' and 'maxvec' are non-zero + */ +static int foo_driver_enable_msix(struct foo_adapter *adapter, + int minvec, int maxvec) +{ + int rc; + + minvec = roundup_pow_of_two(minvec); + maxvec = rounddown_pow_of_two(maxvec); + + if (minvec > maxvec) + return -ERANGE; + +retry: + rc = pci_enable_msix_exact(adapter->pdev, + adapter->msix_entries, maxvec); + + /* + * -ENOSPC is the only error code allowed to be analyzed + */ + if (rc == -ENOSPC) { + if (maxvec == 1) + return -ENOSPC; + + maxvec /= 2; + + if (minvec > maxvec) + return -ENOSPC; + + goto retry; + } else if (rc < 0) { + return rc; + } + + return maxvec; +} + +4.3.3 pci_disable_msix void pci_disable_msix(struct pci_dev *dev) diff --git a/include/linux/pci.h b/include/linux/pci.h index fb57c892b214..33aa2caf0f0c 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1169,8 +1169,23 @@ 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 pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec); +static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec) +{ + int rc = pci_enable_msi_range(dev, nvec, nvec); + if (rc < 0) + return rc; + return 0; +} int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec, int maxvec); +static inline int pci_enable_msix_exact(struct pci_dev *dev, + struct msix_entry *entries, int nvec) +{ + int rc = pci_enable_msix_range(dev, entries, nvec, nvec); + if (rc < 0) + return rc; + return 0; +} #else static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; } static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec) @@ -1189,9 +1204,14 @@ static inline int pci_msi_enabled(void) { return 0; } static inline int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec) { return -ENOSYS; } +static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec) +{ return -ENOSYS; } static inline int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec, int maxvec) { return -ENOSYS; } +static inline int pci_enable_msix_exact(struct pci_dev *dev, + struct msix_entry *entries, int nvec) +{ return -ENOSYS; } #endif #ifdef CONFIG_PCIEPORTBUS From bc03fd3a8a95a13ecce413790f75f6e1a0f54d27 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 17 Jan 2014 17:02:15 +0100 Subject: [PATCH 7/9] ahci: Fix broken fallback to single MSI mode Commit 7b92b4f61ec4 ("PCI/MSI: Remove pci_enable_msi_block_auto()") introduced a regression: if multiple MSI initialization fails, the code falls back to INTx rather than to single MSI. Fixes: 7b92b4f61ec4 ("PCI/MSI: Remove pci_enable_msi_block_auto()") Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas Acked-by: Tejun Heo --- drivers/ata/ahci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index dc2756fb6f33..3c5f35ef7e55 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1170,8 +1170,10 @@ static int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports, nvec = rc; rc = pci_enable_msi_block(pdev, nvec); - if (rc) + if (rc < 0) goto intx; + else if (rc > 0) + goto single_msi; return nvec; From fc061d969f9a44bcf200c557a77fe5e5af8ab363 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Wed, 29 Jan 2014 14:19:43 -0700 Subject: [PATCH 8/9] ahci: Use pci_enable_msi_range() instead of pci_enable_msi_block() pci_enable_msi_block() has been deprecated; use pci_enable_msi_range() instead. [bhelgaas: changelog] Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas Acked-by: Tejun Heo --- drivers/ata/ahci.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 3c5f35ef7e55..023710905289 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1151,13 +1151,13 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host) static int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports, struct ahci_host_priv *hpriv) { - int rc, nvec; + int nvec; if (hpriv->flags & AHCI_HFLAG_NO_MSI) goto intx; - rc = pci_msi_vec_count(pdev); - if (rc < 0) + nvec = pci_msi_vec_count(pdev); + if (nvec < 0) goto intx; /* @@ -1165,21 +1165,19 @@ static int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports, * 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) + if (nvec < n_ports) goto single_msi; - nvec = rc; - rc = pci_enable_msi_block(pdev, nvec); - if (rc < 0) - goto intx; - else if (rc > 0) + nvec = pci_enable_msi_range(pdev, nvec, nvec); + if (nvec == -ENOSPC) goto single_msi; + else if (nvec < 0) + goto intx; return nvec; single_msi: - rc = pci_enable_msi(pdev); - if (rc) + if (pci_enable_msi(pdev)) goto intx; return 1; From 94cccde648d32abe61e2d0d1b18178f3027f1a78 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Fri, 17 Jan 2014 17:02:21 +0100 Subject: [PATCH 9/9] vfio: Use pci_enable_msi_range() and pci_enable_msix_range() pci_enable_msix() and pci_enable_msi_block() have been deprecated; use pci_enable_msix_range() and pci_enable_msi_range() instead. [bhelgaas: changelog] Signed-off-by: Alexander Gordeev Signed-off-by: Bjorn Helgaas Acked-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_intrs.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 210357691dc0..9dd49c9839ac 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -482,15 +482,19 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix) for (i = 0; i < nvec; i++) vdev->msix[i].entry = i; - ret = pci_enable_msix(pdev, vdev->msix, nvec); - if (ret) { + ret = pci_enable_msix_range(pdev, vdev->msix, 1, nvec); + if (ret < nvec) { + if (ret > 0) + pci_disable_msix(pdev); kfree(vdev->msix); kfree(vdev->ctx); return ret; } } else { - ret = pci_enable_msi_block(pdev, nvec); - if (ret) { + ret = pci_enable_msi_range(pdev, 1, nvec); + if (ret < nvec) { + if (ret > 0) + pci_disable_msi(pdev); kfree(vdev->ctx); return ret; }