[4/4] acpi/ghes, cxl/pci: Trace FW-First CXL Protocol Errors

Message ID 20240102150933.161009-5-Smita.KoralahalliChannabasappa@amd.com
State New
Headers
Series acpi/ghes, cper, cxl: Trace FW-First CXL Protocol Errors |

Commit Message

Smita Koralahalli Jan. 2, 2024, 3:09 p.m. UTC
  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 <Smita.KoralahalliChannabasappa@amd.com>
---
 drivers/acpi/apei/ghes.c  |  5 +++++
 drivers/cxl/core/pci.c    | 46 +++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlpci.h      |  3 +++
 drivers/cxl/pci.c         |  5 +++++
 include/linux/cxl-event.h |  1 +
 5 files changed, 60 insertions(+)
  

Comments

Ira Weiny Jan. 2, 2024, 8:27 p.m. UTC | #1
Smita Koralahalli 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 <Smita.KoralahalliChannabasappa@amd.com>

[snip]

>  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..da516982a625 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -6,6 +6,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci-doe.h>
>  #include <linux/aer.h>
> +#include <linux/cper.h>
>  #include <cxlpci.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
> @@ -836,6 +837,51 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL);
>  
> +#define CXL_AER_UNCORRECTABLE	0
> +#define CXL_AER_CORRECTABLE	1

Better defined as an enum?

> +
> +int cper_severity_cxl_aer(int cper_severity)

My gut says that it would be better to hide this conversion in the
GHES/CPER code and send a more generic defined CXL_AER_* severity through.

> +{
> +	switch (cper_severity) {
> +	case CPER_SEV_RECOVERABLE:
> +	case CPER_SEV_FATAL:
> +		return CXL_AER_UNCORRECTABLE;
> +	default:
> +		return CXL_AER_CORRECTABLE;
> +	}
> +}
> +
> +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
> +			       struct cxl_cper_rec_data *data)
> +{
> +	struct cper_cxl_event_sn *dev_serial_num =  &data->rec.hdr.dev_serial_num;
> +	u32 status, fe;
> +	int severity;
> +
> +	severity = cper_severity_cxl_aer(data->severity);
> +
> +	cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) |
> +			dev_serial_num->lower_dw);

This permanently overwrites the serial number read from PCI...

If the serial number does not match up or was not valid (per the check in
the previous patch) lets add a warning.

AFAICT they should match.

Ira

[snip]
  
Smita Koralahalli Jan. 3, 2024, 9:13 p.m. UTC | #2
On 1/2/2024 12:27 PM, Ira Weiny wrote:
> Smita Koralahalli 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 <Smita.KoralahalliChannabasappa@amd.com>
> 
> [snip]
> 
>>   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..da516982a625 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/pci.h>
>>   #include <linux/pci-doe.h>
>>   #include <linux/aer.h>
>> +#include <linux/cper.h>
>>   #include <cxlpci.h>
>>   #include <cxlmem.h>
>>   #include <cxl.h>
>> @@ -836,6 +837,51 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL);
>>   
>> +#define CXL_AER_UNCORRECTABLE	0
>> +#define CXL_AER_CORRECTABLE	1
> 
> Better defined as an enum?

Will change.

> 
>> +
>> +int cper_severity_cxl_aer(int cper_severity)
> 
> My gut says that it would be better to hide this conversion in the
> GHES/CPER code and send a more generic defined CXL_AER_* severity through.

Ok will change.

> 
>> +{
>> +	switch (cper_severity) {
>> +	case CPER_SEV_RECOVERABLE:
>> +	case CPER_SEV_FATAL:
>> +		return CXL_AER_UNCORRECTABLE;
>> +	default:
>> +		return CXL_AER_CORRECTABLE;
>> +	}
>> +}
>> +
>> +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
>> +			       struct cxl_cper_rec_data *data)
>> +{
>> +	struct cper_cxl_event_sn *dev_serial_num =  &data->rec.hdr.dev_serial_num;
>> +	u32 status, fe;
>> +	int severity;
>> +
>> +	severity = cper_severity_cxl_aer(data->severity);
>> +
>> +	cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) |
>> +			dev_serial_num->lower_dw);
> 
> This permanently overwrites the serial number read from PCI...
> 
> If the serial number does not match up or was not valid (per the check in
> the previous patch) lets add a warning.

Sure will add.

Thanks,
Smita

> 
> AFAICT they should match.
> 
> Ira
> 
> [snip]
>
  

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 6471584b2e79..217494c7c884 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -716,6 +716,7 @@  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_rec_data data;
 
 	memset(&data, 0, sizeof(data));
@@ -724,6 +725,10 @@  void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
 		return;
 
 	data.severity = gdata->error_severity;
+
+	guard(rwsem_read)(&cxl_cper_rw_sem);
+	if (cper_callback)
+		cper_callback(event_type, &data);
 }
 
 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..da516982a625 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -6,6 +6,7 @@ 
 #include <linux/pci.h>
 #include <linux/pci-doe.h>
 #include <linux/aer.h>
+#include <linux/cper.h>
 #include <cxlpci.h>
 #include <cxlmem.h>
 #include <cxl.h>
@@ -836,6 +837,51 @@  void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL);
 
+#define CXL_AER_UNCORRECTABLE	0
+#define CXL_AER_CORRECTABLE	1
+
+int 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 cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
+			       struct cxl_cper_rec_data *data)
+{
+	struct cper_cxl_event_sn *dev_serial_num =  &data->rec.hdr.dev_serial_num;
+	u32 status, fe;
+	int severity;
+
+	severity = cper_severity_cxl_aer(data->severity);
+
+	cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) |
+			dev_serial_num->lower_dw);
+
+	if (severity == CXL_AER_CORRECTABLE) {
+		status = data->cxl_ras->cor_status & ~data->cxl_ras->cor_mask;
+
+		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
+	} else {
+		status = data->cxl_ras->uncor_status & ~data->cxl_ras->uncor_mask;
+
+		if (hweight32(status) > 1)
+			fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
+					   data->cxl_ras->cap_control));
+		else
+			fe = status;
+
+		trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
+						  data->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..462f1f9e82b0 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_rec_data;
+void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds,
+			       struct cxl_cper_rec_data *data);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 7edbd53357e5..2b9f4dbf06c9 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, data);
+		return;
+	}
+
 	/* Fabricate a log type */
 	hdr_flags = get_unaligned_le24(data->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 7ba2dfd6619e..b4558f206f59 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)