[PATCH v2 0/2] PCI: Add support for logging Flit Mode TLPs (PCIe6)

[PATCH v2 0/2] PCI: Add support for logging Flit Mode TLPs (PCIe6)

[Ilpo Järvinen]

This series adds support for Flit Mode (PCIe6).

v2:
- Rebased

Ilpo Järvinen (2):
  PCI: Track Flit Mode Status & print it with link status
  PCI: Handle TLP Log in Flit mode

 drivers/pci/hotplug/pciehp_hpc.c |  5 +--
 drivers/pci/pci.c                | 12 ++++---
 drivers/pci/pci.h                |  6 ++--
 drivers/pci/pcie/aer.c           |  1 +
 drivers/pci/pcie/dpc.c           | 18 ++++++++--
 drivers/pci/pcie/tlp.c           | 56 ++++++++++++++++++++++----------
 drivers/pci/probe.c              |  5 +--
 include/linux/aer.h              | 12 +++++--
 include/linux/pci.h              |  1 +
 include/ras/ras_event.h          | 12 +++----
 include/uapi/linux/pci_regs.h    |  6 +++-
 11 files changed, 94 insertions(+), 40 deletions(-)


base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
-- 
2.39.5

[PATCH v2 1/2] PCI: Track Flit Mode Status & print it with link status

[Ilpo Järvinen]

PCIe r6.0 added Flit mode that mainly alters HW behavior but some OS
visible changes are also because of it. The OS visible changes include
differences in the layout of some capabilities and interpretation of
the TLP headers (in diagnostics situations).

To be able to determine which mode the PCIe link is using, store the
Flit Mode Status (PCIe r6.1 sec 7.5.3.20) information in addition to
the link speed into struct pci_bus in pcie_update_link_speed().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/hotplug/pciehp_hpc.c |  5 +++--
 drivers/pci/pci.c                | 12 ++++++++----
 drivers/pci/pci.h                |  3 ++-
 drivers/pci/probe.c              |  5 +++--
 include/linux/pci.h              |  1 +
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index bb5a8d9f03ad..10130ac9f979 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -292,7 +292,7 @@ int pciehp_check_link_status(struct controller *ctrl)
 {
 	struct pci_dev *pdev = ctrl_dev(ctrl);
 	bool found;
-	u16 lnk_status;
+	u16 lnk_status, linksta2;
 
 	if (!pcie_wait_for_link(pdev, true)) {
 		ctrl_info(ctrl, "Slot(%s): No link\n", slot_name(ctrl));
@@ -319,7 +319,8 @@ int pciehp_check_link_status(struct controller *ctrl)
 		return -1;
 	}
 
-	__pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status);
+	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &linksta2);
+	__pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status, linksta2);
 
 	if (!found) {
 		ctrl_info(ctrl, "Slot(%s): No device found\n",
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..313c66863752 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6190,21 +6190,25 @@ void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
 	enum pci_bus_speed speed, speed_cap;
 	struct pci_dev *limiting_dev = NULL;
 	u32 bw_avail, bw_cap;
+	char *flit_mode = "";
 
 	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
 	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
 
+	if (dev->bus && dev->bus->flit_mode)
+		flit_mode = ", in Flit mode";
+
 	if (bw_avail >= bw_cap && verbose)
-		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
+		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)%s\n",
 			 bw_cap / 1000, bw_cap % 1000,
-			 pci_speed_string(speed_cap), width_cap);
+			 pci_speed_string(speed_cap), width_cap, flit_mode);
 	else if (bw_avail < bw_cap)
-		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
+		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)%s\n",
 			 bw_avail / 1000, bw_avail % 1000,
 			 pci_speed_string(speed), width,
 			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
 			 bw_cap / 1000, bw_cap % 1000,
-			 pci_speed_string(speed_cap), width_cap);
+			 pci_speed_string(speed_cap), width_cap, flit_mode);
 }
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 01e51db8d285..9c6a4a980678 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -406,9 +406,10 @@ const char *pci_speed_string(enum pci_bus_speed speed);
 void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
 void pcie_report_downtraining(struct pci_dev *dev);
 
-static inline void __pcie_update_link_speed(struct pci_bus *bus, u16 linksta)
+static inline void __pcie_update_link_speed(struct pci_bus *bus, u16 linksta, u16 linksta2)
 {
 	bus->cur_bus_speed = pcie_link_speed[linksta & PCI_EXP_LNKSTA_CLS];
+	bus->flit_mode = linksta2 & PCI_EXP_LNKSTA2_FLIT;
 }
 void pcie_update_link_speed(struct pci_bus *bus);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b6536ed599c3..e6f11498a345 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -788,10 +788,11 @@ EXPORT_SYMBOL_GPL(pci_speed_string);
 void pcie_update_link_speed(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
-	u16 linksta;
+	u16 linksta, linksta2;
 
 	pcie_capability_read_word(bridge, PCI_EXP_LNKSTA, &linksta);
-	__pcie_update_link_speed(bus, linksta);
+	pcie_capability_read_word(bridge, PCI_EXP_LNKSTA2, &linksta2);
+	__pcie_update_link_speed(bus, linksta, linksta2);
 }
 EXPORT_SYMBOL_GPL(pcie_update_link_speed);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..9862f65d899d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -681,6 +681,7 @@ struct pci_bus {
 	struct bin_attribute	*legacy_mem;	/* Legacy mem */
 	unsigned int		is_added:1;
 	unsigned int		unsafe_warn:1;	/* warned about RW1C config write */
+	bool			flit_mode;	/* Link in Flit mode */
 };
 
 #define to_pci_bus(n)	container_of(n, struct pci_bus, dev)
-- 
2.39.5

[PATCH v2 2/2] PCI: Handle TLP Log in Flit mode

[Ilpo Järvinen]

Flit mode introduced in PCIe r6.0 alters how the TLP Header Log is
presented through AER and DPC Capability registers. The TLP Prefix Log
Register is not present with Flit mode and the register becomes
extension for TLP Header Log (PCIe r6.1 secs 7.8.4.12 & 7.9.14.13).

Adapt pcie_read_tlp_log() and struct pcie_tlp_log to read and store
also the extended TLP Header Log when the link is in Flit mode. As
Prefix Log and Extended TLP Header are not present at the same time,
C union can be used.

Determining whether the error occurred while the Link was in Flit mode
is bit complicated. In case of AER, Advanced Error Capabilities and
Control Register directly tells whether the error was logged in Flit
mode or not (PCIe r6.1 sec 7.8.4.7). DPC Capability (PCIe r6.1 sec
7.9.14), unfortunately, does not contain the same information.

Unlike AER, the DPC capability does not provide way to discern whether
the error was logged in Flit mode (this is confirmed by PCI WG to be an
oversight in the spec). DPC will bring link down immediately following
an error, which make it impossible to acquire the Flit mode status
directly from the Link Status 2 register because Flit Mode Status is
only set in certain Link states (PCIe r6.1 sec 7.5.3.20). As a
workaround, use the flit_mode value stored into the struct pci_bus.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci.h             |  3 +-
 drivers/pci/pcie/aer.c        |  1 +
 drivers/pci/pcie/dpc.c        | 18 +++++++++--
 drivers/pci/pcie/tlp.c        | 56 ++++++++++++++++++++++++-----------
 include/linux/aer.h           | 12 ++++++--
 include/ras/ras_event.h       | 12 ++++----
 include/uapi/linux/pci_regs.h |  6 +++-
 7 files changed, 77 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9c6a4a980678..07cc9be6f80b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -554,7 +554,8 @@ 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 pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
-		      unsigned int tlp_len, struct pcie_tlp_log *log);
+		      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);
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 508474e17183..9c3e8299ad50 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1245,6 +1245,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
 			pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG,
 					  aer + PCI_ERR_PREFIX_LOG,
 					  aer_tlp_log_len(dev, aercc),
+					  aercc & PCI_ERR_CAP_TLP_LOG_FLIT,
 					  &info->tlp);
 		}
 	}
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 242cabd5eeeb..df42f15c9829 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -219,7 +219,9 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
 		goto clear_status;
 	pcie_read_tlp_log(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
 			  cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
-			  dpc_tlp_log_len(pdev), &tlp_log);
+			  dpc_tlp_log_len(pdev),
+			  pdev->subordinate->flit_mode,
+			  &tlp_log);
 	pcie_print_tlp_log(pdev, &tlp_log, dev_fmt(""));
 
 	if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG + 1)
@@ -398,11 +400,21 @@ void pci_dpc_init(struct pci_dev *pdev)
 
 	/* Quirks may set dpc_rp_log_size if device or firmware is buggy */
 	if (!pdev->dpc_rp_log_size) {
+		u16 flags;
+		int ret;
+
+		ret = pcie_capability_read_word(pdev, PCI_EXP_FLAGS, &flags);
+		if (ret)
+			return;
+
 		pdev->dpc_rp_log_size =
 				FIELD_GET(PCI_EXP_DPC_RP_PIO_LOG_SIZE, cap);
+		if (FIELD_GET(PCI_EXP_FLAGS_FLIT, flags))
+			pdev->dpc_rp_log_size += FIELD_GET(PCI_EXP_DPC_RP_PIO_LOG_SIZE4,
+							   cap) << 4;
+
 		if (pdev->dpc_rp_log_size < PCIE_STD_NUM_TLP_HEADERLOG ||
-		    pdev->dpc_rp_log_size > PCIE_STD_NUM_TLP_HEADERLOG + 1 +
-					    PCIE_STD_MAX_TLP_PREFIXLOG) {
+		    pdev->dpc_rp_log_size > PCIE_STD_MAX_TLP_HEADERLOG + 1) {
 			pci_err(pdev, "RP PIO log size %u is invalid\n",
 				pdev->dpc_rp_log_size);
 			pdev->dpc_rp_log_size = 0;
diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
index 0860b5da837f..890d5391d7f5 100644
--- a/drivers/pci/pcie/tlp.c
+++ b/drivers/pci/pcie/tlp.c
@@ -7,6 +7,7 @@
 
 #include <linux/aer.h>
 #include <linux/array_size.h>
+#include <linux/bitfield.h>
 #include <linux/pci.h>
 #include <linux/string.h>
 
@@ -21,6 +22,9 @@
  */
 unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc)
 {
+	if (aercc & PCI_ERR_CAP_TLP_LOG_FLIT)
+		return FIELD_GET(PCI_ERR_CAP_TLP_LOG_SIZE, aercc);
+
 	return PCIE_STD_NUM_TLP_HEADERLOG +
 	       ((aercc & PCI_ERR_CAP_PREFIX_LOG_PRESENT) ?
 		dev->eetlp_prefix_max : 0);
@@ -49,6 +53,7 @@ unsigned int dpc_tlp_log_len(struct pci_dev *dev)
  * @where: PCI Config offset of TLP Header Log
  * @where2: PCI Config offset of TLP Prefix Log
  * @tlp_len: TLP Log length (Header Log + TLP Prefix Log in DWORDs)
+ * @flit: TLP Logged in Flit mode
  * @log: TLP Log structure to fill
  *
  * Fill @log from TLP Header Log registers, e.g., AER or DPC.
@@ -56,28 +61,34 @@ unsigned int dpc_tlp_log_len(struct pci_dev *dev)
  * Return: 0 on success and filled TLP Log structure, <0 on error.
  */
 int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
-		      unsigned int tlp_len, struct pcie_tlp_log *log)
+		      unsigned int tlp_len, bool flit, struct pcie_tlp_log *log)
 {
 	unsigned int i;
 	int off, ret;
-	u32 *to;
+
+	if (tlp_len > ARRAY_SIZE(log->dw))
+		tlp_len = ARRAY_SIZE(log->dw);
 
 	memset(log, 0, sizeof(*log));
 
 	for (i = 0; i < tlp_len; i++) {
-		if (i < PCIE_STD_NUM_TLP_HEADERLOG) {
+		if (i < PCIE_STD_NUM_TLP_HEADERLOG)
 			off = where + i * 4;
-			to = &log->dw[i];
-		} else {
+		else
 			off = where2 + (i - PCIE_STD_NUM_TLP_HEADERLOG) * 4;
-			to = &log->prefix[i - PCIE_STD_NUM_TLP_HEADERLOG];
-		}
 
-		ret = pci_read_config_dword(dev, off, to);
+		ret = pci_read_config_dword(dev, off, &log->dw[i]);
 		if (ret)
 			return pcibios_err_to_errno(ret);
 	}
 
+	/*
+	 * Hard-code non-Flit mode to 4 DWORDs, for now. The exact length
+	 * can only be known if the TLP is parsed.
+	 */
+	log->header_len = flit ? tlp_len : 4;
+	log->flit = flit;
+
 	return 0;
 }
 
@@ -94,22 +105,31 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
 void pcie_print_tlp_log(const struct pci_dev *dev,
 			const struct pcie_tlp_log *log, const char *pfx)
 {
-	char buf[11 * (PCIE_STD_NUM_TLP_HEADERLOG + ARRAY_SIZE(log->prefix)) +
-		 sizeof(EE_PREFIX_STR)];
+	/* EE_PREFIX_STR fits the extended DW space needed for the Flit mode */
+	char buf[11 * PCIE_STD_MAX_TLP_HEADERLOG + 1];
 	unsigned int i;
 	int len;
 
 	len = scnprintf(buf, sizeof(buf), "%#010x %#010x %#010x %#010x",
 			log->dw[0], log->dw[1], log->dw[2], log->dw[3]);
 
-	if (log->prefix[0])
-		len += scnprintf(buf + len, sizeof(buf) - len, EE_PREFIX_STR);
-	for (i = 0; i < ARRAY_SIZE(log->prefix); i++) {
-		if (!log->prefix[i])
-			break;
-		len += scnprintf(buf + len, sizeof(buf) - len,
-				 " %#010x", log->prefix[i]);
+	if (log->flit) {
+		for (i = PCIE_STD_NUM_TLP_HEADERLOG; i < log->header_len; i++) {
+			len += scnprintf(buf + len, sizeof(buf) - len,
+					 " %#010x", log->dw[i]);
+		}
+	} else {
+		if (log->prefix[0])
+			len += scnprintf(buf + len, sizeof(buf) - len,
+					 EE_PREFIX_STR);
+		for (i = 0; i < ARRAY_SIZE(log->prefix); i++) {
+			if (!log->prefix[i])
+				break;
+			len += scnprintf(buf + len, sizeof(buf) - len,
+					 " %#010x", log->prefix[i]);
+		}
 	}
 
-	pci_err(dev, "%sTLP Header: %s\n", pfx, buf);
+	pci_err(dev, "%sTLP Header%s: %s\n", pfx,
+		log->flit ? " (Flit)" : "", buf);
 }
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 947b63091902..02940be66324 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -22,12 +22,20 @@
  */
 #define PCIE_STD_NUM_TLP_HEADERLOG     4
 #define PCIE_STD_MAX_TLP_PREFIXLOG     4
+#define PCIE_STD_MAX_TLP_HEADERLOG	(PCIE_STD_NUM_TLP_HEADERLOG + 10)
 
 struct pci_dev;
 
 struct pcie_tlp_log {
-	u32 dw[PCIE_STD_NUM_TLP_HEADERLOG];
-	u32 prefix[PCIE_STD_MAX_TLP_PREFIXLOG];
+	union {
+		u32 dw[PCIE_STD_MAX_TLP_HEADERLOG];
+		struct {
+			u32 _do_not_use[PCIE_STD_NUM_TLP_HEADERLOG];
+			u32 prefix[PCIE_STD_MAX_TLP_PREFIXLOG];
+		};
+	};
+	u8 header_len;		/* Length of the Logged TLP Header in DWORDs */
+	bool flit;		/* TLP was logged when in Flit mode */
 };
 
 struct aer_capability_regs {
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index e5f7ee0864e7..14c9f943d53f 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -309,7 +309,7 @@ TRACE_EVENT(aer_event,
 		__field(	u32,		status		)
 		__field(	u8,		severity	)
 		__field(	u8, 		tlp_header_valid)
-		__array(	u32, 		tlp_header, 4	)
+		__array(	u32, 		tlp_header, PCIE_STD_MAX_TLP_HEADERLOG)
 	),
 
 	TP_fast_assign(
@@ -318,10 +318,10 @@ TRACE_EVENT(aer_event,
 		__entry->severity	= severity;
 		__entry->tlp_header_valid = tlp_header_valid;
 		if (tlp_header_valid) {
-			__entry->tlp_header[0] = tlp->dw[0];
-			__entry->tlp_header[1] = tlp->dw[1];
-			__entry->tlp_header[2] = tlp->dw[2];
-			__entry->tlp_header[3] = tlp->dw[3];
+			int i;
+
+			for (i = 0; i < PCIE_STD_MAX_TLP_HEADERLOG; i++)
+				__entry->tlp_header[i] = tlp->dw[i];
 		}
 	),
 
@@ -334,7 +334,7 @@ TRACE_EVENT(aer_event,
 		__print_flags(__entry->status, "|", aer_correctable_errors) :
 		__print_flags(__entry->status, "|", aer_uncorrectable_errors),
 		__entry->tlp_header_valid ?
-			__print_array(__entry->tlp_header, 4, 4) :
+			__print_array(__entry->tlp_header, PCIE_STD_MAX_TLP_HEADERLOG, 4) :
 			"Not available")
 );
 
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 3445c4970e4d..2c801c0c968a 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -486,6 +486,7 @@
 #define   PCI_EXP_TYPE_RC_EC	   0xa	/* Root Complex Event Collector */
 #define  PCI_EXP_FLAGS_SLOT	0x0100	/* Slot implemented */
 #define  PCI_EXP_FLAGS_IRQ	0x3e00	/* Interrupt message number */
+#define  PCI_EXP_FLAGS_FLIT	0x8000	/* Flit Mode Supported */
 #define PCI_EXP_DEVCAP		0x04	/* Device capabilities */
 #define  PCI_EXP_DEVCAP_PAYLOAD	0x00000007 /* Max_Payload_Size */
 #define  PCI_EXP_DEVCAP_PHANTOM	0x00000018 /* Phantom functions */
@@ -795,6 +796,8 @@
 #define  PCI_ERR_CAP_ECRC_CHKC		0x00000080 /* ECRC Check Capable */
 #define  PCI_ERR_CAP_ECRC_CHKE		0x00000100 /* ECRC Check Enable */
 #define  PCI_ERR_CAP_PREFIX_LOG_PRESENT	0x00000800 /* TLP Prefix Log Present */
+#define  PCI_ERR_CAP_TLP_LOG_FLIT	0x00040000 /* TLP was logged in Flit Mode */
+#define  PCI_ERR_CAP_TLP_LOG_SIZE	0x00f80000 /* Logged TLP Size (only in Flit mode) */
 #define PCI_ERR_HEADER_LOG	0x1c	/* Header Log Register (16 bytes) */
 #define PCI_ERR_ROOT_COMMAND	0x2c	/* Root Error Command */
 #define  PCI_ERR_ROOT_CMD_COR_EN	0x00000001 /* Correctable Err Reporting Enable */
@@ -1061,8 +1064,9 @@
 #define  PCI_EXP_DPC_CAP_RP_EXT		0x0020	/* Root Port Extensions */
 #define  PCI_EXP_DPC_CAP_POISONED_TLP	0x0040	/* Poisoned TLP Egress Blocking Supported */
 #define  PCI_EXP_DPC_CAP_SW_TRIGGER	0x0080	/* Software Triggering Supported */
-#define  PCI_EXP_DPC_RP_PIO_LOG_SIZE	0x0F00	/* RP PIO Log Size */
+#define  PCI_EXP_DPC_RP_PIO_LOG_SIZE	0x0F00	/* RP PIO Log Size [3:0] */
 #define  PCI_EXP_DPC_CAP_DL_ACTIVE	0x1000	/* ERR_COR signal on DL_Active supported */
+#define  PCI_EXP_DPC_RP_PIO_LOG_SIZE4	0x2000	/* RP PIO Log Size [4] */
 
 #define PCI_EXP_DPC_CTL			0x06	/* DPC control */
 #define  PCI_EXP_DPC_CTL_EN_FATAL	0x0001	/* Enable trigger on ERR_FATAL message */
-- 
2.39.5

Re: [PATCH v2 1/2] PCI: Track Flit Mode Status & print it with link status

[Yazen Ghannam]

On Fri, Feb 07, 2025 at 06:18:35PM +0200, Ilpo Järvinen wrote:
> PCIe r6.0 added Flit mode that mainly alters HW behavior but some OS
> visible changes are also because of it. The OS visible changes include

The first sentence reads oddly. Maybe a slight change?

"...but there are some OS visible changes because of it."

> differences in the layout of some capabilities and interpretation of
> the TLP headers (in diagnostics situations).
> 
> To be able to determine which mode the PCIe link is using, store the
> Flit Mode Status (PCIe r6.1 sec 7.5.3.20) information in addition to
> the link speed into struct pci_bus in pcie_update_link_speed().
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c |  5 +++--
>  drivers/pci/pci.c                | 12 ++++++++----
>  drivers/pci/pci.h                |  3 ++-
>  drivers/pci/probe.c              |  5 +++--
>  include/linux/pci.h              |  1 +
>  5 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index bb5a8d9f03ad..10130ac9f979 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -292,7 +292,7 @@ int pciehp_check_link_status(struct controller *ctrl)
>  {
>  	struct pci_dev *pdev = ctrl_dev(ctrl);
>  	bool found;
> -	u16 lnk_status;
> +	u16 lnk_status, linksta2;
>  
>  	if (!pcie_wait_for_link(pdev, true)) {
>  		ctrl_info(ctrl, "Slot(%s): No link\n", slot_name(ctrl));
> @@ -319,7 +319,8 @@ int pciehp_check_link_status(struct controller *ctrl)
>  		return -1;
>  	}
>  
> -	__pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status);
> +	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &linksta2);
> +	__pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status, linksta2);
>  
>  	if (!found) {
>  		ctrl_info(ctrl, "Slot(%s): No device found\n",
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 869d204a70a3..313c66863752 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6190,21 +6190,25 @@ void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
>  	enum pci_bus_speed speed, speed_cap;
>  	struct pci_dev *limiting_dev = NULL;
>  	u32 bw_avail, bw_cap;
> +	char *flit_mode = "";
>  
>  	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
>  	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
>  
> +	if (dev->bus && dev->bus->flit_mode)
> +		flit_mode = ", in Flit mode";
> +
>  	if (bw_avail >= bw_cap && verbose)
> -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
> +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)%s\n",
>  			 bw_cap / 1000, bw_cap % 1000,
> -			 pci_speed_string(speed_cap), width_cap);
> +			 pci_speed_string(speed_cap), width_cap, flit_mode);
>  	else if (bw_avail < bw_cap)
> -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
> +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)%s\n",
>  			 bw_avail / 1000, bw_avail % 1000,
>  			 pci_speed_string(speed), width,
>  			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
>  			 bw_cap / 1000, bw_cap % 1000,
> -			 pci_speed_string(speed_cap), width_cap);
> +			 pci_speed_string(speed_cap), width_cap, flit_mode);

Does the "Flit mode" message *need* to go into these lines? Could it be
its own message?

 +#include <linux/string_choices.h>

 @@ -6190,21 +6190,25 @@ void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
  	enum pci_bus_speed speed, speed_cap;
  	struct pci_dev *limiting_dev = NULL;
  	u32 bw_avail, bw_cap;
  
  	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
  	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
  
 +	if (dev->bus)
 +		pci_info(dev, "Flit mode: %s\n", str_enabled_disabled(dev->bus->flit_mode);
 +
  	if (bw_avail >= bw_cap && verbose)
 		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
  			 bw_cap / 1000, bw_cap % 1000,
 			 pci_speed_string(speed_cap), width_cap);
  	else if (bw_avail < bw_cap)
 		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
  			 bw_avail / 1000, bw_avail % 1000,
  			 pci_speed_string(speed), width,
  			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
  			 bw_cap / 1000, bw_cap % 1000,
 			 pci_speed_string(speed_cap), width_cap);

>  }
>  
>  /**
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 01e51db8d285..9c6a4a980678 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -406,9 +406,10 @@ const char *pci_speed_string(enum pci_bus_speed speed);
>  void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
>  void pcie_report_downtraining(struct pci_dev *dev);
>  
> -static inline void __pcie_update_link_speed(struct pci_bus *bus, u16 linksta)
> +static inline void __pcie_update_link_speed(struct pci_bus *bus, u16 linksta, u16 linksta2)
>  {
>  	bus->cur_bus_speed = pcie_link_speed[linksta & PCI_EXP_LNKSTA_CLS];
> +	bus->flit_mode = linksta2 & PCI_EXP_LNKSTA2_FLIT;

Can we align on the '='?

Thanks,
Yazen

Re: [PATCH v2 2/2] PCI: Handle TLP Log in Flit mode

[Yazen Ghannam]

On Fri, Feb 07, 2025 at 06:18:36PM +0200, Ilpo Järvinen wrote:
> Flit mode introduced in PCIe r6.0 alters how the TLP Header Log is
> presented through AER and DPC Capability registers. The TLP Prefix Log
> Register is not present with Flit mode and the register becomes
> extension for TLP Header Log (PCIe r6.1 secs 7.8.4.12 & 7.9.14.13).
> 
> Adapt pcie_read_tlp_log() and struct pcie_tlp_log to read and store
> also the extended TLP Header Log when the link is in Flit mode. As
> Prefix Log and Extended TLP Header are not present at the same time,
> C union can be used.
> 
> Determining whether the error occurred while the Link was in Flit mode
> is bit complicated. In case of AER, Advanced Error Capabilities and
> Control Register directly tells whether the error was logged in Flit
> mode or not (PCIe r6.1 sec 7.8.4.7). DPC Capability (PCIe r6.1 sec
> 7.9.14), unfortunately, does not contain the same information.
> 
> Unlike AER, the DPC capability does not provide way to discern whether
> the error was logged in Flit mode (this is confirmed by PCI WG to be an
> oversight in the spec). DPC will bring link down immediately following
> an error, which make it impossible to acquire the Flit mode status
> directly from the Link Status 2 register because Flit Mode Status is
> only set in certain Link states (PCIe r6.1 sec 7.5.3.20). As a
> workaround, use the flit_mode value stored into the struct pci_bus.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/pci.h             |  3 +-
>  drivers/pci/pcie/aer.c        |  1 +
>  drivers/pci/pcie/dpc.c        | 18 +++++++++--
>  drivers/pci/pcie/tlp.c        | 56 ++++++++++++++++++++++++-----------
>  include/linux/aer.h           | 12 ++++++--
>  include/ras/ras_event.h       | 12 ++++----
>  include/uapi/linux/pci_regs.h |  6 +++-
>  7 files changed, 77 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 9c6a4a980678..07cc9be6f80b 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -554,7 +554,8 @@ 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 pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
> -		      unsigned int tlp_len, struct pcie_tlp_log *log);
> +		      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);
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 508474e17183..9c3e8299ad50 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1245,6 +1245,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>  			pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG,
>  					  aer + PCI_ERR_PREFIX_LOG,
>  					  aer_tlp_log_len(dev, aercc),
> +					  aercc & PCI_ERR_CAP_TLP_LOG_FLIT,
>  					  &info->tlp);
>  		}
>  	}

It seems Linux does not support "Multiple Error Handling", correct? We
save the TLP logs once for each device, and the user will need to know
that these are just for the "First" error, right?

Thanks,
Yazen

Re: [PATCH v2 1/2] PCI: Track Flit Mode Status & print it with link status

[Bjorn Helgaas]

On Fri, Feb 21, 2025 at 10:29:48AM -0500, Yazen Ghannam wrote:
> On Fri, Feb 07, 2025 at 06:18:35PM +0200, Ilpo Järvinen wrote:
> > PCIe r6.0 added Flit mode that mainly alters HW behavior but some OS
> > visible changes are also because of it. The OS visible changes include
> 
> The first sentence reads oddly. Maybe a slight change?
> 
> "...but there are some OS visible changes because of it."

Updated locally.

> > +	if (dev->bus && dev->bus->flit_mode)
> > +		flit_mode = ", in Flit mode";
> > +
> >  	if (bw_avail >= bw_cap && verbose)
> > -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
> > +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)%s\n",
> >  			 bw_cap / 1000, bw_cap % 1000,
> > -			 pci_speed_string(speed_cap), width_cap);
> > +			 pci_speed_string(speed_cap), width_cap, flit_mode);
> >  	else if (bw_avail < bw_cap)
> > -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
> > +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)%s\n",
> >  			 bw_avail / 1000, bw_avail % 1000,
> >  			 pci_speed_string(speed), width,
> >  			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
> >  			 bw_cap / 1000, bw_cap % 1000,
> > -			 pci_speed_string(speed_cap), width_cap);
> > +			 pci_speed_string(speed_cap), width_cap, flit_mode);
> 
> Does the "Flit mode" message *need* to go into these lines? Could it be
> its own message?

I suppose it doesn't need to be there, and these bandwidth lines are
already pretty long (my fault, open to suggestions to shorten them),
but I do think it's useful to have related info all on the same line.

>  +#include <linux/string_choices.h>
> 
>  @@ -6190,21 +6190,25 @@ void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
>   	enum pci_bus_speed speed, speed_cap;
>   	struct pci_dev *limiting_dev = NULL;
>   	u32 bw_avail, bw_cap;
>   
>   	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
>   	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
>   
>  +	if (dev->bus)
>  +		pci_info(dev, "Flit mode: %s\n", str_enabled_disabled(dev->bus->flit_mode);
>  +
>   	if (bw_avail >= bw_cap && verbose)
>  		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
>   			 bw_cap / 1000, bw_cap % 1000,
>  			 pci_speed_string(speed_cap), width_cap);
>   	else if (bw_avail < bw_cap)
>  		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
>   			 bw_avail / 1000, bw_avail % 1000,
>   			 pci_speed_string(speed), width,
>   			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
>   			 bw_cap / 1000, bw_cap % 1000,
>  			 pci_speed_string(speed_cap), width_cap);
> 
> >  }

Re: [PATCH v2 0/2] PCI: Add support for logging Flit Mode TLPs (PCIe6)

[Bjorn Helgaas]

On Fri, Feb 07, 2025 at 06:18:34PM +0200, Ilpo Järvinen wrote:
> This series adds support for Flit Mode (PCIe6).
> 
> v2:
> - Rebased
> 
> Ilpo Järvinen (2):
>   PCI: Track Flit Mode Status & print it with link status
>   PCI: Handle TLP Log in Flit mode
> 
>  drivers/pci/hotplug/pciehp_hpc.c |  5 +--
>  drivers/pci/pci.c                | 12 ++++---
>  drivers/pci/pci.h                |  6 ++--
>  drivers/pci/pcie/aer.c           |  1 +
>  drivers/pci/pcie/dpc.c           | 18 ++++++++--
>  drivers/pci/pcie/tlp.c           | 56 ++++++++++++++++++++++----------
>  drivers/pci/probe.c              |  5 +--
>  include/linux/aer.h              | 12 +++++--
>  include/linux/pci.h              |  1 +
>  include/ras/ras_event.h          | 12 +++----
>  include/uapi/linux/pci_regs.h    |  6 +++-
>  11 files changed, 94 insertions(+), 40 deletions(-)

Applied to pci/aer for v6.15, thanks!