Re: [RFC PATCH] PCI/MSI: Only mask all MSI-X entries when MSI-X is used

[Thomas Gleixner <tglx@linutronix.de>]

Stefan,

On Sat, Dec 11 2021 at 14:58, Stefan Roese wrote:
> On 12/11/21 11:17, Thomas Gleixner wrote:
>> Can you try the patch below?
>
> Sure, please see below.
>
>> It might still be that this Marvell part really combines the per entry
>> mask bits from MSI-X with MSI, then we need both.
>
> With your patch applied only (mine not), the Masked+ is gone but still
> the MSI interrupts are not received in the system. So you seem to have
> guessed correctly, that we need both changes.

Groan. How is that device specification compliant?

Vector Control for MSI-X Table Entries
--------------------------------------

"00: Mask bit:  When this bit is set, the function is prohibited from
                sending a message using this MSI-X Table entry.
                ....
                This bit’s state after reset is 1 (entry is masked)."

So how can that work in the first place if that device is PCI
specification compliant? Seems that PCI/SIG compliance program is just
another rubberstamping nonsense.

Can someone who has access to that group please ask them what their
specification compliance stuff is actualy testing?

Sure, that went unnoticed so far on that marvelous device because the
kernel was missing a defense line, but sigh...

> How to continue? Should I integrate your patch into mine and send a new
> version? Or will you send it separately to the list for integration?

Your patch is incomplete. The function can fail later on, which results
in the same problem, no?

So we need something like the below.

Just to satisfy my curiosity:

  The device supports obviously MSI-X, which is preferred over MSI.

  So why is the MSI-X initialization failing in the first place on this
  platform?

Thanks,

        tglx
---
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -722,9 +722,6 @@ static int msix_capability_init(struct p
 		goto out_disable;
 	}
 
-	/* Ensure that all table entries are masked. */
-	msix_mask_all(base, tsize);
-
 	ret = msix_setup_entries(dev, base, entries, nvec, affd);
 	if (ret)
 		goto out_disable;
@@ -751,6 +748,9 @@ static int msix_capability_init(struct p
 	/* Set MSI-X enabled bits and unmask the function */
 	pci_intx_for_msi(dev, 0);
 	dev->msix_enabled = 1;
+
+	/* Ensure that all table entries are masked. */
+	msix_mask_all(base, tsize);
 	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
 	pcibios_free_irq(dev);
@@ -777,7 +777,7 @@ static int msix_capability_init(struct p
 	free_msi_irqs(dev);
 
 out_disable:
-	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE, 0);
 
 	return ret;
 }