July 2018 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 23 Jul '18

23 Jul '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 17: (7 comments) > Patch Set 16: > > (6 comments) > > It seems a little weird to have the bulk of the implementation of fmap-from-rom here while only adding the CLI option in the next patch. Maybe it's a good idea to squash both? Agreed. https://review.coreboot.org/#/c/23203/16//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23203/16//COMMIT_MSG@7 PS16, Line 7: Add support to get layout from fmap (e.g. coreboot rom) > Needs to be updated to mention fmap from rom functionality implementation. Done https://review.coreboot.org/#/c/23203/16/fmap.h File fmap.h: https://review.coreboot.org/#/c/23203/16/fmap.h@1 PS16, Line 1: /* > Probably needs license from cbfstool here Done https://review.coreboot.org/#/c/23203/16/fmap.c File fmap.c: https://review.coreboot.org/#/c/23203/16/fmap.c@1 PS16, Line 1: /* Copyright 2015, Google Inc. > probably needs licence from cbfstool. Done https://review.coreboot.org/#/c/23203/16/layout.c File layout.c: https://review.coreboot.org/#/c/23203/16/layout.c@22 PS16, Line 22: #include "flash.h" > Added headers not needed anymore? Done https://review.coreboot.org/#/c/23203/16/layout.c@104 PS16, Line 104: /* returns the index of the entry (or a negative value if it is not found) */ > accidental newline? Done https://review.coreboot.org/#/c/23203/16/libflashrom.c File libflashrom.c: https://review.coreboot.org/#/c/23203/16/libflashrom.c@456 PS16, Line 456: fmap_bsearch > This is no good for concatenated chips which can occur on Intel hardware :/ Why is that? I don't usually work with concatenated chips... https://review.coreboot.org/#/c/23203/16/libflashrom.c@462 PS16, Line 462: offset > On Intel targets you probably only want and in some reasons even only can, if for instance ME region […] OK. I've also updated the public interface and CLI code to pass in offset and length. I broke apart the interface since it seemed clumsy to have filenames and offset/length arguments in the function signature. To address the problem you mention, I think we need to do a few things: 1. Add region attributes to struct romentry which we can use for things like access permissions. 2. Merge your linked list patch so we can incorporate layout info from multiple sources. 3. Always read IFD when it's known to exist (e.g. all modern Intel-based systems) and set region attributes as needed. 4. Add an error policy handling (something like https://gerrit.chromium.org/gerrit/12117 and https://gerrit.chromium.org/gerrit/46207). For example, in the ME region case we know that access to the ME region is blocked, so if the user is trying to read/write content then we know to err out but if we're just looking for an fmap then we can ignore it and keep searching. I'd still like to see this patch get merged soon as it addresses some immediate issues, but we should try to make layouts easier and more useful by doing these other things too. -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 17 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Mon, 23 Jul 2018 04:37:06 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 23 Jul '18

23 Jul '18

David Hendricks has uploaded a new patch set (#17) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Add support to get layout from fmap (e.g. coreboot rom) Flashmap, or simply fmap, is a binary data format for describing region offsets, sizes, and certain attributes and is widely used by coreboot. This patch adds support for the fmap data format along with --fmap and --fmap-file arguments. Using --fmap will make flashrom to search the ROM content for fmap data. Using --fmap-file will make flashrom search a supplied file for fmap data. An example of how to update the COREBOOT region of a ROM: flashrom -p programmer --fmap -w coreboot.rom -i COREBOOT flashrom -p programmer --fmap-file coreboot.rom -w coreboot.rom -i COREBOOT The fmap functions are mostly copied from cbfstool. Currently it is made mutually exclusive with --ifd while still allowing --layout until we are more clever about this input. Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M Makefile M cli_classic.c M flashrom.8.tmpl A fmap.c A fmap.h M libflashrom.c M libflashrom.h 7 files changed, 545 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/23203/17 -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 17 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by Arthur Heymans (Code Review) 21 Jul '18

21 Jul '18

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 16: (6 comments) It seems a little weird to have the bulk of the implementation of fmap-from-rom here while only adding the CLI option in the next patch. Maybe it's a good idea to squash both? https://review.coreboot.org/#/c/23203/16//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23203/16//COMMIT_MSG@7 PS16, Line 7: Add support to get layout from fmap (e.g. coreboot rom) Needs to be updated to mention fmap from rom functionality implementation. https://review.coreboot.org/#/c/23203/16/fmap.h File fmap.h: https://review.coreboot.org/#/c/23203/16/fmap.h@1 PS16, Line 1: #include <inttypes.h> Probably needs license from cbfstool here https://review.coreboot.org/#/c/23203/16/fmap.c File fmap.c: https://review.coreboot.org/#/c/23203/16/fmap.c@1 PS16, Line 1: #include <ctype.h> probably needs licence from cbfstool. https://review.coreboot.org/#/c/23203/16/layout.c File layout.c: https://review.coreboot.org/#/c/23203/16/layout.c@22 PS16, Line 22: #include <sys/stat.h> Added headers not needed anymore? https://review.coreboot.org/#/c/23203/16/layout.c@104 PS16, Line 104: accidental newline? https://review.coreboot.org/#/c/23203/16/libflashrom.c File libflashrom.c: https://review.coreboot.org/#/c/23203/16/libflashrom.c@462 PS16, Line 462: offset On Intel targets you probably only want and in some reasons even only can, if for instance ME region is not readable, look into the BIOS region. This again will need lsearch in some cases. Maybe the start offset and the end of this loop could be made function arguments? -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Sat, 21 Jul 2018 00:34:06 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by Arthur Heymans (Code Review) 21 Jul '18

21 Jul '18

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 16: (1 comment) https://review.coreboot.org/#/c/23203/16/libflashrom.c File libflashrom.c: https://review.coreboot.org/#/c/23203/16/libflashrom.c@456 PS16, Line 456: fmap_bsearch This is no good for concatenated chips which can occur on Intel hardware :/ -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Sat, 21 Jul 2018 00:16:25 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 20 Jul '18

20 Jul '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 16: (1 comment) https://review.coreboot.org/#/c/23203/15/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23203/15/cli_classic.c@247 PS15, Line 247: cli_classic_abort_usage(); > So again (as said in patchset 10 already): […] You are correct, it was I who missed something. I've added the additional checks in PS16. The newly introduced fmap integer will also be used for OPTION_FMAP in the next patch. -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Fri, 20 Jul 2018 23:41:57 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add ability to read fmap from ROM
by David Hendricks (Code Review) 20 Jul '18

20 Jul '18

Hello Arthur Heymans, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/27480 to look at the new patch set (#6). Change subject: Add ability to read fmap from ROM ...................................................................... Add ability to read fmap from ROM This adds logic to search the ROM for the flashmap. Signed-off-by: David Hendricks <dhendricks(a)fb.com> Change-Id: I83119528afeef7a864e7b774b7b7d60cb9750bb4 --- M cli_classic.c 1 file changed, 21 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/27480/6 -- To view, visit https://review.coreboot.org/27480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I83119528afeef7a864e7b774b7b7d60cb9750bb4 Gerrit-Change-Number: 27480 Gerrit-PatchSet: 6 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> 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: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 20 Jul '18

20 Jul '18

David Hendricks has uploaded a new patch set (#16) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Add support to get layout from fmap (e.g. coreboot rom) Using the --fmap-file argument one can give flashrom a layout in fmap binary format like is found of coreboot roms (it will also look for this on coreboot roms). An example of how to use: "flashrom -p programmer --fmap-file coreboot.rom -w coreboot.rom -i COREBOOT" to only flash the COREBOOT fmap region. The fmap functions are mostly copied from cbfstool. Currently it is made mutually exclusive with --ifd while still allowing --layout until we are more clever about this input. Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M Makefile M cli_classic.c M flashrom.8.tmpl A fmap.c A fmap.h M layout.c M libflashrom.c M libflashrom.h 8 files changed, 419 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/23203/16 -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by Werner Zeh (Code Review) 20 Jul '18

20 Jul '18

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 15: (1 comment) https://review.coreboot.org/#/c/23203/15/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23203/15/cli_classic.c@247 PS15, Line 247: case OPTION_FMAP_FILE: So again (as said in patchset 10 already): If you, for now, want to have --ifd and --fmap-file be exclusive you have to check in the IFD-case for fmapfile as the order on the parameters is not strictly enforced. I know that we need something smarter here one day but for now we have to care about it as the --ifd will win the race without even a warning Or do I miss something? -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Fri, 20 Jul 2018 05:13:06 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add ability to read fmap from ROM
by David Hendricks (Code Review) 20 Jul '18

20 Jul '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/27480 ) Change subject: Add ability to read fmap from ROM ...................................................................... Patch Set 5: Verified+1 PS5 updates the man page -- To view, visit https://review.coreboot.org/27480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83119528afeef7a864e7b774b7b7d60cb9750bb4 Gerrit-Change-Number: 27480 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 20 Jul 2018 01:24:00 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 20 Jul '18

20 Jul '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 15: PS15 updates the man page -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Fri, 20 Jul 2018 01:23:44 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2018