We currently restrict what a VM is allowed to do based on two states: panthor_vm::destroyed and panthor_vm_pgtable::unusable, but we'll soon need a no-unmap restriction to fix the unplug logic.
Instead of adding a third boolean that would reflect this new limitation, let's overhaul the current restriction logic by adding separate restriction flags representing the operations we want to prevent (map, unmap and use). Map and use restrictions are set everywhere we were previously calling panthor_vm_pgtable_declare_unusable() or setting ::destroyed to true, since that's what those two flags were preventing. We also add restriction checks in panthor_vm_pgtable_prepare_[un]map_op_ctx() and panthor_vm_pgtable_exec_op() and drop the ones we had in panthor_vm_bind_job_create() since they are redundant. Signed-off-by: Boris Brezillon <[email protected]> --- drivers/gpu/drm/panthor/panthor_mmu.c | 134 ++++++++++++++++++++++------------ 1 file changed, 88 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c index 041836552953..1b401fd0d2d7 100644 --- a/drivers/gpu/drm/panthor/panthor_mmu.c +++ b/drivers/gpu/drm/panthor/panthor_mmu.c @@ -215,6 +215,25 @@ struct panthor_as_op_ctx { } map; }; +/** + * enum panthor_as_restriction - List of restrictions that can apply to an AS. + * + * An AS always starts unrestricted, but based on the faults or device state + * changes, restrictions can be added over time. Restrictions can't be removed + * though. Once a VM is restricted, a new one must be created to lift the + * restrictions. + */ +enum panthor_as_restriction { + /** @PANTHOR_AS_FORBID_MAP: The AS can't map new buffers. */ + PANTHOR_AS_FORBID_MAP = BIT(0), + + /** @PANTHOR_AS_FORBID_UNMAP: The AS can't remove existing mappings. */ + PANTHOR_AS_FORBID_UNMAP = BIT(1), + + /** @PANTHOR_AS_FORBID_USE: The AS can't become active again. */ + PANTHOR_AS_FORBID_USE = BIT(2), +}; + /** * struct panthor_as - Used to managed a GPU address space. */ @@ -275,25 +294,8 @@ struct panthor_as { struct list_head lru_node; } hw_slot; - /** - * @unusable: True if the AS has turned unusable because something - * bad happened during an asynchronous request. - * - * We don't try to recover from such failures, because this implies - * informing userspace about the specific operation that failed, and - * hoping the userspace driver can replay things from there. This all - * sounds very complicated for little gain. - * - * Instead, we should just flag the AS as unusable, and fail any - * further request targeting this AS. - * - * We also provide a way to query an AS state, so userspace can - * destroy it and create a new one. - * - * As an analogy, this would be mapped to a VK_ERROR_DEVICE_LOST - * situation, where the logical device needs to be re-created. - */ - bool unusable; + /** @restrictions: Bitmask of panthor_as_restriction flags. */ + atomic_t restrictions; /** * @unhandled_fault: Unhandled fault happened. @@ -411,13 +413,6 @@ struct panthor_vm { /** @for_mcu: True if this is the MCU VM. */ bool for_mcu; - /** - * @destroyed: True if the VM was destroyed. - * - * No further bind requests should be queued to a destroyed VM. - */ - bool destroyed; - /** * @dummy: Dummy object used for sparse mappings. * @@ -693,7 +688,9 @@ bool panthor_vm_has_unhandled_faults(struct panthor_vm *vm) */ bool panthor_vm_is_unusable(struct panthor_vm *vm) { - return vm->as->unusable; + return (atomic_read(&vm->as->restrictions) & + (PANTHOR_AS_FORBID_USE | PANTHOR_AS_FORBID_MAP | + PANTHOR_AS_FORBID_UNMAP)); } static void panthor_as_release_hw_slot_locked(struct panthor_as *as) @@ -752,6 +749,11 @@ int panthor_vm_active(struct panthor_vm *vm) mutex_lock(&as->op_lock); mutex_lock(&ptdev->mmu->as.slots_lock); + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_USE) { + ret = -EINVAL; + goto out_unlock; + } + if (refcount_inc_not_zero(&as->active_cnt)) goto out_unlock; @@ -920,21 +922,29 @@ static size_t get_pgsize(u64 addr, size_t size, size_t *count) return SZ_2M; } -static void panthor_as_declare_unusable(struct panthor_as *as) +static void panthor_as_restrict_usage_locked(struct panthor_as *as, + u32 new_restrictions) { struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base); int cookie; - if (as->unusable) - return; + lockdep_assert_held(&as->op_lock); - as->unusable = true; - mutex_lock(&ptdev->mmu->as.slots_lock); - if (as->hw_slot.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) { - panthor_mmu_as_disable(ptdev, as->hw_slot.id, false); - drm_dev_exit(cookie); + if (new_restrictions & PANTHOR_AS_FORBID_USE) { + guard(mutex)(&ptdev->mmu->as.slots_lock); + if (as->hw_slot.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) { + /* Try to disable the AS. If as_disable() passed, this should cause + * a fault on the next memory access. If it failed, a reset is + * scheduled to recover from the GPU hang. + * We intentionally don't call release_as_locked() here, because + * this would mess up with the active_cnt refcount. + */ + panthor_mmu_as_disable(ptdev, as->hw_slot.id, false); + drm_dev_exit(cookie); + } } - mutex_unlock(&ptdev->mmu->as.slots_lock); + + atomic_or(new_restrictions, &as->restrictions); } static void panthor_as_unmap_pages(struct panthor_as *as, u64 iova, u64 size) @@ -970,7 +980,9 @@ static void panthor_as_unmap_pages(struct panthor_as *as, u64 iova, u64 size) * so flag the VM unusable to make sure it's not going * to be used anymore. */ - panthor_as_declare_unusable(as); + panthor_as_restrict_usage_locked(as, + PANTHOR_AS_FORBID_USE | + PANTHOR_AS_FORBID_MAP); /* If we don't make progress, we're screwed. That also means * something else prevents us from unmapping the region, but @@ -1046,7 +1058,9 @@ panthor_as_map_pages(struct panthor_as *as, u64 iova, int prot, * table pages behind. */ panthor_as_unmap_pages(as, start_iova, iova - start_iova); - panthor_as_declare_unusable(as); + panthor_as_restrict_usage_locked(as, + PANTHOR_AS_FORBID_USE | + PANTHOR_AS_FORBID_MAP); return ret; } } @@ -1339,6 +1353,9 @@ static int panthor_as_prepare_map_op_ctx(struct panthor_as_op_ctx *op_ctx, struct sg_table *sgt = NULL; int ret; + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_MAP) + return -EINVAL; + if (!bo) return -EINVAL; @@ -1431,6 +1448,9 @@ static int panthor_as_prepare_unmap_op_ctx(struct panthor_as_op_ctx *op_ctx, u32 pt_count = 0; int ret; + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP) + return -EINVAL; + memset(op_ctx, 0, sizeof(*op_ctx)); op_ctx->va.range = size; op_ctx->va.addr = va; @@ -1640,7 +1660,12 @@ static void panthor_vm_destroy(struct panthor_vm *vm) as = vm->as; ptdev = container_of(as->base.drm, struct panthor_device, base); - vm->destroyed = true; + + scoped_guard(mutex, &as->op_lock) { + panthor_as_restrict_usage_locked(as, + PANTHOR_AS_FORBID_USE | + PANTHOR_AS_FORBID_MAP); + } /* Tell scheduler to stop all GPU work related to this VM */ if (refcount_read(&as->active_cnt) > 0) @@ -2150,7 +2175,7 @@ struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c mutex_lock(&vm->heaps.lock); if (!vm->heaps.pool && create) { - if (vm->destroyed) + if (panthor_vm_is_unusable(vm)) pool = ERR_PTR(-EINVAL); else pool = panthor_heap_pool_create(ptdev, vm); @@ -2520,6 +2545,17 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo) if (!mutex_trylock(&as->op_lock)) return -EDEADLK; + /* Unmaps are forbidden when we failed to communicate with the GPU, + * meaning we can't guarantee that the GPU will see our page table + * updates which might lead to UAF situations. In that case, we + * just skip eviction on this VM. Things should go back to normal + * after a GPU reset. + */ + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP) { + ret = -EBUSY; + goto unlock_op; + } + /* It can be that the vm_bo was already evicted but a new * mapping pointing to this BO got created in the meantime, * thus turning the vm_bo in partially evicted state. In that case @@ -2555,6 +2591,7 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo) vma->evicted = true; } +unlock_op: mutex_unlock(&as->op_lock); if (ret) @@ -2768,7 +2805,7 @@ static int panthor_as_exec_op(struct panthor_as *as, .map.gem.offset = op->map.bo_offset, }; - if (as->unusable) { + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_MAP) { ret = -EINVAL; break; } @@ -2778,6 +2815,11 @@ static int panthor_as_exec_op(struct panthor_as *as, } case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP: + if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP) { + ret = -EINVAL; + break; + } + ret = drm_gpuvm_sm_unmap(&as->base, as, op->va.addr, op->va.range); break; @@ -2789,8 +2831,11 @@ static int panthor_as_exec_op(struct panthor_as *as, panthor_as_unlock_region(as); out: - if (ret && flag_vm_unusable_on_failure) - panthor_as_declare_unusable(as); + if (ret && flag_vm_unusable_on_failure) { + panthor_as_restrict_usage_locked(as, + PANTHOR_AS_FORBID_USE | + PANTHOR_AS_FORBID_MAP); + } as->op_ctx = NULL; mutex_unlock(&as->op_lock); @@ -3112,9 +3157,6 @@ panthor_vm_bind_job_create(struct drm_file *file, if (!vm) return ERR_PTR(-EINVAL); - if (vm->destroyed || vm->as->unusable) - return ERR_PTR(-EINVAL); - job = kzalloc_obj(*job); if (!job) return ERR_PTR(-ENOMEM); -- 2.55.0
