hi Simon,
On Tue, 11 Jul 2023 at 03:09, Simon Glass <s...@chromium.org> wrote:
>
> Hi,
>
> On Sun, 9 Jul 2023 at 07:34, Sughosh Ganu <sughosh.g...@linaro.org> 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 <sughosh.g...@linaro.org>
> > ---
> > Changes since V2:
> > * New patch for generating the capsules and capsule input files
> > through binman.
> >
> > arch/sandbox/dts/u-boot.dtsi | 143 ++++++++++++++++++
> > test/py/tests/test_efi_capsule/conftest.py | 62 --------
> > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -----
> > 3 files changed, 143 insertions(+), 98 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..292fb86a50 100644
> > --- a/arch/sandbox/dts/u-boot.dtsi
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -13,5 +13,148 @@
> > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > };
> > #endif
> > +
> > + binman: binman {
> > + multiple-images;
> > + };
> > +};
> > +
> > +&binman {
> > + itb {
> > + filename = "/tmp/capsules/uboot_bin_env.itb";
>
> You can't really do this, since that dir may not exist. Can you drop the path?
This directory does exist. I am adding logic to add the directory in
patches 7 and 8 to ensure that the /tmp/capsules/ directory exists for
the capsule updates testing, both for CI runs as well as local pytest
test runs.
>
> > +
> > + fit {
> > + description = "Automatic U-Boot environment update";
> > + #address-cells = <2>;
> > +
> > + images {
> > + u-boot-bin {
> > + description = "U-Boot binary on SPI
> > Flash";
> > + data =
> > /incbin/("/tmp/capsules/u-boot.bin.new");
>
> See FIT docs for how to include data in a FIT with binman.
>
> Basically you add it below *
Okay. WIll change this.
>
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + hash-1 {
> > + algo = "sha1";
> > + };
>
> *
> blob {
> filename = "u-boot.bin.new";
> }
>
> Please fix throughout.
>
> > + };
> > + u-boot-env {
> > + description = "U-Boot environment
> > on SPI Flash";
> > + data =
> > /incbin/("/tmp/capsules/u-boot.env.new");
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + 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 = "/tmp/capsules/uboot_bin_env.itb";
> > + capsule = "/tmp/capsules/Test05";
> > + };
> > + };
> > +
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > + capsule7 {
> > + capsule {
> > + image-index = <0x1>;
> > + image-type-id =
> > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > + private-key = "/tmp/capsules/SIGNER.key";
> > + pub-key-cert = "/tmp/capsules/SIGNER.crt";
> > + monotonic-count = <0x1>;
> > + filename = "/tmp/capsules/u-boot.bin.new";
> > + capsule = "/tmp/capsules/Test11";
> > + };
> > + };
> > +
> > + capsule8 {
> > + capsule {
> > + image-index = <0x1>;
> > + image-type-id =
> > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > + private-key = "/tmp/capsules/SIGNER2.key";
> > + pub-key-cert = "/tmp/capsules/SIGNER2.crt";
> > + monotonic-count = <0x1>;
> > + filename = "/tmp/capsules/u-boot.bin.new";
> > + capsule = "/tmp/capsules/Test12";
> > + };
> > + };
> > +
> > + capsule9 {
> > + capsule {
> > + image-index = <0x1>;
> > + image-type-id =
> > "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
> > + private-key = "/tmp/capsules/SIGNER.key";
> > + pub-key-cert = "/tmp/capsules/SIGNER.crt";
> > + monotonic-count = <0x1>;
> > + filename = "/tmp/capsules/uboot_bin_env.itb";
> > + capsule = "/tmp/capsules/Test13";
> > + };
> > + };
> > +
> > + capsule10 {
> > + capsule {
> > + image-index = <0x1>;
> > + image-type-id =
> > "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
> > + private-key = "/tmp/capsules/SIGNER2.key";
> > + pub-key-cert = "/tmp/capsules/SIGNER2.crt";
> > + monotonic-count = <0x1>;
> > + filename = "/tmp/capsules/uboot_bin_env.itb";
> > + capsule = "/tmp/capsules/Test14";
> > + };
> > + };
> > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > +#endif /* CONFIG_EFI_USE_CAPSULE_CFG_FILE */
> > };
> > #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > diff --git a/test/py/tests/test_efi_capsule/conftest.py
> > b/test/py/tests/test_efi_capsule/conftest.py
> > index 9b0f7e635d..b2315b7d51 100644
> > --- a/test/py/tests/test_efi_capsule/conftest.py
> > +++ b/test/py/tests/test_efi_capsule/conftest.py
> > @@ -40,68 +40,6 @@ def efi_capsule_data(request, u_boot_config):
> > check_call('cp %s/arch/sandbox/dts/test.dtb %s/test_sig.dtb' %
> > (u_boot_config.build_dir, data_dir), shell=True)
> >
> > - # Create capsule files
> > - # two regions: one for u-boot.bin and the other for u-boot.env
> > - check_call('cd %s; echo -n u-boot:Old > u-boot.bin.old; echo -n
> > u-boot:New > u-boot.bin.new; echo -n u-boot-env:Old > u-boot.env.old; echo
> > -n u-boot-env:New > u-boot.env.new' % data_dir,
> > - shell=True)
> > - check_call('sed -e \"s?BINFILE1?u-boot.bin.new?\" -e
> > \"s?BINFILE2?u-boot.env.new?\"
> > %s/test/py/tests/test_efi_capsule/uboot_bin_env.its > %s/uboot_bin_env.its'
> > %
> > - (u_boot_config.source_dir, data_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its
> > uboot_bin_env.itb' %
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid
> > 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test01' %
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkeficapsule --index 2 --guid
> > 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test02' %
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid
> > 058B7D83-50D5-4C47-A195-60D86AD341C4 u-boot.bin.new Test03' %
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid
> > 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test04' %
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid
> > 058B7D83-50D5-4C47-A195-60D86AD341C4 uboot_bin_env.itb Test05' %
>
> Please put the GUIDs in variables or a dict and give them names.
Do you mean for the code that is getting added in the binman nodes?
The above code is actually getting removed.
>
> > - (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > -
> > - if capsule_auth_enabled:
> > - # raw firmware signed with proper key
> > - check_call('cd %s; '
> > - '%s/tools/mkeficapsule --index 1 --monotonic-count
> > 1 '
> > - '--private-key SIGNER.key --certificate
> > SIGNER.crt '
> > - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 '
> > - 'u-boot.bin.new Test11'
> > - % (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - # raw firmware signed with *mal* key
> > - check_call('cd %s; '
> > - '%s/tools/mkeficapsule --index 1 --monotonic-count
> > 1 '
> > - '--private-key SIGNER2.key '
> > - '--certificate SIGNER2.crt '
> > - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 '
> > - 'u-boot.bin.new Test12'
> > - % (data_dir, u_boot_config.build_dir),
> > - shell=True)
>
> Please create a function to handle the common code.
Again, this code is being removed.
>
> > - # FIT firmware signed with proper key
> > - check_call('cd %s; '
> > - '%s/tools/mkeficapsule --index 1 --monotonic-count
> > 1 '
> > - '--private-key SIGNER.key --certificate
> > SIGNER.crt '
> > - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 '
> > - 'uboot_bin_env.itb Test13'
> > - % (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > - # FIT firmware signed with *mal* key
> > - check_call('cd %s; '
> > - '%s/tools/mkeficapsule --index 1 --monotonic-count
> > 1 '
> > - '--private-key SIGNER2.key '
> > - '--certificate SIGNER2.crt '
> > - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 '
> > - 'uboot_bin_env.itb Test14'
> > - % (data_dir, u_boot_config.build_dir),
> > - shell=True)
> > -
> > # Create a disk image with EFI system partition
> > check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat %s
> > %s' %
> > (mnt_point, image_path), shell=True)
> > diff --git a/test/py/tests/test_efi_capsule/uboot_bin_env.its
> > b/test/py/tests/test_efi_capsule/uboot_bin_env.its
> > deleted file mode 100644
> > index fc65907481..0000000000
> > --- a/test/py/tests/test_efi_capsule/uboot_bin_env.its
> > +++ /dev/null
> > @@ -1,36 +0,0 @@
> > -/*
> > - * Automatic software update for U-Boot
>
> 'EFI' should be in there somewhere.
Okay. But again, this code is being removed. Do you want me to add
this somewhere else?
>
> > - * Make sure the flashing addresses ('load' prop) is correct for your
> > board!
> > - */
> > -
> > -/dts-v1/;
> > -
> > -/ {
> > - description = "Automatic U-Boot environment update";
> > - #address-cells = <2>;
> > -
> > - images {
> > - u-boot-bin {
> > - description = "U-Boot binary on SPI Flash";
> > - data = /incbin/("BINFILE1");
> > - compression = "none";
> > - type = "firmware";
> > - arch = "sandbox";
> > - load = <0>;
> > - hash-1 {
> > - algo = "sha1";
> > - };
> > - };
> > - u-boot-env {
> > - description = "U-Boot environment on SPI Flash";
> > - data = /incbin/("BINFILE2");
> > - compression = "none";
> > - type = "firmware";
> > - arch = "sandbox";
> > - load = <0>;
> > - hash-1 {
> > - algo = "sha1";
> > - };
> > - };
> > - };
> > -};
> > --
> > 2.34.1
> >
>
> Regards,
> Simon
