Re: [PATCH v2 1/4] image: Add IH_OS_EFI for EFI chain-load boot

On Tue, Dec 10, 2019 at 10:49:09PM +0000, Peter Robinson wrote:

On Tue, Dec 10, 2019 at 6:30 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 12/10/19 9:56 AM, Cristian Ciocaltea wrote:

...

Add a new OS type to be used for chain-loading an EFI compatible firmware or boot loader like GRUB2, possibly in a verified boot scenario.

Bellow is sample ITS file that generates a FIT image supporting secure boot. Please note the presence of 'os = "efi";' line, which identifies the currently introduced OS type:

/ { #address-cells = <1>;

 images {
     efi-grub {
         description = "GRUB EFI";
         data = /incbin/("EFI/BOOT/bootarm.efi");

According to UEFI Spec 2.8 the default file name for 32 bit ARM is BOOTARM.EFI. But GRUB calls the file grubarm.efi.

In Linux the boot<arch>.efi file is provided by shim [1] and used for secure boot etc, I believe the default is also the fall back boot method. I'm unaware of shim currently being built for armv7.

[1] https://github.com/rhboot/shim/

...

So shouldn't we use grubarm.efi here as filename?

My build platform relies on buildroot and that is the default path where the GRUB EFI binary is deployed. I don't know the reasons behind, but most probably they are related to portability/compatibility, as Peter already pointed out.

...

You use EFI/BOOT as directory name. I think this path does not add benefit to the example. The other *.its files also come without any specific path.

Totally agree, I will remove the directory path.

...

Best regards

Heinrich

...

         type = "kernel_noload";
         arch = "arm";
         os = "efi";
         compression = "none";
         load = <0x0>;
         entry = <0x0>;
         hash-1 {
             algo = "sha256";
         };
     };
 };

 configurations {
     default = "config-grub";
     config-grub {
         kernel = "efi-grub";
         signature-1 {
             algo = "sha256,rsa2048";
             sign-images = "kernel";
         };
     };
 };

};

Signed-off-by: Cristian Ciocaltea cristian.ciocaltea@gmail.com

common/image-fit.c | 3 ++- common/image.c | 1 + include/image.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/image-fit.c b/common/image-fit.c index 5c63c769de..19e313bf41 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1925,7 +1925,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr, image_type == IH_TYPE_FPGA || fit_image_check_os(fit, noffset, IH_OS_LINUX) || fit_image_check_os(fit, noffset, IH_OS_U_BOOT) ||

•         fit_image_check_os(fit, noffset, IH_OS_OPENRTOS);
  

•         fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) ||
  

•         fit_image_check_os(fit, noffset, IH_OS_EFI);
  
  /*
   * If either of the checks fail, we should report an error, but
  

diff --git a/common/image.c b/common/image.c index f17fa40c49..2e0e2b0e7f 100644 --- a/common/image.c +++ b/common/image.c @@ -134,6 +134,7 @@ static const table_entry_t uimage_os[] = { { IH_OS_OPENRTOS, "openrtos", "OpenRTOS", }, #endif { IH_OS_OPENSBI, "opensbi", "RISC-V OpenSBI", },

• {       IH_OS_EFI,      "efi",          "EFI Firmware" },
  
  {       -1,             "",             "",                     },
  

  };

diff --git a/include/image.h b/include/image.h index f4d2aaf53e..4a280b78e7 100644 --- a/include/image.h +++ b/include/image.h @@ -157,6 +157,7 @@ enum { IH_OS_ARM_TRUSTED_FIRMWARE, /* ARM Trusted Firmware */ IH_OS_TEE, /* Trusted Execution Environment */ IH_OS_OPENSBI, /* RISC-V OpenSBI */

• IH_OS_EFI,                      /* EFI Firmware (e.g. GRUB2) */
  
  IH_OS_COUNT,
  

  };