[PATCH v5 0/4] Add support for Amazon's Annapurna Labs EDAC for L1/L2

[PATCH v5 0/4] Add support for Amazon's Annapurna Labs EDAC for L1/L2

[Hanna Hawa]

This series adds L1 and L2 caches support for error detection and
correction for Amazon's Annapurna Labs SoCs.
Alpine SoCs supports L1 and L2 single bit correction and two bits detection
capability based on ARM implementation.

Changes since v4:
-----------------
- Added include for cpumask.h in al_l2_edac.c
- Fix RAMID error print according to ARM TRM
- Use for_each_possible_cpu() to parse information for DT.
- Add missing of_node_put() call.

Changes since v3:
-----------------
- Added include for smp.h sysreg.h 
- Use scnprintf instead of snprintf
- Move write_sysreg_s after valid check to minimize the window between
read/write.
- Use IS_ERR_OR_NULL instead of IS_ERR, because
edac_device_alloc_ctl_info may return NULL.

Changes since v2:
-----------------
- Use BIT for single bit instead of GENMASK
- Use BIT_ULL and GENMASK_ULL for 64bit vector
- Fix the mod_name/ctrl_name.

Changes since v1:
-----------------
- Split into two drivers
- Get cpu-mask according to l2-cache handler from devicetree
- Remove parameter casting
- Use GENMASK() in bit mask
- Use FIELD_GET()
- Update define description PLRU_RAM -> PF_RAM
- Use sys_reg() and read_sysreg_s()
- Remove all write/read wrappers
- Check fatal field to set if the error correctable or not
- Remove un-relevant information from error prints.
- Update smp_call_function_single() call function to wait
- remove usage of get_online_cpus/put_online_cpus
- Use on_each_cpu() and smp_call_function_any() instead of loop with for_each_cpu.
- use buffer for error prints and pass to edac API
- Remove edac_op_state set
- Add for loop to report on repeated errors of the same type
- Fix error name of the TLB to be L2_TLB as written in ARM TRM
- Minor change in Kconfig
- Minor changes in commit message

Hanna Hawa (4):
  dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC
  edac: Add support for Amazon's Annapurna Labs L1 EDAC
  dt-bindings: EDAC: Add Amazon's Annapurna Labs L2 EDAC
  edac: Add support for Amazon's Annapurna Labs L2 EDAC

 .../bindings/edac/amazon,al-l1-edac.txt       |  14 ++
 .../bindings/edac/amazon,al-l2-edac.txt       |  20 ++
 MAINTAINERS                                   |  12 ++
 drivers/edac/Kconfig                          |  16 ++
 drivers/edac/Makefile                         |   2 +
 drivers/edac/al_l1_edac.c                     | 158 ++++++++++++++
 drivers/edac/al_l2_edac.c                     | 192 ++++++++++++++++++
 7 files changed, 414 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
 create mode 100644 Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt
 create mode 100644 drivers/edac/al_l1_edac.c
 create mode 100644 drivers/edac/al_l2_edac.c

-- 
2.17.1

[PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[Hanna Hawa]

Document Amazon's Annapurna Labs L1 EDAC SoC binding.

Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
---
 .../devicetree/bindings/edac/amazon,al-l1-edac.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt

diff --git a/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt b/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
new file mode 100644
index 000000000000..2ae8370216bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
@@ -0,0 +1,14 @@
+* Amazon's Annapurna Labs L1 EDAC
+
+Amazon's Annapurna Labs SoCs supports L1 single bit correction and
+two bits detection capability based on ARM implementation.
+
+Required properties:
+- compatible:
+	should be "amazon,al-l1-edac".
+
+Example:
+
+	al_l1_edac {
+		compatible = "amazon,al-l1-edac";
+	};
-- 
2.17.1

[PATCH v5 2/4] edac: Add support for Amazon's Annapurna Labs L1 EDAC

[Hanna Hawa]

Adds support for Amazon's Annapurna Labs L1 EDAC driver to detect and
report L1 errors.

Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
Reviewed-by: James Morse <james.morse@arm.com>
---
 MAINTAINERS               |   6 ++
 drivers/edac/Kconfig      |   8 ++
 drivers/edac/Makefile     |   1 +
 drivers/edac/al_l1_edac.c | 158 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 173 insertions(+)
 create mode 100644 drivers/edac/al_l1_edac.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 77eae44bf5de..fd29ea62ba29 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -743,6 +743,12 @@ F:	drivers/tty/serial/altera_jtaguart.c
 F:	include/linux/altera_uart.h
 F:	include/linux/altera_jtaguart.h
 
+AMAZON ANNAPURNA LABS L1 EDAC
+M:	Hanna Hawa <hhhawa@amazon.com>
+S:	Maintained
+F:	drivers/edac/al_l1_edac.c
+F:	Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
+
 AMAZON ANNAPURNA LABS THERMAL MMIO DRIVER
 M:	Talel Shenhar <talel@amazon.com>
 S:	Maintained
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 200c04ce5b0e..58b92bcb39ce 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -74,6 +74,14 @@ config EDAC_GHES
 
 	  In doubt, say 'Y'.
 
+config EDAC_AL_L1
+	bool "Amazon's Annapurna Labs L1 EDAC"
+	depends on ARCH_ALPINE
+	help
+	  Support for L1 error detection and correction
+	  for Amazon's Annapurna Labs SoCs.
+	  This driver detects errors of L1 caches.
+
 config EDAC_AMD64
 	tristate "AMD64 (Opteron, Athlon64)"
 	depends on AMD_NB && EDAC_DECODE_MCE
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 165ca65e1a3a..caa2dc91e8a0 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_EDAC_GHES)			+= ghes_edac.o
 edac_mce_amd-y				:= mce_amd.o
 obj-$(CONFIG_EDAC_DECODE_MCE)		+= edac_mce_amd.o
 
+obj-$(CONFIG_EDAC_AL_L1)		+= al_l1_edac.o
 obj-$(CONFIG_EDAC_AMD76X)		+= amd76x_edac.o
 obj-$(CONFIG_EDAC_CPC925)		+= cpc925_edac.o
 obj-$(CONFIG_EDAC_I5000)		+= i5000_edac.o
diff --git a/drivers/edac/al_l1_edac.c b/drivers/edac/al_l1_edac.c
new file mode 100644
index 000000000000..9404a2fcaa58
--- /dev/null
+++ b/drivers/edac/al_l1_edac.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#include <asm/sysreg.h>
+#include <linux/bitfield.h>
+#include <linux/smp.h>
+
+#include "edac_device.h"
+#include "edac_module.h"
+
+#define DRV_NAME				"al_l1_edac"
+
+/* Same bit assignments of CPUMERRSR_EL1 in ARM CA57/CA72 */
+#define ARM_CA57_CPUMERRSR_EL1			sys_reg(3, 1, 15, 2, 2)
+#define ARM_CA57_CPUMERRSR_RAM_ID		GENMASK(30, 24)
+#define  ARM_CA57_L1_I_TAG_RAM			0x00
+#define  ARM_CA57_L1_I_DATA_RAM			0x01
+#define  ARM_CA57_L1_D_TAG_RAM			0x08
+#define  ARM_CA57_L1_D_DATA_RAM			0x09
+#define  ARM_CA57_L2_TLB_RAM			0x18
+#define ARM_CA57_CPUMERRSR_VALID		BIT(31)
+#define ARM_CA57_CPUMERRSR_REPEAT		GENMASK_ULL(39, 32)
+#define ARM_CA57_CPUMERRSR_OTHER		GENMASK_ULL(47, 40)
+#define ARM_CA57_CPUMERRSR_FATAL		BIT_ULL(63)
+
+#define AL_L1_EDAC_MSG_MAX			256
+
+static void al_l1_edac_cpumerrsr(void *arg)
+{
+	struct edac_device_ctl_info *edac_dev = arg;
+	int cpu, i;
+	u32 ramid, repeat, other, fatal;
+	u64 val = read_sysreg_s(ARM_CA57_CPUMERRSR_EL1);
+	char msg[AL_L1_EDAC_MSG_MAX];
+	int space, count;
+	char *p;
+
+	if (!(FIELD_GET(ARM_CA57_CPUMERRSR_VALID, val)))
+		return;
+
+	write_sysreg_s(0, ARM_CA57_CPUMERRSR_EL1);
+
+	cpu = smp_processor_id();
+	ramid = FIELD_GET(ARM_CA57_CPUMERRSR_RAM_ID, val);
+	repeat = FIELD_GET(ARM_CA57_CPUMERRSR_REPEAT, val);
+	other = FIELD_GET(ARM_CA57_CPUMERRSR_OTHER, val);
+	fatal = FIELD_GET(ARM_CA57_CPUMERRSR_FATAL, val);
+
+	space = sizeof(msg);
+	p = msg;
+	count = scnprintf(p, space, "CPU%d L1 %serror detected", cpu,
+			  (fatal) ? "Fatal " : "");
+	p += count;
+	space -= count;
+
+	switch (ramid) {
+	case ARM_CA57_L1_I_TAG_RAM:
+		count = scnprintf(p, space, " RAMID='L1-I Tag RAM'");
+		break;
+	case ARM_CA57_L1_I_DATA_RAM:
+		count = scnprintf(p, space, " RAMID='L1-I Data RAM'");
+		break;
+	case ARM_CA57_L1_D_TAG_RAM:
+		count = scnprintf(p, space, " RAMID='L1-D Tag RAM'");
+		break;
+	case ARM_CA57_L1_D_DATA_RAM:
+		count = scnprintf(p, space, " RAMID='L1-D Data RAM'");
+		break;
+	case ARM_CA57_L2_TLB_RAM:
+		count = scnprintf(p, space, " RAMID='L2 TLB RAM'");
+		break;
+	default:
+		count = scnprintf(p, space, " RAMID='unknown'");
+		break;
+	}
+
+	p += count;
+	space -= count;
+	count = scnprintf(p, space,
+			  " repeat=%d, other=%d (CPUMERRSR_EL1=0x%llx)",
+			  repeat, other, val);
+
+	for (i = 0; i < repeat; i++) {
+		if (fatal)
+			edac_device_handle_ue(edac_dev, 0, 0, msg);
+		else
+			edac_device_handle_ce(edac_dev, 0, 0, msg);
+	}
+}
+
+static void al_l1_edac_check(struct edac_device_ctl_info *edac_dev)
+{
+	on_each_cpu(al_l1_edac_cpumerrsr, edac_dev, 1);
+}
+
+static int al_l1_edac_probe(struct platform_device *pdev)
+{
+	struct edac_device_ctl_info *edac_dev;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	edac_dev = edac_device_alloc_ctl_info(0, (char *)dev_name(dev), 1, "L",
+					      1, 1, NULL, 0,
+					      edac_device_alloc_index());
+	if (IS_ERR_OR_NULL(edac_dev))
+		return -ENOMEM;
+
+	edac_dev->edac_check = al_l1_edac_check;
+	edac_dev->dev = dev;
+	edac_dev->mod_name = DRV_NAME;
+	edac_dev->dev_name = dev_name(dev);
+	edac_dev->ctl_name = "L1 cache";
+	platform_set_drvdata(pdev, edac_dev);
+
+	ret = edac_device_add_device(edac_dev);
+	if (ret) {
+		dev_err(dev, "Failed to add L1 edac device\n");
+		goto err;
+	}
+
+	return 0;
+err:
+	edac_device_free_ctl_info(edac_dev);
+
+	return ret;
+}
+
+static int al_l1_edac_remove(struct platform_device *pdev)
+{
+	struct edac_device_ctl_info *edac_dev = platform_get_drvdata(pdev);
+
+	edac_device_del_device(edac_dev->dev);
+	edac_device_free_ctl_info(edac_dev);
+
+	return 0;
+}
+
+static const struct of_device_id al_l1_edac_of_match[] = {
+	{ .compatible = "amazon,al-l1-edac" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, al_l1_edac_of_match);
+
+static struct platform_driver al_l1_edac_driver = {
+	.probe = al_l1_edac_probe,
+	.remove = al_l1_edac_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = al_l1_edac_of_match,
+	},
+};
+module_platform_driver(al_l1_edac_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hanna Hawa <hhhawa@amazon.com>");
+MODULE_DESCRIPTION("Amazon's Annapurna Lab's L1 EDAC Driver");
-- 
2.17.1

[PATCH v5 3/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L2 EDAC

[Hanna Hawa]

Document Amazon's Annapurna Labs L2 EDAC SoC binding.

Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
---
 .../bindings/edac/amazon,al-l2-edac.txt       | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt

diff --git a/Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt b/Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt
new file mode 100644
index 000000000000..7b0b7347b711
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt
@@ -0,0 +1,20 @@
+* Amazon's Annapurna Labs L2 EDAC
+
+Amazon's Annapurna Labs SoCs supports L2 single bit correction and
+two bits detection capability based on ARM implementation.
+
+Required properties:
+- compatible:
+	should be "amazon,al-l2-edac".
+- l2-cache:
+	Phandle to L2 cache handler.
+	This property is used to compare with the CPU node property
+	'next-level-cache' to create cpu-mask with all CPUs that
+	share same L2 cache.
+
+Example:
+
+	al_l2_edac {
+		compatible = "amazon,al-l2-edac";
+		l2-cache = <&cluster0_l2>;
+	};
-- 
2.17.1

[PATCH v5 4/4] edac: Add support for Amazon's Annapurna Labs L2 EDAC

[Hanna Hawa]

Adds support for Amazon's Annapurna Labs L2 EDAC driver to detect and
report L2 errors.

Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
Reviewed-by: James Morse <james.morse@arm.com>
---
 MAINTAINERS               |   6 ++
 drivers/edac/Kconfig      |   8 ++
 drivers/edac/Makefile     |   1 +
 drivers/edac/al_l2_edac.c | 192 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 207 insertions(+)
 create mode 100644 drivers/edac/al_l2_edac.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fd29ea62ba29..a6dcf3d8e12a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -749,6 +749,12 @@ S:	Maintained
 F:	drivers/edac/al_l1_edac.c
 F:	Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
 
+AMAZON ANNAPURNA LABS L2 EDAC
+M:	Hanna Hawa <hhhawa@amazon.com>
+S:	Maintained
+F:	drivers/edac/al_l2_edac.c
+F:	Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt
+
 AMAZON ANNAPURNA LABS THERMAL MMIO DRIVER
 M:	Talel Shenhar <talel@amazon.com>
 S:	Maintained
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 58b92bcb39ce..8bbb745b84ed 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -82,6 +82,14 @@ config EDAC_AL_L1
 	  for Amazon's Annapurna Labs SoCs.
 	  This driver detects errors of L1 caches.
 
+config EDAC_AL_L2
+	bool "Amazon's Annapurna Labs L2 EDAC"
+	depends on ARCH_ALPINE
+	help
+	  Support for L2 error detection and correction
+	  for Amazon's Annapurna Labs SoCs.
+	  This driver detects errors of L2 caches.
+
 config EDAC_AMD64
 	tristate "AMD64 (Opteron, Athlon64)"
 	depends on AMD_NB && EDAC_DECODE_MCE
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index caa2dc91e8a0..60a6b8bbe2f8 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -23,6 +23,7 @@ edac_mce_amd-y				:= mce_amd.o
 obj-$(CONFIG_EDAC_DECODE_MCE)		+= edac_mce_amd.o
 
 obj-$(CONFIG_EDAC_AL_L1)		+= al_l1_edac.o
+obj-$(CONFIG_EDAC_AL_L2)		+= al_l2_edac.o
 obj-$(CONFIG_EDAC_AMD76X)		+= amd76x_edac.o
 obj-$(CONFIG_EDAC_CPC925)		+= cpc925_edac.o
 obj-$(CONFIG_EDAC_I5000)		+= i5000_edac.o
diff --git a/drivers/edac/al_l2_edac.c b/drivers/edac/al_l2_edac.c
new file mode 100644
index 000000000000..3dcca380d2aa
--- /dev/null
+++ b/drivers/edac/al_l2_edac.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#include <asm/sysreg.h>
+#include <linux/bitfield.h>
+#include <linux/cpumask.h>
+#include <linux/of.h>
+#include <linux/smp.h>
+
+#include "edac_device.h"
+#include "edac_module.h"
+
+#define DRV_NAME				"al_l2_edac"
+
+/* Same bit assignments of L2MERRSR_EL1 in ARM CA57/CA72 */
+#define ARM_CA57_L2MERRSR_EL1			sys_reg(3, 1, 15, 2, 3)
+#define ARM_CA57_L2MERRSR_RAMID			GENMASK(30, 24)
+#define  ARM_CA57_L2_TAG_RAM			0x10
+#define  ARM_CA57_L2_DATA_RAM			0x11
+#define  ARM_CA57_L2_SNOOP_RAM			0x12
+#define  ARM_CA57_L2_DIRTY_RAM			0x14
+#define  ARM_CA57_L2_INC_PF_RAM			0x18
+#define ARM_CA57_L2MERRSR_VALID			BIT(31)
+#define ARM_CA57_L2MERRSR_REPEAT		GENMASK_ULL(39, 32)
+#define ARM_CA57_L2MERRSR_OTHER			GENMASK_ULL(47, 40)
+#define ARM_CA57_L2MERRSR_FATAL			BIT_ULL(63)
+
+#define AL_L2_EDAC_MSG_MAX			256
+
+struct al_l2_edac {
+	cpumask_t cluster_cpus;
+};
+
+static void al_l2_edac_l2merrsr(void *arg)
+{
+	struct edac_device_ctl_info *edac_dev = arg;
+	int cpu, i;
+	u32 ramid, repeat, other, fatal;
+	u64 val = read_sysreg_s(ARM_CA57_L2MERRSR_EL1);
+	char msg[AL_L2_EDAC_MSG_MAX];
+	int space, count;
+	char *p;
+
+	if (!(FIELD_GET(ARM_CA57_L2MERRSR_VALID, val)))
+		return;
+
+	write_sysreg_s(0, ARM_CA57_L2MERRSR_EL1);
+
+	cpu = smp_processor_id();
+	ramid = FIELD_GET(ARM_CA57_L2MERRSR_RAMID, val);
+	repeat = FIELD_GET(ARM_CA57_L2MERRSR_REPEAT, val);
+	other = FIELD_GET(ARM_CA57_L2MERRSR_OTHER, val);
+	fatal = FIELD_GET(ARM_CA57_L2MERRSR_FATAL, val);
+
+	space = sizeof(msg);
+	p = msg;
+	count = scnprintf(p, space, "CPU%d L2 %serror detected", cpu,
+			  (fatal) ? "Fatal " : "");
+	p += count;
+	space -= count;
+
+	switch (ramid) {
+	case ARM_CA57_L2_TAG_RAM:
+		count = scnprintf(p, space, " RAMID='L2 Tag RAM'");
+		break;
+	case ARM_CA57_L2_DATA_RAM:
+		count = scnprintf(p, space, " RAMID='L2 Data RAM'");
+		break;
+	case ARM_CA57_L2_SNOOP_RAM:
+		count = scnprintf(p, space, " RAMID='L2 Snoop Tag RAM'");
+		break;
+	case ARM_CA57_L2_DIRTY_RAM:
+		count = scnprintf(p, space, " RAMID='L2 Dirty RAM'");
+		break;
+	case ARM_CA57_L2_INC_PF_RAM:
+		count = scnprintf(p, space, " RAMID='L2 internal metadata'");
+		break;
+	default:
+		count = scnprintf(p, space, " RAMID='unknown'");
+		break;
+	}
+
+	p += count;
+	space -= count;
+
+	count = scnprintf(p, space,
+			  " repeat=%d, other=%d (L2MERRSR_EL1=0x%llx)",
+			  repeat, other, val);
+
+	for (i = 0; i < repeat; i++) {
+		if (fatal)
+			edac_device_handle_ue(edac_dev, 0, 0, msg);
+		else
+			edac_device_handle_ce(edac_dev, 0, 0, msg);
+	}
+}
+
+static void al_l2_edac_check(struct edac_device_ctl_info *edac_dev)
+{
+	struct al_l2_edac *al_l2 = edac_dev->pvt_info;
+
+	smp_call_function_any(&al_l2->cluster_cpus, al_l2_edac_l2merrsr,
+			      edac_dev, 1);
+}
+
+static int al_l2_edac_probe(struct platform_device *pdev)
+{
+	struct edac_device_ctl_info *edac_dev;
+	struct al_l2_edac *al_l2;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+
+	edac_dev = edac_device_alloc_ctl_info(sizeof(*al_l2),
+					      (char *)dev_name(dev), 1, "L", 1,
+					      2, NULL, 0,
+					      edac_device_alloc_index());
+	if (IS_ERR_OR_NULL(edac_dev))
+		return -ENOMEM;
+
+	al_l2 = edac_dev->pvt_info;
+	edac_dev->edac_check = al_l2_edac_check;
+	edac_dev->dev = dev;
+	edac_dev->mod_name = DRV_NAME;
+	edac_dev->dev_name = dev_name(dev);
+	edac_dev->ctl_name = "L2 cache";
+	platform_set_drvdata(pdev, edac_dev);
+
+	for_each_possible_cpu(i) {
+		struct device_node *cpu;
+		struct device_node *cpu_cache, *l2_cache;
+
+		cpu = of_get_cpu_node(i, NULL);
+		cpu_cache = of_find_next_cache_node(cpu);
+		l2_cache = of_parse_phandle(dev->of_node, "l2-cache", 0);
+
+		if (cpu_cache == l2_cache)
+			cpumask_set_cpu(i, &al_l2->cluster_cpus);
+
+		of_node_put(cpu);
+	}
+
+	if (cpumask_empty(&al_l2->cluster_cpus)) {
+		dev_err(dev, "CPU mask is empty for this L2 cache\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	ret = edac_device_add_device(edac_dev);
+	if (ret) {
+		dev_err(dev, "Failed to add L2 edac device\n");
+		goto err;
+	}
+
+	return 0;
+
+err:
+	edac_device_free_ctl_info(edac_dev);
+
+	return ret;
+}
+
+static int al_l2_edac_remove(struct platform_device *pdev)
+{
+	struct edac_device_ctl_info *edac_dev = platform_get_drvdata(pdev);
+
+	edac_device_del_device(edac_dev->dev);
+	edac_device_free_ctl_info(edac_dev);
+
+	return 0;
+}
+
+static const struct of_device_id al_l2_edac_of_match[] = {
+	{ .compatible = "amazon,al-l2-edac" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, al_l2_edac_of_match);
+
+static struct platform_driver al_l2_edac_driver = {
+	.probe = al_l2_edac_probe,
+	.remove = al_l2_edac_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = al_l2_edac_of_match,
+	},
+};
+module_platform_driver(al_l2_edac_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hanna Hawa <hhhawa@amazon.com>");
+MODULE_DESCRIPTION("Amazon's Annapurna Lab's L2 EDAC Driver");
-- 
2.17.1

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[Rob Herring]

On Mon, Aug 05, 2019 at 03:39:08PM +0100, Hanna Hawa wrote:
> Document Amazon's Annapurna Labs L1 EDAC SoC binding.
> 
> Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
> ---
>  .../devicetree/bindings/edac/amazon,al-l1-edac.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
> 
> diff --git a/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt b/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
> new file mode 100644
> index 000000000000..2ae8370216bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt
> @@ -0,0 +1,14 @@
> +* Amazon's Annapurna Labs L1 EDAC
> +
> +Amazon's Annapurna Labs SoCs supports L1 single bit correction and
> +two bits detection capability based on ARM implementation.
> +
> +Required properties:
> +- compatible:
> +	should be "amazon,al-l1-edac".

Why is this even in DT? AFAICT, this is all just CortexA57 core features 
(i.e. nothing Amazon specific). The core type and the ECC capabilities 
are discoverable.

Rob

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[Hawa, Hanna]

On 8/21/2019 10:17 PM, Rob Herring wrote:
> Why is this even in DT? AFAICT, this is all just CortexA57 core features
> (i.e. nothing Amazon specific). The core type and the ECC capabilities
> are discoverable.

Added to the DT in order to easily enable/disable the driver. You are 
correct that they are CortexA57 core features and nothing Amazon 
specific, but it's IMPLEMENTATION DEFINED, meaning that in different 
cortex revisions (e.g. A57) the register bitmap may change. Because of 
that we added an Amazon compatible which corresponds to the specific 
core we are using.

Thanks,
Hanna

> 
> Rob

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[Rob Herring]

On Mon, Aug 26, 2019 at 9:49 AM Hawa, Hanna <hhhawa@amazon.com> wrote:
>
>
>
> On 8/21/2019 10:17 PM, Rob Herring wrote:
> > Why is this even in DT? AFAICT, this is all just CortexA57 core features
> > (i.e. nothing Amazon specific). The core type and the ECC capabilities
> > are discoverable.
>
> Added to the DT in order to easily enable/disable the driver.

That alone is not reason enough to put it in DT. From a DT
perspective, I have no idea what the whims of a OS maintainer are
regarding whether they want all this to be 1 driver or 2 drivers.
(IMO, it should be 1 as this is ECC for an A57. For a core and memory
controller, then 2 seems appropriate.)

> You are
> correct that they are CortexA57 core features and nothing Amazon
> specific, but it's IMPLEMENTATION DEFINED, meaning that in different
> cortex revisions (e.g. A57) the register bitmap may change. Because of
> that we added an Amazon compatible which corresponds to the specific
> core we are using.

'IMPLEMENTATION DEFINED' usually means not defined by the
architecture. This doesn't seem like something that's going to change
with A57 revisions. Plus, the A57 is pretty old now, so we know what's
out there already. In any case, the revision is discoverable too, so
we could deal with changes between revisions in any case.

Rob

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[James Morse]

Hi guys,

On 27/08/2019 14:49, Rob Herring wrote:
> On Mon, Aug 26, 2019 at 9:49 AM Hawa, Hanna <hhhawa@amazon.com> wrote:
>> On 8/21/2019 10:17 PM, Rob Herring wrote:
>>> Why is this even in DT? AFAICT, this is all just CortexA57 core features
>>> (i.e. nothing Amazon specific). The core type and the ECC capabilities
>>> are discoverable.
>>
>> Added to the DT in order to easily enable/disable the driver.
> 
> That alone is not reason enough to put it in DT. From a DT
> perspective, I have no idea what the whims of a OS maintainer are
> regarding whether they want all this to be 1 driver or 2 drivers.
> (IMO, it should be 1 as this is ECC for an A57. For a core and memory
> controller, then 2 seems appropriate.)
> 
>> You are
>> correct that they are CortexA57 core features and nothing Amazon
>> specific, but it's IMPLEMENTATION DEFINED, meaning that in different
>> cortex revisions (e.g. A57) the register bitmap may change. Because of
>> that we added an Amazon compatible which corresponds to the specific
>> core we are using.

I think its that the instruction encoding is in the imp-def space that is important.

CPU-implementers can add whatever registers they find useful here. A57 and A72 both
implemented some ECC registers here. (They are not guaranteed to be the same, but I can't
find any differences).

We need some information from DT because the TRM doesn't say what happens when you read
from these registers on an A57 that doesn't have the 'optional ECC protection'. It could
take an exception due to an unimplemented system register.

The imp-def instruction space may also be trapped by a higher exception level. KVM does
this, and emulates these registers as if they were all undefined.


Thanks,

James

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[Rob Herring]

On Fri, Aug 30, 2019 at 7:45 AM James Morse <james.morse@arm.com> wrote:
>
> Hi guys,
>
> On 27/08/2019 14:49, Rob Herring wrote:
> > On Mon, Aug 26, 2019 at 9:49 AM Hawa, Hanna <hhhawa@amazon.com> wrote:
> >> On 8/21/2019 10:17 PM, Rob Herring wrote:
> >>> Why is this even in DT? AFAICT, this is all just CortexA57 core features
> >>> (i.e. nothing Amazon specific). The core type and the ECC capabilities
> >>> are discoverable.
> >>
> >> Added to the DT in order to easily enable/disable the driver.
> >
> > That alone is not reason enough to put it in DT. From a DT
> > perspective, I have no idea what the whims of a OS maintainer are
> > regarding whether they want all this to be 1 driver or 2 drivers.
> > (IMO, it should be 1 as this is ECC for an A57. For a core and memory
> > controller, then 2 seems appropriate.)
> >
> >> You are
> >> correct that they are CortexA57 core features and nothing Amazon
> >> specific, but it's IMPLEMENTATION DEFINED, meaning that in different
> >> cortex revisions (e.g. A57) the register bitmap may change. Because of
> >> that we added an Amazon compatible which corresponds to the specific
> >> core we are using.
>
> I think its that the instruction encoding is in the imp-def space that is important.
>
> CPU-implementers can add whatever registers they find useful here. A57 and A72 both
> implemented some ECC registers here. (They are not guaranteed to be the same, but I can't
> find any differences).

Two cores potentially being the same only furthers my argument that
this shouldn't be an Amazon driver.

> We need some information from DT because the TRM doesn't say what happens when you read
> from these registers on an A57 that doesn't have the 'optional ECC protection'. It could
> take an exception due to an unimplemented system register.

My read of the TRM is that L2 ECC is always there and L1 ECC/parity is
optional. Furthermore, bit 22 of L2CTRL_EL1 indicates if L1 ECC/parity
is supported or not and there's other non-ECC stuff like cache RAM
timing values in that register.

> The imp-def instruction space may also be trapped by a higher exception level. KVM does
> this, and emulates these registers as if they were all undefined.

So KVM provides a semi-CortexA57? Code that runs on real h/w won't as a guest.

However, if we do need DT to indicate ECC support in a core or not,
then we already have an A57 node and should put that info there.

Rob

Re: [PATCH v5 1/4] dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC

[James Morse]

Hi Rob,

On 30/08/2019 22:50, Rob Herring wrote:
> So KVM provides a semi-CortexA57? Code that runs on real h/w won't as a guest.

KVM provides the architectural bits of Cortex-A57's EL1, when running on A57.

Code that depends on EL2, won't run as a guest. Code that depends on some
non-architectural behaviour of A57 won't work in a guest, (e.g. the PMU)
Features the hypervisor doesn't completely support may get hidden. The aim is to provide
an virtual CPU, it might not be exactly the same as the one you're running on.

Hypervisors have to disable access to the imp-def registers as they may allow the guest to
break its confinement. (e.g. messing with the L2 timing)

Code using imp-def instructions at EL1 needs to know they aren't trapped/disabled by a
higher exception level. If someone wants to emulate these, something would need a model of
what those imp-def instructions do.


Thanks,

James