Re: [PATCH] drm/amdkfd: correct the SVM DMA device unmap direction

["Christian König" <christian.koenig@amd.com>]

Am 05.11.24 um 03:33 schrieb Prike Liang:
> The SVM DMA device unmap direction should be same as
> the DMA map process.

At least of hand that looks like it's only papering over a major problem.

Why are DMA ranges for SVM mapped with a direction in the first place? 
That is usually not something we should do.

Regards,
Christian.

>
> Signed-off-by: Prike Liang <Prike.Liang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 4 ++--
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 6 +++---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 3 ++-
>   3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index eacfeb32f35d..9d83bb9dd004 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -445,7 +445,7 @@ svm_migrate_vma_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
>   			 mpages, cpages, migrate.npages);
>   
> -	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
> +	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages, DMA_TO_DEVICE);
>   
>   out_free:
>   	kvfree(buf);
> @@ -750,7 +750,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   	svm_migrate_copy_done(adev, mfence);
>   	migrate_vma_finalize(&migrate);
>   
> -	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages);
> +	svm_range_dma_unmap_dev(adev->dev, scratch, 0, npages, DMA_FROM_DEVICE);
>   
>   out_free:
>   	kvfree(buf);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 3e2911895c74..c21485fe6cbb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -233,9 +233,9 @@ svm_range_dma_map(struct svm_range *prange, unsigned long *bitmap,
>   }
>   
>   void svm_range_dma_unmap_dev(struct device *dev, dma_addr_t *dma_addr,
> -			 unsigned long offset, unsigned long npages)
> +			 unsigned long offset, unsigned long npages,
> +				enum dma_data_direction dir)
>   {
> -	enum dma_data_direction dir = DMA_BIDIRECTIONAL;
>   	int i;
>   
>   	if (!dma_addr)
> @@ -272,7 +272,7 @@ void svm_range_dma_unmap(struct svm_range *prange)
>   		}
>   		dev = &pdd->dev->adev->pdev->dev;
>   
> -		svm_range_dma_unmap_dev(dev, dma_addr, 0, prange->npages);
> +		svm_range_dma_unmap_dev(dev, dma_addr, 0, prange->npages, DMA_BIDIRECTIONAL);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index bddd24f04669..5370d68bc5b2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -182,7 +182,8 @@ void svm_range_add_list_work(struct svm_range_list *svms,
>   			     enum svm_work_list_ops op);
>   void schedule_deferred_list_work(struct svm_range_list *svms);
>   void svm_range_dma_unmap_dev(struct device *dev, dma_addr_t *dma_addr,
> -			 unsigned long offset, unsigned long npages);
> +			 unsigned long offset, unsigned long npages,
> +			 enum dma_data_direction dir);
>   void svm_range_dma_unmap(struct svm_range *prange);
>   int svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
>   		       uint64_t *svm_priv_data_size);