[coreboot-gerrit] New patch to review for coreboot: ifwitool: Do not calculate checksum for subpart_dir

Furquan Shaikh (furquan@google.com) gerrit at coreboot.org
Fri Jun 10 20:13:23 CEST 2016


Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15145

-gerrit

commit 6c25e085a595e9ad4abc8a7c46b4d6ec9599413f
Author: Furquan Shaikh <furquan at google.com>
Date:   Fri Jun 10 11:08:07 2016 -0700

    ifwitool: Do not calculate checksum for subpart_dir
    
    1. The checksum method that was documented is not correct. So, no use
    filling in a value based on wrong calculations. This can be added back
    once updated information is available.
    2. Checksum does not seem to affect the booting up of SoC. So, fill in 0
    for now.
    
    Change-Id: I0e49ac8e0e04abb6d7c9be70323612bdef309975
    Signed-off-by: Furquan Shaikh <furquan at google.com>
---
 util/cbfstool/ifwitool.c | 55 ++++++------------------------------------------
 1 file changed, 6 insertions(+), 49 deletions(-)

diff --git a/util/cbfstool/ifwitool.c b/util/cbfstool/ifwitool.c
index a65c988..7fca542 100644
--- a/util/cbfstool/ifwitool.c
+++ b/util/cbfstool/ifwitool.c
@@ -101,8 +101,8 @@ struct subpart_dir_header {
 	/* Length of directory header in bytes. */
 	uint8_t header_length;
 	/*
-	 * 8-bit XOR checksum from first byte of header to last byte of
-	 * directory entry.
+	 * TODO(furquan): Add checksum calculation once more details are
+	 * available.
 	 */
 	uint8_t checksum;
 	/* ASCII short name of sub-partition. */
@@ -750,26 +750,7 @@ static void parse_sbpdt(void *data, size_t size)
 			  "S-BPDT");
 }
 
-static uint8_t calc_checksum(struct subpart_dir *s)
-{
-	size_t size = subpart_dir_size(&s->h);
-	uint8_t *data = (uint8_t *)s;
-	uint8_t checksum = 0;
-	size_t i;
-
-	uint8_t old_checksum = s->h.checksum;
-	s->h.checksum = 0;
-
-	for (i = 0; i < size; i++) {
-		checksum ^= data[i];
-	}
-
-	s->h.checksum = old_checksum;
-	return checksum;
-}
-
-static void validate_subpart_dir(struct subpart_dir *s, const char *name,
-				 bool checksum_check)
+static void validate_subpart_dir(struct subpart_dir *s, const char *name)
 {
 	if ((s->h.marker != SUBPART_DIR_MARKER) ||
 	    (s->h.header_version != SUBPART_DIR_HEADER_VERSION_SUPPORTED) ||
@@ -778,27 +759,6 @@ static void validate_subpart_dir(struct subpart_dir *s, const char *name,
 		ERROR("Invalid subpart_dir for %s.\n", name);
 		exit(-1);
 	}
-
-	if (checksum_check == 0)
-		return;
-
-	uint8_t checksum = calc_checksum(s);
-
-	if (checksum != s->h.checksum)
-		ERROR("Invalid checksum for %s(Expected=0x%x,Current=0x%x).\n",
-		      name, checksum, s->h.checksum);
-}
-
-static void validate_subpart_dir_without_checksum(struct subpart_dir *s,
-						  const char *name)
-{
-	validate_subpart_dir(s, name, 0);
-}
-
-static void validate_subpart_dir_with_checksum(struct subpart_dir *s,
-					       const char *name)
-{
-	validate_subpart_dir(s, name, 1);
 }
 
 static void parse_subpart_dir(struct buffer *subpart_dir_buf,
@@ -824,7 +784,7 @@ static void parse_subpart_dir(struct buffer *subpart_dir_buf,
 	memcpy(hdr.name, data + offset, sizeof(hdr.name));
 	offset += sizeof(hdr.name);
 
-	validate_subpart_dir_without_checksum((struct subpart_dir *)&hdr, name);
+	validate_subpart_dir((struct subpart_dir *)&hdr, name);
 
 	assert(size > subpart_dir_size(&hdr));
 	alloc_buffer(subpart_dir_buf, subpart_dir_size(&hdr), "Subpart Dir");
@@ -845,8 +805,6 @@ static void parse_subpart_dir(struct buffer *subpart_dir_buf,
 				     &e[i].rsvd);
 	}
 
-	validate_subpart_dir_with_checksum(subpart_dir, name);
-
 	print_subpart_dir(subpart_dir);
 }
 
@@ -1395,7 +1353,7 @@ static void create_subpart(struct buffer *dst, struct buffer *info[],
 		       buffer_size(info[i]));
 	}
 
-	h->checksum = calc_checksum(buffer_get(&subpart_dir_buff));
+	h->checksum = 0;
 
 	struct subpart_dir *dir = buffer_get(&subpart_dir_buff);
 
@@ -1759,8 +1717,7 @@ static enum ifwi_ret ifwi_dir_replace(int type)
 		s->e[i].offset += offset;
 	}
 
-	/* Re-calculate checksum. */
-	s->h.checksum = calc_checksum(s);
+	s->h.checksum = 0;
 
 	/* Convert members to litte-endian. */
 	subpart_dir_fixup_write_buffer(&subpart_dir_buf);



More information about the coreboot-gerrit mailing list