[PATCH v2] arm64: issue ISB after updating system registers

[Masahiro Yamada]

On Wed, Jun 24, 2020 at 11:07 AM Volodymyr Babchuk
<Volodymyr_Babchuk at epam.com> wrote:
>
> ARM Architecture reference manual clearly states that PE pipeline
> should be flushed after any change to system registers. Refer to
> paragraph "B2.3.5 Memory Barriers" at page B2-92 of "Arm Architecture
> Reference Manual ARMv8 for ARMv8-A Architecture Profile" (ARM DDI
> 0487B.a).
>
> Failing to issue instruction memory synchronization barrier can lead
> to spurious errors, like synchronous exception when accessing FPU
> registers. This is very prominent on CPUs with long instruction
> pipeline, like ARM Cortex A72.
>
> This change fixes the following U-Boot panic:
>
>  "Synchronous Abort" handler, esr 0x1fe00000
>  elr: 00000000800948cc lr : 0000000080091e04
>  x0 : 00000000801ffdc8 x1 : 00000000000000c8
>  x2 : 00000000800979d4 x3 : 00000000801ffc60
>  x4 : 00000000801ffd40 x5 : ffffff80ffffffd8
>  x6 : 00000000801ffd70 x7 : 00000000801ffd70
>  x8 : 000000000000000a x9 : 0000000000000000
>  x10: 0000000000000044 x11: 0000000000000000
>  x12: 0000000000000000 x13: 0000000000000000
>  x14: 0000000000000000 x15: 0000000000000000
>  x16: 000000008008b2e0 x17: 0000000000000000
>  x18: 00000000801ffec0 x19: 00000000800957b0
>  x20: 00000000000000c8 x21: 00000000801ffdc8
>  x22: 000000008009909e x23: 0000000000000000
>  x24: 0000000000000000 x25: 0000000000000000
>  x26: 0000000000000000 x27: 0000000000000000
>  x28: 0000000000000000 x29: 00000000801ffc50
>
>  Code: a94417e4 a90217e4 a9051fe6 a90617e4 (3d801fe0)
>
> While executing instruction
>
>  str     q0, [sp, #112]
>
> in vsnprintf() prologue. This panic was observed only on Cortex A72 so
> far.
>
> This patch places ISBs on other strategic places as well.
>
> Also, this probably is the right fix for the issue workarounded in the
> commit 45f41c13 ("ARM: uniphier: add weird workaround code for LD20")


Thanks for addressing this issue.
Currently, I do not have a board in hand to test this.
(I do not commute to the office due to COVID-19 these days...)

I have another SoC board, but it does not integrate CA72.
I have ever seen this problem only on CA72.

Eventually, I will go to the office, and I can test this.
But, you do not need to wait for my test if other people
review it.

Thank you.







> Reported-by: Oleksandr Andrushchenko <oleksandr_andrushchenko at epam.com>
> Suggested-by: Julien Grall <julien.grall.oss at gmail.com>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk at epam.com>
> CC: Tom Rini <trini at konsulko.com>
> CC: Masahiro Yamada <yamada.masahiro at socionext.com>
> CC: Stefano Stabellini <sstabellini at kernel.org>
>
> --
>
> Changes from v1:
>  - Added ISBs under CONFIG_ARMV8_SET_SMPEN and erratas.
>  - Added Stefano, Julien and Oleksandr
> ---
>  arch/arm/cpu/armv8/start.S | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> index 99d126660d..002698b501 100644
> --- a/arch/arm/cpu/armv8/start.S
> +++ b/arch/arm/cpu/armv8/start.S
> @@ -120,6 +120,7 @@ pie_fixup_done:
>         mov     x0, #3 << 20
>         msr     cpacr_el1, x0                   /* Enable FP/SIMD */
>  0:
> +       isb
>
>         /*
>          * Enable SMPEN bit for coherency.
> @@ -132,6 +133,7 @@ pie_fixup_done:
>         mrs     x0, S3_1_c15_c2_1               /* cpuectlr_el1 */
>         orr     x0, x0, #0x40
>         msr     S3_1_c15_c2_1, x0
> +       isb
>  1:
>  #endif
>
> @@ -233,6 +235,7 @@ apply_a53_core_errata:
>         /* Enable data cache clean as data cache clean/invalidate */
>         orr     x0, x0, #1 << 44
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>         b 0b
>
> @@ -247,6 +250,7 @@ apply_a57_core_errata:
>         /* Disable write streaming no-allocate threshold */
>         orr     x0, x0, #3 << 27
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>
>  #ifdef CONFIG_ARM_ERRATA_826974
> @@ -254,6 +258,7 @@ apply_a57_core_errata:
>         /* Disable speculative load execution ahead of a DMB */
>         orr     x0, x0, #1 << 59
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>
>  #ifdef CONFIG_ARM_ERRATA_833471
> @@ -263,6 +268,7 @@ apply_a57_core_errata:
>             could impact performance. */
>         orr     x0, x0, #1 << 38
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>
>  #ifdef CONFIG_ARM_ERRATA_829520
> @@ -273,6 +279,7 @@ apply_a57_core_errata:
>             could impact performance. */
>         orr     x0, x0, #1 << 4
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>
>  #ifdef CONFIG_ARM_ERRATA_833069
> @@ -280,6 +287,7 @@ apply_a57_core_errata:
>         /* Disable Enable Invalidates of BTB bit */
>         and     x0, x0, #0xE
>         msr     S3_1_c15_c2_0, x0       /* cpuactlr_el1 */
> +       isb
>  #endif
>         b 0b
>  ENDPROC(apply_core_errata)
> --
> 2.27.0



--
Best Regards
Masahiro Yamada