From 2151a3442ee9e60f096ee1c6b7770fcd5ba1e879 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Sun, 25 Oct 2026 16:33:14 +0100 Subject: [PATCH 1/3] drm/gpuvm: initialize BO unmap result Return success when a VM BO has no mappings instead of returning an uninitialized stack value. Deferred-close cleanup can reach this case; a garbage error return marks cleanup failed and blocks the VM. Signed-off-by: Eryk Wieliczko --- drivers/gpu/drm/drm_gpuvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c index 92ab5271e523e9..b381dce6e037b8 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -3302,7 +3302,7 @@ drm_gpuvm_bo_unmap(struct drm_gpuvm_bo *vm_bo, void *priv) { struct drm_gpuva_ops *ops; struct drm_gpuva_op *op; - int ret; + int ret = 0; if (unlikely(!vm_bo->vm)) return -EINVAL; From a8f459d96480e49bc2ae3ee0f9a78e18d9f6389c Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Sun, 25 Oct 2026 14:49:13 +0100 Subject: [PATCH 2/3] rust: drm: gpuvm: add filtered object unmap Allow immediate-mode drivers to remove selected mappings of one GEM without allocating an operation list. Signed-off-by: Eryk Wieliczko --- rust/kernel/drm/gpuvm/mod.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs index 770f03e5dfef31..faecbedfa282fb 100644 --- a/rust/kernel/drm/gpuvm/mod.rs +++ b/rust/kernel/drm/gpuvm/mod.rs @@ -687,24 +687,38 @@ impl GpuVmInnerGuard<'_, T> { /// node or operation list is allocated. Call [`GpuVm::deferred_cleanup`] after releasing this /// guard, including when a callback failed after earlier mappings were removed. pub fn unmap_all(&mut self, unmap: impl FnMut(&mut T, &GpuVa) -> Result) -> Result { - self.unmap_matching(None, unmap) + self.unmap_matching(None, |_| true, unmap) } /// Allocation-free whole-node removal for one GEM object in an immediate-mode VM. /// /// Uses the same translation-removal and deferred-put contract as [`Self::unmap_all`]. This - /// scans the VM list and is intended as a low-memory cleanup fallback. + /// scans the VM list and can be used when the per-object operation list is unavailable. pub fn unmap_object( &mut self, object: &T::Object, unmap: impl FnMut(&mut T, &GpuVa) -> Result, ) -> Result { - self.unmap_matching(Some(object), unmap) + self.unmap_object_where(object, |_| true, unmap) + } + + /// Remove only mappings of an object selected by `matches`, without allocating. + /// + /// This scans the whole VM and uses the same translation-removal and deferred-put contract as + /// [`Self::unmap_all`]. If a callback fails, mappings already visited remain removed. + pub fn unmap_object_where( + &mut self, + object: &T::Object, + matches: impl FnMut(&GpuVa) -> bool, + unmap: impl FnMut(&mut T, &GpuVa) -> Result, + ) -> Result { + self.unmap_matching(Some(object), matches, unmap) } fn unmap_matching( &mut self, object: Option<&T::Object>, + mut matches: impl FnMut(&GpuVa) -> bool, mut unmap: impl FnMut(&mut T, &GpuVa) -> Result, ) -> Result { let vm = self.gpuvm.as_raw(); @@ -728,7 +742,11 @@ impl GpuVmInnerGuard<'_, T> { if object.is_some_and(|object| (*va).gem.obj != object.as_raw()) { continue; } - unmap(&mut *self.gpuvm.data.get(), GpuVa::::from_raw(va))?; + let typed_va = GpuVa::::from_raw(va); + if !matches(typed_va) { + continue; + } + unmap(&mut *self.gpuvm.data.get(), typed_va)?; // Remove it as `OpUnmap::remove()` does. let op = bindings::drm_gpuva_op_unmap { va, keep: false }; bindings::drm_gpuva_unmap(&op); From 13c3371ce23dceabad0fffd1d16ee121adb93dff Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Sun, 25 Oct 2026 16:33:14 +0100 Subject: [PATCH 3/3] drm/asahi: keep new bindings after deferred GEM close A dma-buf reimport can return the same GEM while cleanup for a closed handle is queued. Stamp each binding and remove only mappings covered by the close-time sequence, preserving later maps instead of unmapping them. Keep the indexed BO path when no newer binding exists. A map step that takes its sequence after the close snapshot survives deferred cleanup. Signed-off-by: Eryk Wieliczko --- drivers/gpu/drm/asahi/mmu.rs | 107 +++++++++++++++++++++++++- drivers/gpu/drm/asahi/mmu/bind.rs | 12 ++- drivers/gpu/drm/asahi/mmu/context.rs | 10 ++- drivers/gpu/drm/asahi/mmu/lifetime.rs | 30 +++++--- drivers/gpu/drm/asahi/mmu/shared.rs | 9 ++- 5 files changed, 151 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/asahi/mmu.rs b/drivers/gpu/drm/asahi/mmu.rs index 7717f01d8ee5d0..2bf4a6823024de 100644 --- a/drivers/gpu/drm/asahi/mmu.rs +++ b/drivers/gpu/drm/asahi/mmu.rs @@ -312,6 +312,7 @@ struct VmInner { va_range: Range, page_table: UatPageTable, mapping_epoch: Option>, + bind_sequence: Arc, epoch: Option, mm: mm::Allocator<(), KernelMappingInner>, uat_inner: Arc, @@ -368,6 +369,11 @@ struct VmBinding { /// Data associated with a VM <=> BO pairing #[pin_data] struct VmBoData { + /// Highest sequence assigned to a GPUVA linked to this VM/BO pair. + /// + /// Map steps update this monotonically under the VM reservation before + /// linking the GPUVA. Remap pieces keep their original sequence. + newest_bind: AtomicU64, #[pin] segments: Mutex>>, } @@ -410,6 +416,9 @@ struct StepContext<'ctx> { /// Preallocated resources that can be used when executing the request. resources: &'ctx mut VmOpResources, + + /// Sequence assigned by the successful map for its related metadata. + bind_sequence: u64, } impl<'ctx> StepContext<'ctx> { @@ -422,6 +431,7 @@ impl<'ctx> StepContext<'ctx> { dev, prot, resources, + bind_sequence: 0, }) } @@ -452,6 +462,7 @@ impl<'ctx> StepContext<'ctx> { #[derive(Default, Clone)] struct VmGpuVa { prot: Option, + bind_sequence: u64, } impl gpuvm::DriverGpuVm for VmInner { @@ -470,6 +481,11 @@ impl gpuvm::DriverGpuVm for VmInner { op: OpMap<'op, Self>, ctx: &mut Self::SmContext<'ctx>, ) -> Result, Error> { + let bind_sequence = self.bind_sequence.fetch_update( + Ordering::AcqRel, + Ordering::Acquire, + |value| value.checked_add(1), + ).map_err(|_| EOVERFLOW)? + 1; let _mutation = self.mapping_mutation(); let (map_addr, map_length) = (op.addr(), op.length() as usize); let mut iova = op.addr(); @@ -547,12 +563,18 @@ impl gpuvm::DriverGpuVm for VmInner { return Err(error); } }; + op.vm_bo() + .data() + .newest_bind + .fetch_max(bind_sequence, Ordering::Relaxed); let op = op.insert( gpuva, VmGpuVa { prot: Some(ctx.prot), + bind_sequence, }, ); + ctx.bind_sequence = bind_sequence; self.tlbi_contexts_naming_root(map_addr, map_length); Ok(op) @@ -578,6 +600,7 @@ impl gpuvm::DriverGpuVm for VmInner { ctx: &mut Self::SmContext<'ctx>, ) -> Result, Error> { let orig_prot = op.va_to_unmap().data().prot.ok_or(EINVAL)?; + let orig_bind_sequence = op.va_to_unmap().data().bind_sequence; let orig_invalidated = op.va_to_unmap().flags().contains(GpuVaFlag::INVALIDATED); // Only unmap the hole between prev/next, if they exist @@ -642,9 +665,11 @@ impl gpuvm::DriverGpuVm for VmInner { [prev_gpuva, next_gpuva], VmGpuVa { prot: Some(orig_prot), + bind_sequence: orig_bind_sequence, }, VmGpuVa { prot: Some(orig_prot), + bind_sequence: orig_bind_sequence, }, ); @@ -882,6 +907,7 @@ pub(crate) struct Vm { dev: ARef, id: u64, mapping_epoch: Option>, + bind_sequence: Arc, lifetime: Option>, residency: Option>, context_bindings: Option>, @@ -1704,6 +1730,7 @@ impl Vm { } else { None }; + let bind_sequence = Arc::new(AtomicU64::new(0), GFP_KERNEL)?; let unique_gpuvm = gpuvm::GpuVm::new::( c"Asahi::GpuVm", &dev, @@ -1716,6 +1743,7 @@ impl Vm { va_range: va_range.clone(), page_table, mapping_epoch: mapping_epoch.clone(), + bind_sequence: bind_sequence.clone(), epoch: Some(0), mm, uat_inner, @@ -1730,6 +1758,7 @@ impl Vm { id, gpuvm, mapping_epoch, + bind_sequence, lifetime: None, residency: None, context_bindings: None, @@ -1894,6 +1923,10 @@ impl Vm { self.mapping_epoch.as_ref()?.snapshot() } + fn current_bind_sequence(&self) -> u64 { + self.bind_sequence.load(Ordering::Acquire) + } + /// Get the translation table base for this Vm fn ttb(&self) -> u64 { self.binding.lock().ttb @@ -1950,7 +1983,10 @@ impl Vm { } let vm_bo = self .gpuvm - .obtain(gem, pin_init!(VmBoData { segments <- new_mutex!(None) }))?; + .obtain(gem, pin_init!(VmBoData { + newest_bind: AtomicU64::new(0), + segments <- new_mutex!(None), + }))?; let size = object_range.range(); let mut vm_exec = unsafe { @@ -2031,7 +2067,10 @@ impl Vm { } let vm_bo = self .gpuvm - .obtain(&gem, pin_init!(VmBoData { segments <-new_mutex!(None) }))?; + .obtain(&gem, pin_init!(VmBoData { + newest_bind: AtomicU64::new(0), + segments <- new_mutex!(None), + }))?; // SAFETY: no other UniqRefGpuVm exists so `GpuVm::exec_lock()` is safe let mut vm_exec = unsafe { @@ -2112,7 +2151,10 @@ impl Vm { } let vm_bo = self .gpuvm - .obtain(gem, pin_init!(VmBoData { segments <- new_mutex!(None) }))?; + .obtain(gem, pin_init!(VmBoData { + newest_bind: AtomicU64::new(0), + segments <- new_mutex!(None), + }))?; let mut resources = VmOpResources { preallocated_gpuvas: [ @@ -2338,6 +2380,65 @@ impl Vm { Ok(()) } + fn drop_mappings_before(&self, gem: &gem::Object, sequence: u64) -> Result { + let _residency = self.enter_retirement_residency(); + // Removing whole mappings only does unmaps, so no preallocated VAs + // are needed for the indexed path. + let mut resources = VmOpResources { + preallocated_gpuvas: [None, None, None], + }; + let mut ctx = StepContext::new(None, Prot::default(), &mut resources)?; + + // The BO's newest sequence is checked under the same lock as binds. If + // it is not newer than the close, every mapping belongs to that close + // and the per-object list is safe and substantially cheaper to walk. + let mut vm_exec = match unsafe { self.gpuvm.exec_lock(None, false) } { + Err(error) if error == ENOMEM => { + return self.drop_mappings_filtered(gem, sequence); + } + result => result?, + }; + let Some(bo) = vm_exec.unique_gpuvm.find_bo(gem) else { + return Ok(()); + }; + if bo.data().newest_bind.load(Ordering::Relaxed) > sequence { + drop(vm_exec); + drop(bo); + return self.drop_mappings_filtered(gem, sequence); + } + + mod_dev_dbg!(self.dev, "MMU: deferred bo_unmap\n"); + let result = vm_exec.unique_gpuvm.bo_unmap(&mut ctx, &bo); + if result.is_ok() { + self.untrack_context_object_before(gem, sequence); + self.untrack_shared_object_before(gem, sequence); + } + drop(vm_exec); + drop(bo); + if result == Err(ENOMEM) { + return self.drop_mappings_filtered(gem, sequence); + } + result + } + + /// A newer binding shares the BO list, so select the closed generation by + /// walking the VM without allocating. This is also the ENOMEM fallback. + fn drop_mappings_filtered(&self, gem: &gem::Object, sequence: u64) -> Result { + let mut inner = self.gpuvm.lock_inner(); + let result = inner.unmap_object_where( + gem, + |va| va.data().bind_sequence <= sequence, + VmInner::unmap_gpuva, + ); + if result.is_ok() { + self.untrack_context_object_before(gem, sequence); + self.untrack_shared_object_before(gem, sequence); + } + drop(inner); + self.bo_deferred_cleanup(); + result + } + /// Preserve the indexed fast path normally; memory pressure must not lose /// accepted object cleanup just because an exec or operation list failed. fn drop_mappings_low_memory(&self, gem: &gem::Object) -> Result { diff --git a/drivers/gpu/drm/asahi/mmu/bind.rs b/drivers/gpu/drm/asahi/mmu/bind.rs index e4677a53a5b221..44a1575ecfd2b7 100644 --- a/drivers/gpu/drm/asahi/mmu/bind.rs +++ b/drivers/gpu/drm/asahi/mmu/bind.rs @@ -83,7 +83,10 @@ impl Vm { crate::gem::held_sg_table(gem, dev)?; let vm_bo = self .gpuvm - .obtain(gem, pin_init!(VmBoData { segments <- new_mutex!(None) }))?; + .obtain(gem, pin_init!(VmBoData { + newest_bind: AtomicU64::new(0), + segments <- new_mutex!(None), + }))?; { let mut segments = vm_bo.data().segments.lock(); if segments.is_none() { @@ -156,13 +159,18 @@ impl Vm { flags, }); let retired = result.is_ok().then(|| { - let retired = self.commit_context_binding(&map.gem, &map.context); + let retired = self.commit_context_binding( + &map.gem, + &map.context, + ctx.bind_sequence, + ); self.track_shared_binding( &map.gem, map.addr, map.size, map.offset, map.single_page, + ctx.bind_sequence, ); retired }); diff --git a/drivers/gpu/drm/asahi/mmu/context.rs b/drivers/gpu/drm/asahi/mmu/context.rs index d459a3a91afce4..0b11394aa408c6 100644 --- a/drivers/gpu/drm/asahi/mmu/context.rs +++ b/drivers/gpu/drm/asahi/mmu/context.rs @@ -14,6 +14,7 @@ use kernel::drm::gem::BaseObject; struct Binding { gem: ARef, offset: usize, + bind_sequence: u64, } struct Bindings { generation: u64, @@ -115,6 +116,7 @@ impl Vm { &self, gem: &ARef, update: &PreparedBinding, + bind_sequence: u64, ) -> RetiredBindings { let mut retired = RetiredBindings::new(); if update.touched == 0 { @@ -136,6 +138,7 @@ impl Vm { *slot = Some(Binding { gem: gem.clone(), offset, + bind_sequence, }); } } @@ -173,6 +176,10 @@ impl Vm { } pub(crate) fn untrack_context_object(&self, gem: &gem::Object) { + self.untrack_context_object_before(gem, u64::MAX); + } + + pub(super) fn untrack_context_object_before(&self, gem: &gem::Object, sequence: u64) { let Some(context) = self.context_bindings.as_ref() else { return; }; @@ -181,7 +188,8 @@ impl Vm { for slot in &mut bindings.slots { if slot .as_ref() - .is_some_and(|binding| core::ptr::eq(&*binding.gem, gem)) + .is_some_and(|binding| core::ptr::eq(&*binding.gem, gem) + && binding.bind_sequence <= sequence) { *slot = None; touched = true; diff --git a/drivers/gpu/drm/asahi/mmu/lifetime.rs b/drivers/gpu/drm/asahi/mmu/lifetime.rs index 4b881d6f1ebfd5..30010be497973d 100644 --- a/drivers/gpu/drm/asahi/mmu/lifetime.rs +++ b/drivers/gpu/drm/asahi/mmu/lifetime.rs @@ -47,6 +47,12 @@ impl DriverMappings { } } +struct ClosedObject { + gem: ARef, + /// Highest binding sequence covered by the close or merged closes. + bind_sequence: u64, +} + struct State { active: usize, commits: usize, @@ -57,7 +63,7 @@ struct State { cleanup_failed: bool, close_ranges: Option<(Range, Range)>, unmaps: KVec, - objects: KVec>, + objects: KVec, } #[pin_data] @@ -545,18 +551,22 @@ impl Vm { return self.drop_mappings_now(gem); }; let mut state = lifetime.state.lock(); - if !state - .objects - .iter() - .any(|object| core::ptr::eq(&**object, gem)) - { - state.objects.push(gem.into(), GFP_KERNEL)?; + let bind_sequence = self.current_bind_sequence(); + if let Some(object) = state.objects.iter_mut().find(|object| { + core::ptr::eq(&*object.gem, gem) + }) { + object.bind_sequence = object.bind_sequence.max(bind_sequence); + } else { + state.objects.push(ClosedObject { + gem: gem.into(), + bind_sequence, + }, GFP_KERNEL)?; } if state.active != 0 || state.draining { // Stop new jobs from selecting this accepted cleanup candidate. // The queued object retains its GEM while the candidate is removed. - // Actual unmap repeats this under exec to order any intervening bind. - self.untrack_context_object(gem); + // Preserve a context view installed by a later bind. + self.untrack_context_object_before(gem, bind_sequence); return Ok(()); } state.draining = true; @@ -641,7 +651,7 @@ impl Vm { } } for object in objects { - if let Err(error) = self.drop_mappings_now(&object) { + if let Err(error) = self.drop_mappings_before(&object.gem, object.bind_sequence) { failure.get_or_insert(error); pr_err!("MMU: deferred object unmap failed\n"); } diff --git a/drivers/gpu/drm/asahi/mmu/shared.rs b/drivers/gpu/drm/asahi/mmu/shared.rs index 5b87c55a06b72b..b9e2975c898ff4 100644 --- a/drivers/gpu/drm/asahi/mmu/shared.rs +++ b/drivers/gpu/drm/asahi/mmu/shared.rs @@ -14,6 +14,7 @@ struct Binding { end: u64, object: usize, offset: u64, + bind_sequence: u64, } #[pin_data] @@ -43,6 +44,7 @@ impl Vm { size: u64, offset: u64, single_page: bool, + bind_sequence: u64, ) { let Some(shared) = &self.shared_bindings else { return; @@ -63,6 +65,7 @@ impl Vm { end, object: gem as *const gem::Object as usize, offset, + bind_sequence, }, GFP_KERNEL, ); @@ -86,6 +89,10 @@ impl Vm { } pub(crate) fn untrack_shared_object(&self, gem: &gem::Object) { + self.untrack_shared_object_before(gem, u64::MAX); + } + + pub(super) fn untrack_shared_object_before(&self, gem: &gem::Object, sequence: u64) { let Some(shared) = &self.shared_bindings else { return; }; @@ -94,7 +101,7 @@ impl Vm { } let object = gem as *const gem::Object as usize; let mut bindings = shared.bindings.lock(); - bindings.retain(|entry| entry.object != object); + bindings.retain(|entry| entry.object != object || entry.bind_sequence > sequence); shared.count.store(bindings.len() as u32, Ordering::Relaxed); }