Re: [patch 29/37] PCI/MSI: Use __msi_get_virq() in pci_get_vector()
From: Thomas Gleixner <hidden>
Date: 2021-11-28 21:02:36
Also in:
linux-arm-kernel, linux-iommu, linux-pci, lkml
On Sun, Nov 28 2021 at 19:37, Marc Zyngier wrote:
quoted hunk ↗ jump to hunk
On Sat, 27 Nov 2021 01:22:03 +0000, Thomas Gleixner [off-list ref] wrote: I worked around it with the hack below, but I doubt this is the real thing. portdrv_core.c does complicated things, and I don't completely understand its logic. M.diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c index 1f72bc734226..b15278a5fb4b 100644 --- a/drivers/pci/msi/msi.c +++ b/drivers/pci/msi/msi.c@@ -1092,8 +1092,9 @@ int pci_irq_vector(struct pci_dev *dev, unsigned int nr) int irq = __msi_get_virq(&dev->dev, nr); switch (irq) { - case -ENODEV: return !nr ? dev->irq : -EINVAL; - case -ENOENT: return -EINVAL; + case -ENOENT: + case -ENODEV: + return !nr ? dev->irq : -EINVAL;
Hrm. ENODEV is returned when dev->msi.data == NULL, ENOENT when there is
no MSI entry. But yes, that goes south when the device tried to enable
MSI[X} and then ended up with INTx. It still has dev->msi.data, which
causes it to return -ENOENT, which makes the above go belly up.
Moo, what was I thinking?
Thanks,
tglx
------ a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c@@ -1032,13 +1032,13 @@ EXPORT_SYMBOL(pci_free_irq_vectors); */ int pci_irq_vector(struct pci_dev *dev, unsigned int nr) { - int irq = __msi_get_virq(&dev->dev, nr); + unsigned int irq; - switch (irq) { - case -ENODEV: return !nr ? dev->irq : -EINVAL; - case -ENOENT: return -EINVAL; - } - return irq; + if (!dev->msi_enabled && !dev->msix_enabled) + return !nr ? dev->irq : -EINVAL; + + irq = msi_get_virq(&dev->dev, nr); + return irq ? irq : -EINVAL; } EXPORT_SYMBOL(pci_irq_vector); --- a/include/linux/msi.h +++ b/include/linux/msi.h
@@ -169,21 +169,7 @@ static inline bool msi_device_has_proper } #endif -int __msi_get_virq(struct device *dev, unsigned int index); - -/** - * msi_get_virq - Return Linux interrupt number of a MSI interrupt - * @dev: Device to operate on - * @index: MSI interrupt index to look for (0-based) - * - * Return: The Linux interrupt number on success (> 0), 0 if not found - */ -static inline unsigned int msi_get_virq(struct device *dev, unsigned int index) -{ - int ret = __msi_get_virq(dev, index); - - return ret < 0 ? 0 : ret; -} +unsigned int msi_get_virq(struct device *dev, unsigned int index); /* Helpers to hide struct msi_desc implementation details */ #define msi_desc_to_dev(desc) ((desc)->dev) --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c
@@ -119,21 +119,19 @@ int msi_setup_device_data(struct device } /** - * __msi_get_virq - Return Linux interrupt number of a MSI interrupt + * msi_get_virq - Return Linux interrupt number of a MSI interrupt * @dev: Device to operate on * @index: MSI interrupt index to look for (0-based) * - * Return: The Linux interrupt number on success (> 0) - * -ENODEV when the device is not using MSI - * -ENOENT if no such entry exists + * Return: The Linux interrupt number on success (> 0), 0 if not found */ -int __msi_get_virq(struct device *dev, unsigned int index) +unsigned int msi_get_virq(struct device *dev, unsigned int index) { struct msi_desc *desc; bool pcimsi; if (!dev->msi.data) - return -ENODEV; + return 0; pcimsi = msi_device_has_property(dev, MSI_PROP_PCI_MSI);
@@ -152,9 +150,9 @@ int __msi_get_virq(struct device *dev, u if (desc->msi_index == index) return desc->irq; } - return -ENOENT; + return 0; } -EXPORT_SYMBOL_GPL(__msi_get_virq); +EXPORT_SYMBOL_GPL(msi_get_virq); #ifdef CONFIG_SYSFS static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,