From 094073ff74d0fb163b77de7954693bdd2b186a07 Mon Sep 17 00:00:00 2001 From: Felix Kuehling Date: Fri, 31 Aug 2018 14:36:46 -0400 Subject: [PATCH] Revert "libhsakmt: Try to use CPU addr as GPU addr for userptrs" This reverts commit 84bb9072c0bb8bc65f33804f5acec8eda57ab0cd. This fixes ambiguity when looking up GPU addresses with hsaKmtQueryPointerInfo. hsa_amd_agents_allow_access uses hsaKmtQueryPointerInfo, and depends on finding the correct object from a GPU address. Finding the wrong userptr object based on its CPU address leads to incorrect GPU mappings and results in VM faults. Change-Id: I7c5f571ee6e1f9d32687aa3eab6d96944ad032be Signed-off-by: Felix Kuehling [ROCm/ROCR-Runtime commit: a9bd6e6f8bd517e4282da90a6336b90b4949a849] --- projects/rocr-runtime/src/fmm.c | 69 +++------------------------ projects/rocr-runtime/src/libhsakmt.h | 4 +- 2 files changed, 7 insertions(+), 66 deletions(-) diff --git a/projects/rocr-runtime/src/fmm.c b/projects/rocr-runtime/src/fmm.c index 22c572b43e..24ddddcd5e 100644 --- a/projects/rocr-runtime/src/fmm.c +++ b/projects/rocr-runtime/src/fmm.c @@ -502,38 +502,6 @@ loop: return cur; /* NULL if not found */ } -/* Returns true if there is any object in GPU VM address space - * overlapping the specified address range - */ -static bool vm_exists_overlapping_object(manageable_aperture_t *aper, - const void *addr, uint64_t size) -{ - unsigned long start_addr = (unsigned long)addr; - unsigned long end_addr = start_addr + size; - rbtree_t *tree = vm_object_tree(aper, 0); - rbtree_key_t start_key, end_key; - rbtree_node_t *rn_start, *ln_end; - vm_object_t *cur; - - start_key = rbtree_key(start_addr, 0); - rn_start = rbtree_lookup_nearest(tree, &start_key, LKP_ALL, RIGHT); - if (rn_start) { - cur = vm_object_entry(rn_start, 0); - if ((unsigned long)cur->start < end_addr) - return true; - } - - end_key = rbtree_key(end_addr, 0); - ln_end = rbtree_lookup_nearest(tree, &end_key, LKP_ALL, LEFT); - if (ln_end) { - cur = vm_object_entry(ln_end, 0); - if ((unsigned long)cur->start + cur->size > start_addr) - return true; - } - - return false; -} - static vm_object_t *vm_find_object_by_address(manageable_aperture_t *app, const void *address, uint64_t size) { @@ -1277,7 +1245,7 @@ static void *__fmm_allocate_device(uint32_t gpu_id, uint64_t MemorySizeInBytes, manageable_aperture_t *aperture, uint64_t *mmap_offset, uint32_t flags, vm_object_t **vm_obj) { - void *mem = NULL, *userpage = NULL; + void *mem = NULL; vm_object_t *obj; /* Check that aperture is properly initialized/supported */ @@ -1286,27 +1254,9 @@ static void *__fmm_allocate_device(uint32_t gpu_id, uint64_t MemorySizeInBytes, /* Allocate address space */ pthread_mutex_lock(&aperture->fmm_mutex); - /* If it's a userptr within the GPU-addressable address space, - * and no object is registered yet in the same address range, - * then use the CPU address as GPU address - */ - if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) && mmap_offset) { - userpage = (void *)*mmap_offset; - if (userpage >= aperture->base && - VOID_PTR_ADD(userpage, MemorySizeInBytes - 1) <= aperture->limit && - !vm_exists_overlapping_object(aperture, userpage, MemorySizeInBytes)) - mem = userpage; - } - if (!mem) - mem = aperture_allocate_area(aperture, MemorySizeInBytes); + mem = aperture_allocate_area(aperture, MemorySizeInBytes); pthread_mutex_unlock(&aperture->fmm_mutex); - if (!mem) { - pr_err("Failed to allocate %ld bytes virtual address space\n", - MemorySizeInBytes); - return NULL; - } - /* * Now that we have the area reserved, allocate memory in the device * itself @@ -1318,11 +1268,9 @@ static void *__fmm_allocate_device(uint32_t gpu_id, uint64_t MemorySizeInBytes, * allocation of memory in device failed. * Release region in aperture */ - if (mem != userpage) { - pthread_mutex_lock(&aperture->fmm_mutex); - aperture_release_area(aperture, mem, MemorySizeInBytes); - pthread_mutex_unlock(&aperture->fmm_mutex); - } + pthread_mutex_lock(&aperture->fmm_mutex); + aperture_release_area(aperture, mem, MemorySizeInBytes); + pthread_mutex_unlock(&aperture->fmm_mutex); /* Assign NULL to mem to indicate failure to calling function */ mem = NULL; @@ -1626,12 +1574,7 @@ static void __fmm_release(vm_object_t *object, manageable_aperture_t *aperture) args.handle = object->handle; kmtIoctl(kfd_fd, AMDKFD_IOC_FREE_MEMORY_OF_GPU, &args); - /* Userptrs with CPU addr == GPU addr don't have aperture - * space allocated to them - */ - if (!object->userptr || - (void *)PAGE_ALIGN_DOWN(object->userptr) != object->start) - aperture_release_area(aperture, object->start, object->size); + aperture_release_area(aperture, object->start, object->size); vm_remove_object(aperture, object); pthread_mutex_unlock(&aperture->fmm_mutex); diff --git a/projects/rocr-runtime/src/libhsakmt.h b/projects/rocr-runtime/src/libhsakmt.h index 9646190609..8ed894bf4e 100644 --- a/projects/rocr-runtime/src/libhsakmt.h +++ b/projects/rocr-runtime/src/libhsakmt.h @@ -65,10 +65,8 @@ extern int PAGE_SHIFT; do { if ((uint64_t)PORT_VPTR_TO_UINT64(x) % PAGE_SIZE) return HSAKMT_STATUS_INVALID_PARAMETER; } while(0) #define ALIGN_UP(x,align) (((uint64_t)(x) + (align) - 1) & ~(uint64_t)((align)-1)) -#define ALIGN_DOWN(x,align) ((uint64_t)(x) & ~(uint64_t)((align)-1)) #define ALIGN_UP_32(x,align) (((uint32_t)(x) + (align) - 1) & ~(uint32_t)((align)-1)) -#define PAGE_ALIGN_UP(x) ALIGN_UP((x),PAGE_SIZE) -#define PAGE_ALIGN_DOWN(x) ALIGN_DOWN((x),PAGE_SIZE) +#define PAGE_ALIGN_UP(x) ALIGN_UP(x,PAGE_SIZE) #define BITMASK(n) (((n) < sizeof(1ULL) * CHAR_BIT ? (1ULL << (n)) : 0) - 1ULL) #define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0]))