[PATCH v2] drm/amdkfd: Use exclusive bounds for SVM split alignment checks

[Gerhard Schwanzer]

SVM ranges use inclusive page indices: prange->last is the last page in
the range. The split-remap logic introduced by commit 448ee45353ef
("drm/amdkfd: Use huge page size to check split svm range alignment")
uses ALIGN_DOWN(prange->last, 512) to determine whether the original
range can contain a 2MB huge-page mapping.

That aligns the last page itself down. Thus a range ending one page
before the next 2MB boundary is classified as if the final 2MB block did
not exist. When such a range is split inside that final block, the
split head or tail can be left off the remap list even though it was
derived from an original range that may have PMD mappings.

Use prange->last + 1 as the exclusive upper bound when computing the
original range's last 2MB-aligned boundary. Then use the actual split
boundary for the head and tail alignment checks: tail->start for a tail
split, and new_start for a head split. new_start is equivalent to
head->last + 1 and directly names the exclusive end of the split head.

Using head->last for the head-side check can both remap a head that ends
exactly one page before a 2MB boundary and miss a head whose split
boundary is one page after such a boundary. Philip Yang pointed out in
the review of the original change that this condition should use
head->last + 1 or new_start.

Xiaogang Chen identified the inclusive-last cause and posted the
candidate fix in the regression thread. With the culprit change active
and the local revert not applied, the unchanged C/HSA reproducer
completes 10/10 runs with this change on an RX 7600 XT.

Fixes: 448ee45353ef ("drm/amdkfd: Use huge page size to check split svm range alignment")
Cc: stable@xxxxxxxxxxxxxxx
Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4914
Link: https://lore.kernel.org/stable/IA1PR12MB85172F7FE9157C092EDA46A0E3112@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Link: https://lore.kernel.org/all/32ce2b72-aa16-4202-9f99-92e3cd4408bc@xxxxxxx/
Suggested-by: Xiaogang Chen <xiaogang.chen@xxxxxxx>
Signed-off-by: Gerhard Schwanzer <geschw@xxxxx>
---
Changes in v2:
- Use new_start for the head-side split-boundary checks, matching Philip
  Yang's original v4 review guidance and avoiding aligned-head false
  positives / unaligned-head false negatives.
- Keep the tail-side last + 1 exclusive-bound fix from Xiaogang's public
  candidate, which fixes the reproduced RX 7600 XT SDMA0 fault.

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

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 72cfb4a..59b15d5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1144,7 +1144,7 @@ static int
 svm_range_split_tail(struct svm_range *prange, uint64_t new_last,
 		     struct list_head *insert_list, struct list_head *remap_list)
 {
-	unsigned long last_align_down = ALIGN_DOWN(prange->last, 512);
+	unsigned long last_align_down = ALIGN_DOWN(prange->last + 1, 512);
 	unsigned long start_align = ALIGN(prange->start, 512);
 	bool huge_page_mapping = last_align_down > start_align;
 	struct svm_range *tail = NULL;
@@ -1168,7 +1168,7 @@ static int
 svm_range_split_head(struct svm_range *prange, uint64_t new_start,
 		     struct list_head *insert_list, struct list_head *remap_list)
 {
-	unsigned long last_align_down = ALIGN_DOWN(prange->last, 512);
+	unsigned long last_align_down = ALIGN_DOWN(prange->last + 1, 512);
 	unsigned long start_align = ALIGN(prange->start, 512);
 	bool huge_page_mapping = last_align_down > start_align;
 	struct svm_range *head = NULL;
@@ -1181,8 +1181,8 @@ svm_range_split_head(struct svm_range *prange, uint64_t new_start,
 
 	list_add(&head->list, insert_list);
 
-	if (huge_page_mapping && head->last + 1 > start_align &&
-	    head->last + 1 < last_align_down && (!IS_ALIGNED(head->last, 512)))
+	if (huge_page_mapping && new_start > start_align &&
+	    new_start < last_align_down && !IS_ALIGNED(new_start, 512))
 		list_add(&head->update_list, remap_list);
 
 	return 0;

base-commit: 2c7d5b0a5ec0fc713a7f350806553643e87e6f43
-- 
2.54.0