Re: framebuffer corruption due to overlapping stp instructions on arm64

[Arnd Bergmann]

On Wed, Aug 8, 2018 at 5:15 PM Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
>
> On Wed, Aug 08, 2018 at 04:01:12PM +0100, Richard Earnshaw wrote:
> > On 08/08/18 15:12, Mikulas Patocka wrote:
> > > On Wed, 8 Aug 2018, Catalin Marinas wrote:
> > >> On Fri, Aug 03, 2018 at 01:09:02PM -0400, Mikulas Patocka wrote:
> - failing to write a few bytes
> - writing a few bytes that were written 16 bytes before
> - writing a few bytes that were written 16 bytes after
>
> > The overlapping writes in memcpy never write different values to the
> > same location, so I still feel this must be some sort of HW issue, not a
> > SW one.
>
> So do I (my interpretation is that it combines or rather skips some of
> the writes to the same 16-byte address as it ignores the data strobes).

Maybe it just always writes to the wrong location, 16 bytes apart for one of
the stp instructions. Since we are usually dealing with a pair of overlapping
'stp', both unaligned, that could explain both the missing bytes (we write
data to the wrong place, but overwrite it with the correct data right away)
and the extra copy (we write it to the wrong place, but then write the correct
data to the correct place as well).

This sounds a bit like what the original ARM CPUs did on unaligned
memory access, where a single aligned 4-byte location was accessed,
but the bytes swapped around.

There may be a few more things worth trying out or analysing from
the recorded past failures to understand more about how it goes
wrong:

- For which data lengths does it fail? Having two overlapping
  unaligned stp is something that only happens for 16..96 byte
  memcpy.

- What if we use a pair of str instructions instead of an stp in
  a modified memcpy? Does it now write to still write to the
  wrong place 16 bytes away, just 8 bytes away, or correctly?

- Does it change in any way if we do the overlapping writes
  in the reverse order? E.g. for the 16..64 byte case:

diff --git a/sysdeps/aarch64/memcpy.S b/sysdeps/aarch64/memcpy.S
index 7e1163e6a0..09d0160bdf 100644
--- a/sysdeps/aarch64/memcpy.S
+++ b/sysdeps/aarch64/memcpy.S
@@ -102,11 +102,11 @@ ENTRY (MEMCPY)
        tbz     tmp1, 5, 1f
        ldp     B_l, B_h, [src, 16]
        ldp     C_l, C_h, [srcend, -32]
-       stp     B_l, B_h, [dstin, 16]
        stp     C_l, C_h, [dstend, -32]
+       stp     B_l, B_h, [dstin, 16]
 1:
-       stp     A_l, A_h, [dstin]
        stp     D_l, D_h, [dstend, -16]
+       stp     A_l, A_h, [dstin]
        ret

        .p2align 4

        Arnd