All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/8] Add UALink Loopback support
@ 2026-11-02 15:47 Mukul Joshi
  2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi

This series of patches adds UALink loopback support. This will be
used for validating UALink HW path in manufacturing.

Mukul Joshi (8):
  drm/amdgpu: Add UALink loopback infrastructure
  drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink
  drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  drm/amdgpu: Attribute UALink loopback messages to the correct sender
  drm/amdgpu: Set up UALink rings and metadata for loopback
  drm/amdgpu: Enable UALink loopback mode
  drm/amdgpu: Reject config updates in UALink loopback mode
  drm/amdgpu: Temporarily enable UALink loopback via module parameter

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 548 +++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h |  29 ++
 4 files changed, 506 insertions(+), 85 deletions(-)

-- 
2.56.0


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

* [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 18:17   ` Philip Yang
  2026-11-03 23:18   ` Felix Kuehling
  2026-11-02 15:47 ` [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink Mukul Joshi
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi

Add the infrastructure for UALink loopback mode, where a single GPU
plays both the exporter and the importer role under two fixed
accelerator IDs:

- Add struct amdgpu_ualink_loopback to track the loopback state, along
  with the fixed exporter and importer accel IDs.

- Replace ualink_accel_id() with ualink_exp_accel_id() and
  ualink_imp_accel_id(), which return the exporter and importer accel
  ID in loopback mode.

- Add amdgpu_ualink_loopback_sw_init(), which allocates the ordered
  workqueue used to emulate exporter->importer messages from
  ualink_ip_sw_init(), and amdgpu_ualink_loopback_fini(), which
  destroys it from ualink_ip_sw_fini().

Loopback is not enabled yet, and outside loopback both helpers return
ppod.accel_id, so there is no functional change.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
v1->v2:
- Drop the change in find_peer_adev() as its not needed.

v2->v3:
- Update src_accel_id to the correct value in amdgpu_ualink_peer_remote_init()
  and amdgpu_ualink_peer_remote_fini().
- Add the loopback teardown in amdgpu_ualink_loopback_fini() and call it
  through ualink_ip_sw_fini().

 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 128 +++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h |  29 +++++
 2 files changed, 133 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index ca6432610f71..a98d5bbf0622 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -89,6 +89,41 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
 	return 0;
 }
 
+/*
+ * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
+ *
+ * Allocates the ordered workqueue used to emulate exporter->importer
+ * interrupts in SW. Does nothing unless loopback mode is enabled.
+ */
+static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
+{
+	if (!adev->ualink.loopback.enabled)
+		return 0;
+
+	adev->ualink.loopback.wq =
+		alloc_ordered_workqueue("amdgpu_ualink_loopback", 0);
+	if (!adev->ualink.loopback.wq)
+		return -ENOMEM;
+
+	return 0;
+}
+
+/*
+ * amdgpu_ualink_loopback_fini - tear down UALink loopback mode
+ *
+ * Destroys the workqueue allocated in amdgpu_ualink_loopback_sw_init().
+ * destroy_workqueue() drains any pending self-interrupt work first.
+ */
+static void amdgpu_ualink_loopback_fini(struct amdgpu_device *adev)
+{
+	if (!adev->ualink.loopback.wq)
+		return;
+
+	destroy_workqueue(adev->ualink.loopback.wq);
+	adev->ualink.loopback.wq = NULL;
+	adev->ualink.loopback.enabled = false;
+}
+
 static void amdgpu_ualink_object_fini(struct amdgpu_device *adev)
 {
 	if (!adev->ualink.info)
@@ -1028,6 +1063,31 @@ static struct amdgpu_device *find_peer_adev(unsigned int accel_id)
 	return NULL;
 }
 
+/*
+ * Physical accelerator roles.
+ *
+ * Outside loopback a GPU owns a single accelerator, so both helpers return its
+ * ppod.accel_id and the exporter/importer distinction collapses. In loopback
+ * the one physical GPU plays both roles: the exporter owns the MP nHT and
+ * receives all HW traffic, while the importer is the (virtual) HW sender for
+ * the importer -> exporter path.
+ */
+static inline u32 ualink_exp_accel_id(struct amdgpu_device *adev)
+{
+	if (adev->ualink.loopback.enabled)
+		return adev->ualink.loopback.exporter_accel_id;
+
+	return adev->ualink.info->ppod.accel_id;
+}
+
+static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
+{
+	if (adev->ualink.loopback.enabled)
+		return adev->ualink.loopback.importer_accel_id;
+
+	return adev->ualink.info->ppod.accel_id;
+}
+
 static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
 					 u32 accel_id)
 {
@@ -1533,19 +1593,21 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_ualink_ppod_setup *ppod_setup;
 	struct amdgpu_device *adev = ip_block->adev;
 	struct amdgpu_ualink_info *info;
+	int r;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	ppod_setup = kzalloc(sizeof(*ppod_setup), GFP_KERNEL);
 	vpod_config = kzalloc(sizeof(*vpod_config), GFP_KERNEL);
 	stations = kzalloc(sizeof(*stations), GFP_KERNEL);
 	if (!info || !ppod_setup || !vpod_config || !stations) {
-		kfree(info);
-		kfree(ppod_setup);
-		kfree(vpod_config);
-		kfree(stations);
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto err_free;
 	}
 
+	r = amdgpu_ualink_loopback_sw_init(adev);
+	if (r)
+		goto err_free;
+
 	info->ppod.accel_id = 0xffffffff;
 	info->ppod.bandwidth = 0xffffffff;
 	info->ppod.latency = 0xffffffff;
@@ -1568,10 +1630,18 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
 	adev->ualink.stations = stations;
 
 	return 0;
+
+err_free:
+	kfree(info);
+	kfree(ppod_setup);
+	kfree(vpod_config);
+	kfree(stations);
+	return r;
 }
 
 int ualink_ip_sw_fini(struct amdgpu_ip_block *ip_block)
 {
+	amdgpu_ualink_loopback_fini(ip_block->adev);
 	amdgpu_ualink_object_fini(ip_block->adev);
 	return 0;
 }
@@ -4487,11 +4557,6 @@ static bool amdgpu_ualink_vpod_membership_changed(struct amdgpu_device *adev)
 			     AMDGPU_UALINK_ACCEL_MAX);
 }
 
-static inline u32 ualink_accel_id(struct amdgpu_device *adev)
-{
-	return adev->ualink.info->ppod.accel_id;
-}
-
 static inline enum amdgpu_ualink_addr_mode ualink_addr_mode(struct amdgpu_device *adev)
 {
 	return adev->ualink.info->vpod.addr_mode;
@@ -4837,7 +4902,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
 {
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
 	u64 timeout = msecs_to_jiffies(2000);
-	u32 dst_accel_id = ualink_accel_id(adev);
+	u32 dst_accel_id = ualink_exp_accel_id(adev);
 	struct amdgpu_bo *bos[2];
 	u32 n_bos;
 	struct dma_fence *fence;
@@ -4956,7 +5021,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
 static int amdgpu_ualink_metadata_npa_mapping(struct amdgpu_device *adev)
 {
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
-	u32 dst_accel_id = ualink_accel_id(adev);
+	u32 dst_accel_id = ualink_exp_accel_id(adev);
 	u64 timeout = msecs_to_jiffies(2000);
 	struct amdgpu_bo *bos[2];
 	u32 n_bos;
@@ -5241,7 +5306,7 @@ static void amdgpu_ualink_metadata_fini(struct amdgpu_device *adev)
 {
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
 
-	dev_dbg(adev->dev, "accel_id %u\n", ualink_accel_id(adev));
+	dev_dbg(adev->dev, "accel_id %u\n", ualink_exp_accel_id(adev));
 	amdgpu_bo_free_kernel(&remote->rptr_bo, &remote->rptr_gpu_addr,
 				      &remote->rptr_cpu_addr);
 	amdgpu_bo_free_kernel(&remote->ring_bo, &remote->rb_gpu_addr,
@@ -5288,7 +5353,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
 	}
 
 	dev_dbg(adev->dev, "accel_id %u addr_mode %d fw status 0x%x\n",
-		ualink_accel_id(adev), ualink_addr_mode(adev), status);
+		ualink_exp_accel_id(adev), ualink_addr_mode(adev), status);
 
 	/* Alloc metadata structure for all GPUs */
 	metadata_size = sizeof(struct amdgpu_ualink_metadata) * AMDGPU_UALINK_ACCEL_MAX;
@@ -5361,12 +5426,12 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
 	}
 
 	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS)
-		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_accel_id(adev);
+		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
 
 	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
 		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
 
-		if (accel_id == ualink_accel_id(adev))
+		if (accel_id == ualink_exp_accel_id(adev))
 			continue;
 
 		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
@@ -5392,7 +5457,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
 						     ualink_tlb_wptr_offset(adev, accel_id);
 
 		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
-			accel_id, ualink_accel_id(adev), metadata[accel_id].rb_size);
+			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
 		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
 			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
 		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
@@ -5806,7 +5871,14 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
 				      u64 *wb_npa, u32 type)
 {
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
-	u32 accel_id = ualink_accel_id(adev);
+	/* The GPUID encoded in the write-back NPA address is the routing target
+	 * the remote writes the rptr back to, i.e. THIS GPU. It must be the
+	 * physically-owned accel_id: outside loopback that is our own accel_id,
+	 * and in loopback it is the exporter (the only id the single GPU owns
+	 * and can receive on). The importer is only a logical source id for the
+	 * forward path and must never appear as an NPA GPUID.
+	 */
+	u32 accel_id = ualink_exp_accel_id(adev);
 	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
 	uintptr_t wb;
 	u64 npa;
@@ -5981,19 +6053,19 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
 						   &peer->shootdown, &tlb_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);
+			ualink_exp_accel_id(adev), remote_accel_id, r);
 		return r;
 	}
 
 	if (peer->interrupt.wptr == int_wptr && peer->shootdown.wptr == tlb_wptr) {
 		dev_dbg(adev->dev, "accel_id %u wptr 0x%llx 0x%llx is same as remote %u\n",
-			ualink_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
+			ualink_exp_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
 		return 0;
 	}
 
 	dev_dbg(adev->dev,
 		"accel_id %u wptr 0x%llx 0x%llx sync to remote %u wptr 0x%llx 0x%llx\n",
-		ualink_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
+		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
 		remote_accel_id, int_wptr, tlb_wptr);
 
 	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
@@ -6164,7 +6236,12 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
 	struct amdgpu_ualink_peer *peer;
 	struct amdgpu_ualink_ring *interrupt;
 	struct amdgpu_ualink_ring *shootdown;
-	u32 src_accel_id = ualink_accel_id(adev);
+	/* This sets up the send rings (local sender -> each remote dst). In
+	 * loopback the HW-served direction is importer -> exporter, so the
+	 * local sender is the importer; the skip below then skips the importer
+	 * and sets up the peer[exporter] ring that the HW path needs.
+	 */
+	u32 src_accel_id = ualink_imp_accel_id(adev);
 	u32 dst_accel_id;
 	u64 npa, flags;
 	int r = 0;
@@ -6287,7 +6364,10 @@ static void amdgpu_ualink_peer_remote_fini(struct amdgpu_device *adev)
 {
 	u32 rb_pages = AMDGPU_UALINK_RB_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
-	u32 src_accel_id = ualink_accel_id(adev);
+	/* Mirror peer_remote_init: in loopback the local sender is the
+	 * importer, so unmap the peer[exporter] send ring that was set up.
+	 */
+	u32 src_accel_id = ualink_imp_accel_id(adev);
 	u32 dst_accel_id;
 	struct amdgpu_ualink_peer *peer;
 	struct amdgpu_ualink_ring *ring;
@@ -6374,7 +6454,7 @@ int amdgpu_ualink_sw_init(struct amdgpu_device *adev)
  */
 void amdgpu_ualink_sw_fini(struct amdgpu_device *adev)
 {
-	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_accel_id(adev),
+	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_exp_accel_id(adev),
 		ualink_addr_mode(adev));
 
 	adev->ualink.msg_ctl->send_halt(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
index 85c9a9634a4b..0b1e8dd38666 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
@@ -97,6 +97,17 @@ enum AMDGPU_UALINK_PROTOCOL_MESSAGES {
 #define AMDGPU_UALINK_LIGHTWEIGHT_TLB_SHOOTDOWN		0x1
 #define AMDGPU_UALINK_HEAVYWEIGHT_TLB_SHOOTDOWN		0x2
 
+/* UALink loopback support.
+ *
+ * In loopback mode a single GPU loops UALink requests back to itself for
+ * manufacturing testing. The GPU exposes two fixed accelerator IDs: it acts
+ * as the exporter on one and the importer on the other. Loopback is enabled
+ * with the amdgpu.ualink_loopback module parameter.
+ */
+/* Fixed accelerator IDs used by a loopback GPU. */
+#define AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID		1
+#define AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID		0
+
 enum amdgpu_ualink_conn_state {
 	AMDGPU_UALINK_CONN_NOT_READY			= 0,
 	AMDGPU_UALINK_CONN_IN_PROGRESS			= 1,
@@ -319,6 +330,21 @@ struct amdgpu_ualink_msg_ctl {
 	int (*send_halt)(struct amdgpu_device *adev);
 };
 
+/* UALink loopback mode state.
+ *
+ * When @enabled is true this single GPU acts as both exporter
+ * (@exporter_accel_id) and importer (@importer_accel_id). There is no second
+ * physical GPU, so exporter->importer messages cannot be delivered by the
+ * (single) MP nHT and are instead emulated in SW by feeding them back into
+ * our own IRQ handler via @wq.
+ */
+struct amdgpu_ualink_loopback {
+	bool enabled;
+	u32 exporter_accel_id;
+	u32 importer_accel_id;
+	struct workqueue_struct *wq;
+};
+
 struct amdgpu_ualink_mgr {
 	u64 npa_size;
 	u32 psp_if_ver;
@@ -333,6 +359,9 @@ struct amdgpu_ualink_mgr {
 	/* For remote interrupt and shootdown */
 	struct amdgpu_ualink_remote *remote;
 
+	/* UALink loopback mode state (see struct amdgpu_ualink_loopback) */
+	struct amdgpu_ualink_loopback loopback;
+
 	/* handle irq from ualink client of remote GPUs */
 	struct amdgpu_irq_src irq;
 
-- 
2.56.0


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

* [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
  2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 22:07   ` Philip Yang
  2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx
  Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi,
	Felix Kuehling

The UPDATE_WB_ADDR remote command hands the MP nHT FW the address it
writes command completions back to. So far that has always been an NPA.

The FW can also take a VMID 0 GPUVA, selected by setting DW4 of the
command to 1. Return the VMID 0 GPUVA of the writeback slot from
amdgpu_ualink_get_wb_addr() alongside the NPA, and use it in
UALink loopback mode. There the HW only carries importer -> exporter
traffic, so the FW cannot write the completion back over NPA.

Outside loopback the NPA is still used and DW4 stays 0.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 48 +++++++++++++++-------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index a98d5bbf0622..29a787979af2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -5652,9 +5652,9 @@ static void amdgpu_ualink_emit_update_wb_addr(u32 **cpu_addr_p, u32 wb_data,
 
 	*cpu_addr++ = cmd;
 	*cpu_addr++ = wb_data;
-	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb_npa */
-	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb_npa */
-	*cpu_addr++ = 0;
+	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb address */
+	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb address */
+	*cpu_addr++ = dw2;		/* 1: wb address is VMID 0 GPUVA, 0: NPA */
 	*cpu_addr++ = 0;
 	*cpu_addr++ = 0; /* metadata */
 	*cpu_addr++ = 0; /* metadata */
@@ -5868,7 +5868,7 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
 static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
 				      u32 remote_accel_id,
 				      struct amdgpu_ualink_wb **wb_cpu,
-				      u64 *wb_npa, u32 type)
+				      u64 *wb_npa, u64 *wb_gpu, u32 type)
 {
 	struct amdgpu_ualink_remote *remote = to_remote(adev);
 	/* The GPUID encoded in the write-back NPA address is the routing target
@@ -5881,15 +5881,17 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
 	u32 accel_id = ualink_exp_accel_id(adev);
 	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
 	uintptr_t wb;
-	u64 npa;
+	u64 gpu, npa;
 	u32 offset = 0;
 
 	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
 		wb = (uintptr_t)remote->rb_cpu_addr + rb_size * remote->num_accel;
+		gpu = remote->rb_gpu_addr + rb_size * remote->num_accel;
 		npa = amdgpu_ualink_npa_addr(adev, RB_TYPE_TAILPTR,
 					     remote_accel_id, accel_id);
 	} else {
 		wb = (uintptr_t)remote->rptr_cpu_addr;
+		gpu = remote->rptr_gpu_addr;
 		npa = remote->rptr_npa;
 		npa |= (u64)accel_id << AMDGPU_UALINK_GART_NPA_ADDR_GPUID_SHIFT;
 	}
@@ -5902,9 +5904,11 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
 	*wb_cpu = (struct amdgpu_ualink_wb *)(wb + offset);
 	if (wb_npa)
 		*wb_npa = npa + offset;
+	if (wb_gpu)
+		*wb_gpu = gpu + offset;
 
-	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx\n", accel_id,
-		remote_accel_id, npa + offset);
+	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx gpu 0x%llx\n", accel_id,
+		remote_accel_id, npa + offset, gpu + offset);
 }
 
 struct amdgpu_ualink_wptr_fence_cb {
@@ -6068,9 +6072,9 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
 		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
 		remote_accel_id, int_wptr, tlb_wptr);
 
-	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
+	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL, NULL,
 				  RB_TYPE_REMOTE_INTERRUPT);
-	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL,
+	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL, NULL,
 				  RB_TYPE_TLB_INV);
 
 	peer->interrupt.wptr = int_wptr;
@@ -6104,7 +6108,8 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
 	struct amdgpu_ualink_peer *peer;
 	struct amdgpu_ualink_ring *ring;
 	struct amdgpu_ualink_wb *wb_cpu;
-	u64 wb_npa;
+	u64 wb_npa, wb_gpu, wb_addr;
+	u32 wb_vmid0;
 	int r;
 
 	peer = &remote->peer[remote_accel_id];
@@ -6116,13 +6121,28 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
 		return -EINVAL;
 
 	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &wb_cpu, &wb_npa,
-				  ring_type);
+				  &wb_gpu, ring_type);
+
+	/* UALink loopback: HW only carries importer -> exporter traffic, so the
+	 * MPNHT FW cannot write the completion back over NPA. Hand it the VMID 0
+	 * GPUVA of the writeback instead and flag it in DW4 of the command.
+	 */
+	if (adev->ualink.loopback.enabled) {
+		wb_addr = wb_gpu;
+		wb_vmid0 = 1;
+	} else {
+		wb_addr = wb_npa;
+		wb_vmid0 = 0;
+	}
+
+	dev_dbg(adev->dev, "remote %u ring type %u wb %s 0x%llx\n",
+		remote_accel_id, ring_type, wb_vmid0 ? "gpuva" : "npa", wb_addr);
 
 	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, wb_cpu,
 				       amdgpu_ualink_emit_update_wb_addr,
-				       upper_32_bits(wb_npa),
-				       lower_32_bits(wb_npa),
-				       0, 0);
+				       upper_32_bits(wb_addr),
+				       lower_32_bits(wb_addr),
+				       wb_vmid0, 0);
 	if (!r) {
 		ring->wb_cpu = wb_cpu;
 		ring->ready = true;
-- 
2.56.0


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

* [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
  2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
  2026-11-02 15:47 ` [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 22:30   ` Philip Yang
                     ` (2 more replies)
  2026-11-02 15:47 ` [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender Mukul Joshi
                   ` (4 subsequent siblings)
  7 siblings, 3 replies; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi

In UALink loopback mode a single GPU acts as both the exporter and the
importer. The exporter is the one physical station, so the only path
the HW serves is importer -> exporter. A message the exporter sends to
the importer cannot be delivered by the MP nHT.

Emulate that direction in SW: when loopback is enabled and a remote
interrupt is addressed to the importer, queue it on the loopback
ordered workqueue, which feeds it back into amdgpu_ualink_process_irq()
as if the exporter had sent it. The ordered workqueue keeps successive
messages in order and runs the (possibly sleeping) handlers outside
the sender's context.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 82 ++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 29a787979af2..ef133c60bb9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -60,6 +60,9 @@ static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
 					   u32 remote_accel_id);
 static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
 					  u32 remote_accel_id);
+static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
+						 u32 sender_accel_id, u32 dw0,
+						 u32 dw1, u32 dw2, u32 dw3);
 
 #define STRIP_NPA(addr)						\
 	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
@@ -6220,6 +6223,17 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
 	struct amdgpu_ualink_ring *ring;
 	int r;
 
+	/* UALink loopback: the single MP nHT can only deliver messages addressed
+	 * to the exporter accelerator. Messages addressed to the importer
+	 * (exporter->importer direction) cannot be delivered by HW, so emulate
+	 * them by feeding the message back into our own IRQ handler.
+	 */
+	if (adev->ualink.loopback.enabled &&
+	    remote_accel_id == ualink_imp_accel_id(adev))
+		return amdgpu_ualink_loopback_self_interrupt(adev,
+				ualink_exp_accel_id(adev),
+				dw0, dw1, dw2, dw3);
+
 	peer = &remote->peer[remote_accel_id];
 	ring = &peer->interrupt;
 
@@ -6649,6 +6663,74 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
 	return handled;
 }
 
+/* Work item used to emulate an exporter->importer UALink interrupt in
+ * loopback mode (see amdgpu_ualink_loopback_self_interrupt()).
+ */
+struct amdgpu_ualink_self_irq_work {
+	struct work_struct work;
+	struct amdgpu_device *adev;
+	u32 sender_accel_id;
+	u32 dw[4];
+};
+
+static void amdgpu_ualink_self_irq_worker(struct work_struct *w)
+{
+	struct amdgpu_ualink_self_irq_work *job =
+		container_of(w, struct amdgpu_ualink_self_irq_work, work);
+	struct amdgpu_device *adev = job->adev;
+	struct amdgpu_iv_entry entry = { };
+
+	/* Build an IH entry as if the message had been delivered by the MP nHT
+	 * from the exporter accelerator. amdgpu_ualink_process_irq() consumes
+	 * the sender accel_id from entry->pasid and the four context dwords
+	 * from entry->src_data[]. entry->ih must differ from adev->irq.ih so
+	 * the message is processed here instead of being delegated to the
+	 * UALink soft ring.
+	 */
+	entry.ih = NULL;
+	entry.pasid = job->sender_accel_id;
+	entry.src_data[0] = job->dw[0];
+	entry.src_data[1] = job->dw[1];
+	entry.src_data[2] = job->dw[2];
+	entry.src_data[3] = job->dw[3];
+
+	amdgpu_ualink_process_irq(adev, &adev->ualink.irq, &entry);
+	kfree(job);
+}
+
+/*
+ * amdgpu_ualink_loopback_self_interrupt - deliver a UALink message to ourselves
+ *
+ * Used in loopback mode for the exporter->importer direction, where there is
+ * no HW path. The message is processed asynchronously on an ordered workqueue
+ * so ordering between successive messages is preserved and the (potentially
+ * sleeping) message handlers do not run from the caller's context.
+ */
+static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
+						 u32 sender_accel_id, u32 dw0,
+						 u32 dw1, u32 dw2, u32 dw3)
+{
+	struct amdgpu_ualink_self_irq_work *job;
+
+	if (!adev->ualink.loopback.wq)
+		return -ENODEV;
+
+	job = kzalloc(sizeof(*job), GFP_KERNEL);
+	if (!job)
+		return -ENOMEM;
+
+	job->adev = adev;
+	job->sender_accel_id = sender_accel_id;
+	job->dw[0] = dw0;
+	job->dw[1] = dw1;
+	job->dw[2] = dw2;
+	job->dw[3] = dw3;
+	INIT_WORK(&job->work, amdgpu_ualink_self_irq_worker);
+	queue_work(adev->ualink.loopback.wq, &job->work);
+
+	return 0;
+}
+
 static int amdgpu_ualink_set_irq_state(struct amdgpu_device *adev,
 					struct amdgpu_irq_src *source,
 					u32 type,
-- 
2.56.0


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

* [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
                   ` (2 preceding siblings ...)
  2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 23:04   ` Felix Kuehling
  2026-11-02 15:47 ` [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback Mukul Joshi
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx
  Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi,
	Felix Kuehling

In UALink loopback mode a single GPU acts as both the exporter and the
importer, so the sender accel ID of a message depends on its direction.

- Send HELLO as the opposite role of the destination: a message to the
  exporter is sent as the importer and vice versa. Outside loopback
  this is still the GPU's own accel ID.

- The MP nHT stamps the IH cookie of every HW-delivered message with
  the GPU ID of the single physical station, the exporter. The only HW
  path is importer -> exporter, so attribute these messages to the
  importer. SW-emulated exporter -> importer messages carry no IH ring
  and already name the exporter as the sender.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 32 +++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index ef133c60bb9a..8558dc1a9e84 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -1091,6 +1091,23 @@ static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
 	return adev->ualink.info->ppod.accel_id;
 }
 
+/*
+ * amdgpu_ualink_local_sender_accel_id - sender accel_id for an outgoing message
+ *
+ * Normally a GPU sends messages as its own (single) accelerator. In loopback
+ * mode the same GPU plays both roles, so the sender identity depends on the
+ * destination: a message to the exporter is sent as the importer and vice
+ * versa. Outside loopback both roles collapse to the GPU's own accel_id.
+ */
+static u32 amdgpu_ualink_local_sender_accel_id(struct amdgpu_device *adev,
+					       u32 remote_acc_id)
+{
+	if (remote_acc_id == ualink_exp_accel_id(adev))
+		return ualink_imp_accel_id(adev);
+
+	return ualink_exp_accel_id(adev);
+}
+
 static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
 					 u32 accel_id)
 {
@@ -2249,7 +2266,10 @@ static int amdgpu_ualink_send_hello_msg(struct amdgpu_device *adev,
 
 	dw0 = AMDGPU_UALINK_HELLO_MSG;
 	dw0 |= (remote_acc_id << AMDGPU_UALINK_HELLO_MSG_RECV_ACCID_SHIFT);
-	dw0 |= (adev->ualink.info->ppod.accel_id <<
+	/* In loopback the sender is the opposite role of the destination, so
+	 * the exporter side tracks the connection under the importer accel_id.
+	 */
+	dw0 |= (amdgpu_ualink_local_sender_accel_id(adev, remote_acc_id) <<
 		AMDGPU_UALINK_HELLO_MSG_SENDER_ACCID_SHIFT);
 
 	dev_dbg(adev->dev, "SEND HELLO: HELLO message to remote AccId:%u\n",
@@ -6567,6 +6587,16 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
 		return handled;
 	}
 
+	/* UALink loopback: the MP nHT stamps every HW-delivered message with
+	 * the GPU ID of the single physical station, the exporter. The only
+	 * HW path is importer -> exporter, so the real sender is the importer.
+	 * SW-emulated exporter -> importer messages carry no IH ring and
+	 * already name the exporter as sender.
+	 */
+	if (adev->ualink.loopback.enabled && entry->ih &&
+	    src_acc_id == ualink_exp_accel_id(adev))
+		src_acc_id = ualink_imp_accel_id(adev);
+
 	msg_type = dw0 & AMDGPU_UALINK_MESSAGE_HEADER_MASK;
 	local_acc_id = adev->ualink.info->ppod.accel_id;
 	dev_dbg(adev->dev, "Got MSG: remote acc_id %u msg_type %u\n",
-- 
2.56.0


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

* [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
                   ` (3 preceding siblings ...)
  2026-11-02 15:47 ` [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 23:15   ` Felix Kuehling
  2026-11-02 15:47 ` [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode Mukul Joshi
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi

In UALink loopback mode a single GPU acts as both the exporter and the
importer, and the only path the HW serves is importer -> exporter.
Adjust the per-peer setup accordingly:

- The importer's doorbell reaches the MP nHT stamped with the GPU ID of
  the exporter, and the FW looks up the metadata entry by that ID. So
  publish the importer's rings and tail pointers in metadata[exporter].

- Skip the ring pointer resync. Both ends of every ring are on this
  GPU, so there is no remote peer that can reboot and leave the
  pointers stale.

- Do not take the local dma-buf shortcut on import. It would bypass
  the NPA path that loopback is meant to exercise.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
v2->v3:
- Move updating src_accel_id in peer_remote_init and fini to patch 1.

 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 54 ++++++++++++++++------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 8558dc1a9e84..74bc8a907f5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -3976,7 +3976,13 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	if (amdgpu_ualink_is_local_accel(adev, remote_acc_id))
+	/* In loopback mode the exporter and importer are the same GPU, but the
+	 * import must still go through the full cross-domain NPA path (the
+	 * dma_buf shortcut in amdgpu_ualink_local_import() bypasses NPA). Only
+	 * take the local shortcut for genuine same-pod local accelerators.
+	 */
+	if (!adev->ualink.loopback.enabled &&
+	    amdgpu_ualink_is_local_accel(adev, remote_acc_id))
 		return amdgpu_ualink_local_import(adev, remote_acc_id,
 						  &handle, fd_out);
 
@@ -5452,11 +5458,28 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
 		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
 
 	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
+		struct amdgpu_ualink_metadata *md;
+
 		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
 
 		if (accel_id == ualink_exp_accel_id(adev))
 			continue;
 
+		/* In loopback the single MP nHT services the exporter half:
+		 * the only messages delivered over HW are importer -> exporter
+		 * (source = importer accel_id), while the exporter -> importer
+		 * direction is emulated in SW and never reaches the MP nHT.
+		 *
+		 * The importer's doorbell reaches the MP nHT stamped with the
+		 * GPU ID of the single physical station, the exporter, so the
+		 * FW looks up metadata[exporter]. Publish the importer's rings
+		 * and tail pointers in that entry.
+		 */
+		if (adev->ualink.loopback.enabled)
+			md = &metadata[ualink_exp_accel_id(adev)];
+		else
+			md = &metadata[accel_id];
+
 		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
 
 		if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
@@ -5464,27 +5487,26 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
 			wptr_gpu_addr += wptr_size * accel_id;
 		}
 
-		metadata[accel_id].header = AMDGPU_UALINK_METADATA_HEADER;
-		metadata[accel_id].rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
+		md->header = AMDGPU_UALINK_METADATA_HEADER;
+		md->rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
 
 		/*
 		 * remote shootdown type is 0, remote command type is 1
 		 * with 2MB ring buffer mapping, remote shootdown ring NPA is before interrupt
 		 */
-		metadata[accel_id].ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
-		metadata[accel_id].tlb_inv_rb = rb_gpu_addr;
+		md->ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
+		md->tlb_inv_rb = rb_gpu_addr;
 
-		metadata[accel_id].tailptr_ri = wptr_gpu_addr +
-						ualink_wptr_offset(adev, accel_id);
-		metadata[accel_id].tailptr_tlb_inv = wptr_gpu_addr +
-						     ualink_tlb_wptr_offset(adev, accel_id);
+		md->tailptr_ri = wptr_gpu_addr + ualink_wptr_offset(adev, accel_id);
+		md->tailptr_tlb_inv = wptr_gpu_addr +
+				      ualink_tlb_wptr_offset(adev, accel_id);
 
-		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
-			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
+		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, metadata entry %td, rb_size 0x%x\n",
+			accel_id, ualink_exp_accel_id(adev), md - metadata, md->rb_size);
 		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
-			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
+			md->ri_rb, md->tlb_inv_rb);
 		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
-			metadata[accel_id].tailptr_ri, metadata[accel_id].tailptr_tlb_inv);
+			md->tailptr_ri, md->tailptr_tlb_inv);
 	}
 
 	r = adev->ualink.msg_ctl->send_metadata(adev,
@@ -6069,6 +6091,12 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
 	u64 int_wptr, tlb_wptr;
 	int r;
 
+	/* UALink loopback: both ends of every ring live on this GPU, so there
+	 * is no remote peer that can reboot and leave the pointers stale.
+	 */
+	if (adev->ualink.loopback.enabled)
+		return 0;
+
 	/* ring->wptr and ring->rptr are owned by the send path. */
 	guard(mutex)(&peer->lock);
 
-- 
2.56.0


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

* [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
                   ` (4 preceding siblings ...)
  2026-11-02 15:47 ` [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 23:29   ` Felix Kuehling
  2026-11-02 15:47 ` [PATCHv3 7/8] drm/amdgpu: Reject config updates in " Mukul Joshi
  2026-11-02 15:47 ` [PATCHv3 8/8] drm/amdgpu: Temporarily enable UALink loopback via module parameter Mukul Joshi
  7 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx
  Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi,
	Felix Kuehling

In UALink loopback mode a single GPU acts as both the exporter (accel
ID 1) and the importer (accel ID 0). This lets the existing tests export
a BO and import it on the same device, traversing the UALink HW
pipeline.

Add amdgpu_ualink_loopback_detect(), the single place that decides
whether loopback is enabled. It runs from sw_init, before any per-peer
setup, so the loopback workqueue is allocated with the other one-time
allocations. When loopback is enabled:

- There is no user-space agent to configure the pod, so synthesize the
  pPod/vPod configuration with both accel IDs active instead of
  querying it from PSP. Loopback only supports the source-aliasing
  address mode.

- Activate the device directly instead of through the cross-GPU vpod
  activation path, which needs peer GPUs and a user-space vpod commit.

The mechanism that requests loopback mode is added separately, so
amdgpu_ualink_loopback_detect() leaves loopback disabled for now. The
normal activation path is unchanged when loopback is not enabled.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com> (v2)
---
v2->v3:
- Update all static info inside amdgpu_ualink_loopback_populate_info().
- Drop the amdgpu_ualink_loopback_requested() as amdgpu_ualink_loopback_detect()
  is enough to detect and set loopback mode.

 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 143 +++++++++++++++++++--
 1 file changed, 132 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 74bc8a907f5b..2e1e0a677e35 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -63,6 +63,7 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
 static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
 						 u32 sender_accel_id, u32 dw0,
 						 u32 dw1, u32 dw2, u32 dw3);
+static void activate_accelerator(struct amdgpu_device *adev);
 
 #define STRIP_NPA(addr)						\
 	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
@@ -93,13 +94,120 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
 }
 
 /*
- * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
+ * amdgpu_ualink_loopback_detect - detect UALink loopback mode
  *
- * Allocates the ordered workqueue used to emulate exporter->importer
- * interrupts in SW. Does nothing unless loopback mode is enabled.
+ * This is the only place that decides whether loopback mode is enabled.
+ * There is no mechanism to request loopback mode yet, so it stays disabled.
+ */
+static void amdgpu_ualink_loopback_detect(struct amdgpu_device *adev)
+{
+	adev->ualink.loopback.enabled = false;
+}
+
+/*
+ * amdgpu_ualink_loopback_populate_info - synthesize pPod/vPod config for loopback
+ *
+ * In loopback there is no user-space agent (AIFM / xncmdclient) writing the
+ * UALink sysfs nodes to configure the pod, so populate adev->ualink.info
+ * directly. The layout is fixed and known: a single GPU alone in its own
+ * physical pod, presented as accelerator ID 1 (the exporter) and also owning
+ * accelerator ID 0 (the importer). The values are made self-consistent so
+ * check_vpod_info() would pass and amdgpu_ualink_metadata_init() programs the
+ * importer's ring buffers / GART NPA mappings / MP nHT receive metadata.
+ *
+ * This must run before any per-peer setup, since it sets the accelerator IDs
+ * returned by ualink_exp_accel_id() and ualink_imp_accel_id().
+ */
+static void amdgpu_ualink_loopback_populate_info(struct amdgpu_device *adev)
+{
+	struct amdgpu_ualink_info *info = adev->ualink.info;
+	u32 exporter = AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID;
+	u32 importer = AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID;
+
+	adev->ualink.loopback.exporter_accel_id = exporter;
+	adev->ualink.loopback.importer_accel_id = importer;
+
+	/* Physical pod: this GPU is presented as the exporter accel ID in its
+	 * own pod. ppod.size must be large enough to contain both the exporter
+	 * and importer accel IDs. ppod.id must be unique within the system
+	 * (matters for 1P4G, where every loopback GPU is its own physical pod).
+	 */
+	info->ppod.accel_id = exporter;
+	info->ppod.size = max(exporter, importer) + 1;	/* == 2 */
+	uuid_gen(&info->ppod.id);
+
+	/* Virtual pod: mark BOTH the exporter and the importer active so the
+	 * per-peer setup (amdgpu_ualink_metadata_init()) programs the importer's
+	 * resources for the importer accel ID. Loopback only supports the
+	 * source-aliasing address mode.
+	 */
+	info->vpod.id = 0;
+	info->vpod.size = 2;
+	info->vpod.addr_mode = AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS;
+	bitmap_zero(info->vpod.active_accel_bits, AMDGPU_UALINK_ACCEL_MAX);
+	set_bit(exporter, info->vpod.active_accel_bits);
+	set_bit(importer, info->vpod.active_accel_bits);
+
+	/* Only the exporter (this GPU's own accel ID) is a local accelerator;
+	 * the importer is reached through the (loopback) remote/NPA path.
+	 */
+	info->n_local_accels = 1;
+	info->local_accels[0] = exporter;
+
+	/* Mark the pod vpod-configured so the activation path proceeds. */
+	info->accel_state = AMDGPU_UALINK_ACCEL_STATE_VPOD_CONFIGURED;
+}
+
+/*
+ * amdgpu_ualink_loopback_activate - bring up the UALink manager for loopback
+ *
+ * The normal activation is driven by the sysfs vpod_config/commit path via
+ * activate_local_vpod(), which derives the local accelerators from peer GPUs
+ * and runs a cross-GPU integrity check. Neither applies in loopback (the
+ * importer is not a separate GPU), so activate this device directly.
+ */
+static int amdgpu_ualink_loopback_activate(struct amdgpu_device *adev)
+{
+	mutex_lock(&mgpu_info.mutex);
+	activate_accelerator(adev);
+	mutex_unlock(&mgpu_info.mutex);
+
+	if (adev->ualink.info->accel_state < AMDGPU_UALINK_ACCEL_STATE_ACTIVE) {
+		dev_err(adev->dev, "UALink loopback activation did not reach ACTIVE\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/*
+ * amdgpu_ualink_loopback_init - set up and activate UALink loopback mode
+ *
+ * Called from late init once loopback mode has been detected. Synthesizes the
+ * pod configuration instead of querying it from PSP (there is no user-space
+ * agent to configure the pod in loopback) and activates the device.
+ */
+static int amdgpu_ualink_loopback_init(struct amdgpu_device *adev)
+{
+	amdgpu_ualink_loopback_populate_info(adev);
+
+	dev_info(adev->dev,
+		 "UALink loopback enabled (exporter accel %u, importer accel %u)\n",
+		 adev->ualink.loopback.exporter_accel_id,
+		 adev->ualink.loopback.importer_accel_id);
+
+	return amdgpu_ualink_loopback_activate(adev);
+}
+
+/*
+ * amdgpu_ualink_loopback_sw_init - detect loopback and allocate its resources
+ *
+ * Detects loopback mode and, if it is enabled, allocates the ordered
+ * workqueue used to emulate exporter->importer interrupts in SW.
  */
 static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
 {
+	amdgpu_ualink_loopback_detect(adev);
 	if (!adev->ualink.loopback.enabled)
 		return 0;
 
@@ -499,15 +607,28 @@ int ualink_ip_late_init(struct amdgpu_ip_block *ip_block)
 	if (adev->ualink.mgr_state != AMDGPU_UALINK_INIT_HW)
 		return 0;
 
-	r = psp_ual_query_info(&adev->psp, adev->ualink.psp_if_ver,
-			       adev->ualink.info, &cfg_state);
-	if (r)
-		return r;
+	/* Loopback synthesizes its pod configuration instead of querying it
+	 * from PSP, and has no peer GPUs or user-space vpod commit, so it
+	 * cannot use the normal cross-GPU activation path either.
+	 */
+	if (!adev->ualink.loopback.enabled) {
+		r = psp_ual_query_info(&adev->psp, adev->ualink.psp_if_ver,
+				       adev->ualink.info, &cfg_state);
+		if (r)
+			return r;
 
-	scoped_guard(mutex, &mgpu_info.mutex) {
-		amdgpu_ualink_info_set_accel_state(adev, adev->ualink.info,
-						   cfg_state);
-		__amdgpu_ualink_activate_vpod_locked(adev);
+		scoped_guard(mutex, &mgpu_info.mutex) {
+			amdgpu_ualink_info_set_accel_state(adev,
+							   adev->ualink.info,
+							   cfg_state);
+			__amdgpu_ualink_activate_vpod_locked(adev);
+		}
+	} else {
+		r = amdgpu_ualink_loopback_init(adev);
+		if (r) {
+			dev_err(adev->dev, "Failed to set up UALink loopback: %d\n", r);
+			goto error;
+		}
 	}
 
 	r = amdgpu_ualink_drm_client_create(adev);
-- 
2.56.0


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

* [PATCHv3 7/8] drm/amdgpu: Reject config updates in UALink loopback mode
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
                   ` (5 preceding siblings ...)
  2026-11-02 15:47 ` [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  2026-11-03 23:17   ` Felix Kuehling
  2026-11-02 15:47 ` [PATCHv3 8/8] drm/amdgpu: Temporarily enable UALink loopback via module parameter Mukul Joshi
  7 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi

In loopback mode the driver synthesizes the pPod/vPod configuration,
since there is no user-space agent to configure the pod. Keep anything
else from overwriting it:

- Ignore PSP config update interrupts. Re-querying the configuration
  from PSP would replace the synthesized one. Still send the completion
  so PSP is not left waiting.

- Don't create the pPod setup, vPod config and station config sysfs
  nodes, so the configuration can't be committed from user space. The
  setup, config and stations directories are still created, empty, as
  they are when the config sysfs nodes are compiled out.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
v1->v2:
- Do not create the config directory in sysfs under loopback mode.(Lijo).

v2->v3:
- Update amdgpu_ualink_sysfs_init() based on Felix's suggestion.

 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 68 +++++++++++++---------
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 2e1e0a677e35..cf2c68cf7115 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -491,6 +491,18 @@ int amdgpu_ualink_config_update_handler(struct amdgpu_device *adev)
 			PSP_GFX_INT_CTXT_UAL_CMD_CFG_UPDATE_ID, status);
 	}
 
+	/* The loopback pod configuration is synthesized by the driver, so
+	 * re-querying it from PSP would overwrite it.
+	 */
+	if (adev->ualink.loopback.enabled) {
+		dev_warn(adev->dev,
+			 "UALINK: config update ignored in loopback mode\n");
+		return psp_ual_send_completion(&adev->psp,
+					       adev->ualink.psp_if_ver,
+					       PSP_GFX_INT_CTXT_UAL_CMD_CFG_UPDATE_ID,
+					       0);
+	}
+
 	prev_state = adev->ualink.info->accel_state;
 	prev_vpod_id = adev->ualink.info->vpod.id;
 
@@ -1820,36 +1832,40 @@ int amdgpu_ualink_sysfs_init(struct amdgpu_device *adev)
 		goto err;
 
 #ifdef UALINK_ENABLE_DEPRECATED_CONFIG_SYSFS
-	r = ualink_kobj_add(&adev->ualink.setup->kobj, &info->kobj, "setup",
-			    ualink_ppod_setup_attrs);
-	if (r)
-		goto err_info;
+	/* The loopback pod configuration is synthesized by the driver. */
+	if (!adev->ualink.loopback.enabled) {
+		r = ualink_kobj_add(&adev->ualink.setup->kobj, &info->kobj,
+				    "setup", ualink_ppod_setup_attrs);
+		if (r)
+			goto err_info;
 
-	r = ualink_kobj_add(&adev->ualink.config->kobj, &info->kobj, "config",
-			    ualink_vpod_config_attrs);
-	if (r)
-		goto err_setup;
+		r = ualink_kobj_add(&adev->ualink.config->kobj, &info->kobj,
+				    "config", ualink_vpod_config_attrs);
+		if (r)
+			goto err_setup;
 
-	r = ualink_kobj_add(&adev->ualink.stations->kobj, &info->kobj,
-			    "stations", ualink_station_config_attrs);
-	if (r)
-		goto err_config;
-#else
-	r = kobject_add(&adev->ualink.setup->kobj, &info->kobj, "%s",
-			"setup");
-	if (r)
-		goto err_info;
+		r = ualink_kobj_add(&adev->ualink.stations->kobj, &info->kobj,
+				    "stations", ualink_station_config_attrs);
+		if (r)
+			goto err_config;
+	} else
+#endif
+	{
+		r = kobject_add(&adev->ualink.setup->kobj, &info->kobj, "%s",
+				"setup");
+		if (r)
+			goto err_info;
 
-	r = kobject_add(&adev->ualink.config->kobj, &info->kobj, "%s",
-			"config");
-	if (r)
-		goto err_setup;
+		r = kobject_add(&adev->ualink.config->kobj, &info->kobj, "%s",
+				"config");
+		if (r)
+			goto err_setup;
 
-	r = kobject_add(&adev->ualink.stations->kobj, &info->kobj, "%s",
-			"stations");
-	if (r)
-		goto err_config;
-#endif
+		r = kobject_add(&adev->ualink.stations->kobj, &info->kobj, "%s",
+				"stations");
+		if (r)
+			goto err_config;
+	}
 
 	amdgpu_ualink_xcp_sysfs_init(adev);
 	adev->ualink.sysfs_init = true;
-- 
2.56.0


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

* [PATCHv3 8/8] drm/amdgpu: Temporarily enable UALink loopback via module parameter
  2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
                   ` (6 preceding siblings ...)
  2026-11-02 15:47 ` [PATCHv3 7/8] drm/amdgpu: Reject config updates in " Mukul Joshi
@ 2026-11-02 15:47 ` Mukul Joshi
  7 siblings, 0 replies; 24+ messages in thread
From: Mukul Joshi @ 2026-11-02 15:47 UTC (permalink / raw)
  To: amd-gfx
  Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang, Mukul Joshi,
	Felix Kuehling

Temporarily add the amdgpu.ualink_loopback module parameter to request
UALink loopback mode for manufacturing testing, until the final
mechanism for enabling loopback is in place. Loopback is disabled by
default.

Only amdgpu_ualink_loopback_detect() reads the parameter, so moving to
the final mechanism only needs to replace it there.

Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
Acked-by: Felix Kuehling <felix.kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 13 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c |  5 +++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5a32b54b1e9b..b8be093b8a33 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -270,6 +270,7 @@ extern int amdgpu_rebar;
 extern int amdgpu_wbrf;
 extern int amdgpu_user_queue;
 extern int amdgpu_ptl;
+extern int amdgpu_ualink_loopback;
 
 extern uint amdgpu_hdmi_hpd_debounce_delay_ms;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5e0029530b79..b4aca2def27a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -253,6 +253,7 @@ int amdgpu_rebar = -1; /* auto */
 int amdgpu_user_queue = -1;
 uint amdgpu_hdmi_hpd_debounce_delay_ms;
 int amdgpu_ptl = -1; /* auto */
+int amdgpu_ualink_loopback;
 
 DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
 			"DRM_UT_CORE",
@@ -1165,6 +1166,18 @@ module_param_named(hdmi_hpd_debounce_delay_ms, amdgpu_hdmi_hpd_debounce_delay_ms
 MODULE_PARM_DESC(ptl, "Enable PTL (-1 = auto, 0 = disable (default), 1 = enable, 2 = permanently disable)");
 module_param_named(ptl, amdgpu_ptl, int, 0444);
 
+/**
+ * DOC: ualink_loopback (int)
+ * Enable UALink loopback mode for manufacturing testing. In loopback mode a
+ * single GPU acts as both the exporter (accelerator ID 1) and the importer
+ * (accelerator ID 0) and loops UALink requests back to itself. Possible values:
+ *
+ * - 0 = disable (default)
+ * - 1 = enable
+ */
+MODULE_PARM_DESC(ualink_loopback, "Enable UALink loopback mode (0 = disable (default), 1 = enable)");
+module_param_named(ualink_loopback, amdgpu_ualink_loopback, int, 0444);
+
 /* These devices are not supported by amdgpu.
  * They are supported by the mach64, r128, radeon drivers
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index cf2c68cf7115..410222354bd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -97,11 +97,12 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
  * amdgpu_ualink_loopback_detect - detect UALink loopback mode
  *
  * This is the only place that decides whether loopback mode is enabled.
- * There is no mechanism to request loopback mode yet, so it stays disabled.
+ * Loopback mode is temporarily requested with the amdgpu.ualink_loopback
+ * module parameter.
  */
 static void amdgpu_ualink_loopback_detect(struct amdgpu_device *adev)
 {
-	adev->ualink.loopback.enabled = false;
+	adev->ualink.loopback.enabled = amdgpu_ualink_loopback == 1;
 }
 
 /*
-- 
2.56.0


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

* Re: [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure
  2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
@ 2026-11-03 18:17   ` Philip Yang
  2026-11-03 22:32     ` Felix Kuehling
  2026-11-03 23:18   ` Felix Kuehling
  1 sibling, 1 reply; 24+ messages in thread
From: Philip Yang @ 2026-11-03 18:17 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 15041 bytes --]



On 2026-11-02 10:47, Mukul Joshi wrote:
> Add the infrastructure for UALink loopback mode, where a single GPU
> plays both the exporter and the importer role under two fixed
> accelerator IDs:
>
> - Add struct amdgpu_ualink_loopback to track the loopback state, along
>    with the fixed exporter and importer accel IDs.
>
> - Replace ualink_accel_id() with ualink_exp_accel_id() and
>    ualink_imp_accel_id(), which return the exporter and importer accel
>    ID in loopback mode.
>
> - Add amdgpu_ualink_loopback_sw_init(), which allocates the ordered
>    workqueue used to emulate exporter->importer messages from
>    ualink_ip_sw_init(), and amdgpu_ualink_loopback_fini(), which
>    destroys it from ualink_ip_sw_fini().
>
> Loopback is not enabled yet, and outside loopback both helpers return
> ppod.accel_id, so there is no functional change.
>
> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
> ---
> v1->v2:
> - Drop the change in find_peer_adev() as its not needed.
>
> v2->v3:
> - Update src_accel_id to the correct value in amdgpu_ualink_peer_remote_init()
>    and amdgpu_ualink_peer_remote_fini().
> - Add the loopback teardown in amdgpu_ualink_loopback_fini() and call it
>    through ualink_ip_sw_fini().
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 128 +++++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h |  29 +++++
>   2 files changed, 133 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index ca6432610f71..a98d5bbf0622 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -89,6 +89,41 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +/*
> + * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
> + *
> + * Allocates the ordered workqueue used to emulate exporter->importer
> + * interrupts in SW. Does nothing unless loopback mode is enabled.
> + */
> +static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
> +{
> +	if (!adev->ualink.loopback.enabled)
> +		return 0;
> +
> +	adev->ualink.loopback.wq =
> +		alloc_ordered_workqueue("amdgpu_ualink_loopback", 0);
> +	if (!adev->ualink.loopback.wq)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_fini - tear down UALink loopback mode
> + *
> + * Destroys the workqueue allocated in amdgpu_ualink_loopback_sw_init().
> + * destroy_workqueue() drains any pending self-interrupt work first.
> + */
> +static void amdgpu_ualink_loopback_fini(struct amdgpu_device *adev)
> +{
> +	if (!adev->ualink.loopback.wq)
> +		return;
> +
> +	destroy_workqueue(adev->ualink.loopback.wq);
> +	adev->ualink.loopback.wq = NULL;
> +	adev->ualink.loopback.enabled = false;
> +}
> +
>   static void amdgpu_ualink_object_fini(struct amdgpu_device *adev)
>   {
>   	if (!adev->ualink.info)
> @@ -1028,6 +1063,31 @@ static struct amdgpu_device *find_peer_adev(unsigned int accel_id)
>   	return NULL;
>   }
>   
> +/*
> + * Physical accelerator roles.
> + *
> + * Outside loopback a GPU owns a single accelerator, so both helpers return its
> + * ppod.accel_id and the exporter/importer distinction collapses. In loopback
> + * the one physical GPU plays both roles: the exporter owns the MP nHT and
> + * receives all HW traffic, while the importer is the (virtual) HW sender for
> + * the importer -> exporter path.
> + */
> +static inline u32 ualink_exp_accel_id(struct amdgpu_device *adev)
> +{
> +	if (adev->ualink.loopback.enabled)
> +		return adev->ualink.loopback.exporter_accel_id;
> +
> +	return adev->ualink.info->ppod.accel_id;
> +}
> +
Replacing ualink_accel_id with ualink_exp_accel_id is misleading for the 
non-loopback case, since each device is both an exporter and an importer 
at the same time. I'd prefer to add the loopback-enabled check inside 
ualink_accel_id() instead.

It also looks like ualink_imp_accel_id is only used by 
amdgpu_ualink_process_irq and amdgpu_ualink_remote_interrupt under the 
loopback-enabled condition, so it could be removed as well. That would 
simplify the patch and drop all the ualink_accel_id renames.

> +static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
> +{
> +	if (adev->ualink.loopback.enabled)
> +		return adev->ualink.loopback.importer_accel_id;
> +
> +	return adev->ualink.info->ppod.accel_id;
> +}
> +
>   static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
>   					 u32 accel_id)
>   {
> @@ -1533,19 +1593,21 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>   	struct amdgpu_ualink_ppod_setup *ppod_setup;
>   	struct amdgpu_device *adev = ip_block->adev;
>   	struct amdgpu_ualink_info *info;
> +	int r;
>   
>   	info = kzalloc(sizeof(*info), GFP_KERNEL);
>   	ppod_setup = kzalloc(sizeof(*ppod_setup), GFP_KERNEL);
>   	vpod_config = kzalloc(sizeof(*vpod_config), GFP_KERNEL);
>   	stations = kzalloc(sizeof(*stations), GFP_KERNEL);
>   	if (!info || !ppod_setup || !vpod_config || !stations) {
> -		kfree(info);
> -		kfree(ppod_setup);
> -		kfree(vpod_config);
> -		kfree(stations);
> -		return -ENOMEM;
> +		r = -ENOMEM;
> +		goto err_free;
>   	}
>   
> +	r = amdgpu_ualink_loopback_sw_init(adev);
> +	if (r)
> +		goto err_free;
> +
>   	info->ppod.accel_id = 0xffffffff;
>   	info->ppod.bandwidth = 0xffffffff;
>   	info->ppod.latency = 0xffffffff;
> @@ -1568,10 +1630,18 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>   	adev->ualink.stations = stations;
>   
>   	return 0;
> +
> +err_free:
> +	kfree(info);
> +	kfree(ppod_setup);
> +	kfree(vpod_config);
> +	kfree(stations);
> +	return r;
>   }
>   
>   int ualink_ip_sw_fini(struct amdgpu_ip_block *ip_block)
>   {
> +	amdgpu_ualink_loopback_fini(ip_block->adev);
>   	amdgpu_ualink_object_fini(ip_block->adev);
>   	return 0;
>   }
> @@ -4487,11 +4557,6 @@ static bool amdgpu_ualink_vpod_membership_changed(struct amdgpu_device *adev)
>   			     AMDGPU_UALINK_ACCEL_MAX);
>   }
>   
> -static inline u32 ualink_accel_id(struct amdgpu_device *adev)
> -{
> -	return adev->ualink.info->ppod.accel_id;
> -}
> -
>   static inline enum amdgpu_ualink_addr_mode ualink_addr_mode(struct amdgpu_device *adev)
>   {
>   	return adev->ualink.info->vpod.addr_mode;
> @@ -4837,7 +4902,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>   	u64 timeout = msecs_to_jiffies(2000);
> -	u32 dst_accel_id = ualink_accel_id(adev);
> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>   	struct amdgpu_bo *bos[2];
>   	u32 n_bos;
>   	struct dma_fence *fence;
> @@ -4956,7 +5021,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>   static int amdgpu_ualink_metadata_npa_mapping(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 dst_accel_id = ualink_accel_id(adev);
> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>   	u64 timeout = msecs_to_jiffies(2000);
>   	struct amdgpu_bo *bos[2];
>   	u32 n_bos;
> @@ -5241,7 +5306,7 @@ static void amdgpu_ualink_metadata_fini(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>   
> -	dev_dbg(adev->dev, "accel_id %u\n", ualink_accel_id(adev));
> +	dev_dbg(adev->dev, "accel_id %u\n", ualink_exp_accel_id(adev));
>   	amdgpu_bo_free_kernel(&remote->rptr_bo, &remote->rptr_gpu_addr,
>   				      &remote->rptr_cpu_addr);
>   	amdgpu_bo_free_kernel(&remote->ring_bo, &remote->rb_gpu_addr,
> @@ -5288,7 +5353,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   	}
>   
>   	dev_dbg(adev->dev, "accel_id %u addr_mode %d fw status 0x%x\n",
> -		ualink_accel_id(adev), ualink_addr_mode(adev), status);
> +		ualink_exp_accel_id(adev), ualink_addr_mode(adev), status);
>   
>   	/* Alloc metadata structure for all GPUs */
>   	metadata_size = sizeof(struct amdgpu_ualink_metadata) * AMDGPU_UALINK_ACCEL_MAX;
> @@ -5361,12 +5426,12 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   	}
>   
>   	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS)
> -		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_accel_id(adev);
> +		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>   
>   	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
>   		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>   
> -		if (accel_id == ualink_accel_id(adev))
> +		if (accel_id == ualink_exp_accel_id(adev))
>   			continue;
>   
>   		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
> @@ -5392,7 +5457,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   						     ualink_tlb_wptr_offset(adev, accel_id);
>   
>   		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
> -			accel_id, ualink_accel_id(adev), metadata[accel_id].rb_size);
> +			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
>   		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
>   			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
>   		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
> @@ -5806,7 +5871,14 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>   				      u64 *wb_npa, u32 type)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 accel_id = ualink_accel_id(adev);
> +	/* The GPUID encoded in the write-back NPA address is the routing target
> +	 * the remote writes the rptr back to, i.e. THIS GPU. It must be the
> +	 * physically-owned accel_id: outside loopback that is our own accel_id,
> +	 * and in loopback it is the exporter (the only id the single GPU owns
> +	 * and can receive on). The importer is only a logical source id for the
> +	 * forward path and must never appear as an NPA GPUID.
> +	 */
Maybe move this into the patch comment.

Regards,
Philip
> +	u32 accel_id = ualink_exp_accel_id(adev);
>   	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
>   	uintptr_t wb;
>   	u64 npa;
> @@ -5981,19 +6053,19 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   						   &peer->shootdown, &tlb_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);
> +			ualink_exp_accel_id(adev), remote_accel_id, r);
>   		return r;
>   	}
>   
>   	if (peer->interrupt.wptr == int_wptr && peer->shootdown.wptr == tlb_wptr) {
>   		dev_dbg(adev->dev, "accel_id %u wptr 0x%llx 0x%llx is same as remote %u\n",
> -			ualink_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
> +			ualink_exp_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
>   		return 0;
>   	}
>   
>   	dev_dbg(adev->dev,
>   		"accel_id %u wptr 0x%llx 0x%llx sync to remote %u wptr 0x%llx 0x%llx\n",
> -		ualink_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
> +		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>   		remote_accel_id, int_wptr, tlb_wptr);
>   
>   	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
> @@ -6164,7 +6236,12 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
>   	struct amdgpu_ualink_peer *peer;
>   	struct amdgpu_ualink_ring *interrupt;
>   	struct amdgpu_ualink_ring *shootdown;
> -	u32 src_accel_id = ualink_accel_id(adev);
> +	/* This sets up the send rings (local sender -> each remote dst). In
> +	 * loopback the HW-served direction is importer -> exporter, so the
> +	 * local sender is the importer; the skip below then skips the importer
> +	 * and sets up the peer[exporter] ring that the HW path needs.
> +	 */
> +	u32 src_accel_id = ualink_imp_accel_id(adev);
     if (adev->ualink.loopback.enabled)
src_accel_id= adev->ualink.loopback.importer_accel_id;
>   	u32 dst_accel_id;
>   	u64 npa, flags;
>   	int r = 0;
> @@ -6287,7 +6364,10 @@ static void amdgpu_ualink_peer_remote_fini(struct amdgpu_device *adev)
>   {
>   	u32 rb_pages = AMDGPU_UALINK_RB_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 src_accel_id = ualink_accel_id(adev);
> +	/* Mirror peer_remote_init: in loopback the local sender is the
> +	 * importer, so unmap the peer[exporter] send ring that was set up.
> +	 */
> +	u32 src_accel_id = ualink_imp_accel_id(adev);
>   	u32 dst_accel_id;
>   	struct amdgpu_ualink_peer *peer;
>   	struct amdgpu_ualink_ring *ring;
> @@ -6374,7 +6454,7 @@ int amdgpu_ualink_sw_init(struct amdgpu_device *adev)
>    */
>   void amdgpu_ualink_sw_fini(struct amdgpu_device *adev)
>   {
> -	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_accel_id(adev),
> +	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_exp_accel_id(adev),
>   		ualink_addr_mode(adev));
>   
>   	adev->ualink.msg_ctl->send_halt(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> index 85c9a9634a4b..0b1e8dd38666 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> @@ -97,6 +97,17 @@ enum AMDGPU_UALINK_PROTOCOL_MESSAGES {
>   #define AMDGPU_UALINK_LIGHTWEIGHT_TLB_SHOOTDOWN		0x1
>   #define AMDGPU_UALINK_HEAVYWEIGHT_TLB_SHOOTDOWN		0x2
>   
> +/* UALink loopback support.
> + *
> + * In loopback mode a single GPU loops UALink requests back to itself for
> + * manufacturing testing. The GPU exposes two fixed accelerator IDs: it acts
> + * as the exporter on one and the importer on the other. Loopback is enabled
> + * with the amdgpu.ualink_loopback module parameter.
> + */
> +/* Fixed accelerator IDs used by a loopback GPU. */
> +#define AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID		1
> +#define AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID		0
> +
>   enum amdgpu_ualink_conn_state {
>   	AMDGPU_UALINK_CONN_NOT_READY			= 0,
>   	AMDGPU_UALINK_CONN_IN_PROGRESS			= 1,
> @@ -319,6 +330,21 @@ struct amdgpu_ualink_msg_ctl {
>   	int (*send_halt)(struct amdgpu_device *adev);
>   };
>   
> +/* UALink loopback mode state.
> + *
> + * When @enabled is true this single GPU acts as both exporter
> + * (@exporter_accel_id) and importer (@importer_accel_id). There is no second
> + * physical GPU, so exporter->importer messages cannot be delivered by the
> + * (single) MP nHT and are instead emulated in SW by feeding them back into
> + * our own IRQ handler via @wq.
> + */
> +struct amdgpu_ualink_loopback {
> +	bool enabled;
> +	u32 exporter_accel_id;
> +	u32 importer_accel_id;
> +	struct workqueue_struct *wq;
> +};
> +
>   struct amdgpu_ualink_mgr {
>   	u64 npa_size;
>   	u32 psp_if_ver;
> @@ -333,6 +359,9 @@ struct amdgpu_ualink_mgr {
>   	/* For remote interrupt and shootdown */
>   	struct amdgpu_ualink_remote *remote;
>   
> +	/* UALink loopback mode state (see struct amdgpu_ualink_loopback) */
> +	struct amdgpu_ualink_loopback loopback;
> +
>   	/* handle irq from ualink client of remote GPUs */
>   	struct amdgpu_irq_src irq;
>   

[-- Attachment #2: Type: text/html, Size: 15691 bytes --]

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

* Re: [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink
  2026-11-02 15:47 ` [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink Mukul Joshi
@ 2026-11-03 22:07   ` Philip Yang
  2026-11-03 22:51     ` Felix Kuehling
  0 siblings, 1 reply; 24+ messages in thread
From: Philip Yang @ 2026-11-03 22:07 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 6086 bytes --]



On 2026-11-02 10:47, Mukul Joshi wrote:
> The UPDATE_WB_ADDR remote command hands the MP nHT FW the address it
> writes command completions back to. So far that has always been an NPA.
>
> The FW can also take a VMID 0 GPUVA, selected by setting DW4 of the
> command to 1. Return the VMID 0 GPUVA of the writeback slot from
> amdgpu_ualink_get_wb_addr() alongside the NPA, and use it in
> UALink loopback mode. There the HW only carries importer -> exporter
> traffic, so the FW cannot write the completion back over NPA.
>
> Outside loopback the NPA is still used and DW4 stays 0.
>
> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
> Reviewed-by: Felix Kuehling<felix.kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 48 +++++++++++++++-------
>   1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index a98d5bbf0622..29a787979af2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -5652,9 +5652,9 @@ static void amdgpu_ualink_emit_update_wb_addr(u32 **cpu_addr_p, u32 wb_data,
>   
>   	*cpu_addr++ = cmd;
>   	*cpu_addr++ = wb_data;
> -	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb_npa */
> -	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb_npa */
> -	*cpu_addr++ = 0;
> +	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb address */
> +	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb address */
> +	*cpu_addr++ = dw2;		/* 1: wb address is VMID 0 GPUVA, 0: NPA */
dw0 and dw1 shouldn't have changed — I don't see any difference. Am I 
missing something?
>   	*cpu_addr++ = 0;
>   	*cpu_addr++ = 0; /* metadata */
>   	*cpu_addr++ = 0; /* metadata */
> @@ -5868,7 +5868,7 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
>   static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>   				      u32 remote_accel_id,
>   				      struct amdgpu_ualink_wb **wb_cpu,
> -				      u64 *wb_npa, u32 type)
> +				      u64 *wb_npa, u64 *wb_gpu, u32 type)
>   {
Since wb_npa and wb_gpu are mutually exclusive, this function returns 
one or the other, never both. I'd prefer to keep the interface as is and 
check whether loopback is enabled inside it, returning the corresponding 
wb address.
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>   	/* The GPUID encoded in the write-back NPA address is the routing target
> @@ -5881,15 +5881,17 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>   	u32 accel_id = ualink_exp_accel_id(adev);
>   	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
>   	uintptr_t wb;
> -	u64 npa;
> +	u64 gpu, npa;
>   	u32 offset = 0;
>   
>   	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
>   		wb = (uintptr_t)remote->rb_cpu_addr + rb_size * remote->num_accel;
> +		gpu = remote->rb_gpu_addr + rb_size * remote->num_accel;
>   		npa = amdgpu_ualink_npa_addr(adev, RB_TYPE_TAILPTR,
>   					     remote_accel_id, accel_id);
>   	} else {
>   		wb = (uintptr_t)remote->rptr_cpu_addr;
> +		gpu = remote->rptr_gpu_addr;
>   		npa = remote->rptr_npa;
>   		npa |= (u64)accel_id << AMDGPU_UALINK_GART_NPA_ADDR_GPUID_SHIFT;
>   	}
> @@ -5902,9 +5904,11 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>   	*wb_cpu = (struct amdgpu_ualink_wb *)(wb + offset);
>   	if (wb_npa)
>   		*wb_npa = npa + offset;
*wb_npa = adev->ualink.loopback.enabled ? gpu : npa + offset;
> +	if (wb_gpu)
> +		*wb_gpu = gpu + offset;
>   
> -	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx\n", accel_id,
> -		remote_accel_id, npa + offset);
> +	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx gpu 0x%llx\n", accel_id,
> +		remote_accel_id, npa + offset, gpu + offset);
>   }
>   
>   struct amdgpu_ualink_wptr_fence_cb {
> @@ -6068,9 +6072,9 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>   		remote_accel_id, int_wptr, tlb_wptr);
>   
> -	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
> +	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL, NULL,
>   				  RB_TYPE_REMOTE_INTERRUPT);
> -	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL,
> +	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL, NULL,
>   				  RB_TYPE_TLB_INV);
>   
>   	peer->interrupt.wptr = int_wptr;
> @@ -6104,7 +6108,8 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
>   	struct amdgpu_ualink_peer *peer;
>   	struct amdgpu_ualink_ring *ring;
>   	struct amdgpu_ualink_wb *wb_cpu;
> -	u64 wb_npa;
> +	u64 wb_npa, wb_gpu, wb_addr;
> +	u32 wb_vmid0;
>   	int r;
>   
>   	peer = &remote->peer[remote_accel_id];
> @@ -6116,13 +6121,28 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
>   		return -EINVAL;
>   
>   	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &wb_cpu, &wb_npa,
> -				  ring_type);
> +				  &wb_gpu, ring_type);
> +
> +	/* UALink loopback: HW only carries importer -> exporter traffic, so the
> +	 * MPNHT FW cannot write the completion back over NPA. Hand it the VMID 0
> +	 * GPUVA of the writeback instead and flag it in DW4 of the command.
> +	 */
> +	if (adev->ualink.loopback.enabled) {
> +		wb_addr = wb_gpu;
> +		wb_vmid0 = 1;
> +	} else {
> +		wb_addr = wb_npa;
> +		wb_vmid0 = 0;
> +	}
> +
then wb_vmid0, wb_gpu and wb_addr aren't needed here.

> +	dev_dbg(adev->dev, "remote %u ring type %u wb %s 0x%llx\n",
> +		remote_accel_id, ring_type, wb_vmid0 ? "gpuva" : "npa", wb_addr);
>   
>   	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, wb_cpu,
>   				       amdgpu_ualink_emit_update_wb_addr,
> -				       upper_32_bits(wb_npa),
> -				       lower_32_bits(wb_npa),
> -				       0, 0);
> +				       upper_32_bits(wb_addr),
> +				       lower_32_bits(wb_addr),
> +				       wb_vmid0, 0);

                      adev->ualink.loopback.enabled, 0);

Regards,
Philip

>   	if (!r) {
>   		ring->wb_cpu = wb_cpu;
>   		ring->ready = true;

[-- Attachment #2: Type: text/html, Size: 7333 bytes --]

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

* Re: [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
@ 2026-11-03 22:30   ` Philip Yang
  2026-11-03 23:21   ` Felix Kuehling
  2026-11-04 16:10   ` Philip Yang
  2 siblings, 0 replies; 24+ messages in thread
From: Philip Yang @ 2026-11-03 22:30 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 5137 bytes --]



On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter and the
> importer. The exporter is the one physical station, so the only path
> the HW serves is importer -> exporter. A message the exporter sends to
> the importer cannot be delivered by the MP nHT.
>
> Emulate that direction in SW: when loopback is enabled and a remote
> interrupt is addressed to the importer, queue it on the loopback
> ordered workqueue, which feeds it back into amdgpu_ualink_process_irq()
> as if the exporter had sent it. The ordered workqueue keeps successive
> messages in order and runs the (possibly sleeping) handlers outside
> the sender's context.
>
> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 82 ++++++++++++++++++++++
>   1 file changed, 82 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 29a787979af2..ef133c60bb9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -60,6 +60,9 @@ static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
>   					   u32 remote_accel_id);
>   static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   					  u32 remote_accel_id);
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3);
>   
>   #define STRIP_NPA(addr)						\
>   	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
> @@ -6220,6 +6223,17 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
>   	struct amdgpu_ualink_ring *ring;
>   	int r;
>   
> +	/* UALink loopback: the single MP nHT can only deliver messages addressed
> +	 * to the exporter accelerator. Messages addressed to the importer
> +	 * (exporter->importer direction) cannot be delivered by HW, so emulate
> +	 * them by feeding the message back into our own IRQ handler.
> +	 */
> +	if (adev->ualink.loopback.enabled &&
> +	    remote_accel_id == ualink_imp_accel_id(adev))
> +		return amdgpu_ualink_loopback_self_interrupt(adev,
> +				ualink_exp_accel_id(adev),
> +				dw0, dw1, dw2, dw3);
> +
>   	peer = &remote->peer[remote_accel_id];
>   	ring = &peer->interrupt;
>   
> @@ -6649,6 +6663,74 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
>   	return handled;
>   }
>   
> +/* Work item used to emulate an exporter->importer UALink interrupt in
> + * loopback mode (see amdgpu_ualink_loopback_self_interrupt()).
> + */
> +struct amdgpu_ualink_self_irq_work {
> +	struct work_struct work;
> +	struct amdgpu_device *adev;
> +	u32 sender_accel_id;
> +	u32 dw[4];
> +};
> +
> +static void amdgpu_ualink_self_irq_worker(struct work_struct *w)
> +{
> +	struct amdgpu_ualink_self_irq_work *job =
> +		container_of(w, struct amdgpu_ualink_self_irq_work, work);
> +	struct amdgpu_device *adev = job->adev;
> +	struct amdgpu_iv_entry entry = { };
> +
> +	/* Build an IH entry as if the message had been delivered by the MP nHT
> +	 * from the exporter accelerator. amdgpu_ualink_process_irq() consumes
> +	 * the sender accel_id from entry->pasid and the four context dwords
> +	 * from entry->src_data[]. entry->ih must differ from adev->irq.ih so
> +	 * the message is processed here instead of being delegated to the
> +	 * UALink soft ring.
> +	 */
> +	entry.ih = NULL;
Setting entry.ih to NULL goes straight down to UALink soft-ring path. It 
might be worth making the comment clearer about that.

Other than that, this patch is

Reviewed-by: Philip Yang <philip.yang@amd.com>

> +	entry.pasid = job->sender_accel_id;
> +	entry.src_data[0] = job->dw[0];
> +	entry.src_data[1] = job->dw[1];
> +	entry.src_data[2] = job->dw[2];
> +	entry.src_data[3] = job->dw[3];
> +
> +	amdgpu_ualink_process_irq(adev, &adev->ualink.irq, &entry);
> +	kfree(job);
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_self_interrupt - deliver a UALink message to ourselves
> + *
> + * Used in loopback mode for the exporter->importer direction, where there is
> + * no HW path. The message is processed asynchronously on an ordered workqueue
> + * so ordering between successive messages is preserved and the (potentially
> + * sleeping) message handlers do not run from the caller's context.
> + */
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3)
> +{
> +	struct amdgpu_ualink_self_irq_work *job;
> +
> +	if (!adev->ualink.loopback.wq)
> +		return -ENODEV;
> +
> +	job = kzalloc(sizeof(*job), GFP_KERNEL);
> +	if (!job)
> +		return -ENOMEM;
> +
> +	job->adev = adev;
> +	job->sender_accel_id = sender_accel_id;
> +	job->dw[0] = dw0;
> +	job->dw[1] = dw1;
> +	job->dw[2] = dw2;
> +	job->dw[3] = dw3;
> +	INIT_WORK(&job->work, amdgpu_ualink_self_irq_worker);
> +	queue_work(adev->ualink.loopback.wq, &job->work);
> +
> +	return 0;
> +}
> +
>   static int amdgpu_ualink_set_irq_state(struct amdgpu_device *adev,
>   					struct amdgpu_irq_src *source,
>   					u32 type,

[-- Attachment #2: Type: text/html, Size: 5745 bytes --]

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

* Re: [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure
  2026-11-03 18:17   ` Philip Yang
@ 2026-11-03 22:32     ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 22:32 UTC (permalink / raw)
  To: Philip Yang, Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 16069 bytes --]

On 2026-11-03 13:17, Philip Yang wrote:
>
>
> On 2026-11-02 10:47, Mukul Joshi wrote:
>> Add the infrastructure for UALink loopback mode, where a single GPU
>> plays both the exporter and the importer role under two fixed
>> accelerator IDs:
>>
>> - Add struct amdgpu_ualink_loopback to track the loopback state, along
>>    with the fixed exporter and importer accel IDs.
>>
>> - Replace ualink_accel_id() with ualink_exp_accel_id() and
>>    ualink_imp_accel_id(), which return the exporter and importer accel
>>    ID in loopback mode.
>>
>> - Add amdgpu_ualink_loopback_sw_init(), which allocates the ordered
>>    workqueue used to emulate exporter->importer messages from
>>    ualink_ip_sw_init(), and amdgpu_ualink_loopback_fini(), which
>>    destroys it from ualink_ip_sw_fini().
>>
>> Loopback is not enabled yet, and outside loopback both helpers return
>> ppod.accel_id, so there is no functional change.
>>
>> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
>> ---
>> v1->v2:
>> - Drop the change in find_peer_adev() as its not needed.
>>
>> v2->v3:
>> - Update src_accel_id to the correct value in amdgpu_ualink_peer_remote_init()
>>    and amdgpu_ualink_peer_remote_fini().
>> - Add the loopback teardown in amdgpu_ualink_loopback_fini() and call it
>>    through ualink_ip_sw_fini().
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 128 +++++++++++++++++----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h |  29 +++++
>>   2 files changed, 133 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> index ca6432610f71..a98d5bbf0622 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> @@ -89,6 +89,41 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
>> + *
>> + * Allocates the ordered workqueue used to emulate exporter->importer
>> + * interrupts in SW. Does nothing unless loopback mode is enabled.
>> + */
>> +static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
>> +{
>> +	if (!adev->ualink.loopback.enabled)
>> +		return 0;
>> +
>> +	adev->ualink.loopback.wq =
>> +		alloc_ordered_workqueue("amdgpu_ualink_loopback", 0);
>> +	if (!adev->ualink.loopback.wq)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * amdgpu_ualink_loopback_fini - tear down UALink loopback mode
>> + *
>> + * Destroys the workqueue allocated in amdgpu_ualink_loopback_sw_init().
>> + * destroy_workqueue() drains any pending self-interrupt work first.
>> + */
>> +static void amdgpu_ualink_loopback_fini(struct amdgpu_device *adev)
>> +{
>> +	if (!adev->ualink.loopback.wq)
>> +		return;
>> +
>> +	destroy_workqueue(adev->ualink.loopback.wq);
>> +	adev->ualink.loopback.wq = NULL;
>> +	adev->ualink.loopback.enabled = false;
>> +}
>> +
>>   static void amdgpu_ualink_object_fini(struct amdgpu_device *adev)
>>   {
>>   	if (!adev->ualink.info)
>> @@ -1028,6 +1063,31 @@ static struct amdgpu_device *find_peer_adev(unsigned int accel_id)
>>   	return NULL;
>>   }
>>   
>> +/*
>> + * Physical accelerator roles.
>> + *
>> + * Outside loopback a GPU owns a single accelerator, so both helpers return its
>> + * ppod.accel_id and the exporter/importer distinction collapses. In loopback
>> + * the one physical GPU plays both roles: the exporter owns the MP nHT and
>> + * receives all HW traffic, while the importer is the (virtual) HW sender for
>> + * the importer -> exporter path.
>> + */
>> +static inline u32 ualink_exp_accel_id(struct amdgpu_device *adev)
>> +{
>> +	if (adev->ualink.loopback.enabled)
>> +		return adev->ualink.loopback.exporter_accel_id;
>> +
>> +	return adev->ualink.info->ppod.accel_id;
>> +}
>> +
> Replacing ualink_accel_id with ualink_exp_accel_id is misleading for 
> the non-loopback case, since each device is both an exporter and an 
> importer at the same time. I'd prefer to add the loopback-enabled 
> check inside ualink_accel_id() instead.

It's true that the same GPU acts as both exporter and importer. But in 
any particular code path it acts in only one of those roles. It needs to 
choose the correct accelerator ID depending on its role. I asked for 
making that explicit in the function name. That way at any call site 
it's easy to see which role a GPU plays in the particular context. This 
replaces lots of very similar open-coded conditions that were 
obfuscating the meaning. It also keeps loopback-specific conditions in 
fewer places in the code, making it easier to read and maintain going 
forward.

Regards,
   Felix


>
> It also looks like ualink_imp_accel_id is only used by 
> amdgpu_ualink_process_irq and amdgpu_ualink_remote_interrupt under the 
> loopback-enabled condition, so it could be removed as well. That would 
> simplify the patch and drop all the ualink_accel_id renames.
>
>> +static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
>> +{
>> +	if (adev->ualink.loopback.enabled)
>> +		return adev->ualink.loopback.importer_accel_id;
>> +
>> +	return adev->ualink.info->ppod.accel_id;
>> +}
>> +
>>   static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
>>   					 u32 accel_id)
>>   {
>> @@ -1533,19 +1593,21 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>>   	struct amdgpu_ualink_ppod_setup *ppod_setup;
>>   	struct amdgpu_device *adev = ip_block->adev;
>>   	struct amdgpu_ualink_info *info;
>> +	int r;
>>   
>>   	info = kzalloc(sizeof(*info), GFP_KERNEL);
>>   	ppod_setup = kzalloc(sizeof(*ppod_setup), GFP_KERNEL);
>>   	vpod_config = kzalloc(sizeof(*vpod_config), GFP_KERNEL);
>>   	stations = kzalloc(sizeof(*stations), GFP_KERNEL);
>>   	if (!info || !ppod_setup || !vpod_config || !stations) {
>> -		kfree(info);
>> -		kfree(ppod_setup);
>> -		kfree(vpod_config);
>> -		kfree(stations);
>> -		return -ENOMEM;
>> +		r = -ENOMEM;
>> +		goto err_free;
>>   	}
>>   
>> +	r = amdgpu_ualink_loopback_sw_init(adev);
>> +	if (r)
>> +		goto err_free;
>> +
>>   	info->ppod.accel_id = 0xffffffff;
>>   	info->ppod.bandwidth = 0xffffffff;
>>   	info->ppod.latency = 0xffffffff;
>> @@ -1568,10 +1630,18 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>>   	adev->ualink.stations = stations;
>>   
>>   	return 0;
>> +
>> +err_free:
>> +	kfree(info);
>> +	kfree(ppod_setup);
>> +	kfree(vpod_config);
>> +	kfree(stations);
>> +	return r;
>>   }
>>   
>>   int ualink_ip_sw_fini(struct amdgpu_ip_block *ip_block)
>>   {
>> +	amdgpu_ualink_loopback_fini(ip_block->adev);
>>   	amdgpu_ualink_object_fini(ip_block->adev);
>>   	return 0;
>>   }
>> @@ -4487,11 +4557,6 @@ static bool amdgpu_ualink_vpod_membership_changed(struct amdgpu_device *adev)
>>   			     AMDGPU_UALINK_ACCEL_MAX);
>>   }
>>   
>> -static inline u32 ualink_accel_id(struct amdgpu_device *adev)
>> -{
>> -	return adev->ualink.info->ppod.accel_id;
>> -}
>> -
>>   static inline enum amdgpu_ualink_addr_mode ualink_addr_mode(struct amdgpu_device *adev)
>>   {
>>   	return adev->ualink.info->vpod.addr_mode;
>> @@ -4837,7 +4902,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>>   	u64 timeout = msecs_to_jiffies(2000);
>> -	u32 dst_accel_id = ualink_accel_id(adev);
>> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>>   	struct amdgpu_bo *bos[2];
>>   	u32 n_bos;
>>   	struct dma_fence *fence;
>> @@ -4956,7 +5021,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>>   static int amdgpu_ualink_metadata_npa_mapping(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>> -	u32 dst_accel_id = ualink_accel_id(adev);
>> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>>   	u64 timeout = msecs_to_jiffies(2000);
>>   	struct amdgpu_bo *bos[2];
>>   	u32 n_bos;
>> @@ -5241,7 +5306,7 @@ static void amdgpu_ualink_metadata_fini(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>>   
>> -	dev_dbg(adev->dev, "accel_id %u\n", ualink_accel_id(adev));
>> +	dev_dbg(adev->dev, "accel_id %u\n", ualink_exp_accel_id(adev));
>>   	amdgpu_bo_free_kernel(&remote->rptr_bo, &remote->rptr_gpu_addr,
>>   				      &remote->rptr_cpu_addr);
>>   	amdgpu_bo_free_kernel(&remote->ring_bo, &remote->rb_gpu_addr,
>> @@ -5288,7 +5353,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>   	}
>>   
>>   	dev_dbg(adev->dev, "accel_id %u addr_mode %d fw status 0x%x\n",
>> -		ualink_accel_id(adev), ualink_addr_mode(adev), status);
>> +		ualink_exp_accel_id(adev), ualink_addr_mode(adev), status);
>>   
>>   	/* Alloc metadata structure for all GPUs */
>>   	metadata_size = sizeof(struct amdgpu_ualink_metadata) * AMDGPU_UALINK_ACCEL_MAX;
>> @@ -5361,12 +5426,12 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>   	}
>>   
>>   	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS)
>> -		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_accel_id(adev);
>> +		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>>   
>>   	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
>>   		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>>   
>> -		if (accel_id == ualink_accel_id(adev))
>> +		if (accel_id == ualink_exp_accel_id(adev))
>>   			continue;
>>   
>>   		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
>> @@ -5392,7 +5457,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>   						     ualink_tlb_wptr_offset(adev, accel_id);
>>   
>>   		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
>> -			accel_id, ualink_accel_id(adev), metadata[accel_id].rb_size);
>> +			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
>>   		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
>>   			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
>>   		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
>> @@ -5806,7 +5871,14 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>>   				      u64 *wb_npa, u32 type)
>>   {
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>> -	u32 accel_id = ualink_accel_id(adev);
>> +	/* The GPUID encoded in the write-back NPA address is the routing target
>> +	 * the remote writes the rptr back to, i.e. THIS GPU. It must be the
>> +	 * physically-owned accel_id: outside loopback that is our own accel_id,
>> +	 * and in loopback it is the exporter (the only id the single GPU owns
>> +	 * and can receive on). The importer is only a logical source id for the
>> +	 * forward path and must never appear as an NPA GPUID.
>> +	 */
> Maybe move this into the patch comment.
>
> Regards,
> Philip
>> +	u32 accel_id = ualink_exp_accel_id(adev);
>>   	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
>>   	uintptr_t wb;
>>   	u64 npa;
>> @@ -5981,19 +6053,19 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>>   						   &peer->shootdown, &tlb_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);
>> +			ualink_exp_accel_id(adev), remote_accel_id, r);
>>   		return r;
>>   	}
>>   
>>   	if (peer->interrupt.wptr == int_wptr && peer->shootdown.wptr == tlb_wptr) {
>>   		dev_dbg(adev->dev, "accel_id %u wptr 0x%llx 0x%llx is same as remote %u\n",
>> -			ualink_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
>> +			ualink_exp_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
>>   		return 0;
>>   	}
>>   
>>   	dev_dbg(adev->dev,
>>   		"accel_id %u wptr 0x%llx 0x%llx sync to remote %u wptr 0x%llx 0x%llx\n",
>> -		ualink_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>> +		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>>   		remote_accel_id, int_wptr, tlb_wptr);
>>   
>>   	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
>> @@ -6164,7 +6236,12 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
>>   	struct amdgpu_ualink_peer *peer;
>>   	struct amdgpu_ualink_ring *interrupt;
>>   	struct amdgpu_ualink_ring *shootdown;
>> -	u32 src_accel_id = ualink_accel_id(adev);
>> +	/* This sets up the send rings (local sender -> each remote dst). In
>> +	 * loopback the HW-served direction is importer -> exporter, so the
>> +	 * local sender is the importer; the skip below then skips the importer
>> +	 * and sets up the peer[exporter] ring that the HW path needs.
>> +	 */
>> +	u32 src_accel_id = ualink_imp_accel_id(adev);
>     if (adev->ualink.loopback.enabled)
> src_accel_id= adev->ualink.loopback.importer_accel_id;
>>   	u32 dst_accel_id;
>>   	u64 npa, flags;
>>   	int r = 0;
>> @@ -6287,7 +6364,10 @@ static void amdgpu_ualink_peer_remote_fini(struct amdgpu_device *adev)
>>   {
>>   	u32 rb_pages = AMDGPU_UALINK_RB_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>> -	u32 src_accel_id = ualink_accel_id(adev);
>> +	/* Mirror peer_remote_init: in loopback the local sender is the
>> +	 * importer, so unmap the peer[exporter] send ring that was set up.
>> +	 */
>> +	u32 src_accel_id = ualink_imp_accel_id(adev);
>>   	u32 dst_accel_id;
>>   	struct amdgpu_ualink_peer *peer;
>>   	struct amdgpu_ualink_ring *ring;
>> @@ -6374,7 +6454,7 @@ int amdgpu_ualink_sw_init(struct amdgpu_device *adev)
>>    */
>>   void amdgpu_ualink_sw_fini(struct amdgpu_device *adev)
>>   {
>> -	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_accel_id(adev),
>> +	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_exp_accel_id(adev),
>>   		ualink_addr_mode(adev));
>>   
>>   	adev->ualink.msg_ctl->send_halt(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
>> index 85c9a9634a4b..0b1e8dd38666 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
>> @@ -97,6 +97,17 @@ enum AMDGPU_UALINK_PROTOCOL_MESSAGES {
>>   #define AMDGPU_UALINK_LIGHTWEIGHT_TLB_SHOOTDOWN		0x1
>>   #define AMDGPU_UALINK_HEAVYWEIGHT_TLB_SHOOTDOWN		0x2
>>   
>> +/* UALink loopback support.
>> + *
>> + * In loopback mode a single GPU loops UALink requests back to itself for
>> + * manufacturing testing. The GPU exposes two fixed accelerator IDs: it acts
>> + * as the exporter on one and the importer on the other. Loopback is enabled
>> + * with the amdgpu.ualink_loopback module parameter.
>> + */
>> +/* Fixed accelerator IDs used by a loopback GPU. */
>> +#define AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID		1
>> +#define AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID		0
>> +
>>   enum amdgpu_ualink_conn_state {
>>   	AMDGPU_UALINK_CONN_NOT_READY			= 0,
>>   	AMDGPU_UALINK_CONN_IN_PROGRESS			= 1,
>> @@ -319,6 +330,21 @@ struct amdgpu_ualink_msg_ctl {
>>   	int (*send_halt)(struct amdgpu_device *adev);
>>   };
>>   
>> +/* UALink loopback mode state.
>> + *
>> + * When @enabled is true this single GPU acts as both exporter
>> + * (@exporter_accel_id) and importer (@importer_accel_id). There is no second
>> + * physical GPU, so exporter->importer messages cannot be delivered by the
>> + * (single) MP nHT and are instead emulated in SW by feeding them back into
>> + * our own IRQ handler via @wq.
>> + */
>> +struct amdgpu_ualink_loopback {
>> +	bool enabled;
>> +	u32 exporter_accel_id;
>> +	u32 importer_accel_id;
>> +	struct workqueue_struct *wq;
>> +};
>> +
>>   struct amdgpu_ualink_mgr {
>>   	u64 npa_size;
>>   	u32 psp_if_ver;
>> @@ -333,6 +359,9 @@ struct amdgpu_ualink_mgr {
>>   	/* For remote interrupt and shootdown */
>>   	struct amdgpu_ualink_remote *remote;
>>   
>> +	/* UALink loopback mode state (see struct amdgpu_ualink_loopback) */
>> +	struct amdgpu_ualink_loopback loopback;
>> +
>>   	/* handle irq from ualink client of remote GPUs */
>>   	struct amdgpu_irq_src irq;
>>   
>

[-- Attachment #2: Type: text/html, Size: 16743 bytes --]

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

* Re: [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink
  2026-11-03 22:07   ` Philip Yang
@ 2026-11-03 22:51     ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 22:51 UTC (permalink / raw)
  To: Philip Yang, Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang


On 2026-11-03 17:07, Philip Yang wrote:
>
>
> On 2026-11-02 10:47, Mukul Joshi wrote:
>> The UPDATE_WB_ADDR remote command hands the MP nHT FW the address it
>> writes command completions back to. So far that has always been an NPA.
>>
>> The FW can also take a VMID 0 GPUVA, selected by setting DW4 of the
>> command to 1. Return the VMID 0 GPUVA of the writeback slot from
>> amdgpu_ualink_get_wb_addr() alongside the NPA, and use it in
>> UALink loopback mode. There the HW only carries importer -> exporter
>> traffic, so the FW cannot write the completion back over NPA.
>>
>> Outside loopback the NPA is still used and DW4 stays 0.
>>
>> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
>> Reviewed-by: Felix Kuehling<felix.kuehling@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 48 +++++++++++++++-------
>>   1 file changed, 34 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> index a98d5bbf0622..29a787979af2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> @@ -5652,9 +5652,9 @@ static void amdgpu_ualink_emit_update_wb_addr(u32 **cpu_addr_p, u32 wb_data,
>>   
>>   	*cpu_addr++ = cmd;
>>   	*cpu_addr++ = wb_data;
>> -	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb_npa */
>> -	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb_npa */
>> -	*cpu_addr++ = 0;
>> +	*cpu_addr++ = dw1 & 0xFFFFFFFC;	/* lower32 wb address */
>> +	*cpu_addr++ = dw0 & 0xFFFFF;	/* upper32 wb address */
>> +	*cpu_addr++ = dw2;		/* 1: wb address is VMID 0 GPUVA, 0: NPA */
> dw0 and dw1 shouldn't have changed — I don't see any difference. Am I 
> missing something?

The comment changed. The address may now be an NPA address or a VMID 
GPUVA. Thus the comment shouldn't say that it's an NPA.


>>   	*cpu_addr++ = 0;
>>   	*cpu_addr++ = 0; /* metadata */
>>   	*cpu_addr++ = 0; /* metadata */
>> @@ -5868,7 +5868,7 @@ static int amdgpu_ualink_send_command(struct amdgpu_device *adev,
>>   static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>>   				      u32 remote_accel_id,
>>   				      struct amdgpu_ualink_wb **wb_cpu,
>> -				      u64 *wb_npa, u32 type)
>> +				      u64 *wb_npa, u64 *wb_gpu, u32 type)
>>   {
> Since wb_npa and wb_gpu are mutually exclusive, this function returns 
> one or the other, never both. I'd prefer to keep the interface as is 
> and check whether loopback is enabled inside it, returning the 
> corresponding wb address.
>>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>>   	/* The GPUID encoded in the write-back NPA address is the routing target
>> @@ -5881,15 +5881,17 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>>   	u32 accel_id = ualink_exp_accel_id(adev);
>>   	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
>>   	uintptr_t wb;
>> -	u64 npa;
>> +	u64 gpu, npa;
>>   	u32 offset = 0;
>>   
>>   	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
>>   		wb = (uintptr_t)remote->rb_cpu_addr + rb_size * remote->num_accel;
>> +		gpu = remote->rb_gpu_addr + rb_size * remote->num_accel;
>>   		npa = amdgpu_ualink_npa_addr(adev, RB_TYPE_TAILPTR,
>>   					     remote_accel_id, accel_id);
>>   	} else {
>>   		wb = (uintptr_t)remote->rptr_cpu_addr;
>> +		gpu = remote->rptr_gpu_addr;
>>   		npa = remote->rptr_npa;
>>   		npa |= (u64)accel_id << AMDGPU_UALINK_GART_NPA_ADDR_GPUID_SHIFT;
>>   	}
>> @@ -5902,9 +5904,11 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>>   	*wb_cpu = (struct amdgpu_ualink_wb *)(wb + offset);
>>   	if (wb_npa)
>>   		*wb_npa = npa + offset;
> *wb_npa = adev->ualink.loopback.enabled ? gpu : npa + offset;
>> +	if (wb_gpu)
>> +		*wb_gpu = gpu + offset;
>>   
>> -	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx\n", accel_id,
>> -		remote_accel_id, npa + offset);
>> +	dev_dbg(adev->dev, "source %d remote %d wb npa 0x%llx gpu 0x%llx\n", accel_id,
>> +		remote_accel_id, npa + offset, gpu + offset);
>>   }
>>   
>>   struct amdgpu_ualink_wptr_fence_cb {
>> @@ -6068,9 +6072,9 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>>   		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>>   		remote_accel_id, int_wptr, tlb_wptr);
>>   
>> -	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
>> +	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL, NULL,
>>   				  RB_TYPE_REMOTE_INTERRUPT);
>> -	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL,
>> +	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &tlb_wb, NULL, NULL,
>>   				  RB_TYPE_TLB_INV);
>>   
>>   	peer->interrupt.wptr = int_wptr;
>> @@ -6104,7 +6108,8 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
>>   	struct amdgpu_ualink_peer *peer;
>>   	struct amdgpu_ualink_ring *ring;
>>   	struct amdgpu_ualink_wb *wb_cpu;
>> -	u64 wb_npa;
>> +	u64 wb_npa, wb_gpu, wb_addr;
>> +	u32 wb_vmid0;
>>   	int r;
>>   
>>   	peer = &remote->peer[remote_accel_id];
>> @@ -6116,13 +6121,28 @@ static int amdgpu_ualink_update_wb_address(struct amdgpu_device *adev,
>>   		return -EINVAL;
>>   
>>   	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &wb_cpu, &wb_npa,
>> -				  ring_type);
>> +				  &wb_gpu, ring_type);
>> +
>> +	/* UALink loopback: HW only carries importer -> exporter traffic, so the
>> +	 * MPNHT FW cannot write the completion back over NPA. Hand it the VMID 0
>> +	 * GPUVA of the writeback instead and flag it in DW4 of the command.
>> +	 */
>> +	if (adev->ualink.loopback.enabled) {
>> +		wb_addr = wb_gpu;
>> +		wb_vmid0 = 1;
>> +	} else {
>> +		wb_addr = wb_npa;
>> +		wb_vmid0 = 0;
>> +	}
>> +
> then wb_vmid0, wb_gpu and wb_addr aren't needed here.

We still need to tell MP nHT what type of address we are providing. I 
guess you could just replace wb_vmid0 below with 
adev->ualink.loopback.enabled. But that makes the code harder to read 
and maintain because you have conditions or assumptions about the 
address type in both amdgpu_ualink_update_wb_address and 
amdgpu_ualink_get_wb_addr. If they get out of sync, things break. I 
prefer the explicit version that Mukul wrote, where 
amdgpu_ualink_get_wb_addr returns both addresses, and the choice of 
which one to use is made in only one place.

Regards,
   Felix


>
>> +	dev_dbg(adev->dev, "remote %u ring type %u wb %s 0x%llx\n",
>> +		remote_accel_id, ring_type, wb_vmid0 ? "gpuva" : "npa", wb_addr);
>>   
>>   	r = amdgpu_ualink_send_command(adev, remote_accel_id, ring, wb_cpu,
>>   				       amdgpu_ualink_emit_update_wb_addr,
>> -				       upper_32_bits(wb_npa),
>> -				       lower_32_bits(wb_npa),
>> -				       0, 0);
>> +				       upper_32_bits(wb_addr),
>> +				       lower_32_bits(wb_addr),
>> +				       wb_vmid0, 0);
>
>                       adev->ualink.loopback.enabled, 0);
>
> Regards,
> Philip
>>   	if (!r) {
>>   		ring->wb_cpu = wb_cpu;
>>   		ring->ready = true;
>

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

* Re: [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender
  2026-11-02 15:47 ` [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender Mukul Joshi
@ 2026-11-03 23:04   ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:04 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter and the
> importer, so the sender accel ID of a message depends on its direction.
>
> - Send HELLO as the opposite role of the destination: a message to the
>    exporter is sent as the importer and vice versa. Outside loopback
>    this is still the GPU's own accel ID.
>
> - The MP nHT stamps the IH cookie of every HW-delivered message with
>    the GPU ID of the single physical station, the exporter. The only HW
>    path is importer -> exporter, so attribute these messages to the
>    importer. SW-emulated exporter -> importer messages carry no IH ring
>    and already name the exporter as the sender.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 32 +++++++++++++++++++++-
>   1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index ef133c60bb9a..8558dc1a9e84 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -1091,6 +1091,23 @@ static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
>   	return adev->ualink.info->ppod.accel_id;
>   }
>   
> +/*
> + * amdgpu_ualink_local_sender_accel_id - sender accel_id for an outgoing message
> + *
> + * Normally a GPU sends messages as its own (single) accelerator. In loopback
> + * mode the same GPU plays both roles, so the sender identity depends on the
> + * destination: a message to the exporter is sent as the importer and vice
> + * versa. Outside loopback both roles collapse to the GPU's own accel_id.
> + */
> +static u32 amdgpu_ualink_local_sender_accel_id(struct amdgpu_device *adev,
> +					       u32 remote_acc_id)
> +{
> +	if (remote_acc_id == ualink_exp_accel_id(adev))
> +		return ualink_imp_accel_id(adev);
> +
> +	return ualink_exp_accel_id(adev);
> +}
> +
>   static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
>   					 u32 accel_id)
>   {
> @@ -2249,7 +2266,10 @@ static int amdgpu_ualink_send_hello_msg(struct amdgpu_device *adev,
>   
>   	dw0 = AMDGPU_UALINK_HELLO_MSG;
>   	dw0 |= (remote_acc_id << AMDGPU_UALINK_HELLO_MSG_RECV_ACCID_SHIFT);
> -	dw0 |= (adev->ualink.info->ppod.accel_id <<
> +	/* In loopback the sender is the opposite role of the destination, so
> +	 * the exporter side tracks the connection under the importer accel_id.
> +	 */
> +	dw0 |= (amdgpu_ualink_local_sender_accel_id(adev, remote_acc_id) <<
>   		AMDGPU_UALINK_HELLO_MSG_SENDER_ACCID_SHIFT);
>   
>   	dev_dbg(adev->dev, "SEND HELLO: HELLO message to remote AccId:%u\n",
> @@ -6567,6 +6587,16 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
>   		return handled;
>   	}
>   
> +	/* UALink loopback: the MP nHT stamps every HW-delivered message with
> +	 * the GPU ID of the single physical station, the exporter. The only
> +	 * HW path is importer -> exporter, so the real sender is the importer.
> +	 * SW-emulated exporter -> importer messages carry no IH ring and
> +	 * already name the exporter as sender.
> +	 */
> +	if (adev->ualink.loopback.enabled && entry->ih &&
> +	    src_acc_id == ualink_exp_accel_id(adev))
> +		src_acc_id = ualink_imp_accel_id(adev);

Small nit-pick: The last part of the condition is redundant. If the 
src_acc_id is not the exporter ID then it's the importer ID. So in the 
worst case you overwrite it with the same value. So I'd drop that part 
of the condition. Still

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


> +
>   	msg_type = dw0 & AMDGPU_UALINK_MESSAGE_HEADER_MASK;
>   	local_acc_id = adev->ualink.info->ppod.accel_id;
>   	dev_dbg(adev->dev, "Got MSG: remote acc_id %u msg_type %u\n",

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

* Re: [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback
  2026-11-02 15:47 ` [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback Mukul Joshi
@ 2026-11-03 23:15   ` Felix Kuehling
  2026-11-04 20:20     ` Mukul Joshi
  0 siblings, 1 reply; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:15 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 5890 bytes --]

On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter and the
> importer, and the only path the HW serves is importer -> exporter.
> Adjust the per-peer setup accordingly:
>
> - The importer's doorbell reaches the MP nHT stamped with the GPU ID of
>    the exporter, and the FW looks up the metadata entry by that ID. So
>    publish the importer's rings and tail pointers in metadata[exporter].
>
> - Skip the ring pointer resync. Both ends of every ring are on this
>    GPU, so there is no remote peer that can reboot and leave the
>    pointers stale.
>
> - Do not take the local dma-buf shortcut on import. It would bypass
>    the NPA path that loopback is meant to exercise.
>
> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
> ---
> v2->v3:
> - Move updating src_accel_id in peer_remote_init and fini to patch 1.
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 54 ++++++++++++++++------
>   1 file changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 8558dc1a9e84..74bc8a907f5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -3976,7 +3976,13 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
>   		return -EINVAL;
>   	}
>   
> -	if (amdgpu_ualink_is_local_accel(adev, remote_acc_id))
> +	/* In loopback mode the exporter and importer are the same GPU, but the
> +	 * import must still go through the full cross-domain NPA path (the
> +	 * dma_buf shortcut in amdgpu_ualink_local_import() bypasses NPA). Only
> +	 * take the local shortcut for genuine same-pod local accelerators.
> +	 */
> +	if (!adev->ualink.loopback.enabled &&
> +	    amdgpu_ualink_is_local_accel(adev, remote_acc_id))
>   		return amdgpu_ualink_local_import(adev, remote_acc_id,
>   						  &handle, fd_out);
>   
> @@ -5452,11 +5458,28 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>   
>   	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
> +		struct amdgpu_ualink_metadata *md;
> +
>   		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>   
>   		if (accel_id == ualink_exp_accel_id(adev))
>   			continue;

I think you could drop the condition just below if you change this to

		if (accel_id == ualink_imp_accel_id(adev))
			continue;

It makes no difference if loopback is disabled. And with loopback 
enabled it's the more obvious way to only initialize the ring for the 
exporter ID. Then all the other changes in this function are unnecessary.

Regards,
   Felix


>   
> +		/* In loopback the single MP nHT services the exporter half:
> +		 * the only messages delivered over HW are importer -> exporter
> +		 * (source = importer accel_id), while the exporter -> importer
> +		 * direction is emulated in SW and never reaches the MP nHT.
> +		 *
> +		 * The importer's doorbell reaches the MP nHT stamped with the
> +		 * GPU ID of the single physical station, the exporter, so the
> +		 * FW looks up metadata[exporter]. Publish the importer's rings
> +		 * and tail pointers in that entry.
> +		 */
> +		if (adev->ualink.loopback.enabled)
> +			md = &metadata[ualink_exp_accel_id(adev)];
> +		else
> +			md = &metadata[accel_id];
> +
>   		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
>   
>   		if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
> @@ -5464,27 +5487,26 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   			wptr_gpu_addr += wptr_size * accel_id;
>   		}
>   
> -		metadata[accel_id].header = AMDGPU_UALINK_METADATA_HEADER;
> -		metadata[accel_id].rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
> +		md->header = AMDGPU_UALINK_METADATA_HEADER;
> +		md->rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
>   
>   		/*
>   		 * remote shootdown type is 0, remote command type is 1
>   		 * with 2MB ring buffer mapping, remote shootdown ring NPA is before interrupt
>   		 */
> -		metadata[accel_id].ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
> -		metadata[accel_id].tlb_inv_rb = rb_gpu_addr;
> +		md->ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
> +		md->tlb_inv_rb = rb_gpu_addr;
>   
> -		metadata[accel_id].tailptr_ri = wptr_gpu_addr +
> -						ualink_wptr_offset(adev, accel_id);
> -		metadata[accel_id].tailptr_tlb_inv = wptr_gpu_addr +
> -						     ualink_tlb_wptr_offset(adev, accel_id);
> +		md->tailptr_ri = wptr_gpu_addr + ualink_wptr_offset(adev, accel_id);
> +		md->tailptr_tlb_inv = wptr_gpu_addr +
> +				      ualink_tlb_wptr_offset(adev, accel_id);
>   
> -		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
> -			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
> +		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, metadata entry %td, rb_size 0x%x\n",
> +			accel_id, ualink_exp_accel_id(adev), md - metadata, md->rb_size);
>   		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
> -			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
> +			md->ri_rb, md->tlb_inv_rb);
>   		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
> -			metadata[accel_id].tailptr_ri, metadata[accel_id].tailptr_tlb_inv);
> +			md->tailptr_ri, md->tailptr_tlb_inv);
>   	}
>   
>   	r = adev->ualink.msg_ctl->send_metadata(adev,
> @@ -6069,6 +6091,12 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   	u64 int_wptr, tlb_wptr;
>   	int r;
>   
> +	/* UALink loopback: both ends of every ring live on this GPU, so there
> +	 * is no remote peer that can reboot and leave the pointers stale.
> +	 */
> +	if (adev->ualink.loopback.enabled)
> +		return 0;
> +
>   	/* ring->wptr and ring->rptr are owned by the send path. */
>   	guard(mutex)(&peer->lock);
>   

[-- Attachment #2: Type: text/html, Size: 6433 bytes --]

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

* Re: [PATCHv3 7/8] drm/amdgpu: Reject config updates in UALink loopback mode
  2026-11-02 15:47 ` [PATCHv3 7/8] drm/amdgpu: Reject config updates in " Mukul Joshi
@ 2026-11-03 23:17   ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:17 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

On 2026-11-02 10:47, Mukul Joshi wrote:
> In loopback mode the driver synthesizes the pPod/vPod configuration,
> since there is no user-space agent to configure the pod. Keep anything
> else from overwriting it:
>
> - Ignore PSP config update interrupts. Re-querying the configuration
>    from PSP would replace the synthesized one. Still send the completion
>    so PSP is not left waiting.
>
> - Don't create the pPod setup, vPod config and station config sysfs
>    nodes, so the configuration can't be committed from user space. The
>    setup, config and stations directories are still created, empty, as
>    they are when the config sysfs nodes are compiled out.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>

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


> ---
> v1->v2:
> - Do not create the config directory in sysfs under loopback mode.(Lijo).
>
> v2->v3:
> - Update amdgpu_ualink_sysfs_init() based on Felix's suggestion.
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 68 +++++++++++++---------
>   1 file changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 2e1e0a677e35..cf2c68cf7115 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -491,6 +491,18 @@ int amdgpu_ualink_config_update_handler(struct amdgpu_device *adev)
>   			PSP_GFX_INT_CTXT_UAL_CMD_CFG_UPDATE_ID, status);
>   	}
>   
> +	/* The loopback pod configuration is synthesized by the driver, so
> +	 * re-querying it from PSP would overwrite it.
> +	 */
> +	if (adev->ualink.loopback.enabled) {
> +		dev_warn(adev->dev,
> +			 "UALINK: config update ignored in loopback mode\n");
> +		return psp_ual_send_completion(&adev->psp,
> +					       adev->ualink.psp_if_ver,
> +					       PSP_GFX_INT_CTXT_UAL_CMD_CFG_UPDATE_ID,
> +					       0);
> +	}
> +
>   	prev_state = adev->ualink.info->accel_state;
>   	prev_vpod_id = adev->ualink.info->vpod.id;
>   
> @@ -1820,36 +1832,40 @@ int amdgpu_ualink_sysfs_init(struct amdgpu_device *adev)
>   		goto err;
>   
>   #ifdef UALINK_ENABLE_DEPRECATED_CONFIG_SYSFS
> -	r = ualink_kobj_add(&adev->ualink.setup->kobj, &info->kobj, "setup",
> -			    ualink_ppod_setup_attrs);
> -	if (r)
> -		goto err_info;
> +	/* The loopback pod configuration is synthesized by the driver. */
> +	if (!adev->ualink.loopback.enabled) {
> +		r = ualink_kobj_add(&adev->ualink.setup->kobj, &info->kobj,
> +				    "setup", ualink_ppod_setup_attrs);
> +		if (r)
> +			goto err_info;
>   
> -	r = ualink_kobj_add(&adev->ualink.config->kobj, &info->kobj, "config",
> -			    ualink_vpod_config_attrs);
> -	if (r)
> -		goto err_setup;
> +		r = ualink_kobj_add(&adev->ualink.config->kobj, &info->kobj,
> +				    "config", ualink_vpod_config_attrs);
> +		if (r)
> +			goto err_setup;
>   
> -	r = ualink_kobj_add(&adev->ualink.stations->kobj, &info->kobj,
> -			    "stations", ualink_station_config_attrs);
> -	if (r)
> -		goto err_config;
> -#else
> -	r = kobject_add(&adev->ualink.setup->kobj, &info->kobj, "%s",
> -			"setup");
> -	if (r)
> -		goto err_info;
> +		r = ualink_kobj_add(&adev->ualink.stations->kobj, &info->kobj,
> +				    "stations", ualink_station_config_attrs);
> +		if (r)
> +			goto err_config;
> +	} else
> +#endif
> +	{
> +		r = kobject_add(&adev->ualink.setup->kobj, &info->kobj, "%s",
> +				"setup");
> +		if (r)
> +			goto err_info;
>   
> -	r = kobject_add(&adev->ualink.config->kobj, &info->kobj, "%s",
> -			"config");
> -	if (r)
> -		goto err_setup;
> +		r = kobject_add(&adev->ualink.config->kobj, &info->kobj, "%s",
> +				"config");
> +		if (r)
> +			goto err_setup;
>   
> -	r = kobject_add(&adev->ualink.stations->kobj, &info->kobj, "%s",
> -			"stations");
> -	if (r)
> -		goto err_config;
> -#endif
> +		r = kobject_add(&adev->ualink.stations->kobj, &info->kobj, "%s",
> +				"stations");
> +		if (r)
> +			goto err_config;
> +	}
>   
>   	amdgpu_ualink_xcp_sysfs_init(adev);
>   	adev->ualink.sysfs_init = true;

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

* Re: [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure
  2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
  2026-11-03 18:17   ` Philip Yang
@ 2026-11-03 23:18   ` Felix Kuehling
  1 sibling, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:18 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

On 2026-11-02 10:47, Mukul Joshi wrote:
> Add the infrastructure for UALink loopback mode, where a single GPU
> plays both the exporter and the importer role under two fixed
> accelerator IDs:
>
> - Add struct amdgpu_ualink_loopback to track the loopback state, along
>    with the fixed exporter and importer accel IDs.
>
> - Replace ualink_accel_id() with ualink_exp_accel_id() and
>    ualink_imp_accel_id(), which return the exporter and importer accel
>    ID in loopback mode.
>
> - Add amdgpu_ualink_loopback_sw_init(), which allocates the ordered
>    workqueue used to emulate exporter->importer messages from
>    ualink_ip_sw_init(), and amdgpu_ualink_loopback_fini(), which
>    destroys it from ualink_ip_sw_fini().
>
> Loopback is not enabled yet, and outside loopback both helpers return
> ppod.accel_id, so there is no functional change.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>

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


> ---
> v1->v2:
> - Drop the change in find_peer_adev() as its not needed.
>
> v2->v3:
> - Update src_accel_id to the correct value in amdgpu_ualink_peer_remote_init()
>    and amdgpu_ualink_peer_remote_fini().
> - Add the loopback teardown in amdgpu_ualink_loopback_fini() and call it
>    through ualink_ip_sw_fini().
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 128 +++++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h |  29 +++++
>   2 files changed, 133 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index ca6432610f71..a98d5bbf0622 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -89,6 +89,41 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +/*
> + * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
> + *
> + * Allocates the ordered workqueue used to emulate exporter->importer
> + * interrupts in SW. Does nothing unless loopback mode is enabled.
> + */
> +static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
> +{
> +	if (!adev->ualink.loopback.enabled)
> +		return 0;
> +
> +	adev->ualink.loopback.wq =
> +		alloc_ordered_workqueue("amdgpu_ualink_loopback", 0);
> +	if (!adev->ualink.loopback.wq)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_fini - tear down UALink loopback mode
> + *
> + * Destroys the workqueue allocated in amdgpu_ualink_loopback_sw_init().
> + * destroy_workqueue() drains any pending self-interrupt work first.
> + */
> +static void amdgpu_ualink_loopback_fini(struct amdgpu_device *adev)
> +{
> +	if (!adev->ualink.loopback.wq)
> +		return;
> +
> +	destroy_workqueue(adev->ualink.loopback.wq);
> +	adev->ualink.loopback.wq = NULL;
> +	adev->ualink.loopback.enabled = false;
> +}
> +
>   static void amdgpu_ualink_object_fini(struct amdgpu_device *adev)
>   {
>   	if (!adev->ualink.info)
> @@ -1028,6 +1063,31 @@ static struct amdgpu_device *find_peer_adev(unsigned int accel_id)
>   	return NULL;
>   }
>   
> +/*
> + * Physical accelerator roles.
> + *
> + * Outside loopback a GPU owns a single accelerator, so both helpers return its
> + * ppod.accel_id and the exporter/importer distinction collapses. In loopback
> + * the one physical GPU plays both roles: the exporter owns the MP nHT and
> + * receives all HW traffic, while the importer is the (virtual) HW sender for
> + * the importer -> exporter path.
> + */
> +static inline u32 ualink_exp_accel_id(struct amdgpu_device *adev)
> +{
> +	if (adev->ualink.loopback.enabled)
> +		return adev->ualink.loopback.exporter_accel_id;
> +
> +	return adev->ualink.info->ppod.accel_id;
> +}
> +
> +static inline u32 ualink_imp_accel_id(struct amdgpu_device *adev)
> +{
> +	if (adev->ualink.loopback.enabled)
> +		return adev->ualink.loopback.importer_accel_id;
> +
> +	return adev->ualink.info->ppod.accel_id;
> +}
> +
>   static bool amdgpu_ualink_is_local_accel(struct amdgpu_device *adev,
>   					 u32 accel_id)
>   {
> @@ -1533,19 +1593,21 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>   	struct amdgpu_ualink_ppod_setup *ppod_setup;
>   	struct amdgpu_device *adev = ip_block->adev;
>   	struct amdgpu_ualink_info *info;
> +	int r;
>   
>   	info = kzalloc(sizeof(*info), GFP_KERNEL);
>   	ppod_setup = kzalloc(sizeof(*ppod_setup), GFP_KERNEL);
>   	vpod_config = kzalloc(sizeof(*vpod_config), GFP_KERNEL);
>   	stations = kzalloc(sizeof(*stations), GFP_KERNEL);
>   	if (!info || !ppod_setup || !vpod_config || !stations) {
> -		kfree(info);
> -		kfree(ppod_setup);
> -		kfree(vpod_config);
> -		kfree(stations);
> -		return -ENOMEM;
> +		r = -ENOMEM;
> +		goto err_free;
>   	}
>   
> +	r = amdgpu_ualink_loopback_sw_init(adev);
> +	if (r)
> +		goto err_free;
> +
>   	info->ppod.accel_id = 0xffffffff;
>   	info->ppod.bandwidth = 0xffffffff;
>   	info->ppod.latency = 0xffffffff;
> @@ -1568,10 +1630,18 @@ int ualink_ip_sw_init(struct amdgpu_ip_block *ip_block)
>   	adev->ualink.stations = stations;
>   
>   	return 0;
> +
> +err_free:
> +	kfree(info);
> +	kfree(ppod_setup);
> +	kfree(vpod_config);
> +	kfree(stations);
> +	return r;
>   }
>   
>   int ualink_ip_sw_fini(struct amdgpu_ip_block *ip_block)
>   {
> +	amdgpu_ualink_loopback_fini(ip_block->adev);
>   	amdgpu_ualink_object_fini(ip_block->adev);
>   	return 0;
>   }
> @@ -4487,11 +4557,6 @@ static bool amdgpu_ualink_vpod_membership_changed(struct amdgpu_device *adev)
>   			     AMDGPU_UALINK_ACCEL_MAX);
>   }
>   
> -static inline u32 ualink_accel_id(struct amdgpu_device *adev)
> -{
> -	return adev->ualink.info->ppod.accel_id;
> -}
> -
>   static inline enum amdgpu_ualink_addr_mode ualink_addr_mode(struct amdgpu_device *adev)
>   {
>   	return adev->ualink.info->vpod.addr_mode;
> @@ -4837,7 +4902,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>   	u64 timeout = msecs_to_jiffies(2000);
> -	u32 dst_accel_id = ualink_accel_id(adev);
> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>   	struct amdgpu_bo *bos[2];
>   	u32 n_bos;
>   	struct dma_fence *fence;
> @@ -4956,7 +5021,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
>   static int amdgpu_ualink_metadata_npa_mapping(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 dst_accel_id = ualink_accel_id(adev);
> +	u32 dst_accel_id = ualink_exp_accel_id(adev);
>   	u64 timeout = msecs_to_jiffies(2000);
>   	struct amdgpu_bo *bos[2];
>   	u32 n_bos;
> @@ -5241,7 +5306,7 @@ static void amdgpu_ualink_metadata_fini(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
>   
> -	dev_dbg(adev->dev, "accel_id %u\n", ualink_accel_id(adev));
> +	dev_dbg(adev->dev, "accel_id %u\n", ualink_exp_accel_id(adev));
>   	amdgpu_bo_free_kernel(&remote->rptr_bo, &remote->rptr_gpu_addr,
>   				      &remote->rptr_cpu_addr);
>   	amdgpu_bo_free_kernel(&remote->ring_bo, &remote->rb_gpu_addr,
> @@ -5288,7 +5353,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   	}
>   
>   	dev_dbg(adev->dev, "accel_id %u addr_mode %d fw status 0x%x\n",
> -		ualink_accel_id(adev), ualink_addr_mode(adev), status);
> +		ualink_exp_accel_id(adev), ualink_addr_mode(adev), status);
>   
>   	/* Alloc metadata structure for all GPUs */
>   	metadata_size = sizeof(struct amdgpu_ualink_metadata) * AMDGPU_UALINK_ACCEL_MAX;
> @@ -5361,12 +5426,12 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   	}
>   
>   	if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS)
> -		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_accel_id(adev);
> +		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>   
>   	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
>   		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>   
> -		if (accel_id == ualink_accel_id(adev))
> +		if (accel_id == ualink_exp_accel_id(adev))
>   			continue;
>   
>   		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
> @@ -5392,7 +5457,7 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>   						     ualink_tlb_wptr_offset(adev, accel_id);
>   
>   		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
> -			accel_id, ualink_accel_id(adev), metadata[accel_id].rb_size);
> +			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
>   		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
>   			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
>   		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
> @@ -5806,7 +5871,14 @@ static void amdgpu_ualink_get_wb_addr(struct amdgpu_device *adev,
>   				      u64 *wb_npa, u32 type)
>   {
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 accel_id = ualink_accel_id(adev);
> +	/* The GPUID encoded in the write-back NPA address is the routing target
> +	 * the remote writes the rptr back to, i.e. THIS GPU. It must be the
> +	 * physically-owned accel_id: outside loopback that is our own accel_id,
> +	 * and in loopback it is the exporter (the only id the single GPU owns
> +	 * and can receive on). The importer is only a logical source id for the
> +	 * forward path and must never appear as an NPA GPUID.
> +	 */
> +	u32 accel_id = ualink_exp_accel_id(adev);
>   	u32 rb_size = AMDGPU_GPU_PAGE_ALIGN(2 * AMDGPU_UALINK_RB_SIZE);
>   	uintptr_t wb;
>   	u64 npa;
> @@ -5981,19 +6053,19 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   						   &peer->shootdown, &tlb_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);
> +			ualink_exp_accel_id(adev), remote_accel_id, r);
>   		return r;
>   	}
>   
>   	if (peer->interrupt.wptr == int_wptr && peer->shootdown.wptr == tlb_wptr) {
>   		dev_dbg(adev->dev, "accel_id %u wptr 0x%llx 0x%llx is same as remote %u\n",
> -			ualink_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
> +			ualink_exp_accel_id(adev), int_wptr, tlb_wptr, remote_accel_id);
>   		return 0;
>   	}
>   
>   	dev_dbg(adev->dev,
>   		"accel_id %u wptr 0x%llx 0x%llx sync to remote %u wptr 0x%llx 0x%llx\n",
> -		ualink_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
> +		ualink_exp_accel_id(adev), peer->interrupt.wptr, peer->shootdown.wptr,
>   		remote_accel_id, int_wptr, tlb_wptr);
>   
>   	amdgpu_ualink_get_wb_addr(adev, remote_accel_id, &int_wb, NULL,
> @@ -6164,7 +6236,12 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
>   	struct amdgpu_ualink_peer *peer;
>   	struct amdgpu_ualink_ring *interrupt;
>   	struct amdgpu_ualink_ring *shootdown;
> -	u32 src_accel_id = ualink_accel_id(adev);
> +	/* This sets up the send rings (local sender -> each remote dst). In
> +	 * loopback the HW-served direction is importer -> exporter, so the
> +	 * local sender is the importer; the skip below then skips the importer
> +	 * and sets up the peer[exporter] ring that the HW path needs.
> +	 */
> +	u32 src_accel_id = ualink_imp_accel_id(adev);
>   	u32 dst_accel_id;
>   	u64 npa, flags;
>   	int r = 0;
> @@ -6287,7 +6364,10 @@ static void amdgpu_ualink_peer_remote_fini(struct amdgpu_device *adev)
>   {
>   	u32 rb_pages = AMDGPU_UALINK_RB_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
>   	struct amdgpu_ualink_remote *remote = to_remote(adev);
> -	u32 src_accel_id = ualink_accel_id(adev);
> +	/* Mirror peer_remote_init: in loopback the local sender is the
> +	 * importer, so unmap the peer[exporter] send ring that was set up.
> +	 */
> +	u32 src_accel_id = ualink_imp_accel_id(adev);
>   	u32 dst_accel_id;
>   	struct amdgpu_ualink_peer *peer;
>   	struct amdgpu_ualink_ring *ring;
> @@ -6374,7 +6454,7 @@ int amdgpu_ualink_sw_init(struct amdgpu_device *adev)
>    */
>   void amdgpu_ualink_sw_fini(struct amdgpu_device *adev)
>   {
> -	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_accel_id(adev),
> +	dev_dbg(adev->dev, "halt accel_id %u addr_mode %d\n", ualink_exp_accel_id(adev),
>   		ualink_addr_mode(adev));
>   
>   	adev->ualink.msg_ctl->send_halt(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> index 85c9a9634a4b..0b1e8dd38666 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.h
> @@ -97,6 +97,17 @@ enum AMDGPU_UALINK_PROTOCOL_MESSAGES {
>   #define AMDGPU_UALINK_LIGHTWEIGHT_TLB_SHOOTDOWN		0x1
>   #define AMDGPU_UALINK_HEAVYWEIGHT_TLB_SHOOTDOWN		0x2
>   
> +/* UALink loopback support.
> + *
> + * In loopback mode a single GPU loops UALink requests back to itself for
> + * manufacturing testing. The GPU exposes two fixed accelerator IDs: it acts
> + * as the exporter on one and the importer on the other. Loopback is enabled
> + * with the amdgpu.ualink_loopback module parameter.
> + */
> +/* Fixed accelerator IDs used by a loopback GPU. */
> +#define AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID		1
> +#define AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID		0
> +
>   enum amdgpu_ualink_conn_state {
>   	AMDGPU_UALINK_CONN_NOT_READY			= 0,
>   	AMDGPU_UALINK_CONN_IN_PROGRESS			= 1,
> @@ -319,6 +330,21 @@ struct amdgpu_ualink_msg_ctl {
>   	int (*send_halt)(struct amdgpu_device *adev);
>   };
>   
> +/* UALink loopback mode state.
> + *
> + * When @enabled is true this single GPU acts as both exporter
> + * (@exporter_accel_id) and importer (@importer_accel_id). There is no second
> + * physical GPU, so exporter->importer messages cannot be delivered by the
> + * (single) MP nHT and are instead emulated in SW by feeding them back into
> + * our own IRQ handler via @wq.
> + */
> +struct amdgpu_ualink_loopback {
> +	bool enabled;
> +	u32 exporter_accel_id;
> +	u32 importer_accel_id;
> +	struct workqueue_struct *wq;
> +};
> +
>   struct amdgpu_ualink_mgr {
>   	u64 npa_size;
>   	u32 psp_if_ver;
> @@ -333,6 +359,9 @@ struct amdgpu_ualink_mgr {
>   	/* For remote interrupt and shootdown */
>   	struct amdgpu_ualink_remote *remote;
>   
> +	/* UALink loopback mode state (see struct amdgpu_ualink_loopback) */
> +	struct amdgpu_ualink_loopback loopback;
> +
>   	/* handle irq from ualink client of remote GPUs */
>   	struct amdgpu_irq_src irq;
>   

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

* Re: [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
  2026-11-03 22:30   ` Philip Yang
@ 2026-11-03 23:21   ` Felix Kuehling
  2026-11-04 16:10   ` Philip Yang
  2 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:21 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter and the
> importer. The exporter is the one physical station, so the only path
> the HW serves is importer -> exporter. A message the exporter sends to
> the importer cannot be delivered by the MP nHT.
>
> Emulate that direction in SW: when loopback is enabled and a remote
> interrupt is addressed to the importer, queue it on the loopback
> ordered workqueue, which feeds it back into amdgpu_ualink_process_irq()
> as if the exporter had sent it. The ordered workqueue keeps successive
> messages in order and runs the (possibly sleeping) handlers outside
> the sender's context.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>

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


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 82 ++++++++++++++++++++++
>   1 file changed, 82 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 29a787979af2..ef133c60bb9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -60,6 +60,9 @@ static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
>   					   u32 remote_accel_id);
>   static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   					  u32 remote_accel_id);
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3);
>   
>   #define STRIP_NPA(addr)						\
>   	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
> @@ -6220,6 +6223,17 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
>   	struct amdgpu_ualink_ring *ring;
>   	int r;
>   
> +	/* UALink loopback: the single MP nHT can only deliver messages addressed
> +	 * to the exporter accelerator. Messages addressed to the importer
> +	 * (exporter->importer direction) cannot be delivered by HW, so emulate
> +	 * them by feeding the message back into our own IRQ handler.
> +	 */
> +	if (adev->ualink.loopback.enabled &&
> +	    remote_accel_id == ualink_imp_accel_id(adev))
> +		return amdgpu_ualink_loopback_self_interrupt(adev,
> +				ualink_exp_accel_id(adev),
> +				dw0, dw1, dw2, dw3);
> +
>   	peer = &remote->peer[remote_accel_id];
>   	ring = &peer->interrupt;
>   
> @@ -6649,6 +6663,74 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
>   	return handled;
>   }
>   
> +/* Work item used to emulate an exporter->importer UALink interrupt in
> + * loopback mode (see amdgpu_ualink_loopback_self_interrupt()).
> + */
> +struct amdgpu_ualink_self_irq_work {
> +	struct work_struct work;
> +	struct amdgpu_device *adev;
> +	u32 sender_accel_id;
> +	u32 dw[4];
> +};
> +
> +static void amdgpu_ualink_self_irq_worker(struct work_struct *w)
> +{
> +	struct amdgpu_ualink_self_irq_work *job =
> +		container_of(w, struct amdgpu_ualink_self_irq_work, work);
> +	struct amdgpu_device *adev = job->adev;
> +	struct amdgpu_iv_entry entry = { };
> +
> +	/* Build an IH entry as if the message had been delivered by the MP nHT
> +	 * from the exporter accelerator. amdgpu_ualink_process_irq() consumes
> +	 * the sender accel_id from entry->pasid and the four context dwords
> +	 * from entry->src_data[]. entry->ih must differ from adev->irq.ih so
> +	 * the message is processed here instead of being delegated to the
> +	 * UALink soft ring.
> +	 */
> +	entry.ih = NULL;
> +	entry.pasid = job->sender_accel_id;
> +	entry.src_data[0] = job->dw[0];
> +	entry.src_data[1] = job->dw[1];
> +	entry.src_data[2] = job->dw[2];
> +	entry.src_data[3] = job->dw[3];
> +
> +	amdgpu_ualink_process_irq(adev, &adev->ualink.irq, &entry);
> +	kfree(job);
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_self_interrupt - deliver a UALink message to ourselves
> + *
> + * Used in loopback mode for the exporter->importer direction, where there is
> + * no HW path. The message is processed asynchronously on an ordered workqueue
> + * so ordering between successive messages is preserved and the (potentially
> + * sleeping) message handlers do not run from the caller's context.
> + */
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3)
> +{
> +	struct amdgpu_ualink_self_irq_work *job;
> +
> +	if (!adev->ualink.loopback.wq)
> +		return -ENODEV;
> +
> +	job = kzalloc(sizeof(*job), GFP_KERNEL);
> +	if (!job)
> +		return -ENOMEM;
> +
> +	job->adev = adev;
> +	job->sender_accel_id = sender_accel_id;
> +	job->dw[0] = dw0;
> +	job->dw[1] = dw1;
> +	job->dw[2] = dw2;
> +	job->dw[3] = dw3;
> +	INIT_WORK(&job->work, amdgpu_ualink_self_irq_worker);
> +	queue_work(adev->ualink.loopback.wq, &job->work);
> +
> +	return 0;
> +}
> +
>   static int amdgpu_ualink_set_irq_state(struct amdgpu_device *adev,
>   					struct amdgpu_irq_src *source,
>   					u32 type,

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

* Re: [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode
  2026-11-02 15:47 ` [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode Mukul Joshi
@ 2026-11-03 23:29   ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2026-11-03 23:29 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang

On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter (accel
> ID 1) and the importer (accel ID 0). This lets the existing tests export
> a BO and import it on the same device, traversing the UALink HW
> pipeline.
>
> Add amdgpu_ualink_loopback_detect(), the single place that decides
> whether loopback is enabled. It runs from sw_init, before any per-peer
> setup, so the loopback workqueue is allocated with the other one-time
> allocations. When loopback is enabled:
>
> - There is no user-space agent to configure the pod, so synthesize the
>    pPod/vPod configuration with both accel IDs active instead of
>    querying it from PSP. Loopback only supports the source-aliasing
>    address mode.
>
> - Activate the device directly instead of through the cross-GPU vpod
>    activation path, which needs peer GPUs and a user-space vpod commit.
>
> The mechanism that requests loopback mode is added separately, so
> amdgpu_ualink_loopback_detect() leaves loopback disabled for now. The
> normal activation path is unchanged when loopback is not enabled.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com> (v2)

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


> ---
> v2->v3:
> - Update all static info inside amdgpu_ualink_loopback_populate_info().
> - Drop the amdgpu_ualink_loopback_requested() as amdgpu_ualink_loopback_detect()
>    is enough to detect and set loopback mode.
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 143 +++++++++++++++++++--
>   1 file changed, 132 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 74bc8a907f5b..2e1e0a677e35 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -63,6 +63,7 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
>   						 u32 sender_accel_id, u32 dw0,
>   						 u32 dw1, u32 dw2, u32 dw3);
> +static void activate_accelerator(struct amdgpu_device *adev);
>   
>   #define STRIP_NPA(addr)						\
>   	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
> @@ -93,13 +94,120 @@ static int amdgpu_ualink_drm_client_create(struct amdgpu_device *adev)
>   }
>   
>   /*
> - * amdgpu_ualink_loopback_sw_init - allocate UALink loopback resources
> + * amdgpu_ualink_loopback_detect - detect UALink loopback mode
>    *
> - * Allocates the ordered workqueue used to emulate exporter->importer
> - * interrupts in SW. Does nothing unless loopback mode is enabled.
> + * This is the only place that decides whether loopback mode is enabled.
> + * There is no mechanism to request loopback mode yet, so it stays disabled.
> + */
> +static void amdgpu_ualink_loopback_detect(struct amdgpu_device *adev)
> +{
> +	adev->ualink.loopback.enabled = false;
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_populate_info - synthesize pPod/vPod config for loopback
> + *
> + * In loopback there is no user-space agent (AIFM / xncmdclient) writing the
> + * UALink sysfs nodes to configure the pod, so populate adev->ualink.info
> + * directly. The layout is fixed and known: a single GPU alone in its own
> + * physical pod, presented as accelerator ID 1 (the exporter) and also owning
> + * accelerator ID 0 (the importer). The values are made self-consistent so
> + * check_vpod_info() would pass and amdgpu_ualink_metadata_init() programs the
> + * importer's ring buffers / GART NPA mappings / MP nHT receive metadata.
> + *
> + * This must run before any per-peer setup, since it sets the accelerator IDs
> + * returned by ualink_exp_accel_id() and ualink_imp_accel_id().
> + */
> +static void amdgpu_ualink_loopback_populate_info(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_ualink_info *info = adev->ualink.info;
> +	u32 exporter = AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID;
> +	u32 importer = AMDGPU_UALINK_LOOPBACK_IMPORTER_ACCID;
> +
> +	adev->ualink.loopback.exporter_accel_id = exporter;
> +	adev->ualink.loopback.importer_accel_id = importer;
> +
> +	/* Physical pod: this GPU is presented as the exporter accel ID in its
> +	 * own pod. ppod.size must be large enough to contain both the exporter
> +	 * and importer accel IDs. ppod.id must be unique within the system
> +	 * (matters for 1P4G, where every loopback GPU is its own physical pod).
> +	 */
> +	info->ppod.accel_id = exporter;
> +	info->ppod.size = max(exporter, importer) + 1;	/* == 2 */
> +	uuid_gen(&info->ppod.id);
> +
> +	/* Virtual pod: mark BOTH the exporter and the importer active so the
> +	 * per-peer setup (amdgpu_ualink_metadata_init()) programs the importer's
> +	 * resources for the importer accel ID. Loopback only supports the
> +	 * source-aliasing address mode.
> +	 */
> +	info->vpod.id = 0;
> +	info->vpod.size = 2;
> +	info->vpod.addr_mode = AMDGPU_UALINK_ADDR_MODE_SOURCE_ALIAS;
> +	bitmap_zero(info->vpod.active_accel_bits, AMDGPU_UALINK_ACCEL_MAX);
> +	set_bit(exporter, info->vpod.active_accel_bits);
> +	set_bit(importer, info->vpod.active_accel_bits);
> +
> +	/* Only the exporter (this GPU's own accel ID) is a local accelerator;
> +	 * the importer is reached through the (loopback) remote/NPA path.
> +	 */
> +	info->n_local_accels = 1;
> +	info->local_accels[0] = exporter;
> +
> +	/* Mark the pod vpod-configured so the activation path proceeds. */
> +	info->accel_state = AMDGPU_UALINK_ACCEL_STATE_VPOD_CONFIGURED;
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_activate - bring up the UALink manager for loopback
> + *
> + * The normal activation is driven by the sysfs vpod_config/commit path via
> + * activate_local_vpod(), which derives the local accelerators from peer GPUs
> + * and runs a cross-GPU integrity check. Neither applies in loopback (the
> + * importer is not a separate GPU), so activate this device directly.
> + */
> +static int amdgpu_ualink_loopback_activate(struct amdgpu_device *adev)
> +{
> +	mutex_lock(&mgpu_info.mutex);
> +	activate_accelerator(adev);
> +	mutex_unlock(&mgpu_info.mutex);
> +
> +	if (adev->ualink.info->accel_state < AMDGPU_UALINK_ACCEL_STATE_ACTIVE) {
> +		dev_err(adev->dev, "UALink loopback activation did not reach ACTIVE\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_init - set up and activate UALink loopback mode
> + *
> + * Called from late init once loopback mode has been detected. Synthesizes the
> + * pod configuration instead of querying it from PSP (there is no user-space
> + * agent to configure the pod in loopback) and activates the device.
> + */
> +static int amdgpu_ualink_loopback_init(struct amdgpu_device *adev)
> +{
> +	amdgpu_ualink_loopback_populate_info(adev);
> +
> +	dev_info(adev->dev,
> +		 "UALink loopback enabled (exporter accel %u, importer accel %u)\n",
> +		 adev->ualink.loopback.exporter_accel_id,
> +		 adev->ualink.loopback.importer_accel_id);
> +
> +	return amdgpu_ualink_loopback_activate(adev);
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_sw_init - detect loopback and allocate its resources
> + *
> + * Detects loopback mode and, if it is enabled, allocates the ordered
> + * workqueue used to emulate exporter->importer interrupts in SW.
>    */
>   static int amdgpu_ualink_loopback_sw_init(struct amdgpu_device *adev)
>   {
> +	amdgpu_ualink_loopback_detect(adev);
>   	if (!adev->ualink.loopback.enabled)
>   		return 0;
>   
> @@ -499,15 +607,28 @@ int ualink_ip_late_init(struct amdgpu_ip_block *ip_block)
>   	if (adev->ualink.mgr_state != AMDGPU_UALINK_INIT_HW)
>   		return 0;
>   
> -	r = psp_ual_query_info(&adev->psp, adev->ualink.psp_if_ver,
> -			       adev->ualink.info, &cfg_state);
> -	if (r)
> -		return r;
> +	/* Loopback synthesizes its pod configuration instead of querying it
> +	 * from PSP, and has no peer GPUs or user-space vpod commit, so it
> +	 * cannot use the normal cross-GPU activation path either.
> +	 */
> +	if (!adev->ualink.loopback.enabled) {
> +		r = psp_ual_query_info(&adev->psp, adev->ualink.psp_if_ver,
> +				       adev->ualink.info, &cfg_state);
> +		if (r)
> +			return r;
>   
> -	scoped_guard(mutex, &mgpu_info.mutex) {
> -		amdgpu_ualink_info_set_accel_state(adev, adev->ualink.info,
> -						   cfg_state);
> -		__amdgpu_ualink_activate_vpod_locked(adev);
> +		scoped_guard(mutex, &mgpu_info.mutex) {
> +			amdgpu_ualink_info_set_accel_state(adev,
> +							   adev->ualink.info,
> +							   cfg_state);
> +			__amdgpu_ualink_activate_vpod_locked(adev);
> +		}
> +	} else {
> +		r = amdgpu_ualink_loopback_init(adev);
> +		if (r) {
> +			dev_err(adev->dev, "Failed to set up UALink loopback: %d\n", r);
> +			goto error;
> +		}
>   	}
>   
>   	r = amdgpu_ualink_drm_client_create(adev);

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

* Re: [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
  2026-11-03 22:30   ` Philip Yang
  2026-11-03 23:21   ` Felix Kuehling
@ 2026-11-04 16:10   ` Philip Yang
  2026-11-04 20:31     ` Mukul Joshi
  2 siblings, 1 reply; 24+ messages in thread
From: Philip Yang @ 2026-11-04 16:10 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang

[-- Attachment #1: Type: text/plain, Size: 5368 bytes --]



On 2026-11-02 10:47, Mukul Joshi wrote:
> In UALink loopback mode a single GPU acts as both the exporter and the
> importer. The exporter is the one physical station, so the only path
> the HW serves is importer -> exporter. A message the exporter sends to
> the importer cannot be delivered by the MP nHT.
>
> Emulate that direction in SW: when loopback is enabled and a remote
> interrupt is addressed to the importer, queue it on the loopback
> ordered workqueue, which feeds it back into amdgpu_ualink_process_irq()
> as if the exporter had sent it. The ordered workqueue keeps successive
> messages in order and runs the (possibly sleeping) handlers outside
> the sender's context.
>
> Signed-off-by: Mukul Joshi<mukul.joshi@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 82 ++++++++++++++++++++++
>   1 file changed, 82 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 29a787979af2..ef133c60bb9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -60,6 +60,9 @@ static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
>   					   u32 remote_accel_id);
>   static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>   					  u32 remote_accel_id);
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3);
>   
>   #define STRIP_NPA(addr)						\
>   	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
> @@ -6220,6 +6223,17 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
>   	struct amdgpu_ualink_ring *ring;
>   	int r;
>   
> +	/* UALink loopback: the single MP nHT can only deliver messages addressed
> +	 * to the exporter accelerator. Messages addressed to the importer
> +	 * (exporter->importer direction) cannot be delivered by HW, so emulate
> +	 * them by feeding the message back into our own IRQ handler.
> +	 */
> +	if (adev->ualink.loopback.enabled &&
> +	    remote_accel_id == ualink_imp_accel_id(adev))
> +		return amdgpu_ualink_loopback_self_interrupt(adev,
> +				ualink_exp_accel_id(adev),
> +				dw0, dw1, dw2, dw3);
> +
>   	peer = &remote->peer[remote_accel_id];
>   	ring = &peer->interrupt;
>   
> @@ -6649,6 +6663,74 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
>   	return handled;
>   }
>   
> +/* Work item used to emulate an exporter->importer UALink interrupt in
> + * loopback mode (see amdgpu_ualink_loopback_self_interrupt()).
> + */
> +struct amdgpu_ualink_self_irq_work {
> +	struct work_struct work;
> +	struct amdgpu_device *adev;
> +	u32 sender_accel_id;
> +	u32 dw[4];
> +};
> +
> +static void amdgpu_ualink_self_irq_worker(struct work_struct *w)
> +{
> +	struct amdgpu_ualink_self_irq_work *job =
> +		container_of(w, struct amdgpu_ualink_self_irq_work, work);
> +	struct amdgpu_device *adev = job->adev;
> +	struct amdgpu_iv_entry entry = { };
> +
> +	/* Build an IH entry as if the message had been delivered by the MP nHT
> +	 * from the exporter accelerator. amdgpu_ualink_process_irq() consumes
> +	 * the sender accel_id from entry->pasid and the four context dwords
> +	 * from entry->src_data[]. entry->ih must differ from adev->irq.ih so
> +	 * the message is processed here instead of being delegated to the
> +	 * UALink soft ring.
> +	 */
> +	entry.ih = NULL;
Looking at this again, it seems we could use adev->irq.ih here to 
simulate the message arriving from the real hardware interrupt ring. It 
would then be delegated to the UALink soft ring and handled in the 
interrupt worker, which would let us drop the job structure and 
amdgpu_ualink_self_irq_worker entirely.

The current approach works fine, but that would be simpler and avoid 
introducing another job and work item.

Regards,
Philip
> +	entry.pasid = job->sender_accel_id;
> +	entry.src_data[0] = job->dw[0];
> +	entry.src_data[1] = job->dw[1];
> +	entry.src_data[2] = job->dw[2];
> +	entry.src_data[3] = job->dw[3];
> +
> +	amdgpu_ualink_process_irq(adev, &adev->ualink.irq, &entry);
> +	kfree(job);
> +}
> +
> +/*
> + * amdgpu_ualink_loopback_self_interrupt - deliver a UALink message to ourselves
> + *
> + * Used in loopback mode for the exporter->importer direction, where there is
> + * no HW path. The message is processed asynchronously on an ordered workqueue
> + * so ordering between successive messages is preserved and the (potentially
> + * sleeping) message handlers do not run from the caller's context.
> + */
> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
> +						 u32 sender_accel_id, u32 dw0,
> +						 u32 dw1, u32 dw2, u32 dw3)
> +{
> +	struct amdgpu_ualink_self_irq_work *job;
> +
> +	if (!adev->ualink.loopback.wq)
> +		return -ENODEV;
> +
> +	job = kzalloc(sizeof(*job), GFP_KERNEL);
> +	if (!job)
> +		return -ENOMEM;
> +
> +	job->adev = adev;
> +	job->sender_accel_id = sender_accel_id;
> +	job->dw[0] = dw0;
> +	job->dw[1] = dw1;
> +	job->dw[2] = dw2;
> +	job->dw[3] = dw3;
> +	INIT_WORK(&job->work, amdgpu_ualink_self_irq_worker);
> +	queue_work(adev->ualink.loopback.wq, &job->work);
> +
> +	return 0;
> +}
> +
>   static int amdgpu_ualink_set_irq_state(struct amdgpu_device *adev,
>   					struct amdgpu_irq_src *source,
>   					u32 type,

[-- Attachment #2: Type: text/html, Size: 5904 bytes --]

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

* Re: [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback
  2026-11-03 23:15   ` Felix Kuehling
@ 2026-11-04 20:20     ` Mukul Joshi
  2026-11-05 22:13       ` Kuehling, Felix
  0 siblings, 1 reply; 24+ messages in thread
From: Mukul Joshi @ 2026-11-04 20:20 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang


On 11/3/2026 6:15 PM, Felix Kuehling wrote:
> On 2026-11-02 10:47, Mukul Joshi wrote:
>> In UALink loopback mode a single GPU acts as both the exporter and the
>> importer, and the only path the HW serves is importer -> exporter.
>> Adjust the per-peer setup accordingly:
>>
>> - The importer's doorbell reaches the MP nHT stamped with the GPU ID of
>>   the exporter, and the FW looks up the metadata entry by that ID. So
>>   publish the importer's rings and tail pointers in metadata[exporter].
>>
>> - Skip the ring pointer resync. Both ends of every ring are on this
>>   GPU, so there is no remote peer that can reboot and leave the
>>   pointers stale.
>>
>> - Do not take the local dma-buf shortcut on import. It would bypass
>>   the NPA path that loopback is meant to exercise.
>>
>> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
>> ---
>> v2->v3:
>> - Move updating src_accel_id in peer_remote_init and fini to patch 1.
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 54 ++++++++++++++++------
>>  1 file changed, 41 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> index 8558dc1a9e84..74bc8a907f5b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> @@ -3976,7 +3976,13 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
>>  		return -EINVAL;
>>  	}
>>  
>> -	if (amdgpu_ualink_is_local_accel(adev, remote_acc_id))
>> +	/* In loopback mode the exporter and importer are the same GPU, but the
>> +	 * import must still go through the full cross-domain NPA path (the
>> +	 * dma_buf shortcut in amdgpu_ualink_local_import() bypasses NPA). Only
>> +	 * take the local shortcut for genuine same-pod local accelerators.
>> +	 */
>> +	if (!adev->ualink.loopback.enabled &&
>> +	    amdgpu_ualink_is_local_accel(adev, remote_acc_id))
>>  		return amdgpu_ualink_local_import(adev, remote_acc_id,
>>  						  &handle, fd_out);
>>  
>> @@ -5452,11 +5458,28 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>  		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>>  
>>  	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
>> +		struct amdgpu_ualink_metadata *md;
>> +
>>  		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>>  
>>  		if (accel_id == ualink_exp_accel_id(adev))
>>  			continue;
>
> I think you could drop the condition just below if you change this to
>
> 		if (accel_id == ualink_imp_accel_id(adev))
> 			continue;
>
> It makes no difference if loopback is disabled. And with loopback
> enabled it's the more obvious way to only initialize the ring for the
> exporter ID. Then all the other changes in this function are unnecessary.
>
Unfortunately, this won't work.

In the normal (non-loopback) case, when AccId 0 sends a message to AccId
1, it writes the message to the ring buffer and wptr NPA addresses that
AccId 1 set up for source AccId 0, and then rings the doorbell. 

The remote interrupt the MPNHT on AccId 1 receives carries AccId 0 as
the source, and the MPNHT FW uses that to look up metadata[0], which
holds the ring buffer and wptr addresses for AccId 0.

With loopback it's slightly different. AccId 0 (the importer) still
writes to the ring buffer and wptr NPA addresses for source AccId 0 and
rings the doorbell.

However, the interrupt the MPNHT gets reports the source as AccId 1 (the
exporter).

So the MPNHT FW looks up metadata[1], and for it to find the right
buffers we have to program the importer's (AccId 0) ring buffer and wptr
addresses at index 1.

If we skipped the importer and ran the loop for the exporter instead,
metadata[1] would be filled with AccId 1's own ring buffer and wptr
addresses, which the importer never writes to.

That's why the patch builds the entry from the importer's addresses but
stores it at the exporter's index.


Regards,

Mukul

> Regards,
>   Felix
>
>
>>  
>> +		/* In loopback the single MP nHT services the exporter half:
>> +		 * the only messages delivered over HW are importer -> exporter
>> +		 * (source = importer accel_id), while the exporter -> importer
>> +		 * direction is emulated in SW and never reaches the MP nHT.
>> +		 *
>> +		 * The importer's doorbell reaches the MP nHT stamped with the
>> +		 * GPU ID of the single physical station, the exporter, so the
>> +		 * FW looks up metadata[exporter]. Publish the importer's rings
>> +		 * and tail pointers in that entry.
>> +		 */
>> +		if (adev->ualink.loopback.enabled)
>> +			md = &metadata[ualink_exp_accel_id(adev)];
>> +		else
>> +			md = &metadata[accel_id];
>> +
>>  		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
>>  
>>  		if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
>> @@ -5464,27 +5487,26 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>  			wptr_gpu_addr += wptr_size * accel_id;
>>  		}
>>  
>> -		metadata[accel_id].header = AMDGPU_UALINK_METADATA_HEADER;
>> -		metadata[accel_id].rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
>> +		md->header = AMDGPU_UALINK_METADATA_HEADER;
>> +		md->rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
>>  
>>  		/*
>>  		 * remote shootdown type is 0, remote command type is 1
>>  		 * with 2MB ring buffer mapping, remote shootdown ring NPA is before interrupt
>>  		 */
>> -		metadata[accel_id].ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
>> -		metadata[accel_id].tlb_inv_rb = rb_gpu_addr;
>> +		md->ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
>> +		md->tlb_inv_rb = rb_gpu_addr;
>>  
>> -		metadata[accel_id].tailptr_ri = wptr_gpu_addr +
>> -						ualink_wptr_offset(adev, accel_id);
>> -		metadata[accel_id].tailptr_tlb_inv = wptr_gpu_addr +
>> -						     ualink_tlb_wptr_offset(adev, accel_id);
>> +		md->tailptr_ri = wptr_gpu_addr + ualink_wptr_offset(adev, accel_id);
>> +		md->tailptr_tlb_inv = wptr_gpu_addr +
>> +				      ualink_tlb_wptr_offset(adev, accel_id);
>>  
>> -		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
>> -			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
>> +		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, metadata entry %td, rb_size 0x%x\n",
>> +			accel_id, ualink_exp_accel_id(adev), md - metadata, md->rb_size);
>>  		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
>> -			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
>> +			md->ri_rb, md->tlb_inv_rb);
>>  		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
>> -			metadata[accel_id].tailptr_ri, metadata[accel_id].tailptr_tlb_inv);
>> +			md->tailptr_ri, md->tailptr_tlb_inv);
>>  	}
>>  
>>  	r = adev->ualink.msg_ctl->send_metadata(adev,
>> @@ -6069,6 +6091,12 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>>  	u64 int_wptr, tlb_wptr;
>>  	int r;
>>  
>> +	/* UALink loopback: both ends of every ring live on this GPU, so there
>> +	 * is no remote peer that can reboot and leave the pointers stale.
>> +	 */
>> +	if (adev->ualink.loopback.enabled)
>> +		return 0;
>> +
>>  	/* ring->wptr and ring->rptr are owned by the send path. */
>>  	guard(mutex)(&peer->lock);
>>  

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

* Re: [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback
  2026-11-04 16:10   ` Philip Yang
@ 2026-11-04 20:31     ` Mukul Joshi
  0 siblings, 0 replies; 24+ messages in thread
From: Mukul Joshi @ 2026-11-04 20:31 UTC (permalink / raw)
  To: Philip Yang, amd-gfx; +Cc: Felix.Kuehling, Harish.Kasiviswanathan, philip.yang


On 11/4/2026 11:10 AM, Philip Yang wrote:
>
>
> On 2026-11-02 10:47, Mukul Joshi wrote:
>> In UALink loopback mode a single GPU acts as both the exporter and the
>> importer. The exporter is the one physical station, so the only path
>> the HW serves is importer -> exporter. A message the exporter sends to
>> the importer cannot be delivered by the MP nHT.
>>
>> Emulate that direction in SW: when loopback is enabled and a remote
>> interrupt is addressed to the importer, queue it on the loopback
>> ordered workqueue, which feeds it back into amdgpu_ualink_process_irq()
>> as if the exporter had sent it. The ordered workqueue keeps successive
>> messages in order and runs the (possibly sleeping) handlers outside
>> the sender's context.
>>
>> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 82 ++++++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> index 29a787979af2..ef133c60bb9a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>> @@ -60,6 +60,9 @@ static void amdgpu_ualink_reset_peer_rings(struct amdgpu_device *adev,
>>  					   u32 remote_accel_id);
>>  static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>>  					  u32 remote_accel_id);
>> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
>> +						 u32 sender_accel_id, u32 dw0,
>> +						 u32 dw1, u32 dw2, u32 dw3);
>>  
>>  #define STRIP_NPA(addr)						\
>>  	(((u64)(addr) & ~AMDGPU_UALINK_NPA_ADDR_GPUID_MASK))
>> @@ -6220,6 +6223,17 @@ static int amdgpu_ualink_remote_interrupt(struct amdgpu_device *adev,
>>  	struct amdgpu_ualink_ring *ring;
>>  	int r;
>>  
>> +	/* UALink loopback: the single MP nHT can only deliver messages addressed
>> +	 * to the exporter accelerator. Messages addressed to the importer
>> +	 * (exporter->importer direction) cannot be delivered by HW, so emulate
>> +	 * them by feeding the message back into our own IRQ handler.
>> +	 */
>> +	if (adev->ualink.loopback.enabled &&
>> +	    remote_accel_id == ualink_imp_accel_id(adev))
>> +		return amdgpu_ualink_loopback_self_interrupt(adev,
>> +				ualink_exp_accel_id(adev),
>> +				dw0, dw1, dw2, dw3);
>> +
>>  	peer = &remote->peer[remote_accel_id];
>>  	ring = &peer->interrupt;
>>  
>> @@ -6649,6 +6663,74 @@ static int amdgpu_ualink_process_irq(struct amdgpu_device *adev,
>>  	return handled;
>>  }
>>  
>> +/* Work item used to emulate an exporter->importer UALink interrupt in
>> + * loopback mode (see amdgpu_ualink_loopback_self_interrupt()).
>> + */
>> +struct amdgpu_ualink_self_irq_work {
>> +	struct work_struct work;
>> +	struct amdgpu_device *adev;
>> +	u32 sender_accel_id;
>> +	u32 dw[4];
>> +};
>> +
>> +static void amdgpu_ualink_self_irq_worker(struct work_struct *w)
>> +{
>> +	struct amdgpu_ualink_self_irq_work *job =
>> +		container_of(w, struct amdgpu_ualink_self_irq_work, work);
>> +	struct amdgpu_device *adev = job->adev;
>> +	struct amdgpu_iv_entry entry = { };
>> +
>> +	/* Build an IH entry as if the message had been delivered by the MP nHT
>> +	 * from the exporter accelerator. amdgpu_ualink_process_irq() consumes
>> +	 * the sender accel_id from entry->pasid and the four context dwords
>> +	 * from entry->src_data[]. entry->ih must differ from adev->irq.ih so
>> +	 * the message is processed here instead of being delegated to the
>> +	 * UALink soft ring.
>> +	 */
>> +	entry.ih = NULL;
> Looking at this again, it seems we could use adev->irq.ih here to
> simulate the message arriving from the real hardware interrupt ring.
> It would then be delegated to the UALink soft ring and handled in the
> interrupt worker, which would let us drop the job structure and
> amdgpu_ualink_self_irq_worker entirely.
>
> The current approach works fine, but that would be simpler and avoid
> introducing another job and work item.
>
We discussed that approach with Felix before and found we need to
introduce locking to prevent concurrent access and it can lead to
deadlock if we are expecting a remote interrupt while processing another
interrupt.

Also we need to create the complete IV entry because the delegate call
will traverse the entire IH/IRQ code path in the driver.

As discussed offline, we are OK with the current approach.

Regards,

Mukul

> Regards,
> Philip
>> +	entry.pasid = job->sender_accel_id;
>> +	entry.src_data[0] = job->dw[0];
>> +	entry.src_data[1] = job->dw[1];
>> +	entry.src_data[2] = job->dw[2];
>> +	entry.src_data[3] = job->dw[3];
>> +
>> +	amdgpu_ualink_process_irq(adev, &adev->ualink.irq, &entry);
>> +	kfree(job);
>> +}
>> +
>> +/*
>> + * amdgpu_ualink_loopback_self_interrupt - deliver a UALink message to ourselves
>> + *
>> + * Used in loopback mode for the exporter->importer direction, where there is
>> + * no HW path. The message is processed asynchronously on an ordered workqueue
>> + * so ordering between successive messages is preserved and the (potentially
>> + * sleeping) message handlers do not run from the caller's context.
>> + */
>> +static int amdgpu_ualink_loopback_self_interrupt(struct amdgpu_device *adev,
>> +						 u32 sender_accel_id, u32 dw0,
>> +						 u32 dw1, u32 dw2, u32 dw3)
>> +{
>> +	struct amdgpu_ualink_self_irq_work *job;
>> +
>> +	if (!adev->ualink.loopback.wq)
>> +		return -ENODEV;
>> +
>> +	job = kzalloc(sizeof(*job), GFP_KERNEL);
>> +	if (!job)
>> +		return -ENOMEM;
>> +
>> +	job->adev = adev;
>> +	job->sender_accel_id = sender_accel_id;
>> +	job->dw[0] = dw0;
>> +	job->dw[1] = dw1;
>> +	job->dw[2] = dw2;
>> +	job->dw[3] = dw3;
>> +	INIT_WORK(&job->work, amdgpu_ualink_self_irq_worker);
>> +	queue_work(adev->ualink.loopback.wq, &job->work);
>> +
>> +	return 0;
>> +}
>> +
>>  static int amdgpu_ualink_set_irq_state(struct amdgpu_device *adev,
>>  					struct amdgpu_irq_src *source,
>>  					u32 type,
>

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

* Re: [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback
  2026-11-04 20:20     ` Mukul Joshi
@ 2026-11-05 22:13       ` Kuehling, Felix
  0 siblings, 0 replies; 24+ messages in thread
From: Kuehling, Felix @ 2026-11-05 22:13 UTC (permalink / raw)
  To: Mukul Joshi, amd-gfx; +Cc: Harish.Kasiviswanathan, philip.yang


On 2026-11-04 15:20, Mukul Joshi wrote:
> On 11/3/2026 6:15 PM, Felix Kuehling wrote:
>> On 2026-11-02 10:47, Mukul Joshi wrote:
>>> In UALink loopback mode a single GPU acts as both the exporter and the
>>> importer, and the only path the HW serves is importer -> exporter.
>>> Adjust the per-peer setup accordingly:
>>>
>>> - The importer's doorbell reaches the MP nHT stamped with the GPU ID of
>>>    the exporter, and the FW looks up the metadata entry by that ID. So
>>>    publish the importer's rings and tail pointers in metadata[exporter].
>>>
>>> - Skip the ring pointer resync. Both ends of every ring are on this
>>>    GPU, so there is no remote peer that can reboot and leave the
>>>    pointers stale.
>>>
>>> - Do not take the local dma-buf shortcut on import. It would bypass
>>>    the NPA path that loopback is meant to exercise.
>>>
>>> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
>>> ---
>>> v2->v3:
>>> - Move updating src_accel_id in peer_remote_init and fini to patch 1.
>>>
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 54 ++++++++++++++++------
>>>   1 file changed, 41 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>>> index 8558dc1a9e84..74bc8a907f5b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>>> @@ -3976,7 +3976,13 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
>>>   		return -EINVAL;
>>>   	}
>>>   
>>> -	if (amdgpu_ualink_is_local_accel(adev, remote_acc_id))
>>> +	/* In loopback mode the exporter and importer are the same GPU, but the
>>> +	 * import must still go through the full cross-domain NPA path (the
>>> +	 * dma_buf shortcut in amdgpu_ualink_local_import() bypasses NPA). Only
>>> +	 * take the local shortcut for genuine same-pod local accelerators.
>>> +	 */
>>> +	if (!adev->ualink.loopback.enabled &&
>>> +	    amdgpu_ualink_is_local_accel(adev, remote_acc_id))
>>>   		return amdgpu_ualink_local_import(adev, remote_acc_id,
>>>   						  &handle, fd_out);
>>>   
>>> @@ -5452,11 +5458,28 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>>   		wptr_gpu_addr = remote->rb_gpu_addr + rb_size * ualink_exp_accel_id(adev);
>>>   
>>>   	for_each_set_bit(accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
>>> +		struct amdgpu_ualink_metadata *md;
>>> +
>>>   		dev_dbg(adev->dev, "init for accel_id %u\n", accel_id);
>>>   
>>>   		if (accel_id == ualink_exp_accel_id(adev))
>>>   			continue;
>> I think you could drop the condition just below if you change this to
>>
>> 		if (accel_id == ualink_imp_accel_id(adev))
>> 			continue;
>>
>> It makes no difference if loopback is disabled. And with loopback
>> enabled it's the more obvious way to only initialize the ring for the
>> exporter ID. Then all the other changes in this function are unnecessary.
>>
> Unfortunately, this won't work.
>
> In the normal (non-loopback) case, when AccId 0 sends a message to AccId
> 1, it writes the message to the ring buffer and wptr NPA addresses that
> AccId 1 set up for source AccId 0, and then rings the doorbell.
>
> The remote interrupt the MPNHT on AccId 1 receives carries AccId 0 as
> the source, and the MPNHT FW uses that to look up metadata[0], which
> holds the ring buffer and wptr addresses for AccId 0.
>
> With loopback it's slightly different. AccId 0 (the importer) still
> writes to the ring buffer and wptr NPA addresses for source AccId 0 and
> rings the doorbell.
>
> However, the interrupt the MPNHT gets reports the source as AccId 1 (the
> exporter).
>
> So the MPNHT FW looks up metadata[1], and for it to find the right
> buffers we have to program the importer's (AccId 0) ring buffer and wptr
> addresses at index 1.
>
> If we skipped the importer and ran the loop for the exporter instead,
> metadata[1] would be filled with AccId 1's own ring buffer and wptr
> addresses, which the importer never writes to.
>
> That's why the patch builds the entry from the importer's addresses but
> stores it at the exporter's index.
>
>
> Regards,
>
> Mukul
>
>> Regards,
>>    Felix
>>
>>
>>>   
>>> +		/* In loopback the single MP nHT services the exporter half:
>>> +		 * the only messages delivered over HW are importer -> exporter
>>> +		 * (source = importer accel_id), while the exporter -> importer
>>> +		 * direction is emulated in SW and never reaches the MP nHT.
>>> +		 *
>>> +		 * The importer's doorbell reaches the MP nHT stamped with the
>>> +		 * GPU ID of the single physical station, the exporter, so the
>>> +		 * FW looks up metadata[exporter]. Publish the importer's rings
>>> +		 * and tail pointers in that entry.
>>> +		 */
>>> +		if (adev->ualink.loopback.enabled)
>>> +			md = &metadata[ualink_exp_accel_id(adev)];
>>> +		else
>>> +			md = &metadata[accel_id];
>>> +
>>>   		rb_gpu_addr = remote->rb_gpu_addr + rb_size * accel_id;
>>>   
>>>   		if (ualink_addr_mode(adev) == AMDGPU_UALINK_ADDR_MODE_SOURCE_IDENT) {
>>> @@ -5464,27 +5487,26 @@ static int amdgpu_ualink_metadata_init(struct amdgpu_device *adev)
>>>   			wptr_gpu_addr += wptr_size * accel_id;
>>>   		}
>>>   
>>> -		metadata[accel_id].header = AMDGPU_UALINK_METADATA_HEADER;
>>> -		metadata[accel_id].rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
>>> +		md->header = AMDGPU_UALINK_METADATA_HEADER;
>>> +		md->rb_size = fls(AMDGPU_UALINK_RB_SIZE / 64) - 1;
>>>   
>>>   		/*
>>>   		 * remote shootdown type is 0, remote command type is 1
>>>   		 * with 2MB ring buffer mapping, remote shootdown ring NPA is before interrupt
>>>   		 */
>>> -		metadata[accel_id].ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
>>> -		metadata[accel_id].tlb_inv_rb = rb_gpu_addr;
>>> +		md->ri_rb = rb_gpu_addr + AMDGPU_UALINK_RB_SIZE;
>>> +		md->tlb_inv_rb = rb_gpu_addr;
>>>   
>>> -		metadata[accel_id].tailptr_ri = wptr_gpu_addr +
>>> -						ualink_wptr_offset(adev, accel_id);
>>> -		metadata[accel_id].tailptr_tlb_inv = wptr_gpu_addr +
>>> -						     ualink_tlb_wptr_offset(adev, accel_id);
>>> +		md->tailptr_ri = wptr_gpu_addr + ualink_wptr_offset(adev, accel_id);
>>> +		md->tailptr_tlb_inv = wptr_gpu_addr +
>>> +				      ualink_tlb_wptr_offset(adev, accel_id);

OK, I missed that it's still using the accel_id here to calculate the 
offsets. So what I proposed wouldn't work as needed. The comment above 
explains it too. The patch is

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


>>>   
>>> -		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, rb_size 0x%x\n",
>>> -			accel_id, ualink_exp_accel_id(adev), metadata[accel_id].rb_size);
>>> +		dev_dbg(adev->dev, "init from accel_id %u to accel_id %u, metadata entry %td, rb_size 0x%x\n",
>>> +			accel_id, ualink_exp_accel_id(adev), md - metadata, md->rb_size);
>>>   		dev_dbg(adev->dev, "rb 0x%llx tlb rb 0x%llx\n",
>>> -			metadata[accel_id].ri_rb, metadata[accel_id].tlb_inv_rb);
>>> +			md->ri_rb, md->tlb_inv_rb);
>>>   		dev_dbg(adev->dev, "rb wptr at 0x%llx tlb wptr at 0x%llx\n",
>>> -			metadata[accel_id].tailptr_ri, metadata[accel_id].tailptr_tlb_inv);
>>> +			md->tailptr_ri, md->tailptr_tlb_inv);
>>>   	}
>>>   
>>>   	r = adev->ualink.msg_ctl->send_metadata(adev,
>>> @@ -6069,6 +6091,12 @@ static int amdgpu_ualink_ring_resync_ptrs(struct amdgpu_device *adev,
>>>   	u64 int_wptr, tlb_wptr;
>>>   	int r;
>>>   
>>> +	/* UALink loopback: both ends of every ring live on this GPU, so there
>>> +	 * is no remote peer that can reboot and leave the pointers stale.
>>> +	 */
>>> +	if (adev->ualink.loopback.enabled)
>>> +		return 0;
>>> +
>>>   	/* ring->wptr and ring->rptr are owned by the send path. */
>>>   	guard(mutex)(&peer->lock);
>>>   

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

end of thread, other threads:[~2026-11-05 22:13 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 15:47 [PATCHv3 0/8] Add UALink Loopback support Mukul Joshi
2026-11-02 15:47 ` [PATCHv3 1/8] drm/amdgpu: Add UALink loopback infrastructure Mukul Joshi
2026-11-03 18:17   ` Philip Yang
2026-11-03 22:32     ` Felix Kuehling
2026-11-03 23:18   ` Felix Kuehling
2026-11-02 15:47 ` [PATCHv3 2/8] drm/amdgpu: Support VMID 0 GPUVA completion WB address for UALink Mukul Joshi
2026-11-03 22:07   ` Philip Yang
2026-11-03 22:51     ` Felix Kuehling
2026-11-02 15:47 ` [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback Mukul Joshi
2026-11-03 22:30   ` Philip Yang
2026-11-03 23:21   ` Felix Kuehling
2026-11-04 16:10   ` Philip Yang
2026-11-04 20:31     ` Mukul Joshi
2026-11-02 15:47 ` [PATCHv3 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender Mukul Joshi
2026-11-03 23:04   ` Felix Kuehling
2026-11-02 15:47 ` [PATCHv3 5/8] drm/amdgpu: Set up UALink rings and metadata for loopback Mukul Joshi
2026-11-03 23:15   ` Felix Kuehling
2026-11-04 20:20     ` Mukul Joshi
2026-11-05 22:13       ` Kuehling, Felix
2026-11-02 15:47 ` [PATCHv3 6/8] drm/amdgpu: Enable UALink loopback mode Mukul Joshi
2026-11-03 23:29   ` Felix Kuehling
2026-11-02 15:47 ` [PATCHv3 7/8] drm/amdgpu: Reject config updates in " Mukul Joshi
2026-11-03 23:17   ` Felix Kuehling
2026-11-02 15:47 ` [PATCHv3 8/8] drm/amdgpu: Temporarily enable UALink loopback via module parameter Mukul Joshi

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.