coreboot-gerrit December 2015

coreboot-gerrit@coreboot.org

1 participants
752 discussions

New patch to review for coreboot: cbfstool: adapt "cbfstool copy" to only use fmap regions
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12788 -gerrit commit 1b0e3adf013932cfe1603730e56b4734f261a2e7 Author: Patrick Georgi <pgeorgi(a)google.com> Date: Fri Nov 20 21:48:18 2015 +0100 cbfstool: adapt "cbfstool copy" to only use fmap regions These need to go together, so the commit became a bit larger than typial. - Add an option -R for the copy source fmap region. Use: cbfstool copy -r target-region -R source-region. - Don't generate a CBFS master header because for fmap regions, we assume that the region starts with a file header. Use cbfstool add-master-header to add it afterwards, if necessary. - Don't copy files of type "cbfs master header" (which are what cbfstool add-master-header creates) - Leave room for the master header pointer - Remove -D command line option as it's no longer used. BUG=chromium:445938 BRANCH=none TEST=Manual test on image and integration test w/ bundle_firmware changes. CQ-DEPEND=CL:313770,CL:313771 Change-Id: I2a11cda42caee96aa763f162b5f3bc11bb7992f9 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Signed-off-by: Patrick Georgi <adurbin(a)chromium.org> --- util/cbfstool/cbfs_image.c | 29 ++++++++------------------ util/cbfstool/cbfstool.c | 52 ++++++++++++++++++---------------------------- 2 files changed, 29 insertions(+), 52 deletions(-) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 25aafe9..3820c40 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -390,31 +390,16 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst) { assert(image); - if (!cbfs_is_legacy_cbfs(image)) - return -1; struct cbfs_file *src_entry, *dst_entry; - struct cbfs_header *copy_header; - size_t align, entry_offset; + size_t align; ssize_t last_entry_size; size_t copy_end = buffer_size(dst); - align = image->header.align; - - /* This will work, let's create a copy. */ - copy_header = (struct cbfs_header *)buffer_get(dst); - cbfs_put_header(copy_header, &image->header); + align = CBFS_ENTRY_ALIGNMENT; - copy_header->bootblocksize = 0; - /* FIXME: romsize and offset have a full-flash interpretation even - * though they don't need to and should be region-relative (where - * region is sufficiently specified by the master header pointer. - * But that's for a later change. */ - copy_header->romsize = htonl(dst->offset + buffer_size(dst)); - entry_offset = align_up(sizeof(*copy_header), align); - copy_header->offset = htonl(dst->offset + entry_offset); - dst_entry = (struct cbfs_file *)(buffer_get(dst) + entry_offset); + dst_entry = (struct cbfs_file *)buffer_get(dst); /* Copy non-empty files */ for (src_entry = cbfs_find_first_entry(image); @@ -423,6 +408,7 @@ int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst) size_t entry_size; if ((src_entry->type == htonl(CBFS_COMPONENT_NULL)) || + (src_entry->type == htonl(CBFS_COMPONENT_CBFSHEADER)) || (src_entry->type == htonl(CBFS_COMPONENT_DELETED))) continue; @@ -438,9 +424,12 @@ int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst) } } - /* Last entry size is all the room above it. */ + /* Last entry size is all the room above it, except for top 4 bytes + * which may be used by the master header pointer. This messes with + * the ability to stash something "top-aligned" into the region, but + * keeps things simpler. */ last_entry_size = copy_end - ((void *)dst_entry - buffer_get(dst)) - - cbfs_calculate_file_header_size(""); + - cbfs_calculate_file_header_size("") - sizeof(int32_t); if (last_entry_size < 0) WARN("No room to create the last entry!\n") diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index c9a9564..f00aa30 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -49,6 +49,7 @@ static struct param { const char *filename; const char *fmap; const char *region_name; + const char *source_region; const char *bootblock; const char *ignore_section; uint64_t u64val; @@ -56,8 +57,6 @@ static struct param { uint32_t baseaddress; uint32_t baseaddress_assigned; uint32_t loadaddress; - uint32_t copyoffset; - uint32_t copyoffset_assigned; uint32_t headeroffset; uint32_t headeroffset_assigned; uint32_t entrypoint; @@ -917,34 +916,25 @@ static int cbfs_update_fit(void) static int cbfs_copy(void) { - /* always a valid source region */ struct cbfs_image src_image; + struct buffer src_buf; - if (cbfs_image_from_buffer(&src_image, param.image_region, - param.headeroffset)) + if (!param.source_region) { + ERROR("You need to specify -R/--source-region.\n"); return 1; + } - struct buffer dst_buffer; - - if (cbfs_is_legacy_cbfs(&src_image)) { - if (!param.copyoffset_assigned) { - ERROR("You need to specify -D/--copy-offset.\n"); - return 1; - } - - if (!param.size) { - ERROR("You need to specify -s/--size.\n"); - return 1; - } - - buffer_splice(&dst_buffer, param.image_region, - param.copyoffset, param.size); - } else { - ERROR("This operation is only valid on legacy images having CBFS master headers\n"); + /* Obtain the source region and convert it to a cbfs_image. */ + if (!partitioned_file_read_region(&src_buf, param.image_file, + param.source_region)) { + ERROR("Region not found in image: %s\n", param.source_region); return 1; } - return cbfs_copy_instance(&src_image, &dst_buffer); + if (cbfs_image_from_buffer(&src_image, &src_buf, param.headeroffset)) + return 1; + + return cbfs_copy_instance(&src_image, param.image_region); } static const struct command commands[] = { @@ -955,7 +945,7 @@ static const struct command commands[] = { {"add-stage", "a:H:r:f:n:t:c:b:P:S:yvA:h?", cbfs_add_stage, true, true}, {"add-int", "H:r:i:n:b:vh?", cbfs_add_integer, true, true}, {"add-master-header", "H:r:vh?", cbfs_add_master_header, true, true}, - {"copy", "H:D:s:h?", cbfs_copy, true, true}, + {"copy", "r:R:h?", cbfs_copy, true, true}, {"create", "M:r:s:B:b:H:o:m:vh?", cbfs_create, true, true}, {"extract", "H:r:m:n:f:vh?", cbfs_extract, true, false}, {"layout", "wvh?", cbfs_layout, false, false}, @@ -972,7 +962,6 @@ static struct option long_options[] = { {"bootblock", required_argument, 0, 'B' }, {"cmdline", required_argument, 0, 'C' }, {"compression", required_argument, 0, 'c' }, - {"copy-offset", required_argument, 0, 'D' }, {"empty-fits", required_argument, 0, 'x' }, {"entry-point", required_argument, 0, 'e' }, {"file", required_argument, 0, 'f' }, @@ -980,6 +969,7 @@ static struct option long_options[] = { {"fill-upward", no_argument, 0, 'u' }, {"flashmap", required_argument, 0, 'M' }, {"fmap-regions", required_argument, 0, 'r' }, + {"source-region", required_argument, 0, 'R' }, {"hash-algorithm",required_argument, 0, 'A' }, {"header-offset", required_argument, 0, 'H' }, {"help", no_argument, 0, 'h' }, @@ -1085,9 +1075,8 @@ static void usage(char *name) "Add a legacy CBFS master header\n" " remove [-r image,regions] -n NAME " "Remove a component\n" - " copy -D new_header_offset -s region size \\\n" - " [-H source header offset] " - "Create a copy (duplicate) cbfs instance*\n" + " copy -r image,regions -R source-region " + "Create a copy (duplicate) cbfs instance in fmap\n" " create -m ARCH -s size [-b bootblock offset] \\\n" " [-o CBFS offset] [-H header offset] [-B bootblock] " "Create a legacy ROM file with CBFS master header*\n" @@ -1216,6 +1205,9 @@ int main(int argc, char **argv) case 'r': param.region_name = optarg; break; + case 'R': + param.source_region = optarg; + break; case 'b': param.baseaddress = strtoul(optarg, NULL, 0); // baseaddress may be zero on non-x86, so we @@ -1246,10 +1238,6 @@ int main(int argc, char **argv) optarg, NULL, 0); param.headeroffset_assigned = 1; break; - case 'D': - param.copyoffset = strtoul(optarg, NULL, 0); - param.copyoffset_assigned = 1; - break; case 'a': param.alignment = strtoul(optarg, NULL, 0); break;

1 0

New patch to review for coreboot: cbfstool: use buffer over offset/size pair for cbfs_copy_instance
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12787 -gerrit commit 8c841a01d73c21de1dd2ec53011ad39f744f7edf Author: Patrick Georgi <pgeorgi(a)google.com> Date: Fri Nov 20 19:22:50 2015 +0100 cbfstool: use buffer over offset/size pair for cbfs_copy_instance This allows adding support for FMAP based cbfstool copy more easily. BUG=chromium:445938 BRANCH=tot TEST=bundle_firmware still works Change-Id: I72e7bc4da7d27853e324400f76f86136e3d8726e Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- util/cbfstool/cbfs_image.c | 41 +++++++++++++---------------------------- util/cbfstool/cbfs_image.h | 3 +-- util/cbfstool/cbfstool.c | 33 ++++++++++++++++++++------------- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 1cfe90d..25aafe9 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -387,8 +387,7 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, return 1; } -int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset, - size_t copy_size) +int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst) { assert(image); if (!cbfs_is_legacy_cbfs(image)) @@ -399,37 +398,23 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset, size_t align, entry_offset; ssize_t last_entry_size; - size_t cbfs_offset, cbfs_end; - size_t copy_end = copy_offset + copy_size; + size_t copy_end = buffer_size(dst); align = image->header.align; - cbfs_offset = image->header.offset; - cbfs_end = image->header.romsize; - - if (copy_end > image->buffer.size) { - ERROR("Copy offset out of range: [%zx:%zx)\n", - copy_offset, copy_end); - return 1; - } - - /* Range check requested copy region with source cbfs. */ - if ((copy_offset >= cbfs_offset && copy_offset < cbfs_end) || - (copy_end >= cbfs_offset && copy_end <= cbfs_end)) { - ERROR("New image would overlap old one.\n"); - return 1; - } - /* This will work, let's create a copy. */ - copy_header = (struct cbfs_header *)(image->buffer.data + copy_offset); + copy_header = (struct cbfs_header *)buffer_get(dst); cbfs_put_header(copy_header, &image->header); copy_header->bootblocksize = 0; - /* Romsize is a misnomer. It's the absolute limit of cbfs content.*/ - copy_header->romsize = htonl(copy_end); - entry_offset = align_up(copy_offset + sizeof(*copy_header), align); - copy_header->offset = htonl(entry_offset); - dst_entry = (struct cbfs_file *)(image->buffer.data + entry_offset); + /* FIXME: romsize and offset have a full-flash interpretation even + * though they don't need to and should be region-relative (where + * region is sufficiently specified by the master header pointer. + * But that's for a later change. */ + copy_header->romsize = htonl(dst->offset + buffer_size(dst)); + entry_offset = align_up(sizeof(*copy_header), align); + copy_header->offset = htonl(dst->offset + entry_offset); + dst_entry = (struct cbfs_file *)(buffer_get(dst) + entry_offset); /* Copy non-empty files */ for (src_entry = cbfs_find_first_entry(image); @@ -446,7 +431,7 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset, dst_entry = (struct cbfs_file *)( (uintptr_t)dst_entry + align_up(entry_size, align)); - if ((size_t)((char *)dst_entry - image->buffer.data) >= + if ((size_t)((void *)dst_entry - buffer_get(dst)) >= copy_end) { ERROR("Ran out of room in copy region.\n"); return 1; @@ -454,7 +439,7 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset, } /* Last entry size is all the room above it. */ - last_entry_size = copy_end - ((char *)dst_entry - image->buffer.data) + last_entry_size = copy_end - ((void *)dst_entry - buffer_get(dst)) - cbfs_calculate_file_header_size(""); if (last_entry_size < 0) diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 195494c..00adcc8 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -74,8 +74,7 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, /* Create a duplicate CBFS image. Returns 0 on success, otherwise non-zero. * Will not succeed on new-style images without a master header. */ -int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset, - size_t copy_size); +int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst); /* Releases the CBFS image. Returns 0 on success, otherwise non-zero. */ int cbfs_image_delete(struct cbfs_image *image); diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 2529296..c9a9564 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -917,27 +917,34 @@ static int cbfs_update_fit(void) static int cbfs_copy(void) { - if (!param.copyoffset_assigned) { - ERROR("You need to specify -D/--copy-offset.\n"); - return 1; - } - - if (!param.size) { - ERROR("You need to specify -s/--size.\n"); - return 1; - } + /* always a valid source region */ + struct cbfs_image src_image; - struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, + if (cbfs_image_from_buffer(&src_image, param.image_region, param.headeroffset)) return 1; - if (!cbfs_is_legacy_cbfs(&image)) { + struct buffer dst_buffer; + + if (cbfs_is_legacy_cbfs(&src_image)) { + if (!param.copyoffset_assigned) { + ERROR("You need to specify -D/--copy-offset.\n"); + return 1; + } + + if (!param.size) { + ERROR("You need to specify -s/--size.\n"); + return 1; + } + + buffer_splice(&dst_buffer, param.image_region, + param.copyoffset, param.size); + } else { ERROR("This operation is only valid on legacy images having CBFS master headers\n"); return 1; } - return cbfs_copy_instance(&image, param.copyoffset, param.size); + return cbfs_copy_instance(&src_image, &dst_buffer); } static const struct command commands[] = {

1 0

New patch to review for coreboot: commonlib: add cbfs_vb2_hash_contents()
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12786 -gerrit commit 3b203fa14c972f49fcb85a27ed83591263f6ceef Author: Aaron Durbin <adurbin(a)chromium.org> Date: Tue Dec 15 15:57:11 2015 -0600 commonlib: add cbfs_vb2_hash_contents() Provide a common routine to hash the contents of a cbfs region. The cbfs region is hashed in the following order: 1. potential cbfs header at offset 0 2. potential cbfs header retlative offset at cbfs size - 4 3. For each file the metadata of the file and it is not an empty file then the data as well. BUG=chrome-os-partner:48412 BUG=chromium:445938 BRANCH=None TEST=TBD Change-Id: Ie1e5db5b8a80d9465e88d3f69f5367d887bdf73f Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> --- src/commonlib/cbfs.c | 158 +++++++++++++++++++++++++++++++-- src/commonlib/include/commonlib/cbfs.h | 12 +++ 2 files changed, 165 insertions(+), 5 deletions(-) diff --git a/src/commonlib/cbfs.c b/src/commonlib/cbfs.c index 06d31ed..56faf28 100644 --- a/src/commonlib/cbfs.c +++ b/src/commonlib/cbfs.c @@ -108,6 +108,19 @@ int cbfs_for_each_file(const struct region_device *cbfs, return -1; } +static int cbfsf_file_type(struct cbfsf *fh, uint32_t *ftype) +{ + const size_t sz = sizeof(*ftype); + + if (rdev_readat(&fh->metadata, ftype, + offsetof(struct cbfs_file, type), sz) != sz) + return -1; + + *ftype = read_be32(ftype); + + return 0; +} + int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, const char *name, uint32_t *type) { @@ -148,13 +161,9 @@ int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, if (type != NULL) { uint32_t ftype; - if (rdev_readat(&fh->metadata, &ftype, - offsetof(struct cbfs_file, type), - sizeof(ftype)) != sizeof(ftype)) + if (cbfsf_file_type(fh, &ftype)) break; - ftype = read_be32(&ftype); - if (*type != ftype) { DEBUG(" Unmatched type %x at %zx\n", ftype, rdev_relative_offset(cbfs, @@ -174,3 +183,142 @@ int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, LOG("'%s' not found.\n", name); return -1; } + +static int cbfs_extend_hash_buffer(struct vb2_digest_context *ctx, + void *buf, size_t sz) +{ + return vb2_digest_extend(ctx, buf, sz); +} + +static int cbfs_extend_hash(struct vb2_digest_context *ctx, + const struct region_device *rdev) +{ + uint8_t buffer[1024]; + size_t sz_left; + size_t offset; + + sz_left = region_device_sz(rdev); + offset = 0; + + while (sz_left) { + int rv; + size_t block_sz = MIN(sz_left, sizeof(buffer)); + + if (rdev_readat(rdev, buffer, offset, block_sz) != block_sz) + return VB2_ERROR_UNKNOWN; + + rv = cbfs_extend_hash_buffer(ctx, buffer, block_sz); + + if (rv) + return rv; + + sz_left -= block_sz; + offset += block_sz; + } + + return VB2_SUCCESS; +} + +/* Include offsets of child regions within the parent into the hash. */ +static int cbfs_extend_hash_with_offset(struct vb2_digest_context *ctx, + const struct region_device *p, + const struct region_device *c) +{ + int32_t soffset; + int rv; + + soffset = rdev_relative_offset(p, c); + + if (soffset < 0) + return VB2_ERROR_UNKNOWN; + + /* All offsets in big endian format. */ + write_be32(&soffset, soffset); + + rv = cbfs_extend_hash_buffer(ctx, &soffset, sizeof(soffset)); + + if (rv) + return rv; + + return cbfs_extend_hash(ctx, c); +} + +/* Hash in the potential CBFS header sitting at the beginning of the CBFS + * region as well as relative offset at the end. */ +static int cbfs_extend_hash_master_header(struct vb2_digest_context *ctx, + const struct region_device *cbfs) +{ + struct region_device rdev; + int rv; + + if (rdev_chain(&rdev, cbfs, 0, sizeof(struct cbfs_header))) + return VB2_ERROR_UNKNOWN; + + rv = cbfs_extend_hash_with_offset(ctx, cbfs, &rdev); + + if (rv) + return rv; + + /* Include potential relative offset at end of region. */ + if (rdev_chain(&rdev, cbfs, region_device_sz(cbfs) - sizeof(int32_t), + sizeof(int32_t))) + return VB2_ERROR_UNKNOWN; + + return cbfs_extend_hash_with_offset(ctx, cbfs, &rdev); +} + +int cbfs_vb2_hash_contents(const struct region_device *cbfs, + enum vb2_hash_algorithm hash_alg, void *digest, + size_t digest_sz) +{ + struct vb2_digest_context ctx; + int rv; + struct cbfsf f; + struct cbfsf *prev; + struct cbfsf *fh; + + rv = vb2_digest_init(&ctx, hash_alg); + + if (rv) + return rv; + + rv = cbfs_extend_hash_master_header(&ctx, cbfs); + if (rv) + return rv; + + prev = NULL; + fh = &f; + + while (1) { + uint32_t ftype; + + rv = cbfs_for_each_file(cbfs, prev, fh); + prev = fh; + + if (rv < 0) + return VB2_ERROR_UNKNOWN; + + /* End of CBFS. */ + if (rv > 0) + break; + + rv = cbfs_extend_hash_with_offset(&ctx, cbfs, &fh->metadata); + + if (rv) + return rv; + + /* Include data contents in hash if file is non-empty. */ + if (cbfsf_file_type(fh, &ftype)) + return VB2_ERROR_UNKNOWN; + + if (ftype == CBFS_TYPE_DELETED || ftype == CBFS_TYPE_DELETED2) + continue; + + rv = cbfs_extend_hash_with_offset(&ctx, cbfs, &fh->data); + + if (rv) + return rv; + } + + return vb2_digest_finalize(&ctx, digest, digest_sz); +} diff --git a/src/commonlib/include/commonlib/cbfs.h b/src/commonlib/include/commonlib/cbfs.h index b0a468c..e1019c6 100644 --- a/src/commonlib/include/commonlib/cbfs.h +++ b/src/commonlib/include/commonlib/cbfs.h @@ -18,6 +18,9 @@ #include <commonlib/cbfs_serialized.h> #include <commonlib/region.h> +/* TODO: remove me! This is for vboot_handoff.c's benefit. */ +#define NEED_VB20_INTERNALS +#include <vb2_api.h> /* Object representing cbfs files. */ struct cbfsf { @@ -52,4 +55,13 @@ static inline void cbfs_file_metadata(struct region_device *metadata, int cbfs_for_each_file(const struct region_device *cbfs, const struct cbfsf *prev, struct cbfsf *fh); +/* + * Perform the vb2 hash over the CBFS region skipping empty file contents. + * Caller is responsiblef or providing the hash algorithm as well as storage + * for the final digest. Return 0 on success or non-zero on error. + */ +int cbfs_vb2_hash_contents(const struct region_device *cbfs, + enum vb2_hash_algorithm hash_alg, void *digest, + size_t digest_sz); + #endif

1 0

New patch to review for coreboot: cbfs: Add type ids for empty files
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12785 -gerrit commit eb4cdf5ecd4d4654485d2bf3447e12956d5956ff Author: Patrick Georgi <pgeorgi(a)google.com> Date: Tue Dec 15 10:09:55 2015 +0100 cbfs: Add type ids for empty files We will soon need to handle empty files. BUG=chromium:445938 BRANCH=tot TEST=none Change-Id: Ia72a4bff7d9bb36f6a6648c3dd89e86593d80761 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- src/commonlib/include/commonlib/cbfs_serialized.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/commonlib/include/commonlib/cbfs_serialized.h b/src/commonlib/include/commonlib/cbfs_serialized.h index edef551..679ff07 100644 --- a/src/commonlib/include/commonlib/cbfs_serialized.h +++ b/src/commonlib/include/commonlib/cbfs_serialized.h @@ -62,6 +62,8 @@ Users are welcome to use any other value for their components */ +#define CBFS_TYPE_DELETED 0x00000000 +#define CBFS_TYPE_DELETED2 0xffffffff #define CBFS_TYPE_STAGE 0x10 #define CBFS_TYPE_PAYLOAD 0x20 #define CBFS_TYPE_OPTIONROM 0x30 @@ -130,7 +132,7 @@ struct cbfs_file { char magic[8]; uint32_t len; uint32_t type; - uint32_t checksum; + uint32_t attributes_offset; uint32_t offset; } __attribute__((packed));

1 0

New patch to review for coreboot: commonlib: prepare for cbfstool inclusion
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12784 -gerrit commit 38a258e72245ef143ab17f008f5b894a23dd98a9 Author: Aaron Durbin <adurbin(a)chromium.org> Date: Tue Dec 15 17:49:12 2015 -0600 commonlib: prepare for cbfstool inclusion Some of the files need to be adjusted so that they can be used both in cbfstool as well as coreboot proper. For coreboot, add a <sys/types.h> file such that proper types can be included from both the tools and coreboot. The other chanes are to accomodate stricter checking in cbfstool. BUG=chrome-os-partner:48412 BUG=chromium:445938 BRANCH=None TEST=Built on glados including tools. Booted. Change-Id: I771c6675c64b8837f775427721dd3300a8fa1bc0 Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> --- src/commonlib/include/commonlib/helpers.h | 6 ++++++ src/commonlib/include/commonlib/region.h | 1 + src/commonlib/mem_pool.c | 2 +- src/commonlib/region.c | 14 ++++++++------ src/include/sys/types.h | 1 + 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/commonlib/include/commonlib/helpers.h b/src/commonlib/include/commonlib/helpers.h index 0e337c9..646a66f 100644 --- a/src/commonlib/include/commonlib/helpers.h +++ b/src/commonlib/include/commonlib/helpers.h @@ -2,7 +2,9 @@ #define COMMONLIB_HELPERS_H /* This file is for helpers for both coreboot firmware and its utilities. */ +#ifndef ARRAY_SIZE #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) +#endif #define ALIGN(x,a) __ALIGN_MASK(x,(__typeof__(x))(a)-1UL) #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask)) @@ -54,4 +56,8 @@ const __typeof__( ((type *)0)->member ) *__mptr = (ptr); \ (type *)( (char *)__mptr - offsetof(type,member) );}) +#ifndef __unused +#define __unused __attribute__((unused)) +#endif + #endif /* COMMONLIB_HELPERS_H */ diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index 35d48ad..a13c66c 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -16,6 +16,7 @@ #ifndef _REGION_H_ #define _REGION_H_ +#include <sys/types.h> #include <stdint.h> #include <stddef.h> #include <commonlib/mem_pool.h> diff --git a/src/commonlib/mem_pool.c b/src/commonlib/mem_pool.c index e42e9cb..cb3e726 100644 --- a/src/commonlib/mem_pool.c +++ b/src/commonlib/mem_pool.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */ +#include <commonlib/helpers.h> #include <commonlib/mem_pool.h> -#include <stdlib.h> void *mem_pool_alloc(struct mem_pool *mp, size_t sz) { diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 2cd273a..84b283b 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */ +#include <commonlib/helpers.h> #include <commonlib/region.h> #include <string.h> @@ -133,16 +134,17 @@ void mem_region_device_init(struct mem_region_device *mdev, void *base, } static void *mdev_mmap(const struct region_device *rd, size_t offset, - size_t size) + size_t size __unused) { const struct mem_region_device *mdev; - mdev = container_of(rd, typeof(*mdev), rdev); + mdev = container_of(rd, __typeof__(*mdev), rdev); return &mdev->base[offset]; } -static int mdev_munmap(const struct region_device *rd, void *mapping) +static int mdev_munmap(const struct region_device * rd __unused, + void *mapping __unused) { return 0; } @@ -152,7 +154,7 @@ static ssize_t mdev_readat(const struct region_device *rd, void *b, { const struct mem_region_device *mdev; - mdev = container_of(rd, typeof(*mdev), rdev); + mdev = container_of(rd, __typeof__(*mdev), rdev); memcpy(b, &mdev->base[offset], size); @@ -177,7 +179,7 @@ void *mmap_helper_rdev_mmap(const struct region_device *rd, size_t offset, struct mmap_helper_region_device *mdev; void *mapping; - mdev = container_of((void *)rd, typeof(*mdev), rdev); + mdev = container_of((void *)rd, __typeof__(*mdev), rdev); mapping = mem_pool_alloc(&mdev->pool, size); @@ -196,7 +198,7 @@ int mmap_helper_rdev_munmap(const struct region_device *rd, void *mapping) { struct mmap_helper_region_device *mdev; - mdev = container_of((void *)rd, typeof(*mdev), rdev); + mdev = container_of((void *)rd, __typeof__(*mdev), rdev); mem_pool_free(&mdev->pool, mapping); diff --git a/src/include/sys/types.h b/src/include/sys/types.h new file mode 100644 index 0000000..fa95d57 --- /dev/null +++ b/src/include/sys/types.h @@ -0,0 +1 @@ +#include "../types.h"

1 0

New patch to review for coreboot: commonlib: add common cbfs parsing logic
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12783 -gerrit commit be707efbfed9d634c63df2a9fdcf9d074636e609 Author: Aaron Durbin <adurbin(a)chromium.org> Date: Tue Dec 15 13:33:51 2015 -0600 commonlib: add common cbfs parsing logic To continue sharing more code between the tools and coreboot proper provide cbfs parsing logic in commonlib. A cbfs_for_each_file() function was added to allow one to act on each file found within a cbfs. cbfs_locate() was updated to use that logic. BUG=chrome-os-partner:48412 BUG=chromium:445938 BRANCH=None TEST=Utilized and booted on glados. Change-Id: I1f23841583e78dc3686f106de9eafe1adbef8c9f Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> --- src/commonlib/Makefile.inc | 5 + src/commonlib/cbfs.c | 176 +++++++++++++++++++++++++++++++++ src/commonlib/include/commonlib/cbfs.h | 55 +++++++++++ src/include/cbfs.h | 28 +----- src/lib/cbfs.c | 72 -------------- 5 files changed, 237 insertions(+), 99 deletions(-) diff --git a/src/commonlib/Makefile.inc b/src/commonlib/Makefile.inc index 929bf8e..4545855 100644 --- a/src/commonlib/Makefile.inc +++ b/src/commonlib/Makefile.inc @@ -10,3 +10,8 @@ ramstage-y += region.c smm-y += region.c ramstage-$(CONFIG_PLATFORM_USES_FSP1_1) += fsp1_1_relocate.c + +bootblock-y += cbfs.c +verstage-y += cbfs.c +romstage-y += cbfs.c +ramstage-y += cbfs.c diff --git a/src/commonlib/cbfs.c b/src/commonlib/cbfs.c new file mode 100644 index 0000000..06d31ed --- /dev/null +++ b/src/commonlib/cbfs.c @@ -0,0 +1,176 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2015 Google Inc. + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <console/console.h> +#include <commonlib/cbfs.h> +#include <commonlib/endian.h> +#include <commonlib/helpers.h> +#include <string.h> + +#if !defined(ERROR) +#define ERROR(x...) printk(BIOS_ERR, "CBFS: " x) +#endif +#if !defined(LOG) +#define LOG(x...) printk(BIOS_INFO, "CBFS: " x) +#endif +#if defined(IS_ENABLED) + +#if IS_ENABLED(CONFIG_DEBUG_CBFS) +#define DEBUG(x...) printk(BIOS_SPEW, "CBFS: " x) +#else +#define DEBUG(x...) +#endif + +#elif !defined(DEBUG) +#define DEBUG(x...) +#endif + +static size_t cbfs_next_offset(const struct region_device *cbfs, + const struct cbfsf *f) +{ + size_t offset; + + if (f == NULL) + return 0; + + /* The region_device objects store absolute offets over the whole + * region. Therefore a relative offset needs to be calculated. */ + offset = rdev_relative_offset(cbfs, &f->data); + offset += region_device_sz(&f->data); + + return ALIGN_UP(offset, CBFS_ALIGNMENT); +} + +static int cbfs_end(const struct region_device *cbfs, size_t offset) +{ + if (offset >= region_device_sz(cbfs)) + return 1; + + return 0; +} + +int cbfs_for_each_file(const struct region_device *cbfs, + const struct cbfsf *prev, struct cbfsf *fh) +{ + size_t offset; + + offset = cbfs_next_offset(cbfs, prev); + + /* Try to scan the entire cbfs region looking for file name. */ + while (1) { + struct cbfs_file file; + const size_t fsz = sizeof(file); + + DEBUG("Checking offset %zx\n", offset); + + /* End of region. */ + if (cbfs_end(cbfs, offset)) + return 1; + + /* Can't read file. Nothing else to do but bail out. */ + if (rdev_readat(cbfs, &file, offset, fsz) != fsz) + break; + + if (memcmp(file.magic, CBFS_FILE_MAGIC, sizeof(file.magic))) { + offset++; + offset = ALIGN_UP(offset, CBFS_ALIGNMENT); + continue; + } + + file.len = read_be32(&file.len); + file.offset = read_be32(&file.offset); + + DEBUG("File @ offset %zx size %x\n", offset, file.len); + + /* Keep track of both the metadata and the data for the file. */ + if (rdev_chain(&fh->metadata, cbfs, offset, file.offset)) + break; + + if (rdev_chain(&fh->data, cbfs, offset + file.offset, file.len)) + break; + + /* Success. */ + return 0; + } + + return -1; +} + +int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, + const char *name, uint32_t *type) +{ + struct cbfsf *prev; + + LOG("Locating '%s'\n", name); + + prev = NULL; + + while (1) { + int ret; + char *fname; + int name_match; + const size_t fsz = sizeof(struct cbfs_file); + + ret = cbfs_for_each_file(cbfs, prev, fh); + prev = fh; + + /* Either failed to read or hit the end of the region. */ + if (ret < 0 || ret > 0) + break; + + fname = rdev_mmap(&fh->metadata, fsz, + region_device_sz(&fh->metadata) - fsz); + + if (fname == NULL) + break; + + name_match = !strcmp(fname, name); + rdev_munmap(&fh->metadata, fname); + + if (!name_match) { + DEBUG(" Unmatched '%s' at %zx\n", fname, + rdev_relative_offset(cbfs, &fh->metadata)); + continue; + } + + if (type != NULL) { + uint32_t ftype; + + if (rdev_readat(&fh->metadata, &ftype, + offsetof(struct cbfs_file, type), + sizeof(ftype)) != sizeof(ftype)) + break; + + ftype = read_be32(&ftype); + + if (*type != ftype) { + DEBUG(" Unmatched type %x at %zx\n", ftype, + rdev_relative_offset(cbfs, + &fh->metadata)); + continue; + } + } + + LOG("Found @ offset %zx size %zx\n", + rdev_relative_offset(cbfs, &fh->metadata), + region_device_sz(&fh->data)); + + /* Success. */ + return 0; + } + + LOG("'%s' not found.\n", name); + return -1; +} diff --git a/src/commonlib/include/commonlib/cbfs.h b/src/commonlib/include/commonlib/cbfs.h new file mode 100644 index 0000000..b0a468c --- /dev/null +++ b/src/commonlib/include/commonlib/cbfs.h @@ -0,0 +1,55 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2015 Google Inc. + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _COMMONLIB_CBFS_H_ +#define _COMMONLIB_CBFS_H_ + +#include <commonlib/cbfs_serialized.h> +#include <commonlib/region.h> + +/* Object representing cbfs files. */ +struct cbfsf { + struct region_device metadata; + struct region_device data; +}; + +/* Locate file by name and optional type. Returns 0 on succcess else < 0 on + * error.*/ +int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, + const char *name, uint32_t *type); + +static inline void cbfs_file_data(struct region_device *data, + const struct cbfsf *file) +{ + rdev_chain(data, &file->data, 0, region_device_sz(&file->data)); +} + +static inline void cbfs_file_metadata(struct region_device *metadata, + const struct cbfsf *file) +{ + rdev_chain(metadata, &file->metadata, 0, + region_device_sz(&file->metadata)); +} + +/* + * Provide a handle to each cbfs file within a cbfs. The prev pointer represents + * the previous file (NULL on first invocation). The next object gets filled + * out with the next file. This returns < 0 on error, 0 on finding the next + * file, and > 0 at end of cbfs. + */ +int cbfs_for_each_file(const struct region_device *cbfs, + const struct cbfsf *prev, struct cbfsf *fh); + +#endif diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 7848d6d..0d95387 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -16,19 +16,9 @@ #ifndef _CBFS_H_ #define _CBFS_H_ -#include <commonlib/cbfs_serialized.h> -#include <commonlib/region.h> +#include <commonlib/cbfs.h> #include <program_loading.h> -/* - * CBFS operations consist of the following concepts: - * - region_device for the boot media - * - cbfsd which is a descriptor for representing a cbfs instance - */ - -/* Object representing cbfs files. */ -struct cbfsf; - /*********************************************** * Perform CBFS operations on the boot device. * ***********************************************/ @@ -48,28 +38,12 @@ void *cbfs_boot_map_with_leak(const char *name, uint32_t type, size_t *size); /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); -/* Locate file by name and optional type. Returns 0 on succcess else < 0 on - * error.*/ -int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, - const char *name, uint32_t *type); - /***************************************************************** * Support structures and functions. Direct field access should * * only be done by implementers of cbfs regions -- Not the above * * API. * *****************************************************************/ -struct cbfsf { - struct region_device metadata; - struct region_device data; -}; - -static inline void cbfs_file_data(struct region_device *data, - const struct cbfsf *file) -{ - rdev_chain(data, &file->data, 0, region_device_sz(&file->data)); -} - /* The cbfs_props struct describes the properties associated with a CBFS. */ struct cbfs_props { /* CBFS starts at the following offset within the boot region. */ diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index d68aa39..55a8536 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -68,78 +68,6 @@ void *cbfs_boot_map_with_leak(const char *name, uint32_t type, size_t *size) return rdev_mmap(&fh.data, 0, fsize); } -int cbfs_locate(struct cbfsf *fh, const struct region_device *cbfs, - const char *name, uint32_t *type) -{ - size_t offset = 0; - - LOG("Locating '%s'\n", name); - - /* Try to scan the entire cbfs region looking for file name. */ - while (1) { - struct cbfs_file file; - const size_t fsz = sizeof(file); - char *fname; - int name_match; - size_t datasz; - - DEBUG("Checking offset %zx\n", offset); - - /* Can't read file. Nothing else to do but bail out. */ - if (rdev_readat(cbfs, &file, offset, fsz) != fsz) - break; - - if (memcmp(file.magic, CBFS_FILE_MAGIC, sizeof(file.magic))) { - offset++; - offset = ALIGN_UP(offset, CBFS_ALIGNMENT); - continue; - } - - file.len = ntohl(file.len); - file.type = ntohl(file.type); - file.offset = ntohl(file.offset); - - /* See if names match. */ - fname = rdev_mmap(cbfs, offset + fsz, file.offset - fsz); - - if (fname == NULL) - break; - - name_match = !strcmp(fname, name); - rdev_munmap(cbfs, fname); - - if (!name_match) { - DEBUG(" Unmatched '%s' at %zx\n", fname, offset); - offset += file.offset + file.len; - offset = ALIGN_UP(offset, CBFS_ALIGNMENT); - continue; - } - - if (type != NULL && *type != file.type) { - DEBUG(" Unmatched type %x at %zx\n", file.type, offset); - offset += file.offset + file.len; - offset = ALIGN_UP(offset, CBFS_ALIGNMENT); - continue; - } - - LOG("Found @ offset %zx size %x\n", offset, file.len); - /* File and type match. Keep track of both the metadata and - * the data for the file. */ - if (rdev_chain(&fh->metadata, cbfs, offset, file.offset)) - break; - offset += file.offset; - datasz = file.len; - if (rdev_chain(&fh->data, cbfs, offset, datasz)) - break; - - /* Success. */ - return 0; - } - - LOG("'%s' not found.\n", name); - return -1; -} - static size_t inflate(void *src, void *dst) { if (ENV_BOOTBLOCK || ENV_VERSTAGE)

1 0

New patch to review for coreboot: commonlib: add rdev_relative_offset()
by Aaron Durbin 21 Dec '15

21 Dec '15

Aaron Durbin (adurbin(a)chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12782 -gerrit commit 420d11a033576d63604be45af116e091e1011686 Author: Aaron Durbin <adurbin(a)chromium.org> Date: Tue Dec 15 13:29:41 2015 -0600 commonlib: add rdev_relative_offset() Provide a helper function which returns the relative offset between 2 region_devices which are parent and child. BUG=chrome-os-partner:48412 BUG=chromium:445938 BRANCH=None TEST=Utilized and booted on glados. Change-Id: Ie0041b33e73a6601748f1289e98b6f1f8756eb11 Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> --- src/commonlib/include/commonlib/region.h | 7 +++++++ src/commonlib/region.c | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index cc4ee28..35d48ad 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -121,6 +121,13 @@ static inline void *rdev_mmap_full(const struct region_device *rd) return rdev_mmap(rd, 0, region_device_sz(rd)); } +/* + * Compute relative offset of the child (c) w.r.t. the parent (p). Returns < 0 + * when child is not within the parent's region. + */ +ssize_t rdev_relative_offset(const struct region_device *p, + const struct region_device *c); + struct mem_region_device { char *base; struct region_device rdev; diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 8b0b0d9..2cd273a 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -48,6 +48,18 @@ static const struct region_device *rdev_root(const struct region_device *rdev) return rdev->root; } +ssize_t rdev_relative_offset(const struct region_device *p, + const struct region_device *c) +{ + if (rdev_root(p) != rdev_root(c)) + return -1; + + if (!is_subregion(&p->region, &c->region)) + return -1; + + return region_device_offset(c) - region_device_offset(p); +} + void *rdev_mmap(const struct region_device *rd, size_t offset, size_t size) { const struct region_device *rdev;

1 0

New patch to review for coreboot: soc/intel/fsp_baytrail: Make sure i2c bus is < 7
by Martin Roth 21 Dec '15

21 Dec '15

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12781 -gerrit commit 9ba3b934b670e678639f2931399bdf2aaad70713 Author: Martin Roth <martinroth(a)google.com> Date: Mon Dec 21 13:14:58 2015 -0700 soc/intel/fsp_baytrail: Make sure i2c bus is < 7 Baytrail has I2c Busses 0 to 6, so is supposed to error out if the I2c driver is called with 7 or greater. Due to an off-by-one error it could be called with bus 7. Fixes coverity warning: CID 1287074 (#1 of 1): Out-of-bounds read (OVERRUN) 3. overrun-local: Overrunning array base_adr of 7 4-byte elements at element index 7 (byte offset 28) using index bus (which evaluates to 7). Change-Id: I7caec60298cf27bd669796e0e05e4a896f92befd Signed-off-by: Martin Roth <martinroth(a)google.com> --- src/soc/intel/fsp_baytrail/i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/soc/intel/fsp_baytrail/i2c.c b/src/soc/intel/fsp_baytrail/i2c.c index 6cf07a4..c6c8f65 100644 --- a/src/soc/intel/fsp_baytrail/i2c.c +++ b/src/soc/intel/fsp_baytrail/i2c.c @@ -102,7 +102,7 @@ int i2c_init(unsigned bus) I2C6_MEM_BASE}; char *base_ptr; /* Ensure the desired device is valid */ - if (bus > ARRAY_SIZE(base_adr)) { + if (bus >= ARRAY_SIZE(base_adr)) { printk(BIOS_ERR, "I2C: Only I2C controllers 0...6 are available.\n"); return I2C_ERR; }

1 0

New patch to review for coreboot: drivers/intel/fsp1_0/fsp_util.c: Fix indentation
by Martin Roth 21 Dec '15

21 Dec '15

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12780 -gerrit commit e1014e11dda88c7d21cde78dceccff9d67a4aa68 Author: Martin Roth <martinroth(a)google.com> Date: Mon Dec 21 13:01:45 2015 -0700 drivers/intel/fsp1_0/fsp_util.c: Fix indentation - Also update post code comment to keep under 80 characters. Change-Id: Id0fd0ee5660f2628fe33188855bebb6e3eea8d2e Signed-off-by: Martin Roth <martinroth(a)google.com> --- src/drivers/intel/fsp1_0/fsp_util.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/drivers/intel/fsp1_0/fsp_util.c b/src/drivers/intel/fsp1_0/fsp_util.c index 85ee9a9..a3fef2d 100644 --- a/src/drivers/intel/fsp1_0/fsp_util.c +++ b/src/drivers/intel/fsp1_0/fsp_util.c @@ -214,10 +214,11 @@ void print_fsp_info(void) { if (fsp_header_ptr == NULL) fsp_header_ptr = (void *)find_fsp(); - if ((u32)fsp_header_ptr < 0xff) { - post_code(0x4F); /* output something in case there is no serial */ - die("Can't find the FSP!\n"); - } + + if ((u32)fsp_header_ptr < 0xff) { + post_code(0x4F); /* post code in case there is no serial */ + die("Can't find the FSP!\n"); + } if (FspHobListPtr == NULL) { FspHobListPtr = (void*)*((u32*) cbmem_find(CBMEM_ID_HOB_POINTER));

1 0

New patch to review for coreboot: cpu/allwinner/a10: Fix I2c speed calculation
by Martin Roth 21 Dec '15

21 Dec '15

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12779 -gerrit commit 80e09cbb309bc8afaadf5af11e2b16225de818ba Author: Martin Roth <martinroth(a)google.com> Date: Mon Dec 21 12:51:40 2015 -0700 cpu/allwinner/a10: Fix I2c speed calculation Looking at the A10 datasheet, N should go in bits 2:0, but was being cleared by shifting it left by three bits, then anding it with 7. Fixes coverity warning: CID 1241888 (#1 of 1): Wrong operator used (CONSTANT_EXPRESSION_RESULT) operator_confusion: (n << 3) & (7U /* 7 << 0 */) is always 0 regardless of the values of its operands. This occurs as the bitwise second operand of '|'. Change-Id: I17e71a73adf37a62607e8e5865b1da749d7278aa Signed-off-by: Martin Roth <martinroth(a)google.com> --- src/cpu/allwinner/a10/twi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/allwinner/a10/twi.h b/src/cpu/allwinner/a10/twi.h index c5f2974..3dbb302 100644 --- a/src/cpu/allwinner/a10/twi.h +++ b/src/cpu/allwinner/a10/twi.h @@ -39,7 +39,7 @@ enum twi_status { #define TWI_CLK_M_MASK (0xf << 3) #define TWI_CLK_M(m) (((m - 1) << 3) & TWI_CLK_M_MASK) #define TWI_CLK_N_MASK (0x7 << 0) -#define TWI_CLK_N(n) (((n) << 3) & TWI_CLK_N_MASK) +#define TWI_CLK_N(n) ((n) & TWI_CLK_N_MASK) struct a1x_twi { u32 addr; /**< 0x00: Slave address */

1 0

← Newer
1
...
15
16
17
18
19
20
21
...
76
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2015