* [PATCH v3] PCI: qcom: Add register dump support for PCIe Link Down events
@ 2026-11-07 2:27 Krishna Chaitanya Chundru
2026-11-09 20:07 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-11-07 2:27 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: linux-pci, linux-kernel, linux-arm-msm, Krishna Chaitanya Chundru
When the PCIe link goes down unexpectedly, being able to inspect the
state of key controller registers at the time of failure is valuable
for root-causing the issue.
On a Link Down event, dump the key PCIe controller registers to dmesg
via dev_err() so the state at the time of failure is immediately
visible. As printk() truncates messages longer than LOG_LINE_MAX, the
dump is emitted one line at a time rather than as a single call.
To keep the dump readable given the number of registers involved,
entries are packed as "offset: value" pairs, QCOM_PCIE_DUMP_REGS_PER_LINE
(4) per line, with a section header ("DBI registers:", "PARF
registers:", "MHI registers:") identifying each register group.
Also expose the same dump on demand via a new "regdump" debugfs file
under the existing per-controller debugfs directory, so the register
state can be inspected without needing an actual Link Down event.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v3:
- Remove devcoredump as per offline discussions with Mani.
- Link to v2: https://patch.msgid.link/20260905-regdump-v2-1-f7d34f9d4f46@oss.qualcomm.com
Changes in v2:
- Fixed the sashiko comments, add missed qcom_pcie_ext_parf_dump_regs
usage, print the dump one line at a time instead of as a single dev_err() call,
create debugfs even if mhi is not present.
- Link to v1: https://patch.msgid.link/20260811-regdump-v1-1-f22db3fb37c2@oss.qualcomm.com
---
drivers/pci/controller/dwc/pcie-designware.h | 6 +
drivers/pci/controller/dwc/pcie-qcom.c | 323 ++++++++++++++++++++++++++-
include/uapi/linux/pci_regs.h | 2 +
3 files changed, 327 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 6d788aa00011..1d50aecef380 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -288,6 +288,12 @@
#define RAS_DES_EVENT_COUNTER_DATA 0xc
+/* SD-EQ (Gen5 32 GT/s per-lane pre/post cursor equalization) registers */
+#define PCIE_RAS_DES_SD_EQ_CONTROL1 0xd0
+#define PCIE_RAS_DES_SD_EQ_CONTROL1_LANE_SEL GENMASK(3, 0)
+#define PCIE_RAS_DES_SD_EQ_STATUS2 0xe4
+#define PCIE_RAS_DES_SD_EQ_STATUS3 0xe8
+
/* PTM register definitions */
#define PTM_RES_REQ_CTRL 0x8
#define PTM_RES_CCONTEXT_VALID BIT(0)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d903f10211c2..147aab6f4155 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -34,6 +34,7 @@
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/slab.h>
+#include <linux/string.h>
#include <linux/types.h>
#include <linux/units.h>
@@ -45,6 +46,8 @@
/* PARF registers */
#define PARF_SYS_CTRL 0x00
#define PARF_PM_CTRL 0x20
+#define PARF_PM_STTS 0x24
+#define PARF_PM_STTS_1 0x28
#define PARF_PCS_DEEMPH 0x34
#define PARF_PCS_SWING 0x38
#define PARF_PHY_CTRL 0x40
@@ -63,11 +66,17 @@
#define PARF_STATUS 0x230
#define PARF_SID_OFFSET 0x234
#define PARF_BDF_TRANSLATE_CFG 0x24c
+#define PARF_CLKREQ_OVERRIDE 0x2b0
#define PARF_DBI_BASE_ADDR_V2 0x350
#define PARF_DBI_BASE_ADDR_V2_HI 0x354
#define PARF_SLV_ADDR_SPACE_SIZE_V2 0x358
#define PARF_SLV_ADDR_SPACE_SIZE_V2_HI 0x35c
#define PARF_NO_SNOOP_OVERRIDE 0x3d4
+#define PARF_CORE_ERRORS 0x3c0
+#define PARF_L1SS_SLEEP_MODE_HANDLER_STATUS 0x4d0
+#define PARF_L1SS_SLEEP_MODE_HANDLER_CFG 0x4d4
+#define PARF_INT_ALL_2_STATUS 0x500
+#define PARF_LINK_DOWN_AXI_ECAM_BLOCK_STATUS 0x630
#define PARF_ATU_BASE_ADDR 0x634
#define PARF_ATU_BASE_ADDR_HI 0x638
#define PARF_DEVICE_TYPE 0x1000
@@ -101,8 +110,12 @@
/* MHI registers */
#define PARF_DEBUG_CNT_PM_LINKST_IN_L2 0xc04
+#define PARF_DEBUG_CNT_PM_LINKST_IN_L1SUB 0xc08
#define PARF_DEBUG_CNT_PM_LINKST_IN_L1 0xc0c
#define PARF_DEBUG_CNT_PM_LINKST_IN_L0S 0xc10
+#define PARF_DEBUG_CNT_AUX_CLK_APP_REQ_EXIT_L1 0xc14
+#define PARF_DEBUG_CNT_AUX_CLK_APP_READY_ENTER_L23 0xc18
+#define PARF_DEBUG_CNT_AUX_CLK_APP_READY_ENTER_L1 0xc1c
#define PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L1 0xc84
#define PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L2 0xc88
@@ -207,11 +220,48 @@
#define PERST_DELAY_US 1000
#define FLUSH_TIMEOUT_US 100
+/* Dump buffer size for the link-down register dump */
+#define QCOM_PCIE_DUMP_BUF_SIZE SZ_8K
+
#define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0))
#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_get_link_speed(speed)))
+#define QCOM_PCIE_DUMP_REGS_PER_LINE 4
+
+static const u32 qcom_pcie_parf_dump_regs[] = {
+ PARF_LTSSM,
+ PARF_INT_ALL_STATUS,
+ PARF_SYS_CTRL,
+};
+
+static const u32 qcom_pcie_ext_parf_dump_regs[] = {
+ PARF_PM_STTS,
+ PARF_PM_STTS_1,
+ PARF_INT_ALL_STATUS,
+ PARF_INT_ALL_2_STATUS,
+ PARF_CLKREQ_OVERRIDE,
+ PARF_L1SS_SLEEP_MODE_HANDLER_STATUS,
+ PARF_L1SS_SLEEP_MODE_HANDLER_CFG,
+ PARF_CORE_ERRORS,
+ PARF_LINK_DOWN_AXI_ECAM_BLOCK_STATUS,
+ PARF_STATUS,
+ PARF_SYS_CTRL,
+};
+
+static const u32 qcom_pcie_mhi_dump_regs[] = {
+ PARF_DEBUG_CNT_PM_LINKST_IN_L2,
+ PARF_DEBUG_CNT_PM_LINKST_IN_L1SUB,
+ PARF_DEBUG_CNT_PM_LINKST_IN_L1,
+ PARF_DEBUG_CNT_PM_LINKST_IN_L0S,
+ PARF_DEBUG_CNT_AUX_CLK_APP_REQ_EXIT_L1,
+ PARF_DEBUG_CNT_AUX_CLK_APP_READY_ENTER_L23,
+ PARF_DEBUG_CNT_AUX_CLK_APP_READY_ENTER_L1,
+ PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L1,
+ PARF_DEBUG_CNT_AUX_CLK_IN_L1SUB_L2,
+};
+
struct qcom_pcie_resources_1_0_0 {
struct clk_bulk_data *clks;
int num_clks;
@@ -294,12 +344,15 @@ struct qcom_pcie_ops {
* @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
* snooping
* @firmware_managed: Set if the Root Complex is firmware managed
+ * @has_ext_parf_regs: Set if the PARF block implements the extended
+ * register set used by qcom_pcie_dump_regs().
*/
struct qcom_pcie_cfg {
const struct qcom_pcie_ops *ops;
bool override_no_snoop;
bool firmware_managed;
bool no_l0s;
+ bool has_ext_parf_regs;
};
struct qcom_pcie_perst {
@@ -1692,12 +1745,14 @@ static const struct qcom_pcie_cfg cfg_1_0_0 = {
static const struct qcom_pcie_cfg cfg_1_9_0 = {
.ops = &ops_1_9_0,
+ .has_ext_parf_regs = true,
};
static const struct qcom_pcie_cfg cfg_1_34_0 = {
.ops = &ops_1_9_0,
.override_no_snoop = true,
.no_l0s = true,
+ .has_ext_parf_regs = true,
};
static const struct qcom_pcie_cfg cfg_2_1_0 = {
@@ -1719,6 +1774,7 @@ static const struct qcom_pcie_cfg cfg_2_4_0 = {
static const struct qcom_pcie_cfg cfg_2_7_0 = {
.ops = &ops_2_7_0,
+ .has_ext_parf_regs = true,
};
static const struct qcom_pcie_cfg cfg_2_9_0 = {
@@ -1728,6 +1784,7 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
static const struct qcom_pcie_cfg cfg_sc8280xp = {
.ops = &ops_1_21_0,
.no_l0s = true,
+ .has_ext_parf_regs = true,
};
static const struct qcom_pcie_cfg cfg_fw_managed = {
@@ -1855,6 +1912,260 @@ static int qcom_pcie_set_max_opp(struct device *dev)
return ret;
}
+/*
+ * qcom_pcie_dump_advance_col - advance @col and append a separator to @buf.
+ *
+ * @col tracks the current column within the active section and must be
+ * reset to 0 by the caller at the start of each new section; a newline is
+ * emitted every QCOM_PCIE_DUMP_REGS_PER_LINE entries to keep the dump
+ * compact in dmesg.
+ */
+static size_t qcom_pcie_dump_advance_col(char *buf, size_t buf_size, int *col)
+{
+ *col += 1;
+
+ return scnprintf(buf, buf_size,
+ (*col % QCOM_PCIE_DUMP_REGS_PER_LINE) ? " " : "\n");
+}
+
+static size_t qcom_pcie_dump_reg_val(char *buf, size_t buf_size, int *col,
+ u32 offset, u32 val)
+{
+ size_t len;
+
+ len = scnprintf(buf, buf_size, "0x%03x: 0x%08x", offset, val);
+ len += qcom_pcie_dump_advance_col(buf + len, buf_size - len, col);
+
+ return len;
+}
+
+/* Same as qcom_pcie_dump_reg_val(), but tags the entry with a lane number
+ * since the RAS-DES SD-EQ status registers are read once per lane at a
+ * fixed offset, so the offset alone can't tell entries apart.
+ */
+static size_t qcom_pcie_dump_lane_reg_val(char *buf, size_t buf_size, int *col,
+ int lane, u32 offset, u32 val)
+{
+ size_t len;
+
+ len = scnprintf(buf, buf_size, "L%d/0x%03x: 0x%08x", lane, offset, val);
+ len += qcom_pcie_dump_advance_col(buf + len, buf_size - len, col);
+
+ return len;
+}
+
+static size_t qcom_pcie_dump_reg_table(void __iomem *base, const u32 *regs,
+ int nregs, char *buf, size_t buf_size)
+{
+ size_t len = 0;
+ int col = 0;
+ int i;
+
+ for (i = 0; i < nregs; i++)
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ regs[i], readl_relaxed(base + regs[i]));
+
+ if (col % QCOM_PCIE_DUMP_REGS_PER_LINE)
+ len += scnprintf(buf + len, buf_size - len, "\n");
+
+ return len;
+}
+
+/*
+ * qcom_pcie_fill_dump_buf - fill @buf with key PCIe register values captured
+ * at Link Down time.
+ *
+ * Returns the number of bytes written into @buf.
+ */
+static size_t qcom_pcie_fill_dump_buf(struct qcom_pcie *pcie, char *buf,
+ size_t buf_size)
+{
+ struct dw_pcie *pci = pcie->pci;
+ u16 exp_cap, aer_cap, l1ss_cap, secpci_cap, pl16gt_cap, pl32gt_cap;
+ size_t len = 0;
+ int col = 0;
+ u32 val;
+
+ len += scnprintf(buf + len, buf_size - len, "DBI registers:\n");
+
+ exp_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ if (exp_cap) {
+ val = dw_pcie_readl_dbi(pci, exp_cap + PCI_EXP_LNKCAP);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ exp_cap + PCI_EXP_LNKCAP, val);
+
+ val = dw_pcie_readl_dbi(pci, exp_cap + PCI_EXP_LNKCTL);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ exp_cap + PCI_EXP_LNKCTL, val);
+
+ val = dw_pcie_readl_dbi(pci, exp_cap + PCI_EXP_LNKCTL2);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ exp_cap + PCI_EXP_LNKCTL2, val);
+ }
+
+ aer_cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_ERR);
+ if (aer_cap) {
+ val = dw_pcie_readl_dbi(pci, aer_cap + PCI_ERR_UNCOR_STATUS);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ aer_cap + PCI_ERR_UNCOR_STATUS, val);
+
+ val = dw_pcie_readl_dbi(pci, aer_cap + PCI_ERR_COR_STATUS);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ aer_cap + PCI_ERR_COR_STATUS, val);
+ }
+
+ l1ss_cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
+ if (l1ss_cap) {
+ val = dw_pcie_readl_dbi(pci, l1ss_cap + PCI_L1SS_CTL1);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ l1ss_cap + PCI_L1SS_CTL1, val);
+ }
+
+ secpci_cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_SECPCI);
+ if (secpci_cap) {
+ val = dw_pcie_readl_dbi(pci, secpci_cap + PCI_SECPCI_LE_CTRL);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ secpci_cap + PCI_SECPCI_LE_CTRL, val);
+
+ val = dw_pcie_readl_dbi(pci, secpci_cap + PCI_SECPCI_LE_CTRL + 4);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ secpci_cap + PCI_SECPCI_LE_CTRL + 4, val);
+ }
+
+ pl16gt_cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_PL_16GT);
+ if (pl16gt_cap) {
+ val = dw_pcie_readl_dbi(pci, pl16gt_cap + PCI_PL_16GT_LE_CTRL);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ pl16gt_cap + PCI_PL_16GT_LE_CTRL, val);
+
+ val = dw_pcie_readl_dbi(pci, pl16gt_cap + PCI_PL_16GT_STATUS);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ pl16gt_cap + PCI_PL_16GT_STATUS, val);
+ }
+
+ pl32gt_cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_PL_32GT);
+ if (pl32gt_cap) {
+ val = dw_pcie_readl_dbi(pci, pl32gt_cap + PCI_PL_32GT_LE_CTRL);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ pl32gt_cap + PCI_PL_32GT_LE_CTRL, val);
+
+ val = dw_pcie_readl_dbi(pci, pl32gt_cap + PCI_PL_32GT_STATUS);
+ len += qcom_pcie_dump_reg_val(buf + len, buf_size - len, &col,
+ pl32gt_cap + PCI_PL_32GT_STATUS, val);
+ }
+
+ /*
+ * Gen5 (32 GT/s) local/remote pre/post cursor status, read through the
+ * SD-EQ registers inside the DWC RAS-DES vendor-specific extended
+ * capability. For each lane, write the lane selector into
+ * PCIE_RAS_DES_SD_EQ_CONTROL1 then read back
+ * PCIE_RAS_DES_SD_EQ_STATUS2/3, which carry the local/remote pre/post
+ * cursor values.
+ */
+ if (pcie_get_link_speed(pci->max_link_speed) == PCIE_SPEED_32_0GT) {
+ u16 rasdes_cap = dw_pcie_find_rasdes_capability(pci);
+
+ if (rasdes_cap) {
+ int num_lanes = dw_pcie_link_get_max_link_width(pci);
+ int lane;
+
+ for (lane = 0; lane < num_lanes; lane++) {
+ val = dw_pcie_readl_dbi(pci,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_CONTROL1);
+ val &= ~PCIE_RAS_DES_SD_EQ_CONTROL1_LANE_SEL;
+ val |= lane & PCIE_RAS_DES_SD_EQ_CONTROL1_LANE_SEL;
+ dw_pcie_writel_dbi(pci,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_CONTROL1,
+ val);
+
+ val = dw_pcie_readl_dbi(pci,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS2);
+ len += qcom_pcie_dump_lane_reg_val(buf + len, buf_size - len, &col,
+ lane,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS2,
+ val);
+
+ val = dw_pcie_readl_dbi(pci,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS3);
+ len += qcom_pcie_dump_lane_reg_val(buf + len, buf_size - len, &col,
+ lane,
+ rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS3,
+ val);
+ }
+ }
+ }
+
+ if (col % QCOM_PCIE_DUMP_REGS_PER_LINE)
+ len += scnprintf(buf + len, buf_size - len, "\n");
+
+ len += scnprintf(buf + len, buf_size - len, "\nPARF registers:\n");
+ if (pcie->cfg->has_ext_parf_regs)
+ len += qcom_pcie_dump_reg_table(pcie->parf, qcom_pcie_ext_parf_dump_regs,
+ ARRAY_SIZE(qcom_pcie_ext_parf_dump_regs),
+ buf + len, buf_size - len);
+ else
+ len += qcom_pcie_dump_reg_table(pcie->parf, qcom_pcie_parf_dump_regs,
+ ARRAY_SIZE(qcom_pcie_parf_dump_regs),
+ buf + len, buf_size - len);
+
+ if (!pcie->mhi)
+ return len;
+
+ len += scnprintf(buf + len, buf_size - len, "\nMHI registers:\n");
+ len += qcom_pcie_dump_reg_table(pcie->mhi, qcom_pcie_mhi_dump_regs,
+ ARRAY_SIZE(qcom_pcie_mhi_dump_regs),
+ buf + len, buf_size - len);
+
+ return len;
+}
+
+/*
+ * qcom_pcie_dump_regs - capture a PCIe register dump on Link Down.
+ *
+ * Print the dump directly to dmesg via dev_err() so it is immediately visible
+ * for root-causing the failure.
+ */
+static void qcom_pcie_dump_regs(struct qcom_pcie *pcie)
+{
+ struct device *dev = pcie->pci->dev;
+ char *buf, *line, *pos;
+
+ buf = kmalloc(QCOM_PCIE_DUMP_BUF_SIZE, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ qcom_pcie_fill_dump_buf(pcie, buf, QCOM_PCIE_DUMP_BUF_SIZE);
+
+ /*
+ * printk() truncates messages longer than LOG_LINE_MAX, so print the
+ * dump one line at a time instead of as a single dev_err() call.
+ */
+ dev_err(dev, "PCIe Link Down register dump:\n");
+ pos = buf;
+ while ((line = strsep(&pos, "\n")))
+ dev_err(dev, "%s\n", line);
+
+ kfree(buf);
+}
+
+static int qcom_pcie_regdump_show(struct seq_file *s, void *data)
+{
+ struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private);
+ char *buf;
+ size_t len;
+
+ buf = kmalloc(QCOM_PCIE_DUMP_BUF_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ len = qcom_pcie_fill_dump_buf(pcie, buf, QCOM_PCIE_DUMP_BUF_SIZE);
+ seq_write(s, buf, len);
+
+ kfree(buf);
+
+ return 0;
+}
+
/*
* Qcom PCIe controllers only support one Root Port per controller instance. So
* this function ignores the 'pci_dev' associated with the Root Port and just
@@ -1962,8 +2273,11 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
return;
pcie->debugfs = debugfs_create_dir(name, NULL);
- debugfs_create_devm_seqfile(dev, "link_transition_count", pcie->debugfs,
- qcom_pcie_link_transition_count);
+ if (pcie->mhi)
+ debugfs_create_devm_seqfile(dev, "link_transition_count", pcie->debugfs,
+ qcom_pcie_link_transition_count);
+ debugfs_create_devm_seqfile(dev, "regdump", pcie->debugfs,
+ qcom_pcie_regdump_show);
}
static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
@@ -1983,6 +2297,8 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
qcom_pcie_enable_outbound_blocker(pcie);
mutex_unlock(&pcie->reset_lock);
+ qcom_pcie_dump_regs(pcie);
+
for_each_pci_bridge(port, pp->bridge->bus) {
if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
pci_host_handle_link_down(port);
@@ -2430,8 +2746,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
qcom_pcie_icc_opp_update(pcie);
- if (pcie->mhi)
- qcom_pcie_init_debugfs(pcie);
+ qcom_pcie_init_debugfs(pcie);
return 0;
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index facaa324bd86..0e82d89e747a 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1176,12 +1176,14 @@
#define PCI_SECPCI_LE_CTRL 0x0c /* Lane Equalization Control Register */
/* Physical Layer 16.0 GT/s */
+#define PCI_PL_16GT_STATUS 0x0c /* 16.0 GT/s Status Register */
#define PCI_PL_16GT_LE_CTRL 0x20 /* Lane Equalization Control Register */
#define PCI_PL_16GT_LE_CTRL_DSP_TX_PRESET_MASK 0x0000000F
#define PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_MASK 0x000000F0
#define PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_SHIFT 4
/* Physical Layer 32.0 GT/s */
+#define PCI_PL_32GT_STATUS 0x0c /* 32.0 GT/s Status Register */
#define PCI_PL_32GT_LE_CTRL 0x20 /* Lane Equalization Control Register */
/* Physical Layer 64.0 GT/s */
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20260811-regdump-11719f75de0c
Best regards,
--
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] PCI: qcom: Add register dump support for PCIe Link Down events
2026-11-07 2:27 [PATCH v3] PCI: qcom: Add register dump support for PCIe Link Down events Krishna Chaitanya Chundru
@ 2026-11-09 20:07 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-09 20:07 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] PCI: qcom: unlocked register dump in
qcom_pcie_fill_dump_buf()
- [Medium] PCI: qcom: out-of-bounds PARF reads in
qcom_pcie_fill_dump_buf()
- [Low] PCI: qcom: PARF_INT_ALL_STATUS cleared before
qcom_pcie_dump_regs()
--
commit b30e1ed6bb64a53a1b0da2d0c7ce94bc1f80823d
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
PCI: qcom: Add register dump support for PCIe Link Down events
This commit dumps key DBI, PARF, and MHI controller registers to dmesg on a
PCIe Link Down interrupt and exposes the same register dump via a per-controller
"regdump" debugfs file.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index d903f10211c23..147aab6f4155b 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -207,11 +220,48 @@
> #define PERST_DELAY_US 1000
> #define FLUSH_TIMEOUT_US 100
>
> +/* Dump buffer size for the link-down register dump */
> +#define QCOM_PCIE_DUMP_BUF_SIZE SZ_8K
> +
> #define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0))
>
> #define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
> Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_get_link_speed(speed)))
>
> +#define QCOM_PCIE_DUMP_REGS_PER_LINE 4
> +
> +static const u32 qcom_pcie_parf_dump_regs[] = {
> + PARF_LTSSM,
> + PARF_INT_ALL_STATUS,
> + PARF_SYS_CTRL,
> +};
> +
> +static const u32 qcom_pcie_ext_parf_dump_regs[] = {
> + PARF_PM_STTS,
> + PARF_PM_STTS_1,
> + PARF_INT_ALL_STATUS,
> + PARF_INT_ALL_2_STATUS,
> + PARF_CLKREQ_OVERRIDE,
> + PARF_L1SS_SLEEP_MODE_HANDLER_STATUS,
> + PARF_L1SS_SLEEP_MODE_HANDLER_CFG,
> + PARF_CORE_ERRORS,
> + PARF_LINK_DOWN_AXI_ECAM_BLOCK_STATUS,
> + PARF_STATUS,
> + PARF_SYS_CTRL,
> +};
[ ... ]
> @@ -1855,6 +1912,260 @@ static int qcom_pcie_set_max_opp(struct device *dev)
> return ret;
> }
[ ... ]
> + len += scnprintf(buf + len, buf_size - len, "\nPARF registers:\n");
> + if (pcie->cfg->has_ext_parf_regs)
> + len += qcom_pcie_dump_reg_table(pcie->parf, qcom_pcie_ext_parf_dump_regs,
> + ARRAY_SIZE(qcom_pcie_ext_parf_dump_regs),
> + buf + len, buf_size - len);
> + else
> + len += qcom_pcie_dump_reg_table(pcie->parf, qcom_pcie_parf_dump_regs,
> + ARRAY_SIZE(qcom_pcie_parf_dump_regs),
> + buf + len, buf_size - len);
[Severity: Medium]
Can dumping qcom_pcie_parf_dump_regs[] read past the end of pcie->parf on
cfg_2_1_0 platforms?
qcom_pcie_probe() now registers the "regdump" debugfs file unconditionally on
all non-firmware-managed controllers. On IP v2.1.0 platforms (apq8064 and
ipq8064), has_ext_parf_regs is false and the "parf" MMIO region is only 0x100
bytes:
arch/arm/boot/dts/qcom/qcom-apq8064.dtsi:
reg = <0x1b500000 0x1000>,
<0x1b502000 0x80>,
<0x1b600000 0x100>,
<0x0ff00000 0x100000>;
reg-names = "dbi", "elbi", "parf", "config";
Reading "regdump" on these platforms reads PARF_LTSSM (0x1b0) and
PARF_INT_ALL_STATUS (0x224) beyond the 0x100-byte mapping. Additionally,
v1.0.0 and v2.1.0 controllers report LTSSM state in ELBI_SYS_STTS rather than
PARF_LTSSM, whereas newer controllers with has_ext_parf_regs set only dump
qcom_pcie_ext_parf_dump_regs[] and omit PARF_LTSSM altogether.
[ ... ]
> + if (pcie_get_link_speed(pci->max_link_speed) == PCIE_SPEED_32_0GT) {
> + u16 rasdes_cap = dw_pcie_find_rasdes_capability(pci);
> +
> + if (rasdes_cap) {
> + int num_lanes = dw_pcie_link_get_max_link_width(pci);
> + int lane;
> +
> + for (lane = 0; lane < num_lanes; lane++) {
> + val = dw_pcie_readl_dbi(pci,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_CONTROL1);
> + val &= ~PCIE_RAS_DES_SD_EQ_CONTROL1_LANE_SEL;
> + val |= lane & PCIE_RAS_DES_SD_EQ_CONTROL1_LANE_SEL;
> + dw_pcie_writel_dbi(pci,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_CONTROL1,
> + val);
> +
> + val = dw_pcie_readl_dbi(pci,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS2);
> + len += qcom_pcie_dump_lane_reg_val(buf + len, buf_size - len, &col,
> + lane,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS2,
> + val);
> +
> + val = dw_pcie_readl_dbi(pci,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS3);
> + len += qcom_pcie_dump_lane_reg_val(buf + len, buf_size - len, &col,
> + lane,
> + rasdes_cap + PCIE_RAS_DES_SD_EQ_STATUS3,
> + val);
> + }
> + }
> + }
[ ... ]
> +static int qcom_pcie_regdump_show(struct seq_file *s, void *data)
> +{
> + struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private);
> + char *buf;
> + size_t len;
> +
> + buf = kmalloc(QCOM_PCIE_DUMP_BUF_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + len = qcom_pcie_fill_dump_buf(pcie, buf, QCOM_PCIE_DUMP_BUF_SIZE);
> + seq_write(s, buf, len);
> +
> + kfree(buf);
> +
> + return 0;
> +}
[Severity: Medium]
Can qcom_pcie_fill_dump_buf() access unclocked MMIO registers if it races with
qcom_pcie_reset_root_port()?
Neither qcom_pcie_regdump_show() nor qcom_pcie_dump_regs() holds
pcie->reset_lock while calling qcom_pcie_fill_dump_buf():
CPU0 CPU1
qcom_pcie_reset_root_port() qcom_pcie_regdump_show()
guard(mutex)(&pcie->reset_lock)
qcom_pcie_host_deinit()
pcie->cfg->ops->deinit()
clk_bulk_disable_unprepare()
regulator_bulk_disable()
qcom_pcie_fill_dump_buf()
dw_pcie_readl_dbi()
<-- unclocked MMIO / SError
While qcom_pcie_reset_root_port() reinitializes the Root Port under
pcie->reset_lock, controller clocks and regulators are disabled and resets are
asserted, so concurrent DBI, PARF, or MHI reads and writes from
qcom_pcie_fill_dump_buf() can trigger an immediate bus abort (SError) and
kernel panic. In addition, on 32 GT/s controllers, concurrent callers of
qcom_pcie_fill_dump_buf() can interleave writes to the banked lane selector
PCIE_RAS_DES_SD_EQ_CONTROL1 and read back PCIE_RAS_DES_SD_EQ_STATUS2/3 for the
wrong lane.
Acquiring pcie->reset_lock around the register accesses in both dump paths
would serialize them against Root Port resets and each other.
[ ... ]
> @@ -1983,6 +2297,8 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> qcom_pcie_enable_outbound_blocker(pcie);
> mutex_unlock(&pcie->reset_lock);
>
> + qcom_pcie_dump_regs(pcie);
> +
> for_each_pci_bridge(port, pp->bridge->bus) {
> if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
> pci_host_handle_link_down(port);
[Severity: Low]
Does qcom_pcie_dump_regs() read PARF_INT_ALL_STATUS after it has already been
cleared?
At the start of qcom_pcie_global_irq_thread(), the handler reads
PARF_INT_ALL_STATUS into status and immediately writes status to
PARF_INT_ALL_CLEAR:
drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_global_irq_thread() {
unsigned long status = readl_relaxed(pcie->parf + PARF_INT_ALL_STATUS);
writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
...
}
When qcom_pcie_dump_regs() later dumps PARF_INT_ALL_STATUS from
qcom_pcie_parf_dump_regs[] or qcom_pcie_ext_parf_dump_regs[], the interrupt
status bits that triggered the Link Down event have already been cleared in
hardware.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-regdump-v3-1-c85648af91ee@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-09 20:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 2:27 [PATCH v3] PCI: qcom: Add register dump support for PCIe Link Down events Krishna Chaitanya Chundru
2026-11-09 20:07 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox