When PCIe AER is in FW-First, OS should process CXL Protocol errors from
CPER records.

Reuse the existing work queue cxl_cper_work registered with GHES 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>
---
v2:
	Removed pr_warn for serial number.
	p_err -> rec/p_rec.
---
 drivers/acpi/apei/ghes.c | 14 ++++++++++++++
 drivers/cxl/core/pci.c   | 24 ++++++++++++++++++++++++
 drivers/cxl/cxlpci.h     |  3 +++
 drivers/cxl/pci.c        | 20 ++++++++++++++++++--
 include/cxl/event.h      |  1 +
 5 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 9dcf0f78458f..5082885e1f2c 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
 
 	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
 		return;
+
+	guard(spinlock_irqsave)(&cxl_cper_work_lock);
+
+	if (!cxl_cper_work)
+		return;
+
+	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
+
+	if (!kfifo_put(&cxl_cper_fifo, wd)) {
+		pr_err_ratelimited("CXL CPER kfifo overflow\n");
+		return;
+	}
+
+	schedule_work(cxl_cper_work);
 }
 
 int cxl_cper_register_work(struct work_struct *work)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 5b46bc46aaa9..39ef24c8991f 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -650,6 +650,30 @@ void read_cdat_data(struct cxl_port *port)
 }
 EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
 
+void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
+			struct cxl_cper_prot_err *rec)
+{
+	u32 status, fe;
+
+	if (rec->severity == CXL_AER_CORRECTABLE) {
+		status = rec->cxl_ras.cor_status & ~rec->cxl_ras.cor_mask;
+
+		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
+	} else {
+		status = rec->cxl_ras.uncor_status & ~rec->cxl_ras.uncor_mask;
+
+		if (hweight32(status) > 1)
+			fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
+					   rec->cxl_ras.cap_control));
+		else
+			fe = status;
+
+		trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
+						  rec->cxl_ras.header_log);
+	}
+}
+EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL);
+
 static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds,
 				 void __iomem *ras_base)
 {
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 4da07727ab9c..8acd8f2c39c9 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -129,4 +129,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_prot_err;
+void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
+			struct cxl_cper_prot_err *rec);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 915102f5113f..0a29321921a0 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1064,12 +1064,28 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
 			       &uuid_null, &rec->event);
 }
 
+static void cxl_handle_prot_err(struct cxl_cper_prot_err *rec)
+{
+	struct cxl_dev_state *cxlds;
+
+	cxlds = get_cxl_devstate(rec->segment, rec->bus,
+				 rec->device, rec->function);
+	if (!cxlds)
+		return;
+
+	cxl_trace_prot_err(cxlds, rec);
+}
+
 static void cxl_cper_work_fn(struct work_struct *work)
 {
 	struct cxl_cper_work_data wd;
 
-	while (cxl_cper_kfifo_get(&wd))
-		cxl_handle_cper_event(wd.event_type, &wd.rec);
+	while (cxl_cper_kfifo_get(&wd)) {
+		if (wd.event_type == CXL_CPER_EVENT_PROT_ERR)
+			cxl_handle_prot_err(&wd.p_rec);
+		else
+			cxl_handle_cper_event(wd.event_type, &wd.rec);
+	}
 }
 static DECLARE_WORK(cxl_cper_work, cxl_cper_work_fn);
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index 5b316150556a..d854d8c435db 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -115,6 +115,7 @@ struct cxl_event_record_raw {
 } __packed;
 
 enum cxl_event_type {
+	CXL_CPER_EVENT_PROT_ERR,
 	CXL_CPER_EVENT_GENERIC,
 	CXL_CPER_EVENT_GEN_MEDIA,
 	CXL_CPER_EVENT_DRAM,
-- 
2.17.1

Smita Koralahalli wrote:
> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
> CPER records.
> 
> Reuse the existing work queue cxl_cper_work registered with GHES 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>
> ---
> v2:
> 	Removed pr_warn for serial number.
> 	p_err -> rec/p_rec.
> ---
>  drivers/acpi/apei/ghes.c | 14 ++++++++++++++
>  drivers/cxl/core/pci.c   | 24 ++++++++++++++++++++++++
>  drivers/cxl/cxlpci.h     |  3 +++
>  drivers/cxl/pci.c        | 20 ++++++++++++++++++--
>  include/cxl/event.h      |  1 +
>  5 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 9dcf0f78458f..5082885e1f2c 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>  
>  	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
>  		return;
> +
> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +
> +	if (!cxl_cper_work)
> +		return;
> +
> +	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
> +
> +	if (!kfifo_put(&cxl_cper_fifo, wd)) {
> +		pr_err_ratelimited("CXL CPER kfifo overflow\n");
> +		return;
> +	}
> +
> +	schedule_work(cxl_cper_work);

The cxl_cper_work item is only for cases where the cxl_pci driver might care
about annotating an error report with driver specific details like the
impacted kernel object name, 'struct cxl_memdev', or address translation
for DPA data.

Protocol errors that are not endpoint errors should never be placed in
the cxl_cper_fifo. That is exclusively for errors that cxl_pci needs to
consume.

My expectation is that similar to aer_recover_queue for PCIe protocol
errors CXL needs to grow a cxl_recover_queue that at a minimum triggers
new trace events to dump these records to RAS daemon.

I am struggling to think what useful information cxl_pci could ever
append to a protocol error event.

What is more likely is that later when Terry adds port error handling a
CPER protocol error record could trigger a new cxl_do_recovery() to
react to CXL topology errors that might impact downstream CXL devices.
In that case the notification will come through something like a new
'struct cxl_error_handlers *' hanging off 'struct pci_driver' since
accelerator drivers are going to have distinct error handling from
generic memory expanders.

On 10/2/2024 5:16 PM, Dan Williams wrote:
> Smita Koralahalli wrote:
>> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
>> CPER records.
>>
>> Reuse the existing work queue cxl_cper_work registered with GHES 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>
>> ---
>> v2:
>> 	Removed pr_warn for serial number.
>> 	p_err -> rec/p_rec.
>> ---
>>   drivers/acpi/apei/ghes.c | 14 ++++++++++++++
>>   drivers/cxl/core/pci.c   | 24 ++++++++++++++++++++++++
>>   drivers/cxl/cxlpci.h     |  3 +++
>>   drivers/cxl/pci.c        | 20 ++++++++++++++++++--
>>   include/cxl/event.h      |  1 +
>>   5 files changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 9dcf0f78458f..5082885e1f2c 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>>   
>>   	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
>>   		return;
>> +
>> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
>> +
>> +	if (!cxl_cper_work)
>> +		return;
>> +
>> +	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
>> +
>> +	if (!kfifo_put(&cxl_cper_fifo, wd)) {
>> +		pr_err_ratelimited("CXL CPER kfifo overflow\n");
>> +		return;
>> +	}
>> +
>> +	schedule_work(cxl_cper_work);
> 
> The cxl_cper_work item is only for cases where the cxl_pci driver might care
> about annotating an error report with driver specific details like the
> impacted kernel object name, 'struct cxl_memdev', or address translation
> for DPA data.
> 
> Protocol errors that are not endpoint errors should never be placed in
> the cxl_cper_fifo. That is exclusively for errors that cxl_pci needs to
> consume.
> 
> My expectation is that similar to aer_recover_queue for PCIe protocol
> errors CXL needs to grow a cxl_recover_queue that at a minimum triggers
> new trace events to dump these records to RAS daemon.
> 
> I am struggling to think what useful information cxl_pci could ever
> append to a protocol error event.
> 
> What is more likely is that later when Terry adds port error handling a
> CPER protocol error record could trigger a new cxl_do_recovery() to
> react to CXL topology errors that might impact downstream CXL devices.
> In that case the notification will come through something like a new
> 'struct cxl_error_handlers *' hanging off 'struct pci_driver' since
> accelerator drivers are going to have distinct error handling from
> generic memory expanders.

Hmm, I agree handling device and protocol errors separately. Unless 
Terry has something to add here, I can start working on a minimal 
version of cxl_recover_queue as of now.

Thanks
Smita
>

Smita Koralahalli wrote:
> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
> CPER records.
> 
> Reuse the existing work queue cxl_cper_work registered with GHES 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>
> ---
> v2:
> 	Removed pr_warn for serial number.
> 	p_err -> rec/p_rec.
> ---
>  drivers/acpi/apei/ghes.c | 14 ++++++++++++++
>  drivers/cxl/core/pci.c   | 24 ++++++++++++++++++++++++
>  drivers/cxl/cxlpci.h     |  3 +++
>  drivers/cxl/pci.c        | 20 ++++++++++++++++++--
>  include/cxl/event.h      |  1 +
>  5 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 9dcf0f78458f..5082885e1f2c 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>  
>  	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
>  		return;
> +
> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +
> +	if (!cxl_cper_work)
> +		return;
> +
> +	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
> +
> +	if (!kfifo_put(&cxl_cper_fifo, wd)) {
> +		pr_err_ratelimited("CXL CPER kfifo overflow\n");
> +		return;
> +	}
> +
> +	schedule_work(cxl_cper_work);
>  }
>  
>  int cxl_cper_register_work(struct work_struct *work)
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 5b46bc46aaa9..39ef24c8991f 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -650,6 +650,30 @@ void read_cdat_data(struct cxl_port *port)
>  }
>  EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>  
> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
> +			struct cxl_cper_prot_err *rec)
> +{
> +	u32 status, fe;
> +
> +	if (rec->severity == CXL_AER_CORRECTABLE) {
> +		status = rec->cxl_ras.cor_status & ~rec->cxl_ras.cor_mask;
> +
> +		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
> +	} else {
> +		status = rec->cxl_ras.uncor_status & ~rec->cxl_ras.uncor_mask;
> +
> +		if (hweight32(status) > 1)
> +			fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
> +					   rec->cxl_ras.cap_control));
> +		else
> +			fe = status;
> +
> +		trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
> +						  rec->cxl_ras.header_log);
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL);
> +
>  static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds,
>  				 void __iomem *ras_base)
>  {
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 4da07727ab9c..8acd8f2c39c9 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -129,4 +129,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_prot_err;
> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
> +			struct cxl_cper_prot_err *rec);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 915102f5113f..0a29321921a0 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1064,12 +1064,28 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>  			       &uuid_null, &rec->event);
>  }
>  
> +static void cxl_handle_prot_err(struct cxl_cper_prot_err *rec)
> +{
> +	struct cxl_dev_state *cxlds;
> +
> +	cxlds = get_cxl_devstate(rec->segment, rec->bus,
> +				 rec->device, rec->function);

As mentioned in 2/4 I don't think this is going to work correctly.

> +	if (!cxlds)
> +		return;
> +
> +	cxl_trace_prot_err(cxlds, rec);
> +}
> +
>  static void cxl_cper_work_fn(struct work_struct *work)
>  {
>  	struct cxl_cper_work_data wd;
>  
> -	while (cxl_cper_kfifo_get(&wd))
> -		cxl_handle_cper_event(wd.event_type, &wd.rec);
> +	while (cxl_cper_kfifo_get(&wd)) {
> +		if (wd.event_type == CXL_CPER_EVENT_PROT_ERR)
> +			cxl_handle_prot_err(&wd.p_rec);
> +		else
> +			cxl_handle_cper_event(wd.event_type, &wd.rec);

Why does this need to change?

Have cxl_handle_cper_event() decode the BDF as it does now and call
cxl_handle_prot_err() there?  I think you could drop patch 2/4 entirely
and not have to duplicate the logic.

Ira

> +	}
>  }
>  static DECLARE_WORK(cxl_cper_work, cxl_cper_work_fn);
>  
> diff --git a/include/cxl/event.h b/include/cxl/event.h
> index 5b316150556a..d854d8c435db 100644
> --- a/include/cxl/event.h
> +++ b/include/cxl/event.h
> @@ -115,6 +115,7 @@ struct cxl_event_record_raw {
>  } __packed;
>  
>  enum cxl_event_type {
> +	CXL_CPER_EVENT_PROT_ERR,
>  	CXL_CPER_EVENT_GENERIC,
>  	CXL_CPER_EVENT_GEN_MEDIA,
>  	CXL_CPER_EVENT_DRAM,
> -- 
> 2.17.1
>

On 10/1/2024 8:52 AM, Ira Weiny wrote:
> Smita Koralahalli wrote:
>> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
>> CPER records.
>>
>> Reuse the existing work queue cxl_cper_work registered with GHES 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>
>> ---
>> v2:
>> 	Removed pr_warn for serial number.
>> 	p_err -> rec/p_rec.
>> ---
>>   drivers/acpi/apei/ghes.c | 14 ++++++++++++++
>>   drivers/cxl/core/pci.c   | 24 ++++++++++++++++++++++++
>>   drivers/cxl/cxlpci.h     |  3 +++
>>   drivers/cxl/pci.c        | 20 ++++++++++++++++++--
>>   include/cxl/event.h      |  1 +
>>   5 files changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 9dcf0f78458f..5082885e1f2c 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>>   
>>   	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec))
>>   		return;
>> +
>> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
>> +
>> +	if (!cxl_cper_work)
>> +		return;
>> +
>> +	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
>> +
>> +	if (!kfifo_put(&cxl_cper_fifo, wd)) {
>> +		pr_err_ratelimited("CXL CPER kfifo overflow\n");
>> +		return;
>> +	}
>> +
>> +	schedule_work(cxl_cper_work);
>>   }
>>   
>>   int cxl_cper_register_work(struct work_struct *work)
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 5b46bc46aaa9..39ef24c8991f 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -650,6 +650,30 @@ void read_cdat_data(struct cxl_port *port)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>>   
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> +			struct cxl_cper_prot_err *rec)
>> +{
>> +	u32 status, fe;
>> +
>> +	if (rec->severity == CXL_AER_CORRECTABLE) {
>> +		status = rec->cxl_ras.cor_status & ~rec->cxl_ras.cor_mask;
>> +
>> +		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
>> +	} else {
>> +		status = rec->cxl_ras.uncor_status & ~rec->cxl_ras.uncor_mask;
>> +
>> +		if (hweight32(status) > 1)
>> +			fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
>> +					   rec->cxl_ras.cap_control));
>> +		else
>> +			fe = status;
>> +
>> +		trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
>> +						  rec->cxl_ras.header_log);
>> +	}
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL);
>> +
>>   static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds,
>>   				 void __iomem *ras_base)
>>   {
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 4da07727ab9c..8acd8f2c39c9 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -129,4 +129,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_prot_err;
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> +			struct cxl_cper_prot_err *rec);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 915102f5113f..0a29321921a0 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -1064,12 +1064,28 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>>   			       &uuid_null, &rec->event);
>>   }
>>   
>> +static void cxl_handle_prot_err(struct cxl_cper_prot_err *rec)
>> +{
>> +	struct cxl_dev_state *cxlds;
>> +
>> +	cxlds = get_cxl_devstate(rec->segment, rec->bus,
>> +				 rec->device, rec->function);
> 
> As mentioned in 2/4 I don't think this is going to work correctly.
> 
>> +	if (!cxlds)
>> +		return;
>> +
>> +	cxl_trace_prot_err(cxlds, rec);
>> +}
>> +
>>   static void cxl_cper_work_fn(struct work_struct *work)
>>   {
>>   	struct cxl_cper_work_data wd;
>>   
>> -	while (cxl_cper_kfifo_get(&wd))
>> -		cxl_handle_cper_event(wd.event_type, &wd.rec);
>> +	while (cxl_cper_kfifo_get(&wd)) {
>> +		if (wd.event_type == CXL_CPER_EVENT_PROT_ERR)
>> +			cxl_handle_prot_err(&wd.p_rec);
>> +		else
>> +			cxl_handle_cper_event(wd.event_type, &wd.rec);
> 
> Why does this need to change?
> 
> Have cxl_handle_cper_event() decode the BDF as it does now and call
> cxl_handle_prot_err() there?  I think you could drop patch 2/4 entirely
> and not have to duplicate the logic.
> 
> Ira

Yeah works for me. My intention was to not disturb the 
cxl_handle_cper_event() as that was focussed on handling device errors.

Thanks
Smita
> 
>> +	}
>>   }
>>   static DECLARE_WORK(cxl_cper_work, cxl_cper_work_fn);
>>   
>> diff --git a/include/cxl/event.h b/include/cxl/event.h
>> index 5b316150556a..d854d8c435db 100644
>> --- a/include/cxl/event.h
>> +++ b/include/cxl/event.h
>> @@ -115,6 +115,7 @@ struct cxl_event_record_raw {
>>   } __packed;
>>   
>>   enum cxl_event_type {
>> +	CXL_CPER_EVENT_PROT_ERR,
>>   	CXL_CPER_EVENT_GENERIC,
>>   	CXL_CPER_EVENT_GEN_MEDIA,
>>   	CXL_CPER_EVENT_DRAM,
>> -- 
>> 2.17.1
>>
> 
>