msm_gem_vm_create() creates every drm_gpuvm without DRM_GPUVM_RESV_PROTECTED, on the grounds that it makes drm_gpuvm_bo_evict() lose track of evicted external objects. It does not: drm_gpuvm_bo_evict() still sets drm_gpuvm_bo::evicted on an extobj, and drm_gpuvm_prepare_objects() moves any such extobj onto the evicted list before drm_gpuvm_validate() looks at it. The VM_BIND submit path always calls the two in that order.
Userspace managed VMs already touch the extobj and evicted lists only with the VM's resv held: VMAs are created and linked by VM_BIND under the VM resv, msm_gem_vma_close() asserts it, and every drm_gpuvm_bo_put() which can drop the last reference of a VM_BIND vm_bo runs with it held, via msm_gem_lock_vm_and_obj(), with_vm_locks() or the object free path. The internal spinlocks buy nothing there, so set DRM_GPUVM_RESV_PROTECTED for those VMs. Kernel managed VMs are left alone. The legacy submit path holds a vm_bo reference per BO and drops it in msm_submit_retire() with only the object's resv held, which could be the last reference once the VMA is gone. VM_BIND contexts never get there, the previous patch having made MSM_GEM_SUBMIT reject a submit_bo table from them. This is also what two pass locking in drm_gpuvm requires, which a following patch makes use of. Cc: Abhinav Kumar <[email protected]> Cc: Alice Ryhl <[email protected]> Cc: Anna Maniscalco <[email protected]> Cc: Antonino Maniscalco <[email protected]> Cc: Boris Brezillon <[email protected]> Cc: Danilo Krummrich <[email protected]> Cc: David Airlie <[email protected]> Cc: Dmitry Baryshkov <[email protected]> Cc: Jessica Zhang <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liviu Dudau <[email protected]> Cc: Lyude Paul <[email protected]> Cc: Maarten Lankhorst <[email protected]> Cc: Marijn Suijten <[email protected]> Cc: Maxime Ripard <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Rob Clark <[email protected]> Cc: Rodrigo Vivi <[email protected]> Cc: Sean Paul <[email protected]> Cc: Shuah Khan <[email protected]> Cc: Simona Vetter <[email protected]> Cc: Steven Price <[email protected]> Cc: Thomas Hellström <[email protected]> Cc: Thomas Zimmermann <[email protected]> Signed-off-by: Matthew Brost <[email protected]> Assisted-by: LLM --- v3: - Rely on VM_BIND contexts not being able to pass a submit_bo table, now enforced by the previous patch (Sashiko) --- drivers/gpu/drm/msm/msm_gem_vma.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 1badec3caa7b..c2b2415e86c9 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -818,11 +818,19 @@ msm_gem_vm_create(struct drm_device *drm, struct msm_mmu *mmu, const char *name, u64 va_start, u64 va_size, bool managed) { /* - * We mostly want to use DRM_GPUVM_RESV_PROTECTED, except that - * makes drm_gpuvm_bo_evict() a no-op for extobjs (ie. we loose - * tracking that an extobj is evicted) :facepalm: + * Userspace managed (VM_BIND) VMs only ever touch the gpuvm's extobj + * and evicted lists with the VM's resv held, so use + * DRM_GPUVM_RESV_PROTECTED for those. drm_gpuvm_bo_evict() cannot + * put an extobj on the evicted list there, but it records the + * eviction and drm_gpuvm_prepare_objects() moves it onto the list + * before drm_gpuvm_validate() runs, so nothing is lost. + * + * Kernel managed VMs keep the internal spinlocks, since the legacy + * submit path can drop the last vm_bo reference with only the + * object's resv held (see msm_submit_retire()). VM_BIND contexts + * cannot reach that path, as they may not pass a submit_bo table. */ - enum drm_gpuvm_flags flags = 0; + enum drm_gpuvm_flags flags = managed ? 0 : DRM_GPUVM_RESV_PROTECTED; struct msm_gem_vm *vm; struct drm_gem_object *dummy_gem; int ret = 0; -- 2.34.1
