Re: [PATCH v2] drm/amdkfd: Check ownership of device pages for DMA mapping

[Felix Kuehling]

On 2026-11-06 15:34, Philip Yang wrote:


On 2026-11-05 15:47, Felix Kuehling wrote:
This issue was raised independently in two contexts: pages from DAX
devices and pages from other GPU hives or inherited from other
processed. This patch addresses both concerns together along with
Sashiko comments raised for one of the previous reviews:
https://sashiko.dev/#/patchset/20261103-kfd-svm-foreign-vram-20261103-v1-1-d6282053c...@gmail.com?part=1

Before treating a page as a zone device page belonging to the SVM range,
check that it actually belongs to that range based on the
zone_device_data. Non-private pages belonging to other devices or ranges
can be treated like system memory for the purpose of DMA mapping.

Cc:matthewkre...@gmail.com
Cc:jm5905...@gmail.com
Signed-off-by: Felix Kuehling<felix.kuehl...@amd.com>
Tested with a forked child process after the parent prefetched the SVM 
range to VRAM. The child's access now triggers a GPU page fault — the 
svm_bo belongs to the parent process, so updating the mapping fails — 
instead of crashing the driver on a NULL prange->svm_bo. This is what 
COW should give us: the child must not access the parent's range in VRAM.

Thanks for testing. I assume this was a hand-written low-level test 
because I still wouldn't expect to see this in practice.

I'll respin another version of this patch to avoid having to take and 
release the range lock once per page. I'll prefetch the svm_bo pointer once.

Thanks,
  Felix



Tested-and-Reviewed-by: Philip Yang<philip.y...@amd.com>
---

v2: Addressed code review comments from Sashiko
---
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 24 +++++++++++++++++++++++-
  1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 876476046926..2aade0183578 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -156,6 +156,18 @@ svm_is_valid_dma_mapping_addr(struct device *dev, 
dma_addr_t dma_addr)
               !(dma_addr & SVM_RANGE_VRAM_DOMAIN);
  }
  
+static bool svm_page_is_range_owned(struct page *page, struct svm_range *prange)
+{
+       bool is_owned;
+
+       mutex_lock(&prange->lock);
+       is_owned = is_zone_device_page(page) && prange->ttm_res &&
+               page->zone_device_data == prange->svm_bo;
+       mutex_unlock(&prange->lock);
+
+       return is_owned;
+}
+
  static int
  svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,
                      unsigned long offset, unsigned long npages,
@@ -180,7 +192,7 @@ 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)) {
+               if (svm_page_is_range_owned(page, prange)) {
                        struct amdgpu_device *bo_adev = 
prange->svm_bo->node->adev;
  
  			addr[i] = (hmm_pfns[i] << PAGE_SHIFT) +
@@ -189,6 +201,16 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct 
svm_range *prange,
                        addr[i] |= SVM_RANGE_VRAM_DOMAIN;
                        pr_debug_ratelimited("vram address: 0x%llx\n", addr[i]);
                        continue;
+               } else if (is_device_private_page(page)) {
+                       /* Device private memory from another driver or range.
+                        * This should never happen in practice but it's
+                        * theoretically possible for read-only VMAs after a
+                        * process forks, creates a new KFD context and maps
+                        * the same page.
+                        */
+                       pr_debug("Trying to map device private page from another 
device or range\n");
+                       addr[i] = 0;
+                       return -EINVAL;
                }
                addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
                r = dma_mapping_error(dev, addr[i]);