Add common interface for queuing up the non-fatal HW errors and notify
the registered kernel drivers.The interface supports drivers to register
to receive the callback for the non-fatal HW errors, including the vendor
specific HW errors, for the recovery and supports handling the non-fatal
errors in the process context.
Patch set
1. add the new interface to the APEI driver for the non-fatal HW
error notification.
2. change the existing error handling for the standard errors
to use the above notification interface.
2. add driver to handle HiSilicon hip PCIe controller's errors.
Changes:
V7:
1. Add changes in the APEI driver suggested by Borislav Petkov, for
queuing up all the non-fatal HW errors to the work queue and
notify the registered kernel drivers from the bottom half using
blocking notifier, common interface for both standard and
vendor-spcific errors.
2. Fix for further feedbacks in v5 HIP PCIe error handler driver
by Bjorn Helgaas.
V6:
1. Fix few changes in the patch subject line suggested by Bjorn Helgaas.
V5:
1. Fix comments from James Morse.
1.1 Changed the notification method to use the atomic_notifier_chain.
1.2 Add the error handled status for the user space.
V4:
1. Fix for the following smatch warning in the PCIe error driver,
reported by kbuild test robot<[email protected]>:
warn: should '((((1))) << (9 + i))' be a 64 bit type?
if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i))
^^^ This should be BIT_ULL() because it goes up to 9 + 32.
V3:
1. Fix the comments from Bjorn Helgaas.
V2:
1. Changes in the HiSilicon PCIe controller's error handling driver
for the comments from Bjorn Helgaas.
2. Changes in the APEI interface to support reporting the vendor error
for module with multiple devices, but use the same section type.
In the error handler will use socket id/sub module id etc to distinguish
the device.
V1:
1. Fix comments from James Morse.
2. add driver to handle HiSilicon hip08 PCIe controller's errors,
which is an application of the above interface.
Shiju Jose (5):
ACPI / APEI: Add support to queuing up the non-fatal HW errors and
notify
ACPI / APEI: Add callback for memory errors to the GHES notifier
ACPI / APEI: Add callback for AER to the GHES notifier
ACPI / APEI: Add callback for ARM HW errors to the GHES notifier
ACPI / APEI: Add callback for non-standard HW errors to the GHES
notifier
Yicong Yang (1):
PCI: hip: Add handling of HiSilicon HIP PCIe controller errors
drivers/acpi/apei/ghes.c | 268 ++++++++++++++++---
drivers/pci/controller/Kconfig | 8 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-hisi-error.c | 323 +++++++++++++++++++++++
include/acpi/ghes.h | 28 ++
5 files changed, 595 insertions(+), 33 deletions(-)
create mode 100644 drivers/pci/controller/pcie-hisi-error.c
--
2.17.1
Add callback function for handling the ARM HW errors to the GHES notifier.
Signed-off-by: Shiju Jose <[email protected]>
---
drivers/acpi/apei/ghes.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 67ef1742fc93..3b89c7621a0d 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -564,6 +564,20 @@ static int ghes_handle_aer(struct notifier_block *nb, unsigned long event,
return NOTIFY_STOP;
}
+static int ghes_handle_arm_hw_error(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct acpi_hest_generic_data *gdata = data;
+ struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
+
+ if (!guid_equal((guid_t *)gdata->section_type, &CPER_SEC_PROC_ARM))
+ return NOTIFY_DONE;
+
+ log_arm_hw_error(err);
+
+ return NOTIFY_STOP;
+}
+
static struct notifier_block ghes_notifier_mem_error = {
.notifier_call = ghes_handle_memory_failure,
};
@@ -572,6 +586,10 @@ static struct notifier_block ghes_notifier_aer = {
.notifier_call = ghes_handle_aer,
};
+static struct notifier_block ghes_notifier_arm_hw_error = {
+ .notifier_call = ghes_handle_arm_hw_error,
+};
+
struct ghes_error_handler_list {
const char *name;
struct notifier_block *nb;
@@ -586,6 +604,10 @@ static const struct ghes_error_handler_list ghes_error_handler_list[] = {
.name = "ghes_notifier_aer",
.nb = &ghes_notifier_aer,
},
+ {
+ .name = "ghes_notifier_arm_hw_error",
+ .nb = &ghes_notifier_arm_hw_error,
+ },
};
static BLOCKING_NOTIFIER_HEAD(ghes_event_notify_list);
@@ -670,12 +692,7 @@ static void ghes_do_proc(struct ghes *ghes,
pr_warn(GHES_PFX "ghes event queue full\n");
break;
}
-
- if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
- struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
-
- log_arm_hw_error(err);
- } else {
+ {
void *err = acpi_hest_get_payload(gdata);
log_non_standard_event(sec_type, fru_id, fru_text,
--
2.17.1
Add callback function for the non-standard HW errors to the GHES notifier.
Signed-off-by: Shiju Jose <[email protected]>
---
drivers/acpi/apei/ghes.c | 56 ++++++++++++++++++++++++++--------------
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3b89c7621a0d..0c27ea8ea943 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -578,6 +578,34 @@ static int ghes_handle_arm_hw_error(struct notifier_block *nb,
return NOTIFY_STOP;
}
+static int ghes_handle_non_standard_event(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct acpi_hest_generic_data *gdata = data;
+ void *err = acpi_hest_get_payload(gdata);
+ int sec_sev;
+ guid_t *sec_type;
+ const guid_t *fru_id = &guid_null;
+ char *fru_text = "";
+
+ if (guid_equal((guid_t *)gdata->section_type, &CPER_SEC_PLATFORM_MEM) ||
+ guid_equal((guid_t *)gdata->section_type, &CPER_SEC_PCIE) ||
+ guid_equal((guid_t *)gdata->section_type, &CPER_SEC_PROC_ARM))
+ return NOTIFY_DONE;
+
+ sec_type = (guid_t *)gdata->section_type;
+ sec_sev = ghes_severity(gdata->error_severity);
+ if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
+ fru_id = (guid_t *)gdata->fru_id;
+
+ if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
+ fru_text = gdata->fru_text;
+
+ log_non_standard_event(sec_type, fru_id, fru_text, sec_sev,
+ err, gdata->error_data_length);
+ return NOTIFY_OK;
+}
+
static struct notifier_block ghes_notifier_mem_error = {
.notifier_call = ghes_handle_memory_failure,
};
@@ -590,6 +618,10 @@ static struct notifier_block ghes_notifier_arm_hw_error = {
.notifier_call = ghes_handle_arm_hw_error,
};
+static struct notifier_block ghes_notifier_non_standard_event = {
+ .notifier_call = ghes_handle_non_standard_event,
+};
+
struct ghes_error_handler_list {
const char *name;
struct notifier_block *nb;
@@ -608,6 +640,10 @@ static const struct ghes_error_handler_list ghes_error_handler_list[] = {
.name = "ghes_notifier_arm_hw_error",
.nb = &ghes_notifier_arm_hw_error,
},
+ {
+ .name = "ghes_notifier_non_standard_event",
+ .nb = &ghes_notifier_non_standard_event,
+ },
};
static BLOCKING_NOTIFIER_HEAD(ghes_event_notify_list);
@@ -656,24 +692,13 @@ static DECLARE_WORK(ghes_event_work, ghes_event_work_func);
static void ghes_do_proc(struct ghes *ghes,
const struct acpi_hest_generic_status *estatus)
{
- int sev, sec_sev;
+ int sev;
struct acpi_hest_generic_data *gdata;
- guid_t *sec_type;
- const guid_t *fru_id = &guid_null;
- char *fru_text = "";
struct ghes_event_entry event_entry;
u32 len;
sev = ghes_severity(estatus->error_severity);
apei_estatus_for_each_section(estatus, gdata) {
- sec_type = (guid_t *)gdata->section_type;
- sec_sev = ghes_severity(gdata->error_severity);
- if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
- fru_id = (guid_t *)gdata->fru_id;
-
- if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
- fru_text = gdata->fru_text;
-
len = acpi_hest_get_record_size(gdata);
event_entry.gdata = (void *)gen_pool_alloc(ghes_gdata_pool,
len);
@@ -692,13 +717,6 @@ static void ghes_do_proc(struct ghes *ghes,
pr_warn(GHES_PFX "ghes event queue full\n");
break;
}
- {
- void *err = acpi_hest_get_payload(gdata);
-
- log_non_standard_event(sec_type, fru_id, fru_text,
- sec_sev, err,
- gdata->error_data_length);
- }
}
}
--
2.17.1
From: Yicong Yang <[email protected]>
The HiSilicon HIP PCIe controller is capable of handling errors
on root port and perform port reset separately at each root port.
Add error handling driver for HIP PCIe controller to log
and report recoverable errors. Perform root port reset and restore
link status after the recovery.
Following are some of the PCIe controller's recoverable errors
1. completion transmission timeout error.
2. CRS retry counter over the threshold error.
3. ECC 2 bit errors
4. AXI bresponse/rresponse errors etc.
The driver placed in the drivers/pci/controller/ because the
HIP PCIe controller does not use DWC ip.
Signed-off-by: Yicong Yang <[email protected]>
Signed-off-by: Shiju Jose <[email protected]>
--
drivers/pci/controller/Kconfig | 8 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-hisi-error.c | 336 +++++++++++++++++++++++++++++++
3 files changed, 345 insertions(+)
create mode 100644 drivers/pci/controller/pcie-hisi-error.c
---
drivers/pci/controller/Kconfig | 8 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-hisi-error.c | 323 +++++++++++++++++++++++
3 files changed, 332 insertions(+)
create mode 100644 drivers/pci/controller/pcie-hisi-error.c
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 20bf00f587bd..8bc6111480c8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -268,6 +268,14 @@ config PCI_HYPERV_INTERFACE
The Hyper-V PCI Interface is a helper driver allows other drivers to
have a common interface with the Hyper-V PCI frontend driver.
+config PCIE_HISI_ERR
+ depends on ARM64 || COMPILE_TEST
+ depends on ACPI
+ bool "HiSilicon HIP PCIe controller error handling driver"
+ help
+ Say Y here if you want error handling support
+ for the PCIe controller's errors on HiSilicon HIP SoCs
+
source "drivers/pci/controller/dwc/Kconfig"
source "drivers/pci/controller/cadence/Kconfig"
endmenu
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 01b2502a5323..94f37b3d9929 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o
obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
obj-$(CONFIG_VMD) += vmd.o
obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
+obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
# pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
obj-y += dwc/
diff --git a/drivers/pci/controller/pcie-hisi-error.c b/drivers/pci/controller/pcie-hisi-error.c
new file mode 100644
index 000000000000..cc721070e07b
--- /dev/null
+++ b/drivers/pci/controller/pcie-hisi-error.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for handling the PCIe controller errors on
+ * HiSilicon HIP SoCs.
+ *
+ * Copyright (c) 2018-2019 HiSilicon Limited.
+ */
+
+#include <linux/acpi.h>
+#include <acpi/ghes.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/kfifo.h>
+#include <linux/spinlock.h>
+
+#define HISI_PCIE_ERR_INFO_SIZE 1024
+
+/* HISI PCIe controller error definitions */
+#define HISI_PCIE_ERR_MISC_REGS 33
+
+#define HISI_PCIE_SUB_MODULE_ID_AP 0
+#define HISI_PCIE_SUB_MODULE_ID_TL 1
+#define HISI_PCIE_SUB_MODULE_ID_MAC 2
+#define HISI_PCIE_SUB_MODULE_ID_DL 3
+#define HISI_PCIE_SUB_MODULE_ID_SDI 4
+
+#define HISI_PCIE_LOCAL_VALID_VERSION BIT(0)
+#define HISI_PCIE_LOCAL_VALID_SOC_ID BIT(1)
+#define HISI_PCIE_LOCAL_VALID_SOCKET_ID BIT(2)
+#define HISI_PCIE_LOCAL_VALID_NIMBUS_ID BIT(3)
+#define HISI_PCIE_LOCAL_VALID_SUB_MODULE_ID BIT(4)
+#define HISI_PCIE_LOCAL_VALID_CORE_ID BIT(5)
+#define HISI_PCIE_LOCAL_VALID_PORT_ID BIT(6)
+#define HISI_PCIE_LOCAL_VALID_ERR_TYPE BIT(7)
+#define HISI_PCIE_LOCAL_VALID_ERR_SEVERITY BIT(8)
+#define HISI_PCIE_LOCAL_VALID_ERR_MISC 9
+
+#define HISI_ERR_SEV_RECOVERABLE 0
+#define HISI_ERR_SEV_FATAL 1
+#define HISI_ERR_SEV_CORRECTED 2
+#define HISI_ERR_SEV_NONE 3
+
+static guid_t hisi_pcie_sec_type = GUID_INIT(0xB2889FC9, 0xE7D7, 0x4F9D,
+ 0xA8, 0x67, 0xAF, 0x42, 0xE9, 0x8B, 0xE7, 0x72);
+
+#define HISI_PCIE_CORE_ID(v) ((v) >> 3)
+#define HISI_PCIE_PORT_ID(core, v) (((v) >> 1) + ((core) << 3))
+#define HISI_PCIE_CORE_PORT_ID(v) (((v) % 8) << 1)
+
+struct hisi_pcie_error_data {
+ u64 val_bits;
+ u8 version;
+ u8 soc_id;
+ u8 socket_id;
+ u8 nimbus_id;
+ u8 sub_module_id;
+ u8 core_id;
+ u8 port_id;
+ u8 err_severity;
+ u16 err_type;
+ u8 reserv[2];
+ u32 err_misc[HISI_PCIE_ERR_MISC_REGS];
+};
+
+struct hisi_pcie_error_private {
+ struct notifier_block nb;
+ struct platform_device *pdev;
+};
+
+static char *hisi_pcie_sub_module_name(u8 id)
+{
+ switch (id) {
+ case HISI_PCIE_SUB_MODULE_ID_AP: return "AP Layer";
+ case HISI_PCIE_SUB_MODULE_ID_TL: return "TL Layer";
+ case HISI_PCIE_SUB_MODULE_ID_MAC: return "MAC Layer";
+ case HISI_PCIE_SUB_MODULE_ID_DL: return "DL Layer";
+ case HISI_PCIE_SUB_MODULE_ID_SDI: return "SDI Layer";
+ }
+
+ return "unknown";
+}
+
+static char *hisi_pcie_error_severity(u8 err_sev)
+{
+ switch (err_sev) {
+ case HISI_ERR_SEV_RECOVERABLE: return "recoverable";
+ case HISI_ERR_SEV_FATAL: return "fatal";
+ case HISI_ERR_SEV_CORRECTED: return "corrected";
+ case HISI_ERR_SEV_NONE: return "none";
+ }
+
+ return "unknown";
+}
+
+static int hisi_pcie_port_reset(struct platform_device *pdev,
+ u32 chip_id, u32 port_id)
+{
+ struct device *dev = &pdev->dev;
+ acpi_handle handle = ACPI_HANDLE(dev);
+ union acpi_object arg[3];
+ struct acpi_object_list arg_list;
+ acpi_status s;
+ unsigned long long data = 0;
+
+ arg[0].type = ACPI_TYPE_INTEGER;
+ arg[0].integer.value = chip_id;
+ arg[1].type = ACPI_TYPE_INTEGER;
+ arg[1].integer.value = HISI_PCIE_CORE_ID(port_id);
+ arg[2].type = ACPI_TYPE_INTEGER;
+ arg[2].integer.value = HISI_PCIE_CORE_PORT_ID(port_id);
+
+ arg_list.count = 3;
+ arg_list.pointer = arg;
+
+ s = acpi_evaluate_integer(handle, "RST", &arg_list, &data);
+ if (ACPI_FAILURE(s)) {
+ dev_err(dev, "No RST method\n");
+ return -EIO;
+ }
+
+ if (data) {
+ dev_err(dev, "Failed to Reset\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int hisi_pcie_port_do_recovery(struct platform_device *dev,
+ u32 chip_id, u32 port_id)
+{
+ acpi_status s;
+ struct device *device = &dev->dev;
+ acpi_handle root_handle = ACPI_HANDLE(device);
+ struct acpi_pci_root *pci_root;
+ struct pci_bus *root_bus;
+ struct pci_dev *pdev;
+ u32 domain, busnr, devfn;
+
+ s = acpi_get_parent(root_handle, &root_handle);
+ if (ACPI_FAILURE(s))
+ return -ENODEV;
+ pci_root = acpi_pci_find_root(root_handle);
+ if (!pci_root)
+ return -ENODEV;
+ root_bus = pci_root->bus;
+ domain = pci_root->segment;
+
+ busnr = root_bus->number;
+ devfn = PCI_DEVFN(port_id, 0);
+ pdev = pci_get_domain_bus_and_slot(domain, busnr, devfn);
+ if (!pdev) {
+ dev_info(device, "Fail to get root port %04x:%02x:%02x.%d device\n",
+ domain, busnr, PCI_SLOT(devfn), PCI_FUNC(devfn));
+ return -ENODEV;
+ }
+
+ pci_stop_and_remove_bus_device_locked(pdev);
+ pci_dev_put(pdev);
+
+ if (hisi_pcie_port_reset(dev, chip_id, port_id))
+ return -EIO;
+
+ /*
+ * The initialization time of subordinate devices after
+ * hot reset is no more than 1s, which is required by
+ * the PCI spec v5.0 sec 6.6.1. The time will shorten
+ * if Readiness Notifications mechanisms are used. But
+ * wait 1s here to adapt any conditions.
+ */
+ ssleep(1UL);
+
+ /* add root port and downstream devices */
+ pci_lock_rescan_remove();
+ pci_rescan_bus(root_bus);
+ pci_unlock_rescan_remove();
+
+ return 0;
+}
+
+static void hisi_pcie_handle_error(const struct hisi_pcie_error_data *error,
+ struct platform_device *pdev)
+{
+ char buf[HISI_PCIE_ERR_INFO_SIZE];
+ char *p = buf, *end = buf + sizeof(buf);
+ struct device *dev = &pdev->dev;
+ u32 i;
+ int rc;
+
+ if (error->val_bits == 0) {
+ dev_warn(dev, "%s: no valid error information\n", __func__);
+ return;
+ }
+
+ /* Logging */
+ p += snprintf(p, end - p, "[ Table version=%d ", error->version);
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_SOC_ID)
+ p += snprintf(p, end - p, "SOC ID=%d ", error->soc_id);
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_SOCKET_ID)
+ p += snprintf(p, end - p, "socket ID=%d ", error->socket_id);
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_NIMBUS_ID)
+ p += snprintf(p, end - p, "nimbus ID=%d ", error->nimbus_id);
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_SUB_MODULE_ID)
+ p += snprintf(p, end - p, "sub module=%s ",
+ hisi_pcie_sub_module_name(error->sub_module_id));
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_CORE_ID)
+ p += snprintf(p, end - p, "core ID=core%d ", error->core_id);
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_PORT_ID)
+ p += snprintf(p, end - p, "port ID=port%d ", error->port_id);
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_ERR_SEVERITY)
+ p += snprintf(p, end - p, "error severity=%s ",
+ hisi_pcie_error_severity(error->err_severity));
+
+ if (error->val_bits & HISI_PCIE_LOCAL_VALID_ERR_TYPE)
+ p += snprintf(p, end - p, "error type=0x%x ", error->err_type);
+
+ p += snprintf(p, end - p, "]\n");
+ dev_info(dev, "\nHISI : HIP : PCIe controller error\n");
+ dev_info(dev, "%s\n", buf);
+
+ dev_info(dev, "Reg Dump:\n");
+ for (i = 0; i < HISI_PCIE_ERR_MISC_REGS; i++) {
+ if (error->val_bits &
+ BIT_ULL(HISI_PCIE_LOCAL_VALID_ERR_MISC + i))
+ dev_info(dev,
+ "ERR_MISC_%d=0x%x\n", i, error->err_misc[i]);
+ }
+
+ /* Recovery for the PCIe controller errors */
+ if (error->err_severity == HISI_ERR_SEV_RECOVERABLE) {
+ /* try reset PCI port for the error recovery */
+ rc = hisi_pcie_port_do_recovery(pdev, error->socket_id,
+ HISI_PCIE_PORT_ID(error->core_id, error->port_id));
+ if (rc) {
+ dev_info(dev, "fail to do hisi pcie port reset\n");
+ return;
+ }
+ }
+}
+
+static int hisi_pcie_notify_error(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct acpi_hest_generic_data *gdata = data;
+ const struct hisi_pcie_error_data *error_data =
+ acpi_hest_get_payload(gdata);
+ struct hisi_pcie_error_private *priv =
+ container_of(nb, struct hisi_pcie_error_private, nb);
+ struct platform_device *pdev = priv->pdev;
+ struct device *dev = &pdev->dev;
+ u8 socket;
+
+ if (device_property_read_u8(dev, "socket", &socket))
+ return NOTIFY_DONE;
+
+ if (!guid_equal((guid_t *)gdata->section_type, &hisi_pcie_sec_type) ||
+ error_data->socket_id != socket)
+ return NOTIFY_DONE;
+
+ hisi_pcie_handle_error(error_data, pdev);
+
+ return NOTIFY_OK;
+}
+
+static int hisi_pcie_error_handler_probe(struct platform_device *pdev)
+{
+ struct hisi_pcie_error_private *priv;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->nb.notifier_call = hisi_pcie_notify_error;
+ priv->pdev = pdev;
+ ret = ghes_register_event_notifier(&priv->nb);
+ if (ret) {
+ dev_err(&pdev->dev, "%s : ghes_register_event_notifier fail\n",
+ __func__);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int hisi_pcie_error_handler_remove(struct platform_device *pdev)
+{
+ struct hisi_pcie_error_private *priv = platform_get_drvdata(pdev);
+
+ if (priv)
+ ghes_unregister_event_notifier(&priv->nb);
+
+ kfree(priv);
+
+ return 0;
+}
+
+static const struct acpi_device_id hisi_pcie_acpi_match[] = {
+ { "HISI0361", 0 },
+ { }
+};
+
+static struct platform_driver hisi_pcie_error_handler_driver = {
+ .driver = {
+ .name = "hisi-pcie-error-handler",
+ .acpi_match_table = hisi_pcie_acpi_match,
+ },
+ .probe = hisi_pcie_error_handler_probe,
+ .remove = hisi_pcie_error_handler_remove,
+};
+module_platform_driver(hisi_pcie_error_handler_driver);
+
+MODULE_DESCRIPTION("HiSilicon HIP PCIe controller error handling driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
On Tue, Apr 21, 2020 at 02:21:36PM +0100, Shiju Jose wrote:
> +static int hisi_pcie_error_handler_probe(struct platform_device *pdev)
> +{
> + struct hisi_pcie_error_private *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->nb.notifier_call = hisi_pcie_notify_error;
> + priv->pdev = pdev;
> + ret = ghes_register_event_notifier(&priv->nb);
> + if (ret) {
> + dev_err(&pdev->dev, "%s : ghes_register_event_notifier fail\n",
> + __func__);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +}
> +
> +static int hisi_pcie_error_handler_remove(struct platform_device *pdev)
> +{
> + struct hisi_pcie_error_private *priv = platform_get_drvdata(pdev);
> +
> + if (priv)
> + ghes_unregister_event_notifier(&priv->nb);
Delete this NULL check. The remove function isn't called on stuff which
hasn't been allocated.
The "never free things which haven't been allocated" rule is a good rule
to keep in mind because it makes error handling so much simpler.
> +
> + kfree(priv);
> +
> + return 0;
> +}
regards,
dan carpenter
On Tue, Apr 21, 2020 at 02:21:34PM +0100, Shiju Jose wrote:
> static BLOCKING_NOTIFIER_HEAD(ghes_event_notify_list);
> @@ -670,12 +692,7 @@ static void ghes_do_proc(struct ghes *ghes,
> pr_warn(GHES_PFX "ghes event queue full\n");
> break;
> }
> -
> - if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
> - struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
> -
> - log_arm_hw_error(err);
> - } else {
> + {
> void *err = acpi_hest_get_payload(gdata);
This is ugly. Just move the "void *err;" declaration to the top of the
function so we can delete this block.
>
> log_non_standard_event(sec_type, fru_id, fru_text,
> --
regards,
dan carpenter
Hi Dan,
>-----Original Message-----
>From: Dan Carpenter [mailto:[email protected]]
>Sent: 21 April 2020 15:14
>To: Shiju Jose <[email protected]>
>Cc: [email protected]; [email protected]; linux-
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected];
>[email protected]; [email protected]; Linuxarm
><[email protected]>; Jonathan Cameron
><[email protected]>; tanxiaofei <[email protected]>;
>yangyicong <[email protected]>
>Subject: Re: [RESEND PATCH v7 4/6] ACPI / APEI: Add callback for ARM HW
>errors to the GHES notifier
>
>On Tue, Apr 21, 2020 at 02:21:34PM +0100, Shiju Jose wrote:
>> static BLOCKING_NOTIFIER_HEAD(ghes_event_notify_list);
>> @@ -670,12 +692,7 @@ static void ghes_do_proc(struct ghes *ghes,
>> pr_warn(GHES_PFX "ghes event queue full\n");
>> break;
>> }
>> -
>> - if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
>> - struct cper_sec_proc_arm *err =
>acpi_hest_get_payload(gdata);
>> -
>> - log_arm_hw_error(err);
>> - } else {
>> + {
>> void *err = acpi_hest_get_payload(gdata);
>
>This is ugly. Just move the "void *err;" declaration to the top of the function
>so we can delete this block.
Ok. I will change.
>
>>
>> log_non_standard_event(sec_type, fru_id, fru_text,
>> --
>
>regards,
>dan carpenter
Thanks,
Shiju