| 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: <09e0d961-e19f-30d6-5306-1b35609b7d79@amd.com>
Date: Thu, 23 May 2024 14:21:40 -0700
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To: Alison Schofield <alison.schofield@...el.com>
Cc: linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-cxl@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>,
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>, Bowman Terry <terry.bowman@....com>
Subject: Re: [PATCH 2/4] acpi/ghes, efi/cper: Recognize and process CXL
Protocol Errors.
Hi Alison,
On 5/22/2024 5:03 PM, Alison Schofield wrote:
> On Wed, May 22, 2024 at 03:08:37PM +0000, Smita Koralahalli wrote:
>> UEFI v2.10 section N.2.13 defines a CPER record for CXL Protocol errors.
>>
>> Add GHES support to detect CXL CPER Protocol Error Record and Cache Error
>> Severity, Device ID, Device Serial number and CXL RAS capability struct in
>> struct cxl_cper_prot_err. Include this struct as a member of struct
>> cxl_cper_work_data.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
>> ---
>> drivers/acpi/apei/ghes.c | 10 +++++
>> drivers/firmware/efi/cper_cxl.c | 66 +++++++++++++++++++++++++++++++++
>> include/linux/cxl-event.h | 26 +++++++++++++
>> 3 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 623cc0cb4a65..1a58032770ee 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -717,6 +717,14 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>> schedule_work(cxl_cper_work);
>> }
>>
>> +static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>> +{
>> + struct cxl_cper_work_data wd;
>> +
>> + if (cxl_cper_handle_prot_err_info(gdata, &wd.p_err))
>> + return;
>> +}
>> +
>> int cxl_cper_register_work(struct work_struct *work)
>> {
>> if (cxl_cper_work)
>> @@ -791,6 +799,8 @@ static bool ghes_do_proc(struct ghes *ghes,
>> struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
>>
>> cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec);
>> + } else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
>> + cxl_cper_handle_prot_err(gdata);
>> } else {
>> void *err = acpi_hest_get_payload(gdata);
>>
>> diff --git a/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
>> index 4fd8d783993e..03b9839f3b73 100644
>> --- a/drivers/firmware/efi/cper_cxl.c
>> +++ b/drivers/firmware/efi/cper_cxl.c
>> @@ -8,6 +8,7 @@
>> */
>>
>> #include <linux/cper.h>
>> +#include <acpi/ghes.h>
>> #include "cper_cxl.h"
>>
>> #define PROT_ERR_VALID_AGENT_TYPE BIT_ULL(0)
>> @@ -44,6 +45,17 @@ enum {
>> USP, /* CXL Upstream Switch Port */
>> };
>>
>> +static enum cxl_aer_err_type cper_severity_cxl_aer(int cper_severity)
>> +{
>> + switch (cper_severity) {
>> + case CPER_SEV_RECOVERABLE:
>> + case CPER_SEV_FATAL:
>> + return CXL_AER_UNCORRECTABLE;
>> + default:
>> + return CXL_AER_CORRECTABLE;
>> + }
>> +}
>> +
>> void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_err)
>> {
>> if (prot_err->valid_bits & PROT_ERR_VALID_AGENT_TYPE)
>> @@ -176,3 +188,57 @@ void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_e
>> sizeof(cxl_ras->header_log), 0);
>> }
>> }
>> +
>> +int cxl_cper_handle_prot_err_info(struct acpi_hest_generic_data *gdata,
>> + struct cxl_cper_prot_err *p_err)
>> +{
>> + struct cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
>> + u8 *dvsec_start, *cap_start;
>> +
>> + if (!(prot_err->valid_bits & PROT_ERR_VALID_DEVICE_ID)) {
>> + pr_err(FW_WARN "No Device ID\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * The device ID or agent address is required for CXL RCD, CXL
>> + * SLD, CXL LD, CXL Fabric Manager Managed LD, CXL Root Port,
>> + * CXL Downstream Switch Port and CXL Upstream Switch Port.
>> + */
>> + if (prot_err->agent_type <= 0x7 && prot_err->agent_type != RCH_DP) {
>
> For this check against agent_type, and the similar one below, would a boolean
> array indexed by the agent type work? That would avoid the <= 0x7 and > 0x4
> below. It seems one array would suffice for this case, but naming it isn't obvious
> to me. Maybe it'll be to you.
>
> Something similar to what is done for prot_err_agent_type_strs[]
>
> static const bool agent_requires_id_address_serial[] = {
> true, /* RDC */
> false, /* RCH_DP */
> .
> .
> .
> };
>
>
Noted. Will implement it this way!
Thanks
Smita
>> + p_err->segment = prot_err->agent_addr.segment;
>> + p_err->bus = prot_err->agent_addr.bus;
>> + p_err->device = prot_err->agent_addr.device;
>> + p_err->function = prot_err->agent_addr.function;
>> + } else {
>> + pr_err(FW_WARN "Invalid agent type\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) {
>> + pr_err(FW_WARN "Invalid Protocol Error log\n");
>> + return -EINVAL;
>> + }
>> +
>> + dvsec_start = (u8 *)(prot_err + 1);
>> + cap_start = dvsec_start + prot_err->dvsec_len;
>> + p_err->cxl_ras = *(struct cxl_ras_capability_regs *)cap_start;
>> +
>> + /*
>> + * Set device serial number unconditionally.
>> + *
>> + * Print a warning message if it is not valid. The device serial
>> + * number is required for CXL RCD, CXL SLD, CXL LD and CXL Fabric
>> + * Manager Managed LD.
>> + */
>> + if (!(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER) ||
>> + prot_err->agent_type > 0x4 || prot_err->agent_type == RCH_DP)
>
> then this also can be replaced with
> agent_requires_id_address_serial[prot_err->agent_type]
>
>
> -- Alison
>
>
>> + pr_warn(FW_WARN "No Device Serial number\n");
>> +
>> + p_err->lower_dw = prot_err->dev_serial_num.lower_dw;
>> + p_err->upper_dw = prot_err->dev_serial_num.upper_dw;
>> +
>> + p_err->severity = cper_severity_cxl_aer(gdata->error_severity);
>> +
>> + return 0;
>> +}
>
> snip
>
Powered by blists - more mailing lists