* [PATCH 0/2] Fix kernel panic on niu bind [not found] <20241117234843.19236-1-dullfire.ref@yahoo.com> @ 2024-11-17 23:48 ` dullfire 2024-11-17 23:48 ` [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads dullfire 2024-11-17 23:48 ` [PATCH 2/2] net/niu: niu requires MSIX ENTRY_DATA fields touch before entry reads dullfire 0 siblings, 2 replies; 9+ messages in thread From: dullfire @ 2024-11-17 23:48 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: Jonathan Currier From: Jonathan Currier <dullfire@yahoo.com> Currently, the niu module causes a fatal trap (tested on SPARC system) when setting up it's MSIX interrupt vectors. A simple write to each MSIX's vector table entries' ENTRY_DATA register is sufficient to work around the issue for the current power cycle (note: this means booting a working kernel, and then rebooting without power cycling will allow a kernel without this fix to boot and work). This series implements a struct pci_dev flag indicating ENTRY_DATA must be written to before reads, and then sets the flag in the niu driver. This series is based off of and tested on v6.11.5. Testing on next-20241115 was also done successfully with the following caveats: On my test case (SPARC T2), commit 03cfe0e05650 ("PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers") prevented all PCIe drivers from binding, including niu. For my test case I disabled 03cfe0e05650's device_link_add(). Original mailing list discussion: Link: https://lore.kernel.org/sparclinux/7de14cca-e2fa-49f7-b83e-5f8322cc9e56@yahoo.com/T/ Jonathan Currier (2): PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads net/niu: niu requires MSIX ENTRY_DATA fields touch before entry reads drivers/net/ethernet/sun/niu.c | 2 ++ drivers/pci/msi/msi.c | 2 ++ include/linux/pci.h | 2 ++ 3 files changed, 6 insertions(+) base-commit: 05b1367d372aca98a4e09c1a0e7ff0b9d721b2bc -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2024-11-17 23:48 ` [PATCH 0/2] Fix kernel panic on niu bind dullfire @ 2024-11-17 23:48 ` dullfire 2024-11-21 8:55 ` Paolo Abeni 2024-11-17 23:48 ` [PATCH 2/2] net/niu: niu requires MSIX ENTRY_DATA fields touch before entry reads dullfire 1 sibling, 1 reply; 9+ messages in thread From: dullfire @ 2024-11-17 23:48 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: Jonathan Currier, stable 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 { -- 2.45.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2024-11-17 23:48 ` [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads dullfire @ 2024-11-21 8:55 ` Paolo Abeni 2024-11-21 9:22 ` Dullfire 0 siblings, 1 reply; 9+ messages in thread From: Paolo Abeni @ 2024-11-21 8:55 UTC (permalink / raw) To: dullfire, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2024-11-21 8:55 ` Paolo Abeni @ 2024-11-21 9:22 ` Dullfire 2024-11-21 10:28 ` Paolo Abeni 0 siblings, 1 reply; 9+ messages in thread From: Dullfire @ 2024-11-21 9:22 UTC (permalink / raw) To: Paolo Abeni, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable 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@yahoo.com/T/ 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2024-11-21 9:22 ` Dullfire @ 2024-11-21 10:28 ` Paolo Abeni 2025-01-20 12:38 ` Dullfire 0 siblings, 1 reply; 9+ messages in thread From: Paolo Abeni @ 2024-11-21 10:28 UTC (permalink / raw) To: Dullfire, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable 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@yahoo.com/T/ 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2024-11-21 10:28 ` Paolo Abeni @ 2025-01-20 12:38 ` Dullfire 2025-04-14 16:22 ` Dullfire 0 siblings, 1 reply; 9+ messages in thread From: Dullfire @ 2025-01-20 12:38 UTC (permalink / raw) To: Paolo Abeni, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable On 11/21/24 04:28, Paolo Abeni wrote: > 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@yahoo.com/T/ > > 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 > Thomas, does this work for you, or is there something else you would to see in this series? Sincerely, Jonathan Currier ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2025-01-20 12:38 ` Dullfire @ 2025-04-14 16:22 ` Dullfire 2025-04-14 19:52 ` Thomas Gleixner 0 siblings, 1 reply; 9+ messages in thread From: Dullfire @ 2025-04-14 16:22 UTC (permalink / raw) To: Paolo Abeni, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable On 1/20/25 6:38 AM, Dullfire wrote: > > On 11/21/24 04:28, Paolo Abeni wrote: [...] >> 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 >> > > Thomas, does this work for you, or is there something else you would to see in this series? Since it's been a few months, just wanted to give this a bump, and see if the series is lacking anything. Thanks, Sincerely, Jonathan Currier ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads 2025-04-14 16:22 ` Dullfire @ 2025-04-14 19:52 ` Thomas Gleixner 0 siblings, 0 replies; 9+ messages in thread From: Thomas Gleixner @ 2025-04-14 19:52 UTC (permalink / raw) To: Dullfire, Paolo Abeni, David S . Miller, Eric Dumazet, Jakub Kicinski, Bjorn Helgaas, Jacob Keller, Simon Horman, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: stable On Mon, Apr 14 2025 at 11:22, dullfire@yahoo.com wrote: > On 1/20/25 6:38 AM, Dullfire wrote: >> On 11/21/24 04:28, Paolo Abeni wrote: > [...] >>> 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 >>> >> >> Thomas, does this work for you, or is there something else you would to see in this series? > > Since it's been a few months, just wanted to give this a bump, and see if the series is lacking anything. > Thanks, Sorry for letting this fall through the cracks. I put it back on my todo list... Thanks, tglx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] net/niu: niu requires MSIX ENTRY_DATA fields touch before entry reads 2024-11-17 23:48 ` [PATCH 0/2] Fix kernel panic on niu bind dullfire 2024-11-17 23:48 ` [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads dullfire @ 2024-11-17 23:48 ` dullfire 1 sibling, 0 replies; 9+ messages in thread From: dullfire @ 2024-11-17 23:48 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Jacob Keller, Simon Horman, Thomas Gleixner, Mostafa Saleh, Marc Zyngier, netdev, linux-kernel, linux-pci Cc: Jonathan Currier, stable From: Jonathan Currier <dullfire@yahoo.com> Fix niu_try_msix() to not cause a fatal trap on sparc systems. Set PCI_DEV_FLAGS_MSIX_TOUCH_ENTRY_DATA_FIRST on the struct pci_dev to work around a bug in the hardware or firmware. For each vector entry in the msix table, niu chips will cause a fatal trap if any registers in that entry are read before that entries' ENTRY_DATA register is written to. Testing indicates writes to other registers are not sufficient to prevent the fatal trap, however the value does not appear to matter. This only needs to happen once after power up, so simply rebooting into a kernel lacking this fix will NOT cause the trap. NON-RESUMABLE ERROR: Reporting on cpu 64 NON-RESUMABLE ERROR: TPC [0x00000000005f6900] <msix_prepare_msi_desc+0x90/0xa0> NON-RESUMABLE ERROR: RAW [4010000000000016:00000e37f93e32ff:0000000202000080:ffffffffffffffff NON-RESUMABLE ERROR: 0000000800000000:0000000000000000:0000000000000000:0000000000000000] NON-RESUMABLE ERROR: handle [0x4010000000000016] stick [0x00000e37f93e32ff] NON-RESUMABLE ERROR: type [precise nonresumable] NON-RESUMABLE ERROR: attrs [0x02000080] < ASI sp-faulted priv > NON-RESUMABLE ERROR: raddr [0xffffffffffffffff] NON-RESUMABLE ERROR: insn effective address [0x000000c50020000c] NON-RESUMABLE ERROR: size [0x8] NON-RESUMABLE ERROR: asi [0x00] CPU: 64 UID: 0 PID: 745 Comm: kworker/64:1 Not tainted 6.11.5 #63 Workqueue: events work_for_cpu_fn TSTATE: 0000000011001602 TPC: 00000000005f6900 TNPC: 00000000005f6904 Y: 00000000 Not tainted TPC: <msix_prepare_msi_desc+0x90/0xa0> g0: 00000000000002e9 g1: 000000000000000c g2: 000000c50020000c g3: 0000000000000100 g4: ffff8000470307c0 g5: ffff800fec5be000 g6: ffff800047a08000 g7: 0000000000000000 o0: ffff800014feb000 o1: ffff800047a0b620 o2: 0000000000000011 o3: ffff800047a0b620 o4: 0000000000000080 o5: 0000000000000011 sp: ffff800047a0ad51 ret_pc: 00000000005f7128 RPC: <__pci_enable_msix_range+0x3cc/0x460> l0: 000000000000000d l1: 000000000000c01f l2: ffff800014feb0a8 l3: 0000000000000020 l4: 000000000000c000 l5: 0000000000000001 l6: 0000000020000000 l7: ffff800047a0b734 i0: ffff800014feb000 i1: ffff800047a0b730 i2: 0000000000000001 i3: 000000000000000d i4: 0000000000000000 i5: 0000000000000000 i6: ffff800047a0ae81 i7: 00000000101888b0 I7: <niu_try_msix.constprop.0+0xc0/0x130 [niu]> Call Trace: [<00000000101888b0>] niu_try_msix.constprop.0+0xc0/0x130 [niu] [<000000001018f840>] niu_get_invariants+0x183c/0x207c [niu] [<00000000101902fc>] niu_pci_init_one+0x27c/0x2fc [niu] [<00000000005ef3e4>] local_pci_probe+0x28/0x74 [<0000000000469240>] work_for_cpu_fn+0x8/0x1c [<000000000046b008>] process_scheduled_works+0x144/0x210 [<000000000046b518>] worker_thread+0x13c/0x1c0 [<00000000004710e0>] kthread+0xb8/0xc8 [<00000000004060c8>] ret_from_fork+0x1c/0x2c [<0000000000000000>] 0x0 Kernel panic - not syncing: Non-resumable error. Fixes: 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X entries") Cc: stable@vger.kernel.org Signed-off-by: Jonathan Currier <dullfire@yahoo.com> --- drivers/net/ethernet/sun/niu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c index 41a27ae58ced..f5449b73b9a7 100644 --- a/drivers/net/ethernet/sun/niu.c +++ b/drivers/net/ethernet/sun/niu.c @@ -9058,6 +9058,8 @@ static void niu_try_msix(struct niu *np, u8 *ldg_num_map) msi_vec[i].entry = i; } + pdev->dev_flags |= PCI_DEV_FLAGS_MSIX_TOUCH_ENTRY_DATA_FIRST; + num_irqs = pci_enable_msix_range(pdev, msi_vec, 1, num_irqs); if (num_irqs < 0) { np->flags &= ~NIU_FLAGS_MSIX; -- 2.45.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-14 19:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241117234843.19236-1-dullfire.ref@yahoo.com>
2024-11-17 23:48 ` [PATCH 0/2] Fix kernel panic on niu bind dullfire
2024-11-17 23:48 ` [PATCH 1/2] PCI/MSI: Add MSIX option to write to ENTRY_DATA before any reads dullfire
2024-11-21 8:55 ` Paolo Abeni
2024-11-21 9:22 ` Dullfire
2024-11-21 10:28 ` Paolo Abeni
2025-01-20 12:38 ` Dullfire
2025-04-14 16:22 ` Dullfire
2025-04-14 19:52 ` Thomas Gleixner
2024-11-17 23:48 ` [PATCH 2/2] net/niu: niu requires MSIX ENTRY_DATA fields touch before entry reads dullfire
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).