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.