On 20/03/2025 09:20, Jon Pan-Doh wrote:
diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
similarity index 77%
rename from Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
rename to Documentation/ABI/testing/sysfs-bus-pci-devices-aer
index d1f67bb81d5d..4561653fdbde 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer
@@ -117,3 +117,37 @@ Date: July 2018
KernelVersion: 4.19.0
Contact: linux-pci@xxxxxxxxxxxxxxx, rajatja@xxxxxxxxxx
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/ratelimit_log_enable
Having a dedicated toggle for this makes sense. It would be hard to come
up with a magical number that disables ratelimiting.
+Date: March 2025
+KernelVersion: 6.15.0
+Contact: linux-pci@xxxxxxxxxxxxxxx, pandoh@xxxxxxxxxx
+Description: Writing 1/0 enables/disables AER log ratelimiting. Reading
+ gets whether or not AER is currently enabled. Enabled by
+ default.
+
+What: /sys/bus/pci/devices/<dev>/aer/ratelimit_in_5secs_cor_log
I think this attribute name (and the uncor counterpart) is too wordy. A
user can check what this knob controls by looking up this file or AER
docs, so I'd name it to "ratelimit_burst_cor_log" or something along
these lines.
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 081cef5fc678..f84ae1872fa3 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -631,6 +631,99 @@ const struct attribute_group aer_stats_attr_group = {
.is_visible = aer_stats_attrs_are_visible,
};
+/*
+ * Ratelimit enable toggle uses interval value of
+ * 0: disabled
+ * DEFAULT_RATELIMIT_INTERVAL: enabled
We set that internally, but to the user we are just operating on 0s and
1s. I would connect this comment with what we have in the documentation.
+ */
+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 enable = pdev->aer_report->cor_log_ratelimit.interval != 0;
+
+ return sysfs_emit(buf, "%d\n", enable);
+}
+
+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 (kstrtobool(buf, &enable) < 0)
+ return -EINVAL;
+
+ if (enable)
+ interval = DEFAULT_RATELIMIT_INTERVAL;
+ else
+ interval = 0;
+
+ pdev->aer_report->cor_log_ratelimit.interval = interval;
+ pdev->aer_report->uncor_log_ratelimit.interval = interval;
+ return count;
Nit, suggestion: add a blank line before return (this applies to all
returns in the patch)
All the best,
Karolina
+}
+static DEVICE_ATTR_RW(ratelimit_log_enable);
+
+/*
+ * Ratelimits are doubled as a given error produces 2 logs (root port
+ * and endpoint) that should be under same ratelimit.
+ */
+#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_report->ratelimit.burst / 2); \
+} \
+ \
+ 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 (kstrtoint(buf, 0, &burst) < 0) \
+ return -EINVAL; \
+ \
+ pdev->aer_report->ratelimit.burst = burst * 2; \
+ return count; \
+} \
+static DEVICE_ATTR_RW(name)
+
+aer_ratelimit_burst_attr(ratelimit_in_5secs_cor_log, cor_log_ratelimit);
+aer_ratelimit_burst_attr(ratelimit_in_5secs_uncor_log, uncor_log_ratelimit);
+
+static struct attribute *aer_attrs[] = {
+ &dev_attr_ratelimit_log_enable.attr,
+ &dev_attr_ratelimit_in_5secs_cor_log.attr,
+ &dev_attr_ratelimit_in_5secs_uncor_log.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_report)
+ return 0;
+ return a->mode;
+}
+
+const struct attribute_group aer_attr_group = {
+ .name = "aer",
+ .attrs = aer_attrs,
+ .is_visible = aer_attrs_are_visible,
+};
+
void pci_dev_aer_stats_incr(struct pci_dev *pdev, struct aer_err_info *info)
{
unsigned long status = info->status & ~info->mask;
