On 1/7/26 7:55 PM, Nilay Shroff wrote:

On 1/7/26 3:32 PM, Nam Cao wrote:
Nilay reported that since commit daaa574aba6f ("powerpc/pseries/msi: Switch
to msi_create_parent_irq_domain()"), the NVMe driver cannot enable MSI-X
when the device's MSI-X table size is larger than the firmware's MSI quota
for the device.

This is because the commit changes how rtas_prepare_msi_irqs() is called:

   - Before, it is called when interrupts are allocated at the global
     interrupt domain with nvec_in being the number of allocated interrupts.
     rtas_prepare_msi_irqs() can return a positive number and the allocation
     will be retried.

   - Now, it is called at the creation of per-device interrupt domain with
     nvec_in being the number of interrupts that the device supports. If
     rtas_prepare_msi_irqs() returns positive, domain creation just fails.

For Nilay's NVMe driver case, rtas_prepare_msi_irqs() returns a positive
number (the quota). This causes per-device interrupt domain creation to
fail and thus the NVMe driver cannot enable MSI-X.

Rework to make this scenario works again:

   - pseries_msi_ops_prepare() only prepares as many interrupts as the quota
     permit.

   - pseries_irq_domain_alloc() fails if the device's quota is exceeded.

Now, if the quota is exceeded, pseries_msi_ops_prepare() will only prepare
as allowed by the quota. If device drivers attempt to allocate more
interrupts than the quota permits, pseries_irq_domain_alloc() will return
an error code and msi_handle_pci_fail() will allow device drivers a retry.

Reported-by: Nilay Shroff <[email protected]>
Closes: 
https://lore.kernel.org/linuxppc-dev/[email protected]/
Fixes: daaa574aba6f ("powerpc/pseries/msi: Switch to 
msi_create_parent_irq_domain()")
Signed-off-by: Nam Cao <[email protected]>
Acked-by: Nilay Shroff <[email protected]>
Cc: [email protected]
---
v2:
   - change pseries_msi_ops_prepare()'s allocation logic to match the
     original logic in __pci_enable_msix_range()

   - fix up Nilay's email address
---
  arch/powerpc/platforms/pseries/msi.c | 44 ++++++++++++++++++++++++++--
  1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c 
b/arch/powerpc/platforms/pseries/msi.c
index a82aaa786e9e..edc30cda5dbc 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -19,6 +19,11 @@
#include "pseries.h" +struct pseries_msi_device {
+       unsigned int msi_quota;
+       unsigned int msi_used;
+};
+
  static int query_token, change_token;
#define RTAS_QUERY_FN 0
@@ -433,8 +438,28 @@ static int pseries_msi_ops_prepare(struct irq_domain 
*domain, struct device *dev
        struct msi_domain_info *info = domain->host_data;
        struct pci_dev *pdev = to_pci_dev(dev);
        int type = (info->flags & MSI_FLAG_PCI_MSIX) ? PCI_CAP_ID_MSIX : 
PCI_CAP_ID_MSI;
+       int ret;
+
+       struct pseries_msi_device *pseries_dev __free(kfree)
+               = kmalloc(sizeof(*pseries_dev), GFP_KERNEL);
+       if (!pseries_dev)
+               return -ENOMEM;
+
+       while (1) {
+               ret = rtas_prepare_msi_irqs(pdev, nvec, type, arg);
+               if (!ret)
+                       break;
+               else if (ret > 0)
+                       nvec = ret;
+               else
+                       return ret;
+       }
- return rtas_prepare_msi_irqs(pdev, nvec, type, arg);
+       pseries_dev->msi_quota = nvec;
+       pseries_dev->msi_used = 0;
+
+       arg->scratchpad[0].ptr = no_free_ptr(pseries_dev);
+       return 0;
  }
/*
@@ -443,9 +468,13 @@ static int pseries_msi_ops_prepare(struct irq_domain 
*domain, struct device *dev
   */
  static void pseries_msi_ops_teardown(struct irq_domain *domain, 
msi_alloc_info_t *arg)
  {
+       struct pseries_msi_device *pseries_dev = arg->scratchpad[0].ptr;
        struct pci_dev *pdev = to_pci_dev(domain->dev);
rtas_disable_msi(pdev);
+
+       WARN_ON(pseries_dev->msi_used);
+       kfree(pseries_dev);
  }
static void pseries_msi_shutdown(struct irq_data *d)
@@ -546,12 +575,18 @@ static int pseries_irq_domain_alloc(struct irq_domain 
*domain, unsigned int virq
                                    unsigned int nr_irqs, void *arg)
  {
        struct pci_controller *phb = domain->host_data;
+       struct pseries_msi_device *pseries_dev;
        msi_alloc_info_t *info = arg;
        struct msi_desc *desc = info->desc;
        struct pci_dev *pdev = msi_desc_to_pci_dev(desc);
        int hwirq;
        int i, ret;
+ pseries_dev = info->scratchpad[0].ptr;
+
+       if (pseries_dev->msi_used + nr_irqs > pseries_dev->msi_quota)
+               return -ENOSPC;
+
        hwirq = rtas_query_irq_number(pci_get_pdn(pdev), desc->msi_index);
        if (hwirq < 0) {
                dev_err(&pdev->dev, "Failed to query HW IRQ: %d\n", hwirq);
@@ -567,9 +602,10 @@ static int pseries_irq_domain_alloc(struct irq_domain 
*domain, unsigned int virq
                        goto out;
irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
-                                             &pseries_msi_irq_chip, 
domain->host_data);
+                                             &pseries_msi_irq_chip, 
pseries_dev);
        }
+ pseries_dev->msi_used++;
        return 0;
out:
@@ -582,9 +618,11 @@ static void pseries_irq_domain_free(struct irq_domain 
*domain, unsigned int virq
                                    unsigned int nr_irqs)
  {
        struct irq_data *d = irq_domain_get_irq_data(domain, virq);
-       struct pci_controller *phb = irq_data_get_irq_chip_data(d);
+       struct pseries_msi_device *pseries_dev = irq_data_get_irq_chip_data(d);
+       struct pci_controller *phb = domain->host_data;
pr_debug("%s bridge %pOF %d #%d\n", __func__, phb->dn, virq, nr_irqs);
+       pseries_dev->msi_used -= nr_irqs;
        irq_domain_free_irqs_parent(domain, virq, nr_irqs);
  }
I just tested this change on my system using the latest mainline kernel and it 
works
well for me. So with that, please fell free to add,

Tested-by: Nilay Shroff <[email protected]>

Thanks will pull this in.
Maddy



Reply via email to