[PATCH] drm/amdkfd: avoid conflicting address mappings
Sierra Guiza, Alejandro (Alex)
alex.sierra at amd.com
Thu Sep 30 16:32:44 UTC 2021
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 at amd.com> 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 at amd.com>
>>> ---
>>> 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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list