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

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 <[email protected]>
> Reviewed-by: Max Zhen <[email protected]>
>> ---
>>   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;
>