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

[Lizhi Hou]

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