Re: [PATCH V0 14/21] accel/amdxdna: Fix fence timeline name and context allocation

["Zhang, Yidong (David)" <yidong.zhang@amd.com>]

On 9/28/2026 2:06 PM, Lizhi Hou wrote:
>
> On 9/25/26 18:34, David Zhang wrote:
>> This is part of the fix to align BO reservation locking and fence
>> management with aie2.
>>
>> Fences published into BO reservation objects via dma_resv_add_fence()
>> can outlive the hardware context (e.g. when a BO is exported as a
>> dma-buf and imported by another process). Using hwctx->name for the 
>> fence
>> timeline name risks a use-after-free once the hwctx is destroyed.
>> Switch timeline name to dev_name() which is backed by the device that
>> outlives any individual context.
>>
>> Additionally, allocate a unique fence context via
>> dma_fence_context_alloc(1) for each job fence so that 
>> dma_resv_add_fence()
>> does not evict a prior in-flight job's fence from a shared BO's
>> reservation object when multiple jobs touch the same BO. Also guard
>> hwctx_fini call in amdxdna_hwctx_destroy_rcu() against NULL ops.
>
> Is this aie4 kernel submission specific? aie2 does not publish this 
> fence.
>
> If it does not fix any existing issue, please describe it clearly.

I have addressed your comments with a newer PATCH V1. This change is 
merged with KMQ patches.

It is AIE4 specific.


>
>>
>> The corresponding AIE4 command submission BO locking and fence
>> attachment logic is implemented in a subsequent patch ("accel/amdxdna:
>> Implement AIE4 command packet building and submission").
>>
>> Co-developed-by: Max Zhen <max.zhen@amd.com>
>> Signed-off-by: Max Zhen <max.zhen@amd.com>
>> Signed-off-by: David Zhang <yidong.zhang@amd.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_ctx.c | 29 ++++++++++++++++++++++-------
>>   1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c 
>> b/drivers/accel/amdxdna/amdxdna_ctx.c
>> index 888e857ec558..6ca7774150d5 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
>> @@ -25,7 +25,7 @@
>>   struct amdxdna_fence {
>>       struct dma_fence    base;
>>       spinlock_t        lock; /* for base */
>> -    struct amdxdna_hwctx    *hwctx;
>> +    struct device        *dev;
>>   };
>>     static const char *amdxdna_fence_get_driver_name(struct dma_fence 
>> *fence)
>> @@ -39,7 +39,14 @@ static const char 
>> *amdxdna_fence_get_timeline_name(struct dma_fence *fence)
>>         xdna_fence = container_of(fence, struct amdxdna_fence, base);
>>   -    return xdna_fence->hwctx->name;
>> +    /*
>> +     * Use device name rather than hwctx name: the fence is 
>> published into
>> +     * BO reservation objects via dma_resv_add_fence() and can 
>> outlive the
>> +     * hwctx (e.g. when a BO is exported as a dma-buf and imported by
>> +     * another process). The device outlives any individual context, so
>> +     * dev_name() is safe to call at any point during the fence's 
>> lifetime.
>> +     */
>> +    return dev_name(xdna_fence->dev);
>>   }
>>     static const struct dma_fence_ops fence_ops = {
>> @@ -55,9 +62,17 @@ static struct dma_fence 
>> *amdxdna_fence_create(struct amdxdna_hwctx *hwctx)
>>       if (!fence)
>>           return NULL;
>>   -    fence->hwctx = hwctx;
>> +    fence->dev = hwctx->client->xdna->ddev.dev;
>>       spin_lock_init(&fence->lock);
>> -    dma_fence_init(&fence->base, &fence_ops, &fence->lock, 
>> hwctx->id, 0);
>> +    /*
>> +     * Part of the fix to align BO reservation locking and fence
>> +     * management with AIE2: each job fence needs a unique context so
>> +     * dma_resv_add_fence() does not evict a prior job's fence from a
>> +     * shared BO's reservation object when two in-flight jobs touch
>> +     * the same BO. The corresponding AIE4 command submission locking
>> +     * and fence attachment is implemented in aie4_cmd_submit().
>> +     */
>> +    dma_fence_init(&fence->base, &fence_ops, &fence->lock, 
>> dma_fence_context_alloc(1), 0);
>>       return &fence->base;
>>   }
>>   @@ -81,13 +96,13 @@ static void 
>> amdxdna_hwctx_release_expanded_heap(struct amdxdna_hwctx *hwctx)
>>   static void amdxdna_hwctx_destroy_rcu(struct amdxdna_hwctx *hwctx,
>>                         struct srcu_struct *ss)
>>   {
>> -    struct amdxdna_client *client = hwctx->client;
>> -    struct amdxdna_dev *xdna = client->xdna;
>> +    struct amdxdna_dev *xdna = hwctx->client->xdna;
>>         synchronize_srcu(ss);
>>         /* At this point, user is not able to submit new commands */
>> -    xdna->dev_info->ops->hwctx_fini(hwctx);
>> +    if (xdna->dev_info->ops->hwctx_fini)
>> +        xdna->dev_info->ops->hwctx_fini(hwctx);
>
> This seems unrelated. Please remove from the patch.

Removed.

David

>
> Lizhi
>
>> amdxdna_hwctx_release_expanded_heap(hwctx);
>>       kfree(hwctx->name);