[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44212e87-1c70-4bc7-b3ec-7435729a5157@intel.com>
Date: Fri, 26 Sep 2025 13:44:43 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Terry Bowman <terry.bowman@....com>, dave@...olabs.net,
jonathan.cameron@...wei.com, alison.schofield@...el.com,
dan.j.williams@...el.com, bhelgaas@...gle.com, shiju.jose@...wei.com,
ming.li@...omail.com, Smita.KoralahalliChannabasappa@....com,
rrichter@....com, dan.carpenter@...aro.org,
PradeepVineshReddy.Kodamati@....com, lukas@...ner.de,
Benjamin.Cheatham@....com, sathyanarayanan.kuppuswamy@...ux.intel.com,
linux-cxl@...r.kernel.org, alucerop@....com, ira.weiny@...el.com
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v12 13/25] cxl/pci: Unify CXL trace logging for CXL
Endpoints and CXL Ports
On 9/25/25 3:34 PM, 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.
>
> 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'
>
> Endpoint:
> cxl_aer_correctable_error: memdev=mem3 host=0000:0f:00.0 serial=0 status='CRC Threshold Hit'
> cxl_aer_uncorrectable_error: memdev=mem3 host=0000:0f:00.0 serial: 0 status: 'Cache Byte Enable Parity Error' first_error: 'Cache Byte Enable Parity Error'
>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> Reviewed-by: Shiju Jose <shiju.jose@...wei.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
Reviewed-by: Dave Jiang <dave.jiang@...el.com>
>
> ---
>
> Changes in v11 -> v12:
> - Correct parameters to call trace_cxl_aer_correctable_error()
> - Add reviewed-by for Jonathan and Shiju
>
> Changes in v10->v11:
> - Updated CE and UCE trace routines to maintain consistent TP_Struct ABI
> and unchanged TP_printk() logging.
> ---
> drivers/cxl/core/ras.c | 34 ++++++++++----------
> drivers/cxl/core/trace.h | 68 +++++++---------------------------------
> 2 files changed, 29 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index c66d37d65241..8a3fbc41b51f 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -13,7 +13,7 @@ static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
> {
> u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
>
> - trace_cxl_port_aer_correctable_error(&pdev->dev, status);
> + trace_cxl_aer_correctable_error(&pdev->dev, status, 0);
> }
>
> static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
> @@ -28,8 +28,8 @@ static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
> else
> fe = status;
>
> - trace_cxl_port_aer_uncorrectable_error(&pdev->dev, status, fe,
> - ras_cap.header_log);
> + trace_cxl_aer_uncorrectable_error(&pdev->dev, status, fe,
> + ras_cap.header_log, 0);
> }
>
> static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
> @@ -37,7 +37,7 @@ static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
> {
> u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
>
> - trace_cxl_aer_correctable_error(cxlmd, status);
> + trace_cxl_aer_correctable_error(&cxlmd->dev, status, cxlmd->cxlds->serial);
> }
>
> static void
> @@ -45,6 +45,7 @@ cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
> struct cxl_ras_capability_regs ras_cap)
> {
> u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> u32 fe;
>
> if (hweight32(status) > 1)
> @@ -53,8 +54,9 @@ cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
> else
> fe = status;
>
> - trace_cxl_aer_uncorrectable_error(cxlmd, status, fe,
> - ras_cap.header_log);
> + trace_cxl_aer_uncorrectable_error(&cxlmd->dev, status, fe,
> + ras_cap.header_log,
> + cxlds->serial);
> }
>
> static int match_memdev_by_parent(struct device *dev, const void *uport)
> @@ -126,8 +128,8 @@ void cxl_ras_exit(void)
> cancel_work_sync(&cxl_cper_prot_err_work);
> }
>
> -static void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
> -static bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
> +static void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base);
> +static bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base);
>
> #ifdef CONFIG_CXL_RCH_RAS
> static void cxl_dport_map_rch_aer(struct cxl_dport *dport)
> @@ -237,9 +239,9 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>
> pci_print_aer(pdev, severity, &aer_regs);
> if (severity == AER_CORRECTABLE)
> - cxl_handle_cor_ras(&cxlds->cxlmd->dev, dport->regs.ras);
> + cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlds->serial, dport->regs.ras);
> else
> - cxl_handle_ras(&cxlds->cxlmd->dev, dport->regs.ras);
> + cxl_handle_ras(&cxlds->cxlmd->dev, cxlds->serial, dport->regs.ras);
> }
> #else
> static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
> @@ -281,7 +283,7 @@ void cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device *host)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_dport_init_ras_reporting, "CXL");
>
> -static void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> +static void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> {
> void __iomem *addr;
> u32 status;
> @@ -295,7 +297,7 @@ static void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> status = readl(addr);
> if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
> writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
> - trace_cxl_aer_correctable_error(to_cxl_memdev(dev), status);
> + trace_cxl_aer_correctable_error(dev, status, serial);
> }
> }
>
> @@ -320,7 +322,7 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
> * Log the state of the RAS status registers and prepare them to log the
> * next error status. Return 1 if reset needed.
> */
> -static bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> +static bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> {
> u32 hl[CXL_HEADERLOG_SIZE_U32];
> void __iomem *addr;
> @@ -349,7 +351,7 @@ static bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> }
>
> header_log_copy(ras_base, hl);
> - trace_cxl_aer_uncorrectable_error(to_cxl_memdev(dev), status, fe, hl);
> + trace_cxl_aer_uncorrectable_error(dev, status, fe, hl, serial);
> writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);
>
> return true;
> @@ -371,7 +373,7 @@ void cxl_cor_error_detected(struct pci_dev *pdev)
> if (cxlds->rcd)
> cxl_handle_rdport_errors(cxlds);
>
> - cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlds->regs.ras);
> + cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlds->serial, cxlds->regs.ras);
> }
> }
> EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, "CXL");
> @@ -400,7 +402,7 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> * chance the situation is recoverable dump the status of the RAS
> * capability registers and bounce the active state of the memdev.
> */
> - ue = cxl_handle_ras(&cxlds->cxlmd->dev, cxlds->regs.ras);
> + ue = cxl_handle_ras(&cxlds->cxlmd->dev, cxlds->serial, cxlds->regs.ras);
> }
>
>
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index a53ec4798b12..60b49beb5e3f 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -48,40 +48,13 @@
> { CXL_RAS_UC_IDE_RX_ERR, "IDE Rx Error" } \
> )
>
> -TRACE_EVENT(cxl_port_aer_uncorrectable_error,
> - TP_PROTO(struct device *dev, u32 status, u32 fe, u32 *hl),
> - TP_ARGS(dev, status, fe, hl),
> - TP_STRUCT__entry(
> - __string(device, dev_name(dev))
> - __string(host, dev_name(dev->parent))
> - __field(u32, status)
> - __field(u32, first_error)
> - __array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
> - ),
> - TP_fast_assign(
> - __assign_str(device);
> - __assign_str(host);
> - __entry->status = status;
> - __entry->first_error = fe;
> - /*
> - * Embed the 512B headerlog data for user app retrieval and
> - * parsing, but no need to print this in the trace buffer.
> - */
> - memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
> - ),
> - TP_printk("device=%s host=%s status: '%s' first_error: '%s'",
> - __get_str(device), __get_str(host),
> - show_uc_errs(__entry->status),
> - show_uc_errs(__entry->first_error)
> - )
> -);
> -
> TRACE_EVENT(cxl_aer_uncorrectable_error,
> - TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32 *hl),
> - TP_ARGS(cxlmd, status, fe, hl),
> + TP_PROTO(const struct device *cxlmd, u32 status, u32 fe, u32 *hl,
> + u64 serial),
> + TP_ARGS(cxlmd, status, fe, hl, serial),
> TP_STRUCT__entry(
> - __string(memdev, dev_name(&cxlmd->dev))
> - __string(host, dev_name(cxlmd->dev.parent))
> + __string(memdev, dev_name(cxlmd))
> + __string(host, dev_name(cxlmd->parent))
> __field(u64, serial)
> __field(u32, status)
> __field(u32, first_error)
> @@ -90,7 +63,7 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> TP_fast_assign(
> __assign_str(memdev);
> __assign_str(host);
> - __entry->serial = cxlmd->cxlds->serial;
> + __entry->serial = serial;
> __entry->status = status;
> __entry->first_error = fe;
> /*
> @@ -124,38 +97,19 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> { CXL_RAS_CE_PHYS_LAYER_ERR, "Received Error From Physical Layer" } \
> )
>
> -TRACE_EVENT(cxl_port_aer_correctable_error,
> - TP_PROTO(struct device *dev, u32 status),
> - TP_ARGS(dev, status),
> - TP_STRUCT__entry(
> - __string(device, dev_name(dev))
> - __string(host, dev_name(dev->parent))
> - __field(u32, status)
> - ),
> - TP_fast_assign(
> - __assign_str(device);
> - __assign_str(host);
> - __entry->status = status;
> - ),
> - TP_printk("device=%s host=%s status='%s'",
> - __get_str(device), __get_str(host),
> - show_ce_errs(__entry->status)
> - )
> -);
> -
> TRACE_EVENT(cxl_aer_correctable_error,
> - TP_PROTO(const struct cxl_memdev *cxlmd, u32 status),
> - TP_ARGS(cxlmd, status),
> + TP_PROTO(const struct device *cxlmd, u32 status, u64 serial),
> + TP_ARGS(cxlmd, status, serial),
> TP_STRUCT__entry(
> - __string(memdev, dev_name(&cxlmd->dev))
> - __string(host, dev_name(cxlmd->dev.parent))
> + __string(memdev, dev_name(cxlmd))
> + __string(host, dev_name(cxlmd->parent))
> __field(u64, serial)
> __field(u32, status)
> ),
> TP_fast_assign(
> __assign_str(memdev);
> __assign_str(host);
> - __entry->serial = cxlmd->cxlds->serial;
> + __entry->serial = serial;
> __entry->status = status;
> ),
> TP_printk("memdev=%s host=%s serial=%lld: status: '%s'",
Powered by blists - more mailing lists