Re: [PATCH 1/1] drivers/edac: Add L1 and L2 error detection for A53 and A57
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: 2021-01-07 12:38:11
Also in:
linux-edac
On Thu, Jan 07, 2021 at 11:15:20AM +0000, Mark Rutland wrote:
Hi Sascha, On Thu, Jan 07, 2021 at 11:38:19AM +0100, Sascha Hauer wrote:quoted
The Cortex A53 and A57 cores have error detection capabilities for the L1/L2 Caches, this patch adds a driver for them. Unfortunately there is no robust way to inject errors into the caches, so this driver doesn't contain any code to actually test it. It has been tested though with code taken from an older version of this driver found here: https://lkml.org/lkml/2018/3/14/1203. For reasons stated in this thread the error injection code is not suitable for mainline, so it is removed from the driver. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/edac/Kconfig | 6 + drivers/edac/Makefile | 1 + drivers/edac/cortex_arm64_l1_l2.c | 218 ++++++++++++++++++++++++++++++ 3 files changed, 225 insertions(+) create mode 100644 drivers/edac/cortex_arm64_l1_l2.c[...]quoted
+#define SYS_CPUMERRSR_EL1 sys_reg(3, 1, 15, 2, 2) +#define SYS_L2MERRSR_EL1 sys_reg(3, 1, 15, 2, 3)Just to note -- these registers are IMPLEMENTATION DEFINED, and so may be trapped by a hypervisor or messed around with arbitrarily by FW.quoted
+static void read_errors(void *data) +{ + struct merrsr *merrsr = data; + + merrsr->cpumerr = read_sysreg_s(SYS_CPUMERRSR_EL1); + write_sysreg_s(0, SYS_CPUMERRSR_EL1); + merrsr->l2merr = read_sysreg_s(SYS_L2MERRSR_EL1); + write_sysreg_s(0, SYS_L2MERRSR_EL1); +}... and so here the MRS instructions may behave as UNDEFINED in some cases. Foer example, KVM would trap these via HCR_EL2.TIDCP, and emulate these as UNDEFINED.quoted
+static const struct of_device_id cortex_arm64_edac_of_match[] = { + { .compatible = "arm,cortex-a53" }, + { .compatible = "arm,cortex-a57" }, + {} +}; +MODULE_DEVICE_TABLE(of, cortex_arm64_edac_of_match);... and so it's not safe to probe this based on the CPU compatible string alone. If we really need to support this, it will need its own binding (to describe that the reigsters are safe to access and behave in a useful way).
Rob objected to adding dedicated nodes to attach the driver to. That leaves adding a property like "edac-enabled" or similar to the CPU nodes when edac is desired. Would that be ok? Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel