Add support for Amazon's Annapurna Labs EDAC for L1/L2
Subsystems (2)
v4 cover Add support for Amazon's Annapurna Labs EDAC for L1/L2
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 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
*** BLURB HERE ***
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 | 189 ++++++++++++++++++ 7 files changed, 411 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
Signature · 1 line
2.17.1
PATCH v4 1/4 dt-bindings: EDAC: Add Amazon's Annapurna Labs L1 EDAC
Document Amazon's Annapurna Labs L1 EDAC SoC binding.
--- .../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
Documentation/devicetree/bindings/edac/amazon,al-l1-edac.txt new file +14 -0
@@ -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"; + };
Signature · 1 line
2.17.1
PATCH v4 2/4 edac: Add support for Amazon's Annapurna Labs L1 EDAC
In reply to Hanna HawaAdds support for Amazon's Annapurna Labs L1 EDAC driver to detect and report L1 errors.
--- 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
MAINTAINERS +6 -0
@@ -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 <[email protected]> +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 <[email protected]> S: Maintained
drivers/edac/Kconfig +8 -0
@@ -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
drivers/edac/Makefile +1 -0
@@ -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
drivers/edac/al_l1_edac.c new file +158 -0
158 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v4 3/4 dt-bindings: EDAC: Add Amazon's Annapurna Labs L2 EDAC
In reply to Hanna HawaDocument Amazon's Annapurna Labs L2 EDAC SoC binding.
--- .../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
Documentation/devicetree/bindings/edac/amazon,al-l2-edac.txt new file +20 -0
@@ -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>; + };
Signature · 1 line
2.17.1
PATCH v4 4/4 edac: Add support for Amazon's Annapurna Labs L2 EDAC
In reply to Hanna HawaAdds support for Amazon's Annapurna Labs L2 EDAC driver to detect and report L2 errors.
--- MAINTAINERS | 6 ++ drivers/edac/Kconfig | 8 ++ drivers/edac/Makefile | 1 + drivers/edac/al_l2_edac.c | 189 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+) create mode 100644 drivers/edac/al_l2_edac.c
MAINTAINERS +6 -0
@@ -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 <[email protected]> +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 <[email protected]> S: Maintained
drivers/edac/Kconfig +8 -0
@@ -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
drivers/edac/Makefile +1 -0
@@ -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
drivers/edac/al_l2_edac.c new file +189 -0
189 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [PATCH v4 4/4] edac: Add support for Amazon's Annapurna Labs L2 EDAC
Hi Hanna,
drivers/edac/al_l2_edac.c line 7
Quoted 11 lines · Hanna Hawa
Adds support for Amazon's Annapurna Labs L2 EDAC driver to detect and report L2 errors.
drivers/edac/al_l2_edac.c new file +2
-0@@ -0,0 +1,189 @@ +#include <asm/sysreg.h> +#include <linux/bitfield.h>
#include <linux/cpumask.h> ?
Quoted 2 lines
drivers/edac/al_l2_edac.c new file +2
-0+#include <linux/of.h> +#include <linux/smp.h>
[...]
Quoted 27 lines
drivers/edac/al_l2_edac.c new file +27
-0+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;
drivers/edac/al_l2_edac.c line 71
Quoted 10 lines
drivers/edac/al_l2_edac.c new file +10
-0+ +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 RAM'" );
Nit: The TRMs both call this 'L2 Snoop Tag RAM'. Could we include 'tag' in the description. 'tag' implies its some kind of metadata, so an uncorrected error here affect a now unknown location, its more series than a 'data RAM' error. v8.2 would term this kind of error 'uncontained'.
drivers/edac/al_l2_edac.c line 77
Quoted 6 lines
drivers/edac/al_l2_edac.c new file +6
-0+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 metadat'" );
Nit: metadata
Quoted 20 lines
drivers/edac/al_l2_edac.c new file +20
-0+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); + } +}
[...]
Quoted 13 lines
drivers/edac/al_l2_edac.c new file +13
-0+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 ;
drivers/edac/al_l2_edac.c line 129
Quoted 10 lines
drivers/edac/al_l2_edac.c new file +9
-0+ + 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_online_cpu (i) {
for_each_possible_cpu()?
If you boot with maxcpus= the driver's behaviour changes.
But you are only parsing information from the DT, so you don't really need the CPUs to be online.
drivers/edac/al_l2_edac.c line 133
Quoted 4 lines
drivers/edac/al_l2_edac.c new file +4
-0+struct device_node *cpu; +struct device_node *cpu_cache, *l2_cache; + + cpu =of_get_cpu_node (i,NULL );
(of_get_cpu_node() can return NULL, but I don't think it can ever happen like this)
drivers/edac/al_l2_edac.c line 138
Quoted 5 lines
drivers/edac/al_l2_edac.c new file +5
-0+ 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);
You need to of_node_put() these device_node pointers once you're done with them.
Quoted 11 lines
drivers/edac/al_l2_edac.c new file +11
-0+ } + +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" );
drivers/edac/al_l2_edac.c line 159
Quoted 10 lines
drivers/edac/al_l2_edac.c new file +10
-0+ goto err; + } + +return 0 ; + +err: +edac_device_free_ctl_info (edac_dev); + +return ret; +}
With the of_node_put()ing:
Thanks,
James
Re: [PATCH v4 4/4] edac: Add support for Amazon's Annapurna Labs L2 EDAC
Quoted 12 lines · James Morse
Hi Hanna,
Quoted 8 lines · Hanna Hawa
Adds support for Amazon's Annapurna Labs L2 EDAC driver to detect and report L2 errors.
drivers/edac/al_l2_edac.c new file +0
-0@@ -0,0 +1,189 @@
Quoted 4 lines
drivers/edac/al_l2_edac.c line 7
Quoted 2 lines
drivers/edac/al_l2_edac.c new file +2
-0+#include <asm/sysreg.h> +#include <linux/bitfield.h> #include <
linux/cpumask.h> ?
Will be added.
Quoted 40 lines
Quoted 3 lines
drivers/edac/al_l2_edac.c new file +2
-0+#include <linux/of.h> +#include <linux/smp.h> [...]
Quoted 34 lines
drivers/edac/al_l2_edac.c new file +34
-0+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'" );
Quoted 8 lines
drivers/edac/al_l2_edac.c line 71
Quoted 3 lines
drivers/edac/al_l2_edac.c new file +3
-0+break ; +case ARM_CA57_L2_SNOOP_RAM : + count =scnprintf (p, space," RAMID='L2 Snoop RAM'" );Nit: The TRMs both call this 'L2 Snoop Tag RAM'. Could we include 'tag' in the description. 'tag' implies its some kind of metadata, so an uncorrected error here affect a now unknown location, its more series than a 'data RAM' error. v8.2 would term this kind of error 'uncontained'.
Ack, will be fixed.
Quoted 10 lines
drivers/edac/al_l2_edac.c line 77
Quoted 8 lines
drivers/edac/al_l2_edac.c new file +6
-0+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 metadat'" );Nit: metadata
Ack, will be fixed.
Quoted 46 lines
Quoted 21 lines
drivers/edac/al_l2_edac.c new file +20
-0+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); + } +}[...]
Quoted 21 lines
drivers/edac/al_l2_edac.c new file +21
-0+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);
Quoted 7 lines
drivers/edac/al_l2_edac.c line 129
Quoted 1 line
drivers/edac/al_l2_edac.c new file +1
-0+for_each_online_cpu (i) {
for_each_possible_cpu()?If you boot with maxcpus= the driver's behaviour changes.
But you are only parsing information from the DT, so you don't really need the CPUs to be online.
Agree, for dt parsing no need the online CPUs, if for_each_possible_cpu() used then smp_call_function_any() will run only on the online CPUs in the mask.
Will change to for_each_possible_cpu().
Quoted 9 lines
drivers/edac/al_l2_edac.c line 133
Quoted 6 lines
drivers/edac/al_l2_edac.c new file +4
-0+struct device_node *cpu; +struct device_node *cpu_cache, *l2_cache; + + cpu =of_get_cpu_node (i,NULL );(
of_get_cpu_node()can return NULL, but I don't think it can ever happen like this)
Quoted 7 lines
drivers/edac/al_l2_edac.c line 138
Quoted 5 lines
drivers/edac/al_l2_edac.c new file +5
-0+ 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);You need to
of_node_put()thesedevice_nodepointers once you're done with them.
Will be added.
Quoted 25 lines
Quoted 13 lines
drivers/edac/al_l2_edac.c new file +11
-0+ } + +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" );drivers/edac/al_l2_edac.c line 159
Quoted 10 lines
drivers/edac/al_l2_edac.c new file +10
-0+ goto err; + } + +return 0 ; + +err: +edac_device_free_ctl_info (edac_dev); + +return ret; +}With the of_node_put()ing:
Quoted 1 line
Reviewed-by James Morse <[email protected]>
Thanks for review, will publish v5 with the above fixes.
Thanks,
Hanna
Quoted 6 lines
Thanks,
James