mem: use single cross-core virtual heap

In case of cross core buffers a buffer may be allocated
and freed by different core, depending on which
component is deleted second.
As all control structures of virtual heaps are
stored in uncached aliases, there's no technical
problems with allowing virtual heaps to work cross
core.
The only consideration is that in case of cross core
allocate/free the cache invalidation MUST be performed
on the core that was storing data. It is up to the
memory user to ensure this

Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>

Author: marcinszkudlinski

zephyr/include/sof/lib/regions_mm.h

@@ -77,16 +77,12 @@ struct vmh_heap_config {
 	struct vmh_block_bundle_descriptor block_bundles_table[MAX_MEMORY_ALLOCATORS_COUNT];
 };
 
-struct vmh_heap *vmh_init_heap(const struct vmh_heap_config *cfg,
-		int memory_region_attribute, int core_id, bool allocating_continuously);
+struct vmh_heap *vmh_init_heap(const struct vmh_heap_config *cfg, bool allocating_continuously);
 void *vmh_alloc(struct vmh_heap *heap, uint32_t alloc_size);
 int vmh_free_heap(struct vmh_heap *heap);
 int vmh_free(struct vmh_heap *heap, void *ptr);
-struct vmh_heap *vmh_reconfigure_heap(struct vmh_heap *heap,
-		struct vmh_heap_config *cfg, int core_id, bool allocating_continuously);
 void vmh_get_default_heap_config(const struct sys_mm_drv_region *region,
 		struct vmh_heap_config *cfg);
-struct vmh_heap *vmh_get_heap_by_attribute(uint32_t attr, uint32_t core_id);
 /**
  * @brief Checks if ptr is in range of given memory range
  *

zephyr/lib/alloc.c

@@ -22,7 +22,7 @@
 #include <sof/lib/regions_mm.h>
 
 struct vmh_heap;
-struct vmh_heap *virtual_buffers_heap[CONFIG_MP_MAX_NUM_CPUS];
+struct vmh_heap *virtual_buffers_heap;
 struct k_spinlock vmh_lock;
 
 #undef	HEAPMEM_SIZE
@@ -252,12 +252,11 @@ static bool is_virtual_heap_pointer(void *ptr)
 
 static void virtual_heap_free(void *ptr)
 {
-	struct vmh_heap *const heap = virtual_buffers_heap[cpu_get_id()];
 	int ret;
 
 	ptr = (__sparse_force void *)sys_cache_cached_ptr_get(ptr);
 
-	ret = vmh_free(heap, ptr);
+	ret = vmh_free(virtual_buffers_heap, ptr);
 	if (ret) {
 		tr_err(&zephyr_tr, "Unable to free %p! %d", ptr, ret);
 		k_panic();
@@ -280,17 +279,12 @@ static const struct vmh_heap_config static_hp_buffers = {
 
 static int virtual_heap_init(void)
 {
-	int core;
-
 	k_spinlock_init(&vmh_lock);
 
-	for (core = 0; core < CONFIG_MP_MAX_NUM_CPUS; core++) {
-		struct vmh_heap *heap = vmh_init_heap(&static_hp_buffers, MEM_REG_ATTR_CORE_HEAP,
-						      core, false);
-		if (!heap)
-			tr_err(&zephyr_tr, "Unable to init virtual heap for core %d!", core);
-
-		virtual_buffers_heap[core] = heap;
+	virtual_buffers_heap = vmh_init_heap(&static_hp_buffers, false);
+	if (!virtual_buffers_heap) {
+		tr_err(&zephyr_tr, "Unable to init virtual heap");
+		return -ENOMEM;
 	}
 
 	return 0;
@@ -491,9 +485,6 @@ EXPORT_SYMBOL(rzalloc);
 void *rballoc_align(uint32_t flags, uint32_t caps, size_t bytes,
 		    uint32_t align)
 {
-#if CONFIG_VIRTUAL_HEAP
-	struct vmh_heap *virtual_heap;
-#endif
 	struct k_heap *heap;
 
 	/* choose a heap */
@@ -511,9 +502,8 @@ void *rballoc_align(uint32_t flags, uint32_t caps, size_t bytes,
 
 #if CONFIG_VIRTUAL_HEAP
 	/* Use virtual heap if it is available */
-	virtual_heap = virtual_buffers_heap[cpu_get_id()];
-	if (virtual_heap)
-		return virtual_heap_alloc(virtual_heap, flags, caps, bytes, align);
+	if (virtual_buffers_heap)
+		return virtual_heap_alloc(virtual_buffers_heap, flags, caps, bytes, align);
 #endif /* CONFIG_VIRTUAL_HEAP */
 
 	if (flags & SOF_MEM_FLAG_COHERENT)

zephyr/lib/regions_mm.c

@@ -10,33 +10,27 @@
 #if defined(CONFIG_MM_DRV)
 #include <sof/lib/regions_mm.h>
 
-/* list of vmh_heap objects created */
-static struct list_item vmh_list = LIST_INIT(vmh_list);
 
 /** @struct vmh_heap
  *
  *  @brief This structure holds all information about virtual memory heap
  *  it aggregates information about its allocations and
  *  physical mappings.
  *
- *  @var node generic list member used for list operations
  *  @var virtual_region pointer to virtual region information, it holds its
  *  attributes size and beginning ptr provided by zephyr.
  *  @var physical_blocks_allocators[] a table of block allocators
  *  each representing a virtual regions part in blocks of a given size
  *  governed by sys_mem_blocks API.
  *  @var allocation_sizes[] a table of bit arrays representing sizes of allocations
  *  made in physical_blocks_allocators directly related to physical_blocks_allocators
- *  @var core_id id of the core that heap was created on
  *  @var allocating_continuously configuration value deciding if heap allocations
  *  will be contiguous or single block.
  */
 struct vmh_heap {
-	struct list_item node;
 	const struct sys_mm_drv_region *virtual_region;
 	struct sys_mem_blocks *physical_blocks_allocators[MAX_MEMORY_ALLOCATORS_COUNT];
 	struct sys_bitarray *allocation_sizes[MAX_MEMORY_ALLOCATORS_COUNT];
-	int core_id;
 	bool allocating_continuously;
 };
 
@@ -47,49 +41,32 @@ struct vmh_heap {
  * and config. The heap size overall must be aligned to physical page
  * size.
  * @param cfg Configuration structure with block structure for the heap
- * @param memory_region_attribute A zephyr defined virtual region identifier
- * @param core_id Core id of a heap that will be created
  * @param allocating_continuously Flag deciding if heap can do contiguous allocation.
  *
  * @retval ptr to a new heap if successful.
  * @retval NULL on creation failure.
  */
-struct vmh_heap *vmh_init_heap(const struct vmh_heap_config *cfg,
-	int memory_region_attribute, int core_id, bool allocating_continuously)
+struct vmh_heap *vmh_init_heap(const struct vmh_heap_config *cfg, bool allocating_continuously)
 {
 	const struct sys_mm_drv_region *virtual_memory_regions =
 		sys_mm_drv_query_memory_regions();
 	int i;
 
-	/* Check if we haven't created heap for this region already */
-	if (vmh_get_heap_by_attribute(memory_region_attribute, core_id))
-		return NULL;
-
 	struct vmh_heap *new_heap =
 		rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*new_heap));
 
 	if (!new_heap)
 		return NULL;
 
-	new_heap->core_id = core_id;
-	list_init(&new_heap->node);
 	struct vmh_heap_config new_config = {0};
 
-	/* Search for matching attribute so we place heap on right
-	 * virtual region.
-	 * In case of core heaps regions we count theme from 0 to max
-	 * available cores
-	 */
-	if (memory_region_attribute == MEM_REG_ATTR_CORE_HEAP) {
-		new_heap->virtual_region = &virtual_memory_regions[core_id];
-	} else {
-		for (i = CONFIG_MP_MAX_NUM_CPUS;
-			i < CONFIG_MP_MAX_NUM_CPUS + VIRTUAL_REGION_COUNT; i++) {
+	/* Search for matching attribute so we place heap on shared virtual region */
+	for (i = CONFIG_MP_MAX_NUM_CPUS;
+		i < CONFIG_MP_MAX_NUM_CPUS + VIRTUAL_REGION_COUNT; i++) {
 
-			if (memory_region_attribute == virtual_memory_regions[i].attr) {
-				new_heap->virtual_region = &virtual_memory_regions[i];
-				break;
-			}
+		if (virtual_memory_regions[i].attr == MEM_REG_ATTR_SHARED_HEAP) {
+			new_heap->virtual_region = &virtual_memory_regions[i];
+			break;
 		}
 	}
 
@@ -220,9 +197,6 @@ struct vmh_heap *vmh_init_heap(const struct vmh_heap_config *cfg,
 
 	new_heap->allocating_continuously = allocating_continuously;
 
-	/* Save the new heap pointer to a linked list */
-	list_item_append(&vmh_list, &new_heap->node);
-
 	return new_heap;
 fail:
 	for (i = 0; i < MAX_MEMORY_ALLOCATORS_COUNT; i++) {
@@ -419,9 +393,6 @@ void *vmh_alloc(struct vmh_heap *heap, uint32_t alloc_size)
 {
 	if (!alloc_size)
 		return NULL;
-	/* Only operations on the same core are allowed */
-	if (heap->core_id != cpu_get_id())
-		return NULL;
 
 	void *ptr = NULL;
 	int mem_block_iterator, allocation_error_code = -ENOMEM;
@@ -566,7 +537,6 @@ int vmh_free_heap(struct vmh_heap *heap)
 			rfree(heap->allocation_sizes[i]);
 		}
 	}
-	list_item_del(&heap->node);
 	rfree(heap);
 	return 0;
 }
@@ -587,9 +557,6 @@ int vmh_free(struct vmh_heap *heap, void *ptr)
 {
 	int retval;
 
-	if (heap->core_id != cpu_get_id())
-		return -EINVAL;
-
 	size_t mem_block_iter, i, size_to_free, block_size, ptr_bit_array_offset,
 		ptr_bit_array_position, blocks_to_free;
 	bool ptr_range_found;
@@ -703,32 +670,6 @@ int vmh_free(struct vmh_heap *heap, void *ptr)
 				size_to_free);
 }
 
-/**
- * @brief Reconfigure heap with given config
- *
- * This will destroy the heap from the pointer and recreate it
- * using provided config. Individual region attribute is the
- * "anchor" to the virtual space we use.
- *
- * @param heap Ptr to the heap that should be reconfigured
- * @param cfg heap blocks configuration
- * @param allocating_continuously allow contiguous on the reconfigured heap
- *
- * @retval heap_pointer Returns new heap pointer on success
- * @retval NULL when reconfiguration failed
- */
-struct vmh_heap *vmh_reconfigure_heap(
-	struct vmh_heap *heap, struct vmh_heap_config *cfg,
-	int core_id, bool allocating_continuously)
-{
-	uint32_t region_attribute = heap->virtual_region->attr;
-
-	if (vmh_free_heap(heap))
-		return NULL;
-
-	return vmh_init_heap(cfg, region_attribute, core_id, allocating_continuously);
-}
-
 /**
  * @brief Get default configuration for heap
  *
@@ -757,47 +698,4 @@ void vmh_get_default_heap_config(const struct sys_mm_drv_region *region,
 	}
 }
 
-/**
- * @brief Gets pointer to already created heap identified by
- * provided attribute.
- *
- * @param attr Virtual memory region attribute.
- * @param core_id id of the core that heap was created for (core heap specific).
- *
- * @retval heap ptr on success
- * @retval NULL if there was no heap created fitting the attr.
- */
-struct vmh_heap *vmh_get_heap_by_attribute(uint32_t attr, uint32_t core_id)
-{
-	struct list_item *vm_heaps_iterator;
-	struct vmh_heap *retval;
-
-	if (attr == MEM_REG_ATTR_CORE_HEAP) {
-		/* if we look up cpu heap we need to match its cpuid with addr
-		 * we know that regions keep cpu heaps from 0 to max so we match
-		 * the region of the cpu id with lists one to find match
-		 */
-		const struct sys_mm_drv_region *virtual_memory_region =
-			sys_mm_drv_query_memory_regions();
-		/* we move ptr to cpu vmr */
-		virtual_memory_region = &virtual_memory_region[core_id];
-
-		list_for_item(vm_heaps_iterator, &vmh_list) {
-			retval =
-			    CONTAINER_OF(vm_heaps_iterator, struct vmh_heap, node);
-
-			if (retval->virtual_region->addr == virtual_memory_region->addr)
-				return retval;
-		}
-	} else {
-		list_for_item(vm_heaps_iterator, &vmh_list) {
-			retval =
-			    CONTAINER_OF(vm_heaps_iterator, struct vmh_heap, node);
-			if (retval->virtual_region->attr == attr)
-				return retval;
-		}
-	}
-	return NULL;
-}
-
 #endif /* if defined (CONFIG_MM_DRV) */

zephyr/test/vmh.c

@@ -20,14 +20,11 @@ LOG_MODULE_DECLARE(sof_boot_test, CONFIG_SOF_LOG_LEVEL);
 struct vmh_heap;
 
 /* Test creating and freeing a virtual memory heap */
-static void test_vmh_init_and_free_heap(int memory_region_attribute,
-					struct vmh_heap_config *config,
-					int core_id,
+static void test_vmh_init_and_free_heap(struct vmh_heap_config *config,
 					bool allocating_continuously,
 					bool expect_success)
 {
-	struct vmh_heap *heap = vmh_init_heap(config, memory_region_attribute,
-		core_id, allocating_continuously);
+	struct vmh_heap *heap = vmh_init_heap(config, allocating_continuously);
 	if (expect_success) {
 		zassert_not_null(heap,
 		"Heap initialization expected to succeed but failed");
@@ -147,7 +144,7 @@ static void test_vmh_multiple_allocs(struct vmh_heap *heap, int num_allocs,
 static void test_vmh_alloc_multiple_times(bool allocating_continuously)
 {
 	struct vmh_heap *heap =
-	vmh_init_heap(NULL, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);
+	vmh_init_heap(NULL, allocating_continuously);
 
 	zassert_not_null(heap, "Heap initialization failed");
 
@@ -171,7 +168,7 @@ static void test_vmh_alloc_multiple_times(bool allocating_continuously)
 static void test_vmh_alloc_free(bool allocating_continuously)
 {
 	struct vmh_heap *heap =
-	vmh_init_heap(NULL, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);
+	vmh_init_heap(NULL, allocating_continuously);
 
 	zassert_not_null(heap, "Heap initialization failed");
 
@@ -195,7 +192,7 @@ static void test_vmh_alloc_free(bool allocating_continuously)
 /* Test case for vmh_alloc and vmh_free with and without config */
 static void test_heap_creation(void)
 {
-	test_vmh_init_and_free_heap(MEM_REG_ATTR_CORE_HEAP, NULL, 0, false, true);
+	test_vmh_init_and_free_heap(NULL, 0, false, true);
 
 	/* Try to setup with pre defined heap config */
 	struct vmh_heap_config config = {0};
@@ -208,7 +205,7 @@ static void test_heap_creation(void)
 
 	config.block_bundles_table[1].number_of_blocks = 512;
 
-	test_vmh_init_and_free_heap(MEM_REG_ATTR_CORE_HEAP, &config, 0, false, true);
+	test_vmh_init_and_free_heap(&config, 0, false, true);
 }
 
 /* Test case for alloc/free on configured heap */
@@ -224,7 +221,7 @@ static void test_alloc_on_configured_heap(bool allocating_continuously)
 
 	/* Create continuous allocation heap for success test */
 	struct vmh_heap *heap =
-	vmh_init_heap(&config, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);
+	vmh_init_heap(&config, allocating_continuously);
 
 	/* Will succeed on continuous and fail with single block alloc */
 	test_vmh_alloc_free_check(heap, 512, allocating_continuously);
@@ -234,42 +231,9 @@ static void test_alloc_on_configured_heap(bool allocating_continuously)
 	zassert_equal(ret, 0, "Failed to free heap");
 }
 
-/* Test cases for initializing heaps on all available regions */
-static void test_vmh_init_all_heaps(void)
-{
-	int num_regions = CONFIG_MP_MAX_NUM_CPUS + VIRTUAL_REGION_COUNT;
-	int i;
-	const struct sys_mm_drv_region *virtual_memory_region =
-	sys_mm_drv_query_memory_regions();
-
-	/* Test initializing all types of heaps */
-	for (i = 0; i < num_regions; i++) {
-
-		/* Zeroed size symbolizes end of regions table */
-		if (!virtual_memory_region[i].size)
-			break;
-
-		struct vmh_heap *heap = vmh_init_heap(NULL, virtual_memory_region[i].attr,
-		i, true);
-
-		zassert_not_null(heap, "Heap initialization expected to succeed but failed");
-
-		/* Test if it fails when heap already exists */
-		test_vmh_init_and_free_heap(virtual_memory_region[i].attr, NULL, i, true,
-		false);
-
-		if (heap) {
-			int ret = vmh_free_heap(heap);
-
-			zassert_equal(ret, 0, "Failed to free heap");
-		}
-	}
-}
-
 ZTEST(sof_boot, virtual_memory_heap)
 {
 	test_heap_creation();
-	test_vmh_init_all_heaps();
 	test_alloc_on_configured_heap(true);
 	test_alloc_on_configured_heap(false);
 	test_vmh_alloc_free(true);