Jeremy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
ifwitool: Introduce a skip BPDT parameter
SoCs like apololake can require two Boot Partition Descriptor Table entries. The second BPDT is usually in the ifwi binary which is supplied but ifwitool can only create a BPDT binary with the first BPDT of the supplied ifwi.
This patch introduces a skip BPDT parameter which is passed to the ifwi_parse() function. It allows the caller script to extract any BPDT separately. This is useful to extract the Logical Boot Partition 2 contains in some IFWI to inject it with fmaptool.
The same result could be achieve mixing up dd and ifwitool commands but it would create dependencies on the dd command and more importantly the script or Makefile would need to know the offset where the second BPDT is. With this simple new ifwitool option, the second BPDP (or third, ...) can be located and then extracted in one command without know its offset.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 36 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/1
diff --git a/util/cbfstool/ifwitool.c b/util/cbfstool/ifwitool.c index 1fbb61b..c34e193 100644 --- a/util/cbfstool/ifwitool.c +++ b/util/cbfstool/ifwitool.c @@ -20,6 +20,8 @@
#include "common.h"
+#define max(a, b) (((a) > (b)) ? (a) : (b)) + /* * BPDT is Boot Partition Descriptor Table. It is located at the start of a * logical boot partition(LBP). It stores information about the critical @@ -851,8 +853,20 @@ print_subpart_dir(subpart_dir); }
+/* Parse the bpdt entries to compute the size of the BPDT */ +static size_t bpdt_size(void *data) +{ + struct bpdt *b = (struct bpdt *)data; + size_t i, size = 0; + + for (i = 0; i < b->h.descriptor_count; i++) + size = max(size, b->e[i].offset + b->e[i].size); + + return size; +} + /* Parse input image file to identify different sub-partitions. */ -static int ifwi_parse(void) +static int ifwi_parse(size_t skip_bpdt_nb) { DEBUG("Parsing IFWI image...\n"); const char *image_name = param.image_name; @@ -871,9 +885,13 @@ void *data = buffer_get(buff);
while (offset < buffer_size(buff)) { - if (read_at_le32(data, offset) == BPDT_SIGNATURE) - break; - offset += 4 * KiB; + if (read_at_le32(data, offset) == BPDT_SIGNATURE) { + if (!skip_bpdt_nb) + break; + offset += bpdt_size(buffer_get(buff) + offset); + skip_bpdt_nb--; + } else + offset += 4 * KiB; }
if (offset >= buffer_size(buff)) { @@ -1851,7 +1869,7 @@
static const struct command commands[] = { {"add", "f:n:e:dvh?", ifwi_add}, - {"create", "f:vh?", ifwi_create}, + {"create", "f:s:vh?", ifwi_create}, {"delete", "f:n:vh?", ifwi_delete}, {"extract", "f:n:e:dvh?", ifwi_extract}, {"print", "dh?", ifwi_print}, @@ -1883,6 +1901,7 @@ " replace -f FILE -n NAME [-d -e ENTRY]\n" "OPTIONs:\n" " -f FILE : File to read/write/create/extract\n" + " -s NB : NB of BPDT to skip during IFWI parsing\n" " -d : Perform directory operation\n" " -e ENTRY: Name of directory entry to operate on\n" " -v : Verbose level\n" @@ -1915,6 +1934,7 @@ if (strcmp(cmd, commands[i].name) != 0) continue;
+ size_t skip_bpdt_nb = 0; int c;
while (1) { @@ -1937,6 +1957,16 @@ case 'n': param.subpart_name = optarg; break; + case 's': { + char *endptr; + skip_bpdt_nb = strtoul(optarg, &endptr, 0); + if (*endptr != '\0') { + ERROR("%s: invalid offset\n", optarg); + return 1; + } + break; + } + case 'f': param.file_name = optarg; break; @@ -1958,7 +1988,7 @@ } }
- if (ifwi_parse()) { + if (ifwi_parse(skip_bpdt_nb)) { ERROR("%s: ifwi parsing failed\n", argv[0]); return 1; }
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 1:
I am currently working on the Apololake SoC support and we would like to be able to extract the second BPDT, manipulate it and supply it as the CONFIG_LBP2_FILE. The current ifwitool implementation is limited to the first BPDT it encounters in the ifwi binary. This new skip BPDT parameter allows to use ifwitool in scripts to extract the second BPDT, manipulate it and inject it during the stitching process.
Next step is going to be: create a CONFIG_LBP2_EXTRACT option making use of this new ifwitool option.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c@23 PS1, Line 23: #define max(a, b) (((a) > (b)) ? (a) : (b)) As we do have access to commonlib would you consider using commonlib/helpers.h and the MAX macro from there? Or is there a real need to repeat it here again?
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c@19... PS1, Line 1904: NB What does NB stand for, number?
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#2).
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
ifwitool: Introduce a skip BPDT parameter
SoCs like apololake can require two Boot Partition Descriptor Table entries. The second BPDT is usually in the ifwi binary which is supplied but ifwitool can only create a BPDT binary with the first BPDT of the supplied ifwi.
This patch introduces a skip BPDT parameter which is passed to the ifwi_parse() function. It allows the caller script to extract any BPDT separately. This is useful to extract the Logical Boot Partition 2 contains in some IFWI to inject it with fmaptool.
The same result could be achieve mixing up dd and ifwitool commands but it would create dependencies on the dd command and more importantly the script or Makefile would need to know the offset where the second BPDT is. With this simple new ifwitool option, the second BPDP (or third, ...) can be located and then extracted in one command without know its offset.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 34 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/2
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 2:
(2 comments)
Thanks for your review. I took both comments into account.
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c@23 PS1, Line 23: #define max(a, b) (((a) > (b)) ? (a) : (b))
As we do have access to commonlib would you consider using commonlib/helpers. […]
Good point, I cscoped for lower case only...
Done
https://review.coreboot.org/c/coreboot/+/37660/1/util/cbfstool/ifwitool.c@19... PS1, Line 1904: NB
What does NB stand for, number?
Done
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 2:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@9 PS2, Line 9: apololake nit: apollolake or Apollo lake
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@9 PS2, Line 9: require allow?
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@10 PS2, Line 10: usually in the ifwi binary Isn't that optional?
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@11 PS2, Line 11: BPDT Do you mean IFWI?
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@16 PS2, Line 16: This is useful to extract the Logical Boot Partition : 2 contains in some IFWI to inject it with fmaptool. Just to ensure I understand this correctly. You are using ifwitool with "skip BPDT" parameter to make it use BPDT for LBP2 instead of LBP1 for extracting the sub-partitions within LBP2?
I think the name "skip BPDT" is very confusing. Instead of asking to skip, can this instead accept a parameter which says BPDT # to use? and that BPDT# defaults to 1 for LBP1. Caller is allowed to pass in either 1 or 2 corresponding to the LBP#.
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@23 PS2, Line 23: or third, ... I think IFWI for APL allows only two?
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@23 PS2, Line 23: BPDP BPDT
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c@18... PS2, Line 1870: f:s:vh Why does only create support this option?
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a skip BPDT parameter ......................................................................
Patch Set 2:
BTW: Would it make sense to change the logic so that one can get the wanted LBP instead of skipping n to get n+1?
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#3).
Change subject: ifwitool: introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: introduce a use the Second Logical Boot Partition option
IFWI binaries for ApolloLake can contain two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate this second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 44 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/3
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#4).
Change subject: ifwitool: introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: introduce a use the Second Logical Boot Partition option
IFWI binaries for ApolloLake can contain two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate this second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 38 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/4
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#5).
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 38 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/5
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 5:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@9 PS2, Line 9: apololake
nit: apollolake or Apollo lake
Done
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@9 PS2, Line 9: require
allow?
Done
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@10 PS2, Line 10: usually in the ifwi binary
Isn't that optional?
yes and not. The security engine may require a second logical boot partition.
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@11 PS2, Line 11: BPDT
Do you mean IFWI?
Done
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@16 PS2, Line 16: This is useful to extract the Logical Boot Partition : 2 contains in some IFWI to inject it with fmaptool.
Just to ensure I understand this correctly. […]
The idea is to be able to extract this second Logical Boot Partition and inject it to the final image.
I was not really proud of this implementation either. I reworked/simplified, I hope you are going to like the new design better.
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@23 PS2, Line 23: BPDP
BPDT
Done
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@23 PS2, Line 23: or third, ...
I think IFWI for APL allows only two?
Correct.
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c@18... PS2, Line 1870: f:s:vh
Why does only create support this option?
Because it was linked to the need I identified but technically you are right, it can be extended to the other options which I did and tested.
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 5:
Patch Set 2:
BTW: Would it make sense to change the logic so that one can get the wanted LBP instead of skipping n to get n+1?
I was not really proud of this skip option either. After implementing a couple of different alternatives, I arrived to the conclusion that the simpler the better and I introduced a "use the Second Boot Partition" option.
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#6).
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 38 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/6
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 6:
@Werner Weh, @Furquan Shaikh, any comment on this simplified design ?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 6: Code-Review+1
(4 comments)
Patch Set 6:
@Werner Weh, @Furquan Shaikh, any comment on this simplified design ?
Overall change looks good to me. Some minor nits recommended.
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@10 PS2, Line 10: usually in the ifwi binary
yes and not. The security engine may require a second logical boot partition.
Correct. It depends on how the system is configured.
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@89... PS6, Line 890: buffer_get(buff) + offset nit: (uintptr_t)data + offset?
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@89... PS6, Line 897: ! nit: ... for LBP=%zd\n", param.logical_boot_partition);
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@19... PS6, Line 1960: 1 nit: Can you add LBP1 and LBP2 macros and use those instead of 0/1? Also, param.logical_boot_partition can be set to LBP1 on line 1927 to indicate that by default ifwitool operates on LBP1. However, if 's' argument is provided, it switches to LBP2.
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#7).
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 44 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/7
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 7:
(3 comments)
All done.
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@89... PS6, Line 890: buffer_get(buff) + offset
nit: (uintptr_t)data + offset?
Done
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@89... PS6, Line 897: !
nit: ... for LBP=%zd\n", param. […]
Done
https://review.coreboot.org/c/coreboot/+/37660/6/util/cbfstool/ifwitool.c@19... PS6, Line 1960: 1
nit: Can you add LBP1 and LBP2 macros and use those instead of 0/1? Also, param. […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37660/7/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/7/util/cbfstool/ifwitool.c@88... PS7, Line 886: 0 nit: LBP1
Hello Werner Zeh, Stefan Reinauer, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37660
to look at the new patch set (#8).
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partition. This patch introduces a '-s' optional parameter to manipulate the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 44 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/8
Jeremy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37660/7/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/7/util/cbfstool/ifwitool.c@88... PS7, Line 886: 0
nit: LBP1
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 8: Code-Review+2
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 8: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG@9 PS8, Line 9: Partition Partitions
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG@10 PS8, Line 10: manipulate select?
Patrick Georgi has uploaded a new patch set (#9) to the change originally created by Jeremy Compostella. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partitions. This patch introduces a '-s' optional parameter to select the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com --- M util/cbfstool/ifwitool.c 1 file changed, 44 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37660/9
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@10 PS2, Line 10: usually in the ifwi binary
Correct. It depends on how the system is configured.
Ack
https://review.coreboot.org/c/coreboot/+/37660/2//COMMIT_MSG@16 PS2, Line 16: This is useful to extract the Logical Boot Partition : 2 contains in some IFWI to inject it with fmaptool.
The idea is to be able to extract this second Logical Boot Partition and inject it to the final imag […]
Ack
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG@9 PS8, Line 9: Partition
Partitions
Done
https://review.coreboot.org/c/coreboot/+/37660/8//COMMIT_MSG@10 PS8, Line 10: manipulate
select?
Done
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c File util/cbfstool/ifwitool.c:
https://review.coreboot.org/c/coreboot/+/37660/2/util/cbfstool/ifwitool.c@18... PS2, Line 1870: f:s:vh
Because it was linked to the need I identified but technically you are right, it can be extended to […]
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37660 )
Change subject: ifwitool: Introduce a use the Second Logical Boot Partition option ......................................................................
ifwitool: Introduce a use the Second Logical Boot Partition option
The ApolloLake SoC allows two Logical Boot Partitions. This patch introduces a '-s' optional parameter to select the second Logical Boot Partition.
Change-Id: If32ec11fc7291d52b821bf95c1e186690d06ba11 Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37660 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Werner Zeh werner.zeh@siemens.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/cbfstool/ifwitool.c 1 file changed, 44 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Werner Zeh: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/util/cbfstool/ifwitool.c b/util/cbfstool/ifwitool.c index 1fbb61b..76b84e2 100644 --- a/util/cbfstool/ifwitool.c +++ b/util/cbfstool/ifwitool.c @@ -34,9 +34,13 @@ */ #define BPDT_SIGNATURE (0x000055AA)
+#define LBP1 (0) +#define LBP2 (1) + /* Parameters passed in by caller. */ static struct param { const char *file_name; + size_t logical_boot_partition; const char *subpart_name; const char *image_name; bool dir_ops; @@ -851,6 +855,18 @@ print_subpart_dir(subpart_dir); }
+/* Parse the bpdt entries to compute the size of the BPDT */ +static size_t bpdt_size(void *data) +{ + struct bpdt *b = (struct bpdt *)data; + size_t i, size = 0; + + for (i = 0; i < b->h.descriptor_count; i++) + size = MAX(size, b->e[i].offset + b->e[i].size); + + return size; +} + /* Parse input image file to identify different sub-partitions. */ static int ifwi_parse(void) { @@ -867,17 +883,22 @@ INFO("Buffer %p size 0x%zx\n", buff->data, buff->size);
/* Look for BPDT signature at 4K intervals. */ - size_t offset = 0; + size_t offset = 0, lbp = LBP1; void *data = buffer_get(buff);
while (offset < buffer_size(buff)) { - if (read_at_le32(data, offset) == BPDT_SIGNATURE) - break; - offset += 4 * KiB; + if (read_at_le32(data, offset) == BPDT_SIGNATURE) { + if (lbp == param.logical_boot_partition) + break; + offset += bpdt_size((uint8_t *)data + offset); + lbp++; + } else + offset += 4 * KiB; }
if (offset >= buffer_size(buff)) { - ERROR("Image does not contain BPDT!!\n"); + ERROR("Image does not contain BPDT for LBP=%zd!!\n", + param.logical_boot_partition); return -1; }
@@ -1850,12 +1871,12 @@ };
static const struct command commands[] = { - {"add", "f:n:e:dvh?", ifwi_add}, - {"create", "f:vh?", ifwi_create}, - {"delete", "f:n:vh?", ifwi_delete}, - {"extract", "f:n:e:dvh?", ifwi_extract}, - {"print", "dh?", ifwi_print}, - {"replace", "f:n:e:dvh?", ifwi_replace}, + {"add", "f:n:e:dsvh?", ifwi_add}, + {"create", "f:svh?", ifwi_create}, + {"delete", "f:n:svh?", ifwi_delete}, + {"extract", "f:n:e:dsvh?", ifwi_extract}, + {"print", "dsh?", ifwi_print}, + {"replace", "f:n:e:dsvh?", ifwi_replace}, };
static struct option long_options[] = { @@ -1865,6 +1886,7 @@ {"name", required_argument, 0, 'n'}, {"dir_ops", no_argument, 0, 'd'}, {"verbose", no_argument, 0, 'v'}, + {"second_lbp", no_argument, 0, 's'}, {NULL, 0, 0, 0 } };
@@ -1875,14 +1897,15 @@ " %s [-h]\n" " %s FILE COMMAND [PARAMETERS]\n\n" "COMMANDs:\n" - " add -f FILE -n NAME [-d -e ENTRY]\n" - " create -f FILE\n" - " delete -n NAME\n" - " extract -f FILE -n NAME [-d -e ENTRY]\n" - " print [-d]\n" - " replace -f FILE -n NAME [-d -e ENTRY]\n" + " add -f FILE -n NAME [-d -e ENTRY] [-s]\n" + " create -f FILE [-s]\n" + " delete -n NAME [-s]\n" + " extract -f FILE -n NAME [-d -e ENTRY] [-s]\n" + " print [-d] [-s]\n" + " replace -f FILE -n NAME [-d -e ENTRY] [-s]\n" "OPTIONs:\n" " -f FILE : File to read/write/create/extract\n" + " -s : Use the second Logical Boot Partition\n" " -d : Perform directory operation\n" " -e ENTRY: Name of directory entry to operate on\n" " -v : Verbose level\n" @@ -1906,6 +1929,7 @@ }
param.image_name = argv[1]; + param.logical_boot_partition = LBP1; char *cmd = argv[2]; optind += 2;
@@ -1937,6 +1961,9 @@ case 'n': param.subpart_name = optarg; break; + case 's': + param.logical_boot_partition = LBP2; + break; case 'f': param.file_name = optarg; break;
-
Furquan Shaikh (Code Review) -
Jeremy Compostella (Code Review)
-
Patrick Georgi (Code Review)
-
Werner Zeh (Code Review)