Re: [PATCHv2 4/8] drm/amdgpu: Attribute UALink loopback messages to the correct sender

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

On 10/28/2026 10:00 AM, Lazar, Lijo wrote:
>
>
> On 28-Oct-26 7:23 PM, Mukul Joshi wrote:
>>
>> On 10/28/2026 2:23 AM, Lazar, Lijo wrote:
>>>
>>>
>>> On 28-Oct-26 1:24 AM, 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>
>>>> ---
>>>> v1->v2:
>>>> - Use ualink_exp_accel_id/ualink_imp_accel_id() wrappers in
>>>>     amdgpu_ualink_process_irq(). (Lijo)
>>>>
>>>>    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 6c70be7badec..96a6023f51c5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
>>>> @@ -1090,6 +1090,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))
>>>
>>> +#define AMDGPU_UALINK_LOOPBACK_EXPORTER_ACCID        1
>>>
>>> 1 is a possible id in a regular vpod config also. I think a check of
>>> loopback mode also is required here.
>>
>> We check loopback mode inside the helper, ualink_exp_accel_id().
>>
>
> Thanks for clarifying. Adding a check may be a bit easier to read.
>
> if (!loopback_mode)
>     return adev->ualink.info->ppod.accel_id;
>
The wrappers are handling all this in one place, because we need same
check for imp_accel_id as well.

So, better to keep them in one place I think.


Thanks,

Mukul

> Thanks,
> Lijo
>
>> Thanks,
>>
>> Mukul
>>
>>>
>>> Thanks,
>>> Lijo
>>>
>>>> +        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)
>>>>    {
>>>> @@ -2238,7 +2255,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",
>>>> @@ -6548,6 +6568,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",
>>>
>