[Mesa-dev] [PATCH 20/23] anv: Add flag IGNORE_SIZE_PARAM for anv_bo_cache_import()

[Jason Ekstrand]

On Sat, Sep 2, 2017 at 1:17 AM, Chad Versace <chadversary at chromium.org>
wrote:

> If ANV_BO_CACHE_IMPORT_IGNORE_SIZE_PARAM is set, then the function
> ignores the 'size' parameter. Instead, on a cache hit, it uses the cached
> bo's
> size; on a cache miss, it queries the fd's size.
>

Again, I don't really like the flag.  I see two possible alternate
solutions to your problem:

 1) Call lseek in the VK_ANDROID_native_buffer implementation and pass that
size into bo_cache_import.  This is a bit gross and adds a tiny bit more
overhead to a native buffer import but not much.

 2) Drop the size parameter from anv_bo_cache_import and make it set the
size from the lseek like you want for Android.  Then make the two callers
of anv_bo_cache_import check the size themselves.  There's no exceptionally
good reason why we need to do the size check inside the import function.
Worst case, moving it outside makes the error path a tiny bit more
expensive.  Meh.

 3) If you really care about keeping the size check centralized, do 2) and
add a anv_bo_cache_import_with_size helper which does the size check.

I think I'm mildly inclined towards option 2 on the same principle as
before of keeping the VK_KHR_external_memory_fd details with the
VK_KHR_external_memory_fd implementation.


> This prepares for implementing VK_ANDROID_native_buffer, in which the
> API provides no explicit size for the gralloc buffer. Instead, we must
> query the dma_buf's size with lseek.
> ---
>  src/intel/vulkan/anv_allocator.c | 9 ++++++++-
>  src/intel/vulkan/anv_private.h   | 6 ++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_
> allocator.c
> index 28d00f4d0b2..af5d565b70a 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -1277,7 +1277,8 @@ anv_bo_cache_import(struct anv_device *device,
>     pthread_mutex_lock(&cache->mutex);
>
>     /* The kernel is going to give us whole pages anyway */
> -   size = align_u64(size, 4096);
> +   if (!(flags & ANV_BO_CACHE_IMPORT_IGNORE_SIZE_PARAM))
> +      size = align_u64(size, 4096);
>
>     uint32_t gem_handle = anv_gem_fd_to_handle(device, fd);
>     if (!gem_handle) {
> @@ -1287,6 +1288,9 @@ anv_bo_cache_import(struct anv_device *device,
>
>     struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(cache,
> gem_handle);
>     if (bo) {
> +      if (flags & ANV_BO_CACHE_IMPORT_IGNORE_SIZE_PARAM)
> +         size = bo->bo.size;
> +
>        if (bo->bo.size != size) {
>           pthread_mutex_unlock(&cache->mutex);
>           return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
> @@ -1302,6 +1306,9 @@ anv_bo_cache_import(struct anv_device *device,
>         * this sort of attack but only if it can trust the buffer size.
>         */
>        off_t import_size = lseek(fd, 0, SEEK_END);
> +      if (flags & ANV_BO_CACHE_IMPORT_IGNORE_SIZE_PARAM)
> +         size = import_size;
> +
>        if (import_size == (off_t)-1 || import_size != size) {
>           anv_gem_close(device, gem_handle);
>           pthread_mutex_unlock(&cache->mutex);
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 014848fcef2..9c73939feb0 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -589,6 +589,12 @@ struct anv_bo_cache {
>  enum anv_bo_cache_import_bits {
>     /** Do not close the fd after import. */
>     ANV_BO_CACHE_IMPORT_NO_CLOSE_FD = (1 << 0),
> +
> +   /**
> +    * Ignore the anv_bo_cache_import::size parameter. Instead, on a cache
> hit,
> +    * use the cached bo's size; on a cache miss, query the fd's size.
> +    */
> +   ANV_BO_CACHE_IMPORT_IGNORE_SIZE_PARAM = (1 << 1),
>  };
>  typedef uint32_t anv_bo_cache_import_flags_t;
>
> --
> 2.13.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170902/8a1e9916/attachment-0001.html>