linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 00/20] Rate limit AER logs
@ 2025-05-22 23:21 Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 01/20] PCI/DPC: Initialize aer_err_info before using it Bjorn Helgaas
                   ` (20 more replies)
  0 siblings, 21 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

This work is mostly due to Jon Pan-Doh and Karolina Stolarek.  I rebased
this to v6.15-rc1, factored out some of the trace and statistics updates,
and added some minor cleanups.

I pushed this to pci/aer at
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aer
(head a524e63307cf ("PCI/AER: Add sysfs attributes for log ratelimits"))
and appended the interdiff from v7 to v8 below.

Proposal
========

When using native AER, spammy devices can flood kernel logs with AER errors
and slow/stall execution. Add per-device per-error-severity ratelimits for
more robust error logging. Allow userspace to configure ratelimits via
sysfs knobs.

Motivation
==========

Inconsistent PCIe error handling, exacerbated at datacenter scale (myriad
of devices), affects repairabilitiy flows for fleet operators.

Exposing PCIe errors/debug info in-band for a userspace daemon (e.g.
rasdaemon) to collect/pass on to repairability services will allow for more
predictable repair flows and decrease machine downtime.

Background
==========

AER error spam has been observed many times, both publicly (e.g. [1], [2],
[3]) and privately. While it usually occurs with correctable errors, it can
happen with uncorrectable errors (e.g. during new HW bringup).

There have been previous attempts to add ratelimits to AER logs ([4], [5]).
The most recent attempt[5] has many similarities with the proposed
approach.


v8:
- Rename sysfs ratelimit burst files:
    ratelimit_burst_cor_log -> correctable_ratelimit_burst (Sathy)
    ratelimit_burst_uncor_log -> nonfatal_ratelimit_burst
- Split sysfs ratelimit_log_enable for correctable and nonfatal and make it
  an interval instead of a toggle:
    ratelimit_log_enable -> correctable_ratelimit_interval_ms
                         -> nonfatal_ratelimit_interval_ms
- Rework aer_get_device_error_info() and aer_print_error() to take an index
  instead of pci_dev pointer
- Move trace_aer_event() out of pci_dev_aer_stats_incr() (Jonathan)
- Move AER_FATAL checking to aer_ratelimit() to avoid calling
  __ratelimit(nonfatal_ratelimit) when we know we don't want to ratelimit
  fatal errors (Jonathan)
- Move all Error Source ID string building into aer_print_source() instead
  of putting part in caller (Jonathan)
- Rename struct aer_err_info.ratelimit -> ratelimit_print[] (Jonathan)
- Pass printk level into pcie_print_tlp_log() (Jonathan)
- Rework Error Source ratelimiting vs detail ratelimiting (Jonathan)
v7: https://lore.kernel.org/r/20250520215047.1350603-1-helgaas@kernel.org
- Update sysfs doc target kernel version & date (Ilpo)
- Fix sysfs doc "AER ratelimiting" typo (Ilpo)
- Ratelimit Correctable and Non-Fatal but not Fatal errors (Sathy)
- Rename "struct aer_report" to "aer_info" (Sathy)
- Expand comments about combining ratelimit for multiple devices (Ilpo)
- Rework Error Source logging ratelimiting (Sathy)
- Factor out aer_isr_one_error_type() to reduce code duplication
- Log DPC errors, which are all Fatal, at KERN_ERR (Sathy)
- Improve dpc_process_error() structure (Ilpo)
v6: https://lore.kernel.org/r/20250519213603.1257897-1-helgaas@kernel.org
- Rebase to v6.15-rc1
- Initialize struct aer_err_info completely before using it
- Log DPC Error Source ID only when it's valid
- Consolidate AER Error Source ID logging to one place
- Tidy Error Source ID bus/dev/fn decoding using macros
- Rename aer_print_port_info() to aer_print_source()
- Consolidate trace events and statistic updates to one non-ratelimited place
- Save log level in struct aer_err_info instead of passing as parameter
v5: https://lore.kernel.org/r/20250321015806.954866-1-pandoh@google.com
- Handle multi-error AER by evaluating ratelimits once and storing result
- Reword/rename commit messages/functions/variable
v4: https://lore.kernel.org/r/20250320082057.622983-1-pandoh@google.com
- Fix bug where trace not emitted with malformed aer_err_info
- Extend ratelimit to malformed aer_err_info
- Update commit messages with patch motivation
- Squash AER sysfs filename change (Patch 8)
v3: https://lore.kernel.org/r/20250319084050.366718-1-pandoh@google.com
- Ratelimit aer_print_port_info() (drop Patch 1)
- Add ratelimit enable toggle
- Move trace outside of ratelimit
- Split log level (Patch 2) into two
- More descriptive documentation/sysfs naming
v2: https://lore.kernel.org/r/20250214023543.992372-1-pandoh@google.com
- Rebased on top of pci/aer (6.14.rc-1)
- Split series into log and IRQ ratelimits (defer patch 5)
- Dropped patch 8 (Move AER sysfs)
- Added log level cleanup patch[7] from Karolina's series
- Fixed bug where dpc errors didn't increment counters
- "X callbacks suppressed" message on ratelimit release -> immediately
- Separate documentation into own patch
v1: https://lore.kernel.org/r/20250115074301.3514927-1-pandoh@google.com

[1] https://bugzilla.kernel.org/show_bug.cgi?id=215027
[2] https://bugzilla.kernel.org/show_bug.cgi?id=201517
[3] https://bugzilla.kernel.org/show_bug.cgi?id=196183
[4] https://lore.kernel.org/linux-pci/20230606035442.2886343-2-grundler@chromium.org/
[5] https://lore.kernel.org/linux-pci/cover.1736341506.git.karolina.stolarek@oracle.com/
[6]
https://lore.kernel.org/linux-pci/8bcb8c9a7b38ce3bdaca5a64fe76f08b0b337511.1742202797.git.k
arolina.stolarek@oracle.com/
[7]
https://lore.kernel.org/linux-pci/edd77011aafad4c0654358a26b4e538d0c5a321d.1736341506.git.k
arolina.stolarek@oracle.com/



Bjorn Helgaas (13):
  PCI/DPC: Initialize aer_err_info before using it
  PCI/DPC: Log Error Source ID only when valid
  PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error()
  PCI/AER: Consolidate Error Source ID logging in
    aer_isr_one_error_type()
  PCI/AER: Extract bus/dev/fn in aer_print_port_info() with
    PCI_BUS_NUM(), etc
  PCI/AER: Move aer_print_source() earlier in file
  PCI/AER: Initialize aer_err_info before using it
  PCI/AER: Simplify pci_print_aer()
  PCI/AER: Update statistics before ratelimiting
  PCI/AER: Trace error event before ratelimiting
  PCI/ERR: Add printk level to pcie_print_tlp_log()
  PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to
    index
  PCI/AER: Simplify add_error_device()

Jon Pan-Doh (4):
  PCI/AER: Rename aer_print_port_info() to aer_print_source()
  PCI/AER: Ratelimit correctable and non-fatal error logging
  PCI/AER: Add ratelimits to PCI AER Documentation
  PCI/AER: Add sysfs attributes for log ratelimits

Karolina Stolarek (3):
  PCI/AER: Check log level once and remember it
  PCI/AER: Reduce pci_print_aer() correctable error level to
    KERN_WARNING
  PCI/AER: Rename struct aer_stats to aer_info

 ...es-aer_stats => sysfs-bus-pci-devices-aer} |  44 ++
 Documentation/PCI/pcieaer-howto.rst           |  17 +-
 drivers/pci/pci-sysfs.c                       |   1 +
 drivers/pci/pci.h                             |  13 +-
 drivers/pci/pcie/aer.c                        | 441 +++++++++++++-----
 drivers/pci/pcie/dpc.c                        |  73 +--
 drivers/pci/pcie/tlp.c                        |   6 +-
 include/linux/pci.h                           |   2 +-
 8 files changed, 430 insertions(+), 167 deletions(-)
 rename Documentation/ABI/testing/{sysfs-bus-pci-devices-aer_stats => sysfs-bus-pci-devices-aer} (72%)

-- 
2.43.0


diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
index 01bb577bfee8..5ed284523956 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
@@ -126,28 +126,38 @@ They represent configurable ratelimits of logs per error type.
 
 See Documentation/PCI/pcieaer-howto.rst for more info on ratelimits.
 
-What:		/sys/bus/pci/devices/<dev>/aer/ratelimit_log_enable
+What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_interval_ms
 Date:		May 2025
 KernelVersion:	6.16.0
-Contact:	linux-pci@vger.kernel.org, pandoh@google.com
-Description:	Writing 1/0 enables/disables AER log ratelimiting. Reading
-		gets whether or not AER ratelimiting is currently enabled.
-		Enabled by default.
+Contact:	linux-pci@vger.kernel.org
+Description:	Writing 0 disables AER correctable error log ratelimiting.
+		Writing a positive value sets the ratelimit interval in ms.
+		Default is DEFAULT_RATELIMIT_INTERVAL (5000 ms).
 
-What:		/sys/bus/pci/devices/<dev>/aer/ratelimit_burst_cor_log
+What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_burst
 Date:		May 2025
 KernelVersion:	6.16.0
-Contact:	linux-pci@vger.kernel.org, pandoh@google.com
+Contact:	linux-pci@vger.kernel.org
 Description:	Ratelimit burst for correctable error logs. Writing a value
 		changes the number of errors (burst) allowed per interval
-		(5 second window) before ratelimiting. Reading gets the
-		current ratelimit burst.
+		before ratelimiting. Reading gets the current ratelimit
+		burst. Default is DEFAULT_RATELIMIT_BURST (10).
 
-What:		/sys/bus/pci/devices/<dev>/aer/ratelimit_burst_uncor_log
+What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_interval_ms
 Date:		May 2025
 KernelVersion:	6.16.0
-Contact:	linux-pci@vger.kernel.org, pandoh@google.com
+Contact:	linux-pci@vger.kernel.org
+Description:	Writing 0 disables AER non-fatal uncorrectable error log
+		ratelimiting. Writing a positive value sets the ratelimit
+		interval in ms. Default is DEFAULT_RATELIMIT_INTERVAL
+		(5000 ms).
+
+What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_burst
+Date:		May 2025
+KernelVersion:	6.16.0
+Contact:	linux-pci@vger.kernel.org
 Description:	Ratelimit burst for non-fatal uncorrectable error logs.
 		Writing a value changes the number of errors (burst)
-		allowed per interval (5 second window) before ratelimiting.
-		Reading gets the current ratelimit burst.
+		allowed per interval before ratelimiting. Reading gets the
+		current ratelimit burst. Default is DEFAULT_RATELIMIT_BURST
+		(10).
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a3261e842d6d..eca2812cfd25 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -587,13 +587,14 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
 
 struct aer_err_info {
 	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
+	int ratelimit_print[AER_MAX_MULTI_ERR_DEVICES];
 	int error_dev_num;
 	const char *level;		/* printk level */
 
 	unsigned int id:16;
 
 	unsigned int severity:2;	/* 0:NONFATAL | 1:FATAL | 2:COR */
-	unsigned int ratelimit:1;	/* 0=skip, 1=print */
+	unsigned int root_ratelimit_print:1;	/* 0=skip, 1=print */
 	unsigned int __pad1:4;
 	unsigned int multi_error_valid:1;
 
@@ -606,15 +607,16 @@ struct aer_err_info {
 	struct pcie_tlp_log tlp;	/* TLP Header */
 };
 
-int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
-void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
+int aer_get_device_error_info(struct aer_err_info *info, int i);
+void aer_print_error(struct aer_err_info *info, int i);
 
 int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
 		      unsigned int tlp_len, bool flit,
 		      struct pcie_tlp_log *log);
 unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc);
 void pcie_print_tlp_log(const struct pci_dev *dev,
-			const struct pcie_tlp_log *log, const char *pfx);
+			const struct pcie_tlp_log *log, const char *level,
+			const char *pfx);
 #endif	/* CONFIG_PCIEAER */
 
 #ifdef CONFIG_PCIEPORTBUS
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 9b8dea317a79..6c331695af58 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -91,8 +91,8 @@ struct aer_info {
 	u64 rootport_total_nonfatal_errs;
 
 	/* Ratelimits for errors */
-	struct ratelimit_state cor_log_ratelimit;
-	struct ratelimit_state uncor_log_ratelimit;
+	struct ratelimit_state correctable_ratelimit;
+	struct ratelimit_state nonfatal_ratelimit;
 };
 
 #define AER_LOG_TLP_MASKS		(PCI_ERR_UNC_POISON_TLP|	\
@@ -384,9 +384,9 @@ void pci_aer_init(struct pci_dev *dev)
 
 	dev->aer_info = kzalloc(sizeof(*dev->aer_info), GFP_KERNEL);
 
-	ratelimit_state_init(&dev->aer_info->cor_log_ratelimit,
+	ratelimit_state_init(&dev->aer_info->correctable_ratelimit,
 			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
-	ratelimit_state_init(&dev->aer_info->uncor_log_ratelimit,
+	ratelimit_state_init(&dev->aer_info->nonfatal_ratelimit,
 			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
 
 	/*
@@ -628,83 +628,89 @@ const struct attribute_group aer_stats_attr_group = {
 };
 
 /*
- * Ratelimit enable toggle
- * 0: disabled with ratelimit.interval = 0
- * 1: enabled with ratelimit.interval = nonzero
+ * Ratelimit interval
+ * <=0: disabled with ratelimit.interval = 0
+ * >0: enabled with ratelimit.interval in ms
  */
-static ssize_t ratelimit_log_enable_show(struct device *dev,
-					 struct device_attribute *attr,
-					 char *buf)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	bool enabled = pdev->aer_info->cor_log_ratelimit.interval != 0;
-
-	return sysfs_emit(buf, "%d\n", enabled);
-}
-
-static ssize_t ratelimit_log_enable_store(struct device *dev,
-					  struct device_attribute *attr,
-					  const char *buf, size_t count)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	bool enable;
-	int interval;
-
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	if (kstrtobool(buf, &enable) < 0)
-		return -EINVAL;
-
-	if (enable)
-		interval = DEFAULT_RATELIMIT_INTERVAL;
-	else
-		interval = 0;
-
-	pdev->aer_info->cor_log_ratelimit.interval = interval;
-	pdev->aer_info->uncor_log_ratelimit.interval = interval;
-
-	return count;
-}
-static DEVICE_ATTR_RW(ratelimit_log_enable);
+#define aer_ratelimit_interval_attr(name, ratelimit)			\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.interval);	\
+	}								\
+									\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.interval = interval;		\
+									\
+		return count;						\
+	}								\
+	static DEVICE_ATTR_RW(name);
 
 #define aer_ratelimit_burst_attr(name, ratelimit)			\
 	static ssize_t							\
 	name##_show(struct device *dev, struct device_attribute *attr,	\
 		    char *buf)						\
-{									\
-	struct pci_dev *pdev = to_pci_dev(dev);				\
+	{								\
+		struct pci_dev *pdev = to_pci_dev(dev);			\
 									\
-	return sysfs_emit(buf, "%d\n",					\
-			  pdev->aer_info->ratelimit.burst);		\
-}									\
+		return sysfs_emit(buf, "%d\n",				\
+				  pdev->aer_info->ratelimit.burst);	\
+	}								\
 									\
 	static ssize_t							\
 	name##_store(struct device *dev, struct device_attribute *attr,	\
 		     const char *buf, size_t count)			\
-{									\
-	struct pci_dev *pdev = to_pci_dev(dev);				\
-	int burst;							\
+	{								\
+		struct pci_dev *pdev = to_pci_dev(dev);			\
+		int burst;						\
 									\
-	if (!capable(CAP_SYS_ADMIN))					\
-		return -EPERM;						\
+		if (!capable(CAP_SYS_ADMIN))				\
+			return -EPERM;					\
 									\
-	if (kstrtoint(buf, 0, &burst) < 0)				\
-		return -EINVAL;						\
+		if (kstrtoint(buf, 0, &burst) < 0)			\
+			return -EINVAL;					\
 									\
-	pdev->aer_info->ratelimit.burst = burst;			\
+		pdev->aer_info->ratelimit.burst = burst;		\
 									\
-	return count;							\
-}									\
-static DEVICE_ATTR_RW(name)
+		return count;						\
+	}								\
+	static DEVICE_ATTR_RW(name);
 
-aer_ratelimit_burst_attr(ratelimit_burst_cor_log, cor_log_ratelimit);
-aer_ratelimit_burst_attr(ratelimit_burst_uncor_log, uncor_log_ratelimit);
+#define aer_ratelimit_attrs(name)					\
+	aer_ratelimit_interval_attr(name##_ratelimit_interval_ms,	\
+				    name##_ratelimit)			\
+	aer_ratelimit_burst_attr(name##_ratelimit_burst,		\
+				 name##_ratelimit)
+
+aer_ratelimit_attrs(correctable)
+aer_ratelimit_attrs(nonfatal)
 
 static struct attribute *aer_attrs[] = {
-	&dev_attr_ratelimit_log_enable.attr,
-	&dev_attr_ratelimit_burst_cor_log.attr,
-	&dev_attr_ratelimit_burst_uncor_log.attr,
+	&dev_attr_correctable_ratelimit_interval_ms.attr,
+	&dev_attr_correctable_ratelimit_burst.attr,
+	&dev_attr_nonfatal_ratelimit_interval_ms.attr,
+	&dev_attr_nonfatal_ratelimit_burst.attr,
 	NULL
 };
 
@@ -734,9 +740,6 @@ static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
 	u64 *counter = NULL;
 	struct aer_info *aer_info = pdev->aer_info;
 
-	trace_aer_event(pci_name(pdev), (info->status & ~info->mask),
-			info->severity, info->tlp_header_valid, &info->tlp);
-
 	if (!aer_info)
 		return;
 
@@ -785,10 +788,13 @@ static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
 {
 	struct ratelimit_state *ratelimit;
 
+	if (severity == AER_FATAL)
+		return 1;	/* AER_FATAL not ratelimited */
+
 	if (severity == AER_CORRECTABLE)
-		ratelimit = &dev->aer_info->cor_log_ratelimit;
+		ratelimit = &dev->aer_info->correctable_ratelimit;
 	else
-		ratelimit = &dev->aer_info->uncor_log_ratelimit;
+		ratelimit = &dev->aer_info->nonfatal_ratelimit;
 
 	return __ratelimit(ratelimit);
 }
@@ -817,7 +823,7 @@ static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 }
 
 static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
-			     const char *details)
+			     bool found)
 {
 	u16 source = info->id;
 
@@ -825,18 +831,27 @@ static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
 		 info->multi_error_valid ? "Multiple " : "",
 		 aer_error_severity_string[info->severity],
 		 pci_domain_nr(dev->bus), PCI_BUS_NUM(source),
-		 PCI_SLOT(source), PCI_FUNC(source), details);
+		 PCI_SLOT(source), PCI_FUNC(source),
+		 found ? "" : " (no details found");
 }
 
-void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
+void aer_print_error(struct aer_err_info *info, int i)
 {
-	int layer, agent;
-	int id = pci_dev_id(dev);
+	struct pci_dev *dev;
+	int layer, agent, id;
 	const char *level = info->level;
 
-	pci_dev_aer_stats_incr(dev, info);
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return;
 
-	if (!info->ratelimit)
+	dev = info->dev[i];
+	id = pci_dev_id(dev);
+
+	pci_dev_aer_stats_incr(dev, info);
+	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
+			info->severity, info->tlp_header_valid, &info->tlp);
+
+	if (!info->ratelimit_print[i])
 		return;
 
 	if (!info->status) {
@@ -858,7 +873,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	__aer_print_error(dev, info);
 
 	if (info->tlp_header_valid)
-		pcie_print_tlp_log(dev, &info->tlp, dev_fmt("  "));
+		pcie_print_tlp_log(dev, &info->tlp, level, dev_fmt("  "));
 
 out:
 	if (info->id && info->error_dev_num > 1 && info->id == id)
@@ -903,11 +918,10 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 
 	info.status = status;
 	info.mask = mask;
-	info.tlp_header_valid = tlp_header_valid;
-	if (tlp_header_valid)
-		info.tlp = aer->header_log;
 
 	pci_dev_aer_stats_incr(dev, &info);
+	trace_aer_event(pci_name(dev), (status & ~mask),
+			aer_severity, tlp_header_valid, &aer->header_log);
 
 	if (!aer_ratelimit(dev, info.severity))
 		return;
@@ -925,13 +939,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 		aer_printk(info.level, dev, "aer_uncor_severity: 0x%08x\n",
 			   aer->uncor_severity);
 
-	/*
-	 * pcie_print_tlp_log() uses KERN_ERR, but we only call it when
-	 * tlp_header_valid is set, and info.level is always KERN_ERR in
-	 * that case.
-	 */
 	if (tlp_header_valid)
-		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
+		pcie_print_tlp_log(dev, &aer->header_log, info.level,
+				   dev_fmt("  "));
 }
 EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
 
@@ -942,23 +952,27 @@ EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
  */
 static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
 {
+	int i = e_info->error_dev_num;
+
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return -ENOSPC;
+
+	e_info->dev[i] = pci_dev_get(dev);
+	e_info->error_dev_num++;
+
 	/*
 	 * Ratelimit AER log messages.  "dev" is either the source
 	 * identified by the root's Error Source ID or it has an unmasked
-	 * error logged in its own AER Capability.  If any of these devices
-	 * has not reached its ratelimit, log messages for all of them.
-	 * Messages are emitted when "e_info->ratelimit" is non-zero.
-	 *
-	 * Note that "e_info->ratelimit" was already initialized to 1 for the
-	 * ERR_FATAL case.
+	 * error logged in its own AER Capability.  Messages are emitted
+	 * when "ratelimit_print[i]" is non-zero.  If we will print detail
+	 * for a downstream device, make sure we print the Error Source ID
+	 * from the root as well.
 	 */
-	if (e_info->error_dev_num < AER_MAX_MULTI_ERR_DEVICES) {
-		e_info->dev[e_info->error_dev_num] = pci_dev_get(dev);
-		e_info->ratelimit |= aer_ratelimit(dev, e_info->severity);
-		e_info->error_dev_num++;
-		return 0;
+	if (aer_ratelimit(dev, e_info->severity)) {
+		e_info->ratelimit_print[i] = 1;
+		e_info->root_ratelimit_print = 1;
 	}
-	return -ENOSPC;
+	return 0;
 }
 
 /**
@@ -1337,19 +1351,26 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
 
 /**
  * aer_get_device_error_info - read error status from dev and store it to info
- * @dev: pointer to the device expected to have an error record
  * @info: pointer to structure to store the error record
+ * @i: index into info->dev[]
  *
  * Return: 1 on success, 0 on error.
  *
  * Note that @info is reused among all error devices. Clear fields properly.
  */
-int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
+int aer_get_device_error_info(struct aer_err_info *info, int i)
 {
-	int type = pci_pcie_type(dev);
-	int aer = dev->aer_cap;
+	struct pci_dev *dev;
+	int type, aer;
 	u32 aercc;
 
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return 0;
+
+	dev = info->dev[i];
+	aer = dev->aer_cap;
+	type = pci_pcie_type(dev);
+
 	/* Must reset in this function */
 	info->status = 0;
 	info->tlp_header_valid = 0;
@@ -1401,11 +1422,11 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
 
 	/* Report all before handling them, to not lose records by reset etc. */
 	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
-		if (aer_get_device_error_info(e_info->dev[i], e_info))
-			aer_print_error(e_info->dev[i], e_info);
+		if (aer_get_device_error_info(e_info, i))
+			aer_print_error(e_info, i);
 	}
 	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
-		if (aer_get_device_error_info(e_info->dev[i], e_info))
+		if (aer_get_device_error_info(e_info, i))
 			handle_error_source(e_info->dev[i], e_info);
 	}
 }
@@ -1425,17 +1446,18 @@ static void aer_isr_one_error_type(struct pci_dev *root,
 
 	/*
 	 * If we're going to log error messages, we've already set
-	 * "info->ratelimit" to non-zero (which enables printing) because
-	 * this is either an ERR_FATAL or we found a device with an error
-	 * logged in its AER Capability.
+	 * "info->root_ratelimit_print" and "info->ratelimit_print[i]" to
+	 * non-zero (which enables printing) because this is either an
+	 * ERR_FATAL or we found a device with an error logged in its AER
+	 * Capability.
 	 *
 	 * If we didn't find the Error Source device, at least log the
 	 * Requester ID from the ERR_* Message received by the Root Port or
 	 * RCEC, ratelimited by the RP or RCEC.
 	 */
-	if (info->ratelimit ||
+	if (info->root_ratelimit_print ||
 	    (!found && aer_ratelimit(root, info->severity)))
-		aer_print_source(root, info, found ? "" : " (no details found");
+		aer_print_source(root, info, found);
 
 	if (found)
 		aer_process_err_devices(info);
@@ -1470,14 +1492,12 @@ static void aer_isr_one_error(struct pci_dev *root,
 		aer_isr_one_error_type(root, &e_info);
 	}
 
-	/* Note that messages for ERR_FATAL are never ratelimited */
 	if (status & PCI_ERR_ROOT_UNCOR_RCV) {
 		int fatal = status & PCI_ERR_ROOT_FATAL_RCV;
 		int multi = status & PCI_ERR_ROOT_MULTI_UNCOR_RCV;
 		struct aer_err_info e_info = {
 			.id = ERR_UNCOR_ID(e_src->id),
 			.severity = fatal ? AER_FATAL : AER_NONFATAL,
-			.ratelimit = fatal ? 1 : 0,
 			.level = KERN_ERR,
 			.multi_error_valid = multi ? 1 : 0,
 		};
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 530c5e2cf7e8..fc18349614d7 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -222,7 +222,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
 			  dpc_tlp_log_len(pdev),
 			  pdev->subordinate->flit_mode,
 			  &tlp_log);
-	pcie_print_tlp_log(pdev, &tlp_log, dev_fmt(""));
+	pcie_print_tlp_log(pdev, &tlp_log, KERN_ERR, dev_fmt(""));
 
 	if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG + 1)
 		goto clear_status;
@@ -253,6 +253,10 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
 		info->severity = AER_NONFATAL;
 
 	info->level = KERN_ERR;
+
+	info->dev[0] = dev;
+	info->error_dev_num = 1;
+
 	return 1;
 }
 
@@ -270,9 +274,8 @@ void dpc_process_error(struct pci_dev *pdev)
 		pci_warn(pdev, "containment event, status:%#06x: unmasked uncorrectable error detected\n",
 			 status);
 		if (dpc_get_aer_uncorrect_severity(pdev, &info) &&
-		    aer_get_device_error_info(pdev, &info)) {
-			info.ratelimit = 1;	/* ERR_FATAL; no ratelimit */
-			aer_print_error(pdev, &info);
+		    aer_get_device_error_info(&info, 0)) {
+			aer_print_error(&info, 0);
 			pci_aer_clear_nonfatal_status(pdev);
 			pci_aer_clear_fatal_status(pdev);
 		}
diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
index 890d5391d7f5..71f8fc9ea2ed 100644
--- a/drivers/pci/pcie/tlp.c
+++ b/drivers/pci/pcie/tlp.c
@@ -98,12 +98,14 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
  * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
  * @dev: PCIe device
  * @log: TLP Log structure
+ * @level: Printk log level
  * @pfx: String prefix
  *
  * Prints TLP Header and Prefix Log information held by @log.
  */
 void pcie_print_tlp_log(const struct pci_dev *dev,
-			const struct pcie_tlp_log *log, const char *pfx)
+			const struct pcie_tlp_log *log, const char *level,
+			const char *pfx)
 {
 	/* EE_PREFIX_STR fits the extended DW space needed for the Flit mode */
 	char buf[11 * PCIE_STD_MAX_TLP_HEADERLOG + 1];
@@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
 		}
 	}
 
-	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
+	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
 		log->flit ? " (Flit)" : "", buf);
 }

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 01/20] PCI/DPC: Initialize aer_err_info before using it
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 02/20] PCI/DPC: Log Error Source ID only when valid Bjorn Helgaas
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

Previously the struct aer_err_info "info" was allocated on the stack
without being initialized, so it contained junk except for the fields we
explicitly set later.

Initialize "info" at declaration so it starts as all zeros.

Fixes: 8aefa9b0d910 ("PCI/DPC: Print AER status in DPC event handling")
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-2-helgaas@kernel.org
---
 drivers/pci/pcie/dpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index df42f15c9829..3daaf61c79c9 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -258,7 +258,7 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
 void dpc_process_error(struct pci_dev *pdev)
 {
 	u16 cap = pdev->dpc_cap, status, source, reason, ext_reason;
-	struct aer_err_info info;
+	struct aer_err_info info = {};
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 02/20] PCI/DPC: Log Error Source ID only when valid
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 01/20] PCI/DPC: Initialize aer_err_info before using it Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 03/20] PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error() Bjorn Helgaas
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

DPC Error Source ID is only valid when the DPC Trigger Reason indicates
that DPC was triggered due to reception of an ERR_NONFATAL or ERR_FATAL
Message (PCIe r6.0, sec 7.9.14.5).

When DPC was triggered by ERR_NONFATAL (PCI_EXP_DPC_STATUS_TRIGGER_RSN_NFE)
or ERR_FATAL (PCI_EXP_DPC_STATUS_TRIGGER_RSN_FE) from a downstream device,
log the Error Source ID (decoded into domain/bus/device/function).  Don't
print the source otherwise, since it's not valid.

For DPC trigger due to reception of ERR_NONFATAL or ERR_FATAL, the dmesg
logging changes:

  - pci 0000:00:01.0: DPC: containment event, status:0x000d source:0x0200
  - pci 0000:00:01.0: DPC: ERR_FATAL detected
  + pci 0000:00:01.0: DPC: containment event, status:0x000d, ERR_FATAL received from 0000:02:00.0

and when DPC triggered for other reasons, where DPC Error Source ID is
undefined, e.g., unmasked uncorrectable error:

  - pci 0000:00:01.0: DPC: containment event, status:0x0009 source:0x0200
  - pci 0000:00:01.0: DPC: unmasked uncorrectable error detected
  + pci 0000:00:01.0: DPC: containment event, status:0x0009: unmasked uncorrectable error detected

Previously the "containment event" message was at KERN_INFO and the
"%s detected" message was at KERN_WARNING.  Now the single message is at
KERN_WARNING.

Fixes: 26e515713342 ("PCI: Add Downstream Port Containment driver")
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20250520215047.1350603-3-helgaas@kernel.org
---
 drivers/pci/pcie/dpc.c | 64 ++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 3daaf61c79c9..9d85f1b3b761 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -261,37 +261,45 @@ void dpc_process_error(struct pci_dev *pdev)
 	struct aer_err_info info = {};
 
 	pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
-	pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source);
-
-	pci_info(pdev, "containment event, status:%#06x source:%#06x\n",
-		 status, source);
 
 	reason = status & PCI_EXP_DPC_STATUS_TRIGGER_RSN;
-	ext_reason = status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT;
-	pci_warn(pdev, "%s detected\n",
-		 (reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_UNCOR) ?
-		 "unmasked uncorrectable error" :
-		 (reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_NFE) ?
-		 "ERR_NONFATAL" :
-		 (reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_FE) ?
-		 "ERR_FATAL" :
-		 (ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_RP_PIO) ?
-		 "RP PIO error" :
-		 (ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_SW_TRIGGER) ?
-		 "software trigger" :
-		 "reserved error");
 
-	/* show RP PIO error detail information */
-	if (pdev->dpc_rp_extensions &&
-	    reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_IN_EXT &&
-	    ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_RP_PIO)
-		dpc_process_rp_pio_error(pdev);
-	else if (reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_UNCOR &&
-		 dpc_get_aer_uncorrect_severity(pdev, &info) &&
-		 aer_get_device_error_info(pdev, &info)) {
-		aer_print_error(pdev, &info);
-		pci_aer_clear_nonfatal_status(pdev);
-		pci_aer_clear_fatal_status(pdev);
+	switch (reason) {
+	case PCI_EXP_DPC_STATUS_TRIGGER_RSN_UNCOR:
+		pci_warn(pdev, "containment event, status:%#06x: unmasked uncorrectable error detected\n",
+			 status);
+		if (dpc_get_aer_uncorrect_severity(pdev, &info) &&
+		    aer_get_device_error_info(pdev, &info)) {
+			aer_print_error(pdev, &info);
+			pci_aer_clear_nonfatal_status(pdev);
+			pci_aer_clear_fatal_status(pdev);
+		}
+		break;
+	case PCI_EXP_DPC_STATUS_TRIGGER_RSN_NFE:
+	case PCI_EXP_DPC_STATUS_TRIGGER_RSN_FE:
+		pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID,
+				     &source);
+		pci_warn(pdev, "containment event, status:%#06x, %s received from %04x:%02x:%02x.%d\n",
+			 status,
+			 (reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_FE) ?
+				"ERR_FATAL" : "ERR_NONFATAL",
+			 pci_domain_nr(pdev->bus), PCI_BUS_NUM(source),
+			 PCI_SLOT(source), PCI_FUNC(source));
+		break;
+	case PCI_EXP_DPC_STATUS_TRIGGER_RSN_IN_EXT:
+		ext_reason = status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT;
+		pci_warn(pdev, "containment event, status:%#06x: %s detected\n",
+			 status,
+			 (ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_RP_PIO) ?
+			 "RP PIO error" :
+			 (ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_SW_TRIGGER) ?
+			 "software trigger" :
+			 "reserved error");
+		/* show RP PIO error detail information */
+		if (ext_reason == PCI_EXP_DPC_STATUS_TRIGGER_RSN_RP_PIO &&
+		    pdev->dpc_rp_extensions)
+			dpc_process_rp_pio_error(pdev);
+		break;
 	}
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 03/20] PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 01/20] PCI/DPC: Initialize aer_err_info before using it Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 02/20] PCI/DPC: Log Error Source ID only when valid Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 04/20] PCI/AER: Consolidate Error Source ID logging in aer_isr_one_error_type() Bjorn Helgaas
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

aer_isr_one_error() duplicates the Error Source ID logging and AER error
processing for Correctable Errors and Uncorrectable Errors.  Factor out the
duplicated code to aer_isr_one_error_type().

aer_isr_one_error() doesn't need the struct aer_rpc pointer, so pass it the
Root Port or RCEC pci_dev pointer instead.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Link: https://patch.msgid.link/20250520215047.1350603-4-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index a1cf8c7ef628..568229288ca3 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1273,17 +1273,32 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
 }
 
 /**
- * aer_isr_one_error - consume an error detected by Root Port
- * @rpc: pointer to the Root Port which holds an error
+ * aer_isr_one_error_type - consume a Correctable or Uncorrectable Error
+ *			    detected by Root Port or RCEC
+ * @root: pointer to Root Port or RCEC that signaled AER interrupt
+ * @info: pointer to AER error info
+ */
+static void aer_isr_one_error_type(struct pci_dev *root,
+				   struct aer_err_info *info)
+{
+	aer_print_port_info(root, info);
+
+	if (find_source_device(root, info))
+		aer_process_err_devices(info);
+}
+
+/**
+ * aer_isr_one_error - consume error(s) signaled by an AER interrupt from
+ *		       Root Port or RCEC
+ * @root: pointer to Root Port or RCEC that signaled AER interrupt
  * @e_src: pointer to an error source
  */
-static void aer_isr_one_error(struct aer_rpc *rpc,
+static void aer_isr_one_error(struct pci_dev *root,
 		struct aer_err_source *e_src)
 {
-	struct pci_dev *pdev = rpc->rpd;
 	struct aer_err_info e_info;
 
-	pci_rootport_aer_stats_incr(pdev, e_src);
+	pci_rootport_aer_stats_incr(root, e_src);
 
 	/*
 	 * There is a possibility that both correctable error and
@@ -1297,10 +1312,8 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
 			e_info.multi_error_valid = 1;
 		else
 			e_info.multi_error_valid = 0;
-		aer_print_port_info(pdev, &e_info);
 
-		if (find_source_device(pdev, &e_info))
-			aer_process_err_devices(&e_info);
+		aer_isr_one_error_type(root, &e_info);
 	}
 
 	if (e_src->status & PCI_ERR_ROOT_UNCOR_RCV) {
@@ -1316,10 +1329,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
 		else
 			e_info.multi_error_valid = 0;
 
-		aer_print_port_info(pdev, &e_info);
-
-		if (find_source_device(pdev, &e_info))
-			aer_process_err_devices(&e_info);
+		aer_isr_one_error_type(root, &e_info);
 	}
 }
 
@@ -1340,7 +1350,7 @@ static irqreturn_t aer_isr(int irq, void *context)
 		return IRQ_NONE;
 
 	while (kfifo_get(&rpc->aer_fifo, &e_src))
-		aer_isr_one_error(rpc, &e_src);
+		aer_isr_one_error(rpc->rpd, &e_src);
 	return IRQ_HANDLED;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 04/20] PCI/AER: Consolidate Error Source ID logging in aer_isr_one_error_type()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (2 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 03/20] PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 05/20] PCI/AER: Extract bus/dev/fn in aer_print_port_info() with PCI_BUS_NUM(), etc Bjorn Helgaas
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Previously we decoded the AER Error Source ID in aer_isr_one_error_type(),
then again in find_source_device() if we didn't find any devices with
errors logged in their AER Capabilities.

Consolidate this so we only decode and log the Error Source ID once in
aer_isr_one_error_type().  Add a "found" parameter so we can add a note
when we didn't find any downstream devices with errors logged in their AER
Capability.

This changes the dmesg logging when we found no devices with errors logged:

  - pci 0000:00:01.0: AER: Correctable error message received from 0000:02:00.0
  - pci 0000:00:01.0: AER: found no error details for 0000:02:00.0
  + pci 0000:00:01.0: AER: Correctable error message received from 0000:02:00.0 (no details found)

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20250520215047.1350603-5-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 568229288ca3..fe6d323306a0 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -733,16 +733,17 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 			info->severity, info->tlp_header_valid, &info->tlp);
 }
 
-static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
+static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info,
+				bool found)
 {
 	u8 bus = info->id >> 8;
 	u8 devfn = info->id & 0xff;
 
-	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d\n",
+	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d%s\n",
 		 info->multi_error_valid ? "Multiple " : "",
 		 aer_error_severity_string[info->severity],
 		 pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn),
-		 PCI_FUNC(devfn));
+		 PCI_FUNC(devfn), found ? "" : " (no details found");
 }
 
 #ifdef CONFIG_ACPI_APEI_PCIEAER
@@ -926,15 +927,8 @@ static bool find_source_device(struct pci_dev *parent,
 	else
 		pci_walk_bus(parent->subordinate, find_device_iter, e_info);
 
-	if (!e_info->error_dev_num) {
-		u8 bus = e_info->id >> 8;
-		u8 devfn = e_info->id & 0xff;
-
-		pci_info(parent, "found no error details for %04x:%02x:%02x.%d\n",
-			 pci_domain_nr(parent->bus), bus, PCI_SLOT(devfn),
-			 PCI_FUNC(devfn));
+	if (!e_info->error_dev_num)
 		return false;
-	}
 	return true;
 }
 
@@ -1281,9 +1275,11 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
 static void aer_isr_one_error_type(struct pci_dev *root,
 				   struct aer_err_info *info)
 {
-	aer_print_port_info(root, info);
+	bool found;
 
-	if (find_source_device(root, info))
+	found = find_source_device(root, info);
+	aer_print_port_info(root, info, found);
+	if (found)
 		aer_process_err_devices(info);
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 05/20] PCI/AER: Extract bus/dev/fn in aer_print_port_info() with PCI_BUS_NUM(), etc
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (3 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 04/20] PCI/AER: Consolidate Error Source ID logging in aer_isr_one_error_type() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 06/20] PCI/AER: Rename aer_print_port_info() to aer_print_source() Bjorn Helgaas
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

Use PCI_BUS_NUM(), PCI_SLOT(), PCI_FUNC() to extract the bus number,
device, and function number directly from the Error Source ID.  There's no
need to shift and mask it explicitly.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-6-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index fe6d323306a0..18005615d376 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -736,14 +736,14 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info,
 				bool found)
 {
-	u8 bus = info->id >> 8;
-	u8 devfn = info->id & 0xff;
+	u16 source = info->id;
 
 	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d%s\n",
 		 info->multi_error_valid ? "Multiple " : "",
 		 aer_error_severity_string[info->severity],
-		 pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn),
-		 PCI_FUNC(devfn), found ? "" : " (no details found");
+		 pci_domain_nr(dev->bus), PCI_BUS_NUM(source),
+		 PCI_SLOT(source), PCI_FUNC(source),
+		 found ? "" : " (no details found");
 }
 
 #ifdef CONFIG_ACPI_APEI_PCIEAER
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 06/20] PCI/AER: Rename aer_print_port_info() to aer_print_source()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (4 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 05/20] PCI/AER: Extract bus/dev/fn in aer_print_port_info() with PCI_BUS_NUM(), etc Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 07/20] PCI/AER: Move aer_print_source() earlier in file Bjorn Helgaas
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Krzysztof Wilczyński,
	Bjorn Helgaas

From: Jon Pan-Doh <pandoh@google.com>

Rename aer_print_port_info() to aer_print_source() to be more descriptive.
This logs the Error Source ID logged by a Root Port or Root Complex Event
Collector when it receives an ERR_COR, ERR_NONFATAL, or ERR_FATAL Message.

[bhelgaas: aer_print_rp_info() -> aer_print_source()]

Signed-off-by: Jon Pan-Doh <pandoh@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250520215047.1350603-7-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 18005615d376..8b23ef90345b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -733,8 +733,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 			info->severity, info->tlp_header_valid, &info->tlp);
 }
 
-static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info,
-				bool found)
+static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
+			     bool found)
 {
 	u16 source = info->id;
 
@@ -1278,7 +1278,7 @@ static void aer_isr_one_error_type(struct pci_dev *root,
 	bool found;
 
 	found = find_source_device(root, info);
-	aer_print_port_info(root, info, found);
+	aer_print_source(root, info, found);
 	if (found)
 		aer_process_err_devices(info);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 07/20] PCI/AER: Move aer_print_source() earlier in file
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (5 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 06/20] PCI/AER: Rename aer_print_port_info() to aer_print_source() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 08/20] PCI/AER: Initialize aer_err_info before using it Bjorn Helgaas
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

Move aer_print_source() earlier in the file so a future change can use it
from aer_print_error(), where it's easier to rate limit it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-8-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 8b23ef90345b..c0481550363b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -696,6 +696,19 @@ static void __aer_print_error(struct pci_dev *dev,
 	pci_dev_aer_stats_incr(dev, info);
 }
 
+static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
+			     bool found)
+{
+	u16 source = info->id;
+
+	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d%s\n",
+		 info->multi_error_valid ? "Multiple " : "",
+		 aer_error_severity_string[info->severity],
+		 pci_domain_nr(dev->bus), PCI_BUS_NUM(source),
+		 PCI_SLOT(source), PCI_FUNC(source),
+		 found ? "" : " (no details found");
+}
+
 void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int layer, agent;
@@ -733,19 +746,6 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 			info->severity, info->tlp_header_valid, &info->tlp);
 }
 
-static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
-			     bool found)
-{
-	u16 source = info->id;
-
-	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d%s\n",
-		 info->multi_error_valid ? "Multiple " : "",
-		 aer_error_severity_string[info->severity],
-		 pci_domain_nr(dev->bus), PCI_BUS_NUM(source),
-		 PCI_SLOT(source), PCI_FUNC(source),
-		 found ? "" : " (no details found");
-}
-
 #ifdef CONFIG_ACPI_APEI_PCIEAER
 int cper_severity_to_aer(int cper_severity)
 {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 08/20] PCI/AER: Initialize aer_err_info before using it
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (6 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 07/20] PCI/AER: Move aer_print_source() earlier in file Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 09/20] PCI/AER: Simplify pci_print_aer() Bjorn Helgaas
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

Previously the struct aer_err_info "e_info" was allocated on the stack
without being initialized, so it contained junk except for the fields we
explicitly set later.

Initialize "e_info" at declaration with a designated initializer list,
which initializes the other members to zero.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-9-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index c0481550363b..109f5e0ade8a 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1290,9 +1290,9 @@ static void aer_isr_one_error_type(struct pci_dev *root,
  * @e_src: pointer to an error source
  */
 static void aer_isr_one_error(struct pci_dev *root,
-		struct aer_err_source *e_src)
+			      struct aer_err_source *e_src)
 {
-	struct aer_err_info e_info;
+	u32 status = e_src->status;
 
 	pci_rootport_aer_stats_incr(root, e_src);
 
@@ -1300,30 +1300,25 @@ static void aer_isr_one_error(struct pci_dev *root,
 	 * There is a possibility that both correctable error and
 	 * uncorrectable error being logged. Report correctable error first.
 	 */
-	if (e_src->status & PCI_ERR_ROOT_COR_RCV) {
-		e_info.id = ERR_COR_ID(e_src->id);
-		e_info.severity = AER_CORRECTABLE;
-
-		if (e_src->status & PCI_ERR_ROOT_MULTI_COR_RCV)
-			e_info.multi_error_valid = 1;
-		else
-			e_info.multi_error_valid = 0;
+	if (status & PCI_ERR_ROOT_COR_RCV) {
+		int multi = status & PCI_ERR_ROOT_MULTI_COR_RCV;
+		struct aer_err_info e_info = {
+			.id = ERR_COR_ID(e_src->id),
+			.severity = AER_CORRECTABLE,
+			.multi_error_valid = multi ? 1 : 0,
+		};
 
 		aer_isr_one_error_type(root, &e_info);
 	}
 
-	if (e_src->status & PCI_ERR_ROOT_UNCOR_RCV) {
-		e_info.id = ERR_UNCOR_ID(e_src->id);
-
-		if (e_src->status & PCI_ERR_ROOT_FATAL_RCV)
-			e_info.severity = AER_FATAL;
-		else
-			e_info.severity = AER_NONFATAL;
-
-		if (e_src->status & PCI_ERR_ROOT_MULTI_UNCOR_RCV)
-			e_info.multi_error_valid = 1;
-		else
-			e_info.multi_error_valid = 0;
+	if (status & PCI_ERR_ROOT_UNCOR_RCV) {
+		int fatal = status & PCI_ERR_ROOT_FATAL_RCV;
+		int multi = status & PCI_ERR_ROOT_MULTI_UNCOR_RCV;
+		struct aer_err_info e_info = {
+			.id = ERR_UNCOR_ID(e_src->id),
+			.severity = fatal ? AER_FATAL : AER_NONFATAL,
+			.multi_error_valid = multi ? 1 : 0,
+		};
 
 		aer_isr_one_error_type(root, &e_info);
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 09/20] PCI/AER: Simplify pci_print_aer()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (7 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 08/20] PCI/AER: Initialize aer_err_info before using it Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 10/20] PCI/AER: Update statistics before ratelimiting Bjorn Helgaas
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

Simplify pci_print_aer() by initializing the struct aer_err_info "info"
with a designated initializer list (it was previously initialized with
memset()) and using pci_name().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Link: https://patch.msgid.link/20250520215047.1350603-10-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 109f5e0ade8a..dc5f4bebd613 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -766,7 +766,10 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 {
 	int layer, agent, tlp_header_valid = 0;
 	u32 status, mask;
-	struct aer_err_info info;
+	struct aer_err_info info = {
+		.severity = aer_severity,
+		.first_error = PCI_ERR_CAP_FEP(aer->cap_control),
+	};
 
 	if (aer_severity == AER_CORRECTABLE) {
 		status = aer->cor_status;
@@ -777,14 +780,11 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 		tlp_header_valid = status & AER_LOG_TLP_MASKS;
 	}
 
-	layer = AER_GET_LAYER_ERROR(aer_severity, status);
-	agent = AER_GET_AGENT(aer_severity, status);
-
-	memset(&info, 0, sizeof(info));
-	info.severity = aer_severity;
 	info.status = status;
 	info.mask = mask;
-	info.first_error = PCI_ERR_CAP_FEP(aer->cap_control);
+
+	layer = AER_GET_LAYER_ERROR(aer_severity, status);
+	agent = AER_GET_AGENT(aer_severity, status);
 
 	pci_err(dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n", status, mask);
 	__aer_print_error(dev, &info);
@@ -798,7 +798,7 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	if (tlp_header_valid)
 		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
 
-	trace_aer_event(dev_name(&dev->dev), (status & ~mask),
+	trace_aer_event(pci_name(dev), (status & ~mask),
 			aer_severity, tlp_header_valid, &aer->header_log);
 }
 EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 10/20] PCI/AER: Update statistics before ratelimiting
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (8 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 09/20] PCI/AER: Simplify pci_print_aer() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 11/20] PCI/AER: Trace error event " Bjorn Helgaas
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

There are two AER logging entry points:

  - aer_print_error() is used by DPC (dpc_process_error()) and native AER
    handling (aer_process_err_devices()).

  - pci_print_aer() is used by GHES (aer_recover_work_func()) and CXL
    (cxl_handle_rdport_errors())

Both use __aer_print_error() to print the AER error bits.  Previously
__aer_print_error() also incremented the AER statistics via
pci_dev_aer_stats_incr().

Call pci_dev_aer_stats_incr() early in the entry points instead of in
__aer_print_error() so we update the statistics even if the actual printing
of error bits is rate limited by a future change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-11-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index dc5f4bebd613..53f460bb7e6c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -693,7 +693,6 @@ static void __aer_print_error(struct pci_dev *dev,
 		aer_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
 				info->first_error == i ? " (First)" : "");
 	}
-	pci_dev_aer_stats_incr(dev, info);
 }
 
 static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
@@ -715,6 +714,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	int id = pci_dev_id(dev);
 	const char *level;
 
+	pci_dev_aer_stats_incr(dev, info);
+
 	if (!info->status) {
 		pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
 			aer_error_severity_string[info->severity]);
@@ -783,6 +784,8 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	info.status = status;
 	info.mask = mask;
 
+	pci_dev_aer_stats_incr(dev, &info);
+
 	layer = AER_GET_LAYER_ERROR(aer_severity, status);
 	agent = AER_GET_AGENT(aer_severity, status);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 11/20] PCI/AER: Trace error event before ratelimiting
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (9 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 10/20] PCI/AER: Update statistics before ratelimiting Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 12/20] PCI/AER: Check log level once and remember it Bjorn Helgaas
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Bjorn Helgaas <bhelgaas@google.com>

As with the AER statistics, we always want to emit trace events, even if
the actual dmesg logging is rate limited.

Call trace_aer_event() immediately after pci_dev_aer_stats_incr() so both
happen before ratelimiting.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Link: https://patch.msgid.link/20250520215047.1350603-12-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 53f460bb7e6c..636bcd92afa1 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -715,6 +715,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	const char *level;
 
 	pci_dev_aer_stats_incr(dev, info);
+	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
+			info->severity, info->tlp_header_valid, &info->tlp);
 
 	if (!info->status) {
 		pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
@@ -742,9 +744,6 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 out:
 	if (info->id && info->error_dev_num > 1 && info->id == id)
 		pci_err(dev, "  Error of this Agent is reported first\n");
-
-	trace_aer_event(dev_name(&dev->dev), (info->status & ~info->mask),
-			info->severity, info->tlp_header_valid, &info->tlp);
 }
 
 #ifdef CONFIG_ACPI_APEI_PCIEAER
@@ -785,6 +784,8 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	info.mask = mask;
 
 	pci_dev_aer_stats_incr(dev, &info);
+	trace_aer_event(pci_name(dev), (status & ~mask),
+			aer_severity, tlp_header_valid, &aer->header_log);
 
 	layer = AER_GET_LAYER_ERROR(aer_severity, status);
 	agent = AER_GET_AGENT(aer_severity, status);
@@ -800,9 +801,6 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 
 	if (tlp_header_valid)
 		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
-
-	trace_aer_event(pci_name(dev), (status & ~mask),
-			aer_severity, tlp_header_valid, &aer->header_log);
 }
 EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 12/20] PCI/AER: Check log level once and remember it
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (10 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 11/20] PCI/AER: Trace error event " Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Krzysztof Wilczyński,
	Bjorn Helgaas

From: Karolina Stolarek <karolina.stolarek@oracle.com>

When reporting an AER error, we check its type multiple times to determine
the log level for each message. Do this check only in the top-level
functions (aer_isr_one_error(), pci_print_aer()) and save the level in
struct aer_err_info.

[bhelgaas: save log level in struct aer_err_info instead of passing it
as a parameter]

Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250520215047.1350603-13-helgaas@kernel.org
---
 drivers/pci/pci.h      |  1 +
 drivers/pci/pcie/aer.c | 21 ++++++++++-----------
 drivers/pci/pcie/dpc.c |  1 +
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b81e99cd4b62..705f9ef58acc 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -588,6 +588,7 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
 struct aer_err_info {
 	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
 	int error_dev_num;
+	const char *level;		/* printk level */
 
 	unsigned int id:16;
 
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 636bcd92afa1..f80c78846a14 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -669,21 +669,18 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
 	}
 }
 
-static void __aer_print_error(struct pci_dev *dev,
-			      struct aer_err_info *info)
+static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	const char **strings;
 	unsigned long status = info->status & ~info->mask;
-	const char *level, *errmsg;
+	const char *level = info->level;
+	const char *errmsg;
 	int i;
 
-	if (info->severity == AER_CORRECTABLE) {
+	if (info->severity == AER_CORRECTABLE)
 		strings = aer_correctable_error_string;
-		level = KERN_WARNING;
-	} else {
+	else
 		strings = aer_uncorrectable_error_string;
-		level = KERN_ERR;
-	}
 
 	for_each_set_bit(i, &status, 32) {
 		errmsg = strings[i];
@@ -712,7 +709,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int layer, agent;
 	int id = pci_dev_id(dev);
-	const char *level;
+	const char *level = info->level;
 
 	pci_dev_aer_stats_incr(dev, info);
 	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
@@ -727,8 +724,6 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	layer = AER_GET_LAYER_ERROR(info->severity, info->status);
 	agent = AER_GET_AGENT(info->severity, info->status);
 
-	level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
-
 	aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
 		   aer_error_severity_string[info->severity],
 		   aer_error_layer[layer], aer_agent_string[agent]);
@@ -774,9 +769,11 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	if (aer_severity == AER_CORRECTABLE) {
 		status = aer->cor_status;
 		mask = aer->cor_mask;
+		info.level = KERN_WARNING;
 	} else {
 		status = aer->uncor_status;
 		mask = aer->uncor_mask;
+		info.level = KERN_ERR;
 		tlp_header_valid = status & AER_LOG_TLP_MASKS;
 	}
 
@@ -1306,6 +1303,7 @@ static void aer_isr_one_error(struct pci_dev *root,
 		struct aer_err_info e_info = {
 			.id = ERR_COR_ID(e_src->id),
 			.severity = AER_CORRECTABLE,
+			.level = KERN_WARNING,
 			.multi_error_valid = multi ? 1 : 0,
 		};
 
@@ -1318,6 +1316,7 @@ static void aer_isr_one_error(struct pci_dev *root,
 		struct aer_err_info e_info = {
 			.id = ERR_UNCOR_ID(e_src->id),
 			.severity = fatal ? AER_FATAL : AER_NONFATAL,
+			.level = KERN_ERR,
 			.multi_error_valid = multi ? 1 : 0,
 		};
 
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 9d85f1b3b761..6c98fabdba57 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -252,6 +252,7 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
 	else
 		info->severity = AER_NONFATAL;
 
+	info->level = KERN_ERR;
 	return 1;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (11 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 12/20] PCI/AER: Check log level once and remember it Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:44   ` Sathyanarayanan Kuppuswamy
                     ` (2 more replies)
  2025-05-22 23:21 ` [PATCH v8 14/20] PCI/AER: Reduce pci_print_aer() correctable error level to KERN_WARNING Bjorn Helgaas
                   ` (7 subsequent siblings)
  20 siblings, 3 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

aer_print_error() produces output at a printk level (KERN_ERR/KERN_WARNING/
etc) that depends on the kind of error, and it calls pcie_print_tlp_log(),
which previously always produced output at KERN_ERR.

Add a "level" parameter so aer_print_error() can control the level of the
pcie_print_tlp_log() output to match.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.h      | 3 ++-
 drivers/pci/pcie/aer.c | 5 +++--
 drivers/pci/pcie/dpc.c | 2 +-
 drivers/pci/pcie/tlp.c | 6 ++++--
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 705f9ef58acc..1a9bfc708757 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -613,7 +613,8 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
 		      struct pcie_tlp_log *log);
 unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc);
 void pcie_print_tlp_log(const struct pci_dev *dev,
-			const struct pcie_tlp_log *log, const char *pfx);
+			const struct pcie_tlp_log *log, const char *level,
+			const char *pfx);
 #endif	/* CONFIG_PCIEAER */
 
 #ifdef CONFIG_PCIEPORTBUS
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index f80c78846a14..f0936759ba8b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -734,7 +734,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	__aer_print_error(dev, info);
 
 	if (info->tlp_header_valid)
-		pcie_print_tlp_log(dev, &info->tlp, dev_fmt("  "));
+		pcie_print_tlp_log(dev, &info->tlp, level, dev_fmt("  "));
 
 out:
 	if (info->id && info->error_dev_num > 1 && info->id == id)
@@ -797,7 +797,8 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 			aer->uncor_severity);
 
 	if (tlp_header_valid)
-		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
+		pcie_print_tlp_log(dev, &aer->header_log, info.level,
+				   dev_fmt("  "));
 }
 EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
 
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 6c98fabdba57..7ae1590ea1da 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -222,7 +222,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
 			  dpc_tlp_log_len(pdev),
 			  pdev->subordinate->flit_mode,
 			  &tlp_log);
-	pcie_print_tlp_log(pdev, &tlp_log, dev_fmt(""));
+	pcie_print_tlp_log(pdev, &tlp_log, KERN_ERR, dev_fmt(""));
 
 	if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG + 1)
 		goto clear_status;
diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
index 890d5391d7f5..71f8fc9ea2ed 100644
--- a/drivers/pci/pcie/tlp.c
+++ b/drivers/pci/pcie/tlp.c
@@ -98,12 +98,14 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
  * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
  * @dev: PCIe device
  * @log: TLP Log structure
+ * @level: Printk log level
  * @pfx: String prefix
  *
  * Prints TLP Header and Prefix Log information held by @log.
  */
 void pcie_print_tlp_log(const struct pci_dev *dev,
-			const struct pcie_tlp_log *log, const char *pfx)
+			const struct pcie_tlp_log *log, const char *level,
+			const char *pfx)
 {
 	/* EE_PREFIX_STR fits the extended DW space needed for the Flit mode */
 	char buf[11 * PCIE_STD_MAX_TLP_HEADERLOG + 1];
@@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
 		}
 	}
 
-	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
+	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
 		log->flit ? " (Flit)" : "", buf);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 14/20] PCI/AER: Reduce pci_print_aer() correctable error level to KERN_WARNING
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (12 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 15/20] PCI/AER: Rename struct aer_stats to aer_info Bjorn Helgaas
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Karolina Stolarek <karolina.stolarek@oracle.com>

Some existing logs in pci_print_aer() log with error severity by default.

Convert them to use KERN_WARNING for correctable errors and KERN_ERR for
uncorrectable errors.

[bhelgaas: commit log]
Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-14-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index f0936759ba8b..16779f281b2f 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -787,14 +787,15 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	layer = AER_GET_LAYER_ERROR(aer_severity, status);
 	agent = AER_GET_AGENT(aer_severity, status);
 
-	pci_err(dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n", status, mask);
+	aer_printk(info.level, dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
+		   status, mask);
 	__aer_print_error(dev, &info);
-	pci_err(dev, "aer_layer=%s, aer_agent=%s\n",
-		aer_error_layer[layer], aer_agent_string[agent]);
+	aer_printk(info.level, dev, "aer_layer=%s, aer_agent=%s\n",
+		   aer_error_layer[layer], aer_agent_string[agent]);
 
 	if (aer_severity != AER_CORRECTABLE)
-		pci_err(dev, "aer_uncor_severity: 0x%08x\n",
-			aer->uncor_severity);
+		aer_printk(info.level, dev, "aer_uncor_severity: 0x%08x\n",
+			   aer->uncor_severity);
 
 	if (tlp_header_valid)
 		pcie_print_tlp_log(dev, &aer->header_log, info.level,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 15/20] PCI/AER: Rename struct aer_stats to aer_info
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (13 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 14/20] PCI/AER: Reduce pci_print_aer() correctable error level to KERN_WARNING Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index Bjorn Helgaas
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas,
	Krzysztof Wilczyński

From: Karolina Stolarek <karolina.stolarek@oracle.com>

Update name to reflect the broader definition of structs/variables that are
stored (e.g. ratelimits). This is a preparatory patch for adding rate limit
support.

[bhelgaas: "aer_report" -> "aer_info"]

Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://patch.msgid.link/20250520215047.1350603-15-helgaas@kernel.org
---
 drivers/pci/pcie/aer.c | 46 +++++++++++++++++++++---------------------
 include/linux/pci.h    |  2 +-
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 16779f281b2f..787a953fb331 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -54,8 +54,8 @@ struct aer_rpc {
 	DECLARE_KFIFO(aer_fifo, struct aer_err_source, AER_ERROR_SOURCES_MAX);
 };
 
-/* AER stats for the device */
-struct aer_stats {
+/* AER info for the device */
+struct aer_info {
 
 	/*
 	 * Fields for all AER capable devices. They indicate the errors
@@ -377,7 +377,7 @@ void pci_aer_init(struct pci_dev *dev)
 	if (!dev->aer_cap)
 		return;
 
-	dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL);
+	dev->aer_info = kzalloc(sizeof(*dev->aer_info), GFP_KERNEL);
 
 	/*
 	 * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
@@ -398,8 +398,8 @@ void pci_aer_init(struct pci_dev *dev)
 
 void pci_aer_exit(struct pci_dev *dev)
 {
-	kfree(dev->aer_stats);
-	dev->aer_stats = NULL;
+	kfree(dev->aer_info);
+	dev->aer_info = NULL;
 }
 
 #define AER_AGENT_RECEIVER		0
@@ -537,10 +537,10 @@ static const char *aer_agent_string[] = {
 {									\
 	unsigned int i;							\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
-	u64 *stats = pdev->aer_stats->stats_array;			\
+	u64 *stats = pdev->aer_info->stats_array;			\
 	size_t len = 0;							\
 									\
-	for (i = 0; i < ARRAY_SIZE(pdev->aer_stats->stats_array); i++) {\
+	for (i = 0; i < ARRAY_SIZE(pdev->aer_info->stats_array); i++) {	\
 		if (strings_array[i])					\
 			len += sysfs_emit_at(buf, len, "%s %llu\n",	\
 					     strings_array[i],		\
@@ -551,7 +551,7 @@ static const char *aer_agent_string[] = {
 					     i, stats[i]);		\
 	}								\
 	len += sysfs_emit_at(buf, len, "TOTAL_%s %llu\n", total_string,	\
-			     pdev->aer_stats->total_field);		\
+			     pdev->aer_info->total_field);		\
 	return len;							\
 }									\
 static DEVICE_ATTR_RO(name)
@@ -572,7 +572,7 @@ aer_stats_dev_attr(aer_dev_nonfatal, dev_nonfatal_errs,
 		     char *buf)						\
 {									\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
-	return sysfs_emit(buf, "%llu\n", pdev->aer_stats->field);	\
+	return sysfs_emit(buf, "%llu\n", pdev->aer_info->field);	\
 }									\
 static DEVICE_ATTR_RO(name)
 
@@ -599,7 +599,7 @@ static umode_t aer_stats_attrs_are_visible(struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	if (!pdev->aer_stats)
+	if (!pdev->aer_info)
 		return 0;
 
 	if ((a == &dev_attr_aer_rootport_total_err_cor.attr ||
@@ -623,25 +623,25 @@ static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
 	unsigned long status = info->status & ~info->mask;
 	int i, max = -1;
 	u64 *counter = NULL;
-	struct aer_stats *aer_stats = pdev->aer_stats;
+	struct aer_info *aer_info = pdev->aer_info;
 
-	if (!aer_stats)
+	if (!aer_info)
 		return;
 
 	switch (info->severity) {
 	case AER_CORRECTABLE:
-		aer_stats->dev_total_cor_errs++;
-		counter = &aer_stats->dev_cor_errs[0];
+		aer_info->dev_total_cor_errs++;
+		counter = &aer_info->dev_cor_errs[0];
 		max = AER_MAX_TYPEOF_COR_ERRS;
 		break;
 	case AER_NONFATAL:
-		aer_stats->dev_total_nonfatal_errs++;
-		counter = &aer_stats->dev_nonfatal_errs[0];
+		aer_info->dev_total_nonfatal_errs++;
+		counter = &aer_info->dev_nonfatal_errs[0];
 		max = AER_MAX_TYPEOF_UNCOR_ERRS;
 		break;
 	case AER_FATAL:
-		aer_stats->dev_total_fatal_errs++;
-		counter = &aer_stats->dev_fatal_errs[0];
+		aer_info->dev_total_fatal_errs++;
+		counter = &aer_info->dev_fatal_errs[0];
 		max = AER_MAX_TYPEOF_UNCOR_ERRS;
 		break;
 	}
@@ -653,19 +653,19 @@ static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
 static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
 				 struct aer_err_source *e_src)
 {
-	struct aer_stats *aer_stats = pdev->aer_stats;
+	struct aer_info *aer_info = pdev->aer_info;
 
-	if (!aer_stats)
+	if (!aer_info)
 		return;
 
 	if (e_src->status & PCI_ERR_ROOT_COR_RCV)
-		aer_stats->rootport_total_cor_errs++;
+		aer_info->rootport_total_cor_errs++;
 
 	if (e_src->status & PCI_ERR_ROOT_UNCOR_RCV) {
 		if (e_src->status & PCI_ERR_ROOT_FATAL_RCV)
-			aer_stats->rootport_total_fatal_errs++;
+			aer_info->rootport_total_fatal_errs++;
 		else
-			aer_stats->rootport_total_nonfatal_errs++;
+			aer_info->rootport_total_nonfatal_errs++;
 	}
 }
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0e8e3fd77e96..81a81dbfc873 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -346,7 +346,7 @@ struct pci_dev {
 	u8		hdr_type;	/* PCI header type (`multi' flag masked out) */
 #ifdef CONFIG_PCIEAER
 	u16		aer_cap;	/* AER capability offset */
-	struct aer_stats *aer_stats;	/* AER stats for this device */
+	struct aer_info	*aer_info;	/* AER info for this device */
 #endif
 #ifdef CONFIG_PCIEPORTBUS
 	struct rcec_ea	*rcec_ea;	/* RCEC cached endpoint association */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (14 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 15/20] PCI/AER: Rename struct aer_stats to aer_info Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:58   ` Sathyanarayanan Kuppuswamy
  2025-05-23 11:13   ` Ilpo Järvinen
  2025-05-22 23:21 ` [PATCH v8 17/20] PCI/AER: Simplify add_error_device() Bjorn Helgaas
                   ` (4 subsequent siblings)
  20 siblings, 2 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Previously aer_get_device_error_info() and aer_print_error() took a pointer
to struct aer_err_info and a pointer to a pci_dev.  Typically the pci_dev
was one of the elements of the aer_err_info.dev[] array (DPC was an
exception, where the dev[] array was unused).

Convert aer_get_device_error_info() and aer_print_error() to take an index
into the aer_err_info.dev[] array instead.  A future patch will add
per-device ratelimit information, so the index makes it convenient to find
the ratelimit associated with the device.

To accommodate DPC, set info->dev[0] to the DPC port before using these
interfaces.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.h      |  4 ++--
 drivers/pci/pcie/aer.c | 33 +++++++++++++++++++++++----------
 drivers/pci/pcie/dpc.c |  8 ++++++--
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 1a9bfc708757..e1a28215967f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -605,8 +605,8 @@ struct aer_err_info {
 	struct pcie_tlp_log tlp;	/* TLP Header */
 };
 
-int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
-void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
+int aer_get_device_error_info(struct aer_err_info *info, int i);
+void aer_print_error(struct aer_err_info *info, int i);
 
 int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
 		      unsigned int tlp_len, bool flit,
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 787a953fb331..237741e66d28 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -705,12 +705,18 @@ static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
 		 found ? "" : " (no details found");
 }
 
-void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
+void aer_print_error(struct aer_err_info *info, int i)
 {
-	int layer, agent;
-	int id = pci_dev_id(dev);
+	struct pci_dev *dev;
+	int layer, agent, id;
 	const char *level = info->level;
 
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return;
+
+	dev = info->dev[i];
+	id = pci_dev_id(dev);
+
 	pci_dev_aer_stats_incr(dev, info);
 	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
 			info->severity, info->tlp_header_valid, &info->tlp);
@@ -1193,19 +1199,26 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
 
 /**
  * aer_get_device_error_info - read error status from dev and store it to info
- * @dev: pointer to the device expected to have an error record
  * @info: pointer to structure to store the error record
+ * @i: index into info->dev[]
  *
  * Return: 1 on success, 0 on error.
  *
  * Note that @info is reused among all error devices. Clear fields properly.
  */
-int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
+int aer_get_device_error_info(struct aer_err_info *info, int i)
 {
-	int type = pci_pcie_type(dev);
-	int aer = dev->aer_cap;
+	struct pci_dev *dev;
+	int type, aer;
 	u32 aercc;
 
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return 0;
+
+	dev = info->dev[i];
+	aer = dev->aer_cap;
+	type = pci_pcie_type(dev);
+
 	/* Must reset in this function */
 	info->status = 0;
 	info->tlp_header_valid = 0;
@@ -1257,11 +1270,11 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
 
 	/* Report all before handling them, to not lose records by reset etc. */
 	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
-		if (aer_get_device_error_info(e_info->dev[i], e_info))
-			aer_print_error(e_info->dev[i], e_info);
+		if (aer_get_device_error_info(e_info, i))
+			aer_print_error(e_info, i);
 	}
 	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
-		if (aer_get_device_error_info(e_info->dev[i], e_info))
+		if (aer_get_device_error_info(e_info, i))
 			handle_error_source(e_info->dev[i], e_info);
 	}
 }
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 7ae1590ea1da..fc18349614d7 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -253,6 +253,10 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
 		info->severity = AER_NONFATAL;
 
 	info->level = KERN_ERR;
+
+	info->dev[0] = dev;
+	info->error_dev_num = 1;
+
 	return 1;
 }
 
@@ -270,8 +274,8 @@ void dpc_process_error(struct pci_dev *pdev)
 		pci_warn(pdev, "containment event, status:%#06x: unmasked uncorrectable error detected\n",
 			 status);
 		if (dpc_get_aer_uncorrect_severity(pdev, &info) &&
-		    aer_get_device_error_info(pdev, &info)) {
-			aer_print_error(pdev, &info);
+		    aer_get_device_error_info(&info, 0)) {
+			aer_print_error(&info, 0);
 			pci_aer_clear_nonfatal_status(pdev);
 			pci_aer_clear_fatal_status(pdev);
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 17/20] PCI/AER: Simplify add_error_device()
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (15 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:57   ` Sathyanarayanan Kuppuswamy
  2025-05-23 11:14   ` Ilpo Järvinen
  2025-05-22 23:21 ` [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging Bjorn Helgaas
                   ` (3 subsequent siblings)
  20 siblings, 2 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Return -ENOSPC error early so the usual path through add_error_device() is
the straightline code.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/aer.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 237741e66d28..24f0f5c55256 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -816,12 +816,15 @@ EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
  */
 static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
 {
-	if (e_info->error_dev_num < AER_MAX_MULTI_ERR_DEVICES) {
-		e_info->dev[e_info->error_dev_num] = pci_dev_get(dev);
-		e_info->error_dev_num++;
-		return 0;
-	}
-	return -ENOSPC;
+	int i = e_info->error_dev_num;
+
+	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+		return -ENOSPC;
+
+	e_info->dev[i] = pci_dev_get(dev);
+	e_info->error_dev_num++;
+
+	return 0;
 }
 
 /**
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (16 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 17/20] PCI/AER: Simplify add_error_device() Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:56   ` Sathyanarayanan Kuppuswamy
  2025-08-01 13:16   ` Breno Leitao
  2025-05-22 23:21 ` [PATCH v8 19/20] PCI/AER: Add ratelimits to PCI AER Documentation Bjorn Helgaas
                   ` (2 subsequent siblings)
  20 siblings, 2 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Jon Pan-Doh <pandoh@google.com>

Spammy devices can flood kernel logs with AER errors and slow/stall
execution. Add per-device ratelimits for AER correctable and non-fatal
uncorrectable errors that use the kernel defaults (10 per 5s).  Logging of
fatal errors is not ratelimited.

There are two AER logging entry points:

  - aer_print_error() is used by DPC and native AER

  - pci_print_aer() is used by GHES and CXL

The native AER aer_print_error() case includes a loop that may log details
from multiple devices, which are ratelimited individually.  If we log
details for any device, we also log the Error Source ID from the Root Port
or RCEC.

If no such device details are found, we still log the Error Source from the
ERR_* Message, ratelimited by the Root Port or RCEC that received it.

The DPC aer_print_error() case is not ratelimited, since this only happens
for fatal errors.

The CXL pci_print_aer() case is ratelimited by the Error Source device.

The GHES pci_print_aer() case is via aer_recover_work_func(), which
searches for the Error Source device.  If the device is not found, there's
no per-device ratelimit, so we use a system-wide ratelimit that covers all
error types (correctable, non-fatal, and fatal).

Sargun at Meta reported internally that a flood of AER errors causes RCU
CPU stall warnings and CSD-lock warnings.

Tested using aer-inject[1]. Sent 11 AER errors. Observed 10 errors logged
while AER stats (cat /sys/bus/pci/devices/<dev>/aer_dev_correctable) show
true count of 11.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/gong.chen/aer-inject.git

[bhelgaas: commit log, factor out trace_aer_event() and aer_print_rp_info()
changes to previous patches, enable Error Source logging if any downstream
detail will be printed, don't ratelimit fatal errors, "aer_report" ->
"aer_info", "cor_log_ratelimit" -> "correctable_ratelimit",
"uncor_log_ratelimit" -> "nonfatal_ratelimit"]

Reported-by: Sargun Dhillon <sargun@meta.com>
Signed-off-by: Jon Pan-Doh <pandoh@google.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250520215047.1350603-16-helgaas@kernel.org
---
 drivers/pci/pci.h      |  4 ++-
 drivers/pci/pcie/aer.c | 69 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e1a28215967f..3023c68fe485 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -587,13 +587,15 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
 
 struct aer_err_info {
 	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
+	int ratelimit_print[AER_MAX_MULTI_ERR_DEVICES];
 	int error_dev_num;
 	const char *level;		/* printk level */
 
 	unsigned int id:16;
 
 	unsigned int severity:2;	/* 0:NONFATAL | 1:FATAL | 2:COR */
-	unsigned int __pad1:5;
+	unsigned int root_ratelimit_print:1;	/* 0=skip, 1=print */
+	unsigned int __pad1:4;
 	unsigned int multi_error_valid:1;
 
 	unsigned int first_error:5;
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 24f0f5c55256..ebac126144fc 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -28,6 +28,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/kfifo.h>
+#include <linux/ratelimit.h>
 #include <linux/slab.h>
 #include <acpi/apei.h>
 #include <acpi/ghes.h>
@@ -88,6 +89,10 @@ struct aer_info {
 	u64 rootport_total_cor_errs;
 	u64 rootport_total_fatal_errs;
 	u64 rootport_total_nonfatal_errs;
+
+	/* Ratelimits for errors */
+	struct ratelimit_state correctable_ratelimit;
+	struct ratelimit_state nonfatal_ratelimit;
 };
 
 #define AER_LOG_TLP_MASKS		(PCI_ERR_UNC_POISON_TLP|	\
@@ -379,6 +384,11 @@ void pci_aer_init(struct pci_dev *dev)
 
 	dev->aer_info = kzalloc(sizeof(*dev->aer_info), GFP_KERNEL);
 
+	ratelimit_state_init(&dev->aer_info->correctable_ratelimit,
+			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
+	ratelimit_state_init(&dev->aer_info->nonfatal_ratelimit,
+			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
+
 	/*
 	 * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
 	 * PCI_ERR_COR_MASK, and PCI_ERR_CAP.  Root and Root Complex Event
@@ -669,6 +679,21 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
 	}
 }
 
+static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
+{
+	struct ratelimit_state *ratelimit;
+
+	if (severity == AER_FATAL)
+		return 1;	/* AER_FATAL not ratelimited */
+
+	if (severity == AER_CORRECTABLE)
+		ratelimit = &dev->aer_info->correctable_ratelimit;
+	else
+		ratelimit = &dev->aer_info->nonfatal_ratelimit;
+
+	return __ratelimit(ratelimit);
+}
+
 static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	const char **strings;
@@ -721,6 +746,9 @@ void aer_print_error(struct aer_err_info *info, int i)
 	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
 			info->severity, info->tlp_header_valid, &info->tlp);
 
+	if (!info->ratelimit_print[i])
+		return;
+
 	if (!info->status) {
 		pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
 			aer_error_severity_string[info->severity]);
@@ -790,6 +818,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
 	trace_aer_event(pci_name(dev), (status & ~mask),
 			aer_severity, tlp_header_valid, &aer->header_log);
 
+	if (!aer_ratelimit(dev, info.severity))
+		return;
+
 	layer = AER_GET_LAYER_ERROR(aer_severity, status);
 	agent = AER_GET_AGENT(aer_severity, status);
 
@@ -824,6 +855,18 @@ static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
 	e_info->dev[i] = pci_dev_get(dev);
 	e_info->error_dev_num++;
 
+	/*
+	 * Ratelimit AER log messages.  "dev" is either the source
+	 * identified by the root's Error Source ID or it has an unmasked
+	 * error logged in its own AER Capability.  Messages are emitted
+	 * when "ratelimit_print[i]" is non-zero.  If we will print detail
+	 * for a downstream device, make sure we print the Error Source ID
+	 * from the root as well.
+	 */
+	if (aer_ratelimit(dev, e_info->severity)) {
+		e_info->ratelimit_print[i] = 1;
+		e_info->root_ratelimit_print = 1;
+	}
 	return 0;
 }
 
@@ -918,7 +961,7 @@ static int find_device_iter(struct pci_dev *dev, void *data)
  * e_info->error_dev_num and e_info->dev[], based on the given information.
  */
 static bool find_source_device(struct pci_dev *parent,
-		struct aer_err_info *e_info)
+			       struct aer_err_info *e_info)
 {
 	struct pci_dev *dev = parent;
 	int result;
@@ -1144,9 +1187,10 @@ static void aer_recover_work_func(struct work_struct *work)
 		pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus,
 						   entry.devfn);
 		if (!pdev) {
-			pr_err("no pci_dev for %04x:%02x:%02x.%x\n",
-			       entry.domain, entry.bus,
-			       PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
+			pr_err_ratelimited("%04x:%02x:%02x.%x: no pci_dev found\n",
+					   entry.domain, entry.bus,
+					   PCI_SLOT(entry.devfn),
+					   PCI_FUNC(entry.devfn));
 			continue;
 		}
 		pci_print_aer(pdev, entry.severity, entry.regs);
@@ -1294,7 +1338,22 @@ static void aer_isr_one_error_type(struct pci_dev *root,
 	bool found;
 
 	found = find_source_device(root, info);
-	aer_print_source(root, info, found);
+
+	/*
+	 * If we're going to log error messages, we've already set
+	 * "info->root_ratelimit_print" and "info->ratelimit_print[i]" to
+	 * non-zero (which enables printing) because this is either an
+	 * ERR_FATAL or we found a device with an error logged in its AER
+	 * Capability.
+	 *
+	 * If we didn't find the Error Source device, at least log the
+	 * Requester ID from the ERR_* Message received by the Root Port or
+	 * RCEC, ratelimited by the RP or RCEC.
+	 */
+	if (info->root_ratelimit_print ||
+	    (!found && aer_ratelimit(root, info->severity)))
+		aer_print_source(root, info, found);
+
 	if (found)
 		aer_process_err_devices(info);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 19/20] PCI/AER: Add ratelimits to PCI AER Documentation
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (17 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:21 ` [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits Bjorn Helgaas
  2025-05-23 16:21 ` [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Krzysztof Wilczyński,
	Bjorn Helgaas

From: Jon Pan-Doh <pandoh@google.com>

Add ratelimits section for rationale and defaults.

[bhelgaas: note fatal errors are not ratelimited]

Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
Signed-off-by: Jon Pan-Doh <pandoh@google.com>
Tested-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Acked-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250520215047.1350603-17-helgaas@kernel.org
---
 Documentation/PCI/pcieaer-howto.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
index f013f3b27c82..6fb31516fff1 100644
--- a/Documentation/PCI/pcieaer-howto.rst
+++ b/Documentation/PCI/pcieaer-howto.rst
@@ -85,6 +85,18 @@ In the example, 'Requester ID' means the ID of the device that sent
 the error message to the Root Port. Please refer to PCIe specs for other
 fields.
 
+AER Ratelimits
+--------------
+
+Since error messages can be generated for each transaction, we may see
+large volumes of errors reported. To prevent spammy devices from flooding
+the console/stalling execution, messages are throttled by device and error
+type (correctable vs. non-fatal uncorrectable).  Fatal errors, including
+DPC errors, are not ratelimited.
+
+AER uses the default ratelimit of DEFAULT_RATELIMIT_BURST (10 events) over
+DEFAULT_RATELIMIT_INTERVAL (5 seconds).
+
 AER Statistics / Counters
 -------------------------
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (18 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 19/20] PCI/AER: Add ratelimits to PCI AER Documentation Bjorn Helgaas
@ 2025-05-22 23:21 ` Bjorn Helgaas
  2025-05-22 23:50   ` Sathyanarayanan Kuppuswamy
  2025-05-23 16:21 ` [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
  20 siblings, 1 reply; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-22 23:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

From: Jon Pan-Doh <pandoh@google.com>

Allow userspace to read/write log ratelimits per device (including
enable/disable). Create aer/ sysfs directory to store them and any
future AER configs.

The new sysfs files are:

  /sys/bus/pci/devices/*/aer/correctable_ratelimit_burst
  /sys/bus/pci/devices/*/aer/correctable_ratelimit_interval_ms
  /sys/bus/pci/devices/*/aer/nonfatal_ratelimit_burst
  /sys/bus/pci/devices/*/aer/nonfatal_ratelimit_interval_ms

The default values are ratelimit_burst=10, ratelimit_interval_ms=5000, so
if we try to emit more than 10 messages in a 5 second period, some are
suppressed.

Update AER sysfs ABI filename to reflect the broader scope of AER sysfs
attributes (e.g. stats and ratelimits).

  Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats ->
    sysfs-bus-pci-devices-aer

Tested using aer-inject[1]. Configured correctable log ratelimit to 5.
Sent 6 AER errors. Observed 5 errors logged while AER stats
(cat /sys/bus/pci/devices/<dev>/aer_dev_correctable) shows 6.

Disabled ratelimiting and sent 6 more AER errors. Observed all 6 errors
logged and accounted in AER stats (12 total errors).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/gong.chen/aer-inject.git

[bhelgaas: note fatal errors are not ratelimited, "aer_report" ->
"aer_info", replace ratelimit_log_enable toggle with *_ratelimit_interval_ms]

Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
Signed-off-by: Jon Pan-Doh <pandoh@google.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250520215047.1350603-18-helgaas@kernel.org
---
 ...es-aer_stats => sysfs-bus-pci-devices-aer} |  44 ++++++++
 Documentation/PCI/pcieaer-howto.rst           |   5 +-
 drivers/pci/pci-sysfs.c                       |   1 +
 drivers/pci/pci.h                             |   1 +
 drivers/pci/pcie/aer.c                        | 105 ++++++++++++++++++
 5 files changed, 155 insertions(+), 1 deletion(-)
 rename Documentation/ABI/testing/{sysfs-bus-pci-devices-aer_stats => sysfs-bus-pci-devices-aer} (72%)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
similarity index 72%
rename from Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
rename to Documentation/ABI/testing/sysfs-bus-pci-devices-aer
index d1f67bb81d5d..5ed284523956 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
@@ -117,3 +117,47 @@ Date:		July 2018
 KernelVersion:	4.19.0
 Contact:	linux-pci@vger.kernel.org, rajatja@google.com
 Description:	Total number of ERR_NONFATAL messages reported to rootport.
+
+PCIe AER ratelimits
+-------------------
+
+These attributes show up under all the devices that are AER capable.
+They represent configurable ratelimits of logs per error type.
+
+See Documentation/PCI/pcieaer-howto.rst for more info on ratelimits.
+
+What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_interval_ms
+Date:		May 2025
+KernelVersion:	6.16.0
+Contact:	linux-pci@vger.kernel.org
+Description:	Writing 0 disables AER correctable error log ratelimiting.
+		Writing a positive value sets the ratelimit interval in ms.
+		Default is DEFAULT_RATELIMIT_INTERVAL (5000 ms).
+
+What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_burst
+Date:		May 2025
+KernelVersion:	6.16.0
+Contact:	linux-pci@vger.kernel.org
+Description:	Ratelimit burst for correctable error logs. Writing a value
+		changes the number of errors (burst) allowed per interval
+		before ratelimiting. Reading gets the current ratelimit
+		burst. Default is DEFAULT_RATELIMIT_BURST (10).
+
+What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_interval_ms
+Date:		May 2025
+KernelVersion:	6.16.0
+Contact:	linux-pci@vger.kernel.org
+Description:	Writing 0 disables AER non-fatal uncorrectable error log
+		ratelimiting. Writing a positive value sets the ratelimit
+		interval in ms. Default is DEFAULT_RATELIMIT_INTERVAL
+		(5000 ms).
+
+What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_burst
+Date:		May 2025
+KernelVersion:	6.16.0
+Contact:	linux-pci@vger.kernel.org
+Description:	Ratelimit burst for non-fatal uncorrectable error logs.
+		Writing a value changes the number of errors (burst)
+		allowed per interval before ratelimiting. Reading gets the
+		current ratelimit burst. Default is DEFAULT_RATELIMIT_BURST
+		(10).
diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
index 6fb31516fff1..4b71e2f43ca7 100644
--- a/Documentation/PCI/pcieaer-howto.rst
+++ b/Documentation/PCI/pcieaer-howto.rst
@@ -97,12 +97,15 @@ DPC errors, are not ratelimited.
 AER uses the default ratelimit of DEFAULT_RATELIMIT_BURST (10 events) over
 DEFAULT_RATELIMIT_INTERVAL (5 seconds).
 
+Ratelimits are exposed in the form of sysfs attributes and configurable.
+See Documentation/ABI/testing/sysfs-bus-pci-devices-aer.
+
 AER Statistics / Counters
 -------------------------
 
 When PCIe AER errors are captured, the counters / statistics are also exposed
 in the form of sysfs attributes which are documented at
-Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
+Documentation/ABI/testing/sysfs-bus-pci-devices-aer.
 
 Developer Guide
 ===============
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c6cda56ca52c..278de99b00ce 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1805,6 +1805,7 @@ const struct attribute_group *pci_dev_attr_groups[] = {
 	&pcie_dev_attr_group,
 #ifdef CONFIG_PCIEAER
 	&aer_stats_attr_group,
+	&aer_attr_group,
 #endif
 #ifdef CONFIG_PCIEASPM
 	&aspm_ctrl_attr_group,
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 3023c68fe485..eca2812cfd25 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -965,6 +965,7 @@ void pci_no_aer(void);
 void pci_aer_init(struct pci_dev *dev);
 void pci_aer_exit(struct pci_dev *dev);
 extern const struct attribute_group aer_stats_attr_group;
+extern const struct attribute_group aer_attr_group;
 void pci_aer_clear_fatal_status(struct pci_dev *dev);
 int pci_aer_clear_status(struct pci_dev *dev);
 int pci_aer_raw_clear_status(struct pci_dev *dev);
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ebac126144fc..6c331695af58 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -627,6 +627,111 @@ const struct attribute_group aer_stats_attr_group = {
 	.is_visible = aer_stats_attrs_are_visible,
 };
 
+/*
+ * Ratelimit interval
+ * <=0: disabled with ratelimit.interval = 0
+ * >0: enabled with ratelimit.interval in ms
+ */
+#define aer_ratelimit_interval_attr(name, ratelimit)			\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.interval);	\
+	}								\
+									\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.interval = interval;		\
+									\
+		return count;						\
+	}								\
+	static DEVICE_ATTR_RW(name);
+
+#define aer_ratelimit_burst_attr(name, ratelimit)			\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.burst);	\
+	}								\
+									\
+	static ssize_t							\
+	name##_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->aer_info->ratelimit.burst = burst;		\
+									\
+		return count;						\
+	}								\
+	static DEVICE_ATTR_RW(name);
+
+#define aer_ratelimit_attrs(name)					\
+	aer_ratelimit_interval_attr(name##_ratelimit_interval_ms,	\
+				    name##_ratelimit)			\
+	aer_ratelimit_burst_attr(name##_ratelimit_burst,		\
+				 name##_ratelimit)
+
+aer_ratelimit_attrs(correctable)
+aer_ratelimit_attrs(nonfatal)
+
+static struct attribute *aer_attrs[] = {
+	&dev_attr_correctable_ratelimit_interval_ms.attr,
+	&dev_attr_correctable_ratelimit_burst.attr,
+	&dev_attr_nonfatal_ratelimit_interval_ms.attr,
+	&dev_attr_nonfatal_ratelimit_burst.attr,
+	NULL
+};
+
+static umode_t aer_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->aer_info)
+		return 0;
+
+	return a->mode;
+}
+
+const struct attribute_group aer_attr_group = {
+	.name = "aer",
+	.attrs = aer_attrs,
+	.is_visible = aer_attrs_are_visible,
+};
+
 static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
 				   struct aer_err_info *info)
 {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log()
  2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
@ 2025-05-22 23:44   ` Sathyanarayanan Kuppuswamy
  2025-05-23  9:56   ` Ilpo Järvinen
  2025-05-28  6:38   ` Lukas Wunner
  2 siblings, 0 replies; 36+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-05-22 23:44 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas


On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> aer_print_error() produces output at a printk level (KERN_ERR/KERN_WARNING/
> etc) that depends on the kind of error, and it calls pcie_print_tlp_log(),
> which previously always produced output at KERN_ERR.
>
> Add a "level" parameter so aer_print_error() can control the level of the
> pcie_print_tlp_log() output to match.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>   drivers/pci/pci.h      | 3 ++-
>   drivers/pci/pcie/aer.c | 5 +++--
>   drivers/pci/pcie/dpc.c | 2 +-
>   drivers/pci/pcie/tlp.c | 6 ++++--
>   4 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 705f9ef58acc..1a9bfc708757 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -613,7 +613,8 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>   		      struct pcie_tlp_log *log);
>   unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc);
>   void pcie_print_tlp_log(const struct pci_dev *dev,
> -			const struct pcie_tlp_log *log, const char *pfx);
> +			const struct pcie_tlp_log *log, const char *level,
> +			const char *pfx);
>   #endif	/* CONFIG_PCIEAER */
>   
>   #ifdef CONFIG_PCIEPORTBUS
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f80c78846a14..f0936759ba8b 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -734,7 +734,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>   	__aer_print_error(dev, info);
>   
>   	if (info->tlp_header_valid)
> -		pcie_print_tlp_log(dev, &info->tlp, dev_fmt("  "));
> +		pcie_print_tlp_log(dev, &info->tlp, level, dev_fmt("  "));
>   
>   out:
>   	if (info->id && info->error_dev_num > 1 && info->id == id)
> @@ -797,7 +797,8 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>   			aer->uncor_severity);
>   
>   	if (tlp_header_valid)
> -		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
> +		pcie_print_tlp_log(dev, &aer->header_log, info.level,
> +				   dev_fmt("  "));
>   }
>   EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
>   
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 6c98fabdba57..7ae1590ea1da 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -222,7 +222,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>   			  dpc_tlp_log_len(pdev),
>   			  pdev->subordinate->flit_mode,
>   			  &tlp_log);
> -	pcie_print_tlp_log(pdev, &tlp_log, dev_fmt(""));
> +	pcie_print_tlp_log(pdev, &tlp_log, KERN_ERR, dev_fmt(""));
>   
>   	if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG + 1)
>   		goto clear_status;
> diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
> index 890d5391d7f5..71f8fc9ea2ed 100644
> --- a/drivers/pci/pcie/tlp.c
> +++ b/drivers/pci/pcie/tlp.c
> @@ -98,12 +98,14 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>    * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
>    * @dev: PCIe device
>    * @log: TLP Log structure
> + * @level: Printk log level
>    * @pfx: String prefix
>    *
>    * Prints TLP Header and Prefix Log information held by @log.
>    */
>   void pcie_print_tlp_log(const struct pci_dev *dev,
> -			const struct pcie_tlp_log *log, const char *pfx)
> +			const struct pcie_tlp_log *log, const char *level,
> +			const char *pfx)
>   {
>   	/* EE_PREFIX_STR fits the extended DW space needed for the Flit mode */
>   	char buf[11 * PCIE_STD_MAX_TLP_HEADERLOG + 1];
> @@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
>   		}
>   	}
>   
> -	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
> +	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
>   		log->flit ? " (Flit)" : "", buf);
>   }

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits
  2025-05-22 23:21 ` [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits Bjorn Helgaas
@ 2025-05-22 23:50   ` Sathyanarayanan Kuppuswamy
  0 siblings, 0 replies; 36+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-05-22 23:50 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas

Hi,

On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> From: Jon Pan-Doh <pandoh@google.com>
>
> Allow userspace to read/write log ratelimits per device (including
> enable/disable). Create aer/ sysfs directory to store them and any
> future AER configs.
>
> The new sysfs files are:
>
>    /sys/bus/pci/devices/*/aer/correctable_ratelimit_burst
>    /sys/bus/pci/devices/*/aer/correctable_ratelimit_interval_ms
>    /sys/bus/pci/devices/*/aer/nonfatal_ratelimit_burst
>    /sys/bus/pci/devices/*/aer/nonfatal_ratelimit_interval_ms
>
> The default values are ratelimit_burst=10, ratelimit_interval_ms=5000, so
> if we try to emit more than 10 messages in a 5 second period, some are
> suppressed.
>
> Update AER sysfs ABI filename to reflect the broader scope of AER sysfs
> attributes (e.g. stats and ratelimits).
>
>    Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats ->
>      sysfs-bus-pci-devices-aer
>
> Tested using aer-inject[1]. Configured correctable log ratelimit to 5.
> Sent 6 AER errors. Observed 5 errors logged while AER stats
> (cat /sys/bus/pci/devices/<dev>/aer_dev_correctable) shows 6.
>
> Disabled ratelimiting and sent 6 more AER errors. Observed all 6 errors
> logged and accounted in AER stats (12 total errors).
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/gong.chen/aer-inject.git
>
> [bhelgaas: note fatal errors are not ratelimited, "aer_report" ->
> "aer_info", replace ratelimit_log_enable toggle with *_ratelimit_interval_ms]
>
> Signed-off-by: Karolina Stolarek <karolina.stolarek@oracle.com>
> Signed-off-by: Jon Pan-Doh <pandoh@google.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Link: https://patch.msgid.link/20250520215047.1350603-18-helgaas@kernel.org
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>   ...es-aer_stats => sysfs-bus-pci-devices-aer} |  44 ++++++++
>   Documentation/PCI/pcieaer-howto.rst           |   5 +-
>   drivers/pci/pci-sysfs.c                       |   1 +
>   drivers/pci/pci.h                             |   1 +
>   drivers/pci/pcie/aer.c                        | 105 ++++++++++++++++++
>   5 files changed, 155 insertions(+), 1 deletion(-)
>   rename Documentation/ABI/testing/{sysfs-bus-pci-devices-aer_stats => sysfs-bus-pci-devices-aer} (72%)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> similarity index 72%
> rename from Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> rename to Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> index d1f67bb81d5d..5ed284523956 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
> @@ -117,3 +117,47 @@ Date:		July 2018
>   KernelVersion:	4.19.0
>   Contact:	linux-pci@vger.kernel.org, rajatja@google.com
>   Description:	Total number of ERR_NONFATAL messages reported to rootport.
> +
> +PCIe AER ratelimits
> +-------------------
> +
> +These attributes show up under all the devices that are AER capable.
> +They represent configurable ratelimits of logs per error type.
> +
> +See Documentation/PCI/pcieaer-howto.rst for more info on ratelimits.
> +
> +What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_interval_ms
> +Date:		May 2025
> +KernelVersion:	6.16.0
> +Contact:	linux-pci@vger.kernel.org
> +Description:	Writing 0 disables AER correctable error log ratelimiting.
> +		Writing a positive value sets the ratelimit interval in ms.
> +		Default is DEFAULT_RATELIMIT_INTERVAL (5000 ms).
> +
> +What:		/sys/bus/pci/devices/<dev>/aer/correctable_ratelimit_burst
> +Date:		May 2025
> +KernelVersion:	6.16.0
> +Contact:	linux-pci@vger.kernel.org
> +Description:	Ratelimit burst for correctable error logs. Writing a value
> +		changes the number of errors (burst) allowed per interval
> +		before ratelimiting. Reading gets the current ratelimit
> +		burst. Default is DEFAULT_RATELIMIT_BURST (10).
> +
> +What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_interval_ms
> +Date:		May 2025
> +KernelVersion:	6.16.0
> +Contact:	linux-pci@vger.kernel.org
> +Description:	Writing 0 disables AER non-fatal uncorrectable error log
> +		ratelimiting. Writing a positive value sets the ratelimit
> +		interval in ms. Default is DEFAULT_RATELIMIT_INTERVAL
> +		(5000 ms).
> +
> +What:		/sys/bus/pci/devices/<dev>/aer/nonfatal_ratelimit_burst
> +Date:		May 2025
> +KernelVersion:	6.16.0
> +Contact:	linux-pci@vger.kernel.org
> +Description:	Ratelimit burst for non-fatal uncorrectable error logs.
> +		Writing a value changes the number of errors (burst)
> +		allowed per interval before ratelimiting. Reading gets the
> +		current ratelimit burst. Default is DEFAULT_RATELIMIT_BURST
> +		(10).
> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
> index 6fb31516fff1..4b71e2f43ca7 100644
> --- a/Documentation/PCI/pcieaer-howto.rst
> +++ b/Documentation/PCI/pcieaer-howto.rst
> @@ -97,12 +97,15 @@ DPC errors, are not ratelimited.
>   AER uses the default ratelimit of DEFAULT_RATELIMIT_BURST (10 events) over
>   DEFAULT_RATELIMIT_INTERVAL (5 seconds).
>   
> +Ratelimits are exposed in the form of sysfs attributes and configurable.
> +See Documentation/ABI/testing/sysfs-bus-pci-devices-aer.
> +
>   AER Statistics / Counters
>   -------------------------
>   
>   When PCIe AER errors are captured, the counters / statistics are also exposed
>   in the form of sysfs attributes which are documented at
> -Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> +Documentation/ABI/testing/sysfs-bus-pci-devices-aer.
>   
>   Developer Guide
>   ===============
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index c6cda56ca52c..278de99b00ce 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1805,6 +1805,7 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>   	&pcie_dev_attr_group,
>   #ifdef CONFIG_PCIEAER
>   	&aer_stats_attr_group,
> +	&aer_attr_group,
>   #endif
>   #ifdef CONFIG_PCIEASPM
>   	&aspm_ctrl_attr_group,
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 3023c68fe485..eca2812cfd25 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -965,6 +965,7 @@ void pci_no_aer(void);
>   void pci_aer_init(struct pci_dev *dev);
>   void pci_aer_exit(struct pci_dev *dev);
>   extern const struct attribute_group aer_stats_attr_group;
> +extern const struct attribute_group aer_attr_group;
>   void pci_aer_clear_fatal_status(struct pci_dev *dev);
>   int pci_aer_clear_status(struct pci_dev *dev);
>   int pci_aer_raw_clear_status(struct pci_dev *dev);
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index ebac126144fc..6c331695af58 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -627,6 +627,111 @@ const struct attribute_group aer_stats_attr_group = {
>   	.is_visible = aer_stats_attrs_are_visible,
>   };
>   
> +/*
> + * Ratelimit interval
> + * <=0: disabled with ratelimit.interval = 0
> + * >0: enabled with ratelimit.interval in ms
> + */
> +#define aer_ratelimit_interval_attr(name, ratelimit)			\
> +	static ssize_t							\
> +	name##_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->aer_info->ratelimit.interval);	\
> +	}								\
> +									\
> +	static ssize_t							\
> +	name##_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->aer_info->ratelimit.interval = interval;		\
> +									\
> +		return count;						\
> +	}								\
> +	static DEVICE_ATTR_RW(name);
> +
> +#define aer_ratelimit_burst_attr(name, ratelimit)			\
> +	static ssize_t							\
> +	name##_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->aer_info->ratelimit.burst);	\
> +	}								\
> +									\
> +	static ssize_t							\
> +	name##_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->aer_info->ratelimit.burst = burst;		\
> +									\
> +		return count;						\
> +	}								\
> +	static DEVICE_ATTR_RW(name);
> +
> +#define aer_ratelimit_attrs(name)					\
> +	aer_ratelimit_interval_attr(name##_ratelimit_interval_ms,	\
> +				    name##_ratelimit)			\
> +	aer_ratelimit_burst_attr(name##_ratelimit_burst,		\
> +				 name##_ratelimit)
> +
> +aer_ratelimit_attrs(correctable)
> +aer_ratelimit_attrs(nonfatal)
> +
> +static struct attribute *aer_attrs[] = {
> +	&dev_attr_correctable_ratelimit_interval_ms.attr,
> +	&dev_attr_correctable_ratelimit_burst.attr,
> +	&dev_attr_nonfatal_ratelimit_interval_ms.attr,
> +	&dev_attr_nonfatal_ratelimit_burst.attr,
> +	NULL
> +};
> +
> +static umode_t aer_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->aer_info)
> +		return 0;
> +
> +	return a->mode;
> +}
> +
> +const struct attribute_group aer_attr_group = {
> +	.name = "aer",
> +	.attrs = aer_attrs,
> +	.is_visible = aer_attrs_are_visible,
> +};
> +
>   static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
>   				   struct aer_err_info *info)
>   {

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging
  2025-05-22 23:21 ` [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging Bjorn Helgaas
@ 2025-05-22 23:56   ` Sathyanarayanan Kuppuswamy
  2025-05-23 16:06     ` Bjorn Helgaas
  2025-08-01 13:16   ` Breno Leitao
  1 sibling, 1 reply; 36+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-05-22 23:56 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas


On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> From: Jon Pan-Doh <pandoh@google.com>
>
> Spammy devices can flood kernel logs with AER errors and slow/stall
> execution. Add per-device ratelimits for AER correctable and non-fatal
> uncorrectable errors that use the kernel defaults (10 per 5s).  Logging of
> fatal errors is not ratelimited.
>
> There are two AER logging entry points:
>
>    - aer_print_error() is used by DPC and native AER
>
>    - pci_print_aer() is used by GHES and CXL
>
> The native AER aer_print_error() case includes a loop that may log details
> from multiple devices, which are ratelimited individually.  If we log
> details for any device, we also log the Error Source ID from the Root Port
> or RCEC.
>
> If no such device details are found, we still log the Error Source from the
> ERR_* Message, ratelimited by the Root Port or RCEC that received it.
>
> The DPC aer_print_error() case is not ratelimited, since this only happens
> for fatal errors.
>
> The CXL pci_print_aer() case is ratelimited by the Error Source device.
>
> The GHES pci_print_aer() case is via aer_recover_work_func(), which
> searches for the Error Source device.  If the device is not found, there's
> no per-device ratelimit, so we use a system-wide ratelimit that covers all
> error types (correctable, non-fatal, and fatal).
>
> Sargun at Meta reported internally that a flood of AER errors causes RCU
> CPU stall warnings and CSD-lock warnings.
>
> Tested using aer-inject[1]. Sent 11 AER errors. Observed 10 errors logged
> while AER stats (cat /sys/bus/pci/devices/<dev>/aer_dev_correctable) show
> true count of 11.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/gong.chen/aer-inject.git
>
> [bhelgaas: commit log, factor out trace_aer_event() and aer_print_rp_info()
> changes to previous patches, enable Error Source logging if any downstream
> detail will be printed, don't ratelimit fatal errors, "aer_report" ->
> "aer_info", "cor_log_ratelimit" -> "correctable_ratelimit",
> "uncor_log_ratelimit" -> "nonfatal_ratelimit"]
>
> Reported-by: Sargun Dhillon <sargun@meta.com>
> Signed-off-by: Jon Pan-Doh <pandoh@google.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Link: https://patch.msgid.link/20250520215047.1350603-16-helgaas@kernel.org
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>   drivers/pci/pci.h      |  4 ++-
>   drivers/pci/pcie/aer.c | 69 +++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index e1a28215967f..3023c68fe485 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -587,13 +587,15 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
>   
>   struct aer_err_info {
>   	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
> +	int ratelimit_print[AER_MAX_MULTI_ERR_DEVICES];
>   	int error_dev_num;
>   	const char *level;		/* printk level */
>   
>   	unsigned int id:16;
>   
>   	unsigned int severity:2;	/* 0:NONFATAL | 1:FATAL | 2:COR */
> -	unsigned int __pad1:5;
> +	unsigned int root_ratelimit_print:1;	/* 0=skip, 1=print */
> +	unsigned int __pad1:4;
>   	unsigned int multi_error_valid:1;
>   
>   	unsigned int first_error:5;
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 24f0f5c55256..ebac126144fc 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -28,6 +28,7 @@
>   #include <linux/interrupt.h>
>   #include <linux/delay.h>
>   #include <linux/kfifo.h>
> +#include <linux/ratelimit.h>
>   #include <linux/slab.h>
>   #include <acpi/apei.h>
>   #include <acpi/ghes.h>
> @@ -88,6 +89,10 @@ struct aer_info {
>   	u64 rootport_total_cor_errs;
>   	u64 rootport_total_fatal_errs;
>   	u64 rootport_total_nonfatal_errs;
> +
> +	/* Ratelimits for errors */
> +	struct ratelimit_state correctable_ratelimit;
> +	struct ratelimit_state nonfatal_ratelimit;
>   };
>   
>   #define AER_LOG_TLP_MASKS		(PCI_ERR_UNC_POISON_TLP|	\
> @@ -379,6 +384,11 @@ void pci_aer_init(struct pci_dev *dev)
>   
>   	dev->aer_info = kzalloc(sizeof(*dev->aer_info), GFP_KERNEL);
>   
> +	ratelimit_state_init(&dev->aer_info->correctable_ratelimit,
> +			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
> +	ratelimit_state_init(&dev->aer_info->nonfatal_ratelimit,
> +			     DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
> +
>   	/*
>   	 * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
>   	 * PCI_ERR_COR_MASK, and PCI_ERR_CAP.  Root and Root Complex Event
> @@ -669,6 +679,21 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
>   	}
>   }
>   
> +static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
> +{
> +	struct ratelimit_state *ratelimit;
> +
> +	if (severity == AER_FATAL)
> +		return 1;	/* AER_FATAL not ratelimited */
> +
> +	if (severity == AER_CORRECTABLE)
> +		ratelimit = &dev->aer_info->correctable_ratelimit;
> +	else
> +		ratelimit = &dev->aer_info->nonfatal_ratelimit;
> +
> +	return __ratelimit(ratelimit);
> +}
> +

Why not combine severity checks? May be something like below:

     switch (severity) {
     case AER_NONFATAL:
         return __ratelimit(&dev->aer_info->nonfatal_ratelimit);
     case AER_CORRECTABLE:
         return __ratelimit(&dev->aer_info->correctable_ratelimit);
     default:
         return 1; /* Don't rate-limit fatal errors */
     }

>   static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>   {
>   	const char **strings;
> @@ -721,6 +746,9 @@ void aer_print_error(struct aer_err_info *info, int i)
>   	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
>   			info->severity, info->tlp_header_valid, &info->tlp);
>   
> +	if (!info->ratelimit_print[i])
> +		return;
> +
>   	if (!info->status) {
>   		pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
>   			aer_error_severity_string[info->severity]);
> @@ -790,6 +818,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>   	trace_aer_event(pci_name(dev), (status & ~mask),
>   			aer_severity, tlp_header_valid, &aer->header_log);
>   
> +	if (!aer_ratelimit(dev, info.severity))
> +		return;
> +
>   	layer = AER_GET_LAYER_ERROR(aer_severity, status);
>   	agent = AER_GET_AGENT(aer_severity, status);
>   
> @@ -824,6 +855,18 @@ static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
>   	e_info->dev[i] = pci_dev_get(dev);
>   	e_info->error_dev_num++;
>   
> +	/*
> +	 * Ratelimit AER log messages.  "dev" is either the source
> +	 * identified by the root's Error Source ID or it has an unmasked
> +	 * error logged in its own AER Capability.  Messages are emitted
> +	 * when "ratelimit_print[i]" is non-zero.  If we will print detail
> +	 * for a downstream device, make sure we print the Error Source ID
> +	 * from the root as well.
> +	 */
> +	if (aer_ratelimit(dev, e_info->severity)) {
> +		e_info->ratelimit_print[i] = 1;
> +		e_info->root_ratelimit_print = 1;
> +	}
>   	return 0;
>   }
>   
> @@ -918,7 +961,7 @@ static int find_device_iter(struct pci_dev *dev, void *data)
>    * e_info->error_dev_num and e_info->dev[], based on the given information.
>    */
>   static bool find_source_device(struct pci_dev *parent,
> -		struct aer_err_info *e_info)
> +			       struct aer_err_info *e_info)
>   {
>   	struct pci_dev *dev = parent;
>   	int result;
> @@ -1144,9 +1187,10 @@ static void aer_recover_work_func(struct work_struct *work)
>   		pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus,
>   						   entry.devfn);
>   		if (!pdev) {
> -			pr_err("no pci_dev for %04x:%02x:%02x.%x\n",
> -			       entry.domain, entry.bus,
> -			       PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
> +			pr_err_ratelimited("%04x:%02x:%02x.%x: no pci_dev found\n",
> +					   entry.domain, entry.bus,
> +					   PCI_SLOT(entry.devfn),
> +					   PCI_FUNC(entry.devfn));
>   			continue;
>   		}
>   		pci_print_aer(pdev, entry.severity, entry.regs);
> @@ -1294,7 +1338,22 @@ static void aer_isr_one_error_type(struct pci_dev *root,
>   	bool found;
>   
>   	found = find_source_device(root, info);
> -	aer_print_source(root, info, found);
> +
> +	/*
> +	 * If we're going to log error messages, we've already set
> +	 * "info->root_ratelimit_print" and "info->ratelimit_print[i]" to
> +	 * non-zero (which enables printing) because this is either an
> +	 * ERR_FATAL or we found a device with an error logged in its AER
> +	 * Capability.
> +	 *
> +	 * If we didn't find the Error Source device, at least log the
> +	 * Requester ID from the ERR_* Message received by the Root Port or
> +	 * RCEC, ratelimited by the RP or RCEC.
> +	 */
> +	if (info->root_ratelimit_print ||
> +	    (!found && aer_ratelimit(root, info->severity)))
> +		aer_print_source(root, info, found);
> +
>   	if (found)
>   		aer_process_err_devices(info);
>   }

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 17/20] PCI/AER: Simplify add_error_device()
  2025-05-22 23:21 ` [PATCH v8 17/20] PCI/AER: Simplify add_error_device() Bjorn Helgaas
@ 2025-05-22 23:57   ` Sathyanarayanan Kuppuswamy
  2025-05-23 11:14   ` Ilpo Järvinen
  1 sibling, 0 replies; 36+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-05-22 23:57 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas


On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Return -ENOSPC error early so the usual path through add_error_device() is
> the straightline code.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>   drivers/pci/pcie/aer.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 237741e66d28..24f0f5c55256 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -816,12 +816,15 @@ EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
>    */
>   static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
>   {
> -	if (e_info->error_dev_num < AER_MAX_MULTI_ERR_DEVICES) {
> -		e_info->dev[e_info->error_dev_num] = pci_dev_get(dev);
> -		e_info->error_dev_num++;
> -		return 0;
> -	}
> -	return -ENOSPC;
> +	int i = e_info->error_dev_num;
> +
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return -ENOSPC;
> +
> +	e_info->dev[i] = pci_dev_get(dev);
> +	e_info->error_dev_num++;
> +
> +	return 0;
>   }
>   
>   /**

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index
  2025-05-22 23:21 ` [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index Bjorn Helgaas
@ 2025-05-22 23:58   ` Sathyanarayanan Kuppuswamy
  2025-05-23 11:13   ` Ilpo Järvinen
  1 sibling, 0 replies; 36+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-05-22 23:58 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas


On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Previously aer_get_device_error_info() and aer_print_error() took a pointer
> to struct aer_err_info and a pointer to a pci_dev.  Typically the pci_dev
> was one of the elements of the aer_err_info.dev[] array (DPC was an
> exception, where the dev[] array was unused).
>
> Convert aer_get_device_error_info() and aer_print_error() to take an index
> into the aer_err_info.dev[] array instead.  A future patch will add
> per-device ratelimit information, so the index makes it convenient to find
> the ratelimit associated with the device.
>
> To accommodate DPC, set info->dev[0] to the DPC port before using these
> interfaces.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>   drivers/pci/pci.h      |  4 ++--
>   drivers/pci/pcie/aer.c | 33 +++++++++++++++++++++++----------
>   drivers/pci/pcie/dpc.c |  8 ++++++--
>   3 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 1a9bfc708757..e1a28215967f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -605,8 +605,8 @@ struct aer_err_info {
>   	struct pcie_tlp_log tlp;	/* TLP Header */
>   };
>   
> -int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
> -void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
> +int aer_get_device_error_info(struct aer_err_info *info, int i);
> +void aer_print_error(struct aer_err_info *info, int i);
>   
>   int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>   		      unsigned int tlp_len, bool flit,
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 787a953fb331..237741e66d28 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -705,12 +705,18 @@ static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
>   		 found ? "" : " (no details found");
>   }
>   
> -void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
> +void aer_print_error(struct aer_err_info *info, int i)
>   {
> -	int layer, agent;
> -	int id = pci_dev_id(dev);
> +	struct pci_dev *dev;
> +	int layer, agent, id;
>   	const char *level = info->level;
>   
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return;
> +
> +	dev = info->dev[i];
> +	id = pci_dev_id(dev);
> +
>   	pci_dev_aer_stats_incr(dev, info);
>   	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
>   			info->severity, info->tlp_header_valid, &info->tlp);
> @@ -1193,19 +1199,26 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
>   
>   /**
>    * aer_get_device_error_info - read error status from dev and store it to info
> - * @dev: pointer to the device expected to have an error record
>    * @info: pointer to structure to store the error record
> + * @i: index into info->dev[]
>    *
>    * Return: 1 on success, 0 on error.
>    *
>    * Note that @info is reused among all error devices. Clear fields properly.
>    */
> -int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
> +int aer_get_device_error_info(struct aer_err_info *info, int i)
>   {
> -	int type = pci_pcie_type(dev);
> -	int aer = dev->aer_cap;
> +	struct pci_dev *dev;
> +	int type, aer;
>   	u32 aercc;
>   
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return 0;
> +
> +	dev = info->dev[i];
> +	aer = dev->aer_cap;
> +	type = pci_pcie_type(dev);
> +
>   	/* Must reset in this function */
>   	info->status = 0;
>   	info->tlp_header_valid = 0;
> @@ -1257,11 +1270,11 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
>   
>   	/* Report all before handling them, to not lose records by reset etc. */
>   	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
> -		if (aer_get_device_error_info(e_info->dev[i], e_info))
> -			aer_print_error(e_info->dev[i], e_info);
> +		if (aer_get_device_error_info(e_info, i))
> +			aer_print_error(e_info, i);
>   	}
>   	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
> -		if (aer_get_device_error_info(e_info->dev[i], e_info))
> +		if (aer_get_device_error_info(e_info, i))
>   			handle_error_source(e_info->dev[i], e_info);
>   	}
>   }
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 7ae1590ea1da..fc18349614d7 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -253,6 +253,10 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
>   		info->severity = AER_NONFATAL;
>   
>   	info->level = KERN_ERR;
> +
> +	info->dev[0] = dev;
> +	info->error_dev_num = 1;
> +
>   	return 1;
>   }
>   
> @@ -270,8 +274,8 @@ void dpc_process_error(struct pci_dev *pdev)
>   		pci_warn(pdev, "containment event, status:%#06x: unmasked uncorrectable error detected\n",
>   			 status);
>   		if (dpc_get_aer_uncorrect_severity(pdev, &info) &&
> -		    aer_get_device_error_info(pdev, &info)) {
> -			aer_print_error(pdev, &info);
> +		    aer_get_device_error_info(&info, 0)) {
> +			aer_print_error(&info, 0);
>   			pci_aer_clear_nonfatal_status(pdev);
>   			pci_aer_clear_fatal_status(pdev);
>   		}

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log()
  2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
  2025-05-22 23:44   ` Sathyanarayanan Kuppuswamy
@ 2025-05-23  9:56   ` Ilpo Järvinen
  2025-05-28  6:38   ` Lukas Wunner
  2 siblings, 0 replies; 36+ messages in thread
From: Ilpo Järvinen @ 2025-05-23  9:56 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Sathyanarayanan Kuppuswamy, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, LKML, linuxppc-dev,
	Bjorn Helgaas

[-- Attachment #1: Type: text/plain, Size: 3928 bytes --]

On Thu, 22 May 2025, Bjorn Helgaas wrote:

> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> aer_print_error() produces output at a printk level (KERN_ERR/KERN_WARNING/
> etc) that depends on the kind of error, and it calls pcie_print_tlp_log(),
> which previously always produced output at KERN_ERR.
> 
> Add a "level" parameter so aer_print_error() can control the level of the
> pcie_print_tlp_log() output to match.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/pci.h      | 3 ++-
>  drivers/pci/pcie/aer.c | 5 +++--
>  drivers/pci/pcie/dpc.c | 2 +-
>  drivers/pci/pcie/tlp.c | 6 ++++--
>  4 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 705f9ef58acc..1a9bfc708757 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -613,7 +613,8 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>  		      struct pcie_tlp_log *log);
>  unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc);
>  void pcie_print_tlp_log(const struct pci_dev *dev,
> -			const struct pcie_tlp_log *log, const char *pfx);
> +			const struct pcie_tlp_log *log, const char *level,
> +			const char *pfx);
>  #endif	/* CONFIG_PCIEAER */
>  
>  #ifdef CONFIG_PCIEPORTBUS
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f80c78846a14..f0936759ba8b 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -734,7 +734,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  	__aer_print_error(dev, info);
>  
>  	if (info->tlp_header_valid)
> -		pcie_print_tlp_log(dev, &info->tlp, dev_fmt("  "));
> +		pcie_print_tlp_log(dev, &info->tlp, level, dev_fmt("  "));
>  
>  out:
>  	if (info->id && info->error_dev_num > 1 && info->id == id)
> @@ -797,7 +797,8 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>  			aer->uncor_severity);
>  
>  	if (tlp_header_valid)
> -		pcie_print_tlp_log(dev, &aer->header_log, dev_fmt("  "));
> +		pcie_print_tlp_log(dev, &aer->header_log, info.level,
> +				   dev_fmt("  "));
>  }
>  EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
>  
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 6c98fabdba57..7ae1590ea1da 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -222,7 +222,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>  			  dpc_tlp_log_len(pdev),
>  			  pdev->subordinate->flit_mode,
>  			  &tlp_log);
> -	pcie_print_tlp_log(pdev, &tlp_log, dev_fmt(""));
> +	pcie_print_tlp_log(pdev, &tlp_log, KERN_ERR, dev_fmt(""));
>  
>  	if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG + 1)
>  		goto clear_status;
> diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
> index 890d5391d7f5..71f8fc9ea2ed 100644
> --- a/drivers/pci/pcie/tlp.c
> +++ b/drivers/pci/pcie/tlp.c
> @@ -98,12 +98,14 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>   * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
>   * @dev: PCIe device
>   * @log: TLP Log structure
> + * @level: Printk log level
>   * @pfx: String prefix
>   *
>   * Prints TLP Header and Prefix Log information held by @log.
>   */
>  void pcie_print_tlp_log(const struct pci_dev *dev,
> -			const struct pcie_tlp_log *log, const char *pfx)
> +			const struct pcie_tlp_log *log, const char *level,
> +			const char *pfx)
>  {
>  	/* EE_PREFIX_STR fits the extended DW space needed for the Flit mode */
>  	char buf[11 * PCIE_STD_MAX_TLP_HEADERLOG + 1];
> @@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
>  		}
>  	}
>  
> -	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
> +	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
>  		log->flit ? " (Flit)" : "", buf);
>  }
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index
  2025-05-22 23:21 ` [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index Bjorn Helgaas
  2025-05-22 23:58   ` Sathyanarayanan Kuppuswamy
@ 2025-05-23 11:13   ` Ilpo Järvinen
  2025-05-23 16:12     ` Bjorn Helgaas
  1 sibling, 1 reply; 36+ messages in thread
From: Ilpo Järvinen @ 2025-05-23 11:13 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Sathyanarayanan Kuppuswamy, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, LKML, linuxppc-dev,
	Bjorn Helgaas

[-- Attachment #1: Type: text/plain, Size: 5435 bytes --]

On Thu, 22 May 2025, Bjorn Helgaas wrote:

> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Previously aer_get_device_error_info() and aer_print_error() took a pointer
> to struct aer_err_info and a pointer to a pci_dev.  Typically the pci_dev
> was one of the elements of the aer_err_info.dev[] array (DPC was an
> exception, where the dev[] array was unused).
> 
> Convert aer_get_device_error_info() and aer_print_error() to take an index
> into the aer_err_info.dev[] array instead.  A future patch will add
> per-device ratelimit information, so the index makes it convenient to find
> the ratelimit associated with the device.
> 
> To accommodate DPC, set info->dev[0] to the DPC port before using these
> interfaces.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/pci.h      |  4 ++--
>  drivers/pci/pcie/aer.c | 33 +++++++++++++++++++++++----------
>  drivers/pci/pcie/dpc.c |  8 ++++++--
>  3 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 1a9bfc708757..e1a28215967f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -605,8 +605,8 @@ struct aer_err_info {
>  	struct pcie_tlp_log tlp;	/* TLP Header */
>  };
>  
> -int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
> -void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
> +int aer_get_device_error_info(struct aer_err_info *info, int i);
> +void aer_print_error(struct aer_err_info *info, int i);
>  
>  int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>  		      unsigned int tlp_len, bool flit,
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 787a953fb331..237741e66d28 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -705,12 +705,18 @@ static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
>  		 found ? "" : " (no details found");
>  }
>  
> -void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
> +void aer_print_error(struct aer_err_info *info, int i)
>  {
> -	int layer, agent;
> -	int id = pci_dev_id(dev);
> +	struct pci_dev *dev;
> +	int layer, agent, id;
>  	const char *level = info->level;
>  
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return;

Are these OoB checks actually indication of a logic error in the caller 
side which would perhaps warrant using
	if (WARN_ON_ONCE(i >= AER_MAX_MULTI_ERR_DEVICES))
?

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> +
> +	dev = info->dev[i];
> +	id = pci_dev_id(dev);
> +
>  	pci_dev_aer_stats_incr(dev, info);
>  	trace_aer_event(pci_name(dev), (info->status & ~info->mask),
>  			info->severity, info->tlp_header_valid, &info->tlp);
> @@ -1193,19 +1199,26 @@ EXPORT_SYMBOL_GPL(aer_recover_queue);
>  
>  /**
>   * aer_get_device_error_info - read error status from dev and store it to info
> - * @dev: pointer to the device expected to have an error record
>   * @info: pointer to structure to store the error record
> + * @i: index into info->dev[]
>   *
>   * Return: 1 on success, 0 on error.
>   *
>   * Note that @info is reused among all error devices. Clear fields properly.
>   */
> -int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
> +int aer_get_device_error_info(struct aer_err_info *info, int i)
>  {
> -	int type = pci_pcie_type(dev);
> -	int aer = dev->aer_cap;
> +	struct pci_dev *dev;
> +	int type, aer;
>  	u32 aercc;
>  
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return 0;
> +
> +	dev = info->dev[i];
> +	aer = dev->aer_cap;
> +	type = pci_pcie_type(dev);
> +
>  	/* Must reset in this function */
>  	info->status = 0;
>  	info->tlp_header_valid = 0;
> @@ -1257,11 +1270,11 @@ static inline void aer_process_err_devices(struct aer_err_info *e_info)
>  
>  	/* Report all before handling them, to not lose records by reset etc. */
>  	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
> -		if (aer_get_device_error_info(e_info->dev[i], e_info))
> -			aer_print_error(e_info->dev[i], e_info);
> +		if (aer_get_device_error_info(e_info, i))
> +			aer_print_error(e_info, i);
>  	}
>  	for (i = 0; i < e_info->error_dev_num && e_info->dev[i]; i++) {
> -		if (aer_get_device_error_info(e_info->dev[i], e_info))
> +		if (aer_get_device_error_info(e_info, i))
>  			handle_error_source(e_info->dev[i], e_info);
>  	}
>  }
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 7ae1590ea1da..fc18349614d7 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -253,6 +253,10 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
>  		info->severity = AER_NONFATAL;
>  
>  	info->level = KERN_ERR;
> +
> +	info->dev[0] = dev;
> +	info->error_dev_num = 1;
> +
>  	return 1;
>  }
>  
> @@ -270,8 +274,8 @@ void dpc_process_error(struct pci_dev *pdev)
>  		pci_warn(pdev, "containment event, status:%#06x: unmasked uncorrectable error detected\n",
>  			 status);
>  		if (dpc_get_aer_uncorrect_severity(pdev, &info) &&
> -		    aer_get_device_error_info(pdev, &info)) {
> -			aer_print_error(pdev, &info);
> +		    aer_get_device_error_info(&info, 0)) {
> +			aer_print_error(&info, 0);
>  			pci_aer_clear_nonfatal_status(pdev);
>  			pci_aer_clear_fatal_status(pdev);
>  		}
> 

-- 
 i.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 17/20] PCI/AER: Simplify add_error_device()
  2025-05-22 23:21 ` [PATCH v8 17/20] PCI/AER: Simplify add_error_device() Bjorn Helgaas
  2025-05-22 23:57   ` Sathyanarayanan Kuppuswamy
@ 2025-05-23 11:14   ` Ilpo Järvinen
  1 sibling, 0 replies; 36+ messages in thread
From: Ilpo Järvinen @ 2025-05-23 11:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Sathyanarayanan Kuppuswamy, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, LKML, linuxppc-dev,
	Bjorn Helgaas

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Thu, 22 May 2025, Bjorn Helgaas wrote:

> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Return -ENOSPC error early so the usual path through add_error_device() is
> the straightline code.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/pcie/aer.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 237741e66d28..24f0f5c55256 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -816,12 +816,15 @@ EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
>   */
>  static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
>  {
> -	if (e_info->error_dev_num < AER_MAX_MULTI_ERR_DEVICES) {
> -		e_info->dev[e_info->error_dev_num] = pci_dev_get(dev);
> -		e_info->error_dev_num++;
> -		return 0;
> -	}
> -	return -ENOSPC;
> +	int i = e_info->error_dev_num;
> +
> +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> +		return -ENOSPC;
> +
> +	e_info->dev[i] = pci_dev_get(dev);
> +	e_info->error_dev_num++;
> +
> +	return 0;
>  }
>  
>  /**
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging
  2025-05-22 23:56   ` Sathyanarayanan Kuppuswamy
@ 2025-05-23 16:06     ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-23 16:06 UTC (permalink / raw)
  To: Sathyanarayanan Kuppuswamy
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas

On Thu, May 22, 2025 at 04:56:56PM -0700, Sathyanarayanan Kuppuswamy wrote:
> On 5/22/25 4:21 PM, Bjorn Helgaas wrote:
> > From: Jon Pan-Doh <pandoh@google.com>
> > 
> > Spammy devices can flood kernel logs with AER errors and slow/stall
> > execution. Add per-device ratelimits for AER correctable and non-fatal
> > uncorrectable errors that use the kernel defaults (10 per 5s).  Logging of
> > fatal errors is not ratelimited.

> > +static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
> > +{
> > +	struct ratelimit_state *ratelimit;
> > +
> > +	if (severity == AER_FATAL)
> > +		return 1;	/* AER_FATAL not ratelimited */
> > +
> > +	if (severity == AER_CORRECTABLE)
> > +		ratelimit = &dev->aer_info->correctable_ratelimit;
> > +	else
> > +		ratelimit = &dev->aer_info->nonfatal_ratelimit;
> > +
> > +	return __ratelimit(ratelimit);
> > +}
> > +
> 
> Why not combine severity checks? May be something like below:
> 
>     switch (severity) {
>     case AER_NONFATAL:
>         return __ratelimit(&dev->aer_info->nonfatal_ratelimit);
>     case AER_CORRECTABLE:
>         return __ratelimit(&dev->aer_info->correctable_ratelimit);
>     default:
>         return 1; /* Don't rate-limit fatal errors */
>     }

Beautiful, adopted, thank you!

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index
  2025-05-23 11:13   ` Ilpo Järvinen
@ 2025-05-23 16:12     ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-23 16:12 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Sathyanarayanan Kuppuswamy, Lukas Wunner, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, LKML, linuxppc-dev,
	Bjorn Helgaas

On Fri, May 23, 2025 at 02:13:52PM +0300, Ilpo Järvinen wrote:
> On Thu, 22 May 2025, Bjorn Helgaas wrote:
> 
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > Previously aer_get_device_error_info() and aer_print_error() took a pointer
> > to struct aer_err_info and a pointer to a pci_dev.  Typically the pci_dev
> > was one of the elements of the aer_err_info.dev[] array (DPC was an
> > exception, where the dev[] array was unused).

> > -void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
> > +void aer_print_error(struct aer_err_info *info, int i)
> >  {
> > -	int layer, agent;
> > -	int id = pci_dev_id(dev);
> > +	struct pci_dev *dev;
> > +	int layer, agent, id;
> >  	const char *level = info->level;
> >  
> > +	if (i >= AER_MAX_MULTI_ERR_DEVICES)
> > +		return;
> 
> Are these OoB checks actually indication of a logic error in the caller 
> side which would perhaps warrant using
> 	if (WARN_ON_ONCE(i >= AER_MAX_MULTI_ERR_DEVICES))
> ?

Good idea, thanks!  I hope we can someday get rid of this info->dev[]
array and the headaches associated with it.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 00/20] Rate limit AER logs
  2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
                   ` (19 preceding siblings ...)
  2025-05-22 23:21 ` [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits Bjorn Helgaas
@ 2025-05-23 16:21 ` Bjorn Helgaas
  20 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2025-05-23 16:21 UTC (permalink / raw)
  To: linux-pci
  Cc: Jon Pan-Doh, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas

On Thu, May 22, 2025 at 06:21:06PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> This work is mostly due to Jon Pan-Doh and Karolina Stolarek.  I rebased
> this to v6.15-rc1, factored out some of the trace and statistics updates,
> and added some minor cleanups.
> 
> I pushed this to pci/aer at
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aer
> (head a524e63307cf ("PCI/AER: Add sysfs attributes for log ratelimits"))
> and appended the interdiff from v7 to v8 below.
> 
> Proposal
> ========
> 
> When using native AER, spammy devices can flood kernel logs with AER errors
> and slow/stall execution. Add per-device per-error-severity ratelimits for
> more robust error logging. Allow userspace to configure ratelimits via
> sysfs knobs.
> 
> Motivation
> ==========
> 
> Inconsistent PCIe error handling, exacerbated at datacenter scale (myriad
> of devices), affects repairabilitiy flows for fleet operators.
> 
> Exposing PCIe errors/debug info in-band for a userspace daemon (e.g.
> rasdaemon) to collect/pass on to repairability services will allow for more
> predictable repair flows and decrease machine downtime.
> 
> Background
> ==========
> 
> AER error spam has been observed many times, both publicly (e.g. [1], [2],
> [3]) and privately. While it usually occurs with correctable errors, it can
> happen with uncorrectable errors (e.g. during new HW bringup).
> 
> There have been previous attempts to add ratelimits to AER logs ([4], [5]).
> The most recent attempt[5] has many similarities with the proposed
> approach.
> 
> 
> v8:
> - Rename sysfs ratelimit burst files:
>     ratelimit_burst_cor_log -> correctable_ratelimit_burst (Sathy)
>     ratelimit_burst_uncor_log -> nonfatal_ratelimit_burst
> - Split sysfs ratelimit_log_enable for correctable and nonfatal and make it
>   an interval instead of a toggle:
>     ratelimit_log_enable -> correctable_ratelimit_interval_ms
>                          -> nonfatal_ratelimit_interval_ms
> - Rework aer_get_device_error_info() and aer_print_error() to take an index
>   instead of pci_dev pointer
> - Move trace_aer_event() out of pci_dev_aer_stats_incr() (Jonathan)
> - Move AER_FATAL checking to aer_ratelimit() to avoid calling
>   __ratelimit(nonfatal_ratelimit) when we know we don't want to ratelimit
>   fatal errors (Jonathan)
> - Move all Error Source ID string building into aer_print_source() instead
>   of putting part in caller (Jonathan)
> - Rename struct aer_err_info.ratelimit -> ratelimit_print[] (Jonathan)
> - Pass printk level into pcie_print_tlp_log() (Jonathan)
> - Rework Error Source ratelimiting vs detail ratelimiting (Jonathan)
> v7: https://lore.kernel.org/r/20250520215047.1350603-1-helgaas@kernel.org
> - Update sysfs doc target kernel version & date (Ilpo)
> - Fix sysfs doc "AER ratelimiting" typo (Ilpo)
> - Ratelimit Correctable and Non-Fatal but not Fatal errors (Sathy)
> - Rename "struct aer_report" to "aer_info" (Sathy)
> - Expand comments about combining ratelimit for multiple devices (Ilpo)
> - Rework Error Source logging ratelimiting (Sathy)
> - Factor out aer_isr_one_error_type() to reduce code duplication
> - Log DPC errors, which are all Fatal, at KERN_ERR (Sathy)
> - Improve dpc_process_error() structure (Ilpo)
> v6: https://lore.kernel.org/r/20250519213603.1257897-1-helgaas@kernel.org
> - Rebase to v6.15-rc1
> - Initialize struct aer_err_info completely before using it
> - Log DPC Error Source ID only when it's valid
> - Consolidate AER Error Source ID logging to one place
> - Tidy Error Source ID bus/dev/fn decoding using macros
> - Rename aer_print_port_info() to aer_print_source()
> - Consolidate trace events and statistic updates to one non-ratelimited place
> - Save log level in struct aer_err_info instead of passing as parameter
> v5: https://lore.kernel.org/r/20250321015806.954866-1-pandoh@google.com
> - Handle multi-error AER by evaluating ratelimits once and storing result
> - Reword/rename commit messages/functions/variable
> v4: https://lore.kernel.org/r/20250320082057.622983-1-pandoh@google.com
> - Fix bug where trace not emitted with malformed aer_err_info
> - Extend ratelimit to malformed aer_err_info
> - Update commit messages with patch motivation
> - Squash AER sysfs filename change (Patch 8)
> v3: https://lore.kernel.org/r/20250319084050.366718-1-pandoh@google.com
> - Ratelimit aer_print_port_info() (drop Patch 1)
> - Add ratelimit enable toggle
> - Move trace outside of ratelimit
> - Split log level (Patch 2) into two
> - More descriptive documentation/sysfs naming
> v2: https://lore.kernel.org/r/20250214023543.992372-1-pandoh@google.com
> - Rebased on top of pci/aer (6.14.rc-1)
> - Split series into log and IRQ ratelimits (defer patch 5)
> - Dropped patch 8 (Move AER sysfs)
> - Added log level cleanup patch[7] from Karolina's series
> - Fixed bug where dpc errors didn't increment counters
> - "X callbacks suppressed" message on ratelimit release -> immediately
> - Separate documentation into own patch
> v1: https://lore.kernel.org/r/20250115074301.3514927-1-pandoh@google.com
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=215027
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=201517
> [3] https://bugzilla.kernel.org/show_bug.cgi?id=196183
> [4] https://lore.kernel.org/linux-pci/20230606035442.2886343-2-grundler@chromium.org/
> [5] https://lore.kernel.org/linux-pci/cover.1736341506.git.karolina.stolarek@oracle.com/
> [6]
> https://lore.kernel.org/linux-pci/8bcb8c9a7b38ce3bdaca5a64fe76f08b0b337511.1742202797.git.k
> arolina.stolarek@oracle.com/
> [7]
> https://lore.kernel.org/linux-pci/edd77011aafad4c0654358a26b4e538d0c5a321d.1736341506.git.k
> arolina.stolarek@oracle.com/
> 
> 
> 
> Bjorn Helgaas (13):
>   PCI/DPC: Initialize aer_err_info before using it
>   PCI/DPC: Log Error Source ID only when valid
>   PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error()
>   PCI/AER: Consolidate Error Source ID logging in
>     aer_isr_one_error_type()
>   PCI/AER: Extract bus/dev/fn in aer_print_port_info() with
>     PCI_BUS_NUM(), etc
>   PCI/AER: Move aer_print_source() earlier in file
>   PCI/AER: Initialize aer_err_info before using it
>   PCI/AER: Simplify pci_print_aer()
>   PCI/AER: Update statistics before ratelimiting
>   PCI/AER: Trace error event before ratelimiting
>   PCI/ERR: Add printk level to pcie_print_tlp_log()
>   PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to
>     index
>   PCI/AER: Simplify add_error_device()
> 
> Jon Pan-Doh (4):
>   PCI/AER: Rename aer_print_port_info() to aer_print_source()
>   PCI/AER: Ratelimit correctable and non-fatal error logging
>   PCI/AER: Add ratelimits to PCI AER Documentation
>   PCI/AER: Add sysfs attributes for log ratelimits
> 
> Karolina Stolarek (3):
>   PCI/AER: Check log level once and remember it
>   PCI/AER: Reduce pci_print_aer() correctable error level to
>     KERN_WARNING
>   PCI/AER: Rename struct aer_stats to aer_info
> 
>  ...es-aer_stats => sysfs-bus-pci-devices-aer} |  44 ++
>  Documentation/PCI/pcieaer-howto.rst           |  17 +-
>  drivers/pci/pci-sysfs.c                       |   1 +
>  drivers/pci/pci.h                             |  13 +-
>  drivers/pci/pcie/aer.c                        | 441 +++++++++++++-----
>  drivers/pci/pcie/dpc.c                        |  73 +--
>  drivers/pci/pcie/tlp.c                        |   6 +-
>  include/linux/pci.h                           |   2 +-
>  8 files changed, 430 insertions(+), 167 deletions(-)
>  rename Documentation/ABI/testing/{sysfs-bus-pci-devices-aer_stats => sysfs-bus-pci-devices-aer} (72%)

I applied this series to pci/aer for v6.16 with the updates below
suggested by Sathy and Ilpo.

My heartfelt thanks to the authors, Jon and Karolina, for seeing the
need for this and putting in the effort, and to all the reviewers who
put so much time and care into reading and polishing it on such a
tight schedule.


diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 6c331695af58..70ac66188367 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -786,17 +786,14 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
 
 static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
 {
-	struct ratelimit_state *ratelimit;
-
-	if (severity == AER_FATAL)
-		return 1;	/* AER_FATAL not ratelimited */
-
-	if (severity == AER_CORRECTABLE)
-		ratelimit = &dev->aer_info->correctable_ratelimit;
-	else
-		ratelimit = &dev->aer_info->nonfatal_ratelimit;
-
-	return __ratelimit(ratelimit);
+	switch (severity) {
+	case AER_NONFATAL:
+		return __ratelimit(&dev->aer_info->nonfatal_ratelimit);
+	case AER_CORRECTABLE:
+		return __ratelimit(&dev->aer_info->correctable_ratelimit);
+	default:
+		return 1;	/* Don't ratelimit fatal errors */
+	}
 }
 
 static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
@@ -841,7 +838,7 @@ void aer_print_error(struct aer_err_info *info, int i)
 	int layer, agent, id;
 	const char *level = info->level;
 
-	if (i >= AER_MAX_MULTI_ERR_DEVICES)
+	if (WARN_ON_ONCE(i >= AER_MAX_MULTI_ERR_DEVICES))
 		return;
 
 	dev = info->dev[i];

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log()
  2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
  2025-05-22 23:44   ` Sathyanarayanan Kuppuswamy
  2025-05-23  9:56   ` Ilpo Järvinen
@ 2025-05-28  6:38   ` Lukas Wunner
  2025-05-28 10:00     ` Ilpo Järvinen
  2 siblings, 1 reply; 36+ messages in thread
From: Lukas Wunner @ 2025-05-28  6:38 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Jonathan Cameron,
	Sargun Dhillon, Paul E . McKenney, Mahesh J Salgaonkar,
	Oliver O'Halloran, Kai-Heng Feng, Keith Busch, Robert Richter,
	Terry Bowman, Shiju Jose, Dave Jiang, linux-kernel, linuxppc-dev,
	Bjorn Helgaas

On Thu, May 22, 2025 at 06:21:19PM -0500, Bjorn Helgaas wrote:
> @@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
>  		}
>  	}
>  
> -	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
> +	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
>  		log->flit ? " (Flit)" : "", buf);
>  }

Nit: pci_printk() ?

The definition in include/linux/pci.h was retained by fab874e12593.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log()
  2025-05-28  6:38   ` Lukas Wunner
@ 2025-05-28 10:00     ` Ilpo Järvinen
  0 siblings, 0 replies; 36+ messages in thread
From: Ilpo Järvinen @ 2025-05-28 10:00 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Bjorn Helgaas, linux-pci, Jon Pan-Doh, Karolina Stolarek,
	Weinan Liu, Martin Petersen, Ben Fuller, Drew Walton,
	Anil Agrawal, Tony Luck, Sathyanarayanan Kuppuswamy,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	LKML, linuxppc-dev, Bjorn Helgaas

On Wed, 28 May 2025, Lukas Wunner wrote:

> On Thu, May 22, 2025 at 06:21:19PM -0500, Bjorn Helgaas wrote:
> > @@ -130,6 +132,6 @@ void pcie_print_tlp_log(const struct pci_dev *dev,
> >  		}
> >  	}
> >  
> > -	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
> > +	dev_printk(level, &dev->dev, "%sTLP Header%s: %s\n", pfx,
> >  		log->flit ? " (Flit)" : "", buf);
> >  }
> 
> Nit: pci_printk() ?
> 
> The definition in include/linux/pci.h was retained by fab874e12593.

If pci_printk() is taken into use once again, there's 56d305b24d64 ("PCI: 
Remove pci_printk()") queued already in pci/misc which should be dropped 
in that case.

-- 
 i.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging
  2025-05-22 23:21 ` [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging Bjorn Helgaas
  2025-05-22 23:56   ` Sathyanarayanan Kuppuswamy
@ 2025-08-01 13:16   ` Breno Leitao
  2025-08-01 13:35     ` Breno Leitao
  1 sibling, 1 reply; 36+ messages in thread
From: Breno Leitao @ 2025-08-01 13:16 UTC (permalink / raw)
  To: Bjorn Helgaas, pandoh
  Cc: linux-pci, Jon Pan-Doh, Karolina Stolarek, Weinan Liu,
	Martin Petersen, Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas, kernel-team, gustavold

Hello Jon, Bjorn,

On Thu, May 22, 2025 at 06:21:24PM -0500, Bjorn Helgaas wrote:
> @@ -790,6 +818,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>  	trace_aer_event(pci_name(dev), (status & ~mask),
>  			aer_severity, tlp_header_valid, &aer->header_log);
>  
> +	if (!aer_ratelimit(dev, info.severity))
> +		return;

I am seeing a kernel NULL pointer in the aer_ratelimit(), where
dev->aer_info is NULL. This is happening on linus final 6.16 commit id.

Here is dmesg:

	{1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 0
	{1}[Hardware Error]: It has been corrected by h/w and requires no further action
	{1}[Hardware Error]: event severity: corrected
	{1}[Hardware Error]:  Error 0, type: corrected
	{1}[Hardware Error]:   section_type: PCIe error
	{1}[Hardware Error]:   port_type: 4, root port
	{1}[Hardware Error]:   version: 3.0
	{1}[Hardware Error]:   command: 0x0540, status: 0x0010
	{1}[Hardware Error]:   device_id: 0000:00:00.0
	{1}[Hardware Error]:   slot: 0
	{1}[Hardware Error]:   secondary_bus: 0x00
	{1}[Hardware Error]:   vendor_id: 0x8086, device_id: 0x2020
	{1}[Hardware Error]:   class_code: 060000
	{1}[Hardware Error]:   aer_cor_status: 0x00001000, aer_cor_mask: 0x00002000
	{1}[Hardware Error]:   aer_uncor_status: 0x00000000, aer_uncor_mask: 0x00100000
	{1}[Hardware Error]:   aer_uncor_severity: 0x000e3030
	{1}[Hardware Error]:   TLP Header: 00000000 00000000 00000000 00000000
	BUG: kernel NULL pointer dereference, address: 0000000000000264
	#PF: supervisor read access in kernel mode
	#PF: error_code(0x0000) - not-present page
	PGD d4c32b067 P4D d4c32b067 PUD 4e4b84067 PMD 0 
	Oops: Oops: 0000 [#1] SMP
	CPU: 0 UID: 0 PID: 1553927 Comm: kworker/0:0 Kdump: loaded Tainted: G S          E       6.16.0-0_fbk0_rc1_0_geb76fb6facf5 #1 NONE 
	Tainted: [S]=CPU_OUT_OF_SPEC, [E]=UNSIGNED_MODULE
	Hardware name: Quanta Delta Lake MP 29F0EMA01C0/Delta Lake-Class1, BIOS F0E_3A21 06/27/2024
	Workqueue: events aer_recover_work_func
	RIP: 0010:___ratelimit+0xc/0x1b0
	Code: 48 8b 3d ef aa 59 02 e8 72 75 31 ff ff 8b 10 78 54 84 75 da 31 c0 5b c3 cc cc cc cc cc cc 55 41 57 41 56 41 54 53 50 48 89 fb <4c> 63 7f 04 4d 85 ff 0f 9e c0 8b 6f 08 85 ed 0f 9e c1 08 c1 80 f9
	RSP: 0018:ffffc90028337d10 EFLAGS: 00010206
	RAX: ffffc900011f9174 RBX: 0000000000000260 RCX: 0000000000000000
	RDX: 0000000000001000 RSI: ffffffff827ea3c0 RDI: 0000000000000260
	RBP: 0000000000000002 R08: 8080808080808080 R09: fefefefefefefeff
	R10: 000073746e657665 R11: 8080000000000000 R12: 0000000000001000
	R13: 0000000000000002 R14: ffff8882877da000 R15: ffffc900011f9158
	FS:  0000000000000000(0000) GS:ffff8890b1cf9000(0000) knlGS:0000000000000000
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: 0000000000000264 CR3: 00000003cee6a004 CR4: 00000000007726f0
	PKRU: 55555554
	Call Trace:
	<TASK>
	pci_print_aer+0x141/0x360
	aer_recover_work_func+0xb5/0x130
	process_scheduled_works+0x1a4/0x360
	worker_thread+0x2df/0x3b0
	kthread+0x1d2/0x1f0
	? pr_cont_work+0x1c0/0x1c0
	? finish_task_switch+0x213/0x2e0
	? kthread_blkcg+0x30/0x30
	ret_from_fork+0x69/0xe0
	? kthread_blkcg+0x30/0x30
	ret_from_fork_asm+0x11/0x20
	</TASK>

Here is the decoded stack from the crash dump:

	>>> trace
	#0  ___ratelimit (lib/ratelimit.c:33:17)
	#1  aer_ratelimit (drivers/pci/pcie/aer.c:0:2)
	#2  pci_print_aer (drivers/pci/pcie/aer.c:923:7)
	#3  aer_recover_work_func (drivers/pci/pcie/aer.c:1298:3)
	#4  process_one_work (kernel/workqueue.c:3238:2)
	#5  process_scheduled_works (kernel/workqueue.c:3321:3)
	#6  worker_thread (kernel/workqueue.c:3402:4)
	#7  kthread (kernel/kthread.c:464:9)
	#8  ret_from_fork (arch/x86/kernel/process.c:148:3)
	#9  ret_from_fork_asm+0x11/0x16 (arch/x86/entry/entry_64.S:245)

And aer_info is NULL

	>>> trace[2]['dev'].aer_info
	(struct aer_info *)0x0

So, somehow the PCI was released at this point?

Here is the state of the device at the crash time:

	>>> trace[2]['dev']
	*(struct pci_dev *)0xffff8882877da000 = {
		.bus_list = (struct list_head){
			.next = (struct list_head *)0xffff8882877db000,
			.prev = (struct list_head *)0xffff888287674428,
		},
		.bus = (struct pci_bus *)0xffff888287674400,
		.subordinate = (struct pci_bus *)0x0,
		.sysdata = (void *)0xffff888284c94b38,
		.procent = (struct proc_dir_entry *)0xffff88828a515980,
		.slot = (struct pci_slot *)0x0,
		.devfn = (unsigned int)0,
		.vendor = (unsigned short)32902,
		.device = (unsigned short)8224,
		.subsystem_vendor = (unsigned short)32902,
		.subsystem_device = (unsigned short)0,
		.class = (unsigned int)393216,
		.revision = (u8)11,
		.hdr_type = (u8)0,
		.aer_cap = (u16)0,
		.aer_info = (struct aer_info *)0x0,
		.rcec_ea = (struct rcec_ea *)0x0,
		.rcec = (struct pci_dev *)0x0,
		.devcap = (u32)32768,
		.rebar_cap = (u16)0,
		.pcie_cap = (u8)144,
		.msi_cap = (u8)0,
		.msix_cap = (u8)0,
		.pcie_mpss = (u8)0,
		.rom_base_reg = (u8)48,
		.pin = (u8)1,
		.pcie_flags_reg = (u16)66,
		.dma_alias_mask = (unsigned long *)0x0,
		.driver = (struct pci_driver *)0x0,
		.dma_mask = (u64)4294967295,
		.dma_parms = (struct device_dma_parameters){
			.max_segment_size = (unsigned int)65536,
			.min_align_mask = (unsigned int)0,
			.segment_boundary_mask = (unsigned long)4294967295,
		},
		.current_state = (pci_power_t)0,
		.pm_cap = (u8)224,
		.pme_support = (unsigned int)0,
		.pme_poll = (unsigned int)0,
		.pinned = (unsigned int)0,
		.config_rrs_sv = (unsigned int)0,
		.imm_ready = (unsigned int)0,
		.d1_support = (unsigned int)0,
		.d2_support = (unsigned int)0,
		.no_d1d2 = (unsigned int)0,
		.no_d3cold = (unsigned int)0,
		.bridge_d3 = (unsigned int)1,
		.d3cold_allowed = (unsigned int)1,
		.mmio_always_on = (unsigned int)1,
		.wakeup_prepared = (unsigned int)0,
		.skip_bus_pm = (unsigned int)0,
		.ignore_hotplug = (unsigned int)0,
		.hotplug_user_indicators = (unsigned int)0,
		.clear_retrain_link = (unsigned int)0,
		.d3hot_delay = (unsigned int)10,
		.d3cold_delay = (unsigned int)100,
		.l1ss = (u16)0,
		.link_state = (struct pcie_link_state *)0x0,
		.ltr_path = (unsigned int)0,
		.pasid_no_tlp = (unsigned int)0,
		.eetlp_prefix_max = (unsigned int)0,
		.error_state = (pci_channel_state_t)1,
		.dev = (struct device){
			.kobj = (struct kobject){
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.entry = (struct list_head){
					.next = (struct list_head *)0xffff8882877db0d0,
					.prev = (struct list_head *)0xffff888287674520,
				},
				.parent = (struct kobject *)0xffff888287674000,
				.kset = (struct kset *)0xffff888281f19240,
				.ktype = (const struct kobj_type *)device_ktype+0x0 = 0xffffffff82a41fd0,
				.sd = (struct kernfs_node *)0xffff888287859220,
				.kref = (struct kref){
					.refcount = (refcount_t){
						.refs = (atomic_t){
							.counter = (int)5,
						},
					},
				},
				.state_initialized = (unsigned int)1,
				.state_in_sysfs = (unsigned int)1,
				.state_add_uevent_sent = (unsigned int)1,
				.state_remove_uevent_sent = (unsigned int)0,
				.uevent_suppress = (unsigned int)0,
			},
			.parent = (struct device *)0xffff888287674000,
			.p = (struct device_private *)0xffff88828771e000,
			.init_name = (const char *)0x0,
			.type = (const struct device_type *)pci_dev_type+0x0 = 0xffffffff82a03fe0,
			.bus = (const struct bus_type *)pci_bus_type+0x0 = 0xffffffff82a057f8,
			.driver = (struct device_driver *)0x0,
			.platform_data = (void *)0x0,
			.driver_data = (void *)0x0,
			.mutex = (struct mutex){
				.owner = (atomic_long_t){
					.counter = (s64)0,
				},
				.wait_lock = (raw_spinlock_t){
					.raw_lock = (arch_spinlock_t){
						.val = (atomic_t){
							.counter = (int)0,
						},
						.locked = (u8)0,
						.pending = (u8)0,
						.locked_pending = (u16)0,
						.tail = (u16)0,
					},
				},
				.osq = (struct optimistic_spin_queue){
					.tail = (atomic_t){
						.counter = (int)0,
					},
				},
				.wait_list = (struct list_head){
					.next = (struct list_head *)0xffff8882877da158,
					.prev = (struct list_head *)0xffff8882877da158,
				},
			},
			.links = (struct dev_links_info){
				.suppliers = (struct list_head){
					.next = (struct list_head *)0xffff8882877da168,
					.prev = (struct list_head *)0xffff8882877da168,
				},
				.consumers = (struct list_head){
					.next = (struct list_head *)0xffff8882877da178,
					.prev = (struct list_head *)0xffff8882877da178,
				},
				.defer_sync = (struct list_head){
					.next = (struct list_head *)0xffff8882877da188,
					.prev = (struct list_head *)0xffff8882877da188,
				},
				.status = (enum dl_dev_state)DL_DEV_NO_DRIVER,
			},
			.power = (struct dev_pm_info){
				.power_state = (pm_message_t){
					.event = (int)0,
				},
				.can_wakeup = (bool)0,
				.async_suspend = (bool)1,
				.in_dpm_list = (bool)0,
				.is_prepared = (bool)0,
				.is_suspended = (bool)0,
				.is_noirq_suspended = (bool)0,
				.is_late_suspended = (bool)0,
				.no_pm = (bool)0,
				.early_init = (bool)1,
				.direct_complete = (bool)0,
				.driver_flags = (u32)0,
				.lock = (spinlock_t){
					.rlock = (struct raw_spinlock){
						.raw_lock = (arch_spinlock_t){
							.val = (atomic_t){
								.counter = (int)0,
							},
							.locked = (u8)0,
							.pending = (u8)0,
							.locked_pending = (u16)0,
							.tail = (u16)0,
						},
					},
				},
				.should_wakeup = (bool)0,
				.subsys_data = (struct pm_subsys_data *)0x0,
				.set_latency_tolerance = (void (*)(struct device *, s32))0x0,
				.qos = (struct dev_pm_qos *)0x0,
			},
			.pm_domain = (struct dev_pm_domain *)0x0,
			.msi = (struct dev_msi_info){
				.domain = (struct irq_domain *)0xffff888281000000,
				.data = (struct msi_device_data *)0x0,
			},
			.dma_mask = (u64 *)0xffff8882877da088,
			.coherent_dma_mask = (u64)4294967295,
			.bus_dma_limit = (u64)0,
			.dma_range_map = (const struct bus_dma_region *)0x0,
			.dma_parms = (struct device_dma_parameters *)0xffff8882877da090,
			.dma_pools = (struct list_head){
				.next = (struct list_head *)0xffff8882877da210,
				.prev = (struct list_head *)0xffff8882877da210,
			},
			.cma_area = (struct cma *)0x0,
			.dma_io_tlb_mem = (struct io_tlb_mem *)io_tlb_default_mem.llvm.9097096581342952386+0x0 = 0xffffffff8477f850,
			.archdata = (struct dev_archdata){},
			.of_node = (struct device_node *)0x0,
			.fwnode = (struct fwnode_handle *)0xffff888287484010,
			.numa_node = (int)0,
			.devt = (dev_t)0,
			.id = (u32)0,
			.devres_lock = (spinlock_t){
				.rlock = (struct raw_spinlock){
					.raw_lock = (arch_spinlock_t){
						.val = (atomic_t){
							.counter = (int)0,
						},
						.locked = (u8)0,
						.pending = (u8)0,
						.locked_pending = (u16)0,
						.tail = (u16)0,
					},
				},
			},
			.devres_head = (struct list_head){
				.next = (struct list_head *)0xffff8882877da250,
				.prev = (struct list_head *)0xffff8882877da250,
			},
			.class = (const struct class *)0x0,
			.groups = (const struct attribute_group **)0x0,
			.release = (void (*)(struct device *))pci_release_dev+0x0 = 0xffffffff81b11680,
			.iommu_group = (struct iommu_group *)0x0,
			.iommu = (struct dev_iommu *)0x0,
			.physical_location = (struct device_physical_location *)0x0,
			.removable = (enum device_removable)DEVICE_REMOVABLE_NOT_SUPPORTED,
			.offline_disabled = (bool)0,
			.offline = (bool)0,
			.of_node_reused = (bool)0,
			.state_synced = (bool)0,
			.can_match = (bool)0,
			.dma_skip_sync = (bool)0,
			.dma_iommu = (bool)0,
		},
		.cfg_size = (int)4096,
		.irq = (unsigned int)0,
		.resource = (struct resource [17]){
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
			{
				.start = (resource_size_t)0,
				.end = (resource_size_t)0,
				.name = (const char *)0xffff888287645e50 = "0000:00:00.0",
				.flags = (unsigned long)0,
				.desc = (unsigned long)0,
				.parent = (struct resource *)0x0,
				.sibling = (struct resource *)0x0,
				.child = (struct resource *)0x0,
			},
		},
		.driver_exclusive_resource = (struct resource){
			.start = (resource_size_t)0,
			.end = (resource_size_t)18446744073709551615,
			.name = (const char *)0xffffffff82901265 = "PCI Exclusive",
			.flags = (unsigned long)0,
			.desc = (unsigned long)0,
			.parent = (struct resource *)0x0,
			.sibling = (struct resource *)0x0,
			.child = (struct resource *)0x0,
		},
		.transparent = (unsigned int)0,
		.io_window = (unsigned int)0,
		.pref_window = (unsigned int)0,
		.pref_64_window = (unsigned int)0,
		.multifunction = (unsigned int)0,
		.is_busmaster = (unsigned int)0,
		.no_msi = (unsigned int)0,
		.no_64bit_msi = (unsigned int)0,
		.block_cfg_access = (unsigned int)0,
		.broken_parity_status = (unsigned int)0,
		.irq_reroute_variant = (unsigned int)0,
		.msi_enabled = (unsigned int)0,
		.msix_enabled = (unsigned int)0,
		.ari_enabled = (unsigned int)0,
		.ats_enabled = (unsigned int)0,
		.pasid_enabled = (unsigned int)0,
		.pri_enabled = (unsigned int)0,
		.tph_enabled = (unsigned int)0,
		.is_managed = (unsigned int)0,
		.is_msi_managed = (unsigned int)0,
		.needs_freset = (unsigned int)0,
		.state_saved = (unsigned int)0,
		.is_physfn = (unsigned int)0,
		.is_virtfn = (unsigned int)0,
		.is_hotplug_bridge = (unsigned int)0,
		.shpc_managed = (unsigned int)0,
		.is_thunderbolt = (unsigned int)0,
		.untrusted = (unsigned int)0,
		.external_facing = (unsigned int)0,
		.broken_intx_masking = (unsigned int)0,
		.io_window_1k = (unsigned int)0,
		.irq_managed = (unsigned int)0,
		.non_compliant_bars = (unsigned int)0,
		.is_probed = (unsigned int)0,
		.link_active_reporting = (unsigned int)1,
		.no_vf_scan = (unsigned int)0,
		.no_command_memory = (unsigned int)0,
		.rom_bar_overlap = (unsigned int)0,
		.rom_attr_enabled = (unsigned int)0,
		.non_mappable_bars = (unsigned int)0,
		.dev_flags = (pci_dev_flags_t)0,
		.enable_cnt = (atomic_t){
			.counter = (int)0,
		},
		.pcie_cap_lock = (spinlock_t){
			.rlock = (struct raw_spinlock){
				.raw_lock = (arch_spinlock_t){
					.val = (atomic_t){
						.counter = (int)0,
					},
					.locked = (u8)0,
					.pending = (u8)0,
					.locked_pending = (u16)0,
					.tail = (u16)0,
				},
			},
		},
		.saved_config_space = (u32 [16]){},
		.saved_cap_space = (struct hlist_head){
			.first = (struct hlist_node *)0xffff8882867cf300,
		},
		.res_attr = (struct bin_attribute *[17]){},
		.res_attr_wc = (struct bin_attribute *[17]){},
		.broken_cmd_compl = (unsigned int)0,
		.ptm_cap = (u16)0,
		.ptm_root = (unsigned int)0,
		.ptm_enabled = (unsigned int)0,
		.ptm_granularity = (u8)0,
		.msix_base = (void *)0x0,
		.msi_lock = (raw_spinlock_t){
			.raw_lock = (arch_spinlock_t){
				.val = (atomic_t){
					.counter = (int)0,
				},
				.locked = (u8)0,
				.pending = (u8)0,
				.locked_pending = (u16)0,
				.tail = (u16)0,
			},
		},
		.vpd = (struct pci_vpd){
			.lock = (struct mutex){
				.owner = (atomic_long_t){
					.counter = (s64)0,
				},
				.wait_lock = (raw_spinlock_t){
					.raw_lock = (arch_spinlock_t){
						.val = (atomic_t){
							.counter = (int)0,
						},
						.locked = (u8)0,
						.pending = (u8)0,
						.locked_pending = (u16)0,
						.tail = (u16)0,
					},
				},
				.osq = (struct optimistic_spin_queue){
					.tail = (atomic_t){
						.counter = (int)0,
					},
				},
				.wait_list = (struct list_head){
					.next = (struct list_head *)0xffff8882877da8b0,
					.prev = (struct list_head *)0xffff8882877da8b0,
				},
			},
			.len = (unsigned int)0,
			.cap = (u8)0,
		},
		.dpc_cap = (u16)0,
		.dpc_rp_extensions = (unsigned int)0,
		.dpc_rp_log_size = (u8)0,
		.link_bwctrl = (struct pcie_bwctrl_data *)0x0,
		.sriov = (struct pci_sriov *)0x0,
		.physfn = (struct pci_dev *)0x0,
		.ats_cap = (u16)0,
		.ats_stu = (u8)0,
		.pri_cap = (u16)0,
		.pri_reqs_alloc = (u32)0,
		.pasid_required = (unsigned int)0,
		.pasid_cap = (u16)0,
		.pasid_features = (u16)0,
		.p2pdma = (struct pci_p2pdma *)0x0,
		.doe_mbs = (struct xarray){
			.xa_lock = (spinlock_t){
				.rlock = (struct raw_spinlock){
					.raw_lock = (arch_spinlock_t){
						.val = (atomic_t){
							.counter = (int)0,
						},
						.locked = (u8)0,
						.pending = (u8)0,
						.locked_pending = (u16)0,
						.tail = (u16)0,
					},
				},
			},
			.xa_flags = (gfp_t)0,
			.xa_head = (void *)0x0,
		},
		.acs_cap = (u16)0,
		.supported_speeds = (u8)14,
		.rom = (phys_addr_t)0,
		.romlen = (size_t)0,
		.driver_override = (const char *)0x0,
		.priv_flags = (unsigned long)129,
		.reset_methods = (u8 [8]){},
	}

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging
  2025-08-01 13:16   ` Breno Leitao
@ 2025-08-01 13:35     ` Breno Leitao
  0 siblings, 0 replies; 36+ messages in thread
From: Breno Leitao @ 2025-08-01 13:35 UTC (permalink / raw)
  To: Bjorn Helgaas, pandoh
  Cc: linux-pci, Karolina Stolarek, Weinan Liu, Martin Petersen,
	Ben Fuller, Drew Walton, Anil Agrawal, Tony Luck,
	Ilpo Järvinen, Sathyanarayanan Kuppuswamy, Lukas Wunner,
	Jonathan Cameron, Sargun Dhillon, Paul E . McKenney,
	Mahesh J Salgaonkar, Oliver O'Halloran, Kai-Heng Feng,
	Keith Busch, Robert Richter, Terry Bowman, Shiju Jose, Dave Jiang,
	linux-kernel, linuxppc-dev, Bjorn Helgaas, kernel-team, gustavold

On Fri, Aug 01, 2025 at 06:16:29AM -0700, Breno Leitao wrote:
> Hello Jon, Bjorn,
> 
> On Thu, May 22, 2025 at 06:21:24PM -0500, Bjorn Helgaas wrote:
> > @@ -790,6 +818,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
> >  	trace_aer_event(pci_name(dev), (status & ~mask),
> >  			aer_severity, tlp_header_valid, &aer->header_log);
> >  
> > +	if (!aer_ratelimit(dev, info.severity))
> > +		return;
> 
> I am seeing a kernel NULL pointer in the aer_ratelimit(), where
> dev->aer_info is NULL. This is happening on linus final 6.16 commit id.

Upon closer examination of the code, it appears we can replicate the
functionality of `pci_dev_aer_stats_incr()`, which is similarly invoked
within this code path.

commit 1b4ef90e8397eaf2bc4d0f8a2127d2d75c7ff5e0
Author: Breno Leitao <leitao@debian.org>
Date:   Fri Aug 1 06:32:26 2025 -0700

    PCI/AER: Check for NULL aer_info before ratelimiting in pci_print_aer()
    
    Similarly to pci_dev_aer_stats_incr(), pci_print_aer() may be called
    when dev->aer_info is NULL. Add a NULL check before proceeding to avoid
    calling aer_ratelimit() with a NULL aer_info pointer, returning 1, which
    does not rate limit, given this is fatal.
    
    This prevents a kernel crash triggered by dereferencing a NULL pointer
    in aer_ratelimit(), ensuring safer handling of PCI devices that lack
    AER info. This change aligns pci_print_aer() with pci_dev_aer_stats_incr()
    which already performs this NULL check.
    
    Signed-off-by: Breno Leitao <leitao@debian.org>
    Fixes: a57f2bfb4a5863 ("PCI/AER: Ratelimit correctable and non-fatal
    error logging")

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 70ac661883672..b5f96fde4dcda 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -786,6 +786,9 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
 
 static int aer_ratelimit(struct pci_dev *dev, unsigned int severity)
 {
+	if (!dev->aer_info)
+		return 1;
+
 	switch (severity) {
 	case AER_NONFATAL:
 		return __ratelimit(&dev->aer_info->nonfatal_ratelimit);

^ permalink raw reply related	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2025-08-01 13:35 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 23:21 [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 01/20] PCI/DPC: Initialize aer_err_info before using it Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 02/20] PCI/DPC: Log Error Source ID only when valid Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 03/20] PCI/AER: Factor COR/UNCOR error handling out from aer_isr_one_error() Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 04/20] PCI/AER: Consolidate Error Source ID logging in aer_isr_one_error_type() Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 05/20] PCI/AER: Extract bus/dev/fn in aer_print_port_info() with PCI_BUS_NUM(), etc Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 06/20] PCI/AER: Rename aer_print_port_info() to aer_print_source() Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 07/20] PCI/AER: Move aer_print_source() earlier in file Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 08/20] PCI/AER: Initialize aer_err_info before using it Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 09/20] PCI/AER: Simplify pci_print_aer() Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 10/20] PCI/AER: Update statistics before ratelimiting Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 11/20] PCI/AER: Trace error event " Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 12/20] PCI/AER: Check log level once and remember it Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 13/20] PCI/ERR: Add printk level to pcie_print_tlp_log() Bjorn Helgaas
2025-05-22 23:44   ` Sathyanarayanan Kuppuswamy
2025-05-23  9:56   ` Ilpo Järvinen
2025-05-28  6:38   ` Lukas Wunner
2025-05-28 10:00     ` Ilpo Järvinen
2025-05-22 23:21 ` [PATCH v8 14/20] PCI/AER: Reduce pci_print_aer() correctable error level to KERN_WARNING Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 15/20] PCI/AER: Rename struct aer_stats to aer_info Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 16/20] PCI/AER: Convert aer_get_device_error_info(), aer_print_error() to index Bjorn Helgaas
2025-05-22 23:58   ` Sathyanarayanan Kuppuswamy
2025-05-23 11:13   ` Ilpo Järvinen
2025-05-23 16:12     ` Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 17/20] PCI/AER: Simplify add_error_device() Bjorn Helgaas
2025-05-22 23:57   ` Sathyanarayanan Kuppuswamy
2025-05-23 11:14   ` Ilpo Järvinen
2025-05-22 23:21 ` [PATCH v8 18/20] PCI/AER: Ratelimit correctable and non-fatal error logging Bjorn Helgaas
2025-05-22 23:56   ` Sathyanarayanan Kuppuswamy
2025-05-23 16:06     ` Bjorn Helgaas
2025-08-01 13:16   ` Breno Leitao
2025-08-01 13:35     ` Breno Leitao
2025-05-22 23:21 ` [PATCH v8 19/20] PCI/AER: Add ratelimits to PCI AER Documentation Bjorn Helgaas
2025-05-22 23:21 ` [PATCH v8 20/20] PCI/AER: Add sysfs attributes for log ratelimits Bjorn Helgaas
2025-05-22 23:50   ` Sathyanarayanan Kuppuswamy
2025-05-23 16:21 ` [PATCH v8 00/20] Rate limit AER logs Bjorn Helgaas

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).