Re: [PATCH 4/6 v6] acpi/ghes: Add helper for CXL protocol errors checks
From: Fabio M. De Francesco <hidden>
Date: 2025-11-04 17:41:19
Also in:
linux-acpi, linux-cxl, linux-pci, lkml
On Tuesday, October 28, 2025 3:54:15 PM Central European Standard Time Jonathan Cameron wrote:
On Thu, 23 Oct 2025 14:25:39 +0200 "Fabio M. De Francesco" [off-list ref] wrote:quoted
Move the CPER CXL protocol errors validity out ofvalidity checkquoted
cxl_cper_post_prot_err() to cxl_cper_sec_prot_err_valid() and limit theto new cxl_cper_sec_prot_err_valid() as otherwise it sounds like it already exists.quoted
serial number check only to CXL agents that are CXL devices (UEFI v2.10, Appendix N.2.13).Perhaps a little more here on why. I assume because you are going to have a second user for it, but good to say that. Also serves to justify the export.
Hi Jonathan, All the corrections you made will be applied to the next version.
quoted
Signed-off-by: Fabio M. De Francesco <redacted> --- drivers/acpi/apei/ghes.c | 32 ++++++++++++++++++++++---------- include/cxl/event.h | 10 ++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-)diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index d6fe5f020e96..e69ae864f43d 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c@@ -706,30 +706,42 @@ static DEFINE_KFIFO(cxl_cper_prot_err_fifo, struct cxl_cper_prot_err_work_data, static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock); struct work_struct *cxl_cper_prot_err_work; -static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err, - int severity) +int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)Useful to return an error number? Or would a bool be better given it is either valid or not?
I prefer to return more information when reasonable and leave the callers free to use or ignore the specific error number. Fabio
Otherwise looks good to me, Jonathanquoted
{ - struct cxl_cper_prot_err_work_data wd; - u8 *dvsec_start, *cap_start; - if (!(prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS)) { pr_err_ratelimited("CXL CPER invalid agent type\n"); - return; + return -EINVAL; } if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) { pr_err_ratelimited("CXL CPER invalid protocol error log\n"); - return; + return -EINVAL; } if (prot_err->err_len != sizeof(struct cxl_ras_capability_regs)) { pr_err_ratelimited("CXL CPER invalid RAS Cap size (%u)\n", prot_err->err_len); - return; + return -EINVAL; } - if (!(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER)) - pr_warn(FW_WARN "CXL CPER no device serial number\n"); + if ((prot_err->agent_type == RCD || prot_err->agent_type == DEVICE || + prot_err->agent_type == LD || prot_err->agent_type == FMLD) && + !(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER)) + pr_warn_ratelimited(FW_WARN + "CXL CPER no device serial number\n"); + + return 0; +} +EXPORT_SYMBOL_GPL(cxl_cper_sec_prot_err_valid); + +static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err, + int severity) +{ + struct cxl_cper_prot_err_work_data wd; + u8 *dvsec_start, *cap_start; + + if (cxl_cper_sec_prot_err_valid(prot_err)) + return; guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);