arm64: arch_timer: Add workaround for hisilicon-161010101 erratum
Subsystems (6)
v8 cover arm64: arch_timer: Add workaround for hisilicon-161010101 erratum
Erratum Hisilicon-161010101 says that the ARM generic timer counter "has the potential to contain an erroneous value when the timer value changes". Accesses to TVAL (both read and write) are also affected due to the implicit counter read. Accesses to CVAL are not affected.
The workaround is to reread the system count registers until the value of the second read is larger than the first one by less than 32, the system counter can be guaranteed not to return wrong value twice by back-to-back read and the error value is always larger than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
v2: Introducing a new generic erratum handling mechanism for fsl,a008585 and hisilicon,161601.
Significant rework based on feedback, including seperate the fsl erratum a008585 to another patch, update the erratum name and remove unwanted code.
v3: Introducing the erratum_workaround_set_sne generic function for fsl erratum a008585
and make the #define __fsl_a008585_read_reg to be private to the .c file instead of being globally visible. After discussion with Marc and Will, a consensus decision was made to remove the commandline parameter for enabling fsl,erratum-a008585 erratum, and make some generic name more specific, export timer_unstable_counter_workaround for module access.
Significant rework based on feedback, including fix some alignment problem, make the #define __hisi_161601_read_reg to be private to the .c file instead of being globally visible, add more accurate annotation and modify a bit of logical format to enable arch_timer_read_ool_enabled, remove the kernel commandline parameter clocksource.arm_arch_timer.hisilicon-161601.
Introduce a generic aquick framework for erratum in ACPI mode.
v4: rename the quirk handler parameter to make it more generic, and
avoid break loop when handling the quirk becasue it need to support multi quirks handler.
update some data structures for acpi mode.
v5: Adapt the new kernel-parameters.txt for latest kernel version.
Set the retries of reread system counter to 50, because it is possible that some interrupts may lead to more than twice read errors and break the loop, it will trigger the warning, so we set the number of retries far beyond the number of iterations the loop has been observed to take.
v6: The last 2 patches in the previous version about the ACPI mode will conflict witch Fuwei's
GTDT patches, so remove the ACPI part and only support the DT base code for this patch set.
We have trigger a bug when select the CONFIG_FUNCTION_GRAPH_TRACER and enable function_graph to /sys/kernel/debug/tracing/current_tracer, the system will stall into an endless loop, it looks like that the ftrace_graph_caller will be related to xxx.read_cntvct_el0 and read the system counter again, so mark the xxx.read_cntvct_el0 with notrace to fix the problem.
v7: Introduce a new general config symbol named CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND to enable the workaround
for any chips which has similar arch timer erratum just like "fsl,erratum_a008585" and "hisilicon,erratum_161601", modify the struct arch_timer_erratum_workaround to be compatible different chip erratum more easily, and reconstruction some code base on the new config symbol and struct, thanks to Marc's suggestion.
v8: The original erratum ID could not cover all modules, which only specified <Errata-Prefix><SerialNum>, so after
discussion with the soc team, we decide to use the new ID "161010101" for this timer erratum which consist of <Errata-Prefix><SeriesFlag><ModuleID><SerialNum> and also update the hisilicon erratum official documents.
Ding Tianhong (4):
arm64: arch_timer: Add device tree binding for hisilicon-161010101
erratum
arm64: arch_timer: Introduce a generic erratum handing mechanism for
fsl-a008585
arm64: arch_timer: Work around Erratum Hisilicon-161010101
arm64: arch timer: Add timer erratum property for Hip05-d02 and
Hip06-d03Documentation/admin-guide/kernel-parameters.txt | 9 -- Documentation/arm64/silicon-errata.txt | 43 +++--- .../devicetree/bindings/arm/arch_timer.txt | 8 ++ arch/arm64/boot/dts/hisilicon/hip05.dtsi | 1 + arch/arm64/boot/dts/hisilicon/hip06.dtsi | 1 + arch/arm64/include/asm/arch_timer.h | 38 ++---- drivers/clocksource/Kconfig | 18 +++ drivers/clocksource/arm_arch_timer.c | 150 +++++++++++++++------ 8 files changed, 173 insertions(+), 95 deletions(-)
Signature · 1 line
1.9.0
PATCH RESEND v8 1/4 arm64: arch_timer: Add device tree binding for hisilicon-161010101 erratum
This erratum describes a bug in logic outside the core, so MIDR can't be used to identify its presence, and reading an SoC-specific revision register from common arch timer code would be awkward. So, describe it in the device tree.
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/arm/arch_timer.txt +8 -0
@@ -31,6 +31,14 @@ to deliver its interrupts via SPIs. This also affects writes to the tval register, due to the implicit counter read. +- hisilicon,erratum-161010101 : A boolean property. Indicates the presence of + erratum 161010101, which says that reading the counter is unreliable unless + reading twice on the register and the value of the second read is larger + than the first by less than 32. If the verification is unsuccessful, then + discard the value of this read and repeat this procedure until the verification + is successful. This also affects writes to the tval register, due to the + implicit counter read. + ** Optional properties: - arm,cpu-registers-not-fw-configured : Firmware does not initialize
Signature · 1 line
1.9.0
Mark Rutland
maintainerPATCH RESEND v8 1/4 arm64: arch_timer: Add device tree binding for hisilicon-161010101 erratum
Quoted 16 lines · Ding Tianhong
This erratum describes a bug in logic outside the core, so MIDR can't be used to identify its presence, and reading an SoC-specific revision register from common arch timer code would be awkward. So, describe it in the device tree.
Signed-off-by Ding Tianhong <[email protected]>Acked-by Rob Herring <[email protected]>--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/arm/arch_timer.txt +0
-0
Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 40
Quoted 10 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +7
-0@@ -31,6 +31,14 @@ to deliver its interrupts via SPIs. This also affects writes to the tval register, due to the implicit counter read. +- hisilicon,erratum-161010101 : A boolean property. Indicates the presence of + erratum 161010101, which says that reading the counter is unreliable unless + reading twice on the register and the value of the second read is larger + than the first by less than 32. If the verification is unsuccessful, then + discard the value of this read and repeat this procedure until the verification + is successful. This also affects writes to the tval register, due to the + implicit counter read.
This describes the workaround, which shouldn't be necessary.
My understanding (from the cover letter) is that reads of the {virtual,physical} counters may return a value precisely 32 above the true value.
So it would be better to say:
- hisilicon,erratum-161010101 : A boolean property. Indicates the presence of Hisilicon erratum 161010101, which says that reading the counters is unreliable in some cases, and reads may return a value 32 beyond the correct value. This also affects writes to the tval registers, due to the implicit counter read.
Thanks,
Mark.
PATCH RESEND v8 1/4 arm64: arch_timer: Add device tree binding for hisilicon-161010101 erratum
Quoted 36 lines · Mark Rutland
Quoted 16 lines · Ding Tianhong
This erratum describes a bug in logic outside the core, so MIDR can't be used to identify its presence, and reading an SoC-specific revision register from common arch timer code would be awkward. So, describe it in the device tree.
Signed-off-by Ding Tianhong <[email protected]>Acked-by Rob Herring <[email protected]>--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/arm/arch_timer.txt +0
-0Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 40
Quoted 10 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +7
-0@@ -31,6 +31,14 @@ to deliver its interrupts via SPIs. This also affects writes to the tval register, due to the implicit counter read. +- hisilicon,erratum-161010101 : A boolean property. Indicates the presence of + erratum 161010101, which says that reading the counter is unreliable unless + reading twice on the register and the value of the second read is larger + than the first by less than 32. If the verification is unsuccessful, then + discard the value of this read and repeat this procedure until the verification + is successful. This also affects writes to the tval register, due to the + implicit counter read.This describes the workaround, which shouldn't be necessary.
My understanding (from the cover letter) is that reads of the {virtual,physical} counters may return a value precisely 32 above the true value.
So it would be better to say:
Quoted 8 lines
- hisilicon,erratum-161010101 : A boolean property. Indicates the presence of Hisilicon erratum 161010101, which says that reading the counters is unreliable in some cases, and reads may return a value 32 beyond the correct value. This also affects writes to the tval registers, due to the implicit counter read.
Thanks,
Mark.
Looks more accurate.
Thanks.
Ding
Quoted 3 lines
.
PATCH RESEND v8 2/4 arm64: arch_timer: Introduce a generic erratum handing mechanism for fsl-a008585
In reply to Ding TianhongThe workaround for hisilicon,161010101 will check the return value of the system counter by different way, in order to distinguish with the fsl-a008585 workaround, introduce a new generic erratum handing mechanism for fsl-a008585 and rename some functions.
After discussion with Marc and Will, a consensus decision was made to remove the commandline parameter for enabling fsl,erratum-a008585 erratum.
--- Documentation/admin-guide/kernel-parameters.txt | 9 -- arch/arm64/include/asm/arch_timer.h | 38 +++------ drivers/clocksource/Kconfig | 8 ++ drivers/clocksource/arm_arch_timer.c | 105 ++++++++++++++---------- 4 files changed, 84 insertions(+), 76 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +0 -9
@@ -539,15 +539,6 @@ loops can be debugged more effectively on production systems. - clocksource.arm_arch_timer.fsl-a008585= - [ARM64] - Format: <bool> - Enable/disable the workaround of Freescale/NXP - erratum A-008585. This can be useful for KVM - guests, if the guest device tree doesn't show the - erratum. If unspecified, the workaround is - enabled based on the device tree. - clearcpuid=BITNUM [X86] Disable CPUID feature X for the kernel. See arch/x86/include/asm/cpufeatures.h for the valid bit
arch/arm64/include/asm/arch_timer.h +13 -25
@@ -29,41 +29,29 @@#include <clocksource/arm_arch_timer.h> -#if IS_ENABLED (CONFIG_FSL_ERRATUM_A008585 ) +#if IS_ENABLED (CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND )extern struct static_key_false arch_timer_read_ool_enabled; -#define needs_fsl_a008585_workaround () \ +#define needs_unstable_timer_counter_workaround () \ static_branch_unlikely(&arch_timer_read_ool_enabled)#else -#define needs_fsl_a008585_workaround () false +#define needs_unstable_timer_counter_workaround () false#endif -u32 __fsl_a008585_read_cntp_tval_el0 (void ); -u32 __fsl_a008585_read_cntv_tval_el0 (void ); -u64 __fsl_a008585_read_cntvct_el0 (void ); -/* -* The number of retries is an arbitrary value well beyond the highest number -* of iterations the loop has been observed to take. -*/ -#define __fsl_a008585_read_reg (reg) ({ \ - u64 _old, _new; \ - int _retries = 200; \ - \ - do { \ - _old = read_sysreg(reg); \ - _new = read_sysreg(reg); \ - _retries--; \ - } while (unlikely(_old != _new) && _retries); \ - \ - WARN_ON_ONCE(!_retries); \ - _new; \ -}) +struct arch_timer_erratum_workaround { +const char *id;/* Indicate the Erratum ID */ +u32 (*read_cntp_tval_el0)(void ); +u32 (*read_cntv_tval_el0)(void ); +u64 (*read_cntvct_el0)(void ); +}; + +extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;#define arch_timer_reg_read_stable (reg) \ ({ \ u64 _val; \ - if (needs_fsl_a008585_workaround()) \ - _val = __fsl_a008585_read_##reg(); \ + if (needs_unstable_timer_counter_workaround()) \ + _val = timer_unstable_counter_workaround->read_##reg();\ else \ _val = read_sysreg(reg); \ _val; \
drivers/clocksource/Kconfig +8 -0
@@ -325,10 +325,18 @@ config ARM_ARCH_TIMER_EVTSTREAM This must be disabled for hardware validation purposes to detect any hardware anomalies of missing events. +config ARM_ARCH_TIMER_OOL_WORKAROUND + bool "Workaround for arm arch timer unstable counter" + depends on FSL_ERRATUM_A008585 + help + This option would only be enabled by Freescale/NXP Erratum A-008585 + or something else chip has similar erratum. + config FSL_ERRATUM_A008585 bool "Workaround for Freescale/NXP Erratum A-008585" default y depends on ARM_ARCH_TIMER && ARM64 + select ARM_ARCH_TIMER_OOL_WORKAROUND help This option enables a workaround for Freescale/NXP Erratum A-008585 ("ARM generic timer may contain an erroneous
drivers/clocksource/arm_arch_timer.c +63 -42
174 lines not on this page · raw message
Signature · 1 line
1.9.0
PATCH RESEND v8 3/4 arm64: arch_timer: Work around Erratum Hisilicon-161010101
In reply to Ding TianhongErratum Hisilicon-161010101 says that the ARM generic timer counter "has the potential to contain an erroneous value when the timer value changes". Accesses to TVAL (both read and write) are also affected due to the implicit counter read. Accesses to CVAL are not affected.
The workaround is to reread the system count registers until the value of the second read is larger than the first one by less than 32, the system counter can be guaranteed not to return wrong value twice by back-to-back read and the error value is always larger than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
The hisilicon erratum CONFIG name is too long, breaking the line format in silicon-errata.txt, so extended the character spacing to fit all the erratum config.
--- Documentation/arm64/silicon-errata.txt | 43 ++++++++++++++--------------- drivers/clocksource/Kconfig | 12 ++++++++- drivers/clocksource/arm_arch_timer.c | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 22 deletions(-)
Documentation/arm64/silicon-errata.txt +22 -21
@@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the Linux Kernel and will be updated when new workarounds are committed and backported to stable kernels. -| Implementor | Component | Erratum ID | Kconfig | -+----------------+-----------------+-----------------+-------------------------+ -| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | -| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | -| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | -| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | -| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | -| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | -| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | -| ARM | Cortex-A57 | #852523 | N/A | -| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | -| ARM | Cortex-A72 | #853709 | N/A | -| ARM | MMU-500 | #841119,#826419 | N/A | -| | | | | -| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | -| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 | -| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | -| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 | -| Cavium | ThunderX SMMUv2 | #27704 | N/A | -| | | | | -| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 | +| Implementor | Component | Erratum ID | Kconfig | ++----------------+-----------------+-----------------+---------------------------------+ +| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | +| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | +| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | +| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | +| ARM | Cortex-A57 | #852523 | N/A | +| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | +| ARM | Cortex-A72 | #853709 | N/A | +| ARM | MMU-500 | #841119,#826419 | N/A | +| | | | | +| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | +| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 | +| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | +| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 | +| Cavium | ThunderX SMMUv2 | #27704 | N/A | +| | | | | +| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 | +| Hisilicon | Hip0{5,6,7} | #161010101 | HISILICON_ERRATUM_161010101 |
drivers/clocksource/Kconfig +11 -1
@@ -327,7 +327,7 @@ config ARM_ARCH_TIMER_EVTSTREAM config ARM_ARCH_TIMER_OOL_WORKAROUND bool "Workaround for arm arch timer unstable counter" - depends on FSL_ERRATUM_A008585 + depends on FSL_ERRATUM_A008585 || HISILICON_ERRATUM_161010101 help This option would only be enabled by Freescale/NXP Erratum A-008585 or something else chip has similar erratum.
@@ -343,6 +343,16 @@ config FSL_ERRATUM_A008585 value"). The workaround will only be active if the fsl,erratum-a008585 property is found in the timer node. +config HISILICON_ERRATUM_161010101 + bool "Workaround for Hisilicon Erratum 161010101" + default y + select ARM_ARCH_TIMER_OOL_WORKAROUND + depends on ARM_ARCH_TIMER && ARM64 + help + This option enables a workaround for Hisilicon Erratum + 161010101. The workaround will be active if the hisilicon,erratum-161010101 + property is found in the timer node. + config ARM_GLOBAL_TIMER bool "Support for the ARM global timer" if COMPILE_TEST select CLKSRC_OF if OF
drivers/clocksource/arm_arch_timer.c +49 -0
61 lines not on this page · raw message
Signature · 1 line
1.9.0
PATCH RESEND v8 4/4 arm64: arch timer: Add timer erratum property for Hip05-d02 and Hip06-d03
In reply to Ding TianhongEnable workaround for hisilicon erratum 161010101 on Hip05-d02 and Hip06-d03 board.
--- arch/arm64/boot/dts/hisilicon/hip05.dtsi | 1 + arch/arm64/boot/dts/hisilicon/hip06.dtsi | 1 + 2 files changed, 2 insertions(+)
arch/arm64/boot/dts/hisilicon/hip05.dtsi +1 -0
@@ -281,6 +281,7 @@ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; + hisilicon,erratum-161010101; }; pmu {
arch/arm64/boot/dts/hisilicon/hip06.dtsi +1 -0
@@ -260,6 +260,7 @@ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; + hisilicon,erratum-161010101; }; pmu {
Signature · 1 line
1.9.0