On 10/6/2026 5:44 PM, Felix Kuehling wrote:
On 2026-09-04 15:54, Xiaogang.Chen wrote:
From: Xiaogang Chen<[email protected]>
Update kfd svm driver to migrate device-private THP introduced from
HMM core
migration function. Select this function by flag
MIGRATE_VMA_SELECT_COMPOUND
when call migrate_vma_setup. kfd migration procedure is updated
according to
collected page type that can be either compound folio(physical
continuous) or
normal size page.
Signed-off-by: Xiaogang Chen<[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 271 ++++++++++++++++++-----
1 file changed, 219 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index bbf0fefd5722..af39e547c1fa 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -298,7 +298,7 @@ svm_migrate_copy_to_vram(struct kfd_node *node,
struct svm_range *prange,
u64 mpages = 0;
dma_addr_t *src;
u64 *dst;
- u64 i, j;
+ u64 i, j, k, l, m;
int r = 0;
pr_debug("svms 0x%p [0x%lx 0x%lx 0x%llx]\n", prange->svms,
prange->start,
@@ -309,59 +309,158 @@ svm_migrate_copy_to_vram(struct kfd_node
*node, struct svm_range *prange,
amdgpu_res_first(prange->ttm_res, ttm_res_offset,
npages << PAGE_SHIFT, &cursor);
- for (i = j = 0; (i < npages) && (mpages < migrate->cpages); i++) {
+ for (i = j = m = 0; (i < npages) && (mpages < migrate->cpages);) {
struct page *spage;
+ unsigned long cur_dst_pfn;
+ bool is_large = false;
- 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], 0);
- migrate->dst[i] = migrate_pfn(migrate->dst[i]);
+ cur_dst_pfn = svm_migrate_addr_to_pfn(adev,
+ cursor.start + (m << PAGE_SHIFT));
+
+ /* when migrate->src[i] has MIGRATE_PFN_COMPOUND set the src
page
+ * is compound THP; its vm address is HPAGE_PMD_SIZE aligned
and
+ * its MIGRATE_PFN_MIGRATE is set
+ */
+ if ((m + HPAGE_PMD_NR) <= (cursor.size >> PAGE_SHIFT) &&
+ (i + HPAGE_PMD_NR) <= npages &&
+ (migrate->src[i] & MIGRATE_PFN_COMPOUND) &&
+ IS_ALIGNED(cur_dst_pfn, HPAGE_PMD_NR)) {
+
Unnecessary empty line.
ok
+ is_large = true;
+ k = HPAGE_PMD_NR;
is_large is redundant. Just use (k > 1).
+ } else
+ k = 1;
Bad indentation.
ok
+
+ /* for THP src[0] alwas MIGRATE_PFN_MIGRATE
+ * just the first migrate->dst need be setup, others are zero
+ */
+ if (is_large) {
+
+ dst[i] = cursor.start + (m << PAGE_SHIFT);
+ svm_migrate_get_vram_page(prange, cur_dst_pfn,
+ HPAGE_PMD_ORDER);
+
+ migrate->dst[i] = migrate_pfn(cur_dst_pfn);
+ migrate->dst[i] |= MIGRATE_PFN_COMPOUND;
+
+ for (l=1; l < k; l++)
+ migrate->dst[i+l] = 0;
+
+ mpages++;
+
+ } else if ((migrate->src[i] & MIGRATE_PFN_MIGRATE)) {
+ dst[i] = cursor.start + (m << PAGE_SHIFT);
+ svm_migrate_get_vram_page(prange, cur_dst_pfn, 0);
+ migrate->dst[i] = migrate_pfn(cur_dst_pfn);
mpages++;
}
- spage = migrate_pfn_to_page(migrate->src[i]);
- if (spage && !is_zone_device_page(spage)) {
- src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
- 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;
+
+ if (is_large) {
+ if (j) {
+ /* migrate previous accumulated pages */
+ r = svm_migrate_copy_memory_gart(
+ adev, src + i - j,
+ dst + i - j, j,
+ FROM_RAM_TO_VRAM,
+ mfence);
+
+ if (r)
+ goto out_free_vram_pages;
+
+ j = 0;
+ }
+
+ /* for THP check if the first src page is valid
+ * if not valid skip following HPAGE_PMD_NR - 1 pages
+ */
+ spage = migrate_pfn_to_page(migrate->src[i]);
+ if (spage && !is_zone_device_page(spage)) {
+ /* dma_map continuous HPAGE_PMD_NR sys ram pages */
+ src[i] = dma_map_page(dev, spage, 0,
PAGE_SIZE*HPAGE_PMD_NR,
+ DMA_BIDIRECTIONAL);
+
+ r = dma_mapping_error(dev, src[i]);
+ if (r) {
+ dev_err(dev, "%s: fail %d dma_map_page\n",
+ __func__, r);
+ goto out_free_vram_pages;
+ }
+
+ /* get dma address for following HPAGE_PMD_NR-1 pages
+ * since src pages are continuous their dma addresses
+ * are continuous too.
+ */
+ for (l=1; l < k; l++)
+ src[i + l] = src[i] + l*PAGE_SIZE;
+
+ /* migrate the HPAGE_PMD_NR pages above */
+ r = svm_migrate_copy_memory_gart(
+ adev, src + i,
+ dst + i, HPAGE_PMD_NR,
+ FROM_RAM_TO_VRAM,
+ mfence);
+
+ /* mark head page dma mapping as THP, tail pages dma
addr
+ * are set to 0 for following dma_unmap
+ */
+ src[i] |= SVM_RANGE_DMA_THP;
+ for (l = 1; l < k; l++)
+ src[i + l] = 0;
I hope this doesn't break partial mapping or unmapping. We'd need to
be sure that code always aligns addresses to huge-page boundaries and
gets the whole huge page. A safer alternative would be to use a
different flag for the second and subsequent compound pages. So you'd
still have the DMA addresses, but you could ignore them for DMA
unmapping.
For THP system memory core HMM provides
spage=migrate_pfn_to_page(migrate->src[i]) is 2MB aligned. dma adress
src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE*HPAGE_PMD_NR,..), is also
2MB aligned since spage is 2MB aliged and size is 2MB.
I put SVM_RANGE_DMA_THP bit just at dma addr of head page. The following
511 dma address is put 0. At svm_range_dma_unmap_dev I should have
jumped i by 512 when hit SVM_RANGE_DMA_THP:
if (dma_addr[i] & SVM_RANGE_DMA_THP) {
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);
+
+ if (r)
+ goto out_free_vram_pages;
+
+ j = 0;
}
} else {
- if (j) {
+ /* single normal page case */
+ spage = migrate_pfn_to_page(migrate->src[i]);
+ if (spage && !is_zone_device_page(spage)) {
+ src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
+ DMA_BIDIRECTIONAL);
+
+ r = dma_mapping_error(dev, src[i]);
+
+ if (r) {
+ dev_err(dev, "%s: fail %d dma_map_page\n",
+ __func__, r);
+ goto out_free_vram_pages;
+ }
+ j += 1;
+
+ } else if (j) {
r = svm_migrate_copy_memory_gart(
adev, src + i - j,
dst + i - j, j,
FROM_RAM_TO_VRAM,
mfence);
+
if (r)
goto out_free_vram_pages;
- amdgpu_res_next(&cursor, (j + 1) << PAGE_SHIFT);
+
j = 0;
- } else {
- amdgpu_res_next(&cursor, PAGE_SIZE);
}
- continue;
}
- pr_debug_ratelimited("dma mapping src to 0x%llx, pfn
0x%lx\n",
- src[i] >> PAGE_SHIFT, page_to_pfn(spage));
+ pr_debug_ratelimited("dma mapping %lld pages, src to 0x%llx,
pfn 0x%lx\n",
+ k, src[i] >> PAGE_SHIFT, migrate->src[i] >>
MIGRATE_PFN_SHIFT);
+ i += k;
+ m += k;
+
+ if (m >= (cursor.size >> PAGE_SHIFT)) {
+ if (j > 0) {
+ r = svm_migrate_copy_memory_gart(adev, src + i - j,
+ dst + i - j, j,
+ FROM_RAM_TO_VRAM,
+ mfence);
Are you sure this is correct? The old code incremented i after this
copy was done. Your new code does it before. I think that will mess up
your address calculations.
The old code changes i at for-loop: for (i = j = 0; i < npages; i++) .
Here I update i after handle page(either one 4k page or one THP), then
increase i(either 1 or 512). When reach current vram segment boundary,
migrate previous accumulated 4k pages. THP has been migrated before.
Here the code handles the 4k pages that was accumulated when reach to
current vram segment boundary.
+ if (r)
+ goto out_free_vram_pages;
+ }
+
+ amdgpu_res_next(&cursor, m*PAGE_SIZE);
- /* 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,
- mfence);
- if (r)
- goto out_free_vram_pages;
- amdgpu_res_next(&cursor, (j + 1) * PAGE_SIZE);
j = 0;
- } else {
- j++;
+ m = 0;
}
}
@@ -410,17 +509,24 @@ svm_migrate_vma_to_vram(struct kfd_node
*node, struct svm_range *prange,
struct kfd_process_device *pdd;
struct dma_fence *mfence = NULL;
struct migrate_vma migrate = { 0 };
+ bool is_private_device = false;
unsigned long cpages = 0;
unsigned long mpages = 0;
dma_addr_t *scratch;
void *buf;
int r = -ENOMEM;
+ is_private_device = svm_is_private_zone(adev);
+
memset(&migrate, 0, sizeof(migrate));
migrate.vma = vma;
migrate.start = start;
migrate.end = end;
migrate.flags = MIGRATE_VMA_SELECT_SYSTEM;
+
+ if (is_private_device && ((end - start) >> PAGE_SHIFT) >=
HPAGE_PMD_NR)
+ migrate.flags = migrate.flags | MIGRATE_VMA_SELECT_COMPOUND;
+
Why do you apply this only to device_private memory. This should work
just as well for device_coherent on MI200 A+A.
Current work is for private device memory only. Need update core HMM
code for coherent device memory to support THP.
Driver's work to enable device THP actually has no real difference
between private and coherence device memory.
migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
buf = kvcalloc(npages,
@@ -609,6 +715,9 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
u64 addr;
int r = 0;
+ u64 l, k;
+ bool is_large = false;
+
pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start,
prange->last);
@@ -616,8 +725,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
src = (u64 *)(scratch + npages);
dst = scratch;
-
- for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
+ for (i = 0, j = 0; i < npages;) {
If you reset k = 1, you could keep the increment in the loop header.
Just update it to
for (i = 0, j = 0, k = 1; i < npages; i += k, addr += k*PAGE_SIZE,
k = 1)
ok, that makes code more concise.
struct page *spage;
spage = migrate_pfn_to_page(migrate->src[i]);
@@ -633,6 +741,9 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
goto out_oom;
j = 0;
}
+
+ addr += PAGE_SIZE;
+ i++;
continue;
}
src[i] = svm_migrate_addr(adev, spage);
@@ -646,7 +757,22 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
j = 0;
}
- dpage = svm_migrate_get_sys_page(migrate->vma, addr, 0);
+ if(IS_ALIGNED(page_to_pfn(spage), HPAGE_PMD_NR) &&
There should be a space after "if". I see a few more coding style
issues below. Please run check_patch.pl to check for common coding
style issues.
ok, will check code style next submit.
+ (addr + HPAGE_PMD_SIZE) <= migrate->end &&
+ IS_ALIGNED (addr, HPAGE_PMD_SIZE) &&
+ migrate->src[i] & MIGRATE_PFN_COMPOUND) {
+
+ is_large = true;
+ k = HPAGE_PMD_NR;
is_large is redundant. You could just use (k > 1).
ok
+
+ dpage = svm_migrate_get_sys_page(migrate->vma, addr,
+ HPAGE_PMD_ORDER);
Do we need a fallback to small pages if if huge-page allocation fails?
I thought about that for system page allocation: if kernel cannot
provide THP system memory allocate in regular page base, if still
cannot, fail svm_migrate_get_sys_page.
Same for device memory allocation when allocate THP from ttm. If fail,
fallback to 4k page based allocation.
Before migration check driver allocated page is THP or not, then use
different procedure.
+ } else {
+ k = 1;
+ is_large = false;
+ 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);
@@ -654,21 +780,59 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
goto out_oom;
}
- dst[i] = dma_map_page(dev, dpage, 0, PAGE_SIZE,
DMA_BIDIRECTIONAL);
+ dst[i] = dma_map_page(dev, dpage, 0, PAGE_SIZE*k,
DMA_BIDIRECTIONAL);
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;
Why did you remove this?
ok, dst[i] is still used at svm_range_dma_unmap_dev for error handling
case.
goto out_oom;
}
- pr_debug_ratelimited("dma mapping dst to 0x%llx, pfn
0x%lx\n",
- dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
-
migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
+ if (is_large)
+ migrate->dst[i] |= MIGRATE_PFN_COMPOUND;
Looks like you can merge that into the next if-block just below.
Yes, I do not know what I thought when did it. Maybe forgot cleaning
code after removed some debug code here.
- dpage = NULL;
- j++;
+ if (is_large) {
+ /* migrate previous accumulated pages */
+ if(j) {
+ r = svm_migrate_copy_memory_gart(adev, dst + i - j,
+ src + i - j, j, FROM_VRAM_TO_RAM,
mfence);
+ if (r)
+ goto out_oom;
+ j = 0;
+ }
+
+ for (l = 1; l < k; l++) {
+
+ src[i + l] = src[i] + l*PAGE_SIZE;
+ dst[i + l] = dst[i] + l*PAGE_SIZE;
+ migrate->dst[i + l] = 0;
+ }
+
+ /* migrate the HPAGE_PMD_NR pages above */
+ /* svm_migrate_copy_memory_gart will add a paramter to
indicate
+ * the migration is for 2MB THP
+ */
+ r = svm_migrate_copy_memory_gart(
+ adev, dst + i,
+ src + i, HPAGE_PMD_NR,
+ FROM_VRAM_TO_RAM,
+ mfence);
+
+ /* mark head page dma mapping as THP, tail pages dma addr
+ * are set to 0 for following dma_unmap
+ */
+ dst[i] |= SVM_RANGE_DMA_THP;
+ for (l = 1; l < k; l++)
+ dst[i + l] = 0;
I hope this doesn't break partial mapping or unmapping. We'd need to
be sure that code always aligns addresses to huge-page boundaries and
gets the whole huge page.
Same as migration from sys to device. dst[i] is dma address for THP
system page. It is 2MB aligned and the mapping size is
PAGE_SIZE*k(k=512) for THP.
+
+ if (r)
+ goto out_oom;
+
+ } else
+ j++;
+
+ addr += PAGE_SIZE*k;
+ i += k;
}
if (j > 0)
@@ -687,12 +851,9 @@ svm_migrate_copy_to_ram(struct amdgpu_device
*adev, struct svm_range *prange,
/* release previous allocated sys pages and unmap dma
address */
while (i--) {
- if (dst[i]) {
- dma_unmap_page(dev, dst[i], PAGE_SIZE,
- DMA_BIDIRECTIONAL);
- dst[i] = 0;
- }
-
+ /* follwing svm_range_dma_unmap_dev will do dma unmap
anyway
+ * not need do dma unmap here
+ */
dpage = migrate_pfn_to_page(migrate->dst[i]);
if (!dpage)
continue;
@@ -733,6 +894,7 @@ svm_migrate_vma_to_ram(struct kfd_node *node,
struct svm_range *prange,
unsigned long cpages = 0;
unsigned long mpages = 0;
struct amdgpu_device *adev = node->adev;
+ bool is_private_device = false;
struct kfd_process_device *pdd;
struct dma_fence *mfence = NULL;
struct migrate_vma migrate = { 0 };
@@ -740,6 +902,8 @@ svm_migrate_vma_to_ram(struct kfd_node *node,
struct svm_range *prange,
void *buf;
int r = -ENOMEM;
+ is_private_device = svm_is_private_zone(adev);
+
memset(&migrate, 0, sizeof(migrate));
migrate.vma = vma;
migrate.start = start;
@@ -750,6 +914,9 @@ svm_migrate_vma_to_ram(struct kfd_node *node,
struct svm_range *prange,
else
migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
+ if (is_private_device && ((end - start) >> PAGE_SHIFT) >=
HPAGE_PMD_NR)
+ migrate.flags = migrate.flags | MIGRATE_VMA_SELECT_COMPOUND;
Why do you apply this only to device_private memory. This should work
just as well for device_coherent on MI200 A+A.
Current work is for private device only. Need update core HMM code for
coherent device memory to support THP.
Driver's work to enable device THP actually has no real difference
between private and coherence device memory since driver uses same
migration path.
+
buf = kvcalloc(npages,
2 * sizeof(*migrate.src) + sizeof(u64) +
sizeof(dma_addr_t),
GFP_KERNEL);
@@ -1132,7 +1299,7 @@ int kgd2kfd_init_zone_device(struct
amdgpu_device *adev)
amdgpu_amdkfd_reserve_system_mem(SVM_HMM_PAGE_STRUCT_SIZE(size));
- pr_info("HMM registered %ldMB device memory\n", size >> 20);
+ pr_info("---XCHEN 3.2 HMM registered %ldMB device memory\n",
size >> 20);
This looks like it's not meant to be submitted.
Sorry about that. I should have removed debug stuff put during triage
before submit.
Thanks
Xiaogang
Regards,
Felix
return 0;
}
