Re: [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes

David Wu Wed, 05 Aug 2026 11:21:41 -0700

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 PTEupdates and not frees.let me work on another patch to address your concern and explain thedetails.


thanks,
David


Regards,
Christian.

Signed-off-by: David (Ming Qiang) Wu <[email protected]>
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,

Re: [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes

Reply via email to