From e1fe4a64d3f319731152bb48e6cb15e899452b15 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 7 Nov 2022 13:49:51 -0500 Subject: [PATCH] panfrost: Require 64-byte alignment on imports While Panfrost allocates linear images with strides that are a multiple of 64 bytes, other dma-buf producers on the system may not satisfy this requirement. However, at least on v7 and newer, any image with a regular format must have a stride that is a multiple of 64 bytes. This fixes a real bug in an application that created a linear R8_UNORM image with stride 480 bytes, imported it as an EGL_image, and then tried to texture from it with the GPU. Previously, the driver allowed this situation but it resulted in an imprecise fault from the GPU. This patch corrects the driver to reject the import as invalid due to the unaligned stride, ensuring we never attempt to texture from such a resource. To implement, we add some new layout queries to centralize knowledge about the stride alignment requirements, and we sprinkle in asserts to show how the invariant is upheld throughout the lifecycle of image creation to texturing. Cc: mesa-stable Signed-off-by: Alyssa Rosenzweig Reviewed-by: Boris Brezillon Part-of: (cherry picked from commit 811f8a19469722bea32f3c539b8cf0939fe3b057) --- .pick_status.json | 2 +- src/panfrost/lib/pan_layout.c | 54 ++++++++++++++++++++++++++++++++-- src/panfrost/lib/pan_texture.c | 8 +++++ src/panfrost/lib/pan_texture.h | 6 ++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 7186762074b..f31cd2e7600 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -220,7 +220,7 @@ "description": "panfrost: Require 64-byte alignment on imports", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index 127ad6c7815..51b511a9a9d 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -275,6 +275,34 @@ panfrost_texture_offset(const struct pan_image_layout *layout, (surface_idx * layout->slices[level].surface_stride); } +/* + * Return the minimum stride alignment in bytes for a given texture format. + * + * There is no format on any supported Mali with a minimum alignment greater + * than 64 bytes, but 64 bytes is the required alignment of all regular formats + * in v7 and newer. If this alignment is not met, imprecise faults may be + * raised. + * + * This may not be necessary on older hardware but we enforce it there too for + * uniformity. If this poses a problem there, we'll need a solution that can + * handle v7 as well. + * + * Certain non-regular formats require smaller power-of-two alignments. + * This requirement could be loosened in the future if there is a compelling + * reason, by making this query more precise. + */ +uint32_t +pan_stride_align_B(UNUSED enum pipe_format format) +{ + return 64; +} + +bool +pan_is_stride_aligned(enum pipe_format format, uint32_t stride_B) +{ + return (stride_B % pan_stride_align_B(format)) == 0; +} + bool pan_image_layout_init(struct pan_image_layout *layout, const struct pan_image_explicit_layout *explicit_layout) @@ -288,8 +316,15 @@ pan_image_layout_init(struct pan_image_layout *layout, layout->nr_slices > 1 || layout->crc_mode == PAN_IMAGE_CRC_INBAND)) return false; - /* Mandate 64 byte alignement */ - if (explicit_layout && (explicit_layout->offset & 63)) + /* Require both offsets and strides to be aligned to the hardware + * requirement. Panfrost allocates offsets and strides like this, so + * this requirement is satisfied by any image that was exported from + * another process with Panfrost. However, it does restrict imports of + * EGL external images. + */ + if (explicit_layout && + !(pan_is_stride_aligned(layout->format, explicit_layout->offset) && + pan_is_stride_aligned(layout->format, explicit_layout->row_stride))) return false; unsigned fmt_blocksize = util_format_get_blocksize(layout->format); @@ -343,10 +378,19 @@ pan_image_layout_init(struct pan_image_layout *layout, row_stride = explicit_layout->row_stride; } else if (linear) { - /* Keep lines alignment on 64 byte for performance */ + /* Keep lines alignment on 64 byte for performance. + * + * Note that this is a multiple of the minimum + * stride alignment, so the hardware requirement is + * satisfied as a result. + */ row_stride = ALIGN_POT(row_stride, 64); } + + assert(pan_is_stride_aligned(layout->format, row_stride) && + "alignment gauranteed in both code paths"); + unsigned slice_one_size = row_stride * (effective_height / block_size.height); /* Compute AFBC sizes if necessary */ @@ -386,6 +430,10 @@ pan_image_layout_init(struct pan_image_layout *layout, slice->surface_stride = slice_one_size; + assert(pan_is_stride_aligned(layout->format, slice->surface_stride) && + "integer multiple of aligned is still aligned, " + "and AFBC header is at least 64 byte aligned"); + /* Compute AFBC sizes if necessary */ offset += slice_full_size; diff --git a/src/panfrost/lib/pan_texture.c b/src/panfrost/lib/pan_texture.c index 446fffe234c..9fa8c4496f7 100644 --- a/src/panfrost/lib/pan_texture.c +++ b/src/panfrost/lib/pan_texture.c @@ -247,10 +247,18 @@ panfrost_get_surface_strides(const struct pan_image_layout *layout, * repurposed as a Y offset which we don't use */ *row_stride = PAN_ARCH < 7 ? 0 : slice->row_stride; *surf_stride = slice->afbc.surface_stride; + + /* Row stride alignment requirement does not apply to AFBC */ } else { *row_stride = slice->row_stride; *surf_stride = slice->surface_stride; + + /* Particular for linear, the row stride must be aligned */ + assert(pan_is_stride_aligned(layout->format, *row_stride)); } + + /* All surface strides are aligned, required for linear */ + assert(pan_is_stride_aligned(layout->format, *surf_stride)); } static mali_ptr diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index bd773fa374e..875f008e71b 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -248,6 +248,12 @@ panfrost_from_legacy_stride(unsigned legacy_stride, enum pipe_format format, uint64_t modifier); +uint32_t +pan_stride_align_B(enum pipe_format format); + +bool +pan_is_stride_aligned(enum pipe_format format, unsigned stride_B); + struct pan_surface { union { mali_ptr data;