* [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(¶ms, 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(¶ms.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(¶ms, 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(¶ms.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(¶ms, 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(¶ms.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.