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.
+ is_large = true;
+ k = HPAGE_PMD_NR;
is_large is redundant. Just use (k > 1).
+ } else
+ k = 1;
Bad indentation.
+
+ /* 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.
+
+ 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.
+ 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.
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)
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.
+ (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).
+
+ 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?
+ } 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?
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.
- 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.
+
+ 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.
+
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.
Regards,
Felix
return 0;
}
