[PATCH v2 00/38] binman: Add support for bintools and missing tools

At present binman uses binary tools (like cbfstool, futiltiy, lz4) in an ad-hoc manner. Various parts of binman use tools.Run() to run tools as needed. If a tool is missing, an error is produced and binman stops.
However this is not ideal. CI systems want to be able to complete the build, even if tools are missing. Ideally binman would deal with missing binary tools the same way it deals with missing binary blobs: make a note of it and move on.
This series introduces this feature to binman.
`Bintool` is the name binman gives to a binary tool which it uses to create and manipulate binaries that binman cannot handle itself.
Binman provides various features to manage bintools:
- Determining whether the tool is currently installed - Downloading or building the tool - Determining the version of the tool that is installed - Deciding which tools are needed to build an image
As with external blobs, bintools (which are like 'external' tools) can be missing. When building an image requires a bintool and it is not installed, binman detects this and reports the problem, but continues to build an image.
Of course the image will not work, but binman reports which bintools are needed and also provide a way to fetch them.
The final patch shows how this works in practice with a chosen board. The Odroid-C2 is quite a complicated image with many steps. It is an ideal example for how Binman can be used.
The series is available at u-boot-dm/bin-working
Changes in v2: - Substantial rewrite, introducing the concept of bintools
Simon Glass (38): Makefile: Fake external blobs by default with binman binman: Tweak elf tests for a toolchain change mkimage: Show the external-offset error binman: Expand the external FIT test a little patman: Allow running a tool and returning the full result buildman: Move the download function to tools patman: Tidy up the download function a little patman: Add a function to find a tool on the path binman: Write fake blobs to the output directory binman: Drop the image name from the fake-blob message binman: Allow faked blobs in blob-ext-list binman: Correct path for fip_util binman: Add installation instructions binman: Add support for bintools binman: Plumb in support for bintools binman: Add tests for bintool binman: Add a bintool implementation for cbfstool binman: Add a bintool implementation for fiptool binman: Add a bintool implementation for futility binman: Add a bintool implementation for ifwitool binman: Add a bintool implementation for mkimage binman: Enable bintool tests including cmdline processing binman: Convert to using the CBFS bintool binman: Convert to using the FIP bintool binman: Convert to using the futility bintool binman: Convert to using the ifwitool bintool binman: Convert to using the mkimage bintool binman: Move compression into binman binman: Tidy up pylint warnings in comp_util binman: Add a bintool implementation for lz4 binman: Convert to using the lz4 bintool binman: Add a bintool implementation for lzma_alone binman: Convert to using the lzma_alone bintool binman: Plumb in support for missing bintools binman: Complete test coverage of comp_util binman: Add a command to generate bintool docs binman: Add documentation for bintools RFC: Move Odroid-C2 to use binman to produce the image
Makefile | 2 +- arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 107 +++++ arch/arm/mach-meson/Kconfig | 1 + doc/board/amlogic/odroid-c4.rst | 127 ++--- doc/develop/package/bintools.rst | 1 + tools/binman/binman.rst | 98 +++- tools/binman/bintool.py | 466 +++++++++++++++++++ tools/binman/bintool_test.py | 353 ++++++++++++++ tools/binman/bintools.rst | 139 ++++++ tools/binman/btool/_aml_common.py | 47 ++ tools/binman/btool/_testing.py | 36 ++ tools/binman/btool/aml_encrypt_g12a.py | 82 ++++ tools/binman/btool/aml_encrypt_g12b.py | 83 ++++ tools/binman/btool/cbfstool.py | 219 +++++++++ tools/binman/btool/fiptool.py | 123 +++++ tools/binman/btool/futility.py | 178 +++++++ tools/binman/btool/ifwitool.py | 166 +++++++ tools/binman/btool/lz4.py | 140 ++++++ tools/binman/btool/lzma_alone.py | 126 +++++ tools/binman/btool/mkimage.py | 80 ++++ tools/binman/cbfs_util.py | 33 +- tools/binman/cbfs_util_test.py | 59 +-- tools/binman/cmdline.py | 12 + tools/binman/comp_util.py | 76 +++ tools/binman/control.py | 50 +- tools/binman/elf_test.py | 8 +- tools/binman/entries.rst | 138 ++++++ tools/binman/entry.py | 67 ++- tools/binman/etype/aml_encrypt.py | 262 +++++++++++ tools/binman/etype/blob.py | 8 +- tools/binman/etype/blob_ext_list.py | 1 + tools/binman/etype/fit.py | 20 +- tools/binman/etype/gbb.py | 37 +- tools/binman/etype/intel_ifwi.py | 25 +- tools/binman/etype/mkimage.py | 13 +- tools/binman/etype/section.py | 19 +- tools/binman/etype/vblock.py | 32 +- tools/binman/fip_util.py | 26 -- tools/binman/fip_util_test.py | 25 +- tools/binman/ftest.py | 173 ++++++- tools/binman/image.py | 14 + tools/binman/main.py | 11 +- tools/binman/missing-blob-help | 6 + tools/binman/test/162_fit_external.dts | 2 +- tools/binman/test/213_aml_encrypt.dts | 51 ++ tools/binman/test/214_list_no_dtb.dts | 23 + tools/binman/test/218_blob_ext_list_fake.dts | 14 + tools/buildman/toolchain.py | 46 +- tools/fit_image.c | 5 +- tools/patman/tools.py | 238 +++++----- 50 files changed, 3619 insertions(+), 449 deletions(-) create mode 120000 doc/develop/package/bintools.rst create mode 100644 tools/binman/bintool.py create mode 100644 tools/binman/bintool_test.py create mode 100644 tools/binman/bintools.rst create mode 100644 tools/binman/btool/_aml_common.py create mode 100644 tools/binman/btool/_testing.py create mode 100644 tools/binman/btool/aml_encrypt_g12a.py create mode 100644 tools/binman/btool/aml_encrypt_g12b.py create mode 100644 tools/binman/btool/cbfstool.py create mode 100644 tools/binman/btool/fiptool.py create mode 100644 tools/binman/btool/futility.py create mode 100644 tools/binman/btool/ifwitool.py create mode 100644 tools/binman/btool/lz4.py create mode 100644 tools/binman/btool/lzma_alone.py create mode 100644 tools/binman/btool/mkimage.py create mode 100644 tools/binman/comp_util.py create mode 100644 tools/binman/etype/aml_encrypt.py create mode 100644 tools/binman/test/213_aml_encrypt.dts create mode 100644 tools/binman/test/214_list_no_dtb.dts create mode 100644 tools/binman/test/218_blob_ext_list_fake.dts

This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 948ecb546bf..926be6a764f 100644 --- a/Makefile +++ b/Makefile @@ -1316,6 +1316,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ --toolpath $(objtree)/tools \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ build -u -d u-boot.dtb -O . -m --allow-missing \ + --fake-ext-blobs \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ $(foreach f,$(BINMAN_INDIRS),-I $(f)) \ @@ -1327,7 +1328,6 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \ -a spl-dtb=$(CONFIG_SPL_OF_REAL) \ -a tpl-dtb=$(CONFIG_TPL_OF_REAL) \ - $(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \ $(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex

This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
Rasmus

On Mon, Oct 10, 2022 at 12:15:58PM +0200, Rasmus Villemoes wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
I've brought this up before as well, and it's not as bad as I feared it would be to add this, so I'll post a patch shortly.

Hi Rasmus,
On Mon, 10 Oct 2022 at 04:16, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
How come it doesn't fail? Building should produce an error 101 in this case. Does it not? Please send the details if there is something wrong.
For gitlab the code is something like:
./tools/buildman/buildman -o /tmp -P -E -W aarch64 || ret=$?; if [[ $ret -ne 0 ]]; then ./tools/buildman/buildman -o /tmp -seP; exit $ret; fi;
which is why it continues building.
BTW, since this patch they go in a subdir, BTW, so they don't affect a subsequent build:
7960a0a2895 binman: Put fake files in a subdirectory
Regards. Simon

On Mon, Oct 10, 2022 at 09:19:39AM -0600, Simon Glass wrote:
Hi Rasmus,
On Mon, 10 Oct 2022 at 04:16, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
How come it doesn't fail? Building should produce an error 101 in this case. Does it not? Please send the details if there is something wrong.
No, it does not. You can see this for example with: $ make galileo_config ... $ make -sj16 Image 'main-section' is missing external blobs and is non-functional: intel-cmc Image 'main-section' has faked external blobs and is non-functional: rmu.bin
Some images are invalid $ echo $? 0

Hi Tom,
On Mon, 10 Oct 2022 at 09:24, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 10, 2022 at 09:19:39AM -0600, Simon Glass wrote:
Hi Rasmus,
On Mon, 10 Oct 2022 at 04:16, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
How come it doesn't fail? Building should produce an error 101 in this case. Does it not? Please send the details if there is something wrong.
No, it does not. You can see this for example with: $ make galileo_config ... $ make -sj16 Image 'main-section' is missing external blobs and is non-functional: intel-cmc Image 'main-section' has faked external blobs and is non-functional: rmu.bin
Some images are invalid $ echo $? 0
Ah OK, that is a bug, though. I suppose I normally use buildman so don't see it, and we have no test for it.
I'll take a look when I get the -N thing done.
Regards, Simon

On Mon, Oct 10, 2022 at 09:33:59AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 10 Oct 2022 at 09:24, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 10, 2022 at 09:19:39AM -0600, Simon Glass wrote:
Hi Rasmus,
On Mon, 10 Oct 2022 at 04:16, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
How come it doesn't fail? Building should produce an error 101 in this case. Does it not? Please send the details if there is something wrong.
No, it does not. You can see this for example with: $ make galileo_config ... $ make -sj16 Image 'main-section' is missing external blobs and is non-functional: intel-cmc Image 'main-section' has faked external blobs and is non-functional: rmu.bin
Some images are invalid $ echo $? 0
Ah OK, that is a bug, though. I suppose I normally use buildman so don't see it, and we have no test for it.
I'll take a look when I get the -N thing done.
Well, it's a design feature that buildman shows too. See the series I just posted, we've been unconditionally passing binman the flags to always allow and fake missing binaries.

Hi Tom,
On Mon, 10 Oct 2022 at 09:36, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 10, 2022 at 09:33:59AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 10 Oct 2022 at 09:24, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 10, 2022 at 09:19:39AM -0600, Simon Glass wrote:
Hi Rasmus,
On Mon, 10 Oct 2022 at 04:16, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 10/01/2022 04.13, Simon Glass wrote:
This behaviour is necessary with boards where the binman description requires processing external blobs, since these may be missing.
Enable it by default, so that CI is happy. Warnings indicate that a valid image is not produced, as with the --allow-missing option.
I know I have talked and warned about this before, and now I was actually bitten by it IRL.
Can we _please_ stop doing this by default. I understand why upstream U-Boot's CI system needs this, but it should be possible for that CI system to set an environment variable or pass a make parameter to opt-in to generating these fake blobs.
Generating potentially non-functional binaries by default, and currently without even a way to opt out of that (that I have found), is frankly not sane. Yeah, a warning is printed, but that easily scrolls away or in the case of automated builds is hidden away in some log file people would only ever look at if the job failed.
I want my/our CI to _fail hard_ when I have failed to update the Yocto metadata to stage the necessary blobs before do_compile. And if upstream U-Boot want to continue to have this by default, can we at the very least get an ergonomic way to opt out (preferably a CONFIG_ option I can set or clear in my out-of-tree defconfig files).
How come it doesn't fail? Building should produce an error 101 in this case. Does it not? Please send the details if there is something wrong.
No, it does not. You can see this for example with: $ make galileo_config ... $ make -sj16 Image 'main-section' is missing external blobs and is non-functional: intel-cmc Image 'main-section' has faked external blobs and is non-functional: rmu.bin
Some images are invalid $ echo $? 0
Ah OK, that is a bug, though. I suppose I normally use buildman so don't see it, and we have no test for it.
I'll take a look when I get the -N thing done.
Well, it's a design feature that buildman shows too. See the series I just posted, we've been unconditionally passing binman the flags to always allow and fake missing binaries.
Yes, I see it. Will take a look after N:
Regrads, Simon

Some newer toolchains do not create a symbol for the .ucode section that this test relies on. Update the test to use the symbol that is explicitly created, instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/elf_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index ac69a95b654..f7272584878 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -99,17 +99,17 @@ class TestElf(unittest.TestCase): """Test that we can obtain a symbol from the ELF file""" fname = self.ElfTestFile('u_boot_ucode_ptr') syms = elf.GetSymbols(fname, []) - self.assertIn('.ucode', syms) + self.assertIn('_dt_ucode_base_size', syms)
def testRegexSymbols(self): """Test that we can obtain from the ELF file by regular expression""" fname = self.ElfTestFile('u_boot_ucode_ptr') syms = elf.GetSymbols(fname, ['ucode']) - self.assertIn('.ucode', syms) + self.assertIn('_dt_ucode_base_size', syms) syms = elf.GetSymbols(fname, ['missing']) - self.assertNotIn('.ucode', syms) + self.assertNotIn('_dt_ucode_base_size', syms) syms = elf.GetSymbols(fname, ['missing', 'ucode']) - self.assertIn('.ucode', syms) + self.assertIn('_dt_ucode_base_size', syms)
def testMissingFile(self): """Test that a missing file is detected"""

Some newer toolchains do not create a symbol for the .ucode section that this test relies on. Update the test to use the symbol that is explicitly created, instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/elf_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

This is a debug message at present, which is not very helpful. Print out the error so that action can be taken.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/fit_image.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/fit_image.c b/tools/fit_image.c index f4f372ba62f..d5cab96a9b4 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -524,8 +524,9 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname) /* Check if an offset for the external data was set. */ if (params->external_offset > 0) { if (params->external_offset < new_size) { - debug("External offset %x overlaps FIT length %x\n", - params->external_offset, new_size); + fprintf(stderr, + "External offset %x overlaps FIT length %x\n", + params->external_offset, new_size); ret = -EINVAL; goto err; }

This is a debug message at present, which is not very helpful. Print out the error so that action can be taken.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/fit_image.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

At present this does not check that the external data is in the expected place. Use a non-zero offset for the external data and check it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/ftest.py | 17 +++++++++++++++++ tools/binman/test/162_fit_external.dts | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a9d9160967c..f44128de6f3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3713,11 +3713,28 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('162_fit_external.dts') fit_data = data[len(U_BOOT_DATA):-2] # _testing is 2 bytes
+ # Size of the external-data region as set up by mkimage + external_data_size = len(U_BOOT_DATA) + 2 + expected_size = (len(U_BOOT_DATA) + 0x400 + + tools.Align(external_data_size, 4) + + len(U_BOOT_NODTB_DATA)) + # The data should be outside the FIT dtb = fdt.Fdt.FromData(fit_data) dtb.Scan() fnode = dtb.GetNode('/images/kernel') self.assertNotIn('data', fnode.props) + self.assertEqual(len(U_BOOT_DATA), + fdt_util.fdt32_to_cpu(fnode.props['data-size'].value)) + fit_pos = 0x400; + self.assertEqual( + fit_pos, + fdt_util.fdt32_to_cpu(fnode.props['data-position'].value)) + + self.assertEquals(expected_size, len(data)) + actual_pos = len(U_BOOT_DATA) + fit_pos + self.assertEqual(U_BOOT_DATA + b'aa', + data[actual_pos:actual_pos + external_data_size])
def testSectionIgnoreHashSignature(self): """Test that sections ignore hash, signature nodes for its data""" diff --git a/tools/binman/test/162_fit_external.dts b/tools/binman/test/162_fit_external.dts index 19518e05a58..6f2a629a985 100644 --- a/tools/binman/test/162_fit_external.dts +++ b/tools/binman/test/162_fit_external.dts @@ -10,7 +10,7 @@ u-boot { }; fit { - fit,external-offset = <0>; + fit,external-offset = <0x400>; description = "test-desc"; #address-cells = <1>;

At present this does not check that the external data is in the expected place. Use a non-zero offset for the external data and check it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/ftest.py | 17 +++++++++++++++++ tools/binman/test/162_fit_external.dts | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Add a new function which returns the entire result from running a tool, not just stdout. Update Run() to use this and to return stdout on error, if stderr is empty, since some unfortunate tools write their error output to stdout rather than stderr.
Move building of the PATH to a separate function.
Make the exception catching more specific, to catch just ValueError, since broad exceptions are a pain to debug.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/tools.py | 56 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 10 deletions(-)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 86c4f616206..7af4a52a8fd 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -313,7 +313,22 @@ def GetTargetCompileTool(name, cross_compile=None): target_name = name return target_name, extra_args
-def Run(name, *args, **kwargs): +def get_env_with_path(): + """Get an updated environment with the PATH variable set correctly + + If there are any search paths set, these need to come first in the PATH so + that these override any other version of the tools. + + Returns: + dict: New environment with PATH updated, or None if there are not search + paths + """ + if tool_search_paths: + env = dict(os.environ) + env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] + return env + +def run_result(name, *args, **kwargs): """Run a tool with some arguments
This runs a 'tool', which is a program used by binman to process files and @@ -326,6 +341,7 @@ def Run(name, *args, **kwargs): for_host: True to resolve the command to the version for the host for_target: False to run the command as-is, without resolving it to the version for the compile target + raise_on_error: Raise an error if the command fails (True by default)
Returns: CommandResult object @@ -334,10 +350,8 @@ def Run(name, *args, **kwargs): binary = kwargs.get('binary') for_host = kwargs.get('for_host', False) for_target = kwargs.get('for_target', not for_host) - env = None - if tool_search_paths: - env = dict(os.environ) - env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] + raise_on_error = kwargs.get('raise_on_error', True) + env = get_env_with_path() if for_target: name, extra_args = GetTargetCompileTool(name) args = tuple(extra_args) + args @@ -349,11 +363,12 @@ def Run(name, *args, **kwargs): result = command.RunPipe([all_args], capture=True, capture_stderr=True, env=env, raise_on_error=False, binary=binary) if result.return_code: - raise ValueError("Error %d running '%s': %s" % - (result.return_code,' '.join(all_args), - result.stderr)) - return result.stdout - except: + if raise_on_error: + raise ValueError("Error %d running '%s': %s" % + (result.return_code,' '.join(all_args), + result.stderr or result.stdout)) + return result + except ValueError: if env and not PathHasFile(env['PATH'], name): msg = "Please install tool '%s'" % name package = packages.get(name) @@ -362,6 +377,27 @@ def Run(name, *args, **kwargs): raise ValueError(msg) raise
+def Run(name, *args, **kwargs): + """Run a tool with some arguments + + This runs a 'tool', which is a program used by binman to process files and + perhaps produce some output. Tools can be located on the PATH or in a + search path. + + Args: + name: Command name to run + args: Arguments to the tool + for_host: True to resolve the command to the version for the host + for_target: False to run the command as-is, without resolving it + to the version for the compile target + + Returns: + CommandResult object + """ + result = run_result(name, *args, **kwargs) + if result is not None: + return result.stdout + def Filename(fname): """Resolve a file path to an absolute path.

Add a new function which returns the entire result from running a tool, not just stdout. Update Run() to use this and to return stdout on error, if stderr is empty, since some unfortunate tools write their error output to stdout rather than stderr.
Move building of the PATH to a separate function.
Make the exception catching more specific, to catch just ValueError, since broad exceptions are a pain to debug.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/tools.py | 56 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 10 deletions(-)
Applied to u-boot-dm, thanks!

This function is handy for binman as well. Move it into the shared 'tools' module.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/buildman/toolchain.py | 46 +------------------------------------ tools/patman/tools.py | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 45 deletions(-)
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 4e2471f3e37..bcae5ef7415 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -515,50 +515,6 @@ class Toolchains: return arch, links return None
- def Download(self, url): - """Download a file to a temporary directory - - Args: - url: URL to download - Returns: - Tuple: - Temporary directory name - Full path to the downloaded archive file in that directory, - or None if there was an error while downloading - """ - print('Downloading: %s' % url) - leaf = url.split('/')[-1] - tmpdir = tempfile.mkdtemp('.buildman') - response = urllib.request.urlopen(url) - fname = os.path.join(tmpdir, leaf) - fd = open(fname, 'wb') - meta = response.info() - size = int(meta.get('Content-Length')) - done = 0 - block_size = 1 << 16 - status = '' - - # Read the file in chunks and show progress as we go - while True: - buffer = response.read(block_size) - if not buffer: - print(chr(8) * (len(status) + 1), '\r', end=' ') - break - - done += len(buffer) - fd.write(buffer) - status = r'%10d MiB [%3d%%]' % (done // 1024 // 1024, - done * 100 // size) - status = status + chr(8) * (len(status) + 1) - print(status, end=' ') - sys.stdout.flush() - fd.close() - if done != size: - print('Error, failed to download') - os.remove(fname) - fname = None - return tmpdir, fname - def Unpack(self, fname, dest): """Unpack a tar file
@@ -615,7 +571,7 @@ class Toolchains: os.mkdir(dest)
# Download the tar file for this toolchain and unpack it - tmpdir, tarfile = self.Download(url) + tmpdir, tarfile = tools.Download(url) if not tarfile: return 1 print(col.Color(col.GREEN, 'Unpacking to: %s' % dest), end=' ') diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 7af4a52a8fd..2f817f61670 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -10,6 +10,7 @@ import shutil import struct import sys import tempfile +import urllib.request
from patman import command from patman import tout @@ -632,3 +633,47 @@ def PrintFullHelp(fname): if not pager: pager = ['more'] command.Run(*pager, fname) + +def Download(url): + """Download a file to a temporary directory + + Args: + url: URL to download + Returns: + Tuple: + Temporary directory name + Full path to the downloaded archive file in that directory, + or None if there was an error while downloading + """ + print('Downloading: %s' % url) + leaf = url.split('/')[-1] + tmpdir = tempfile.mkdtemp('.buildman') + response = urllib.request.urlopen(url) + fname = os.path.join(tmpdir, leaf) + fd = open(fname, 'wb') + meta = response.info() + size = int(meta.get('Content-Length')) + done = 0 + block_size = 1 << 16 + status = '' + + # Read the file in chunks and show progress as we go + while True: + buffer = response.read(block_size) + if not buffer: + print(chr(8) * (len(status) + 1), '\r', end=' ') + break + + done += len(buffer) + fd.write(buffer) + status = r'%10d MiB [%3d%%]' % (done // 1024 // 1024, + done * 100 // size) + status = status + chr(8) * (len(status) + 1) + print(status, end=' ') + sys.stdout.flush() + fd.close() + if done != size: + print('Error, failed to download') + os.remove(fname) + fname = None + return tmpdir, fname

This function is handy for binman as well. Move it into the shared 'tools' module.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/buildman/toolchain.py | 46 +------------------------------------ tools/patman/tools.py | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 45 deletions(-)
Applied to u-boot-dm, thanks!

Reverse the order of the return tuple, so that the filename is first. This seems more obvious than putting the temporary directory first.
Correct a bug that leaves a space on the final line.
Allow the caller to control the name of the temporary directory.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/buildman/toolchain.py | 2 +- tools/patman/tools.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index bcae5ef7415..adc75a7a0b7 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -571,7 +571,7 @@ class Toolchains: os.mkdir(dest)
# Download the tar file for this toolchain and unpack it - tmpdir, tarfile = tools.Download(url) + tarfile, tmpdir = tools.Download(url, '.buildman') if not tarfile: return 1 print(col.Color(col.GREEN, 'Unpacking to: %s' % dest), end=' ') diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 2f817f61670..24e2bf567b8 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -634,20 +634,22 @@ def PrintFullHelp(fname): pager = ['more'] command.Run(*pager, fname)
-def Download(url): +def Download(url, tmpdir_pattern='.patman'): """Download a file to a temporary directory
Args: - url: URL to download + url (str): URL to download + tmpdir_pattern (str): pattern to use for the temporary directory + Returns: Tuple: - Temporary directory name Full path to the downloaded archive file in that directory, or None if there was an error while downloading + Temporary directory name """ - print('Downloading: %s' % url) + print('- downloading: %s' % url) leaf = url.split('/')[-1] - tmpdir = tempfile.mkdtemp('.buildman') + tmpdir = tempfile.mkdtemp(tmpdir_pattern) response = urllib.request.urlopen(url) fname = os.path.join(tmpdir, leaf) fd = open(fname, 'wb') @@ -671,9 +673,11 @@ def Download(url): status = status + chr(8) * (len(status) + 1) print(status, end=' ') sys.stdout.flush() + print('\r', end='') + sys.stdout.flush() fd.close() if done != size: print('Error, failed to download') os.remove(fname) fname = None - return tmpdir, fname + return fname, tmpdir

Reverse the order of the return tuple, so that the filename is first. This seems more obvious than putting the temporary directory first.
Correct a bug that leaves a space on the final line.
Allow the caller to control the name of the temporary directory.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/buildman/toolchain.py | 2 +- tools/patman/tools.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-)
Applied to u-boot-dm, thanks!

The Run() function automatically uses the PATH variable to locate a tool when running it. Add a function that does this manually, so we don't have to run a tool to find out if it is present.
This is needed by the new Bintool class, which wants to check which tools are present.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/tools.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 24e2bf567b8..a27db05ff2a 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -378,6 +378,29 @@ def run_result(name, *args, **kwargs): raise ValueError(msg) raise
+def tool_find(name): + """Search the current path for a tool + + This uses both PATH and any value from SetToolPaths() to search for a tool + + Args: + name (str): Name of tool to locate + + Returns: + str: Full path to tool if found, else None + """ + name = os.path.expanduser(name) # Expand paths containing ~ + paths = [] + pathvar = os.environ.get('PATH') + if pathvar: + paths = pathvar.split(':') + if tool_search_paths: + paths += tool_search_paths + for path in paths: + fname = os.path.join(path, name) + if os.path.isfile(fname) and os.access(fname, os.X_OK): + return fname + def Run(name, *args, **kwargs): """Run a tool with some arguments

The Run() function automatically uses the PATH variable to locate a tool when running it. Add a function that does this manually, so we don't have to run a tool to find out if it is present.
This is needed by the new Bintool class, which wants to check which tools are present.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/tools.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
Applied to u-boot-dm, thanks!

At present binman writes fake blobs to the current directory. This is not very helpful, since the files serve no useful purpose once binman has finished. They clutter up the source directory and affect future runs, since the files in the current directory are often used in preference to those in the board directory.
To avoid these problems, write them to the output directory instead.
Move the file-creation code to the Entry base class, so it can be used by any entry type that needs it. This is required since some entry types, such as Entry_blob_ext_list, are not subclasses of Entry_blob.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/control.py | 8 +++++--- tools/binman/entry.py | 20 ++++++++++++++++++++ tools/binman/etype/blob.py | 8 +------- tools/binman/ftest.py | 1 - 4 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py index 4b3ce23fb4c..f4c1fd01568 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -577,9 +577,11 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, faked_list = [] image.CheckFakedBlobs(faked_list) if faked_list: - tout.Warning("Image '%s:%s' has faked external blobs and is non-functional: %s" % - (image.name, image.image_name, - ' '.join([e.GetDefaultFilename() for e in faked_list]))) + tout.Warning( + "Image '%s:%s' has faked external blobs and is non-functional: %s" % + (image.name, image.image_name, + ' '.join([os.path.basename(e.GetDefaultFilename()) + for e in faked_list]))) return bool(missing_list) or bool(faked_list)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 54cc3726b9d..bac90bbbcde 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -7,6 +7,7 @@ from collections import namedtuple import importlib import os +import pathlib import sys
from dtoc import fdt_util @@ -972,6 +973,25 @@ features to produce new behaviours. if self.missing: missing_list.append(self)
+ def check_fake_fname(self, fname): + """If the file is missing and the entry allows fake blobs, fake it + + Sets self.faked to True if faked + + Args: + fname (str): Filename to check + + Returns: + fname (str): Filename of faked file + """ + if self.allow_fake and not pathlib.Path(fname).is_file(): + outfname = tools.GetOutputFilename(os.path.basename(fname)) + with open(outfname, "wb") as out: + out.truncate(1024) + self.faked = True + return outfname + return fname + def CheckFakedBlobs(self, faked_blobs_list): """Check if any entries in this section have faked external blobs
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 65ebb2ecf4d..59728f368ec 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -5,8 +5,6 @@ # Entry-type module for blobs, which are binary objects read from files #
-import pathlib - from binman.entry import Entry from binman import state from dtoc import fdt_util @@ -38,16 +36,12 @@ class Entry_blob(Entry): self._filename = fdt_util.GetString(self._node, 'filename', self.etype)
def ObtainContents(self): - if self.allow_fake and not pathlib.Path(self._filename).is_file(): - with open(self._filename, "wb") as out: - out.truncate(1024) - self.faked = True - self._filename = self.GetDefaultFilename() self._pathname = tools.GetInputFilename(self._filename, self.external and self.section.GetAllowMissing()) # Allow the file to be missing if not self._pathname: + self._pathname = self.check_fake_fname(self._filename) self.SetContents(b'') self.missing = True return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f44128de6f3..6a7647311ba 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4981,7 +4981,6 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertRegex( err, "Image '.*' has faked external blobs and is non-functional: .*") - os.remove('binman_faking_test_blob')
if __name__ == "__main__":

This is not really needed and it makes the message different from the missing-blob message. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/control.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py index f4c1fd01568..e0d2b3879d8 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -578,10 +578,9 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, image.CheckFakedBlobs(faked_list) if faked_list: tout.Warning( - "Image '%s:%s' has faked external blobs and is non-functional: %s" % - (image.name, image.image_name, - ' '.join([os.path.basename(e.GetDefaultFilename()) - for e in faked_list]))) + "Image '%s' has faked external blobs and is non-functional: %s" % + (image.name, ' '.join([os.path.basename(e.GetDefaultFilename()) + for e in faked_list]))) return bool(missing_list) or bool(faked_list)

This is not really needed and it makes the message different from the missing-blob message. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/control.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

Since this is a list of blobs, each blob should have the ability to be faked, as with blob-ext. Update the Entry base class to set allow_fake and use the base class in the section code also, so that this propagagtes to blob-ext-list, which is not a section.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/entry.py | 2 +- tools/binman/etype/blob_ext_list.py | 1 + tools/binman/etype/section.py | 1 + tools/binman/ftest.py | 8 ++++++++ tools/binman/test/218_blob_ext_list_fake.dts | 14 ++++++++++++++ 5 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/218_blob_ext_list_fake.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index bac90bbbcde..e4a1f2d5d5c 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -960,7 +960,7 @@ features to produce new behaviours. Args: allow_fake: True if allowed, False if not allowed """ - pass + self.allow_fake = allow_fake
def CheckMissing(self, missing_list): """Check if any entries in this section have missing external blobs diff --git a/tools/binman/etype/blob_ext_list.py b/tools/binman/etype/blob_ext_list.py index 136ae819946..29c9092dc35 100644 --- a/tools/binman/etype/blob_ext_list.py +++ b/tools/binman/etype/blob_ext_list.py @@ -37,6 +37,7 @@ class Entry_blob_ext_list(Entry_blob): missing = False pathnames = [] for fname in self._filenames: + fname = self.check_fake_fname(fname) pathname = tools.GetInputFilename( fname, self.external and self.section.GetAllowMissing()) # Allow the file to be missing diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 7a55d032318..fdd4cbb21ad 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -805,6 +805,7 @@ class Entry_section(Entry): Args: allow_fake_blob: True if allowed, False if not allowed """ + super().SetAllowFakeBlob(allow_fake) for entry in self._entries.values(): entry.SetAllowFakeBlob(allow_fake)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6a7647311ba..ac6aabbf9c3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4982,6 +4982,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap err, "Image '.*' has faked external blobs and is non-functional: .*")
+ def testExtblobListFaked(self): + """Test an extblob with missing external blob that are faked""" + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('216_blob_ext_list_missing.dts', + allow_fake_blobs=True) + err = stderr.getvalue() + self.assertRegex(err, "Image 'main-section'.*faked.*: blob-ext-list") +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/218_blob_ext_list_fake.dts b/tools/binman/test/218_blob_ext_list_fake.dts new file mode 100644 index 00000000000..54ee54fdaab --- /dev/null +++ b/tools/binman/test/218_blob_ext_list_fake.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + blob-ext-list { + filenames = "refcode.bin", "fake-file"; + }; + }; +};

Since this is a list of blobs, each blob should have the ability to be faked, as with blob-ext. Update the Entry base class to set allow_fake and use the base class in the section code also, so that this propagagtes to blob-ext-list, which is not a section.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/entry.py | 2 +- tools/binman/etype/blob_ext_list.py | 1 + tools/binman/etype/section.py | 1 + tools/binman/ftest.py | 8 ++++++++ tools/binman/test/218_blob_ext_list_fake.dts | 14 ++++++++++++++ 5 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/218_blob_ext_list_fake.dts
Applied to u-boot-dm, thanks!

This should be imported from the binman module. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/fip_util_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/fip_util_test.py b/tools/binman/fip_util_test.py index 4839b298762..06827f59322 100755 --- a/tools/binman/fip_util_test.py +++ b/tools/binman/fip_util_test.py @@ -22,7 +22,7 @@ sys.path.insert(2, os.path.join(OUR_PATH, '..')) # pylint: disable=C0413 from patman import test_util from patman import tools -import fip_util +from binman import fip_util
HAVE_FIPTOOL = True try:

This should be imported from the binman module. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/fip_util_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Explain how to install binman, since it is not obvious.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/binman.rst | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 3e063d1f86c..9dbe582ade2 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -185,14 +185,37 @@ Binman is intended to replace all of this, with ifdtool left to handle only the configuration of the Intel-format descriptor.
-Running binman --------------- +Installing binman +-----------------
First install prerequisites, e.g::
sudo apt-get install python-pyelftools python3-pyelftools lzma-alone \ liblz4-tool
+You can run binman directly if you put it on your PATH. But if you want to +install into your `~/.local` Python directory, use:: + + pip install tools/patman tools/dtoc tools/binman + +Note that binman makes use of libraries from patman and dtoc, which is why these +need to be installed. Also you need `libfdt` and `pylibfdt` which can be +installed like this:: + + git clone git://git.kernel.org/pub/scm/utils/dtc/dtc.git + cd dtc + pip install . + make NO_PYTHON=1 install + +This installs the `libfdt.so` library into `~/lib` so you can use +`LD_LIBRARY_PATH=~/lib` when running binman. If you want to install it in the +system-library directory, replace the last line with:: + + make NO_PYTHON=1 PREFIX=/ install + +Running binman +-------------- + Type::
binman build -b <board_name>

Explain how to install binman, since it is not obvious.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/binman.rst | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

Binman requires various tools to actually work, such as 'lz4' to compress data and 'futility' to sign Chrome OS firmware. At present these are handled in an ad-hoc manner and there is no easy way to find out what tools are needd to build an image, nor where to obtain them.
Add an implementation of 'bintool', a base class which implements this functionality. When a bintool is required, it can be requested from this module, then executed. When the tool is missing, it can provide a way to obtain it.
Note that this uses Command directly, not the tools.Run() function. This allows proper handling of missing tools and avoids needing to catch and re-raise exceptions.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool.py | 421 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 421 insertions(+) create mode 100644 tools/binman/bintool.py
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py new file mode 100644 index 00000000000..34102dafa2a --- /dev/null +++ b/tools/binman/bintool.py @@ -0,0 +1,421 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Base class for all bintools + +This defines the common functionality for all bintools, including running +the tool, checking its version and fetching it if needed. +""" + +import collections +import glob +import importlib +import multiprocessing +import os +import shutil +import tempfile +import urllib.error + +from patman import command +from patman import terminal +from patman import tools +from patman import tout + +BINMAN_DIR = os.path.dirname(os.path.realpath(__file__)) + +# Format string for listing bintools, see also the header in list_all() +FORMAT = '%-16.16s %-12.12s %-26.26s %s' + +# List of known modules, to avoid importing the module multiple times +modules = {} + +# Possible ways of fetching a tool (FETCH_COUNT is number of ways) +FETCH_ANY, FETCH_BIN, FETCH_BUILD, FETCH_COUNT = range(4) + +FETCH_NAMES = { + FETCH_ANY: 'any method', + FETCH_BIN: 'binary download', + FETCH_BUILD: 'build from source' + } + +# Status of tool fetching +FETCHED, FAIL, PRESENT, STATUS_COUNT = range(4) + +DOWNLOAD_DESTDIR = os.path.join(os.getenv('HOME'), 'bin') + +class Bintool: + """Tool which operates on binaries to help produce entry contents + + This is the base class for all bintools + """ + # List of bintools to regard as missing + missing_list = [] + + def __init__(self, name, desc): + self.name = name + self.desc = desc + + @staticmethod + def find_bintool_class(btype): + """Look up the bintool class for bintool + + Args: + byte: Bintool to use, e.g. 'mkimage' + + Returns: + The bintool class object if found, else a tuple: + module name that could not be found + exception received + """ + # Convert something like 'u-boot' to 'u_boot' since we are only + # interested in the type. + module_name = btype.replace('-', '_') + module = modules.get(module_name) + + # Import the module if we have not already done so + if not module: + try: + module = importlib.import_module('binman.btool.' + module_name) + except ImportError as exc: + return module_name, exc + modules[module_name] = module + + # Look up the expected class name + return getattr(module, 'Bintool%s' % module_name) + + @staticmethod + def create(name): + """Create a new bintool object + + Args: + name (str): Bintool to create, e.g. 'mkimage' + + Returns: + A new object of the correct type (a subclass of Binutil) + """ + cls = Bintool.find_bintool_class(name) + if isinstance(cls, tuple): + raise ValueError("Cannot import bintool module '%s': %s" % cls) + + # Call its constructor to get the object we want. + obj = cls(name) + return obj + + def show(self): + """Show a line of information about a bintool""" + if self.is_present(): + version = self.version() + else: + version = '-' + print(FORMAT % (self.name, version, self.desc, + self.get_path() or '(not found)')) + + @classmethod + def set_missing_list(cls, missing_list): + cls.missing_list = missing_list or [] + + @staticmethod + def get_tool_list(include_testing=False): + """Get a list of the known tools + + Returns: + list of str: names of all tools known to binman + """ + files = glob.glob(os.path.join(BINMAN_DIR, 'btool/*')) + names = [os.path.splitext(os.path.basename(fname))[0] + for fname in files] + names = [name for name in names if name[0] != '_'] + if include_testing: + names.append('_testing') + return sorted(names) + + @staticmethod + def list_all(): + """List all the bintools known to binman""" + names = Bintool.get_tool_list() + print(FORMAT % ('Name', 'Version', 'Description', 'Path')) + print(FORMAT % ('-' * 15,'-' * 11, '-' * 25, '-' * 30)) + for name in names: + btool = Bintool.create(name) + btool.show() + + def is_present(self): + """Check if a bintool is available on the system + + Returns: + bool: True if available, False if not + """ + if self.name in self.missing_list: + return False + return bool(self.get_path()) + + def get_path(self): + """Get the path of a bintool + + Returns: + str: Path to the tool, if available, else None + """ + return tools.tool_find(self.name) + + def fetch_tool(self, method, col, skip_present): + """Fetch a single tool + + Args: + method (FETCH_...): Method to use + col (terminal.Color): Color terminal object + skip_present (boo;): Skip fetching if it is already present + + Returns: + int: Result of fetch either FETCHED, FAIL, PRESENT + """ + def try_fetch(meth): + res = None + try: + res = self.fetch(meth) + except urllib.error.URLError as uerr: + message = uerr.reason + print(col.Color(col.RED, f'- {message}')) + + except ValueError as exc: + print(f'Exception: {exc}') + return res + + if skip_present and self.is_present(): + return PRESENT + print(col.Color(col.YELLOW, 'Fetch: %s' % self.name)) + if method == FETCH_ANY: + for try_method in range(1, FETCH_COUNT): + print(f'- trying method: {FETCH_NAMES[try_method]}') + result = try_fetch(try_method) + if result: + break + else: + result = try_fetch(method) + if not result: + return FAIL + if result is not True: + fname, tmpdir = result + dest = os.path.join(DOWNLOAD_DESTDIR, self.name) + print(f"- writing to '{dest}'") + shutil.move(fname, dest) + if tmpdir: + shutil.rmtree(tmpdir) + return FETCHED + + @staticmethod + def fetch_tools(method, names_to_fetch): + """Fetch bintools from a suitable place + + This fetches or builds the requested bintools so that they can be used + by binman + + Args: + names_to_fetch (list of str): names of bintools to fetch + + Returns: + True on success, False on failure + """ + def show_status(color, prompt, names): + print(col.Color( + color, f'{prompt}:%s{len(names):2}: %s' % + (' ' * (16 - len(prompt)), ' '.join(names)))) + + col = terminal.Color() + skip_present = False + name_list = names_to_fetch + if len(names_to_fetch) == 1 and names_to_fetch[0] in ['all', 'missing']: + name_list = Bintool.get_tool_list() + if names_to_fetch[0] == 'missing': + skip_present = True + print(col.Color(col.YELLOW, + 'Fetching tools: %s' % ' '.join(name_list))) + status = collections.defaultdict(list) + for name in name_list: + btool = Bintool.create(name) + result = btool.fetch_tool(method, col, skip_present) + status[result].append(name) + if result == FAIL: + if method == FETCH_ANY: + print('- failed to fetch with all methods') + else: + print(f"- method '{FETCH_NAMES[method]}' is not supported") + + if len(name_list) > 1: + if skip_present: + show_status(col.GREEN, 'Already present', status[PRESENT]) + show_status(col.GREEN, 'Tools fetched', status[FETCHED]) + if status[FAIL]: + show_status(col.RED, 'Failures', status[FAIL]) + return not status[FAIL] + + def run_cmd_result(self, *args, binary=False, raise_on_error=True): + """Run the bintool using command-line arguments + + Args: + args (list of str): Arguments to provide, in addition to the bintool + name + binary (bool): True to return output as bytes instead of str + raise_on_error (bool): True to raise a ValueError exception if the + tool returns a non-zero return code + + Returns: + CommandResult: Resulting output from the bintool, or None if the + tool is not present + """ + if self.name in self.missing_list: + return None + name = os.path.expanduser(self.name) # Expand paths containing ~ + all_args = (name,) + args + env = tools.get_env_with_path() + tout.Detail(f"bintool: {' '.join(all_args)}") + result = command.RunPipe( + [all_args], capture=True, capture_stderr=True, env=env, + raise_on_error=False, binary=binary) + + if result.return_code: + # Return None if the tool was not found. In this case there is no + # output from the tool and it does not appear on the path. We still + # try to run it (as above) since RunPipe() allows faking the tool's + # output + if not any([result.stdout, result.stderr, tools.tool_find(name)]): + tout.Info(f"bintool '{name}' not found") + return None + if raise_on_error: + tout.Info(f"bintool '{name}' failed") + raise ValueError("Error %d running '%s': %s" % + (result.return_code, ' '.join(all_args), + result.stderr or result.stdout)) + if result.stdout: + tout.Debug(result.stdout) + if result.stderr: + tout.Debug(result.stderr) + return result + + def run_cmd(self, *args, binary=False): + """Run the bintool using command-line arguments + + Args: + args (list of str): Arguments to provide, in addition to the bintool + name + binary (bool): True to return output as bytes instead of str + + Returns: + str or bytes: Resulting stdout from the bintool + """ + result = self.run_cmd_result(*args, binary=binary) + if result: + return result.stdout + + @classmethod + def build_from_git(cls, git_repo, make_target, bintool_path): + """Build a bintool from a git repo + + This clones the repo in a temporary directory, builds it with 'make', + then returns the filename of the resulting executable bintool + + Args: + git_repo (str): URL of git repo + make_target (str): Target to pass to 'make' to build the tool + bintool_path (str): Relative path of the tool in the repo, after + build is complete + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + or None on error + """ + tmpdir = tempfile.mkdtemp(prefix='binmanf.') + print(f"- clone git repo '{git_repo}' to '{tmpdir}'") + tools.Run('git', 'clone', '--depth', '1', git_repo, tmpdir) + print(f"- build target '{make_target}'") + tools.Run('make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}', + make_target) + fname = os.path.join(tmpdir, bintool_path) + if not os.path.exists(fname): + print(f"- File '{fname}' was not produced") + return None + return fname, tmpdir + + @classmethod + def fetch_from_url(cls, url): + """Fetch a bintool from a URL + + Args: + url (str): URL to fetch from + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + """ + fname, tmpdir = tools.Download(url) + tools.Run('chmod', 'a+x', fname) + return fname, tmpdir + + @classmethod + def fetch_from_drive(cls, drive_id): + """Fetch a bintool from Google drive + + Args: + drive_id (str): ID of file to fetch. For a URL of the form + 'https://drive.google.com/file/d/xxx/view?usp=sharing' the value + passed here should be 'xxx' + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + """ + url = f'https://drive.google.com/uc?export=download&id=%7Bdrive_id%7D' + return cls.fetch_from_url(url) + + @classmethod + def apt_install(cls, package): + """Install a bintool using the 'aot' tool + + This requires use of servo so may request a password + + Args: + package (str): Name of package to install + + Returns: + True, assuming it completes without error + """ + args = ['sudo', 'apt', 'install', '-y', package] + print('- %s' % ' '.join(args)) + tools.Run(*args) + return True + + # pylint: disable=W0613 + def fetch(self, method): + """Fetch handler for a bintool + + This should be implemented by the base class + + Args: + method (FETCH_...): Method to use + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + or True if the file was fetched and already installed + or None if no fetch() implementation is available + + Raises: + Valuerror: Fetching could not be completed + """ + print(f"No method to fetch bintool '{self.name}'") + return False + + # pylint: disable=R0201 + def version(self): + """Version handler for a bintool + + This should be implemented by the base class + + Returns: + str: Version string for this bintool + """ + return 'unknown'

Binman requires various tools to actually work, such as 'lz4' to compress data and 'futility' to sign Chrome OS firmware. At present these are handled in an ad-hoc manner and there is no easy way to find out what tools are needd to build an image, nor where to obtain them.
Add an implementation of 'bintool', a base class which implements this functionality. When a bintool is required, it can be requested from this module, then executed. When the tool is missing, it can provide a way to obtain it.
Note that this uses Command directly, not the tools.Run() function. This allows proper handling of missing tools and avoids needing to catch and re-raise exceptions.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool.py | 421 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 421 insertions(+) create mode 100644 tools/binman/bintool.py
Applied to u-boot-dm, thanks!

Support collecting the available bintools needed by an image, by scanning the entries in the image.
Also add a command-line interface to access the basic bintool features, such as listing the bintools and fetching them if needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cmdline.py | 7 +++++++ tools/binman/control.py | 17 ++++++++++++++++- tools/binman/entry.py | 22 ++++++++++++++++++++++ tools/binman/etype/section.py | 4 ++++ tools/binman/ftest.py | 1 + tools/binman/image.py | 14 ++++++++++++++ 6 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 6c685954619..92cc14b6fc7 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -167,4 +167,11 @@ controlled by a description in the board device tree.''' test_parser.add_argument('tests', nargs='*', help='Test names to run (omit for all)')
+ tool_parser = subparsers.add_parser('tool', help='Check bintools') + tool_parser.add_argument('-l', '--list', action='store_true', + help='List all known bintools') + tool_parser.add_argument('-f', '--fetch', action='store_true', + help='fetch a bintool from a known location (or: all/missing)') + tool_parser.add_argument('bintools', type=str, nargs='*') + return parser.parse_args(argv) diff --git a/tools/binman/control.py b/tools/binman/control.py index e0d2b3879d8..5b10f192360 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -14,6 +14,7 @@ import re import sys from patman import tools
+from binman import bintool from binman import cbfs_util from binman import elf from patman import command @@ -487,6 +488,7 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): # without changing the device-tree size, thus ensuring that our # entry offsets remain the same. for image in images.values(): + image.CollectBintools() image.ExpandEntries() if update_fdt: image.AddMissingProperties(True) @@ -606,7 +608,7 @@ def Binman(args): from binman.image import Image from binman import state
- if args.cmd in ['ls', 'extract', 'replace']: + if args.cmd in ['ls', 'extract', 'replace', 'tool']: try: tout.Init(args.verbosity) tools.PrepareOutputDir(None) @@ -621,6 +623,19 @@ def Binman(args): ReplaceEntries(args.image, args.filename, args.indir, args.paths, do_compress=not args.compressed, allow_resize=not args.fix_size, write_map=args.map) + + if args.cmd == 'tool': + tools.SetToolPaths(args.toolpath) + if args.list: + bintool.Bintool.list_all() + elif args.fetch: + if not args.bintools: + raise ValueError( + "Please specify bintools to fetch or 'all' or 'missing'") + bintool.Bintool.fetch_tools(bintool.FETCH_ANY, + args.bintools) + else: + raise ValueError("Invalid arguments to 'tool' subcommand") except: raise finally: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e4a1f2d5d5c..9cd900670e1 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -10,6 +10,7 @@ import os import pathlib import sys
+from binman import bintool from dtoc import fdt_util from patman import tools from patman.tools import ToHex, ToHexSize @@ -74,6 +75,7 @@ class Entry(object): allow_fake: Allow creating a dummy fake file if the blob file is not available. This is mainly used for testing. external: True if this entry contains an external binary blob + bintools: Bintools used by this entry (only populated for Image) """ def __init__(self, section, etype, node, name_prefix=''): # Put this here to allow entry-docs and help to work without libfdt @@ -105,6 +107,7 @@ class Entry(object): self.external = False self.allow_missing = False self.allow_fake = False + self.bintools = {}
@staticmethod def FindEntryClass(etype, expanded): @@ -1065,3 +1068,22 @@ features to produce new behaviours. value: Help text """ pass + + def AddBintools(self, tools): + """Add the bintools used by this entry type + + Args: + tools (dict of Bintool): + """ + pass + + @classmethod + def AddBintool(self, tools, name): + """Add a new bintool to the tools used by this etype + + Args: + name: Name of the tool + """ + btool = bintool.Bintool.create(name) + tools[name] = btool + return btool diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index fdd4cbb21ad..221a64cd032 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -880,3 +880,7 @@ class Entry_section(Entry): def CheckAltFormats(self, alt_formats): for entry in self._entries.values(): entry.CheckAltFormats(alt_formats) + + def AddBintools(self, tools): + for entry in self._entries.values(): + entry.AddBintools(tools) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ac6aabbf9c3..179326c42a3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -18,6 +18,7 @@ import sys import tempfile import unittest
+from binman import bintool from binman import cbfs_util from binman import cmdline from binman import control diff --git a/tools/binman/image.py b/tools/binman/image.py index f0a7d65299e..0f0c1d29e80 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -82,6 +82,7 @@ class Image(section.Entry_section): self.missing_etype = missing_etype self.use_expanded = use_expanded self.test_section_timeout = False + self.bintools = {} if not test: self.ReadNode()
@@ -394,3 +395,16 @@ class Image(section.Entry_section): self._CollectEntries(entries, entries_by_name, self) return self.LookupSymbol(sym_name, optional, msg, base_addr, entries_by_name) + + def CollectBintools(self): + """Collect all the bintools used by this image + + Returns: + Dict of bintools: + key: name of tool + value: Bintool object + """ + bintools = {} + super().AddBintools(bintools) + self.bintools = bintools + return bintools

Support collecting the available bintools needed by an image, by scanning the entries in the image.
Also add a command-line interface to access the basic bintool features, such as listing the bintools and fetching them if needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cmdline.py | 7 +++++++ tools/binman/control.py | 17 ++++++++++++++++- tools/binman/entry.py | 22 ++++++++++++++++++++++ tools/binman/etype/section.py | 4 ++++ tools/binman/ftest.py | 1 + tools/binman/image.py | 14 ++++++++++++++ 6 files changed, 64 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Add tests to cover the bintool functionality.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool_test.py | 353 +++++++++++++++++++++++++++++++++ tools/binman/btool/_testing.py | 36 ++++ 2 files changed, 389 insertions(+) create mode 100644 tools/binman/bintool_test.py create mode 100644 tools/binman/btool/_testing.py
diff --git a/tools/binman/bintool_test.py b/tools/binman/bintool_test.py new file mode 100644 index 00000000000..3d6bcdab9d1 --- /dev/null +++ b/tools/binman/bintool_test.py @@ -0,0 +1,353 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# Written by Simon Glass sjg@chromium.org +# + +"""Tests for the Bintool class""" + +import collections +import os +import shutil +import tempfile +import unittest +import unittest.mock +import urllib.error + +from binman import bintool +from binman.bintool import Bintool + +from patman import command +from patman import terminal +from patman import test_util +from patman import tools + +# pylint: disable=R0904 +class TestBintool(unittest.TestCase): + """Tests for the Bintool class""" + def setUp(self): + # Create a temporary directory for test files + self._indir = tempfile.mkdtemp(prefix='bintool.') + self.seq = None + self.count = None + self.fname = None + self.btools = None + + def tearDown(self): + """Remove the temporary input directory and its contents""" + if self._indir: + shutil.rmtree(self._indir) + self._indir = None + + def test_missing_btype(self): + """Test that unknown bintool types are detected""" + with self.assertRaises(ValueError) as exc: + Bintool.create('missing') + self.assertIn("No module named 'binman.btool.missing'", + str(exc.exception)) + + def test_fresh_bintool(self): + """Check that the _testing bintool is not cached""" + btest = Bintool.create('_testing') + btest.present = True + btest2 = Bintool.create('_testing') + self.assertFalse(btest2.present) + + def test_version(self): + """Check handling of a tool being present or absent""" + btest = Bintool.create('_testing') + with test_util.capture_sys_output() as (stdout, _): + btest.show() + self.assertFalse(btest.is_present()) + self.assertIn('-', stdout.getvalue()) + btest.present = True + self.assertTrue(btest.is_present()) + self.assertEqual('123', btest.version()) + with test_util.capture_sys_output() as (stdout, _): + btest.show() + self.assertIn('123', stdout.getvalue()) + + def test_fetch_present(self): + """Test fetching of a tool""" + btest = Bintool.create('_testing') + btest.present = True + col = terminal.Color() + self.assertEqual(bintool.PRESENT, + btest.fetch_tool(bintool.FETCH_ANY, col, True)) + + @classmethod + def check_fetch_url(cls, fake_download, method): + """Check the output from fetching a tool + + Args: + fake_download (function): Function to call instead of + tools.Download() + method (bintool.FETCH_...: Fetch method to use + + Returns: + str: Contents of stdout + """ + btest = Bintool.create('_testing') + col = terminal.Color() + with unittest.mock.patch.object(tools, 'Download', + side_effect=fake_download): + with test_util.capture_sys_output() as (stdout, _): + btest.fetch_tool(method, col, False) + return stdout.getvalue() + + def test_fetch_url_err(self): + """Test an error while fetching a tool from a URL""" + def fail_download(url): + """Take the tools.Download() function by raising an exception""" + raise urllib.error.URLError('my error') + + stdout = self.check_fetch_url(fail_download, bintool.FETCH_ANY) + self.assertIn('my error', stdout) + + def test_fetch_url_exception(self): + """Test an exception while fetching a tool from a URL""" + def cause_exc(url): + raise ValueError('exc error') + + stdout = self.check_fetch_url(cause_exc, bintool.FETCH_ANY) + self.assertIn('exc error', stdout) + + def test_fetch_method(self): + """Test fetching using a particular method""" + def fail_download(url): + """Take the tools.Download() function by raising an exception""" + raise urllib.error.URLError('my error') + + stdout = self.check_fetch_url(fail_download, bintool.FETCH_BIN) + self.assertIn('my error', stdout) + + def test_fetch_pass_fail(self): + """Test fetching multiple tools with some passing and some failing""" + def handle_download(_): + """Take the tools.Download() function by writing a file""" + if self.seq: + raise urllib.error.URLError('not found') + self.seq += 1 + tools.WriteFile(fname, expected) + return fname, dirname + + expected = b'this is a test' + dirname = os.path.join(self._indir, 'download_dir') + os.mkdir(dirname) + fname = os.path.join(dirname, 'downloaded') + destdir = os.path.join(self._indir, 'dest_dir') + os.mkdir(destdir) + dest_fname = os.path.join(destdir, '_testing') + self.seq = 0 + + with unittest.mock.patch.object(bintool, 'DOWNLOAD_DESTDIR', destdir): + with unittest.mock.patch.object(tools, 'Download', + side_effect=handle_download): + with test_util.capture_sys_output() as (stdout, _): + Bintool.fetch_tools(bintool.FETCH_ANY, ['_testing'] * 2) + self.assertTrue(os.path.exists(dest_fname)) + data = tools.ReadFile(dest_fname) + self.assertEqual(expected, data) + + lines = stdout.getvalue().splitlines() + self.assertTrue(len(lines) > 2) + self.assertEqual('Tools fetched: 1: _testing', lines[-2]) + self.assertEqual('Failures: 1: _testing', lines[-1]) + + def test_tool_list(self): + """Test listing available tools""" + self.assertGreater(len(Bintool.get_tool_list()), 3) + + def check_fetch_all(self, method): + """Helper to check the operation of fetching all tools""" + + # pylint: disable=W0613 + def fake_fetch(method, col, skip_present): + """Fakes the Binutils.fetch() function + + Returns FETCHED and FAIL on alternate calls + """ + self.seq += 1 + result = bintool.FETCHED if self.seq & 1 else bintool.FAIL + self.count[result] += 1 + return result + + self.seq = 0 + self.count = collections.defaultdict(int) + with unittest.mock.patch.object(bintool.Bintool, 'fetch_tool', + side_effect=fake_fetch): + with test_util.capture_sys_output() as (stdout, _): + Bintool.fetch_tools(method, ['all']) + lines = stdout.getvalue().splitlines() + self.assertIn(f'{self.count[bintool.FETCHED]}: ', lines[-2]) + self.assertIn(f'{self.count[bintool.FAIL]}: ', lines[-1]) + + def test_fetch_all(self): + """Test fetching all tools""" + self.check_fetch_all(bintool.FETCH_ANY) + + def test_fetch_all_specific(self): + """Test fetching all tools with a specific method""" + self.check_fetch_all(bintool.FETCH_BIN) + + def test_fetch_missing(self): + """Test fetching missing tools""" + # pylint: disable=W0613 + def fake_fetch2(method, col, skip_present): + """Fakes the Binutils.fetch() function + + Returns PRESENT only for the '_testing' bintool + """ + btool = list(self.btools.values())[self.seq] + self.seq += 1 + print('fetch', btool.name) + if btool.name == '_testing': + return bintool.PRESENT + return bintool.FETCHED + + # Preload a list of tools to return when get_tool_list() and create() + # are called + all_tools = Bintool.get_tool_list(True) + self.btools = collections.OrderedDict() + for name in all_tools: + self.btools[name] = Bintool.create(name) + self.seq = 0 + with unittest.mock.patch.object(bintool.Bintool, 'fetch_tool', + side_effect=fake_fetch2): + with unittest.mock.patch.object(bintool.Bintool, + 'get_tool_list', + side_effect=[all_tools]): + with unittest.mock.patch.object(bintool.Bintool, 'create', + side_effect=self.btools.values()): + with test_util.capture_sys_output() as (stdout, _): + Bintool.fetch_tools(bintool.FETCH_ANY, ['missing']) + lines = stdout.getvalue().splitlines() + num_tools = len(self.btools) + fetched = [line for line in lines if 'Tools fetched:' in line].pop() + present = [line for line in lines if 'Already present:' in line].pop() + self.assertIn(f'{num_tools - 1}: ', fetched) + self.assertIn('1: ', present) + + def check_build_method(self, write_file): + """Check the output from fetching using the BUILD method + + Args: + write_file (bool): True to write the output file when 'make' is + called + + Returns: + tuple: + str: Filename of written file (or missing 'make' output) + str: Contents of stdout + """ + def fake_run(*cmd): + if cmd[0] == 'make': + # See Bintool.build_from_git() + tmpdir = cmd[2] + self.fname = os.path.join(tmpdir, 'pathname') + if write_file: + tools.WriteFile(self.fname, b'hello') + + btest = Bintool.create('_testing') + col = terminal.Color() + self.fname = None + with unittest.mock.patch.object(bintool, 'DOWNLOAD_DESTDIR', + self._indir): + with unittest.mock.patch.object(tools, 'Run', side_effect=fake_run): + with test_util.capture_sys_output() as (stdout, _): + btest.fetch_tool(bintool.FETCH_BUILD, col, False) + fname = os.path.join(self._indir, '_testing') + return fname if write_file else self.fname, stdout.getvalue() + + def test_build_method(self): + """Test fetching using the build method""" + fname, stdout = self.check_build_method(write_file=True) + self.assertTrue(os.path.exists(fname)) + self.assertIn(f"writing to '{fname}", stdout) + + def test_build_method_fail(self): + """Test fetching using the build method when no file is produced""" + fname, stdout = self.check_build_method(write_file=False) + self.assertFalse(os.path.exists(fname)) + self.assertIn(f"File '{fname}' was not produced", stdout) + + def test_install(self): + """Test fetching using the install method""" + btest = Bintool.create('_testing') + btest.install = True + col = terminal.Color() + with unittest.mock.patch.object(tools, 'Run', return_value=None): + with test_util.capture_sys_output() as _: + result = btest.fetch_tool(bintool.FETCH_BIN, col, False) + self.assertEqual(bintool.FETCHED, result) + + def test_no_fetch(self): + """Test fetching when there is no method""" + btest = Bintool.create('_testing') + btest.disable = True + col = terminal.Color() + with test_util.capture_sys_output() as _: + result = btest.fetch_tool(bintool.FETCH_BIN, col, False) + self.assertEqual(bintool.FAIL, result) + + def test_all_bintools(self): + """Test that all bintools can handle all available fetch types""" + def handle_download(_): + """Take the tools.Download() function by writing a file""" + tools.WriteFile(fname, expected) + return fname, dirname + + def fake_run(*cmd): + if cmd[0] == 'make': + # See Bintool.build_from_git() + tmpdir = cmd[2] + self.fname = os.path.join(tmpdir, 'pathname') + tools.WriteFile(self.fname, b'hello') + + expected = b'this is a test' + dirname = os.path.join(self._indir, 'download_dir') + os.mkdir(dirname) + fname = os.path.join(dirname, 'downloaded') + + with unittest.mock.patch.object(tools, 'Run', side_effect=fake_run): + with unittest.mock.patch.object(tools, 'Download', + side_effect=handle_download): + with test_util.capture_sys_output() as _: + for name in Bintool.get_tool_list(): + btool = Bintool.create(name) + for method in range(bintool.FETCH_COUNT): + result = btool.fetch(method) + self.assertTrue(result is not False) + if result is not True and result is not None: + result_fname, _ = result + self.assertTrue(os.path.exists(result_fname)) + data = tools.ReadFile(result_fname) + self.assertEqual(expected, data) + os.remove(result_fname) + + def test_all_bintool_versions(self): + """Test handling of bintool version when it cannot be run""" + all_tools = Bintool.get_tool_list() + for name in all_tools: + btool = Bintool.create(name) + with unittest.mock.patch.object( + btool, 'run_cmd_result', return_value=command.CommandResult()): + self.assertEqual('unknown', btool.version()) + + def test_force_missing(self): + btool = Bintool.create('_testing') + btool.present = True + self.assertTrue(btool.is_present()) + + btool.present = None + Bintool.set_missing_list(['_testing']) + self.assertFalse(btool.is_present()) + + def test_failed_command(self): + """Check that running a command that does not exist returns None""" + btool = Bintool.create('_testing') + result = btool.run_cmd_result('fred') + self.assertIsNone(result) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/binman/btool/_testing.py b/tools/binman/btool/_testing.py new file mode 100644 index 00000000000..4005e8a8a5d --- /dev/null +++ b/tools/binman/btool/_testing.py @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool used for testing + +This is not a real bintool, just one used for testing""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintool_testing(bintool.Bintool): + """Bintool used for testing""" + def __init__(self, name): + super().__init__(name, 'testing') + self.present = False + self.install = False + self.disable = False + + def is_present(self): + if self.present is None: + return super().is_present() + return self.present + + def version(self): + return '123' + + def fetch(self, method): + if self.disable: + return super().fetch(method) + if method == bintool.FETCH_BIN: + if self.install: + return self.apt_install('package') + return self.fetch_from_drive('junk') + if method == bintool.FETCH_BUILD: + return self.build_from_git('url', 'target', 'pathname') + return None

Add tests to cover the bintool functionality.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool_test.py | 353 +++++++++++++++++++++++++++++++++ tools/binman/btool/_testing.py | 36 ++++ 2 files changed, 389 insertions(+) create mode 100644 tools/binman/bintool_test.py create mode 100644 tools/binman/btool/_testing.py
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to run CBFS tests. It supports the features needed by the tests as well as fetching a binary from Google Drive. Building it from source is very slow since it is not separately supported by the coreboot build system and it builds an entire gcc toolchain before starting.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/cbfstool.py | 219 +++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 tools/binman/btool/cbfstool.py
diff --git a/tools/binman/btool/cbfstool.py b/tools/binman/btool/cbfstool.py new file mode 100644 index 00000000000..29be2d8a2b5 --- /dev/null +++ b/tools/binman/btool/cbfstool.py @@ -0,0 +1,219 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for cbfstool + +cfstool provides a number of features useful with Coreboot Filesystem binaries. + +Documentation is at https://www.coreboot.org/CBFS + +Source code is at https://github.com/coreboot/coreboot/blob/master/util/cbfstool/cbfstool.c + +Here is the help: + +cbfstool: Management utility for CBFS formatted ROM images + +USAGE: + cbfstool [-h] + cbfstool FILE COMMAND [-v] [PARAMETERS]... + +OPTIONs: + -H header_offset Do not search for header; use this offset* + -T Output top-aligned memory address + -u Accept short data; fill upward/from bottom + -d Accept short data; fill downward/from top + -F Force action + -g Generate position and alignment arguments + -U Unprocessed; don't decompress or make ELF + -v Provide verbose output + -h Display this help message + +COMMANDs: + add [-r image,regions] -f FILE -n NAME -t TYPE [-A hash] \ + [-c compression] [-b base-address | -a alignment] \ + [-p padding size] [-y|--xip if TYPE is FSP] \ + [-j topswap-size] (Intel CPUs only) [--ibb] + Add a component + -j valid size: 0x10000 0x20000 0x40000 0x80000 0x100000 + add-payload [-r image,regions] -f FILE -n NAME [-A hash] \ + [-c compression] [-b base-address] \ + (linux specific: [-C cmdline] [-I initrd]) + Add a payload to the ROM + add-stage [-r image,regions] -f FILE -n NAME [-A hash] \ + [-c compression] [-b base] [-S section-to-ignore] \ + [-a alignment] [-y|--xip] [-P page-size] [--ibb] + Add a stage to the ROM + add-flat-binary [-r image,regions] -f FILE -n NAME \ + [-A hash] -l load-address -e entry-point \ + [-c compression] [-b base] + Add a 32bit flat mode binary + add-int [-r image,regions] -i INTEGER -n NAME [-b base] + Add a raw 64-bit integer value + add-master-header [-r image,regions] \ + [-j topswap-size] (Intel CPUs only) + Add a legacy CBFS master header + remove [-r image,regions] -n NAME + Remove a component + compact -r image,regions + Defragment CBFS image. + copy -r image,regions -R source-region + Create a copy (duplicate) cbfs instance in fmap + create -m ARCH -s size [-b bootblock offset] \ + [-o CBFS offset] [-H header offset] [-B bootblock] + Create a legacy ROM file with CBFS master header* + create -M flashmap [-r list,of,regions,containing,cbfses] + Create a new-style partitioned firmware image + locate [-r image,regions] -f FILE -n NAME [-P page-size] \ + [-a align] [-T] + Find a place for a file of that size + layout [-w] + List mutable (or, with -w, readable) image regions + print [-r image,regions] + Show the contents of the ROM + extract [-r image,regions] [-m ARCH] -n NAME -f FILE [-U] + Extracts a file from ROM + write [-F] -r image,regions -f file [-u | -d] [-i int] + Write file into same-size [or larger] raw region + read [-r fmap-region] -f file + Extract raw region contents into binary file + truncate [-r fmap-region] + Truncate CBFS and print new size on stdout + expand [-r fmap-region] + Expand CBFS to span entire region +OFFSETs: + Numbers accompanying -b, -H, and -o switches* may be provided + in two possible formats: if their value is greater than + 0x80000000, they are interpreted as a top-aligned x86 memory + address; otherwise, they are treated as an offset into flash. +ARCHes: + arm64, arm, mips, ppc64, power8, riscv, x86, unknown +TYPEs: + bootblock, cbfs header, stage, simple elf, fit, optionrom, bootsplash, raw, + vsa, mbi, microcode, fsp, mrc, cmos_default, cmos_layout, spd, + mrc_cache, mma, efi, struct, deleted, null + +* Note that these actions and switches are only valid when + working with legacy images whose structure is described + primarily by a CBFS master header. New-style images, in + contrast, exclusively make use of an FMAP to describe their + layout: this must minimally contain an 'FMAP' section + specifying the location of this FMAP itself and a 'COREBOOT' + section describing the primary CBFS. It should also be noted + that, when working with such images, the -F and -r switches + default to 'COREBOOT' for convenience, and both the -b switch to + CBFS operations and the output of the locate action become + relative to the selected CBFS region's lowest address. + The one exception to this rule is the top-aligned address, + which is always relative to the end of the entire image + rather than relative to the local region; this is true for + for both input (sufficiently large) and output (-T) data. + + +Since binman has a native implementation of CBFS (see cbfs_util.py), we don't +actually need this tool, except for sanity checks in the tests. +""" + +from binman import bintool + +class Bintoolcbfstool(bintool.Bintool): + """Coreboot filesystem (CBFS) tool + + This bintool supports creating new CBFS images and adding files to an + existing image, i.e. the features needed by binman. + + It also supports fetching a binary cbfstool, since building it from source + is fairly slow. + + Documentation about CBFS is at https://www.coreboot.org/CBFS + """ + def __init__(self, name): + super().__init__(name, 'Manipulate CBFS files') + + def create_new(self, cbfs_fname, size, arch='x86'): + """Create a new CBFS + + Args: + cbfs_fname (str): Filename of CBFS to create + size (int): Size of CBFS in bytes + arch (str): Architecture for which this CBFS is intended + + Returns: + str: Tool output + """ + args = [cbfs_fname, 'create', '-s', f'{size:#x}', '-m', arch] + return self.run_cmd(*args) + + # pylint: disable=R0913 + def add_raw(self, cbfs_fname, name, fname, compress=None, base=None): + """Add a raw file to the CBFS + + Args: + cbfs_fname (str): Filename of CBFS to create + name (str): Name to use inside the CBFS + fname (str): Filename of file to add + compress (str): Compression to use (cbfs_util.COMPRESS_NAMES) or + None for None + base (int): Address to place the file, or None for anywhere + + Returns: + str: Tool output + """ + args = [cbfs_fname, + 'add', + '-n', name, + '-t', 'raw', + '-f', fname, + '-c', compress or 'none'] + if base: + args += ['-b', f'{base:#x}'] + return self.run_cmd(*args) + + def add_stage(self, cbfs_fname, name, fname): + """Add a stage file to the CBFS + + Args: + cbfs_fname (str): Filename of CBFS to create + name (str): Name to use inside the CBFS + fname (str): Filename of file to add + + Returns: + str: Tool output + """ + args = [cbfs_fname, + 'add-stage', + '-n', name, + '-f', fname + ] + return self.run_cmd(*args) + + def fail(self): + """Run cbfstool with invalid arguments to check it reports failure + + This is really just a sanity check + + Returns: + CommandResult: Result from running the bad command + """ + args = ['missing-file', 'bad-command'] + return self.run_cmd_result(*args) + + def fetch(self, method): + """Fetch handler for cbfstool + + This installs cbfstool by downloading from Google Drive. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched and now installed, None if a method + other than FETCH_BIN was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + fname, tmpdir = self.fetch_from_drive( + '1IOnE0Qvy97d-0WOCwF64xBGpKSY2sMtJ') + return fname, tmpdir

Add a Bintool for this, which is used to run CBFS tests. It supports the features needed by the tests as well as fetching a binary from Google Drive. Building it from source is very slow since it is not separately supported by the coreboot build system and it builds an entire gcc toolchain before starting.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/cbfstool.py | 219 +++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 tools/binman/btool/cbfstool.py
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to run FIP tests. It supports the features needed by the tests as well as building a binary from the git tree.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/fiptool.py | 123 ++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 tools/binman/btool/fiptool.py
diff --git a/tools/binman/btool/fiptool.py b/tools/binman/btool/fiptool.py new file mode 100644 index 00000000000..c6d71cebc21 --- /dev/null +++ b/tools/binman/btool/fiptool.py @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for fiptool + +fiptool provides a way to package firmware in an ARM Trusted Firmware Firmware +Image Package (ATF FIP) format. It is used with Trusted Firmware A, for example. + +Documentation is at: +https://trustedfirmware-a.readthedocs.io/en/latest/getting_started/tools-bui... + +Source code is at: +https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git + +Here is the help: + +usage: fiptool [--verbose] <command> [<args>] +Global options supported: + --verbose Enable verbose output for all commands. + +Commands supported: + info List images contained in FIP. + create Create a new FIP with the given images. + update Update an existing FIP with the given images. + unpack Unpack images from FIP. + remove Remove images from FIP. + version Show fiptool version. + help Show help for given command. + +""" + +from binman import bintool + +class Bintoolfiptool(bintool.Bintool): + """Image generation for ARM Trusted Firmware + + This bintool supports running `fiptool` with some basic parameters as + neeed by binman. + + It also supports build fiptool from source. + + fiptool provides a way to package firmware in an ARM Trusted Firmware + Firmware Image Package (ATF FIP) format. It is used with Trusted Firmware A, + for example. + + See `TF-A FIP tool documentation`_ for more information. + + .. _`TF-A FIP tool documentation`: + https://trustedfirmware-a.readthedocs.io/en/latest/getting_started/tools-bui... + """ + def __init__(self, name): + super().__init__(name, 'Manipulate ATF FIP files') + + def info(self, fname): + """Get info on a FIP image + + Args: + fname (str): Filename to check + + Returns: + str: Tool output + """ + args = ['info', fname] + return self.run_cmd(*args) + + # pylint: disable=R0913 + def create_new(self, fname, align, plat_toc_flags, fwu, tb_fw, blob_uuid, + blob_file): + """Create a new FIP + + Args: + fname (str): Filename to write to + align (int): Alignment to use for entries + plat_toc_flags (int): Flags to use for the TOC header + fwu (str): Filename for the fwu entry + tb_fw (str): Filename for the tb_fw entry + blob_uuid (str): UUID for the blob entry + blob_file (str): Filename for the blob entry + + Returns: + str: Tool output + """ + args = [ + 'create', + '--align', f'{align:x}', + '--plat-toc-flags', f'{plat_toc_flags:#x}', + '--fwu', fwu, + '--tb-fw', tb_fw, + '--blob', f'uuid={blob_uuid},file={blob_file}', + fname] + return self.run_cmd(*args) + + def create_bad(self): + """Run fiptool with invalid arguments""" + args = ['create', '--fred'] + return self.run_cmd_result(*args) + + def fetch(self, method): + """Fetch handler for fiptool + + This builds the tool from source + + Returns: + tuple: + str: Filename of fetched file to copy to a suitable directory + str: Name of temp directory to remove, or None + """ + if method != bintool.FETCH_BUILD: + return None + result = self.build_from_git( + 'https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git', + 'fiptool', + 'tools/fiptool/fiptool') + return result + + def version(self): + """Version handler for fiptool + + Returns: + str: Version number of fiptool + """ + out = self.run_cmd('version').strip() + return out or super().version()

Add a Bintool for this, which is used to run FIP tests. It supports the features needed by the tests as well as building a binary from the git tree.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/fiptool.py | 123 ++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 tools/binman/btool/fiptool.py
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to sign Chrome OS images and build the Google Binary Block (GBB). It supports the features needed by binman as well as fetching a binary from Google Drive. Building it from source is possible but is left for another time, as it requires at least one other library.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/futility.py | 178 +++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 tools/binman/btool/futility.py
diff --git a/tools/binman/btool/futility.py b/tools/binman/btool/futility.py new file mode 100644 index 00000000000..614daaade43 --- /dev/null +++ b/tools/binman/btool/futility.py @@ -0,0 +1,178 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for futility + +futility (flash utility) is a tool for working with Chromium OS flash images. +This implements just the features used by Binman. + +Documentation is at: + https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/mai... + +Source code: + https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/mas... + +Here is the help: +Usage: futility [options] COMMAND [args...] + +This is the unified firmware utility, which will eventually replace +most of the distinct verified boot tools formerly produced by the +vboot_reference package. + +When symlinked under the name of one of those previous tools, it should +fully implement the original behavior. It can also be invoked directly +as futility, followed by the original name as the first argument. + +Global options: + + --vb1 Use only vboot v1.0 binary formats + --vb21 Use only vboot v2.1 binary formats + --debug Be noisy about what's going on + +The following commands are built-in: + + bdb Common boot flow utility + create Create a keypair from an RSA .pem file + dump_fmap Display FMAP contents from a firmware image + dump_kernel_config Prints the kernel command line + gbb Manipulate the Google Binary Block (GBB) + gbb_utility Legacy name for `gbb` command + help Show a bit of help (you're looking at it) + load_fmap Replace the contents of specified FMAP areas + pcr Simulate a TPM PCR extension operation + show Display the content of various binary components + sign Sign / resign various binary components + update Update system firmware + validate_rec_mrc Validates content of Recovery MRC cache + vbutil_firmware Verified boot firmware utility + vbutil_kernel Creates, signs, and verifies the kernel partition + vbutil_key Wraps RSA keys with vboot headers + vbutil_keyblock Creates, signs, and verifies a keyblock + verify Verify the signatures of various binary components + version Show the futility source revision and build date +""" + +from binman import bintool + +class Bintoolfutility(bintool.Bintool): + """Handles the 'futility' tool + + futility (flash utility) is a tool for working with Chromium OS flash + images. This Bintool implements just the features used by Binman, related to + GBB creation and firmware signing. + + A binary version of the tool can be fetched. + + See `Chromium OS vboot documentation`_ for more information. + + .. _`Chromium OS vboot documentation`: + https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/mai... + """ + def __init__(self, name): + super().__init__(name, 'Chromium OS firmware utility') + + def gbb_create(self, fname, sizes): + """Create a new Google Binary Block + + Args: + fname (str): Filename to write to + sizes (list of int): Sizes of each regions: + hwid_size, rootkey_size, bmpfv_size, recoverykey_size + + Returns: + str: Tool output + """ + args = [ + 'gbb_utility', + '-c', + ','.join(['%#x' % size for size in sizes]), + fname + ] + return self.run_cmd(*args) + + # pylint: disable=R0913 + def gbb_set(self, fname, hwid, rootkey, recoverykey, flags, bmpfv): + """Set the parameters in a Google Binary Block + + Args: + fname (str): Filename to update + hwid (str): Hardware ID to use + rootkey (str): Filename of root key, e.g. 'root_key.vbpubk' + recoverykey (str): Filename of recovery key, + e.g. 'recovery_key.vbpubk' + flags (int): GBB flags to use + bmpfv (str): Filename of firmware bitmaps (bmpblk file) + + Returns: + str: Tool output + """ + args = ['gbb_utility' + '-s', + f'--hwid={hwid}', + f'--rootkey={rootkey}', + f'--recoverykey={recoverykey}', + f'--flags={flags}', + f'--bmpfv={bmpfv}', + fname + ] + return self.run_cmd(*args) + + def sign_firmware(self, vblock, keyblock, signprivate, version, firmware, + kernelkey, flags): + """Sign firmware to create a vblock file + + Args: + vblock (str): Filename to write the vblock too + keyblock (str): Filename of keyblock file + signprivate (str): Filename of private key + version (int): Version number + firmware (str): Filename of firmware binary to sign + kernelkey (str): Filename of kernel key + flags (int): Preamble flags + + Returns: + str: Tool output + """ + args = [ + 'vbutil_firmware', + '--vblock', vblock, + '--keyblock', keyblock, + '--signprivate', signprivate, + '--version', version, + '--fw', firmware, + '--kernelkey', kernelkey, + '--flags', flags + ] + return self.run_cmd(*args) + + def fetch(self, method): + """Fetch handler for futility + + This installs futility using a binary download. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched, None if a method other than FETCH_BIN + was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + fname, tmpdir = self.fetch_from_drive( + '1hdsInzsE4aJbmBeJ663kYgjOQyW1I-E0') + return fname, tmpdir + + def version(self): + """Version handler for futility + + Returns: + str: Version string for futility + """ + out = self.run_cmd('version').strip() + if not out: + return super().version() + return out

Add a Bintool for this, which is used to sign Chrome OS images and build the Google Binary Block (GBB). It supports the features needed by binman as well as fetching a binary from Google Drive. Building it from source is possible but is left for another time, as it requires at least one other library.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/futility.py | 178 +++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 tools/binman/btool/futility.py
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to build Intel IFWI images. It supports the features needed by the tests as well as downloading a binary from Google Drive. Although this is built in the U-Boot tree, it is not currently included with u-boot-tools, so it may be useful to install a binary on the system.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/ifwitool.py | 166 +++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 tools/binman/btool/ifwitool.py
diff --git a/tools/binman/btool/ifwitool.py b/tools/binman/btool/ifwitool.py new file mode 100644 index 00000000000..96778fce87f --- /dev/null +++ b/tools/binman/btool/ifwitool.py @@ -0,0 +1,166 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for ifwitool + +ifwitool provides a way to package firmware in an Intel Firmware Image (IFWI) +file on some Intel SoCs, e.g. Apolo Lake. + +Documentation is not really available so far as I can tell + +Source code is at tools/ifwitool.c which is a cleaned-up version of +https://github.com/coreboot/coreboot/blob/master/util/cbfstool/ifwitool.c + +Here is the help: + +ifwitool: Utility for IFWI manipulation + +USAGE: + /tmp/b/sandbox/tools/ifwitool [-h] + /tmp/b/sandbox/tools/ifwitool FILE COMMAND [PARAMETERS] + +COMMANDs: + add -f FILE -n NAME [-d -e ENTRY] + create -f FILE + delete -n NAME + extract -f FILE -n NAME [-d -e ENTRY] + print [-d] + replace -f FILE -n NAME [-d -e ENTRY] +OPTIONs: + -f FILE : File to read/write/create/extract + -d : Perform directory operation + -e ENTRY: Name of directory entry to operate on + -v : Verbose level + -h : Help message + -n NAME : Name of sub-partition to operate on + +NAME should be one of: +SMIP(SMIP) +RBEP(CSE_RBE) +FTPR(CSE_BUP) +UCOD(Microcode) +IBBP(Bootblock) +S_BPDT(S-BPDT) +OBBP(OEM boot block) +NFTP(CSE_MAIN) +ISHP(ISH) +DLMP(CSE_IDLM) +IFP_OVERRIDE(IFP_OVERRIDE) +DEBUG_TOKENS(Debug Tokens) +UFS_PHY(UFS Phy) +UFS_GPP(UFS GPP) +PMCP(PMC firmware) +IUNP(IUNIT) +NVM_CONFIG(NVM Config) +UEP(UEP) +UFS_RATE_B(UFS Rate B Config) +""" + +from binman import bintool + +class Bintoolifwitool(bintool.Bintool): + """Handles the 'ifwitool' tool + + This bintool supports running `ifwitool` with some basic parameters as + neeed by binman. It includes creating a file from a FIT as well as adding, + replacing, deleting and extracting subparts. + + The tool is built as part of U-Boot, but a binary version can be fetched if + required. + + ifwitool provides a way to package firmware in an Intel Firmware Image + (IFWI) file on some Intel SoCs, e.g. Apolo Lake. + """ + def __init__(self, name): + super().__init__(name, 'Manipulate Intel IFWI files') + + def create_ifwi(self, intel_fit, ifwi_file): + """Create a new IFWI file, using an existing Intel FIT binary + + Args: + intel_fit (str): Filename of exist Intel FIT file + ifwi_file (str): Output filename to write the new IFWI too + + Returns: + str: Tool output + """ + args = [intel_fit, 'create', '-f', ifwi_file] + return self.run_cmd(*args) + + def delete_subpart(self, ifwi_file, subpart): + """Delete a subpart within the IFWI file + + Args: + ifwi_file (str): IFWI filename to update + subpart (str): Name of subpart to delete, e.g. 'OBBP' + + Returns: + str: Tool output + """ + args = [ifwi_file, 'delete', '-n', subpart] + return self.run_cmd(*args) + + # pylint: disable=R0913 + def add_subpart(self, ifwi_file, subpart, entry_name, infile, + replace=False): + """Add or replace a subpart within the IFWI file + + Args: + ifwi_file (str): IFWI filename to update + subpart (str): Name of subpart to add/replace + entry_nme (str): Name of entry to add/replace + replace (bool): True to replace the existing entry, False to add a + new one + + Returns: + str: Tool output + """ + args = [ + ifwi_file, + 'replace' if replace else 'add', + '-n', subpart, + '-d', '-e', entry_name, + '-f', infile, + ] + return self.run_cmd(*args) + + def extract(self, ifwi_file, subpart, entry_name, outfile): + """Extract a subpart from the IFWI file + + Args: + ifwi_file (str): IFWI filename to extract from + subpart (str): Name of subpart to extract + entry_nme (str): Name of entry to extract + + Returns: + str: Tool output + """ + args = [ + ifwi_file, + 'extract', + '-n', subpart, + '-d', '-e', entry_name, + '-f', outfile, + ] + return self.run_cmd(*args) + + def fetch(self, method): + """Fetch handler for ifwitool + + This installs ifwitool using a binary download. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched, None if a method other than FETCH_BIN + was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + fname, tmpdir = self.fetch_from_drive( + '18JDghOxlt2Hcc5jv51O1t6uNVHQ0XKJS') + return fname, tmpdir

Add a Bintool for this, which is used to build Intel IFWI images. It supports the features needed by the tests as well as downloading a binary from Google Drive. Although this is built in the U-Boot tree, it is not currently included with u-boot-tools, so it may be useful to install a binary on the system.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/ifwitool.py | 166 +++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 tools/binman/btool/ifwitool.py
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to build images for use by U-Boot. It supports the features needed by binman as well as installing via the u-boot-tools packages. Although this is built in the U-Boot tree, it is still useful to install a binary on the system.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/mkimage.py | 80 +++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tools/binman/btool/mkimage.py
diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py new file mode 100644 index 00000000000..c85bfe053cf --- /dev/null +++ b/tools/binman/btool/mkimage.py @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for mkimage""" + +import re + +from binman import bintool + +class Bintoolmkimage(bintool.Bintool): + """Image generation for U-Boot + + This bintool supports running `mkimage` with some basic parameters as + neeed by binman. + + Normally binman uses the mkimage built by U-Boot. But when run outside the + U-Boot build system, binman can use the version installed in your system. + Support is provided for fetching this on Debian-like systems, using apt. + """ + def __init__(self, name): + super().__init__(name, 'Generate image for U-Boot') + + # pylint: disable=R0913 + def run(self, reset_timestamp=False, output_fname=None, external=False, + pad=None, version=False): + """Run mkimage + + Args: + reset_timestamp: True to update the timestamp in the FIT + output_fname: Output filename to write to + external: True to create an 'external' FIT, where the binaries are + located outside the main data structure + pad: Bytes to use for padding the FIT devicetree output. This allows + other things to be easily added later, if required, such as + signatures + version: True to get the mkimage version + """ + args = [] + if external: + args.append('-E') + if pad: + args += ['-p', f'{pad:x}'] + if reset_timestamp: + args.append('-t') + if output_fname: + args += ['-F', output_fname] + if version: + args.append('-V') + return self.run_cmd(*args) + + def fetch(self, method): + """Fetch handler for mkimage + + This installs mkimage using the apt utility. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched and now installed, None if a method + other than FETCH_BIN was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + return self.apt_install('u-boot-tools') + + def version(self): + """Version handler for mkimage + + Returns: + str: Version string for mkimage + """ + out = self.run(version=True).strip() + if not out: + return super().version() + m_version = re.match(r'mkimage version (.*)', out) + return m_version.group(1) if m_version else out

Add a Bintool for this, which is used to build images for use by U-Boot. It supports the features needed by binman as well as installing via the u-boot-tools packages. Although this is built in the U-Boot tree, it is still useful to install a binary on the system.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/mkimage.py | 80 +++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tools/binman/btool/mkimage.py
Applied to u-boot-dm, thanks!

The tests rely on having at least 5 bintool implementions. Now that we have this, enable them. Add tests for the binman 'tool' subcommand.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/ftest.py | 32 ++++++++++++++++++++++++++++++++ tools/binman/main.py | 7 ++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 179326c42a3..92bcb740884 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -17,6 +17,8 @@ import struct import sys import tempfile import unittest +import unittest.mock +import urllib.error
from binman import bintool from binman import cbfs_util @@ -4991,6 +4993,36 @@ fdt fdtmap Extract the devicetree blob from the fdtmap err = stderr.getvalue() self.assertRegex(err, "Image 'main-section'.*faked.*: blob-ext-list")
+ def testListBintools(self): + args = ['tool', '--list'] + with test_util.capture_sys_output() as (stdout, _): + self._DoBinman(*args) + out = stdout.getvalue().splitlines() + self.assertTrue(len(out) >= 2) + + def testFetchBintools(self): + def fail_download(url): + """Take the tools.Download() function by raising an exception""" + raise urllib.error.URLError('my error') + + args = ['tool'] + with self.assertRaises(ValueError) as e: + self._DoBinman(*args) + self.assertIn("Invalid arguments to 'tool' subcommand", + str(e.exception)) + + args = ['tool', '--fetch'] + with self.assertRaises(ValueError) as e: + self._DoBinman(*args) + self.assertIn('Please specify bintools to fetch', str(e.exception)) + + args = ['tool', '--fetch', '_testing'] + with unittest.mock.patch.object(tools, 'Download', + side_effect=fail_download): + with test_util.capture_sys_output() as (stdout, _): + self._DoBinman(*args) + self.assertIn('failed to fetch with all methods', stdout.getvalue()) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/main.py b/tools/binman/main.py index 35944f314a2..dcf20290f2b 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -68,6 +68,7 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath): name to execute (as in 'binman test testSections', for example) toolpath: List of paths to use for tools """ + from binman import bintool_test from binman import cbfs_util_test from binman import elf_test from binman import entry_test @@ -85,9 +86,9 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath): test_util.RunTestSuites( result, debug, verbosity, test_preserve_dirs, processes, test_name, toolpath, - [entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt, - elf_test.TestElf, image_test.TestImage, cbfs_util_test.TestCbfs, - fip_util_test.TestFip]) + [bintool_test.TestBintool, entry_test.TestEntry, ftest.TestFunctional, + fdt_test.TestFdt, elf_test.TestElf, image_test.TestImage, + cbfs_util_test.TestCbfs, fip_util_test.TestFip])
return test_util.ReportResult('binman', test_name, result)

The tests rely on having at least 5 bintool implementions. Now that we have this, enable them. Add tests for the binman 'tool' subcommand.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/ftest.py | 32 ++++++++++++++++++++++++++++++++ tools/binman/main.py | 7 ++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!

Update the CBFS tests to use this bintool, instead of running cbfstool directly. This simplifies the overall code and provides more consistency, as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 24 ---------------- tools/binman/cbfs_util_test.py | 51 +++++++++++++--------------------- 2 files changed, 19 insertions(+), 56 deletions(-)
diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 39973371b93..00664bcf432 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -861,27 +861,3 @@ class CbfsReader(object): val += data[:pos] break return val.decode('utf-8') - - -def cbfstool(fname, *cbfs_args, **kwargs): - """Run cbfstool with provided arguments - - If the tool fails then this function raises an exception and prints out the - output and stderr. - - Args: - fname: Filename of CBFS - *cbfs_args: List of arguments to pass to cbfstool - - Returns: - CommandResult object containing the results - """ - args = ['cbfstool', fname] + list(cbfs_args) - if kwargs.get('base') is not None: - args += ['-b', '%#x' % kwargs['base']] - result = command.RunPipe([args], capture=not VERBOSE, - capture_stderr=not VERBOSE, raise_on_error=False) - if result.return_code: - print(result.stderr, file=sys.stderr) - raise Exception("Failed to run (error %d): '%s'" % - (result.return_code, ' '.join(args))) diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index 2c62c8a0f81..70b42795bfd 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -16,6 +16,7 @@ import struct import tempfile import unittest
+from binman import bintool from binman import cbfs_util from binman.cbfs_util import CbfsWriter from binman import elf @@ -45,11 +46,8 @@ class TestCbfs(unittest.TestCase): # compressing files tools.PrepareOutputDir(None)
- cls.have_cbfstool = True - try: - tools.Run('which', 'cbfstool') - except: - cls.have_cbfstool = False + cls.cbfstool = bintool.Bintool.create('cbfstool') + cls.have_cbfstool = cls.cbfstool.is_present()
cls.have_lz4 = True try: @@ -177,19 +175,19 @@ class TestCbfs(unittest.TestCase): if not self.have_cbfstool or not self.have_lz4: return None cbfs_fname = os.path.join(self._indir, 'test.cbfs') - cbfs_util.cbfstool(cbfs_fname, 'create', '-m', arch, '-s', '%#x' % size) + self.cbfstool.create_new(cbfs_fname, size, arch) if base: base = [(1 << 32) - size + b for b in base] - cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot', '-t', 'raw', - '-c', compress and compress[0] or 'none', - '-f', tools.GetInputFilename( - compress and 'compress' or 'u-boot.bin'), - base=base[0] if base else None) - cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot-dtb', '-t', 'raw', - '-c', compress and compress[1] or 'none', - '-f', tools.GetInputFilename( - compress and 'compress' or 'u-boot.dtb'), - base=base[1] if base else None) + self.cbfstool.add_raw( + cbfs_fname, 'u-boot', + tools.GetInputFilename(compress and 'compress' or 'u-boot.bin'), + compress[0] if compress else None, + base[0] if base else None) + self.cbfstool.add_raw( + cbfs_fname, 'u-boot-dtb', + tools.GetInputFilename(compress and 'compress' or 'u-boot.dtb'), + compress[1] if compress else None, + base[1] if base else None) return cbfs_fname
def _compare_expected_cbfs(self, data, cbfstool_fname): @@ -223,18 +221,9 @@ class TestCbfs(unittest.TestCase): """Test failure to run cbfstool""" if not self.have_cbfstool: self.skipTest('No cbfstool available') - try: - # In verbose mode this test fails since stderr is not captured. Fix - # this by turning off verbosity. - old_verbose = cbfs_util.VERBOSE - cbfs_util.VERBOSE = False - with test_util.capture_sys_output() as (_stdout, stderr): - with self.assertRaises(Exception) as e: - cbfs_util.cbfstool('missing-file', 'bad-command') - finally: - cbfs_util.VERBOSE = old_verbose - self.assertIn('Unknown command', stderr.getvalue()) - self.assertIn('Failed to run', str(e.exception)) + with self.assertRaises(ValueError) as exc: + out = self.cbfstool.fail() + self.assertIn('cbfstool missing-file bad-command', str(exc.exception))
def test_cbfs_raw(self): """Test base handling of a Coreboot Filesystem (CBFS)""" @@ -515,10 +504,8 @@ class TestCbfs(unittest.TestCase): # Compare against what cbfstool creates if self.have_cbfstool: cbfs_fname = os.path.join(self._indir, 'test.cbfs') - cbfs_util.cbfstool(cbfs_fname, 'create', '-m', 'x86', '-s', - '%#x' % size) - cbfs_util.cbfstool(cbfs_fname, 'add-stage', '-n', 'u-boot', - '-f', elf_fname) + self.cbfstool.create_new(cbfs_fname, size) + self.cbfstool.add_stage(cbfs_fname, 'u-boot', elf_fname) self._compare_expected_cbfs(data, cbfs_fname)
def test_cbfs_raw_compress(self):

Update the CBFS tests to use this bintool, instead of running cbfstool directly. This simplifies the overall code and provides more consistency, as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 24 ---------------- tools/binman/cbfs_util_test.py | 51 +++++++++++++--------------------- 2 files changed, 19 insertions(+), 56 deletions(-)
Applied to u-boot-dm, thanks!

Update the FIP tests to use this bintool, instead of running fiptool directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/fip_util.py | 26 -------------------------- tools/binman/fip_util_test.py | 23 ++++++++--------------- 2 files changed, 8 insertions(+), 41 deletions(-)
diff --git a/tools/binman/fip_util.py b/tools/binman/fip_util.py index 5f7dbc04d56..868d0b6b16d 100755 --- a/tools/binman/fip_util.py +++ b/tools/binman/fip_util.py @@ -623,31 +623,5 @@ directory''') return 0
-def fiptool(fname, *fip_args): - """Run fiptool with provided arguments - - If the tool fails then this function raises an exception and prints out the - output and stderr. - - Args: - fname (str): Filename of FIP - *fip_args: List of arguments to pass to fiptool - - Returns: - CommandResult: object containing the results - - Raises: - ValueError: the tool failed to run - """ - args = ['fiptool', fname] + list(fip_args) - result = command.RunPipe([args], capture=not VERBOSE, - capture_stderr=not VERBOSE, raise_on_error=False) - if result.return_code: - print(result.stderr, file=sys.stderr) - raise ValueError("Failed to run (error %d): '%s'" % - (result.return_code, ' '.join(args))) - return result - - if __name__ == "__main__": sys.exit(main(sys.argv[1:], OUR_FILE)) # pragma: no cover diff --git a/tools/binman/fip_util_test.py b/tools/binman/fip_util_test.py index 06827f59322..4d2093b3a4f 100755 --- a/tools/binman/fip_util_test.py +++ b/tools/binman/fip_util_test.py @@ -22,13 +22,11 @@ sys.path.insert(2, os.path.join(OUR_PATH, '..')) # pylint: disable=C0413 from patman import test_util from patman import tools +from binman import bintool from binman import fip_util
-HAVE_FIPTOOL = True -try: - tools.Run('which', 'fiptool') -except ValueError: - HAVE_FIPTOOL = False +FIPTOOL = bintool.Bintool.create('fiptool') +HAVE_FIPTOOL = FIPTOOL.is_present()
# pylint: disable=R0902,R0904 class TestFip(unittest.TestCase): @@ -286,13 +284,13 @@ blah blah''', binary=False) data = fip.get_data() fname = tools.GetOutputFilename('data.fip') tools.WriteFile(fname, data) - result = fip_util.fiptool('info', fname) + result = FIPTOOL.info(fname) self.assertEqual( '''Firmware Updater NS_BL2U: offset=0xB0, size=0x7, cmdline="--fwu" Trusted Boot Firmware BL2: offset=0xC0, size=0xE, cmdline="--tb-fw" 00010203-0405-0607-0809-0A0B0C0D0E0F: offset=0xD0, size=0xE, cmdline="--blob" ''', - result.stdout) + result)
fwu_data = b'my data' tb_fw_data = b'some more data' @@ -315,11 +313,7 @@ Trusted Boot Firmware BL2: offset=0xC0, size=0xE, cmdline="--tb-fw"
fname = tools.GetOutputFilename('data.fip') uuid = 'e3b78d9e-4a64-11ec-b45c-fba2b9b49788' - fip_util.fiptool('create', '--align', '8', '--plat-toc-flags', '0x123', - '--fwu', fwu, - '--tb-fw', tb_fw, - '--blob', f'uuid={uuid},file={other_fw}', - fname) + FIPTOOL.create_new(fname, 8, 0x123, fwu, tb_fw, uuid, other_fw)
return fip_util.FipReader(tools.ReadFile(fname))
@@ -396,9 +390,8 @@ Trusted Boot Firmware BL2: offset=0xC0, size=0xE, cmdline="--tb-fw" """Check some error reporting from fiptool""" with self.assertRaises(Exception) as err: with test_util.capture_sys_output(): - fip_util.fiptool('create', '--fred') - self.assertIn("Failed to run (error 1): 'fiptool create --fred'", - str(err.exception)) + FIPTOOL.create_bad() + self.assertIn("unrecognized option '--fred'", str(err.exception))
if __name__ == '__main__':

Update the FIP tests to use this bintool, instead of running fiptool directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/fip_util.py | 26 -------------------------- tools/binman/fip_util_test.py | 23 ++++++++--------------- 2 files changed, 8 insertions(+), 41 deletions(-)
Applied to u-boot-dm, thanks!

Update the GBB and vblock entry types to use this bintool, instead of running futility directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/gbb.py | 37 +++++++++++++++++++++--------------- tools/binman/etype/vblock.py | 32 ++++++++++++++++++------------- 2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/tools/binman/etype/gbb.py b/tools/binman/etype/gbb.py index 41554eba8f6..ca8af1be424 100644 --- a/tools/binman/etype/gbb.py +++ b/tools/binman/etype/gbb.py @@ -77,20 +77,27 @@ class Entry_gbb(Entry): bmpfv_size = gbb_size - 0x2180 if bmpfv_size < 0: self.Raise('GBB is too small (minimum 0x2180 bytes)') - sizes = [0x100, 0x1000, bmpfv_size, 0x1000] - sizes = ['%#x' % size for size in sizes] keydir = tools.GetInputFilename(self.keydir) - gbb_set_command = [ - 'gbb_utility', '-s', - '--hwid=%s' % self.hardware_id, - '--rootkey=%s/root_key.vbpubk' % keydir, - '--recoverykey=%s/recovery_key.vbpubk' % keydir, - '--flags=%d' % self.gbb_flags, - '--bmpfv=%s' % tools.GetInputFilename(self.bmpblk), - fname] - - tools.Run('futility', 'gbb_utility', '-c', ','.join(sizes), fname) - tools.Run('futility', *gbb_set_command) - - self.SetContents(tools.ReadFile(fname)) + + stdout = self.futility.gbb_create( + fname, [0x100, 0x1000, bmpfv_size, 0x1000]) + if stdout is not None: + stdout = self.futility.gbb_set( + fname, + hwid=self.hardware_id, + rootkey='%s/root_key.vbpubk' % keydir, + recoverykey='%s/recovery_key.vbpubk' % keydir, + flags=self.gbb_flags, + bmpfv=tools.GetInputFilename(self.bmpblk)) + + if stdout is not None: + self.SetContents(tools.ReadFile(fname)) + else: + # Bintool is missing; just use the required amount of zero data + self.record_missing_bintool(self.futility) + self.SetContents(tools.GetBytes(0, gbb_size)) + return True + + def AddBintools(self, tools): + self.futility = self.AddBintool(tools, 'futility') diff --git a/tools/binman/etype/vblock.py b/tools/binman/etype/vblock.py index c0a6a28c943..8bbba273ab6 100644 --- a/tools/binman/etype/vblock.py +++ b/tools/binman/etype/vblock.py @@ -38,6 +38,7 @@ class Entry_vblock(Entry_collection): """ def __init__(self, section, etype, node): super().__init__(section, etype, node) + self.futility = None (self.keydir, self.keyblock, self.signprivate, self.version, self.kernelkey, self.preamble_flags) = self.GetEntryArgsOrProps([ EntryArg('keydir', str), @@ -68,19 +69,21 @@ class Entry_vblock(Entry_collection): input_fname = tools.GetOutputFilename('input.%s' % uniq) tools.WriteFile(input_fname, input_data) prefix = self.keydir + '/' - args = [ - 'vbutil_firmware', - '--vblock', output_fname, - '--keyblock', prefix + self.keyblock, - '--signprivate', prefix + self.signprivate, - '--version', '%d' % self.version, - '--fv', input_fname, - '--kernelkey', prefix + self.kernelkey, - '--flags', '%d' % self.preamble_flags, - ] - #out.Notice("Sign '%s' into %s" % (', '.join(self.value), self.label)) - stdout = tools.Run('futility', *args) - return tools.ReadFile(output_fname) + stdout = self.futility.sign_firmware( + vblock=output_fname, + keyblock=prefix + self.keyblock, + signprivate=prefix + self.signprivate, + version=f'{self.version,}', + firmware=input_fname, + kernelkey=prefix + self.kernelkey, + flags=f'{self.preamble_flags}') + if stdout is not None: + data = tools.ReadFile(output_fname) + else: + # Bintool is missing; just use 4KB of zero data + self.record_missing_bintool(self.futility) + data = tools.GetBytes(0, 4096) + return data
def ObtainContents(self): data = self.GetVblock(False) @@ -93,3 +96,6 @@ class Entry_vblock(Entry_collection): # The blob may have changed due to WriteSymbols() data = self.GetVblock(True) return self.ProcessContentsUpdate(data) + + def AddBintools(self, tools): + self.futility = self.AddBintool(tools, 'futility')

Update the GBB and vblock entry types to use this bintool, instead of running futility directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/gbb.py | 37 +++++++++++++++++++++--------------- tools/binman/etype/vblock.py | 32 ++++++++++++++++++------------- 2 files changed, 41 insertions(+), 28 deletions(-)
Applied to u-boot-dm, thanks!

Update the ifwi entry type to use this bintool, instead of running ifwitool directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/intel_ifwi.py | 25 +++++++++++++++++++------ tools/binman/ftest.py | 4 ++-- tools/patman/tools.py | 31 ------------------------------- 3 files changed, 21 insertions(+), 39 deletions(-)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index ecbd78df5e5..ed14046ba6e 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -59,15 +59,23 @@ class Entry_intel_ifwi(Entry_blob_ext): if self._convert_fit: inname = self._pathname outname = tools.GetOutputFilename('ifwi.bin') - tools.RunIfwiTool(inname, tools.CMD_CREATE, outname) + if self.ifwitool.create_ifwi(inname, outname) is None: + # Bintool is missing; just create a zeroed ifwi.bin + self.record_missing_bintool(self.ifwitool) + self.SetContents(tools.GetBytes(0, 1024)) + self._filename = 'ifwi.bin' self._pathname = outname else: # Provide a different code path here to ensure we have test coverage outname = self._pathname
- # Delete OBBP if it is there, then add the required new items. - tools.RunIfwiTool(outname, tools.CMD_DELETE, subpart='OBBP') + # Delete OBBP if it is there, then add the required new items + if self.ifwitool.delete_subpart(outname, 'OBBP') is None: + # Bintool is missing; just use zero data + self.record_missing_bintool(self.ifwitool) + self.SetContents(tools.GetBytes(0, 1024)) + return True
for entry in self._ifwi_entries.values(): # First get the input data and put it in a file @@ -76,9 +84,11 @@ class Entry_intel_ifwi(Entry_blob_ext): input_fname = tools.GetOutputFilename('input.%s' % uniq) tools.WriteFile(input_fname, data)
- tools.RunIfwiTool(outname, - tools.CMD_REPLACE if entry._ifwi_replace else tools.CMD_ADD, - input_fname, entry._ifwi_subpart, entry._ifwi_entry_name) + # At this point we know that ifwitool is present, so we don't need + # to check for None here + self.ifwitool.add_subpart( + outname, entry._ifwi_subpart, entry._ifwi_entry_name, + input_fname, entry._ifwi_replace)
self.ReadBlobContents() return True @@ -132,3 +142,6 @@ class Entry_intel_ifwi(Entry_blob_ext): if not self.missing: for entry in self._ifwi_entries.values(): entry.WriteSymbols(self) + + def AddBintools(self, tools): + self.ifwitool = self.AddBintool(tools, 'ifwitool') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 92bcb740884..f543d173997 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2314,8 +2314,8 @@ class TestFunctional(unittest.TestCase): # We expect to find the TPL wil in subpart IBBP entry IBBL image_fname = tools.GetOutputFilename('image.bin') tpl_fname = tools.GetOutputFilename('tpl.out') - tools.RunIfwiTool(image_fname, tools.CMD_EXTRACT, fname=tpl_fname, - subpart='IBBP', entry_name='IBBL') + ifwitool = bintool.Bintool.create('ifwitool') + ifwitool.extract(image_fname, 'IBBP', 'IBBL', tpl_fname)
tpl_data = tools.ReadFile(tpl_fname) self.assertEqual(U_BOOT_TPL_DATA, tpl_data[:len(U_BOOT_TPL_DATA)]) diff --git a/tools/patman/tools.py b/tools/patman/tools.py index a27db05ff2a..072b024646d 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -596,37 +596,6 @@ def Decompress(indata, algo, with_header=True): raise ValueError("Unknown algorithm '%s'" % algo) return data
-CMD_CREATE, CMD_DELETE, CMD_ADD, CMD_REPLACE, CMD_EXTRACT = range(5) - -IFWITOOL_CMDS = { - CMD_CREATE: 'create', - CMD_DELETE: 'delete', - CMD_ADD: 'add', - CMD_REPLACE: 'replace', - CMD_EXTRACT: 'extract', - } - -def RunIfwiTool(ifwi_file, cmd, fname=None, subpart=None, entry_name=None): - """Run ifwitool with the given arguments: - - Args: - ifwi_file: IFWI file to operation on - cmd: Command to execute (CMD_...) - fname: Filename of file to add/replace/extract/create (None for - CMD_DELETE) - subpart: Name of sub-partition to operation on (None for CMD_CREATE) - entry_name: Name of directory entry to operate on, or None if none - """ - args = ['ifwitool', ifwi_file] - args.append(IFWITOOL_CMDS[cmd]) - if fname: - args += ['-f', fname] - if subpart: - args += ['-n', subpart] - if entry_name: - args += ['-d', '-e', entry_name] - Run(*args) - def ToHex(val): """Convert an integer value (or None) to a string

Update the ifwi entry type to use this bintool, instead of running ifwitool directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/intel_ifwi.py | 25 +++++++++++++++++++------ tools/binman/ftest.py | 4 ++-- tools/patman/tools.py | 31 ------------------------------- 3 files changed, 21 insertions(+), 39 deletions(-)
Applied to u-boot-dm, thanks!

Update the fit and mkimage entry types to use this bintool, instead of running mkimage directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/fit.py | 20 ++++++++++++++++---- tools/binman/etype/mkimage.py | 13 +++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index b41187df80a..6e5f020c502 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -134,6 +134,7 @@ class Entry_fit(Entry): self._fdts = fdts.split() self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt', str)])[0] + self.mkimage = None
def ReadNode(self): self.ReadEntries() @@ -250,13 +251,21 @@ class Entry_fit(Entry): tools.WriteFile(input_fname, data) tools.WriteFile(output_fname, data)
- args = [] + args = {} ext_offset = self._fit_props.get('fit,external-offset') if ext_offset is not None: - args += ['-E', '-p', '%x' % fdt_util.fdt32_to_cpu(ext_offset.value)] - tools.Run('mkimage', '-t', '-F', output_fname, *args) + args = { + 'external': True, + 'pad': fdt_util.fdt32_to_cpu(ext_offset.value) + } + if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, + **args) is not None: + self.SetContents(tools.ReadFile(output_fname)) + else: + # Bintool is missing; just use empty data as the output + self.record_missing_bintool(self.mkimage) + self.SetContents(tools.GetBytes(0, 1024))
- self.SetContents(tools.ReadFile(output_fname)) return True
def _BuildInput(self, fdt): @@ -295,3 +304,6 @@ class Entry_fit(Entry): def SetAllowMissing(self, allow_missing): for section in self._fit_sections.values(): section.SetAllowMissing(allow_missing) + + def AddBintools(self, tools): + self.mkimage = self.AddBintool(tools, 'mkimage') diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 190398728ec..201ee4b5696 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -51,8 +51,14 @@ class Entry_mkimage(Entry): input_fname = tools.GetOutputFilename('mkimage.%s' % uniq) tools.WriteFile(input_fname, data) output_fname = tools.GetOutputFilename('mkimage-out.%s' % uniq) - tools.Run('mkimage', '-d', input_fname, *self._args, output_fname) - self.SetContents(tools.ReadFile(output_fname)) + if self.mkimage.run_cmd('-d', input_fname, *self._args, + output_fname) is not None: + self.SetContents(tools.ReadFile(output_fname)) + else: + # Bintool is missing; just use the input data as the output + self.record_missing_bintool(self.mkimage) + self.SetContents(data) + return True
def ReadEntries(self): @@ -81,3 +87,6 @@ class Entry_mkimage(Entry): """ for entry in self._mkimage_entries.values(): entry.CheckFakedBlobs(faked_blobs_list) + + def AddBintools(self, tools): + self.mkimage = self.AddBintool(tools, 'mkimage')

Update the fit and mkimage entry types to use this bintool, instead of running mkimage directly. This simplifies the code and provides more consistency as well as supporting missing bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/etype/fit.py | 20 ++++++++++++++++---- tools/binman/etype/mkimage.py | 13 +++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-)
Applied to u-boot-dm, thanks!

The compression functions are not actually used by patman, so we don't need then in the tools module. Also we want to change them to use bintools, which patman will not support.
Move these into a new comp_util module, within binman.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 9 ++-- tools/binman/comp_util.py | 88 +++++++++++++++++++++++++++++++++++ tools/binman/entry.py | 3 +- tools/binman/etype/section.py | 3 +- tools/binman/ftest.py | 9 ++-- tools/patman/tools.py | 79 ------------------------------- 6 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 tools/binman/comp_util.py
diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 00664bcf432..2b4178a6854 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -20,6 +20,7 @@ import io import struct import sys
+from binman import comp_util from binman import elf from patman import command from patman import tools @@ -240,9 +241,9 @@ class CbfsFile(object): """Handle decompressing data if necessary""" indata = self.data if self.compress == COMPRESS_LZ4: - data = tools.Decompress(indata, 'lz4', with_header=False) + data = comp_util.Decompress(indata, 'lz4', with_header=False) elif self.compress == COMPRESS_LZMA: - data = tools.Decompress(indata, 'lzma', with_header=False) + data = comp_util.Decompress(indata, 'lzma', with_header=False) else: data = indata self.memlen = len(data) @@ -361,9 +362,9 @@ class CbfsFile(object): elif self.ftype == TYPE_RAW: orig_data = data if self.compress == COMPRESS_LZ4: - data = tools.Compress(orig_data, 'lz4', with_header=False) + data = comp_util.Compress(orig_data, 'lz4', with_header=False) elif self.compress == COMPRESS_LZMA: - data = tools.Compress(orig_data, 'lzma', with_header=False) + data = comp_util.Compress(orig_data, 'lzma', with_header=False) self.memlen = len(orig_data) self.data_len = len(data) attr = struct.pack(ATTR_COMPRESSION_FORMAT, diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py new file mode 100644 index 00000000000..541e1919dd6 --- /dev/null +++ b/tools/binman/comp_util.py @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Utilities to compress and decompress data""" + +import struct +import tempfile + +from patman import tools + +def Compress(indata, algo, with_header=True): + """Compress some data using a given algorithm + + Note that for lzma this uses an old version of the algorithm, not that + provided by xz. + + This requires 'lz4' and 'lzma_alone' tools. It also requires an output + directory to be previously set up, by calling PrepareOutputDir(). + + Care is taken to use unique temporary files so that this function can be + called from multiple threads. + + Args: + indata: Input data to compress + algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + + Returns: + Compressed data + """ + if algo == 'none': + return indata + fname = tempfile.NamedTemporaryFile(prefix='%s.comp.tmp' % algo, + dir=tools.GetOutputDir()).name + tools.WriteFile(fname, indata) + if algo == 'lz4': + data = tools.Run('lz4', '--no-frame-crc', '-B4', '-5', '-c', fname, + binary=True) + # cbfstool uses a very old version of lzma + elif algo == 'lzma': + outfname = tempfile.NamedTemporaryFile(prefix='%s.comp.otmp' % algo, + dir=tools.GetOutputDir()).name + tools.Run('lzma_alone', 'e', fname, outfname, '-lc1', '-lp0', '-pb0', + '-d8') + data = tools.ReadFile(outfname) + elif algo == 'gzip': + data = tools.Run('gzip', '-c', fname, binary=True) + else: + raise ValueError("Unknown algorithm '%s'" % algo) + if with_header: + hdr = struct.pack('<I', len(data)) + data = hdr + data + return data + +def Decompress(indata, algo, with_header=True): + """Decompress some data using a given algorithm + + Note that for lzma this uses an old version of the algorithm, not that + provided by xz. + + This requires 'lz4' and 'lzma_alone' tools. It also requires an output + directory to be previously set up, by calling PrepareOutputDir(). + + Args: + indata: Input data to decompress + algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + + Returns: + Compressed data + """ + if algo == 'none': + return indata + if with_header: + data_len = struct.unpack('<I', indata[:4])[0] + indata = indata[4:4 + data_len] + fname = tools.GetOutputFilename('%s.decomp.tmp' % algo) + with open(fname, 'wb') as fd: + fd.write(indata) + if algo == 'lz4': + data = tools.Run('lz4', '-dc', fname, binary=True) + elif algo == 'lzma': + outfname = tools.GetOutputFilename('%s.decomp.otmp' % algo) + tools.Run('lzma_alone', 'd', fname, outfname) + data = tools.ReadFile(outfname, binary=True) + elif algo == 'gzip': + data = tools.Run('gzip', '-cd', fname, binary=True) + else: + raise ValueError("Unknown algorithm '%s'" % algo) + return data diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 9cd900670e1..5281dcdab6a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -11,6 +11,7 @@ import pathlib import sys
from binman import bintool +from binman import comp_util from dtoc import fdt_util from patman import tools from patman.tools import ToHex, ToHexSize @@ -1034,7 +1035,7 @@ features to produce new behaviours. self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata) - data = tools.Compress(indata, self.compress) + data = comp_util.Compress(indata, self.compress) return data
@classmethod diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 221a64cd032..66121cb29a2 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -13,6 +13,7 @@ import concurrent.futures import re import sys
+from binman import comp_util from binman.entry import Entry from binman import state from dtoc import fdt_util @@ -775,7 +776,7 @@ class Entry_section(Entry): data = parent_data[offset:offset + child.size] if decomp: indata = data - data = tools.Decompress(indata, child.compress) + data = comp_util.Decompress(indata, child.compress) if child.uncomp_size: tout.Info("%s: Decompressing data size %#x with algo '%s' to data size %#x" % (child.GetPath(), len(indata), child.compress, diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f543d173997..90d7c3cf593 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -23,6 +23,7 @@ import urllib.error from binman import bintool from binman import cbfs_util from binman import cmdline +from binman import comp_util from binman import control from binman import elf from binman import elf_test @@ -1926,7 +1927,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()
def _decompress(self, data): - return tools.Decompress(data, 'lz4') + return comp_util.Decompress(data, 'lz4')
def testCompress(self): """Test compression of blobs""" @@ -2805,7 +2806,7 @@ class TestFunctional(unittest.TestCase): def testExtractCbfsRaw(self): """Test extracting CBFS compressed data without decompressing it""" data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False) - dtb = tools.Decompress(data, 'lzma', with_header=False) + dtb = comp_util.Decompress(data, 'lzma', with_header=False) self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
def testExtractBadEntry(self): @@ -4232,13 +4233,13 @@ class TestFunctional(unittest.TestCase):
# Check compressed data section1 = self._decompress(rest) - expect1 = tools.Compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') + expect1 = comp_util.Compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') self.assertEquals(expect1, rest[:len(expect1)]) self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):]
section2 = self._decompress(rest1) - expect2 = tools.Compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') + expect2 = comp_util.Compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') self.assertEquals(expect2, rest1[:len(expect2)]) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):] diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 072b024646d..5dfecaf917b 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -7,7 +7,6 @@ import glob import os import shlex import shutil -import struct import sys import tempfile import urllib.request @@ -518,84 +517,6 @@ def ToString(bval): """ return bval.decode('utf-8')
-def Compress(indata, algo, with_header=True): - """Compress some data using a given algorithm - - Note that for lzma this uses an old version of the algorithm, not that - provided by xz. - - This requires 'lz4' and 'lzma_alone' tools. It also requires an output - directory to be previously set up, by calling PrepareOutputDir(). - - Care is taken to use unique temporary files so that this function can be - called from multiple threads. - - Args: - indata: Input data to compress - algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') - - Returns: - Compressed data - """ - if algo == 'none': - return indata - fname = tempfile.NamedTemporaryFile(prefix='%s.comp.tmp' % algo, - dir=outdir).name - WriteFile(fname, indata) - if algo == 'lz4': - data = Run('lz4', '--no-frame-crc', '-B4', '-5', '-c', fname, - binary=True) - # cbfstool uses a very old version of lzma - elif algo == 'lzma': - outfname = tempfile.NamedTemporaryFile(prefix='%s.comp.otmp' % algo, - dir=outdir).name - Run('lzma_alone', 'e', fname, outfname, '-lc1', '-lp0', '-pb0', '-d8') - data = ReadFile(outfname) - elif algo == 'gzip': - data = Run('gzip', '-c', fname, binary=True) - else: - raise ValueError("Unknown algorithm '%s'" % algo) - if with_header: - hdr = struct.pack('<I', len(data)) - data = hdr + data - return data - -def Decompress(indata, algo, with_header=True): - """Decompress some data using a given algorithm - - Note that for lzma this uses an old version of the algorithm, not that - provided by xz. - - This requires 'lz4' and 'lzma_alone' tools. It also requires an output - directory to be previously set up, by calling PrepareOutputDir(). - - Args: - indata: Input data to decompress - algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') - - Returns: - Compressed data - """ - if algo == 'none': - return indata - if with_header: - data_len = struct.unpack('<I', indata[:4])[0] - indata = indata[4:4 + data_len] - fname = GetOutputFilename('%s.decomp.tmp' % algo) - with open(fname, 'wb') as fd: - fd.write(indata) - if algo == 'lz4': - data = Run('lz4', '-dc', fname, binary=True) - elif algo == 'lzma': - outfname = GetOutputFilename('%s.decomp.otmp' % algo) - Run('lzma_alone', 'd', fname, outfname) - data = ReadFile(outfname, binary=True) - elif algo == 'gzip': - data = Run('gzip', '-cd', fname, binary=True) - else: - raise ValueError("Unknown algorithm '%s'" % algo) - return data - def ToHex(val): """Convert an integer value (or None) to a string

The compression functions are not actually used by patman, so we don't need then in the tools module. Also we want to change them to use bintools, which patman will not support.
Move these into a new comp_util module, within binman.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 9 ++-- tools/binman/comp_util.py | 88 +++++++++++++++++++++++++++++++++++ tools/binman/entry.py | 3 +- tools/binman/etype/section.py | 3 +- tools/binman/ftest.py | 9 ++-- tools/patman/tools.py | 79 ------------------------------- 6 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 tools/binman/comp_util.py
Applied to u-boot-dm, thanks!

Tweak some naming and comments to resolve these. Use WriteFile() to write the file.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 8 ++++---- tools/binman/comp_util.py | 19 +++++++++---------- tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 2 +- tools/binman/ftest.py | 8 ++++---- 5 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 2b4178a6854..eea7868b16c 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -241,9 +241,9 @@ class CbfsFile(object): """Handle decompressing data if necessary""" indata = self.data if self.compress == COMPRESS_LZ4: - data = comp_util.Decompress(indata, 'lz4', with_header=False) + data = comp_util.decompress(indata, 'lz4', with_header=False) elif self.compress == COMPRESS_LZMA: - data = comp_util.Decompress(indata, 'lzma', with_header=False) + data = comp_util.decompress(indata, 'lzma', with_header=False) else: data = indata self.memlen = len(data) @@ -362,9 +362,9 @@ class CbfsFile(object): elif self.ftype == TYPE_RAW: orig_data = data if self.compress == COMPRESS_LZ4: - data = comp_util.Compress(orig_data, 'lz4', with_header=False) + data = comp_util.compress(orig_data, 'lz4', with_header=False) elif self.compress == COMPRESS_LZMA: - data = comp_util.Compress(orig_data, 'lzma', with_header=False) + data = comp_util.compress(orig_data, 'lzma', with_header=False) self.memlen = len(orig_data) self.data_len = len(data) attr = struct.pack(ATTR_COMPRESSION_FORMAT, diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 541e1919dd6..7e741cb62cc 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -8,7 +8,7 @@ import tempfile
from patman import tools
-def Compress(indata, algo, with_header=True): +def compress(indata, algo, with_header=True): """Compress some data using a given algorithm
Note that for lzma this uses an old version of the algorithm, not that @@ -21,11 +21,11 @@ def Compress(indata, algo, with_header=True): called from multiple threads.
Args: - indata: Input data to compress - algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + indata (bytes): Input data to compress + algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma')
Returns: - Compressed data + bytes: Compressed data """ if algo == 'none': return indata @@ -51,7 +51,7 @@ def Compress(indata, algo, with_header=True): data = hdr + data return data
-def Decompress(indata, algo, with_header=True): +def decompress(indata, algo, with_header=True): """Decompress some data using a given algorithm
Note that for lzma this uses an old version of the algorithm, not that @@ -61,11 +61,11 @@ def Decompress(indata, algo, with_header=True): directory to be previously set up, by calling PrepareOutputDir().
Args: - indata: Input data to decompress - algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + indata (bytes): Input data to decompress + algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma')
Returns: - Compressed data + (bytes) Compressed data """ if algo == 'none': return indata @@ -73,8 +73,7 @@ def Decompress(indata, algo, with_header=True): data_len = struct.unpack('<I', indata[:4])[0] indata = indata[4:4 + data_len] fname = tools.GetOutputFilename('%s.decomp.tmp' % algo) - with open(fname, 'wb') as fd: - fd.write(indata) + tools.WriteFile(fname, indata) if algo == 'lz4': data = tools.Run('lz4', '-dc', fname, binary=True) elif algo == 'lzma': diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 5281dcdab6a..b4323d5147b 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1035,7 +1035,7 @@ features to produce new behaviours. self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata) - data = comp_util.Compress(indata, self.compress) + data = comp_util.compress(indata, self.compress) return data
@classmethod diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 66121cb29a2..f9d3dc37e4a 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -776,7 +776,7 @@ class Entry_section(Entry): data = parent_data[offset:offset + child.size] if decomp: indata = data - data = comp_util.Decompress(indata, child.compress) + data = comp_util.decompress(indata, child.compress) if child.uncomp_size: tout.Info("%s: Decompressing data size %#x with algo '%s' to data size %#x" % (child.GetPath(), len(indata), child.compress, diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 90d7c3cf593..779b8997ab0 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1927,7 +1927,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()
def _decompress(self, data): - return comp_util.Decompress(data, 'lz4') + return comp_util.decompress(data, 'lz4')
def testCompress(self): """Test compression of blobs""" @@ -2806,7 +2806,7 @@ class TestFunctional(unittest.TestCase): def testExtractCbfsRaw(self): """Test extracting CBFS compressed data without decompressing it""" data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False) - dtb = comp_util.Decompress(data, 'lzma', with_header=False) + dtb = comp_util.decompress(data, 'lzma', with_header=False) self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
def testExtractBadEntry(self): @@ -4233,13 +4233,13 @@ class TestFunctional(unittest.TestCase):
# Check compressed data section1 = self._decompress(rest) - expect1 = comp_util.Compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') + expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') self.assertEquals(expect1, rest[:len(expect1)]) self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):]
section2 = self._decompress(rest1) - expect2 = comp_util.Compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') + expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') self.assertEquals(expect2, rest1[:len(expect2)]) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):]

Tweak some naming and comments to resolve these. Use WriteFile() to write the file.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util.py | 8 ++++---- tools/binman/comp_util.py | 19 +++++++++---------- tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 2 +- tools/binman/ftest.py | 8 ++++---- 5 files changed, 19 insertions(+), 20 deletions(-)
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to compress and decompress data. It supports the features needed by binman as well as installing via the lz4 package.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/lz4.py | 140 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tools/binman/btool/lz4.py
diff --git a/tools/binman/btool/lz4.py b/tools/binman/btool/lz4.py new file mode 100644 index 00000000000..d165f52da92 --- /dev/null +++ b/tools/binman/btool/lz4.py @@ -0,0 +1,140 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for lz4 + +lz4 allows compression and decompression of files. + +Documentation is available via:: + + man lz4 + +Here is the help: + +*** LZ4 command line interface 64-bits v1.9.3, by Yann Collet *** +Usage : + lz4 [arg] [input] [output] + +input : a filename + with no FILE, or when FILE is - or stdin, read standard input +Arguments : + -1 : Fast compression (default) + -9 : High compression + -d : decompression (default for .lz4 extension) + -z : force compression + -D FILE: use FILE as dictionary + -f : overwrite output without prompting + -k : preserve source files(s) (default) +--rm : remove source file(s) after successful de/compression + -h/-H : display help/long help and exit + +Advanced arguments : + -V : display Version number and exit + -v : verbose mode + -q : suppress warnings; specify twice to suppress errors too + -c : force write to standard output, even if it is the console + -t : test compressed file integrity + -m : multiple input files (implies automatic output filenames) + -r : operate recursively on directories (sets also -m) + -l : compress using Legacy format (Linux kernel compression) + -B# : cut file into blocks of size # bytes [32+] + or predefined block size [4-7] (default: 7) + -BI : Block Independence (default) + -BD : Block dependency (improves compression ratio) + -BX : enable block checksum (default:disabled) +--no-frame-crc : disable stream checksum (default:enabled) +--content-size : compressed frame includes original size (default:not present) +--list FILE : lists information about .lz4 files (useful for files compressed + with --content-size flag) +--[no-]sparse : sparse mode (default:enabled on file, disabled on stdout) +--favor-decSpeed: compressed files decompress faster, but are less compressed +--fast[=#]: switch to ultra fast compression level (default: 1) +--best : same as -12 +Benchmark arguments : + -b# : benchmark file(s), using # compression level (default : 1) + -e# : test all compression levels from -bX to # (default : 1) + -i# : minimum evaluation time in seconds (default : 3s) +""" + +import re +import tempfile + +from binman import bintool +from patman import tools + +# pylint: disable=C0103 +class Bintoollz4(bintool.Bintool): + """Compression/decompression using the LZ4 algorithm + + This bintool supports running `lz4` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man lz4 + """ + def __init__(self, name): + super().__init__(name, 'lz4 compression') + + def compress(self, indata): + """Compress data with lz4 + + Args: + indata (bytes): Data to compress + + Returns: + bytes: Compressed data + """ + with tempfile.NamedTemporaryFile(prefix='comp.tmp', + dir=tools.GetOutputDir()) as tmp: + tools.WriteFile(tmp.name, indata) + args = ['--no-frame-crc', '-B4', '-5', '-c', tmp.name] + return self.run_cmd(*args, binary=True) + + def decompress(self, indata): + """Decompress data with lz4 + + Args: + indata (bytes): Data to decompress + + Returns: + bytes: Decompressed data + """ + with tempfile.NamedTemporaryFile(prefix='decomp.tmp', + dir=tools.GetOutputDir()) as inf: + tools.WriteFile(inf.name, indata) + args = ['-cd', inf.name] + return self.run_cmd(*args, binary=True) + + def fetch(self, method): + """Fetch handler for lz4 + + This installs the lz4 package using the apt utility. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched and now installed, None if a method + other than FETCH_BIN was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + return self.apt_install('lz4') + + def version(self): + """Version handler + + Returns: + str: Version number of lz4 + """ + out = self.run_cmd('-V').strip() + if not out: + return super().version() + m_version = re.match(r'.* (v[0-9.]*),.*', out) + return m_version.group(1) if m_version else out

Add a Bintool for this, which is used to compress and decompress data. It supports the features needed by binman as well as installing via the lz4 package.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/lz4.py | 140 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tools/binman/btool/lz4.py
Applied to u-boot-dm, thanks!

Update the code to use this bintool, instead of running lz4 directly. This simplifies the code and provides more consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util_test.py | 8 ++------ tools/binman/comp_util.py | 10 +++++++--- tools/binman/ftest.py | 8 +------- 3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index 70b42795bfd..494f6145edb 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -19,6 +19,7 @@ import unittest from binman import bintool from binman import cbfs_util from binman.cbfs_util import CbfsWriter +from binman import comp_util from binman import elf from patman import test_util from patman import tools @@ -49,12 +50,7 @@ class TestCbfs(unittest.TestCase): cls.cbfstool = bintool.Bintool.create('cbfstool') cls.have_cbfstool = cls.cbfstool.is_present()
- cls.have_lz4 = True - try: - tools.Run('lz4', '--no-frame-crc', '-c', - tools.GetInputFilename('u-boot.bin'), binary=True) - except: - cls.have_lz4 = False + cls.have_lz4 = comp_util.HAVE_LZ4
@classmethod def tearDownClass(cls): diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 7e741cb62cc..baa29798bed 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -6,8 +6,13 @@ import struct import tempfile
+from binman import bintool from patman import tools
+LZ4 = bintool.Bintool.create('lz4') +HAVE_LZ4 = LZ4.is_present() + + def compress(indata, algo, with_header=True): """Compress some data using a given algorithm
@@ -33,8 +38,7 @@ def compress(indata, algo, with_header=True): dir=tools.GetOutputDir()).name tools.WriteFile(fname, indata) if algo == 'lz4': - data = tools.Run('lz4', '--no-frame-crc', '-B4', '-5', '-c', fname, - binary=True) + data = LZ4.compress(indata) # cbfstool uses a very old version of lzma elif algo == 'lzma': outfname = tempfile.NamedTemporaryFile(prefix='%s.comp.otmp' % algo, @@ -75,7 +79,7 @@ def decompress(indata, algo, with_header=True): fname = tools.GetOutputFilename('%s.decomp.tmp' % algo) tools.WriteFile(fname, indata) if algo == 'lz4': - data = tools.Run('lz4', '-dc', fname, binary=True) + data = LZ4.decompress(indata) elif algo == 'lzma': outfname = tools.GetOutputFilename('%s.decomp.otmp' % algo) tools.Run('lzma_alone', 'd', fname, outfname) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 779b8997ab0..19461c927c3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -197,13 +197,7 @@ class TestFunctional(unittest.TestCase):
TestFunctional._MakeInputFile('env.txt', ENV_DATA)
- # Travis-CI may have an old lz4 - cls.have_lz4 = True - try: - tools.Run('lz4', '--no-frame-crc', '-c', - os.path.join(cls._indir, 'u-boot.bin'), binary=True) - except: - cls.have_lz4 = False + cls.have_lz4 = comp_util.HAVE_LZ4
@classmethod def tearDownClass(cls):

Update the code to use this bintool, instead of running lz4 directly. This simplifies the code and provides more consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cbfs_util_test.py | 8 ++------ tools/binman/comp_util.py | 10 +++++++--- tools/binman/ftest.py | 8 +------- 3 files changed, 10 insertions(+), 16 deletions(-)
Applied to u-boot-dm, thanks!

Add a Bintool for this, which is used to compress and decompress data. It supports the features needed by binman as well as installing via the lzma-alone package.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/lzma_alone.py | 126 +++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 tools/binman/btool/lzma_alone.py
diff --git a/tools/binman/btool/lzma_alone.py b/tools/binman/btool/lzma_alone.py new file mode 100644 index 00000000000..d7c62dfd2a5 --- /dev/null +++ b/tools/binman/btool/lzma_alone.py @@ -0,0 +1,126 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for lzma_alone + +lzma_alone allows compression and decompression of files, using an older version +of lzma. + +Documentation is available via:: + + man lzma_alone + +Here is the help: + +LZMA 9.22 beta : Igor Pavlov : Public domain : 2011-04-18 + +Usage: LZMA <e|d> inputFile outputFile [<switches>...] + e: encode file + d: decode file + b: Benchmark +<Switches> + -a{N}: set compression mode - [0, 1], default: 1 (max) + -d{N}: set dictionary size - [12, 30], default: 23 (8MB) + -fb{N}: set number of fast bytes - [5, 273], default: 128 + -mc{N}: set number of cycles for match finder + -lc{N}: set number of literal context bits - [0, 8], default: 3 + -lp{N}: set number of literal pos bits - [0, 4], default: 0 + -pb{N}: set number of pos bits - [0, 4], default: 2 + -mf{MF_ID}: set Match Finder: [bt2, bt3, bt4, hc4], default: bt4 + -mt{N}: set number of CPU threads + -eos: write End Of Stream marker + -si: read data from stdin + -so: write data to stdout +""" + +import re +import tempfile + +from binman import bintool +from patman import tools + +# pylint: disable=C0103 +class Bintoollzma_alone(bintool.Bintool): + """Compression/decompression using the LZMA algorithm + + This bintool supports running `lzma_alone` to compress and decompress data, + as used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man lzma_alone + """ + def __init__(self, name): + super().__init__(name, 'lzma_alone compression') + + def compress(self, indata): + """Compress data with lzma_alone + + Args: + indata (bytes): Data to compress + + Returns: + bytes: Compressed data + """ + with tempfile.NamedTemporaryFile(prefix='comp.tmp', + dir=tools.GetOutputDir()) as inf: + tools.WriteFile(inf.name, indata) + with tempfile.NamedTemporaryFile(prefix='compo.otmp', + dir=tools.GetOutputDir()) as outf: + args = ['e', inf.name, outf.name, '-lc1', '-lp0', '-pb0', '-d8'] + self.run_cmd(*args, binary=True) + return tools.ReadFile(outf.name) + + def decompress(self, indata): + """Decompress data with lzma_alone + + Args: + indata (bytes): Data to decompress + + Returns: + bytes: Decompressed data + """ + with tempfile.NamedTemporaryFile(prefix='decomp.tmp', + dir=tools.GetOutputDir()) as inf: + tools.WriteFile(inf.name, indata) + with tempfile.NamedTemporaryFile(prefix='compo.otmp', + dir=tools.GetOutputDir()) as outf: + args = ['d', inf.name, outf.name] + self.run_cmd(*args, binary=True) + return tools.ReadFile(outf.name, binary=True) + + def fetch(self, method): + """Fetch handler for lzma_alone + + This installs the lzma-alone package using the apt utility. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched and now installed, None if a method + other than FETCH_BIN was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + return self.apt_install('lzma-alone') + + def version(self): + """Version handler + + Returns: + str: Version number of lzma_alone + """ + out = self.run_cmd_result('', raise_on_error=False).stderr.strip() + lines = out.splitlines() + if not lines: + return super().version() + out = lines[0] + # e.g. LZMA 9.22 beta : Igor Pavlov : Public domain : 2011-04-18 + m_version = re.match(r'LZMA ([^:]*).*', out) + return m_version.group(1).strip() if m_version else out

Add a Bintool for this, which is used to compress and decompress data. It supports the features needed by binman as well as installing via the lzma-alone package.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/lzma_alone.py | 126 +++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 tools/binman/btool/lzma_alone.py
Applied to u-boot-dm, thanks!

Update the code to use this bintool, instead of running lzma_alone directly. This simplifies the code and provides more consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/comp_util.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index baa29798bed..2f78bab9bbb 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -12,6 +12,9 @@ from patman import tools LZ4 = bintool.Bintool.create('lz4') HAVE_LZ4 = LZ4.is_present()
+LZMA_ALONE = bintool.Bintool.create('lzma_alone') +HAVE_LZMA_ALONE = LZMA_ALONE.is_present() +
def compress(indata, algo, with_header=True): """Compress some data using a given algorithm @@ -41,11 +44,7 @@ def compress(indata, algo, with_header=True): data = LZ4.compress(indata) # cbfstool uses a very old version of lzma elif algo == 'lzma': - outfname = tempfile.NamedTemporaryFile(prefix='%s.comp.otmp' % algo, - dir=tools.GetOutputDir()).name - tools.Run('lzma_alone', 'e', fname, outfname, '-lc1', '-lp0', '-pb0', - '-d8') - data = tools.ReadFile(outfname) + data = LZMA_ALONE.compress(indata) elif algo == 'gzip': data = tools.Run('gzip', '-c', fname, binary=True) else: @@ -81,9 +80,7 @@ def decompress(indata, algo, with_header=True): if algo == 'lz4': data = LZ4.decompress(indata) elif algo == 'lzma': - outfname = tools.GetOutputFilename('%s.decomp.otmp' % algo) - tools.Run('lzma_alone', 'd', fname, outfname) - data = tools.ReadFile(outfname, binary=True) + data = LZMA_ALONE.decompress(indata) elif algo == 'gzip': data = tools.Run('gzip', '-cd', fname, binary=True) else:

Update the code to use this bintool, instead of running lzma_alone directly. This simplifies the code and provides more consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/comp_util.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
Applied to u-boot-dm, thanks!

Bintools can be missing, in which case binman continues operation but reports an invalid image. Plumb in support for this and add tests for entry types which use bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 12 ++++++- tools/binman/entry.py | 20 ++++++++++++ tools/binman/etype/section.py | 11 +++++++ tools/binman/ftest.py | 60 ++++++++++++++++++++++++++++++++++- 5 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 92cc14b6fc7..5ccb2383885 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -105,6 +105,8 @@ controlled by a description in the board device tree.''' help='Use fake device tree contents (for testing only)') build_parser.add_argument('--fake-ext-blobs', action='store_true', help='Create fake ext blobs with dummy content (for testing only)') + build_parser.add_argument('--force-missing-bintools', type=str, + help='Comma-separated list of bintools to consider missing (for testing)') build_parser.add_argument('-i', '--image', type=str, action='append', help='Image filename to build (if not specified, build all)') build_parser.add_argument('-I', '--indir', action='append', diff --git a/tools/binman/control.py b/tools/binman/control.py index 5b10f192360..bbd7773c314 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -583,7 +583,14 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, "Image '%s' has faked external blobs and is non-functional: %s" % (image.name, ' '.join([os.path.basename(e.GetDefaultFilename()) for e in faked_list]))) - return bool(missing_list) or bool(faked_list) + missing_bintool_list = [] + image.check_missing_bintools(missing_bintool_list) + if missing_bintool_list: + tout.Warning( + "Image '%s' has missing bintools and is non-functional: %s" % + (image.name, ' '.join([os.path.basename(bintool.name) + for bintool in missing_bintool_list]))) + return any([missing_list, faked_list, missing_bintool_list])
def Binman(args): @@ -688,6 +695,9 @@ def Binman(args): # Set the first image to timeout, used in testThreadTimeout() images[list(images.keys())[0]].test_section_timeout = True invalid = False + bintool.Bintool.set_missing_list( + args.force_missing_bintools.split(',') if + args.force_missing_bintools else None) for image in images.values(): invalid |= ProcessImage(image, args.update_fdt, args.map, allow_missing=args.allow_missing, diff --git a/tools/binman/entry.py b/tools/binman/entry.py index b4323d5147b..08770ec5f0b 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -77,6 +77,7 @@ class Entry(object): available. This is mainly used for testing. external: True if this entry contains an external binary blob bintools: Bintools used by this entry (only populated for Image) + missing_bintools: List of missing bintools for this entry """ def __init__(self, section, etype, node, name_prefix=''): # Put this here to allow entry-docs and help to work without libfdt @@ -109,6 +110,7 @@ class Entry(object): self.allow_missing = False self.allow_fake = False self.bintools = {} + self.missing_bintools = []
@staticmethod def FindEntryClass(etype, expanded): @@ -1015,6 +1017,24 @@ features to produce new behaviours. """ return self.allow_missing
+ def record_missing_bintool(self, bintool): + """Record a missing bintool that was needed to produce this entry + + Args: + bintool (Bintool): Bintool that was missing + """ + self.missing_bintools.append(bintool) + + def check_missing_bintools(self, missing_list): + """Check if any entries in this section have missing bintools + + If there are missing bintools, these are added to the list + + Args: + missing_list: List of Bintool objects to be added to + """ + missing_list += self.missing_bintools + def GetHelpTags(self): """Get the tags use for missing-blob help
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index f9d3dc37e4a..bb375e9063d 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -832,6 +832,17 @@ class Entry_section(Entry): for entry in self._entries.values(): entry.CheckFakedBlobs(faked_blobs_list)
+ def check_missing_bintools(self, missing_list): + """Check if any entries in this section have missing bintools + + If there are missing bintools, these are added to the list + + Args: + missing_list: List of Bintool objects to be added to + """ + for entry in self._entries.values(): + entry.check_missing_bintools(missing_list) + def _CollectEntries(self, entries, entries_by_name, add_entry): """Collect all the entries in an section
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 19461c927c3..6e1c4985b09 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -310,7 +310,8 @@ class TestFunctional(unittest.TestCase): entry_args=None, images=None, use_real_dtb=False, use_expanded=False, verbosity=None, allow_missing=False, allow_fake_blobs=False, extra_indirs=None, threads=None, - test_section_timeout=False, update_fdt_in_elf=None): + test_section_timeout=False, update_fdt_in_elf=None, + force_missing_bintools=''): """Run binman with a given test file
Args: @@ -339,6 +340,8 @@ class TestFunctional(unittest.TestCase): test_section_timeout: True to force the first time to timeout, as used in testThreadTimeout() update_fdt_in_elf: Value to pass with --update-fdt-in-elf=xxx + force_missing_tools (str): comma-separated list of bintools to + regard as missing
Returns: int return code, 0 on success @@ -373,6 +376,8 @@ class TestFunctional(unittest.TestCase): args.append('-M') if allow_fake_blobs: args.append('--fake-ext-blobs') + if force_missing_bintools: + args += ['--force-missing-bintools', force_missing_bintools] if update_fdt_in_elf: args += ['--update-fdt-in-elf', update_fdt_in_elf] if images: @@ -1713,6 +1718,18 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/gbb': GBB must have a fixed size", str(e.exception))
+ def testGbbMissing(self): + """Test that binman still produces an image if futility is missing""" + entry_args = { + 'keydir': 'devkeys', + } + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('071_gbb.dts', force_missing_bintools='futility', + entry_args=entry_args) + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: futility") + def _HandleVblockCommand(self, pipe_list): """Fake calls to the futility utility
@@ -1798,6 +1815,19 @@ class TestFunctional(unittest.TestCase): expected_hashval = m.digest() self.assertEqual(expected_hashval, hashval)
+ def testVblockMissing(self): + """Test that binman still produces an image if futility is missing""" + entry_args = { + 'keydir': 'devkeys', + } + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('074_vblock.dts', + force_missing_bintools='futility', + entry_args=entry_args) + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: futility") + def testTpl(self): """Test that an image with TPL and its device tree can be created""" # ELF file with a '__bss_size' symbol @@ -2335,6 +2365,16 @@ class TestFunctional(unittest.TestCase): self.assertIn('Could not complete processing of contents', str(e.exception))
+ def testIfwiMissing(self): + """Test that binman still produces an image if ifwitool is missing""" + self._SetupIfwi('fitimage.bin') + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('111_x86_rom_ifwi.dts', + force_missing_bintools='ifwitool') + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: ifwitool") + def testCbfsOffset(self): """Test a CBFS with files at particular offsets
@@ -3614,6 +3654,15 @@ class TestFunctional(unittest.TestCase): # Just check that the data appears in the file somewhere self.assertIn(U_BOOT_SPL_DATA, data)
+ def testMkimageMissing(self): + """Test that binman still produces an image if mkimage is missing""" + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('156_mkimage.dts', + force_missing_bintools='mkimage') + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: mkimage") + def testExtblob(self): """Test an image with an external blob""" data = self._DoReadFile('157_blob_ext.dts') @@ -3734,6 +3783,15 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_DATA + b'aa', data[actual_pos:actual_pos + external_data_size])
+ def testFitMissing(self): + """Test that binman still produces a FIT image if mkimage is missing""" + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('162_fit_external.dts', + force_missing_bintools='mkimage') + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: mkimage") + def testSectionIgnoreHashSignature(self): """Test that sections ignore hash, signature nodes for its data""" data = self._DoReadFile('165_section_ignore_hash_signature.dts')

Bintools can be missing, in which case binman continues operation but reports an invalid image. Plumb in support for this and add tests for entry types which use bintools.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 12 ++++++- tools/binman/entry.py | 20 ++++++++++++ tools/binman/etype/section.py | 11 +++++++ tools/binman/ftest.py | 60 ++++++++++++++++++++++++++++++++++- 5 files changed, 103 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

Drop the unused gzip code, update comments and add a test for an invalid algorithm. The temporary file is not needed now, so drop that also.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/comp_util.py | 16 ++-------------- tools/binman/ftest.py | 9 +++++++++ 2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 2f78bab9bbb..dc76adab352 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -25,28 +25,20 @@ def compress(indata, algo, with_header=True): This requires 'lz4' and 'lzma_alone' tools. It also requires an output directory to be previously set up, by calling PrepareOutputDir().
- Care is taken to use unique temporary files so that this function can be - called from multiple threads. - Args: indata (bytes): Input data to compress - algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
Returns: bytes: Compressed data """ if algo == 'none': return indata - fname = tempfile.NamedTemporaryFile(prefix='%s.comp.tmp' % algo, - dir=tools.GetOutputDir()).name - tools.WriteFile(fname, indata) if algo == 'lz4': data = LZ4.compress(indata) # cbfstool uses a very old version of lzma elif algo == 'lzma': data = LZMA_ALONE.compress(indata) - elif algo == 'gzip': - data = tools.Run('gzip', '-c', fname, binary=True) else: raise ValueError("Unknown algorithm '%s'" % algo) if with_header: @@ -65,7 +57,7 @@ def decompress(indata, algo, with_header=True):
Args: indata (bytes): Input data to decompress - algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') + algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
Returns: (bytes) Compressed data @@ -75,14 +67,10 @@ def decompress(indata, algo, with_header=True): if with_header: data_len = struct.unpack('<I', indata[:4])[0] indata = indata[4:4 + data_len] - fname = tools.GetOutputFilename('%s.decomp.tmp' % algo) - tools.WriteFile(fname, indata) if algo == 'lz4': data = LZ4.decompress(indata) elif algo == 'lzma': data = LZMA_ALONE.decompress(indata) - elif algo == 'gzip': - data = tools.Run('gzip', '-cd', fname, binary=True) else: raise ValueError("Unknown algorithm '%s'" % algo) return data diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6e1c4985b09..a3454ddb104 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5076,6 +5076,15 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self._DoBinman(*args) self.assertIn('failed to fetch with all methods', stdout.getvalue())
+ def testInvalidCompress(self): + with self.assertRaises(ValueError) as e: + comp_util.compress(b'', 'invalid') + self.assertIn("Unknown algorithm 'invalid'", str(e.exception)) + + with self.assertRaises(ValueError) as e: + comp_util.decompress(b'1234', 'invalid') + self.assertIn("Unknown algorithm 'invalid'", str(e.exception)) +
if __name__ == "__main__": unittest.main()

Drop the unused gzip code, update comments and add a test for an invalid algorithm. The temporary file is not needed now, so drop that also.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/comp_util.py | 16 ++-------------- tools/binman/ftest.py | 9 +++++++++ 2 files changed, 11 insertions(+), 14 deletions(-)
Applied to u-boot-dm, thanks!

Each bintool has some documentation which can be useful for the user. Add a new command that collects this and writes it into a .rst file.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool.py | 45 +++++++++++++++++++++++++++++++++++++++++ tools/binman/cmdline.py | 3 +++ tools/binman/control.py | 14 ++++++++++++- tools/binman/ftest.py | 15 ++++++++++++++ tools/binman/main.py | 4 ++++ 5 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 34102dafa2a..e2e5660d167 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -387,6 +387,51 @@ class Bintool: tools.Run(*args) return True
+ @staticmethod + def WriteDocs(modules, test_missing=None): + """Write out documentation about the various bintools to stdout + + Args: + modules: List of modules to include + test_missing: Used for testing. This is a module to report + as missing + """ + print('''.. SPDX-License-Identifier: GPL-2.0+ + +Binman bintool Documentation +============================ + +This file describes the bintools (binary tools) supported by binman. Bintools +are binman's name for external executables that it runs to generate or process +binaries. It is fairly easy to create new bintools. Just add a new file to the +'btool' directory. You can use existing bintools as examples. + + +''') + modules = sorted(modules) + missing = [] + for name in modules: + module = Bintool.find_bintool_class(name) + docs = getattr(module, '__doc__') + if test_missing == name: + docs = None + if docs: + lines = docs.splitlines() + first_line = lines[0] + rest = [line[4:] for line in lines[1:]] + hdr = 'Bintool: %s: %s' % (name, first_line) + print(hdr) + print('-' * len(hdr)) + print('\n'.join(rest)) + print() + print() + else: + missing.append(name) + + if missing: + raise ValueError('Documentation is missing for modules: %s' % + ', '.join(missing)) + # pylint: disable=W0613 def fetch(self, method): """Fetch handler for a bintool diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 5ccb2383885..0626b850f48 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -129,6 +129,9 @@ controlled by a description in the board device tree.''' build_parser.add_argument('--update-fdt-in-elf', type=str, help='Update an ELF file with the output dtb: infile,outfile,begin_sym,end_sym')
+ subparsers.add_parser( + 'bintool-docs', help='Write out bintool documentation (see bintool.rst)') + subparsers.add_parser( 'entry-docs', help='Write out entry documentation (see entries.rst)')
diff --git a/tools/binman/control.py b/tools/binman/control.py index bbd7773c314..2daad05b804 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -140,7 +140,7 @@ def WriteEntryDocs(modules, test_missing=None):
Args: modules: List of Module objects to get docs for - test_missing: Used for testing only, to force an entry's documeentation + test_missing: Used for testing only, to force an entry's documentation to show as missing even if it is present. Should be set to None in normal use. """ @@ -148,6 +148,18 @@ def WriteEntryDocs(modules, test_missing=None): Entry.WriteDocs(modules, test_missing)
+def write_bintool_docs(modules, test_missing=None): + """Write out documentation for all bintools + + Args: + modules: List of Module objects to get docs for + test_missing: Used for testing only, to force an entry's documentation + to show as missing even if it is present. Should be set to None in + normal use. + """ + bintool.Bintool.WriteDocs(modules, test_missing) + + def ListEntries(image_fname, entry_paths): """List the entries in an image
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a3454ddb104..ca200ae9f8f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5085,6 +5085,21 @@ fdt fdtmap Extract the devicetree blob from the fdtmap comp_util.decompress(b'1234', 'invalid') self.assertIn("Unknown algorithm 'invalid'", str(e.exception))
+ def testBintoolDocs(self): + """Test for creation of bintool documentation""" + with test_util.capture_sys_output() as (stdout, stderr): + control.write_bintool_docs(control.bintool.Bintool.get_tool_list()) + self.assertTrue(len(stdout.getvalue()) > 0) + + def testBintoolDocsMissing(self): + """Test handling of missing bintool documentation""" + with self.assertRaises(ValueError) as e: + with test_util.capture_sys_output() as (stdout, stderr): + control.write_bintool_docs( + control.bintool.Bintool.get_tool_list(), 'mkimage') + self.assertIn('Documentation is missing for modules: mkimage', + str(e.exception)) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/main.py b/tools/binman/main.py index dcf20290f2b..03462e7bb8b 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -35,6 +35,7 @@ sys.pycache_prefix = os.path.relpath(our_path, srctree) # in PYTHONPATH) sys.path.insert(2, our1_path)
+from binman import bintool from patman import test_util
# Bring in the libfdt module @@ -129,6 +130,9 @@ def RunBinman(args): args.test_preserve_dirs, args.tests, args.toolpath)
+ elif args.cmd == 'bintool-docs': + control.write_bintool_docs(bintool.Bintool.get_tool_list()) + elif args.cmd == 'entry-docs': control.WriteEntryDocs(control.GetEntryModules())

Each bintool has some documentation which can be useful for the user. Add a new command that collects this and writes it into a .rst file.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/bintool.py | 45 +++++++++++++++++++++++++++++++++++++++++ tools/binman/cmdline.py | 3 +++ tools/binman/control.py | 14 ++++++++++++- tools/binman/ftest.py | 15 ++++++++++++++ tools/binman/main.py | 4 ++++ 5 files changed, 80 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Add this documention to explain how bintools are used and which ones are available.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
doc/develop/package/bintools.rst | 1 + tools/binman/binman.rst | 71 +++++++++++++++++++ tools/binman/bintools.rst | 115 +++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+) create mode 120000 doc/develop/package/bintools.rst create mode 100644 tools/binman/bintools.rst
diff --git a/doc/develop/package/bintools.rst b/doc/develop/package/bintools.rst new file mode 120000 index 00000000000..7ef3d75e935 --- /dev/null +++ b/doc/develop/package/bintools.rst @@ -0,0 +1 @@ +../../../tools/binman/bintools.rst \ No newline at end of file diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 9dbe582ade2..db2234bd8ff 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1027,6 +1027,77 @@ by increasing the -v/--verbosity from the default of 1: You can use BINMAN_VERBOSE=5 (for example) when building to select this.
+Bintools +======== + +`Bintool` is the name binman gives to a binary tool which it uses to create and +manipulate binaries that binman cannot handle itself. Bintools are often +necessary since Binman only supports a subset of the available file formats +natively. + +Many SoC vendors invent ways to load code into their SoC using new file formats, +sometimes changing the format with successive SoC generations. Sometimes the +tool is available as Open Source. Sometimes it is a pre-compiled binary that +must be downloaded from the vendor's website. Sometimes it is available in +source form but difficult or slow to build. + +Even for images that use bintools, binman still assembles the image from its +image description. It may handle parts of the image natively and part with +various bintools. + +Binman relies on these tools so provides various features to manage them: + +- Determining whether the tool is currently installed +- Downloading or building the tool +- Determining the version of the tool that is installed +- Deciding which tools are needed to build an image + +The Bintool class is an interface to the tool, a thin level of abstration, using +Python functions to run the tool for each purpose (e.g. creating a new +structure, adding a file to an existing structure) rather than just lists of +string arguments. + +As with external blobs, bintools (which are like 'external' tools) can be +missing. When building an image requires a bintool and it is not installed, +binman detects this and reports the problem, but continues to build an image. +This is useful in CI systems which want to check that everything is correct but +don't have access to the bintools. + +To make this work, all calls to bintools (e.g. with Bintool.run_cmd()) must cope +with the tool being missing, i.e. when None is returned, by: + +- Calling self.record_missing_bintool() +- Setting up some fake contents so binman can continue + +Of course the image will not work, but binman reports which bintools are needed +and also provide a way to fetch them. + +To see the available bintools, use:: + + binman tool --list + +To fetch tools which are missing, use:: + + binman tool --fetch missing + +You can also use `--fetch all` to fetch all tools or `--fetch <tool>` to fetch +a particular tool. Some tools are built from source code, in which case you will +need to have at least the `build-essential` and `git` packages installed. + +Bintool Documentation +===================== + +To provide details on the various bintools supported by binman, bintools.rst is +generated from the source code using: + + binman bintool-docs >tools/binman/bintools.rst + +.. toctree:: + :maxdepth: 2 + + bintools + + Technical details =================
diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst new file mode 100644 index 00000000000..edb373ab59b --- /dev/null +++ b/tools/binman/bintools.rst @@ -0,0 +1,115 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Binman bintool Documentation +============================ + +This file describes the bintools (binary tools) supported by binman. Bintools +are binman's name for external executables that it runs to generate or process +binaries. It is fairly easy to create new bintools. Just add a new file to the +'btool' directory. You can use existing bintools as examples. + + + +Bintool: cbfstool: Coreboot filesystem (CBFS) tool +-------------------------------------------------- + +This bintool supports creating new CBFS images and adding files to an +existing image, i.e. the features needed by binman. + +It also supports fetching a binary cbfstool, since building it from source +is fairly slow. + +Documentation about CBFS is at https://www.coreboot.org/CBFS + + + +Bintool: fiptool: Image generation for ARM Trusted Firmware +----------------------------------------------------------- + +This bintool supports running `fiptool` with some basic parameters as +neeed by binman. + +It also supports build fiptool from source. + +fiptool provides a way to package firmware in an ARM Trusted Firmware +Firmware Image Package (ATF FIP) format. It is used with Trusted Firmware A, +for example. + +See `TF-A FIP tool documentation`_ for more information. + +.. _`TF-A FIP tool documentation`: + https://trustedfirmware-a.readthedocs.io/en/latest/getting_started/tools-bui... + + + +Bintool: futility: Handles the 'futility' tool +---------------------------------------------- + +futility (flash utility) is a tool for working with Chromium OS flash +images. This Bintool implements just the features used by Binman, related to +GBB creation and firmware signing. + +A binary version of the tool can be fetched. + +See `Chromium OS vboot documentation`_ for more information. + +.. _`Chromium OS vboot documentation`: + https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/mai... + + + +Bintool: ifwitool: Handles the 'ifwitool' tool +---------------------------------------------- + +This bintool supports running `ifwitool` with some basic parameters as +neeed by binman. It includes creating a file from a FIT as well as adding, +replacing, deleting and extracting subparts. + +The tool is built as part of U-Boot, but a binary version can be fetched if +required. + +ifwitool provides a way to package firmware in an Intel Firmware Image +(IFWI) file on some Intel SoCs, e.g. Apolo Lake. + + + +Bintool: lz4: Compression/decompression using the LZ4 algorithm +--------------------------------------------------------------- + +This bintool supports running `lz4` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man lz4 + + + +Bintool: lzma_alone: Compression/decompression using the LZMA algorithm +----------------------------------------------------------------------- + +This bintool supports running `lzma_alone` to compress and decompress data, +as used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man lzma_alone + + + +Bintool: mkimage: Image generation for U-Boot +--------------------------------------------- + +This bintool supports running `mkimage` with some basic parameters as +neeed by binman. + +Normally binman uses the mkimage built by U-Boot. But when run outside the +U-Boot build system, binman can use the version installed in your system. +Support is provided for fetching this on Debian-like systems, using apt. + + +

Add this documention to explain how bintools are used and which ones are available.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
doc/develop/package/bintools.rst | 1 + tools/binman/binman.rst | 71 +++++++++++++++++++ tools/binman/bintools.rst | 115 +++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+) create mode 120000 doc/develop/package/bintools.rst create mode 100644 tools/binman/bintools.rst
Applied to u-boot-dm, thanks!

This shows how binman can be used to replace the long and complicated instructions with an automated build. It is still complicated to read but users don't have to worry about the details.
It needs some tidying up and only supports Odroid-C2 at present.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Substantial rewrite, introducing the concept of bintools
arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 107 ++++++++ arch/arm/mach-meson/Kconfig | 1 + doc/board/amlogic/odroid-c4.rst | 127 +++------ tools/binman/bintools.rst | 24 ++ tools/binman/btool/_aml_common.py | 47 ++++ tools/binman/btool/aml_encrypt_g12a.py | 82 ++++++ tools/binman/btool/aml_encrypt_g12b.py | 83 ++++++ tools/binman/entries.rst | 138 ++++++++++ tools/binman/etype/aml_encrypt.py | 262 +++++++++++++++++++ tools/binman/ftest.py | 9 + tools/binman/missing-blob-help | 6 + tools/binman/test/213_aml_encrypt.dts | 51 ++++ tools/binman/test/214_list_no_dtb.dts | 23 ++ 13 files changed, 868 insertions(+), 92 deletions(-) create mode 100644 tools/binman/btool/_aml_common.py create mode 100644 tools/binman/btool/aml_encrypt_g12a.py create mode 100644 tools/binman/btool/aml_encrypt_g12b.py create mode 100644 tools/binman/etype/aml_encrypt.py create mode 100644 tools/binman/test/213_aml_encrypt.dts create mode 100644 tools/binman/test/214_list_no_dtb.dts
diff --git a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi index 963bf96b256..b221ce6920b 100644 --- a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi +++ b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi @@ -6,6 +6,113 @@
#include "meson-sm1-u-boot.dtsi"
+/{ + binman { + /* run --bootmk on all the included inputs */ + aml-encrypt { + missing-msg = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bootmk"; + aml-level = "v3"; + + /* produce a bl2, containing signed bl2 binaries */ + bl2 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl2sig"; + + /* sign the binary contaiing bl2 and acs */ + aml-input { + type = "section"; + bl2 { + type = "blob-ext"; + size = <0xe000>; + filename = "bl2.bin"; + }; + acs { + type = "blob-ext"; + size = <0x1000>; + filename = "acs.bin"; + }; + }; + }; + + /* produce a bl30, containing signed bl30 binaries */ + bl30 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-level = "v3"; + aml-type = "bl30"; + + /* sign the binary contaiing bl30 and bl301 */ + aml-input { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl30sig"; + aml-level = "v3"; + + /* + * put bl30 and bl301 together, with + * the necessary paddiung + */ + aml-input { + type = "section"; + bl30 { + type = "blob-ext"; + size = <0xa000>; + filename = "bl30.bin"; + }; + bl301 { + type = "blob-ext"; + size = <0x3400>; + filename = "bl301.bin"; + }; + }; + }; + }; + + /* sign the bl31 binary */ + bl31 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-input = "bl31.img"; + aml-level = "v3"; + aml-type = "bl31"; + }; + + /* sign the bl33 binary (which is U-Boot) */ + bl33 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-compress = "lz4"; + aml-level = "v3"; + aml-type = "bl33"; + + aml-input { + type = "u-boot"; + }; + }; + + /* add the various DDR blobs */ + aml-ddrfw { + missing-msg = "aml-ddrfw"; + type = "blob-ext-list"; + filenames = "ddr4_1d.fw", "ddr4_2d.fw", + "ddr3_1d.fw", "piei.fw", + "lpddr4_1d.fw", "lpddr4_2d.fw", + "diag_lpddr4.fw", "aml_ddr.fw", + "lpddr3_1d.fw"; + }; + }; + + fdtmap { + }; + }; +}; + ðmac { snps,reset-gpio = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>; snps,reset-delays-us = <0 10000 1000000>; diff --git a/arch/arm/mach-meson/Kconfig b/arch/arm/mach-meson/Kconfig index 6cba2c40dda..bcb87ea243c 100644 --- a/arch/arm/mach-meson/Kconfig +++ b/arch/arm/mach-meson/Kconfig @@ -48,6 +48,7 @@ config MESON_AXG config MESON_G12A bool "G12A" select MESON64_COMMON + select BINMAN help Select this if your SoC is an S905X/D2
diff --git a/doc/board/amlogic/odroid-c4.rst b/doc/board/amlogic/odroid-c4.rst index f66d60a54d1..5eae1e66e3a 100644 --- a/doc/board/amlogic/odroid-c4.rst +++ b/doc/board/amlogic/odroid-c4.rst @@ -22,17 +22,8 @@ applies for HC4.
Schematics are available on the manufacturer website.
-U-Boot compilation ------------------- - -.. code-block:: bash - - $ export CROSS_COMPILE=aarch64-none-elf- - $ make odroid-c4_defconfig - $ make - -Image creation --------------- +Setting up binary blobs +-----------------------
Amlogic doesn't provide sources for the firmware and for tools needed to create the bootloader image, so it is necessary to obtain them from @@ -40,98 +31,50 @@ the git tree published by the board vendor:
.. code-block:: bash
- $ wget https://releases.linaro.org/archive/13.11/components/toolchain/binaries/gcc-... - $ wget https://releases.linaro.org/archive/13.11/components/toolchain/binaries/gcc-... - $ tar xvfJ gcc-linaro-aarch64-none-elf-4.8-2013.11_linux.tar.xz - $ tar xvfJ gcc-linaro-arm-none-eabi-4.8-2013.11_linux.tar.xz - $ export PATH=$PWD/gcc-linaro-aarch64-none-elf-4.8-2013.11_linux/bin:$PWD/gcc-linaro-arm-none-eabi-4.8-2013.11_linux/bin:$PATH + # This may be needed with this older U-Boot release + apt remove libfdt-dev
- $ DIR=odroid-c4 - $ git clone --depth 1 \ + wget https://releases.linaro.org/archive/13.11/components/toolchain/binaries/gcc-... + wget https://releases.linaro.org/archive/13.11/components/toolchain/binaries/gcc-... + tar xvfJ gcc-linaro-aarch64-none-elf-4.8-2013.11_linux.tar.xz + tar xvfJ gcc-linaro-arm-none-eabi-4.8-2013.11_linux.tar.xz + export PATH=$PWD/gcc-linaro-aarch64-none-elf-4.8-2013.11_linux/bin:$PWD/gcc-linaro-arm-none-eabi-4.8-2013.11_linux/bin:$PATH + + DIR=odroid-c4 + git clone --depth 1 \ https://github.com/hardkernel/u-boot.git -b odroidg12-v2015.01 \ $DIR
- $ cd odroid-c4 - $ make odroidc4_defconfig - $ make - $ export UBOOTDIR=$PWD + cd odroid-c4 + make odroidc4_defconfig + make + export UBOOTDIR=$PWD + +U-Boot compilation +------------------
Go back to mainline U-Boot source tree then :
.. code-block:: bash
- $ mkdir fip - - $ wget https://github.com/BayLibre/u-boot/releases/download/v2017.11-libretech-cc/b... -O fip/blx_fix.sh - $ cp $UBOOTDIR/build/scp_task/bl301.bin fip/ - $ cp $UBOOTDIR/build/board/hardkernel/odroidc4/firmware/acs.bin fip/ - $ cp $UBOOTDIR/fip/g12a/bl2.bin fip/ - $ cp $UBOOTDIR/fip/g12a/bl30.bin fip/ - $ cp $UBOOTDIR/fip/g12a/bl31.img fip/ - $ cp $UBOOTDIR/fip/g12a/ddr3_1d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/ddr4_1d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/ddr4_2d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/diag_lpddr4.fw fip/ - $ cp $UBOOTDIR/fip/g12a/lpddr3_1d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/lpddr4_1d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/lpddr4_2d.fw fip/ - $ cp $UBOOTDIR/fip/g12a/piei.fw fip/ - $ cp $UBOOTDIR/fip/g12a/aml_ddr.fw fip/ - $ cp u-boot.bin fip/bl33.bin - - $ sh fip/blx_fix.sh \ - fip/bl30.bin \ - fip/zero_tmp \ - fip/bl30_zero.bin \ - fip/bl301.bin \ - fip/bl301_zero.bin \ - fip/bl30_new.bin \ - bl30 - - $ sh fip/blx_fix.sh \ - fip/bl2.bin \ - fip/zero_tmp \ - fip/bl2_zero.bin \ - fip/acs.bin \ - fip/bl21_zero.bin \ - fip/bl2_new.bin \ - bl2 - - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bl30sig --input fip/bl30_new.bin \ - --output fip/bl30_new.bin.g12a.enc \ - --level v3 - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bl3sig --input fip/bl30_new.bin.g12a.enc \ - --output fip/bl30_new.bin.enc \ - --level v3 --type bl30 - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bl3sig --input fip/bl31.img \ - --output fip/bl31.img.enc \ - --level v3 --type bl31 - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bl3sig --input fip/bl33.bin --compress lz4 \ - --output fip/bl33.bin.enc \ - --level v3 --type bl33 --compress lz4 - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bl2sig --input fip/bl2_new.bin \ - --output fip/bl2.n.bin.sig - $ $UBOOTDIR/fip/g12a/aml_encrypt_g12a --bootmk \ - --output fip/u-boot.bin \ - --bl2 fip/bl2.n.bin.sig \ - --bl30 fip/bl30_new.bin.enc \ - --bl31 fip/bl31.img.enc \ - --bl33 fip/bl33.bin.enc \ - --ddrfw1 fip/ddr4_1d.fw \ - --ddrfw2 fip/ddr4_2d.fw \ - --ddrfw3 fip/ddr3_1d.fw \ - --ddrfw4 fip/piei.fw \ - --ddrfw5 fip/lpddr4_1d.fw \ - --ddrfw6 fip/lpddr4_2d.fw \ - --ddrfw7 fip/diag_lpddr4.fw \ - --ddrfw8 fip/aml_ddr.fw \ - --ddrfw9 fip/lpddr3_1d.fw \ - --level v3 + $ export CROSS_COMPILE=aarch64-none-elf- + $ make odroid-c4_defconfig + $ BINMAN_TOOLPATHS=$UBOOTDIR/odroid-c4/fip/g12a \ + BINMAN_INDIRS="$UBOOTDIR/fip/g12a \ + $UBOOTDIR/build/board/hardkernel/odroidc4/firmware \ + $UBOOTDIR/build/scp_task" make
and then write the image to SD with:
.. code-block:: bash
- $ DEV=/dev/your_sd_device - $ dd if=fip/u-boot.bin.sd.bin of=$DEV conv=fsync,notrunc bs=512 skip=1 seek=1 - $ dd if=fip/u-boot.bin.sd.bin of=$DEV conv=fsync,notrunc bs=1 count=444 + DEV=/dev/your_sd_device + dd if=image.bin of=$DEV conv=fsync,notrunc + +If you copy the `$UBOOTDIR/odroid-c4/fip/g12a/aml_encrypt_g12a` tool somewhere +in your path, you can omit the `BINMAN_TOOLPATHS` option. The `BINMAN_INDIRS` +variable provides a space-seperated list of directories containing the various +binary blobs needed by the build. + +To see these, look at the Binman image description in +`arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi`. diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst index edb373ab59b..7f77b7fe9dc 100644 --- a/tools/binman/bintools.rst +++ b/tools/binman/bintools.rst @@ -10,6 +10,30 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the
+Bintool: aml_encrypt_g12a: Handles the 'aml_encrypt_g12a' tool +-------------------------------------------------------------- + +This bintool supports running `aml_encrypt_g12a` to support creation of +Amlogic images in binman. + +aml_encrypt_g12a provides a way to package firmware for Amlogic devices. + +It is also possible to fetch a binary version of the tool. + + + +Bintool: aml_encrypt_g12b: Handles the 'aml_encrypt_g12b' tool +-------------------------------------------------------------- + +This bintool supports running `aml_encrypt_g12b` to support creation of +Amlogic images in binman. + +aml_encrypt_g12b provides a way to package firmware for Amlogic devices. + +It is also possible to fetch a binary version of the tool. + + + Bintool: cbfstool: Coreboot filesystem (CBFS) tool --------------------------------------------------
diff --git a/tools/binman/btool/_aml_common.py b/tools/binman/btool/_aml_common.py new file mode 100644 index 00000000000..6d03cfa173e --- /dev/null +++ b/tools/binman/btool/_aml_common.py @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Common bintool implementation for aml_encrypt_g12a/b""" + +import re + +from binman import bintool + +# pylint: disable=C0103 +class aml_common(bintool.Bintool): + """Common class for aml_encrypt_g12a/b tools""" + def version(self): + """Version handler + + Returns: + str: Version number of tool + """ + result = self.run_cmd_result('', raise_on_error=False) + lines = result.stdout.strip().splitlines() + if not lines: + return super().version() + out = lines[0] + # AMLOGIC-G12A-G12B-SIG-module : Ver-0.07 Built on Nov 1 2018 17:12:32 + m_version = re.match(r'.*Ver-([^ ]*).*', out) + return m_version.group(1).strip() if m_version else out + + def fetch(self, method): + """Fetch handler for aml_encrypt_g12a/b + + This installs a binary version of this tool. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched, None if a method other than FETCH_BIN + was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != bintool.FETCH_BIN: + return None + fname, tmpdir = self.fetch_from_drive( + '1o_OiFB5Lf9ib-nPuQDLub8OPuJ0mKIum') + return fname, tmpdir diff --git a/tools/binman/btool/aml_encrypt_g12a.py b/tools/binman/btool/aml_encrypt_g12a.py new file mode 100644 index 00000000000..112949a090a --- /dev/null +++ b/tools/binman/btool/aml_encrypt_g12a.py @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for aml_encrypt_g12a + +aml_encrypt_g12a provides a way to package firmware for Amlogic devices. + +Documentation is not really available so far as I can tell + +Source code also does not seem to be available, but there are some alternate +open-source tools in C: + +https://github.com/afaerber/meson-tools + GXBB, GXL & GXM only +https://github.com/repk/gxlimg + GXBB, GXL, GXM & AXG only +https://github.com/angerman/meson64-tools + developed for G12B, should work on G12A & SM1 + +Here is the help: + + AMLOGIC-G12A-G12B-SIG-module : Ver-0.07 Built on Nov 1 2018 17:12:32 + + aml_encrypt_g12a --rsagen --keylen len --exp exp --ned rkey --nex puk + --nxd prk --txt tkey --aes akey + keylen : RSA key length:1024,2048,4096 + exp : Exponent of RSA:3,0x10001,0x1374B + ned : RSA key file name + nex : RSA PUK file name + nxd : RSA PRVK file name + txt : RSA key file with text format + aes : AES key file without IV,GX series used only + output : signatured keymax + + aml_encrypt_g12a --keysig --rkey1 rsakey [--rkey2 ..][--rkey3 ..] + [--rkey4 ..] --ukey1 rsakey --skey rsakey --output sig-rsa-key + rkey1[2,3,4] : root RSA public key for create keymax + ukey1 : user RSA public key for create keymax + skey : root RSA private key for signature keymax + output : signatured keymax + + aml_encrypt_g12a --keybnd --ukey file --rootkeymax file [--aeskey file] + --output file + ukey : user RSA key to be used for secure boot + rootkeymax : signatured keymax for secure boot + aeskey : AES key to be used for secure boot + output : keys package for secure boot + + aml_encrypt_g12a --bootsig --input file --amluserkey file --output file + input : uboot image to be processed for normal or secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted uboot image + + aml_encrypt_g12a --imgsig --input file --amluserkey file --output file + input : image to be processed for secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted kernel image + + aml_encrypt_g12a --binsig --input file --amluserkey file --output file + input : binary file to be processed for secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted binary file + +Note that the aml_encrypt_g12a and aml_encrypt_g12b executables appear to be +identical. Perhaps the executable name affects the behaviour? +""" + +from binman.btool import _aml_common + +# pylint: disable=C0103 +class Bintoolaml_encrypt_g12a(_aml_common.aml_common): + """Handles the 'aml_encrypt_g12a' tool + + This bintool supports running `aml_encrypt_g12a` to support creation of + Amlogic images in binman. + + aml_encrypt_g12a provides a way to package firmware for Amlogic devices. + + It is also possible to fetch a binary version of the tool. + """ + def __init__(self, name): + super().__init__(name, 'Amlogic encrypt g12a') diff --git a/tools/binman/btool/aml_encrypt_g12b.py b/tools/binman/btool/aml_encrypt_g12b.py new file mode 100644 index 00000000000..ea5da846373 --- /dev/null +++ b/tools/binman/btool/aml_encrypt_g12b.py @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +"""Bintool implementation for aml_encrypt_g12b + +aml_encrypt_g12b provides a way to package firmware for Amlogic devices. + +Documentation is not really available so far as I can tell + +Source code also does not seem to be available, but there are some alternate +open-source tools in C: + +https://github.com/afaerber/meson-tools + GXBB, GXL & GXM only +https://github.com/repk/gxlimg + GXBB, GXL, GXM & AXG only +https://github.com/angerman/meson64-tools + developed for G12B, should work on G12A & SM1 + +Here is the help: + + AMLOGIC-G12A-G12B-SIG-module : Ver-0.07 Built on Nov 1 2018 17:12:32 + + aml_encrypt_g12b --rsagen --keylen len --exp exp --ned rkey --nex puk + --nxd prk --txt tkey --aes akey + keylen : RSA key length:1024,2048,4096 + exp : Exponent of RSA:3,0x10001,0x1374B + ned : RSA key file name + nex : RSA PUK file name + nxd : RSA PRVK file name + txt : RSA key file with text format + aes : AES key file without IV,GX series used only + output : signatured keymax + + aml_encrypt_g12b --keysig --rkey1 rsakey [--rkey2 ..][--rkey3 ..] + [--rkey4 ..] --ukey1 rsakey --skey rsakey --output sig-rsa-key + rkey1[2,3,4] : root RSA public key for create keymax + ukey1 : user RSA public key for create keymax + skey : root RSA private key for signature keymax + output : signatured keymax + + aml_encrypt_g12b --keybnd --ukey file --rootkeymax file [--aeskey file] + --output file + ukey : user RSA key to be used for secure boot + rootkeymax : signatured keymax for secure boot + aeskey : AES key to be used for secure boot + output : keys package for secure boot + + aml_encrypt_g12b --bootsig --input file --amluserkey file --output file + input : uboot image to be processed for normal or secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted uboot image + + aml_encrypt_g12b --imgsig --input file --amluserkey file --output file + input : image to be processed for secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted kernel image + + aml_encrypt_g12b --binsig --input file --amluserkey file --output file + input : binary file to be processed for secure boot + amluserkey : amlogic key package for secure boot + output : signatured and encrypted binary file + +Note that the aml_encrypt_g12a and aml_encrypt_g12b executables appear to be +identical. Perhaps the executable name affects the behaviour? + +""" + +from binman.btool import _aml_common + +# pylint: disable=C0103 +class Bintoolaml_encrypt_g12b(_aml_common.aml_common): + """Handles the 'aml_encrypt_g12b' tool + + This bintool supports running `aml_encrypt_g12b` to support creation of + Amlogic images in binman. + + aml_encrypt_g12b provides a way to package firmware for Amlogic devices. + + It is also possible to fetch a binary version of the tool. + """ + def __init__(self, name): + super().__init__(name, 'Amlogic encrypt g12b') diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index c47f7df0980..0cbc3315337 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -11,6 +11,144 @@ features to produce new behaviours.
+Entry: aml-encrypt: Amlogic encryption support +---------------------------------------------- + +Some Amlogic chips use encryption with various firmware binaries. This +entry supports running one of the tools to generate the required data +formats. + +Available parameters are: + +aml-algo + Algorithm to use, either "g12a" or "g12b" + +aml-op + Operation to perform, one of "bootmk", "bl2sig", "bl3sig", "bl30sig" + +aml-level + Level, typically "v3" + +aml-compress + Optional compression type, e.g. "lz4" + +aml-type + Binary type to process, one of "bl30", "bl31", "bl32" + +The data to pass to the tool for processing is defined by an 'aml-input' +property (for a single file) or subnode (for more flexibility). The subnode +can be any entry type, including a section. + +For cases where multiple inputs are provided to one invocation of the tool, +these are named, again using subnodes, corresponding to the tool flags. +Available inputs are acs, bl2, bl30, sbl301, bl31, bl33 and aml-ddrfw. The +last one is a bit different, in that it takes the form of a blob-ext-list, +i.e. it has a 'filenames' property with a list of the DDR firmware binaries. + +Here is an example:: + + /* run --bootmk on all the included inputs */ + aml-encrypt { + missing-msg = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bootmk"; + aml-level = "v3"; + + /* produce a bl2, containing signed bl2 binaries */ + bl2 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl2sig"; + + /* sign the binary contaiing bl2 and acs */ + aml-input { + type = "section"; + bl2 { + type = "blob-ext"; + size = <0xe000>; + filename = "bl2.bin"; + }; + acs { + type = "blob-ext"; + size = <0x1000>; + filename = "acs.bin"; + }; + }; + }; + + /* produce a bl30, containing signed bl30 binaries */ + bl30 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-level = "v3"; + aml-type = "bl30"; + + /* sign the binary contaiing bl30 and bl301 */ + aml-input { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl30sig"; + aml-level = "v3"; + + /* + * put bl30 and bl301 together, with + * the necessary paddiung + */ + aml-input { + type = "section"; + bl30 { + type = "blob-ext"; + size = <0xa000>; + filename = "bl30.bin"; + }; + bl301 { + type = "blob-ext"; + size = <0x3400>; + filename = "bl301.bin"; + }; + }; + }; + }; + + /* sign the bl31 binary */ + bl31 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-input = "bl31.img"; + aml-level = "v3"; + aml-type = "bl31"; + }; + + /* sign the bl33 binary (which is U-Boot) */ + bl33 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-compress = "lz4"; + aml-level = "v3"; + aml-type = "bl33"; + + aml-input { + type = "u-boot"; + }; + }; + + /* add the various DDR blobs */ + aml-ddrfw { + missing-msg = "aml-ddrfw"; + type = "blob-ext-list"; + filenames = "ddr4_1d.fw", "ddr4_2d.fw", + "ddr3_1d.fw", "piei.fw", + "lpddr4_1d.fw", "lpddr4_2d.fw", + "diag_lpddr4.fw", "aml_ddr.fw", + "lpddr3_1d.fw"; + }; + }; + + + Entry: atf-bl31: ARM Trusted Firmware (ATF) BL31 blob -----------------------------------------------------
diff --git a/tools/binman/etype/aml_encrypt.py b/tools/binman/etype/aml_encrypt.py new file mode 100644 index 00000000000..e9651abc416 --- /dev/null +++ b/tools/binman/etype/aml_encrypt.py @@ -0,0 +1,262 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for producing an image using aml-encrypt-g12a +# + +from collections import OrderedDict + +from binman.entry import Entry +from binman.etype.section import Entry_section +from binman.etype.blob_ext import Entry_blob_ext +from binman.etype.blob_ext_list import Entry_blob_ext_list +from dtoc import fdt_util +from patman import tools +from patman import tout + +DDR_FW_COUNT = 9 + +class Entry_aml_encrypt(Entry_section): + """Amlogic encryption support + + Some Amlogic chips use encryption with various firmware binaries. This + entry supports running one of the tools to generate the required data + formats. + + Available parameters are: + + aml-algo + Algorithm to use, either "g12a" or "g12b" + + aml-op + Operation to perform, one of "bootmk", "bl2sig", "bl3sig", "bl30sig" + + aml-level + Level, typically "v3" + + aml-compress + Optional compression type, e.g. "lz4" + + aml-type + Binary type to process, one of "bl30", "bl31", "bl32" + + The data to pass to the tool for processing is defined by an 'aml-input' + property (for a single file) or subnode (for more flexibility). The subnode + can be any entry type, including a section. + + For cases where multiple inputs are provided to one invocation of the tool, + these are named, again using subnodes, corresponding to the tool flags. + Available inputs are acs, bl2, bl30, sbl301, bl31, bl33 and aml-ddrfw. The + last one is a bit different, in that it takes the form of a blob-ext-list, + i.e. it has a 'filenames' property with a list of the DDR firmware binaries. + + Here is an example:: + + /* run --bootmk on all the included inputs */ + aml-encrypt { + missing-msg = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bootmk"; + aml-level = "v3"; + + /* produce a bl2, containing signed bl2 binaries */ + bl2 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl2sig"; + + /* sign the binary contaiing bl2 and acs */ + aml-input { + type = "section"; + bl2 { + type = "blob-ext"; + size = <0xe000>; + filename = "bl2.bin"; + }; + acs { + type = "blob-ext"; + size = <0x1000>; + filename = "acs.bin"; + }; + }; + }; + + /* produce a bl30, containing signed bl30 binaries */ + bl30 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-level = "v3"; + aml-type = "bl30"; + + /* sign the binary contaiing bl30 and bl301 */ + aml-input { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl30sig"; + aml-level = "v3"; + + /* + * put bl30 and bl301 together, with + * the necessary paddiung + */ + aml-input { + type = "section"; + bl30 { + type = "blob-ext"; + size = <0xa000>; + filename = "bl30.bin"; + }; + bl301 { + type = "blob-ext"; + size = <0x3400>; + filename = "bl301.bin"; + }; + }; + }; + }; + + /* sign the bl31 binary */ + bl31 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-input = "bl31.img"; + aml-level = "v3"; + aml-type = "bl31"; + }; + + /* sign the bl33 binary (which is U-Boot) */ + bl33 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-compress = "lz4"; + aml-level = "v3"; + aml-type = "bl33"; + + aml-input { + type = "u-boot"; + }; + }; + + /* add the various DDR blobs */ + aml-ddrfw { + missing-msg = "aml-ddrfw"; + type = "blob-ext-list"; + filenames = "ddr4_1d.fw", "ddr4_2d.fw", + "ddr3_1d.fw", "piei.fw", + "lpddr4_1d.fw", "lpddr4_2d.fw", + "diag_lpddr4.fw", "aml_ddr.fw", + "lpddr3_1d.fw"; + }; + }; + """ + def __init__(self, section, etype, node): + super().__init__(section, etype, node) + self._entries = OrderedDict() + self.align_default = None + self._aml_algo = None + self._aml_op = None + self._aml_level = None + self.g12a = None + self.g12b = None + + def ReadNode(self): + super().ReadNode() + self._aml_algo = fdt_util.GetString(self._node, 'aml-algo') + self._aml_op = fdt_util.GetString(self._node, 'aml-op') + self._aml_level = fdt_util.GetString(self._node, 'aml-level') + self._aml_input = fdt_util.GetString(self._node, 'aml-input') + self._aml_compress = fdt_util.GetString(self._node, 'aml-compress') + self._aml_type = fdt_util.GetString(self._node, 'aml-type') + self.ReadEntries() + + def ReadEntries(self): + """Read the subnodes to find out what should go in this image""" + for node in self._node.subnodes: + etype = None + if node.name.startswith('aml-') and 'type' not in node.props: + etype = 'blob-ext' + entry = Entry.Create(self, node, etype) + entry.ReadNode() + self._entries[entry.name] = entry + + def BuildSectionData(self, required): + uniq = self.GetUniqueName() + output_fname = tools.GetOutputFilename('aml-out.%s' % uniq) + args = [f'--{self._aml_op}', '--output', output_fname] + if self._aml_level: + args += ['--level', f'{self._aml_level}'] + if self._aml_compress: + args += ['--compress', f'{self._aml_compress}'] + if self._aml_type: + args += ['--type', f'{self._aml_type}'] + if self._aml_input: + input_pathname = tools.GetInputFilename( + self._aml_input, + self.section.GetAllowMissing()) + if not input_pathname: + missing = True + input_pathname = self.check_fake_fname(self._aml_input) + args += ['--input', f'{input_pathname}'] + + missing = False + for entry in self._entries.values(): + # First get the input data and put it in a file. If not available, + # try later. + entry_data = entry.GetData(required) + if not required and entry_data is None: + return None + flag_name = entry.name.replace('aml-', '') # Drop the aml- prefix + if isinstance(entry, Entry_blob_ext_list): + for i, pathname in enumerate(entry._pathnames): + args += [f'--{flag_name}{i + 1}', pathname] + elif isinstance(entry, Entry_blob_ext): + pathname = entry._pathname + args += [f'--{flag_name}', pathname] + else: + data = self.GetPaddedDataForEntry(entry, entry_data) + fname = tools.GetOutputFilename('aml-in.%s' % + entry.GetUniqueName()) + tools.WriteFile(fname, data) + args += [f'--{flag_name}', fname] + if entry.missing: + missing = True + + if missing: + self.missing = True + return b'' + + tout.Debug(f"Node '{self._node.path}': running: %s" % ' '.join(args)) + to_run = self.g12a if self._aml_algo == 'g12a' else self.g12b + + out = to_run.run_cmd(*args) + + # If an input file (or subnode!) is providing the input, the tools + # writes to the requested output file. Otherwise it uses the output file + # as a template for three files that it writes, ending in '.sd.bin', + # 'usb.bl2' and 'usb.tpl'. We use the first one as the image output + if self._aml_input or self._node.FindNode('aml-input'): + real_outfile = output_fname + else: + real_outfile = f'{output_fname}.sd.bin' + if out is not None: + data = tools.ReadFile(real_outfile) + else: + data = tools.GetBytes(0, 1024) + return data + + def SetImagePos(self, image_pos): + Entry.SetImagePos(self, image_pos) + + def SetCalculatedProperties(self): + Entry.SetCalculatedProperties(self) + + def CheckEntries(self): + Entry.CheckEntries(self) + + def AddBintools(self, tools): + self.g12a = self.AddBintool(tools, 'aml_encrypt_g12a') + self.g12b = self.AddBintool(tools, 'aml_encrypt_g12b') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ca200ae9f8f..c9aac143bf8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5100,6 +5100,15 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIn('Documentation is missing for modules: mkimage', str(e.exception))
+ def testAmlEncrypt(self): + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('213_aml_encrypt.dts', allow_missing=True, + allow_fake_blobs=True) + err = stderr.getvalue() + self.assertRegex( + err, + "Image '.*' has faked external blobs and is non-functional: .*") +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help index 551ca87f6cb..d58f954eae8 100644 --- a/tools/binman/missing-blob-help +++ b/tools/binman/missing-blob-help @@ -33,3 +33,9 @@ k3-rti-wdt-firmware: If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for the R5F core(s) to trigger the system reset. One possible source is https://github.com/siemens/k3-rti-wdt. + +aml-encrypt: +Some AML messages + +aml-ddrfw +Amlogic DDR firmware files are missing diff --git a/tools/binman/test/213_aml_encrypt.dts b/tools/binman/test/213_aml_encrypt.dts new file mode 100644 index 00000000000..f97e23b0e40 --- /dev/null +++ b/tools/binman/test/213_aml_encrypt.dts @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + aml-encrypt { + missing-msg = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bootmk"; + aml-level = "v3"; + aml-compress = "lz4"; + aml-type = "bl30"; + + aml-bl2 { + filename = "bl2.n.bin.sig"; + }; + aml-bl30 { + filename = "bl30_new.bin.enc"; + }; + aml-bl31 { + filename = "bl31.img.enc"; + }; + aml-bl33 { + type = "_testing"; + return-contents-later; + }; + aml-ddrfw { + missing-msg = "aml-ddrfw"; + type = "blob-ext-list"; + filenames = "ddr4_1d.fw", "ddr4_2d.fw", + "ddr3_1d.fw", "piei.fw", + "lpddr4_1d.fw", "lpddr4_2d.fw", + "diag_lpddr4.fw", "aml_ddr.fw", + "lpddr3_1d.fw"; + }; + }; + + bl31 { + type = "aml-encrypt"; + aml-algo = "g12a"; + aml-op = "bl3sig"; + aml-input = "bl31.img"; + aml-level = "v3"; + aml-type = "bl31"; + }; + + }; +}; diff --git a/tools/binman/test/214_list_no_dtb.dts b/tools/binman/test/214_list_no_dtb.dts new file mode 100644 index 00000000000..47ecd058644 --- /dev/null +++ b/tools/binman/test/214_list_no_dtb.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x300>; + atf-bl31 { + filename = "bl31.bin"; + }; + scp { + filename = "scp.bin"; + }; + fdtmap { + }; + image-header { + location = "end"; + }; + }; +};

Hi custodians,
On Sun, 9 Jan 2022 at 20:14, Simon Glass sjg@chromium.org wrote:
At present binman uses binary tools (like cbfstool, futiltiy, lz4) in an ad-hoc manner. Various parts of binman use tools.Run() to run tools as needed. If a tool is missing, an error is produced and binman stops.
However this is not ideal. CI systems want to be able to complete the build, even if tools are missing. Ideally binman would deal with missing binary tools the same way it deals with missing binary blobs: make a note of it and move on.
This series introduces this feature to binman.
`Bintool` is the name binman gives to a binary tool which it uses to create and manipulate binaries that binman cannot handle itself.
Binman provides various features to manage bintools:
- Determining whether the tool is currently installed
- Downloading or building the tool
- Determining the version of the tool that is installed
- Deciding which tools are needed to build an image
As with external blobs, bintools (which are like 'external' tools) can be missing. When building an image requires a bintool and it is not installed, binman detects this and reports the problem, but continues to build an image.
Of course the image will not work, but binman reports which bintools are needed and also provide a way to fetch them.
The final patch shows how this works in practice with a chosen board. The Odroid-C2 is quite a complicated image with many steps. It is an ideal example for how Binman can be used.
The series is available at u-boot-dm/bin-working
Changes in v2:
- Substantial rewrite, introducing the concept of bintools
I'm just wondering if people have had a chance to look at this and might have feedback?
Also, I am wondering about adding the ability to download binary blobs where they are missing.
Regards, Simon
Regards, Simon
participants (3)
-
Rasmus Villemoes
-
Simon Glass
-
Tom Rini