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

[Alex Deucher]

On Tue, Jun 16, 2026 at 6:56 AM Gerhard Schwanzer <geschw@xxxxx> wrote:
>
> 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>

Sorry, I missed this patch earlier.
Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>

Applied.  thanks.

Alex

> ---
> 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
>
>