On Thu, May 31, 2018 at 2:21 PM, Scott D Phillips < scott.d.phill...@intel.com> wrote:
> Jason Ekstrand <ja...@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. > I think the worst case is that someone will try to make a vertex buffer out of it and maybe get cache corruption. I think that case can theoretically lead to hangs (if it's used for the gpu_memcpy of surface states) but it's pretty hard to hit. I think leaving it as a warning is probably ok.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev