Re: [PATCH v1 1/8] mm/migrate_device: Add migrate PFN flag to track device private pages

[Jordan Niethe <jniethe@nvidia.com>]

Hi,


On 1/1/26 04:03, Kuehling, Felix wrote:
> On 2025-12-30 23:31, Jordan Niethe wrote:
>> A future change will remove device private pages from the physical
>> address space. This will mean that device private pages no longer have
>> normal PFN and must be handled separately.
>>
>> Prepare for this by adding a MIGRATE_PFN_DEVICE flag to indicate
>> that a migrate pfn contains a PFN for a device private page.
> 
> Thanks for doing this. Some comments inline regarding DEVICE_COHERENT 
> pages. I suspect this will have ripple effects on the rest of the patch 
> series, at least in patch 8, but I haven't looked at that in detail yet.


Ah thanks - I missed that MEMORY_DEVICE_COHERENT and 
MEMORY_DEVICE_PRIVATE were
sharing some code paths.


> 
> 
>>
>> Signed-off-by: Jordan Niethe <jniethe@nvidia.com>
>> Signed-off-by: Alistair Popple <apopple@nvidia.com>
>>
>> ---
>> v1:
>> - Update for HMM huge page support
>> - Update existing drivers to use MIGRATE_PFN_DEVICE
>> ---
>>   arch/powerpc/kvm/book3s_hv_uvmem.c       |  2 +-
>>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  3 ++-
>>   drivers/gpu/drm/drm_pagemap.c            |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_dmem.c   |  2 +-
>>   include/linux/migrate.h                  |  1 +
>>   lib/test_hmm.c                           |  4 ++--
>>   mm/migrate_device.c                      | 11 ++++++++---
>>   7 files changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/ 
>> book3s_hv_uvmem.c
>> index e5000bef90f2..dac5d6454920 100644
>> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
>> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
>> @@ -784,7 +784,7 @@ static int kvmppc_svm_page_in(struct 
>> vm_area_struct *vma,
>>           }
>>       }
>> -    *mig.dst = migrate_pfn(page_to_pfn(dpage));
>> +    *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_DEVICE;
>>       migrate_vma_pages(&mig);
>>   out_finalize:
>>       migrate_vma_finalize(&mig);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/ 
>> drm/amd/amdkfd/kfd_migrate.c
>> index af53e796ea1b..0257c6e7f680 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> @@ -303,7 +303,8 @@ svm_migrate_copy_to_vram(struct kfd_node *node, 
>> struct svm_range *prange,
>>               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]);
>> -            migrate->dst[i] = migrate_pfn(migrate->dst[i]);
>> +            migrate->dst[i] = migrate_pfn(migrate->dst[i]) |
>> +                      MIGRATE_PFN_DEVICE;
> 
> On some of our GPUs we use DEVICE_COHERENT pages. These are pages that 
> are coherently accessible by the CPU and peer devices in the system 
> physical address space. Therefore, this needs to be conditional. Maybe 
> add something like adev->kfd.migrate_pfn_flag that gets initialized 
> conditionally in kgd2kfd_init_zone_device. Then add ... | adev- 
>  >kfd.migrate_pfn_flag here.
> 
> 
>>               mpages++;
>>           }
>>           spage = migrate_pfn_to_page(migrate->src[i]);
>> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/ 
>> drm_pagemap.c
>> index 37d7cfbbb3e8..0c756d73419f 100644
>> --- a/drivers/gpu/drm/drm_pagemap.c
>> +++ b/drivers/gpu/drm/drm_pagemap.c
>> @@ -404,7 +404,7 @@ int drm_pagemap_migrate_to_devmem(struct 
>> drm_pagemap_devmem *devmem_allocation,
>>           struct page *page = pfn_to_page(migrate.dst[i]);
>>           pages[i] = page;
>> -        migrate.dst[i] = migrate_pfn(migrate.dst[i]);
>> +        migrate.dst[i] = migrate_pfn(migrate.dst[i]) | 
>> MIGRATE_PFN_DEVICE;
>>           drm_pagemap_get_devmem_page(page, zdd);
>>       }
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/ 
>> nouveau/nouveau_dmem.c
>> index 58071652679d..2bd80c6f5bcd 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
>> @@ -766,7 +766,7 @@ static unsigned long 
>> nouveau_dmem_migrate_copy_one(struct nouveau_drm *drm,
>>           ((paddr >> PAGE_SHIFT) << NVIF_VMM_PFNMAP_V0_ADDR_SHIFT);
>>       if (src & MIGRATE_PFN_WRITE)
>>           *pfn |= NVIF_VMM_PFNMAP_V0_W;
>> -    mpfn = migrate_pfn(page_to_pfn(dpage));
>> +    mpfn = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_DEVICE;
>>       if (folio_order(page_folio(dpage)))
>>           mpfn |= MIGRATE_PFN_COMPOUND;
>>       return mpfn;
>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
>> index 26ca00c325d9..52f65cd5c932 100644
>> --- a/include/linux/migrate.h
>> +++ b/include/linux/migrate.h
>> @@ -126,6 +126,7 @@ static inline int migrate_misplaced_folio(struct 
>> folio *folio, int node)
>>   #define MIGRATE_PFN_MIGRATE    (1UL << 1)
>>   #define MIGRATE_PFN_WRITE    (1UL << 3)
>>   #define MIGRATE_PFN_COMPOUND    (1UL << 4)
>> +#define MIGRATE_PFN_DEVICE    (1UL << 5)
>>   #define MIGRATE_PFN_SHIFT    6
>>   static inline struct page *migrate_pfn_to_page(unsigned long mpfn)
>> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
>> index 8af169d3873a..19681904a666 100644
>> --- a/lib/test_hmm.c
>> +++ b/lib/test_hmm.c
>> @@ -727,7 +727,7 @@ static void dmirror_migrate_alloc_and_copy(struct 
>> migrate_vma *args,
>>                   rpage = BACKING_PAGE(dpage);
>>                   rpage->zone_device_data = dmirror;
>> -                *dst = migrate_pfn(page_to_pfn(dpage)) | write;
>> +                *dst = migrate_pfn(page_to_pfn(dpage)) | 
>> MIGRATE_PFN_DEVICE | write;
> 
> This needs to be conditional on dmirror->mdevice->zone_device_type.

Good catch.

> 
> 
>>                   src_page = pfn_to_page(spfn + i);
>>                   if (spage)
>> @@ -754,7 +754,7 @@ static void dmirror_migrate_alloc_and_copy(struct 
>> migrate_vma *args,
>>           pr_debug("migrating from sys to dev pfn src: 0x%lx pfn dst: 
>> 0x%lx\n",
>>                page_to_pfn(spage), page_to_pfn(dpage));
>> -        *dst = migrate_pfn(page_to_pfn(dpage)) | write;
>> +        *dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_DEVICE | 
>> write;
> 
> Same here.

Agree.

> 
> 
>>           if (is_large) {
>>               int i;
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 23379663b1e1..5d108ddf1a97 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -199,6 +199,7 @@ static int migrate_vma_collect_huge_pmd(pmd_t 
>> *pmdp, unsigned long start,
>>           (migrate->flags & MIGRATE_VMA_SELECT_COMPOUND) &&
>>           (IS_ALIGNED(start, HPAGE_PMD_SIZE) &&
>>            IS_ALIGNED(end, HPAGE_PMD_SIZE))) {
>> +        unsigned long device_private = 0;
>>           struct page_vma_mapped_walk pvmw = {
>>               .ptl = ptl,
>> @@ -208,10 +209,13 @@ static int migrate_vma_collect_huge_pmd(pmd_t 
>> *pmdp, unsigned long start,
>>           };
>>           unsigned long pfn = page_to_pfn(folio_page(folio, 0));
>> +        if (folio_is_device_private(folio))
>> +            device_private = MIGRATE_PFN_DEVICE;
>>           migrate->src[migrate->npages] = migrate_pfn(pfn) | write
>>                           | MIGRATE_PFN_MIGRATE
>> -                        | MIGRATE_PFN_COMPOUND;
>> +                        | MIGRATE_PFN_COMPOUND
>> +                        | device_private;
>>           migrate->dst[migrate->npages++] = 0;
>>           migrate->cpages++;
>>           ret = set_pmd_migration_entry(&pvmw, folio_page(folio, 0));
>> @@ -329,7 +333,8 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>               }
>>               mpfn = migrate_pfn(page_to_pfn(page)) |
>> -                    MIGRATE_PFN_MIGRATE;
>> +                    MIGRATE_PFN_MIGRATE |
>> +                    MIGRATE_PFN_DEVICE;
> 
> I think this also needs to be conditional to distinguish DEVICE_COHERENT 
> pages.

Agree

> 
> 
>>               if (softleaf_is_device_private_write(entry))
>>                   mpfn |= MIGRATE_PFN_WRITE;
>>           } else {
>> @@ -1368,7 +1373,7 @@ static unsigned long 
>> migrate_device_pfn_lock(unsigned long pfn)
>>           return 0;
>>       }
>> -    return migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
>> +    return migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE | MIGRATE_PFN_DEVICE;
> 
> Same here.

Maybe a page to migrate pfn helper function will be useful.

Thanks,
Jordan.

> 
> Regards,
>    Felix
> 
> 
>>   }
>>   /**