From: Lizhi Hou <lizhi.hou@amd.com>
To: Max Zhen <max.zhen@amd.com>, <ogabbay@kernel.org>,
<quic_jhugo@quicinc.com>, <dri-devel@lists.freedesktop.org>,
<mario.limonciello@amd.com>, <karol.wachowski@linux.intel.com>,
<thomas.hellstrom@linux.intel.com>,
<himal.prasad.ghimiray@intel.com>, <honghuan@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>,
<linux-kernel@vger.kernel.org>, <sonal.santan@amd.com>
Subject: Re: [PATCH V1 2/2] accel/amdxdna: use drm_gpusvm_pages for user mapping tracking
Date: Tue, 10 Nov 2026 10:16:39 -0800 [thread overview]
Message-ID: <a12185d5-fe6c-4714-78b9-5c45a45c7c30@amd.com> (raw)
In-Reply-To: <b76bb18c-a0e5-4a49-8866-888df3bd9a30@amd.com>
Applied drm-misc-next
On 11/9/26 11:44, Max Zhen wrote:
>
>
> On 11/6/2026 Fri 10:42, Lizhi Hou wrote:
>> From: Matthew Brost <matthew.brost@intel.com>
>>
>> In PASID mode the device accesses user mappings of a BO through the
>> IOMMU, but cannot take page faults, so the driver faults the CPU pages
>> in with HMM before submitting a job and waits for the job from the MMU
>> notifier. The HMM PFNs themselves are never used.
>>
>> The open coded implementation of this has a few problems:
>>
>> - Each mapping embeds a struct hmm_range, and keeps the PFN array it
>> points at allocated for the lifetime of the mapping, although the
>> PFNs are dropped as soon as the fault returns.
>> - The per mapping invalid flag duplicates what the notifier sequence
>> number already tracks, and must be kept in sync with it.
>> - The notifier lock is taken in write mode on the populate path.
>> - Submission locks all reservations, finds the first BO with an
>> invalidated mapping, unlocks everything, faults that single BO in and
>> starts over, once per invalidated BO.
>>
>> drm_gpusvm already implements the fault and retry loop, including
>> faulting without a DMA mapping (&drm_gpusvm_ctx.no_dma_map), and its
>> pages only mode does not require the driver to use drm_gpusvm ranges.
>> Use it:
>>
>> - Replace the device notifier_lock with a pages only struct drm_gpusvm
>> and its notifier lock.
>> - Replace the embedded hmm_range and the invalid/unmapped flags with a
>> struct drm_gpusvm_pages. A mapping is stale when
>> drm_gpusvm_pages_read_retry() says so and it is not unmapped.
>> - Fault in all stale mappings with drm_gpusvm_get_pages() before
>> locking the reservations, then check that none went stale again
>> under the notifier lock, which is held until the job fences are
>> published, as before.
>> - The invalidate callback only bumps the sequence number and marks the
>> mapping unmapped on MMU_NOTIFY_UNMAP under the write lock, before
>> waiting for the jobs.
>>
>> A temporary PFN array is now allocated per fault instead of keeping one
>> per mapping, which also removes the race above.
>>
>> Assisted-by: LLM
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> [Lizhi: Merge with latest code]
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>> ---
>> drivers/accel/amdxdna/Kconfig | 2 +-
>> drivers/accel/amdxdna/aie2_ctx.c | 31 +--
>> drivers/accel/amdxdna/aie4_ctx.c | 29 +--
>> drivers/accel/amdxdna/amdxdna_gem.c | 312 +++++++++++++-----------
>> drivers/accel/amdxdna/amdxdna_gem.h | 23 +-
>> drivers/accel/amdxdna/amdxdna_pci_drv.c | 15 +-
>> drivers/accel/amdxdna/amdxdna_pci_drv.h | 3 +-
>> 7 files changed, 213 insertions(+), 202 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/Kconfig
>> b/drivers/accel/amdxdna/Kconfig
>> index eddff490b4f75..3721eb580b71a 100644
>> --- a/drivers/accel/amdxdna/Kconfig
>> +++ b/drivers/accel/amdxdna/Kconfig
>> @@ -7,10 +7,10 @@ config DRM_ACCEL_AMDXDNA
>> depends on PCI && HAS_IOMEM
>> depends on X86_64
>> depends on AMD_PMF || !AMD_PMF
>> + select DRM_GPUSVM
>> select DRM_SCHED
>> select DRM_GEM_SHMEM_HELPER
>> select FW_LOADER
>> - select HMM_MIRROR
>> help
>> Choose this option to enable support for NPU integrated into AMD
>> client CPUs like AMD Ryzen AI 300 Series. AMD NPU can be used to
>> diff --git a/drivers/accel/amdxdna/aie2_ctx.c
>> b/drivers/accel/amdxdna/aie2_ctx.c
>> index e1c0e0689b12e..552c30ea6e387 100644
>> --- a/drivers/accel/amdxdna/aie2_ctx.c
>> +++ b/drivers/accel/amdxdna/aie2_ctx.c
>> @@ -1116,8 +1116,7 @@ int aie2_cmd_submit(struct amdxdna_hwctx
>> *hwctx, struct amdxdna_sched_job *job,
>> struct amdxdna_dev *xdna = hwctx->client->xdna;
>> struct ww_acquire_ctx acquire_ctx;
>> struct dma_fence_chain *chain;
>> - struct amdxdna_gem_obj *abo;
>> - unsigned long timeout = 0;
>> + unsigned long deadline;
>> int ret, i;
>> ret = down_interruptible(&hwctx->priv->job_sem);
>> @@ -1140,7 +1139,15 @@ int aie2_cmd_submit(struct amdxdna_hwctx
>> *hwctx, struct amdxdna_sched_job *job,
>> goto free_chain;
>> }
>> + deadline = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> retry:
>> + for (i = 0; i < job->bo_cnt; i++) {
>> + ret = amdxdna_gem_umap_populate(to_xdna_obj(job->bos[i]),
>> + deadline);
>> + if (ret)
>> + goto cleanup_job;
>> + }
>> +
>> ret = drm_gem_lock_reservations(job->bos, job->bo_cnt,
>> &acquire_ctx);
>> if (ret) {
>> XDNA_WARN(xdna, "Failed to lock BOs, ret %d", ret);
>> @@ -1156,23 +1163,11 @@ int aie2_cmd_submit(struct amdxdna_hwctx
>> *hwctx, struct amdxdna_sched_job *job,
>> }
>> }
>> - down_read(&xdna->notifier_lock);
>> + drm_gpusvm_notifier_lock(&xdna->gpusvm);
>> for (i = 0; i < job->bo_cnt; i++) {
>> - abo = to_xdna_obj(job->bos[i]);
>> - if (abo->mem.map_invalid) {
>> - up_read(&xdna->notifier_lock);
>> + if (amdxdna_gem_umap_stale(to_xdna_obj(job->bos[i]))) {
>> + drm_gpusvm_notifier_unlock(&xdna->gpusvm);
>> drm_gem_unlock_reservations(job->bos, job->bo_cnt,
>> &acquire_ctx);
>> - if (!timeout) {
>> - timeout = jiffies +
>> - msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> - } else if (time_after(jiffies, timeout)) {
>> - ret = -ETIME;
>> - goto cleanup_job;
>> - }
>> -
>> - ret = amdxdna_populate_range(abo);
>> - if (ret)
>> - goto cleanup_job;
>> goto retry;
>> }
>> }
>> @@ -1192,7 +1187,7 @@ int aie2_cmd_submit(struct amdxdna_hwctx
>> *hwctx, struct amdxdna_sched_job *job,
>> drm_syncobj_add_point(hwctx->priv->syncobj, chain,
>> job->out_fence, *seq);
>> mutex_unlock(&hwctx->priv->io_lock);
>> - up_read(&xdna->notifier_lock);
>> + drm_gpusvm_notifier_unlock(&xdna->gpusvm);
>> drm_gem_unlock_reservations(job->bos, job->bo_cnt, &acquire_ctx);
>> aie2_job_put(job);
>> diff --git a/drivers/accel/amdxdna/aie4_ctx.c
>> b/drivers/accel/amdxdna/aie4_ctx.c
>> index a3f0a69b93b75..0db7dcdf106ba 100644
>> --- a/drivers/accel/amdxdna/aie4_ctx.c
>> +++ b/drivers/accel/amdxdna/aie4_ctx.c
>> @@ -1059,8 +1059,7 @@ static int fence_job_bos(struct amdxdna_dev
>> *xdna, struct amdxdna_sched_job *job
>> {
>> struct ww_acquire_ctx acquire_ctx;
>> struct drm_gem_object **objs;
>> - struct amdxdna_gem_obj *abo;
>> - unsigned long timeout = 0;
>> + unsigned long deadline;
>> u32 cnt = 0;
>> int ret;
>> u32 i;
>> @@ -1076,7 +1075,13 @@ static int fence_job_bos(struct amdxdna_dev
>> *xdna, struct amdxdna_sched_job *job
>> for (i = 0; i < job->aie4_cmd_bo_cnt; i++)
>> add_lock_obj(objs, &cnt, to_gobj(job->aie4_cmd_bos[i]));
>> + deadline = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> retry:
>> + for (i = 0; i < cnt; i++) {
>> + ret = amdxdna_gem_umap_populate(to_xdna_obj(objs[i]),
>> deadline);
>> + if (ret)
>> + goto free_objs;
>> + }
>> ret = drm_gem_lock_reservations(objs, cnt, &acquire_ctx);
>> if (ret) {
>> XDNA_WARN(xdna, "Failed to lock BOs, ret %d", ret);
>> @@ -1091,23 +1096,11 @@ static int fence_job_bos(struct amdxdna_dev
>> *xdna, struct amdxdna_sched_job *job
>> }
>> }
>> - down_read(&xdna->notifier_lock);
>> + drm_gpusvm_notifier_lock(&xdna->gpusvm);
>> for (i = 0; i < cnt; i++) {
>> - abo = to_xdna_obj(objs[i]);
>> - if (abo->mem.map_invalid) {
>> - up_read(&xdna->notifier_lock);
>> + if (amdxdna_gem_umap_stale(to_xdna_obj(objs[i]))) {
>> + drm_gpusvm_notifier_unlock(&xdna->gpusvm);
>> drm_gem_unlock_reservations(objs, cnt, &acquire_ctx);
>> - if (!timeout) {
>> - timeout = jiffies +
>> - msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> - } else if (time_after(jiffies, timeout)) {
>> - ret = -ETIME;
>> - goto free_objs;
>> - }
>> -
>> - ret = amdxdna_populate_range(abo);
>> - if (ret)
>> - goto free_objs;
>> goto retry;
>> }
>> }
>> @@ -1115,7 +1108,7 @@ static int fence_job_bos(struct amdxdna_dev
>> *xdna, struct amdxdna_sched_job *job
>> job->out_fence = dma_fence_get(job->fence);
>> for (i = 0; i < cnt; i++)
>> dma_resv_add_fence(objs[i]->resv, job->out_fence,
>> DMA_RESV_USAGE_WRITE);
>> - up_read(&xdna->notifier_lock);
>> + drm_gpusvm_notifier_unlock(&xdna->gpusvm);
>> unlock:
>> drm_gem_unlock_reservations(objs, cnt, &acquire_ctx);
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 09e48c794e1b0..57d87fa98d671 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -232,37 +232,46 @@ static bool amdxdna_hmm_invalidate(struct
>> mmu_interval_notifier *mni,
>> {
>> struct amdxdna_umap *mapp = container_of(mni, struct
>> amdxdna_umap, notifier);
>> struct amdxdna_gem_obj *abo = mapp->abo;
>> - struct amdxdna_dev *xdna;
>> + struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
>> + bool unreg;
>> if (!mmu_notifier_range_blockable(range))
>> return false;
>> - if (mapp->unmapped)
>> - return true;
>> -
>> - xdna = to_xdna_dev(to_gobj(abo)->dev);
>> XDNA_DBG(xdna, "Invalidating range 0x%lx, 0x%lx, type %d",
>> - mapp->range.start, mapp->range.end, abo->type);
>> + mapp->start, mapp->end, abo->type);
>> - down_write(&xdna->notifier_lock);
>> - abo->mem.map_invalid = true;
>> - mapp->invalid = true;
>> - mmu_interval_set_seq(&mapp->notifier, cur_seq);
>> - up_write(&xdna->notifier_lock);
>> + down_write(&xdna->gpusvm.notifier_lock);
>> + mmu_interval_set_seq(mni, cur_seq);
>> + /*
>> + * Flag the mapping as gone before the wait below, so that a
>> submission
>> + * racing with the unmap does not try to fault it in again.
>> + */
>> + unreg = !mapp->pages.flags.unmapped && range->event ==
>> MMU_NOTIFY_UNMAP;
>> + if (unreg)
>> + drm_gpusvm_pages_set_unmapped(&xdna->gpusvm, &mapp->pages, 1);
>> + up_write(&xdna->gpusvm.notifier_lock);
>> /*
>> * The pages must not be released while the device may still
>> access
>> * them, so this wait must be neither interruptible nor time
>> limited.
>> + *
>> + * Every invalidation of the mapping has to wait, including the
>> ones
>> + * which find it already flagged: the flag is set above before
>> the wait,
>> + * so it does not tell whether the jobs of a concurrent
>> invalidation
>> + * have finished.
>> */
>> dma_resv_wait_timeout(to_gobj(abo)->resv, DMA_RESV_USAGE_BOOKKEEP,
>> false, MAX_SCHEDULE_TIMEOUT);
>> - if (range->event == MMU_NOTIFY_UNMAP) {
>> - down_write(&xdna->notifier_lock);
>> - mapp->unmapped = true;
>> + /*
>> + * Queue the unregistration only once the jobs are done: it
>> drops the
>> + * last reference of the mapping, and
>> mmu_interval_notifier_remove()
>> + * waits for this callback, which would pin the single worker of
>> the
>> + * ordered WQ_MEM_RECLAIM notifier_wq for as long as a job runs.
>> + */
>> + if (unreg)
>> queue_work(xdna->notifier_wq, &abo->hmm_unreg_work);
>> - up_write(&xdna->notifier_lock);
>> - }
>> return true;
>> }
>> @@ -271,12 +280,25 @@ static const struct mmu_interval_notifier_ops
>> amdxdna_hmm_ops = {
>> .invalidate = amdxdna_hmm_invalidate,
>> };
>> -static inline bool compare_range(struct amdxdna_umap *mapp,
>> - struct mm_struct *mm,
>> - unsigned long start, unsigned long end)
>> +static bool amdxdna_umap_match(struct amdxdna_umap *mapp,
>> + struct mm_struct *mm,
>> + unsigned long start, unsigned long end)
>> {
>> - return (!mapp->unmapped && mapp->notifier.mm == mm &&
>> - mapp->range.start == start && mapp->range.end == end);
>> + return !mapp->pages.flags.unmapped && mapp->notifier.mm == mm &&
>> + mapp->start == start && mapp->end == end;
>> +}
>> +
>> +static bool amdxdna_umap_stale(struct drm_gpusvm *gpusvm,
>> + struct amdxdna_umap *mapp)
>> +{
>> + /*
>> + * The pages of a BO whose mappings mmap() populated upfront are
>> never
>> + * faulted in. Neither are the ones of a mapping which is gone:
>> device
>> + * accesses through it are rejected by the IOMMU, so there is
>> nothing
>> + * to fault in for it.
>> + */
>> + return mapp->abo->mem.map_invalid && !mapp->pages.flags.unmapped &&
>> + drm_gpusvm_pages_read_retry(gpusvm, &mapp->pages,
>> &mapp->notifier);
>> }
>> static void amdxdna_hmm_unregister(struct amdxdna_gem_obj *abo,
>> @@ -285,15 +307,28 @@ static void amdxdna_hmm_unregister(struct
>> amdxdna_gem_obj *abo,
>> struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
>> struct amdxdna_umap *mapp;
>> - down_write(&xdna->notifier_lock);
>> + down_write(&xdna->gpusvm.notifier_lock);
>> list_for_each_entry(mapp, &abo->mem.umap_list, node) {
>> - if (!compare_range(mapp, vma->vm_mm, vma->vm_start,
>> vma->vm_end))
>> + if (!amdxdna_umap_match(mapp, vma->vm_mm, vma->vm_start,
>> vma->vm_end))
>> continue;
>> - mapp->unmapped = true;
>> + drm_gpusvm_pages_set_unmapped(&xdna->gpusvm, &mapp->pages, 1);
>> queue_work(xdna->notifier_wq, &abo->hmm_unreg_work);
>> }
>> - up_write(&xdna->notifier_lock);
>> + up_write(&xdna->gpusvm.notifier_lock);
>> +}
>> +
>> +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);
>> + kfree(mapp);
>> +}
>> +
>> +static void amdxdna_umap_put(struct amdxdna_umap *mapp)
>> +{
>> + kref_put(&mapp->refcnt, amdxdna_umap_release);
>> }
>> static void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo,
>> bool force)
>> @@ -302,121 +337,119 @@ static void amdxdna_hmm_unreg_umaps(struct
>> amdxdna_gem_obj *abo, bool force)
>> struct amdxdna_umap *mapp, *tmp;
>> LIST_HEAD(dead);
>> - down_write(&xdna->notifier_lock);
>> + down_write(&xdna->gpusvm.notifier_lock);
>> list_for_each_entry_safe(mapp, tmp, &abo->mem.umap_list, node) {
>> - if (!force && !mapp->unmapped)
>> + if (!force && !mapp->pages.flags.unmapped)
>> continue;
>> - mapp->unmapped = true;
>> + drm_gpusvm_pages_set_unmapped(&xdna->gpusvm, &mapp->pages, 1);
>> list_move(&mapp->node, &dead);
>> }
>> if (list_empty(&abo->mem.umap_list))
>> abo->mem.uva = AMDXDNA_INVALID_ADDR;
>> - up_write(&xdna->notifier_lock);
>> + up_write(&xdna->gpusvm.notifier_lock);
>> list_for_each_entry_safe(mapp, tmp, &dead, node)
>> amdxdna_umap_put(mapp);
>> }
>> -static void amdxdna_umap_release(struct kref *ref)
>> +static void amdxdna_hmm_unreg_work(struct work_struct *work)
>> {
>> - 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);
>> -}
>> + struct amdxdna_gem_obj *abo = container_of(work, struct
>> amdxdna_gem_obj,
>> + hmm_unreg_work);
>> -void amdxdna_umap_put(struct amdxdna_umap *mapp)
>> -{
>> - kref_put(&mapp->refcnt, amdxdna_umap_release);
>> + amdxdna_hmm_unreg_umaps(abo, false);
>> }
>> -int amdxdna_populate_range(struct amdxdna_gem_obj *abo)
>> +/**
>> + * amdxdna_gem_umap_populate() - Fault in the invalidated user
>> mappings of a BO
>> + * @abo: The BO.
>> + * @deadline: Jiffies after which to give up.
>> + *
>> + * The device accesses user mappings through the PASID and cannot
>> take page
>> + * faults, so the CPU pages of every mapping of @abo invalidated
>> since it was
>> + * last faulted are faulted in again here. This is only a snapshot,
>> the caller
>> + * must check amdxdna_gem_umap_stale() under the notifier lock
>> before using
>> + * the mappings and retry if it fails.
>> + *
>> + * Two hardware contexts may fault the same mapping at the same
>> time. That is
>> + * left unserialized: with &drm_gpusvm_ctx.no_dma_map the only state
>> recorded
>> + * for a mapping is its notifier sequence, and the loser of such a
>> race can
>> + * only store an older sequence, which makes
>> amdxdna_gem_umap_stale() report
>> + * stale and the caller fault again.
>> + *
>> + * Return: 0 on success, -ETIME if @deadline passed, negative error
>> code on
>> + * other failures.
>> + */
>> +int amdxdna_gem_umap_populate(struct amdxdna_gem_obj *abo,
>> + unsigned long deadline)
>> {
>> struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
>> + struct drm_gpusvm *gpusvm = &xdna->gpusvm;
>> struct amdxdna_umap *mapp;
>> - unsigned long timeout;
>> - struct mm_struct *mm;
>> - bool found;
>> + bool found, unmapped;
>> int ret;
>> - timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> -again:
>> - found = false;
>> - down_write(&xdna->notifier_lock);
>> - list_for_each_entry(mapp, &abo->mem.umap_list, node) {
>> - /*
>> - * Skip entries that have already been unmapped.
>> - *
>> - * If userspace unmaps the address and later submits I/O using
>> - * it, the IOMMU will reject the access and report a fault.
>> - * Ignore such entries here.
>> - */
>> - if (mapp->unmapped)
>> - continue;
>> + for (;;) {
>> + struct drm_gpusvm_ctx ctx = { .no_dma_map = 1 };
>> - if (mapp->invalid && kref_get_unless_zero(&mapp->refcnt)) {
>> - found = true;
>> - break;
>> + found = false;
>> + drm_gpusvm_notifier_lock(gpusvm);
>> + list_for_each_entry(mapp, &abo->mem.umap_list, node) {
>> + if (amdxdna_umap_stale(gpusvm, mapp) &&
>> + kref_get_unless_zero(&mapp->refcnt)) {
>> + found = true;
>> + break;
>> + }
>> }
>> - }
>> -
>> - if (!found) {
>> - /*
>> - * This also covers the case where all mappings have been
>> - * removed. There are no invalid mappings left to process.
>> - * Any subsequent I/O using the unmapped address will be
>> - * rejected by the IOMMU.
>> - */
>> - abo->mem.map_invalid = false;
>> - up_write(&xdna->notifier_lock);
>> - return 0;
>> - }
>> -
>> - up_write(&xdna->notifier_lock);
>> -
>> - mm = mapp->notifier.mm;
>> - if (!mmget_not_zero(mm)) {
>> - amdxdna_umap_put(mapp);
>> - return -EFAULT;
>> - }
>> + drm_gpusvm_notifier_unlock(gpusvm);
>> + if (!found)
>> + return 0;
>> - mutex_lock(&mapp->lock);
>> - ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
>> - if (ret) {
>> - mutex_unlock(&mapp->lock);
>> - goto put_mm;
>> - }
>> + if (time_after(jiffies, deadline)) {
>> + amdxdna_umap_put(mapp);
>> + return -ETIME;
>> + }
>> - 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);
>> + ctx.read_only = mapp->read_only;
>> + ret = drm_gpusvm_get_pages(gpusvm, &mapp->pages, 1,
>> + mapp->notifier.mm, &mapp->notifier,
>> + mapp->start, mapp->end, &ctx);
>> + if (ret == -EFAULT) {
>> + drm_gpusvm_notifier_lock(gpusvm);
>> + unmapped = mapp->pages.flags.unmapped;
>> + drm_gpusvm_notifier_unlock(gpusvm);
>> + if (unmapped)
>> + ret = 0;
>> + }
>> amdxdna_umap_put(mapp);
>> - mmput(mm);
>> - goto again;
>> + if (ret)
>> + return ret == -EBUSY ? -ETIME : ret;
>> }
>> - mapp->invalid = false;
>> - up_write(&xdna->notifier_lock);
>> - mutex_unlock(&mapp->lock);
>> - amdxdna_umap_put(mapp);
>> - mmput(mm);
>> - goto again;
>> -
>> -put_mm:
>> - amdxdna_umap_put(mapp);
>> - mmput(mm);
>> - return ret == -EBUSY ? -ETIME : ret;
>> }
>> -static void amdxdna_hmm_unreg_work(struct work_struct *work)
>> +/**
>> + * amdxdna_gem_umap_stale() - Check whether a BO needs its user
>> mappings faulted
>> + * @abo: The BO.
>> + *
>> + * Must be called with the notifier lock held, which must be kept
>> held until
>> + * the fences of the jobs using @abo are published in its
>> reservation object.
>> + *
>> + * Return: True if amdxdna_gem_umap_populate() must be called for @abo.
>> + */
>> +bool amdxdna_gem_umap_stale(struct amdxdna_gem_obj *abo)
>> {
>> - struct amdxdna_gem_obj *abo = container_of(work, struct
>> amdxdna_gem_obj,
>> - hmm_unreg_work);
>> + struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
>> + struct amdxdna_umap *mapp;
>> - amdxdna_hmm_unreg_umaps(abo, false);
>> + lockdep_assert_held(&xdna->gpusvm.notifier_lock);
>> +
>> + list_for_each_entry(mapp, &abo->mem.umap_list, node) {
>> + if (amdxdna_umap_stale(&xdna->gpusvm, mapp))
>> + return true;
>> + }
>> +
>> + return false;
>> }
>> static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo,
>> @@ -426,7 +459,6 @@ static int amdxdna_hmm_register(struct
>> amdxdna_gem_obj *abo,
>> unsigned long len = vma->vm_end - vma->vm_start;
>> unsigned long addr = vma->vm_start;
>> struct amdxdna_umap *mapp;
>> - unsigned long nr_pages;
>> int ret;
>> /*
>> @@ -441,38 +473,34 @@ static int amdxdna_hmm_register(struct
>> amdxdna_gem_obj *abo,
>> return 0;
>> }
>> - down_read(&xdna->notifier_lock);
>> + down_read(&xdna->gpusvm.notifier_lock);
>> list_for_each_entry(mapp, &abo->mem.umap_list, node) {
>> - if (compare_range(mapp, current->mm, addr, addr + len)) {
>> - up_read(&xdna->notifier_lock);
>> + if (amdxdna_umap_match(mapp, current->mm, addr, addr + len)) {
>> + up_read(&xdna->gpusvm.notifier_lock);
>> return 0;
>> }
>> }
>> - up_read(&xdna->notifier_lock);
>> + up_read(&xdna->gpusvm.notifier_lock);
>> mapp = kzalloc_obj(*mapp);
>> if (!mapp)
>> return -ENOMEM;
>> - nr_pages = (PAGE_ALIGN(addr + len) - (addr & PAGE_MASK)) >>
>> PAGE_SHIFT;
>> - mapp->range.hmm_pfns = kvzalloc_objs(*mapp->range.hmm_pfns,
>> nr_pages);
>> - if (!mapp->range.hmm_pfns) {
>> - ret = -ENOMEM;
>> - goto free_map;
>> - }
>> + /*
>> + * Never faulted, so stale until the first
>> amdxdna_gem_umap_populate(),
>> + * should the pages of the BO have to be faulted in at all.
>> + */
>> + drm_gpusvm_init_pages(&mapp->pages, &xdna->ddev);
>> + mapp->start = addr;
>> + mapp->end = addr + len;
>> - mapp->range.notifier = &mapp->notifier;
>> - mapp->range.start = vma->vm_start;
>> - mapp->range.end = vma->vm_end;
>> /*
>> * Access permissions are fixed at mmap() time. Changing them
>> later
>> - * with mprotect() is not supported: the range keeps requesting the
>> + * with mprotect() is not supported: the mapping keeps
>> requesting the
>> * original permissions, so the application may see a fault
>> failure
>> * or an IOMMU fault.
>> */
>> - mapp->range.default_flags = HMM_PFN_REQ_FAULT;
>> - if (vma->vm_flags & VM_WRITE)
>> - mapp->range.default_flags |= HMM_PFN_REQ_WRITE;
>> + mapp->read_only = !(vma->vm_flags & VM_WRITE);
>> mapp->abo = abo;
>> kref_init(&mapp->refcnt);
>> @@ -483,23 +511,17 @@ static int amdxdna_hmm_register(struct
>> amdxdna_gem_obj *abo,
>> &amdxdna_hmm_ops);
>> if (ret) {
>> XDNA_ERR(xdna, "Insert mmu notifier failed, ret %d", ret);
>> - goto free_pfns;
>> + kfree(mapp);
>> + return ret;
>> }
>> - mutex_init(&mapp->lock);
>> - down_write(&xdna->notifier_lock);
>> + down_write(&xdna->gpusvm.notifier_lock);
>> if (list_empty(&abo->mem.umap_list))
>> abo->mem.uva = addr;
>> list_add_tail(&mapp->node, &abo->mem.umap_list);
>> - up_write(&xdna->notifier_lock);
>> + up_write(&xdna->gpusvm.notifier_lock);
>> return 0;
>> -
>> -free_pfns:
>> - kvfree(mapp->range.hmm_pfns);
>> -free_map:
>> - kfree(mapp);
>> - return ret;
>> }
>> static struct amdxdna_gem_obj *
>> @@ -547,21 +569,13 @@ 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)
>> +static void amdxdna_mark_map_invalid(struct amdxdna_gem_obj *abo)
>> {
>> struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
>> - struct amdxdna_umap *mapp;
>> - down_write(&xdna->notifier_lock);
>> + down_write(&xdna->gpusvm.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);
>> + up_write(&xdna->gpusvm.notifier_lock);
>> }
>> static int amdxdna_remap_pages(struct amdxdna_gem_obj *abo,
>> @@ -628,7 +642,7 @@ static int amdxdna_insert_pages(struct
>> amdxdna_gem_obj *abo,
>> }
>> if (!(vma->vm_flags & VM_PFNMAP))
>> - amdxdna_mark_mapp_invalid(abo, vma);
>> + amdxdna_mark_map_invalid(abo);
>> /* Drop the reference drm_gem_mmap_obj() acquired.*/
>> drm_gem_object_put(to_gobj(abo));
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h
>> b/drivers/accel/amdxdna/amdxdna_gem.h
>> index 5c429869992e8..2deead107d6b1 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.h
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
>> @@ -7,19 +7,19 @@
>> #define _AMDXDNA_GEM_H_
>> #include <drm/drm_gem_shmem_helper.h>
>> -#include <linux/hmm.h>
>> +#include <drm/drm_gpusvm.h>
>> #include <linux/iova.h>
>> #include "amdxdna_pci_drv.h"
>> struct amdxdna_umap {
>> struct mmu_interval_notifier notifier;
>> - struct hmm_range range;
>> + struct drm_gpusvm_pages pages;
>> + unsigned long start;
>> + unsigned long end;
>> + bool read_only;
>> struct amdxdna_gem_obj *abo;
>> struct list_head node;
>> struct kref refcnt;
>> - bool invalid;
>> - bool unmapped;
>> - struct mutex lock; /* protect range */
>> };
>> struct amdxdna_mem {
>> @@ -27,10 +27,16 @@ struct amdxdna_mem {
>> u64 dma_addr;
>> size_t size;
>> struct list_head umap_list;
>> + /*
>> + * True, if the pages of the user mappings have to be faulted in
>> before
>> + * the device accesses them. A mapping which mmap() populated
>> upfront,
>> + * which is a VM_PFNMAP one, must not be faulted in:
>> hmm_range_fault()
>> + * rejects such a VMA.
>> + */
>> bool map_invalid;
>> /*
>> * Cache the first mmap uva as PASID addr, which can be
>> accessed by driver
>> - * without taking notifier_lock.
>> + * without taking the gpusvm notifier lock.
>> */
>> u64 uva;
>> };
>> @@ -103,8 +109,9 @@ static inline u64 amdxdna_obj_dma_addr(struct
>> amdxdna_gem_obj *abo)
>> abo->mem.dma_addr : amdxdna_gem_uva(abo);
>> }
>> -void amdxdna_umap_put(struct amdxdna_umap *mapp);
>> -int amdxdna_populate_range(struct amdxdna_gem_obj *abo);
>> +int amdxdna_gem_umap_populate(struct amdxdna_gem_obj *abo,
>> + unsigned long deadline);
>> +bool amdxdna_gem_umap_stale(struct amdxdna_gem_obj *abo);
>> void amdxdna_gem_heap_free(struct amdxdna_client *client, struct
>> amdxdna_gem_obj *abo);
>> struct drm_gem_object *
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> index f233eb70a2957..18bd7bdd12969 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> @@ -363,6 +363,7 @@ static void amdxdna_xdna_drm_release(struct
>> drm_device *drm, void *res)
>> struct amdxdna_dev *xdna = res;
>> amdxdna_carveout_fini(xdna);
>> + drm_gpusvm_fini(&xdna->gpusvm);
>> }
>> static int amdxdna_probe(struct pci_dev *pdev, const struct
>> pci_device_id *id)
>> @@ -389,18 +390,18 @@ static int amdxdna_probe(struct pci_dev *pdev,
>> const struct pci_device_id *id)
>> if (ret)
>> return ret;
>> - init_rwsem(&xdna->notifier_lock);
>> + ret = drm_gpusvm_init(&xdna->gpusvm, "amdxdna", NULL, 0, 0, 0,
>> NULL,
>> + NULL, 0);
>> + if (ret)
>> + return ret;
>> +
>> INIT_LIST_HEAD(&xdna->client_list);
>> pci_set_drvdata(pdev, xdna);
>> ret = drmm_add_action(ddev, amdxdna_xdna_drm_release, xdna);
>> - if (ret)
>> + if (ret) {
>> + drm_gpusvm_fini(&xdna->gpusvm);
>> return ret;
>> -
>> - if (IS_ENABLED(CONFIG_LOCKDEP)) {
>> - fs_reclaim_acquire(GFP_KERNEL);
>> - might_lock(&xdna->notifier_lock);
>> - fs_reclaim_release(GFP_KERNEL);
>> }
>> ret = amdxdna_iommu_init(xdna);
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> b/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> index 27593190dcf67..7663b883d4f8b 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> @@ -7,6 +7,7 @@
>> #define _AMDXDNA_PCI_DRV_H_
>> #include <drm/amdxdna_accel.h>
>> +#include <drm/drm_gpusvm.h>
>> #include <drm/drm_mm.h>
>> #include <drm/drm_print.h>
>> #include <linux/iommu.h>
>> @@ -118,7 +119,7 @@ struct amdxdna_dev {
>> struct list_head client_list;
>> struct mutex client_lock; /* client_list */
>> struct amdxdna_drm_query_firmware_version fw_ver;
>> - struct rw_semaphore notifier_lock; /* for mmu notifier*/
>> + struct drm_gpusvm gpusvm; /* notifier lock for user
>> mappings */
>> struct workqueue_struct *notifier_wq;
>> struct iommu_group *group;
>
next prev parent reply other threads:[~2026-11-10 18:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-06 18:42 [PATCH V1 1/2] drm/gpusvm: add drm_gpusvm_pages unmapped and retry helpers Lizhi Hou
2026-11-06 18:42 ` [PATCH V1 2/2] accel/amdxdna: use drm_gpusvm_pages for user mapping tracking Lizhi Hou
2026-11-09 19:44 ` Max Zhen
2026-11-10 18:16 ` Lizhi Hou [this message]
2026-11-09 8:56 ` [PATCH V1 1/2] drm/gpusvm: add drm_gpusvm_pages unmapped and retry helpers Huang, Honglei1
2026-11-09 19:53 ` Lizhi Hou
2026-11-09 22:35 ` Matthew Brost
2026-11-10 18:16 ` Lizhi Hou
2026-11-09 22:34 ` Matthew Brost
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a12185d5-fe6c-4714-78b9-5c45a45c7c30@amd.com \
--to=lizhi.hou@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=himal.prasad.ghimiray@intel.com \
--cc=honghuan@amd.com \
--cc=karol.wachowski@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=matthew.brost@intel.com \
--cc=max.zhen@amd.com \
--cc=ogabbay@kernel.org \
--cc=quic_jhugo@quicinc.com \
--cc=sonal.santan@amd.com \
--cc=thomas.hellstrom@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®