On 7/20/2026 7:00 PM, Jonathan Cameron wrote:
> On Fri, 17 Jul 2026 17:27:03 -0500
> Terry Bowman <[email protected]> wrote:
>
>> From: Dan Williams <[email protected]>
>>
>> Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_*
>> trace events instead of a plain struct device * derived at the
>> caller. The trace event helpers then derive the right strings for
>> endpoints, switch ports, root ports, and RCH downstream ports
>> consistently across the CPER and native AER paths.
>>
>> The unified cxl_aer_* events keep "memdev" as the legacy field
>> (endpoint events populate it with the memdev name; non-endpoint
>> events emit memdev="") and add new "port" and "dport" string fields
>> populated for all CXL device classes. Updated userspace can key
>> off "port" and "dport" without a parallel set of events.
>>
>> Remove the separate cxl_port_aer_uncorrectable_error and
>> cxl_port_aer_correctable_error trace events. All CXL AER events now
>> use the unified cxl_aer_* events with port and dport fields.
>>
>> Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and
>> the unified trace helpers, replacing the per-port-type branching and
>> bus_find_device() memdev lookup.
>>
>> The TP_printk format string places "port=%s dport=%s" between
>> "memdev=%s" and "host=%s", changing the text-mode field order from
>> the pre-patch output. This does not affect consumers such as
>> rasdaemon that use libtraceevent to parse fields by name rather than
>> by fixed text position.
>>
>> For non-endpoint events (switch port, root port, RCH dport),
>> "memdev" is empty and "port"/"dport" carry the topology information.
>>
>> The serial number is retrieved via pci_get_dsn() which performs live
>> PCI configuration space reads. A following patch ("PCI: Cache PCI
>> DSN into pci_dev->dsn during probe") replaces these with a cached
>> serial number to avoid config space access in error handlers and panic
>> paths.
>>
>> Below are examples of the different CXL devices' error trace logs
>> after this patch:
>>
>> ---------------------
>> | CXL RP - 0C:00.0 |
>> ---------------------
>> |
>> ---------------------
>> | CXL USP - 0D:00.0 |
>> ---------------------
>> |
>> --------------------
>> | CXL DSP - 0E:00.0 |
>> --------------------
>> |
>> ---------------------
>> | CXL EP - 0F:00.0 |
>> ---------------------
>>
>> Root Port:
>> cxl_aer_correctable_error: memdev= port=port1 dport=0000:0c:00.0 \
>> host=pci0000:0c serial=0: status: 'Memory Data ECC Error'
>>
>> cxl_aer_uncorrectable_error: memdev= port=port1 dport=0000:0c:00.0 \
>> host=pci0000:0c serial=0: status: 'Cache Address Parity Error' \
>> first_error: 'Cache Address Parity Error'
>>
>> Upstream Switch Port:
>> cxl_aer_correctable_error: memdev= port=port2 dport= host=0000:0d:00.0 \
>> serial=0: status: 'Memory Data ECC Error'
>>
>> UCE NA - Upstream Switch Port UCE's are handled in the portdrv driver's
>> PCI AER callbacks that are not CXL aware.
>>
>> Downstream Switch Port:
>> cxl_aer_correctable_error: memdev= port=port2 dport=0000:0e:00.0 \
>> host=0000:0d:00.0 serial=0: status: 'Memory Data ECC Error'
>>
>> cxl_aer_uncorrectable_error: memdev= port=port2 dport=0000:0e:00.0 \
>> host=0000:0d:00.0 serial=0: status: 'Cache Address Parity Error' \
>> first_error: 'Cache Address Parity Error'
>>
>> Endpoint:
>> cxl_aer_uncorrectable_error: memdev=mem1 port=endpoint4 dport= \
>> host=0000:0f:00.0 serial=0: status: 'Cache Address Parity Error' \
>> first_error: 'Cache Address Parity Error'
>>
>> cxl_aer_correctable_error: memdev=mem1 port=endpoint4 dport=
>> host=0000:0f:00.0 \
>> serial=0: status: 'Memory Data ECC Error'
>>
>> Co-developed-by: Terry Bowman <[email protected]>
>> Signed-off-by: Terry Bowman <[email protected]>
>> Signed-off-by: Dan Williams <[email protected]>
>
> One question inline about reference counts for the port.
>
>>
>> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
>> index d5dc2c22565da..acf40b2396c3b 100644
>> --- a/drivers/cxl/core/ras.c
>> +++ b/drivers/cxl/core/ras.c
> ...
>>
>> @@ -109,47 +77,34 @@ static struct cxl_port *find_cxl_port_by_dev(struct
>> device *dev, struct cxl_dpor
>>
>> void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
>> {
>> + struct cxl_dport *dport;
>> unsigned int devfn = PCI_DEVFN(data->prot_err.agent_addr.device,
>> data->prot_err.agent_addr.function);
>> - struct pci_dev *pdev __free(pci_dev_put) =
>> - pci_get_domain_bus_and_slot(data->prot_err.agent_addr.segment,
>> - data->prot_err.agent_addr.bus,
>> - devfn);
>> - struct cxl_memdev *cxlmd;
>> - int port_type;
>> -
>> - if (!pdev)
>> - return;
>> -
>> - port_type = pci_pcie_type(pdev);
>> - if (port_type == PCI_EXP_TYPE_ROOT_PORT ||
>> - port_type == PCI_EXP_TYPE_DOWNSTREAM ||
>> - port_type == PCI_EXP_TYPE_UPSTREAM) {
>> - if (data->severity == AER_CORRECTABLE)
>> - cxl_cper_trace_corr_port_prot_err(pdev, data->ras_cap);
>> - else
>> - cxl_cper_trace_uncorr_port_prot_err(pdev,
>> data->ras_cap);
>> -
>> + struct pci_dev *pdev __free(pci_dev_put) = pci_get_domain_bus_and_slot(
>> + data->prot_err.agent_addr.segment,
>> data->prot_err.agent_addr.bus, devfn);
>> + if (!pdev) {
>> + pr_err_ratelimited("Failed to find CPER device in CXL
>> topology\n");
>> return;
>> }
>>
>> - guard(device)(&pdev->dev);
>> - if (!pdev->dev.driver) {
>> - dev_warn_ratelimited(&pdev->dev,
>> - "Device is unbound, abort CPER error
>> handling\n");
>> + struct cxl_port *port __free(put_cxl_port) =
>> find_cxl_port_by_dev(&pdev->dev, NULL);
>> + if (!port) {
>> + dev_err_ratelimited(&pdev->dev,
>> + "Failed to find parent port device in CXL
>> topology\n");
>> return;
>> }
>>
>> - struct device *mem_dev __free(put_device) = bus_find_device(
>> - &cxl_bus_type, NULL, pdev, match_memdev_by_parent);
>> - if (!mem_dev)
>> - return;
>> + guard(device)(&port->dev);
>
> Don't we have a reference for this from find_cxl_port_by_dev()?
>
> Superficially scope looks the same.
>
>
I somehow missed this yesterday. Yes, find_cxl_port_by_dev() ref increments.
>> +
>> + /* dport is NULL for Endpoint and Upstream Port devices */
>> + dport = cxl_find_dport_by_dev(port, &pdev->dev);
>>
>> - cxlmd = to_cxl_memdev(mem_dev);
>> if (data->severity == AER_CORRECTABLE)
>> - cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
>> + cxl_cper_trace_corr_prot_err(port, dport, pci_get_dsn(pdev),
>> + &data->ras_cap);
>> else
>> - cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
>> + cxl_cper_trace_uncorr_prot_err(port, dport, pci_get_dsn(pdev),
>> + &data->ras_cap);
>> }
>> EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
>>
>> @@ -240,14 +195,14 @@ void cxl_do_recovery(struct pci_dev *pdev, struct
>> cxl_port *port, struct cxl_dpo
>> return;
>> }
>>
>> - if (cxl_handle_ras(port, dport, ras_base))
>> + if (cxl_handle_ras(port, dport, ras_base, pci_get_dsn(pdev)))
>> panic("CXL cachemem error");
>>
>> dev_dbg(&pdev->dev,
>> "CXL UCE signaled but no CXL RAS status bits set\n");
>> }
>>
>> -void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
>> void __iomem *ras_base)
>> +void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
>> void __iomem *ras_base, u64 serial)
> That's over even the modern 100 char limit. Needs a line break.
>> {
>
Yup, I'll linewrap at 75-80. Thanks.
- Terry