* [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob
@ 2022-04-10 21:05 kevans
2022-04-11 18:14 ` Heinrich Schuchardt
2022-05-19 0:35 ` AKASHI Takahiro
0 siblings, 2 replies; 5+ messages in thread
From: kevans @ 2022-04-10 21:05 UTC (permalink / raw)
To: u-boot; +Cc: Kyle Evans, Heinrich Schuchardt
From: Kyle Evans <kevans@FreeBSD.org>
Up until commit 5f59518a7b1ae ("efi_loader: setting boot device"), we
could boot an arbitrary blob with bootefi. Indeed, efi_run_image() even
has a special case for missing device paths indicating a payload that
was directly loaded via JTAG, for example.
Restore the ability to inject a UEFI payload into memory and `bootefi`
it. If the address passed isn't the last PE-COFF loaded, then we'll
wipe out the pre-existing DP/Image information and let efi_run_image()
synthesize a memory device path.
An image size is required if we're booting an arbitrary payload, and
the FDT argument has been changed to accept `-`. The size could be
deduced from the image header, but it's required anyways as an explicit
acknowledgment that one's trying to boot an arbitrary payload rather
than accidentally using the wrong address in the single-addr form.
If the image address matches the last loaded PE_COFF, it is an error
to pass the incorrect size (but the size may be omitted).
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
---
Changes since v1:
- Specifying an address that isn't the last-known without a size produces the
same error as in the past ("No UEFI binary known at ...")
- Specifying a size with the last known address that doesn't match the last
recorded size will get: Size does not match known UEFI binary at %s
cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++--------
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 53d9f0e0dc..0d3aecb2a1 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -425,7 +425,7 @@ static int do_efibootmgr(void)
* @image_opt: string of image start address
* Return: status code
*/
-static int do_bootefi_image(const char *image_opt)
+static int do_bootefi_image(const char *image_opt, const char *size_opt)
{
void *image_buf;
unsigned long addr, size;
@@ -444,13 +444,35 @@ static int do_bootefi_image(const char *image_opt)
if (!addr)
return CMD_RET_USAGE;
+ size = 0;
+ if (size_opt) {
+ size = strtoul(size_opt, NULL, 16);
+ /* Check that a numeric value was passed */
+ if (!size)
+ return CMD_RET_USAGE;
+ }
+
image_buf = map_sysmem(addr, 0);
if (image_buf != image_addr) {
- log_err("No UEFI binary known at %s\n", image_opt);
- return CMD_RET_FAILURE;
+ /* Fake device path -- we must have a size. */
+
+ if (size == 0) {
+ printf("No UEFI binary known at %s\n", image_opt);
+ return CMD_RET_FAILURE;
+ }
+
+ if (image_addr)
+ efi_clear_bootdev();
+ } else {
+ if (size != 0 && size != image_size) {
+ printf("Size does not match known UEFI binary at %s\n",
+ image_opt);
+ return CMD_RET_FAILURE;
+ }
+
+ size = image_size;
}
- size = image_size;
}
ret = efi_run_image(image_buf, size);
@@ -654,7 +676,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
return CMD_RET_FAILURE;
}
- if (argc > 2) {
+ if (argc > 2 && strcmp(argv[2], "-") != 0) {
uintptr_t fdt_addr;
fdt_addr = hextoul(argv[2], NULL);
@@ -677,15 +699,19 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
return do_efi_selftest();
#endif
- return do_bootefi_image(argv[1]);
+ return do_bootefi_image(argv[1], argc > 3 ? argv[3] : NULL);
}
#ifdef CONFIG_SYS_LONGHELP
static char bootefi_help_text[] =
- "<image address> [fdt address]\n"
+ "<image address> [fdt address [image size]]\n"
" - boot EFI payload stored at address <image address>.\n"
" If specified, the device tree located at <fdt address> gets\n"
" exposed as EFI configuration table.\n"
+ " The image size is required if this is not a preloaded image, but\n"
+ " it must be omitted if the image was preloaded.\n"
+ " The <fdt address> parameter accepts '-' to indicate FDT already\n"
+ " installed or pre-specified in fdt_addr or fdtcontroladdr.\n"
#ifdef CONFIG_CMD_BOOTEFI_HELLO
"bootefi hello\n"
" - boot a sample Hello World application stored within U-Boot\n"
@@ -707,7 +733,7 @@ static char bootefi_help_text[] =
#endif
U_BOOT_CMD(
- bootefi, 3, 0, do_bootefi,
+ bootefi, 4, 0, do_bootefi,
"Boots an EFI payload from memory",
bootefi_help_text
);
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob
2022-04-10 21:05 [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob kevans
@ 2022-04-11 18:14 ` Heinrich Schuchardt
2022-04-11 19:19 ` Kyle Evans
2022-05-19 0:35 ` AKASHI Takahiro
1 sibling, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2022-04-11 18:14 UTC (permalink / raw)
To: kevans; +Cc: Heinrich Schuchardt, u-boot
On 4/10/22 23:05, kevans@FreeBSD.org wrote:
> From: Kyle Evans <kevans@FreeBSD.org>
>
> Up until commit 5f59518a7b1ae ("efi_loader: setting boot device"), we
> could boot an arbitrary blob with bootefi. Indeed, efi_run_image() even
> has a special case for missing device paths indicating a payload that
> was directly loaded via JTAG, for example.
>
> Restore the ability to inject a UEFI payload into memory and `bootefi`
> it. If the address passed isn't the last PE-COFF loaded, then we'll
> wipe out the pre-existing DP/Image information and let efi_run_image()
> synthesize a memory device path.
>
> An image size is required if we're booting an arbitrary payload, and
> the FDT argument has been changed to accept `-`. The size could be
> deduced from the image header, but it's required anyways as an explicit
> acknowledgment that one's trying to boot an arbitrary payload rather
> than accidentally using the wrong address in the single-addr form.
>
> If the image address matches the last loaded PE_COFF, it is an error
> to pass the incorrect size (but the size may be omitted).
>
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> ---
>
> Changes since v1:
> - Specifying an address that isn't the last-known without a size produces the
> same error as in the past ("No UEFI binary known at ...")
> - Specifying a size with the last known address that doesn't match the last
> recorded size will get: Size does not match known UEFI binary at %s
>
> cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 53d9f0e0dc..0d3aecb2a1 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -425,7 +425,7 @@ static int do_efibootmgr(void)
> * @image_opt: string of image start address
> * Return: status code
> */
> -static int do_bootefi_image(const char *image_opt)
> +static int do_bootefi_image(const char *image_opt, const char *size_opt)
> {
> void *image_buf;
> unsigned long addr, size;
> @@ -444,13 +444,35 @@ static int do_bootefi_image(const char *image_opt)
> if (!addr)
> return CMD_RET_USAGE;
>
> + size = 0;
> + if (size_opt) {
> + size = strtoul(size_opt, NULL, 16);
> + /* Check that a numeric value was passed */
> + if (!size)
> + return CMD_RET_USAGE;
> + }
> +
> image_buf = map_sysmem(addr, 0);
>
> if (image_buf != image_addr) {
> - log_err("No UEFI binary known at %s\n", image_opt);
> - return CMD_RET_FAILURE;
> + /* Fake device path -- we must have a size. */
> +
> + if (size == 0) {
U-Boot prefers (!size) over (size == 0).
> + printf("No UEFI binary known at %s\n", image_opt);
We should use logerr() here.
> + return CMD_RET_FAILURE;
> + }
> +
> + if (image_addr)
> + efi_clear_bootdev();
> + } else {
> + if (size != 0 && size != image_size) {
> + printf("Size does not match known UEFI binary at %s\n",
> + image_opt);
ditto.
> + return CMD_RET_FAILURE;
> + }
> +
> + size = image_size;
> }
> - size = image_size;
> }
> ret = efi_run_image(image_buf, size);
>
> @@ -654,7 +676,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> return CMD_RET_FAILURE;
> }
>
> - if (argc > 2) {
> + if (argc > 2 && strcmp(argv[2], "-") != 0) {
> uintptr_t fdt_addr;
>
> fdt_addr = hextoul(argv[2], NULL);
> @@ -677,15 +699,19 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> return do_efi_selftest();
> #endif
>
> - return do_bootefi_image(argv[1]);
> + return do_bootefi_image(argv[1], argc > 3 ? argv[3] : NULL);
> }
>
> #ifdef CONFIG_SYS_LONGHELP
> static char bootefi_help_text[] =
> - "<image address> [fdt address]\n"
> + "<image address> [fdt address [image size]]\n"
> " - boot EFI payload stored at address <image address>.\n"
> " If specified, the device tree located at <fdt address> gets\n"
> " exposed as EFI configuration table.\n"
> + " The image size is required if this is not a preloaded image, but\n"
> + " it must be omitted if the image was preloaded.\n"
It may be omitted. See code above.
I can take care of the necessary changes.
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> + " The <fdt address> parameter accepts '-' to indicate FDT already\n"
> + " installed or pre-specified in fdt_addr or fdtcontroladdr.\n"
> #ifdef CONFIG_CMD_BOOTEFI_HELLO
> "bootefi hello\n"
> " - boot a sample Hello World application stored within U-Boot\n"
> @@ -707,7 +733,7 @@ static char bootefi_help_text[] =
> #endif
>
> U_BOOT_CMD(
> - bootefi, 3, 0, do_bootefi,
> + bootefi, 4, 0, do_bootefi,
> "Boots an EFI payload from memory",
> bootefi_help_text
> );
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob
2022-04-11 18:14 ` Heinrich Schuchardt
@ 2022-04-11 19:19 ` Kyle Evans
0 siblings, 0 replies; 5+ messages in thread
From: Kyle Evans @ 2022-04-11 19:19 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot
On Mon, Apr 11, 2022 at 1:14 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 4/10/22 23:05, kevans@FreeBSD.org wrote:
> > From: Kyle Evans <kevans@FreeBSD.org>
> >
> > Up until commit 5f59518a7b1ae ("efi_loader: setting boot device"), we
> > could boot an arbitrary blob with bootefi. Indeed, efi_run_image() even
> > has a special case for missing device paths indicating a payload that
> > was directly loaded via JTAG, for example.
> >
> > Restore the ability to inject a UEFI payload into memory and `bootefi`
> > it. If the address passed isn't the last PE-COFF loaded, then we'll
> > wipe out the pre-existing DP/Image information and let efi_run_image()
> > synthesize a memory device path.
> >
> > An image size is required if we're booting an arbitrary payload, and
> > the FDT argument has been changed to accept `-`. The size could be
> > deduced from the image header, but it's required anyways as an explicit
> > acknowledgment that one's trying to boot an arbitrary payload rather
> > than accidentally using the wrong address in the single-addr form.
> >
> > If the image address matches the last loaded PE_COFF, it is an error
> > to pass the incorrect size (but the size may be omitted).
> >
> > Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> > ---
> >
> > Changes since v1:
> > - Specifying an address that isn't the last-known without a size produces the
> > same error as in the past ("No UEFI binary known at ...")
> > - Specifying a size with the last known address that doesn't match the last
> > recorded size will get: Size does not match known UEFI binary at %s
> >
> > cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 34 insertions(+), 8 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 53d9f0e0dc..0d3aecb2a1 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -425,7 +425,7 @@ static int do_efibootmgr(void)
> > * @image_opt: string of image start address
> > * Return: status code
> > */
> > -static int do_bootefi_image(const char *image_opt)
> > +static int do_bootefi_image(const char *image_opt, const char *size_opt)
> > {
> > void *image_buf;
> > unsigned long addr, size;
> > @@ -444,13 +444,35 @@ static int do_bootefi_image(const char *image_opt)
> > if (!addr)
> > return CMD_RET_USAGE;
> >
> > + size = 0;
> > + if (size_opt) {
> > + size = strtoul(size_opt, NULL, 16);
> > + /* Check that a numeric value was passed */
> > + if (!size)
> > + return CMD_RET_USAGE;
> > + }
> > +
> > image_buf = map_sysmem(addr, 0);
> >
> > if (image_buf != image_addr) {
> > - log_err("No UEFI binary known at %s\n", image_opt);
> > - return CMD_RET_FAILURE;
> > + /* Fake device path -- we must have a size. */
> > +
> > + if (size == 0) {
>
> U-Boot prefers (!size) over (size == 0).
>
Ah, interesting, thanks for the note! I recall seeing a warning about
this from patman about pointers/NULL, but hadn't thought to apply the
same here.
> > + printf("No UEFI binary known at %s\n", image_opt);
>
>
> We should use logerr() here.
>
My apologies, I missed that the message I was restoring used the
correct mechanism.
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + if (image_addr)
> > + efi_clear_bootdev();
> > + } else {
> > + if (size != 0 && size != image_size) {
> > + printf("Size does not match known UEFI binary at %s\n",
> > + image_opt);
>
> ditto.
>
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + size = image_size;
> > }
> > - size = image_size;
> > }
> > ret = efi_run_image(image_buf, size);
> >
> > @@ -654,7 +676,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > return CMD_RET_FAILURE;
> > }
> >
> > - if (argc > 2) {
> > + if (argc > 2 && strcmp(argv[2], "-") != 0) {
> > uintptr_t fdt_addr;
> >
> > fdt_addr = hextoul(argv[2], NULL);
> > @@ -677,15 +699,19 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > return do_efi_selftest();
> > #endif
> >
> > - return do_bootefi_image(argv[1]);
> > + return do_bootefi_image(argv[1], argc > 3 ? argv[3] : NULL);
> > }
> >
> > #ifdef CONFIG_SYS_LONGHELP
> > static char bootefi_help_text[] =
> > - "<image address> [fdt address]\n"
> > + "<image address> [fdt address [image size]]\n"
> > " - boot EFI payload stored at address <image address>.\n"
> > " If specified, the device tree located at <fdt address> gets\n"
> > " exposed as EFI configuration table.\n"
> > + " The image size is required if this is not a preloaded image, but\n"
> > + " it must be omitted if the image was preloaded.\n"
>
> It may be omitted. See code above.
>
> I can take care of the necessary changes.
>
> Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob
2022-04-10 21:05 [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob kevans
2022-04-11 18:14 ` Heinrich Schuchardt
@ 2022-05-19 0:35 ` AKASHI Takahiro
2022-05-19 11:09 ` Heinrich Schuchardt
1 sibling, 1 reply; 5+ messages in thread
From: AKASHI Takahiro @ 2022-05-19 0:35 UTC (permalink / raw)
To: kevans, Heinrich Schuchardt; +Cc: u-boot
I thought I had sent out my comment when this patch was submitted first time,
but it seems that it has never reached out to ML.
On Sun, Apr 10, 2022 at 04:05:55PM -0500, kevans@FreeBSD.org wrote:
> From: Kyle Evans <kevans@FreeBSD.org>
>
> Up until commit 5f59518a7b1ae ("efi_loader: setting boot device"), we
> could boot an arbitrary blob with bootefi. Indeed, efi_run_image() even
> has a special case for missing device paths indicating a payload that
> was directly loaded via JTAG, for example.
>
> Restore the ability to inject a UEFI payload into memory and `bootefi`
> it. If the address passed isn't the last PE-COFF loaded, then we'll
> wipe out the pre-existing DP/Image information and let efi_run_image()
> synthesize a memory device path.
>
> An image size is required if we're booting an arbitrary payload, and
> the FDT argument has been changed to accept `-`. The size could be
> deduced from the image header, but it's required anyways as an explicit
> acknowledgment that one's trying to boot an arbitrary payload rather
> than accidentally using the wrong address in the single-addr form.
The syntax seems to be odd and I don't think it is user-friendly
"<image address> [fdt address [image size]]\n"
as an image-related parameter,<image size>, doesn't follow <address>
immediately but comes *after* <fdt address>.
Instead, I would suggest
bootefi <image addr>[:<image size>] [<fdt addr>]
(or [<image size>@]<image addr>)
I believe that "<addr>:<size>" notation is quite intuitive and non-misleading.
Moreover, a similar syntax can be seen, for example, in booti command.
-Takahiro Akashi
> If the image address matches the last loaded PE_COFF, it is an error
> to pass the incorrect size (but the size may be omitted).
>
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> ---
>
> Changes since v1:
> - Specifying an address that isn't the last-known without a size produces the
> same error as in the past ("No UEFI binary known at ...")
> - Specifying a size with the last known address that doesn't match the last
> recorded size will get: Size does not match known UEFI binary at %s
>
> cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 53d9f0e0dc..0d3aecb2a1 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -425,7 +425,7 @@ static int do_efibootmgr(void)
> * @image_opt: string of image start address
> * Return: status code
> */
> -static int do_bootefi_image(const char *image_opt)
> +static int do_bootefi_image(const char *image_opt, const char *size_opt)
> {
> void *image_buf;
> unsigned long addr, size;
> @@ -444,13 +444,35 @@ static int do_bootefi_image(const char *image_opt)
> if (!addr)
> return CMD_RET_USAGE;
>
> + size = 0;
> + if (size_opt) {
> + size = strtoul(size_opt, NULL, 16);
> + /* Check that a numeric value was passed */
> + if (!size)
> + return CMD_RET_USAGE;
> + }
> +
> image_buf = map_sysmem(addr, 0);
>
> if (image_buf != image_addr) {
> - log_err("No UEFI binary known at %s\n", image_opt);
> - return CMD_RET_FAILURE;
> + /* Fake device path -- we must have a size. */
> +
> + if (size == 0) {
> + printf("No UEFI binary known at %s\n", image_opt);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (image_addr)
> + efi_clear_bootdev();
> + } else {
> + if (size != 0 && size != image_size) {
> + printf("Size does not match known UEFI binary at %s\n",
> + image_opt);
> + return CMD_RET_FAILURE;
> + }
> +
> + size = image_size;
> }
> - size = image_size;
> }
> ret = efi_run_image(image_buf, size);
>
> @@ -654,7 +676,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> return CMD_RET_FAILURE;
> }
>
> - if (argc > 2) {
> + if (argc > 2 && strcmp(argv[2], "-") != 0) {
> uintptr_t fdt_addr;
>
> fdt_addr = hextoul(argv[2], NULL);
> @@ -677,15 +699,19 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> return do_efi_selftest();
> #endif
>
> - return do_bootefi_image(argv[1]);
> + return do_bootefi_image(argv[1], argc > 3 ? argv[3] : NULL);
> }
>
> #ifdef CONFIG_SYS_LONGHELP
> static char bootefi_help_text[] =
> - "<image address> [fdt address]\n"
> + "<image address> [fdt address [image size]]\n"
> " - boot EFI payload stored at address <image address>.\n"
> " If specified, the device tree located at <fdt address> gets\n"
> " exposed as EFI configuration table.\n"
> + " The image size is required if this is not a preloaded image, but\n"
> + " it must be omitted if the image was preloaded.\n"
> + " The <fdt address> parameter accepts '-' to indicate FDT already\n"
> + " installed or pre-specified in fdt_addr or fdtcontroladdr.\n"
> #ifdef CONFIG_CMD_BOOTEFI_HELLO
> "bootefi hello\n"
> " - boot a sample Hello World application stored within U-Boot\n"
> @@ -707,7 +733,7 @@ static char bootefi_help_text[] =
> #endif
>
> U_BOOT_CMD(
> - bootefi, 3, 0, do_bootefi,
> + bootefi, 4, 0, do_bootefi,
> "Boots an EFI payload from memory",
> bootefi_help_text
> );
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob
2022-05-19 0:35 ` AKASHI Takahiro
@ 2022-05-19 11:09 ` Heinrich Schuchardt
0 siblings, 0 replies; 5+ messages in thread
From: Heinrich Schuchardt @ 2022-05-19 11:09 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: u-boot, kevans
On 5/19/22 02:35, AKASHI Takahiro wrote:
> I thought I had sent out my comment when this patch was submitted first time,
> but it seems that it has never reached out to ML.
>
> On Sun, Apr 10, 2022 at 04:05:55PM -0500, kevans@FreeBSD.org wrote:
>> From: Kyle Evans <kevans@FreeBSD.org>
>>
>> Up until commit 5f59518a7b1ae ("efi_loader: setting boot device"), we
>> could boot an arbitrary blob with bootefi. Indeed, efi_run_image() even
>> has a special case for missing device paths indicating a payload that
>> was directly loaded via JTAG, for example.
>>
>> Restore the ability to inject a UEFI payload into memory and `bootefi`
>> it. If the address passed isn't the last PE-COFF loaded, then we'll
>> wipe out the pre-existing DP/Image information and let efi_run_image()
>> synthesize a memory device path.
>>
>> An image size is required if we're booting an arbitrary payload, and
>> the FDT argument has been changed to accept `-`. The size could be
>> deduced from the image header, but it's required anyways as an explicit
>> acknowledgment that one's trying to boot an arbitrary payload rather
>> than accidentally using the wrong address in the single-addr form.
>
> The syntax seems to be odd and I don't think it is user-friendly
> "<image address> [fdt address [image size]]\n"
> as an image-related parameter,<image size>, doesn't follow <address>
> immediately but comes *after* <fdt address>.
>
> Instead, I would suggest
> bootefi <image addr>[:<image size>] [<fdt addr>]
> (or [<image size>@]<image addr>)
>
> I believe that "<addr>:<size>" notation is quite intuitive and non-misleading.
> Moreover, a similar syntax can be seen, for example, in booti command.
I will create a patch for this.
Best regards
Heinrich
>
> -Takahiro Akashi
>
>> If the image address matches the last loaded PE_COFF, it is an error
>> to pass the incorrect size (but the size may be omitted).
>>
>> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
>> ---
>>
>> Changes since v1:
>> - Specifying an address that isn't the last-known without a size produces the
>> same error as in the past ("No UEFI binary known at ...")
>> - Specifying a size with the last known address that doesn't match the last
>> recorded size will get: Size does not match known UEFI binary at %s
>>
>> cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 53d9f0e0dc..0d3aecb2a1 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -425,7 +425,7 @@ static int do_efibootmgr(void)
>> * @image_opt: string of image start address
>> * Return: status code
>> */
>> -static int do_bootefi_image(const char *image_opt)
>> +static int do_bootefi_image(const char *image_opt, const char *size_opt)
>> {
>> void *image_buf;
>> unsigned long addr, size;
>> @@ -444,13 +444,35 @@ static int do_bootefi_image(const char *image_opt)
>> if (!addr)
>> return CMD_RET_USAGE;
>>
>> + size = 0;
>> + if (size_opt) {
>> + size = strtoul(size_opt, NULL, 16);
>> + /* Check that a numeric value was passed */
>> + if (!size)
>> + return CMD_RET_USAGE;
>> + }
>> +
>> image_buf = map_sysmem(addr, 0);
>>
>> if (image_buf != image_addr) {
>> - log_err("No UEFI binary known at %s\n", image_opt);
>> - return CMD_RET_FAILURE;
>> + /* Fake device path -- we must have a size. */
>> +
>> + if (size == 0) {
>> + printf("No UEFI binary known at %s\n", image_opt);
>> + return CMD_RET_FAILURE;
>> + }
>> +
>> + if (image_addr)
>> + efi_clear_bootdev();
>> + } else {
>> + if (size != 0 && size != image_size) {
>> + printf("Size does not match known UEFI binary at %s\n",
>> + image_opt);
>> + return CMD_RET_FAILURE;
>> + }
>> +
>> + size = image_size;
>> }
>> - size = image_size;
>> }
>> ret = efi_run_image(image_buf, size);
>>
>> @@ -654,7 +676,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>> return CMD_RET_FAILURE;
>> }
>>
>> - if (argc > 2) {
>> + if (argc > 2 && strcmp(argv[2], "-") != 0) {
>> uintptr_t fdt_addr;
>>
>> fdt_addr = hextoul(argv[2], NULL);
>> @@ -677,15 +699,19 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>> return do_efi_selftest();
>> #endif
>>
>> - return do_bootefi_image(argv[1]);
>> + return do_bootefi_image(argv[1], argc > 3 ? argv[3] : NULL);
>> }
>>
>> #ifdef CONFIG_SYS_LONGHELP
>> static char bootefi_help_text[] =
>> - "<image address> [fdt address]\n"
>> + "<image address> [fdt address [image size]]\n"
>> " - boot EFI payload stored at address <image address>.\n"
>> " If specified, the device tree located at <fdt address> gets\n"
>> " exposed as EFI configuration table.\n"
>> + " The image size is required if this is not a preloaded image, but\n"
>> + " it must be omitted if the image was preloaded.\n"
>> + " The <fdt address> parameter accepts '-' to indicate FDT already\n"
>> + " installed or pre-specified in fdt_addr or fdtcontroladdr.\n"
>> #ifdef CONFIG_CMD_BOOTEFI_HELLO
>> "bootefi hello\n"
>> " - boot a sample Hello World application stored within U-Boot\n"
>> @@ -707,7 +733,7 @@ static char bootefi_help_text[] =
>> #endif
>>
>> U_BOOT_CMD(
>> - bootefi, 3, 0, do_bootefi,
>> + bootefi, 4, 0, do_bootefi,
>> "Boots an EFI payload from memory",
>> bootefi_help_text
>> );
>> --
>> 2.33.0
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-19 11:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-10 21:05 [PATCH v2] cmd: bootefi: restore ability to boot arbitrary blob kevans
2022-04-11 18:14 ` Heinrich Schuchardt
2022-04-11 19:19 ` Kyle Evans
2022-05-19 0:35 ` AKASHI Takahiro
2022-05-19 11:09 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox