Restructure and fix GHES PCIe AER handling
cover Restructure and fix GHES PCIe AER handling
First, break the PCIe AER handling out into its own function to separate it from the standard GHES processing
Then fix the AER handling to process all errors in the AER driver rather than only handling recoverable errors.
Tyler Baicar (2):
acpi: apei: handle PCIe AER errors in separate function
acpi: apei: call into AER handling regardless of severity
drivers/acpi/apei/ghes.c | 62 +++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 30 deletions(-)
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v3 2/2 acpi: apei: call into AER handling regardless of severity
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/acpi/apei/ghes.c +3 -5
@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif } -static void ghes_handle_aer (struct acpi_hest_generic_data *gdata,int sev,int sec_sev) +static void ghes_handle_aer (struct acpi_hest_generic_data *gdata) {#ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && +if (pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) {unsigned int devfn;int aer_severity;
@@ -519,7 +517,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_handle_memory_failure (gdata, sev); }else if (guid_equal (sec_type, &CPER_SEC_PCIE )) { -ghes_handle_aer (gdata, sev, sec_sev); +ghes_handle_aer (gdata); }else if (guid_equal (sec_type, &CPER_SEC_PROC_ARM )) {struct cper_sec_proc_arm *err =acpi_hest_get_payload (gdata);
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Quoted 16 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/acpi/apei/ghes.c +0
-0
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 459
Quoted 2 lines
drivers/acpi/apei/ghes.c +0
-0@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif }
Where did the explanatory comment go?
+/*
+ * PCIe AER errors need to be sent to the AER driver for reporting and
+ * recovery. The GHES severities map to the following AER severities and
+ * require the following handling:
+ *
+ * GHES_SEV_CORRECTABLE -> AER_CORRECTABLE
+ * These need to be reported by the AER driver but no recovery is
+ * necessary.
+ * GHES_SEV_RECOVERABLE -> AER_NONFATAL
+ * GHES_SEV_RECOVERABLE && CPER_SEC_RESET -> AER_FATAL
+ * These both need to be reported and recovered from by the AER driver.
+ * GHES_SEV_PANIC does not make it to this handling since the kernel must
+ * panic.
+ */
<--- ???
Quoted 23 lines
drivers/acpi/apei/ghes.c +3
-5-static void ghes_handle_aer (struct acpi_hest_generic_data *gdata,int sev,int sec_sev) +static void ghes_handle_aer (struct acpi_hest_generic_data *gdata) {#ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && +if (pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) {unsigned int devfn;int aer_severity;@@ -519,7 +517,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_handle_memory_failure (gdata, sev); }else if (guid_equal (sec_type, &CPER_SEC_PCIE )) { -ghes_handle_aer (gdata, sev, sec_sev); +ghes_handle_aer (gdata); }else if (guid_equal (sec_type, &CPER_SEC_PROC_ARM )) {struct cper_sec_proc_arm *err =acpi_hest_get_payload (gdata);
Signature · 5 lines
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Quoted 12 lines · Borislav Petkov
Quoted 11 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
Quoted 8 lines
drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 459
Quoted 7 lines
drivers/acpi/apei/ghes.c +0
-0@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif }Where did the explanatory comment go?
Ah, forgot to but that back in. I'll send an update shortly.
Thanks,
Tyler
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Quoted 1 line · Tyler Baicar
Ah, forgot to but that back in. I'll send an update shortly.
Just the one patch which needs updating pls, as a reply to the the respective message.
Thx.
Signature · 5 lines
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
In reply to Borislav PetkovQuoted 36 lines · Borislav Petkov
Quoted 16 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/acpi/apei/ghes.c +0
-0drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 459
Quoted 2 lines
drivers/acpi/apei/ghes.c +0
-0@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif }Where did the explanatory comment go?
+/*
+ * PCIe AER errors need to be sent to the AER driver for reporting and
+ * recovery. The GHES severities map to the following AER severities and
+ * require the following handling:
+ *
+ *GHES_SEV_CORRECTABLE->AER_CORRECTABLE
+ * These need to be reported by the AER driver but no recovery is
+ * necessary.
+ *GHES_SEV_RECOVERABLE->AER_NONFATAL
+ *GHES_SEV_RECOVERABLE&&CPER_SEC_RESET->AER_FATAL
+ * These both need to be reported and recovered from by the AER driver.
+ *GHES_SEV_PANICdoes not make it to this handling since the kernel must
+ * panic.
+ */
Quoted 1 line
<--- ???
Updated patch including the comment:
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
--- drivers/acpi/apei/ghes.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
drivers/acpi/apei/ghes.c +17 -5
@@ -458,14 +458,26 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif } -static void ghes_handle_aer (struct acpi_hest_generic_data *gdata,int sev,int sec_sev) +/* +* PCIe AER errors need to be sent to the AER driver for reporting and +* recovery. The GHES severities map to the following AER severities and +* require the following handling: +* +* GHES_SEV_CORRECTABLE -> AER_CORRECTABLE +* These need to be reported by the AER driver but no recovery is +* necessary. +* GHES_SEV_RECOVERABLE -> AER_NONFATAL +* GHES_SEV_RECOVERABLE && CPER_SEC_RESET -> AER_FATAL +* These both need to be reported and recovered from by the AER driver. +* GHES_SEV_PANIC does not make it to this handling since the kernel must +* panic. +*/ +static void ghes_handle_aer (struct acpi_hest_generic_data *gdata) {#ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && +if (pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) {unsigned int devfn;int aer_severity;
@@ -519,7 +531,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_handle_memory_failure (gdata, sev); }else if (guid_equal (sec_type, &CPER_SEC_PCIE )) { -ghes_handle_aer (gdata, sev, sec_sev); +ghes_handle_aer (gdata); }else if (guid_equal (sec_type, &CPER_SEC_PROC_ARM )) {struct cper_sec_proc_arm *err =acpi_hest_get_payload (gdata);
--
Thanks,
Tyler
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Quoted 37 lines · Tyler Baicar
Quoted 36 lines · Borislav Petkov
Quoted 16 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/acpi/apei/ghes.c +0
-0drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int · line 459
Quoted 2 lines
drivers/acpi/apei/ghes.c +0
-0@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif }Where did the explanatory comment go?
+/*
+ * PCIe AER errors need to be sent to the AER driver for reporting and
+ * recovery. The GHES severities map to the following AER severities and
+ * require the following handling:
+ *
+ *GHES_SEV_CORRECTABLE->AER_CORRECTABLE
+ * These need to be reported by the AER driver but no recovery is
+ * necessary.
+ *GHES_SEV_RECOVERABLE->AER_NONFATAL
+ *GHES_SEV_RECOVERABLE&&CPER_SEC_RESET->AER_FATAL
+ * These both need to be reported and recovered from by the AER driver.
+ *GHES_SEV_PANICdoes not make it to this handling since the kernel must
+ * panic.
+ */
Quoted 2 lines
Quoted 1 line
<--- ???
Updated patch including the comment:
When you decide to do the reckless thing of pasting a patch into thunderbird on *windoze*, first send it to yourself only and try applying it.
Because I see this:
[boris@pd: ~/kernel/linux> test-apply.sh /tmp/tbaicar.02
checking file drivers/acpi/apei/ghes.c
patch: **** malformed patch at line 64: @@ -519,7 +531,7 @@ static void ghes_do_proc(struct ghes *ghes,
Not good.
Signature · 5 lines
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
In reply to Tyler BaicarQuoted 5 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
It will also call do_recovery() regardless of the severity for AER correctable errors. Correctable errors include those error conditions where hardware can recover without any loss of information. Hardware corrects these errors and software intervention is not required.
So we'd better modify the code as below.
drivers/pci/pcie/aer/aerdrv_core.c +2 -1
@@ -633,7 +633,8 @@ static void aer_recover_work_func(struct work_struct *work)continue ; }cper_print_aer (pdev, entry.severity, entry.regs); -do_recovery (pdev, entry.severity); +if (entry.severity !=AER_CORRECTABLE ) +do_recovery (pdev, entry.severity);pci_dev_put (pdev); } }
Thanks,
Dongdong
Quoted 37 lines
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/acpi/apei/ghes.c +3
-5@@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif } -static void ghes_handle_aer (struct acpi_hest_generic_data *gdata,int sev,int sec_sev) +static void ghes_handle_aer (struct acpi_hest_generic_data *gdata) {#ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && +if (pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) {unsigned int devfn;int aer_severity;@@ -519,7 +517,7 @@ static void ghes_do_proc(struct ghes *ghes,ghes_handle_memory_failure (gdata, sev); }else if (guid_equal (sec_type, &CPER_SEC_PCIE )) { -ghes_handle_aer (gdata, sev, sec_sev); +ghes_handle_aer (gdata); }else if (guid_equal (sec_type, &CPER_SEC_PROC_ARM )) {struct cper_sec_proc_arm *err =acpi_hest_get_payload (gdata);
Re: [PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Quoted 17 lines · Dongdong Liu
Quoted 5 lines · Tyler Baicar
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity
It will also call
do_recovery()regardless of the severity for AER correctable errors. Correctable errors include those error conditions where hardware can recover without any loss of information. Hardware corrects these errors and software intervention is not required.
So we'd better modify the code as below.drivers/pci/pcie/aer/aerdrv_core.c +0
-0
drivers/pci/pcie/aer/aerdrv_core.c static void aer_recover_work_func(struct work_struct *work) · line 640
Quoted 9 lines
drivers/pci/pcie/aer/aerdrv_core.c +2
-1@@ -633,7 +633,8 @@ static void aer_recover_work_func(struct work_struct *work) continue; } cper_print_aer(pdev, entry.severity, entry.regs); - do_recovery(pdev, entry.severity); + if(entry.severity != AER_CORRECTABLE) + do_recovery(pdev, entry.severity); pci_dev_put(pdev); } }
Hello Dongdong,
Yes, I have a patch for this that needs to be picked up.
https://lkml.org/lkml/2017/8/28/848
Thanks,
Tyler
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/2 acpi: apei: handle PCIe AER errors in separate function
In reply to Tyler BaicarMove PCIe AER error handling code into a separate function.
--- drivers/acpi/apei/ghes.c | 64 +++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 30 deletions(-)
drivers/acpi/apei/ghes.c +34 -30
@@ -458,6 +458,39 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int#endif } +static void ghes_handle_aer (struct acpi_hest_generic_data *gdata,int sev,int sec_sev) +{ +#ifdef CONFIG_ACPI_APEI_PCIEAER +struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); + +if (sev ==GHES_SEV_RECOVERABLE && + sec_sev ==GHES_SEV_RECOVERABLE && + pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && + pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) { +unsigned int devfn; +int aer_severity; + + devfn =PCI_DEVFN (pcie_err->device_id.device, + pcie_err->device_id.function); + aer_severity =cper_severity_to_aer (gdata->error_severity); + +/* +* If firmware reset the component to contain +* the error, we must reinitialize it before +* use, so treat it as a fatal AER error. +*/ +if (gdata->flags &CPER_SEC_RESET ) + aer_severity =AER_FATAL ; + +aer_recover_queue (pcie_err->device_id.segment, + pcie_err->device_id.bus, + devfn, aer_severity, + (struct aer_capability_regs *) + pcie_err->aer_info); + } +#endif +} +static void ghes_do_proc (struct ghes *ghes,const struct acpi_hest_generic_status *estatus) {
@@ -485,38 +518,9 @@ static void ghes_do_proc(struct ghes *ghes,arch_apei_report_mem_error (sev, mem_err);ghes_handle_memory_failure (gdata, sev); } -#ifdef CONFIG_ACPI_APEI_PCIEAER else if (guid_equal (sec_type, &CPER_SEC_PCIE )) { -struct cper_sec_pcie *pcie_err =acpi_hest_get_payload (gdata); - -if (sev ==GHES_SEV_RECOVERABLE && - sec_sev ==GHES_SEV_RECOVERABLE && - pcie_err->validation_bits &CPER_PCIE_VALID_DEVICE_ID && - pcie_err->validation_bits &CPER_PCIE_VALID_AER_INFO ) { -unsigned int devfn; -int aer_severity; - - devfn =PCI_DEVFN (pcie_err->device_id.device, - pcie_err->device_id.function); - aer_severity =cper_severity_to_aer (gdata->error_severity); - -/* -* If firmware reset the component to contain -* the error, we must reinitialize it before -* use, so treat it as a fatal AER error. -*/ -if (gdata->flags &CPER_SEC_RESET ) - aer_severity =AER_FATAL ; - -aer_recover_queue (pcie_err->device_id.segment, - pcie_err->device_id.bus, - devfn, aer_severity, - (struct aer_capability_regs *) - pcie_err->aer_info); - } - +ghes_handle_aer (gdata, sev, sec_sev); } -#endif else if (guid_equal (sec_type, &CPER_SEC_PROC_ARM )) {struct cper_sec_proc_arm *err =acpi_hest_get_payload (gdata);
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 1/2] acpi: apei: handle PCIe AER errors in separate function
Quoted 6 lines · Tyler Baicar
Move PCIe AER error handling code into a separate function.
Signed-off-by Tyler Baicar <[email protected]>--- drivers/acpi/apei/ghes.c | 64 +++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 30 deletions(-)
Signature · 5 lines
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--