drm/amdkfd: Only take the VRAM path for KFD device pages in svm_range_dma_map_dev
Subsystems (4)
PATCH 1/1 drm/amdkfd: Only take the VRAM path for KFD device pages in svm_range_dma_map_dev
svm_range_dma_map_dev() treats every ZONE_DEVICE page returned by hmm_range_fault() as VRAM of the range's SVM BO. KFD's own VRAM pages are MEMORY_DEVICE_PRIVATE or MEMORY_DEVICE_COHERENT, but fsdax (MEMORY_DEVICE_FS_DAX) and device-dax (MEMORY_DEVICE_GENERIC) pages are ZONE_DEVICE too. They are system memory.
When a process maps a file on a filesystem mounted with -o dax and lets the GPU access it (for example hipMemcpy() from the mapping, which registers the range through the SVM ioctl), the range is in system memory and prange->svm_bo is NULL, so the VRAM branch dereferences it:
BUG: kernel NULL pointer dereference, address: 0000000000000078 RIP: 0010:svm_range_dma_map_dev+0x13b/0x300 [amdgpu] Call Trace: svm_range_validate_and_map+0x600/0xa70 [amdgpu] svm_range_set_attr+0x450/0x8c0 [amdgpu] svm_ioctl+0x62/0x80 [amdgpu] kfd_ioctl_svm+0x5e/0xe0 [amdgpu] kfd_ioctl+0x532/0x680 [amdgpu]
The process is then left unkillable and later KFD calls in other processes can block, until a reboot.
Even with an SVM BO the computed address would be wrong for such a page. Take the VRAM path only for device private and device coherent pages and DMA map every other page, as for normal system memory.
---
Notes:
Reproducer (needs a filesystem mounted with -o dax; an emulated pmem from memmap=4G!12G and "ndctl create-namespace -m fsdax" works too):
fd = open("/mnt/pmem/file", O_RDONLY); /* >= 64 MiB, on DAX */
src = mmap(NULL, 64 << 20, PROT_READ, MAP_SHARED, fd, 0); hipMalloc(&dst, 64 << 20);
hipMemcpy(dst, src, 64 << 20, hipMemcpyHostToDevice);
An application doing this (hipMemcpy from an mmap of a file on the DAX mount) oopsed as in the commit message. The minimal program above was written afterwards and has only been run on the patched kernel.
Tested on Ubuntu 7.0.0-38-generic (based on v7.0.14) with this patch applied to that tree (it applies cleanly), 2x Radeon Pro V620 (Navi 21, gfx1030), ROCm 7.14, Intel Optane in fsdax mode, XFS mounted dax=always:
- Before: the application oopsed in svm_range_dma_map_dev on every start (3 of 3, on GPU 0 and on GPU 1); the task was left unkillable and a later hipHostRegister in another process blocked in __synchronize_srcu until a reboot.
- After: hipMemcpy and hipHostRegister + copy from the DAX mapping, 64 MiB and 1 GiB, on both GPUs, data read back equal; the same on a non-DAX file; the application (an LLM server mapping a 111 GB model file from the DAX mount) starts and answers requests. No oops.
- With dynamic debug on svm_range_dma_map_dev, the pages DMA mapped during the copy have pfns inside the pmem region, and KFD's own VRAM pages in the same call still take the VRAM branch.
Not tested: MEMORY_DEVICE_COHERENT systems (the GPU memory registered as coherent), device-dax (/dev/daxX.Y) mappings, other ASICs.
A stricter test would be "the page belongs to a KFD pgmap", e.g. page_pgmap(page)->ops == &svm_migrate_pgmap_ops, which also rules out another driver's device-coherent pages. I used the type test because it needs no new helper; happy to switch if you prefer that.
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/amd/amdkfd/kfd_svm.c +6 -1
@@ -180,7 +180,12 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,dma_unmap_page (dev, addr[i],PAGE_SIZE , dir); page =hmm_pfn_to_page (hmm_pfns[i]); -if (is_zone_device_page (page)) { +/* +* Only KFD's own VRAM pages (device private or device coherent) +* take the VRAM path. Other ZONE_DEVICE pages, such as fsdax or +* device-dax memory, are system memory and are DMA mapped below. +*/ +if (is_device_private_page (page) ||is_device_coherent_page (page)) {struct amdgpu_device *bo_adev = prange->svm_bo->node->adev; addr[i] = (hmm_pfns[i] << PAGE_SHIFT) +
Signature · 1 line
2.43.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/amdkfd: dma_map_page() fails on DAX ZONE_DEVICE pages with DMA_API_DEBUG --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/amdkfd: dma_map_page() fails on DAX ZONE_DEVICE pages with DMA_API_DEBUG --
commit 0eacef2f328edc8a87aa6b82601cf66a9676e3be
Author: Matthew Krebs <[email protected]>
drm/amdkfd: Only take the VRAM path for KFD device pages in svm_range_dma_map_dev
This commit restricts the VRAM mapping branch in svm_range_dma_map_dev() to device-private and device-coherent pages so that other ZONE_DEVICE pages, such as fsdax and device-dax mappings, are DMA mapped instead of dereferencing a potentially NULL prange->svm_bo.
Quoted 5 lines
drivers/gpu/drm/amd/amdkfd/kfd_svm.c +0
-0
drivers/gpu/drm/amd/amdkfd/kfd_svm.c svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange, · line 188
Quoted 10 lines
drivers/gpu/drm/amd/amdkfd/kfd_svm.c +6
-1@@ -180,7 +180,12 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,dma_unmap_page (dev, addr[i],PAGE_SIZE , dir); page =hmm_pfn_to_page (hmm_pfns[i]); -if (is_zone_device_page (page)) { +/* +* Only KFD's own VRAM pages (device private or device coherent) +* take the VRAM path. Other ZONE_DEVICE pages, such as fsdax or +* device-dax memory, are system memory and are DMA mapped below. +*/ +if (is_device_private_page (page) ||is_device_coherent_page (page)) {
[Severity: Medium]
When fsdax (MEMORY_DEVICE_FS_DAX) or device-dax (MEMORY_DEVICE_GENERIC) pages skip this branch in drivers/gpu/drm/amd/amdkfd/kfd_svm.c:svm_range_dma_map_dev(), they fall through to dma_map_page():
drivers/gpu/drm/amd/amdkfd/kfd_svm.c:svm_range_dma_map_dev() {
...
if (is_device_private_page(page) || is_device_coherent_page(page)) {
...
continue;
}
addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
...
}In kernel/dma/mapping.c:dma_map_page_attrs(), all ZONE_DEVICE pages are rejected when CONFIG_DMA_API_DEBUG is enabled:
kernel/dma/mapping.c:dma_map_page_attrs() {
...
if (IS_ENABLED(CONFIG_DMA_API_DEBUG) &&
WARN_ON_ONCE(is_zone_device_page(page)))
return DMA_MAPPING_ERROR;
...
}Won't mapping a fsdax or device-dax range through the KFD SVM ioctl (kfd_ioctl_svm() -> svm_ioctl() -> svm_range_set_attr() -> svm_range_validate_and_map() -> svm_range_dma_map() -> svm_range_dma_map_dev()) trigger this WARN_ON_ONCE() and fail with -ENOMEM when CONFIG_DMA_API_DEBUG is enabled?
Should svm_range_dma_map_dev() and its unmap path use dma_map_phys() and dma_unmap_phys() (similar to hmm_dma_map_pfn() in mm/hmm.c) instead of dma_map_page() and dma_unmap_page() when mapping non-VRAM ZONE_DEVICE pages?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Felix Kuehling
maintainerQuoted 25 lines · Matthew Krebs
[You don't often get email [email protected]. Learn why this is important athttps://aka.ms/LearnAboutSenderIdentification ]
svm_range_dma_map_dev()treats everyZONE_DEVICEpage returned byhmm_range_fault()as VRAM of the range's SVM BO. KFD's own VRAM pages areMEMORY_DEVICE_PRIVATEorMEMORY_DEVICE_COHERENT, but fsdax (MEMORY_DEVICE_FS_DAX) and device-dax (MEMORY_DEVICE_GENERIC) pages areZONE_DEVICEtoo. They are system memory.When a process maps a file on a filesystem mounted with -o dax and lets the GPU access it (for example
hipMemcpy()from the mapping, which registers the range through the SVM ioctl), the range is in system memory andprange->svm_bois NULL, so the VRAM branch dereferences it:BUG: kernel NULL pointer dereference, address: 0000000000000078 RIP: 0010:svm_range_dma_map_dev+0x13b/0x300 [amdgpu] Call Trace: svm_range_validate_and_map+0x600/0xa70 [amdgpu] svm_range_set_attr+0x450/0x8c0 [amdgpu] svm_ioctl+0x62/0x80 [amdgpu] kfd_ioctl_svm+0x5e/0xe0 [amdgpu] kfd_ioctl+0x532/0x680 [amdgpu]The process is then left unkillable and later KFD calls in other processes can block, until a reboot.
Quoted 8 lines
Even with an SVM BO the computed address would be wrong for such a page. Take the VRAM path only for device private and device coherent pages and DMA map every other page, as for normal system memory.
Fixes 1d5dbfe6c06a ("drm/amdkfd: classify and map mixed svm range pages in GPU")Assisted-by Claude:claude-opus-5-5Signed-off-by Matthew Krebs<[email protected]>
This may be superseded or enhanced by Aveline Noir's patch "drm/amdkfd: reject foreign device pages in svm_range_dma_map_dev()" that seems to attempt to cover a wider class of errors in the same area. I think the bottom line is, that we want to be sure that the page we're looking at is actually a zone-device page from the same BO. I think in your case we want to fall back to mapping the memory as system memory. In Aveline's case we have to fail. I'll look for a sensible way to combine these two patches.
I have one suggestion inline ...
Quoted 47 lines
---
Notes:
Reproducer (needs a filesystem mounted with -o dax; an emulated pmem from memmap=4G!12G and "ndctl create-namespace -m fsdax" works too):fd =
open("/mnt/pmem/file", O_RDONLY); /* >= 64 MiB, on DAX */
src =mmap(NULL, 64 << 20, PROT_READ, MAP_SHARED, fd, 0);hipMalloc(&dst, 64 << 20);
hipMemcpy(dst, src, 64 << 20, hipMemcpyHostToDevice);An application doing this (hipMemcpy from an mmap of a file on the DAX mount) oopsed as in the commit message. The minimal program above was written afterwards and has only been run on the patched kernel.
Tested on Ubuntu 7.0.0-38-generic (based on v7.0.14) with this patch applied to that tree (it applies cleanly), 2x Radeon Pro V620 (Navi 21, gfx1030), ROCm 7.14, Intel Optane in fsdax mode, XFS mounted dax=always:
- Before: the application oopsed in
svm_range_dma_map_devon every start (3 of 3, on GPU 0 and on GPU 1); the task was left unkillable and a later hipHostRegister in another process blocked in__synchronize_srcuuntil a reboot.
- After: hipMemcpy and hipHostRegister + copy from the DAX mapping, 64 MiB and 1 GiB, on both GPUs, data read back equal; the same on a non-DAX file; the application (an LLM server mapping a 111 GB model file from the DAX mount) starts and answers requests. No oops.
- With dynamic debug onsvm_range_dma_map_dev, the pages DMA mapped during the copy have pfns inside the pmem region, and KFD's own VRAM pages in the same call still take the VRAM branch.Not tested:
MEMORY_DEVICE_COHERENTsystems (the GPU memory registered as coherent), device-dax (/dev/daxX.Y) mappings, other ASICs.A stricter test would be "the page belongs to a KFD pgmap", e.g. page_pgmap(page)->ops ==
&svm_migrate_pgmap_ops, which also rules out another driver's device-coherent pages. I used the type test because it needs no new helper; happy to switch if you prefer that.drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/amd/amdkfd/kfd_svm.c +0
-0
drivers/gpu/drm/amd/amdkfd/kfd_svm.c svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange, · line 188
Quoted 10 lines
drivers/gpu/drm/amd/amdkfd/kfd_svm.c +6
-1@@ -180,7 +180,12 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,dma_unmap_page (dev, addr[i],PAGE_SIZE , dir); page =hmm_pfn_to_page (hmm_pfns[i]); -if (is_zone_device_page (page)) { +/* +* Only KFD's own VRAM pages (device private or device coherent) +* take the VRAM path. Other ZONE_DEVICE pages, such as fsdax or +* device-dax memory, are system memory and are DMA mapped below. +*/ +if (is_device_private_page (page) ||is_device_coherent_page (page)) {
Could we check the page_pgmap(page) here as a more reliable way to check that the page is actually coming from our driver, rather than just using the page type? I could see hypothetical cases where a device_coherent page comes from a different driver and should be treated like system memory as well. Looking at the pagemap would catch that.
if (is_zone_device_page(page) && prange->svm_bo &&
page_pgmap(page) ==prange->svm_bo->node->adev->kfd.pgmap) {
/* local device memory, no DMA mapping */
continue;
} else if (WARN_ONCE(is_zone_device_private(page))) {
/* fail ... device private memory from another driver, this should never happen */
}
/* system memory case with DMA mapping */Regards,
Felix
Quoted 5 lines
drivers/gpu/drm/amd/amdkfd/kfd_svm.c +0
-1struct amdgpu_device *bo_adev = prange->svm_bo->node->adev; addr[i] = (hmm_pfns[i] << PAGE_SHIFT) + --2.43.0