From: Dave Jiang <dave.jiang@intel.com>
To: linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org,
linux-acpi@vger.kernel.org
Cc: bhelgaas@google.com, ilpo.jarvinen@linux.intel.com,
rafael@kernel.org, lenb@kernel.org, bp@alien8.de,
guohanjun@huawei.com, mchehab@kernel.org,
xueshuai@linux.alibaba.com, jic23@kernel.org,
terry.bowman@amd.com, sashiko-bot@kernel.org
Subject: [PATCH v3] PCI/AER: Map a raw AER Capability image field by field
Date: Fri, 11 Sep 2026 09:47:05 -0700 [thread overview]
Message-ID: <20260911164705.841407-1-dave.jiang@intel.com> (raw)
struct aer_capability_regs is not the hardware layout: the embedded
struct pcie_tlp_log spans 60 bytes where the Header Log it stands in for
is 16, so a flat copy misplaces everything behind it.
extlog_print_pcie(), ghes_handle_aer() and cxl_rch_get_aer_info() work
around that by stopping at the Header Log, dropping everything past
offset 44. Flit mode is never decoded either: the Flit bit and Logged
TLP Size sit at offset 0x18, unread, so the Flit DWORDs at 0x38 are lost
and the log prints as non-Flit.
Add aer_cap_regs_unpack() to map the registers individually, taking the
TLP Log layout from the Flit bit as pcie_read_tlp_log() does. Clamp the
logged length: Logged TLP Size is 5 bits wide, so an untrusted value
reaches 31 where dw[] holds 14 and pcie_print_tlp_log() walks
header_len unbounded.
Take the image little-endian, as both sources are: CPER by definition,
PCIe registers on the wire. Read it with get_unaligned_le32(), since a
CPER section sits wherever firmware put it, and take the two source IDs
out of the Error Source Identification register by bit position rather
than byte offset. cxl_rch_get_aer_info() uses readl(), so it hands back
__le32.
Convert all three callers. cxl_rch_get_aer_info() now reads the whole
MMIO block, so size its ioremap by the hardware registers (96 bytes)
rather than by sizeof(struct aer_capability_regs) (100). Shrinking the
mapping is safe: the read it replaces stopped at 44 bytes.
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260904172337.1409775-1-dave.jiang@intel.com?part=5
Link: https://lore.kernel.org/linux-cxl/CAJZ5v0hDv11cPuztPZsaDd7uwD_49KznJy=tzuRO+dZc=CnAEQ@mail.gmail.com/
Assisted-by: LLM
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3 (sashiko-bot):
- Give the helper one endianness. v2 fed it a little-endian CPER stream
and a host-order readl() array, so on big-endian the 16-bit source IDs
came out of the wrong half of the register. No change on little-endian.
Depends on "[PATCH v6 00/13] ACPI: APEI: GHES: Collection of fixes for
issues reported by sashiko", now picked up by Rafael; please apply after
it. Patches 5 and 12 are build dependencies - this rewrites the aer_info
copies they add - and patch 7 is functional: without its
ACPI_APEI_PCIEAER guard fix, extlog_print_pcie() compiles out. The
drivers/pci and drivers/cxl changes are independent.
No Fixes: tag on purpose. Patches 5 and 12 closed the out-of-bounds
reads; what is left is diagnostic completeness, and this adds an
exported helper and rewrites three call sites.
This is the follow-up Rafael asked for in the Link: above, and also
covers the pre-existing ras_rch.c findings reported there.
---
drivers/acpi/acpi_extlog.c | 14 +-----
drivers/acpi/apei/ghes.c | 15 +------
drivers/cxl/core/ras_rch.c | 27 ++++--------
drivers/pci/pcie/tlp.c | 88 ++++++++++++++++++++++++++++++++++++++
include/linux/aer.h | 9 ++++
5 files changed, 107 insertions(+), 46 deletions(-)
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 9e61354a807b..bf5f7b1a9aea 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -156,19 +156,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
aer_severity = cper_severity_to_aer(severity);
- /*
- * struct pcie_tlp_log is larger than the hardware layout, so aer_info
- * only maps onto the struct up to the four Header Log DWORDs. Copy that
- * much, then place the TLP Prefix Log from where the hardware keeps it.
- * Everything else stays zero: nothing reads root_command, root_status or
- * the error source IDs, and header_len and flit are software-only.
- */
- memcpy(&aer_regs, pcie_err->aer_info,
- offsetof(struct aer_capability_regs, header_log) +
- PCIE_STD_NUM_TLP_HEADERLOG * sizeof(u32));
- memcpy(aer_regs.header_log.prefix,
- pcie_err->aer_info + PCI_ERR_PREFIX_LOG,
- sizeof(aer_regs.header_log.prefix));
+ aer_cap_regs_unpack(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));
domain = pcie_err->device_id.segment;
bus = pcie_err->device_id.bus;
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 08c985e729d6..ade1cc8c9f8d 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -668,20 +668,7 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
if (!aer_info)
return;
- /*
- * Map aer_info onto the struct as extlog_print_pcie() does:
- * copy up to the four Header Log DWORDs, then place the TLP
- * Prefix Log from where the hardware keeps it. The rest stays
- * zero, so firmware cannot drive the pcie_print_tlp_log() loop
- * over dw[] out of bounds.
- */
- memset(aer_info, 0, sizeof(struct aer_capability_regs));
- memcpy(aer_info, pcie_err->aer_info,
- offsetof(struct aer_capability_regs, header_log) +
- PCIE_STD_NUM_TLP_HEADERLOG * sizeof(u32));
- memcpy(aer_info->header_log.prefix,
- pcie_err->aer_info + PCI_ERR_PREFIX_LOG,
- sizeof(aer_info->header_log.prefix));
+ aer_cap_regs_unpack(aer_info, pcie_err->aer_info, sizeof(pcie_err->aer_info));
aer_recover_queue(pcie_err->device_id.segment,
pcie_err->device_id.bus,
diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
index e0e01aa5eba6..50abbdc80722 100644
--- a/drivers/cxl/core/ras_rch.c
+++ b/drivers/cxl/core/ras_rch.c
@@ -19,7 +19,7 @@ void cxl_dport_map_rch_aer(struct cxl_dport *dport)
aer_phys = aer_cap + dport->rcrb.base;
dport->regs.dport_aer =
devm_cxl_iomap_block(host, aer_phys,
- sizeof(struct aer_capability_regs));
+ PCIE_AER_CAP_HW_SIZE);
}
}
@@ -58,31 +58,20 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport)
static bool cxl_rch_get_aer_info(void __iomem *aer_base,
struct aer_capability_regs *aer_regs)
{
- /*
- * Bound the copy to the physically-defined AER registers (header
- * through the 16-byte Header Log). struct aer_capability_regs is a
- * software layout whose embedded struct pcie_tlp_log is larger than
- * the on-wire AER capability; copying sizeof(*aer_regs) would
- * over-read the RCRB-mapped MMIO block.
- */
- int read_cnt = (PCI_ERR_HEADER_LOG + 16) / sizeof(u32);
- u32 *aer_regs_buf = (u32 *)aer_regs;
- int n;
+ /* A flat copy cannot fill the struct; aer_cap_regs_unpack() places it. */
+ __le32 raw[PCIE_AER_CAP_HW_SIZE / sizeof(__le32)];
if (!aer_base)
return false;
/*
- * Zero the destination so the software-only tail fields
- * (e.g. header_log.header_len) are deterministic rather than
- * left as uninitialized stack, which could drive a bogus loop
- * length in pcie_print_tlp_log().
+ * Use readl() to guarantee 32-bit accesses; it returns host order, so
+ * put the registers back little-endian for aer_cap_regs_unpack().
*/
- memset(aer_regs, 0, sizeof(*aer_regs));
+ for (int n = 0; n < ARRAY_SIZE(raw); n++)
+ raw[n] = cpu_to_le32(readl(aer_base + n * sizeof(u32)));
- /* Use readl() to guarantee 32-bit accesses */
- for (n = 0; n < read_cnt; n++)
- aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));
+ aer_cap_regs_unpack(aer_regs, raw, sizeof(raw));
writel(aer_regs->uncor_status, aer_base + PCI_ERR_UNCOR_STATUS);
writel(aer_regs->cor_status, aer_base + PCI_ERR_COR_STATUS);
diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
index 71f8fc9ea2ed..7f94a09ac7c7 100644
--- a/drivers/pci/pcie/tlp.c
+++ b/drivers/pci/pcie/tlp.c
@@ -8,8 +8,11 @@
#include <linux/aer.h>
#include <linux/array_size.h>
#include <linux/bitfield.h>
+#include <linux/build_bug.h>
+#include <linux/minmax.h>
#include <linux/pci.h>
#include <linux/string.h>
+#include <linux/unaligned.h>
#include "../pci.h"
@@ -92,6 +95,91 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
return 0;
}
+/* The Prefix Log registers hold dw[4..13] in Flit mode. */
+static_assert((PCI_ERR_PREFIX_LOG +
+ (PCIE_STD_MAX_TLP_HEADERLOG - PCIE_STD_NUM_TLP_HEADERLOG) *
+ sizeof(u32)) == PCIE_AER_CAP_HW_SIZE);
+
+/**
+ * aer_cap_regs_unpack - Convert a raw AER Capability image to the kernel layout
+ * @regs: Destination, fully initialised
+ * @raw: AER Capability register block in hardware order, little-endian
+ * @raw_len: Bytes readable at @raw
+ *
+ * struct aer_capability_regs is not the hardware layout: struct pcie_tlp_log
+ * spans 60 bytes where the Header Log it stands in for is 16, so a flat copy
+ * misplaces everything behind it. Map the registers one by one instead,
+ * taking the TLP Log layout from the Flit bit as pcie_read_tlp_log() does.
+ *
+ * @raw is not necessarily aligned. Registers past @raw_len are left zero, so
+ * a short image cannot be read past.
+ */
+void aer_cap_regs_unpack(struct aer_capability_regs *regs, const void *raw,
+ size_t raw_len)
+{
+ unsigned int i, tlp_len;
+ bool flit;
+
+ memset(regs, 0, sizeof(*regs));
+
+ if (raw_len < PCI_ERR_HEADER_LOG)
+ return;
+
+ /* The Extended Capability Header, at offset 0, has no define */
+ regs->header = get_unaligned_le32(raw);
+ regs->uncor_status = get_unaligned_le32(raw + PCI_ERR_UNCOR_STATUS);
+ regs->uncor_mask = get_unaligned_le32(raw + PCI_ERR_UNCOR_MASK);
+ regs->uncor_severity = get_unaligned_le32(raw + PCI_ERR_UNCOR_SEVER);
+ regs->cor_status = get_unaligned_le32(raw + PCI_ERR_COR_STATUS);
+ regs->cor_mask = get_unaligned_le32(raw + PCI_ERR_COR_MASK);
+ regs->cap_control = get_unaligned_le32(raw + PCI_ERR_CAP);
+
+ flit = FIELD_GET(PCI_ERR_CAP_TLP_LOG_FLIT, regs->cap_control);
+ if (flit) {
+ tlp_len = FIELD_GET(PCI_ERR_CAP_TLP_LOG_SIZE, regs->cap_control);
+ } else {
+ /*
+ * dw[4..7] alias the Prefix Log. Take all four whatever
+ * eetlp_prefix_max says; pcie_print_tlp_log() stops at the
+ * first zero one.
+ */
+ tlp_len = PCIE_STD_NUM_TLP_HEADERLOG + PCIE_STD_MAX_TLP_PREFIXLOG;
+ }
+
+ tlp_len = min(tlp_len, ARRAY_SIZE(regs->header_log.dw));
+
+ for (i = 0; i < tlp_len; i++) {
+ unsigned int off;
+
+ if (i < PCIE_STD_NUM_TLP_HEADERLOG)
+ off = PCI_ERR_HEADER_LOG + i * sizeof(u32);
+ else
+ off = PCI_ERR_PREFIX_LOG +
+ (i - PCIE_STD_NUM_TLP_HEADERLOG) * sizeof(u32);
+
+ if (off + sizeof(u32) > raw_len)
+ break;
+ regs->header_log.dw[i] = get_unaligned_le32(raw + off);
+ }
+
+ /* @i may be short of tlp_len; non-Flit needs the TLP parsed, so cap at 4 */
+ regs->header_log.header_len = flit ? i : min(i, PCIE_STD_NUM_TLP_HEADERLOG);
+ regs->header_log.flit = flit;
+
+ if (raw_len >= PCI_ERR_ROOT_COMMAND + sizeof(u32))
+ regs->root_command = get_unaligned_le32(raw + PCI_ERR_ROOT_COMMAND);
+ if (raw_len >= PCI_ERR_ROOT_STATUS + sizeof(u32))
+ regs->root_status = get_unaligned_le32(raw + PCI_ERR_ROOT_STATUS);
+ if (raw_len >= PCI_ERR_ROOT_ERR_SRC + sizeof(u32)) {
+ u32 src = get_unaligned_le32(raw + PCI_ERR_ROOT_ERR_SRC);
+
+ /* One register: ERR_COR in 15:0, ERR_FATAL/NONFATAL in 31:16 */
+ regs->cor_err_source = src;
+ regs->uncor_err_source = src >> 16;
+ }
+}
+EXPORT_SYMBOL_GPL(aer_cap_regs_unpack);
+
#define EE_PREFIX_STR " E-E Prefixes:"
/**
diff --git a/include/linux/aer.h b/include/linux/aer.h
index df0f5c382286..043ee76b6f53 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -24,6 +24,13 @@
#define PCIE_STD_MAX_TLP_PREFIXLOG 4
#define PCIE_STD_MAX_TLP_HEADERLOG (PCIE_STD_NUM_TLP_HEADERLOG + 10)
+/*
+ * Size of the AER Capability register block in hardware, long enough for the
+ * longest Flit mode TLP Log. struct aer_capability_regs below is larger and
+ * laid out differently; use aer_cap_regs_unpack() to convert.
+ */
+#define PCIE_AER_CAP_HW_SIZE 96
+
struct pci_dev;
struct pcie_tlp_log {
@@ -68,6 +75,8 @@ static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
void pci_print_aer(struct pci_dev *dev, int aer_severity,
struct aer_capability_regs *aer);
+void aer_cap_regs_unpack(struct aer_capability_regs *regs, const void *raw,
+ size_t raw_len);
int cper_severity_to_aer(int cper_severity);
void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
int severity, struct aer_capability_regs *aer_regs);
base-commit: 86adf9d0f7b074e0a1310c1bc4db66cac5c83eec
--
2.54.0
next reply other threads:[~2026-09-11 16:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-11 16:47 Dave Jiang [this message]
2026-09-11 16:57 ` [PATCH v3] PCI/AER: Map a raw AER Capability image field by field sashiko-bot
2026-09-11 23:13 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260911164705.841407-1-dave.jiang@intel.com \
--to=dave.jiang@intel.com \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=guohanjun@huawei.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jic23@kernel.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=rafael@kernel.org \
--cc=sashiko-bot@kernel.org \
--cc=terry.bowman@amd.com \
--cc=xueshuai@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox