* [PATCH] binman: Allow FIT binaries to have missing external blobs
@ 2020-09-01 10:35 Simon Glass
2020-09-01 10:36 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2020-09-01 10:35 UTC (permalink / raw)
To: u-boot
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 {
+ };
+ };
+};
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] binman: Allow FIT binaries to have missing external blobs
2020-09-01 10:35 [PATCH] binman: Allow FIT binaries to have missing external blobs Simon Glass
@ 2020-09-01 10:36 ` Simon Glass
2020-09-01 12:23 ` Alper Nebi Yasak
0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2020-09-01 10:36 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] binman: Allow FIT binaries to have missing external blobs
2020-09-01 10:36 ` Simon Glass
@ 2020-09-01 12:23 ` Alper Nebi Yasak
2020-09-01 15:17 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Alper Nebi Yasak @ 2020-09-01 12:23 UTC (permalink / raw)
To: u-boot
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 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)
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] binman: Allow FIT binaries to have missing external blobs
2020-09-01 12:23 ` Alper Nebi Yasak
@ 2020-09-01 15:17 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2020-09-01 15:17 UTC (permalink / raw)
To: u-boot
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 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)
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-01 15:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 10:35 [PATCH] binman: Allow FIT binaries to have missing external blobs Simon Glass
2020-09-01 10:36 ` Simon Glass
2020-09-01 12:23 ` Alper Nebi Yasak
2020-09-01 15:17 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.