Re: Regression fyi: pci usb3 card in 32bit P4 s478 stopped working

[Thomas Gleixner <tglx@kernel.org>]

On Wed, May 06 2026 at 21:15, Thomas Gleixner wrote:
> Seems to be a special case and I don't have access to this Marvell NVME
> device to validate that a potential fix might work.
>
> But looking at the context when this commit was merged and the
> information in the thread discussing the whole issue then this was
> required because the PCI/MSI code back then did not check upfront
> whether MSIX is supported by the underlying PCI interrupt domain.
>
> Since 6.2 (roughly a year after the above commit) this changed so that
> the code in question can't be reached anymore when the underlying
> interrupt domain does not support MSI-X, which in turn makes this hack
> moot.
>
> That means we can revert to the original behavior on kernels >= 6.2,
> which still leaves us with the gap of 5.10, 5.15, 6.1 LTS kernels.
>
> That makes me realize in hindsight that we should have created a quirk
> for that broken Marvell NVME device instead of playing games with the
> specification. Even if that spcificiation is not worth the paper it is
> written on as demonstrated by that Marvell device which has been
> rubberstamped as fully compliant. Oh well...
>
> The more I think about it, the more I tend to revert that commit all the
> way back to 5.10 and wait for people to complain about that broken
> Marvell device again. If they show up, we fix it properly with a quirk
> even if it's a major nuisance.
>
> Roman, can you confirm that the patch below makes your setup work again
> on 6.18+ kernels?

Duh! Inserted the non-compiling version and then hit send before
noticing... Proper patch below.

Thanks,

        tglx
---
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -656,11 +656,15 @@ static void msix_update_entries(struct p
 	}
 }
 
-static void msix_mask_all(void __iomem *base, int tsize)
+static void msix_mask_all(struct pci_dev *dev, int tsize)
 {
 	u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
+	void __iomem *base = dev->msix_base;
 	int i;
 
+	if (pci_msi_domain_supports(dev, MSI_FLAG_NO_MASK, DENY_LEGACY))
+		return;
+
 	for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
 		writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);
 }
@@ -724,7 +728,6 @@ static int msix_capability_init(struct p
 	 */
 	pci_msix_clear_and_set_ctrl(dev, 0, PCI_MSIX_FLAGS_MASKALL |
 				    PCI_MSIX_FLAGS_ENABLE);
-
 	/* Mark it enabled so setup functions can query it */
 	dev->msix_enabled = 1;
 
@@ -737,6 +740,12 @@ static int msix_capability_init(struct p
 		goto out_disable;
 	}
 
+	/*
+	 * Ensure that all table entries are masked to prevent stale entries
+	 * from firing in a crash kernel.
+	 */
+	msix_mask_all(dev, tsize);
+
 	ret = msix_setup_interrupts(dev, entries, nvec, affd);
 	if (ret)
 		goto out_unmap;
@@ -744,17 +753,6 @@ static int msix_capability_init(struct p
 	/* Disable INTX */
 	pci_intx_for_msi(dev, 0);
 
-	if (!pci_msi_domain_supports(dev, MSI_FLAG_NO_MASK, DENY_LEGACY)) {
-		/*
-		 * Ensure that all table entries are masked to prevent
-		 * stale entries from firing in a crash kernel.
-		 *
-		 * Done late to deal with a broken Marvell NVME device
-		 * which takes the MSI-X mask bits into account even
-		 * when MSI-X is disabled, which prevents MSI delivery.
-		 */
-		msix_mask_all(dev->msix_base, tsize);
-	}
 	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
 	pcibios_free_irq(dev);