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

[Tom Rini]

On Tue, Jul 07, 2020 at 08:37:53PM +0900, Masahiro Yamada wrote:
> 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"

Yes, thanks.  I've been wanting to put this directly to master (and not
part of the next merge) for easier bisecting should there be a problem.
I'm running this (and a few other risky patches) through CI now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200707/78e7d62b/attachment.sig>