Re: [PATCH] drm/amdkfd: svm range always mapped flag not working on APU

2023-12-14 Thread Felix Kuehling



On 2023-12-14 13:58, Philip Yang wrote:

On gfx943 APU there is VRAM and page migration,


Is there a "no" missing here?



  queue CWSR area, svm
range with always mapped flag, is not mapped to GPU correctly. This
works fine if retry fault on CWSR area can be recovered, but could cause
deadlock if there is another retry fault recover waiting for CWSR to
finish.

Fix this by mapping svm range with always mapped flag to GPU with ACCESS
attribute if XNACK ON.

There is side effect, because all GPUs have ACCESS attribute by default
on new svm range with XNACK on, the CWSR area will be mapped to all GPUs
after this change. This side effect will be fixed with Thunk change to
set CWSR svm range with ACCESS_IN_PLACE attribute on the GPU that user
queue is created.

Signed-off-by: Philip Yang 


With the commit description fixed, this patch is

Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 18 --
  1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 2834fb351818..41656e85ee57 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1607,18 +1607,24 @@ static int svm_range_validate_and_map(struct mm_struct 
*mm,
if (test_bit(gpuidx, prange->bitmap_access))
bitmap_set(ctx->bitmap, gpuidx, 1);
}
+
+   /*
+* If prange is already mapped or with always mapped flag,
+* update mapping on GPUs with ACCESS attribute
+*/
+   if (bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
+   if (prange->mapped_to_gpu ||
+   prange->flags & 
KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)
+   bitmap_copy(ctx->bitmap, prange->bitmap_access, 
MAX_GPU_INSTANCE);
+   }
} else {
bitmap_or(ctx->bitmap, prange->bitmap_access,
  prange->bitmap_aip, MAX_GPU_INSTANCE);
}
  
  	if (bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {

-   bitmap_copy(ctx->bitmap, prange->bitmap_access, 
MAX_GPU_INSTANCE);
-   if (!prange->mapped_to_gpu ||
-   bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
-   r = 0;
-   goto free_ctx;
-   }
+   r = 0;
+   goto free_ctx;
}
  
  	if (prange->actual_loc && !prange->ttm_res) {


[PATCH] drm/amdkfd: svm range always mapped flag not working on APU

2023-12-14 Thread Philip Yang
On gfx943 APU there is VRAM and page migration, queue CWSR area, svm
range with always mapped flag, is not mapped to GPU correctly. This
works fine if retry fault on CWSR area can be recovered, but could cause
deadlock if there is another retry fault recover waiting for CWSR to
finish.

Fix this by mapping svm range with always mapped flag to GPU with ACCESS
attribute if XNACK ON.

There is side effect, because all GPUs have ACCESS attribute by default
on new svm range with XNACK on, the CWSR area will be mapped to all GPUs
after this change. This side effect will be fixed with Thunk change to
set CWSR svm range with ACCESS_IN_PLACE attribute on the GPU that user
queue is created.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 2834fb351818..41656e85ee57 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1607,18 +1607,24 @@ static int svm_range_validate_and_map(struct mm_struct 
*mm,
if (test_bit(gpuidx, prange->bitmap_access))
bitmap_set(ctx->bitmap, gpuidx, 1);
}
+
+   /*
+* If prange is already mapped or with always mapped flag,
+* update mapping on GPUs with ACCESS attribute
+*/
+   if (bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
+   if (prange->mapped_to_gpu ||
+   prange->flags & 
KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)
+   bitmap_copy(ctx->bitmap, prange->bitmap_access, 
MAX_GPU_INSTANCE);
+   }
} else {
bitmap_or(ctx->bitmap, prange->bitmap_access,
  prange->bitmap_aip, MAX_GPU_INSTANCE);
}
 
if (bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
-   bitmap_copy(ctx->bitmap, prange->bitmap_access, 
MAX_GPU_INSTANCE);
-   if (!prange->mapped_to_gpu ||
-   bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
-   r = 0;
-   goto free_ctx;
-   }
+   r = 0;
+   goto free_ctx;
}
 
if (prange->actual_loc && !prange->ttm_res) {
-- 
2.35.1