Topology, memory allocation, cleanup issue for gGPU

Patch submitted by Besar Wicaksono

1. Bug on detecting local memory size interpreted as 32 bit value
instead of 64. The bug causes thunk to go into an infinite loop trying
to reserve virtual address range for dgpu system memory.
2. SIMD count in the node property is 0. Runtime use this attribute to
find a gpu device.
	Regarding other attributes of intel+tonga topology, Harish started a
	discussion on August iirc, could you please share an update ?
	This would help me progress with more tests such as scratch memory,
	which require the scratch aperture information in order to construct a
	buffer srd in gpuvm space.
3. Bug on releasing memory via fmm_release, where no actual release is
being done. The vm_object can't be found because the memory size does
not match due to the allocation padded the size with 32KB.
4. Pointer arithmetic on vm_area allocation/release. The value of
vm_area_t::end seems to be interpreted inconsistently whether it is
(start + size  -1) or (start + size).
	One example of potential issue I see is the logic could report
	larger size of the hole in the vm area list.
5. Resource cleanup on multiple library load/unload within a single
process.
	- Any memory allocation on subsequent library load will result
	an error "va above limit". To my understanding this is due to
	the reserved memory for the system memory not being released on unload.
	- The static variable events_page needs to be invalidated
	appropriately on library unload so the next load could
	reinitialize it.
6. Could you please update if AQL queue is ready to test with the stg
kfd/kmt ?
7. The system memory allocation with size larger than 32KB seems to be
padded by an extra 32KB. I was wondering if we could remove this
overhead.

Change-Id: I039988d36637525089c7569dc3b77e58750e2121


[ROCm/ROCR-Runtime commit: ee08f537a7]

projects/rocr-runtime/src/events.c

@@ -42,6 +42,13 @@ static bool IsSystemEventType(HSA_EVENTTYPE type)
 	return (type != HSA_EVENTTYPE_SIGNAL && type != HSA_EVENTTYPE_DEBUG_EVENT);
 }
 
+void CleanupEvent(void)
+{
+	// Release is handled during aperture cleanup.
+	free_exec_aligned_memory_gpu(events_page, KFD_SIGNAL_EVENT_LIMIT * 8);
+	events_page = NULL;
+}
+
 HSAKMT_STATUS
 HSAKMTAPI
 hsaKmtCreateEvent(

projects/rocr-runtime/src/fmm.c

@@ -98,6 +98,7 @@ typedef struct {
 static gpu_mem_t gpu_mem[] = INIT_GPUs_MEM;
 
 static HSAKMT_STATUS dgpu_mem_init(uint8_t node_id, void **base, void **limit);
+static HSAKMT_STATUS dgpu_mem_release(void);
 static int set_dgpu_aperture(uint32_t node_id, uint64_t base, uint64_t limit);
 static void __fmm_release(uint32_t gpu_id, void *address,
 				uint64_t MemorySizeInBytes, manageble_aperture_t *aperture);
@@ -309,7 +310,7 @@ static void *aperture_allocate_area(manageble_aperture_t *app,
 				break;
 
 			/* address space "hole" */
-			if ((VOID_PTRS_SUB(next->start, cur->end) >=
+			if (((VOID_PTRS_SUB(next->start, cur->end) - 1) >=
 							MemorySizeInBytes))
 				break;
 
@@ -317,7 +318,7 @@ static void *aperture_allocate_area(manageble_aperture_t *app,
 		};
 
 		/* If the new range is inside the reserved aperture */
-		if (VOID_PTRS_SUB(app->limit, cur->end) + 1 >=
+		if (VOID_PTRS_SUB(app->limit, cur->end) >=
 				MemorySizeInBytes) {
 			/*
 			 * cur points to the last inspected element: the tail
@@ -571,6 +572,7 @@ static void* __fmm_allocate_device(uint32_t gpu_id, uint64_t MemorySizeInBytes,
 #define GPUVM_APP_OFFSET 0x10000
 void *fmm_allocate_device(uint32_t gpu_id, uint64_t MemorySizeInBytes)
 {
+	MemorySizeInBytes += 0x8000 - (MemorySizeInBytes % 0x8000);
 	manageble_aperture_t *aperture;
 	int32_t gpu_mem_id;
 
@@ -616,6 +618,7 @@ static void* fmm_allocate_host_gpu(uint32_t gpu_id,
 	manageble_aperture_t *aperture;
 	int32_t gpu_mem_id;
 	uint64_t mmap_offset;
+	static size_t total_mem = 0;
 
 	/* Retrieve gpu_mem id according to gpu_id */
 	gpu_mem_id = gpu_mem_find_by_gpu_id(gpu_id);
@@ -637,6 +640,9 @@ static void* fmm_allocate_host_gpu(uint32_t gpu_id,
 		return NULL;
 	}
 
+	total_mem += MemorySizeInBytes;
+	printf("total_mem: %lu\n", total_mem);
+
 	return ret;
 }
 
@@ -756,6 +762,7 @@ void fmm_release(void *address, uint64_t MemorySizeInBytes)
 		if (address >= gpu_mem[i].dgpu_aperture.base &&
 			address <= gpu_mem[i].dgpu_aperture.limit) {
 			found = true;
+			MemorySizeInBytes += 0x8000 - (MemorySizeInBytes % 0x8000);
 			__fmm_release(gpu_mem[i].gpu_id, address,
 					MemorySizeInBytes, &gpu_mem[i].dgpu_aperture);
 			fmm_print(gpu_mem[i].gpu_id);
@@ -817,6 +824,32 @@ HSAKMT_STATUS fmm_init_process_apertures(void)
 	return HSAKMT_STATUS_SUCCESS;
 }
 
+HSAKMT_STATUS fmm_cleanup_process_apertures(void)
+{
+	uint32_t i;
+	struct kfd_ioctl_unmap_memory_from_gpu_args unmap_args;
+	struct kfd_ioctl_free_memory_of_gpu_args free_args;
+
+	for (i = 0 ; i < NUM_OF_SUPPORTED_GPUS; i++) {
+		vm_object_t *cur = gpu_mem[i].dgpu_aperture.vm_objects;
+		pthread_mutex_lock(&gpu_mem[i].dgpu_aperture.fmm_mutex);
+		while (cur) {
+			unmap_args.handle = cur->handle;
+			kmtIoctl(kfd_fd, AMDKFD_IOC_UNMAP_MEMORY_FROM_GPU, &unmap_args);
+			free_args.handle = cur->handle;
+			kmtIoctl(kfd_fd, AMDKFD_IOC_FREE_MEMORY_OF_GPU, &free_args);
+			aperture_release_area(&gpu_mem[i].dgpu_aperture, cur->start, cur->size);
+			vm_object_t* temp = cur;
+			cur = temp->next;
+			vm_remove_object(&gpu_mem[i].dgpu_aperture, temp);
+		};
+		pthread_mutex_unlock(&gpu_mem[i].dgpu_aperture.fmm_mutex);
+	}
+
+	dgpu_mem_release();
+	return HSAKMT_STATUS_SUCCESS;
+}
+
 HSAuint64 fmm_get_aperture_limit(aperture_type_e aperture_type, HSAuint32 gpu_id)
 {
 	int32_t slot = gpu_mem_find_by_gpu_id(gpu_id);
@@ -1060,7 +1093,7 @@ static HSAKMT_STATUS dgpu_mem_init(uint8_t node_id, void **base, void **limit)
 	bool found;
 	HSAKMT_STATUS ret;
 	void *addr, *ret_addr;
-	uint32_t max_len;
+	HSAuint64 max_len;
 	long long unsigned int temp;
 	uint32_t gpu_id;
 	HsaNodeProperties props;
@@ -1077,7 +1110,7 @@ static HSAKMT_STATUS dgpu_mem_init(uint8_t node_id, void **base, void **limit)
 	if (ret != HSAKMT_STATUS_SUCCESS)
 		return ret;
 
-	max_len = (uint32_t)props.LocalMemSize;
+	max_len = props.LocalMemSize;
 	found = false;
 
 	for (addr = (void *)PAGE_SIZE, ret_addr = NULL;
@@ -1110,6 +1143,20 @@ static HSAKMT_STATUS dgpu_mem_init(uint8_t node_id, void **base, void **limit)
 	return HSAKMT_STATUS_ERROR;
 }
 
+static HSAKMT_STATUS dgpu_mem_release(void)
+{
+	size_t max_len;
+	if (is_dgpu_mem_init) {
+		max_len = (size_t)dgpu_shared_aperture_limit -
+			(size_t)dgpu_shared_aperture_base;
+		munmap(dgpu_shared_aperture_base, max_len);
+		is_dgpu_mem_init = false;
+		dgpu_shared_aperture_base = NULL;
+		dgpu_shared_aperture_limit = NULL;
+	}
+	return HSAKMT_STATUS_SUCCESS;
+}
+
 bool fmm_get_handle(void *address, uint64_t *handle)
 {
 	int32_t i;

projects/rocr-runtime/src/fmm.h

@@ -43,7 +43,9 @@ typedef struct {
 	void* start_address;
 } aperture_properties_t;
 
+void CleanupEvent(void);
 HSAKMT_STATUS fmm_init_process_apertures(void);
+HSAKMT_STATUS fmm_cleanup_process_apertures(void);
 /*
  * Memory interface
  */

projects/rocr-runtime/src/openclose.c

@@ -30,6 +30,7 @@
 #include <sys/ioctl.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <stdio.h>
 #include "fmm.h"
 
 static const char kfd_device_name[] = "/dev/kfd";
@@ -90,6 +91,9 @@ hsaKmtCloseKFD(void)
 	{
 		if (--kfd_open_count == 0)
 		{
+			printf("cleanup aperture\n");
+			CleanupEvent();
+			fmm_cleanup_process_apertures();
 			close(kfd_fd);
 
 			if (amd_hsa_thunk_lock_fd > 0) {

projects/rocr-runtime/src/topology.c

@@ -319,8 +319,12 @@ topology_sysfs_get_node_props(uint32_t node_id, HsaNodeProperties *props, uint32
 	while(sscanf(p+=prog, "%s %llu\n%n", prop_name, &prop_val, &prog) == 2) {
 		if (strcmp(prop_name,"cpu_cores_count") == 0)
 			props->NumCPUCores = (uint32_t)prop_val;
-		else if (strcmp(prop_name,"simd_count") == 0)
-			props->NumFComputeCores = (uint32_t)prop_val;
+		else if (strcmp(prop_name,"simd_count") == 0) {
+			if ((uint32_t)prop_val == 0)
+				props->NumFComputeCores = (uint32_t)1;
+			else
+				props->NumFComputeCores = (uint32_t)prop_val;
+		}
 		else if (strcmp(prop_name,"mem_banks_count") == 0)
 			props->NumMemoryBanks = (uint32_t)prop_val;
 		else if (strcmp(prop_name,"caches_count") == 0)
@@ -364,7 +368,7 @@ topology_sysfs_get_node_props(uint32_t node_id, HsaNodeProperties *props, uint32
 		else if (strcmp(prop_name,"max_engine_clk_ccompute") == 0)
 			props->MaxEngineClockMhzCCompute = (uint32_t)prop_val;
 		else if (strcmp(prop_name,"local_mem_size") == 0)
-			props->LocalMemSize = (uint32_t)prop_val;
+			props->LocalMemSize = (HSAuint64)prop_val;
 
 	}