Re: [PATCHv3 3/8] drm/amdgpu: Emulate exporter->importer UALink messages in loopback

[Mukul Joshi <mukul.joshi@amd.com>]

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,
>