AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/amdgpu: Flush the pasid on all XCCs in parallel
@ 2026-09-28 18:52 David Yat Sin
  2026-09-29 14:22 ` Kuehling, Felix
  0 siblings, 1 reply; 2+ messages in thread
From: David Yat Sin @ 2026-09-28 18:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, philip.yang, David Yat Sin, Felix Kuehling

amdgpu_vm_flush_compute_tlb() walks the XCCs of the compute partition one
at a time, and amdgpu_gmc_flush_gpu_tlb_pasid() submits the invalidation
to one XCC's KIQ and then busy-waits for its fence before the caller can
move on to the next. On a partition that owns every XCC of the device
that is num_xcc round trips back to back.

The XCCs do not depend on each other here. Each has its own KIQ ring,
ring lock and fence sequence, the page tables are already updated before
any invalidation is issued, and no invalidation needs another XCC to have
finished first.

Split the submit out of amdgpu_gmc_flush_gpu_tlb_pasid() and add
amdgpu_gmc_flush_gpu_tlb_pasid_xccs(), which queues the invalidation on
every XCC in the mask before collecting any of the fences, so the round
trips overlap. amdgpu_gmc_flush_gpu_tlb_pasid() becomes a one bit mask
call into it, so both entry points share the reset-domain handling and
the KIQ dispatch.

Note that amdgpu_vm_flush_compute_tlb() no longer stops at the first XCC
that fails. Every XCC in the mask is flushed regardless and the first
error is returned, which is a deliberate behaviour change for callers:
one XCC failing to queue its invalidation no longer leaves the remaining
XCCs unflushed.

Measured on gfx950 with all 8 XCCs in one SPX partition, revoking host
memory access for a pasid mapped on every XCC. Traced with ftrace,
amdgpu_vm_flush_compute_tlb() drops from 141 us to 65 us, against 23 us
for the slowest individual XCC flush. The KFD SVM ioctl carrying that
flush drops from 0.14 ms to 0.05 ms at p50.

Assisted-by: Cursor:claude-opus-5
Signed-off-by: David Yat Sin <David.YatSin@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 233 +++++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  10 +-
 3 files changed, 173 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 1bf2a42fa63c..167f1344f8f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -782,98 +782,197 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	dev_err(adev->dev, "Error flushing GPU TLB using the SDMA (%d)!\n", r);
 }
 
-int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
-				   uint32_t flush_type, bool all_hub,
-				   uint32_t inst)
+static void amdgpu_gmc_flush_pasid_regs(struct amdgpu_device *adev, u16 pasid,
+					u32 flush_type, bool all_hub,
+					u32 inst)
+{
+	if (!adev->gmc.gmc_funcs->flush_gpu_tlb_pasid)
+		return;
+
+	if (adev->gmc.flush_tlb_needs_extra_type_2)
+		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, 2, all_hub,
+							 inst);
+
+	if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
+		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, 0, all_hub,
+							 inst);
+
+	adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, flush_type, all_hub,
+						 inst);
+}
+
+/*
+ * Queue the invalidation on one XCC and return its fence without waiting, so
+ * that a caller flushing several XCCs can have them in flight together.  The
+ * KIQ ring, its lock and its fence sequence are per XCC, so the submissions do
+ * not interfere with each other.
+ */
+static int amdgpu_gmc_flush_pasid_kiq_submit(struct amdgpu_device *adev,
+					     u16 pasid, u32 flush_type,
+					     bool all_hub, u32 inst,
+					     u32 *seq)
 {
-	struct amdgpu_ring *ring = &adev->gfx.kiq[inst].ring;
 	struct amdgpu_kiq *kiq = &adev->gfx.kiq[inst];
+	struct amdgpu_ring *ring = &kiq->ring;
 	unsigned int ndw;
-	int r, cnt = 0;
-	uint32_t seq;
+	int r;
 
-	/*
-	 * A GPU reset should flush all TLBs anyway, so no need to do
-	 * this while one is ongoing.
-	 */
-	if (!down_read_trylock(&adev->reset_domain->sem))
-		return 0;
+	/* one flush + 8 dwords fence */
+	ndw = kiq->pmf->invalidate_tlbs_size + 8;
 
-	if (!adev->gmc.flush_pasid_uses_kiq || !ring->sched.ready) {
+	if (adev->gmc.flush_tlb_needs_extra_type_2)
+		ndw += kiq->pmf->invalidate_tlbs_size;
 
-		if (!adev->gmc.gmc_funcs->flush_gpu_tlb_pasid) {
-			r = 0;
-			goto error_unlock_reset;
-		}
+	if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
+		ndw += kiq->pmf->invalidate_tlbs_size;
 
-		if (adev->gmc.flush_tlb_needs_extra_type_2)
-			adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
-								 2, all_hub,
-								 inst);
+	spin_lock(&kiq->ring_lock);
+	r = amdgpu_ring_alloc(ring, ndw);
+	if (r) {
+		spin_unlock(&kiq->ring_lock);
+		return r;
+	}
 
-		if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
-			adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
-								 0, all_hub,
-								 inst);
+	if (adev->gmc.flush_tlb_needs_extra_type_2)
+		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 2, all_hub);
 
-		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
-							 flush_type, all_hub,
-							 inst);
-		r = 0;
-	} else {
-		/* 2 dwords flush + 8 dwords fence */
-		ndw = kiq->pmf->invalidate_tlbs_size + 8;
+	if (flush_type == 2 && adev->gmc.flush_tlb_needs_extra_type_0)
+		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 0, all_hub);
 
-		if (adev->gmc.flush_tlb_needs_extra_type_2)
-			ndw += kiq->pmf->invalidate_tlbs_size;
-
-		if (adev->gmc.flush_tlb_needs_extra_type_0)
-			ndw += kiq->pmf->invalidate_tlbs_size;
+	kiq->pmf->kiq_invalidate_tlbs(ring, pasid, flush_type, all_hub);
+	r = amdgpu_fence_emit_polling(ring, seq, MAX_KIQ_REG_WAIT);
+	if (r) {
+		amdgpu_ring_undo(ring);
+		spin_unlock(&kiq->ring_lock);
+		return r;
+	}
 
-		spin_lock(&adev->gfx.kiq[inst].ring_lock);
-		r = amdgpu_ring_alloc(ring, ndw);
-		if (r) {
-			spin_unlock(&adev->gfx.kiq[inst].ring_lock);
-			goto error_unlock_reset;
-		}
-		if (adev->gmc.flush_tlb_needs_extra_type_2)
-			kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 2, all_hub);
+	amdgpu_ring_commit(ring);
+	spin_unlock(&kiq->ring_lock);
 
-		if (flush_type == 2 && adev->gmc.flush_tlb_needs_extra_type_0)
-			kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 0, all_hub);
+	return 0;
+}
 
-		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, flush_type, all_hub);
-		r = amdgpu_fence_emit_polling(ring, &seq, MAX_KIQ_REG_WAIT);
-		if (r) {
-			amdgpu_ring_undo(ring);
-			spin_unlock(&adev->gfx.kiq[inst].ring_lock);
-			goto error_unlock_reset;
-		}
+/*
+ * Wait for an invalidation queued by amdgpu_gmc_flush_pasid_kiq_submit().
+ *
+ * Bailing out because a reset became pending is reported as success: the reset
+ * flushes all TLBs anyway, so the invalidation no longer has to complete.  Only
+ * running out of tries is an error.
+ */
+static int amdgpu_gmc_flush_pasid_kiq_wait(struct amdgpu_device *adev,
+					   u32 inst, u32 seq)
+{
+	struct amdgpu_ring *ring = &adev->gfx.kiq[inst].ring;
+	int cnt = 0;
+	signed long r;
 
-		amdgpu_ring_commit(ring);
-		spin_unlock(&adev->gfx.kiq[inst].ring_lock);
+	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
 
+	might_sleep();
+	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY &&
+	       !amdgpu_reset_pending(adev->reset_domain)) {
+		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
 		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+	}
+
+	if (cnt > MAX_KIQ_REG_TRY) {
+		dev_err(adev->dev, "timeout waiting for kiq fence\n");
+		return -ETIME;
+	}
+
+	return 0;
+}
+
+/**
+ * amdgpu_gmc_flush_gpu_tlb_pasid_xccs - flush a pasid on several XCCs at once
+ *
+ * @adev: amdgpu_device pointer
+ * @pasid: pasid to be flushed
+ * @flush_type: the flush type
+ * @all_hub: flush all hubs
+ * @xcc_mask: mask of the XCCs to flush
+ *
+ * Submits the invalidation to every XCC in @xcc_mask before waiting for any of
+ * them, so the round trips overlap instead of running back to back.  Flushing
+ * one XCC at a time costs num_xcc times the latency of a single one, which on
+ * a partition holding every XCC of the device is most of the cost of a compute
+ * TLB flush.
+ *
+ * The XCCs are independent of each other here: the page tables are already
+ * updated before any invalidation is issued, and nothing in an invalidation
+ * depends on another XCC having completed its own.
+ *
+ * Returns:
+ * 0 for success, the first error otherwise.  Every XCC is flushed even if one
+ * of them fails.
+ */
+int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid,
+					u32 flush_type, bool all_hub,
+					u32 xcc_mask)
+{
+	u32 seq[AMDGPU_MAX_GC_INSTANCES];
+	unsigned long pending = 0;
+	int xcc, r = 0, err;
 
-		might_sleep();
-		while (r < 1 && cnt++ < MAX_KIQ_REG_TRY &&
-		       !amdgpu_reset_pending(adev->reset_domain)) {
-			msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
-			r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+	/*
+	 * A GPU reset should flush all TLBs anyway, so no need to do
+	 * this while one is ongoing.
+	 *
+	 * Unlike the one XCC at a time flush this holds the read side across
+	 * every submit and every wait, so a reset's down_write() can only get
+	 * in once the whole mask is done.  The waits are sequential, which
+	 * bounds that at num_xcc * MAX_KIQ_REG_TRY * MAX_KIQ_REG_BAILOUT_INTERVAL
+	 * in the worst case, and amdgpu_gmc_flush_pasid_kiq_wait() cuts each
+	 * wait short as soon as a reset becomes pending.
+	 */
+	if (!down_read_trylock(&adev->reset_domain->sem))
+		return 0;
+
+	for_each_inst(xcc, xcc_mask) {
+		struct amdgpu_ring *ring;
+
+		if (WARN_ON_ONCE(xcc >= AMDGPU_MAX_GC_INSTANCES)) {
+			if (!r)
+				r = -EINVAL;
+			break;
 		}
 
-		if (cnt > MAX_KIQ_REG_TRY) {
-			dev_err(adev->dev, "timeout waiting for kiq fence\n");
-			r = -ETIME;
-		} else
-			r = 0;
+		ring = &adev->gfx.kiq[xcc].ring;
+		if (!adev->gmc.flush_pasid_uses_kiq || !ring->sched.ready) {
+			amdgpu_gmc_flush_pasid_regs(adev, pasid, flush_type,
+						    all_hub, xcc);
+			continue;
+		}
+
+		err = amdgpu_gmc_flush_pasid_kiq_submit(adev, pasid, flush_type,
+							all_hub, xcc, &seq[xcc]);
+		if (err) {
+			if (!r)
+				r = err;
+			continue;
+		}
+
+		pending |= BIT(xcc);
+	}
+
+	for_each_set_bit(xcc, &pending, AMDGPU_MAX_GC_INSTANCES) {
+		err = amdgpu_gmc_flush_pasid_kiq_wait(adev, xcc, seq[xcc]);
+		if (err && !r)
+			r = err;
 	}
 
-error_unlock_reset:
 	up_read(&adev->reset_domain->sem);
 	return r;
 }
 
+int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, u16 pasid,
+				   u32 flush_type, bool all_hub, u32 inst)
+{
+	return amdgpu_gmc_flush_gpu_tlb_pasid_xccs(adev, pasid, flush_type,
+						   all_hub, BIT(inst));
+}
+
 void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index cffd5ca3fc66..76e6f1f96913 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -444,9 +444,11 @@ int amdgpu_gmc_ras_sw_init(struct amdgpu_device *adev);
 int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev);
 void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 			      uint32_t vmhub, uint32_t flush_type);
-int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
-				   uint32_t flush_type, bool all_hub,
-				   uint32_t inst);
+int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, u16 pasid,
+				   u32 flush_type, bool all_hub, u32 inst);
+int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid,
+					u32 flush_type, bool all_hub,
+					u32 xcc_mask);
 void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 29a66e39f3d6..25a74d497255 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1723,7 +1723,6 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
 {
 	uint64_t tlb_seq = amdgpu_vm_tlb_seq(vm);
 	bool all_hub = false;
-	int xcc = 0, r = 0;
 
 	WARN_ON_ONCE(!vm->is_compute_context);
 
@@ -1739,13 +1738,8 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
 	    adev->family == AMDGPU_FAMILY_RV)
 		all_hub = true;
 
-	for_each_inst(xcc, xcc_mask) {
-		r = amdgpu_gmc_flush_gpu_tlb_pasid(adev, vm->pasid, flush_type,
-						   all_hub, xcc);
-		if (r)
-			break;
-	}
-	return r;
+	return amdgpu_gmc_flush_gpu_tlb_pasid_xccs(adev, vm->pasid, flush_type,
+						   all_hub, xcc_mask);
 }
 
 /**
-- 
2.34.1


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

* Re: [PATCH v2] drm/amdgpu: Flush the pasid on all XCCs in parallel
  2026-09-28 18:52 [PATCH v2] drm/amdgpu: Flush the pasid on all XCCs in parallel David Yat Sin
@ 2026-09-29 14:22 ` Kuehling, Felix
  0 siblings, 0 replies; 2+ messages in thread
From: Kuehling, Felix @ 2026-09-29 14:22 UTC (permalink / raw)
  To: David Yat Sin, amd-gfx; +Cc: philip.yang

On 2026-09-28 14:52, David Yat Sin wrote:
> amdgpu_vm_flush_compute_tlb() walks the XCCs of the compute partition one
> at a time, and amdgpu_gmc_flush_gpu_tlb_pasid() submits the invalidation
> to one XCC's KIQ and then busy-waits for its fence before the caller can
> move on to the next. On a partition that owns every XCC of the device
> that is num_xcc round trips back to back.
>
> The XCCs do not depend on each other here. Each has its own KIQ ring,
> ring lock and fence sequence, the page tables are already updated before
> any invalidation is issued, and no invalidation needs another XCC to have
> finished first.
>
> Split the submit out of amdgpu_gmc_flush_gpu_tlb_pasid() and add
> amdgpu_gmc_flush_gpu_tlb_pasid_xccs(), which queues the invalidation on
> every XCC in the mask before collecting any of the fences, so the round
> trips overlap. amdgpu_gmc_flush_gpu_tlb_pasid() becomes a one bit mask
> call into it, so both entry points share the reset-domain handling and
> the KIQ dispatch.
>
> Note that amdgpu_vm_flush_compute_tlb() no longer stops at the first XCC
> that fails. Every XCC in the mask is flushed regardless and the first
> error is returned, which is a deliberate behaviour change for callers:
> one XCC failing to queue its invalidation no longer leaves the remaining
> XCCs unflushed.
>
> Measured on gfx950 with all 8 XCCs in one SPX partition, revoking host
> memory access for a pasid mapped on every XCC. Traced with ftrace,
> amdgpu_vm_flush_compute_tlb() drops from 141 us to 65 us, against 23 us
> for the slowest individual XCC flush. The KFD SVM ioctl carrying that
> flush drops from 0.14 ms to 0.05 ms at p50.
>
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: David Yat Sin <David.YatSin@amd.com>

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 233 +++++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |   8 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  10 +-
>   3 files changed, 173 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 1bf2a42fa63c..167f1344f8f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -782,98 +782,197 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>   	dev_err(adev->dev, "Error flushing GPU TLB using the SDMA (%d)!\n", r);
>   }
>   
> -int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
> -				   uint32_t flush_type, bool all_hub,
> -				   uint32_t inst)
> +static void amdgpu_gmc_flush_pasid_regs(struct amdgpu_device *adev, u16 pasid,
> +					u32 flush_type, bool all_hub,
> +					u32 inst)
> +{
> +	if (!adev->gmc.gmc_funcs->flush_gpu_tlb_pasid)
> +		return;
> +
> +	if (adev->gmc.flush_tlb_needs_extra_type_2)
> +		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, 2, all_hub,
> +							 inst);
> +
> +	if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
> +		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, 0, all_hub,
> +							 inst);
> +
> +	adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid, flush_type, all_hub,
> +						 inst);
> +}
> +
> +/*
> + * Queue the invalidation on one XCC and return its fence without waiting, so
> + * that a caller flushing several XCCs can have them in flight together.  The
> + * KIQ ring, its lock and its fence sequence are per XCC, so the submissions do
> + * not interfere with each other.
> + */
> +static int amdgpu_gmc_flush_pasid_kiq_submit(struct amdgpu_device *adev,
> +					     u16 pasid, u32 flush_type,
> +					     bool all_hub, u32 inst,
> +					     u32 *seq)
>   {
> -	struct amdgpu_ring *ring = &adev->gfx.kiq[inst].ring;
>   	struct amdgpu_kiq *kiq = &adev->gfx.kiq[inst];
> +	struct amdgpu_ring *ring = &kiq->ring;
>   	unsigned int ndw;
> -	int r, cnt = 0;
> -	uint32_t seq;
> +	int r;
>   
> -	/*
> -	 * A GPU reset should flush all TLBs anyway, so no need to do
> -	 * this while one is ongoing.
> -	 */
> -	if (!down_read_trylock(&adev->reset_domain->sem))
> -		return 0;
> +	/* one flush + 8 dwords fence */
> +	ndw = kiq->pmf->invalidate_tlbs_size + 8;
>   
> -	if (!adev->gmc.flush_pasid_uses_kiq || !ring->sched.ready) {
> +	if (adev->gmc.flush_tlb_needs_extra_type_2)
> +		ndw += kiq->pmf->invalidate_tlbs_size;
>   
> -		if (!adev->gmc.gmc_funcs->flush_gpu_tlb_pasid) {
> -			r = 0;
> -			goto error_unlock_reset;
> -		}
> +	if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
> +		ndw += kiq->pmf->invalidate_tlbs_size;
>   
> -		if (adev->gmc.flush_tlb_needs_extra_type_2)
> -			adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
> -								 2, all_hub,
> -								 inst);
> +	spin_lock(&kiq->ring_lock);
> +	r = amdgpu_ring_alloc(ring, ndw);
> +	if (r) {
> +		spin_unlock(&kiq->ring_lock);
> +		return r;
> +	}
>   
> -		if (adev->gmc.flush_tlb_needs_extra_type_0 && flush_type == 2)
> -			adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
> -								 0, all_hub,
> -								 inst);
> +	if (adev->gmc.flush_tlb_needs_extra_type_2)
> +		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 2, all_hub);
>   
> -		adev->gmc.gmc_funcs->flush_gpu_tlb_pasid(adev, pasid,
> -							 flush_type, all_hub,
> -							 inst);
> -		r = 0;
> -	} else {
> -		/* 2 dwords flush + 8 dwords fence */
> -		ndw = kiq->pmf->invalidate_tlbs_size + 8;
> +	if (flush_type == 2 && adev->gmc.flush_tlb_needs_extra_type_0)
> +		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 0, all_hub);
>   
> -		if (adev->gmc.flush_tlb_needs_extra_type_2)
> -			ndw += kiq->pmf->invalidate_tlbs_size;
> -
> -		if (adev->gmc.flush_tlb_needs_extra_type_0)
> -			ndw += kiq->pmf->invalidate_tlbs_size;
> +	kiq->pmf->kiq_invalidate_tlbs(ring, pasid, flush_type, all_hub);
> +	r = amdgpu_fence_emit_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	if (r) {
> +		amdgpu_ring_undo(ring);
> +		spin_unlock(&kiq->ring_lock);
> +		return r;
> +	}
>   
> -		spin_lock(&adev->gfx.kiq[inst].ring_lock);
> -		r = amdgpu_ring_alloc(ring, ndw);
> -		if (r) {
> -			spin_unlock(&adev->gfx.kiq[inst].ring_lock);
> -			goto error_unlock_reset;
> -		}
> -		if (adev->gmc.flush_tlb_needs_extra_type_2)
> -			kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 2, all_hub);
> +	amdgpu_ring_commit(ring);
> +	spin_unlock(&kiq->ring_lock);
>   
> -		if (flush_type == 2 && adev->gmc.flush_tlb_needs_extra_type_0)
> -			kiq->pmf->kiq_invalidate_tlbs(ring, pasid, 0, all_hub);
> +	return 0;
> +}
>   
> -		kiq->pmf->kiq_invalidate_tlbs(ring, pasid, flush_type, all_hub);
> -		r = amdgpu_fence_emit_polling(ring, &seq, MAX_KIQ_REG_WAIT);
> -		if (r) {
> -			amdgpu_ring_undo(ring);
> -			spin_unlock(&adev->gfx.kiq[inst].ring_lock);
> -			goto error_unlock_reset;
> -		}
> +/*
> + * Wait for an invalidation queued by amdgpu_gmc_flush_pasid_kiq_submit().
> + *
> + * Bailing out because a reset became pending is reported as success: the reset
> + * flushes all TLBs anyway, so the invalidation no longer has to complete.  Only
> + * running out of tries is an error.
> + */
> +static int amdgpu_gmc_flush_pasid_kiq_wait(struct amdgpu_device *adev,
> +					   u32 inst, u32 seq)
> +{
> +	struct amdgpu_ring *ring = &adev->gfx.kiq[inst].ring;
> +	int cnt = 0;
> +	signed long r;
>   
> -		amdgpu_ring_commit(ring);
> -		spin_unlock(&adev->gfx.kiq[inst].ring_lock);
> +	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
>   
> +	might_sleep();
> +	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY &&
> +	       !amdgpu_reset_pending(adev->reset_domain)) {
> +		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
>   		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	}
> +
> +	if (cnt > MAX_KIQ_REG_TRY) {
> +		dev_err(adev->dev, "timeout waiting for kiq fence\n");
> +		return -ETIME;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * amdgpu_gmc_flush_gpu_tlb_pasid_xccs - flush a pasid on several XCCs at once
> + *
> + * @adev: amdgpu_device pointer
> + * @pasid: pasid to be flushed
> + * @flush_type: the flush type
> + * @all_hub: flush all hubs
> + * @xcc_mask: mask of the XCCs to flush
> + *
> + * Submits the invalidation to every XCC in @xcc_mask before waiting for any of
> + * them, so the round trips overlap instead of running back to back.  Flushing
> + * one XCC at a time costs num_xcc times the latency of a single one, which on
> + * a partition holding every XCC of the device is most of the cost of a compute
> + * TLB flush.
> + *
> + * The XCCs are independent of each other here: the page tables are already
> + * updated before any invalidation is issued, and nothing in an invalidation
> + * depends on another XCC having completed its own.
> + *
> + * Returns:
> + * 0 for success, the first error otherwise.  Every XCC is flushed even if one
> + * of them fails.
> + */
> +int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid,
> +					u32 flush_type, bool all_hub,
> +					u32 xcc_mask)
> +{
> +	u32 seq[AMDGPU_MAX_GC_INSTANCES];
> +	unsigned long pending = 0;
> +	int xcc, r = 0, err;
>   
> -		might_sleep();
> -		while (r < 1 && cnt++ < MAX_KIQ_REG_TRY &&
> -		       !amdgpu_reset_pending(adev->reset_domain)) {
> -			msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> -			r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	/*
> +	 * A GPU reset should flush all TLBs anyway, so no need to do
> +	 * this while one is ongoing.
> +	 *
> +	 * Unlike the one XCC at a time flush this holds the read side across
> +	 * every submit and every wait, so a reset's down_write() can only get
> +	 * in once the whole mask is done.  The waits are sequential, which
> +	 * bounds that at num_xcc * MAX_KIQ_REG_TRY * MAX_KIQ_REG_BAILOUT_INTERVAL
> +	 * in the worst case, and amdgpu_gmc_flush_pasid_kiq_wait() cuts each
> +	 * wait short as soon as a reset becomes pending.
> +	 */
> +	if (!down_read_trylock(&adev->reset_domain->sem))
> +		return 0;
> +
> +	for_each_inst(xcc, xcc_mask) {
> +		struct amdgpu_ring *ring;
> +
> +		if (WARN_ON_ONCE(xcc >= AMDGPU_MAX_GC_INSTANCES)) {
> +			if (!r)
> +				r = -EINVAL;
> +			break;
>   		}
>   
> -		if (cnt > MAX_KIQ_REG_TRY) {
> -			dev_err(adev->dev, "timeout waiting for kiq fence\n");
> -			r = -ETIME;
> -		} else
> -			r = 0;
> +		ring = &adev->gfx.kiq[xcc].ring;
> +		if (!adev->gmc.flush_pasid_uses_kiq || !ring->sched.ready) {
> +			amdgpu_gmc_flush_pasid_regs(adev, pasid, flush_type,
> +						    all_hub, xcc);
> +			continue;
> +		}
> +
> +		err = amdgpu_gmc_flush_pasid_kiq_submit(adev, pasid, flush_type,
> +							all_hub, xcc, &seq[xcc]);
> +		if (err) {
> +			if (!r)
> +				r = err;
> +			continue;
> +		}
> +
> +		pending |= BIT(xcc);
> +	}
> +
> +	for_each_set_bit(xcc, &pending, AMDGPU_MAX_GC_INSTANCES) {
> +		err = amdgpu_gmc_flush_pasid_kiq_wait(adev, xcc, seq[xcc]);
> +		if (err && !r)
> +			r = err;
>   	}
>   
> -error_unlock_reset:
>   	up_read(&adev->reset_domain->sem);
>   	return r;
>   }
>   
> +int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, u16 pasid,
> +				   u32 flush_type, bool all_hub, u32 inst)
> +{
> +	return amdgpu_gmc_flush_gpu_tlb_pasid_xccs(adev, pasid, flush_type,
> +						   all_hub, BIT(inst));
> +}
> +
>   void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
>   				      uint32_t reg0, uint32_t reg1,
>   				      uint32_t ref, uint32_t mask,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index cffd5ca3fc66..76e6f1f96913 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -444,9 +444,11 @@ int amdgpu_gmc_ras_sw_init(struct amdgpu_device *adev);
>   int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev);
>   void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>   			      uint32_t vmhub, uint32_t flush_type);
> -int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
> -				   uint32_t flush_type, bool all_hub,
> -				   uint32_t inst);
> +int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, u16 pasid,
> +				   u32 flush_type, bool all_hub, u32 inst);
> +int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid,
> +					u32 flush_type, bool all_hub,
> +					u32 xcc_mask);
>   void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
>   				      uint32_t reg0, uint32_t reg1,
>   				      uint32_t ref, uint32_t mask,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 29a66e39f3d6..25a74d497255 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1723,7 +1723,6 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
>   {
>   	uint64_t tlb_seq = amdgpu_vm_tlb_seq(vm);
>   	bool all_hub = false;
> -	int xcc = 0, r = 0;
>   
>   	WARN_ON_ONCE(!vm->is_compute_context);
>   
> @@ -1739,13 +1738,8 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
>   	    adev->family == AMDGPU_FAMILY_RV)
>   		all_hub = true;
>   
> -	for_each_inst(xcc, xcc_mask) {
> -		r = amdgpu_gmc_flush_gpu_tlb_pasid(adev, vm->pasid, flush_type,
> -						   all_hub, xcc);
> -		if (r)
> -			break;
> -	}
> -	return r;
> +	return amdgpu_gmc_flush_gpu_tlb_pasid_xccs(adev, vm->pasid, flush_type,
> +						   all_hub, xcc_mask);
>   }
>   
>   /**

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

end of thread, other threads:[~2026-09-29 14:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-28 18:52 [PATCH v2] drm/amdgpu: Flush the pasid on all XCCs in parallel David Yat Sin
2026-09-29 14:22 ` Kuehling, Felix

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox