[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <074f5f77-7bef-4857-97fe-b68ee9b0afaf@linux.intel.com>
Date: Tue, 22 Jul 2025 12:24:39 -0700
From: Marc Herbert <marc.herbert@...ux.intel.com>
To: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-cxl@...r.kernel.org
Cc: Ard Biesheuvel <ardb@...nel.org>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Yazen Ghannam <yazen.ghannam@....com>, Terry Bowman <terry.bowman@....com>,
Dave Jiang <dave.jiang@...el.com>, tony.luck@...el.com,
Gregory Price <gourry@...rry.net>
Subject: "invalid agent type: 1" in acpi/ghes, cper: Recognize and cache CXL
Protocol errors
Hi Smita,
The code below triggers the error "invalid agent type: 1" in Intel
validation (internal issue 15018133056)
It's not clear to anyone we asked why you did not include RCH_DP in
the `switch (prot_err->agent_type)` in cxl_cper_post_prot_err() below.
I can see how RCH_DP is special in cxl_cper_PRINT_prot_err() and I can
even understand (despite my near-zero CPER knowledge) some of the
special cases there. But in cxl_cper_post_prot_err() here, it's not
clear why RCH_DP would be rejected. Could this be an oversight? If not,
a comment with a short explanation would not hurt.
Marc
PS: the newer cxl_cper_post_prot_err() code is longer and does
something with `wd`. That's irrelevant for this test case since the
function errors and returns earlier anyway.
On 2025-01-23 00:44, Smita Koralahalli wrote:
> Add support in GHES to detect and process CXL CPER Protocol errors, as
> defined in UEFI v2.10, section N.2.13.
>
> Define struct cxl_cper_prot_err_work_data to cache CXL protocol error
> information, including RAS capabilities and severity, for further
> handling.
>
> These cached CXL CPER records will later be processed by workqueues
> within the CXL subsystem.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Reviewed-by: Dave Jiang <dave.jiang@...el.com>
> Reviewed-by: Ira Weiny <ira.weiny@...el.com>
> ---
> drivers/acpi/apei/ghes.c | 54 ++++++++++++++++++++++++++++++++++++++++
> include/cxl/event.h | 6 +++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b72772494655..4d725d988c43 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -674,6 +674,56 @@ static void ghes_defer_non_standard_event(struct acpi_hest_generic_data *gdata,
> schedule_work(&entry->work);
> }
>
> +static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> + int severity)
> +{
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
> + 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;
> + }
> +
> + if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) {
> + pr_err_ratelimited("CXL CPER invalid protocol error log\n");
> + return;
> + }
> +
> + 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;
> + }
> +
> + if (!(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER))
> + pr_warn(FW_WARN "CXL CPER no device serial number\n");
> +
> + switch (prot_err->agent_type) {
> + case RCD:
> + case DEVICE:
> + case LD:
> + case FMLD:
> + case RP:
> + case DSP:
> + case USP:
> + memcpy(&wd.prot_err, prot_err, sizeof(wd.prot_err));
> +
> + dvsec_start = (u8 *)(prot_err + 1);
> + cap_start = dvsec_start + prot_err->dvsec_len;
> +
> + memcpy(&wd.ras_cap, cap_start, sizeof(wd.ras_cap));
> + wd.severity = cper_severity_to_aer(severity);
> + break;
> + default:
> + pr_err_ratelimited("CXL CPER invalid agent type: %d\n",
> + prot_err->agent_type);
> + return;
> + }
> +#endif
> +}
> +
> /* Room for 8 entries for each of the 4 event log queues */
> #define CXL_CPER_FIFO_DEPTH 32
> DEFINE_KFIFO(cxl_cper_fifo, struct cxl_cper_work_data, CXL_CPER_FIFO_DEPTH);
> @@ -777,6 +827,10 @@ static bool ghes_do_proc(struct ghes *ghes,
> }
> else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
> queued = ghes_handle_arm_hw_error(gdata, sev, sync);
> + } else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
> + struct cxl_cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
> +
> + cxl_cper_post_prot_err(prot_err, gdata->error_severity);
> } else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
> struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
>
> diff --git a/include/cxl/event.h b/include/cxl/event.h
> index 66d85fc87701..ee1c3dec62fa 100644
> --- a/include/cxl/event.h
> +++ b/include/cxl/event.h
> @@ -232,6 +232,12 @@ struct cxl_ras_capability_regs {
> u32 header_log[16];
> };
>
> +struct cxl_cper_prot_err_work_data {
> + struct cxl_cper_sec_prot_err prot_err;
> + struct cxl_ras_capability_regs ras_cap;
> + int severity;
> +};
> +
> #ifdef CONFIG_ACPI_APEI_GHES
> int cxl_cper_register_work(struct work_struct *work);
> int cxl_cper_unregister_work(struct work_struct *work);
Powered by blists - more mailing lists