Re: [PATCH v2 1/4] drm/amdkfd: Add awareness of THP of device and system RAM in kfd svm driver

["Chen, Xiaogang" <xiaogang.chen@amd.com>]

On 10/6/2026 4:40 PM, Felix Kuehling wrote:
> On 2026-09-04 15:54, Xiaogang.Chen wrote:
>> From: Xiaogang Chen <xiaogang.chen@amd.com>
>>
>> Extend kfd/svm function to allocate HPAGE_PMD_SIZE based device 
>> memory by buddy
>> allocator, each drm_buddy_block is HPAGE_PMD_SIZE aligned and to 
>> allocate THP
>> system ram by vma_alloc_folio.
>>
>> Introduce SVM_RANGE_DMA_THP flag that indicates dma map of THP. THP dma
>> addresss will use this flag.
>>
>> Add dev_pagemap_ops->folio_split callback that is called by 
>> folio_split when
>> core MM splits device memory folio.
>>
>> These are preparations for following support for (THP) migration of zone
>> device-private memory, no function change.
>>
>> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 50 +++++++++++++++++++-----
>>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 15 +++++--
>>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h     |  1 +
>>   3 files changed, 54 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> index 253365a8257e..813f3c1d29dc 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> @@ -217,14 +217,19 @@ svm_migrate_addr_to_pfn(struct amdgpu_device 
>> *adev, unsigned long addr)
>>   }
>>     static void
>> -svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn)
>> +svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn,
>> +               int order)
>>   {
>>       struct page *page;
>> +    struct folio *folio;
>>         page = pfn_to_page(pfn);
>> +    folio = page_folio(page);
>> +
>> +    zone_device_folio_init(folio, folio->pgmap, order);
>> +
>> +    folio_set_zone_device_data(folio, prange->svm_bo);
>
> I see that folio_set_zone_device_data checks that folio is 
> device_private. This will fail for device_coherent pages. We can't use 
> this function without breaking MI200 A+A.

Yes, I was looking if this work has changes that can affect 
device_coherent. It is one of them. This work is for GPU with device 
private memory by now. I will change this part to:

if (folio_is_device_private(folio))
     folio_set_zone_device_data(folio, prange->svm_bo);
else
     folio->page.zone_device_data = prange->svm_bo;

>
>
>>       svm_range_bo_ref(prange->svm_bo);
>> -    page->zone_device_data = prange->svm_bo;
>> -    zone_device_page_init(page, page_pgmap(page), 0);
>>   }
>>     static void
>> @@ -247,11 +252,17 @@ svm_migrate_addr(struct amdgpu_device *adev, 
>> struct page *page)
>>   }
>>     static struct page *
>> -svm_migrate_get_sys_page(struct vm_area_struct *vma, unsigned long 
>> addr)
>> +svm_migrate_get_sys_page(struct vm_area_struct *vma, unsigned long 
>> addr,
>> +              unsigned long order)
>>   {
>>       struct page *page;
>>   -    page = alloc_page_vma(GFP_HIGHUSER, vma, addr);
>> +    if (order)
>> +        page = folio_page(vma_alloc_folio(GFP_HIGHUSER,
>> +                          order, vma, addr), 0);
>> +    else
>> +        page = alloc_page_vma(GFP_HIGHUSER, vma, addr);
>> +
>>       if (page)
>>           lock_page(page);
>>   @@ -265,8 +276,12 @@ static unsigned long 
>> svm_migrate_successful_pages(struct migrate_vma *migrate)
>>         for (i = 0; i < migrate->npages; i++) {
>>           if (migrate->dst[i] & MIGRATE_PFN_VALID &&
>> -            migrate->src[i] & MIGRATE_PFN_MIGRATE)
>> -            mpages++;
>> +            migrate->src[i] & MIGRATE_PFN_MIGRATE) {
>> +                if (migrate->dst[i] & MIGRATE_PFN_COMPOUND)
>> +                    mpages += HPAGE_PMD_NR;
>> +                else
>> +                    mpages++;
>> +            }
>
> Please fix the indentation.  This is one tab too deep. The alignment 
> of the condition should remain unchanged.
ok
>
>
>>       }
>>       return mpages;
>>   }
>> @@ -300,7 +315,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, 
>> struct svm_range *prange,
>>           if (migrate->src[i] & MIGRATE_PFN_MIGRATE) {
>>               dst[i] = cursor.start + (j << PAGE_SHIFT);
>>               migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
>> -            svm_migrate_get_vram_page(prange, migrate->dst[i]);
>> +            svm_migrate_get_vram_page(prange, migrate->dst[i], 0);
>>               migrate->dst[i] = migrate_pfn(migrate->dst[i]);
>>               mpages++;
>>           }
>> @@ -568,6 +583,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, 
>> uint32_t best_loc,
>>       return r < 0 ? r : 0;
>>   }
>>   +/* folio can be compound folio or single page */
>>   static void svm_migrate_folio_free(struct folio *folio)
>>   {
>>       struct page *page = &folio->page;
>> @@ -630,7 +646,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>               j = 0;
>>           }
>>   -        dpage = svm_migrate_get_sys_page(migrate->vma, addr);
>> +        dpage = svm_migrate_get_sys_page(migrate->vma, addr, 0);
>>           if (!dpage) {
>>               pr_debug("failed get page svms 0x%p [0x%lx 0x%lx]\n",
>>                    prange->svms, prange->start, prange->last);
>> @@ -1033,9 +1049,25 @@ static vm_fault_t svm_migrate_to_ram(struct 
>> vm_fault *vmf)
>>       return r ? VM_FAULT_SIGBUS : 0;
>>   }
>>   +static void svm_migrate_folio_split(struct folio *head, struct 
>> folio *tail)
>> +{
>> +    struct svm_range_bo *svm_bo;
>> +
>> +    if (tail == NULL)
>> +        return;
>> +
>> +    tail->pgmap = head->pgmap;
>> +    tail->mapping = head->mapping;
>> +
>> +    svm_bo = folio_zone_device_data(head);
>> +    folio_set_zone_device_data(tail, svm_bo);
>> +    svm_range_bo_ref(svm_bo);
>> +}
>> +
>>   static const struct dev_pagemap_ops svm_migrate_pgmap_ops = {
>>       .folio_free        = svm_migrate_folio_free,
>>       .migrate_to_ram        = svm_migrate_to_ram,
>> +    .folio_split        = svm_migrate_folio_split,
>>   };
>>     /* Each VRAM page uses sizeof(struct page) on system memory */
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> index fa4054d51f60..6b783d12bce4 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> @@ -245,7 +245,16 @@ void svm_range_dma_unmap_dev(struct device *dev, 
>> dma_addr_t *dma_addr,
>>           if (!svm_is_valid_dma_mapping_addr(dev, dma_addr[i]))
>>               continue;
>>           pr_debug_ratelimited("unmap 0x%llx\n", dma_addr[i] >> 
>> PAGE_SHIFT);
>> -        dma_unmap_page(dev, dma_addr[i], PAGE_SIZE, dir);
>> +
>> +        /* dma unmap of THP */
>> +        if (dma_addr[i] & SVM_RANGE_DMA_THP) {
>> +
>
> Unnecessary empty line.
>
>
ok
>> +            dma_addr[i] &= ~SVM_RANGE_DMA_THP;
>> +            dma_unmap_page(dev, dma_addr[i], PAGE_SIZE*HPAGE_PMD_NR,
>> +                                   DMA_BIDIRECTIONAL);
>> +        } else
>> +            dma_unmap_page(dev, dma_addr[i], PAGE_SIZE, dir);
>
> The else-branch should also use {} braces if the if-branch does.
ok
>
>
>> +
>>           dma_addr[i] = 0;
>>       }
>>   }
>> @@ -578,10 +587,10 @@ svm_range_vram_node_new(struct kfd_node *node, 
>> struct svm_range *prange,
>>       }
>>         memset(&bp, 0, sizeof(bp));
>> -    bp.size = prange->npages * PAGE_SIZE;
>> +    bp.size = ALIGN(prange->npages * PAGE_SIZE, HPAGE_PMD_SIZE);
>
> This wastes memory for small ranges. Maybe guard this so it only 
> applies to ranges that are larger than HPAGE_PMD_NR pages.

I am think this too. The waste is 2MB at most instead of current 4KB. I 
will change allocation size/aliment according to request size.

Regards

Xiaogang

>
>
>>       bp.bo_ptr_size = sizeof(struct svm_range_bo);
>>       bp.destroy = svm_range_bo_destroy;
>> -    bp.byte_align = PAGE_SIZE;
>> +    bp.byte_align = HPAGE_PMD_SIZE;
>
> Same as above. There is no need to align smaller allocations.
>
> Regards,
>   Felix
>
>
>>       bp.domain = AMDGPU_GEM_DOMAIN_VRAM;
>>       bp.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>>       bp.flags |= clear ? AMDGPU_GEM_CREATE_VRAM_CLEARED : 0;
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
>> index c7d7adae4476..f2b3a05cd8cf 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
>> @@ -35,6 +35,7 @@
>>   #include "kfd_priv.h"
>>     #define SVM_RANGE_VRAM_DOMAIN (1UL << 0)
>> +#define SVM_RANGE_DMA_THP (1UL << 1)
>>   #define SVM_ADEV_PGMAP_OWNER(adev)\
>>               ((adev)->hive ? (void *)(adev)->hive : (void *)(adev))