[RFC] binman: add support for creating dummy files for external blobs
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com --- Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
diff --git a/Makefile b/Makefile index f911f70344..1a833a1637 100644 --- a/Makefile +++ b/Makefile @@ -1307,6 +1307,7 @@ 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_SPL_OF_REAL) \ + $(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \ $(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index d6156df408..2b29981cb4 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -52,6 +52,8 @@ controlled by a description in the board device tree.''' help='Configuration file (.dtb) to use') build_parser.add_argument('--fake-dtb', action='store_true', 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('-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 0dbcbc28e9..f95aadd1f3 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -8,6 +8,7 @@ from collections import OrderedDict import glob import os +import pathlib import pkg_resources import re
@@ -401,6 +402,17 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths, return image
+def PrepareFakeExtBlobs(images): + for (_, image) in images.items(): + for entry in image._entries.values(): + if entry.allow_dummy: + if not pathlib.Path(entry._filename).is_file(): + tout.Warning("Missing external blob '%s', fake it" % + entry._filename) + with open(entry._filename, "wb") as out: + out.truncate(1024) + + def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree
@@ -629,6 +641,10 @@ def Binman(args):
images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt, use_expanded) + + if args.fake_ext_blobs: + PrepareFakeExtBlobs(images) + if args.test_section_timeout: # Set the first image to timeout, used in testThreadTimeout() images[list(images.keys())[0]].test_section_timeout = True diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 70222718ea..2f5c38d15a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -100,6 +100,7 @@ class Entry(object): self.missing = False self.external = False self.allow_missing = False + self.allow_dummy = False
@staticmethod def Lookup(node_path, etype, expanded): diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index d6b0ca17c3..85bc9fe6b8 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -26,3 +26,4 @@ class Entry_blob_ext(Entry_blob): def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node) self.external = True + self.allow_dummy = True
On Tue, Nov 16, 2021 at 11:47:10AM +0100, Heiko Thiery wrote:
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com
Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
Since this ends up being a problem for a number of boards, does anyone have comments on this? At first glance, this seems reasonable to me, and a follow-up making CI make use of this would be handy to finish evaluating.
On 23.11.21 16:29, Tom Rini wrote:
On Tue, Nov 16, 2021 at 11:47:10AM +0100, Heiko Thiery wrote:
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com
Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
Since this ends up being a problem for a number of boards, does anyone have comments on this? At first glance, this seems reasonable to me, and a follow-up making CI make use of this would be handy to finish evaluating.
It looks reasonable for me and it is not different as we did before binman reporting warnings that the binary does not work. Fine if thsi can be merged, I will unblock some i.MX boards (Heiko's for example) that I cannot currently merge due to CI failures.
Regards, Stefano
Hi Stefano,
Am Di., 23. Nov. 2021 um 16:44 Uhr schrieb Stefano Babic sbabic@denx.de:
On 23.11.21 16:29, Tom Rini wrote:
On Tue, Nov 16, 2021 at 11:47:10AM +0100, Heiko Thiery wrote:
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com
Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
Since this ends up being a problem for a number of boards, does anyone have comments on this? At first glance, this seems reasonable to me, and a follow-up making CI make use of this would be handy to finish evaluating.
It looks reasonable for me and it is not different as we did before binman reporting warnings that the binary does not work. Fine if thsi can be merged, I will unblock some i.MX boards (Heiko's for example) that I cannot currently merge due to CI failures.
Before you add my patch for the imx8mq board we need to make a change in buildman to make the binman flag option "--fake-ext-blobs" usable. I haven't done that yet, because I wanted to wait if this patch will be accepted.
Hi Heiko,
On Tue, 16 Nov 2021 at 03:47, Heiko Thiery heiko.thiery@gmail.com wrote:
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com
Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
This seems like a reasonable solution to me.
diff --git a/Makefile b/Makefile index f911f70344..1a833a1637 100644 --- a/Makefile +++ b/Makefile @@ -1307,6 +1307,7 @@ 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_SPL_OF_REAL) \
$(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \ $(BINMAN_$(@F))OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index d6156df408..2b29981cb4 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -52,6 +52,8 @@ controlled by a description in the board device tree.''' help='Configuration file (.dtb) to use') build_parser.add_argument('--fake-dtb', action='store_true', 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)')diff --git a/tools/binman/control.py b/tools/binman/control.py index 0dbcbc28e9..f95aadd1f3 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -8,6 +8,7 @@ from collections import OrderedDict import glob import os +import pathlib import pkg_resources import re
@@ -401,6 +402,17 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths, return image
+def PrepareFakeExtBlobs(images):
Please add a function comment
- for (_, image) in images.items():
for entry in image._entries.values():if entry.allow_dummy:if not pathlib.Path(entry._filename).is_file():tout.Warning("Missing external blob '%s', fake it" %entry._filename)with open(entry._filename, "wb") as out:out.truncate(1024)
Won't this create files for all external blobs that are missing? Is that the intent? If so, we can do this in Entry_blob.ObtainContents().
Should we use a size of 0? Or will that make some tools complain?
def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree
@@ -629,6 +641,10 @@ def Binman(args):
images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt, use_expanded)
if args.fake_ext_blobs:PrepareFakeExtBlobs(images)
This should go in ProcessImage(). See the SetAllowMissing() and see if you can follow the same pattern. Basically we tell each section that it should allow missing blobs, then check that in Entry_blot.ObtainContents()
if args.test_section_timeout: # Set the first image to timeout, used in testThreadTimeout() images[list(images.keys())[0]].test_section_timeout = Truediff --git a/tools/binman/entry.py b/tools/binman/entry.py index 70222718ea..2f5c38d15a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -100,6 +100,7 @@ class Entry(object): self.missing = False self.external = False self.allow_missing = False
self.allow_dummy = False
Please add a comment about this above.
@staticmethod def Lookup(node_path, etype, expanded):diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index d6b0ca17c3..85bc9fe6b8 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -26,3 +26,4 @@ class Entry_blob_ext(Entry_blob): def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node) self.external = True
self.allow_dummy = True-- 2.30.2
Also please run 'binman test -T' as you need a test for this.
Regards, Simon
Hi Simon,
Am Di., 23. Nov. 2021 um 17:15 Uhr schrieb Simon Glass sjg@chromium.org:
Hi Heiko,
On Tue, 16 Nov 2021 at 03:47, Heiko Thiery heiko.thiery@gmail.com wrote:
While converting to binman for an imx8mq board, it has been found that building in the u-boot CI fails. This is because an imx8mq requires an external binary (signed_hdmi_imx8m.bin). If this file cannot be found mkimage fails. To be able to build this board in the u-boot CI a binman option (--fake-ext-blobs) is introduced that can be switched on via the u-boot makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are created.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com
Makefile | 1 + tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 16 ++++++++++++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob_ext.py | 1 + 5 files changed, 21 insertions(+)
This seems like a reasonable solution to me.
diff --git a/Makefile b/Makefile index f911f70344..1a833a1637 100644 --- a/Makefile +++ b/Makefile @@ -1307,6 +1307,7 @@ 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_SPL_OF_REAL) \
$(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \ $(BINMAN_$(@F))
This will only work if we add the BINMAN_FAKE_EXT_BLOBS=y to the make call as argument/option. This means we also have to change the buildman for the CI. Or do you know if it is also possible to use an environment variable to achieve this? Then we could simply set the env var for the CI builds.
currently: make CROSS_COMPILE=aarch64-linux-gnu- -j4 BINMAN_FAKE_EXT_BLOBS=y
nice to have export BINMAN_FAKE_EXT_BLOBS=y make CROSS_COMPILE=aarch64-linux-gnu- -j4
What do you think?
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index d6156df408..2b29981cb4 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -52,6 +52,8 @@ controlled by a description in the board device tree.''' help='Configuration file (.dtb) to use') build_parser.add_argument('--fake-dtb', action='store_true', 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)')diff --git a/tools/binman/control.py b/tools/binman/control.py index 0dbcbc28e9..f95aadd1f3 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -8,6 +8,7 @@ from collections import OrderedDict import glob import os +import pathlib import pkg_resources import re
@@ -401,6 +402,17 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths, return image
+def PrepareFakeExtBlobs(images):
Please add a function comment
- for (_, image) in images.items():
for entry in image._entries.values():if entry.allow_dummy:if not pathlib.Path(entry._filename).is_file():tout.Warning("Missing external blob '%s', fake it" %entry._filename)with open(entry._filename, "wb") as out:out.truncate(1024)Won't this create files for all external blobs that are missing? Is that the intent? If so, we can do this in Entry_blob.ObtainContents().
Yes this will create the dummy files for all external blobs.
Should we use a size of 0? Or will that make some tools complain?
mkimage will complain about files that are too small.
def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree
@@ -629,6 +641,10 @@ def Binman(args):
images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt, use_expanded)
if args.fake_ext_blobs:PrepareFakeExtBlobs(images)This should go in ProcessImage(). See the SetAllowMissing() and see if you can follow the same pattern. Basically we tell each section that it should allow missing blobs, then check that in Entry_blot.ObtainContents()
I changed this to the proposed. I had to add this also to the 'Entry_mkimage' when I add the external blob inside this node.
spl { filename = "spl.bin";
mkimage { args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x7e1000";
blob { filename = "u-boot-spl-ddr.bin"; }; hdmi_fw: blob-ext@5 { filename = "signed_hdmi_imx8m.bin"; }; }; };
if args.test_section_timeout: # Set the first image to timeout, used in testThreadTimeout() images[list(images.keys())[0]].test_section_timeout = Truediff --git a/tools/binman/entry.py b/tools/binman/entry.py index 70222718ea..2f5c38d15a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -100,6 +100,7 @@ class Entry(object): self.missing = False self.external = False self.allow_missing = False
self.allow_dummy = FalsePlease add a comment about this above.
Ok.
@staticmethod def Lookup(node_path, etype, expanded):diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index d6b0ca17c3..85bc9fe6b8 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -26,3 +26,4 @@ class Entry_blob_ext(Entry_blob): def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node) self.external = True
self.allow_dummy = True-- 2.30.2
Also please run 'binman test -T' as you need a test for this.
Currently I'm struggling doing that.
participants (4)
-
Heiko Thiery -
Simon Glass -
Stefano Babic -
Tom Rini