diff --git a/drivers/gpu/drm/asahi/mmu.rs b/drivers/gpu/drm/asahi/mmu.rs index 3522010949e35..c4afbc0127d70 100644 --- a/drivers/gpu/drm/asahi/mmu.rs +++ b/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) { - 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 { - 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) -> 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; diff --git a/drivers/gpu/drm/asahi/mmu/bind.rs b/drivers/gpu/drm/asahi/mmu/bind.rs index 0b3d85077aacb..0fa3f304af1d4 100644 --- a/drivers/gpu/drm/asahi/mmu/bind.rs +++ b/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 = diff --git a/rust/helpers/drm_gpuvm.c b/rust/helpers/drm_gpuvm.c index 30fb1777fbf2b..d42d3d3491006 100644 --- a/rust/helpers/drm_gpuvm.c +++ b/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) { diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs index 146f3c9ed8362..d873eb4ef6384 100644 --- a/rust/kernel/drm/gpuvm/mod.rs +++ b/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 GpuVm { &'a self, obj: Option<&T::Object>, interruptible: bool, + ) -> Result> { + // 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> { + // 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> { // 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 GpuVm { 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.