[PATCH 1/3] drm/amdkfd: Fix error path at svm_migrate_copy_to_ram

[PATCH 1/3] drm/amdkfd: Fix error path at svm_migrate_copy_to_ram

[Xiaogang.Chen]

From: Xiaogang Chen <xiaogang.chen@amd.com>

If page migration from device to sys ram fails for some reasons driver needs
release and unlock allocated system pages. To do that driver should use page
physical address, or pfn, then get struct page*. Current driver uses dma
address(for adev) that is not correct with IOMMU enabled, or even in general.

The patch releases and unlocks allocated system pages based on where migration
failed by struct page* of sys ram pages. Also dma_unmap correspodent system
ram pages at error path.

Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 46 ++++++++++++++++--------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 226e76ae0be7..aea82da5f575 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -258,15 +258,6 @@ svm_migrate_get_sys_page(struct vm_area_struct *vma, unsigned long addr)
 	return page;
 }
 
-static void svm_migrate_put_sys_page(unsigned long addr)
-{
-	struct page *page;
-
-	page = pfn_to_page(addr >> PAGE_SHIFT);
-	unlock_page(page);
-	put_page(page);
-}
-
 static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
 {
 	unsigned long mpages = 0;
@@ -591,9 +582,10 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			dma_addr_t *scratch, u64 npages)
 {
 	struct device *dev = adev->dev;
-	u64 *src;
+	struct page *dpage = NULL;
 	dma_addr_t *dst;
-	struct page *dpage;
+	u64 *src;
+
 	u64 i = 0, j;
 	u64 addr;
 	int r = 0;
@@ -647,6 +639,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 		r = dma_mapping_error(dev, dst[i]);
 		if (r) {
 			dev_err(adev->dev, "%s: fail %d dma_map_page\n", __func__, r);
+			dst[i] = 0;
 			goto out_oom;
 		}
 
@@ -654,17 +647,40 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
 
 		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
+
+		dpage = NULL;
 		j++;
 	}
 
-	r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
-					 FROM_VRAM_TO_RAM, mfence);
-
+	if (j > 0)
+		r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
+						 FROM_VRAM_TO_RAM, mfence);
 out_oom:
 	if (r) {
 		pr_debug("failed %d copy to ram\n", r);
+
+		/* first release current dpage when dma_map_page fail */
+		if (dpage) {
+			unlock_page(dpage);
+			put_page(dpage);
+			dpage = NULL;
+		}
+
+		/* release previous allocated sys pages and unmap dma address */
 		while (i--) {
-			svm_migrate_put_sys_page(dst[i]);
+
+			if (dst[i] && !dma_mapping_error(dev, dst[i])) {
+				dma_unmap_page(dev, dst[i], PAGE_SIZE,
+					       DMA_BIDIRECTIONAL);
+				dst[i] = 0;
+			}
+
+			dpage = migrate_pfn_to_page(migrate->dst[i]);
+			if (!dpage)
+				continue;
+
+			unlock_page(dpage);
+			put_page(dpage);
 			migrate->dst[i] = 0;
 		}
 	}
-- 
2.34.1

[PATCH 2/3] drm/amdkfd: Fix the case that vm range is hole at svm_migrate_copy_to_vram

[Xiaogang.Chen]

From: Xiaogang Chen <xiaogang.chen@amd.com>

When migration vm range is hole at cpu side(MIGRATE_PFN_MIGRATE set +
MIGRATE_PFN_VALID unset) driver still allocates device pages. There is no
dma map of src pages and migration. j is 0 and svm_migrate_copy_memory_gart()
will return an uninitialized r. That can trigger out_free_vram_pages to drop
all VRAM just set up.

Initialize r and only call the last svm_migrate_copy_memory_gart if j > 0.

Current code postponed the last page to the final copy. This patch flushes on
the last page when reach to the end of current drm_buddy_block; avoids another
svm_migrate_copy_memory_gart.

Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index aea82da5f575..263dae49bb3e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -284,7 +284,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 	dma_addr_t *src;
 	u64 *dst;
 	u64 i, j;
-	int r;
+	int r = 0;
 
 	pr_debug("svms 0x%p [0x%lx 0x%lx 0x%llx]\n", prange->svms, prange->start,
 		 prange->last, ttm_res_offset);
@@ -310,6 +310,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 					      DMA_BIDIRECTIONAL);
 			r = dma_mapping_error(dev, src[i]);
 			if (r) {
+				src[i] = 0;
 				dev_err(dev, "%s: fail %d dma_map_page\n",
 					__func__, r);
 				goto out_free_vram_pages;
@@ -334,7 +335,8 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 		pr_debug_ratelimited("dma mapping src to 0x%llx, pfn 0x%lx\n",
 				     src[i] >> PAGE_SHIFT, page_to_pfn(spage));
 
-		if (j >= (cursor.size >> PAGE_SHIFT) - 1 && i < npages - 1) {
+		/* accumulated j + 1 pages reach end of current drm_buddy_block */
+		if (j + 1 >= (cursor.size >> PAGE_SHIFT)) {
 			r = svm_migrate_copy_memory_gart(adev, src + i - j,
 							 dst + i - j, j + 1,
 							 FROM_RAM_TO_VRAM,
@@ -348,7 +350,8 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
 		}
 	}
 
-	r = svm_migrate_copy_memory_gart(adev, src + i - j, dst + i - j, j,
+	if (j > 0)
+		r = svm_migrate_copy_memory_gart(adev, src + i - j, dst + i - j, j,
 					 FROM_RAM_TO_VRAM, mfence);
 
 out_free_vram_pages:
-- 
2.34.1

Re: [PATCH 2/3] drm/amdkfd: Fix the case that vm range is hole at svm_migrate_copy_to_vram

[Felix Kuehling]

On 2026-08-17 09:59, Xiaogang.Chen wrote:
> From: Xiaogang Chen <xiaogang.chen@amd.com>
>
> When migration vm range is hole at cpu side(MIGRATE_PFN_MIGRATE set +
> MIGRATE_PFN_VALID unset) driver still allocates device pages. There is no
> dma map of src pages and migration. j is 0 and svm_migrate_copy_memory_gart()
> will return an uninitialized r. That can trigger out_free_vram_pages to drop
> all VRAM just set up.

Good catch. The first fix I would expect here, is that 
svm_migrate_copy_memory_gart should never return an uninitialized r. I'd 
defensively initialize r in svm_migrate_copy_memory_gart as well, even 
if you never plan to call it with npages = 0. With that fixed, the patch is

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


>
> Initialize r and only call the last svm_migrate_copy_memory_gart if j > 0.
>
> Current code postponed the last page to the final copy. This patch flushes on
> the last page when reach to the end of current drm_buddy_block; avoids another
> svm_migrate_copy_memory_gart.
>
> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index aea82da5f575..263dae49bb3e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -284,7 +284,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   	dma_addr_t *src;
>   	u64 *dst;
>   	u64 i, j;
> -	int r;
> +	int r = 0;
>   
>   	pr_debug("svms 0x%p [0x%lx 0x%lx 0x%llx]\n", prange->svms, prange->start,
>   		 prange->last, ttm_res_offset);
> @@ -310,6 +310,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   					      DMA_BIDIRECTIONAL);
>   			r = dma_mapping_error(dev, src[i]);
>   			if (r) {
> +				src[i] = 0;
>   				dev_err(dev, "%s: fail %d dma_map_page\n",
>   					__func__, r);
>   				goto out_free_vram_pages;
> @@ -334,7 +335,8 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   		pr_debug_ratelimited("dma mapping src to 0x%llx, pfn 0x%lx\n",
>   				     src[i] >> PAGE_SHIFT, page_to_pfn(spage));
>   
> -		if (j >= (cursor.size >> PAGE_SHIFT) - 1 && i < npages - 1) {
> +		/* accumulated j + 1 pages reach end of current drm_buddy_block */
> +		if (j + 1 >= (cursor.size >> PAGE_SHIFT)) {
>   			r = svm_migrate_copy_memory_gart(adev, src + i - j,
>   							 dst + i - j, j + 1,
>   							 FROM_RAM_TO_VRAM,
> @@ -348,7 +350,8 @@ svm_migrate_copy_to_vram(struct kfd_node *node, struct svm_range *prange,
>   		}
>   	}
>   
> -	r = svm_migrate_copy_memory_gart(adev, src + i - j, dst + i - j, j,
> +	if (j > 0)
> +		r = svm_migrate_copy_memory_gart(adev, src + i - j, dst + i - j, j,
>   					 FROM_RAM_TO_VRAM, mfence);
>   
>   out_free_vram_pages:

[PATCH 3/3] drm/amdkfd: Check cpu page faulted vmf->page got migrated to system RAM

[Xiaogang.Chen]

From: Xiaogang Chen <xiaogang.chen@amd.com>

Cpu page fault hander __handle_mm_fault calls dev_pagemap_ops->migrate_to_ram
when faulted page is device private memory. This callback needs return either
success or vm_fault_t to let handler know what happened when handling the fault.

Current driver returns success if it did not hit a hard error. That does not
means the device page is now in RAM. Some cases like fault page pinned,
lock fail or other mapping prevent faulted device page got migrated.

Then the CPU instruction will fault again(retry loop), unless something else
changed the PTE.

The patch explicitly checks if the faulted page vmf->page got migrated to
system RAM. If not, returns error to let cpu page fault core handler handle
the page fault in error path.

Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 28 +++++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 263dae49bb3e..656197dee9b1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -582,8 +582,9 @@ static void svm_migrate_folio_free(struct folio *folio)
 static int
 svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			struct migrate_vma *migrate, struct dma_fence **mfence,
-			dma_addr_t *scratch, u64 npages)
+			dma_addr_t *scratch, u64 npages, bool *fault_handled)
 {
+	struct page *fault_page = migrate->fault_page;
 	struct device *dev = adev->dev;
 	struct page *dpage = NULL;
 	dma_addr_t *dst;
@@ -646,6 +647,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			goto out_oom;
 		}
 
+		/* if this spage is not migrated the function returns error
+		 * final decide whether the fault got handled is decided by
+		 * fault_handled and this function returned value
+		 */
+		if (fault_handled && fault_page && fault_page == spage)
+			*fault_handled = true;
+
 		pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn 0x%lx\n",
 				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
 
@@ -701,6 +709,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
  * @node: kfd node device to migrate from
  * @trigger: reason of migration
  * @fault_page: is from vmf->page, svm_migrate_to_ram(), this is CPU page fault callback
+ * @fault_handled: whether CPU page fault got handled
  *
  * Context: Process context, caller hold mmap read lock, prange->migrate_mutex
  *
@@ -711,7 +720,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 static long
 svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 		       struct vm_area_struct *vma, u64 start, u64 end,
-		       uint32_t trigger, struct page *fault_page)
+		       uint32_t trigger, struct page *fault_page, bool *fault_handled)
 {
 	struct kfd_process *p = container_of(prange->svms, struct kfd_process, svms);
 	u64 npages = (end - start) >> PAGE_SHIFT;
@@ -771,7 +780,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
 		pr_debug("0x%lx pages collected\n", cpages);
 
 	r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
-				    scratch, npages);
+				    scratch, npages, fault_handled);
 	migrate_vma_pages(&migrate);
 
 	mpages = svm_migrate_successful_pages(&migrate);
@@ -816,8 +825,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 			    unsigned long start_mgr, unsigned long last_mgr,
 			    uint32_t trigger, struct page *fault_page)
 {
-	struct kfd_node *node;
+	bool fault_handled = false;
 	struct vm_area_struct *vma;
+	struct kfd_node *node;
 	unsigned long addr;
 	unsigned long start;
 	unsigned long end;
@@ -861,7 +871,7 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 
 		next = min(vma->vm_end, end);
 		r = svm_migrate_vma_to_ram(node, prange, vma, addr, next, trigger,
-			fault_page);
+			fault_page, &fault_handled);
 		if (r < 0) {
 			pr_debug("failed %ld to migrate prange %p\n", r, prange);
 			break;
@@ -886,6 +896,10 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
 		}
 	}
 
+	/* need handle cpu page fault, but not handled */
+	if (fault_page && !fault_handled)
+		return -1;
+
 	return r < 0 ? r : 0;
 }
 
@@ -1027,7 +1041,9 @@ static vm_fault_t svm_migrate_to_ram(struct vm_fault *vmf)
 out_unlock_svms:
 	mutex_unlock(&p->svms.lock);
 out_unref_process:
-	pr_debug("CPU fault svms 0x%p address 0x%lx done\n", &p->svms, addr);
+	pr_debug("CPU fault svms 0x%p address 0x%lx done with erro=%d\n",
+		 &p->svms, addr, r);
+
 	kfd_unref_process(p);
 out_mmput:
 	mmput(mm);
-- 
2.34.1

Re: [PATCH 3/3] drm/amdkfd: Check cpu page faulted vmf->page got migrated to system RAM

[Felix Kuehling]

On 2026-08-17 09:59, Xiaogang.Chen wrote:
> From: Xiaogang Chen <xiaogang.chen@amd.com>
>
> Cpu page fault hander __handle_mm_fault calls dev_pagemap_ops->migrate_to_ram
> when faulted page is device private memory. This callback needs return either
> success or vm_fault_t to let handler know what happened when handling the fault.
>
> Current driver returns success if it did not hit a hard error. That does not
> means the device page is now in RAM. Some cases like fault page pinned,
> lock fail or other mapping prevent faulted device page got migrated.
>
> Then the CPU instruction will fault again(retry loop), unless something else
> changed the PTE.
>
> The patch explicitly checks if the faulted page vmf->page got migrated to
> system RAM. If not, returns error to let cpu page fault core handler handle
> the page fault in error path.

Are you sure this is the right thing to do? If the errors are transient 
in nature, then maybe retrying is the better option. What happens if you 
return an error? Does the application die with a SIGBUS?

Should we consult with an HMM maintainer here? Did you check how other 
drivers with HMM support deal with this situation?

Regards,
   Felix


>
> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 28 +++++++++++++++++++-----
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 263dae49bb3e..656197dee9b1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -582,8 +582,9 @@ static void svm_migrate_folio_free(struct folio *folio)
>   static int
>   svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   			struct migrate_vma *migrate, struct dma_fence **mfence,
> -			dma_addr_t *scratch, u64 npages)
> +			dma_addr_t *scratch, u64 npages, bool *fault_handled)
>   {
> +	struct page *fault_page = migrate->fault_page;
>   	struct device *dev = adev->dev;
>   	struct page *dpage = NULL;
>   	dma_addr_t *dst;
> @@ -646,6 +647,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   			goto out_oom;
>   		}
>   
> +		/* if this spage is not migrated the function returns error
> +		 * final decide whether the fault got handled is decided by
> +		 * fault_handled and this function returned value
> +		 */
> +		if (fault_handled && fault_page && fault_page == spage)
> +			*fault_handled = true;
> +
>   		pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn 0x%lx\n",
>   				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
>   
> @@ -701,6 +709,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>    * @node: kfd node device to migrate from
>    * @trigger: reason of migration
>    * @fault_page: is from vmf->page, svm_migrate_to_ram(), this is CPU page fault callback
> + * @fault_handled: whether CPU page fault got handled
>    *
>    * Context: Process context, caller hold mmap read lock, prange->migrate_mutex
>    *
> @@ -711,7 +720,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   static long
>   svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   		       struct vm_area_struct *vma, u64 start, u64 end,
> -		       uint32_t trigger, struct page *fault_page)
> +		       uint32_t trigger, struct page *fault_page, bool *fault_handled)
>   {
>   	struct kfd_process *p = container_of(prange->svms, struct kfd_process, svms);
>   	u64 npages = (end - start) >> PAGE_SHIFT;
> @@ -771,7 +780,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range *prange,
>   		pr_debug("0x%lx pages collected\n", cpages);
>   
>   	r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
> -				    scratch, npages);
> +				    scratch, npages, fault_handled);
>   	migrate_vma_pages(&migrate);
>   
>   	mpages = svm_migrate_successful_pages(&migrate);
> @@ -816,8 +825,9 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   			    unsigned long start_mgr, unsigned long last_mgr,
>   			    uint32_t trigger, struct page *fault_page)
>   {
> -	struct kfd_node *node;
> +	bool fault_handled = false;
>   	struct vm_area_struct *vma;
> +	struct kfd_node *node;
>   	unsigned long addr;
>   	unsigned long start;
>   	unsigned long end;
> @@ -861,7 +871,7 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   
>   		next = min(vma->vm_end, end);
>   		r = svm_migrate_vma_to_ram(node, prange, vma, addr, next, trigger,
> -			fault_page);
> +			fault_page, &fault_handled);
>   		if (r < 0) {
>   			pr_debug("failed %ld to migrate prange %p\n", r, prange);
>   			break;
> @@ -886,6 +896,10 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   		}
>   	}
>   
> +	/* need handle cpu page fault, but not handled */
> +	if (fault_page && !fault_handled)
> +		return -1;
> +
>   	return r < 0 ? r : 0;
>   }
>   
> @@ -1027,7 +1041,9 @@ static vm_fault_t svm_migrate_to_ram(struct vm_fault *vmf)
>   out_unlock_svms:
>   	mutex_unlock(&p->svms.lock);
>   out_unref_process:
> -	pr_debug("CPU fault svms 0x%p address 0x%lx done\n", &p->svms, addr);
> +	pr_debug("CPU fault svms 0x%p address 0x%lx done with erro=%d\n",
> +		 &p->svms, addr, r);
> +
>   	kfd_unref_process(p);
>   out_mmput:
>   	mmput(mm);

Re: [PATCH 3/3] drm/amdkfd: Check cpu page faulted vmf->page got migrated to system RAM

[Chen, Xiaogang]

On 8/18/2026 4:36 PM, Felix Kuehling wrote:
> On 2026-08-17 09:59, Xiaogang.Chen wrote:
>> From: Xiaogang Chen <xiaogang.chen@amd.com>
>>
>> Cpu page fault hander __handle_mm_fault calls 
>> dev_pagemap_ops->migrate_to_ram
>> when faulted page is device private memory. This callback needs 
>> return either
>> success or vm_fault_t to let handler know what happened when handling 
>> the fault.
>>
>> Current driver returns success if it did not hit a hard error. That 
>> does not
>> means the device page is now in RAM. Some cases like fault page pinned,
>> lock fail or other mapping prevent faulted device page got migrated.
>>
>> Then the CPU instruction will fault again(retry loop), unless 
>> something else
>> changed the PTE.
>>
>> The patch explicitly checks if the faulted page vmf->page got 
>> migrated to
>> system RAM. If not, returns error to let cpu page fault core handler 
>> handle
>> the page fault in error path.
>
> Are you sure this is the right thing to do? If the errors are 
> transient in nature, then maybe retrying is the better option. What 
> happens if you return an error? Does the application die with a SIGBUS?
>
> Should we consult with an HMM maintainer here? Did you check how other 
> drivers with HMM support deal with this situation?

I notice this issue by reading code, not from real case. pinned 
/not-migratable vmf->page -> no copy from this page to sys-ram. It will 
trigger infinite retry,

I checked drm_pagemap_migrate_to_ram()(xe driver) and 
nouveau_dmem_migrate_to_ram(nouveau driver) that handle cpu page fault 
from device private memory access. They do same as amdgpu driver: skip 
pinned page or populate skips the slot that is (!(src[i] & 
MIGRATE_PFN_MIGRATE)), so those bytes are not copied, and the function 
returns 0.

For NVIDIA Linux open GPU kernel modules at 
https://github.com/NVIDIA/open-gpu-kernel-modules: It is more 
complicated. I asked Cursor analysis: NVIDIA UVM has the same 
CPU-visible outcome for a pinned vmf->page, but it does not return 
VM_FAULT_SIGBUS, return NV_WARN_MORE_PROCESSING_REQUIRED instead, then 
retry inside kernel.

This patch returns VM_FAULT_SIGBUS for that case, then fault handler 
sends SIGBUS signal, the default handler of this signal is killing the 
process. This may not be people want.

It is a blur area. kernel-doc for migrate_vma_setup():

  * If the caller cannot migrate a device page
  * back to system memory, then it must return VM_FAULT_SIGBUS, which 
has severe
  * consequences for the userspace process, so it must be avoided if at all
  * possible.

Summary:

Current xe/nouveau/amdgpu driver do not return VM_FAULT_SIGBUS, just 
think it is success. That may cause #PF handler retry loop between user 
and kernel space.

Nivida driver does retry inside kernel.

This patch detects the case then return  VM_FAULT_SIGBUS.


Maybe you said retry fault handler inside kernel is a better way: we let 
driver return VM_FAULT_RETRY for pinned vmf->page, let kernel retry 
handle_mm_fault, then hope the pin will be drop soon?


Regards

Xiaogang


>
> Regards,
>   Felix
>
>
>>
>> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 28 +++++++++++++++++++-----
>>   1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> index 263dae49bb3e..656197dee9b1 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
>> @@ -582,8 +582,9 @@ static void svm_migrate_folio_free(struct folio 
>> *folio)
>>   static int
>>   svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct 
>> svm_range *prange,
>>               struct migrate_vma *migrate, struct dma_fence **mfence,
>> -            dma_addr_t *scratch, u64 npages)
>> +            dma_addr_t *scratch, u64 npages, bool *fault_handled)
>>   {
>> +    struct page *fault_page = migrate->fault_page;
>>       struct device *dev = adev->dev;
>>       struct page *dpage = NULL;
>>       dma_addr_t *dst;
>> @@ -646,6 +647,13 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>               goto out_oom;
>>           }
>>   +        /* if this spage is not migrated the function returns error
>> +         * final decide whether the fault got handled is decided by
>> +         * fault_handled and this function returned value
>> +         */
>> +        if (fault_handled && fault_page && fault_page == spage)
>> +            *fault_handled = true;
>> +
>>           pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn 0x%lx\n",
>>                        dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
>>   @@ -701,6 +709,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>    * @node: kfd node device to migrate from
>>    * @trigger: reason of migration
>>    * @fault_page: is from vmf->page, svm_migrate_to_ram(), this is 
>> CPU page fault callback
>> + * @fault_handled: whether CPU page fault got handled
>>    *
>>    * Context: Process context, caller hold mmap read lock, 
>> prange->migrate_mutex
>>    *
>> @@ -711,7 +720,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device 
>> *adev, struct svm_range *prange,
>>   static long
>>   svm_migrate_vma_to_ram(struct kfd_node *node, struct svm_range 
>> *prange,
>>                  struct vm_area_struct *vma, u64 start, u64 end,
>> -               uint32_t trigger, struct page *fault_page)
>> +               uint32_t trigger, struct page *fault_page, bool 
>> *fault_handled)
>>   {
>>       struct kfd_process *p = container_of(prange->svms, struct 
>> kfd_process, svms);
>>       u64 npages = (end - start) >> PAGE_SHIFT;
>> @@ -771,7 +780,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node, 
>> struct svm_range *prange,
>>           pr_debug("0x%lx pages collected\n", cpages);
>>         r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
>> -                    scratch, npages);
>> +                    scratch, npages, fault_handled);
>>       migrate_vma_pages(&migrate);
>>         mpages = svm_migrate_successful_pages(&migrate);
>> @@ -816,8 +825,9 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>                   unsigned long start_mgr, unsigned long last_mgr,
>>                   uint32_t trigger, struct page *fault_page)
>>   {
>> -    struct kfd_node *node;
>> +    bool fault_handled = false;
>>       struct vm_area_struct *vma;
>> +    struct kfd_node *node;
>>       unsigned long addr;
>>       unsigned long start;
>>       unsigned long end;
>> @@ -861,7 +871,7 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>             next = min(vma->vm_end, end);
>>           r = svm_migrate_vma_to_ram(node, prange, vma, addr, next, 
>> trigger,
>> -            fault_page);
>> +            fault_page, &fault_handled);
>>           if (r < 0) {
>>               pr_debug("failed %ld to migrate prange %p\n", r, prange);
>>               break;
>> @@ -886,6 +896,10 @@ int svm_migrate_vram_to_ram(struct svm_range 
>> *prange, struct mm_struct *mm,
>>           }
>>       }
>>   +    /* need handle cpu page fault, but not handled */
>> +    if (fault_page && !fault_handled)
>> +        return -1;
>> +
>>       return r < 0 ? r : 0;
>>   }
>>   @@ -1027,7 +1041,9 @@ static vm_fault_t svm_migrate_to_ram(struct 
>> vm_fault *vmf)
>>   out_unlock_svms:
>>       mutex_unlock(&p->svms.lock);
>>   out_unref_process:
>> -    pr_debug("CPU fault svms 0x%p address 0x%lx done\n", &p->svms, 
>> addr);
>> +    pr_debug("CPU fault svms 0x%p address 0x%lx done with erro=%d\n",
>> +         &p->svms, addr, r);
>> +
>>       kfd_unref_process(p);
>>   out_mmput:
>>       mmput(mm);

Re: [PATCH 3/3] drm/amdkfd: Check cpu page faulted vmf->page got migrated to system RAM

[Kuehling, Felix]

On 2026-08-19 11:16, Chen, Xiaogang wrote:
> Maybe you said retry fault handler inside kernel is a better way: we 
> let driver return VM_FAULT_RETRY for pinned vmf->page, let kernel 
> retry handle_mm_fault, then hope the pin will be drop soon?

I agree. Pages should never be pinned for a very long time. In other 
words, I believe this is all working as it should and this patch is 
unnecessary.

Regards,
   Felix

Re: [PATCH 1/3] drm/amdkfd: Fix error path at svm_migrate_copy_to_ram

[Felix Kuehling]

On 2026-08-17 09:59, Xiaogang.Chen wrote:
> From: Xiaogang Chen <xiaogang.chen@amd.com>
>
> If page migration from device to sys ram fails for some reasons driver needs
> release and unlock allocated system pages. To do that driver should use page
> physical address, or pfn, then get struct page*. Current driver uses dma
> address(for adev) that is not correct with IOMMU enabled, or even in general.
>
> The patch releases and unlocks allocated system pages based on where migration
> failed by struct page* of sys ram pages. Also dma_unmap correspodent system
> ram pages at error path.
>
> Signed-off-by: Xiaogang Chen <xiaogang.chen@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 46 ++++++++++++++++--------
>   1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 226e76ae0be7..aea82da5f575 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -258,15 +258,6 @@ svm_migrate_get_sys_page(struct vm_area_struct *vma, unsigned long addr)
>   	return page;
>   }
>   
> -static void svm_migrate_put_sys_page(unsigned long addr)
> -{
> -	struct page *page;
> -
> -	page = pfn_to_page(addr >> PAGE_SHIFT);
> -	unlock_page(page);
> -	put_page(page);
> -}
> -
>   static unsigned long svm_migrate_successful_pages(struct migrate_vma *migrate)
>   {
>   	unsigned long mpages = 0;
> @@ -591,9 +582,10 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   			dma_addr_t *scratch, u64 npages)
>   {
>   	struct device *dev = adev->dev;
> -	u64 *src;
> +	struct page *dpage = NULL;
>   	dma_addr_t *dst;
> -	struct page *dpage;
> +	u64 *src;
> +
>   	u64 i = 0, j;
>   	u64 addr;
>   	int r = 0;
> @@ -647,6 +639,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   		r = dma_mapping_error(dev, dst[i]);
>   		if (r) {
>   			dev_err(adev->dev, "%s: fail %d dma_map_page\n", __func__, r);
> +			dst[i] = 0;
>   			goto out_oom;
>   		}
>   
> @@ -654,17 +647,40 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   				     dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
>   
>   		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
> +
> +		dpage = NULL;
>   		j++;
>   	}
>   
> -	r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
> -					 FROM_VRAM_TO_RAM, mfence);
> -
> +	if (j > 0)
> +		r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
> +						 FROM_VRAM_TO_RAM, mfence);
>   out_oom:
>   	if (r) {
>   		pr_debug("failed %d copy to ram\n", r);
> +
> +		/* first release current dpage when dma_map_page fail */
> +		if (dpage) {
> +			unlock_page(dpage);
> +			put_page(dpage);
> +			dpage = NULL;

Minor nit-pick: You don't really need to set dpage to NULL here. The 
loop below doesn't either.


> +		}
> +
> +		/* release previous allocated sys pages and unmap dma address */
>   		while (i--) {
> -			svm_migrate_put_sys_page(dst[i]);
> +
> +			if (dst[i] && !dma_mapping_error(dev, dst[i])) {

Why do you need to check dma_mapping_error here again? If there was an 
error above, you already set dst[i] = 0.

With those two issues fixed, the patch is

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> +				dma_unmap_page(dev, dst[i], PAGE_SIZE,
> +					       DMA_BIDIRECTIONAL);
> +				dst[i] = 0;
> +			}
> +
> +			dpage = migrate_pfn_to_page(migrate->dst[i]);
> +			if (!dpage)
> +				continue;
> +
> +			unlock_page(dpage);
> +			put_page(dpage);
>   			migrate->dst[i] = 0;
>   		}
>   	}