Re: [PATCH V4 15/19] accel/amdxdna: Make populate_range common for AIE2 and AIE4

[Lizhi Hou <lizhi.hou@amd.com>]

On 10/9/26 15:53, David Zhang wrote:
> When a user mapping of a BO is invalidated, the MMU notifier sets
> map_invalid and command submission must fault the range back in
> before attaching the job fence. AIE2 does this in
> aie2_populate_range(), which AIE4 command submission also needs.
>
> Move aie2_populate_range() from aie2_ctx.c into amdxdna_gem.c as
> amdxdna_populate_range(), next to the MMU notifier code, and declare
> it in amdxdna_gem.h. No functional change for AIE2.
>
> Signed-off-by: David Zhang <yidong.zhang@amd.com>
> ---
>   drivers/accel/amdxdna/aie2_ctx.c    | 75 +----------------------------
>   drivers/accel/amdxdna/amdxdna_gem.c | 73 ++++++++++++++++++++++++++++
>   drivers/accel/amdxdna/amdxdna_gem.h |  1 +
>   3 files changed, 75 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 8c1b29964f8f..05ed71e641c3 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -1105,79 +1105,6 @@ int aie2_hwctx_sync_debug_bo(struct amdxdna_hwctx *hwctx, u32 debug_bo_hdl)
>   	return ret;
>   }
>   
> -static int aie2_populate_range(struct amdxdna_gem_obj *abo)
> -{
> -	struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> -	struct amdxdna_umap *mapp;
> -	unsigned long timeout;
> -	struct mm_struct *mm;
> -	bool found;
> -	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;
> -
> -		if (mapp->invalid && 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;
> -	}
> -
> -	ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
> -	if (ret)
> -		goto put_mm;
> -
> -	down_write(&xdna->notifier_lock);
> -	if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {
> -		up_write(&xdna->notifier_lock);
> -		amdxdna_umap_put(mapp);
> -		mmput(mm);
> -		goto again;
> -	}
> -	mapp->invalid = false;
> -	up_write(&xdna->notifier_lock);
> -	amdxdna_umap_put(mapp);
> -	mmput(mm);
> -	goto again;
> -
> -put_mm:
> -	amdxdna_umap_put(mapp);
> -	mmput(mm);
> -	return ret == -EBUSY ? -ETIME : ret;
> -}
> -
>   int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, u64 *seq)
>   {
>   	struct amdxdna_dev *xdna = hwctx->client->xdna;
> @@ -1237,7 +1164,7 @@ int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,
>   				goto cleanup_job;
>   			}
>   
> -			ret = aie2_populate_range(abo);
> +			ret = amdxdna_populate_range(abo);
>   			if (ret)
>   				goto cleanup_job;
>   			goto retry;
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index e1b72cef51bd..320604ec157f 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -337,6 +337,79 @@ void amdxdna_umap_put(struct amdxdna_umap *mapp)
>   	kref_put(&mapp->refcnt, amdxdna_umap_release);
>   }
>   
> +int amdxdna_populate_range(struct amdxdna_gem_obj *abo)
> +{
> +	struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> +	struct amdxdna_umap *mapp;
> +	unsigned long timeout;
> +	struct mm_struct *mm;
> +	bool found;
> +	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;
> +
> +		if (mapp->invalid && 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;
> +	}
> +
> +	ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
> +	if (ret)
> +		goto put_mm;
> +
> +	down_write(&xdna->notifier_lock);
> +	if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {
> +		up_write(&xdna->notifier_lock);
> +		amdxdna_umap_put(mapp);
> +		mmput(mm);
> +		goto again;
> +	}
> +	mapp->invalid = false;
> +	up_write(&xdna->notifier_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)
>   {
>   	struct amdxdna_gem_obj *abo = container_of(work, struct amdxdna_gem_obj,
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
> index 9b4aa21a37c9..977548e19ae4 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.h
> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
> @@ -103,6 +103,7 @@ static inline u64 amdxdna_obj_dma_addr(struct amdxdna_gem_obj *abo)
>   }
>   
>   void amdxdna_umap_put(struct amdxdna_umap *mapp);
> +int amdxdna_populate_range(struct amdxdna_gem_obj *abo);
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   void amdxdna_gem_heap_free(struct amdxdna_client *client, struct amdxdna_gem_obj *abo);
>   
>   struct drm_gem_object *