<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Sep 2, 2017 at 1:17 AM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">If ANV_BO_CACHE_IMPORT_IGNORE_<wbr>SIZE_PARAM is set, then the function<br>
ignores the 'size' parameter. Instead, on a cache hit, it uses the cached bo's<br>
size; on a cache miss, it queries the fd's size.<br></blockquote><div><br></div><div>Again, I don't really like the flag.  I see two possible alternate solutions to your problem:</div><div><br></div><div> 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.</div><div><br></div><div> 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.</div><div><br></div><div> 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.<br></div><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This prepares for implementing VK_ANDROID_native_buffer, in which the<br>
API provides no explicit size for the gralloc buffer. Instead, we must<br>
query the dma_buf's size with lseek.<br>
---<br>
 src/intel/vulkan/anv_<wbr>allocator.c | 9 ++++++++-<br>
 src/intel/vulkan/anv_private.h   | 6 ++++++<br>
 2 files changed, 14 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/src/intel/vulkan/anv_<wbr>allocator.c b/src/intel/vulkan/anv_<wbr>allocator.c<br>
index 28d00f4d0b2..af5d565b70a 100644<br>
--- a/src/intel/vulkan/anv_<wbr>allocator.c<br>
+++ b/src/intel/vulkan/anv_<wbr>allocator.c<br>
@@ -1277,7 +1277,8 @@ anv_bo_cache_import(struct anv_device *device,<br>
    pthread_mutex_lock(&cache-><wbr>mutex);<br>
<br>
    /* The kernel is going to give us whole pages anyway */<br>
-   size = align_u64(size, 4096);<br>
+   if (!(flags & ANV_BO_CACHE_IMPORT_IGNORE_<wbr>SIZE_PARAM))<br>
+      size = align_u64(size, 4096);<br>
<br>
    uint32_t gem_handle = anv_gem_fd_to_handle(device, fd);<br>
    if (!gem_handle) {<br>
@@ -1287,6 +1288,9 @@ anv_bo_cache_import(struct anv_device *device,<br>
<br>
    struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(<wbr>cache, gem_handle);<br>
    if (bo) {<br>
+      if (flags & ANV_BO_CACHE_IMPORT_IGNORE_<wbr>SIZE_PARAM)<br>
+         size = bo->bo.size;<br>
+<br>
       if (bo->bo.size != size) {<br>
          pthread_mutex_unlock(&cache-><wbr>mutex);<br>
          return vk_error(VK_ERROR_INVALID_<wbr>EXTERNAL_HANDLE_KHR);<br>
@@ -1302,6 +1306,9 @@ anv_bo_cache_import(struct anv_device *device,<br>
        * this sort of attack but only if it can trust the buffer size.<br>
        */<br>
       off_t import_size = lseek(fd, 0, SEEK_END);<br>
+      if (flags & ANV_BO_CACHE_IMPORT_IGNORE_<wbr>SIZE_PARAM)<br>
+         size = import_size;<br>
+<br>
       if (import_size == (off_t)-1 || import_size != size) {<br>
          anv_gem_close(device, gem_handle);<br>
          pthread_mutex_unlock(&cache-><wbr>mutex);<br>
diff --git a/src/intel/vulkan/anv_<wbr>private.h b/src/intel/vulkan/anv_<wbr>private.h<br>
index 014848fcef2..9c73939feb0 100644<br>
--- a/src/intel/vulkan/anv_<wbr>private.h<br>
+++ b/src/intel/vulkan/anv_<wbr>private.h<br>
@@ -589,6 +589,12 @@ struct anv_bo_cache {<br>
 enum anv_bo_cache_import_bits {<br>
    /** Do not close the fd after import. */<br>
    ANV_BO_CACHE_IMPORT_NO_CLOSE_<wbr>FD = (1 << 0),<br>
+<br>
+   /**<br>
+    * Ignore the anv_bo_cache_import::size parameter. Instead, on a cache hit,<br>
+    * use the cached bo's size; on a cache miss, query the fd's size.<br>
+    */<br>
+   ANV_BO_CACHE_IMPORT_IGNORE_<wbr>SIZE_PARAM = (1 << 1),<br>
 };<br>
 typedef uint32_t anv_bo_cache_import_flags_t;<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.13.5<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>