[coreboot-gerrit] Patch set updated for coreboot: 809aaeb Refactor usage of walkcbfs to permit access to CBFS headers

Alexandru Gagniuc (mr.nuke.me@gmail.com) gerrit at coreboot.org
Mon Dec 9 16:01:55 CET 2013


Alexandru Gagniuc (mr.nuke.me at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4504

-gerrit

commit 809aaebd6ba6bec6bc76829d058388c57a32ba6b
Author: Alexandru Gagniuc <mr.nuke.me at gmail.com>
Date:   Sun Dec 8 01:13:43 2013 -0600

    Refactor usage of walkcbfs to permit access to CBFS headers
    
    walkcbfs() is used only with ROMCC. Besides finding stages during the
    bootblock, it's also used when applying microcode updates during the
    bootblock phase. The function used to return only a pointer to the data of
    the CBFS file, while making the header completely inaccessible. Since the
    header contains the length of the CBFS file, the caller did not have a way
    to know how long the data was. Then, other conventions had to be used to
    determine the EOF, which might present problems if the user replaces the
    CBFS file. This is not an issue when jumping to a stage (romstage), but can
    present problems when accessing a microcode file which has not been
    NULL-terminated.
    
    Refactor walkcbfs_asm to return a pointer to the CBFS file header rather
    than the data. Rename walkcbfs() to walkcbfs_head(), and reimplement a new
    walkcbfs() based on walkcbfs_head(). Thus current usage of walkcbfs()
    remains unaffected.
    The code has been verified to run successfully under qemu.
    
    Subsequent patches will change usage of walkcbfs() to walkcbfs_head where
    knowing the length of the data is needed.
    
    Change-Id: I21cbf19e130e1480e2749754e5d5130d36036f8e
    Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
---
 src/arch/x86/include/arch/cbfs.h | 21 +++++++++++++++++++--
 src/arch/x86/lib/walkcbfs.S      |  6 ++----
 src/include/cbfs_core.h          | 14 +++++++++++---
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/src/arch/x86/include/arch/cbfs.h b/src/arch/x86/include/arch/cbfs.h
index 8a61d6e..964eb4e 100644
--- a/src/arch/x86/include/arch/cbfs.h
+++ b/src/arch/x86/include/arch/cbfs.h
@@ -20,7 +20,9 @@
 #ifndef __INCLUDE_ARCH_CBFS__
 #define __INCLUDE_ARCH_CBFS__
 
-static void *walkcbfs(char *target)
+#include <cbfs_core.h>
+
+static struct cbfs_file *walkcbfs_head(char *target)
 {
 	void *entry;
 	asm volatile (
@@ -30,6 +32,16 @@ static void *walkcbfs(char *target)
 	return entry;
 }
 
+static void *walkcbfs(char *target)
+{
+	struct cbfs_file *head = walkcbfs_head(target);
+	if ((u32)head != 0)
+		return CBFS_SUBHEADER(head);
+
+	/* We should never reach this if 'target' exists */
+	return (void *)0;
+}
+
 /* just enough to support findstage. copied because the original version doesn't easily pass through romcc */
 struct cbfs_stage_restricted {
 	unsigned long compression;
@@ -38,7 +50,12 @@ struct cbfs_stage_restricted {
 
 static inline unsigned long findstage(char* target)
 {
-	return ((struct cbfs_stage_restricted *)walkcbfs(target))->entry;
+	struct cbfs_stage_restricted *stage = walkcbfs(target);
+	if ((u32)stage != 0)
+		return stage->entry;
+
+	/* We should never reach this if 'target' exists */
+	return 0;
 }
 
 static inline void call(unsigned long addr, unsigned long bist)
diff --git a/src/arch/x86/lib/walkcbfs.S b/src/arch/x86/lib/walkcbfs.S
index 2dc9617..60eb8b5 100644
--- a/src/arch/x86/lib/walkcbfs.S
+++ b/src/arch/x86/lib/walkcbfs.S
@@ -18,7 +18,7 @@
 /*
  * input %esi: filename
  * input %esp: return address (not pointer to return address!)
- * output %eax: entry point
+ * output %eax: pointer to CBFS header
  * clobbers %ebx, %ecx, %edi
  */
 walkcbfs_asm:
@@ -59,9 +59,7 @@ walker:
 	jnz tryharder
 
 	/* we found it! */
-	mov CBFS_FILE_OFFSET(%ebx), %eax
-	bswap %eax
-	add %ebx, %eax
+	mov %ebx, %eax
 	jmp *%esp
 
 tryharder:
diff --git a/src/include/cbfs_core.h b/src/include/cbfs_core.h
index 08fe815..04b5dd7 100644
--- a/src/include/cbfs_core.h
+++ b/src/include/cbfs_core.h
@@ -134,6 +134,15 @@ struct cbfs_file {
 	uint32_t offset;
 } __attribute__((packed));
 
+#define CBFS_NAME(_c) (((char *) (_c)) + sizeof(struct cbfs_file))
+#define CBFS_SUBHEADER(_p) ( (void *) ((((uint8_t *) (_p)) + ntohl((_p)->offset))) )
+
+/*
+ * ROMCC does not understand uint64_t, so we hide future definitions as they are
+ * unlikely to be ever needed from ROMCC
+ */
+#ifndef __ROMCC__
+
 /*** Component sub-headers ***/
 
 /* Following are component sub-headers for the "standard"
@@ -177,9 +186,6 @@ struct cbfs_optionrom {
 	uint32_t len;
 } __attribute__((packed));
 
-#define CBFS_NAME(_c) (((char *) (_c)) + sizeof(struct cbfs_file))
-#define CBFS_SUBHEADER(_p) ( (void *) ((((uint8_t *) (_p)) + ntohl((_p)->offset))) )
-
 #define CBFS_MEDIA_INVALID_MAP_ADDRESS	((void*)(0xffffffff))
 #define CBFS_DEFAULT_MEDIA		((void*)(0x0))
 
@@ -225,4 +231,6 @@ int cbfs_decompress(int algo, void *src, void *dst, int len);
  *  on failure */
 const struct cbfs_header *cbfs_get_header(struct cbfs_media *media);
 
+#endif /* __ROMCC__ */
+
 #endif



More information about the coreboot-gerrit mailing list