libc/picolibc: Rework malloc arena setup

Picolibc inherited its malloc arena configuration from newlib instead of
from minimal libc. This ended up making it a bit too fragile to run the
full set of zephyr tests. In particular:

 * Z_MALLOC_PARTITION_EXISTS would get set when not used

 * Setting an arena size depended on a bunch of other values, including
   whether the system had an MMU or MPU, and whether the MPU required
   power-of-two alignment or not.

This patch cleans things up so that there is a single heap size specifier,
PICOLIBC_HEAP_SIZE.

 * If PICOLIBC_HEAP_SIZE is positive, this sets the size of the heap. On
   MMU systems, picolibc will only use the remaining memory if that's
   smaller.

 * If PICOLIBC_HEAP_SIZE is zero, then there is no heap available and
   malloc will always fail. This also disables Z_MALLOC_PARTITION_EXISTS.

 * If PICOLIBC_HEAP_SIZE is negative, then picolibc uses all remaining
   memory for the malloc heap.

The defaults are designed to allow tests to work without requiring
additional settings.

 * For MMU enabled systems, the default value is 1048576. It would be nice
   to have this use 'all available memory', but that's difficult to manage
   as the API which returns free memory (k_mem_free_get) doesn't take into
   account the amount of free virtual address space.

 * For MPU enabled systems which require power-of-two aligned MPU regions,
   the default value is 64kB.

 * For other systems, the default value is -1, indicating that all
   available memory be used for the malloc arena.

Signed-off-by: Keith Packard <[email protected]>

Author: keith-packard

include/zephyr/sys/libc-hooks.h

@@ -57,20 +57,17 @@ extern struct k_mem_partition z_malloc_partition;
  */
 #define Z_MALLOC_PARTITION_EXISTS 1
 #endif /* CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE > 0 */
+
 #elif defined(CONFIG_PICOLIBC)
-/* If we are using picolibc, the heap arena is in one of two areas:
- *  - If we have an MPU that requires power of two alignment, the heap bounds
- *    must be specified in Kconfig via CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE.
- *  - Otherwise, the heap arena on most arches starts at a suitably
- *    aligned base addreess after the `_end` linker symbol, through to the end
- *    of system RAM.
+/*
+ * When using picolibc, we need z_malloc_partition whenever
+ * the heap size is not zero and there is an mpu or an mmu
  */
-#if (!defined(CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT) || \
-(defined(CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT) && \
-CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE))
+#if CONFIG_PICOLIBC_HEAP_SIZE != 0 && \
+(defined(CONFIG_MPU) || defined(CONFIG_MMU))
 #define Z_MALLOC_PARTITION_EXISTS 1
-extern struct k_mem_partition z_malloc_partition;
 #endif
+
 #endif /* CONFIG_PICOLIBC */
 
 #ifdef Z_MALLOC_PARTITION_EXISTS

lib/libc/picolibc/Kconfig

@@ -8,29 +8,24 @@ config PICOLIBC_USE_MODULE
 	help
 	  Use picolibc module instead of picolibc included with toolchain
 
-config PICOLIBC_ALIGNED_HEAP_SIZE
-	int "Picolibc aligned heap size (bytes)"
-	depends on MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT
-	depends on USERSPACE
-	default 0
+config PICOLIBC_HEAP_SIZE
+	int "Picolibc heap size (bytes)"
+	default 1048576 if MMU
+	default 65536 if USERSPACE && MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT
+	default -1
 	help
+	  Indicates the amount of memory which will be used by the picolibc
+	  malloc() heap.
+
 	  If user mode is enabled, and MPU hardware has requirements that
 	  regions be sized to a power of two and aligned to their size,
-	  and user mode threads need to access this heap, then this is necessary
-	  to properly define an MPU region for the heap.
+	  then this must be defined as a power of two or a compile error
+	  will result.
 
-	  If this is left at 0, then remaining system RAM will be used for this
-	  area and it may not be possible to program it as an MPU region.
+	  If set to zero, then no malloc() heap will be available.
 
-config PICOLIBC_LIBC_MAX_MAPPED_REGION_SIZE
-	int "Maximum memory mapped for picolibc heap"
-	depends on MMU
-	default 1048576
-	help
-	  On MMU-based systems, indicates the maximum amount of memory which
-	  will be used for the picolibc malloc() heap. The actual amount of
-	  memory used will be the minimum of this value and the amount of
-	  free physical memory at kernel boot.
+	  If set to -1, then all remaining system RAM will be used for this
+	  area.
 
 config PICOLIBC_IO_LONG_LONG
 	bool "support for long long in integer-only printf/scanf"

lib/libc/picolibc/libc-hooks.c

@@ -27,77 +27,138 @@
 #define LIBC_DATA	K_APP_DMEM(z_libc_partition)
 
 #ifdef CONFIG_MMU
-#ifdef CONFIG_USERSPACE
+
+/* When there is an MMU, allocate the heap at startup time */
+
+# if Z_MALLOC_PARTITION_EXISTS
 struct k_mem_partition z_malloc_partition;
-#endif
+# endif
 
 LIBC_BSS static unsigned char *heap_base;
 LIBC_BSS static size_t max_heap_size;
 
-#define HEAP_BASE		((uintptr_t) heap_base)
-#define MAX_HEAP_SIZE		max_heap_size
-#define USE_MALLOC_PREPARE	1
+# define HEAP_BASE		((uintptr_t) heap_base)
+# define MAX_HEAP_SIZE		max_heap_size
+
+# define USE_MALLOC_PREPARE	1
+
+#elif CONFIG_PICOLIBC_HEAP_SIZE == 0
+
+/* No heap at all */
+# define HEAP_BASE	0
+# define MAX_HEAP_SIZE	0
+
+#else /* CONFIG_PICOLIBC_HEAP_SIZE != 0 */
+
+/* Figure out alignment requirement */
+# ifdef Z_MALLOC_PARTITION_EXISTS
+
+#  if defined(CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT)
+#   if CONFIG_PICOLIBC_HEAP_SIZE < 0
+#    error CONFIG_PICOLIBC_HEAP_SIZE must be defined on this target
+#   endif
+#   if (CONFIG_PICOLIBC_HEAP_SIZE & (CONFIG_PICOLIBC_HEAP_SIZE - 1)) != 0
+#    error CONFIG_PICOLIBC_HEAP_SIZE must be power of two on this target
+#   endif
+#   define HEAP_ALIGN	CONFIG_PICOLIBC_HEAP_SIZE
+#  elif defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+#   define HEAP_ALIGN	CONFIG_ARM_MPU_REGION_MIN_ALIGN_AND_SIZE
+#  elif defined(CONFIG_ARC)
+#   define HEAP_ALIGN	Z_ARC_MPU_ALIGN
+#  elif defined(CONFIG_RISCV)
+#   define HEAP_ALIGN	Z_RISCV_STACK_GUARD_SIZE
+#  else
+/*
+ * Default to 64-bytes; we'll get a run-time error if this doesn't work.
+ */
+#   define HEAP_ALIGN	64
+#  endif /* CONFIG_<arch> */
+
+# else /* Z_MALLOC_PARTITION_EXISTS */
+
+#  define HEAP_ALIGN	sizeof(double)
 
-#elif CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE
+# endif /* else Z_MALLOC_PARTITION_EXISTS */
+
+# if CONFIG_PICOLIBC_HEAP_SIZE > 0
+
+/* Static allocation of heap in BSS */
+
+#  ifdef Z_MALLOC_PARTITION_EXISTS
 K_APPMEM_PARTITION_DEFINE(z_malloc_partition);
-#define MALLOC_BSS	K_APP_BMEM(z_malloc_partition)
+#   define MALLOC_BSS	K_APP_BMEM(z_malloc_partition)
+#  else
+#   define MALLOC_BSS
+#  endif
+
+MALLOC_BSS static unsigned char __aligned(HEAP_ALIGN)
+	heap_base[CONFIG_PICOLIBC_HEAP_SIZE];
 
-/* Compiler will throw an error if the provided value isn't a power of two */
-MALLOC_BSS static unsigned char __aligned(CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE)
-	heap_base[CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE];
+#  define HEAP_BASE	((uintptr_t) heap_base)
+#  define MAX_HEAP_SIZE	CONFIG_PICOLIBC_HEAP_SIZE
 
-#define HEAP_BASE	((uintptr_t) heap_base)
-#define MAX_HEAP_SIZE	CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE
+# else /* CONFIG_PICOLIBC_HEAP_SIZE > 0 */
 
-#else /* Not MMU or CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE */
-/* Heap base and size are determined based on the available unused SRAM,
- * in the interval from a properly aligned address after the linker symbol
- * `_end`, to the end of SRAM
+/*
+ * Heap base and size are determined based on the available unused SRAM, in the
+ * interval from a properly aligned address after the linker symbol `_end`, to
+ * the end of SRAM
  */
-#define USED_RAM_END_ADDR   POINTER_TO_UINT(&_end)
 
-#ifdef Z_MALLOC_PARTITION_EXISTS
-/* Need to be able to program a memory protection region from HEAP_BASE
- * to the end of RAM so that user threads can get at it.
- * Implies that the base address needs to be suitably aligned since the
- * bounds have to go in a k_mem_partition.
+#  ifdef Z_MALLOC_PARTITION_EXISTS
+/*
+ * Need to be able to program a memory protection region from HEAP_BASE to the
+ * end of RAM so that user threads can get at it.  Implies that the base address
+ * needs to be suitably aligned since the bounds have to go in a
+ * k_mem_partition.
  */
 struct k_mem_partition z_malloc_partition;
-/* TODO: Need a generic Kconfig for the MPU region granularity */
-#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
-#define HEAP_BASE	ROUND_UP(USED_RAM_END_ADDR, \
-				 CONFIG_ARM_MPU_REGION_MIN_ALIGN_AND_SIZE)
-#elif defined(CONFIG_ARC)
-#define HEAP_BASE	ROUND_UP(USED_RAM_END_ADDR, Z_ARC_MPU_ALIGN)
-#elif defined(CONFIG_RISCV)
-#define HEAP_BASE	ROUND_UP(USED_RAM_END_ADDR, Z_RISCV_STACK_GUARD_SIZE)
-#else
-#error "Unsupported platform"
-#endif /* CONFIG_<arch> */
-#else /* !Z_MALLOC_PARTITION_EXISTS */
-/* No partition, heap can just start wherever _end is */
-#define HEAP_BASE	USED_RAM_END_ADDR
-#endif /* Z_MALLOC_PARTITION_EXISTS */
-
-#ifdef CONFIG_XTENSA
+
+#   define USE_MALLOC_PREPARE	1
+
+#  endif /* Z_MALLOC_PARTITION_EXISTS */
+
+#  define USED_RAM_END_ADDR   POINTER_TO_UINT(&_end)
+
+/*
+ * No partition, heap can just start wherever _end is, with
+ * suitable alignment
+ */
+
+#  define HEAP_BASE	ROUND_UP(USED_RAM_END_ADDR, HEAP_ALIGN)
+
+#  ifdef CONFIG_XTENSA
 extern char _heap_sentry[];
-#define MAX_HEAP_SIZE  (POINTER_TO_UINT(_heap_sentry) - HEAP_BASE)
-#else
-#define MAX_HEAP_SIZE	(KB(CONFIG_SRAM_SIZE) - \
+#   define MAX_HEAP_SIZE  (POINTER_TO_UINT(_heap_sentry) - HEAP_BASE)
+#  else
+#   define MAX_HEAP_SIZE	(KB(CONFIG_SRAM_SIZE) - \
 			 (HEAP_BASE - CONFIG_SRAM_BASE_ADDRESS))
-#endif
+#  endif /* CONFIG_XTENSA */
 
-#endif /* CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE */
+# endif /* CONFIG_PICOLIBC_HEAP_SIZE < 0 */
 
+#endif /* CONFIG_PICOLIBC_HEAP_SIZE == 0 */
+
+#ifdef USE_MALLOC_PREPARE
 
 static int malloc_prepare(const struct device *unused)
 {
 	ARG_UNUSED(unused);
 
 #ifdef CONFIG_MMU
-	max_heap_size = MIN(CONFIG_PICOLIBC_LIBC_MAX_MAPPED_REGION_SIZE,
+
+	/* With an MMU, the heap is allocated at runtime */
+
+# if CONFIG_PICOLIBC_HEAP_SIZE < 0
+#  define MMU_MAX_HEAP_SIZE PTRDIFF_MAX
+# else
+#  define MMU_MAX_HEAP_SIZE CONFIG_PICOLIBC_HEAP_SIZE
+# endif
+	max_heap_size = MIN(MMU_MAX_HEAP_SIZE,
 			    k_mem_free_get());
 
+	max_heap_size &= ~(CONFIG_MMU_PAGE_SIZE-1);
+
 	if (max_heap_size != 0) {
 		heap_base = k_mem_map(max_heap_size, K_MEM_PERM_RW);
 		__ASSERT(heap_base != NULL,
@@ -116,6 +177,8 @@ static int malloc_prepare(const struct device *unused)
 
 SYS_INIT(malloc_prepare, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
 
+#endif /* USE_MALLOC_PREPARE */
+
 LIBC_BSS static uintptr_t heap_sz;
 
 static int (*_stdout_hook)(int);