[Mesa-dev] [v4, 18/23] anv/allocator: Support softpin in the BO cache

[Scott D Phillips]

Jason Ekstrand <jason at jlekstrand.net> writes:

> ---
>  src/intel/vulkan/anv_allocator.c | 54 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
> index 697da5f..f18e015 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -1240,7 +1240,8 @@ anv_bo_cache_lookup(struct anv_bo_cache *cache, uint32_t gem_handle)
>  #define ANV_BO_CACHE_SUPPORTED_FLAGS \
>     (EXEC_OBJECT_WRITE | \
>      EXEC_OBJECT_ASYNC | \
> -    EXEC_OBJECT_SUPPORTS_48B_ADDRESS)
> +    EXEC_OBJECT_SUPPORTS_48B_ADDRESS | \
> +    EXEC_OBJECT_PINNED)
>  
>  VkResult
>  anv_bo_cache_alloc(struct anv_device *device,
> @@ -1269,6 +1270,14 @@ anv_bo_cache_alloc(struct anv_device *device,
>  
>     bo->bo.flags = bo_flags;
>  
> +   if (!anv_vma_alloc(device, &bo->bo)) {
> +      anv_gem_close(device, bo->bo.gem_handle);
> +      vk_free(&device->alloc, bo);
> +      return vk_errorf(device->instance, NULL,
> +                       VK_ERROR_OUT_OF_DEVICE_MEMORY,
> +                       "failed to allocate virtual address for BO");
> +   }
> +
>     assert(bo->bo.gem_handle);
>  
>     pthread_mutex_lock(&cache->mutex);
> @@ -1310,6 +1319,38 @@ anv_bo_cache_import(struct anv_device *device,
>        new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_WRITE;
>        new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_ASYNC;
>        new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +      new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_PINNED;
> +
> +      /* It's theoretically possible for a BO to get imported such that it's
> +       * both pinned and not pinned.  The only way this can happen is if it
> +       * gets imported as both a semaphore and a memory object and that would
> +       * be an application error.  Just fail out in that case.
> +       */
> +      if ((bo->bo.flags & EXEC_OBJECT_PINNED) !=
> +          (bo_flags & EXEC_OBJECT_PINNED)) {
> +         pthread_mutex_unlock(&cache->mutex);
> +         return vk_errorf(device->instance, NULL,
> +                          VK_ERROR_INVALID_EXTERNAL_HANDLE,
> +                          "The same BO was imported two different ways");
> +      }
> +
> +      /* It's also theoretically possible that someone could export a BO from
> +       * one heap and import it into another or to import the same BO into two
> +       * different heaps.  If this happens, we could potentially end up both
> +       * allowing and disallowing 48-bit addresses.  There's not much we can
> +       * do about it if we're pinning so we just throw a warning and hope no
> +       * app is actually that stupid.
> +       */
> +      if ((new_flags & EXEC_OBJECT_PINNED) &&
> +          (bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) &&
> +          !(bo_flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) {
> +         vk_debug_report(&device->instance->debug_report_callbacks,
> +                         VK_DEBUG_REPORT_WARNING_BIT_EXT,
> +                         VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
> +                         (uint64_t) (uintptr_t) device,
> +                         0, 0, "anv",
> +                         "Import of the same BO on multiple heaps");
> +      }

Hmm, it seems like we should error here too. If the BO is getting used
in a way that doesn't support 48 bit addresses then at best we can hit
one of our known iffy cases, and at worst we might cram a 64 bit address
into a narrower offset field and then possibly get GPU hangs. Or if
there is no such case like this then I think it probably needs some more
explanation about why this isn't an error.