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

[Bowman, Terry]

On 9/2/2026 3:57 PM, Cheatham, Benjamin wrote:
> 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@xxxxxxxxxx>
>> Signed-off-by: Dan Williams <djbw@xxxxxxxxxx>
>> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
>>
>> ---
>>
> 
> One small nit, but otherwise LGTM:
> Reviewed-by: Ben Cheatham <benjamin.cheatham@xxxxxxx>
> 
> ...
> 
>>  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.

I'll keep the panic-site comment (it's the "why" at the point of consequence) and will 
remove the duplicated coherency/corruption sentence from the block comment above (done 
in 2/9) so there's no duplication at any point in the series.

-Terry

>> +	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");