Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48811 )
Change subject: fmaptool: Add option to dump fmap ......................................................................
fmaptool: Add option to dump fmap
The fmap library brings all the code to dump the fmap in human readable form to the user, but coreboot does not include a utility that lets you read the fmap from an image. fmaptool is the ideal candidate for dealing with fmaps, so add
$ fmaptool -d <romimage>
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: If255f0c0bf7be3910eca67340633491bdc0a9e7e --- M util/cbfstool/fmaptool.c 1 file changed, 57 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/48811/1
diff --git a/util/cbfstool/fmaptool.c b/util/cbfstool/fmaptool.c index 4ba5967..a5ddfb9 100644 --- a/util/cbfstool/fmaptool.c +++ b/util/cbfstool/fmaptool.c @@ -8,6 +8,10 @@ #include <stdio.h> #include <string.h> #include <unistd.h> +#include <fcntl.h> +#include <errno.h> +#include <sys/stat.h> +#include <sys/mman.h>
#define STDIN_FILENAME_SENTINEL "-"
@@ -40,13 +44,15 @@ stderr); fputs("\nUSAGE:\n", stderr); fprintf(stderr, - "\t%s [-h <header output file>] [-R <region output file>] <fmd input file> <binary output file>\n", - invoked_as); + "\t%s [-h <header output file>] [-R <region output file>]" + " <fmd input file> <binary output file>\n" + "\t%s -d romfile\n", invoked_as, invoked_as); fputs("\nMANDATORY ARGUMENTS:\n", stderr); fprintf(stderr, "<fmd input file> may be '%s' to read from standard input\n", STDIN_FILENAME_SENTINEL); fputs("<binary output file> must be a regular file\n", stderr); + fputs("<romfile> rom image with FMAP to be dumped.\n", stderr); fputs("\nOPTIONAL SWITCHES:\n", stderr); fprintf(stderr, "-h\tAlso produce a C header defining %s to the FMAP section's flash offset.\n", @@ -60,6 +66,51 @@ stderr); }
+static int dump_fmap(char *filename) +{ + int fd; + int rc = FMAPTOOL_EXIT_SUCCESS; + struct stat s; + uint8_t *blob; + off_t fmap_offset; + + fd = open(filename, O_RDONLY); + if (fd < 0) { + printf("unable to open file "%s": %s\n", + filename, strerror(errno)); + rc = FMAPTOOL_EXIT_BAD_INPUT_PATH; + goto do_exit_1; + } + if (fstat(fd, &s) < 0) { + printf("unable to stat file "%s": %s\n", + filename, strerror(errno)); + rc = FMAPTOOL_EXIT_BAD_INPUT_PATH; + goto do_exit_1; + } + + blob = mmap(NULL, s.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + if (blob == MAP_FAILED) { + printf("unable to mmap file %s: %s\n", + filename, strerror(errno)); + rc = FMAPTOOL_EXIT_BAD_INPUT_PATH; + goto do_exit_1; + } + + fmap_offset = fmap_find(blob, s.st_size); + if (fmap_offset < 0) { + rc = FMAPTOOL_EXIT_MISSING_FMAP_SECTION; + goto do_exit_2; + } else { + fmap_print((struct fmap *)(blob + fmap_offset)); + } + +do_exit_2: + munmap(blob, s.st_size); +do_exit_1: + close(fd); + return rc; +} + static void list_cbfs_section_names(FILE *out) { cbfs_section_iterator_t cbfs_it = cbfs_sections_iterator(); @@ -172,7 +223,7 @@
bool show_usage = false; int each_arg; - while (!show_usage && (each_arg = getopt(argc, argv, ":h:R:")) != -1) { + while (!show_usage && (each_arg = getopt(argc, argv, ":h:R:d:")) != -1) { switch (each_arg) { case 'h': args.header_filename = optarg; @@ -185,6 +236,9 @@ optopt); show_usage = true; break; + case 'd': + dump_fmap(optarg); + return FMAPTOOL_EXIT_SUCCESS; default: fprintf(stderr, "-%c: Unexpected command-line switch\n", optopt);
Hello Furquan Shaikh, Martin Roth, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48811
to look at the new patch set (#2).
Change subject: fmaptool: Add option to dump fmap ......................................................................
fmaptool: Add option to dump fmap
The fmap library brings all the code to dump the fmap in human readable form to the user, but coreboot does not include a utility that lets you read the fmap from an image. fmaptool is the ideal candidate for dealing with fmaps, so add
$ fmaptool -d <romimage>
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: If255f0c0bf7be3910eca67340633491bdc0a9e7e --- M util/cbfstool/fmaptool.c 1 file changed, 57 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/48811/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48811 )
Change subject: fmaptool: Add option to dump fmap ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG@10 PS2, Line 10: coreboot does not include a utility : that lets you read the fmap from an image It does, actually? coreboot can build futility and `futility dump_fmap -h` does exactly that. Can we just stick to that rather than implementing a second one? (fmaptool in particular is intended to be a build-only tool that's not normally going to be installed on target hosts, so I think it's not that suitable for this.)
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48811 )
Change subject: fmaptool: Add option to dump fmap ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG@10 PS2, Line 10: coreboot does not include a utility : that lets you read the fmap from an image
It does, actually? coreboot can build futility and `futility dump_fmap -h` does exactly that. […]
Hm.. ok I missed that. I suggest to drop this functionality from futility. Futility unfortunately only runs on a small subset of machines. It seems like having the fmap tool dump fmaps makes a lot more sense than the Chrome OS tool doing all of that and more (and as a plus, it's only controlled by Chrome OS folks)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48811 )
Change subject: fmaptool: Add option to dump fmap ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48811/2//COMMIT_MSG@10 PS2, Line 10: coreboot does not include a utility : that lets you read the fmap from an image
Hm.. ok I missed that. I suggest to drop this functionality from futility. […]
I mean... we won't drop this from futility, it's been there for ages and we use it all the time. Not sure what you mean by small subset of machines? futility should run everywhere other coreboot build utilities run, there's no fundamental difference there... of course we don't have continuous integration for anything other than x86 Linux, but that's a problem with the coreboot infra, not specific to futility. If you find that futility doesn't build or work on a specific platform, we're happy to take fixes (e.g. Idwer submitted some to make it run on FreeBSD recently).
If you really want to have this somewhere else I don't mind, especially since it's just a small call to an existing library. It just seems like unnecessary duplication to me. Especially since the futility version looks much better (dump_fmap -h gives you a nice indented view of nested sections, this just prints them all very raw and so verbose it fills multiple screens). And also because this is just an intermediate build tool that's not really getting installed or used independently anywhere. If you want this functionality somewhere else, I think putting it into cbfstool would make more sense (it's pretty much an FMAP manipulator with cbfstool read / cbfstool write already anyway).
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48811?usp=email )
Change subject: fmaptool: Add option to dump fmap ......................................................................
Abandoned
-
Julius Werner (Code Review) -
Stefan Reinauer (Code Review)