[Mesa-dev] [PATCH v2] isl: don't crash when creating a huge image

[Samuel Iglesias Gonsálvez]

On 11/01/18 23:27, Jason Ekstrand wrote:
> Sorry.  It's taken a bit of time but the WG has a decision and that is
> that we are supposed to throw VK_ERROR_OUT_OF_DEVICE_MEMORY in this
> case.  I believe you had an alternate version of the patch that did
> something like that.  If so, we should revive it.
>

Thanks for the info!

I am thinking on keep this patch and add the following one on top like this:

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index ba932ba47c3..652f28460d9 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -335,6 +335,9 @@ make_surface(const struct anv_device *dev,
     */
    assert(ok);
 
+   if (anv_surf->isl.size == UINT64_MAX)
+      return VK_ERROR_OUT_OF_DEVICE_MEMORY;
+
    image->planes[plane].aux_usage = ISL_AUX_USAGE_NONE;
 
    add_surface(image, anv_surf, plane);

Hence, we fail in vkCreateImage() with VK_ERROR_OUT_OF_DEVICE_MEMORY as
the suggested change to the spec, with zero changes to other users of
isl_surf_init() . If you agree, I will send both patches again for review.

Sam

> On Thu, Jan 11, 2018 at 5:04 AM, Samuel Iglesias Gonsálvez
> <siglesias at igalia.com <mailto:siglesias at igalia.com>> wrote:
>
>     This patch is still unreviewed.
>
>     Sam
>
>
>     On 14/11/17 09:45, Samuel Iglesias Gonsálvez wrote:
>     > The HW has some limits but, according to the spec, we can create
>     > the image as it has not yet any memory backing it. This patch
>     > logs a debug error and set the size to the UINT64_MAX in order to
>     > avoid allocating actual memory later.
>     >
>     > Fixes the crashes on BDW for the following tests:
>     >
>     > dEQP-VK.pipeline.render_to_image.core.2d_array.huge.*
>     > dEQP-VK.pipeline.render_to_image.core.cube_array.huge.*
>     >
>     > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com
>     <mailto:siglesias at igalia.com>>
>     > ---
>     >  src/intel/isl/isl.c | 13 +++++++++----
>     >  1 file changed, 9 insertions(+), 4 deletions(-)
>     >
>     > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
>     > index 59f512fc050..cd7f2fcd4cb 100644
>     > --- a/src/intel/isl/isl.c
>     > +++ b/src/intel/isl/isl.c
>     > @@ -26,6 +26,7 @@
>     >  #include <stdio.h>
>     >
>     >  #include "genxml/genX_bits.h"
>     > +#include "common/intel_log.h"
>     >
>     >  #include "isl.h"
>     >  #include "isl_gen4.h"
>     > @@ -1481,8 +1482,10 @@ isl_surf_init_s(const struct isl_device *dev,
>     >         *
>     >         * This comment is applicable to all Pre-gen9 platforms.
>     >         */
>     > -      if (size > (uint64_t) 1 << 31)
>     > -         return false;
>     > +      if (size > (uint64_t) 1 << 31) {
>     > +         intel_logd("%s: Surface size is bigger than the
>     supported by the HW: %ld > (1 << 31)", __func__, size);
>     > +         size = UINT64_MAX;
>     > +      }
>     >     } else {
>     >        /* From the Skylake PRM Vol 5, Maximum Surface Size in Bytes:
>     >         *    "In addition to restrictions on maximum height,
>     width, and depth,
>     > @@ -1490,8 +1493,10 @@ isl_surf_init_s(const struct isl_device *dev,
>     >         *     All pixels within the surface must be contained
>     within 2^38 bytes
>     >         *     of the base address."
>     >         */
>     > -      if (size > (uint64_t) 1 << 38)
>     > -         return false;
>     > +      if (size > (uint64_t) 1 << 38) {
>     > +         intel_logd("%s: Surface size is bigger than the
>     supported by the HW: %ld > (1 << 38)", __func__, size);
>     > +         size = UINT64_MAX;
>     > +      }
>     >     }
>     >
>     >     *surf = (struct isl_surf) {
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180112/0f16501e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180112/0f16501e/attachment.sig>