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

[Lizhi Hou]

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.

> 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@xxxxxxx>
> Signed-off-by: Max Zhen <max.zhen@xxxxxxx>
> Signed-off-by: David Zhang <yidong.zhang@xxxxxxx>
> ---
>   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.

Lizhi

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