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

[Alistair Popple <apopple@nvidia.com>]

On 2026-01-01 at 04:03 +1100, "Kuehling, Felix" <felix.kuehling@amd.com> 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.

Thanks for catching the DEVICE_COHERENT aspects, they need to remain as real
PFNs. I think the flag should be renamed to MIGRATE_PFN_DEVICE_PRIVATE to make
this explicit.

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

I don't think you need a new flag, you could just make it conditional on
adev->kfd.pgmap.type == MEMORY_DEVICE_PRIVATE.

Note the need for this conditional is specific to the AMD driver and HMM tests -
AFAIK there are no other uses of DEVICE_COHERENT in the kernel.

 - Alistair

> >   			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.
> 
> 
> >   				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.
> 
> 
> >   		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.
> 
> 
> >   			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.
> 
> Regards,
>   Felix
> 
> 
> >   }
> >   /**