On Mon, Oct 12, 2026 at 09:10:15PM -0700, Max Zhen wrote:
>
>
> 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 <[email protected]>
> Reviewed-by: Max Zhen <[email protected]>
I was browsing dri-devel and this hmm interaction annoyed me enough to
ask my AI to fix this and converge properly on a DRM framework:
https://gitlab.freedesktop.org/mbrost/xe-kernel-driver-svn-perf-6-15-2025/-/commits/local_dev/amd_hmm_cleanup
Improper HMM or abuse affects the entire kernel, hence we should
converge.
Please consider the AI suggestions (last 3 patches) instead of open
coding this.
Matt
> > ---
> > 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 {
>
