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

[Simon Glass]

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...


>
> 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.

> +
> +                                       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?

Regards,
Simon