* [PATCH 1/5] PCI/portdrv: Group PCI Exp Cap services into a single mask
2026-07-14 18:39 [PATCH 0/5] PCIe Flit Logging Ext Capability Support Yazen Ghannam
@ 2026-07-14 18:39 ` Yazen Ghannam
2026-07-14 19:11 ` sashiko-bot
2026-07-14 18:39 ` [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability Yazen Ghannam
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Yazen Ghannam @ 2026-07-14 18:39 UTC (permalink / raw)
To: linux-pci; +Cc: linux-kernel, bhelgaas, Yazen Ghannam
PME, hotplug, and bandwidth notification are all services of the PCI
Express Capability and share a single MSI/MSI-X interrupt vector. The
set of these services is open-coded as a bitmask in more than one place,
which is repetitive and easy to get out of sync as services are added.
Define a single PCIE_PORT_SERVICE_EXPCAP mask and use it in place of the
open-coded expressions. No functional change intended.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/pci/pcie/portdrv.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 2d6aa488fe7b..53144c99cb3c 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -29,6 +29,10 @@
*/
#define PCIE_PORT_MAX_MSI_ENTRIES 32
+#define PCIE_PORT_SERVICE_EXPCAP (PCIE_PORT_SERVICE_PME | \
+ PCIE_PORT_SERVICE_HP | \
+ PCIE_PORT_SERVICE_BWCTRL)
+
#define get_descriptor_id(type, service) (((type - 4) << 8) | service)
struct portdrv_service_data {
@@ -67,8 +71,7 @@ static int pcie_message_numbers(struct pci_dev *dev, int mask,
* 7.8.2, 7.10.10, 7.31.2.
*/
- if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP |
- PCIE_PORT_SERVICE_BWCTRL)) {
+ if (mask & PCIE_PORT_SERVICE_EXPCAP) {
pcie_capability_read_word(dev, PCI_EXP_FLAGS, ®16);
*pme = FIELD_GET(PCI_EXP_FLAGS_IRQ, reg16);
nvec = *pme + 1;
@@ -149,8 +152,7 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
}
/* PME, hotplug and bandwidth notification share an MSI/MSI-X vector */
- if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP |
- PCIE_PORT_SERVICE_BWCTRL)) {
+ if (mask & PCIE_PORT_SERVICE_EXPCAP) {
pcie_irq = pci_irq_vector(dev, pme);
irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pcie_irq;
irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pcie_irq;
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability
2026-07-14 18:39 [PATCH 0/5] PCIe Flit Logging Ext Capability Support Yazen Ghannam
2026-07-14 18:39 ` [PATCH 1/5] PCI/portdrv: Group PCI Exp Cap services into a single mask Yazen Ghannam
@ 2026-07-14 18:39 ` Yazen Ghannam
2026-07-14 19:17 ` sashiko-bot
2026-07-14 18:39 ` [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging Yazen Ghannam
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Yazen Ghannam @ 2026-07-14 18:39 UTC (permalink / raw)
To: linux-pci; +Cc: linux-kernel, bhelgaas, Yazen Ghannam
From: Avadhut Naik <avadhut.naik@amd.com>
PCIe r6.0 introduced Flit Logging Extended Capability for logging
erroneous flits. This extended capability must be implemented by ports
and RCRBs that support PCIe Gen6 Flit Mode.[1]
Introduce support required for the Extended Capability as a port service
driver.
[1] PCI Express® Base Specification Revision 6.0, section 7.7.8
[Yazen: Fix ups, etc.]
Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
Co-developed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/pci/pci.h | 6 ++
drivers/pci/pcie/Kconfig | 10 ++
drivers/pci/pcie/Makefile | 1 +
drivers/pci/pcie/flit.c | 170 ++++++++++++++++++++++++++++++++++
drivers/pci/pcie/portdrv.c | 15 ++-
drivers/pci/pcie/portdrv.h | 10 +-
drivers/pci/probe.c | 1 +
include/linux/pci.h | 4 +
include/uapi/linux/pci_regs.h | 14 ++-
9 files changed, 227 insertions(+), 4 deletions(-)
create mode 100644 drivers/pci/pcie/flit.c
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..31d04366b980 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -894,6 +894,12 @@ static inline void pci_dpc_init(struct pci_dev *pdev) { }
static inline bool pci_dpc_recovered(struct pci_dev *pdev) { return false; }
#endif
+#ifdef CONFIG_PCIE_FLIT
+void pci_flit_init(struct pci_dev *pdev);
+#else
+static inline void pci_flit_init(struct pci_dev *pdev) { }
+#endif
+
#ifdef CONFIG_PCIEPORTBUS
void pci_rcec_init(struct pci_dev *dev);
void pci_rcec_exit(struct pci_dev *dev);
diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 207c2deae35f..1ee3c85e63c1 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -146,3 +146,13 @@ config PCIE_EDR
the PCI Firmware Specification r3.2. Enable this if you want to
support hybrid DPC model which uses both firmware and OS to
implement DPC.
+
+config PCIE_FLIT
+ bool "PCI Express Flit Mode Error Logging support"
+ depends on PCIEPORTBUS
+ help
+ This enables support for the PCI Express Flit Logging Extended
+ Capability, which logs errors encountered by a port operating
+ in PCIe Gen6 Flit Mode. If your system doesn't have this
+ capability or you do not want to use this feature, it is safe
+ to answer N.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index b0b43a18c304..14a5222920b9 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_PCIE_PME) += pme.o
obj-$(CONFIG_PCIE_DPC) += dpc.o
obj-$(CONFIG_PCIE_PTM) += ptm.o
obj-$(CONFIG_PCIE_EDR) += edr.o
+obj-$(CONFIG_PCIE_FLIT) += flit.o
diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
new file mode 100644
index 000000000000..12f2b6fbf1de
--- /dev/null
+++ b/drivers/pci/pcie/flit.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Service driver for PCIe Flit Logging Capability
+ *
+ * Copyright (c) 2026, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Avadhut Naik <Avadhut.Naik@amd.com>
+ * Yazen Ghannam <Yazen.Ghannam@amd.com>
+ */
+
+#define pr_fmt(fmt) "Flit: " fmt
+#define dev_fmt pr_fmt
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/pci.h>
+#include "portdrv.h"
+#include "../pci.h"
+
+void pci_flit_init(struct pci_dev *pdev)
+{
+ u16 cap;
+
+ if (pcie_capability_read_word(pdev, PCI_EXP_FLAGS, &cap))
+ return;
+
+ if (!(cap & PCI_EXP_FLAGS_FLIT))
+ return;
+
+ pdev->flit_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_FLIT);
+ if (pdev->flit_cap)
+ pci_dbg(pdev, "Flit Mode Error Logging Capability present.\n");
+}
+
+static void flit_cntr_enable(struct pci_dev *pdev)
+{
+ u16 flit = pdev->flit_cap;
+ u16 reg;
+
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, ®);
+ pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, reg);
+
+ /*
+ * NOTE: The "Set Events to Count" and "Trigger Event Count" fields
+ * are left as set by the platform.
+ */
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, ®);
+ reg |= PCI_FLIT_ERR_CNTR_EN;
+ reg |= PCI_FLIT_ERR_CNTR_INTR_EN;
+ pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, reg);
+}
+
+static void flit_cntr_disable(struct pcie_device *dev)
+{
+ struct pci_dev *pdev = dev->port;
+ u16 ctrl, flit = pdev->flit_cap;
+
+ /* Disable both Error Counter and Interrupt generation */
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &ctrl);
+ ctrl &= ~(PCI_FLIT_ERR_CNTR_EN | PCI_FLIT_ERR_CNTR_INTR_EN);
+ pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, ctrl);
+}
+
+static irqreturn_t flit_isr(int irq, void *context)
+{
+ struct pcie_device *dev = (struct pcie_device *)context;
+ struct pci_dev *pdev = dev->port;
+ u16 flit = pdev->flit_cap;
+ u16 cntr_ctrl, cntr_sta;
+ u32 err_log1, err_log2;
+
+ /* Read and log Counter and Error Log Registers. */
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &cntr_ctrl);
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, &cntr_sta);
+
+ pci_info(pdev, HW_ERR "Counter Control: 0x%04x Counter Status: 0x%04x\n", cntr_ctrl, cntr_sta);
+
+ do {
+ pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, &err_log1);
+
+ if (!(err_log1 & PCI_FLIT_ERR_LOG_VALID))
+ break;
+
+ pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
+ pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
+
+ pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
+ } while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
+
+ /*
+ * Re-enabling the counter is the interrupt acknowledgment.
+ *
+ * The status bit checked in flit_irq() (Interrupt Generated based on
+ * Trigger Event Count) is cleared by a 0->1 transition of Flit Error
+ * Counter Enable, per PCIe r6.4 Table 7-96.
+ */
+ cntr_ctrl |= PCI_FLIT_ERR_CNTR_EN;
+ pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, cntr_ctrl);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t flit_irq(int irq, void *context)
+{
+ struct pcie_device *dev = (struct pcie_device *)context;
+ struct pci_dev *pdev = dev->port;
+ u16 flit = pdev->flit_cap;
+ u16 cntr_sta, cntr_ctrl;
+
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, &cntr_sta);
+ if (!(cntr_sta & PCI_FLIT_INTR_GEN_CNTR))
+ return IRQ_NONE;
+
+ pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &cntr_ctrl);
+ cntr_ctrl &= ~PCI_FLIT_ERR_CNTR_EN;
+ pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, cntr_ctrl);
+ return IRQ_WAKE_THREAD;
+}
+
+static int flit_probe(struct pcie_device *dev)
+{
+ struct device *device = &dev->device;
+ struct pci_dev *pdev = dev->port;
+ int status;
+
+ status = devm_request_threaded_irq(device, dev->irq, flit_irq,
+ flit_isr, IRQF_SHARED,
+ "pcie-flit", dev);
+ if (status) {
+ pci_warn(pdev, "request Flit IRQ%d failed: %d\n", dev->irq, status);
+ return status;
+ }
+
+ flit_cntr_enable(pdev);
+ pci_info(pdev, "enabled with IRQ %d\n", dev->irq);
+
+ return 0;
+}
+
+static int flit_cntr_suspend(struct pcie_device *dev)
+{
+ flit_cntr_disable(dev);
+ return 0;
+}
+
+static int flit_cntr_resume(struct pcie_device *dev)
+{
+ flit_cntr_enable(dev->port);
+ return 0;
+}
+
+static void flit_cntr_remove(struct pcie_device *dev)
+{
+ flit_cntr_disable(dev);
+}
+
+static struct pcie_port_service_driver flitdriver = {
+ .name = "flit",
+ .port_type = PCIE_ANY_PORT,
+ .service = PCIE_PORT_SERVICE_FLIT,
+ .probe = flit_probe,
+ .suspend = flit_cntr_suspend,
+ .resume = flit_cntr_resume,
+ .remove = flit_cntr_remove,
+};
+
+int __init pcie_flit_init(void)
+{
+ return pcie_port_service_register(&flitdriver);
+}
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 53144c99cb3c..1a649d6f4ab8 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -31,7 +31,8 @@
#define PCIE_PORT_SERVICE_EXPCAP (PCIE_PORT_SERVICE_PME | \
PCIE_PORT_SERVICE_HP | \
- PCIE_PORT_SERVICE_BWCTRL)
+ PCIE_PORT_SERVICE_BWCTRL | \
+ PCIE_PORT_SERVICE_FLIT)
#define get_descriptor_id(type, service) (((type - 4) << 8) | service)
@@ -151,12 +152,16 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
return nr_entries;
}
- /* PME, hotplug and bandwidth notification share an MSI/MSI-X vector */
+ /*
+ * PME, hotplug, flit error counter, and bandwidth notification share
+ * an MSI/MSI-X vector
+ */
if (mask & PCIE_PORT_SERVICE_EXPCAP) {
pcie_irq = pci_irq_vector(dev, pme);
irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pcie_irq;
irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pcie_irq;
irqs[PCIE_PORT_SERVICE_BWCTRL_SHIFT] = pcie_irq;
+ irqs[PCIE_PORT_SERVICE_FLIT_SHIFT] = pcie_irq;
}
if (mask & PCIE_PORT_SERVICE_AER)
@@ -280,6 +285,11 @@ static int get_port_device_capability(struct pci_dev *dev)
services |= PCIE_PORT_SERVICE_BWCTRL;
}
+#ifdef CONFIG_PCIE_FLIT
+ if (dev->flit_cap)
+ services |= PCIE_PORT_SERVICE_FLIT;
+#endif
+
return services;
}
@@ -836,6 +846,7 @@ static void __init pcie_init_services(void)
pcie_dpc_init();
pcie_bwctrl_init();
pcie_hp_init();
+ pcie_flit_init();
}
static int __init pcie_portdrv_init(void)
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index cc58bf2f2c84..db80faecf34c 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -22,8 +22,10 @@
#define PCIE_PORT_SERVICE_DPC (1 << PCIE_PORT_SERVICE_DPC_SHIFT)
#define PCIE_PORT_SERVICE_BWCTRL_SHIFT 4 /* Bandwidth Controller (notifications) */
#define PCIE_PORT_SERVICE_BWCTRL (1 << PCIE_PORT_SERVICE_BWCTRL_SHIFT)
+#define PCIE_PORT_SERVICE_FLIT_SHIFT 5 /* Flit Logging */
+#define PCIE_PORT_SERVICE_FLIT (1 << PCIE_PORT_SERVICE_FLIT_SHIFT)
-#define PCIE_PORT_DEVICE_MAXSERVICES 5
+#define PCIE_PORT_DEVICE_MAXSERVICES 6
extern bool pcie_ports_dpc_native;
@@ -51,6 +53,12 @@ int pcie_dpc_init(void);
static inline int pcie_dpc_init(void) { return 0; }
#endif
+#ifdef CONFIG_PCIE_FLIT
+int pcie_flit_init(void);
+#else
+static inline int pcie_flit_init(void) { return 0; }
+#endif
+
int pcie_bwctrl_init(void);
/* Port Type */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dd0abbc63e18..a1ec44b04eb0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2666,6 +2666,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
pci_pasid_init(dev); /* Process Address Space ID */
pci_acs_init(dev); /* Access Control Services */
pci_ptm_init(dev); /* Precision Time Measurement */
+ pci_flit_init(dev); /* Flit Logging */
pci_aer_init(dev); /* Advanced Error Reporting */
pci_dpc_init(dev); /* Downstream Port Containment */
pci_rcec_init(dev); /* Root Complex Event Collector */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ebb5b9d76360..a8d8dbac6640 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -592,6 +592,10 @@ struct pci_dev {
u8 tph_mode; /* TPH mode */
u8 tph_req_type; /* TPH requester type */
#endif
+
+#ifdef CONFIG_PCIE_FLIT
+ u16 flit_cap; /* Flit Logging Capabilities */
+#endif
};
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index facaa324bd86..774eeed80bed 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -763,7 +763,8 @@
#define PCI_EXT_CAP_ID_DEV3 0x2F /* Device 3 Capability/Control/Status */
#define PCI_EXT_CAP_ID_IDE 0x30 /* Integrity and Data Encryption */
#define PCI_EXT_CAP_ID_PL_64GT 0x31 /* Physical Layer 64.0 GT/s */
-#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PL_64GT
+#define PCI_EXT_CAP_ID_FLIT 0x32 /* Flit Error Logging */
+#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_FLIT
#define PCI_EXT_CAP_DSN_SIZEOF 12
#define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
@@ -1187,6 +1188,17 @@
/* Physical Layer 64.0 GT/s */
#define PCI_PL_64GT_LE_CTRL 0x20 /* Lane Equalization Control Register */
+/* Flit Mode Error Logging */
+#define PCI_FLIT_ERR_LOG1 0x04 /* Flit Error Log 1 Register */
+#define PCI_FLIT_ERR_LOG_VALID 0x1 /* Flit Error Log Valid */
+#define PCI_FLIT_ERR_LOG_MORE 0x2000 /* More Entries Valid */
+#define PCI_FLIT_ERR_LOG2 0x08 /* Flit Error Log 2 Register */
+#define PCI_FLIT_ERR_CNTR_CTRL 0x0c /* Flit Error Counter Control Register */
+#define PCI_FLIT_ERR_CNTR_EN 0x0001 /* Flit Error Counter Enable */
+#define PCI_FLIT_ERR_CNTR_INTR_EN 0x0002 /* Flit Error Counter Interrupt Enable */
+#define PCI_FLIT_ERR_CNTR_STA 0x0e /* Flit Error Counter Status Register */
+#define PCI_FLIT_INTR_GEN_CNTR 0x0008 /* Interrupt on Trigger Event Count */
+
/* Native PCIe Enclosure Management */
#define PCI_NPEM_CAP 0x04 /* NPEM capability register */
#define PCI_NPEM_CAP_CAPABLE 0x00000001 /* NPEM Capable */
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability
2026-07-14 18:39 ` [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability Yazen Ghannam
@ 2026-07-14 19:17 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-14 19:17 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Multiple execution contexts perform Read-Modify-Write (RMW) operations on the `PCI_FLIT_ERR_CNTR_CTRL` register without mutual exclusion, creating a race condition that can leave hardware interrupts enabled after driver removal or suspend, resulting in an unhandled IRQ storm.
- [High] Failure to properly mask level-triggered INTx interrupts in the hard IRQ handler leads to an IRQ storm and CPU lockup.
- [High] Infinite loop in threaded IRQ handler on surprise device removal or PCI error response.
- [Medium] Platform-configured Flit Error Counter settings are lost across D3cold suspend/resume transitions.
- [Medium] Unbounded loop in `flit_isr` drains hardware error logs, risking Denial of Service (DoS) and soft lockups.
--
--- Patch [2]: [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability ---
commit 2eae6403f78aaec770133667c1980b90da860010
Author: Avadhut Naik <avadhut.naik@amd.com>
PCI/FLIT: Add support for Flit Logging Extended Capability
This commit introduces a port service driver to support the PCI Express Flit
Logging Extended Capability. This feature allows logging of erroneous flits
on ports operating in PCIe Gen6 Flit Mode.
> diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
> new file mode 100644
> index 0000000000000..12f2b6fbf1de0
> --- /dev/null
> +++ b/drivers/pci/pcie/flit.c
> @@ -0,0 +1,170 @@
[ ... ]
> +static void flit_cntr_disable(struct pcie_device *dev)
> +{
> + struct pci_dev *pdev = dev->port;
> + u16 ctrl, flit = pdev->flit_cap;
> +
> + /* Disable both Error Counter and Interrupt generation */
> + pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &ctrl);
> + ctrl &= ~(PCI_FLIT_ERR_CNTR_EN | PCI_FLIT_ERR_CNTR_INTR_EN);
> + pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, ctrl);
> +}
[Severity: High]
Multiple execution contexts perform Read-Modify-Write (RMW) operations on the
PCI_FLIT_ERR_CNTR_CTRL register without mutual exclusion.
If flit_cntr_disable() runs concurrently with flit_isr() during driver
unbind or hot-unplug, could the lockless clear of the EN and INTR_EN bits
be silently overwritten by flit_isr() writing back its stale cached value
with INTR_EN set?
Since flit_irq() also performs a lockless RMW from hardirq context, should
all accesses to this control register be protected by a spinlock to prevent
leaving hardware interrupts enabled after driver removal?
[ ... ]
> +static irqreturn_t flit_isr(int irq, void *context)
> +{
[ ... ]
> + do {
> + pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, &err_log1);
> +
> + if (!(err_log1 & PCI_FLIT_ERR_LOG_VALID))
> + break;
> +
> + pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
> + pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
> +
> + pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
> + } while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
[Severity: High]
If the PCIe link goes down or the device is surprise-removed,
pci_read_config_dword() will return an error response of 0xFFFFFFFF (~0).
Since (~0 & PCI_FLIT_ERR_LOG_VALID) and (~0 & PCI_FLIT_ERR_LOG_MORE) are
both true, will this trap the threaded handler in an infinite loop?
[Severity: Medium]
Does this do/while loop need a maximum iteration limit?
If a faulty or malicious PCIe device streams flit errors and continually
asserts the MORE bit faster than the CPU can process them, could this
indefinitely stall the CPU in the threaded IRQ handler and cause a soft
lockup?
[ ... ]
> +static irqreturn_t flit_irq(int irq, void *context)
> +{
> + struct pcie_device *dev = (struct pcie_device *)context;
> + struct pci_dev *pdev = dev->port;
> + u16 flit = pdev->flit_cap;
> + u16 cntr_sta, cntr_ctrl;
> +
> + pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, &cntr_sta);
> + if (!(cntr_sta & PCI_FLIT_INTR_GEN_CNTR))
> + return IRQ_NONE;
[Severity: High]
Similar to the loop in flit_isr() above, if the device is disconnected and
pci_read_config_word() sets cntr_sta to 0xFFFF, will this erroneously skip
the IRQ_NONE return and wake the thread?
> +
> + pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &cntr_ctrl);
> + cntr_ctrl &= ~PCI_FLIT_ERR_CNTR_EN;
> + pci_write_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, cntr_ctrl);
> + return IRQ_WAKE_THREAD;
> +}
[Severity: High]
When using shared legacy INTx interrupts, the hard IRQ handler clears
PCI_FLIT_ERR_CNTR_EN but fails to clear PCI_FLIT_ERR_CNTR_INTR_EN.
Since the interrupt status bit is only cleared on a 0 to 1 transition of
the EN bit (which happens later in the threaded handler), the INTx line
remains physically asserted.
Because flit_probe() does not use IRQF_ONESHOT, won't returning
IRQ_WAKE_THREAD unmask the IRQ line and immediately re-trigger the hard
IRQ handler, starving the threaded handler and creating an infinite IRQ
storm?
[ ... ]
> +static int flit_cntr_suspend(struct pcie_device *dev)
> +{
> + flit_cntr_disable(dev);
> + return 0;
> +}
[Severity: Medium]
The suspend hook disables the counter but does not read or save the
platform-configured "Set Events to Count" and "Trigger Event Count"
fields.
When flit_cntr_enable() is called on resume, it reads the hardware-reset
value (typically 0) and writes it back with the enable bits set. Will this
permanently erase the platform-specific error logging thresholds across a
D3cold suspend and resume cycle?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714183950.1620016-1-yazen.ghannam@amd.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging
2026-07-14 18:39 [PATCH 0/5] PCIe Flit Logging Ext Capability Support Yazen Ghannam
2026-07-14 18:39 ` [PATCH 1/5] PCI/portdrv: Group PCI Exp Cap services into a single mask Yazen Ghannam
2026-07-14 18:39 ` [PATCH 2/5] PCI/FLIT: Add support for Flit Logging Extended Capability Yazen Ghannam
@ 2026-07-14 18:39 ` Yazen Ghannam
2026-07-14 19:36 ` sashiko-bot
2026-07-14 18:39 ` [PATCH 4/5] PCI/FLIT: Add trace event for flit errors Yazen Ghannam
2026-07-14 18:39 ` [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging Yazen Ghannam
4 siblings, 1 reply; 11+ messages in thread
From: Yazen Ghannam @ 2026-07-14 18:39 UTC (permalink / raw)
To: linux-pci; +Cc: linux-kernel, bhelgaas, Yazen Ghannam
The Flit Logging Extended Capability may be managed by the platform
firmware. Negotiate ownership through _OSC before the OS enables the
Flit Error Counter, and only register the port service when the OS is
granted native control.
Advertise support via a new OSC_PCI_EXPRESS_FLIT_CONTROL bit, record
the negotiated result in the host bridge's native_flit flag, and gate
the Flit port service on it.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/acpi/pci_root.c | 6 ++++++
drivers/pci/pcie/portdrv.c | 2 +-
drivers/pci/probe.c | 1 +
include/linux/acpi.h | 1 +
include/linux/pci.h | 1 +
5 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 4c06c3ffd0cb..e6f74b8129a3 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -136,6 +136,7 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
{ OSC_PCI_EXPRESS_CAPABILITY_CONTROL, "PCIeCapability" },
{ OSC_PCI_EXPRESS_LTR_CONTROL, "LTR" },
{ OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
+ { OSC_PCI_EXPRESS_FLIT_CONTROL, "FlitErrorLogging" },
};
static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
@@ -523,6 +524,9 @@ static u32 calculate_control(void)
if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
control |= OSC_PCI_EXPRESS_DPC_CONTROL;
+ if (IS_ENABLED(CONFIG_PCIE_FLIT))
+ control |= OSC_PCI_EXPRESS_FLIT_CONTROL;
+
return control;
}
@@ -1039,6 +1043,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
host_bridge->native_ltr = 0;
if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
host_bridge->native_dpc = 0;
+ if (!(root->osc_control_set & OSC_PCI_EXPRESS_FLIT_CONTROL))
+ host_bridge->native_flit = 0;
if (!(root->osc_ext_control_set & OSC_CXL_ERROR_REPORTING_CONTROL))
host_bridge->native_cxl_error = 0;
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 1a649d6f4ab8..ef224e7243f3 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -286,7 +286,7 @@ static int get_port_device_capability(struct pci_dev *dev)
}
#ifdef CONFIG_PCIE_FLIT
- if (dev->flit_cap)
+ if (dev->flit_cap && host->native_flit)
services |= PCIE_PORT_SERVICE_FLIT;
#endif
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a1ec44b04eb0..f9183a4ea3b3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -674,6 +674,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
bridge->native_pme = 1;
bridge->native_ltr = 1;
bridge->native_dpc = 1;
+ bridge->native_flit = 1;
bridge->domain_nr = PCI_DOMAIN_NR_NOT_SET;
bridge->native_cxl_error = 1;
bridge->dev.type = &pci_host_bridge_type;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 10d6c6c11bdf..507cd985ff14 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -644,6 +644,7 @@ extern u32 osc_sb_native_usb4_control;
#define OSC_PCI_EXPRESS_CAPABILITY_CONTROL 0x00000010
#define OSC_PCI_EXPRESS_LTR_CONTROL 0x00000020
#define OSC_PCI_EXPRESS_DPC_CONTROL 0x00000080
+#define OSC_PCI_EXPRESS_FLIT_CONTROL 0x00000800
/* CXL _OSC: Capabilities DWORD 4: Support Field */
#define OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT 0x00000001
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a8d8dbac6640..4e4d1d9eb7fe 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -660,6 +660,7 @@ struct pci_host_bridge {
unsigned int native_pme:1; /* OS may use PCIe PME */
unsigned int native_ltr:1; /* OS may use PCIe LTR */
unsigned int native_dpc:1; /* OS may use PCIe DPC */
+ unsigned int native_flit:1; /* OS may use PCIe Flit logging */
unsigned int native_cxl_error:1; /* OS may use CXL RAS/Events */
unsigned int preserve_config:1; /* Preserve FW resource setup */
unsigned int size_windows:1; /* Enable root bus sizing */
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging
2026-07-14 18:39 ` [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging Yazen Ghannam
@ 2026-07-14 19:36 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-14 19:36 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `pcie_ports=native` command line override is ignored for FLIT error logging.
- [Medium] Missing PCI error recovery callback (`slot_reset`) in `flitdriver`.
--
--- Patch [3]: [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging ---
commit 45b0bbc04c567497a5e02247081509663ead4f63
Author: Yazen Ghannam <yazen.ghannam@amd.com>
PCI/FLIT: Negotiate _OSC control for Flit Error Logging
The Flit Logging Extended Capability may be managed by the platform firmware.
This patch negotiates ownership through _OSC before the OS enables the Flit
Error Counter and registers the port service only when the OS is granted
native control.
[ ... ]
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 1a649d6f4ab8a..ef224e7243f38 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -286,7 +286,7 @@ static int get_port_device_capability(struct pci_dev *dev)
> }
>
> #ifdef CONFIG_PCIE_FLIT
> - if (dev->flit_cap)
> + if (dev->flit_cap && host->native_flit)
[Severity: Medium]
Does this check need to include the pcie_ports_native override condition?
Other port services like AER and PME check for pcie_ports_native or their
respective native control flag. Without this, the pcie_ports=native command
line parameter will be ignored for FLIT error logging if the platform
firmware did not grant native control.
> services |= PCIE_PORT_SERVICE_FLIT;
> #endif
>
[ ... ]
Additionally, looking at the port service driver definition:
drivers/pci/pcie/flit.c:flitdriver
static struct pcie_port_service_driver flitdriver = {
.name = "flit",
.port_type = PCIE_ANY_PORT,
.service = PCIE_PORT_SERVICE_FLIT,
.probe = flit_probe,
.suspend = flit_cntr_suspend,
.resume = flit_cntr_resume,
.remove = flit_cntr_remove,
};
[Severity: Medium]
Is a .slot_reset callback needed here to re-enable FLIT logging following
a slot reset?
After a PCI error recovery (e.g., AER or DPC) triggers a slot reset, the FLIT
error counter configuration is cleared by the hardware. Because the FLIT
capability is not saved and restored by the generic PCI core during reset,
error logging could remain permanently disabled for that port.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714183950.1620016-1-yazen.ghannam@amd.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] PCI/FLIT: Add trace event for flit errors
2026-07-14 18:39 [PATCH 0/5] PCIe Flit Logging Ext Capability Support Yazen Ghannam
` (2 preceding siblings ...)
2026-07-14 18:39 ` [PATCH 3/5] PCI/FLIT: Negotiate _OSC control for Flit Error Logging Yazen Ghannam
@ 2026-07-14 18:39 ` Yazen Ghannam
2026-07-14 19:16 ` sashiko-bot
2026-07-14 18:39 ` [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging Yazen Ghannam
4 siblings, 1 reply; 11+ messages in thread
From: Yazen Ghannam @ 2026-07-14 18:39 UTC (permalink / raw)
To: linux-pci; +Cc: linux-kernel, bhelgaas, Yazen Ghannam
Emit a trace event for each erroneous flit logged by the Flit Error
Counter, capturing the counter control/status and both Error Log
registers. This provides a structured, low-overhead record of flit
errors that userspace RAS tooling can consume without parsing the
kernel log.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/pci/pcie/Kconfig | 1 +
drivers/pci/pcie/flit.c | 2 ++
include/ras/ras_event.h | 48 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 1ee3c85e63c1..6a0f3abfbfe7 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -150,6 +150,7 @@ config PCIE_EDR
config PCIE_FLIT
bool "PCI Express Flit Mode Error Logging support"
depends on PCIEPORTBUS
+ select RAS
help
This enables support for the PCI Express Flit Logging Extended
Capability, which logs errors encountered by a port operating
diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
index 12f2b6fbf1de..83cf53f39291 100644
--- a/drivers/pci/pcie/flit.c
+++ b/drivers/pci/pcie/flit.c
@@ -15,6 +15,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/pci.h>
+#include <ras/ras_event.h>
#include "portdrv.h"
#include "../pci.h"
@@ -84,6 +85,7 @@ static irqreturn_t flit_isr(int irq, void *context)
pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
+ trace_flit_event(pci_name(pdev), cntr_ctrl, cntr_sta, err_log1, err_log2);
pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
} while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index fdb785fa4613..0f02928b5cda 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -381,6 +381,54 @@ TRACE_EVENT(aer_event,
"Not available")
);
#endif /* CONFIG_PCIEAER */
+
+/*
+ * PCIe Flit Error Logging trace event
+ *
+ * These events are generated when the Flit Error Counter of a PCIe Gen6
+ * Flit Mode port records one or more erroneous flits.
+ *
+ * char * dev_name - The name of the slot where the device resides
+ * ([domain:]bus:device.function).
+ * u16 cntr_ctrl - Flit Error Counter Control register value.
+ * u16 cntr_sta - Flit Error Counter Status register value.
+ * u32 err_log1 - Flit Error Log 1 register value.
+ * u32 err_log2 - Flit Error Log 2 register value.
+ */
+#ifdef CONFIG_PCIE_FLIT
+TRACE_EVENT(flit_event,
+ TP_PROTO(const char *dev_name,
+ const u16 cntr_ctrl,
+ const u16 cntr_sta,
+ const u32 err_log1,
+ const u32 err_log2),
+
+ TP_ARGS(dev_name, cntr_ctrl, cntr_sta, err_log1, err_log2),
+
+ TP_STRUCT__entry(
+ __string( dev_name, dev_name )
+ __field( u16, cntr_ctrl )
+ __field( u16, cntr_sta )
+ __field( u32, err_log1 )
+ __field( u32, err_log2 )
+ ),
+
+ TP_fast_assign(
+ __assign_str(dev_name);
+ __entry->cntr_ctrl = cntr_ctrl;
+ __entry->cntr_sta = cntr_sta;
+ __entry->err_log1 = err_log1;
+ __entry->err_log2 = err_log2;
+ ),
+
+ TP_printk("%s cntr_ctrl:0x%04x cntr_sta:0x%04x err_log1:0x%08x err_log2:0x%08x",
+ __get_str(dev_name),
+ __entry->cntr_ctrl,
+ __entry->cntr_sta,
+ __entry->err_log1,
+ __entry->err_log2)
+);
+#endif /* CONFIG_PCIE_FLIT */
#endif /* _TRACE_HW_EVENT_MC_H */
/* This part must be outside protection */
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/5] PCI/FLIT: Add trace event for flit errors
2026-07-14 18:39 ` [PATCH 4/5] PCI/FLIT: Add trace event for flit errors Yazen Ghannam
@ 2026-07-14 19:16 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-14 19:16 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Infinite loop in threaded IRQ handler due to missing check for PCI device removal/error.
--
--- Patch [4]: [PATCH 4/5] PCI/FLIT: Add trace event for flit errors ---
commit e2a5453df78634ff77f29016af766d1994cda605
Author: Yazen Ghannam <yazen.ghannam@amd.com>
PCI/FLIT: Add trace event for flit errors
This commit emits a trace event for each erroneous flit logged by the Flit
Error Counter. It captures the counter control/status and both Error Log
registers to provide a structured record for userspace RAS tooling.
> diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
> index 12f2b6fbf1de0..83cf53f392913 100644
> --- a/drivers/pci/pcie/flit.c
> +++ b/drivers/pci/pcie/flit.c
[ ... ]
> @@ -84,6 +85,7 @@ static irqreturn_t flit_isr(int irq, void *context)
>
> pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
> pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
> + trace_flit_event(pci_name(pdev), cntr_ctrl, cntr_sta, err_log1, err_log2);
>
> pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
> } while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
[Severity: High]
If the PCI device is hot-unplugged or the link goes down while flit_isr()
is executing, pci_read_config_dword() will fail and set err_log1 to all 1s
(0xFFFFFFFF).
Because 0xFFFFFFFF has all bits set, it will bypass any valid bit checks and
the loop condition (err_log1 & PCI_FLIT_ERR_LOG_MORE) will evaluate to true.
Could this trap the kernel thread in an infinite loop and flood the trace
buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714183950.1620016-1-yazen.ghannam@amd.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging
2026-07-14 18:39 [PATCH 0/5] PCIe Flit Logging Ext Capability Support Yazen Ghannam
` (3 preceding siblings ...)
2026-07-14 18:39 ` [PATCH 4/5] PCI/FLIT: Add trace event for flit errors Yazen Ghannam
@ 2026-07-14 18:39 ` Yazen Ghannam
2026-07-14 19:18 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Yazen Ghannam @ 2026-07-14 18:39 UTC (permalink / raw)
To: linux-pci; +Cc: linux-kernel, bhelgaas, Yazen Ghannam
A high rate of flit errors can flood the kernel log with error counter
and error log messages. Model the throttling on the AER capability: give
each device its own ratelimit_state, initialized in pci_flit_init() with
the default interval and burst, rather than a single static per-call-site
limiter shared across all ports.
Take one ratelimit decision per interrupt and use it to gate the console
output, so a storm on one port cannot suppress logging on another. Leave
the flit_error trace event unthrottled so RAS tooling still records every
erroneous flit while the log is limited.
Expose the interval and burst through per-device sysfs attributes
(flit_ratelimit_interval_ms, flit_ratelimit_burst), gated on CAP_SYS_ADMIN,
so the limit can be tuned or disabled at runtime.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Assisted-by: Claude:claude-opus-4-8
---
drivers/pci/pci-sysfs.c | 3 +
drivers/pci/pci.h | 1 +
drivers/pci/pcie/flit.c | 119 ++++++++++++++++++++++++++++++++++++++--
include/linux/pci.h | 2 +
4 files changed, 121 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index e985a3854f8d..e85953c249f2 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1932,6 +1932,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
&aer_stats_attr_group,
&aer_attr_group,
#endif
+#ifdef CONFIG_PCIE_FLIT
+ &flit_attr_group,
+#endif
#ifdef CONFIG_PCIEASPM
&aspm_ctrl_attr_group,
#endif
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 31d04366b980..28ff4243d125 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -896,6 +896,7 @@ static inline bool pci_dpc_recovered(struct pci_dev *pdev) { return false; }
#ifdef CONFIG_PCIE_FLIT
void pci_flit_init(struct pci_dev *pdev);
+extern const struct attribute_group flit_attr_group;
#else
static inline void pci_flit_init(struct pci_dev *pdev) { }
#endif
diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
index 83cf53f39291..7cb5aff7c552 100644
--- a/drivers/pci/pcie/flit.c
+++ b/drivers/pci/pcie/flit.c
@@ -12,9 +12,12 @@
#define pr_fmt(fmt) "Flit: " fmt
#define dev_fmt pr_fmt
+#include <linux/capability.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/pci.h>
+#include <linux/ratelimit.h>
+#include <linux/sysfs.h>
#include <ras/ras_event.h>
#include "portdrv.h"
#include "../pci.h"
@@ -30,10 +33,106 @@ void pci_flit_init(struct pci_dev *pdev)
return;
pdev->flit_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_FLIT);
- if (pdev->flit_cap)
- pci_dbg(pdev, "Flit Mode Error Logging Capability present.\n");
+ if (!pdev->flit_cap)
+ return;
+
+ ratelimit_state_init(&pdev->flit_ratelimit, DEFAULT_RATELIMIT_INTERVAL,
+ DEFAULT_RATELIMIT_BURST);
+
+ pci_dbg(pdev, "Flit Mode Error Logging Capability present.\n");
+}
+
+/*
+ * Ratelimit interval
+ * <=0: disabled with ratelimit.interval = 0
+ * >0: enabled with ratelimit.interval in ms
+ */
+static ssize_t flit_ratelimit_interval_ms_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ return sysfs_emit(buf, "%d\n",
+ jiffies_to_msecs(pdev->flit_ratelimit.interval));
+}
+
+static ssize_t flit_ratelimit_interval_ms_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int interval;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (kstrtoint(buf, 0, &interval) < 0)
+ return -EINVAL;
+
+ if (interval <= 0)
+ interval = 0;
+ else
+ interval = msecs_to_jiffies(interval);
+
+ pdev->flit_ratelimit.interval = interval;
+
+ return count;
+}
+static DEVICE_ATTR_RW(flit_ratelimit_interval_ms);
+
+static ssize_t flit_ratelimit_burst_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ return sysfs_emit(buf, "%d\n", pdev->flit_ratelimit.burst);
+}
+
+static ssize_t flit_ratelimit_burst_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int burst;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (kstrtoint(buf, 0, &burst) < 0)
+ return -EINVAL;
+
+ pdev->flit_ratelimit.burst = burst;
+
+ return count;
+}
+static DEVICE_ATTR_RW(flit_ratelimit_burst);
+
+static struct attribute *flit_attrs[] = {
+ &dev_attr_flit_ratelimit_interval_ms.attr,
+ &dev_attr_flit_ratelimit_burst.attr,
+ NULL
+};
+
+static umode_t flit_attrs_are_visible(struct kobject *kobj,
+ struct attribute *a, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ if (!pdev->flit_cap)
+ return 0;
+
+ return a->mode;
}
+const struct attribute_group flit_attr_group = {
+ .name = "flit",
+ .attrs = flit_attrs,
+ .is_visible = flit_attrs_are_visible,
+};
+
static void flit_cntr_enable(struct pci_dev *pdev)
{
u16 flit = pdev->flit_cap;
@@ -70,12 +169,22 @@ static irqreturn_t flit_isr(int irq, void *context)
u16 flit = pdev->flit_cap;
u16 cntr_ctrl, cntr_sta;
u32 err_log1, err_log2;
+ bool print;
/* Read and log Counter and Error Log Registers. */
pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_CTRL, &cntr_ctrl);
pci_read_config_word(pdev, flit + PCI_FLIT_ERR_CNTR_STA, &cntr_sta);
- pci_info(pdev, HW_ERR "Counter Control: 0x%04x Counter Status: 0x%04x\n", cntr_ctrl, cntr_sta);
+ /*
+ * Take a single ratelimit decision per interrupt and use it to gate
+ * the console output. The trace event is always emitted so RAS
+ * tooling records every erroneous flit even while the log is throttled.
+ */
+ print = __ratelimit(&pdev->flit_ratelimit);
+
+ if (print)
+ pci_info(pdev, HW_ERR "Counter Control: 0x%04x Counter Status: 0x%04x\n",
+ cntr_ctrl, cntr_sta);
do {
pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, &err_log1);
@@ -84,7 +193,9 @@ static irqreturn_t flit_isr(int irq, void *context)
break;
pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
- pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
+ if (print)
+ pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n",
+ err_log1, err_log2);
trace_flit_event(pci_name(pdev), cntr_ctrl, cntr_sta, err_log1, err_log2);
pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4e4d1d9eb7fe..01df7c84a658 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -40,6 +40,7 @@
#include <linux/io.h>
#include <linux/resource_ext.h>
#include <linux/msi_api.h>
+#include <linux/ratelimit_types.h>
#include <uapi/linux/pci.h>
#include <linux/pci_ids.h>
@@ -595,6 +596,7 @@ struct pci_dev {
#ifdef CONFIG_PCIE_FLIT
u16 flit_cap; /* Flit Logging Capabilities */
+ struct ratelimit_state flit_ratelimit; /* Flit error log ratelimit */
#endif
};
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging
2026-07-14 18:39 ` [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging Yazen Ghannam
@ 2026-07-14 19:18 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-14 19:18 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Sysfs Group Directory Leak for devices without FLIT capability.
- [Medium] Missing input validation allows negative burst values, permanently suppressing flit error logs.
Pre-existing issues:
- [High] Surprise hot-unplug or malicious endpoint causes an infinite loop in `flit_isr`, leading to a hard CPU lockup.
--
--- Patch [5]: [PATCH 5/5] PCI/FLIT: Ratelimit flit error logging ---
commit 3a5e2834e7b9f5641abd9dbcf3215773fe5a6904
Author: Yazen Ghannam <yazen.ghannam@amd.com>
PCI/FLIT: Ratelimit flit error logging
This commit introduces a per-device ratelimit state for flit error logging
to prevent console log flooding. It exposes tuning parameters via per-device
sysfs attributes and applies the ratelimiter inside the interrupt handler.
> diff --git a/drivers/pci/pcie/flit.c b/drivers/pci/pcie/flit.c
> index 83cf53f392913..7cb5aff7c552c 100644
> --- a/drivers/pci/pcie/flit.c
> +++ b/drivers/pci/pcie/flit.c
[ ... ]
> +static ssize_t flit_ratelimit_burst_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + int burst;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (kstrtoint(buf, 0, &burst) < 0)
> + return -EINVAL;
> +
> + pdev->flit_ratelimit.burst = burst;
[Severity: Medium]
Does this allow negative values to be assigned to the burst limit?
If a user writes a negative string (like "-5"), kstrtoint() will parse it
successfully into a negative integer. If burst is negative, the core
___ratelimit() check (rs->burst <= rs->printed) will permanently evaluate to
true, which would silently suppress all future flit error logs.
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(flit_ratelimit_burst);
> +
> +static struct attribute *flit_attrs[] = {
> + &dev_attr_flit_ratelimit_interval_ms.attr,
> + &dev_attr_flit_ratelimit_burst.attr,
> + NULL
> +};
> +
> +static umode_t flit_attrs_are_visible(struct kobject *kobj,
> + struct attribute *a, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + if (!pdev->flit_cap)
> + return 0;
[Severity: Medium]
Because this attribute group has a name (".name = "flit""), does returning 0
here leak an empty sysfs directory for devices without the FLIT capability?
Returning 0 suppresses the individual attributes, but the sysfs core will
still create an empty "/sys/bus/pci/devices/.../flit/" directory. Should
this return SYSFS_GROUP_INVISIBLE instead to suppress the entire directory?
> +
> + return a->mode;
> }
>
> +const struct attribute_group flit_attr_group = {
> + .name = "flit",
> + .attrs = flit_attrs,
> + .is_visible = flit_attrs_are_visible,
> +};
[ ... ]
> do {
> pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, &err_log1);
>
> if (!(err_log1 & PCI_FLIT_ERR_LOG_VALID))
> break;
>
> pci_read_config_dword(pdev, flit + PCI_FLIT_ERR_LOG2, &err_log2);
> - pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n", err_log1, err_log2);
> + if (print)
> + pci_info(pdev, HW_ERR " Error Log1: 0x%08x Error Log2: 0x%08x\n",
> + err_log1, err_log2);
> trace_flit_event(pci_name(pdev), cntr_ctrl, cntr_sta, err_log1, err_log2);
>
> pci_write_config_dword(pdev, flit + PCI_FLIT_ERR_LOG1, err_log1);
> - } while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
> + } while (err_log1 & PCI_FLIT_ERR_LOG_MORE);
[Severity: High]
This is a pre-existing issue, but is this loop safe against surprise device
hot-unplugs?
If the device is disconnected, pci_read_config_dword() will fail and assign
~0 (0xFFFFFFFF) to err_log1. Because ~0 has all bits set, the initial check
for PCI_FLIT_ERR_LOG_VALID will pass.
The loop condition checking for PCI_FLIT_ERR_LOG_MORE will also pass, creating
an unconditional infinite loop that could cause a hard CPU lockup.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714183950.1620016-1-yazen.ghannam@amd.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread