[PATCH v8] drm/amdgpu: Resync UALink ring wptr after reboot

[Philip Yang <Philip.Yang@amd.com>]

When one side of UALink nodes reboots, its ring wptr resets to 0 while
the peer keeps a stale value. Read the remote wptr and adopt it as the
local wptr and rptr to resync, when setting up the connection, before
the first message is sent.

On the sdma path, read the wptr in a fence callback because the IB buffer
holding the result is recycled once the fence signals, so reading it
after dma_fence_wait_timeout() is too late. The lsdma path copies
synchronously, so it reads the wptr directly.

Take peer->lock in the callers (amdgpu_ualink_remote_interrupt and
amdgpu_ualink_remote_shootdown) instead of in amdgpu_ualink_send_command,
because amdgpu_ualink_update_wb_address() also reads the remote wptr and
must run under the same lock.

On connection reset, amdgpu_ualink_reset_peer_rings() only clears
ring->ready; the remote wptr is re-read on the next update wb command.

v8:
  - Fix regression with concurrent import ioctls: do the idle check and
    ring reset in amdgpu_ualink_setup_connection() under conn_state->lock,
    and return -EAGAIN for IN_PROGRESS/PENDING first.
  - Reset the peer rings only, the imp/exp xarray cleanup is not needed to
    resync wptr.

v7:
  - With v6 reboot one node of vPOD test passed on minirack.
  - Use the per-remote imported and exported handle lists for the idle
    check instead of walking the xarrays.
  - Rename amdgpu_ualink_resync_peer_rings() to
    amdgpu_ualink_reset_peer_rings(), it only clears ring->ready.

v6:
  - Check idleness per remote accelerator instead of globally, and reset
    the stale connection at the import ioctl so the peer re-runs the
    HELLO handshake after a reboot.

v5:
  - Resync the ring pointers at the import ioctl when the connection is
    completely idle (no exported or imported BOs), so the first command
    from a new application after a remote reboot/reset succeeds.
  - Put the sdma wptr read fence callback on the stack instead of
    allocating it; it is cancelled on timeout so it never outlives the
    call.

v4:
  - Cancel the sdma fence callback on the wait timeout/failure path so it
    cannot write into the caller's stack after the function returns, and
    free the callback state in one place.

v3:
  - Resync both rings at connection setup instead of on the send path.
  - Set the writeback rptr too, the stale value disagrees with the
    resynced wptr until the remote FW writes it back.

v2:
  - Match the FW change: resync wptr only on the first send or after a
    timeout, instead of checking the remote periodically.

Suggested-by: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 288 +++++++++++++++++----
 1 file changed, 244 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index ab564202f550..5c6ef467043c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -55,6 +55,9 @@ static int amdgpu_ualink_remote_shootdown(struct amdgpu_device *adev,
 					  u32 size_in_pages, u32 flush_type);
 static void __amdgpu_ualink_activate_vpod_locked(struct amdgpu_device *adev);
 static bool amdgpu_ualink_vpod_membership_changed(struct amdgpu_device *adev);
+static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
+					   u32 remote_accel_id);
+static void amdgpu_ualink_metadata_fini(struct amdgpu_device *adev);
 
 #define STRIP_NPA(addr)						\
 	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
@@ -2304,6 +2307,14 @@ static void amdgpu_ualink_process_hello_msg(struct amdgpu_device *adev,
 	mutex_unlock(&conn_state->lock);
 }
 
+/* True if nothing is exported to, or imported from, this remote accelerator. */
+static bool amdgpu_ualink_peer_idle(struct amdgpu_device *adev, u32 remote_accel_id)
+{
+	/* Unlocked: a racing import at worst costs one redundant ring resync. */
+	return list_empty(&adev->ualink.imp_handles_list[remote_accel_id]) &&
+	       list_empty(&adev->ualink.exp_handles_list[remote_accel_id]);
+}
+
 static int amdgpu_ualink_setup_connection(struct amdgpu_device *adev,
 					  u32 remote_acc_id)
 {
@@ -2325,13 +2336,21 @@ static int amdgpu_ualink_setup_connection(struct amdgpu_device *adev,
 	 * already done by another thread.
 	 */
 	mutex_lock(&conn_state->lock);
+	if (conn_state->state == AMDGPU_UALINK_CONN_IN_PROGRESS ||
+	    conn_state->state == AMDGPU_UALINK_CONN_PENDING) {
+		r = -EAGAIN;
+		goto out;
+	}
+
+	/* Peer is idle, it may have rebooted, force both rings to resync wptr
+	 * before the NPA handshake.
+	 */
+	if (amdgpu_ualink_peer_idle(adev, remote_acc_id))
+		amdgpu_ualink_reset_peer_rings(adev, remote_acc_id);
+
 	if (conn_state->state == AMDGPU_UALINK_CONN_ESTABLISHED) {
 		r = 0;
 		goto out;
-	} else if (conn_state->state == AMDGPU_UALINK_CONN_IN_PROGRESS ||
-		   conn_state->state == AMDGPU_UALINK_CONN_PENDING) {
-		r = -EAGAIN;
-		goto out;
 	}
 
 	conn_state->state = AMDGPU_UALINK_CONN_IN_PROGRESS;
@@ -3671,7 +3690,7 @@ static int amdgpu_ualink_do_import_handle(struct amdgpu_device *adev,
 		dev_warn(adev->dev,
 			"IMPORT: NPA-REQ send failed to remote AccId:%u\n",
 			remote_acc_id);
-		return r;
+		goto reset_conn;
 	}
 
 	/* Wait for the NPA_RSP to come back */
@@ -5512,6 +5531,184 @@ static void amdgpu_ualink_emit_update_wb_addr(u32 **cpu_addr_p, u32 wb_data,
 typedef void (*ualink_emit_packet)(u32 **cpu_addr_p, u32 wb, u32 dw0,
 				   u32 dw1, u32 dw2, u32 dw3);
 
+struct amdgpu_ualink_wptr_fence_cb {
+	struct dma_fence_cb base;
+	u64 *wptr_cpu;
+	u64 result;
+};
+
+static void amdgpu_ualink_read_wptr_fence_cb(struct dma_fence *fence,
+					     struct dma_fence_cb *cb)
+{
+	struct amdgpu_ualink_wptr_fence_cb *wptr_cb;
+
+	wptr_cb = container_of(cb, typeof(*wptr_cb), base);
+	wptr_cb->result = *wptr_cb->wptr_cpu;
+}
+
+static int amdgpu_ualink_read_remote_wptr(struct amdgpu_device *adev,
+					  u32 remote_accel_id,
+					  struct amdgpu_ualink_ring *ring,
+					  u64 *wptr)
+{
+	struct amdgpu_ualink_remote *remote = to_remote(adev);
+	struct amdgpu_ualink_peer *peer = &remote->peer[remote_accel_id];
+	struct amdgpu_ualink_wptr_fence_cb wptr_cb = { };
+	struct amdgpu_ring *sdma_ring;
+	struct dma_fence *fence;
+	struct amdgpu_job *job;
+	struct amdgpu_ib *ib;
+	u32 ndw, ndw_copy_cmd;
+	u64 wptr_gpu, *wptr_cpu;
+	int r = 0;
+
+	*wptr = 0;
+
+	/* 1 sdma copy command to read wptr */
+	ndw_copy_cmd = ALIGN(adev->mman.buffer_funcs->copy_num_dw, 8);
+
+	/* wptr 2 dwords */
+	ndw = ndw_copy_cmd + 2;
+	r = amdgpu_job_alloc_with_ib(adev, &peer->entity, AMDGPU_FENCE_OWNER_VM,
+				     ndw * 4, AMDGPU_IB_POOL_IMMEDIATE,
+				     AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER, &job);
+	if (r)
+		return r;
+
+	ib = &job->ibs[0];
+	wptr_gpu = ib->gpu_addr + ndw_copy_cmd * 4;
+	wptr_cpu = (u64 *)(ib->ptr + ndw_copy_cmd);
+
+	dev_dbg(adev->dev, "read wptr from remote %u npa gart wptr 0x%llx use %s\n",
+		remote_accel_id, ring->wptr_npa_gart,
+		remote->use_lsdma ? "lsdma" : "sdma");
+
+	if (remote->use_lsdma) {
+		r = amdgpu_lsdma_copy_mem(adev, ring->wptr_npa_gart, wptr_gpu, 8);
+		if (!r)
+			*wptr = *wptr_cpu;
+		amdgpu_job_free(job);
+		goto out;
+	}
+
+	wptr_cb.wptr_cpu = wptr_cpu;
+
+	amdgpu_emit_copy_buffer(adev, ib, ring->wptr_npa_gart, wptr_gpu, 8, 0);
+
+	sdma_ring = &adev->sdma.instance[0].ring;
+	amdgpu_ring_pad_ib(sdma_ring, ib);
+	WARN_ON(ib->length_dw > ndw_copy_cmd);
+
+	fence = amdgpu_job_submit(job);
+
+	r = dma_fence_add_callback(fence, &wptr_cb.base,
+				   amdgpu_ualink_read_wptr_fence_cb);
+	if (r == -ENOENT) {
+		/* Fence already signaled; the callback won't run, do it inline. */
+		amdgpu_ualink_read_wptr_fence_cb(fence, &wptr_cb.base);
+		r = 0;
+		goto out_put_fence;
+	} else if (r) {
+		/* -EINVAL: NULL fence or func, not expected here. */
+		goto out_put_fence;
+	}
+
+	r = dma_fence_wait_timeout(fence, false, AMDGPU_FENCE_JIFFIES_TIMEOUT);
+	if (r > 0) {
+		r = 0;	/* read wptr successfully */
+		goto out_put_fence;
+	}
+
+	dev_dbg(adev->dev, "remote %u sdma fence wait return r %d\n",
+		remote_accel_id, r);
+
+	if (r == 0)
+		r = -ETIME;
+
+	/*
+	 * Timed out: the callback is still armed and wptr_cb is on our stack.
+	 * Cancel it so it can't fire after we return and write into the freed
+	 * stack frame.
+	 */
+	dma_fence_remove_callback(fence, &wptr_cb.base);
+
+out_put_fence:
+	*wptr = wptr_cb.result;
+	dma_fence_put(fence);
+
+out:
+	dev_dbg(adev->dev, "remote %u wptr 0x%llx return %d\n",
+		remote_accel_id, *wptr, r);
+	return r;
+}
+
+/**
+ * amdgpu_ualink_ring_resync_ptrs - sync local ring pointers from remote
+ * @adev: amdgpu device pointer
+ * @remote_accel_id: remote accelerator ID
+ * @ring: ring buffer to resync
+ * @wb_cpu: CPU virtual address of the ring writeback buffer
+ *
+ * After a reboot on either side the two ends disagree on wptr. Read the
+ * remote wptr and copy it into the local wptr and rptr to resync.
+ *
+ * Return 0 on success or a negative error code if the remote read failed.
+ */
+static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
+					  u32 remote_accel_id,
+					  struct amdgpu_ualink_ring *ring,
+					  struct amdgpu_ualink_wb *wb_cpu)
+{
+	u64 remote_wptr;
+	int r;
+
+	/* Read remote wptr and adopt it as our wptr and rptr. */
+	r = amdgpu_ualink_read_remote_wptr(adev, remote_accel_id, ring, &remote_wptr);
+	if (r) {
+		dev_dbg(adev->dev, "accel_id %u read remote %u wptr failed %d\n",
+			ualink_accel_id(adev), remote_accel_id, r);
+		return r;
+	}
+
+	dev_dbg(adev->dev, "accel_id %u wptr 0x%llx sync to remote %u wptr 0x%llx\n",
+		ualink_accel_id(adev), ring->wptr, remote_accel_id, remote_wptr);
+
+	ring->wptr = remote_wptr;
+	ring->rptr = remote_wptr;
+
+	/* Drop the stale writeback rptr, it disagrees with the resynced wptr. */
+	WRITE_ONCE(wb_cpu->rptr, remote_wptr);
+	return 0;
+}
+
+/*
+ * Refresh rptr from the writeback and check the ring has room for one more
+ * command. The caller must hold the peer lock.
+ *
+ * Return 0 on success or a negative error code.
+ */
+static int amdgpu_ualink_ring_check_space(struct amdgpu_device *adev,
+					  u32 remote_accel_id,
+					  struct amdgpu_ualink_ring *ring,
+					  struct amdgpu_ualink_wb *wb_cpu)
+{
+	ring->rptr = READ_ONCE(wb_cpu->rptr);
+
+	if (WARN_ON_ONCE(ring->rptr > ring->wptr)) {
+		dev_err(adev->dev, "accel_id %u ring overflow wptr 0x%llx rptr 0x%llx\n",
+			remote_accel_id, ring->wptr, ring->rptr);
+		return -EFAULT;
+	}
+
+	if ((ring->wptr + 1 - ring->rptr) >= ring->rb_size) {
+		dev_err(adev->dev, "accel_id %u command ring full wptr 0x%llx rptr 0x%llx\n",
+			remote_accel_id, ring->wptr, ring->rptr);
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
 static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 				      u32 remote_accel_id,
 				      struct amdgpu_ualink_ring *ring,
@@ -5537,6 +5734,10 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 
 	peer = &remote->peer[remote_accel_id];
 
+	r = amdgpu_ualink_ring_check_space(adev, remote_accel_id, ring, wb_cpu);
+	if (r)
+		goto out;
+
 	/*
 	 * 3 sdma copy commands: write data to ring buffer, update wptr, ring doorbell
 	 *
@@ -5553,25 +5754,7 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 				     ndw * 4, AMDGPU_IB_POOL_IMMEDIATE,
 				     AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER, &job);
 	if (r)
-		return r;
-
-	mutex_lock(&peer->lock);
-
-	ring->rptr = READ_ONCE(wb_cpu->rptr);
-
-	if (WARN_ON_ONCE(ring->rptr > ring->wptr)) {
-		dev_err(adev->dev, "accel_id %u ring overflow wptr 0x%llx rptr 0x%llx\n",
-			remote_accel_id, ring->wptr, ring->rptr);
-		r = -EFAULT;
-		goto unlock_free;
-	}
-
-	if ((ring->wptr + 1 - ring->rptr) >= ring->rb_size) {
-		dev_err(adev->dev, "accel_id %u command ring full wptr 0x%llx rptr 0x%llx\n",
-			remote_accel_id, ring->wptr, ring->rptr);
-		r = -ENOSPC;
-		goto unlock_free;
-	}
+		goto out;
 
 	ib = &job->ibs[0];
 	src = ib->gpu_addr + ndw_copy_cmd * 4;
@@ -5629,12 +5812,11 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 			dev_dbg(adev->dev,
 				"remote %u lsdma copy failed (r %d), skip completion wait\n",
 				remote_accel_id, r);
-			mutex_unlock(&peer->lock);
 			return r;
 		}
 
 		goto out_wait_complete;
-        }
+	}
 
 	sdma_ring = &adev->sdma.instance[0].ring;
 
@@ -5665,12 +5847,9 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 	if (r <= 0) {
 		dev_dbg(adev->dev, "remote %u sdma fence wait return r %d\n",
 			remote_accel_id, r);
-
 		if (r == 0)
 			r = -ETIME;
-
-		mutex_unlock(&peer->lock);
-		return r;
+		goto out;
 	}
 
 out_wait_complete:
@@ -5679,21 +5858,15 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 	 */
 	r = amdgpu_ualink_remote_wait_timeout(adev, remote_accel_id, wb_cpu, seq);
 
-	/* increase local copy ring wptr, only if FW not timeout */
-	if (r != -ETIME)
-		ring->wptr++;
-
 	/*
-	 * Release ring lock after the remote FW handle command completes to
-	 * prevent race conditions.
+	 * Advance wptr unless the remote timed out, even if the FW returned an
+	 * error status. On timeout the caller resets the connection, which
+	 * resyncs wptr from the remote in amdgpu_ualink_setup_connection.
 	 */
-	mutex_unlock(&peer->lock);
-	return r;
-
+	if (r != -ETIME)
+		ring->wptr++;
 
-unlock_free:
-	mutex_unlock(&peer->lock);
-	amdgpu_job_free(job);
+out:
 	dev_dbg(adev->dev, "ret r = %d\n", r);
 	return r;
 }
@@ -5733,6 +5906,19 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
 		remote_accel_id, npa + offset);
 }
 
+static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
+					   u32 remote_accel_id)
+{
+	struct amdgpu_ualink_peer *peer = &to_remote(adev)->peer[remote_accel_id];
+
+	dev_dbg(adev->dev, "remote accel_id %u\n", remote_accel_id);
+
+	scoped_guard(mutex, &peer->lock) {
+		peer->interrupt.ready = false;
+		peer->shootdown.ready = false;
+	}
+}
+
 static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
 					   u32 remote_accel_id, u32 ring_type)
 {
@@ -5754,6 +5940,10 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
 	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &wb_cpu, &wb_npa,
 				  ring_type);
 
+	r = amdgpu_ualink_ring_resync_ptrs(adev, remote_accel_id, ring, wb_cpu);
+	if (r)
+		return r;
+
 	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, wb_cpu,
 				       amdgpu_ualink_emit_update_wb_addr,
 				       upper_32_bits(wb_npa),
@@ -5795,18 +5985,23 @@ static int amdgpu_ualink_remote_shootdown(struct amdgpu_device *adev,
 
 	peer = &remote->peer[remote_accel_id];
 	ring = &peer->shootdown;
+
+	mutex_lock(&peer->lock);
+
 	if (!ring->ready) {
 		dev_dbg(adev->dev, "accel_id %u ring not ready\n", remote_accel_id);
 		r = amdgpu_ualink_update_wb_address(adev, remote_accel_id,
 						    RB_TYPE_TLB_INV);
 		if (r)
-			return r;
+			goto out_unlock;
 	}
 
 	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, ring->wb_cpu,
 				       amdgpu_ualink_emit_shootdown,
 				       flush_type, upper_32_bits(addr),
 				       lower_32_bits(addr), size_in_pages);
+out_unlock:
+	mutex_unlock(&peer->lock);
 	return r;
 }
 
@@ -5836,17 +6031,22 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
 
 	peer = &remote->peer[remote_accel_id];
 	ring = &peer->interrupt;
+
+	mutex_lock(&peer->lock);
+
 	if (!ring->ready) {
 		dev_dbg(adev->dev, "accel_id %u ring not ready\n", remote_accel_id);
 		r = amdgpu_ualink_update_wb_address(adev, remote_accel_id,
 						    RB_TYPE_REMOTE_INTERRUPT);
 		if (r)
-			return r;
+			goto out_unlock;
 	}
 
 	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, ring->wb_cpu,
 				       amdgpu_ualink_emit_interrupt,
 				       dw0, dw1, dw2, dw3);
+out_unlock:
+	mutex_unlock(&peer->lock);
 	return r;
 }
 
-- 
2.50.1