Re: [PATCH v18 09/13] cxl: Update CXL Endpoint AER handler

"Bowman, Terry" <[email protected]>
Newsgroups org.kernel.vger.linux-cxl,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-pci
Message-ID <[email protected]>
On 7/17/2026 5:53 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unconditional kernel panic on PCIe link failure for idle/unused CXL endpoint devices.

This is a false positive.

The comment addresses the corner case when the device is removed from the bus
even momentarily or transiently. In this case the memory coherency will be lost
as cache lines will not be accessible and continuing operation risks silent data
corruption. The kernel panic is intentional.

-Terry

> --
> 
> --- Patch [9]: [PATCH v18 09/13] cxl: Update CXL Endpoint AER handler ---
> commit 01bcb4332c20e7d1dcce013851a595e79f03a892
> Author: Terry Bowman <[email protected]>
> 
> cxl: Update CXL Endpoint AER handler
> 
> This commit updates the CXL endpoint AER error handler. It documents the
> unconditional CXL RAS read policy and relies on this policy for CXL endpoint
> error handling.
> 
>> Document the unconditional CXL RAS read policy: on a dead link,
>> readl() returns 0xFFFFFFFF which is interpreted as UCE bits set and
>> triggers a panic. If RAS registers are not mapped the read is
>> skipped and the frozen/perm_failure switch cases defer to AER
>> recovery for devices without active CXL.mem traffic.
> 
> Is this assumption about RAS registers not being mapped accurate? 
> 
> Looking at cxl_endpoint_port_probe(), it appears to unconditionally map the
> RAS registers via devm_cxl_port_ras_setup() for any present endpoint.
> 
>> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
>> --- a/drivers/cxl/core/ras.c
>> +++ b/drivers/cxl/core/ras.c
> 
> [ ... ]
> 
>> @@ -349,10 +347,18 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>>  		}
>>  
>>  		/*
>> -		 * 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 because CXL.mem cache
>> +		 * coherency is already lost; continuing risks silent data
>> +		 * corruption across interleaved HDM regions.
>> +		 *
>> +		 * On a dead link readl() returns 0xFFFFFFFF which sets all
>> +		 * UCE bits and also triggers the panic - this is intentional.
>> +		 * If RAS registers are not mapped the read is skipped, the
>> +		 * panic is not reached, and the frozen/perm_failure switch
>> +		 * cases below handle AER recovery for devices without active
>> +		 * CXL.mem traffic.
>>  		 */
>>  		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
> 
> [Severity: High]
> Does this regression cause a system-wide panic for idle or unused CXL endpoint
> devices?
> 
> If devm_cxl_port_ras_setup() unconditionally maps the RAS registers during
> probe, cxl_handle_ras() will always execute the readl() here. During a
> transient PCIe link failure (like an AER frozen state) or a surprise hot
> unplug, the link is dead, so readl() returns 0xFFFFFFFF.
> 
> Because this sets the uncorrectable error (UCE) bits, cxl_handle_ras() will
> return true. Doesn't this mean any transient PCIe link failure will now
> unconditionally crash the system due to the subsequent if (ue) panic check,
> breaking standard PCIe error containment?
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.