[Mesa-dev] [PATCH 2/2] i965/tiled_memcpy: Protect against wrong alignments

[Roland Scheidegger]

Am 06.04.2016 um 01:50 schrieb Jason Ekstrand:
> On Tue, Apr 5, 2016 at 4:26 PM, Chad Versace <chad.versace at intel.com
> <mailto:chad.versace at intel.com>> wrote:
> 
>     On 04/04/2016 06:04 PM, Jason Ekstrand wrote:
>     > It's possible, when doing an x-tiled copy, to end up with a case
>     where the
>     > bytes parameter is equal to 16 but the pointer is not actually
>     aligned.
>     > This causes asserts in debug mode and segfaults in release builds
>     due to
>     > doing an aligned operation on an unaligned pointer.
>     >
>     > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93962
>     > ---
>     >  src/mesa/drivers/dri/i965/intel_tiled_memcpy.c | 48
>     +++++++++++++-------------
>     >  1 file changed, 24 insertions(+), 24 deletions(-)
>     >
>     > diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>     b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>     > index 19079d0..823d8b0 100644
>     > --- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>     > +++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>     > @@ -85,19 +85,19 @@ rgba8_copy_aligned_dst(void *dst, const void
>     *src, size_t bytes)
>     >     uint8_t const *s = src;
>     >
>     >  #ifdef __SSSE3__
>     > -   if (bytes == 16) {
>     > -      assert(!(((uintptr_t)dst) & 0xf));
>     > -      rgba8_copy_16_aligned_dst(d+ 0, s+ 0);
>     > -      return dst;
>     > -   }
>     > +   if ((((uintptr_t)dst) & 0xf) == 0) {
>     > +      if (bytes == 16) {
>     > +         rgba8_copy_16_aligned_dst(d+ 0, s+ 0);
>     > +         return dst;
>     > +      }
> 
>     [snip]
> 
>     I see the bug... Sometimes (dst, src) points into (tiled, linear
>     user ptr) memory, and
>     other times into (linear user ptr, tiled) memory. So we can never
>     assume that either
>     address is aligned, as it may be a user ptr.
> 
> 
> No, that is not correct.  We use the aligned_dst version for when dst is
> tiled and aligned_src for when src is tiled.  The problem is that, when
> copying a 4 pixel wide region from a X-tiled buffer, it can end up
> getting called with bytes == 16 and an unaligned pointer.
>  

FWIW if that's happening frequently enough could just use unaligned
loads/stores. There's no penalty for those either way if the memory is
actually aligned on modern intel cpus (albeit your p4 is going to
disagree very heavily), and even if it's not aligned, the penalty should
be smaller than using scalar code...

Roland