- flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Fix linking with libpayload
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Hello Philippe Mathieu-Daudé, Paul Menzel, David Hendricks, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/18738 to look at the new patch set (#16). Change subject: Fix linking with libpayload ...................................................................... Fix linking with libpayload o Move flashbuses_to_text() to flashrom.c, it's not a cli function. o Guard `!defined(HAVE_STRNLEN)`. This guard was introduced in 23e10b87 (Add a bunch of new/tested stuff and various small changes 24) to support older BSDs. It's probably completely broken because HAVE_STRNLEN is presumably a GNU autotools thing. But we can't fix it without retesting these older BSDs. Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M cli_common.c M flash.h M flashrom.c M helpers.c 4 files changed, 36 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/18738/16 -- To view, visit https://review.coreboot.org/18738 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Gerrit-Change-Number: 18738 Gerrit-PatchSet: 16 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Fix linking with libpayload
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18738 ) Change subject: Fix linking with libpayload ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/#/c/18738/6//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/18738/6//COMMIT_MSG@9 PS6, Line 9: o flashbuses_to_text() is a cli function and should actually be moved : or not be called from flashrom.c. > Why not move flashbuses_to_text() to flashrom.c? Probably because I couldn't bear to move broken code (all these possible NULL pointers) without fixing it. But it seems the right thing, so I moved it now. -- To view, visit https://review.coreboot.org/18738 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Gerrit-Change-Number: 18738 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 14 Jun 2017 11:51:52 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Fix linking with libpayload
by build bot (Jenkins) (Code Review) 14 Jun '17

14 Jun '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18738 ) Change subject: Fix linking with libpayload ...................................................................... Patch Set 15: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom_gerrit/355/ : FAILURE https://qa.coreboot.org/job/flashrom-customrules/365/ : SUCCESS -- To view, visit https://review.coreboot.org/18738 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Gerrit-Change-Number: 18738 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 14 Jun 2017 11:49:03 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Fix linking with libpayload
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Hello Philippe Mathieu-Daudé, Paul Menzel, David Hendricks, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/18738 to look at the new patch set (#15). Change subject: Fix linking with libpayload ...................................................................... Fix linking with libpayload o Move flashbuses_to_text() to flashrom.c, it's not a cli function. o Guard `!defined(HAVE_STRNLEN)`. This guard was introduced in 23e10b87 (Add a bunch of new/tested stuff and various small changes 24) to support older BSDs. It's probably completely broken because HAVE_STRNLEN is presumably a GNU autotools thing. But we can't fix it without retesting these older BSDs. Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M cli_common.c M flash.h M flashrom.c M helpers.c 4 files changed, 36 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/18738/15 -- To view, visit https://review.coreboot.org/18738 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Gerrit-Change-Number: 18738 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Fix linking with libpayload
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18738 ) Change subject: Fix linking with libpayload ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/#/c/18738/6/helpers.c File helpers.c: https://review.coreboot.org/#/c/18738/6/helpers.c@95 PS6, Line 95: #if defined(__DJGPP__) > Hmm, I wonder if it's still useful to have a check for glibc compatibility These feature_test_macros(7) work the other way around: You define them before including any header file and the header file declares the guarded functions (see dmi.c for an example). So in case of glibc, we can just assume that we have strnlen(). I've done some more testing and investigation: The HAVE_STRNLEN check was introduced for older BSDs in 23e10b8. But I guess it was copied from examples that assume GNU autotools are used. It's working because the linker seems to ignore the conflicting imple- mentation. I'll extend the `!defined(HAVE_STRNLEN)` with a libpayload guard. Just removing it would break these older BSDs again. -- To view, visit https://review.coreboot.org/18738 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987 Gerrit-Change-Number: 18738 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 14 Jun 2017 11:28:39 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Enable writes with active ME
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18740 ) Change subject: Enable writes with active ME ...................................................................... Patch Set 15: I think we should take the least-options route. Enable `--noverify-all` automatically and report that, including the warning to not include the ME region. If the user didn't specify sane regions for this case, we'll just fail to read the ME region and bail out at that point. Easiest way to do this would be to move the flags into the not yet existing programmer (or global) context. So I'll postpone this change until we made a decision on that front. The current state should al- ready work with `-p internal:ich_spi_force=yes --noverify-all`. -- To view, visit https://review.coreboot.org/18740 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef Gerrit-Change-Number: 18740 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 14 Jun 2017 10:48:16 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Whitelist Roda/RV11 laptop
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/18739 ) Change subject: Whitelist Roda/RV11 laptop ...................................................................... Whitelist Roda/RV11 laptop Change-Id: I036c1f8cb914c8e3cca9d17eb221b582d7414ae9 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/18739 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M board_enable.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index c85bd15..b6e17ef 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2458,6 +2458,7 @@ {0x10DE, 0x0360, 0x1462, 0x7250, 0x10DE, 0x0368, 0x1462, 0x7250, NULL, NULL, NULL, P3, "MSI", "MS-7250 (K9N SLI)", 0, OK, nvidia_mcp_gpio2_raise}, {0x1011, 0x0019, 0xaa55, 0xaa55, 0x8086, 0x7190, 0, 0, NULL, NULL, NULL, P3, "Nokia", "IP530", 0, OK, fdc37b787_gpio50_raise_3f0}, {0x8086, 0x3B30, 0x1025, 0x0379, 0x8086, 0x3B09, 0x1025, 0x0379, "^EasyNote LM85$", NULL, NULL, P2, "Packard Bell","EasyNote LM85", 0, OK, p2_whitelist_laptop}, + {0x8086, 0x0154, 0x8086, 0x0154, 0x8086, 0x1e55, 0x8086, 0x1e55, "RV11$", "Roda", "Lizard RV11", P2, "Roda", "RV11", 0, OK, p2_whitelist_laptop}, {0x8086, 0x24d3, 0x144d, 0xb025, 0x8086, 0x1050, 0x144d, 0xb025, NULL, NULL, NULL, P3, "Samsung", "Polaris 32", 0, OK, intel_ich_gpio21_raise}, {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", P3, "Shuttle", "AK31", 0, OK, w836xx_memw_enable_2e}, {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, P3, "Shuttle", "AK38N", 256, OK, NULL}, -- To view, visit https://review.coreboot.org/18739 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I036c1f8cb914c8e3cca9d17eb221b582d7414ae9 Gerrit-Change-Number: 18739 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Add option to read ROM layout from IFD Add an option --ifd to read the ROM layout from an Intel Firmware Descriptor (IFD). Works the same as the -l option, if given, -i specifies the images to update. v2: o Rebased on libflashrom, use libflashrom interface. o Use functions from ich_descriptors.c. v3: o Move ich_descriptors.o to LIB_OBJS, thus build it independent of arch and programmers. o Bail out if we aren't compiled for little endian. o Update flashrom.8.tmpl. v4: o Incorporated David's comments. o Removed single-character `-d` option. v5: Changed region names to match the output of `ifdtool --layout ...` Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/17953 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M Makefile M cli_classic.c M flash.h M flashrom.8.tmpl M flashrom.c M ich_descriptors.c M ich_descriptors.h M layout.c M layout.h M libflashrom.c M libflashrom.h 11 files changed, 195 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/Makefile b/Makefile index 01a179d..65026da 100644 --- a/Makefile +++ b/Makefile @@ -519,7 +519,7 @@ ############################################################################### # Library code. -LIB_OBJS = libflashrom.o layout.o flashrom.o udelay.o programmer.o helpers.o +LIB_OBJS = libflashrom.o layout.o flashrom.o udelay.o programmer.o helpers.o ich_descriptors.o ############################################################################### # Frontend related stuff. @@ -731,7 +731,7 @@ PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o internal.o ifeq ($(ARCH), x86) PROGRAMMER_OBJS += it87spi.o it85spi.o sb600spi.o amd_imc.o wbsio_spi.o mcp6x_spi.o -PROGRAMMER_OBJS += ichspi.o ich_descriptors.o dmi.o +PROGRAMMER_OBJS += ichspi.o dmi.o ifeq ($(CONFIG_INTERNAL_DMI), yes) FEATURE_CFLAGS += -D'CONFIG_INTERNAL_DMI=1' endif diff --git a/cli_classic.c b/cli_classic.c index 7e3dfd5..0a09cfd 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -42,7 +42,7 @@ "-z|" #endif "-p <programmername>[:<parameters>] [-c <chipname>]\n" - "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-N] [-f]]\n" + "[-E|(-r|-w|-v) <file>] [(-l <layoutfile>|--ifd) [-i <imagename>]...] [-n] [-N] [-f]]\n" "[-V[V[V]]] [-o <logfile>]\n\n", name); printf(" -h | --help print this help text\n" @@ -57,6 +57,7 @@ " -n | --noverify don't auto-verify\n" " -N | --noverify-all verify included regions only (cf. -i)\n" " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" + " --ifd read layout from an Intel Firmware Descriptor\n" " -i | --image <name> only flash image <name> from flash layout\n" " -o | --output <logfile> log output to <logfile>\n" " -L | --list-supported print supported devices\n" @@ -99,12 +100,13 @@ struct flashctx *fill_flash; const char *name; int namelen, opt, i, j; - int startchip = -1, chipcount = 0, option_index = 0, force = 0; + int startchip = -1, chipcount = 0, option_index = 0, force = 0, ifd = 0; #if CONFIG_PRINT_WIKI == 1 int list_supported_wiki = 0; #endif int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; + struct flashrom_layout *layout = NULL; enum programmer prog = PROGRAMMER_INVALID; int ret = 0; @@ -120,6 +122,7 @@ {"verbose", 0, NULL, 'V'}, {"force", 0, NULL, 'f'}, {"layout", 1, NULL, 'l'}, + {"ifd", 0, NULL, 0x0100}, {"image", 1, NULL, 'i'}, {"list-supported", 0, NULL, 'L'}, {"list-supported-wiki", 0, NULL, 'z'}, @@ -220,7 +223,18 @@ "more than once. Aborting.\n"); cli_classic_abort_usage(); } + if (ifd) { + fprintf(stderr, "Error: --layout and --ifd both specified. Aborting.\n"); + cli_classic_abort_usage(); + } layoutfile = strdup(optarg); + break; + case 0x0100: + if (layoutfile) { + fprintf(stderr, "Error: --layout and --ifd both specified. Aborting.\n"); + cli_classic_abort_usage(); + } + ifd = 1; break; case 'i': tempstr = strdup(optarg); @@ -376,7 +390,7 @@ ret = 1; goto out; } - if (process_include_args()) { + if (!ifd && process_include_args(get_global_layout())) { ret = 1; goto out; } @@ -529,9 +543,15 @@ goto out_shutdown; } - if (layoutfile) - flashrom_layout_set(fill_flash, get_global_layout()); + if (layoutfile) { + layout = get_global_layout(); + } else if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) || + process_include_args(layout))) { + ret = 1; + goto out_shutdown; + } + flashrom_layout_set(fill_flash, layout); flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force); flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch); flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it); @@ -551,6 +571,8 @@ else if (verify_it) ret = do_verify(fill_flash, filename); + flashrom_layout_release(layout); + out_shutdown: programmer_shutdown(); out: diff --git a/flash.h b/flash.h index 7c7ecd5..47c32f4 100644 --- a/flash.h +++ b/flash.h @@ -287,6 +287,8 @@ int selfcheck(void); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); +int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); +void finalize_flash_access(struct flashctx *); int do_read(struct flashctx *, const char *filename); int do_erase(struct flashctx *); int do_write(struct flashctx *, const char *const filename); @@ -354,7 +356,6 @@ /* layout.c */ int register_include_arg(char *name); -int process_include_args(void); int read_romlayout(const char *name); int normalize_romentries(const struct flashctx *flash); void layout_cleanup(void); diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 34e7f02..143d76c 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -48,8 +48,8 @@ \fB\-p\fR <programmername>[:<parameters>] [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] \ [\fB\-c\fR <chipname>] - [\fB\-l\fR <file> [\fB\-i\fR <image>]] [\fB\-n\fR] [\fB\-N\fR] \ -[\fB\-f\fR]] + [(\fB\-l\fR <file>|\fB\-\-ifd\fR) [\fB\-i\fR <image>]] \ +[\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR]] [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION .B flashrom @@ -195,6 +195,22 @@ .sp Overlapping sections are not supported. .TP +.B "\-\-ifd" +Read ROM layout from Intel Firmware Descriptor. +.sp +flashrom supports ROM layouts given by an Intel Firmware Descriptor +(IFD). The on-chip descriptor will be read and used to generate the +layout. If you need to change the layout, you have to update the IFD +only first. +.sp +The following ROM images may be present in an IFD: +.sp + fd the IFD itself + bios the host firmware aka. BIOS + me Intel Management Engine firmware + gbe gigabit ethernet firmware + pd platform specific data +.TP .B "\-i, \-\-image <imagename>" Only flash region/image .B <imagename> diff --git a/flashrom.c b/flashrom.c index 325a0c1..503a199 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2168,9 +2168,9 @@ return 0; } -static int prepare_flash_access(struct flashctx *const flash, - const bool read_it, const bool write_it, - const bool erase_it, const bool verify_it) +int prepare_flash_access(struct flashctx *const flash, + const bool read_it, const bool write_it, + const bool erase_it, const bool verify_it) { if (chip_safety_check(flash, flash->flags.force, read_it, write_it, erase_it, verify_it)) { msg_cerr("Aborting.\n"); @@ -2193,7 +2193,7 @@ return 0; } -static void finalize_flash_access(struct flashctx *const flash) +void finalize_flash_access(struct flashctx *const flash) { unmap_flash(flash); } diff --git a/ich_descriptors.c b/ich_descriptors.c index a0b2c9a..b6453ce 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -23,6 +23,7 @@ #ifdef ICH_DESCRIPTORS_FROM_DUMP_ONLY #include <stdio.h> +#include <string.h> #define print(t, ...) printf(__VA_ARGS__) #endif @@ -38,7 +39,7 @@ #include "programmer.h" #ifndef min -#define min(a, b) (a < b) ? a : b +#define min(a, b) (((a) < (b)) ? (a) : (b)) #endif void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl) @@ -916,4 +917,42 @@ msg_pdbg2(" done.\n"); return ICH_RET_OK; } + +/** + * @brief Read a layout from the dump of an Intel ICH descriptor. + * + * @param layout Pointer where to store the layout. + * @param dump The descriptor dump to read from. + * @param len The length of the descriptor dump. + * + * @return 0 on success, + * 1 if the descriptor couldn't be parsed. + */ +int layout_from_ich_descriptors(struct ich_layout *const layout, const void *const dump, const size_t len) +{ + static const char *regions[] = { "fd", "bios", "me", "gbe", "pd" }; + + struct ich_descriptors desc; + if (read_ich_descriptors_from_dump(dump, len, &desc)) + return 1; + + memset(layout, 0x00, sizeof(*layout)); + + size_t i, j; + for (i = 0, j = 0; i < min(desc.content.NR + 1, ARRAY_SIZE(regions)); ++i) { + const chipoff_t base = ICH_FREG_BASE(desc.region.FLREGs[i]); + const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]) + 0xfff; + if (limit <= base) + continue; + layout->entries[j].start = base; + layout->entries[j].end = limit; + layout->entries[j].included = false; + snprintf(layout->entries[j].name, sizeof(layout->entries[j].name), "%s", regions[i]); + ++j; + } + layout->base.entries = layout->entries; + layout->base.num_entries = j; + return 0; +} + #endif /* ICH_DESCRIPTORS_FROM_DUMP_ONLY */ diff --git a/ich_descriptors.h b/ich_descriptors.h index e355e54..ecf44bf 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -584,4 +584,6 @@ int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc); int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx); +int layout_from_ich_descriptors(struct ich_layout *, const void *dump, size_t len); + #endif /* __ICH_DESCRIPTORS_H__ */ diff --git a/layout.c b/layout.c index 2d53295..9bf0b03 100644 --- a/layout.c +++ b/layout.c @@ -35,7 +35,7 @@ static char *include_args[MAX_ROMLAYOUT]; static int num_include_args = 0; /* the number of valid include_args. */ -const struct flashrom_layout *get_global_layout(void) +struct flashrom_layout *get_global_layout(void) { return &layout; } @@ -132,17 +132,17 @@ } /* returns the index of the entry (or a negative value if it is not found) */ -static int find_romentry(char *name) +static int find_romentry(struct flashrom_layout *const l, char *name) { int i; - if (layout.num_entries == 0) + if (l->num_entries == 0) return -1; msg_gspew("Looking for region \"%s\"... ", name); - for (i = 0; i < layout.num_entries; i++) { - if (!strcmp(layout.entries[i].name, name)) { - layout.entries[i].included = 1; + for (i = 0; i < l->num_entries; i++) { + if (!strcmp(l->entries[i].name, name)) { + l->entries[i].included = 1; msg_gspew("found.\n"); return i; } @@ -154,7 +154,7 @@ /* process -i arguments * returns 0 to indicate success, >0 to indicate failure */ -int process_include_args(void) +int process_include_args(struct flashrom_layout *const l) { int i; unsigned int found = 0; @@ -163,7 +163,7 @@ return 0; /* User has specified an area, but no layout file is loaded. */ - if (layout.num_entries == 0) { + if (l->num_entries == 0) { msg_gerr("Region requested (with -i \"%s\"), " "but no layout data is available.\n", include_args[0]); @@ -171,7 +171,7 @@ } for (i = 0; i < num_include_args; i++) { - if (find_romentry(include_args[i]) < 0) { + if (find_romentry(l, include_args[i]) < 0) { msg_gerr("Invalid region specified: \"%s\".\n", include_args[i]); return 1; diff --git a/layout.h b/layout.h index 349cebc..c93d754 100644 --- a/layout.h +++ b/layout.h @@ -57,6 +57,13 @@ struct romentry entry; }; -const struct flashrom_layout *get_global_layout(void); +struct ich_layout { + struct flashrom_layout base; + struct romentry entries[5]; +}; + +struct flashrom_layout *get_global_layout(void); + +int process_include_args(struct flashrom_layout *); #endif /* !__LAYOUT_H__ */ diff --git a/libflashrom.c b/libflashrom.c index 5447bae..1176e02 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -31,6 +31,8 @@ #include "flash.h" #include "programmer.h" #include "layout.h" +#include "hwaccess.h" +#include "ich_descriptors.h" #include "libflashrom.h" /** @@ -305,6 +307,86 @@ } /** + * @brief Read a layout from the Intel ICH descriptor in the flash. + * + * Optionally verify that the layout matches the one in the given + * descriptor dump. + * + * @param[out] layout Points to a struct flashrom_layout pointer that + * gets set if the descriptor is read and parsed + * successfully. + * @param[in] flashctx Flash context to read the descriptor from flash. + * @param[in] dump The descriptor dump to compare to or NULL. + * @param[in] len The length of the descriptor dump. + * + * @return 0 on success, + * 6 if descriptor parsing isn't implemented for the host, + * 5 if the descriptors don't match, + * 4 if the descriptor dump couldn't be parsed, + * 3 if the descriptor on flash couldn't be parsed, + * 2 if the descriptor on flash couldn't be read, + * 1 on any other error. + */ +int flashrom_layout_read_from_ifd(struct flashrom_layout **const layout, struct flashctx *const flashctx, + const void *const dump, const size_t len) +{ +#ifndef __FLASHROM_LITTLE_ENDIAN__ + return 6; +#else + struct ich_layout dump_layout; + int ret = 1; + + void *const desc = malloc(0x1000); + struct ich_layout *const chip_layout = malloc(sizeof(*chip_layout)); + if (!desc || !chip_layout) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + if (prepare_flash_access(flashctx, true, false, false, false)) + goto _free_ret; + + msg_cinfo("Reading ich descriptor... "); + if (flashctx->chip->read(flashctx, desc, 0, 0x1000)) { + msg_cerr("Read operation failed!\n"); + msg_cinfo("FAILED.\n"); + ret = 2; + goto _finalize_ret; + } + msg_cinfo("done.\n"); + + if (layout_from_ich_descriptors(chip_layout, desc, 0x1000)) { + ret = 3; + goto _finalize_ret; + } + + if (dump) { + if (layout_from_ich_descriptors(&dump_layout, dump, len)) { + ret = 4; + goto _finalize_ret; + } + + if (chip_layout->base.num_entries != dump_layout.base.num_entries || + memcmp(chip_layout->entries, dump_layout.entries, sizeof(dump_layout.entries))) { + ret = 5; + goto _finalize_ret; + } + } + + *layout = (struct flashrom_layout *)chip_layout; + ret = 0; + +_finalize_ret: + finalize_flash_access(flashctx); +_free_ret: + if (ret) + free(chip_layout); + free(desc); + return ret; +#endif +} + +/** * @brief Free a layout. * * @param layout Layout to free. diff --git a/libflashrom.h b/libflashrom.h index 42b02f9..1937194 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -63,6 +63,7 @@ int flashrom_image_verify(struct flashrom_flashctx *, const void *buffer, size_t buffer_len); struct flashrom_layout; +int flashrom_layout_read_from_ifd(struct flashrom_layout **, struct flashrom_flashctx *, const void *dump, size_t len); int flashrom_layout_include_region(struct flashrom_layout *, const char *name); void flashrom_layout_release(struct flashrom_layout *); void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *); -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-Change-Number: 17953 Gerrit-PatchSet: 16 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Make read_ich_descriptors_from_dump() available in flashrom
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/17952 ) Change subject: Make read_ich_descriptors_from_dump() available in flashrom ...................................................................... Make read_ich_descriptors_from_dump() available in flashrom I didn't really know what I was doing and hope removing the #ifdefs doesn't have negative side effects. The idea is to make the functions generally available for external flashing (e.g. you might want to flash an Intel machine using an ARM device as programmer). Beware of big endian trouble, I guess. :-P Change-Id: Ib3d38a622a581afee87b49777e775942cc901fc8 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/17952 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M ich_descriptors.c M ich_descriptors.h M util/ich_descriptors_tool/Makefile 3 files changed, 9 insertions(+), 27 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/ich_descriptors.c b/ich_descriptors.c index ec75a8a..a0b2c9a 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -19,14 +19,13 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#if defined(__i386__) || defined(__x86_64__) - #include "ich_descriptors.h" -#ifdef ICH_DESCRIPTORS_FROM_DUMP - +#ifdef ICH_DESCRIPTORS_FROM_DUMP_ONLY #include <stdio.h> #define print(t, ...) printf(__VA_ARGS__) +#endif + #define DESCRIPTOR_MODE_SIGNATURE 0x0ff0a55a /* The upper map is located in the word before the 256B-long OEM section at the * end of the 4kB-long flash descriptor. @@ -34,12 +33,9 @@ #define UPPER_MAP_OFFSET (4096 - 256 - 4) #define getVTBA(flumap) (((flumap)->FLUMAP1 << 4) & 0x00000ff0) -#else /* ICH_DESCRIPTORS_FROM_DUMP */ - +#include <string.h> #include "flash.h" /* for msg_* */ #include "programmer.h" - -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ #ifndef min #define min(a, b) (a < b) ? a : b @@ -69,12 +65,12 @@ prettyprint_ich_descriptor_component(cs, desc); prettyprint_ich_descriptor_region(desc); prettyprint_ich_descriptor_master(&desc->master); -#ifdef ICH_DESCRIPTORS_FROM_DUMP +#ifdef ICH_DESCRIPTORS_FROM_DUMP_ONLY if (cs >= CHIPSET_ICH8) { prettyprint_ich_descriptor_upper_map(&desc->upper); prettyprint_ich_descriptor_straps(cs, desc); } -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ +#endif /* ICH_DESCRIPTORS_FROM_DUMP_ONLY */ } void prettyprint_ich_descriptor_content(const struct ich_desc_content *cont) @@ -300,8 +296,6 @@ (mstr->GbE_plat_r) ?'r':' ', (mstr->GbE_plat_w) ?'w':' '); msg_pdbg2("\n"); } - -#ifdef ICH_DESCRIPTORS_FROM_DUMP void prettyprint_ich_descriptor_straps_ich8(const struct ich_descriptors *desc) { @@ -792,7 +786,7 @@ return ICH_RET_OK; } -#else /* ICH_DESCRIPTORS_FROM_DUMP */ +#ifndef ICH_DESCRIPTORS_FROM_DUMP_ONLY /** Returns the integer representation of the component density with index \em idx in bytes or -1 if the correct size can not be determined. */ @@ -922,5 +916,4 @@ msg_pdbg2(" done.\n"); return ICH_RET_OK; } -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ -#endif /* defined(__i386__) || defined(__x86_64__) */ +#endif /* ICH_DESCRIPTORS_FROM_DUMP_ONLY */ diff --git a/ich_descriptors.h b/ich_descriptors.h index 2c21598..e355e54 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -19,7 +19,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#if defined(__i386__) || defined(__x86_64__) #ifndef __ICH_DESCRIPTORS_H__ #define __ICH_DESCRIPTORS_H__ 1 @@ -250,7 +249,6 @@ }; }; -#ifdef ICH_DESCRIPTORS_FROM_DUMP struct ich_desc_north_strap { union { uint32_t STRPs[1]; /* current maximum: ich8 */ @@ -561,18 +559,15 @@ }; } vscc_table[128]; }; -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ struct ich_descriptors { struct ich_desc_content content; struct ich_desc_component component; struct ich_desc_region region; struct ich_desc_master master; -#ifdef ICH_DESCRIPTORS_FROM_DUMP struct ich_desc_north_strap north; struct ich_desc_south_strap south; struct ich_desc_upper_map upper; -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ }; void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc); @@ -582,17 +577,11 @@ void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc); void prettyprint_ich_descriptor_master(const struct ich_desc_master *master); -#ifdef ICH_DESCRIPTORS_FROM_DUMP - void prettyprint_ich_descriptor_upper_map(const struct ich_desc_upper_map *umap); void prettyprint_ich_descriptor_straps(enum ich_chipset cs, const struct ich_descriptors *desc); int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc); -#else /* ICH_DESCRIPTORS_FROM_DUMP */ - int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc); int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx); -#endif /* ICH_DESCRIPTORS_FROM_DUMP */ #endif /* __ICH_DESCRIPTORS_H__ */ -#endif /* defined(__i386__) || defined(__x86_64__) */ diff --git a/util/ich_descriptors_tool/Makefile b/util/ich_descriptors_tool/Makefile index 9cbc150..e312996 100644 --- a/util/ich_descriptors_tool/Makefile +++ b/util/ich_descriptors_tool/Makefile @@ -32,7 +32,7 @@ FLASHROM_CFLAGS += -MMD -MP -MF $(DEPPATH)/$((a)F).d # enables functions that populate the descriptor structs from plain binary dumps -FLASHROM_CFLAGS += -D ICH_DESCRIPTORS_FROM_DUMP +FLASHROM_CFLAGS += -D ICH_DESCRIPTORS_FROM_DUMP_ONLY FLASHROM_CFLAGS += $(patsubst %,-I%,$(EXTRAINCDIRS)) OBJ = $(OBJATH)/$(SRC:%.c=%.o) -- To view, visit https://review.coreboot.org/17952 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: Ib3d38a622a581afee87b49777e775942cc901fc8 Gerrit-Change-Number: 17952 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: cli_classic: Remove layout-for-write-only check
by Nico Huber (Code Review) 14 Jun '17

14 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/17951 ) Change subject: cli_classic: Remove layout-for-write-only check ...................................................................... cli_classic: Remove layout-for-write-only check Change-Id: I0f5e9623ca75bc4503aeb45ae346d7573c0fef2c Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/17951 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M cli_classic.c 1 file changed, 0 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 391fc5a..7e3dfd5 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -376,12 +376,6 @@ ret = 1; goto out; } - if (layoutfile != NULL && !write_it) { - msg_gerr("Layout files are currently supported for write operations only.\n"); - ret = 1; - goto out; - } - if (process_include_args()) { ret = 1; goto out; -- To view, visit https://review.coreboot.org/17951 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I0f5e9623ca75bc4503aeb45ae346d7573c0fef2c Gerrit-Change-Number: 17951 Gerrit-PatchSet: 16 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit