[coreboot-gerrit] Patch set updated for coreboot: a13f15c Remove unnecessary mappings in cbfs_media

Naman Govil (namangov@gmail.com) gerrit at coreboot.org
Thu Jul 10 11:41:49 CEST 2014


Naman Govil (namangov at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6169

-gerrit

commit a13f15c635c00e11d3796fe44901f62056456641
Author: Naman Govil <namangov at gmail.com>
Date:   Mon Jun 30 22:57:11 2014 +0530

    Remove unnecessary mappings in cbfs_media
    
    Reducing the sram usage while booting by removing unwanted map()s by creating
    a structure to replace it with appropriate read()s
    
    Change-Id: If503368553308a2fdd3ecc1a312a11f14c7a707f
    Signed-off-by: Naman Govil <namangov at gmail.com>
---
 src/arch/x86/boot/smbios.c |   6 +-
 src/include/cbfs_core.h    |  27 +++++--
 src/lib/cbfs.c             |  81 +++++++++++++++------
 src/lib/cbfs_core.c        | 176 +++++++++++++++++++++++++--------------------
 4 files changed, 184 insertions(+), 106 deletions(-)

diff --git a/src/arch/x86/boot/smbios.c b/src/arch/x86/boot/smbios.c
index ce47a8b..bd81fa6 100644
--- a/src/arch/x86/boot/smbios.c
+++ b/src/arch/x86/boot/smbios.c
@@ -152,11 +152,11 @@ static int smbios_write_type0(unsigned long *current, int handle)
 #endif
 
 	{
-		const struct cbfs_header *header;
+		struct cbfs_header header;
 		u32 romsize = CONFIG_ROM_SIZE;
-		header = cbfs_get_header(CBFS_DEFAULT_MEDIA);
-		if (header != CBFS_HEADER_INVALID_ADDRESS)
+		if (!cbfs_get_header(CBFS_DEFAULT_MEDIA, &header)) {
 			romsize = ntohl(header->romsize);
+			}
 		t->bios_rom_size = (romsize / 65535) - 1;
 	}
 
diff --git a/src/include/cbfs_core.h b/src/include/cbfs_core.h
index a1d8127..7c94be0 100644
--- a/src/include/cbfs_core.h
+++ b/src/include/cbfs_core.h
@@ -4,6 +4,7 @@
  * Copyright (C) 2008 Jordan Crouse <jordan at cosmicpenguin.net>
  * Copyright (C) 2012 Google, Inc.
  * Copyright (C) 2013 The Chromium OS Authors. All rights reserved.
+ * Copyright (C) 2014 Naman Govil <namangov at gmail.com>
  *
  * This file is dual-licensed. You can choose between:
  *   - The GNU GPL, version 2, as published by the Free Software Foundation
@@ -189,6 +190,12 @@ struct cbfs_optionrom {
 #define CBFS_MEDIA_INVALID_MAP_ADDRESS	((void*)(0xffffffff))
 #define CBFS_DEFAULT_MEDIA		((void*)(0x0))
 
+struct cbfs_file_handle {
+	uint32_t data_offset;
+	uint32_t data_len;
+	struct cbfs_file file;
+};
+
 /* Media for CBFS to load files. */
 struct cbfs_media {
 
@@ -220,16 +227,24 @@ struct cbfs_media {
 /* returns pointer to a file entry inside CBFS or NULL */
 struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name);
 
-/* returns pointer to file content inside CBFS after if type is correct */
-void *cbfs_get_file_content(struct cbfs_media *media, const char *name,
-			    int type, size_t *sz);
 
 /* returns decompressed size on success, 0 on failure */
 int cbfs_decompress(int algo, void *src, void *dst, int len);
 
-/* returns a pointer to CBFS master header, or CBFS_HEADER_INVALID_ADDRESS
- *  on failure */
-const struct cbfs_header *cbfs_get_header(struct cbfs_media *media);
+/* finds the CBFS master header and fills it in a cbfs_header structure,
+   return 0 on success and <0 if header not found */
+int cbfs_get_header(struct cbfs_media *media, struct cbfs_header *header);
+
+/* Returns success (0) on finding the file requested by verifying name/type;
+  -1 if file not found
+ The absolute data_offset to the file is stored */
+int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handle *f,
+		const char *name, int type);
+
+
+/* returns pointer to file content inside CBFS after verifying if type is correct */
+void *cbfs_get_file_content(struct cbfs_media *media, const char *name,
+		int type, size_t *sz);
 
 #endif /* __ROMCC__ */
 
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index dc08937..e6c9afa 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2008, Jordan Crouse <jordan at cosmicpenguin.net>
  * Copyright (C) 2013 The Chromium OS Authors. All rights reserved.
- *
+ * Copyright (C) 2014, Naman Govil <namangov at gmail.com>
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; version 2 of the License.
@@ -119,38 +119,79 @@ void *cbfs_load_optionrom(struct cbfs_media *media, uint16_t vendor,
 
 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, NULL);
+	struct cbfs_file_handle f;
+	struct cbfs_stage stage;
+	int c;
+	ssize_t value_read;
+	void * data;
+	ssize_t v_read;
+	struct cbfs_media default_media;
+
+	if (media == CBFS_DEFAULT_MEDIA) {
+			media = &default_media;
+			if (init_default_cbfs_media(media) != 0) {
+			ERROR("Failed to initialize default media.\n");
+			return (void *)-1;
+		}
+	}
+
+	c = cbfs_find_file(media, &f, name, CBFS_TYPE_STAGE);
+
+	if (c < 0) {
+		ERROR("Stage not loaded\n");
+		return (void *)-1;
+	}
+
+	value_read = media->read(media, &stage, f.data_offset, sizeof(stage));
 	/* this is a mess. There is no ntohll. */
 	/* for now, assume compatible byte order until we solve this. */
+	if (value_read != sizeof(stage))
+		return (void *)-1;
+
 	uint32_t entry;
 	uint32_t final_size;
 
-	if (stage == NULL)
-		return (void *) -1;
+	DEBUG("Read Complete @offset = 0x%x and length = %d\n",
+			f.data_offset, value_read);
 
 	LOG("loading stage %s @ 0x%x (%d bytes), entry @ 0x%llx\n",
 			name,
-			(uint32_t) stage->load, stage->memlen,
-			stage->entry);
+			(uint32_t) stage.load, stage.memlen,
+			stage.entry);
 
-	final_size = cbfs_decompress(stage->compression,
-				     ((unsigned char *) stage) +
-				     sizeof(struct cbfs_stage),
-				     (void *) (uint32_t) stage->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,
-	       stage->memlen - final_size);
+	if(stage.compression == CBFS_COMPRESS_NONE) {
+		//No compression; hence we can directly read
+		DEBUG("Read Done\n");
+		v_read = media->read(media, (void *) (uintptr_t) stage.load,
+				f.data_offset + sizeof(stage), f.data_len);
+		if (v_read != f.data_len)
+			return (void *)-1;
 
-	DEBUG("stage loaded.\n");
+		final_size = f.data_len;
+	}
+	else {
+		data = media->map(media, f.data_offset + sizeof(stage), f.data_len);
+		if (data == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
+			ERROR("Map not successful");
+			return (void *) -1;
+		}
+
+		DEBUG("Map Done\n");
+		final_size = cbfs_decompress(stage.compression, data,
+				     (void *) (uint32_t) stage.load,
+				     stage.len);
+
+		if (!final_size)
+			return (void *) -1;
+	}
 
-	entry = stage->entry;
-	// entry = ntohll(stage->entry);
+	/* Stages rely the below clearing so that the bss is initialized. */
+	memset((void *)((uintptr_t)stage.load + final_size), 0,
+	       stage.memlen - final_size);
 
+	DEBUG("stage loaded.\n");
+	entry = stage.entry;
 	return (void *) entry;
 }
 
diff --git a/src/lib/cbfs_core.c b/src/lib/cbfs_core.c
index 50c037e..be80b0a 100644
--- a/src/lib/cbfs_core.c
+++ b/src/lib/cbfs_core.c
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2011 secunet Security Networks AG
  * Copyright (C) 2013 The Chromium OS Authors. All rights reserved.
+ * Copyright (C) 2014 Naman Govil <namangov at gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,150 +53,171 @@
 #include <cbfs.h>
 #include <string.h>
 
-/* returns a pointer to CBFS master header, or CBFS_HEADER_INVALID_ADDRESS
- *  on failure */
-const struct cbfs_header *cbfs_get_header(struct cbfs_media *media)
+/* fills in the header structure with a pointer to CBFS master header,
+   returns 0 on success and <0 if header not found */
+int cbfs_get_header(struct cbfs_media *media, struct cbfs_header *header)
 {
-	const struct cbfs_header *header;
 	struct cbfs_media default_media;
+	ssize_t header_size;
 
 	if (media == CBFS_DEFAULT_MEDIA) {
 		media = &default_media;
 		if (init_default_cbfs_media(media) != 0) {
 			ERROR("Failed to initialize default media.\n");
-			return CBFS_HEADER_INVALID_ADDRESS;
+			return -1;
 		}
 	}
 
 	media->open(media);
 	DEBUG("CBFS_HEADER_ROM_ADDRESS: 0x%x/0x%x\n", CBFS_HEADER_ROM_ADDRESS,
 	      CONFIG_ROM_SIZE);
-	header = media->map(media, CBFS_HEADER_ROM_ADDRESS, sizeof(*header));
+	header_size = media->read(media, (void *)header ,
+			CBFS_HEADER_ROM_ADDRESS, sizeof(*header));
 	media->close(media);
 
-	if (header == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
+	header->offset = ntohl(header->offset);
+	header->align = ntohl(header->align);
+	header->romsize = ntohl(header->romsize);
+	header->magic = ntohl(header->magic);
+	header->version = ntohl(header->version);
+	header->architecture = ntohl(header->architecture);
+	header->bootblocksize = ntohl(header->bootblocksize);
+	header->pad[1] = ntohl(header->pad[1]);
+
+	if (header_size != sizeof(*header)) {
 		ERROR("Failed to load CBFS header from 0x%x\n",
 		      CBFS_HEADER_ROM_ADDRESS);
-		return CBFS_HEADER_INVALID_ADDRESS;
+		return -1;
 	}
-
-	if (CBFS_HEADER_MAGIC != ntohl(header->magic)) {
+	else if (CBFS_HEADER_MAGIC != header->magic) {
 		ERROR("Could not find valid CBFS master header at %x: "
 		      "%x vs %x.\n", CBFS_HEADER_ROM_ADDRESS, CBFS_HEADER_MAGIC,
-		      ntohl(header->magic));
+		      header->magic);
 		if (header->magic == 0xffffffff) {
 			ERROR("Maybe ROM is not mapped properly?\n");
 		}
-		return CBFS_HEADER_INVALID_ADDRESS;
+		return -1;
 	}
-	return header;
+	return 0;
 }
 
 /* public API starts here*/
-struct cbfs_file *cbfs_get_file(struct cbfs_media *media, const char *name)
+/* This functions finds the absolute data_offset of a file searched for by name/type
+   Returns 0 on success and -1 on failure
+ */
+int cbfs_find_file(struct cbfs_media *media, struct cbfs_file_handle *f,
+		const char *name, int type)
 {
-	const char *file_name;
-	uint32_t offset, align, romsize, name_len;
-	const struct cbfs_header *header;
-	struct cbfs_file file, *file_ptr;
+	uint32_t offset, align, romsize,name_len;
 	struct cbfs_media default_media;
+	ssize_t value_read;
+	const char *file_name;
+	struct cbfs_header header;
 
 	if (media == CBFS_DEFAULT_MEDIA) {
 		media = &default_media;
 		if (init_default_cbfs_media(media) != 0) {
 			ERROR("Failed to initialize default media.\n");
-			return NULL;
+			return -1;
 		}
 	}
 
-	if (CBFS_HEADER_INVALID_ADDRESS == (header = cbfs_get_header(media)))
-		return NULL;
+	if (cbfs_get_header(media, &header))
+		return -1; // error
 
-	// Logical offset (for source media) of first file.
-	offset = ntohl(header->offset);
-	align = ntohl(header->align);
-	romsize = ntohl(header->romsize);
+	// Offset (for source media) of first file.
+	offset = header.offset;
+	align = header.align;
+	romsize = header.romsize;
 
-	// TODO Add a "size" in CBFS header for a platform independent way to
-	// determine the end of CBFS data.
 #if defined(CONFIG_ARCH_X86) && CONFIG_ARCH_X86
-	romsize -= htonl(header->bootblocksize);
+	romsize -= htonl(header.bootblocksize);
 #endif
 	DEBUG("CBFS location: 0x%x~0x%x, align: %d\n", offset, romsize, align);
-
 	DEBUG("Looking for '%s' starting from 0x%x.\n", name, offset);
 	media->open(media);
-	while (offset < romsize &&
-	       media->read(media, &file, offset, sizeof(file)) == sizeof(file)) {
-		if (memcmp(CBFS_FILE_MAGIC, file.magic,
-			   sizeof(file.magic)) != 0) {
+	while (offset < romsize)
+	{
+		value_read = media->read(media, &f->file, offset, sizeof(f->file));
+		if(value_read != sizeof(f->file)){
+			return -1; //error: since read not successful
+		}
+		//make all ntohl() at once place to avoid any gibberish later
+		f->file.len = ntohl(f->file.len);
+		f->file.offset = ntohl(f->file.offset);
+		f->file.type = ntohl(f->file.type);
+
+		if (memcmp(CBFS_FILE_MAGIC, f->file.magic,
+			   sizeof(f->file.magic)) != 0) {
 			uint32_t new_align = align;
 			if (offset % align)
 				new_align += align - (offset % align);
-			ERROR("ERROR: No file header found at 0x%x - "
-			      "try next aligned address: 0x%x.\n", offset,
-			      offset + new_align);
+			ERROR("ERROR: No file header found at 0x%x - try next aligned address: 0x%x.\n", offset,offset+new_align);
 			offset += new_align;
 			continue;
 		}
-		name_len = ntohl(file.offset) - sizeof(file);
-		DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset,
-		      name_len);
-
-		// load file name (arbitrary length).
-		file_name = (const char *)media->map(
-				media, offset + sizeof(file), name_len);
-		if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
-			ERROR("ERROR: Failed to get filename: 0x%x.\n", offset);
-		} else if (strcmp(file_name, name) == 0) {
-			int file_offset = ntohl(file.offset),
-			    file_len = ntohl(file.len);
-			DEBUG("Found file (offset=0x%x, len=%d).\n",
-			    offset + file_offset, file_len);
-			media->unmap(media, file_name);
-			file_ptr = media->map(media, offset,
-					      file_offset + file_len);
-			media->close(media);
-			return file_ptr;
-		} else {
-			DEBUG(" (unmatched file @0x%x: %s)\n", offset,
-			      file_name);
-			media->unmap(media, file_name);
-		}
 
+		if (f->file.type == type) {
+
+			name_len = f->file.offset - sizeof(f->file);
+			DEBUG(" - load entry 0x%x file name (%d bytes)...\n", offset,name_len);
+			// load file name (arbitrary length).
+			file_name = (const char *)media->map(media, offset + sizeof(f->file), name_len);
+			//this mapping done to verify name of file
+			if (file_name == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
+				ERROR("ERROR: Failed to get filename: 0x%x.\n", offset);
+			} else if (strcmp(file_name, name) == 0) {
+					f->data_offset = offset + f->file.offset;
+					f->data_len = f->file.len;
+					media->unmap(media, file_name);
+					DEBUG("Found file:offset = 0x%x, len=%d\n", f->data_offset, f->data_len);
+					return 0;
+			} else {
+				DEBUG("unmatched file offset = 0x%x : %s\n", offset, file_name);
+				media->unmap(media,file_name);
+			}
+		}
 		// Move to next file.
-		offset += ntohl(file.len) + ntohl(file.offset);
+		offset += f->file.len + f->file.offset;
 		if (offset % align)
 			offset += align - (offset % align);
+		DEBUG("Going for next offset\n");
 	}
 	media->close(media);
-	LOG("WARNING: '%s' not found.\n", name);
-	return NULL;
+	LOG("Warning: '%s' not found\n",name);
+	return -1;
 }
 
-void *cbfs_get_file_content(struct cbfs_media *media, const char *name,
-			    int type, size_t *sz)
-{
-	struct cbfs_file *file = cbfs_get_file(media, name);
 
-	if (sz)
-		*sz = 0;
+/*Returns pointer to file content inside CBFS after verifying type
+ */
+void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz)
+{
+	struct cbfs_file_handle f;
+	int c;
+	struct cbfs_media default_media;
 
-	if (file == NULL) {
-		ERROR("Could not find file '%s'.\n", name);
-		return NULL;
+	if (media == CBFS_DEFAULT_MEDIA) {
+		media = &default_media;
+		if (init_default_cbfs_media(media) != 0) {
+			ERROR("Failed to initialize default media.\n");
+			return NULL;
+		}
 	}
+	c = cbfs_find_file(media, &f, name, type);
 
-	if (ntohl(file->type) != type) {
-		ERROR("File '%s' is of type %x, but we requested %x.\n", name,
-		      ntohl(file->type), type);
+	if (c < 0) {
+		ERROR("File not found\n");
 		return NULL;
 	}
 
+	DEBUG("Found file. Will be mapping it now!\n");
 	if (sz)
-		*sz = ntohl(file->len);
+		*sz = 0;
 
-	return (void *)CBFS_SUBHEADER(file);
+	if (sz)
+		*sz = f.data_len;
+	return media->map(media, f.data_offset, f.data_len);
 }
 
 int cbfs_decompress(int algo, void *src, void *dst, int len)



More information about the coreboot-gerrit mailing list