Re: [PATCH v20 0/9] Enable CXL PCIe Port Protocol Error handling and logging

[Lukas Wunner <lukas@wunner.de>]

On Wed, Sep 02, 2026 at 08:39:24AM -0500, Terry Bowman wrote:
> Today the kernel handles native CXL.cachemem RAS only for Endpoints and
> Restricted CXL Host (RCH) Downstream Ports. Root Ports, Upstream Switch
> Ports, and Downstream Switch Ports are uncovered. This series introduces
> a unified CXL protocol error path for all CXL device types, in both VH
> and RCH topologies.

I'm trying to make sense of the existing code.  I know this wasn't
introduced by you (but by Robert in 0a867568bb0d), but you've moved
this code around recently and are extending it in this series.

Errors of the RCH downstream port are signaled as internal errors
of an RCEC on the root bus.  Their handling was plumbed into:

aer_process_err_devices()
  handle_error_source()
    cxl_rch_handle_error()

First of all, handle_error_source() calls pci_aer_handle_error(),
which (for Fatal Errors) will issue an FLR of the RCEC.  Note that
Uncorrectable Internal Errors have Fatal severity by default
(PCIe r7.0 sec 7.8.4.4).

If the Uncorrectable Internal Error was the only error, then why is the
RCEC being reset?  It's just serving as a conduit to inform that there
are CXL errors at the downstream port.  There's no reason at all to
issue an FLR to the RCEC in that case.

Second, cxl_rch_handle_error() then walks all the RCiEPs reporting to
the RCEC.  This also looks weird to me.  Can there ever be more than
one RCiEP?  I think not, but maybe I'm missing something.  If there's
only ever a single RCiEP reporting to the RCEC, why perform a walk?

What we actually want to do is retrieve the CXL errors from the
downstream port's RCRB, but this is done in a fairly roundabout way:
cxl_rch_handle_error() walks the RCiEPs (aka RCDs), invokes the
->error_detected() callback for each, which is cxl_error_detected().
That will then call cxl_handle_rdport_errors() to find the RCH
downstream port to which the RCD is attached.

Isn't there a simpler way to find the RCH downstream port from which
the error originated?  Why do we have to go through the RCDs?

Also, putting this in cxl_error_detected() has a weird side effect:
The function is also invoked when the RCD upstream port experienced
an error.  But because the retrieval, reporting and handling of
RCH downstream port errors was put into this function, those errors
are reported and handled as a side effect of RCD upstream port errors.
What sense does this make?

The commit message of 0a867568bb0d provides the following hint as
to why this approach was chosen:

    The reason for choosing this implementation is that the AER service
    driver claims the RCEC device, but does not allow it to register a
    custom specific handler to support CXL. Connecting the RCEC hard-wired
    with a CXL handler does not work, as the CXL subsystem might not be
    present all the time.

So I think the point may have been to make this work even if the cxl
module is not loaded?  Is that all?  Is that the only reason?
We have try_module_get() and symbol_get() helpers.  You could just
call those prior to invoking functions implemented by the cxl module
from the AER driver.  There's plenty of precedent for that in the kernel.

The problem is that the present approach is fairly complex and has
side effects which make it difficult to understand and reason about
the error handling.  In my view, this should be cleaned up first
before bolting more functionality on top of it.

Thanks,

Lukas