MSI-X vectors can be allocated after the initial MSI-X enablement, but this
needs explicit support of the underlying interrupt domains.
Provide a function to query the ability and functions to allocate/free
individual vectors post-enable.
The allocation can either request a specific index in the MSI-X table or
with the index argument MSI_ANY_INDEX it allocates the next free vector.
The return value is a struct msi_map which on success contains both index
and the Linux interrupt number. In case of failure index is negative and
the Linux interrupt number is 0.
The allocation function is for a single MSI-X index at a time as that's
sufficient for the most urgent use case VFIO to get rid of the 'disable
MSI-X, reallocate, enable-MSI-X' cycle which is prone to lost interrupts
and redirections to the legacy and obviously unhandled INTx.
As single index allocation is also sufficient for the use cases Jason
Gunthorpe pointed out: Allocation of a MSI-X or IMS vector for a network
queue. See Link below.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Link: https://lore.kernel.org/all/[email protected]
---
V2: Added Link to previous discussions (Bjorn)
---
drivers/pci/msi/api.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/msi/irqdomain.c | 3 +
include/linux/pci.h | 6 +++
3 files changed, 75 insertions(+), 1 deletion(-)
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -113,6 +113,73 @@ int pci_enable_msix_range(struct pci_dev
EXPORT_SYMBOL(pci_enable_msix_range);
/**
+ * pci_msix_can_alloc_dyn - Query whether dynamic allocation after enabling
+ * MSI-X is supported
+ *
+ * @dev: PCI device to operate on
+ *
+ * Return: True if supported, false otherwise
+ */
+bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
+{
+ if (!dev->msix_cap)
+ return false;
+
+ return pci_msi_domain_supports(dev, MSI_FLAG_PCI_MSIX_ALLOC_DYN, DENY_LEGACY);
+}
+EXPORT_SYMBOL_GPL(pci_msix_can_alloc_dyn);
+
+/**
+ * pci_msix_alloc_irq_at - Allocate an MSI-X interrupt after enabling MSI-X
+ * at a given MSI-X vector index or any free vector index
+ *
+ * @dev: PCI device to operate on
+ * @index: Index to allocate. If @index == MSI_ANY_INDEX this allocates
+ * the next free index in the MSI-X table
+ * @affdesc: Optional pointer to an affinity descriptor structure. NULL otherwise
+ *
+ * Return: A struct msi_map
+ *
+ * On success msi_map::index contains the allocated index (>= 0) and
+ * msi_map::virq the allocated Linux interrupt number (> 0).
+ *
+ * On fail msi_map::index contains the error code and msi_map::virq
+ * is set to 0.
+ */
+struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
+ const struct irq_affinity_desc *affdesc)
+{
+ struct msi_map map = { .index = -ENOTSUPP };
+
+ if (!dev->msix_enabled)
+ return map;
+
+ if (!pci_msix_can_alloc_dyn(dev))
+ return map;
+
+ return msi_domain_alloc_irq_at(&dev->dev, MSI_DEFAULT_DOMAIN, index, affdesc, NULL);
+}
+EXPORT_SYMBOL_GPL(pci_msix_alloc_irq_at);
+
+/**
+ * pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain
+ * which was allocated via pci_msix_alloc_irq_at()
+ *
+ * @dev: The PCI device to operate on
+ * @map: A struct msi_map describing the interrupt to free
+ * as returned from the allocation function.
+ */
+void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map)
+{
+ if (WARN_ON_ONCE(map.index < 0 || map.virq <= 0))
+ return;
+ if (WARN_ON_ONCE(!pci_msix_can_alloc_dyn(dev)))
+ return;
+ msi_domain_free_irqs_range(&dev->dev, MSI_DEFAULT_DOMAIN, map.index, map.index);
+}
+EXPORT_SYMBOL_GPL(pci_msix_free_irq);
+
+/**
* pci_disable_msix() - Disable MSI-X interrupt mode on device
* @dev: the PCI device to operate on
*
--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -225,7 +225,8 @@ static struct msi_domain_template pci_ms
},
.info = {
- .flags = MSI_COMMON_FLAGS | MSI_FLAG_PCI_MSIX,
+ .flags = MSI_COMMON_FLAGS | MSI_FLAG_PCI_MSIX |
+ MSI_FLAG_PCI_MSIX_ALLOC_DYN,
.bus_token = DOMAIN_BUS_PCI_DEVICE_MSIX,
},
};
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -38,6 +38,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/resource_ext.h>
+#include <linux/msi_api.h>
#include <uapi/linux/pci.h>
#include <linux/pci_ids.h>
@@ -1559,6 +1560,11 @@ int pci_alloc_irq_vectors_affinity(struc
unsigned int max_vecs, unsigned int flags,
struct irq_affinity *affd);
+bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
+struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
+ const struct irq_affinity_desc *affdesc);
+void pci_msix_free_irq(struct pci_dev *pdev, struct msi_map map);
+
void pci_free_irq_vectors(struct pci_dev *dev);
int pci_irq_vector(struct pci_dev *dev, unsigned int nr);
const struct cpumask *pci_irq_get_affinity(struct pci_dev *pdev, int vec);
> From: Thomas Gleixner <[email protected]>
> Sent: Monday, November 21, 2022 10:38 PM
>
> +/**
> + * pci_msix_alloc_irq_at - Allocate an MSI-X interrupt after enabling MSI-X
> + * at a given MSI-X vector index or any free vector
> index
> + *
> + * @dev: PCI device to operate on
> + * @index: Index to allocate. If @index == MSI_ANY_INDEX this allocates
> + * the next free index in the MSI-X table
%MSI_ANY_INDEX as done in other places.
> +
> +/**
> + * pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain
> + * which was allocated via pci_msix_alloc_irq_at()
> + *
> + * @dev: The PCI device to operate on
> + * @map: A struct msi_map describing the interrupt to free
> + * as returned from the allocation function.
> + */
Can this be used on preallocated descriptors? If not any guard required
or just treat it as a driver responsibility to not misuse?
> +void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map)
> +{
> + if (WARN_ON_ONCE(map.index < 0 || map.virq <= 0))
map.virq cannot be negative.
On Thu, Nov 24 2022 at 02:58, Kevin Tian wrote:
>> +
>> +/**
>> + * pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain
>> + * which was allocated via pci_msix_alloc_irq_at()
>> + *
>> + * @dev: The PCI device to operate on
>> + * @map: A struct msi_map describing the interrupt to free
>> + * as returned from the allocation function.
>> + */
>
> Can this be used on preallocated descriptors? If not any guard required
> or just treat it as a driver responsibility to not misuse?
The preallocated descriptors are only relevant during setup, but not
post setup. So yes, you can do:
pci_alloc_irq_vectors(dev, 10, 10, PCI_IRQ_MSIX);
and then
map = { .index = 9, .virq = $IRQ };
pci_msix_free_irq(dev, &map);
It just works.
>> +void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map)
>> +{
>> + if (WARN_ON_ONCE(map.index < 0 || map.virq <= 0))
>
> map.virq cannot be negative.
Why? Callers can make up whatever they want, no?
Thanks,
tglx
> From: Thomas Gleixner <[email protected]>
> Sent: Thursday, November 24, 2022 5:09 PM
> >> +void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map)
> >> +{
> >> + if (WARN_ON_ONCE(map.index < 0 || map.virq <= 0))
> >
> > map.virq cannot be negative.
>
> Why? Callers can make up whatever they want, no?
>
obviously callers can. Read too fast...
Then following check should be fixed too:
+void pci_ims_free_irq(struct pci_dev *dev, struct msi_map map)
+{
+ if (WARN_ON_ONCE(map.index < 0 || !map.virq))
+ return;
+ msi_domain_free_irqs_range(&dev->dev, MSI_SECONDARY_DOMAIN, map.index, map.index);
+}
The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: e6cc6f175566dd21a3f6e384c24593b1c751dd74
Gitweb: https://git.kernel.org/tip/e6cc6f175566dd21a3f6e384c24593b1c751dd74
Author: Reinette Chatre <[email protected]>
AuthorDate: Tue, 14 Feb 2023 13:13:20 -08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 21 Feb 2023 08:25:14 +01:00
PCI/MSI: Clarify usage of pci_msix_free_irq()
pci_msix_free_irq() is used to free an interrupt on a PCI/MSI-X interrupt
domain.
The API description specifies that the interrupt to be freed was allocated
via pci_msix_alloc_irq_at(). This description limits the usage of
pci_msix_free_irq() since pci_msix_free_irq() can also be used to free
MSI-X interrupts allocated with, for example, pci_alloc_irq_vectors().
Remove the text stating that the interrupt to be freed had to be allocated
with pci_msix_alloc_irq_at(). The needed struct msi_map need not be from
pci_msix_alloc_irq_at() but can be created from scratch using
pci_irq_vector() to obtain the Linux IRQ number. Highlight that
pci_msix_free_irq() cannot be used to disable MSI-X to guide users that,
for example, pci_free_irq_vectors() remains to be needed.
Signed-off-by: Reinette Chatre <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
Link: https://lore.kernel.org/lkml/87r0xsd8j4.ffs@tglx
Link: https://lore.kernel.org/r/4c3e7a50d6e70f408812cd7ab199c6b4b326f9de.1676408572.git.reinette.chatre@intel.com
---
drivers/pci/msi/api.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
index b8009aa..be679aa 100644
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -163,11 +163,11 @@ EXPORT_SYMBOL_GPL(pci_msix_alloc_irq_at);
/**
* pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain
- * which was allocated via pci_msix_alloc_irq_at()
*
* @dev: The PCI device to operate on
* @map: A struct msi_map describing the interrupt to free
- * as returned from the allocation function.
+ *
+ * Undo an interrupt vector allocation. Does not disable MSI-X.
*/
void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map)
{