This patchset adds trace event support for FW-First Protocol Errors.
This series depends on:
https://lore.kernel.org/linux-cxl/[email protected]
Link to v1:
https://lore.kernel.org/all/[email protected]
Smita Koralahalli (4):
acpi/ghes, cxl: Create a common CXL struct to handle different CXL
CPER records
efi/cper, cxl: Make definitions and structures global
acpi/ghes, efi/cper: Recognize and process CXL Protocol Errors.
acpi/ghes, cxl/pci: Trace FW-First CXL Protocol Errors
drivers/acpi/apei/ghes.c | 22 ++++++++-
drivers/cxl/core/pci.c | 29 ++++++++++++
drivers/cxl/cxlpci.h | 3 ++
drivers/cxl/pci.c | 13 ++++--
drivers/firmware/efi/cper_cxl.c | 79 ++++++++++++++++++++++++++++-----
drivers/firmware/efi/cper_cxl.h | 7 +--
include/linux/cper.h | 4 ++
include/linux/cxl-event.h | 31 ++++++++++++-
8 files changed, 166 insertions(+), 22 deletions(-)
--
2.17.1
In preparation to add tracepoint support, move protocol error UUID
definition to a common location and make CXL RAS capability struct
global for use across different modules.
Signed-off-by: Smita Koralahalli <[email protected]>
---
v2:
No change.
---
drivers/firmware/efi/cper_cxl.c | 11 -----------
drivers/firmware/efi/cper_cxl.h | 7 ++-----
include/linux/cper.h | 4 ++++
include/linux/cxl-event.h | 11 +++++++++++
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
index a55771b99a97..4fd8d783993e 100644
--- a/drivers/firmware/efi/cper_cxl.c
+++ b/drivers/firmware/efi/cper_cxl.c
@@ -18,17 +18,6 @@
#define PROT_ERR_VALID_DVSEC BIT_ULL(5)
#define PROT_ERR_VALID_ERROR_LOG BIT_ULL(6)
-/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
-struct cxl_ras_capability_regs {
- u32 uncor_status;
- u32 uncor_mask;
- u32 uncor_severity;
- u32 cor_status;
- u32 cor_mask;
- u32 cap_control;
- u32 header_log[16];
-};
-
static const char * const prot_err_agent_type_strs[] = {
"Restricted CXL Device",
"Restricted CXL Host Downstream Port",
diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
index 86bfcf7909ec..6f8c00495708 100644
--- a/drivers/firmware/efi/cper_cxl.h
+++ b/drivers/firmware/efi/cper_cxl.h
@@ -7,14 +7,11 @@
* Author: Smita Koralahalli <[email protected]>
*/
+#include <linux/cxl-event.h>
+
#ifndef LINUX_CPER_CXL_H
#define LINUX_CPER_CXL_H
-/* CXL Protocol Error Section */
-#define CPER_SEC_CXL_PROT_ERR \
- GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
- 0x4B, 0x77, 0x10, 0x48)
-
#pragma pack(1)
/* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */
diff --git a/include/linux/cper.h b/include/linux/cper.h
index c1a7dc325121..2cbf0a93785a 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -89,6 +89,10 @@ enum {
#define CPER_NOTIFY_DMAR \
GUID_INIT(0x667DD791, 0xC6B3, 0x4c27, 0x8A, 0x6B, 0x0F, 0x8E, \
0x72, 0x2D, 0xEB, 0x41)
+/* CXL Protocol Error Section */
+#define CPER_SEC_CXL_PROT_ERR \
+ GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
+ 0x4B, 0x77, 0x10, 0x48)
/*
* Flags bits definitions for flags in struct cper_record_header
diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
index 6ce839c59749..3a41dd5723e8 100644
--- a/include/linux/cxl-event.h
+++ b/include/linux/cxl-event.h
@@ -141,6 +141,17 @@ struct cxl_cper_event_rec {
union cxl_event event;
} __packed;
+/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
+struct cxl_ras_capability_regs {
+ u32 uncor_status;
+ u32 uncor_mask;
+ u32 uncor_severity;
+ u32 cor_status;
+ u32 cor_mask;
+ u32 cap_control;
+ u32 header_log[16];
+};
+
struct cxl_cper_event_info {
struct cxl_cper_event_rec rec;
};
--
2.17.1
UEFI v2.10 section N.2.13 defines a CPER record for CXL Protocol errors.
Add GHES support to detect CXL CPER Protocol record and cache error
severity, device_id, serial number and CXL RAS capability struct in
struct cxl_cper_event_info.
Signed-off-by: Smita Koralahalli <[email protected]>
---
v2:
Change to sub-struct for protocol error specific elemenets.
Set serial number unconditionally.
Copy entire cxl_ras_capability_regs struct rather than pointer.
Calculate error severity in efi/cper and change to enum.
---
drivers/acpi/apei/ghes.c | 11 ++++++
drivers/firmware/efi/cper_cxl.c | 68 +++++++++++++++++++++++++++++++++
include/linux/cxl-event.h | 13 +++++++
3 files changed, 92 insertions(+)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 60b615d361d3..1d4f3d68a0bc 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -714,6 +714,14 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
cper_callback(event_type, &info);
}
+void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
+{
+ struct cxl_cper_event_info info;
+
+ if (cxl_cper_handle_prot_err_info(gdata, &info))
+ return;
+}
+
int cxl_cper_register_callback(cxl_cper_callback callback)
{
guard(rwsem_write)(&cxl_cper_rw_sem);
@@ -768,6 +776,9 @@ 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);
}
+ else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
+ cxl_cper_handle_prot_err(gdata);
+ }
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/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
index 4fd8d783993e..9b9b8c8f1157 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,59 @@ 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_event_info *info)
+{
+ struct cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
+ struct cper_cxl_event_devid *device_id = &info->rec.hdr.device_id;
+ struct cper_cxl_event_sn *dev_serial_num = &info->rec.hdr.dev_serial_num;
+ size_t size = sizeof(*prot_err) + prot_err->dvsec_len;
+
+ if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) {
+ pr_err(FW_WARN "Not a valid protocol error log\n");
+ return -EINVAL;
+ }
+
+ if (!(prot_err->valid_bits & PROT_ERR_VALID_DEVICE_ID)) {
+ pr_err(FW_WARN "Not a valid Device ID\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Set device serial number unconditionally.
+ *
+ * Print a warning message if it is not valid. The device serial
+ * number is considered valid for CXL 1.1 device, CXL 2.0 device,
+ * CXL 2.0 Logical device, or CXL 2.0 Fabric Manager Managed
+ * Logical Device.
+ */
+ if (!(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER) ||
+ prot_err->agent_type > 0x4 || prot_err->agent_type == RCH_DP)
+ pr_warn(FW_WARN "Not a valid serial number\n");
+
+ dev_serial_num->lower_dw = prot_err->dev_serial_num.lower_dw;
+ dev_serial_num->upper_dw = prot_err->dev_serial_num.upper_dw;
+
+ /*
+ * The device ID or agent address is only valid for CXL 1.1 device,
+ * CXL 2.0 device, CXL 2.0 Logical device, CXL 2.0 Fabric Manager
+ * Managed Logical Device, CXL Root Port, CXL Downstream Switch
+ * Port, or CXL Upstream Switch Port.
+ */
+ if (prot_err->agent_type <= 0x7 && prot_err->agent_type != RCH_DP) {
+ device_id->segment_num = prot_err->agent_addr.segment;
+ device_id->bus_num = prot_err->agent_addr.bus;
+ device_id->device_num = prot_err->agent_addr.device;
+ device_id->func_num = prot_err->agent_addr.function;
+ } else {
+ pr_err(FW_WARN "Not a valid agent type\n");
+ return -EINVAL;
+ }
+
+ info->p_err.cxl_ras = *(struct cxl_ras_capability_regs *)((long)prot_err + size);
+
+ info->p_err.severity = cper_severity_cxl_aer(gdata->error_severity);
+
+ return 0;
+}
diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
index 3a41dd5723e8..08e3979de9a3 100644
--- a/include/linux/cxl-event.h
+++ b/include/linux/cxl-event.h
@@ -152,13 +152,26 @@ struct cxl_ras_capability_regs {
u32 header_log[16];
};
+enum cxl_aer_err_type {
+ CXL_AER_UNCORRECTABLE,
+ CXL_AER_CORRECTABLE,
+};
+
struct cxl_cper_event_info {
struct cxl_cper_event_rec rec;
+ struct cxl_cper_prot_err {
+ struct cxl_ras_capability_regs cxl_ras;
+ int severity;
+ } p_err;
};
typedef void (*cxl_cper_callback)(enum cxl_event_type type,
struct cxl_cper_event_info *info);
+struct acpi_hest_generic_data;
+int cxl_cper_handle_prot_err_info(struct acpi_hest_generic_data *gdata,
+ struct cxl_cper_event_info *info);
+
#ifdef CONFIG_ACPI_APEI_GHES
int cxl_cper_register_callback(cxl_cper_callback callback);
int cxl_cper_unregister_callback(cxl_cper_callback callback);
--
2.17.1
Currently defined cxl_cper_callback interface between CXL subsystem and
GHES module is just confined to handling CXL Component errors only.
Extend this callback to process CXL Protocol errors as well. Achieve
by defining a new struct cxl_cper_event_info to include cxl_cper_event_rec
and other fields of CXL protocol errors which will be defined in future
patches.
Signed-off-by: Smita Koralahalli <[email protected]>
---
v2:
cxl_cper_rec_data -> cxl_cper_event_info
data -> info
---
drivers/acpi/apei/ghes.c | 6 +++++-
drivers/cxl/pci.c | 8 ++++----
include/linux/cxl-event.h | 6 +++++-
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index aed465d2fd68..60b615d361d3 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -693,6 +693,10 @@ static cxl_cper_callback cper_callback;
static void cxl_cper_post_event(enum cxl_event_type event_type,
struct cxl_cper_event_rec *rec)
{
+ struct cxl_cper_event_info info;
+
+ info.rec = *(struct cxl_cper_event_rec *)rec;
+
if (rec->hdr.length <= sizeof(rec->hdr) ||
rec->hdr.length > sizeof(*rec)) {
pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
@@ -707,7 +711,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
guard(rwsem_read)(&cxl_cper_rw_sem);
if (cper_callback)
- cper_callback(event_type, rec);
+ cper_callback(event_type, &info);
}
int cxl_cper_register_callback(cxl_cper_callback callback)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index b14237f824cf..1ad240ead4fd 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -972,9 +972,9 @@ static struct pci_driver cxl_pci_driver = {
#define CXL_EVENT_HDR_FLAGS_REC_SEVERITY GENMASK(1, 0)
static void cxl_cper_event_call(enum cxl_event_type ev_type,
- struct cxl_cper_event_rec *rec)
+ struct cxl_cper_event_info *info)
{
- struct cper_cxl_event_devid *device_id = &rec->hdr.device_id;
+ struct cper_cxl_event_devid *device_id = &info->rec.hdr.device_id;
struct pci_dev *pdev __free(pci_dev_put) = NULL;
enum cxl_event_log_type log_type;
struct cxl_dev_state *cxlds;
@@ -996,11 +996,11 @@ static void cxl_cper_event_call(enum cxl_event_type ev_type,
return;
/* Fabricate a log type */
- hdr_flags = get_unaligned_le24(rec->event.generic.hdr.flags);
+ hdr_flags = get_unaligned_le24(info->rec.event.generic.hdr.flags);
log_type = FIELD_GET(CXL_EVENT_HDR_FLAGS_REC_SEVERITY, hdr_flags);
cxl_event_trace_record(cxlds->cxlmd, log_type, ev_type,
- &uuid_null, &rec->event);
+ &uuid_null, &info->rec.event);
}
static int __init cxl_pci_driver_init(void)
diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
index 17eadee819b6..6ce839c59749 100644
--- a/include/linux/cxl-event.h
+++ b/include/linux/cxl-event.h
@@ -141,8 +141,12 @@ struct cxl_cper_event_rec {
union cxl_event event;
} __packed;
+struct cxl_cper_event_info {
+ struct cxl_cper_event_rec rec;
+};
+
typedef void (*cxl_cper_callback)(enum cxl_event_type type,
- struct cxl_cper_event_rec *rec);
+ struct cxl_cper_event_info *info);
#ifdef CONFIG_ACPI_APEI_GHES
int cxl_cper_register_callback(cxl_cper_callback callback);
--
2.17.1
When PCIe AER is in FW-First, OS should process CXL Protocol errors from
CPER records. These CPER records obtained from GHES module, will rely on
a registered callback to be notified to the CXL subsystem in order to be
processed.
Call the existing cxl_cper_callback to notify the CXL subsystem on a
Protocol error.
The defined trace events cxl_aer_uncorrectable_error and
cxl_aer_correctable_error currently trace native CXL AER errors. Reuse
them to trace FW-First Protocol Errors.
Signed-off-by: Smita Koralahalli <[email protected]>
---
v2:
Added warning for serial number check.
Moved severity determination to previous patch.
---
drivers/acpi/apei/ghes.c | 5 +++++
drivers/cxl/core/pci.c | 29 +++++++++++++++++++++++++++++
drivers/cxl/cxlpci.h | 3 +++
drivers/cxl/pci.c | 5 +++++
include/linux/cxl-event.h | 1 +
5 files changed, 43 insertions(+)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 1d4f3d68a0bc..4318b602e797 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -716,10 +716,15 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
{
+ enum cxl_event_type event_type = CXL_CPER_EVENT_PROT_ERR;
struct cxl_cper_event_info info;
if (cxl_cper_handle_prot_err_info(gdata, &info))
return;
+
+ guard(rwsem_read)(&cxl_cper_rw_sem);
+ if (cper_callback)
+ cper_callback(event_type, &info);
}
int cxl_cper_register_callback(cxl_cper_callback callback)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 37e1652afbc7..bde8ebf5e4b3 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -836,6 +836,35 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
}
EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL);
+void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
+ struct cxl_cper_event_info *info)
+{
+ struct cper_cxl_event_sn *dev_serial_num = &info->rec.hdr.dev_serial_num;
+ u32 status, fe;
+
+ if (((u64)dev_serial_num->upper_dw << 32 |
+ dev_serial_num->lower_dw) != cxlds->serial)
+ pr_warn("The device serial number in CPER differs or isn't valid\n");
+
+ if (info->p_err.severity == CXL_AER_CORRECTABLE) {
+ status = info->p_err.cxl_ras.cor_status & ~info->p_err.cxl_ras.cor_mask;
+
+ trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
+ } else {
+ status = info->p_err.cxl_ras.uncor_status & ~info->p_err.cxl_ras.uncor_mask;
+
+ if (hweight32(status) > 1)
+ fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
+ info->p_err.cxl_ras.cap_control));
+ else
+ fe = status;
+
+ trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
+ info->p_err.cxl_ras.header_log);
+ }
+}
+EXPORT_SYMBOL_NS_GPL(cxl_prot_err_trace_record, CXL);
+
static void cxl_handle_rdport_cor_ras(struct cxl_dev_state *cxlds,
struct cxl_dport *dport)
{
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 0fa4799ea316..216003d4aec1 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -93,4 +93,7 @@ void read_cdat_data(struct cxl_port *port);
void cxl_cor_error_detected(struct pci_dev *pdev);
pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
pci_channel_state_t state);
+struct cxl_cper_event_info;
+void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
+ struct cxl_cper_event_info *info);
#endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 1ad240ead4fd..515983e7df10 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -995,6 +995,11 @@ static void cxl_cper_event_call(enum cxl_event_type ev_type,
if (!cxlds)
return;
+ if (ev_type == CXL_CPER_EVENT_PROT_ERR) {
+ cxl_prot_err_trace_record(cxlds, info);
+ return;
+ }
+
/* Fabricate a log type */
hdr_flags = get_unaligned_le24(info->rec.event.generic.hdr.flags);
log_type = FIELD_GET(CXL_EVENT_HDR_FLAGS_REC_SEVERITY, hdr_flags);
diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
index 08e3979de9a3..96cc88aa04f3 100644
--- a/include/linux/cxl-event.h
+++ b/include/linux/cxl-event.h
@@ -113,6 +113,7 @@ enum cxl_event_type {
CXL_CPER_EVENT_GEN_MEDIA,
CXL_CPER_EVENT_DRAM,
CXL_CPER_EVENT_MEM_MODULE,
+ CXL_CPER_EVENT_PROT_ERR,
};
#define CPER_CXL_DEVICE_ID_VALID BIT(0)
--
2.17.1
On Tue, 9 Jan 2024 03:47:52 +0000
Smita Koralahalli <[email protected]> wrote:
> Currently defined cxl_cper_callback interface between CXL subsystem and
> GHES module is just confined to handling CXL Component errors only.
>
> Extend this callback to process CXL Protocol errors as well. Achieve
> by defining a new struct cxl_cper_event_info to include cxl_cper_event_rec
> and other fields of CXL protocol errors which will be defined in future
> patches.
>
> Signed-off-by: Smita Koralahalli <[email protected]>
Hi Smita,
I guess this will get effected by the mess around the reporting that
Ira is fixing but in meantime some comments on the current code.
> ---
> v2:
> cxl_cper_rec_data -> cxl_cper_event_info
> data -> info
> ---
> drivers/acpi/apei/ghes.c | 6 +++++-
> drivers/cxl/pci.c | 8 ++++----
> include/linux/cxl-event.h | 6 +++++-
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index aed465d2fd68..60b615d361d3 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -693,6 +693,10 @@ static cxl_cper_callback cper_callback;
> static void cxl_cper_post_event(enum cxl_event_type event_type,
> struct cxl_cper_event_rec *rec)
> {
> + struct cxl_cper_event_info info;
> +
> + info.rec = *(struct cxl_cper_event_rec *)rec;
Why cast?
> +
> if (rec->hdr.length <= sizeof(rec->hdr) ||
> rec->hdr.length > sizeof(*rec)) {
> pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
> @@ -707,7 +711,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>
> guard(rwsem_read)(&cxl_cper_rw_sem);
> if (cper_callback)
> - cper_callback(event_type, rec);
> + cper_callback(event_type, &info);
> }
>
> int cxl_cper_register_callback(cxl_cper_callback callback)
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b14237f824cf..1ad240ead4fd 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -972,9 +972,9 @@ static struct pci_driver cxl_pci_driver = {
>
> #define CXL_EVENT_HDR_FLAGS_REC_SEVERITY GENMASK(1, 0)
> static void cxl_cper_event_call(enum cxl_event_type ev_type,
> - struct cxl_cper_event_rec *rec)
> + struct cxl_cper_event_info *info)
> {
> - struct cper_cxl_event_devid *device_id = &rec->hdr.device_id;
> + struct cper_cxl_event_devid *device_id = &info->rec.hdr.device_id;
> struct pci_dev *pdev __free(pci_dev_put) = NULL;
> enum cxl_event_log_type log_type;
> struct cxl_dev_state *cxlds;
> @@ -996,11 +996,11 @@ static void cxl_cper_event_call(enum cxl_event_type ev_type,
> return;
>
> /* Fabricate a log type */
> - hdr_flags = get_unaligned_le24(rec->event.generic.hdr.flags);
> + hdr_flags = get_unaligned_le24(info->rec.event.generic.hdr.flags);
> log_type = FIELD_GET(CXL_EVENT_HDR_FLAGS_REC_SEVERITY, hdr_flags);
>
> cxl_event_trace_record(cxlds->cxlmd, log_type, ev_type,
> - &uuid_null, &rec->event);
> + &uuid_null, &info->rec.event);
> }
>
> static int __init cxl_pci_driver_init(void)
> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
> index 17eadee819b6..6ce839c59749 100644
> --- a/include/linux/cxl-event.h
> +++ b/include/linux/cxl-event.h
> @@ -141,8 +141,12 @@ struct cxl_cper_event_rec {
> union cxl_event event;
> } __packed;
>
> +struct cxl_cper_event_info {
> + struct cxl_cper_event_rec rec;
Only parts of this will be relevant to the protocol errors.
Maybe worth doing a union with the first part of rec in both
structures but not the union cxl_event in the protocol error.
Keep it all anonymous to avoid yet another structure in the
reads/and writes though.
> +};
> +
> typedef void (*cxl_cper_callback)(enum cxl_event_type type,
> - struct cxl_cper_event_rec *rec);
> + struct cxl_cper_event_info *info);
>
> #ifdef CONFIG_ACPI_APEI_GHES
> int cxl_cper_register_callback(cxl_cper_callback callback);
On Tue, 9 Jan 2024 03:47:53 +0000
Smita Koralahalli <[email protected]> wrote:
> In preparation to add tracepoint support, move protocol error UUID
> definition to a common location and make CXL RAS capability struct
> global for use across different modules.
>
> Signed-off-by: Smita Koralahalli <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> v2:
> No change.
> ---
> drivers/firmware/efi/cper_cxl.c | 11 -----------
> drivers/firmware/efi/cper_cxl.h | 7 ++-----
> include/linux/cper.h | 4 ++++
> include/linux/cxl-event.h | 11 +++++++++++
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
> index a55771b99a97..4fd8d783993e 100644
> --- a/drivers/firmware/efi/cper_cxl.c
> +++ b/drivers/firmware/efi/cper_cxl.c
> @@ -18,17 +18,6 @@
> #define PROT_ERR_VALID_DVSEC BIT_ULL(5)
> #define PROT_ERR_VALID_ERROR_LOG BIT_ULL(6)
>
> -/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
> -struct cxl_ras_capability_regs {
> - u32 uncor_status;
> - u32 uncor_mask;
> - u32 uncor_severity;
> - u32 cor_status;
> - u32 cor_mask;
> - u32 cap_control;
> - u32 header_log[16];
> -};
> -
> static const char * const prot_err_agent_type_strs[] = {
> "Restricted CXL Device",
> "Restricted CXL Host Downstream Port",
> diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
> index 86bfcf7909ec..6f8c00495708 100644
> --- a/drivers/firmware/efi/cper_cxl.h
> +++ b/drivers/firmware/efi/cper_cxl.h
> @@ -7,14 +7,11 @@
> * Author: Smita Koralahalli <[email protected]>
> */
>
> +#include <linux/cxl-event.h>
> +
> #ifndef LINUX_CPER_CXL_H
> #define LINUX_CPER_CXL_H
>
> -/* CXL Protocol Error Section */
> -#define CPER_SEC_CXL_PROT_ERR \
> - GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
> - 0x4B, 0x77, 0x10, 0x48)
> -
> #pragma pack(1)
>
> /* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index c1a7dc325121..2cbf0a93785a 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -89,6 +89,10 @@ enum {
> #define CPER_NOTIFY_DMAR \
> GUID_INIT(0x667DD791, 0xC6B3, 0x4c27, 0x8A, 0x6B, 0x0F, 0x8E, \
> 0x72, 0x2D, 0xEB, 0x41)
> +/* CXL Protocol Error Section */
> +#define CPER_SEC_CXL_PROT_ERR \
> + GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
> + 0x4B, 0x77, 0x10, 0x48)
>
> /*
> * Flags bits definitions for flags in struct cper_record_header
> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
> index 6ce839c59749..3a41dd5723e8 100644
> --- a/include/linux/cxl-event.h
> +++ b/include/linux/cxl-event.h
> @@ -141,6 +141,17 @@ struct cxl_cper_event_rec {
> union cxl_event event;
> } __packed;
>
> +/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
> +struct cxl_ras_capability_regs {
> + u32 uncor_status;
> + u32 uncor_mask;
> + u32 uncor_severity;
> + u32 cor_status;
> + u32 cor_mask;
> + u32 cap_control;
> + u32 header_log[16];
> +};
> +
> struct cxl_cper_event_info {
> struct cxl_cper_event_rec rec;
> };
On Tue, 9 Jan 2024 03:47:54 +0000
Smita Koralahalli <[email protected]> wrote:
> UEFI v2.10 section N.2.13 defines a CPER record for CXL Protocol errors.
>
> Add GHES support to detect CXL CPER Protocol record and cache error
> severity, device_id, serial number and CXL RAS capability struct in
> struct cxl_cper_event_info.
>
> Signed-off-by: Smita Koralahalli <[email protected]>
> ---
> v2:
> Change to sub-struct for protocol error specific elemenets.
> Set serial number unconditionally.
> Copy entire cxl_ras_capability_regs struct rather than pointer.
> Calculate error severity in efi/cper and change to enum.
> ---
> drivers/acpi/apei/ghes.c | 11 ++++++
> drivers/firmware/efi/cper_cxl.c | 68 +++++++++++++++++++++++++++++++++
> include/linux/cxl-event.h | 13 +++++++
> 3 files changed, 92 insertions(+)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 60b615d361d3..1d4f3d68a0bc 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -714,6 +714,14 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
> cper_callback(event_type, &info);
> }
>
> +void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
> +{
> + struct cxl_cper_event_info info;
> +
> + if (cxl_cper_handle_prot_err_info(gdata, &info))
> + return;
> +}
> +
> int cxl_cper_register_callback(cxl_cper_callback callback)
> {
> guard(rwsem_write)(&cxl_cper_rw_sem);
> @@ -768,6 +776,9 @@ 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);
> }
> + else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
> + cxl_cper_handle_prot_err(gdata);
> + }
> 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/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
> index 4fd8d783993e..9b9b8c8f1157 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,59 @@ 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_event_info *info)
> +{
> + struct cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
> + struct cper_cxl_event_devid *device_id = &info->rec.hdr.device_id;
> + struct cper_cxl_event_sn *dev_serial_num = &info->rec.hdr.dev_serial_num;
> + size_t size = sizeof(*prot_err) + prot_err->dvsec_len;
Not obvious what this is size of. I'd rename it to reflect that's only
the distance to the end of the dvsec copy.
Or just compute the pointer below directly by putting this maths inline.
> +
> + if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) {
> + pr_err(FW_WARN "Not a valid protocol error log\n");
> + return -EINVAL;
> + }
> +
> + if (!(prot_err->valid_bits & PROT_ERR_VALID_DEVICE_ID)) {
> + pr_err(FW_WARN "Not a valid Device ID\n");
"No device ID\n"
is more accurate description.
I'd move this down to next to where we check the data is valid.
So keep each validity check next to where it matters rather than
a bunch of checks up here. (mostly because I started writing you
didn't check it was valid down there before remembering this
earlier code :)
> + return -EINVAL;
> + }
> +
> + /*
> + * Set device serial number unconditionally.
> + *
> + * Print a warning message if it is not valid. The device serial
> + * number is considered valid for CXL 1.1 device, CXL 2.0 device,
is required for
perhaps? These all got renamed in the CXL spec. We should use that naming
because it deliberately avoids limiting to particular spec versions.
CXL RCD, CXL SLD, CXL LD,
> + * CXL 2.0 Logical device, or CXL 2.0 Fabric Manager Managed
> + * Logical Device.
Not sure what this is now called.. :(
> + */
> + if (!(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER) ||
> + prot_err->agent_type > 0x4 || prot_err->agent_type == RCH_DP)
> + pr_warn(FW_WARN "Not a valid serial number\n");
> +
> + dev_serial_num->lower_dw = prot_err->dev_serial_num.lower_dw;
> + dev_serial_num->upper_dw = prot_err->dev_serial_num.upper_dw;
> +
> + /*
> + * The device ID or agent address is only valid for CXL 1.1 device,
> + * CXL 2.0 device, CXL 2.0 Logical device, CXL 2.0 Fabric Manager
> + * Managed Logical Device, CXL Root Port, CXL Downstream Switch
> + * Port, or CXL Upstream Switch Port.
> + */
> + if (prot_err->agent_type <= 0x7 && prot_err->agent_type != RCH_DP) {
> + device_id->segment_num = prot_err->agent_addr.segment;
> + device_id->bus_num = prot_err->agent_addr.bus;
> + device_id->device_num = prot_err->agent_addr.device;
> + device_id->func_num = prot_err->agent_addr.function;
> + } else {
> + pr_err(FW_WARN "Not a valid agent type\n");
> + return -EINVAL;
> + }
> +
> + info->p_err.cxl_ras = *(struct cxl_ras_capability_regs *)((long)prot_err + size);
Casting to a long isn't nice. Keep it as a pointer for this maths
a u8 * or void * would work. Particularly if you did it as something
a bit more self documenting like
u8 *dvsec_start = (u8 *)(prot_err + 1);
u8 *cap_start = dvsec_start + prot_err->dvsec_length;
info->p_err.cxl_ras = *(struct cxl_ras_capability_regs *)cap_start;
> +
> + info->p_err.severity = cper_severity_cxl_aer(gdata->error_severity);
> +
> + return 0;
> +}
On Tue, 9 Jan 2024 03:47:55 +0000
Smita Koralahalli <[email protected]> wrote:
> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
> CPER records. These CPER records obtained from GHES module, will rely on
> a registered callback to be notified to the CXL subsystem in order to be
> processed.
>
> Call the existing cxl_cper_callback to notify the CXL subsystem on a
> Protocol error.
>
> The defined trace events cxl_aer_uncorrectable_error and
> cxl_aer_correctable_error currently trace native CXL AER errors. Reuse
> them to trace FW-First Protocol Errors.
>
> Signed-off-by: Smita Koralahalli <[email protected]>
Other than the bits of this that will change due to Ira's look good to me.
Jonathan
On Tue, 9 Jan 2024 at 04:48, Smita Koralahalli
<[email protected]> wrote:
>
> In preparation to add tracepoint support, move protocol error UUID
> definition to a common location and make CXL RAS capability struct
> global for use across different modules.
>
> Signed-off-by: Smita Koralahalli <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
> ---
> v2:
> No change.
> ---
> drivers/firmware/efi/cper_cxl.c | 11 -----------
> drivers/firmware/efi/cper_cxl.h | 7 ++-----
> include/linux/cper.h | 4 ++++
> include/linux/cxl-event.h | 11 +++++++++++
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c
> index a55771b99a97..4fd8d783993e 100644
> --- a/drivers/firmware/efi/cper_cxl.c
> +++ b/drivers/firmware/efi/cper_cxl.c
> @@ -18,17 +18,6 @@
> #define PROT_ERR_VALID_DVSEC BIT_ULL(5)
> #define PROT_ERR_VALID_ERROR_LOG BIT_ULL(6)
>
> -/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
> -struct cxl_ras_capability_regs {
> - u32 uncor_status;
> - u32 uncor_mask;
> - u32 uncor_severity;
> - u32 cor_status;
> - u32 cor_mask;
> - u32 cap_control;
> - u32 header_log[16];
> -};
> -
> static const char * const prot_err_agent_type_strs[] = {
> "Restricted CXL Device",
> "Restricted CXL Host Downstream Port",
> diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
> index 86bfcf7909ec..6f8c00495708 100644
> --- a/drivers/firmware/efi/cper_cxl.h
> +++ b/drivers/firmware/efi/cper_cxl.h
> @@ -7,14 +7,11 @@
> * Author: Smita Koralahalli <[email protected]>
> */
>
> +#include <linux/cxl-event.h>
> +
> #ifndef LINUX_CPER_CXL_H
> #define LINUX_CPER_CXL_H
>
> -/* CXL Protocol Error Section */
> -#define CPER_SEC_CXL_PROT_ERR \
> - GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
> - 0x4B, 0x77, 0x10, 0x48)
> -
> #pragma pack(1)
>
> /* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index c1a7dc325121..2cbf0a93785a 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -89,6 +89,10 @@ enum {
> #define CPER_NOTIFY_DMAR \
> GUID_INIT(0x667DD791, 0xC6B3, 0x4c27, 0x8A, 0x6B, 0x0F, 0x8E, \
> 0x72, 0x2D, 0xEB, 0x41)
> +/* CXL Protocol Error Section */
> +#define CPER_SEC_CXL_PROT_ERR \
> + GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \
> + 0x4B, 0x77, 0x10, 0x48)
>
> /*
> * Flags bits definitions for flags in struct cper_record_header
> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
> index 6ce839c59749..3a41dd5723e8 100644
> --- a/include/linux/cxl-event.h
> +++ b/include/linux/cxl-event.h
> @@ -141,6 +141,17 @@ struct cxl_cper_event_rec {
> union cxl_event event;
> } __packed;
>
> +/* CXL RAS Capability Structure, CXL v3.0 sec 8.2.4.16 */
> +struct cxl_ras_capability_regs {
> + u32 uncor_status;
> + u32 uncor_mask;
> + u32 uncor_severity;
> + u32 cor_status;
> + u32 cor_mask;
> + u32 cap_control;
> + u32 header_log[16];
> +};
> +
> struct cxl_cper_event_info {
> struct cxl_cper_event_rec rec;
> };
> --
> 2.17.1
>
>