EDAC, armv8: Add Cache Error Reporting driver for ARMv8 processors

Interrupt based EDAC driver for ARMv8 processors that implement RAS for error detection of CPU caches and lso allows optional polling of error syndrome registers if interrupts are not supported.

Signed-off-by Kyle Yan <[email protected]>

---
 drivers/edac/Kconfig      |  21 ++
 drivers/edac/Makefile     |   1 +
 drivers/edac/armv8_edac.c | 489 ++++++++++++++++++++++++++++++++++++++++++++++

 3 files changed, 511 insertions(+)
 create mode 100644 drivers/edac/armv8_edac.c

drivers/edac/Kconfig +6 -0

index 96afb2a..47a68e3 100644
@@ -457,4 +457,25 @@ config EDAC_XGENE
 	  Support for error detection and correction on the
 	  APM X-Gene family of SOCs.
 
+config EDAC_ARMV8
+	depends on (ARM || ARM64)
+	tristate "ARMv8 L1/L2/L3/SCU Caches ECC"
+	help
+	   Support for error detection and correction on ARMv8 cores
+	   supporting RAS features. Reports errors caught by ARMv8

drivers/edac/Kconfig +5 -0

         When this is enabled, the polling rate can be set as
+	   a module parameter. By default, it will call the polling function
+	   every second.
+	   This option should only be used if the associated interrupt lines
+	   are not enabled.
+

endif # EDAC

drivers/edac/Makefile +1 -0

index 0fd9ffa..57113ba 100644
@@ -43,6 +43,7 @@ obj-$(CONFIG_EDAC_IE31200)		+= ie31200_edac.o
 obj-$(CONFIG_EDAC_X38)			+= x38_edac.o
 obj-$(CONFIG_EDAC_I82860)		+= i82860_edac.o
 obj-$(CONFIG_EDAC_R82600)		+= r82600_edac.o
+obj-$(CONFIG_EDAC_ARMV8)		+= armv8_edac.o
 
 amd64_edac_mod-y := amd64_edac.o
 amd64_edac_mod-$(CONFIG_EDAC_DEBUG) += amd64_edac_dbg.o

drivers/edac/armv8_edac.c new file +72 -0

new file mode 100644
index 0000000..d986c47
@@ -0,0 +1,489 @@
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/edac.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/cpu_pm.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+
+#include "edac_mc.h"
+#include "edac_device.h"
+
+static int poll_msec = 1000;
+module_param(poll_msec, int, 0444);
+
+static bool panic_on_ue = 0;
+module_param_named(panic_on_ue, panic_on_ue, bool, 0664);
+
+#define L1 0x0
+#define L2 0x1
+#define L3 0x2
+
+#define EDAC_CPU	"armv8_edac"
+
+#define ERRXSTATUS_VALID(a)	((a >> 30) & 0x1)
+#define ERRXSTATUS_UE(a)	((a >> 29) & 0x1)
+#define ERRXSTATUS_SERR(a)	(a & 0xFF)
+
+#define ERRXMISC_LVL(a)		((a >> 1) & 0x7)
+#define ERRXMISC_WAY(a)		((a >> 28) & 0xF)
+
+#define ERRXCTLR_ENABLE		0x10f
+#define ERRXMISC_OVERFLOW	0x7F7F00000000ULL
+
+static inline void set_errxctlr_el1(void)
+{
+	asm volatile("msr s3_0_c5_c4_1, %0" : : "r" (ERRXCTLR_ENABLE));
+}
+
+static inline void set_errxmisc_overflow(void)
+{
+	asm volatile("msr s3_0_c5_c5_0, %0" : : "r" (ERRXMISC_OVERFLOW));
+}
+
+static inline void write_errselr_el1(u64 val)
+{
+	asm volatile("msr s3_0_c5_c3_1, %0" : : "r" (val));
+}
+
+static inline u64 read_errxstatus_el1(void)
+{
+	u64 val;
+
+	asm volatile("mrs %0, s3_0_c5_c4_2" : "=r" (val));
+	return val;
+}
+
+static inline u64 read_errxmisc_el1(void)
+{

drivers/edac/armv8_edac.c new file +51 -0

+
+struct errors_edac {
+	const char * const msg;
+	void (*func)(struct edac_device_ctl_info *edac_dev,
+			int inst_nr, int block_nr, const char *msg);
+};
+
+static const struct errors_edac errors[] = {
+	{ "L1 Correctable Error", edac_device_handle_ce },
+	{ "L1 Uncorrectable Error", edac_device_handle_ue },
+	{ "L2 Correctable Error", edac_device_handle_ce },
+	{ "L2 Uncorrectable Error", edac_device_handle_ue },
+	{ "L3 Correctable Error", edac_device_handle_ce },
+	{ "L3 Uncorrectable Error", edac_device_handle_ue },
+};
+
+#define L1_CE 0
+#define L1_UE 1
+#define L2_CE 2
+#define L2_UE 3
+#define L3_CE 4
+#define L3_UE 5
+
+#define DATA_BUF_ERR		0x2
+#define CACHE_DATA_ERR		0x6
+#define CACHE_TAG_DIRTY_ERR	0x7
+#define TLB_PARITY_ERR_DATA	0x8
+#define TLB_PARITY_ERR_TAG	0x9
+#define BUS_ERROR		0x12
+
+struct erp_drvdata {
+	struct edac_device_ctl_info *edev_ctl;
+	struct erp_drvdata __percpu **erp_cpu_drvdata;
+	struct notifier_block nb_pm;
+	int ppi;
+};
+
+static struct erp_drvdata *panic_handler_drvdata;
+
+static DEFINE_SPINLOCK(armv8_edac_lock);
+
+static void l1_l2_irq_enable(void *info)
+{
+	int irq = *(int *)info;
+
+	enable_percpu_irq(irq, IRQ_TYPE_LEVEL_HIGH);
+}
+
+static int request_erp_irq(struct platform_device *pdev, const char *propname,
+			const char *desc, irq_handler_t handler,
+			void *ed, int percpu)

drivers/edac/armv8_edac.c new file +235 -0

+		goto out;
+	}
+
+	if (!percpu) {
+		rc = devm_request_threaded_irq(&pdev->dev, r->start, NULL,
+					       handler,
+					       IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+					       desc,
+					       ed);
+
+		if (rc) {
+			pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s). Proceeding anyway.\n",
+			       (int) r->start, rc, propname, desc);
+			goto out;
+		}
+
+	} else {
+		drv->erp_cpu_drvdata = alloc_percpu(struct erp_drvdata *);
+		if (!drv->erp_cpu_drvdata) {
+			pr_err("Failed to allocate percpu erp data\n");
+			goto out;
+		}
+
+		*raw_cpu_ptr(drv->erp_cpu_drvdata) = drv;
+		rc = request_percpu_irq(r->start, handler, desc,
+				drv->erp_cpu_drvdata);
+
+		if (rc) {
+			pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s). Proceeding anyway.\n",
+			       (int) r->start, rc, propname, desc);
+			goto out_free;
+		}
+
+		drv->ppi = r->start;
+		on_each_cpu(l1_l2_irq_enable, &(r->start), 1);
+	}
+
+	return 0;
+
+out_free:
+	free_percpu(drv->erp_cpu_drvdata);
+	drv->erp_cpu_drvdata = NULL;
+out:
+	return 1;
+}
+
+static void dump_err_reg(int errorcode, int level, u64 errxstatus, u64 errxmisc,
+	struct edac_device_ctl_info *edev_ctl)
+{
+	edac_printk(KERN_CRIT, EDAC_CPU, "ERRXSTATUS_EL1: %llx\n", errxstatus);
+	edac_printk(KERN_CRIT, EDAC_CPU, "ERRXMISC_EL1: %llx\n", errxmisc);
+	edac_printk(KERN_CRIT, EDAC_CPU, "Cache level: L%d\n", level+1);
+
+	switch (ERRXSTATUS_SERR(errxstatus)) {
+	case DATA_BUF_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from internal data buffer\n");
+		break;
+
+	case CACHE_DATA_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache data RAM\n");
+		break;
+
+	case CACHE_TAG_DIRTY_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache tag or dirty RAM\n");
+		break;
+
+	case TLB_PARITY_ERR_DATA:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB DATA RAM\n");
+		break;
+
+	case TLB_PARITY_ERR_TAG:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB TAG RAM\n");
+		break;
+
+	case BUS_ERROR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Bus Error\n");
+		break;
+	}
+
+	if (level == L3)
+		edac_printk(KERN_CRIT, EDAC_CPU,
+			"Way: %d\n", (int) ERRXMISC_WAY(errxmisc));
+	else
+		edac_printk(KERN_CRIT, EDAC_CPU,
+			"Way: %d\n", (int) ERRXMISC_WAY(errxmisc) >> 2);
+
+	edev_ctl->panic_on_ue = panic_on_ue;
+	errors[errorcode].func(edev_ctl, smp_processor_id(),
+				level, errors[errorcode].msg);
+}
+
+static void armv8_parse_l1_l2_cache_error(u64 errxstatus, u64 errxmisc,
+	struct edac_device_ctl_info *edev_ctl)
+{
+	switch (ERRXMISC_LVL(errxmisc)) {
+	case L1:
+		if (ERRXSTATUS_UE(errxstatus))
+			dump_err_reg(L1_UE, L1, errxstatus, errxmisc,
+					edev_ctl);
+		else
+			dump_err_reg(L1_CE, L1, errxstatus, errxmisc,
+					edev_ctl);
+		break;
+	case L2:
+		if (ERRXSTATUS_UE(errxstatus))
+			dump_err_reg(L2_UE, L2, errxstatus, errxmisc,
+					edev_ctl);
+		else
+			dump_err_reg(L2_CE, L2, errxstatus, errxmisc,
+					edev_ctl);
+		break;
+	default:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Unknown ERRXMISC_LVL value\n");
+	}
+}
+
+static bool armv8_check_l1_l2_ecc(void *info)
+{
+	struct edac_device_ctl_info *edev_ctl = info;
+	u64 errxstatus;
+	u64 errxmisc;
+	unsigned long flags;
+
+	spin_lock_irqsave(&armv8_edac_lock, flags);
+	write_errselr_el1(0);
+	errxstatus = read_errxstatus_el1();
+
+	if (ERRXSTATUS_VALID(errxstatus)) {
+		errxmisc = read_errxmisc_el1();
+		edac_printk(KERN_CRIT, EDAC_CPU,
+		"CPU%d detected a L1/L2 cache error\n",
+		smp_processor_id());
+
+		armv8_parse_l1_l2_cache_error(errxstatus, errxmisc, edev_ctl);
+		clear_errxstatus_valid(errxstatus);
+		spin_unlock_irqrestore(&armv8_edac_lock, flags);
+		return true;
+	}
+	spin_unlock_irqrestore(&armv8_edac_lock, flags);
+	return false;
+}
+
+static bool armv8_check_l3_scu_error(struct edac_device_ctl_info *edev_ctl)
+{
+	u64 errxstatus = 0;
+	u64 errxmisc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&armv8_edac_lock, flags);
+	write_errselr_el1(1);
+	errxstatus = read_errxstatus_el1();
+	errxmisc = read_errxmisc_el1();
+
+	if (ERRXSTATUS_VALID(errxstatus) &&
+		ERRXMISC_LVL(errxmisc) == L3) {
+		if (ERRXSTATUS_UE(errxstatus)) {
+			edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 uncorrectable error\n");
+			dump_err_reg(L3_UE, L3, errxstatus, errxmisc,
+				edev_ctl);
+		} else {
+			edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 correctable error\n");
+			dump_err_reg(L3_CE, L3, errxstatus, errxmisc,
+				edev_ctl);
+		}
+
+		clear_errxstatus_valid(errxstatus);
+		spin_unlock_irqrestore(&armv8_edac_lock, flags);
+		return true;
+	}
+	spin_unlock_irqrestore(&armv8_edac_lock, flags);
+	return false;
+}
+
+static void armv8_check_l1_l2_ecc_helper(void *info)
+{
+	armv8_check_l1_l2_ecc(info);
+}
+
+void armv8_poll_cache_errors(struct edac_device_ctl_info *edev_ctl)
+{
+	int cpu;
+
+	if (!edev_ctl)
+		edev_ctl = panic_handler_drvdata->edev_ctl;
+
+	armv8_check_l3_scu_error(edev_ctl);
+	for_each_possible_cpu(cpu) {
+		smp_call_function_single(cpu, armv8_check_l1_l2_ecc_helper,
+			edev_ctl, 0);
+	}
+}
+
+static irqreturn_t armv8_l1_l2_handler(int irq, void *drvdata)
+{
+	if (armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl))
+		return IRQ_HANDLED;
+	return IRQ_NONE;
+}
+
+static irqreturn_t armv8_l3_scu_handler(int irq, void *drvdata)
+{
+	struct erp_drvdata *drv = drvdata;
+	struct edac_device_ctl_info *edev_ctl = drv->edev_ctl;
+
+	if (armv8_check_l3_scu_error(edev_ctl))
+		return IRQ_HANDLED;
+	return IRQ_NONE;
+}
+
+static void initialize_registers(void *info)
+{
+	set_errxctlr_el1();
+	set_errxmisc_overflow();
+}
+
+static void init_regs_on_cpu(bool all_cpus)
+{
+	int cpu;
+
+	write_errselr_el1(0);
+	if (all_cpus) {
+		for_each_possible_cpu(cpu)
+			smp_call_function_single(cpu, initialize_registers,
+						NULL, 1);
+	} else {
+		initialize_registers(NULL);
+	}
+
+	write_errselr_el1(1);
+	initialize_registers(NULL);
+}
+
+static int armv8_pmu_cpu_pm_notify(struct notifier_block *self,
+				unsigned long action, void *v)
+{

drivers/edac/armv8_edac.c new file +39 -0

+
+static int armv8_cpu_erp_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct erp_drvdata *drv;
+	int rc = 0;
+	int fail = 0;
+
+	init_regs_on_cpu(true);
+
+	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+
+	if (!drv)
+		return -ENOMEM;
+
+	drv->edev_ctl = edac_device_alloc_ctl_info(0, "cpu",
+					num_possible_cpus(), "L", 3, 1, NULL, 0,
+					edac_device_alloc_index());
+
+	if (!drv->edev_ctl)
+		return -ENOMEM;
+
+	if (IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+		drv->edev_ctl->edac_check = armv8_poll_cache_errors;
+		drv->edev_ctl->poll_msec = poll_msec;
+	}
+
+	drv->edev_ctl->dev = dev;
+	drv->edev_ctl->mod_name = dev_name(dev);
+	drv->edev_ctl->dev_name = dev_name(dev);
+	drv->edev_ctl->ctl_name = "cache";
+	drv->edev_ctl->panic_on_ue = panic_on_ue;
+	drv->nb_pm.notifier_call = armv8_pmu_cpu_pm_notify;
+	platform_set_drvdata(pdev, drv);
+
+	rc = edac_device_add_device(drv->edev_ctl);
+	if (rc)
+		goto out_mem;
+

drivers/edac/armv8_edac.c new file +29 -0

+
+		if (fail == of_irq_count(dev->of_node)) {
+			pr_err("ERP: Could not request any IRQs. Giving up.\n");
+			rc = -ENODEV;
+			goto out_dev;
+		}
+	}
+
+	cpu_pm_register_notifier(&(drv->nb_pm));
+
+	return 0;
+
+out_dev:
+	edac_device_del_device(dev);
+out_mem:
+	edac_device_free_ctl_info(drv->edev_ctl);
+	return rc;
+}
+
+static int armv8_cpu_erp_remove(struct platform_device *pdev)
+{
+	struct erp_drvdata *drv = dev_get_drvdata(&pdev->dev);
+	struct edac_device_ctl_info *edac_ctl = drv->edev_ctl;
+
+	if (drv->erp_cpu_drvdata) {
+		free_percpu_irq(drv->ppi, drv->erp_cpu_drvdata);
+		free_percpu(drv->erp_cpu_drvdata);
+	}
+

Hi,

Quoted 3 lines · Kyle Yan

Interrupt based EDAC driver for ARMv8 processors that implement RAS for error detection of CPU caches and lso allows optional polling of error syndrome registers if interrupts are not supported.

Is this using the architectural RAS extensions, or something specific to
QC CPUs? It would be good to call this out explicitly.

If it's the latter, this needs rewording to be clear that this is specific to QC CPUs, and is not a generic ARMv8 feature.

Is there any interaction with SDEI that we need to be aware of?

Quoted 7 lines

Signed-off-by Kyle Yan <[email protected]>

---
 drivers/edac/Kconfig      |  21 ++
 drivers/edac/Makefile     |   1 +
 drivers/edac/armv8_edac.c | 489 

++++++++++++++++++++++++++++++++++++++++++++++

I see that there's DT parsing code, but no binding. Please add one, as per Documentation/devicetree/bindings/submitting-patches.txt.

If this is architectural, how is this expected to work on ACPI systems?

Quoted 24 lines

 3 files changed, 511 insertions(+)
 create mode 100644 drivers/edac/armv8_edac.c

drivers/edac/Kconfig +8 -0

index 96afb2a..47a68e3 100644
@@ -457,4 +457,25 @@ config EDAC_XGENE
 	  Support for error detection and correction on the
 	  APM X-Gene family of SOCs.
 
+config EDAC_ARMV8
+	depends on (ARM || ARM64)
+	tristate "ARMv8 L1/L2/L3/SCU Caches ECC"
+	help
+	   Support for error detection and correction on ARMv8 cores
+	   supporting RAS features. Reports errors caught by ARMv8
+	   ECC mechanism.
+	   For debugging issues having to do with stability and overall 

system

drivers/edac/Kconfig +4 -0

+	   health, you should probably say 'Y' here.
+
+config EDAC_ARMV8_POLL
+	depends on EDAC_ARMV8

drivers/edac/Kconfig

Quoted 5 lines

drivers/edac/Kconfig +3 -0

+	bool "Poll on ARMv8 ECC registers"
+	help
+	   This option chooses whether or not you want to poll on the 

Kryo3xx

drivers/edac/Kconfig +1 -0

+	   ECC registers.

Are these registers specific to Kryo3xx (i.e. IMPLEMENTATION DEFINED), or are they defined by the RAS extensions?

Quoted 99 lines

When this is enabled, the polling rate can be set as

drivers/edac/Kconfig +5 -0

+	   a module parameter. By default, it will call the polling function
+	   every second.
+	   This option should only be used if the associated interrupt lines
+	   are not enabled.
+
 endif # EDAC

drivers/edac/Makefile +1 -0

index 0fd9ffa..57113ba 100644
@@ -43,6 +43,7 @@ obj-$(CONFIG_EDAC_IE31200)		+= ie31200_edac.o
 obj-$(CONFIG_EDAC_X38)			+= x38_edac.o
 obj-$(CONFIG_EDAC_I82860)		+= i82860_edac.o
 obj-$(CONFIG_EDAC_R82600)		+= r82600_edac.o
+obj-$(CONFIG_EDAC_ARMV8)		+= armv8_edac.o
 
 amd64_edac_mod-y := amd64_edac.o
 amd64_edac_mod-$(CONFIG_EDAC_DEBUG) += amd64_edac_dbg.o

drivers/edac/armv8_edac.c new file +1 -0

new file mode 100644
index 0000000..d986c47
@@ -0,0 +1,489 @@
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights 

reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */

drivers/edac/armv8_edac.c new file +61 -0

+
+#include <linux/kernel.h>
+#include <linux/edac.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/cpu_pm.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+
+#include "edac_mc.h"
+#include "edac_device.h"
+
+static int poll_msec = 1000;
+module_param(poll_msec, int, 0444);
+
+static bool panic_on_ue = 0;
+module_param_named(panic_on_ue, panic_on_ue, bool, 0664);
+
+#define L1 0x0
+#define L2 0x1
+#define L3 0x2
+
+#define EDAC_CPU	"armv8_edac"
+
+#define ERRXSTATUS_VALID(a)	((a >> 30) & 0x1)
+#define ERRXSTATUS_UE(a)	((a >> 29) & 0x1)
+#define ERRXSTATUS_SERR(a)	(a & 0xFF)
+
+#define ERRXMISC_LVL(a)		((a >> 1) & 0x7)
+#define ERRXMISC_WAY(a)		((a >> 28) & 0xF)
+
+#define ERRXCTLR_ENABLE		0x10f
+#define ERRXMISC_OVERFLOW	0x7F7F00000000ULL
+
+static inline void set_errxctlr_el1(void)
+{
+	asm volatile("msr s3_0_c5_c4_1, %0" : : "r" (ERRXCTLR_ENABLE));
+}
+
+static inline void set_errxmisc_overflow(void)
+{
+	asm volatile("msr s3_0_c5_c5_0, %0" : : "r" (ERRXMISC_OVERFLOW));
+}
+
+static inline void write_errselr_el1(u64 val)
+{
+	asm volatile("msr s3_0_c5_c3_1, %0" : : "r" (val));
+}
+
+static inline u64 read_errxstatus_el1(void)
+{
+	u64 val;
+
+	asm volatile("mrs %0, s3_0_c5_c4_2" : "=r" (val));
+	return val;
+}
+
+static inline u64 read_errxmisc_el1(void)
+{

drivers/edac/armv8_edac.c line 82

Quoted 10 lines

drivers/edac/armv8_edac.c new file +10 -0

+	u64 val;
+
+	asm volatile("mrs %0, s3_0_c5_c5_0" : "=r" (val));
+	return val;
+}
+
+static inline void clear_errxstatus_valid(u64 val)
+{
+	asm volatile("msr s3_0_c5_c4_2, %0" : : "r" (val));
+}

Please use {read,write}_sysreg().

If these registers are defined by ARMv8, please place definitions in arm64's <asm/sysreg.h>.

Quoted 57 lines

drivers/edac/armv8_edac.c new file +49 -0

+
+struct errors_edac {
+	const char * const msg;
+	void (*func)(struct edac_device_ctl_info *edac_dev,
+			int inst_nr, int block_nr, const char *msg);
+};
+
+static const struct errors_edac errors[] = {
+	{ "L1 Correctable Error", edac_device_handle_ce },
+	{ "L1 Uncorrectable Error", edac_device_handle_ue },
+	{ "L2 Correctable Error", edac_device_handle_ce },
+	{ "L2 Uncorrectable Error", edac_device_handle_ue },
+	{ "L3 Correctable Error", edac_device_handle_ce },
+	{ "L3 Uncorrectable Error", edac_device_handle_ue },
+};
+
+#define L1_CE 0
+#define L1_UE 1
+#define L2_CE 2
+#define L2_UE 3
+#define L3_CE 4
+#define L3_UE 5
+
+#define DATA_BUF_ERR		0x2
+#define CACHE_DATA_ERR		0x6
+#define CACHE_TAG_DIRTY_ERR	0x7
+#define TLB_PARITY_ERR_DATA	0x8
+#define TLB_PARITY_ERR_TAG	0x9
+#define BUS_ERROR		0x12
+
+struct erp_drvdata {
+	struct edac_device_ctl_info *edev_ctl;
+	struct erp_drvdata __percpu **erp_cpu_drvdata;
+	struct notifier_block nb_pm;
+	int ppi;
+};
+
+static struct erp_drvdata *panic_handler_drvdata;
+
+static DEFINE_SPINLOCK(armv8_edac_lock);
+
+static void l1_l2_irq_enable(void *info)
+{
+	int irq = *(int *)info;
+
+	enable_percpu_irq(irq, IRQ_TYPE_LEVEL_HIGH);
+}
+
+static int request_erp_irq(struct platform_device *pdev, const char 

*propname,

drivers/edac/armv8_edac.c new file +7 -0

+			const char *desc, irq_handler_t handler,
+			void *ed, int percpu)
+{
+	int rc;
+	struct resource *r;
+	struct erp_drvdata *drv = ed;
+

drivers/edac/armv8_edac.c line 143

Quoted 6 lines

drivers/edac/armv8_edac.c new file +4 -0

+	r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, propname);
+
+	if (!r) {
+		pr_err("ARMv8 CPU ERP: Could not find <%s> IRQ property. Proceeding 

anyway.\n",

drivers/edac/armv8_edac.c new file +1 -0

+			propname);

What is "ERP" ?

Quoted 250 lines

drivers/edac/armv8_edac.c new file +12 -0

+		goto out;
+	}
+
+	if (!percpu) {
+		rc = devm_request_threaded_irq(&pdev->dev, r->start, NULL,
+					       handler,
+					       IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+					       desc,
+					       ed);
+
+		if (rc) {
+			pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s). 

Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +17 -0

+			       (int) r->start, rc, propname, desc);
+			goto out;
+		}
+
+	} else {
+		drv->erp_cpu_drvdata = alloc_percpu(struct erp_drvdata *);
+		if (!drv->erp_cpu_drvdata) {
+			pr_err("Failed to allocate percpu erp data\n");
+			goto out;
+		}
+
+		*raw_cpu_ptr(drv->erp_cpu_drvdata) = drv;
+		rc = request_percpu_irq(r->start, handler, desc,
+				drv->erp_cpu_drvdata);
+
+		if (rc) {
+			pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s). 

Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +18 -0

+			       (int) r->start, rc, propname, desc);
+			goto out_free;
+		}
+
+		drv->ppi = r->start;
+		on_each_cpu(l1_l2_irq_enable, &(r->start), 1);
+	}
+
+	return 0;
+
+out_free:
+	free_percpu(drv->erp_cpu_drvdata);
+	drv->erp_cpu_drvdata = NULL;
+out:
+	return 1;
+}
+
+static void dump_err_reg(int errorcode, int level, u64 errxstatus, 

u64 errxmisc,

drivers/edac/armv8_edac.c new file +3 -0

+	struct edac_device_ctl_info *edev_ctl)
+{
+	edac_printk(KERN_CRIT, EDAC_CPU, "ERRXSTATUS_EL1: %llx\n", 

errxstatus);

drivers/edac/armv8_edac.c new file +6 -0

+	edac_printk(KERN_CRIT, EDAC_CPU, "ERRXMISC_EL1: %llx\n", errxmisc);
+	edac_printk(KERN_CRIT, EDAC_CPU, "Cache level: L%d\n", level+1);
+
+	switch (ERRXSTATUS_SERR(errxstatus)) {
+	case DATA_BUF_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from internal data 

buffer\n");

drivers/edac/armv8_edac.c new file +4 -0

+		break;
+
+	case CACHE_DATA_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache data 

RAM\n");

drivers/edac/armv8_edac.c new file +4 -0

+		break;
+
+	case CACHE_TAG_DIRTY_ERR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache tag or dirty 

RAM\n");

drivers/edac/armv8_edac.c new file +28 -0

+		break;
+
+	case TLB_PARITY_ERR_DATA:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB DATA RAM\n");
+		break;
+
+	case TLB_PARITY_ERR_TAG:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB TAG RAM\n");
+		break;
+
+	case BUS_ERROR:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Bus Error\n");
+		break;
+	}
+
+	if (level == L3)
+		edac_printk(KERN_CRIT, EDAC_CPU,
+			"Way: %d\n", (int) ERRXMISC_WAY(errxmisc));
+	else
+		edac_printk(KERN_CRIT, EDAC_CPU,
+			"Way: %d\n", (int) ERRXMISC_WAY(errxmisc) >> 2);
+
+	edev_ctl->panic_on_ue = panic_on_ue;
+	errors[errorcode].func(edev_ctl, smp_processor_id(),
+				level, errors[errorcode].msg);
+}
+
+static void armv8_parse_l1_l2_cache_error(u64 errxstatus, u64 

errxmisc,

drivers/edac/armv8_edac.c new file +51 -0

+	struct edac_device_ctl_info *edev_ctl)
+{
+	switch (ERRXMISC_LVL(errxmisc)) {
+	case L1:
+		if (ERRXSTATUS_UE(errxstatus))
+			dump_err_reg(L1_UE, L1, errxstatus, errxmisc,
+					edev_ctl);
+		else
+			dump_err_reg(L1_CE, L1, errxstatus, errxmisc,
+					edev_ctl);
+		break;
+	case L2:
+		if (ERRXSTATUS_UE(errxstatus))
+			dump_err_reg(L2_UE, L2, errxstatus, errxmisc,
+					edev_ctl);
+		else
+			dump_err_reg(L2_CE, L2, errxstatus, errxmisc,
+					edev_ctl);
+		break;
+	default:
+		edac_printk(KERN_CRIT, EDAC_CPU, "Unknown ERRXMISC_LVL value\n");
+	}
+}
+
+static bool armv8_check_l1_l2_ecc(void *info)
+{
+	struct edac_device_ctl_info *edev_ctl = info;
+	u64 errxstatus;
+	u64 errxmisc;
+	unsigned long flags;
+
+	spin_lock_irqsave(&armv8_edac_lock, flags);
+	write_errselr_el1(0);
+	errxstatus = read_errxstatus_el1();
+
+	if (ERRXSTATUS_VALID(errxstatus)) {
+		errxmisc = read_errxmisc_el1();
+		edac_printk(KERN_CRIT, EDAC_CPU,
+		"CPU%d detected a L1/L2 cache error\n",
+		smp_processor_id());
+
+		armv8_parse_l1_l2_cache_error(errxstatus, errxmisc, edev_ctl);
+		clear_errxstatus_valid(errxstatus);
+		spin_unlock_irqrestore(&armv8_edac_lock, flags);
+		return true;
+	}
+	spin_unlock_irqrestore(&armv8_edac_lock, flags);
+	return false;
+}
+
+static bool armv8_check_l3_scu_error(struct edac_device_ctl_info 

*edev_ctl)

drivers/edac/armv8_edac.c new file +14 -0

+{
+	u64 errxstatus = 0;
+	u64 errxmisc = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&armv8_edac_lock, flags);
+	write_errselr_el1(1);
+	errxstatus = read_errxstatus_el1();
+	errxmisc = read_errxmisc_el1();
+
+	if (ERRXSTATUS_VALID(errxstatus) &&
+		ERRXMISC_LVL(errxmisc) == L3) {
+		if (ERRXSTATUS_UE(errxstatus)) {
+			edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 uncorrectable 

error\n");

drivers/edac/armv8_edac.c new file +4 -0

+			dump_err_reg(L3_UE, L3, errxstatus, errxmisc,
+				edev_ctl);
+		} else {
+			edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 correctable 

error\n");

drivers/edac/armv8_edac.c new file +78 -0

+			dump_err_reg(L3_CE, L3, errxstatus, errxmisc,
+				edev_ctl);
+		}
+
+		clear_errxstatus_valid(errxstatus);
+		spin_unlock_irqrestore(&armv8_edac_lock, flags);
+		return true;
+	}
+	spin_unlock_irqrestore(&armv8_edac_lock, flags);
+	return false;
+}
+
+static void armv8_check_l1_l2_ecc_helper(void *info)
+{
+	armv8_check_l1_l2_ecc(info);
+}
+
+void armv8_poll_cache_errors(struct edac_device_ctl_info *edev_ctl)
+{
+	int cpu;
+
+	if (!edev_ctl)
+		edev_ctl = panic_handler_drvdata->edev_ctl;
+
+	armv8_check_l3_scu_error(edev_ctl);
+	for_each_possible_cpu(cpu) {
+		smp_call_function_single(cpu, armv8_check_l1_l2_ecc_helper,
+			edev_ctl, 0);
+	}
+}
+
+static irqreturn_t armv8_l1_l2_handler(int irq, void *drvdata)
+{
+	if (armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl))
+		return IRQ_HANDLED;
+	return IRQ_NONE;
+}
+
+static irqreturn_t armv8_l3_scu_handler(int irq, void *drvdata)
+{
+	struct erp_drvdata *drv = drvdata;
+	struct edac_device_ctl_info *edev_ctl = drv->edev_ctl;
+
+	if (armv8_check_l3_scu_error(edev_ctl))
+		return IRQ_HANDLED;
+	return IRQ_NONE;
+}
+
+static void initialize_registers(void *info)
+{
+	set_errxctlr_el1();
+	set_errxmisc_overflow();
+}
+
+static void init_regs_on_cpu(bool all_cpus)
+{
+	int cpu;
+
+	write_errselr_el1(0);
+	if (all_cpus) {
+		for_each_possible_cpu(cpu)
+			smp_call_function_single(cpu, initialize_registers,
+						NULL, 1);
+	} else {
+		initialize_registers(NULL);
+	}
+
+	write_errselr_el1(1);
+	initialize_registers(NULL);
+}
+
+static int armv8_pmu_cpu_pm_notify(struct notifier_block *self,
+				unsigned long action, void *v)
+{
+	switch (action) {
+	case CPU_PM_EXIT:
+		init_regs_on_cpu(false);
+		armv8_check_l3_scu_error(panic_handler_drvdata->edev_ctl);

drivers/edac/armv8_edac.c line 388

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+		armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl);
+		break;
+	}
+
+	return NOTIFY_OK;
+}

What about CPU hotplug?

Quoted 44 lines

drivers/edac/armv8_edac.c new file +44 -0

+
+static int armv8_cpu_erp_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct erp_drvdata *drv;
+	int rc = 0;
+	int fail = 0;
+
+	init_regs_on_cpu(true);
+
+	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+
+	if (!drv)
+		return -ENOMEM;
+
+	drv->edev_ctl = edac_device_alloc_ctl_info(0, "cpu",
+					num_possible_cpus(), "L", 3, 1, NULL, 0,
+					edac_device_alloc_index());
+
+	if (!drv->edev_ctl)
+		return -ENOMEM;
+
+	if (IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+		drv->edev_ctl->edac_check = armv8_poll_cache_errors;
+		drv->edev_ctl->poll_msec = poll_msec;
+	}
+
+	drv->edev_ctl->dev = dev;
+	drv->edev_ctl->mod_name = dev_name(dev);
+	drv->edev_ctl->dev_name = dev_name(dev);
+	drv->edev_ctl->ctl_name = "cache";
+	drv->edev_ctl->panic_on_ue = panic_on_ue;
+	drv->nb_pm.notifier_call = armv8_pmu_cpu_pm_notify;
+	platform_set_drvdata(pdev, drv);
+
+	rc = edac_device_add_device(drv->edev_ctl);
+	if (rc)
+		goto out_mem;
+
+	panic_handler_drvdata = drv;
+
+	if (!IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+		fail += request_erp_irq(pdev, "l1-l2-irq",
+				"l1_l2_irq",

drivers/edac/armv8_edac.c line 437

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+				armv8_l1_l2_handler, drv, 1);
+
+		fail += request_erp_irq(pdev, "l3-scu-irq",
+				"l3_scu_irq",
+				armv8_l3_scu_handler, drv, 0);

SCU isn't an architectural concept, and a combined l1-l2 interrupt sounds very specific to a particular implementation.

My understanding was that with the RAS extensions, SError would be used to notify on RAS conditions, so I'm confused as to why we'd need regular interrupts here.

Quoted 34 lines

drivers/edac/armv8_edac.c new file +34 -0

+
+		if (fail == of_irq_count(dev->of_node)) {
+			pr_err("ERP: Could not request any IRQs. Giving up.\n");
+			rc = -ENODEV;
+			goto out_dev;
+		}
+	}
+
+	cpu_pm_register_notifier(&(drv->nb_pm));
+
+	return 0;
+
+out_dev:
+	edac_device_del_device(dev);
+out_mem:
+	edac_device_free_ctl_info(drv->edev_ctl);
+	return rc;
+}
+
+static int armv8_cpu_erp_remove(struct platform_device *pdev)
+{
+	struct erp_drvdata *drv = dev_get_drvdata(&pdev->dev);
+	struct edac_device_ctl_info *edac_ctl = drv->edev_ctl;
+
+	if (drv->erp_cpu_drvdata) {
+		free_percpu_irq(drv->ppi, drv->erp_cpu_drvdata);
+		free_percpu(drv->erp_cpu_drvdata);
+	}
+
+	edac_device_del_device(edac_ctl->dev);
+	edac_device_free_ctl_info(edac_ctl);
+
+	return 0;
+}

drivers/edac/armv8_edac.c line 476

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+
+static const struct of_device_id armv8_cpu_erp_match_table[] = {
+	{ .compatible = "arm,armv8-cpu-erp" },
+	{ }
+};

This needs a binding document, laying out precisely what this is intended to describe.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 5 lines · Mark Rutland

Quoted 3 lines · Kyle Yan

Interrupt based EDAC driver for ARMv8 processors that implement RAS for error detection of CPU caches and lso allows optional polling of error syndrome registers if interrupts are not supported.

Quoted 5 lines

Is this using the architectural RAS extensions, or something specific to QC CPUs? It would be good to call this out explicitly.

If it's the latter, this needs rewording to be clear that this is specific to QC CPUs, and is not a generic ARMv8 feature.

This is not specific to QC CPUs and looks at the system registers defined by the RAS extensions.

Quoted 2 lines

If this is architectural, how is this expected to work on ACPI systems?

The current design of this driver has only been tested/used on mobile devices so I do not yet know how it will work on ACPI systems.

Quoted 10 lines

drivers/edac/Kconfig config EDAC_XGENE · line 461

Quoted 10 lines

drivers/edac/Kconfig +2 -0

index 96afb2a..47a68e3 100644
@@ -457,4 +457,25 @@ config EDAC_XGENE
       Support for error detection and correction on the
       APM X-Gene family of SOCs.
 
+config EDAC_ARMV8
+    depends on (ARM || ARM64)

Quoted 9 lines

Quoted 9 lines

drivers/edac/Kconfig +9 -0

+    tristate "ARMv8 L1/L2/L3/SCU Caches ECC"
+    help
+       Support for error detection and correction on ARMv8 cores
+       supporting RAS features. Reports errors caught by ARMv8
+       ECC mechanism.
+       For debugging issues having to do with stability and overall system
+       health, you should probably say 'Y' here.
+
+config EDAC_ARMV8_POLL

Quoted 9 lines

drivers/edac/Kconfig config EDAC_XGENE · line 475

Quoted 5 lines

drivers/edac/Kconfig +5 -0

+    depends on EDAC_ARMV8
+    bool "Poll on ARMv8 ECC registers"
+    help
+       This option chooses whether or not you want to poll on the Kryo3xx
+       ECC registers.

Are these registers specific to Kryo3xx (i.e. IMPLEMENTATION DEFINED), or are they defined by the RAS extensions?

Oops! Typo here. The registers are defined by RAS extensions.

Quoted 3 lines

Quoted 3 lines

drivers/edac/Kconfig +2 -0

         When this is enabled, the polling rate can be set as
+       a module parameter. By default, it will call the polling function
+       every second.

Quoted 2 lines

drivers/edac/Kconfig config EDAC_XGENE · line 480

Quoted 2 lines

drivers/edac/Kconfig +2 -0

+       This option should only be used if the associated interrupt lines
+       are not enabled.

Quoted 19 lines

Quoted 19 lines

drivers/edac/armv8_edac.c new file +13 -0

new file mode 100644
index 0000000..d986c47
@@ -0,0 +1,489 @@
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>

Quoted 8 lines

drivers/edac/armv8_edac.c line 21

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+#include <linux/edac.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/cpu_pm.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>

Quoted 12 lines

Quoted 12 lines

drivers/edac/armv8_edac.c new file +12 -0

+#include "edac_mc.h"
+#include "edac_device.h"
+
+static int poll_msec = 1000;
+module_param(poll_msec, int, 0444);
+
+static bool panic_on_ue = 0;
+module_param_named(panic_on_ue, panic_on_ue, bool, 0664);
+
+#define L1 0x0
+#define L2 0x1
+#define L3 0x2

Quoted 8 lines

drivers/edac/armv8_edac.c line 41

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+#define EDAC_CPU    "armv8_edac"
+
+#define ERRXSTATUS_VALID(a)    ((a >> 30) & 0x1)
+#define ERRXSTATUS_UE(a)    ((a >> 29) & 0x1)
+#define ERRXSTATUS_SERR(a)    (a & 0xFF)
+
+#define ERRXMISC_LVL(a)        ((a >> 1) & 0x7)
+#define ERRXMISC_WAY(a)        ((a >> 28) & 0xF)

Quoted 1 line

drivers/edac/armv8_edac.c line 42

Quoted 1 line

drivers/edac/armv8_edac.c new file +1 -0

+#define ERRXCTLR_ENABLE        0x10f

Quoted 1 line

drivers/edac/armv8_edac.c line 43

Quoted 1 line

drivers/edac/armv8_edac.c new file +1 -0

+#define ERRXMISC_OVERFLOW    0x7F7F00000000ULL

Quoted 8 lines

drivers/edac/armv8_edac.c line 47

Quoted 4 lines

drivers/edac/armv8_edac.c new file +4 -0

+static inline void set_errxctlr_el1(void)
+{
+    asm volatile("msr s3_0_c5_c4_1, %0" : : "r" (ERRXCTLR_ENABLE));
+}

Please use {read,write}_sysreg().

Quoted 3 lines

If these registers are defined by ARMv8, please place definitions in arm64's <asm/sysreg.h>.

Will do.

Quoted 16 lines

Quoted 16 lines

drivers/edac/armv8_edac.c new file +16 -0

+
+struct errors_edac {
+    const char * const msg;
+    void (*func)(struct edac_device_ctl_info *edac_dev,
+            int inst_nr, int block_nr, const char *msg);
+};
+
+static const struct errors_edac errors[] = {
+    { "L1 Correctable Error", edac_device_handle_ce },
+    { "L1 Uncorrectable Error", edac_device_handle_ue },
+    { "L2 Correctable Error", edac_device_handle_ce },
+    { "L2 Uncorrectable Error", edac_device_handle_ue },
+    { "L3 Correctable Error", edac_device_handle_ce },
+    { "L3 Uncorrectable Error", edac_device_handle_ue },
+};
+

Quoted 6 lines

drivers/edac/armv8_edac.c line 69

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+#define L1_CE 0
+#define L1_UE 1
+#define L2_CE 2
+#define L2_UE 3
+#define L3_CE 4
+#define L3_UE 5

Quoted 5 lines

drivers/edac/armv8_edac.c line 74

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+#define DATA_BUF_ERR        0x2
+#define CACHE_DATA_ERR        0x6
+#define CACHE_TAG_DIRTY_ERR    0x7
+#define TLB_PARITY_ERR_DATA    0x8
+#define TLB_PARITY_ERR_TAG    0x9

Quoted 1 line

drivers/edac/armv8_edac.c line 75

Quoted 1 line

drivers/edac/armv8_edac.c new file +1 -0

+#define BUS_ERROR        0x12

Quoted 9 lines

Quoted 9 lines

drivers/edac/armv8_edac.c new file +9 -0

+struct erp_drvdata {
+    struct edac_device_ctl_info *edev_ctl;
+    struct erp_drvdata __percpu **erp_cpu_drvdata;
+    struct notifier_block nb_pm;
+    int ppi;
+};
+
+static struct erp_drvdata *panic_handler_drvdata;
+

Quoted 8 lines

drivers/edac/armv8_edac.c line 92

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+static DEFINE_SPINLOCK(armv8_edac_lock);
+
+static void l1_l2_irq_enable(void *info)
+{
+    int irq = *(int *)info;
+
+    enable_percpu_irq(irq, IRQ_TYPE_LEVEL_HIGH);
+}

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+static int request_erp_irq(struct platform_device *pdev, const char *propname,
+            const char *desc, irq_handler_t handler,
+            void *ed, int percpu)
+{
+    int rc;
+    struct resource *r;
+    struct erp_drvdata *drv = ed;
+

Quoted 8 lines

Quoted 6 lines

drivers/edac/armv8_edac.c new file +4 -0

+    r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, propname);
+
+    if (!r) {
+        pr_err("ARMv8 CPU ERP: Could not find <%s> IRQ property. Proceeding

anyway.\n",
+ propname);

What is "ERP" ?

Error Reporting. I may just rename it to EDAC or list it out in Documentation.

Quoted 17 lines

Quoted 17 lines

drivers/edac/armv8_edac.c new file +12 -0

+        goto out;
+    }
+
+    if (!percpu) {
+        rc = devm_request_threaded_irq(&pdev->dev, r->start, NULL,
+                           handler,
+                           IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+                           desc,
+                           ed);
+
+        if (rc) {
+            pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s).

Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +4 -0

+                   (int) r->start, rc, propname, desc);
+            goto out;
+        }
+

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    } else {
+        drv->erp_cpu_drvdata = alloc_percpu(struct erp_drvdata *);
+        if (!drv->erp_cpu_drvdata) {
+            pr_err("Failed to allocate percpu erp data\n");
+            goto out;
+        }
+
+        *raw_cpu_ptr(drv->erp_cpu_drvdata) = drv;

Quoted 73 lines

Quoted 73 lines

drivers/edac/armv8_edac.c new file +5 -0

+        rc = request_percpu_irq(r->start, handler, desc,
+                drv->erp_cpu_drvdata);
+
+        if (rc) {
+            pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s / %s).

Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +18 -0

+                   (int) r->start, rc, propname, desc);
+            goto out_free;
+        }
+
+        drv->ppi = r->start;
+        on_each_cpu(l1_l2_irq_enable, &(r->start), 1);
+    }
+
+    return 0;
+
+out_free:
+    free_percpu(drv->erp_cpu_drvdata);
+    drv->erp_cpu_drvdata = NULL;
+out:
+    return 1;
+}
+
+static void dump_err_reg(int errorcode, int level, u64 errxstatus, u64

errxmisc,

drivers/edac/armv8_edac.c new file +8 -0

+    struct edac_device_ctl_info *edev_ctl)
+{
+    edac_printk(KERN_CRIT, EDAC_CPU, "ERRXSTATUS_EL1: %llx\n", errxstatus);
+    edac_printk(KERN_CRIT, EDAC_CPU, "ERRXMISC_EL1: %llx\n", errxmisc);
+    edac_printk(KERN_CRIT, EDAC_CPU, "Cache level: L%d\n", level+1);
+    switch (ERRXSTATUS_SERR(errxstatus)) {
+    case DATA_BUF_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from internal data

buffer\n");

drivers/edac/armv8_edac.c new file +8 -0

+        break;
+
+    case CACHE_DATA_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache data RAM\n");
+        break;
+
+    case CACHE_TAG_DIRTY_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache tag or dirty

RAM\n");

drivers/edac/armv8_edac.c new file +29 -0

+        break;
+
+    case TLB_PARITY_ERR_DATA:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB DATA RAM\n");
+        break;
+
+    case TLB_PARITY_ERR_TAG:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB TAG RAM\n");
+        break;
+
+    case BUS_ERROR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Bus Error\n");
+        break;
+    }
 
+    if (level == L3)
+        edac_printk(KERN_CRIT, EDAC_CPU,
+            "Way: %d\n", (int) ERRXMISC_WAY(errxmisc));
+    else
+        edac_printk(KERN_CRIT, EDAC_CPU,
+            "Way: %d\n", (int) ERRXMISC_WAY(errxmisc) >> 2);
+
+    edev_ctl->panic_on_ue = panic_on_ue;
+    errors[errorcode].func(edev_ctl, smp_processor_id(),
+                level, errors[errorcode].msg);
+}
+
+static void armv8_parse_l1_l2_cache_error(u64 errxstatus, u64 errxmisc,
+    struct edac_device_ctl_info *edev_ctl)
+{

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    switch (ERRXMISC_LVL(errxmisc)) {
+    case L1:
+        if (ERRXSTATUS_UE(errxstatus))
+            dump_err_reg(L1_UE, L1, errxstatus, errxmisc,
+                    edev_ctl);
+        else
+            dump_err_reg(L1_CE, L1, errxstatus, errxmisc,
+                    edev_ctl);

Quoted 22 lines

Quoted 22 lines

drivers/edac/armv8_edac.c new file +22 -0

+        break;
+    case L2:
+        if (ERRXSTATUS_UE(errxstatus))
+            dump_err_reg(L2_UE, L2, errxstatus, errxmisc,
+                    edev_ctl);
+        else
+            dump_err_reg(L2_CE, L2, errxstatus, errxmisc,
+                    edev_ctl);
+        break;
+    default:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Unknown ERRXMISC_LVL value\n");
+    }
+}
+
+static bool armv8_check_l1_l2_ecc(void *info)
+{
+    struct edac_device_ctl_info *edev_ctl = info;
+    u64 errxstatus;
+    u64 errxmisc;
+    unsigned long flags;
+
+    spin_lock_irqsave(&armv8_edac_lock, flags);

Quoted 1 line

Quoted 1 line

+ write_errselr_el1(0);

Quoted 24 lines

Quoted 24 lines

drivers/edac/armv8_edac.c new file +24 -0

+    errxstatus = read_errxstatus_el1();
+
+    if (ERRXSTATUS_VALID(errxstatus)) {
+        errxmisc = read_errxmisc_el1();
+        edac_printk(KERN_CRIT, EDAC_CPU,
+        "CPU%d detected a L1/L2 cache error\n",
+        smp_processor_id());
+
+        armv8_parse_l1_l2_cache_error(errxstatus, errxmisc, edev_ctl);
+        clear_errxstatus_valid(errxstatus);
+        spin_unlock_irqrestore(&armv8_edac_lock, flags);
+        return true;
+    }
+    spin_unlock_irqrestore(&armv8_edac_lock, flags);
+    return false;
+}
+
+static bool armv8_check_l3_scu_error(struct edac_device_ctl_info *edev_ctl)
+{
+    u64 errxstatus = 0;
+    u64 errxmisc = 0;
+    unsigned long flags;
+
+    spin_lock_irqsave(&armv8_edac_lock, flags);

Quoted 1 line

Quoted 1 line

+ write_errselr_el1(1);

Quoted 7 lines

Quoted 7 lines

drivers/edac/armv8_edac.c new file +7 -0

+    errxstatus = read_errxstatus_el1();
+    errxmisc = read_errxmisc_el1();
+
+    if (ERRXSTATUS_VALID(errxstatus) &&
+        ERRXMISC_LVL(errxmisc) == L3) {
+        if (ERRXSTATUS_UE(errxstatus)) {
+            edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 uncorrectable

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

error\n");

drivers/edac/armv8_edac.c new file +4 -0

+            dump_err_reg(L3_UE, L3, errxstatus, errxmisc,
+                edev_ctl);
+        } else {
+            edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 correctable

error\n");

drivers/edac/armv8_edac.c new file +2 -0

+            dump_err_reg(L3_CE, L3, errxstatus, errxmisc,
+                edev_ctl);

Quoted 18 lines

Quoted 18 lines

drivers/edac/armv8_edac.c new file +18 -0

+        }
+
+        clear_errxstatus_valid(errxstatus);
+        spin_unlock_irqrestore(&armv8_edac_lock, flags);
+        return true;
+    }
+    spin_unlock_irqrestore(&armv8_edac_lock, flags);
+    return false;
+}
+
+static void armv8_check_l1_l2_ecc_helper(void *info)
+{
+    armv8_check_l1_l2_ecc(info);
+}
+
+void armv8_poll_cache_errors(struct edac_device_ctl_info *edev_ctl)
+{
+    int cpu;

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+
+    if (!edev_ctl)
+        edev_ctl = panic_handler_drvdata->edev_ctl;
+
+    armv8_check_l3_scu_error(edev_ctl);
+    for_each_possible_cpu(cpu) {
+        smp_call_function_single(cpu, armv8_check_l1_l2_ecc_helper,
+            edev_ctl, 0);

Quoted 23 lines

Quoted 23 lines

drivers/edac/armv8_edac.c new file +23 -0

+    }
+}
+
+static irqreturn_t armv8_l1_l2_handler(int irq, void *drvdata)
+{
+    if (armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl))
+        return IRQ_HANDLED;
+    return IRQ_NONE;
+}
+
+static irqreturn_t armv8_l3_scu_handler(int irq, void *drvdata)
+{
+    struct erp_drvdata *drv = drvdata;
+    struct edac_device_ctl_info *edev_ctl = drv->edev_ctl;
+
+    if (armv8_check_l3_scu_error(edev_ctl))
+        return IRQ_HANDLED;
+    return IRQ_NONE;
+}
+
+static void initialize_registers(void *info)
+{
+    set_errxctlr_el1();

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    set_errxmisc_overflow();
+}
+
+static void init_regs_on_cpu(bool all_cpus)
+{
+    int cpu;
+
+    write_errselr_el1(0);

Quoted 4 lines

drivers/edac/armv8_edac.c

Quoted 4 lines

drivers/edac/armv8_edac.c new file +4 -0

+    if (all_cpus) {
+        for_each_possible_cpu(cpu)
+            smp_call_function_single(cpu, initialize_registers,
+                        NULL, 1);

Quoted 6 lines

drivers/edac/armv8_edac.c

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+    } else {
+        initialize_registers(NULL);
+    }
+
+    write_errselr_el1(1);
+    initialize_registers(NULL);

Quoted 9 lines

Quoted 9 lines

drivers/edac/armv8_edac.c new file +9 -0

+}
+
+static int armv8_pmu_cpu_pm_notify(struct notifier_block *self,
+                unsigned long action, void *v)
+{
+    switch (action) {
+    case CPU_PM_EXIT:
+        init_regs_on_cpu(false);
+        armv8_check_l3_scu_error(panic_handler_drvdata->edev_ctl);

Quoted 9 lines

drivers/edac/armv8_edac.c

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+        armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl);
+        break;
+    }
+
+    return NOTIFY_OK;
+}

What about CPU hotplug?

Agreed that CPU hotplug will be required for the small window between hotplugging back in and LPM exit.

Quoted 32 lines

Quoted 32 lines

drivers/edac/armv8_edac.c new file +31 -0

+
+static int armv8_cpu_erp_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct erp_drvdata *drv;
+    int rc = 0;
+    int fail = 0;
+
+    init_regs_on_cpu(true);
+
+    drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+
+    if (!drv)
+        return -ENOMEM;
+
+    drv->edev_ctl = edac_device_alloc_ctl_info(0, "cpu",
+                    num_possible_cpus(), "L", 3, 1, NULL, 0,
+                    edac_device_alloc_index());
+
+    if (!drv->edev_ctl)
+        return -ENOMEM;
+
+    if (IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+        drv->edev_ctl->edac_check = armv8_poll_cache_errors;
+        drv->edev_ctl->poll_msec = poll_msec;
+    }
+
+    drv->edev_ctl->dev = dev;
+    drv->edev_ctl->mod_name = dev_name(dev);
+    drv->edev_ctl->dev_name = dev_name(dev);
 
+    drv->edev_ctl->ctl_name = "cache";

Quoted 10 lines

Quoted 10 lines

drivers/edac/armv8_edac.c new file +10 -0

+    drv->edev_ctl->panic_on_ue = panic_on_ue;
+    drv->nb_pm.notifier_call = armv8_pmu_cpu_pm_notify;
+    platform_set_drvdata(pdev, drv);
+
+    rc = edac_device_add_device(drv->edev_ctl);
+    if (rc)
+        goto out_mem;
+
+    panic_handler_drvdata = drv;
+

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    if (!IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+        fail += request_erp_irq(pdev, "l1-l2-irq",
+                "l1_l2_irq",
+                armv8_l1_l2_handler, drv, 1);
+
+        fail += request_erp_irq(pdev, "l3-scu-irq",
+                "l3_scu_irq",
+                armv8_l3_scu_handler, drv, 0);

Quoted 2 lines

SCU isn't an architectural concept, and a combined l1-l2 interrupt sounds very specific to a particular implementation.

Can do a rename to something more akin to "private_cache_irq" and "shared_cache_irq".

Quoted 3 lines

My understanding was that with the RAS extensions, SError would be used to notify on RAS conditions, so I'm confused as to why we'd need regular interrupts here.

There may be cases where interrupts are preferred. To my knowledge (though I may mistaken), SErrors would not catch correctable errors?

Quoted 33 lines

Quoted 33 lines

drivers/edac/armv8_edac.c new file +33 -0

+        if (fail == of_irq_count(dev->of_node)) {
+            pr_err("ERP: Could not request any IRQs. Giving up.\n");
+            rc = -ENODEV;
+            goto out_dev;
+        }
+    }
+
+    cpu_pm_register_notifier(&(drv->nb_pm));
+
+    return 0;
+
+out_dev:
+    edac_device_del_device(dev);
+out_mem:
+    edac_device_free_ctl_info(drv->edev_ctl);
+    return rc;
+}
+
+static int armv8_cpu_erp_remove(struct platform_device *pdev)
+{
+    struct erp_drvdata *drv = dev_get_drvdata(&pdev->dev);
+    struct edac_device_ctl_info *edac_ctl = drv->edev_ctl;
+
+    if (drv->erp_cpu_drvdata) {
+        free_percpu_irq(drv->ppi, drv->erp_cpu_drvdata);
+        free_percpu(drv->erp_cpu_drvdata);
+    }
+
+    edac_device_del_device(edac_ctl->dev);
+    edac_device_free_ctl_info(edac_ctl);
+
+    return 0;
+}

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+
+static const struct of_device_id armv8_cpu_erp_match_table[] = {
+    { .compatible = "arm,armv8-cpu-erp" },
+    { }
+};

This needs a binding document, laying out precisely what this is intended to describe.

Hi Kyle,

(Thanks for looping me in Mark!)

Quoted 11 lines · Kyle Yan

Quoted 10 lines · Mark Rutland

Quoted 4 lines · Kyle Yan

Interrupt based EDAC driver for ARMv8 processors that implement RAS for error detection of CPU caches and lso allows optional polling
of error syndrome registers if interrupts are not supported.

Is this using the architectural RAS extensions, or something specific to
QC CPUs? It would be good to call this out explicitly.

Quoted 6 lines

Quoted 2 lines

If it's the latter, this needs rewording to be clear that this is specific to QC CPUs, and is not a generic ARMv8 feature.

This is not specific to QC CPUs and looks at the system registers defined by the
RAS extensions.

Excellent, so it should work on the FVP too!

[...]

Quoted 6 lines

Quoted 3 lines

If this is architectural, how is this expected to work on ACPI systems?

The current design of this driver has only been tested/used on mobile devices so
I do not yet know how it will work on ACPI systems.

For ACPI systems wanting to do kernel-first I assume there will need to be some
description of where the mmio RAS nodes are in the address space and what
interrupts etc they generate. This would be equivalent to the data in the DT.

How come you don't read ERRIDR to find out how many nodes are behind the CPU
interface registers? Surely you have to poll all of them?

If you're polling this, you must have to poke the system register every-second
on every-CPU. Wouldn't it be better to use the memory-mapped interface on one
CPU to do this?

You aren't touching ERR<n>FR at all, this tells us whether the node(?) supports
error recovery interrupts, counter formats etc.

Quoted 10 lines

Quoted 10 lines

drivers/edac/Kconfig config EDAC_XGENE · line 461

Quoted 10 lines

drivers/edac/Kconfig +2 -0

index 96afb2a..47a68e3 100644
@@ -457,4 +457,25 @@ config EDAC_XGENE
       Support for error detection and correction on the
       APM X-Gene family of SOCs.
 
+config EDAC_ARMV8
+    depends on (ARM || ARM64)

As you are accessing system registers this should depend on 'ARM64_RAS_EXTN'
too, and register accesses need to be behind something like cpus_have_const_cap(ARM64_RAS_EXTN).

The patch that adds these bits is here:
https://www.spinics.net/lists/arm-kernel/msg628997.html

(I don't know how cpufeature detection works on 32bit)

Quoted 21 lines

Quoted 12 lines

Quoted 12 lines

drivers/edac/Kconfig +6 -0

+    tristate "ARMv8 L1/L2/L3/SCU Caches ECC"
+    help
+       Support for error detection and correction on ARMv8 cores
+       supporting RAS features. Reports errors caught by ARMv8
+       ECC mechanism.
+       For debugging issues having to do with stability and overall 

system

drivers/edac/Kconfig +5 -0

+       health, you should probably say 'Y' here.
+
+config EDAC_ARMV8_POLL
+    depends on EDAC_ARMV8
+    bool "Poll on ARMv8 ECC registers"

Quoted 9 lines

Quoted 4 lines

drivers/edac/Kconfig +2 -0

+    help
+       This option chooses whether or not you want to poll on the 

Kryo3xx
+ ECC registers.

Are these registers specific to Kryo3xx (i.e. IMPLEMENTATION DEFINED),
or are they defined by the RAS extensions?

Quoted 6 lines

Oops! Typo here. The registers are defined by RAS extensions.

Quoted 4 lines

Quoted 4 lines

When this is enabled, the polling rate can be set as
+ a module parameter. By default, it will call the polling
function
+ every second.

ACPI's firmware-first polling mechanism allows firmware to specify the polling
rate. How was 'every second' picked? Does it depend on the SoC?

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

+ This option should only be used if the associated interrupt
lines
+ are not enabled.

How does the user know?

Quoted 22 lines

Quoted 22 lines

Quoted 22 lines

drivers/edac/armv8_edac.c new file +1 -0

new file mode 100644
index 0000000..d986c47
@@ -0,0 +1,489 @@
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights 

reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */

drivers/edac/armv8_edac.c new file +2 -0

+
+#include <linux/kernel.h>

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+#include <linux/edac.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/cpu_pm.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>

(Nit: alphabetical order makes for fewer conflicts in the future)

Quoted 13 lines

Quoted 12 lines

Quoted 12 lines

drivers/edac/armv8_edac.c new file +12 -0

+#include "edac_mc.h"
+#include "edac_device.h"
+
+static int poll_msec = 1000;
+module_param(poll_msec, int, 0444);
+
+static bool panic_on_ue = 0;
+module_param_named(panic_on_ue, panic_on_ue, bool, 0664);
+
+#define L1 0x0
+#define L2 0x1
+#define L3 0x2

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+#define EDAC_CPU    "armv8_edac"
+
+#define ERRXSTATUS_VALID(a)    ((a >> 30) & 0x1)
+#define ERRXSTATUS_UE(a)    ((a >> 29) & 0x1)
+#define ERRXSTATUS_SERR(a)    (a & 0xFF)
+
+#define ERRXMISC_LVL(a)        ((a >> 1) & 0x7)
+#define ERRXMISC_WAY(a)        ((a >> 28) & 0xF)

Which ERR<n>MISC is this? [0] has two. At first glance these are both implementation-defined, so we can't rely on the values.

(MISC0 has some archtitected-layout, it seems we're expected to know which
layout it will be based on ERR<n>FR - but I can't see 'lvl' or 'way' in any of
those layouts).

You don't check ERR<n>STATUS.MV to know if the 'Miscallaneous Registers Valid'.

Quoted 1 line

Quoted 1 line

Quoted 1 line

+#define ERRXCTLR_ENABLE 0x10f

This looks like a magic value to set some bits. Could you add defines to name
each one, then combine so we know what was set.
This register has one of two layouts, it seems each control picks a layout. How
can we know if the read/write bits are combined or separate? It looks like this
in ERR<n>FR too.

Quoted 1 line

Quoted 1 line

Quoted 1 line

+#define ERRXMISC_OVERFLOW 0x7F7F00000000ULL

This looks like a magic value written to a register with an impdef layout.

[...]

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 4 lines

drivers/edac/armv8_edac.c new file +4 -0

+static inline void set_errxctlr_el1(void)
+{
+    asm volatile("msr s3_0_c5_c4_1, %0" : : "r" (ERRXCTLR_ENABLE));
+}

Please use {read,write}_sysreg().

Quoted 4 lines

Quoted 3 lines

If these registers are defined by ARMv8, please place definitions in arm64's <asm/sysreg.h>.

Will do.

Yes please!

(The s3_0_c5_c4_1 syntax isn't supported by all toolchains, see 72c5839515260dc
"arm64: gicv3: Allow GICv3 compilation with older binutils")

You also need to guard these with something like cpus_have_const_cap(ARM64_RAS_EXTN) so CPUs without the RAS extensions don't try
to access an unimplemented system register. Testing once during probe is
probably the best place to do this.

Quoted 17 lines

Quoted 16 lines

Quoted 16 lines

drivers/edac/armv8_edac.c new file +16 -0

+
+struct errors_edac {
+    const char * const msg;
+    void (*func)(struct edac_device_ctl_info *edac_dev,
+            int inst_nr, int block_nr, const char *msg);
+};
+
+static const struct errors_edac errors[] = {
+    { "L1 Correctable Error", edac_device_handle_ce },
+    { "L1 Uncorrectable Error", edac_device_handle_ue },
+    { "L2 Correctable Error", edac_device_handle_ce },
+    { "L2 Uncorrectable Error", edac_device_handle_ue },
+    { "L3 Correctable Error", edac_device_handle_ce },
+    { "L3 Uncorrectable Error", edac_device_handle_ue },
+};
+

Quoted 6 lines

Quoted 6 lines

drivers/edac/armv8_edac.c

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+#define L1_CE 0
+#define L1_UE 1
+#define L2_CE 2
+#define L2_UE 3
+#define L3_CE 4
+#define L3_UE 5

This are read from ERR<n>MISC, I can't find this in the spec[0], are these
specific to your implementation?

Quoted 5 lines

Quoted 5 lines

drivers/edac/armv8_edac.c

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+#define DATA_BUF_ERR        0x2
+#define CACHE_DATA_ERR        0x6
+#define CACHE_TAG_DIRTY_ERR    0x7
+#define TLB_PARITY_ERR_DATA    0x8
+#define TLB_PARITY_ERR_TAG    0x9

(Nit: 'TAG' is the example address/control data that has become corrupt. We
don't know how the cache is designed, 'metadata' may be a better choice of word
here).

Quoted 1 line

Quoted 1 line

Quoted 1 line

+#define BUS_ERROR 0x12

There are 21 of these and only 2 are implementation defined. How come you don't
define all of them?

All these get used for is to map to a name, would a table be better? (e.g. arch/arm64/mm/fault.c::fault_info)

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

drivers/edac/armv8_edac.c new file +9 -0

+struct erp_drvdata {
+    struct edac_device_ctl_info *edev_ctl;
+    struct erp_drvdata __percpu **erp_cpu_drvdata;
+    struct notifier_block nb_pm;
+    int ppi;
+};
+
+static struct erp_drvdata *panic_handler_drvdata;
+

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+static DEFINE_SPINLOCK(armv8_edac_lock);
+
+static void l1_l2_irq_enable(void *info)
+{
+    int irq = *(int *)info;
+
+    enable_percpu_irq(irq, IRQ_TYPE_LEVEL_HIGH);
+}

(Shouldn't the 'percpu' and 'level_high' be information learnt from the DT?)

Quoted 41 lines

Quoted 11 lines

Quoted 11 lines

+static int request_erp_irq(struct platform_device *pdev, const char *propname,

drivers/edac/armv8_edac.c new file +8 -0

+            const char *desc, irq_handler_t handler,
+            void *ed, int percpu)
+{
+    int rc;
+    struct resource *r;
+    struct erp_drvdata *drv = ed;
+
+    r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, 

propname);

Quoted 8 lines

Quoted 6 lines

drivers/edac/armv8_edac.c new file +3 -0

+
+    if (!r) {
+        pr_err("ARMv8 CPU ERP: Could not find <%s> IRQ property. 

Proceeding
anyway.\n",
+ propname);

What is "ERP" ?

Error Reporting. I may just rename it to EDAC or list it out in Documentation.

Quoted 18 lines

Quoted 18 lines

drivers/edac/armv8_edac.c new file +12 -0

+        goto out;
+    }
+
+    if (!percpu) {
+        rc = devm_request_threaded_irq(&pdev->dev, r->start, NULL,
+                           handler,
+                           IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+                           desc,
+                           ed);
+
+        if (rc) {
+            pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s 

/ %s).
Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +4 -0

+                   (int) r->start, rc, propname, desc);
+            goto out;
+        }
+

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    } else {
+        drv->erp_cpu_drvdata = alloc_percpu(struct erp_drvdata *);
+        if (!drv->erp_cpu_drvdata) {
+            pr_err("Failed to allocate percpu erp data\n");
+            goto out;
+        }
+
+        *raw_cpu_ptr(drv->erp_cpu_drvdata) = drv;

raw_cpu_ptr(). You call this from armv8_cpu_erp_probe(), so you're probably
preemtible. Could you use this_cpu_ptr() and enable CONFIG_DEBUG_PREEMPT.

(How come you only set this per-cpu cookie on one cpu?)

Quoted 84 lines

Quoted 84 lines

Quoted 84 lines

drivers/edac/armv8_edac.c new file +5 -0

+        rc = request_percpu_irq(r->start, handler, desc,
+                drv->erp_cpu_drvdata);
+
+        if (rc) {
+            pr_err("ARMv8 CPU ERP: Failed to request IRQ %d: %d (%s 

/ %s).
Proceeding anyway.\n",

drivers/edac/armv8_edac.c new file +18 -0

+                   (int) r->start, rc, propname, desc);
+            goto out_free;
+        }
+
+        drv->ppi = r->start;
+        on_each_cpu(l1_l2_irq_enable, &(r->start), 1);
+    }
+
+    return 0;
+
+out_free:
+    free_percpu(drv->erp_cpu_drvdata);
+    drv->erp_cpu_drvdata = NULL;
+out:
+    return 1;
+}
+
+static void dump_err_reg(int errorcode, int level, u64 errxstatus, 

u64
errxmisc,

drivers/edac/armv8_edac.c new file +3 -0

+    struct edac_device_ctl_info *edev_ctl)
+{
+    edac_printk(KERN_CRIT, EDAC_CPU, "ERRXSTATUS_EL1: %llx\n", 

errxstatus);
+ edac_printk(KERN_CRIT, EDAC_CPU, "ERRXMISC_EL1: %llx\n",
errxmisc);
+ edac_printk(KERN_CRIT, EDAC_CPU, "Cache level: L%d\n",
level+1);

drivers/edac/armv8_edac.c new file +3 -0

+    switch (ERRXSTATUS_SERR(errxstatus)) {
+    case DATA_BUF_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from internal 

data
buffer\n");

drivers/edac/armv8_edac.c new file +4 -0

+        break;
+
+    case CACHE_DATA_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache data 

RAM\n");

drivers/edac/armv8_edac.c new file +4 -0

+        break;
+
+    case CACHE_TAG_DIRTY_ERR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "ECC Error from cache tag 

or dirty
RAM\n");

drivers/edac/armv8_edac.c new file +4 -0

+        break;
+
+    case TLB_PARITY_ERR_DATA:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB DATA 

RAM\n");

drivers/edac/armv8_edac.c new file +4 -0

+        break;
+
+    case TLB_PARITY_ERR_TAG:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Parity error on TLB TAG 

RAM\n");

drivers/edac/armv8_edac.c new file +19 -0

+        break;
+
+    case BUS_ERROR:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Bus Error\n");
+        break;
+    }
 
+    if (level == L3)
+        edac_printk(KERN_CRIT, EDAC_CPU,
+            "Way: %d\n", (int) ERRXMISC_WAY(errxmisc));
+    else
+        edac_printk(KERN_CRIT, EDAC_CPU,
+            "Way: %d\n", (int) ERRXMISC_WAY(errxmisc) >> 2);
+
+    edev_ctl->panic_on_ue = panic_on_ue;
+    errors[errorcode].func(edev_ctl, smp_processor_id(),
+                level, errors[errorcode].msg);
+}
+
+static void armv8_parse_l1_l2_cache_error(u64 errxstatus, u64 

errxmisc,

drivers/edac/armv8_edac.c new file +2 -0

+    struct edac_device_ctl_info *edev_ctl)
+{

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    switch (ERRXMISC_LVL(errxmisc)) {
+    case L1:
+        if (ERRXSTATUS_UE(errxstatus))
+            dump_err_reg(L1_UE, L1, errxstatus, errxmisc,
+                    edev_ctl);
+        else
+            dump_err_reg(L1_CE, L1, errxstatus, errxmisc,
+                    edev_ctl);

If the UE bit is clear the error may have been 'corrected or deferred'. I guess
we assume deferred errors don't happen, so !UE means Corrected-Error.

Quoted 24 lines

Quoted 23 lines

Quoted 23 lines

drivers/edac/armv8_edac.c new file +11 -0

+        break;
+    case L2:
+        if (ERRXSTATUS_UE(errxstatus))
+            dump_err_reg(L2_UE, L2, errxstatus, errxmisc,
+                    edev_ctl);
+        else
+            dump_err_reg(L2_CE, L2, errxstatus, errxmisc,
+                    edev_ctl);
+        break;
+    default:
+        edac_printk(KERN_CRIT, EDAC_CPU, "Unknown ERRXMISC_LVL 

value\n");

drivers/edac/armv8_edac.c new file +11 -0

+    }
+}
+
+static bool armv8_check_l1_l2_ecc(void *info)
+{
+    struct edac_device_ctl_info *edev_ctl = info;
+    u64 errxstatus;
+    u64 errxmisc;
+    unsigned long flags;
+
+    spin_lock_irqsave(&armv8_edac_lock, flags);

Quoted 1 line

Quoted 1 line

Quoted 1 line

+ write_errselr_el1(0);

How do we know how many nodes backed by the cpu interface there are? How do we know '0' is the one we want?

Quoted 27 lines

Quoted 26 lines

Quoted 26 lines

drivers/edac/armv8_edac.c new file +9 -0

+    errxstatus = read_errxstatus_el1();
+
+    if (ERRXSTATUS_VALID(errxstatus)) {
+        errxmisc = read_errxmisc_el1();
+        edac_printk(KERN_CRIT, EDAC_CPU,
+        "CPU%d detected a L1/L2 cache error\n",
+        smp_processor_id());
+
+        armv8_parse_l1_l2_cache_error(errxstatus, errxmisc, 

edev_ctl);

drivers/edac/armv8_edac.c new file +9 -0

+        clear_errxstatus_valid(errxstatus);
+        spin_unlock_irqrestore(&armv8_edac_lock, flags);
+        return true;
+    }
+    spin_unlock_irqrestore(&armv8_edac_lock, flags);
+    return false;
+}
+
+static bool armv8_check_l3_scu_error(struct edac_device_ctl_info 

*edev_ctl)

drivers/edac/armv8_edac.c new file +6 -0

+{
+    u64 errxstatus = 0;
+    u64 errxmisc = 0;
+    unsigned long flags;
+
+    spin_lock_irqsave(&armv8_edac_lock, flags);

Quoted 1 line

Quoted 1 line

Quoted 1 line

+ write_errselr_el1(1);

How do we know '1' is the one we want?

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

drivers/edac/armv8_edac.c new file +7 -0

+    errxstatus = read_errxstatus_el1();
+    errxmisc = read_errxmisc_el1();
+
+    if (ERRXSTATUS_VALID(errxstatus) &&
+        ERRXMISC_LVL(errxmisc) == L3) {
+        if (ERRXSTATUS_UE(errxstatus)) {
+            edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 

uncorrectable
error\n");

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +4 -0

+            dump_err_reg(L3_UE, L3, errxstatus, errxmisc,
+                edev_ctl);
+        } else {
+            edac_printk(KERN_CRIT, EDAC_CPU, "Detected L3 

correctable
error\n");

drivers/edac/armv8_edac.c new file +2 -0

+            dump_err_reg(L3_CE, L3, errxstatus, errxmisc,
+                edev_ctl);

What about deferred errors?

Quoted 18 lines

Quoted 18 lines

Quoted 18 lines

drivers/edac/armv8_edac.c new file +18 -0

+        }
+
+        clear_errxstatus_valid(errxstatus);
+        spin_unlock_irqrestore(&armv8_edac_lock, flags);
+        return true;
+    }
+    spin_unlock_irqrestore(&armv8_edac_lock, flags);
+    return false;
+}
+
+static void armv8_check_l1_l2_ecc_helper(void *info)
+{
+    armv8_check_l1_l2_ecc(info);
+}
+
+void armv8_poll_cache_errors(struct edac_device_ctl_info *edev_ctl)
+{
+    int cpu;

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+
+    if (!edev_ctl)
+        edev_ctl = panic_handler_drvdata->edev_ctl;
+
+    armv8_check_l3_scu_error(edev_ctl);
+    for_each_possible_cpu(cpu) {
+        smp_call_function_single(cpu, armv8_check_l1_l2_ecc_helper,
+            edev_ctl, 0);

on_each_cpu()?

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

drivers/edac/armv8_edac.c new file +23 -0

+    }
+}
+
+static irqreturn_t armv8_l1_l2_handler(int irq, void *drvdata)
+{
+    if (armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl))
+        return IRQ_HANDLED;
+    return IRQ_NONE;
+}
+
+static irqreturn_t armv8_l3_scu_handler(int irq, void *drvdata)
+{
+    struct erp_drvdata *drv = drvdata;
+    struct edac_device_ctl_info *edev_ctl = drv->edev_ctl;
+
+    if (armv8_check_l3_scu_error(edev_ctl))
+        return IRQ_HANDLED;
+    return IRQ_NONE;
+}
+
+static void initialize_registers(void *info)
+{
+    set_errxctlr_el1();

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    set_errxmisc_overflow();
+}
+
+static void init_regs_on_cpu(bool all_cpus)
+{
+    int cpu;
+
+    write_errselr_el1(0);

This only happens on the local CPU, the other end of your smp_call_function_single() may have junk in this register.

Quoted 4 lines

Quoted 4 lines

drivers/edac/armv8_edac.c

Quoted 4 lines

drivers/edac/armv8_edac.c new file +4 -0

+    if (all_cpus) {
+        for_each_possible_cpu(cpu)
+            smp_call_function_single(cpu, initialize_registers,
+                        NULL, 1);

on_each_cpu()?

Quoted 6 lines

Quoted 6 lines

drivers/edac/armv8_edac.c

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+    } else {
+        initialize_registers(NULL);
+    }
+
+    write_errselr_el1(1);
+    initialize_registers(NULL);

You seem to magically-know that this 'record 1' is shared between all CPUs and
visible via the cpu interface.

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

drivers/edac/armv8_edac.c new file +9 -0

+}
+
+static int armv8_pmu_cpu_pm_notify(struct notifier_block *self,
+                unsigned long action, void *v)
+{
+    switch (action) {
+    case CPU_PM_EXIT:
+        init_regs_on_cpu(false);
+        armv8_check_l3_scu_error(panic_handler_drvdata->edev_ctl);

Quoted 9 lines

drivers/edac/armv8_edac.c

Quoted 6 lines

drivers/edac/armv8_edac.c new file +6 -0

+        armv8_check_l1_l2_ecc(panic_handler_drvdata->edev_ctl);
+        break;
+    }
+
+    return NOTIFY_OK;
+}

What about CPU hotplug?

Quoted 35 lines

Agreed that CPU hotplug will be required for the small window between hotplugging back in and LPM exit.

Quoted 32 lines

Quoted 32 lines

drivers/edac/armv8_edac.c new file +31 -0

+
+static int armv8_cpu_erp_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct erp_drvdata *drv;
+    int rc = 0;
+    int fail = 0;
+
+    init_regs_on_cpu(true);
+
+    drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+
+    if (!drv)
+        return -ENOMEM;
+
+    drv->edev_ctl = edac_device_alloc_ctl_info(0, "cpu",
+                    num_possible_cpus(), "L", 3, 1, NULL, 0,
+                    edac_device_alloc_index());
+
+    if (!drv->edev_ctl)
+        return -ENOMEM;
+
+    if (IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+        drv->edev_ctl->edac_check = armv8_poll_cache_errors;
+        drv->edev_ctl->poll_msec = poll_msec;
+    }
+
+    drv->edev_ctl->dev = dev;
+    drv->edev_ctl->mod_name = dev_name(dev);
+    drv->edev_ctl->dev_name = dev_name(dev);
 
+    drv->edev_ctl->ctl_name = "cache";

How do we know records '0' and '1' are 'cache'?

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

drivers/edac/armv8_edac.c new file +10 -0

+    drv->edev_ctl->panic_on_ue = panic_on_ue;
+    drv->nb_pm.notifier_call = armv8_pmu_cpu_pm_notify;
+    platform_set_drvdata(pdev, drv);
+
+    rc = edac_device_add_device(drv->edev_ctl);
+    if (rc)
+        goto out_mem;
+
+    panic_handler_drvdata = drv;
+

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 8 lines

drivers/edac/armv8_edac.c new file +8 -0

+    if (!IS_ENABLED(CONFIG_EDAC_ARMV8_POLL)) {
+        fail += request_erp_irq(pdev, "l1-l2-irq",
+                "l1_l2_irq",
+                armv8_l1_l2_handler, drv, 1);
+
+        fail += request_erp_irq(pdev, "l3-scu-irq",
+                "l3_scu_irq",
+                armv8_l3_scu_handler, drv, 0);

How do we know 'l1/l2' is per-cpu and l2 isn't? I'd expect this information to
come from the DT.

Quoted 6 lines

Quoted 2 lines

SCU isn't an architectural concept, and a combined l1-l2 interrupt sounds very specific to a particular implementation.

Can do a rename to something more akin to "private_cache_irq" and "shared_cache_irq".

Quoted 8 lines

Quoted 4 lines

My understanding was that with the RAS extensions, SError would be used
to notify on RAS conditions, so I'm confused as to why we'd need regular interrupts here.

There may be cases where interrupts are preferred. To my knowledge (though I may
mistaken), SErrors would not catch correctable errors?

With the v8.2 RAS Extensions both synchronous and asynchronous external-aborts
have a severity in the ESR which tells us corrected/restartable/recoverable/uncontainable.

Error nodes may signal an 'error recovery interrupt' or 'fatal handling interrupt' when the in-band external-abort mechanisms can't be used. (Sections
3.3 to 3.5 of [0]).

Quoted 34 lines

Quoted 34 lines

Quoted 34 lines

drivers/edac/armv8_edac.c new file +2 -0

+        if (fail == of_irq_count(dev->of_node)) {
+            pr_err("ERP: Could not request any IRQs. Giving 

up.\n");

drivers/edac/armv8_edac.c new file +31 -0

+            rc = -ENODEV;
+            goto out_dev;
+        }
+    }
+
+    cpu_pm_register_notifier(&(drv->nb_pm));
+
+    return 0;
+
+out_dev:
+    edac_device_del_device(dev);
+out_mem:
+    edac_device_free_ctl_info(drv->edev_ctl);
+    return rc;
+}
+
+static int armv8_cpu_erp_remove(struct platform_device *pdev)
+{
+    struct erp_drvdata *drv = dev_get_drvdata(&pdev->dev);
+    struct edac_device_ctl_info *edac_ctl = drv->edev_ctl;
+
+    if (drv->erp_cpu_drvdata) {
+        free_percpu_irq(drv->ppi, drv->erp_cpu_drvdata);
+        free_percpu(drv->erp_cpu_drvdata);
+    }
+
+    edac_device_del_device(edac_ctl->dev);
+    edac_device_free_ctl_info(edac_ctl);
+
+    return 0;
+}

Quoted 8 lines

Quoted 8 lines

drivers/edac/armv8_edac.c

Quoted 5 lines

drivers/edac/armv8_edac.c new file +5 -0

+
+static const struct of_device_id armv8_cpu_erp_match_table[] = {
+    { .compatible = "arm,armv8-cpu-erp" },
+    { }
+};

This needs a binding document, laying out precisely what this is intended to describe.

Thanks,

James

Quoted 15 lines · James Morse

Quoted 14 lines · Kyle Yan

Quoted 10 lines · Mark Rutland

Quoted 3 lines · Kyle Yan

Interrupt based EDAC driver for ARMv8 processors that implement RAS for error detection of CPU caches and lso allows optional polling of error syndrome registers if interrupts are not supported.

Is this using the architectural RAS extensions, or something specific to QC CPUs? It would be good to call this out explicitly.

If it's the latter, this needs rewording to be clear that this is specific to QC CPUs, and is not a generic ARMv8 feature.

This is not specific to QC CPUs and looks at the system registers defined by the RAS extensions.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

drivers/edac/armv8_edac.c new file +1 -0

new file mode 100644
 
+static void init_regs_on_cpu(bool all_cpus)

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

drivers/edac/armv8_edac.c

Quoted 2 lines

drivers/edac/armv8_edac.c new file +2 -0

+    write_errselr_el1(1);
+    initialize_registers(NULL);

You seem to magically-know that this 'record 1' is shared between all CPUs and visible via the cpu interface.

Yes this portion (and coming to the realization that a lot of other parts) of the code is specific to our topology. Is there any way to detect this?

ERRIDR reports
how many records there are but not what each record is for. Without this information,
not sure how there could be a generic interrupt based driver.

I should probably amend a previous statement about this all being defined in RAS. Didn't realize this initially
but as you have mentioned, even though the registers are architected, the decoding/bits within the registers are
implementation defined.
So within the driver, some of the "magically known"
values are because as it is right now,
the decodings and some behavior is specific to our implementation.

To more generalize this, would it be better to (after checking the IDR/FR registers) to instead just dump out the
ERRXSTATUS/ERRXMISC directly, and then have registered callbacks to actually properly decode the values into something more readable?