All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes
@ 2026-08-04 15:26 David (Ming Qiang) Wu
  2026-08-05 14:58 ` Christian König
  0 siblings, 1 reply; 3+ messages in thread
From: David (Ming Qiang) Wu @ 2026-08-04 15:26 UTC (permalink / raw)
  To: amd-gfx; +Cc: Christian.Koenig

On hardware where EFC (Encode Frame Copy) is unavailable or the source
frame format requires conversion, the VA-API frontend falls back to a
GFX/compute shader blit (vl_compositor) to perform color space conversion
before VCN encode.  That blit goes through the GFX/compute ring and bumps
the VM's shared tlb_seq.  Every subsequent VCN encode job then finds
flushed_updates < tlb_seq and forces a full MMHUB TLB flush, even though
no MMHUB-relevant mappings changed.  On multi-instance VCN hardware the
same issue is compounded by the dedicated PIPE_CONTEXT_COMPUTE_ONLY
scheduling context Mesa creates.

Fix this by splitting the single tlb_seq into two counters:

  tlb_seq    - bumped only when mappings accessed by GFXHUB change
  tlb_seq_mm - bumped only when mappings accessed by MMHUB change

To know which hubs are relevant for a given PT update, add a vmhub_mask
parameter to amdgpu_vm_update_range().  The mask is derived from the set
of rings in each command submission (collected in amdgpu_cs.c and stored
in bo_va->vmhub_mask / mapping->vmhub_mask).  When vmhub_mask is 0 (BO
has never been submitted), both counters are bumped conservatively.

PDE updates and XGMI/GFX8 legacy paths bump both counters since they
affect all hubs.  The fault handler uses GENMASK(AMDGPU_MAX_VMHUBS-1,0)
because the faulting hub is unknown at that point.

MMHUB VMID allocation (amdgpu_ids.c) now compares flushed_updates against
tlb_seq_mm instead of tlb_seq, so VCN VMIDs no longer force a TLB flush
when only GFX/compute page tables changed.

Signed-off-by: David (Ming Qiang) Wu <David.Wu3@amd.com>
Assisted-by: Claude:Claude-Sonnet-4-6
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 37 ++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c    |  9 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  5 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 65 +++++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     | 32 ++++++++++-
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c       |  5 +-
 6 files changed, 129 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index afc1c631d55a..33c1e9341774 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1136,19 +1136,34 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
 	 * with p->ticket. But removing it caused test regressions, so I'm
 	 * leaving it here for now.
 	 */
-	amdgpu_bo_list_for_each_entry(e, p->bo_list) {
-		bo_va = e->bo_va;
-		if (bo_va == NULL)
-			continue;
+	/* Collect vmhub bitmask for all rings in this submission */
+	{
+		uint16_t sub_vmhub_mask = 0;
 
-		r = amdgpu_vm_bo_update(adev, bo_va, false);
-		if (r)
-			return r;
+		for (i = 0; i < p->gang_size; ++i) {
+			struct amdgpu_ring *ring = amdgpu_job_ring(p->jobs[i]);
 
-		r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
-				      GFP_KERNEL);
-		if (r)
-			return r;
+			if (ring)
+				sub_vmhub_mask |= BIT(ring->vm_hub);
+		}
+
+		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
+			bo_va = e->bo_va;
+			if (bo_va == NULL)
+				continue;
+
+			/* Track which hubs have accessed this BO for per-hub tlb_seq */
+			bo_va->vmhub_mask |= sub_vmhub_mask;
+
+			r = amdgpu_vm_bo_update(adev, bo_va, false);
+			if (r)
+				return r;
+
+			r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
+					      GFP_KERNEL);
+			if (r)
+				return r;
+		}
 	}
 
 	r = amdgpu_vm_handle_moved(adev, vm, &p->exec.ticket);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 8a2d64f0ebc3..9183569d340a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -331,7 +331,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm,
 	unsigned vmhub = ring->vm_hub;
 	uint64_t fence_context = adev->fence_context + ring->idx;
 	bool needs_flush = vm->use_cpu_for_update;
-	uint64_t updates = amdgpu_vm_tlb_seq(vm);
+	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
 	int r;
 
 	*id = vm->reserved_vmid[vmhub];
@@ -395,7 +396,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm,
 	unsigned vmhub = ring->vm_hub;
 	struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
 	uint64_t fence_context = adev->fence_context + ring->idx;
-	uint64_t updates = amdgpu_vm_tlb_seq(vm);
+	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
 	int r;
 
 	job->vm_needs_flush = vm->use_cpu_for_update;
@@ -492,7 +494,8 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 
 	job->gds_switch_needed = amdgpu_vmid_gds_switch_needed(id, job);
 	if (job->vm_needs_flush) {
-		id->flushed_updates = amdgpu_vm_tlb_seq(vm);
+		id->flushed_updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+			amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
 		dma_fence_put(id->last_flush);
 		id->last_flush = NULL;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index ff11a0903499..1d6373f1d985 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -70,6 +70,8 @@ struct amdgpu_bo_va_mapping {
 	uint64_t			__subtree_last;
 	uint64_t			offset;
 	uint32_t			flags;
+	/* bitmask of VMHUB indices whose TLBs have cached this mapping's PTEs */
+	uint16_t			vmhub_mask;
 };
 
 /* User space allocated BO in a VM */
@@ -91,6 +93,9 @@ struct amdgpu_bo_va {
 
 	bool				is_xgmi;
 
+	/* bitmask of VMHUB indices that have ever submitted jobs referencing this BO */
+	uint16_t			vmhub_mask;
+
 	/*
 	 * protected by vm reservation lock
 	 * if non-zero, cannot unmap from GPU because user queues may still access it
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index aac8ace9d7a6..b05befa1d5c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -121,6 +121,11 @@ struct amdgpu_vm_tlb_seq_struct {
 	 */
 	struct amdgpu_vm *vm;
 
+	/**
+	 * @vmhub_mask: which hub counters to increment (bitmask of VMHUB indices)
+	 */
+	uint16_t vmhub_mask;
+
 	/**
 	 * @cb: callback
 	 */
@@ -1008,8 +1013,10 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
 	if (r)
 		goto error;
 
-	if (flush_tlb_needed)
+	if (flush_tlb_needed) {
 		atomic64_inc(&vm->tlb_seq);
+		atomic64_inc(&vm->tlb_seq_mm);
+	}
 
 	list_for_each_entry_safe(entry, tmp, &vm->kernel.needs_update,
 				 vm_status)
@@ -1031,9 +1038,25 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence,
 				 struct dma_fence_cb *cb)
 {
 	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
+	bool any_gfx, any_mm;
+	unsigned int i;
 
 	tlb_cb = container_of(cb, typeof(*tlb_cb), cb);
-	atomic64_inc(&tlb_cb->vm->tlb_seq);
+
+	any_gfx = any_mm = false;
+	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+		if (!(tlb_cb->vmhub_mask & BIT(i)))
+			continue;
+		if (AMDGPU_IS_GFXHUB(i))
+			any_gfx = true;
+		else
+			any_mm = true;
+	}
+	if (any_gfx)
+		atomic64_inc(&tlb_cb->vm->tlb_seq);
+	if (any_mm)
+		atomic64_inc(&tlb_cb->vm->tlb_seq_mm);
+
 	kfree(tlb_cb);
 }
 
@@ -1054,6 +1077,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
 	struct amdgpu_vm *vm = params->vm;
 
 	tlb_cb->vm = vm;
+	tlb_cb->vmhub_mask = params->vmhub_mask;
 	if (!fence || !*fence) {
 		amdgpu_vm_tlb_seq_cb(NULL, &tlb_cb->cb);
 		return;
@@ -1106,7 +1130,8 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
  */
 int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			   bool immediate, bool unlocked, bool flush_tlb,
-			   bool allow_override, struct amdgpu_sync *sync,
+			   bool allow_override, uint16_t vmhub_mask,
+			   struct amdgpu_sync *sync,
 			   uint64_t start, uint64_t last, uint64_t flags,
 			   uint64_t offset, uint64_t vram_base,
 			   struct ttm_resource *res, dma_addr_t *pages_addr,
@@ -1129,13 +1154,20 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	/* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
 	 * heavy-weight flush TLB unconditionally.
 	 */
-	flush_tlb |= adev->gmc.xgmi.num_physical_nodes &&
-		     amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0);
+	if (adev->gmc.xgmi.num_physical_nodes &&
+	    amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0)) {
+		flush_tlb = true;
+		/* XGMI: all hubs need flushing */
+		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
+	}
 
 	/*
 	 * On GFX8 and older any 8 PTE block with a valid bit set enters the TLB
 	 */
-	flush_tlb |= amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0);
+	if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0)) {
+		flush_tlb = true;
+		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
+	}
 
 	memset(&params, 0, sizeof(params));
 	params.adev = adev;
@@ -1144,6 +1176,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	params.pages_addr = pages_addr;
 	params.unlocked = unlocked;
 	params.needs_flush = flush_tlb;
+	params.vmhub_mask = vmhub_mask;
 	params.override_pte = allow_override && adev->gmc.override_pte;
 	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
 
@@ -1366,9 +1399,18 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 
 		trace_amdgpu_vm_bo_update(mapping);
 
+		/*
+		 * If vmhub_mask is 0, no ring has submitted a job with this BO
+		 * yet, so we don't know which hubs may have cached its PTEs.
+		 * Be conservative and bump all counters.
+		 */
 		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb,
-					   !uncached, &sync, mapping->start,
-					   mapping->last, update_flags,
+					   !uncached,
+					   bo_va->vmhub_mask ?
+					     bo_va->vmhub_mask :
+					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
+					   &sync,
+					   mapping->start, mapping->last, update_flags,
 					   mapping->offset, vram_base, mem,
 					   pages_addr, last_update);
 		if (r)
@@ -1572,6 +1614,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 		list_del(&mapping->list);
 
 		r = amdgpu_vm_update_range(adev, vm, false, false, true, false,
+					   mapping->vmhub_mask ?
+					     mapping->vmhub_mask :
+					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
 					   &sync, mapping->start, mapping->last,
 					   0, 0, 0, NULL, NULL, &f);
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
@@ -1989,6 +2034,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
 
 	list_del(&mapping->list);
 	amdgpu_vm_it_remove(mapping, &vm->va);
+	/* Copy hub mask before clearing bo_va pointer (freed list loses the link) */
+	mapping->vmhub_mask = bo_va->vmhub_mask;
 	mapping->bo_va = NULL;
 	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
 
@@ -2212,6 +2259,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
 	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
 		list_del(&mapping->list);
 		amdgpu_vm_it_remove(mapping, &vm->va);
+		mapping->vmhub_mask = bo_va->vmhub_mask;
 		mapping->bo_va = NULL;
 		trace_amdgpu_vm_bo_unmap(bo_va, mapping);
 		list_add(&mapping->list, &vm->freed);
@@ -3048,6 +3096,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
 	}
 
 	r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
+				   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
 				   NULL, addr, addr, flags, value, 0, NULL, NULL, NULL);
 	if (r)
 		goto error_unlock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 2f8234560764..deeb43a7ef86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -313,6 +313,11 @@ struct amdgpu_vm_update_params {
 	 */
 	bool needs_flush;
 
+	/**
+	 * @vmhub_mask: bitmask of VMHUB indices whose tlb_seq counters to bump
+	 */
+	uint16_t vmhub_mask;
+
 	/**
 	 * @override_pte: true for memory that is not uncached and gmc override function is
 	 * implemented to allow MTYPE to be overridden for NUMA local memory.
@@ -403,6 +408,8 @@ struct amdgpu_vm {
 
 	/* Last finished delayed update */
 	atomic64_t		tlb_seq;
+	/* Separate TLB flush sequence for MMHUB rings (VCN/JPEG/VPE) */
+	atomic64_t		tlb_seq_mm;
 	struct dma_fence	*last_tlb_flush;
 	atomic64_t		kfd_last_flushed_seq;
 	uint64_t		tlb_fence_context;
@@ -528,7 +535,8 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
 			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
 int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			   bool immediate, bool unlocked, bool flush_tlb,
-			   bool allow_override, struct amdgpu_sync *sync,
+			   bool allow_override, uint16_t vmhub_mask,
+			   struct amdgpu_sync *sync,
 			   uint64_t start, uint64_t last, uint64_t flags,
 			   uint64_t offset, uint64_t vram_base,
 			   struct ttm_resource *res, dma_addr_t *pages_addr,
@@ -649,6 +657,28 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
 	return atomic64_read(&vm->tlb_seq);
 }
 
+/**
+ * amdgpu_vm_tlb_seq_mm - return MMHUB-specific tlb flush sequence number
+ * @vm: the amdgpu_vm structure to query
+ *
+ * Returns the tlb flush sequence number for MMHUB rings (VCN/JPEG/VPE).
+ * Only bumped when mappings accessed by MMHUB engines change.
+ */
+static inline uint64_t amdgpu_vm_tlb_seq_mm(struct amdgpu_vm *vm)
+{
+	unsigned long flags;
+	spinlock_t *lock;
+
+	rcu_read_lock();
+	lock = dma_fence_spinlock(vm->last_tlb_flush);
+	rcu_read_unlock();
+
+	spin_lock_irqsave(lock, flags);
+	spin_unlock_irqrestore(lock, flags);
+
+	return atomic64_read(&vm->tlb_seq_mm);
+}
+
 /*
  * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
  * happens while holding this lock anywhere to prevent deadlocks when
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 30ad10bbd47e..fb888f200f70 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1372,7 +1372,9 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		return -EINVAL;
 	}
 
-	return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
+	return amdgpu_vm_update_range(adev, vm, false, true, true, false,
+				      GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
+				      NULL, gpu_start,
 				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
 				      fence);
 }
@@ -1490,6 +1492,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
 			 pte_flags);
 
 		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb, true,
+					   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
 					   NULL, gpu_start, gpu_end,
 					   pte_flags,
 					   (last_start - prange->start) << PAGE_SHIFT,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes
  2026-08-04 15:26 [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes David (Ming Qiang) Wu
@ 2026-08-05 14:58 ` Christian König
  2026-08-05 18:21   ` David Wu
  0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2026-08-05 14:58 UTC (permalink / raw)
  To: David (Ming Qiang) Wu, amd-gfx

On 8/4/26 17:26, David (Ming Qiang) Wu wrote:
> On hardware where EFC (Encode Frame Copy) is unavailable or the source
> frame format requires conversion, the VA-API frontend falls back to a
> GFX/compute shader blit (vl_compositor) to perform color space conversion
> before VCN encode.  That blit goes through the GFX/compute ring and bumps
> the VM's shared tlb_seq.  Every subsequent VCN encode job then finds
> flushed_updates < tlb_seq and forces a full MMHUB TLB flush, even though
> no MMHUB-relevant mappings changed.  On multi-instance VCN hardware the
> same issue is compounded by the dedicated PIPE_CONTEXT_COMPUTE_ONLY
> scheduling context Mesa creates.
> 
> Fix this by splitting the single tlb_seq into two counters:
> 
>   tlb_seq    - bumped only when mappings accessed by GFXHUB change
>   tlb_seq_mm - bumped only when mappings accessed by MMHUB change
> 
> To know which hubs are relevant for a given PT update, add a vmhub_mask
> parameter to amdgpu_vm_update_range().  The mask is derived from the set
> of rings in each command submission (collected in amdgpu_cs.c and stored
> in bo_va->vmhub_mask / mapping->vmhub_mask).  When vmhub_mask is 0 (BO
> has never been submitted), both counters are bumped conservatively.
> 
> PDE updates and XGMI/GFX8 legacy paths bump both counters since they
> affect all hubs.  The fault handler uses GENMASK(AMDGPU_MAX_VMHUBS-1,0)
> because the faulting hub is unknown at that point.
> 
> MMHUB VMID allocation (amdgpu_ids.c) now compares flushed_updates against
> tlb_seq_mm instead of tlb_seq, so VCN VMIDs no longer force a TLB flush
> when only GFX/compute page tables changed.

Absolutely clear NAK to the whole idea.

If we unmap something from a VM we *must* invalidate all TLBs who potentially can access them. Oherwise we get massive security problems.

The whole idea here is a NO-GO.

Regards,
Christian.

> 
> Signed-off-by: David (Ming Qiang) Wu <David.Wu3@amd.com>
> Assisted-by: Claude:Claude-Sonnet-4-6
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 37 ++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c    |  9 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  5 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 65 +++++++++++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     | 32 ++++++++++-
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c       |  5 +-
>  6 files changed, 129 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index afc1c631d55a..33c1e9341774 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1136,19 +1136,34 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>  	 * with p->ticket. But removing it caused test regressions, so I'm
>  	 * leaving it here for now.
>  	 */
> -	amdgpu_bo_list_for_each_entry(e, p->bo_list) {
> -		bo_va = e->bo_va;
> -		if (bo_va == NULL)
> -			continue;
> +	/* Collect vmhub bitmask for all rings in this submission */
> +	{
> +		uint16_t sub_vmhub_mask = 0;
>  
> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
> -		if (r)
> -			return r;
> +		for (i = 0; i < p->gang_size; ++i) {
> +			struct amdgpu_ring *ring = amdgpu_job_ring(p->jobs[i]);
>  
> -		r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
> -				      GFP_KERNEL);
> -		if (r)
> -			return r;
> +			if (ring)
> +				sub_vmhub_mask |= BIT(ring->vm_hub);
> +		}
> +
> +		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
> +			bo_va = e->bo_va;
> +			if (bo_va == NULL)
> +				continue;
> +
> +			/* Track which hubs have accessed this BO for per-hub tlb_seq */
> +			bo_va->vmhub_mask |= sub_vmhub_mask;
> +
> +			r = amdgpu_vm_bo_update(adev, bo_va, false);
> +			if (r)
> +				return r;
> +
> +			r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
> +					      GFP_KERNEL);
> +			if (r)
> +				return r;
> +		}
>  	}
>  
>  	r = amdgpu_vm_handle_moved(adev, vm, &p->exec.ticket);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 8a2d64f0ebc3..9183569d340a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -331,7 +331,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm,
>  	unsigned vmhub = ring->vm_hub;
>  	uint64_t fence_context = adev->fence_context + ring->idx;
>  	bool needs_flush = vm->use_cpu_for_update;
> -	uint64_t updates = amdgpu_vm_tlb_seq(vm);
> +	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
> +		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>  	int r;
>  
>  	*id = vm->reserved_vmid[vmhub];
> @@ -395,7 +396,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm,
>  	unsigned vmhub = ring->vm_hub;
>  	struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>  	uint64_t fence_context = adev->fence_context + ring->idx;
> -	uint64_t updates = amdgpu_vm_tlb_seq(vm);
> +	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
> +		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>  	int r;
>  
>  	job->vm_needs_flush = vm->use_cpu_for_update;
> @@ -492,7 +494,8 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>  
>  	job->gds_switch_needed = amdgpu_vmid_gds_switch_needed(id, job);
>  	if (job->vm_needs_flush) {
> -		id->flushed_updates = amdgpu_vm_tlb_seq(vm);
> +		id->flushed_updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
> +			amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>  		dma_fence_put(id->last_flush);
>  		id->last_flush = NULL;
>  	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index ff11a0903499..1d6373f1d985 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -70,6 +70,8 @@ struct amdgpu_bo_va_mapping {
>  	uint64_t			__subtree_last;
>  	uint64_t			offset;
>  	uint32_t			flags;
> +	/* bitmask of VMHUB indices whose TLBs have cached this mapping's PTEs */
> +	uint16_t			vmhub_mask;
>  };
>  
>  /* User space allocated BO in a VM */
> @@ -91,6 +93,9 @@ struct amdgpu_bo_va {
>  
>  	bool				is_xgmi;
>  
> +	/* bitmask of VMHUB indices that have ever submitted jobs referencing this BO */
> +	uint16_t			vmhub_mask;
> +
>  	/*
>  	 * protected by vm reservation lock
>  	 * if non-zero, cannot unmap from GPU because user queues may still access it
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index aac8ace9d7a6..b05befa1d5c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -121,6 +121,11 @@ struct amdgpu_vm_tlb_seq_struct {
>  	 */
>  	struct amdgpu_vm *vm;
>  
> +	/**
> +	 * @vmhub_mask: which hub counters to increment (bitmask of VMHUB indices)
> +	 */
> +	uint16_t vmhub_mask;
> +
>  	/**
>  	 * @cb: callback
>  	 */
> @@ -1008,8 +1013,10 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>  	if (r)
>  		goto error;
>  
> -	if (flush_tlb_needed)
> +	if (flush_tlb_needed) {
>  		atomic64_inc(&vm->tlb_seq);
> +		atomic64_inc(&vm->tlb_seq_mm);
> +	}
>  
>  	list_for_each_entry_safe(entry, tmp, &vm->kernel.needs_update,
>  				 vm_status)
> @@ -1031,9 +1038,25 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence,
>  				 struct dma_fence_cb *cb)
>  {
>  	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
> +	bool any_gfx, any_mm;
> +	unsigned int i;
>  
>  	tlb_cb = container_of(cb, typeof(*tlb_cb), cb);
> -	atomic64_inc(&tlb_cb->vm->tlb_seq);
> +
> +	any_gfx = any_mm = false;
> +	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +		if (!(tlb_cb->vmhub_mask & BIT(i)))
> +			continue;
> +		if (AMDGPU_IS_GFXHUB(i))
> +			any_gfx = true;
> +		else
> +			any_mm = true;
> +	}
> +	if (any_gfx)
> +		atomic64_inc(&tlb_cb->vm->tlb_seq);
> +	if (any_mm)
> +		atomic64_inc(&tlb_cb->vm->tlb_seq_mm);
> +
>  	kfree(tlb_cb);
>  }
>  
> @@ -1054,6 +1077,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>  	struct amdgpu_vm *vm = params->vm;
>  
>  	tlb_cb->vm = vm;
> +	tlb_cb->vmhub_mask = params->vmhub_mask;
>  	if (!fence || !*fence) {
>  		amdgpu_vm_tlb_seq_cb(NULL, &tlb_cb->cb);
>  		return;
> @@ -1106,7 +1130,8 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>   */
>  int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  			   bool immediate, bool unlocked, bool flush_tlb,
> -			   bool allow_override, struct amdgpu_sync *sync,
> +			   bool allow_override, uint16_t vmhub_mask,
> +			   struct amdgpu_sync *sync,
>  			   uint64_t start, uint64_t last, uint64_t flags,
>  			   uint64_t offset, uint64_t vram_base,
>  			   struct ttm_resource *res, dma_addr_t *pages_addr,
> @@ -1129,13 +1154,20 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  	/* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
>  	 * heavy-weight flush TLB unconditionally.
>  	 */
> -	flush_tlb |= adev->gmc.xgmi.num_physical_nodes &&
> -		     amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0);
> +	if (adev->gmc.xgmi.num_physical_nodes &&
> +	    amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0)) {
> +		flush_tlb = true;
> +		/* XGMI: all hubs need flushing */
> +		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
> +	}
>  
>  	/*
>  	 * On GFX8 and older any 8 PTE block with a valid bit set enters the TLB
>  	 */
> -	flush_tlb |= amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0);
> +	if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0)) {
> +		flush_tlb = true;
> +		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
> +	}
>  
>  	memset(&params, 0, sizeof(params));
>  	params.adev = adev;
> @@ -1144,6 +1176,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  	params.pages_addr = pages_addr;
>  	params.unlocked = unlocked;
>  	params.needs_flush = flush_tlb;
> +	params.vmhub_mask = vmhub_mask;
>  	params.override_pte = allow_override && adev->gmc.override_pte;
>  	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
>  
> @@ -1366,9 +1399,18 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>  
>  		trace_amdgpu_vm_bo_update(mapping);
>  
> +		/*
> +		 * If vmhub_mask is 0, no ring has submitted a job with this BO
> +		 * yet, so we don't know which hubs may have cached its PTEs.
> +		 * Be conservative and bump all counters.
> +		 */
>  		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb,
> -					   !uncached, &sync, mapping->start,
> -					   mapping->last, update_flags,
> +					   !uncached,
> +					   bo_va->vmhub_mask ?
> +					     bo_va->vmhub_mask :
> +					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
> +					   &sync,
> +					   mapping->start, mapping->last, update_flags,
>  					   mapping->offset, vram_base, mem,
>  					   pages_addr, last_update);
>  		if (r)
> @@ -1572,6 +1614,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>  		list_del(&mapping->list);
>  
>  		r = amdgpu_vm_update_range(adev, vm, false, false, true, false,
> +					   mapping->vmhub_mask ?
> +					     mapping->vmhub_mask :
> +					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>  					   &sync, mapping->start, mapping->last,
>  					   0, 0, 0, NULL, NULL, &f);
>  		amdgpu_vm_free_mapping(adev, vm, mapping, f);
> @@ -1989,6 +2034,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>  
>  	list_del(&mapping->list);
>  	amdgpu_vm_it_remove(mapping, &vm->va);
> +	/* Copy hub mask before clearing bo_va pointer (freed list loses the link) */
> +	mapping->vmhub_mask = bo_va->vmhub_mask;
>  	mapping->bo_va = NULL;
>  	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>  
> @@ -2212,6 +2259,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
>  	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>  		list_del(&mapping->list);
>  		amdgpu_vm_it_remove(mapping, &vm->va);
> +		mapping->vmhub_mask = bo_va->vmhub_mask;
>  		mapping->bo_va = NULL;
>  		trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>  		list_add(&mapping->list, &vm->freed);
> @@ -3048,6 +3096,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
>  	}
>  
>  	r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
> +				   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>  				   NULL, addr, addr, flags, value, 0, NULL, NULL, NULL);
>  	if (r)
>  		goto error_unlock;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2f8234560764..deeb43a7ef86 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -313,6 +313,11 @@ struct amdgpu_vm_update_params {
>  	 */
>  	bool needs_flush;
>  
> +	/**
> +	 * @vmhub_mask: bitmask of VMHUB indices whose tlb_seq counters to bump
> +	 */
> +	uint16_t vmhub_mask;
> +
>  	/**
>  	 * @override_pte: true for memory that is not uncached and gmc override function is
>  	 * implemented to allow MTYPE to be overridden for NUMA local memory.
> @@ -403,6 +408,8 @@ struct amdgpu_vm {
>  
>  	/* Last finished delayed update */
>  	atomic64_t		tlb_seq;
> +	/* Separate TLB flush sequence for MMHUB rings (VCN/JPEG/VPE) */
> +	atomic64_t		tlb_seq_mm;
>  	struct dma_fence	*last_tlb_flush;
>  	atomic64_t		kfd_last_flushed_seq;
>  	uint64_t		tlb_fence_context;
> @@ -528,7 +535,8 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>  			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
>  int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  			   bool immediate, bool unlocked, bool flush_tlb,
> -			   bool allow_override, struct amdgpu_sync *sync,
> +			   bool allow_override, uint16_t vmhub_mask,
> +			   struct amdgpu_sync *sync,
>  			   uint64_t start, uint64_t last, uint64_t flags,
>  			   uint64_t offset, uint64_t vram_base,
>  			   struct ttm_resource *res, dma_addr_t *pages_addr,
> @@ -649,6 +657,28 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
>  	return atomic64_read(&vm->tlb_seq);
>  }
>  
> +/**
> + * amdgpu_vm_tlb_seq_mm - return MMHUB-specific tlb flush sequence number
> + * @vm: the amdgpu_vm structure to query
> + *
> + * Returns the tlb flush sequence number for MMHUB rings (VCN/JPEG/VPE).
> + * Only bumped when mappings accessed by MMHUB engines change.
> + */
> +static inline uint64_t amdgpu_vm_tlb_seq_mm(struct amdgpu_vm *vm)
> +{
> +	unsigned long flags;
> +	spinlock_t *lock;
> +
> +	rcu_read_lock();
> +	lock = dma_fence_spinlock(vm->last_tlb_flush);
> +	rcu_read_unlock();
> +
> +	spin_lock_irqsave(lock, flags);
> +	spin_unlock_irqrestore(lock, flags);
> +
> +	return atomic64_read(&vm->tlb_seq_mm);
> +}
> +
>  /*
>   * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
>   * happens while holding this lock anywhere to prevent deadlocks when
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 30ad10bbd47e..fb888f200f70 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1372,7 +1372,9 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  		return -EINVAL;
>  	}
>  
> -	return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
> +	return amdgpu_vm_update_range(adev, vm, false, true, true, false,
> +				      GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
> +				      NULL, gpu_start,
>  				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
>  				      fence);
>  }
> @@ -1490,6 +1492,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>  			 pte_flags);
>  
>  		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb, true,
> +					   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>  					   NULL, gpu_start, gpu_end,
>  					   pte_flags,
>  					   (last_start - prange->start) << PAGE_SHIFT,


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes
  2026-08-05 14:58 ` Christian König
@ 2026-08-05 18:21   ` David Wu
  0 siblings, 0 replies; 3+ messages in thread
From: David Wu @ 2026-08-05 18:21 UTC (permalink / raw)
  To: Christian König, David (Ming Qiang) Wu, amd-gfx

On 2026-08-05 10:58, Christian König wrote:

> On 8/4/26 17:26, David (Ming Qiang) Wu wrote:
>> On hardware where EFC (Encode Frame Copy) is unavailable or the source
>> frame format requires conversion, the VA-API frontend falls back to a
>> GFX/compute shader blit (vl_compositor) to perform color space conversion
>> before VCN encode.  That blit goes through the GFX/compute ring and bumps
>> the VM's shared tlb_seq.  Every subsequent VCN encode job then finds
>> flushed_updates < tlb_seq and forces a full MMHUB TLB flush, even though
>> no MMHUB-relevant mappings changed.  On multi-instance VCN hardware the
>> same issue is compounded by the dedicated PIPE_CONTEXT_COMPUTE_ONLY
>> scheduling context Mesa creates.
>>
>> Fix this by splitting the single tlb_seq into two counters:
>>
>>    tlb_seq    - bumped only when mappings accessed by GFXHUB change
>>    tlb_seq_mm - bumped only when mappings accessed by MMHUB change
>>
>> To know which hubs are relevant for a given PT update, add a vmhub_mask
>> parameter to amdgpu_vm_update_range().  The mask is derived from the set
>> of rings in each command submission (collected in amdgpu_cs.c and stored
>> in bo_va->vmhub_mask / mapping->vmhub_mask).  When vmhub_mask is 0 (BO
>> has never been submitted), both counters are bumped conservatively.
>>
>> PDE updates and XGMI/GFX8 legacy paths bump both counters since they
>> affect all hubs.  The fault handler uses GENMASK(AMDGPU_MAX_VMHUBS-1,0)
>> because the faulting hub is unknown at that point.
>>
>> MMHUB VMID allocation (amdgpu_ids.c) now compares flushed_updates against
>> tlb_seq_mm instead of tlb_seq, so VCN VMIDs no longer force a TLB flush
>> when only GFX/compute page tables changed.
> Absolutely clear NAK to the whole idea.
>
> If we unmap something from a VM we *must* invalidate all TLBs who potentially can access them. Oherwise we get massive security problems.
>
> The whole idea here is a NO-GO.
the concern is real - The optimization in theory should only do PTE 
updates and not frees.
let me work on another patch to address your concern and explain the 
details.

thanks,
David
>
> Regards,
> Christian.
>
>> Signed-off-by: David (Ming Qiang) Wu <David.Wu3@amd.com>
>> Assisted-by: Claude:Claude-Sonnet-4-6
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 37 ++++++++----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c    |  9 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  5 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 65 +++++++++++++++++++---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     | 32 ++++++++++-
>>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c       |  5 +-
>>   6 files changed, 129 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index afc1c631d55a..33c1e9341774 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -1136,19 +1136,34 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>>   	 * with p->ticket. But removing it caused test regressions, so I'm
>>   	 * leaving it here for now.
>>   	 */
>> -	amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>> -		bo_va = e->bo_va;
>> -		if (bo_va == NULL)
>> -			continue;
>> +	/* Collect vmhub bitmask for all rings in this submission */
>> +	{
>> +		uint16_t sub_vmhub_mask = 0;
>>   
>> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
>> -		if (r)
>> -			return r;
>> +		for (i = 0; i < p->gang_size; ++i) {
>> +			struct amdgpu_ring *ring = amdgpu_job_ring(p->jobs[i]);
>>   
>> -		r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
>> -				      GFP_KERNEL);
>> -		if (r)
>> -			return r;
>> +			if (ring)
>> +				sub_vmhub_mask |= BIT(ring->vm_hub);
>> +		}
>> +
>> +		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>> +			bo_va = e->bo_va;
>> +			if (bo_va == NULL)
>> +				continue;
>> +
>> +			/* Track which hubs have accessed this BO for per-hub tlb_seq */
>> +			bo_va->vmhub_mask |= sub_vmhub_mask;
>> +
>> +			r = amdgpu_vm_bo_update(adev, bo_va, false);
>> +			if (r)
>> +				return r;
>> +
>> +			r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
>> +					      GFP_KERNEL);
>> +			if (r)
>> +				return r;
>> +		}
>>   	}
>>   
>>   	r = amdgpu_vm_handle_moved(adev, vm, &p->exec.ticket);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> index 8a2d64f0ebc3..9183569d340a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> @@ -331,7 +331,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm,
>>   	unsigned vmhub = ring->vm_hub;
>>   	uint64_t fence_context = adev->fence_context + ring->idx;
>>   	bool needs_flush = vm->use_cpu_for_update;
>> -	uint64_t updates = amdgpu_vm_tlb_seq(vm);
>> +	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
>> +		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>>   	int r;
>>   
>>   	*id = vm->reserved_vmid[vmhub];
>> @@ -395,7 +396,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm,
>>   	unsigned vmhub = ring->vm_hub;
>>   	struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>>   	uint64_t fence_context = adev->fence_context + ring->idx;
>> -	uint64_t updates = amdgpu_vm_tlb_seq(vm);
>> +	uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
>> +		amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>>   	int r;
>>   
>>   	job->vm_needs_flush = vm->use_cpu_for_update;
>> @@ -492,7 +494,8 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>>   
>>   	job->gds_switch_needed = amdgpu_vmid_gds_switch_needed(id, job);
>>   	if (job->vm_needs_flush) {
>> -		id->flushed_updates = amdgpu_vm_tlb_seq(vm);
>> +		id->flushed_updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
>> +			amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
>>   		dma_fence_put(id->last_flush);
>>   		id->last_flush = NULL;
>>   	}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index ff11a0903499..1d6373f1d985 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -70,6 +70,8 @@ struct amdgpu_bo_va_mapping {
>>   	uint64_t			__subtree_last;
>>   	uint64_t			offset;
>>   	uint32_t			flags;
>> +	/* bitmask of VMHUB indices whose TLBs have cached this mapping's PTEs */
>> +	uint16_t			vmhub_mask;
>>   };
>>   
>>   /* User space allocated BO in a VM */
>> @@ -91,6 +93,9 @@ struct amdgpu_bo_va {
>>   
>>   	bool				is_xgmi;
>>   
>> +	/* bitmask of VMHUB indices that have ever submitted jobs referencing this BO */
>> +	uint16_t			vmhub_mask;
>> +
>>   	/*
>>   	 * protected by vm reservation lock
>>   	 * if non-zero, cannot unmap from GPU because user queues may still access it
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index aac8ace9d7a6..b05befa1d5c0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -121,6 +121,11 @@ struct amdgpu_vm_tlb_seq_struct {
>>   	 */
>>   	struct amdgpu_vm *vm;
>>   
>> +	/**
>> +	 * @vmhub_mask: which hub counters to increment (bitmask of VMHUB indices)
>> +	 */
>> +	uint16_t vmhub_mask;
>> +
>>   	/**
>>   	 * @cb: callback
>>   	 */
>> @@ -1008,8 +1013,10 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>>   	if (r)
>>   		goto error;
>>   
>> -	if (flush_tlb_needed)
>> +	if (flush_tlb_needed) {
>>   		atomic64_inc(&vm->tlb_seq);
>> +		atomic64_inc(&vm->tlb_seq_mm);
>> +	}
>>   
>>   	list_for_each_entry_safe(entry, tmp, &vm->kernel.needs_update,
>>   				 vm_status)
>> @@ -1031,9 +1038,25 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence,
>>   				 struct dma_fence_cb *cb)
>>   {
>>   	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
>> +	bool any_gfx, any_mm;
>> +	unsigned int i;
>>   
>>   	tlb_cb = container_of(cb, typeof(*tlb_cb), cb);
>> -	atomic64_inc(&tlb_cb->vm->tlb_seq);
>> +
>> +	any_gfx = any_mm = false;
>> +	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
>> +		if (!(tlb_cb->vmhub_mask & BIT(i)))
>> +			continue;
>> +		if (AMDGPU_IS_GFXHUB(i))
>> +			any_gfx = true;
>> +		else
>> +			any_mm = true;
>> +	}
>> +	if (any_gfx)
>> +		atomic64_inc(&tlb_cb->vm->tlb_seq);
>> +	if (any_mm)
>> +		atomic64_inc(&tlb_cb->vm->tlb_seq_mm);
>> +
>>   	kfree(tlb_cb);
>>   }
>>   
>> @@ -1054,6 +1077,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>>   	struct amdgpu_vm *vm = params->vm;
>>   
>>   	tlb_cb->vm = vm;
>> +	tlb_cb->vmhub_mask = params->vmhub_mask;
>>   	if (!fence || !*fence) {
>>   		amdgpu_vm_tlb_seq_cb(NULL, &tlb_cb->cb);
>>   		return;
>> @@ -1106,7 +1130,8 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>>    */
>>   int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>   			   bool immediate, bool unlocked, bool flush_tlb,
>> -			   bool allow_override, struct amdgpu_sync *sync,
>> +			   bool allow_override, uint16_t vmhub_mask,
>> +			   struct amdgpu_sync *sync,
>>   			   uint64_t start, uint64_t last, uint64_t flags,
>>   			   uint64_t offset, uint64_t vram_base,
>>   			   struct ttm_resource *res, dma_addr_t *pages_addr,
>> @@ -1129,13 +1154,20 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>   	/* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
>>   	 * heavy-weight flush TLB unconditionally.
>>   	 */
>> -	flush_tlb |= adev->gmc.xgmi.num_physical_nodes &&
>> -		     amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0);
>> +	if (adev->gmc.xgmi.num_physical_nodes &&
>> +	    amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0)) {
>> +		flush_tlb = true;
>> +		/* XGMI: all hubs need flushing */
>> +		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
>> +	}
>>   
>>   	/*
>>   	 * On GFX8 and older any 8 PTE block with a valid bit set enters the TLB
>>   	 */
>> -	flush_tlb |= amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0);
>> +	if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0)) {
>> +		flush_tlb = true;
>> +		vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
>> +	}
>>   
>>   	memset(&params, 0, sizeof(params));
>>   	params.adev = adev;
>> @@ -1144,6 +1176,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>   	params.pages_addr = pages_addr;
>>   	params.unlocked = unlocked;
>>   	params.needs_flush = flush_tlb;
>> +	params.vmhub_mask = vmhub_mask;
>>   	params.override_pte = allow_override && adev->gmc.override_pte;
>>   	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
>>   
>> @@ -1366,9 +1399,18 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>>   
>>   		trace_amdgpu_vm_bo_update(mapping);
>>   
>> +		/*
>> +		 * If vmhub_mask is 0, no ring has submitted a job with this BO
>> +		 * yet, so we don't know which hubs may have cached its PTEs.
>> +		 * Be conservative and bump all counters.
>> +		 */
>>   		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb,
>> -					   !uncached, &sync, mapping->start,
>> -					   mapping->last, update_flags,
>> +					   !uncached,
>> +					   bo_va->vmhub_mask ?
>> +					     bo_va->vmhub_mask :
>> +					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>> +					   &sync,
>> +					   mapping->start, mapping->last, update_flags,
>>   					   mapping->offset, vram_base, mem,
>>   					   pages_addr, last_update);
>>   		if (r)
>> @@ -1572,6 +1614,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>>   		list_del(&mapping->list);
>>   
>>   		r = amdgpu_vm_update_range(adev, vm, false, false, true, false,
>> +					   mapping->vmhub_mask ?
>> +					     mapping->vmhub_mask :
>> +					     GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>>   					   &sync, mapping->start, mapping->last,
>>   					   0, 0, 0, NULL, NULL, &f);
>>   		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>> @@ -1989,6 +2034,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>>   
>>   	list_del(&mapping->list);
>>   	amdgpu_vm_it_remove(mapping, &vm->va);
>> +	/* Copy hub mask before clearing bo_va pointer (freed list loses the link) */
>> +	mapping->vmhub_mask = bo_va->vmhub_mask;
>>   	mapping->bo_va = NULL;
>>   	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>>   
>> @@ -2212,6 +2259,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
>>   	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>>   		list_del(&mapping->list);
>>   		amdgpu_vm_it_remove(mapping, &vm->va);
>> +		mapping->vmhub_mask = bo_va->vmhub_mask;
>>   		mapping->bo_va = NULL;
>>   		trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>>   		list_add(&mapping->list, &vm->freed);
>> @@ -3048,6 +3096,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
>>   	}
>>   
>>   	r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
>> +				   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>>   				   NULL, addr, addr, flags, value, 0, NULL, NULL, NULL);
>>   	if (r)
>>   		goto error_unlock;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 2f8234560764..deeb43a7ef86 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -313,6 +313,11 @@ struct amdgpu_vm_update_params {
>>   	 */
>>   	bool needs_flush;
>>   
>> +	/**
>> +	 * @vmhub_mask: bitmask of VMHUB indices whose tlb_seq counters to bump
>> +	 */
>> +	uint16_t vmhub_mask;
>> +
>>   	/**
>>   	 * @override_pte: true for memory that is not uncached and gmc override function is
>>   	 * implemented to allow MTYPE to be overridden for NUMA local memory.
>> @@ -403,6 +408,8 @@ struct amdgpu_vm {
>>   
>>   	/* Last finished delayed update */
>>   	atomic64_t		tlb_seq;
>> +	/* Separate TLB flush sequence for MMHUB rings (VCN/JPEG/VPE) */
>> +	atomic64_t		tlb_seq_mm;
>>   	struct dma_fence	*last_tlb_flush;
>>   	atomic64_t		kfd_last_flushed_seq;
>>   	uint64_t		tlb_fence_context;
>> @@ -528,7 +535,8 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>>   			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
>>   int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>   			   bool immediate, bool unlocked, bool flush_tlb,
>> -			   bool allow_override, struct amdgpu_sync *sync,
>> +			   bool allow_override, uint16_t vmhub_mask,
>> +			   struct amdgpu_sync *sync,
>>   			   uint64_t start, uint64_t last, uint64_t flags,
>>   			   uint64_t offset, uint64_t vram_base,
>>   			   struct ttm_resource *res, dma_addr_t *pages_addr,
>> @@ -649,6 +657,28 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
>>   	return atomic64_read(&vm->tlb_seq);
>>   }
>>   
>> +/**
>> + * amdgpu_vm_tlb_seq_mm - return MMHUB-specific tlb flush sequence number
>> + * @vm: the amdgpu_vm structure to query
>> + *
>> + * Returns the tlb flush sequence number for MMHUB rings (VCN/JPEG/VPE).
>> + * Only bumped when mappings accessed by MMHUB engines change.
>> + */
>> +static inline uint64_t amdgpu_vm_tlb_seq_mm(struct amdgpu_vm *vm)
>> +{
>> +	unsigned long flags;
>> +	spinlock_t *lock;
>> +
>> +	rcu_read_lock();
>> +	lock = dma_fence_spinlock(vm->last_tlb_flush);
>> +	rcu_read_unlock();
>> +
>> +	spin_lock_irqsave(lock, flags);
>> +	spin_unlock_irqrestore(lock, flags);
>> +
>> +	return atomic64_read(&vm->tlb_seq_mm);
>> +}
>> +
>>   /*
>>    * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
>>    * happens while holding this lock anywhere to prevent deadlocks when
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> index 30ad10bbd47e..fb888f200f70 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> @@ -1372,7 +1372,9 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>   		return -EINVAL;
>>   	}
>>   
>> -	return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
>> +	return amdgpu_vm_update_range(adev, vm, false, true, true, false,
>> +				      GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>> +				      NULL, gpu_start,
>>   				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
>>   				      fence);
>>   }
>> @@ -1490,6 +1492,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>>   			 pte_flags);
>>   
>>   		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb, true,
>> +					   GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
>>   					   NULL, gpu_start, gpu_end,
>>   					   pte_flags,
>>   					   (last_start - prange->start) << PAGE_SHIFT,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-05 18:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04 15:26 [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes David (Ming Qiang) Wu
2026-08-05 14:58 ` Christian König
2026-08-05 18:21   ` David Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.