From: Jonathan Currier dullfire@yahoo.com
Commit 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X entries") introduces a readl() from ENTRY_VECTOR_CTRL before the writel() to ENTRY_DATA. This is correct, however some hardware, like the Sun Neptune chips, the niu module, will cause an error and/or fatal trap if any MSIX table entry is read before the corresponding ENTRY_DATA field is written to. This patch adds an optional early writel() in msix_prepare_msi_desc().
Cc: stable@vger.kernel.org Signed-off-by: Jonathan Currier dullfire@yahoo.com --- drivers/pci/msi/msi.c | 2 ++ include/linux/pci.h | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c index 3a45879d85db..50d87fb5e37f 100644 --- a/drivers/pci/msi/msi.c +++ b/drivers/pci/msi/msi.c @@ -611,6 +611,8 @@ void msix_prepare_msi_desc(struct pci_dev *dev, struct msi_desc *desc) if (desc->pci.msi_attrib.can_mask) { void __iomem *addr = pci_msix_desc_addr(desc);
+ if (dev->dev_flags & PCI_DEV_FLAGS_MSIX_TOUCH_ENTRY_DATA_FIRST) + writel(0, addr + PCI_MSIX_ENTRY_DATA); desc->pci.msix_ctrl = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL); } } diff --git a/include/linux/pci.h b/include/linux/pci.h index 37d97bef060f..b8b95b58d522 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -245,6 +245,8 @@ enum pci_dev_flags { PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 11), /* Device does honor MSI masking despite saying otherwise */ PCI_DEV_FLAGS_HAS_MSI_MASKING = (__force pci_dev_flags_t) (1 << 12), + /* Device requires write to PCI_MSIX_ENTRY_DATA before any MSIX reads */ + PCI_DEV_FLAGS_MSIX_TOUCH_ENTRY_DATA_FIRST = (__force pci_dev_flags_t) (1 << 13), };
enum pci_irq_reroute_variant {
On 11/18/24 00:48, dullfire@yahoo.com wrote:
From: Jonathan Currier dullfire@yahoo.com
Commit 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X entries") introduces a readl() from ENTRY_VECTOR_CTRL before the writel() to ENTRY_DATA. This is correct, however some hardware, like the Sun Neptune chips, the niu module, will cause an error and/or fatal trap if any MSIX table entry is read before the corresponding ENTRY_DATA field is written to. This patch adds an optional early writel() in msix_prepare_msi_desc().
Why the issue can't be addressed into the relevant device driver? It looks like an H/W bug, a driver specific fix looks IMHO more fitting.
A cross subsystem series, like this one, gives some extra complication to maintainers.
Thanks,
Paolo
On 11/21/24 02:55, Paolo Abeni wrote:
On 11/18/24 00:48, dullfire@yahoo.com wrote:
From: Jonathan Currier dullfire@yahoo.com
Commit 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X entries") introduces a readl() from ENTRY_VECTOR_CTRL before the writel() to ENTRY_DATA. This is correct, however some hardware, like the Sun Neptune chips, the niu module, will cause an error and/or fatal trap if any MSIX table entry is read before the corresponding ENTRY_DATA field is written to. This patch adds an optional early writel() in msix_prepare_msi_desc().
Why the issue can't be addressed into the relevant device driver? It looks like an H/W bug, a driver specific fix looks IMHO more fitting.
I considered this approach, and thus asked about it in the mailing lists here: https://lore.kernel.org/sparclinux/7de14cca-e2fa-49f7-b83e-5f8322cc9e56@yaho... It sounds like you are suggesting approach 1 (add the MSIX register writes to niu). I did do a quick and dirty test of that. However it ended up requiring msix_map_region(), and pci_msix_desc_addr(), both of are internal to the msi code, and not exported or in pubic headers. The msix_table_size() macro was also needed. I could either export those functions, or reproduce their code in the niu driver. However, as Thomas' suggestion seemed very simple and elegant to me, I decided to got with it.
If it is actually preferable to either copy those msix functions into niu, they are not very large, or export them (GPL I would assume?) let me know and I can make that change.
A cross subsystem series, like this one, gives some extra complication to maintainers.
Thanks,
Paolo
Sincerely, Jonathan Currier
On 11/21/24 10:22, Dullfire wrote:
On 11/21/24 02:55, Paolo Abeni wrote:
On 11/18/24 00:48, dullfire@yahoo.com wrote:
From: Jonathan Currier dullfire@yahoo.com
Commit 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X entries") introduces a readl() from ENTRY_VECTOR_CTRL before the writel() to ENTRY_DATA. This is correct, however some hardware, like the Sun Neptune chips, the niu module, will cause an error and/or fatal trap if any MSIX table entry is read before the corresponding ENTRY_DATA field is written to. This patch adds an optional early writel() in msix_prepare_msi_desc().
Why the issue can't be addressed into the relevant device driver? It looks like an H/W bug, a driver specific fix looks IMHO more fitting.
I considered this approach, and thus asked about it in the mailing lists here: https://lore.kernel.org/sparclinux/7de14cca-e2fa-49f7-b83e-5f8322cc9e56@yaho...
I forgot about such thread, thank you for the reminder. Since the more hackish code is IRQ specific, if Thomas is fine with that, I'll not oppose.
A cross subsystem series, like this one, gives some extra complication to maintainers.
The niu driver is not exactly under very active development, I guess the whole series could go via the IRQ subsystem, if Thomas agrees.
Cheers,
Paolo
linux-stable-mirror@lists.linaro.org
-
Dullfire -
dullfire＠yahoo.com
-
Paolo Abeni