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

[Zhang, Yidong (David)]

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@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.

Removed.

David

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