[PATCH] binman: Allow FIT binaries to have missing external blobs
At present if a FIT references a missing external blob, binman reports an error, even if the image is supposed to allow this.
Propagate this setting down to the child sections of the FIT, so that the behaviour is consistent.
This is a fix-up patch to:
binman: Build FIT image subentries with the section etype
and will be squashed into that.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/fit.py | 15 ++++++++ tools/binman/ftest.py | 8 +++++ tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tools/binman/test/168_fit_missing_blob.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index acba53aa92c..615b2fd8778 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -169,3 +169,18 @@ class Entry_fit(Entry): fdt.Sync(auto_resize=True) data = fdt.GetContents() return data + + def CheckMissing(self, missing_list): + """Check if any entries in this FIT have missing external blobs + + If there are missing blobs, the entries are added to the list + + Args: + missing_list: List of Entry objects to be added to + """ + for path, section in self._fit_sections.items(): + section.CheckMissing(missing_list) + + def SetAllowMissing(self, allow_missing): + for section in self._fit_sections.values(): + section.SetAllowMissing(allow_missing) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 762535b5a74..e672967dbaa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase): U_BOOT_DTB_DATA) self.assertEqual(expected, data)
+ def testFitExtblobMissingOk(self): + """Test a FIT with a missing external blob that is allowed""" + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('168_fit_missing_blob.dts', + allow_missing=True) + err = stderr.getvalue() + self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext") + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts new file mode 100644 index 00000000000..e007aa41d8a --- /dev/null +++ b/tools/binman/test/168_fit_missing_blob.dts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + kernel { + description = "ATF BL31"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <00000000>; + entry = <00000000>; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + blob-ext { + filename = "missing"; + }; + }; + }; + }; + u-boot-nodtb { + }; + }; +};
Hi Alper,
On Tue, 1 Sep 2020 at 04:35, Simon Glass sjg@chromium.org wrote:
At present if a FIT references a missing external blob, binman reports an error, even if the image is supposed to allow this.
Propagate this setting down to the child sections of the FIT, so that the behaviour is consistent.
This is a fix-up patch to:
binman: Build FIT image subentries with the section etype
and will be squashed into that.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/etype/fit.py | 15 ++++++++ tools/binman/ftest.py | 8 +++++ tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tools/binman/test/168_fit_missing_blob.dts
I found that evb-rk3288 fails to build with the final patch from your series. See u-boot-dm/testing for the tree.
Regards, Simon
On 01/09/2020 13:36, Simon Glass wrote:
Hi Alper,
I found that evb-rk3288 fails to build with the final patch from your series. See u-boot-dm/testing for the tree.
I had seen your patch and assumed that nothing was already using the allow-missing functionality in its FITs (since you're adding it now). Looks like the version in your tree (aa3f219e) succeeds in the pipeline, so I think there's nothing for me to do now. Thanks for fixing it.
At present if a FIT references a missing external blob, binman reports an error, even if the image is supposed to allow this.
Propagate this setting down to the child sections of the FIT, so that the behaviour is consistent.
This is a fix-up patch to:
binman: Build FIT image subentries with the section etype
and will be squashed into that.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/etype/fit.py | 15 ++++++++ tools/binman/ftest.py | 8 +++++ tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tools/binman/test/168_fit_missing_blob.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index acba53aa92c..615b2fd8778 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -169,3 +169,18 @@ class Entry_fit(Entry): fdt.Sync(auto_resize=True) data = fdt.GetContents() return data
- def CheckMissing(self, missing_list):
"""Check if any entries in this FIT have missing external blobsIf there are missing blobs, the entries are added to the listArgs:missing_list: List of Entry objects to be added to"""for path, section in self._fit_sections.items():section.CheckMissing(missing_list)- def SetAllowMissing(self, allow_missing):
for section in self._fit_sections.values():section.SetAllowMissing(allow_missing)
I saw GetAllowMissing() in section.py, doesn't this also need it?
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 762535b5a74..e672967dbaa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase): U_BOOT_DTB_DATA) self.assertEqual(expected, data)
- def testFitExtblobMissingOk(self):
"""Test a FIT with a missing external blob that is allowed"""with test_util.capture_sys_output() as (stdout, stderr):self._DoTestFile('168_fit_missing_blob.dts',allow_missing=True)err = stderr.getvalue()self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts new file mode 100644 index 00000000000..e007aa41d8a --- /dev/null +++ b/tools/binman/test/168_fit_missing_blob.dts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
- #address-cells = <1>;
- #size-cells = <1>;
- binman {
u-boot {};fit {description = "test-desc";#address-cells = <1>;fit,fdt-list = "of-list";images {kernel {description = "ATF BL31";type = "kernel";arch = "ppc";os = "linux";compression = "gzip";load = <00000000>;entry = <00000000>;hash-1 {algo = "crc32";};hash-2 {algo = "sha1";};blob-ext {filename = "missing";};};};};u-boot-nodtb {};- };
+};
Anyway, feel free to add if you need to:
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
(or Acked-by, or neither)
Hi Alper,
On Tue, 1 Sep 2020 at 06:23, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On 01/09/2020 13:36, Simon Glass wrote:
Hi Alper,
I found that evb-rk3288 fails to build with the final patch from your series. See u-boot-dm/testing for the tree.
I had seen your patch and assumed that nothing was already using the allow-missing functionality in its FITs (since you're adding it now). Looks like the version in your tree (aa3f219e) succeeds in the pipeline, so I think there's nothing for me to do now. Thanks for fixing it.
That's right. It's a little bit subtle. In fact we rely on support for missing entries already, but switching over to use sections for the FIT contents means that those sections are not marked to 'allow missing'. So in fact we fail when we cannot find the file.
I did try to do this as a separate patch, but switching to sections is what creates the issue which is why I think I have to squash the patch.
At present if a FIT references a missing external blob, binman reports an error, even if the image is supposed to allow this.
Propagate this setting down to the child sections of the FIT, so that the behaviour is consistent.
This is a fix-up patch to:
binman: Build FIT image subentries with the section etype
and will be squashed into that.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/etype/fit.py | 15 ++++++++ tools/binman/ftest.py | 8 +++++ tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tools/binman/test/168_fit_missing_blob.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index acba53aa92c..615b2fd8778 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -169,3 +169,18 @@ class Entry_fit(Entry): fdt.Sync(auto_resize=True) data = fdt.GetContents() return data
- def CheckMissing(self, missing_list):
"""Check if any entries in this FIT have missing external blobsIf there are missing blobs, the entries are added to the listArgs:missing_list: List of Entry objects to be added to"""for path, section in self._fit_sections.items():section.CheckMissing(missing_list)- def SetAllowMissing(self, allow_missing):
for section in self._fit_sections.values():section.SetAllowMissing(allow_missing)I saw GetAllowMissing() in section.py, doesn't this also need it?
That is in Entry so we can just rely on that. The FIT itself cannot be missing.
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 762535b5a74..e672967dbaa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase): U_BOOT_DTB_DATA) self.assertEqual(expected, data)
- def testFitExtblobMissingOk(self):
"""Test a FIT with a missing external blob that is allowed"""with test_util.capture_sys_output() as (stdout, stderr):self._DoTestFile('168_fit_missing_blob.dts',allow_missing=True)err = stderr.getvalue()self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts new file mode 100644 index 00000000000..e007aa41d8a --- /dev/null +++ b/tools/binman/test/168_fit_missing_blob.dts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {u-boot {};fit {description = "test-desc";#address-cells = <1>;fit,fdt-list = "of-list";images {kernel {description = "ATF BL31";type = "kernel";arch = "ppc";os = "linux";compression = "gzip";load = <00000000>;entry = <00000000>;hash-1 {algo = "crc32";};hash-2 {algo = "sha1";};blob-ext {filename = "missing";};};};};u-boot-nodtb {};};+};
Anyway, feel free to add if you need to:
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
(or Acked-by, or neither)
Thanks!
Regards, Simon
participants (2)
-
Alper Nebi Yasak -
Simon Glass