| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <dfc6cedd-c125-4201-a98b-cbaa84a4f370@amd.com>
Date: Mon, 28 Jul 2025 09:25:55 -0700
From: "Koralahalli Channabasappa, Smita"
<Smita.KoralahalliChannabasappa@....com>
To: Marc Herbert <marc.herbert@...ux.intel.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: Re: "invalid agent type: 1" in acpi/ghes, cper: Recognize and cache
CXL Protocol errors
Hi Marc,
On 7/22/2025 12:24 PM, Marc Herbert wrote:
> 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.
You're right. RCH_DP was excluded because it doesn’t report a valid SBDF
in the CPER record. Instead, it provides only the RCRB base address.
I haven't thoroughly investigated whether SBDF can be reliably derived
from the RCRB base. There might be a platform-specific mechanism for
that, but at the time, it seemed non-trivial to implement. Introducing
additional infrastructure solely to support RCH_DP felt like it was
adding more complexity.
I agree that a brief comment explaining this rationale would help. I'm
okay if you plan to include a fixup for this along with the one for the
device serial number.
Thanks
Smita
>
>
> 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