Re: [RESEND v13 12/25] cxl/pci: Unify CXL trace logging for CXL Endpoints and CXL Ports

[dan.j.williams]

Bowman, Terry wrote:
> 
> 
> On 11/19/2025 3:23 PM, dan.j.williams@xxxxxxxxx wrote:
> > Terry Bowman wrote:
> >> CXL currently has separate trace routines for CXL Port errors and CXL
> >> Endpoint errors. This is inconvenient for the user because they must enable
> >> 2 sets of trace routines. Make updates to the trace logging such that a
> >> single trace routine logs both CXL Endpoint and CXL Port protocol errors.
> > No, this is not inconvient, this is required for compatible evolution of
> > tracepoints. The change in this patch breaks compatibility as it
> > violates the expectation the type and order of TP_ARGS does not change
> > from one kernel to next.
> >
> >> Keep the trace log fields 'memdev' and 'host'. While these are not accurate
> >> for non-Endpoints the fields will remain as-is to prevent breaking
> >> userspace RAS trace consumers.
> >>
> >> Add serial number parameter to the trace logging. This is used for EPs
> >> and 0 is provided for CXL port devices without a serial number.
> >>
> >> Leave the correctable and uncorrectable trace routines' TP_STRUCT__entry()
> >> unchanged with respect to member data types and order.
> >>
> >> Below is output of correctable and uncorrectable protocol error logging.
> >> CXL Root Port and CXL Endpoint examples are included below.
> >>
> >> Root Port:
> >> cxl_aer_correctable_error: memdev=0000:0c:00.0 host=pci0000:0c serial: 0 status='CRC Threshold Hit'
> >> cxl_aer_uncorrectable_error: memdev=0000:0c:00.0 host=pci0000:0c serial: 0 status: 'Cache Byte Enable Parity Error' first_error: 'Cache Byte Enable Parity Error'
> > A root port is not a "memdev", another awkward side effect of trying to
> > combine 2 trace points with different use cases.
> >
> > So a NAK from me for this change (unless there is an strong reason for
> > Linux to inflict the compat breakage), please keep the separate
> > tracepoints they are for distinctly different use cases. A memdev
> > protocol error is contained to that memdev, a port protocol error
> > implicates every CXL.cachemem descendant of that port.
> I misunderstood this comment from previous code review:
> https://lore.kernel.org/linux-cxl/67aea897cfe55_2d1e294ca@xxxxxxxxxxxxxxxxxxxxxxxxx.notmuch/#t

No, you did not misunderstand, I just did not realize at the time I was
asking for compatibility breakage with that suggestion. Apologies for
that thrash.

> Are you OK with the following format for Port devices? Or let me know what format is needed.
> cxl_port_aer_correctable_error: device=port1 parent=root0 status='Received Error From Physical Layer'
> cxl_port_aer_uncorrectable_error: device=port1 parent=root0 status: 'Memory Byte Enable Parity Error' first_error: 'Memory Byte Enable Parity Erro'

That looks good to me.

Also, I realize this patch set has gone through many revisions. We
really need to get at least some of these pre-req patches into a topic
branch so they do not need to keep being sent out in this large series.