Re: [U-Boot] [PATCH v4 03/10] Exynos542x: Add workaround for ARM errata 798870
Hi Nishanth,
On 17:13-20150224, Nishanth Menon wrote:
On 13:27-20150220, Akshay Saraswat wrote:
This patch adds workaround for ARM errata 798870 which says "If back-to-back speculative cache line fills (fill A and fill B) are issued from the L1 data cache of a CPU to the L2 cache, the second request (fill B) is then cancelled, and the second request would have detected a hazard against a recent write or eviction (write B) to the same cache line as fill B then the L2 logic might deadlock."
Signed-off-by: Kimoon Kim kimoon.kim@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com Reviewed-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
Changes since v3:
- Added errata number in comment.
- Moved changes to arm generic armv7.h
Changes since v2:
- No change.
Changes since v1:
- Added Reviewed-by & Tested-by.
- Added space before */ on line # 40.
arch/arm/include/asm/armv7.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index a13da23..a2040b7 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -69,6 +69,22 @@ #define CP15DSB asm volatile ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)) #define CP15DMB asm volatile ("mcr p15, 0, %0, c7, c10, 5" : : "r" (0))
+/*
- Workaround for ARM errata # 798870
- Set L2ACTLR[7] to reissue any memory transaction in the L2 that has been
- stalled for 1024 cycles to verify that its hazard condition still exists.
- */
+static inline void v7_enable_l2_hazard_detect(void) +{
- uint32_t val;
- /* L2ACTLR[7]: Enable hazard detect timeout */
- asm volatile ("mrc p15, 1, %0, c15, c0, 0\n\t" : "=r"(val));
- val |= (1 << 7);
- asm volatile ("mcr p15, 1, %0, c15, c0, 0\n\t" : : "r"(val));
This wont work for us in DRA7/OMAP5 L2ACTLR cannot be modified by u-boot. has to go to secure world using smc call.
+}
+void v7_en_l2_hazard_detect(void); void v7_outer_cache_enable(void); void v7_outer_cache_disable(void); void v7_outer_cache_flush_all(void);
How about this - using the series: https://patchwork.ozlabs.org/patch/443261/ https://patchwork.ozlabs.org/patch/443264/ https://patchwork.ozlabs.org/patch/443268/ https://patchwork.ozlabs.org/patch/443265/ https://patchwork.ozlabs.org/patch/443263/ https://patchwork.ozlabs.org/patch/443262/ https://patchwork.ozlabs.org/patch/443267/ https://patchwork.ozlabs.org/patch/443266/ https://patchwork.ozlabs.org/patch/443260/
I think the following might work for exynos?
Yes, that will probably do and if you look at v3 of this patch, we were doing something similar. But I was convinced with the below mentioned comments I recieved over v3, so updated it.
" Having the ARM errata number mentioned in the comment here would make the purpose of this code much more clear to anyone looking at it later in the future. Also isn't this a general purpose Cortex-A15 r2pX workaround, also potentially useful for the other non-Exynos SoCs too? "
I don't think it hurts to have a generic function with ARM errata workaround implementation. Whoever wish to use it can call it in their boot path. And it's not even getting executed right now for any SoC other than Exynos542x, so those who don't want it need not bother about it. This was the intention. :)
Please let me know if you still want me to move it from armv7.h to exynos/lowlevel_init.c
diff --git a/arch/arm/cpu/armv7/exynos/lowlevel_init.c b/arch/arm/cpu/armv7/exynos/lowlevel_init.c index 83e1dcfc1e13..0157105524ca 100644 --- a/arch/arm/cpu/armv7/exynos/lowlevel_init.c +++ b/arch/arm/cpu/armv7/exynos/lowlevel_init.c @@ -42,6 +42,23 @@ enum { DO_POWER = 1 << 4, };
+#ifdef CONFIG_ARM_ARCH_CP15_ERRATA +void arch_cp15_errata_workaround(u32 midr, u32 variant, u32 rev, u32 comb) +{ +#ifdef CONFIG_ARM_ERRATA_798870
- if (comb >= 0x20 && comb < 0x30) {
uint32_t l2actlr;mrc_l2_aux_ctlr(l2actlr);/* Disable ACE DVM/CMO message broadcas */l2actlr |= (1 << 7);mcr_l2_aux_ctlr(l2actlr);mrc_l2_ctlr(l2actlr);- }
+#endif +} +#endif
int do_lowlevel_init(void) { uint32_t reset_status; -- Regards, Nishanth Menon
Regards, Akshay Saraswat
On Wed, Feb 25, 2015 at 2:27 AM, Akshay Saraswat akshay.s@samsung.com wrote:
Hi Nishanth,
On 17:13-20150224, Nishanth Menon wrote:
On 13:27-20150220, Akshay Saraswat wrote:
This patch adds workaround for ARM errata 798870 which says "If back-to-back speculative cache line fills (fill A and fill B) are issued from the L1 data cache of a CPU to the L2 cache, the second request (fill B) is then cancelled, and the second request would have detected a hazard against a recent write or eviction (write B) to the same cache line as fill B then the L2 logic might deadlock."
Signed-off-by: Kimoon Kim kimoon.kim@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com Reviewed-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
Changes since v3:
- Added errata number in comment.
- Moved changes to arm generic armv7.h
Changes since v2:
- No change.
Changes since v1:
- Added Reviewed-by & Tested-by.
- Added space before */ on line # 40.
arch/arm/include/asm/armv7.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index a13da23..a2040b7 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -69,6 +69,22 @@ #define CP15DSB asm volatile ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)) #define CP15DMB asm volatile ("mcr p15, 0, %0, c7, c10, 5" : : "r" (0))
+/*
- Workaround for ARM errata # 798870
- Set L2ACTLR[7] to reissue any memory transaction in the L2 that has been
- stalled for 1024 cycles to verify that its hazard condition still exists.
- */
+static inline void v7_enable_l2_hazard_detect(void) +{
- uint32_t val;
- /* L2ACTLR[7]: Enable hazard detect timeout */
- asm volatile ("mrc p15, 1, %0, c15, c0, 0\n\t" : "=r"(val));
- val |= (1 << 7);
- asm volatile ("mcr p15, 1, %0, c15, c0, 0\n\t" : : "r"(val));
This wont work for us in DRA7/OMAP5 L2ACTLR cannot be modified by u-boot. has to go to secure world using smc call.
+}
+void v7_en_l2_hazard_detect(void); void v7_outer_cache_enable(void); void v7_outer_cache_disable(void); void v7_outer_cache_flush_all(void);
How about this - using the series: https://patchwork.ozlabs.org/patch/443261/ https://patchwork.ozlabs.org/patch/443264/ https://patchwork.ozlabs.org/patch/443268/ https://patchwork.ozlabs.org/patch/443265/ https://patchwork.ozlabs.org/patch/443263/ https://patchwork.ozlabs.org/patch/443262/ https://patchwork.ozlabs.org/patch/443267/ https://patchwork.ozlabs.org/patch/443266/ https://patchwork.ozlabs.org/patch/443260/
I think the following might work for exynos?
Yes, that will probably do and if you look at v3 of this patch, we were doing something similar. But I was convinced with the below mentioned comments I recieved over v3, so updated it.
" Having the ARM errata number mentioned in the comment here would make the purpose of this code much more clear to anyone looking at it later in the future. Also isn't this a general purpose Cortex-A15 r2pX workaround, also potentially useful for the other non-Exynos SoCs too? "
I don't think it hurts to have a generic function with ARM errata workaround implementation. Whoever wish to use it can call it in their boot path. And it's not even getting executed right now for any SoC other than Exynos542x, so those who don't want it need not bother about it. This was the intention. :)
Please let me know if you still want me to move it from armv7.h to exynos/lowlevel_init.c
I think I can do a V3 on my series for a middle path between the two approaches. Assuming that exynos does not need any specific custom smc call for the l2actlr update. will split every erratum into a separate patch of it's own.
diff --git a/arch/arm/cpu/armv7/exynos/lowlevel_init.c b/arch/arm/cpu/armv7/exynos/lowlevel_init.c index 83e1dcfc1e13..0157105524ca 100644 --- a/arch/arm/cpu/armv7/exynos/lowlevel_init.c +++ b/arch/arm/cpu/armv7/exynos/lowlevel_init.c @@ -42,6 +42,23 @@ enum { DO_POWER = 1 << 4, };
+#ifdef CONFIG_ARM_ARCH_CP15_ERRATA +void arch_cp15_errata_workaround(u32 midr, u32 variant, u32 rev, u32 comb) +{ +#ifdef CONFIG_ARM_ERRATA_798870
if (comb >= 0x20 && comb < 0x30) {uint32_t l2actlr;mrc_l2_aux_ctlr(l2actlr);/* Disable ACE DVM/CMO message broadcas */l2actlr |= (1 << 7);mcr_l2_aux_ctlr(l2actlr);mrc_l2_ctlr(l2actlr);}+#endif +} +#endif
int do_lowlevel_init(void) { uint32_t reset_status; -- Regards, Nishanth Menon
Regards, Akshay Saraswat _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Akshay Saraswat akshay.s@samsung.com writes:
[...]
I don't think it hurts to have a generic function with ARM errata workaround implementation. Whoever wish to use it can call it in their boot path. And it's not even getting executed right now for any SoC other than Exynos542x, so those who don't want it need not bother about it. This was the intention. :)
What about exynos542x platforms which also have secure firmware? Are you testing this on any of those (e.g. exynos5422-odroid-xu3?)
Kevin
participants (3)
-
Akshay Saraswat -
Kevin Hilman -
Nishanth Menon