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

[Masahiro Yamada]

On Mon, Jul 6, 2020 at 1:33 PM Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
>
> Hi.
>
> On Mon, Jun 29, 2020 at 12:29 AM Masahiro Yamada <masahiroy at kernel.org> wrote:
> >
> > 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.
> >
>
>
> Today I tested this patch.
>
> Yes, it fixes the CA72 problem on my board.
>
> Tested-by: Masahiro Yamada <yamada.masahiro at socionext.com>
>


Now that MW is open, I want to see this patch in mainline
because it fixes the CA-72 problem in the right way.


There was no object except two minor comments from Andre.


Can Tom fix up the commit description when he picks it up?


  "... after any change to system registers."
     --->
  "... after changes to some system registers."


  "instruction memory synchronization barrier"
    --->
  "instruction synchronization barrier"


Thanks.
-- 
Best Regards
Masahiro Yamada