Re: [PATCH v4 12/12] sandbox: capsule: Generate capsule related files through binman

2023-07-19 Thread Sughosh Ganu
hi Simon,

On Wed, 19 Jul 2023 at 06:41, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Mon, 17 Jul 2023 at 05:18, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Sun, 16 Jul 2023 at 05:12, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu  
> > > wrote:
> > > >
> > > > The EFI capsule files can now be generated as part of u-boot
> > > > build. This is done through binman. Add capsule entry nodes in the
> > > > u-boot.dtsi for the sandbox architecture for generating the
> > > > capsules. Remove the corresponding generation of capsules from the
> > > > capsule update conftest file.
> > > >
> > > > The capsules are generated through the config file for the sandbox
> > > > variant, and through explicit parameters for the sandbox_flattree
> > > > variant.
> > > >
> > > > Also generate the FIT image used for testing the capsule update
> > > > feature on the sandbox_flattree variant through binman. Remove the now
> > > > superfluous its file which was used for generating this FIT image.
> > > >
> > > > Signed-off-by: Sughosh Ganu 
> > > > ---
> > > > Changes since V3:
> > > > * Use blob nodes instead of incbin for including the binaries in FIT
> > > >   image.
> > > > * Enable generation of capsules with versioning support.
> > > >
> > > >  arch/sandbox/dts/u-boot.dtsi  | 265 ++
> > > >  test/py/tests/test_efi_capsule/conftest.py| 127 -
> > > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 ---
> > > >  3 files changed, 265 insertions(+), 163 deletions(-)
> > > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> > > >
> > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > index 60bd004937..7b0250ac81 100644
> > > > --- a/arch/sandbox/dts/u-boot.dtsi
> > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > @@ -13,5 +13,270 @@
> > > > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > };
> > > >  #endif
> > > > +
> > > > +   binman: binman {
> > > > +   multiple-images;
> > > > +   };
> > > > +};
> > > > +
> > > > + {
> > > > +   itb {
> > > > +   filename = "/tmp/capsules/uboot_bin_env.itb";
> > > > +
> > > > +   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 = 
> > > > "/tmp/capsules/u-boot.bin.new";
> > > > +   };
> > > > +
> > > > +   hash-1 {
> > > > +   algo = "sha1";
> > > > +   };
> > > > +   };
> > > > +   u-boot-env {
> > > > +   description = "U-Boot 
> > > > environment on SPI Flash";
> > > > +   compression = "none";
> > > > +   type = "firmware";
> > > > +   arch = "sandbox";
> > > > +   load = <0>;
> > > > +   blob {
> > > > +   filename = 
> > > > "/tmp/capsules/u-boot.env.new";
> > > > +   };
> > > > +
> > > > +   hash-1 {
> > > > +   algo = "sha1";
> > > > +   };
> > > > +   };
> > > > +   };
> > > > +   };
> > > > +   };
> > > > +
> > > > +#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
> > > > +   capsule1 {
> > > > +   capsule {
> > > > +   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
> > > > +   };
> > > > +   };
> > > > +#else
> > > > +   capsule2 {
> > > > +   capsule {
> > > > +   image-index = <0x1>;
> > > > +   image-type-id = 
> > > > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > >
> > > We seem to have a persistent problem with these appearing in the source 
> > > code.
> > >
> > > Perhaps you could add them to a header file and use
> > > GUID_MEANINGFUL_NAME here instead (also below).
> > >
> > > In general, GUIDs 

Re: [PATCH v4 12/12] sandbox: capsule: Generate capsule related files through binman

2023-07-18 Thread Simon Glass
Hi Sughosh,

On Mon, 17 Jul 2023 at 05:18, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Sun, 16 Jul 2023 at 05:12, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu  wrote:
> > >
> > > The EFI capsule files can now be generated as part of u-boot
> > > build. This is done through binman. Add capsule entry nodes in the
> > > u-boot.dtsi for the sandbox architecture for generating the
> > > capsules. Remove the corresponding generation of capsules from the
> > > capsule update conftest file.
> > >
> > > The capsules are generated through the config file for the sandbox
> > > variant, and through explicit parameters for the sandbox_flattree
> > > variant.
> > >
> > > Also generate the FIT image used for testing the capsule update
> > > feature on the sandbox_flattree variant through binman. Remove the now
> > > superfluous its file which was used for generating this FIT image.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > > Changes since V3:
> > > * Use blob nodes instead of incbin for including the binaries in FIT
> > >   image.
> > > * Enable generation of capsules with versioning support.
> > >
> > >  arch/sandbox/dts/u-boot.dtsi  | 265 ++
> > >  test/py/tests/test_efi_capsule/conftest.py| 127 -
> > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 ---
> > >  3 files changed, 265 insertions(+), 163 deletions(-)
> > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> > >
> > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > index 60bd004937..7b0250ac81 100644
> > > --- a/arch/sandbox/dts/u-boot.dtsi
> > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > @@ -13,5 +13,270 @@
> > > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > };
> > >  #endif
> > > +
> > > +   binman: binman {
> > > +   multiple-images;
> > > +   };
> > > +};
> > > +
> > > + {
> > > +   itb {
> > > +   filename = "/tmp/capsules/uboot_bin_env.itb";
> > > +
> > > +   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 = 
> > > "/tmp/capsules/u-boot.bin.new";
> > > +   };
> > > +
> > > +   hash-1 {
> > > +   algo = "sha1";
> > > +   };
> > > +   };
> > > +   u-boot-env {
> > > +   description = "U-Boot environment 
> > > on SPI Flash";
> > > +   compression = "none";
> > > +   type = "firmware";
> > > +   arch = "sandbox";
> > > +   load = <0>;
> > > +   blob {
> > > +   filename = 
> > > "/tmp/capsules/u-boot.env.new";
> > > +   };
> > > +
> > > +   hash-1 {
> > > +   algo = "sha1";
> > > +   };
> > > +   };
> > > +   };
> > > +   };
> > > +   };
> > > +
> > > +#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
> > > +   capsule1 {
> > > +   capsule {
> > > +   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
> > > +   };
> > > +   };
> > > +#else
> > > +   capsule2 {
> > > +   capsule {
> > > +   image-index = <0x1>;
> > > +   image-type-id = 
> > > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> >
> > We seem to have a persistent problem with these appearing in the source 
> > code.
> >
> > Perhaps you could add them to a header file and use
> > GUID_MEANINGFUL_NAME here instead (also below).
> >
> > In general, GUIDs should not be open-coded.
>
> Okay. Will it be okay if I add these to a sandbox_capule.h. Earlier, I
> had similar GUID macros in the sandbox config header, and you had
> asked me to move them to the board file.

These are littered all over the place. Perhaps a new 'efi_guid.h'
header file that has all of 

Re: [PATCH v4 12/12] sandbox: capsule: Generate capsule related files through binman

2023-07-17 Thread Sughosh Ganu
hi Simon,

On Sun, 16 Jul 2023 at 05:12, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu  wrote:
> >
> > The EFI capsule files can now be generated as part of u-boot
> > build. This is done through binman. Add capsule entry nodes in the
> > u-boot.dtsi for the sandbox architecture for generating the
> > capsules. Remove the corresponding generation of capsules from the
> > capsule update conftest file.
> >
> > The capsules are generated through the config file for the sandbox
> > variant, and through explicit parameters for the sandbox_flattree
> > variant.
> >
> > Also generate the FIT image used for testing the capsule update
> > feature on the sandbox_flattree variant through binman. Remove the now
> > superfluous its file which was used for generating this FIT image.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> > Changes since V3:
> > * Use blob nodes instead of incbin for including the binaries in FIT
> >   image.
> > * Enable generation of capsules with versioning support.
> >
> >  arch/sandbox/dts/u-boot.dtsi  | 265 ++
> >  test/py/tests/test_efi_capsule/conftest.py| 127 -
> >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 ---
> >  3 files changed, 265 insertions(+), 163 deletions(-)
> >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> >
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > index 60bd004937..7b0250ac81 100644
> > --- a/arch/sandbox/dts/u-boot.dtsi
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -13,5 +13,270 @@
> > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > };
> >  #endif
> > +
> > +   binman: binman {
> > +   multiple-images;
> > +   };
> > +};
> > +
> > + {
> > +   itb {
> > +   filename = "/tmp/capsules/uboot_bin_env.itb";
> > +
> > +   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 = 
> > "/tmp/capsules/u-boot.bin.new";
> > +   };
> > +
> > +   hash-1 {
> > +   algo = "sha1";
> > +   };
> > +   };
> > +   u-boot-env {
> > +   description = "U-Boot environment 
> > on SPI Flash";
> > +   compression = "none";
> > +   type = "firmware";
> > +   arch = "sandbox";
> > +   load = <0>;
> > +   blob {
> > +   filename = 
> > "/tmp/capsules/u-boot.env.new";
> > +   };
> > +
> > +   hash-1 {
> > +   algo = "sha1";
> > +   };
> > +   };
> > +   };
> > +   };
> > +   };
> > +
> > +#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
> > +   capsule1 {
> > +   capsule {
> > +   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
> > +   };
> > +   };
> > +#else
> > +   capsule2 {
> > +   capsule {
> > +   image-index = <0x1>;
> > +   image-type-id = 
> > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
> We seem to have a persistent problem with these appearing in the source code.
>
> Perhaps you could add them to a header file and use
> GUID_MEANINGFUL_NAME here instead (also below).
>
> In general, GUIDs should not be open-coded.

Okay. Will it be okay if I add these to a sandbox_capule.h. Earlier, I
had similar GUID macros in the sandbox config header, and you had
asked me to move them to the board file.

>
>
> > +   filename = "/tmp/capsules/u-boot.bin.new";
> > +   capsule = "/tmp/capsules/Test01";
>
> There is something odd here. You should not need to specify an
> absolute pathname and should not use /tmp

The /tmp/capsules/ directory is being used for collating all the
capsule testing related files. Both the input files as well as the
output capsule files are 

Re: [PATCH v4 12/12] sandbox: capsule: Generate capsule related files through binman

2023-07-15 Thread Simon Glass
Hi Sughosh,

On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu  wrote:
>
> The EFI capsule files can now be generated as part of u-boot
> build. This is done through binman. Add capsule entry nodes in the
> u-boot.dtsi for the sandbox architecture for generating the
> capsules. Remove the corresponding generation of capsules from the
> capsule update conftest file.
>
> The capsules are generated through the config file for the sandbox
> variant, and through explicit parameters for the sandbox_flattree
> variant.
>
> Also generate the FIT image used for testing the capsule update
> feature on the sandbox_flattree variant through binman. Remove the now
> superfluous its file which was used for generating this FIT image.
>
> Signed-off-by: Sughosh Ganu 
> ---
> Changes since V3:
> * Use blob nodes instead of incbin for including the binaries in FIT
>   image.
> * Enable generation of capsules with versioning support.
>
>  arch/sandbox/dts/u-boot.dtsi  | 265 ++
>  test/py/tests/test_efi_capsule/conftest.py| 127 -
>  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 ---
>  3 files changed, 265 insertions(+), 163 deletions(-)
>  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
>
> diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> index 60bd004937..7b0250ac81 100644
> --- a/arch/sandbox/dts/u-boot.dtsi
> +++ b/arch/sandbox/dts/u-boot.dtsi
> @@ -13,5 +13,270 @@
> capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> };
>  #endif
> +
> +   binman: binman {
> +   multiple-images;
> +   };
> +};
> +
> + {
> +   itb {
> +   filename = "/tmp/capsules/uboot_bin_env.itb";
> +
> +   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 = 
> "/tmp/capsules/u-boot.bin.new";
> +   };
> +
> +   hash-1 {
> +   algo = "sha1";
> +   };
> +   };
> +   u-boot-env {
> +   description = "U-Boot environment on 
> SPI Flash";
> +   compression = "none";
> +   type = "firmware";
> +   arch = "sandbox";
> +   load = <0>;
> +   blob {
> +   filename = 
> "/tmp/capsules/u-boot.env.new";
> +   };
> +
> +   hash-1 {
> +   algo = "sha1";
> +   };
> +   };
> +   };
> +   };
> +   };
> +
> +#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
> +   capsule1 {
> +   capsule {
> +   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
> +   };
> +   };
> +#else
> +   capsule2 {
> +   capsule {
> +   image-index = <0x1>;
> +   image-type-id = 
> "09D7CF52-0720-4710-91D1-08469B7FE9C8";

We seem to have a persistent problem with these appearing in the source code.

Perhaps you could add them to a header file and use
GUID_MEANINGFUL_NAME here instead (also below).

In general, GUIDs should not be open-coded.


> +   filename = "/tmp/capsules/u-boot.bin.new";
> +   capsule = "/tmp/capsules/Test01";

There is something odd here. You should not need to specify an
absolute pathname and should not use /tmp


> +   };
> +   };
> +
> +   capsule3 {
> +   capsule {
> +   image-index = <0x2>;
> +   image-type-id = 
> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> +   filename = "/tmp/capsules/u-boot.env.new";
> +   capsule = "/tmp/capsules/Test02";
> +   };
> +   };
> +
> +   capsule4 {
> +   capsule {
> +   image-index = <0x1>;
> +   image-type-id = 
> "058B7D83-50D5-4C47-A195-60D86AD341C4";
> +   filename = 

[PATCH v4 12/12] sandbox: capsule: Generate capsule related files through binman

2023-07-15 Thread Sughosh Ganu
The EFI capsule files can now be generated as part of u-boot
build. This is done through binman. Add capsule entry nodes in the
u-boot.dtsi for the sandbox architecture for generating the
capsules. Remove the corresponding generation of capsules from the
capsule update conftest file.

The capsules are generated through the config file for the sandbox
variant, and through explicit parameters for the sandbox_flattree
variant.

Also generate the FIT image used for testing the capsule update
feature on the sandbox_flattree variant through binman. Remove the now
superfluous its file which was used for generating this FIT image.

Signed-off-by: Sughosh Ganu 
---
Changes since V3:
* Use blob nodes instead of incbin for including the binaries in FIT
  image.
* Enable generation of capsules with versioning support.

 arch/sandbox/dts/u-boot.dtsi  | 265 ++
 test/py/tests/test_efi_capsule/conftest.py| 127 -
 .../tests/test_efi_capsule/uboot_bin_env.its  |  36 ---
 3 files changed, 265 insertions(+), 163 deletions(-)
 delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its

diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
index 60bd004937..7b0250ac81 100644
--- a/arch/sandbox/dts/u-boot.dtsi
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -13,5 +13,270 @@
capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
};
 #endif
+
+   binman: binman {
+   multiple-images;
+   };
+};
+
+ {
+   itb {
+   filename = "/tmp/capsules/uboot_bin_env.itb";
+
+   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 = 
"/tmp/capsules/u-boot.bin.new";
+   };
+
+   hash-1 {
+   algo = "sha1";
+   };
+   };
+   u-boot-env {
+   description = "U-Boot environment on 
SPI Flash";
+   compression = "none";
+   type = "firmware";
+   arch = "sandbox";
+   load = <0>;
+   blob {
+   filename = 
"/tmp/capsules/u-boot.env.new";
+   };
+
+   hash-1 {
+   algo = "sha1";
+   };
+   };
+   };
+   };
+   };
+
+#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
+   capsule1 {
+   capsule {
+   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
+   };
+   };
+#else
+   capsule2 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+   filename = "/tmp/capsules/u-boot.bin.new";
+   capsule = "/tmp/capsules/Test01";
+   };
+   };
+
+   capsule3 {
+   capsule {
+   image-index = <0x2>;
+   image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+   filename = "/tmp/capsules/u-boot.env.new";
+   capsule = "/tmp/capsules/Test02";
+   };
+   };
+
+   capsule4 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4";
+   filename = "/tmp/capsules/u-boot.bin.new";
+   capsule = "/tmp/capsules/Test03";
+   };
+   };
+
+   capsule5 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
+   filename = "/tmp/capsules/uboot_bin_env.itb";
+   capsule = "/tmp/capsules/Test04";
+   };
+   };
+
+   capsule6 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4";
+   filename =