[PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
Tom Rini
trini at konsulko.com
Sun Aug 6 00:18:16 CEST 2023
On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass 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...
>
>
> >
> > 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?
And why are these tests in the generic looking file and not one of the
test dts files? This looks like something that would make implementation
in real life confusing and non-obvious.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230805/fb0f301b/attachment.sig>
More information about the U-Boot
mailing list