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

[sashiko-bot@kernel.org]

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.
--

--- Patch [9]: [PATCH v18 09/13] cxl: Update CXL Endpoint AER handler ---
commit 01bcb4332c20e7d1dcce013851a595e79f03a892
Author: Terry Bowman <terry.bowman@amd.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717222706.3540281-1-terry.bowman@amd.com?part=9