[coreboot-gerrit] New patch to review for coreboot: f0b9c50 libpayload: dma_malloc: Prevent warm reboot problems and add debugging

Isaac Christensen (isaac.christensen@se-eng.com) gerrit at coreboot.org
Wed Aug 13 20:20:58 CEST 2014


Isaac Christensen (isaac.christensen at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6645

-gerrit

commit f0b9c50b34544c9c591000e8353df02608dff3aa
Author: Julius Werner <jwerner at chromium.org>
Date:   Fri Sep 13 18:21:46 2013 -0700

    libpayload: dma_malloc: Prevent warm reboot problems and add debugging
    
    Since the DMA memory is allocated by Coreboot (outside of the payload's
    linker script), it won't get zeroed upon loading like the heap.
    Therefore, a warm reboot that doesn't reset memory may leave stale
    malloc cookies lying around and misinterpret them as memory that is
    still in use on the next boot. After several boots this may fill up the
    whole DMA memory and lead to OOM conditions.
    
    Therefore, this patch explicitly wipes the first cookie in
    init_dma_memory() to prevent that from happening. It also expands the
    existing memory allocator debugging code to cover the DMA parts, which
    was very helpful in identifying this particular problem.
    
    Change-Id: I6e2083c286ff8ec865b22dd922c39c456944b451
    Signed-off-by: Julius Werner <jwerner at chromium.org>
    Reviewed-on: https://chromium-review.googlesource.com/169455
    Reviewed-by: Stefan Reinauer <reinauer at google.com>
    (cherry picked from commit 8e5e1784638563b865553125cd5dab1d36a5d2cb)
    Signed-off-by: Isaac Christensen <isaac.christensen at se-eng.com>
---
 payloads/libpayload/include/stdlib.h | 34 ++++++++++++--
 payloads/libpayload/libc/malloc.c    | 89 ++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 37 deletions(-)

diff --git a/payloads/libpayload/include/stdlib.h b/payloads/libpayload/include/stdlib.h
index d3448b9..1ed92d5 100644
--- a/payloads/libpayload/include/stdlib.h
+++ b/payloads/libpayload/include/stdlib.h
@@ -101,7 +101,35 @@
 	 printf("PRE memalign\n"); \
 	 print_malloc_map(); \
 	 ptr = memalign(a,s); \
-	 printf("POST realloc (ptr = %p)\n", ptr); \
+	 printf("POST memalign (ptr = %p)\n", ptr); \
+	 print_malloc_map(); \
+	 ptr; \
+	 })
+#define dma_malloc(s) \
+	({ \
+	 extern void print_malloc_map(void); \
+	 extern void *dma_malloc(size_t); \
+	 void *ptr; \
+	 printf("dma_malloc(%u) called from %s:%s:%d...\n", s, __FILE__, \
+	        __func__, __LINE__);\
+	 printf("PRE dma_malloc\n"); \
+	 print_malloc_map(); \
+	 ptr = dma_malloc(s); \
+	 printf("POST dma_malloc (ptr = %p)\n", ptr); \
+	 print_malloc_map(); \
+	 ptr; \
+	 })
+#define dma_memalign(a,s) \
+	({ \
+	 extern void print_malloc_map(void); \
+	 extern void *dma_memalign(size_t, size_t); \
+	 void *ptr; \
+	 printf("dma_memalign(%u, %u) called from %s:%s:%d...\n", a, s, \
+	        __FILE__, __func__, __LINE__);\
+	 printf("PRE dma_memalign\n"); \
+	 print_malloc_map(); \
+	 ptr = dma_memalign(a,s); \
+	 printf("POST dma_memalign (ptr = %p)\n", ptr); \
 	 print_malloc_map(); \
 	 ptr; \
 	 })
@@ -111,10 +139,10 @@ void *malloc(size_t size);
 void *calloc(size_t nmemb, size_t size);
 void *realloc(void *ptr, size_t size);
 void *memalign(size_t align, size_t size);
-#endif
-void init_dma_memory(void *start, u32 size);
 void *dma_malloc(size_t size);
 void *dma_memalign(size_t align, size_t size);
+#endif
+void init_dma_memory(void *start, u32 size);
 int dma_initialized(void);
 int dma_coherent(void *ptr);
 /** @} */
diff --git a/payloads/libpayload/libc/malloc.c b/payloads/libpayload/libc/malloc.c
index 11dfeef..0f2845d 100644
--- a/payloads/libpayload/libc/malloc.c
+++ b/payloads/libpayload/libc/malloc.c
@@ -46,11 +46,21 @@ struct memory_type {
 	void *start;
 	void *end;
 	struct align_region_t* align_regions;
+#ifdef CONFIG_LP_DEBUG_MALLOC
+	int magic_initialized;
+	size_t minimal_free;
+	const char *name;
+#endif
 };
 
 extern char _heap, _eheap;	/* Defined in the ldscript. */
 
-static struct memory_type default_type = { (void *)&_heap, (void *)&_eheap, NULL };
+static struct memory_type default_type =
+	{ (void *)&_heap, (void *)&_eheap, NULL
+#ifdef CONFIG_LP_DEBUG_MALLOC
+	, 0, 0, "HEAP"
+#endif
+	};
 static struct memory_type *const heap = &default_type;
 static struct memory_type *dma = &default_type;
 
@@ -75,26 +85,31 @@ typedef u64 hdrtype_t;
 static int free_aligned(void* addr, struct memory_type *type);
 void print_malloc_map(void);
 
-#ifdef CONFIG_LP_DEBUG_MALLOC
-static int heap_initialized = 0;
-static int minimal_free = 0;
-#endif
-
 void init_dma_memory(void *start, u32 size)
 {
-#ifdef CONFIG_LP_DEBUG_MALLOC
 	if (dma_initialized()) {
-		printf("WARNING: %s called twice!\n");
+		printf("ERROR: %s called twice!\n", __func__);
 		return;
 	}
 
-	printf("Initializing cache-coherent DMA memory at [%p:%p]\n", start, start + size);
-#endif
+	/*
+	 * DMA memory might not be zeroed by Coreboot on stage loading, so make
+	 * sure we clear the magic cookie from last boot.
+	 */
+	*(hdrtype_t *)start = 0;
 
 	dma = malloc(sizeof(*dma));
 	dma->start = start;
 	dma->end = start + size;
 	dma->align_regions = NULL;
+
+#ifdef CONFIG_LP_DEBUG_MALLOC
+	dma->minimal_free = 0;
+	dma->magic_initialized = 0;
+	dma->name = "DMA";
+
+	printf("Initialized cache-coherent DMA memory at [%p:%p]\n", start, start + size);
+#endif
 }
 
 int dma_initialized()
@@ -108,16 +123,6 @@ int dma_coherent(void *ptr)
 	return !dma_initialized() || (dma->start <= ptr && dma->end > ptr);
 }
 
-static void setup(hdrtype_t volatile *start, int size)
-{
-	*start = FREE_BLOCK(size);
-
-#ifdef CONFIG_LP_DEBUG_MALLOC
-	heap_initialized = 1;
-	minimal_free  = size;
-#endif
-}
-
 static void *alloc(int len, struct memory_type *type)
 {
 	hdrtype_t header;
@@ -130,8 +135,14 @@ static void *alloc(int len, struct memory_type *type)
 		return (void *)NULL;
 
 	/* Make sure the region is setup correctly. */
-	if (!HAS_MAGIC(*ptr))
-		setup(ptr, (int)((type->end - type->start) - HDRSIZE));
+	if (!HAS_MAGIC(*ptr)) {
+		size_t size = (type->end - type->start) - HDRSIZE;
+		*ptr = FREE_BLOCK(size);
+#ifdef CONFIG_LP_DEBUG_MALLOC
+		type->magic_initialized = 1;
+		type->minimal_free = size;
+#endif
+	}
 
 	/* Find some free space. */
 	do {
@@ -452,24 +463,29 @@ void *dma_memalign(size_t align, size_t size)
 #ifdef CONFIG_LP_DEBUG_MALLOC
 void print_malloc_map(void)
 {
-	void *ptr = heap->start;
-	int free_memory = 0;
+	struct memory_type *type = heap;
+	void *ptr;
+	int free_memory;
 
-	while (ptr < heap->end) {
+again:
+	ptr = type->start;
+	free_memory = 0;
+
+	while (ptr < type->end) {
 		hdrtype_t hdr = *((hdrtype_t *) ptr);
 
 		if (!HAS_MAGIC(hdr)) {
-			if (heap_initialized)
-				printf("Poisoned magic - we're toast\n");
+			if (type->magic_initialized)
+				printf("%s: Poisoned magic - we're toast\n", type->name);
 			else
-				printf("No magic yet - going to initialize\n");
+				printf("%s: No magic yet - going to initialize\n", type->name);
 			break;
 		}
 
 		/* FIXME: Verify the size of the block. */
 
-		printf("%x: %s (%x bytes)\n",
-		       (unsigned int)(ptr - heap->start),
+		printf("%s %x: %s (%x bytes)\n", type->name,
+		       (unsigned int)(ptr - type->start),
 		       hdr & FLAG_FREE ? "FREE" : "USED", SIZE(hdr));
 
 		if (hdr & FLAG_FREE)
@@ -478,9 +494,14 @@ void print_malloc_map(void)
 		ptr += HDRSIZE + SIZE(hdr);
 	}
 
-	if (free_memory && (minimal_free > free_memory))
-		minimal_free = free_memory;
-	printf("Maximum memory consumption: %d bytes\n",
-		(unsigned int)(heap->end - heap->start) - HDRSIZE - minimal_free);
+	if (free_memory && (type->minimal_free > free_memory))
+		type->minimal_free = free_memory;
+	printf("%s: Maximum memory consumption: %u bytes\n", type->name,
+		(type->end - type->start) - HDRSIZE - type->minimal_free);
+
+	if (type != dma) {
+		type = dma;
+		goto again;
+	}
 }
 #endif



More information about the coreboot-gerrit mailing list