[PATCH v8 7/9] sandbox: capsule: Generate capsule related files through binman

[Sughosh Ganu]

hi Simon,

On Thu, 10 Aug 2023 at 22:57, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > The EFI capsule files can now be generated as part of u-boot
> > build through binman. Add capsule entry nodes for the sandbox
> > architecture for generating the capsules. These capsules are then used
> > for testing the EFI capsule update functionality on the sandbox
> > platforms.
> >
> > Remove the corresponding logic in the test setup which was used for
> > generation of these capsule which is now superfluous.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V7:
> > * Move the capsule generation logic to sandbox_capsule.dtsi and
> >   include that explicitly in test.dts and sandbox.dts.
> > * Drop the u-boot.dtsi file which kept the capsule and signature
> >   nodes.
> > * Remove capsule generation logic from capsule update test setup.
> > * Keep the logic to embed the public key in DTB in the test setup.
> >
> >  arch/sandbox/dts/sandbox.dts                  |   4 +
> >  arch/sandbox/dts/sandbox_capsule.dtsi         | 340 ++++++++++++++++++
> >  arch/sandbox/dts/test.dts                     |   4 +
> >  include/sandbox_efi_capsule.h                 |  21 ++
> >  test/py/tests/test_efi_capsule/conftest.py    | 155 +-------
> >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> >  6 files changed, 385 insertions(+), 175 deletions(-)
> >  create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi
> >  create mode 100644 include/sandbox_efi_capsule.h
> >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> >
> > diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
> > index 12d3eff5fa..8d234ed216 100644
> > --- a/arch/sandbox/dts/sandbox.dts
> > +++ b/arch/sandbox/dts/sandbox.dts
> > @@ -106,3 +106,7 @@
> >  #if IS_ENABLED(CONFIG_SUPPORT_VPL)
> >  #include "sandbox_vpl.dtsi"
> >  #endif
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +#include "sandbox_capsule.dtsi"
> > +#endif
>
> I believe you've missed the vpl fix. You need to update
> sandbox_vpl.dtsi along the lines I indicated, so you don't need the
> extra #ifdef here.

No, I did not miss the fix for the vpl issue that you had suggested.
But since we are no longer working with a u-boot.dtsi which gets
included for every variant, I put this logic which is similar to what
is done for including the sandbox_vpl.dtsi above. My thought behind
this was that the capsule generation is not needed on the variants
which do not enable capsule support, like sandbox_{spl,vpl} etc. Do
you want to build capsules for all variants, including the ones which
do not enable capsule update functionality?

-sughosh

>
> > diff --git a/arch/sandbox/dts/sandbox_capsule.dtsi b/arch/sandbox/dts/sandbox_capsule.dtsi
> > new file mode 100644
> > index 0000000000..c38fbd33b1
> > --- /dev/null
> > +++ b/arch/sandbox/dts/sandbox_capsule.dtsi
> > @@ -0,0 +1,340 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Devicetree file with binman nodes for generating capsules.
> > + */
> > +
> > +#include <sandbox_efi_capsule.h>
> > +
> > +/ {
> > +       binman: binman {
> > +               multiple-images;
>
> This is what triggers the need for an update. It is just too confusing
> to have one sandbox build use multiple images and not others.
>
> So this line needs to move to a previous patch:
>
> - add it to sandbox.dts, test.dts
> - tidy up sandbox_vpl.dtsi
>
> > +       };
> > +};
> > +
> > +&binman {
> > +       itb {
> > +               filename = UBOOT_FIT_IMAGE;
> > +
> > +               fit {
> > +                       description = "Automatic U-Boot environment update";
> > +                       #address-cells = <2>;
> > +
> > +                       images {
> > +                               u-boot-bin {
> > +                                       description = "U-Boot binary on SPI Flash";
> > +                                       compression = "none";
> > +                                       type = "firmware";
> > +                                       arch = "sandbox";
> > +                                       load = <0>;
> > +                                       text {
> > +                                               text = "u-boot:New";
> > +                                       };
> > +
> > +                                       hash-1 {
> > +                                               algo = "sha1";
> > +                                       };
> > +                               };
> > +                               u-boot-env {
> > +                                       description = "U-Boot environment on SPI Flash";
> > +                                       compression = "none";
> > +                                       type = "firmware";
> > +                                       arch = "sandbox";
> > +                                       load = <0>;
> > +                                       text {
> > +                                               text = "u-boot-env:New";
> > +                                       };
> > +
> > +                                       hash-1 {
> > +                                               algo = "sha1";
> > +                                       };
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       capsule1 {
> > +               filename = "Test01";
> > +               capsule {
> > +                       type = "efi-capsule";
>
> Can you use efi-capsule for the name, so you can omit this line?
>
> [..]
>
> Regards,
> Simon