Re: [PATCH V1] accel/amxdna: fix page-insertion errors in amdxdna_insert_pages()

[Lizhi Hou]

Applied to drm-misc-fixes

On 7/23/26 11:03, Max Zhen wrote:
> On 7/23/2026 Thu 00:42, Lizhi Hou wrote:
> > Two error paths in amdxdna_insert_pages() called vma->vm_ops->close(vma)
> > before returning an error code to the caller.  This is incorrect:
> > amdxdna_gem_obj_mmap() registers an HMM interval notifier before calling
> > amdxdna_insert_pages(), and on a hard error it jumps to hmm_unreg to 
> > undo
> > that registration.  Calling vm_ops->close() manually — which drops the
> > shmem pages_pin_count and the GEM object reference that backs the VMA —
> > before the mmap syscall has even returned causes those resources to be
> > released while the VMA is still alive.  The kernel VMA teardown will 
> > call
> > vm_ops->close() a second time when the process later unmaps the range,
> > producing a reference count underflow.
> >
> > Replace both hard-error returns with a deferred-fault approach that 
> > keeps
> > the VMA alive and retries page insertion through the HMM range-fault 
> > path.
> >
> > Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
> > Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
> Reviewed-by: Max Zhen <max.zhen@xxxxxxx>
> > ---
> >   drivers/accel/amdxdna/amdxdna_gem.c | 24 ++++++++++++++++++++----
> >   1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
> > b/drivers/accel/amdxdna/amdxdna_gem.c
> > index d6fe6fb41286..aed110ad1c1e 100644
> > --- a/drivers/accel/amdxdna/amdxdna_gem.c
> > +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> > @@ -435,6 +435,23 @@ static void amdxdna_gem_dev_obj_free(struct 
> > drm_gem_object *gobj)
> >       amdxdna_gem_destroy_obj(abo);
> >   }
> >   +static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo,
> > +                      struct vm_area_struct *vma)
> > +{
> > +    struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> > +    struct amdxdna_umap *mapp;
> > +
> > +    down_write(&xdna->notifier_lock);
> > +    abo->mem.map_invalid = true;
> > +    list_for_each_entry(mapp, &abo->mem.umap_list, node) {
> > +        if (compare_range(mapp, vma->vm_mm, vma->vm_start, 
> > vma->vm_end)) {
> > +            mapp->invalid = true;
> > +            break;
> > +        }
> > +    }
> > +    up_write(&xdna->notifier_lock);
> > +}
> > +
> >   static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
> >                   struct vm_area_struct *vma)
> >   {
> > @@ -456,8 +473,7 @@ static int amdxdna_insert_pages(struct 
> > amdxdna_gem_obj *abo,
> >                         &num_pages);
> >           if (ret) {
> >               XDNA_ERR(xdna, "Failed insert pages %d", ret);
> > -            vma->vm_ops->close(vma);
> > -            return ret;
> > +            amdxdna_mark_mapp_invalid(abo, vma);
> >           }
> >             return 0;
> > @@ -477,9 +493,9 @@ static int amdxdna_insert_pages(struct 
> > amdxdna_gem_obj *abo,
> >           fault_ret = handle_mm_fault(vma, vma->vm_start + offset,
> >                           FAULT_FLAG_WRITE, NULL);
> >           if (fault_ret & VM_FAULT_ERROR) {
> > -            vma->vm_ops->close(vma);
> >               XDNA_ERR(xdna, "Fault in page failed");
> > -            return -EFAULT;
> > +            amdxdna_mark_mapp_invalid(abo, vma);
> > +            break;
> >           }
> >             offset += PAGE_SIZE;