Re: [PATCH V1] accel/amdxdna: Serialize HMM range faults on one umap

[Max Zhen <max.zhen@amd.com>]

On 10/12/2026 Mon 13:31, Lizhi Hou wrote:
> Two hardware contexts can submit jobs that use the same GEM object while
> a user mapping is invalid. aie2_cmd_submit() drops the reservation and
> notifier_lock before aie2_populate_range(), and that function takes a
> reference on the first invalid umap without clearing invalid until the
> fault returns. Both submissions then call
> hmm_range_fault_unlocked_timeout() on the same struct hmm_range. That
> structure is not safe for concurrent callers. The calls write
> notifier_seq and hmm_pfns[] at the same time, so mmu_interval_read_retry()
> can see the other call's sequence, miss an invalidation, and clear
> invalid. The job is then pushed while the pages may already be gone.
> Give each umap its own mutex and hold it from the fault through the retry
> check and the store that clears invalid. notifier_lock is dropped before
> this mutex is taken, and the fault drops mmap_lock before notifier_lock
> is taken again.
> 
> Fixes: aac243092b70 ("accel/amdxdna: Add command execution")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/aie2_ctx.c    | 7 ++++++-
>   drivers/accel/amdxdna/amdxdna_gem.c | 2 ++
>   drivers/accel/amdxdna/amdxdna_gem.h | 1 +
>   3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index d927c8c9d557..4db67a789c34 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -1155,19 +1155,24 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
>   		return -EFAULT;
>   	}
>   
> +	mutex_lock(&mapp->lock);
>   	ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
> -	if (ret)
> +	if (ret) {
> +		mutex_unlock(&mapp->lock);
>   		goto put_mm;
> +	}
>   
>   	down_write(&xdna->notifier_lock);
>   	if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {
>   		up_write(&xdna->notifier_lock);
> +		mutex_unlock(&mapp->lock);
>   		amdxdna_umap_put(mapp);
>   		mmput(mm);
>   		goto again;
>   	}
>   	mapp->invalid = false;
>   	up_write(&xdna->notifier_lock);
> +	mutex_unlock(&mapp->lock);
>   	amdxdna_umap_put(mapp);
>   	mmput(mm);
>   	goto again;
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index f14484bc31ec..37bfa5058095 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -318,6 +318,7 @@ static void amdxdna_umap_release(struct kref *ref)
>   	struct amdxdna_umap *mapp = container_of(ref, struct amdxdna_umap, refcnt);
>   
>   	mmu_interval_notifier_remove(&mapp->notifier);
> +	mutex_destroy(&mapp->lock);
>   	kvfree(mapp->range.hmm_pfns);
>   	kfree(mapp);
>   }
> @@ -402,6 +403,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo,
>   		goto free_pfns;
>   	}
>   
> +	mutex_init(&mapp->lock);
>   	down_write(&xdna->notifier_lock);
>   	if (list_empty(&abo->mem.umap_list))
>   		abo->mem.uva = addr;
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
> index 9b4aa21a37c9..456a537eb68c 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.h
> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
> @@ -19,6 +19,7 @@ struct amdxdna_umap {
>   	struct kref			refcnt;
>   	bool				invalid;
>   	bool				unmapped;
> +	struct mutex			lock; /* protect range */
>   };
>   
>   struct amdxdna_mem {