[coreboot-gerrit] Patch set updated for coreboot: 8a8d82f cbfs: 64-bit cleanups

Ronald G. Minnich (rminnich@gmail.com) gerrit at coreboot.org
Sun Nov 17 17:42:23 CET 2013


Ronald G. Minnich (rminnich at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4037

-gerrit

commit 8a8d82f72d6c62070a59a996ff1a1e33203ec536
Author: Ronald G. Minnich <rminnich at google.com>
Date:   Mon Nov 11 10:36:28 2013 -0800

    cbfs: 64-bit cleanups
    
    cbfs used u32 in a number of cases where uintptr_t was
    correct. This change builds for both 64-bit and 32-bit
    boards.
    
    Also, add a function to convert 64-bit struct members
    to a void *. Note that it accomodates the incorrect
    byte ordering from cbfs -- which has been prevalent
    in coreboot since the beginning, and which we will not
    fix this time around. People might be upset if they
    suddenly can't read their flash images with cbfs.
    
    Change-Id: If42c722a8a9e8d565d3827f65ed6c2cb8e90ba60
    Signed-off-by: Ronald G. Minnich <rminnich at google.com>
---
 src/lib/cbfs.c     | 42 ++++++++++++++++++++++++++++++++----------
 src/lib/cbmem.c    |  2 +-
 src/lib/selfboot.c |  9 +++++----
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 4ccc9e6..1b9732c 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -254,13 +254,33 @@ void * cbfs_load_stage(struct cbfs_media *media, const char *name)
 
 #else
 
+static void *cbfs_pointer(u64 value)
+{
+	void *v;
+
+	/* While the goal was that cbfs data was bigendian,
+	 * these fields are in fact little endian. It's a bug.
+	 * But fixing it won't be simple as it will break
+	 * backwards compatibility with already built images.
+	 * For now, go with what works: use .
+	 */
+	/* there has to be a better way to do this. Do we need
+	 * CONFIG_WORD_SIZE or something?
+	 */
+#if (CONFIG_ARCH_X86 == 1) || (CONFIG_ARCH_ARMV7 == 1)
+	v = (void *)le32_to_cpu(value);
+#else
+	v = (void *)le64_to_cpu(value);
+#endif
+	return v;
+}
+
 void * cbfs_load_stage(struct cbfs_media *media, const char *name)
 {
 	struct cbfs_stage *stage = (struct cbfs_stage *)
 		cbfs_get_file_content(media, name, CBFS_TYPE_STAGE);
-	/* this is a mess. There is no ntohll. */
-	/* for now, assume compatible byte order until we solve this. */
-	uint32_t entry;
+
+	void *load, *entry;
 	uint32_t final_size;
 
 	if (stage == NULL)
@@ -271,29 +291,30 @@ void * cbfs_load_stage(struct cbfs_media *media, const char *name)
 			(uint32_t) stage->load, stage->memlen,
 			stage->entry);
 
+	load = cbfs_pointer(stage->load);
 	final_size = cbfs_decompress(stage->compression,
 				     ((unsigned char *) stage) +
 				     sizeof(struct cbfs_stage),
-				     (void *) (uint32_t) stage->load,
+				     load,
 				     stage->len);
 	if (!final_size)
 		return (void *) -1;
 
 	/* Stages rely the below clearing so that the bss is initialized. */
-	memset((void *)((uintptr_t)stage->load + final_size), 0,
+	memset((load + final_size), 0,
 	       stage->memlen - final_size);
 
 	DEBUG("stage loaded.\n");
 
-	entry = stage->entry;
-	// entry = ntohll(stage->entry);
+	entry = cbfs_pointer(stage->entry);
 
-	return (void *) entry;
+	return entry;
 }
 #endif /* CONFIG_RELOCATABLE_RAMSTAGE */
 
 int cbfs_execute_stage(struct cbfs_media *media, const char *name)
 {
+	void * entry;
 	struct cbfs_stage *stage = (struct cbfs_stage *)
 		cbfs_get_file_content(media, name, CBFS_TYPE_STAGE);
 
@@ -307,8 +328,9 @@ int cbfs_execute_stage(struct cbfs_media *media, const char *name)
 	}
 
 	/* FIXME: This isn't right */
-	LOG("run @ %p\n", (void *) ntohl((uint32_t) stage->entry));
-	return run_address((void *)(uintptr_t)ntohll(stage->entry));
+	entry = cbfs_pointer(stage->entry);
+	LOG("run @ %p\n",  entry);
+	return run_address(entry);
 }
 
 #if !CONFIG_ALT_CBFS_LOAD_PAYLOAD
diff --git a/src/lib/cbmem.c b/src/lib/cbmem.c
index de49816..6449b55 100644
--- a/src/lib/cbmem.c
+++ b/src/lib/cbmem.c
@@ -200,7 +200,7 @@ void *cbmem_add(u32 id, u64 size)
 	cbmem_toc[0].base += size;
 	cbmem_toc[0].size -= size;
 
-	return (void *)(u32)cbmem_toc[i].base;
+	return (void *)(uintptr_t)cbmem_toc[i].base;
 }
 
 void *cbmem_find(u32 id)
diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c
index f69ad14..222eae2 100644
--- a/src/lib/selfboot.c
+++ b/src/lib/selfboot.c
@@ -305,7 +305,7 @@ static int relocate_segment(unsigned long buffer, struct segment *seg)
 static int build_self_segment_list(
 	struct segment *head,
 	struct lb_memory *mem,
-	struct cbfs_payload *payload, u32 *entry)
+	struct cbfs_payload *payload, uintptr_t *entry)
 {
 	struct segment *new;
 	struct segment *ptr;
@@ -332,8 +332,9 @@ static int build_self_segment_list(
 			new->s_memsz = ntohl(segment->mem_len);
 			new->compression = ntohl(segment->compression);
 
-			new->s_srcaddr = (u32) ((unsigned char *)first_segment)
-						+ ntohl(segment->offset);
+			new->s_srcaddr = (uintptr_t)
+				((unsigned char *)first_segment)
+				+ ntohl(segment->offset);
 			new->s_filesz = ntohl(segment->len);
 			printk(BIOS_DEBUG, "  New segment dstaddr 0x%lx memsize 0x%lx srcaddr 0x%lx filesize 0x%lx\n",
 				new->s_dstaddr, new->s_memsz, new->s_srcaddr, new->s_filesz);
@@ -504,7 +505,7 @@ static int load_self_segments(
 
 void *selfload(struct lb_memory *mem, struct cbfs_payload *payload)
 {
-	u32 entry=0;
+	uintptr_t entry = 0;
 	struct segment head;
 
 	/* Preprocess the self segments */



More information about the coreboot-gerrit mailing list