Re: [PATCH v20 9/9] Documentation: cxl: Document CXL protocol error handling

[Bowman, Terry]

On 9/8/2026 1:39 PM, Jonathan Cameron wrote:
> On Wed, 2 Sep 2026 08:39:33 -0500
> Terry Bowman <terry.bowman@xxxxxxx> wrote:
> 
>> Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst
>> describing the end-to-end CXL protocol error path: AER ingress, the
>> AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special
>> cases, severity policy, trace events, and a source code map.
>>
>> This documents the architecture introduced by the preceding patches in
>> this series.
>>
>> Assisted-by: Claude:claude-opus-4.8
> Assited-by: LLM
Ok, will switch to "Assisted-by: LLM" across the series.

> for all of these (kernel documentation was recently changed on this).
>> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
>> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>
>> Reviewed-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> 
> 
> ...
> 
>> diff --git a/Documentation/driver-api/cxl/linux/protocol-error-handling.rst b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
>> new file mode 100644
>> index 0000000000000..1da71d0409a05
>> --- /dev/null
>> +++ b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
>> @@ -0,0 +1,441 @@
>> +Error flow
>> +==========
>> +
>> +.. code-block:: text
>> +
>> +   CXL device raises AER Internal Error
>> +   (PCI_ERR_COR_INTERNAL or PCI_ERR_UNC_INTN)
>> +                   |
>> +                   v
>> +   +--------------------------------------+
>> +   | AER core (aer.c)                     |
>> +   |  aer_irq() -> aer_isr()              |
>> +   |  -> find_source_device()             |
>> +   |  -> handle_error_source(dev, info)   |
>> +   +--------------------------------------+
>> +                   |
>> +                   v
>> +   +--------------------------------------+
>> +   | handle_error_source() dispatch       |
>> +   |                                      |
>> +   |  1. cxl_rch_handle_error()           |
>> +   |     [always; filters internally.     |
>> +   |      RC_END enters the kfifo here    |
>> +   |      via pcie_walk_rcec(), NOT via   |
>> +   |      is_cxl_error() below]           |
>> +   |                                      |
>> +   |  2. if is_cxl_error():               |
>> +   |       cxl_forward_error()            |
>> +   |       [enqueue to kfifo; EP/RP/USP/  |
>> +   |        DSP only, RC_END excluded]    |
>> +   |                                      |
>> +   |  3. if cxl_pending && non-CE:        |
>> +   |       cxl_proto_err_wait_for_empty() |
>> +   |       [sync drain before recovery]   |
>> +   |                                      |
>> +   |  4. pci_aer_handle_error() [always]  |
>> +   +--------------------------------------+
>> +                   |
>> +          (kfifo -> workqueue)
>> +                   |
>> +                   v
>> +   +--------------------------------------+
>> +   | __cxl_proto_err_work_fn() consumer   |
>> +   |                                      |
>> +   |  if is_cxl_restricted(pdev):         |
>> +   |    cxl_handle_rdport_errors()        |
>> +   |    [RCH dport RAS first]             |
>> +   |                                      |
>> +   |  cxl_handle_proto_error()            |
>> +   +--------------------------------------+
>> +            |                |
>> +            v                v
>> +   +-----------------+  +--------------------+
>> +   | CE              |  | UCE                |
>> +   | cxl_handle_     |  | cxl_do_recovery()  |
>> +   |   cor_ras()     |  |  read RAS status   |
>> +   | trace + clear   |  |  trace + panic     |
>> +   +-----------------+  +--------------------+
> 
> Why so narrow. Seems like bits of this diag would be more readable if
> you use the whole 80 chars?
> 

Yes, would be more readable with wider boxes. I'll make all wider.

> 
>> +
>> +.. code-block:: text
>> +
>> +   Fatal UCE on Endpoint (VH Endpoint or RCD; link down, no AER status)
>> +                   |
>> +                   v
>> +   +--------------------------------------+
>> +   | PCIe core error recovery             |
>> +   |  pcie_do_recovery()                  |
>> +   |  -> report_error_detected()          |
>> +   |  -> cxl_pci_error_detected()         |
>> +   |     [pci_error_handlers callback in  |
>> +   |      cxl_core/ras.c; the RAS handler,|
>> +   |      NOT the AER kfifo path]         |
>> +   +--------------------------------------+
>> +                   |
>> +                   v
>> +   +--------------------------------------+
>> +   | cxl_pci_error_detected()             |
>> +   |                                      |
>> +   |  if is_cxl_restricted(pdev):         |
>> +   |    cxl_handle_rdport_errors()        |
>> +   |    [RCD-only: RCH Dport RAS first]   |
>> +   |                                      |
>> +   |  if port->dev.driver == NULL:        |
>> +   |    return DISCONNECT [port unbound]  |
>> +   |                                      |
>> +   |  cxl_handle_ras(port, NULL,          |
>> +   |                 to_ras_base(...),    |
>> +   |                 pdev->dsn)           |
>> +   |    [EP RAS read, independent of      |
>> +   |     channel state (not skipped for   |
>> +   |     io_normal); dead link            |
>> +   |     readl()==0xFFFFFFFF sets all UE  |
>> +   |     bits -> panic]                   |
>> +   |                                      |
>> +   |  if ue: panic("CXL cachemem error")  |
>> +   |                                      |
>> +   |  else switch (channel state):        |
>> +   |    io_normal      -> CAN_RECOVER     |
>> +   |    io_frozen      -> release driver, |
>> +   |                      NEED_RESET      |
>> +   |    perm_failure   -> DISCONNECT      |
>> +   +--------------------------------------+
> 
> Similar.  I have a new favourite irritation - overly narrow LLM (I guess)
> generated diagrams!
> 
> 
> 
>> +.. code-block:: text
>> +
>> +   Platform firmware CPER record (CPER_SEC_CXL_PROT_ERR)
>> +            |
>> +            v
>> +   +----------------------+
>> +   | GHES/APEI (ghes.c)   |
>> +   |  ghes_do_proc()      |
>> +   |  cxl_cper_post_      |
>> +   |    prot_err()        |
>> +   |  kfifo_put(CPER-CXL) |
>> +   |  schedule_work()     |
>> +   +----------------------+
> Ouch. Definitely wider here too to avoid splitting those function names.
> 
>> +            |
>> +            v
>> +   +----------------------+
>> +   | CPER-CXL kfifo       |
>> +   | + work_struct        |
>> +   +----------------------+
>> +            |
>> +            v
>> +   +----------------------+
>> +   | cxl_cper_prot_err_   |
>> +   |   work_fn() consumer |
>> +   | (cxl_core/ras.c)     |
>> +   |  drain kfifo ->      |
>> +   +----------------------+
>> +            |
>> +            v
>> +   +--------------------------------+
>> +   | cxl_cper_handle_prot_err()     |
>> +   |  pci_get_domain_bus_and_slot() |
>> +   |  find_cxl_port_by_dev()        |
>> +   |  cxl_find_dport_by_dev()       |
>> +   |                                |
>> +   |  if CE: trace correctable      |
>> +   |  else:  trace uncorrectable    |
>> +   |  [trace-only; no panic,        |
>> +   |   no cxl_do_recovery()]        |
>> +   +--------------------------------+
> 
>> +Severity policy
>> +===============
> 
>> +**Fatal UCE on EP/USP** - A fatal event brings the link down, so the AER
>> +core reads no AER status and is_cxl_error() cannot enqueue the event to the
>> +kfifo. Endpoints and RCDs are instead handled through the
>> +pci_error_handlers .error_detected callback (cxl_pci_error_detected()),
>> +which reads the CXL RAS registers when they are mapped and panics on any UE
>> +bit. If the RAS registers are unmapped the read is skipped without a panic,
>> +because this path has no prior confirmation that the error is CXL internal.
>> +Upstream Ports bound to portdrv fall back to standard AER recovery - a known
>> +limitation. See "Fatal UCE flow for Endpoints and RCDs" above for the full
>> +path and channel-state handling.
> 
> This 'known limitation' language kind of implies there is a solution. I'm curious,
> do you have one in mind?  I can sort of see maybe that the class of UCE that leaves
> CXL.io up is larger than that for PCIe so maybe it would be worth logic to probe
> the device and see if we can get to it's registers?  Anyhow, job for another day.
> 

Yes, we may be able to update aer_get_device_error_info() to check for upstream link 
health and if its intact then can possibly read the AER registers. I was leaving this 
for future improvement.

> 
> This looks good to me and even the diag things is just a 'make it prettier' so
> Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>

Thanks

-Terry