Re: [PATCH v18 06/13] PCI: Establish common CXL Port protocol error flow

On Fri, 17 Jul 2026 17:26:59 -0500
Terry Bowman [off-list ref] wrote:

Add CXL protocol error dispatch in handle_error_source() using
is_cxl_error() and cxl_forward_error() to route errors through the
AER-CXL kfifo. Expand is_cxl_error() from Endpoint-only to include
Root Port, Upstream Port, and Downstream Port device types. The
producer and consumer go live in the same commit to avoid silently
dropping CXL errors during bisect.

Take a stab at making this description a lot more concise. I'd like
to just be seeing a very brief summary of flow + clearly highlighting
of potentially controversial choices.

For uncorrectable events, call cxl_proto_err_flush() to ensure CXL RAS
registers are read, panic policy is applied, and CXL state is cleared
before pci_aer_handle_error() drives PCIe recovery. Without the flush,
AER recovery can tear down drivers and unmap the CXL RAS iomaps while
the kfifo consumer is still reading them. Correctable events do not
need the flush and run asynchronously. RCH kfifo support is added in
the following patch ("PCI/CXL: Add RCH support to CXL handlers").

Add cxl_handle_proto_error() to dispatch correctable and uncorrectable
errors through the CXL RAS helpers. Add cxl_do_recovery() to coordinate
uncorrectable recovery. Panic when a UCE is confirmed by a successful
CXL RAS status register read. If the RAS registers cannot be read the
UCE cannot be confirmed and panic is not triggered. Gate error handling
on the port driver being bound to avoid processing errors on disabled
devices.

The kfifo consumer holds guard(device)(&port->dev) and checks
port->dev.driver before accessing RAS registers, serializing against
driver unbind and devm iomap teardown. For UCE, cxl_proto_err_flush()
runs the worker synchronously before AER recovery, ensuring the device
is present during RAS register access.

Add to_ras_base() to centralize RAS base lookup: dport->regs.ras for
Root/Downstream Ports, port->regs.ras for Upstream Ports and Endpoints.
Use to_ras_base() to access the CXL devices' RAS registers as it will
provide an avenue to inject status simulation during testing.

Add CXL RAS logging in cxl_handle_cor_ras() and cxl_handle_ras(). The
existing cxl_cor_error_detected() and cxl_error_detected() AER
callbacks remain for all Endpoints and are reworked to use
find_cxl_port_by_uport() and to_ras_base(), with UCE now triggering
panic unconditionally. These callbacks are further updated in the
following patch ("PCI/CXL: Add RCH support to CXL handlers").

Fix a pre-existing race for cxlds between cxl_handle_rdport_errors() and
cxl_memdev_shutdown() by holding a cxlmd device scoped_guard() around
the rdport call. Release the lock before taking the Port lock to avoid
the lock inversion.

Co-developed-by: Dan Williams <djbw@kernel.org>
Signed-off-by: Dan Williams <djbw@kernel.org>
Signed-off-by: Terry Bowman <redacted>

A few minor things about the code inline.

Thanks,

Jonathan

I'd state this is optional.

+ * lookups (Root Port, Downstream Port), NULL otherwise.
+ *
+ * Return a 'struct cxl_port' with an elevated reference if found. Use
+ * __free(put_cxl_port) to release.
+ */
+static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dport **dport)
+{
+	if (dport)
+		*dport = NULL;
+	if (!dev_is_pci(dev))
+		return NULL;
+
+	switch (pci_pcie_type(to_pci_dev(dev))) {
+	case PCI_EXP_TYPE_ROOT_PORT:
+	case PCI_EXP_TYPE_DOWNSTREAM:
+		return find_cxl_port_by_dport(dev, dport);
+	case PCI_EXP_TYPE_UPSTREAM:
+	case PCI_EXP_TYPE_ENDPOINT:
+	case PCI_EXP_TYPE_RC_END:
+		return find_cxl_port_by_uport(dev);
+	}
+
+	return NULL;
+}

...

Unless this gets more complex later you can just use a guard() as exits
scope here anyway.

+	}
+
+	scoped_guard(device, &port->dev) {

Similar here. I haven't read on though for this one so if it's needed
later just ignore me.

...

 
+	/*
+	 * CXL.mem UCE means cache coherency is lost. Continuing risks
+	 * silent data corruption across interleaved HDM regions.

It is a problem whether interleave or not.  So maybe just stop
at corruption.

+	 */
+	if (ue)
+		panic("CXL cachemem error");

...

...

+static void __cxl_proto_err_work_fn(struct cxl_proto_err_work_data *wd)
+{
+	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&wd->pdev->dev, NULL);
+	if (!port) {
+		dev_err_ratelimited(&wd->pdev->dev,
+				    "Failed to find parent port device in CXL topology\n");
+		return;
+	}

I'd put a blank line here.

+	guard(device)(&port->dev);
+	if (!port->dev.driver) {
+		dev_err_ratelimited(&port->dev,
+				    "Port device is unbound, abort error handling\n");
+		return;
+	}
+
+	struct cxl_dport *dport = cxl_find_dport_by_dev(port, &wd->pdev->dev);
+	if (!dport && (pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+		       pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_DOWNSTREAM)) {
+		dev_err_ratelimited(&wd->pdev->dev,
+				    "Failed to find dport device in CXL topology\n");
+		return;
+	}
+
+	cxl_handle_proto_error(wd->pdev, port, dport, wd->severity);
+}

+
+	/*
+	 * Wait for UCE CXL work to complete before AER recovery
+	 * tears down the device. CE can run asynchronously.
+	 */
+	if (cxl_pending && info->severity != AER_CORRECTABLE)
+		cxl_proto_err_flush();

This kind of makes me wonder if the naming of _flush() is clear.
Normally I'd kind of expect that to be dumping any queued up errors.

Perhaps cxl_proto_err_wait_for_empty() or something like that?

I guess flush_work() set the precedent so I'm too later on this one!

Jonathan