[PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman

[Sughosh Ganu]

hi Simon,

On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for
> > the sandbox architecture for generating the capsules. These capsules
> > are then used for testing the EFI capsule update functionality on the
> > sandbox platforms. Also add binman nodes for generating the input
> > files used for these capsules.
> >
> > Remove the corresponding logic which was used for generation of these
> > input files which is now superfluous.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V6:
> > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> >   input filenames.
> > * Generate the capsule input files through binman text entries.
> >
> >  arch/sandbox/dts/u-boot.dtsi                  | 363 ++++++++++++++++++
> >  include/sandbox_efi_capsule.h                 |  11 +
> >  test/py/tests/test_efi_capsule/conftest.py    | 168 +-------
> >  test/py/tests/test_efi_capsule/signature.dts  |  10 -
> >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> >  5 files changed, 385 insertions(+), 203 deletions(-)
> >  delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
>
> I think you are still getting confused with using filenames when you
> don't need to. See below...

No, I don't think so. This is being done on purpose, since I want
these files to be created. These are then copied to the efi capsule
update tests' data directory, and are then used in testing the
feature. Maybe if you want, I can put a comment to that effect.

>
>
> >
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > index 60bd004937..f1b16b41c2 100644
> > --- a/arch/sandbox/dts/u-boot.dtsi
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -7,11 +7,374 @@
> >   */
> >
> >  #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +
> > +#include <sandbox_efi_capsule.h>
> > +
> >  / {
> >  #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> >         signature {
> >                 capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> >         };
> >  #endif
> > +
> > +       binman: binman {
> > +               multiple-images;
> > +       };
> > +};
> > +
> > +&binman {
> > +       input1 {
> > +               filename = UBOOT_BIN_IMAGE_OLD;
> > +               text {
> > +                       text = "u-boot:Old";
> > +               };
> > +       };
> > +
> > +       input2 {
> > +               filename = UBOOT_BIN_IMAGE_NEW;
> > +               text {
> > +                       text = "u-boot:New";
> > +               };
> > +       };
> > +
> > +       input3 {
> > +               filename = UBOOT_ENV_IMAGE_OLD;
> > +               text {
> > +                       text = "u-boot-env:Old";
> > +               };
> > +       };
> > +
> > +       input4 {
> > +               filename = UBOOT_ENV_IMAGE_NEW;
> > +               text {
> > +                       text = "u-boot-env:New";
> > +               };
> > +       };
>
> All of those nodes can be removed
>
> > +
> > +       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>;
> > +                                       blob {
> > +                                               filename = UBOOT_BIN_IMAGE_NEW;
> > +                                       };
>
> instead of this blob node, you should be able to write:
>
> text {
>     text = "u-boot:Old";
> };
>
> There is no need to provide filenames in every node.

I know, please check above.

>
> > +
> > +                                       hash-1 {
> > +                                               algo = "sha1";
> > +                                       };
> > +                               };
> > +                               u-boot-env {
> > +                                       description = "U-Boot environment on SPI Flash";
> > +                                       compression = "none";
> > +                                       type = "firmware";
> > +                                       arch = "sandbox";
> > +                                       load = <0>;
> > +                                       blob {
> > +                                               filename = UBOOT_ENV_IMAGE_NEW;
> > +                                       };
>
> same here and below
>
> > +
> > +                                       hash-1 {
> > +                                               algo = "sha1";
> > +                                       };
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       capsule1 {
> > +               filename = "Test01";
> > +               capsule {
> > +                       type = "efi-capsule";
> > +                       image-index = <0x1>;
> > +                       image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
> > +
> > +                       blob {
> > +                               filename = UBOOT_BIN_IMAGE_NEW;
> > +                       };
>
> again this should be a text node
>
> same below
>
> > +               };
> > +       };
> > +
> > +       capsule2 {
> > +               filename = "Test02";
> > +               capsule {
> > +                       type = "efi-capsule";
> > +                       image-index = <0x2>;
> > +                       image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID;
> > +
> > +                       blob {
> > +                               filename = UBOOT_ENV_IMAGE_NEW;
> > +                       };
> > +               };
> > +       };
> > +
> > +       capsule3 {
> > +               filename = "Test03";
> > +               capsule {
> > +                       type = "efi-capsule";
> > +                       image-index = <0x1>;
> > +                       image-type-id = SANDBOX_INCORRECT_GUID;
> > +
> > +                       blob {
> > +                               filename = UBOOT_BIN_IMAGE_NEW;
> > +                       };
> > +               };
> > +       };
> > +
> > +       capsule4 {
> > +               filename = "Test04";
> > +               capsule {
> > +                       type = "efi-capsule";
> > +                       image-index = <0x1>;
> > +                       image-type-id = SANDBOX_FIT_IMAGE_GUID;
> > +
> > +                       blob {
> > +                               filename =
> [..]
>
> > +       capsule19 {
> > +               filename = "Test115";
>
> [..]
>
> We really need to talk about these tests. So many cases! Can you not
> reduce this?

Unfortunately no. These capsules are all needed for the feature
testing. Which is why I was asking yesterday if you wanted these
generated for normal builds as well, or only for CI runs.

-sughosh