Re: [PATCH v20 2/9] PCI: Establish common CXL Port protocol error flow

["Cheatham, Benjamin" <benjamin.cheatham@amd.com>]

On 9/2/2026 8:39 AM, Terry Bowman wrote:
> Establish a single CXL protocol error path shared by CXL Virtual
> Hierarchy (VH) and Restricted CXL Host (RCH) topologies. AER dispatch in
> handle_error_source() routes CXL protocol errors, gated by
> is_cxl_error(), through the AER-CXL kfifo to a cxl_core consumer for
> logging and recovery. Producer and consumer go live together so no CXL
> error is silently dropped across a bisect.
> 
> is_cxl_error() expands from Endpoint-only to also cover Root Port,
> Upstream Port, and Downstream Port. RCDs report on behalf of an upstream
> RCH Downstream Port and instead reach the kfifo via
> cxl_rch_handle_error().
> 
> For uncorrectable errors, cxl_proto_err_wait_for_empty() drains the CXL
> plane (RAS read, panic policy, state clear) before pci_aer_handle_error()
> drives PCIe recovery, so recovery does not tear down RAS iomaps while the
> consumer is still reading them. Correctable errors run asynchronously.
> 
> Panic policy: cxl_do_recovery() panics on a confirmed UCE, and also when
> the RAS registers cannot be mapped -- an unconfirmable UCE is treated
> conservatively as fatal since CXL.mem coherency may be lost. A
> mapped-but-clear status is logged as spurious with no panic.
> 
> to_ras_base() centralizes RAS base lookup (dport->regs.ras for
> Root/Downstream Ports, port->regs.ras otherwise) and provides an
> injection point for RAS status simulation during testing. The
> cxl_cor_error_detected() AER callback is removed; correctable Endpoint
> errors now route through the kfifo like every other CXL protocol error.
> 
> Update cxl_handle_rdport_errors() with locking to prevent dport from
> being freed and RAS from being unmapped.
> 
> At this step cxl_handle_rdport_errors() still dispatches a single
> severity per pass (matching the pre-series baseline). The following
> patch, "cxl/ras: Handle RCH correctable and uncorrectable errors in one
> pass", processes a simultaneously signalled CE and UCE together.
> 
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> 
> ---
> 

One small nit, but otherwise LGTM:
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

...

>  pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>  				    pci_channel_state_t state)
>  {
> -	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> -	struct cxl_memdev *cxlmd = cxlds->cxlmd;
> -	struct device *dev = &cxlmd->dev;
> -	bool ue;
> +	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> +	bool ue = false;
> +
> +	if (!port)
> +		return PCI_ERS_RESULT_DISCONNECT;
> +
> +	if (is_cxl_restricted(pdev))
> +		cxl_handle_rdport_errors(pdev);
>  
> -	scoped_guard(device, dev) {
> -		if (!dev->driver) {
> +	scoped_guard(device, &port->dev) {
> +		if (!port->dev.driver) {
>  			dev_warn(&pdev->dev,
> -				 "%s: memdev disabled, abort error handling\n",
> -				 dev_name(dev));
> +				 "%s: port disabled, abort error handling\n",
> +				 dev_name(&port->dev));
>  			return PCI_ERS_RESULT_DISCONNECT;
>  		}
>  
> -		if (cxlds->rcd)
> -			cxl_handle_rdport_errors(cxlds);
>  		/*
> -		 * A frozen channel indicates an impending reset which is fatal to
> -		 * CXL.mem operation, and will likely crash the system. On the off
> -		 * chance the situation is recoverable dump the status of the RAS
> -		 * capability registers and bounce the active state of the memdev.
> +		 * The CXL RAS read is unconditional regardless of channel
> +		 * state. Any uncorrectable error bit set in the CXL RAS
> +		 * status register triggers a panic below because CXL.mem
> +		 * cache coherency is already lost; continuing risks silent
> +		 * data corruption.
>  		 */
> -		ue = cxl_handle_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
> +		ue = cxl_handle_ras(port->uport_dev, to_ras_base(port, NULL));
>  	}
>  
> +	/*
> +	 * CXL.mem UCE means cache coherency is lost. Continuing risks
> +	 * silent data corruption.
> +	 */

Don't need this comment and the last sentence in the comment above.
> +	if (ue)
> +		panic("CXL cachemem error");
> +
>  	switch (state) {
>  	case pci_channel_io_normal:
> -		if (ue) {
> -			device_release_driver(dev);
> -			return PCI_ERS_RESULT_NEED_RESET;
> -		}
>  		return PCI_ERS_RESULT_CAN_RECOVER;
>  	case pci_channel_io_frozen:
>  		dev_warn(&pdev->dev,
>  			 "%s: frozen state error detected, disable CXL.mem\n",
> -			 dev_name(dev));
> -		device_release_driver(dev);
> +			 dev_name(port->uport_dev));
> +		device_release_driver(port->uport_dev);
>  		return PCI_ERS_RESULT_NEED_RESET;
>  	case pci_channel_io_perm_failure:
>  		dev_warn(&pdev->dev,
> @@ -335,3 +371,82 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>  	return PCI_ERS_RESULT_NEED_RESET;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");