drm/asahi: lock only the VM reservation for VM queries and VA updates

drivers/gpu/drm/asahi/mmu.rs

@@ -1305,9 +1305,7 @@ impl KernelMapping {
             return Err(EINVAL);
         }
         let address = self.iova().checked_add(offset as u64).ok_or(EOVERFLOW)?;
-        lock_vm(&self.0.owner, None, false)?
-            .page_table
-            .translate_iova(address)
+        self.0.owner.lock_inner().page_table.translate_iova(address)
     }
 
     /// Writes a userspace timestamp through the retained GEM mapping. Aligned results
@@ -2250,9 +2248,7 @@ impl Vm {
     }
 
     pub(crate) fn covers_range(&self, address: u64, size: u64, read: bool, write: bool) -> bool {
-        let Ok(mut inner) = lock_vm(&self.gpuvm, None, false) else {
-            return false;
-        };
+        let mut inner = self.gpuvm.lock_inner();
         Self::covers_range_locked(&mut inner, address, size, read, write)
     }
 
@@ -2267,9 +2263,7 @@ impl Vm {
         if stride <= block_size {
             return false;
         }
-        let Ok(mut inner) = lock_vm(&self.gpuvm, None, false) else {
-            return false;
-        };
+        let mut inner = self.gpuvm.lock_inner();
         addresses.iter().all(|&address| {
             let Some(guard) = address.checked_add(block_size) else {
                 return false;
@@ -2279,12 +2273,12 @@ impl Vm {
         })
     }
 
-    /// Checks all exact request tuples under one GPUVM exec lock. Access bit0
+    /// Checks all exact request tuples under one hold of the VM reservation. Access bit0
     /// requests reading and bit1 writing. The epoch names this same snapshot.
     pub(crate) fn covers_ranges_batch(&self, ranges: &[(u64, u64, u8)]) -> (bool, Option<u64>) {
-        let Ok(mut inner) = lock_vm(&self.gpuvm, None, false) else {
-            return (false, None);
-        };
+        // Every mapping change holds the VM reservation, so readers need only that one. The full
+        // exec lock would also lock every external BO, making each submit O(external BOs).
+        let mut inner = self.gpuvm.lock_inner();
         let epoch = inner.epoch;
         let covered = ranges.iter().all(|&(address, size, access)| {
             Self::covers_range_locked(&mut inner, address, size, access & 1 != 0, access & 2 != 0)
@@ -2387,7 +2381,7 @@ impl Vm {
 
         let size = object_range.range();
         let mut vm_exec = unsafe {
-            self.gpuvm.exec_lock(/*Some(gem)*/ None, false)?
+            self.gpuvm.exec_lock_vm(/*Some(gem)*/ None, false)?
         };
         let inner = vm_exec.unique_gpuvm.data();
 
@@ -2467,7 +2461,7 @@ impl Vm {
         // SAFETY: no other UniqRefGpuVm exists so `GpuVm::exec_lock()` is safe
         let mut vm_exec = unsafe {
             self.gpuvm
-                .exec_lock(/*Some(&gem)*/ None, false)
+                .exec_lock_vm(/*Some(&gem)*/ None, false)
                 .inspect_err(|_| {
                     dev_err!(self.dev, "map_at(): Failed to exec_lock gpuvm\n");
                 })?
@@ -2555,7 +2549,7 @@ impl Vm {
         let mut ctx = StepContext::new(Some(device.as_ref().as_ref()), prot, &mut resources)?;
 
         let mut vm_exec = unsafe {
-            self.gpuvm.exec_lock(/*Some(&gem)*/ None, false)?
+            self.gpuvm.exec_lock_vm(/*Some(&gem)*/ None, false)?
         };
         let inner = vm_exec.unique_gpuvm.data();
 
@@ -2626,7 +2620,7 @@ impl Vm {
         size: usize,
         prot: Prot,
     ) -> Result<KernelMapping> {
-        let mut vm_exec = unsafe { self.gpuvm.exec_lock(None, false)? };
+        let mut vm_exec = unsafe { self.gpuvm.exec_lock_vm(None, false)? };
 
         if (iova as usize | phys | size) & UAT_PGMSK != 0 {
             dev_err!(
@@ -2730,7 +2724,7 @@ impl Vm {
         let mut resources = VmOpResources::for_unmap()?;
         let mut ctx = StepContext::new(dev, Prot::default(), &mut resources)?;
 
-        let mut vm_exec = unsafe { self.gpuvm.exec_lock(None, false)? };
+        let mut vm_exec = unsafe { self.gpuvm.exec_lock_vm(None, false)? };
 
         mod_dev_dbg!(self.dev, "MMU: sm_unmap: {:#x}:{:#x}\n", iova, size);
         let _commit = if user {
@@ -2761,7 +2755,7 @@ impl Vm {
         let mut ctx = StepContext::new(None, Prot::default(), &mut resources)?;
 
         // SAFETY: See `lock_vm()`.
-        let mut vm_exec = match unsafe { self.gpuvm.exec_lock(None, false) } {
+        let mut vm_exec = match unsafe { self.gpuvm.exec_lock_vm(None, false) } {
             Err(error) if error == ENOMEM => return self.drop_mappings_low_memory(gem),
             result => result?,
         };
@@ -3136,7 +3130,7 @@ impl Uat {
             return Err(EINVAL);
         }
         let root = {
-            let owner = lock_vm(&vm.gpuvm, None, false)?;
+            let owner = vm.gpuvm.lock_inner();
             if owner.is_kernel || !Arc::ptr_eq(&owner.uat_inner, &self.inner) {
                 return Err(EINVAL);
             }
@@ -3281,7 +3275,7 @@ impl Uat {
             return Err(EINVAL);
         }
         {
-            let owner = lock_vm(&vm.gpuvm, None, false)?;
+            let owner = vm.gpuvm.lock_inner();
             if owner.is_kernel
                 || !core::ptr::eq(Arc::as_ptr(&owner.uat_inner), Arc::as_ptr(&self.inner))
             {
@@ -3663,7 +3657,7 @@ impl Vm {
     /// Diagnostic reservation occupancy (including guards), not a placement promise.
     /// Removal can progress between queries; allocations are excluded by the VM lock.
     pub(crate) fn capacity(&self, range: Range<u64>) -> Result<(u64, u64)> {
-        let inner = lock_vm(&self.gpuvm, None, false)?;
+        let inner = self.gpuvm.lock_inner();
         let mut next = range.start;
         let mut used = 0;
         let mut largest = 0;

drivers/gpu/drm/asahi/mmu/bind.rs

@@ -132,7 +132,7 @@ impl Vm {
                 PreparedUserBindOp::Map(map) => {
                     // SAFETY: See `lock_vm()`. As in `Vm::bind_object()`, the object itself is
                     // not locked: the map step may still populate its pages.
-                    let mut inner = unsafe { self.gpuvm.exec_lock(None, true)? };
+                    let mut inner = unsafe { self.gpuvm.exec_lock_vm(None, true)? };
                     match self.mapping_commit() {
                         Ok(commit) => {
                             let (flags, repeat) = if map.single_page {
@@ -177,7 +177,7 @@ impl Vm {
                 }
                 PreparedUserBindOp::Unmap(unmap) => {
                     // SAFETY: See `lock_vm()`.
-                    let mut inner = unsafe { self.gpuvm.exec_lock(None, true)? };
+                    let mut inner = unsafe { self.gpuvm.exec_lock_vm(None, true)? };
                     match self.mapping_commit() {
                         Ok(commit) => {
                             let mut ctx =

rust/helpers/drm_gpuvm.c

@@ -23,6 +23,12 @@ bool rust_helper_drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
 	return drm_gpuvm_is_extobj(gpuvm, obj);
 }
 
+__rust_helper
+bool rust_helper_drm_exec_is_contended(struct drm_exec *exec)
+{
+	return drm_exec_is_contended(exec);
+}
+
 __rust_helper
 void rust_helper_drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec)
 {

rust/kernel/drm/gpuvm/mod.rs

@@ -60,6 +60,43 @@ pub use self::vm_bo::*;
 mod va;
 pub use self::va::*;
 
+/// `drm_gpuvm_exec_lock()` without `drm_gpuvm_prepare_objects()`: the
+/// `drm_exec_until_all_locked()` loop over the VM reservation and `vm_exec.extra`.
+///
+/// # Safety
+///
+/// `vm_exec` must be initialized as for `drm_gpuvm_exec_lock()`.
+unsafe fn exec_lock_vm_raw(vm_exec: &mut bindings::drm_gpuvm_exec) -> core::ffi::c_int {
+    let exec = &raw mut vm_exec.exec;
+    // SAFETY: Per the safety requirements, `exec` belongs to a valid `drm_gpuvm_exec`.
+    unsafe { bindings::drm_exec_init(exec, vm_exec.flags, 0) };
+    let mut ret = 0;
+    // SAFETY (whole loop): `exec` was initialized above and `vm_exec.vm` is a valid GPUVM.
+    // `drm_exec_cleanup()` drops all locks and restarts after contention, as the C macros do.
+    while unsafe { bindings::drm_exec_cleanup(exec) } {
+        ret = unsafe { bindings::drm_gpuvm_prepare_vm(vm_exec.vm, exec, vm_exec.num_fences) };
+        if unsafe { bindings::drm_exec_is_contended(exec) } {
+            continue;
+        }
+        if ret == 0 {
+            if let Some(extra) = vm_exec.extra.fn_ {
+                ret = unsafe { extra(vm_exec) };
+                if unsafe { bindings::drm_exec_is_contended(exec) } {
+                    continue;
+                }
+            }
+        }
+        if ret != 0 {
+            break;
+        }
+    }
+    if ret != 0 {
+        // SAFETY: Finalize the context initialized above, as `drm_gpuvm_exec_lock()` does on error.
+        unsafe { bindings::drm_exec_fini(exec) };
+    }
+    ret
+}
+
 pub(super) unsafe extern "C" fn exec_lock_gem_object(
     vm_exec: *mut bindings::drm_gpuvm_exec,
 ) -> core::ffi::c_int {
@@ -260,6 +297,36 @@ impl<T: DriverGpuVm> GpuVm<T> {
         &'a self,
         obj: Option<&T::Object>,
         interruptible: bool,
+    ) -> Result<GpuVmExec<T>> {
+        // SAFETY: Same contract as this function.
+        unsafe { self.exec_lock_impl(obj, interruptible, true) }
+    }
+
+    /// HACK: added for the asahi downstream driver. Like [`GpuVm::exec_lock`], but locks only
+    /// the VM's shared reservation (and `obj` if it is external), not every external object.
+    ///
+    /// Enough for VA updates of this immediate-mode, `DRM_GPUVM_RESV_PROTECTED` VM: they need
+    /// the VM reservation and each object's gpuva lock, which the link and unlink paths take.
+    /// Callers must not need external objects' reservations (fences, validation, eviction).
+    /// Locking all of them made each update cost O(external objects).
+    ///
+    /// # Safety
+    ///
+    /// Same as [`GpuVm::exec_lock`].
+    pub unsafe fn exec_lock_vm<'a>(
+        &'a self,
+        obj: Option<&T::Object>,
+        interruptible: bool,
+    ) -> Result<GpuVmExec<T>> {
+        // SAFETY: Same contract as this function.
+        unsafe { self.exec_lock_impl(obj, interruptible, false) }
+    }
+
+    unsafe fn exec_lock_impl<'a>(
+        &'a self,
+        obj: Option<&T::Object>,
+        interruptible: bool,
+        all_objects: bool,
     ) -> Result<GpuVmExec<T>> {
         // Do not try to lock the object if it is internal (since it is already locked).
         let is_ext = obj.map(|a| self.is_extobj(a)).unwrap_or(false);
@@ -286,10 +353,16 @@ impl<T: DriverGpuVm> GpuVm<T> {
             GFP_KERNEL,
         )?;
 
-        // SAFETY: The object is valid and was initialized above. On failure,
-        // `drm_gpuvm_exec_lock()` has already finalized the exec context, so the box only needs to
-        // be freed, which dropping it does.
-        to_result(unsafe { bindings::drm_gpuvm_exec_lock(&mut *vm_exec) })?;
+        // SAFETY: The object is valid and was initialized above. On failure, both lock functions
+        // have already finalized the exec context, so the box only needs to be freed, which
+        // dropping it does.
+        to_result(unsafe {
+            if all_objects {
+                bindings::drm_gpuvm_exec_lock(&mut *vm_exec)
+            } else {
+                exec_lock_vm_raw(&mut *vm_exec)
+            }
+        })?;
 
         // Build the guard, and take its VM reference, only once the locks are held: its `Drop`
         // unlocks them.