From a8963fcc0bb2a2427de2a7b82042f4e80784fb5f Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 6 Oct 2021 07:10:18 -0700 Subject: [PATCH 1/4] zephyr: Correct heap cache management The heap code was invalidating blocks on allocation, but that's the wrong side of the pipe. By definition, new heap memory will/should/must be written before being used (because the memory is undefined), so any cached contents are irrelevant as they'll be overwritten. But when the user is finished with the block and frees it, there may still be live dirty cache lines in the region on the current CPU. Those must be invalidated, otherwise they will be evicted from the cache at some point in the future, on top of the memory region now being used for different purposes on another CPU. Remove the invalidate on allocation. Add it back in free. Leverage a new Zephyr sys_heap_usable_size() routine to get the size so we don't have to store it in an extra header. Signed-off-by: Andy Ross --- zephyr/wrapper.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/zephyr/wrapper.c b/zephyr/wrapper.c index c630b7c6500b..3a865a113ca0 100644 --- a/zephyr/wrapper.c +++ b/zephyr/wrapper.c @@ -157,12 +157,6 @@ static void *heap_alloc_aligned_cached(struct k_heap *h, size_t min_align, size_ ptr = heap_alloc_aligned(h, align, aligned_size); if (ptr) { ptr = uncache_to_cache(ptr); - - /* - * Heap can be used by different cores, so cache - * needs to be invalidated before next user - */ - z_xtensa_cache_inv(ptr, aligned_size); } return ptr; @@ -175,6 +169,11 @@ static void heap_free(struct k_heap *h, void *mem) { k_spinlock_key_t key = k_spin_lock(&h->lock); +#ifdef ENABLE_CACHED_HEAP + z_xtensa_cache_flush_inv(z_soc_cached_ptr(mem), + sys_heap_usable_size(h, mem)); +#endif + sys_heap_free(&h->heap, mem); k_spin_unlock(&h->lock, key); From a51406ce83320379b451b1ef88d1af28ac915939 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 15 Sep 2021 15:08:41 +0300 Subject: [PATCH 2/4] zephyr: reimplement cached heap zone on single Zephyr sys_heap Implement the cached SOF heap zones using a single sys_heap object. Compared to old implementation which used two separate sys_heaps, this implementation enables dynamic cached/uncached partitioning of the heap. To ensure coherency of the Zephyr heap operation, if cached heap is enabled, all allocations must be aligned to dcache linesize. Add a Kconfig option CONFIG_SOF_ZEPHYR_HEAP_CACHED to turn on/off the cached heap logic, with default set to no for all platforms. Signed-off-by: Kai Vehmanen --- zephyr/Kconfig | 8 +++++ zephyr/wrapper.c | 94 ++++++++++++++++++------------------------------ 2 files changed, 42 insertions(+), 60 deletions(-) diff --git a/zephyr/Kconfig b/zephyr/Kconfig index c3ae6a1d4007..df62bc89e5e8 100644 --- a/zephyr/Kconfig +++ b/zephyr/Kconfig @@ -1,3 +1,11 @@ if SOF rsource "../Kconfig.sof" + +config SOF_ZEPHYR_HEAP_CACHED + bool "Cached Zephyr heap for SOF memory non-shared zones" + default n + help + Enable cached heap by mapping cached SOF memory zones to different + Zephyr sys_heap objects and enable caching for non-shared zones. + endif diff --git a/zephyr/wrapper.c b/zephyr/wrapper.c index 3a865a113ca0..147c6a54be65 100644 --- a/zephyr/wrapper.c +++ b/zephyr/wrapper.c @@ -56,14 +56,9 @@ DECLARE_TR_CTX(zephyr_tr, SOF_UUID(zephyr_uuid), LOG_LEVEL_INFO); /* The Zephyr heap */ -/* use cached heap for non-shared allocations */ -/*#define ENABLE_CACHED_HEAP 1*/ - #ifdef CONFIG_IMX #define HEAPMEM_SIZE (HEAP_SYSTEM_SIZE + HEAP_RUNTIME_SIZE + HEAP_BUFFER_SIZE) -#undef ENABLE_CACHED_HEAP - /* * Include heapmem variable in .heap_mem section, otherwise the HEAPMEM_SIZE is * duplicated in two sections and the sdram0 region overflows. @@ -81,34 +76,22 @@ __section(".heap_mem") static uint8_t __aligned(64) heapmem[HEAPMEM_SIZE]; #if (CONFIG_HP_MEMORY_BANKS < 16) /* e.g. APL */ #if defined __XCC__ -#define HEAP_SIZE 0x28000 +#define HEAPMEM_SIZE 0x28000 #else -#define HEAP_SIZE 0x30000 +#define HEAPMEM_SIZE 0x30000 #endif #elif (CONFIG_HP_MEMORY_BANKS < 30) /* e.g. JSL */ -#define HEAP_SIZE 0x80000 +#define HEAPMEM_SIZE 0x80000 #elif (CONFIG_HP_MEMORY_BANKS < 45) /* e.g. TGL-H */ -#define HEAP_SIZE 0x100000 +#define HEAPMEM_SIZE 0x100000 #else /* e.g. CNL/ICL/TGL */ -#define HEAP_SIZE 0x200000 +#define HEAPMEM_SIZE 0x200000 #endif -#ifdef ENABLE_CACHED_HEAP -/* hard code the cached portion at the moment */ -#define HEAP_SYSTEM_CACHED_SIZE (HEAP_SIZE / 2) -#else -#define HEAP_SYSTEM_CACHED_SIZE 0 -#endif -#define HEAPMEM_SIZE (HEAP_SIZE - HEAP_SYSTEM_CACHED_SIZE) - static uint8_t __aligned(PLATFORM_DCACHE_ALIGN)heapmem[HEAPMEM_SIZE]; -#ifdef ENABLE_CACHED_HEAP -static uint8_t __aligned(PLATFORM_DCACHE_ALIGN)heapmem_cached[HEAP_SYSTEM_CACHED_SIZE]; -static struct k_heap sof_heap_cached; -#endif #endif @@ -119,22 +102,19 @@ static int statics_init(const struct device *unused) ARG_UNUSED(unused); sys_heap_init(&sof_heap.heap, heapmem, HEAPMEM_SIZE); -#ifdef ENABLE_CACHED_HEAP - sys_heap_init(&sof_heap_cached.heap, heapmem_cached, HEAP_SYSTEM_CACHED_SIZE); -#endif + return 0; } SYS_INIT(statics_init, PRE_KERNEL_1, CONFIG_KERNEL_INIT_PRIORITY_OBJECTS); -static void *heap_alloc_aligned(struct k_heap *h, size_t align, size_t bytes) +static void *heap_alloc_aligned(struct k_heap *h, size_t min_align, size_t bytes) { - void *ret = NULL; - - k_spinlock_key_t key = k_spin_lock(&h->lock); - - ret = sys_heap_aligned_alloc(&h->heap, align, bytes); + k_spinlock_key_t key; + void *ret; + key = k_spin_lock(&h->lock); + ret = sys_heap_aligned_alloc(&h->heap, min_align, bytes); k_spin_unlock(&h->lock, key); return ret; @@ -142,9 +122,6 @@ static void *heap_alloc_aligned(struct k_heap *h, size_t align, size_t bytes) static void *heap_alloc_aligned_cached(struct k_heap *h, size_t min_align, size_t bytes) { -#ifdef ENABLE_CACHED_HEAP - unsigned int align = MAX(PLATFORM_DCACHE_ALIGN, min_align); - unsigned int aligned_size = ALIGN_UP(bytes, align); void *ptr; /* @@ -152,26 +129,37 @@ static void *heap_alloc_aligned_cached(struct k_heap *h, size_t min_align, size_ * heap allocation. To ensure no allocated cached buffer * overlaps the same cacheline with the metadata chunk, * align both allocation start and size of allocation - * to cacheline. + * to cacheline. As cached and non-cached allocations are + * mixed, same rules need to be followed for both type of + * allocations. */ - ptr = heap_alloc_aligned(h, align, aligned_size); - if (ptr) { - ptr = uncache_to_cache(ptr); - } +#ifdef CONFIG_SOF_ZEPHYR_HEAP_CACHED + min_align = MAX(PLATFORM_DCACHE_ALIGN, min_align); + bytes = ALIGN_UP(bytes, min_align); +#endif - return ptr; -#else - return heap_alloc_aligned(&sof_heap, min_align, bytes); + ptr = heap_alloc_aligned(h, min_align, bytes); + +#ifdef CONFIG_SOF_ZEPHYR_HEAP_CACHED + if (ptr) + ptr = z_soc_cached_ptr(ptr); #endif + + return ptr; } static void heap_free(struct k_heap *h, void *mem) { k_spinlock_key_t key = k_spin_lock(&h->lock); +#ifdef CONFIG_SOF_ZEPHYR_HEAP_CACHED + void *mem_uncached; -#ifdef ENABLE_CACHED_HEAP - z_xtensa_cache_flush_inv(z_soc_cached_ptr(mem), - sys_heap_usable_size(h, mem)); + if (is_cached(mem)) { + mem_uncached = z_soc_uncached_ptr(mem); + z_xtensa_cache_flush_inv(mem, sys_heap_usable_size(&h->heap, mem_uncached)); + + mem = mem_uncached; + } #endif sys_heap_free(&h->heap, mem); @@ -181,7 +169,7 @@ static void heap_free(struct k_heap *h, void *mem) static inline bool zone_is_cached(enum mem_zone zone) { -#ifndef ENABLE_CACHED_HEAP +#ifndef CONFIG_SOF_ZEPHYR_HEAP_CACHED return false; #endif @@ -196,11 +184,7 @@ void *rmalloc(enum mem_zone zone, uint32_t flags, uint32_t caps, size_t bytes) if (zone_is_cached(zone)) return heap_alloc_aligned_cached(&sof_heap, 0, bytes); -#ifdef ENABLE_CACHED_HEAP - return heap_alloc_aligned(&sof_heap_shared, 8, bytes); -#else return heap_alloc_aligned(&sof_heap, 8, bytes); -#endif } /* Use SOF_MEM_ZONE_BUFFER at the moment */ @@ -274,16 +258,6 @@ void rfree(void *ptr) if (!ptr) return; -#ifdef ENABLE_CACHED_HEAP - /* select heap based on address range */ - if (is_uncached(ptr)) { - heap_free(&sof_heap_shared, ptr); - return; - } - - ptr = cache_to_uncache(ptr); -#endif - heap_free(&sof_heap, ptr); } From cc32696f7551fb8e0b2f365dbb0305566c2d3340 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Fri, 8 Oct 2021 20:03:36 +0300 Subject: [PATCH 3/4] zephyr: enable cached heap by default for Intel cAVS platforms Set default as yes for SOF_ZEPHYR_HEAP_CACHED for Intel cAVS. Signed-off-by: Kai Vehmanen --- zephyr/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/zephyr/Kconfig b/zephyr/Kconfig index df62bc89e5e8..635095a138a0 100644 --- a/zephyr/Kconfig +++ b/zephyr/Kconfig @@ -3,6 +3,7 @@ rsource "../Kconfig.sof" config SOF_ZEPHYR_HEAP_CACHED bool "Cached Zephyr heap for SOF memory non-shared zones" + default y if CAVS default n help Enable cached heap by mapping cached SOF memory zones to different From e01d8c91f1478e4b2471a2bf0d479b48bf8f85fd Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 11 Oct 2021 17:37:08 +0300 Subject: [PATCH 4/4] zephyr: align non-cached allocs to PLATFORM_DCACHE_ALIGN The XTOS allocator aligns non-cachec allocations to the platform cacheline size. Test results indicate some SOF application code relies on this behaviour, so align Zephyr's rmalloc implementation to match the behaviour. Signed-off-by: Kai Vehmanen --- zephyr/wrapper.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/zephyr/wrapper.c b/zephyr/wrapper.c index 147c6a54be65..1ed5f929a269 100644 --- a/zephyr/wrapper.c +++ b/zephyr/wrapper.c @@ -184,7 +184,11 @@ void *rmalloc(enum mem_zone zone, uint32_t flags, uint32_t caps, size_t bytes) if (zone_is_cached(zone)) return heap_alloc_aligned_cached(&sof_heap, 0, bytes); - return heap_alloc_aligned(&sof_heap, 8, bytes); + /* + * XTOS alloc implementation has used dcache alignment, + * so SOF application code is expecting this behaviour. + */ + return heap_alloc_aligned(&sof_heap, PLATFORM_DCACHE_ALIGN, bytes); } /* Use SOF_MEM_ZONE_BUFFER at the moment */