Re: [PATCH V5 14/19] accel/amdxdna: Move HMM invalidate wait into common GEM code

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

On 10/19/26 09:47, David Zhang wrote:
> When a user mapping of a BO is invalidated, the driver must wait for
> the BO's reservation fences before the pages are released. AIE2 does
> this in aie2_hmm_invalidate() through the .hmm_invalidate callback,
> and AIE4 needs the same wait.
>
> Do the wait directly in amdxdna_hmm_invalidate() in amdxdna_gem.c and
> remove the .hmm_invalidate callback and aie2_hmm_invalidate(). Devices
> that do not attach fences to user BOs return from the wait at once.
>
> Co-developed-by: Hayden Laccabue <Hayden.Laccabue@amd.com>
> Signed-off-by: Hayden Laccabue <Hayden.Laccabue@amd.com>
> Signed-off-by: David Zhang <yidong.zhang@amd.com>
> ---
>   drivers/accel/amdxdna/aie2_ctx.c        | 11 -----------
>   drivers/accel/amdxdna/aie2_pci.c        |  1 -
>   drivers/accel/amdxdna/aie2_pci.h        |  1 -
>   drivers/accel/amdxdna/amdxdna_gem.c     |  9 +++++++--
>   drivers/accel/amdxdna/amdxdna_pci_drv.h |  1 -
>   5 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 6f63a8f94215..1c3cc32f119b 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -1288,17 +1288,6 @@ int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,
>   	return ret;
>   }
>   
> -void aie2_hmm_invalidate(struct amdxdna_gem_obj *abo,
> -			 unsigned long cur_seq)
> -{
> -	/*
> -	 * The pages must not be released while the device may still access
> -	 * them, so this wait must be neither interruptible nor time limited.
> -	 */
> -	dma_resv_wait_timeout(to_gobj(abo)->resv, DMA_RESV_USAGE_BOOKKEEP,
> -			      false, MAX_SCHEDULE_TIMEOUT);
> -}
> -
>   int aie2_hwctx_heap_expand(struct amdxdna_hwctx *hwctx,
>   			   struct amdxdna_gem_obj *heap)
>   {
> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
> index 1b029d33a24b..690ad035463c 100644
> --- a/drivers/accel/amdxdna/aie2_pci.c
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -1228,7 +1228,6 @@ const struct amdxdna_dev_ops aie2_ops = {
>   	.hwctx_config = aie2_hwctx_config,
>   	.hwctx_sync_debug_bo = aie2_hwctx_sync_debug_bo,
>   	.cmd_submit = aie2_cmd_submit,
> -	.hmm_invalidate = aie2_hmm_invalidate,
>   	.get_array = aie2_get_array,
>   	.get_dev_revision = aie2_get_dev_rev,
>   	.hwctx_heap_expand = aie2_hwctx_heap_expand,
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> index 709e7545c65f..13391275df31 100644
> --- a/drivers/accel/amdxdna/aie2_pci.h
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -272,7 +272,6 @@ int aie2_hwctx_sync_debug_bo(struct amdxdna_hwctx *hwctx, u32 debug_bo_hdl);
>   void aie2_hwctx_suspend(struct amdxdna_client *client);
>   int aie2_hwctx_resume(struct amdxdna_client *client);
>   int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, u64 *seq);
> -void aie2_hmm_invalidate(struct amdxdna_gem_obj *abo, unsigned long cur_seq);
>   int aie2_hwctx_heap_expand(struct amdxdna_hwctx *hwctx, struct amdxdna_gem_obj *heap);
>   
>   #endif /* _AIE2_PCI_H_ */
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 37bfa5058095..0af1506f6363 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -12,6 +12,7 @@
>   #include <drm/gpu_scheduler.h>
>   #include <linux/dma-buf.h>
>   #include <linux/dma-direct.h>
> +#include <linux/dma-resv.h>
>   #include <linux/iosys-map.h>
>   #include <linux/pagemap.h>
>   #include <linux/swap.h>
> @@ -249,8 +250,12 @@ static bool amdxdna_hmm_invalidate(struct mmu_interval_notifier *mni,
>   	mmu_interval_set_seq(&mapp->notifier, cur_seq);
>   	up_write(&xdna->notifier_lock);
>   
> -	if (xdna->dev_info->ops->hmm_invalidate)
> -		xdna->dev_info->ops->hmm_invalidate(abo, cur_seq);
> +	/*
> +	 * The pages must not be released while the device may still access
> +	 * them, so this wait must be neither interruptible nor time limited.
> +	 */
> +	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);
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> index b27492e2bf8a..274e5b609882 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> @@ -63,7 +63,6 @@ struct amdxdna_dev_ops {
>   	int (*hwctx_config)(struct amdxdna_hwctx *hwctx, u32 type, u64 value, void *buf, u32 size);
>   	int (*hwctx_sync_debug_bo)(struct amdxdna_hwctx *hwctx, u32 debug_bo_hdl);
>   	int (*hwctx_heap_expand)(struct amdxdna_hwctx *hwctx, struct amdxdna_gem_obj *heap);
> -	void (*hmm_invalidate)(struct amdxdna_gem_obj *abo, unsigned long cur_seq);
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   	int (*cmd_submit)(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, u64 *seq);
>   	int (*cmd_wait)(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout);
>   	int (*get_aie_info)(struct amdxdna_client *client, struct amdxdna_drm_get_info *args);