Re: [PATCH] drm/amdkfd: avoid conflicting address mappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 9/29/2021 9:15 PM, Felix Kuehling wrote:
On 2021-09-29 7:35 p.m., Mike Lothian wrote:
Hi

This patch is causing a compile failure for me

drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_chardev.c:1254:25: error:
unused variable 'svms' [-Werror,-Wunused-variable]
        struct svm_range_list *svms = &p->svms;
                               ^
1 error generated.

I'll turn off Werror
I guess the struct svm_range_list *svms declaration should be under #if IS_ENABLED(CONFIG_HSA_AMD_SVM). Alternatively, we could get rid of it and use p->svms directly (it's used in 3 places in that function).

Would you like to propose a patch for that?

I have submitted the patch that fix this for review

Regards,
Alex Sierra


Thanks,
  Felix



On Mon, 19 Jul 2021 at 22:19, Alex Sierra <alex.sierra@xxxxxxx> wrote:
[Why]
Avoid conflict with address ranges mapped by SVM
mechanism that try to be allocated again through
ioctl_alloc in the same process. And viceversa.

[How]
For ioctl_alloc_memory_of_gpu allocations
Check if the address range passed into ioctl memory
alloc does not exist already in the kfd_process
svms->objects interval tree.

For SVM allocations
Look for the address range into the interval tree VA from
the VM inside of each pdds used in a kfd_process.

Signed-off-by: Alex Sierra <alex.sierra@xxxxxxx>
---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 ++++
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 79 +++++++++++++++++++-----
  2 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 67541c30327a..f39baaa22a62 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1251,6 +1251,7 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
         struct kfd_process_device *pdd;
         void *mem;
         struct kfd_dev *dev;
+       struct svm_range_list *svms = &p->svms;
         int idr_handle;
         long err;
         uint64_t offset = args->mmap_offset;
@@ -1259,6 +1260,18 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
         if (args->size == 0)
                 return -EINVAL;

+#if IS_ENABLED(CONFIG_HSA_AMD_SVM)
+       mutex_lock(&svms->lock);
+       if (interval_tree_iter_first(&svms->objects,
+                                    args->va_addr >> PAGE_SHIFT,
+                                    (args->va_addr + args->size - 1) >> PAGE_SHIFT)) {
+               pr_err("Address: 0x%llx already allocated by SVM\n",
+                       args->va_addr);
+               mutex_unlock(&svms->lock);
+               return -EADDRINUSE;
+       }
+       mutex_unlock(&svms->lock);
+#endif
         dev = kfd_device_by_id(args->gpu_id);
         if (!dev)
                 return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 31f3f24cef6a..043ee0467916 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2581,9 +2581,54 @@ int svm_range_list_init(struct kfd_process *p)
         return 0;
  }

+/**
+ * svm_range_is_vm_bo_mapped - check if virtual address range mapped already
+ * @p: current kfd_process
+ * @start: range start address, in pages
+ * @last: range last address, in pages
+ *
+ * The purpose is to avoid virtual address ranges already allocated by
+ * kfd_ioctl_alloc_memory_of_gpu ioctl.
+ * It looks for each pdd in the kfd_process.
+ *
+ * Context: Process context
+ *
+ * Return 0 - OK, if the range is not mapped.
+ * Otherwise error code:
+ * -EADDRINUSE - if address is mapped already by kfd_ioctl_alloc_memory_of_gpu + * -ERESTARTSYS - A wait for the buffer to become unreserved was interrupted by
+ * a signal. Release all buffer reservations and return to user-space.
+ */
+static int
+svm_range_is_vm_bo_mapped(struct kfd_process *p, uint64_t start, uint64_t last)
+{
+       uint32_t i;
+       int r;
+
+       for (i = 0; i < p->n_pdds; i++) {
+               struct amdgpu_vm *vm;
+
+               if (!p->pdds[i]->drm_priv)
+                       continue;
+
+               vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
+               r = amdgpu_bo_reserve(vm->root.bo, false);
+               if (r)
+                       return r;
+               if (interval_tree_iter_first(&vm->va, start, last)) {
+                       pr_debug("Range [0x%llx 0x%llx] already mapped\n", start, last);
+                       amdgpu_bo_unreserve(vm->root.bo);
+                       return -EADDRINUSE;
+               }
+               amdgpu_bo_unreserve(vm->root.bo);
+       }
+
+       return 0;
+}
+
  /**
   * svm_range_is_valid - check if virtual address range is valid
- * @mm: current process mm_struct
+ * @mm: current kfd_process
   * @start: range start address, in pages
   * @size: range size, in pages
   *
@@ -2592,28 +2637,27 @@ int svm_range_list_init(struct kfd_process *p)
   * Context: Process context
   *
   * Return:
- *  true - valid svm range
- *  false - invalid svm range
+ *  0 - OK, otherwise error code
   */
-static bool
-svm_range_is_valid(struct mm_struct *mm, uint64_t start, uint64_t size)
+static int
+svm_range_is_valid(struct kfd_process *p, uint64_t start, uint64_t size)
  {
         const unsigned long device_vma = VM_IO | VM_PFNMAP | VM_MIXEDMAP;
         struct vm_area_struct *vma;
         unsigned long end;
+       unsigned long start_unchg = start;

         start <<= PAGE_SHIFT;
         end = start + (size << PAGE_SHIFT);
-
         do {
-               vma = find_vma(mm, start);
+               vma = find_vma(p->mm, start);
                 if (!vma || start < vma->vm_start ||
                     (vma->vm_flags & device_vma))
-                       return false;
+                       return -EFAULT;
                 start = min(end, vma->vm_end);
         } while (start < end);

-       return true;
+       return svm_range_is_vm_bo_mapped(p, start_unchg, (end - 1) >> PAGE_SHIFT);
  }

  /**
@@ -2913,9 +2957,9 @@ svm_range_set_attr(struct kfd_process *p, uint64_t start, uint64_t size,

         svm_range_list_lock_and_flush_work(svms, mm);

-       if (!svm_range_is_valid(mm, start, size)) {
-               pr_debug("invalid range\n");
-               r = -EFAULT;
+       r = svm_range_is_valid(p, start, size);
+       if (r) {
+               pr_debug("invalid range r=%d\n", r);
                 mmap_write_unlock(mm);
                 goto out;
         }
@@ -3016,17 +3060,18 @@ svm_range_get_attr(struct kfd_process *p, uint64_t start, uint64_t size,
         uint32_t flags = 0xffffffff;
         int gpuidx;
         uint32_t i;
+       int r = 0;

         pr_debug("svms 0x%p [0x%llx 0x%llx] nattr 0x%x\n", &p->svms, start,
                  start + size - 1, nattr);

         mmap_read_lock(mm);
-       if (!svm_range_is_valid(mm, start, size)) {
-               pr_debug("invalid range\n");
-               mmap_read_unlock(mm);
-               return -EINVAL;
-       }
+       r = svm_range_is_valid(p, start, size);
         mmap_read_unlock(mm);
+       if (r) {
+               pr_debug("invalid range r=%d\n", r);
+               return r;
+       }

         for (i = 0; i < nattr; i++) {
                 switch (attrs[i].type) {
--
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux