[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Cristian Ciocaltea]

Currently the only way to run an EFI binary like GRUB2 is via the
'bootefi' command, which cannot be used in a verified boot scenario.

The obvious solution to this limitation is to add support for
booting FIT images containing those EFI binaries.

The implementation relies on a new image type - IH_OS_EFI - which
can be created by using 'os = "efi"' inside an ITS file:

/ {
    #address-cells = <1>;

    images {
        efi-grub {
            description = "GRUB EFI";
            data = /incbin/("EFI/BOOT/bootarm.efi");
            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";
            };
        };
    };
};

The bootm command has been extended to handle the IH_OS_EFI images.
To enable this feature, a new configuration option has been added:
BOOTM_EFI

I tested the solution using the 'qemu_arm' board:

=> load scsi 0:1 ${kernel_addr_r} efi-image.fit
=> bootm ${kernel_addr_r}#config-grub


Cristian Ciocaltea (2):
  image: Add IH_OS_EFI for EFI chain-load boot
  bootm: Add a bootm command for type IH_OS_EFI

 cmd/Kconfig        |  9 ++++++++-
 cmd/bootefi.c      |  2 +-
 common/bootm_os.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
 common/image-fit.c |  3 ++-
 common/image.c     |  1 +
 include/bootm.h    |  2 ++
 include/image.h    |  1 +
 7 files changed, 59 insertions(+), 3 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH 1/2] image: Add IH_OS_EFI for EFI chain-load boot

[Cristian Ciocaltea]

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");
            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,
 };
-- 
2.17.1

[U-Boot] [PATCH 2/2] bootm: Add a bootm command for type IH_OS_EFI

[Cristian Ciocaltea]

Add support for booting EFI binaries contained in FIT images.
A typical usage scenario is chain-loading GRUB2 in a verified
boot environment.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 cmd/Kconfig       |  9 ++++++++-
 cmd/bootefi.c     |  2 +-
 common/bootm_os.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/bootm.h   |  2 ++
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index cf982ff65e..1bec840f5a 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -263,6 +263,13 @@ config CMD_BOOTI
 	help
 	  Boot an AArch64 Linux Kernel image from memory.
 
+config BOOTM_EFI
+	bool "Support booting EFI OS images"
+	depends on CMD_BOOTEFI
+	default y
+	help
+	  Support booting EFI images via the bootm command.
+
 config BOOTM_LINUX
 	bool "Support booting Linux OS images"
 	depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI
@@ -316,7 +323,7 @@ config CMD_BOOTEFI
 	depends on EFI_LOADER
 	default y
 	help
-	  Boot an EFI image from memory.
+	  Boot an EFI binary from memory.
 
 config CMD_BOOTEFI_HELLO_COMPILE
 	bool "Compile a standard EFI hello world binary for testing"
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index f613cce7e2..f25d639dfe 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -553,7 +553,7 @@ static int do_efi_selftest(void)
  * @argv:	command line arguments
  * Return:	status code
  */
-static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	efi_status_t ret;
 
diff --git a/common/bootm_os.c b/common/bootm_os.c
index 6fb7d658da..706151913a 100644
--- a/common/bootm_os.c
+++ b/common/bootm_os.c
@@ -462,6 +462,47 @@ static int do_bootm_tee(int flag, int argc, char * const argv[],
 }
 #endif
 
+#ifdef CONFIG_BOOTM_EFI
+static int do_bootm_efi(int flag, int argc, char * const argv[],
+			bootm_headers_t *images)
+{
+	int ret;
+	int local_argc = 2;
+	char *local_args[3];
+	char str_efi_addr[16];
+	char str_fdt_addr[16];
+
+	if (flag != BOOTM_STATE_OS_GO)
+		return 0;
+
+	/* Locate FDT etc */
+	ret = bootm_find_images(flag, argc, argv);
+	if (ret)
+		return ret;
+
+	printf("## Transferring control to EFI (at address %08lx) ...\n",
+	       images->ep);
+	bootstage_mark(BOOTSTAGE_ID_RUN_OS);
+
+	local_args[0] = argv[0];
+
+	/* Write efi addr into string */
+	sprintf(str_efi_addr, "%lx", images->ep);
+	/* and provide it via the arguments */
+	local_args[1] = str_efi_addr;
+
+	if (images->ft_len) {
+		/* Write fdt addr into string */
+		sprintf(str_fdt_addr, "%lx", (unsigned long)images->ft_addr);
+		/* and provide it via the arguments */
+		local_args[2] = str_fdt_addr;
+		local_argc = 3;
+	}
+
+	return do_bootefi(NULL, 0, local_argc, local_args);
+}
+#endif
+
 static boot_os_fn *boot_os[] = {
 	[IH_OS_U_BOOT] = do_bootm_standalone,
 #ifdef CONFIG_BOOTM_LINUX
@@ -498,6 +539,9 @@ static boot_os_fn *boot_os[] = {
 #ifdef CONFIG_BOOTM_OPTEE
 	[IH_OS_TEE] = do_bootm_tee,
 #endif
+#ifdef CONFIG_BOOTM_EFI
+	[IH_OS_EFI] = do_bootm_efi,
+#endif
 };
 
 /* Allow for arch specific config before we boot */
diff --git a/include/bootm.h b/include/bootm.h
index edeeacb0df..a0da86dc32 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -37,7 +37,9 @@ typedef int boot_os_fn(int flag, int argc, char * const argv[],
 extern boot_os_fn do_bootm_linux;
 extern boot_os_fn do_bootm_vxworks;
 
+int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_bootelf(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+
 void lynxkdi_boot(image_header_t *hdr);
 
 boot_os_fn *bootm_os_get_boot_func(int os);
-- 
2.17.1

[U-Boot] [PATCH 2/2] bootm: Add a bootm command for type IH_OS_EFI

[Heinrich Schuchardt]

On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
> Add support for booting EFI binaries contained in FIT images.
> A typical usage scenario is chain-loading GRUB2 in a verified
> boot environment.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> ---
>   cmd/Kconfig       |  9 ++++++++-
>   cmd/bootefi.c     |  2 +-
>   common/bootm_os.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   include/bootm.h   |  2 ++
>   4 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index cf982ff65e..1bec840f5a 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -263,6 +263,13 @@ config CMD_BOOTI
>   	help
>   	  Boot an AArch64 Linux Kernel image from memory.
>
> +config BOOTM_EFI
> +	bool "Support booting EFI OS images"
> +	depends on CMD_BOOTEFI
> +	default y
> +	help
> +	  Support booting EFI images via the bootm command.
> +
>   config BOOTM_LINUX
>   	bool "Support booting Linux OS images"
>   	depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI
> @@ -316,7 +323,7 @@ config CMD_BOOTEFI
>   	depends on EFI_LOADER
>   	default y
>   	help
> -	  Boot an EFI image from memory.
> +	  Boot an EFI binary from memory.
>
>   config CMD_BOOTEFI_HELLO_COMPILE
>   	bool "Compile a standard EFI hello world binary for testing"
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index f613cce7e2..f25d639dfe 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -553,7 +553,7 @@ static int do_efi_selftest(void)
>    * @argv:	command line arguments
>    * Return:	status code
>    */
> -static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   {
>   	efi_status_t ret;
>
> diff --git a/common/bootm_os.c b/common/bootm_os.c
> index 6fb7d658da..706151913a 100644
> --- a/common/bootm_os.c
> +++ b/common/bootm_os.c
> @@ -462,6 +462,47 @@ static int do_bootm_tee(int flag, int argc, char * const argv[],
>   }
>   #endif
>
> +#ifdef CONFIG_BOOTM_EFI
> +static int do_bootm_efi(int flag, int argc, char * const argv[],
> +			bootm_headers_t *images)
> +{
> +	int ret;
> +	int local_argc = 2;
> +	char *local_args[3];
> +	char str_efi_addr[16];
> +	char str_fdt_addr[16];
> +
> +	if (flag != BOOTM_STATE_OS_GO)
> +		return 0;
> +
> +	/* Locate FDT etc */
> +	ret = bootm_find_images(flag, argc, argv);
> +	if (ret)
> +		return ret;
> +
> +	printf("## Transferring control to EFI (at address %08lx) ...\n",
> +	       images->ep);
> +	bootstage_mark(BOOTSTAGE_ID_RUN_OS);
> +
> +	local_args[0] = argv[0];
> +
> +	/* Write efi addr into string */
> +	sprintf(str_efi_addr, "%lx", images->ep);

I think we should avoid transferring arguments as strings. It is
preferable to separate do_bootefi() into an argument parser and an
execution part.

Let's CC the developers who have contributed to common/bootm_os.c.

Best regards

Heinrich

> +	/* and provide it via the arguments */
> +	local_args[1] = str_efi_addr;
> +
> +	if (images->ft_len) {
> +		/* Write fdt addr into string */
> +		sprintf(str_fdt_addr, "%lx", (unsigned long)images->ft_addr);
> +		/* and provide it via the arguments */
> +		local_args[2] = str_fdt_addr;
> +		local_argc = 3;
> +	}
> +
> +	return do_bootefi(NULL, 0, local_argc, local_args);
> +}
> +#endif
> +
>   static boot_os_fn *boot_os[] = {
>   	[IH_OS_U_BOOT] = do_bootm_standalone,
>   #ifdef CONFIG_BOOTM_LINUX
> @@ -498,6 +539,9 @@ static boot_os_fn *boot_os[] = {
>   #ifdef CONFIG_BOOTM_OPTEE
>   	[IH_OS_TEE] = do_bootm_tee,
>   #endif
> +#ifdef CONFIG_BOOTM_EFI
> +	[IH_OS_EFI] = do_bootm_efi,
> +#endif
>   };
>
>   /* Allow for arch specific config before we boot */
> diff --git a/include/bootm.h b/include/bootm.h
> index edeeacb0df..a0da86dc32 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -37,7 +37,9 @@ typedef int boot_os_fn(int flag, int argc, char * const argv[],
>   extern boot_os_fn do_bootm_linux;
>   extern boot_os_fn do_bootm_vxworks;
>
> +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>   int do_bootelf(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> +
>   void lynxkdi_boot(image_header_t *hdr);
>
>   boot_os_fn *bootm_os_get_boot_func(int os);
>

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Heinrich Schuchardt]

On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
> Currently the only way to run an EFI binary like GRUB2 is via the
> 'bootefi' command, which cannot be used in a verified boot scenario.
>
> The obvious solution to this limitation is to add support for
> booting FIT images containing those EFI binaries.
>
> The implementation relies on a new image type - IH_OS_EFI - which
> can be created by using 'os = "efi"' inside an ITS file:
>
> / {
>      #address-cells = <1>;
>
>      images {
>          efi-grub {
>              description = "GRUB EFI";
>              data = /incbin/("EFI/BOOT/bootarm.efi");
>              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";
>              };
>          };
>      };
> };
>
> The bootm command has been extended to handle the IH_OS_EFI images.
> To enable this feature, a new configuration option has been added:
> BOOTM_EFI
>
> I tested the solution using the 'qemu_arm' board:
>
> => load scsi 0:1 ${kernel_addr_r} efi-image.fit
> => bootm ${kernel_addr_r}#config-grub

Thanks a lot for the patch series which makes good sense to me.

I think we should pass addresses and not strings to cmd/bootefi.c. This
will need a bit of refactoring as already addressed in a comment to
patch 2/2.

Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
doc/uImage.FIT/howto.txt should be updated.

I cc the contributors given by
scripts/get_maintainer.pl -f common/bootm_os.c

Best regards

Heinrich

>
>
> Cristian Ciocaltea (2):
>    image: Add IH_OS_EFI for EFI chain-load boot
>    bootm: Add a bootm command for type IH_OS_EFI
>
>   cmd/Kconfig        |  9 ++++++++-
>   cmd/bootefi.c      |  2 +-
>   common/bootm_os.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   common/image-fit.c |  3 ++-
>   common/image.c     |  1 +
>   include/bootm.h    |  2 ++
>   include/image.h    |  1 +
>   7 files changed, 59 insertions(+), 3 deletions(-)
>

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Cristian Ciocaltea]

On Tue, Nov 26, 2019 at 07:31:39PM +0100, Heinrich Schuchardt wrote:
> On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
> > Currently the only way to run an EFI binary like GRUB2 is via the
> > 'bootefi' command, which cannot be used in a verified boot scenario.
> > 
> > The obvious solution to this limitation is to add support for
> > booting FIT images containing those EFI binaries.
> > 
> > The implementation relies on a new image type - IH_OS_EFI - which
> > can be created by using 'os = "efi"' inside an ITS file:
> > 
> > / {
> >      #address-cells = <1>;
> > 
> >      images {
> >          efi-grub {
> >              description = "GRUB EFI";
> >              data = /incbin/("EFI/BOOT/bootarm.efi");
> >              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";
> >              };
> >          };
> >      };
> > };
> > 
> > The bootm command has been extended to handle the IH_OS_EFI images.
> > To enable this feature, a new configuration option has been added:
> > BOOTM_EFI
> > 
> > I tested the solution using the 'qemu_arm' board:
> > 
> > => load scsi 0:1 ${kernel_addr_r} efi-image.fit
> > => bootm ${kernel_addr_r}#config-grub
> 
> Thanks a lot for the patch series which makes good sense to me.
> 
> I think we should pass addresses and not strings to cmd/bootefi.c. This
> will need a bit of refactoring as already addressed in a comment to
> patch 2/2.
> 
> Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
> doc/uImage.FIT/howto.txt should be updated.
> 
> I cc the contributors given by
> scripts/get_maintainer.pl -f common/bootm_os.c
> 
> Best regards
> 
> Heinrich
> 

Thanks for the feedback, Heinrich!

Instead of creating new function(s), I think we could simply extend
  int do_bootefi_image(const char *image_opt)
with a new parameter to hold the fdt address and move here the call
to 'efi_install_fdt()', which is now performed by 'do_bootefi()'.

However, I'm not sure about changing the data types, i.e. from
'char *' to ulong, for the following reasons:
1. image_opt may have a different meaning in addition to efi address
2. fdt address may not be provided, so we need somehow to detect an
invalid value

Kind regards,
Cristian

> > 
> > 
> > Cristian Ciocaltea (2):
> >    image: Add IH_OS_EFI for EFI chain-load boot
> >    bootm: Add a bootm command for type IH_OS_EFI
> > 
> >   cmd/Kconfig        |  9 ++++++++-
> >   cmd/bootefi.c      |  2 +-
> >   common/bootm_os.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >   common/image-fit.c |  3 ++-
> >   common/image.c     |  1 +
> >   include/bootm.h    |  2 ++
> >   include/image.h    |  1 +
> >   7 files changed, 59 insertions(+), 3 deletions(-)
> > 
> 

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Heinrich Schuchardt]

On 11/27/19 8:45 PM, Cristian Ciocaltea wrote:
> On Tue, Nov 26, 2019 at 07:31:39PM +0100, Heinrich Schuchardt wrote:
>> On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
>>> Currently the only way to run an EFI binary like GRUB2 is via the
>>> 'bootefi' command, which cannot be used in a verified boot scenario.
>>>
>>> The obvious solution to this limitation is to add support for
>>> booting FIT images containing those EFI binaries.
>>>
>>> The implementation relies on a new image type - IH_OS_EFI - which
>>> can be created by using 'os = "efi"' inside an ITS file:
>>>
>>> / {
>>>       #address-cells = <1>;
>>>
>>>       images {
>>>           efi-grub {
>>>               description = "GRUB EFI";
>>>               data = /incbin/("EFI/BOOT/bootarm.efi");
>>>               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";
>>>               };
>>>           };
>>>       };
>>> };
>>>
>>> The bootm command has been extended to handle the IH_OS_EFI images.
>>> To enable this feature, a new configuration option has been added:
>>> BOOTM_EFI
>>>
>>> I tested the solution using the 'qemu_arm' board:
>>>
>>> => load scsi 0:1 ${kernel_addr_r} efi-image.fit
>>> => bootm ${kernel_addr_r}#config-grub
>>
>> Thanks a lot for the patch series which makes good sense to me.
>>
>> I think we should pass addresses and not strings to cmd/bootefi.c. This
>> will need a bit of refactoring as already addressed in a comment to
>> patch 2/2.
>>
>> Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
>> doc/uImage.FIT/howto.txt should be updated.
>>
>> I cc the contributors given by
>> scripts/get_maintainer.pl -f common/bootm_os.c
>>
>> Best regards
>>
>> Heinrich
>>
>
> Thanks for the feedback, Heinrich!
>
> Instead of creating new function(s), I think we could simply extend
>    int do_bootefi_image(const char *image_opt)
> with a new parameter to hold the fdt address and move here the call
> to 'efi_install_fdt()', which is now performed by 'do_bootefi()'.

efi_install_fdt() has to be called for the 'bootefi bootmgr' command too
so the refactoring is a bit more complicated. I have started on that.

The first step is to change efi_install_fdt() to expect the argument as
address instead of a string.

https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-pass-address-to-efi_install_fdt.patch

fdt_addr==NULL indicates no device tree supplied by user.

Best regards

Heinrich

>
> However, I'm not sure about changing the data types, i.e. from
> 'char *' to ulong, for the following reasons:
> 1. image_opt may have a different meaning in addition to efi address
> 2. fdt address may not be provided, so we need somehow to detect an
> invalid value
>
> Kind regards,
> Cristian
>
>>>
>>>
>>> Cristian Ciocaltea (2):
>>>     image: Add IH_OS_EFI for EFI chain-load boot
>>>     bootm: Add a bootm command for type IH_OS_EFI
>>>
>>>    cmd/Kconfig        |  9 ++++++++-
>>>    cmd/bootefi.c      |  2 +-
>>>    common/bootm_os.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>    common/image-fit.c |  3 ++-
>>>    common/image.c     |  1 +
>>>    include/bootm.h    |  2 ++
>>>    include/image.h    |  1 +
>>>    7 files changed, 59 insertions(+), 3 deletions(-)
>>>
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Heinrich Schuchardt]

On 11/28/19 8:20 AM, Heinrich Schuchardt wrote:
> On 11/27/19 8:45 PM, Cristian Ciocaltea wrote:
>> On Tue, Nov 26, 2019 at 07:31:39PM +0100, Heinrich Schuchardt wrote:
>>> On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
>>>> Currently the only way to run an EFI binary like GRUB2 is via the
>>>> 'bootefi' command, which cannot be used in a verified boot scenario.
>>>>
>>>> The obvious solution to this limitation is to add support for
>>>> booting FIT images containing those EFI binaries.
>>>>
>>>> The implementation relies on a new image type - IH_OS_EFI - which
>>>> can be created by using 'os = "efi"' inside an ITS file:
>>>>
>>>> / {
>>>>       #address-cells = <1>;
>>>>
>>>>       images {
>>>>           efi-grub {
>>>>               description = "GRUB EFI";
>>>>               data = /incbin/("EFI/BOOT/bootarm.efi");
>>>>               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";
>>>>               };
>>>>           };
>>>>       };
>>>> };
>>>>
>>>> The bootm command has been extended to handle the IH_OS_EFI images.
>>>> To enable this feature, a new configuration option has been added:
>>>> BOOTM_EFI
>>>>
>>>> I tested the solution using the 'qemu_arm' board:
>>>>
>>>> => load scsi 0:1 ${kernel_addr_r} efi-image.fit
>>>> => bootm ${kernel_addr_r}#config-grub
>>>
>>> Thanks a lot for the patch series which makes good sense to me.
>>>
>>> I think we should pass addresses and not strings to cmd/bootefi.c. This
>>> will need a bit of refactoring as already addressed in a comment to
>>> patch 2/2.
>>>
>>> Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
>>> doc/uImage.FIT/howto.txt should be updated.
>>>
>>> I cc the contributors given by
>>> scripts/get_maintainer.pl -f common/bootm_os.c
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>
>> Thanks for the feedback, Heinrich!
>>
>> Instead of creating new function(s), I think we could simply extend
>>    int do_bootefi_image(const char *image_opt)
>> with a new parameter to hold the fdt address and move here the call
>> to 'efi_install_fdt()', which is now performed by 'do_bootefi()'.
>
> efi_install_fdt() has to be called for the 'bootefi bootmgr' command too
> so the refactoring is a bit more complicated. I have started on that.
>
> The first step is to change efi_install_fdt() to expect the argument as
> address instead of a string.
>
> https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-pass-address-to-efi_install_fdt.patch
>
>
> fdt_addr==NULL indicates no device tree supplied by user.
>
> Best regards
>
> Heinrich
>
>>
>> However, I'm not sure about changing the data types, i.e. from
>> 'char *' to ulong, for the following reasons:
>> 1. image_opt may have a different meaning in addition to efi address
>> 2. fdt address may not be provided, so we need somehow to detect an >> invalid value
>>
>> Kind regards,
>> Cristian
>>

Hello Christian,

patch series
efi_loader: prepare for FIT images
https://lists.denx.de/pipermail/u-boot/2019-December/393192.html
is now available. It offers these functions:

/* Install device tree */
efi_status_t efi_install_fdt(uintptr_t fdt_addr);
/* Run loaded UEFI image */
efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);

Could you, please, rebase your patches on this patch series.

Please, call efi_install_fdt with EFI_FDT_USE_INTERNAL if no device tree
is supplied by the FIT image.

The patch series is also available in branch efi-2020-04 at
https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git

Best regards

Heinrich

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Cristian Ciocaltea]

On Sun, Dec 08, 2019 at 01:25:27AM +0100, Heinrich Schuchardt wrote:
> On 11/28/19 8:20 AM, Heinrich Schuchardt wrote:
> > On 11/27/19 8:45 PM, Cristian Ciocaltea wrote:
> > > On Tue, Nov 26, 2019 at 07:31:39PM +0100, Heinrich Schuchardt wrote:
> > > > On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
> > > > > Currently the only way to run an EFI binary like GRUB2 is via the
> > > > > 'bootefi' command, which cannot be used in a verified boot scenario.
> > > > > 
> > > > > The obvious solution to this limitation is to add support for
> > > > > booting FIT images containing those EFI binaries.
> > > > > 
> > > > > The implementation relies on a new image type - IH_OS_EFI - which
> > > > > can be created by using 'os = "efi"' inside an ITS file:
> > > > > 
> > > > > / {
> > > > >       #address-cells = <1>;
> > > > > 
> > > > >       images {
> > > > >           efi-grub {
> > > > >               description = "GRUB EFI";
> > > > >               data = /incbin/("EFI/BOOT/bootarm.efi");
> > > > >               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";
> > > > >               };
> > > > >           };
> > > > >       };
> > > > > };
> > > > > 
> > > > > The bootm command has been extended to handle the IH_OS_EFI images.
> > > > > To enable this feature, a new configuration option has been added:
> > > > > BOOTM_EFI
> > > > > 
> > > > > I tested the solution using the 'qemu_arm' board:
> > > > > 
> > > > > => load scsi 0:1 ${kernel_addr_r} efi-image.fit
> > > > > => bootm ${kernel_addr_r}#config-grub
> > > > 
> > > > Thanks a lot for the patch series which makes good sense to me.
> > > > 
> > > > I think we should pass addresses and not strings to cmd/bootefi.c. This
> > > > will need a bit of refactoring as already addressed in a comment to
> > > > patch 2/2.
> > > > 
> > > > Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
> > > > doc/uImage.FIT/howto.txt should be updated.
> > > > 
> > > > I cc the contributors given by
> > > > scripts/get_maintainer.pl -f common/bootm_os.c
> > > > 
> > > > Best regards
> > > > 
> > > > Heinrich
> > > > 
> > > 
> > > Thanks for the feedback, Heinrich!
> > > 
> > > Instead of creating new function(s), I think we could simply extend
> > >    int do_bootefi_image(const char *image_opt)
> > > with a new parameter to hold the fdt address and move here the call
> > > to 'efi_install_fdt()', which is now performed by 'do_bootefi()'.
> > 
> > efi_install_fdt() has to be called for the 'bootefi bootmgr' command too
> > so the refactoring is a bit more complicated. I have started on that.
> > 
> > The first step is to change efi_install_fdt() to expect the argument as
> > address instead of a string.
> > 
> > https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-pass-address-to-efi_install_fdt.patch
> > 
> > 
> > fdt_addr==NULL indicates no device tree supplied by user.
> > 
> > Best regards
> > 
> > Heinrich
> > 
> > > 
> > > However, I'm not sure about changing the data types, i.e. from
> > > 'char *' to ulong, for the following reasons:
> > > 1. image_opt may have a different meaning in addition to efi address
> > > 2. fdt address may not be provided, so we need somehow to detect an >> invalid value
> > > 
> > > Kind regards,
> > > Cristian
> > > 
> 
> Hello Christian,
> 
> patch series
> efi_loader: prepare for FIT images
> https://lists.denx.de/pipermail/u-boot/2019-December/393192.html
> is now available. It offers these functions:
> 
> /* Install device tree */
> efi_status_t efi_install_fdt(uintptr_t fdt_addr);
> /* Run loaded UEFI image */
> efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
> 
> Could you, please, rebase your patches on this patch series.
> 
> Please, call efi_install_fdt with EFI_FDT_USE_INTERNAL if no device tree
> is supplied by the FIT image.
> 
> The patch series is also available in branch efi-2020-04 at
> https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
> 
> Best regards
> 
> Heinrich

Hello Heinrich,

Thanks for the patch series!
I will send the updated patches by latest tomorrow EOD.

You also mentioned updating the documentation in 
doc/uefi/u-boot_on_efi.rst and doc/uImage.FIT/howto.txt.

I've checked those documents and their content is quite generic,
not particularly related to this work. The former describes
how to build and run u-boot as an EFI application/payload, while
the later shows how to build and use FIT images.

If you agree, I could instead add a new ITS file in uImage.FIT folder
and describe there the new functionality.

Kind regards,
Cristian

[U-Boot] [PATCH 0/2] Add support for booting EFI FIT images

[Heinrich Schuchardt]

On 12/9/19 9:59 AM, Cristian Ciocaltea wrote:
> On Sun, Dec 08, 2019 at 01:25:27AM +0100, Heinrich Schuchardt wrote:
>> On 11/28/19 8:20 AM, Heinrich Schuchardt wrote:
>>> On 11/27/19 8:45 PM, Cristian Ciocaltea wrote:
>>>> On Tue, Nov 26, 2019 at 07:31:39PM +0100, Heinrich Schuchardt wrote:
>>>>> On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
>>>>>> Currently the only way to run an EFI binary like GRUB2 is via the
>>>>>> 'bootefi' command, which cannot be used in a verified boot scenario.
>>>>>>
>>>>>> The obvious solution to this limitation is to add support for
>>>>>> booting FIT images containing those EFI binaries.
>>>>>>
>>>>>> The implementation relies on a new image type - IH_OS_EFI - which
>>>>>> can be created by using 'os = "efi"' inside an ITS file:
>>>>>>
>>>>>> / {
>>>>>>        #address-cells = <1>;
>>>>>>
>>>>>>        images {
>>>>>>            efi-grub {
>>>>>>                description = "GRUB EFI";
>>>>>>                data = /incbin/("EFI/BOOT/bootarm.efi");
>>>>>>                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";
>>>>>>                };
>>>>>>            };
>>>>>>        };
>>>>>> };
>>>>>>
>>>>>> The bootm command has been extended to handle the IH_OS_EFI images.
>>>>>> To enable this feature, a new configuration option has been added:
>>>>>> BOOTM_EFI
>>>>>>
>>>>>> I tested the solution using the 'qemu_arm' board:
>>>>>>
>>>>>> => load scsi 0:1 ${kernel_addr_r} efi-image.fit
>>>>>> => bootm ${kernel_addr_r}#config-grub
>>>>>
>>>>> Thanks a lot for the patch series which makes good sense to me.
>>>>>
>>>>> I think we should pass addresses and not strings to cmd/bootefi.c. This
>>>>> will need a bit of refactoring as already addressed in a comment to
>>>>> patch 2/2.
>>>>>
>>>>> Additionally the documentation in doc/uefi/u-boot_on_efi.rst and
>>>>> doc/uImage.FIT/howto.txt should be updated.
>>>>>
>>>>> I cc the contributors given by
>>>>> scripts/get_maintainer.pl -f common/bootm_os.c
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>
>>>> Thanks for the feedback, Heinrich!
>>>>
>>>> Instead of creating new function(s), I think we could simply extend
>>>>     int do_bootefi_image(const char *image_opt)
>>>> with a new parameter to hold the fdt address and move here the call
>>>> to 'efi_install_fdt()', which is now performed by 'do_bootefi()'.
>>>
>>> efi_install_fdt() has to be called for the 'bootefi bootmgr' command too
>>> so the refactoring is a bit more complicated. I have started on that.
>>>
>>> The first step is to change efi_install_fdt() to expect the argument as
>>> address instead of a string.
>>>
>>> https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-pass-address-to-efi_install_fdt.patch
>>>
>>>
>>> fdt_addr==NULL indicates no device tree supplied by user.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>>
>>>> However, I'm not sure about changing the data types, i.e. from
>>>> 'char *' to ulong, for the following reasons:
>>>> 1. image_opt may have a different meaning in addition to efi address
>>>> 2. fdt address may not be provided, so we need somehow to detect an >> invalid value
>>>>
>>>> Kind regards,
>>>> Cristian
>>>>
>>
>> Hello Christian,
>>
>> patch series
>> efi_loader: prepare for FIT images
>> https://lists.denx.de/pipermail/u-boot/2019-December/393192.html
>> is now available. It offers these functions:
>>
>> /* Install device tree */
>> efi_status_t efi_install_fdt(uintptr_t fdt_addr);
>> /* Run loaded UEFI image */
>> efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
>>
>> Could you, please, rebase your patches on this patch series.
>>
>> Please, call efi_install_fdt with EFI_FDT_USE_INTERNAL if no device tree
>> is supplied by the FIT image.
>>
>> The patch series is also available in branch efi-2020-04 at
>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
>>
>> Best regards
>>
>> Heinrich
>
> Hello Heinrich,
>
> Thanks for the patch series!
> I will send the updated patches by latest tomorrow EOD.
>
> You also mentioned updating the documentation in
> doc/uefi/u-boot_on_efi.rst and doc/uImage.FIT/howto.txt.
>
> I've checked those documents and their content is quite generic,
> not particularly related to this work. The former describes
> how to build and run u-boot as an EFI application/payload, while
> the later shows how to build and use FIT images.
>
> If you agree, I could instead add a new ITS file in uImage.FIT folder
> and describe there the new functionality.

Having an example in uImage.Fit will be very helpful. But I still think
the capability to start UEFI binaries via FIT images should be mentioned
in the documentation. Just add a section after "Executing a UEFI binary"
in doc/uefi/uefi.rst, e.g.

Launching a UEFI binary from a FIT image
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

A signed FIT image can be used to securely boot a UEFI image via the
bootm command. A sample configuration is provided as file
doc/uImage.FIT/uefi.its. See doc/uImage.FIT/howto.txt for an
introduction to FIT images.

Best regards

Heinrich

>
> Kind regards,
> Cristian
>
>

[PATCH 2/2] bootm: Add a bootm command for type IH_OS_EFI

[Simon Glass]

Hi Cristian,

On Sun, 24 Nov 2019 at 23:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/24/19 9:11 PM, Cristian Ciocaltea wrote:
> > Add support for booting EFI binaries contained in FIT images.
> > A typical usage scenario is chain-loading GRUB2 in a verified
> > boot environment.
> >
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > ---
> >   cmd/Kconfig       |  9 ++++++++-
> >   cmd/bootefi.c     |  2 +-
> >   common/bootm_os.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >   include/bootm.h   |  2 ++
> >   4 files changed, 55 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index cf982ff65e..1bec840f5a 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -263,6 +263,13 @@ config CMD_BOOTI
> >       help
> >         Boot an AArch64 Linux Kernel image from memory.
> >
> > +config BOOTM_EFI
> > +     bool "Support booting EFI OS images"
> > +     depends on CMD_BOOTEFI
> > +     default y
> > +     help
> > +       Support booting EFI images via the bootm command.
> > +
> >   config BOOTM_LINUX
> >       bool "Support booting Linux OS images"
> >       depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI
> > @@ -316,7 +323,7 @@ config CMD_BOOTEFI
> >       depends on EFI_LOADER
> >       default y
> >       help
> > -       Boot an EFI image from memory.
> > +       Boot an EFI binary from memory.
> >
> >   config CMD_BOOTEFI_HELLO_COMPILE
> >       bool "Compile a standard EFI hello world binary for testing"
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index f613cce7e2..f25d639dfe 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -553,7 +553,7 @@ static int do_efi_selftest(void)
> >    * @argv:   command line arguments
> >    * Return:  status code
> >    */
> > -static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >   {
> >       efi_status_t ret;
> >
> > diff --git a/common/bootm_os.c b/common/bootm_os.c
> > index 6fb7d658da..706151913a 100644
> > --- a/common/bootm_os.c
> > +++ b/common/bootm_os.c
> > @@ -462,6 +462,47 @@ static int do_bootm_tee(int flag, int argc, char * const argv[],
> >   }
> >   #endif
> >
> > +#ifdef CONFIG_BOOTM_EFI
> > +static int do_bootm_efi(int flag, int argc, char * const argv[],
> > +                     bootm_headers_t *images)
> > +{
> > +     int ret;
> > +     int local_argc = 2;
> > +     char *local_args[3];
> > +     char str_efi_addr[16];
> > +     char str_fdt_addr[16];
> > +
> > +     if (flag != BOOTM_STATE_OS_GO)
> > +             return 0;
> > +
> > +     /* Locate FDT etc */
> > +     ret = bootm_find_images(flag, argc, argv);
> > +     if (ret)
> > +             return ret;
> > +
> > +     printf("## Transferring control to EFI (at address %08lx) ...\n",
> > +            images->ep);
> > +     bootstage_mark(BOOTSTAGE_ID_RUN_OS);
> > +
> > +     local_args[0] = argv[0];
> > +
> > +     /* Write efi addr into string */
> > +     sprintf(str_efi_addr, "%lx", images->ep);
>
> I think we should avoid transferring arguments as strings. It is
> preferable to separate do_bootefi() into an argument parser and an
> execution part.
>
> Let's CC the developers who have contributed to common/bootm_os.c.

Yes I think it would be good to create a function to hold most of the
code from do_bootefi(), call the function with appropriate parameters
and changing do_bootefi() to call that new function.

>
> Best regards
>
> Heinrich
>
> > +     /* and provide it via the arguments */
> > +     local_args[1] = str_efi_addr;
> > +
> > +     if (images->ft_len) {
> > +             /* Write fdt addr into string */
> > +             sprintf(str_fdt_addr, "%lx", (unsigned long)images->ft_addr);
> > +             /* and provide it via the arguments */
> > +             local_args[2] = str_fdt_addr;
> > +             local_argc = 3;
> > +     }
> > +
> > +     return do_bootefi(NULL, 0, local_argc, local_args);
> > +}
> > +#endif
> > +
> >   static boot_os_fn *boot_os[] = {
> >       [IH_OS_U_BOOT] = do_bootm_standalone,
> >   #ifdef CONFIG_BOOTM_LINUX
> > @@ -498,6 +539,9 @@ static boot_os_fn *boot_os[] = {
> >   #ifdef CONFIG_BOOTM_OPTEE
> >       [IH_OS_TEE] = do_bootm_tee,
> >   #endif
> > +#ifdef CONFIG_BOOTM_EFI
> > +     [IH_OS_EFI] = do_bootm_efi,
> > +#endif
> >   };
> >
> >   /* Allow for arch specific config before we boot */
> > diff --git a/include/bootm.h b/include/bootm.h
> > index edeeacb0df..a0da86dc32 100644
> > --- a/include/bootm.h
> > +++ b/include/bootm.h
> > @@ -37,7 +37,9 @@ typedef int boot_os_fn(int flag, int argc, char * const argv[],
> >   extern boot_os_fn do_bootm_linux;
> >   extern boot_os_fn do_bootm_vxworks;
> >
> > +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);

Please add a function comment.

> >   int do_bootelf(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > +
> >   void lynxkdi_boot(image_header_t *hdr);
> >
> >   boot_os_fn *bootm_os_get_boot_func(int os);
> >
>

Regards,
Simon

[PATCH 2/2] bootm: Add a bootm command for type IH_OS_EFI

[Heinrich Schuchardt]

On 12/27/19 5:41 PM, Simon Glass wrote:
>> I think we should avoid transferring arguments as strings. It is
>> preferable to separate do_bootefi() into an argument parser and an
>> execution part.
>>
>> Let's CC the developers who have contributed to common/bootm_os.c.
> Yes I think it would be good to create a function to hold most of the
> code from do_bootefi(), call the function with appropriate parameters
> and changing do_bootefi() to call that new function.
>

Hello Simon,

you are responding to an outdated version of the patch. Please, see

https://patchwork.ozlabs.org/patch/1215251/
[v4,2/5] bootm: Add a bootm command for type IH_OS_EFI

Best regards

Heinrich