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.masah...@socionext.com> wrote: > > > > Hi. > > > > On Mon, Jun 29, 2020 at 12:29 AM Masahiro Yamada <masahi...@kernel.org> > > wrote: > > > > > > On Wed, Jun 24, 2020 at 11:07 AM Volodymyr Babchuk > > > <volodymyr_babc...@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.masah...@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
signature.asc
Description: PGP signature