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

[Thomas Gleixner <tglx@linutronix.de>]

Stefan,

On Fri, Dec 10 2021 at 17:10, Stefan Roese wrote:
> I've debugged the MSI integration of the ZynqMP PCIe rootport driver
> (pcie-xilinx-nwl.c) and found no issues there. Also the MSI framework
> in the Kernel did not reveal any problems - at least for me. Looking
> a bit deeper into the lspci output, I found an interesting difference
> between v5.4 and v5.10 (or later).
>
> v5.4:
> 04:00.0 Non-Volatile memory controller: Marvell Technology Group Ltd. Device 1321 (rev 02) (prog-if 02 [NVM Express])
>         ...
> 	Capabilities: [50] MSI: Enable+ Count=1/1 Maskable+ 64bit+
> 		Address: 00000000fd480000  Data: 0004
> 		Masking: 00000000  Pending: 00000000
> 	Capabilities: [70] Express (v2) Endpoint, MSI 00
> 	...
> 	Capabilities: [b0] MSI-X: Enable- Count=67 Masked-
> 		Vector table: BAR=0 offset=00002000
> 		PBA: BAR=0 offset=00003000
> 	...
>
> v5.10:
> 04:00.0 Non-Volatile memory controller: Marvell Technology Group Ltd. Device 1321 (rev 02) (prog-if 02 [NVM Express])
>         ...
>         Capabilities: [50] MSI: Enable+ Count=1/1 Maskable+ 64bit+
>                 Address: 00000000fd480000  Data: 0004
>                 Masking: 00000000  Pending: 00000000
>         Capabilities: [70] Express (v2) Endpoint, MSI 00
>         ...
>         Capabilities: [b0] MSI-X: Enable- Count=67 Masked+
>                 Vector table: BAR=0 offset=00002000
>                 PBA: BAR=0 offset=00003000
>         ...
>
> So the only difference here being the "Masked+" compared to the
> "Masked-" in the working v5.4 Kernel. Testing in this area has shown,
> that the root cause for the masked bit being set was the call to
> msix_mask_all() in msix_capability_init(). Without this, all works just
> fine and the MSI interrupts are received again by the NVMe driver.

Not really. The Masked+ in the capabilities entry has nothing to do with
the entries in the table being masked. The Masked+ reflects the
PCI_MSIX_FLAGS_MASKALL bit in the MSI-X control register.

That is set early on and not cleared in the error handling path. The
error handling just clears the MSIX_FLAGS_ENABLE bit.

Can you try the patch 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.

Thanks,

        tglx
---
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -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;
 }