[PATCH 0/3] efi_loader: setting boot device

older
[ANN] U-Boot v2022.04 released

Heinrich Schuchardt

12 Jan 2021 12 Jan '21

8:58 p.m.

Up to now the bootefi command uses the last file loaded to determine the boot partition. This has led to user irritation when the fdt has been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image.

The first patch carves out the test function. The second make use of the PE-COFF check. The third add the display of the boot device and file path for easier testing.

Heinrich Schuchardt (3): efi_loader: print boot device and file path in helloworld efi_loader: carve out efi_check_pe() efi_loader: setting boot device

-- 2.29.2

Show replies by date

Heinrich Schuchardt

12 Jan 12 Jan

8:58 p.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

Let helloworld.efi print the device path of the boot device and the file path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/helloworld.c | 167 +++++++++++++++++++++++++++++------- 1 file changed, 137 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 9ae2ee3389..5c8b7a96f9 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -1,43 +1,41 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * EFI hello world + * Hello world EFI application * - * Copyright (c) 2016 Google, Inc - * Written by Simon Glass sjg@chromium.org + * Copyright 2020, Heinrich Schuchardt xypron.glpk@gmx.de * - * This program demonstrates calling a boottime service. - * It writes a greeting and the load options to the console. + * This test program is used to test the invocation of an EFI application. + * It writes + * + * * a greeting + * * the firmware's UEFI version + * * the installed configuration tables + * * the boot device's device path and the file path + * + * to the console. */

-#include <common.h> #include <efi_api.h>

static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID; +static const efi_guid_t device_path_to_text_protocol_guid = + EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; +static const efi_guid_t device_path_guid = EFI_DEVICE_PATH_PROTOCOL_GUID; static const efi_guid_t fdt_guid = EFI_FDT_GUID; static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;

+static struct efi_system_table *systable; +static struct efi_boot_services *boottime; +static struct efi_simple_text_output_protocol *con_out; + /** - * efi_main() - entry point of the EFI application. - * - * @handle: handle of the loaded image - * @systable: system table - * @return: status code + * print_uefi_revision() - print UEFI revision number */ -efi_status_t EFIAPI efi_main(efi_handle_t handle, - struct efi_system_table *systable) +static void print_uefi_revision(void) { - struct efi_simple_text_output_protocol *con_out = systable->con_out; - struct efi_boot_services *boottime = systable->boottime; - struct efi_loaded_image *loaded_image; - efi_status_t ret; - efi_uintn_t i; u16 rev[] = L"0.0.0";

- /* UEFI requires CR LF */ - con_out->output_string(con_out, L"Hello, world!\r\n"); - - /* Print the revision number */ rev[0] = (systable->hdr.revision >> 16) + '0'; rev[4] = systable->hdr.revision & 0xffff; for (; rev[4] >= 10;) { @@ -53,15 +51,15 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string(con_out, L"Running on UEFI "); con_out->output_string(con_out, rev); con_out->output_string(con_out, L"\r\n"); +} + +/** + * print_config_tables() - print configuration tables + */ +static void print_config_tables(void) +{ + efi_uintn_t i;

- /* Get the loaded image protocol */ - ret = boottime->handle_protocol(handle, &loaded_image_guid, - (void **)&loaded_image); - if (ret != EFI_SUCCESS) { - con_out->output_string - (con_out, L"Cannot open loaded image protocol\r\n"); - goto out; - } /* Find configuration tables */ for (i = 0; i < systable->nr_tables; ++i) { if (!memcmp(&systable->tables[i].guid, &fdt_guid, @@ -77,6 +75,16 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string (con_out, L"Have SMBIOS table\r\n"); } +} + +/** + * print_load_options() - print load options + * + * @systable: system table + * @con_out: simple text output protocol + */ +void print_load_options(struct efi_loaded_image *loaded_image) +{ /* Output the load options */ con_out->output_string(con_out, L"Load options: "); if (loaded_image->load_options_size && loaded_image->load_options) @@ -85,6 +93,105 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, else con_out->output_string(con_out, L"<none>"); con_out->output_string(con_out, L"\r\n"); +} + +/** + * print_device_path() - print device path + * + * @device_path: device path to print + * @dp2txt: device path to text protocol + */ +efi_status_t print_device_path(struct efi_device_path *device_path, + struct efi_device_path_to_text_protocol *dp2txt) +{ + u16 *string; + efi_status_t ret; + + if (!device_path) { + con_out->output_string(con_out, L"<none>\r\n"); + return EFI_SUCCESS; + } + + string = dp2txt->convert_device_path_to_text(device_path, true, false); + if (!string) { + con_out->output_string + (con_out, L"Cannot convert device path to text\r\n"); + return EFI_OUT_OF_RESOURCES; + } + con_out->output_string(con_out, string); + con_out->output_string(con_out, L"\r\n"); + ret = boottime->free_pool(string); + if (ret != EFI_SUCCESS) { + con_out->output_string(con_out, L"Cannot free pool memory\r\n"); + return ret; + } + return EFI_SUCCESS; +} + +/** + * efi_main() - entry point of the EFI application. + * + * @handle: handle of the loaded image + * @systab: system table + * @return: status code + */ +efi_status_t EFIAPI efi_main(efi_handle_t handle, + struct efi_system_table *systab) +{ + struct efi_loaded_image *loaded_image; + struct efi_device_path_to_text_protocol *device_path_to_text; + struct efi_device_path *device_path; + efi_status_t ret; + + systable = systab; + boottime = systable->boottime; + con_out = systable->con_out; + + /* UEFI requires CR LF */ + con_out->output_string(con_out, L"Hello, world!\r\n"); + + print_uefi_revision(); + print_config_tables(); + + /* Get the loaded image protocol */ + ret = boottime->handle_protocol(handle, &loaded_image_guid, + (void **)&loaded_image); + if (ret != EFI_SUCCESS) { + con_out->output_string + (con_out, L"Cannot open loaded image protocol\r\n"); + goto out; + } + print_load_options(loaded_image); + + /* Get the device path to text protocol */ + ret = boottime->locate_protocol(&device_path_to_text_protocol_guid, + NULL, (void **)&device_path_to_text); + if (ret != EFI_SUCCESS) { + con_out->output_string + (con_out, L"Cannot open device path to text protocol\r\n"); + goto out; + } + if (!loaded_image->device_handle) { + con_out->output_string + (con_out, L"Missing device handle\r\n"); + goto out; + } + ret = boottime->handle_protocol(loaded_image->device_handle, + &device_path_guid, + (void **)&device_path); + if (ret != EFI_SUCCESS) { + con_out->output_string + (con_out, L"Missing devide path for device handle\r\n"); + goto out; + } + con_out->output_string(con_out, L"Boot device: "); + ret = print_device_path(device_path, device_path_to_text); + if (ret != EFI_SUCCESS) + goto out; + con_out->output_string(con_out, L"File path: "); + ret = print_device_path(loaded_image->file_path, device_path_to_text); + if (ret != EFI_SUCCESS) + goto out;

out: boottime->exit(handle, ret, 0, NULL); -- 2.29.2

Simon Glass

14 Jan 14 Jan

4:42 p.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Let helloworld.efi print the device path of the boot device and the file path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167 +++++++++++++++++++++++++++++------- 1 file changed, 137 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

AKASHI Takahiro

15 Jan 15 Jan

2:56 a.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

Heinrich,

On Tue, Jan 12, 2021 at 08:58:40PM +0100, Heinrich Schuchardt wrote:

...

Let helloworld.efi print the device path of the boot device and the file path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167 +++++++++++++++++++++++++++++------- 1 file changed, 137 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 9ae2ee3389..5c8b7a96f9 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -1,43 +1,41 @@ // SPDX-License-Identifier: GPL-2.0+ /*

EFI hello world

Hello world EFI application

Copyright (c) 2016 Google, Inc

Written by Simon Glass sjg@chromium.org

Copyright 2020, Heinrich Schuchardt xypron.glpk@gmx.de

This program demonstrates calling a boottime service.

It writes a greeting and the load options to the console.

This test program is used to test the invocation of an EFI application.

It writes

a greeting

the firmware's UEFI version

the installed configuration tables

the boot device's device path and the file path

If this kind of information is quite useful for users, why not add that (printing) feature as an option of bootefi (or efidebug)? I'm afraid that most users who are irritated as you said won't be able to imagine such information be printed by helloworld app.

-Takahiro Akashi

...

to the console.

*/

-#include <common.h> #include <efi_api.h>

static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID; +static const efi_guid_t device_path_to_text_protocol_guid =

EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;

+static const efi_guid_t device_path_guid = EFI_DEVICE_PATH_PROTOCOL_GUID; static const efi_guid_t fdt_guid = EFI_FDT_GUID; static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;

+static struct efi_system_table *systable; +static struct efi_boot_services *boottime; +static struct efi_simple_text_output_protocol *con_out;

/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systable: system table

@return: status code

print_uefi_revision() - print UEFI revision number

*/

-efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systable)
+static void print_uefi_revision(void) {

struct efi_simple_text_output_protocol *con_out = systable->con_out;

struct efi_boot_services *boottime = systable->boottime;

struct efi_loaded_image *loaded_image;

efi_status_t ret;

efi_uintn_t i; u16 rev[] = L"0.0.0";

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

/* Print the revision number */ rev[0] = (systable->hdr.revision >> 16) + '0'; rev[4] = systable->hdr.revision & 0xffff; for (; rev[4] >= 10;) {

@@ -53,15 +51,15 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string(con_out, L"Running on UEFI "); con_out->output_string(con_out, rev); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_config_tables() - print configuration tables

*/

+static void print_config_tables(void) +{

efi_uintn_t i;
/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
} /* Find configuration tables */ for (i = 0; i < systable->nr_tables; ++i) { if (!memcmp(&systable->tables[i].guid, &fdt_guid,
@@ -77,6 +75,16 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string (con_out, L"Have SMBIOS table\r\n"); } +}

+/**

print_load_options() - print load options

@systable: system table

@con_out: simple text output protocol

*/

+void print_load_options(struct efi_loaded_image *loaded_image) +{ /* Output the load options */ con_out->output_string(con_out, L"Load options: "); if (loaded_image->load_options_size && loaded_image->load_options) @@ -85,6 +93,105 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, else con_out->output_string(con_out, L"<none>"); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_device_path() - print device path

@device_path: device path to print

@dp2txt: device path to text protocol

*/

+efi_status_t print_device_path(struct efi_device_path *device_path,
	       struct efi_device_path_to_text_protocol *dp2txt)
+{
u16 *string;

efi_status_t ret;

if (!device_path) {
con_out->output_string(con_out, L"<none>\r\n");
return EFI_SUCCESS;
}

string = dp2txt->convert_device_path_to_text(device_path, true, false);

if (!string) {
con_out->output_string
	(con_out, L"Cannot convert device path to text\r\n");
return EFI_OUT_OF_RESOURCES;
}

con_out->output_string(con_out, string);

con_out->output_string(con_out, L"\r\n");

ret = boottime->free_pool(string);

if (ret != EFI_SUCCESS) {
con_out->output_string(con_out, L"Cannot free pool memory\r\n");
return ret;
}

return EFI_SUCCESS;
+}

+/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systab: system table

@return: status code

*/

+efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systab)
+{
struct efi_loaded_image *loaded_image;

struct efi_device_path_to_text_protocol *device_path_to_text;

struct efi_device_path *device_path;

efi_status_t ret;

systable = systab;

boottime = systable->boottime;

con_out = systable->con_out;

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

print_uefi_revision();

print_config_tables();

/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
}

print_load_options(loaded_image);

/* Get the device path to text protocol */

ret = boottime->locate_protocol(&device_path_to_text_protocol_guid,
			NULL, (void **)&device_path_to_text);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open device path to text protocol\r\n");
goto out;
}

if (!loaded_image->device_handle) {
con_out->output_string
	(con_out, L"Missing device handle\r\n");
goto out;
}

ret = boottime->handle_protocol(loaded_image->device_handle,
			&device_path_guid,
			(void **)&device_path);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Missing devide path for device handle\r\n");
goto out;
}

con_out->output_string(con_out, L"Boot device: ");

ret = print_device_path(device_path, device_path_to_text);

if (ret != EFI_SUCCESS)
goto out;
con_out->output_string(con_out, L"File path: ");

ret = print_device_path(loaded_image->file_path, device_path_to_text);

if (ret != EFI_SUCCESS)
goto out;
out: boottime->exit(handle, ret, 0, NULL); -- 2.29.2

Heinrich Schuchardt

4:12 a.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:

...

Heinrich,

On Tue, Jan 12, 2021 at 08:58:40PM +0100, Heinrich Schuchardt wrote:

...
Let helloworld.efi print the device path of the boot device and the

file

...
path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167

+++++++++++++++++++++++++++++-------

...
1 file changed, 137 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/helloworld.c

b/lib/efi_loader/helloworld.c

...
index 9ae2ee3389..5c8b7a96f9 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -1,43 +1,41 @@ // SPDX-License-Identifier: GPL-2.0+ /*

EFI hello world

Hello world EFI application

Copyright (c) 2016 Google, Inc

Written by Simon Glass sjg@chromium.org

Copyright 2020, Heinrich Schuchardt xypron.glpk@gmx.de

This program demonstrates calling a boottime service.

It writes a greeting and the load options to the console.

This test program is used to test the invocation of an EFI

application.

...

It writes

a greeting

the firmware's UEFI version

the installed configuration tables

the boot device's device path and the file path

If this kind of information is quite useful for users, why not add that (printing) feature as an option of bootefi (or efidebug)? I'm afraid that most users who are irritated as you said won't be able to imagine such information be printed by helloworld app.

The file path is written in

https://github.com/trini/u-boot/blob/master/cmd/bootefi.c#L471

Device paths are not really user friendly. So I would not like to write it there.

Best regards

Heinrich

...

-Takahiro Akashi

...

to the console.

*/

-#include <common.h> #include <efi_api.h>

static const efi_guid_t loaded_image_guid =

EFI_LOADED_IMAGE_PROTOCOL_GUID;

...
+static const efi_guid_t device_path_to_text_protocol_guid =

EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;

+static const efi_guid_t device_path_guid =

EFI_DEVICE_PATH_PROTOCOL_GUID;

...
static const efi_guid_t fdt_guid = EFI_FDT_GUID; static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;

+static struct efi_system_table *systable; +static struct efi_boot_services *boottime; +static struct efi_simple_text_output_protocol *con_out;

/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systable: system table

@return: status code

print_uefi_revision() - print UEFI revision number

*/

-efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systable)
+static void print_uefi_revision(void) {

struct efi_simple_text_output_protocol *con_out =
systable->con_out;

...
struct efi_boot_services *boottime = systable->boottime;

struct efi_loaded_image *loaded_image;

efi_status_t ret;

efi_uintn_t i; u16 rev[] = L"0.0.0";

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

/* Print the revision number */ rev[0] = (systable->hdr.revision >> 16) + '0'; rev[4] = systable->hdr.revision & 0xffff; for (; rev[4] >= 10;) {

@@ -53,15 +51,15 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string(con_out, L"Running on UEFI "); con_out->output_string(con_out, rev); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_config_tables() - print configuration tables

*/

+static void print_config_tables(void) +{

efi_uintn_t i;
/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
} /* Find configuration tables */ for (i = 0; i < systable->nr_tables; ++i) { if (!memcmp(&systable->tables[i].guid, &fdt_guid,
@@ -77,6 +75,16 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string (con_out, L"Have SMBIOS table\r\n"); } +}

+/**

print_load_options() - print load options

@systable: system table

@con_out: simple text output protocol

*/

+void print_load_options(struct efi_loaded_image *loaded_image) +{ /* Output the load options */ con_out->output_string(con_out, L"Load options: "); if (loaded_image->load_options_size && loaded_image->load_options) @@ -85,6 +93,105 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, else con_out->output_string(con_out, L"<none>"); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_device_path() - print device path

@device_path: device path to print

@dp2txt: device path to text protocol

*/

+efi_status_t print_device_path(struct efi_device_path *device_path,
	       struct efi_device_path_to_text_protocol *dp2txt)
+{
u16 *string;

efi_status_t ret;

if (!device_path) {
con_out->output_string(con_out, L"<none>\r\n");
return EFI_SUCCESS;
}

string = dp2txt->convert_device_path_to_text(device_path, true,
false);

...
if (!string) {
con_out->output_string
	(con_out, L"Cannot convert device path to text\r\n");
return EFI_OUT_OF_RESOURCES;
}

con_out->output_string(con_out, string);

con_out->output_string(con_out, L"\r\n");

ret = boottime->free_pool(string);

if (ret != EFI_SUCCESS) {
con_out->output_string(con_out, L"Cannot free pool memory\r\n");
return ret;
}

return EFI_SUCCESS;
+}

+/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systab: system table

@return: status code

*/

+efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systab)
+{
struct efi_loaded_image *loaded_image;

struct efi_device_path_to_text_protocol *device_path_to_text;

struct efi_device_path *device_path;

efi_status_t ret;

systable = systab;

boottime = systable->boottime;

con_out = systable->con_out;

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

print_uefi_revision();

print_config_tables();

/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
}

print_load_options(loaded_image);

/* Get the device path to text protocol */

ret = boottime->locate_protocol(&device_path_to_text_protocol_guid,
			NULL, (void **)&device_path_to_text);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open device path to text protocol\r\n");
goto out;
}

if (!loaded_image->device_handle) {
con_out->output_string
	(con_out, L"Missing device handle\r\n");
goto out;
}

ret = boottime->handle_protocol(loaded_image->device_handle,
			&device_path_guid,
			(void **)&device_path);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Missing devide path for device handle\r\n");
goto out;
}

con_out->output_string(con_out, L"Boot device: ");

ret = print_device_path(device_path, device_path_to_text);

if (ret != EFI_SUCCESS)
goto out;
con_out->output_string(con_out, L"File path: ");

ret = print_device_path(loaded_image->file_path,
device_path_to_text);

...
if (ret != EFI_SUCCESS)
goto out;
out: boottime->exit(handle, ret, 0, NULL); -- 2.29.2

AKASHI Takahiro

5:29 a.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

On Fri, Jan 15, 2021 at 04:12:18AM +0100, Heinrich Schuchardt wrote:

...

Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:

...
Heinrich,

On Tue, Jan 12, 2021 at 08:58:40PM +0100, Heinrich Schuchardt wrote:

...
Let helloworld.efi print the device path of the boot device and the

file

...
path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167

+++++++++++++++++++++++++++++-------

...
1 file changed, 137 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/helloworld.c

b/lib/efi_loader/helloworld.c

...
index 9ae2ee3389..5c8b7a96f9 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -1,43 +1,41 @@ // SPDX-License-Identifier: GPL-2.0+ /*

EFI hello world

Hello world EFI application

Copyright (c) 2016 Google, Inc

Written by Simon Glass sjg@chromium.org

Copyright 2020, Heinrich Schuchardt xypron.glpk@gmx.de

This program demonstrates calling a boottime service.

It writes a greeting and the load options to the console.

This test program is used to test the invocation of an EFI

application.

...

It writes

a greeting

the firmware's UEFI version

the installed configuration tables

the boot device's device path and the file path

If this kind of information is quite useful for users, why not add that (printing) feature as an option of bootefi (or efidebug)? I'm afraid that most users who are irritated as you said won't be able to imagine such information be printed by helloworld app.

The file path is written in

https://github.com/trini/u-boot/blob/master/cmd/bootefi.c#L471

Device paths are not really user friendly.

So why do you want to print such info at helloworld?

I guess that, according to your cover letter, you have in your mind some cases where an user may get in trouble relating to the boot device. Right?

...

So I would not like to write it there.

What I meant to suggest is to add an option, -v or -h, to bootefi, which prints verbose (and helpful) information for users to identify a cause. I can easily imagine users may blindly try to add -[v|h] when they see an error message even if they don't know there is such an option:)

-Takahiro Akashi

...

Best regards

Heinrich

...
-Takahiro Akashi

...

to the console.

*/

-#include <common.h> #include <efi_api.h>

static const efi_guid_t loaded_image_guid =

EFI_LOADED_IMAGE_PROTOCOL_GUID;

...
+static const efi_guid_t device_path_to_text_protocol_guid =

EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;

+static const efi_guid_t device_path_guid =

EFI_DEVICE_PATH_PROTOCOL_GUID;

...
static const efi_guid_t fdt_guid = EFI_FDT_GUID; static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;

+static struct efi_system_table *systable; +static struct efi_boot_services *boottime; +static struct efi_simple_text_output_protocol *con_out;

/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systable: system table

@return: status code

print_uefi_revision() - print UEFI revision number

*/

-efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systable)
+static void print_uefi_revision(void) {

struct efi_simple_text_output_protocol *con_out =
systable->con_out;

...
struct efi_boot_services *boottime = systable->boottime;

struct efi_loaded_image *loaded_image;

efi_status_t ret;

efi_uintn_t i; u16 rev[] = L"0.0.0";

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

/* Print the revision number */ rev[0] = (systable->hdr.revision >> 16) + '0'; rev[4] = systable->hdr.revision & 0xffff; for (; rev[4] >= 10;) {

@@ -53,15 +51,15 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string(con_out, L"Running on UEFI "); con_out->output_string(con_out, rev); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_config_tables() - print configuration tables

*/

+static void print_config_tables(void) +{

efi_uintn_t i;
/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
} /* Find configuration tables */ for (i = 0; i < systable->nr_tables; ++i) { if (!memcmp(&systable->tables[i].guid, &fdt_guid,
@@ -77,6 +75,16 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, con_out->output_string (con_out, L"Have SMBIOS table\r\n"); } +}

+/**

print_load_options() - print load options

@systable: system table

@con_out: simple text output protocol

*/

+void print_load_options(struct efi_loaded_image *loaded_image) +{ /* Output the load options */ con_out->output_string(con_out, L"Load options: "); if (loaded_image->load_options_size && loaded_image->load_options) @@ -85,6 +93,105 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle, else con_out->output_string(con_out, L"<none>"); con_out->output_string(con_out, L"\r\n"); +}

+/**

print_device_path() - print device path

@device_path: device path to print

@dp2txt: device path to text protocol

*/

+efi_status_t print_device_path(struct efi_device_path *device_path,
	       struct efi_device_path_to_text_protocol *dp2txt)
+{
u16 *string;

efi_status_t ret;

if (!device_path) {
con_out->output_string(con_out, L"<none>\r\n");
return EFI_SUCCESS;
}

string = dp2txt->convert_device_path_to_text(device_path, true,
false);

...
if (!string) {
con_out->output_string
	(con_out, L"Cannot convert device path to text\r\n");
return EFI_OUT_OF_RESOURCES;
}

con_out->output_string(con_out, string);

con_out->output_string(con_out, L"\r\n");

ret = boottime->free_pool(string);

if (ret != EFI_SUCCESS) {
con_out->output_string(con_out, L"Cannot free pool memory\r\n");
return ret;
}

return EFI_SUCCESS;
+}

+/**

efi_main() - entry point of the EFI application.

@handle: handle of the loaded image

@systab: system table

@return: status code

*/

+efi_status_t EFIAPI efi_main(efi_handle_t handle,
	     struct efi_system_table *systab)
+{
struct efi_loaded_image *loaded_image;

struct efi_device_path_to_text_protocol *device_path_to_text;

struct efi_device_path *device_path;

efi_status_t ret;

systable = systab;

boottime = systable->boottime;

con_out = systable->con_out;

/* UEFI requires CR LF */

con_out->output_string(con_out, L"Hello, world!\r\n");

print_uefi_revision();

print_config_tables();

/* Get the loaded image protocol */

ret = boottime->handle_protocol(handle, &loaded_image_guid,
			(void **)&loaded_image);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open loaded image protocol\r\n");
goto out;
}

print_load_options(loaded_image);

/* Get the device path to text protocol */

ret = boottime->locate_protocol(&device_path_to_text_protocol_guid,
			NULL, (void **)&device_path_to_text);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Cannot open device path to text protocol\r\n");
goto out;
}

if (!loaded_image->device_handle) {
con_out->output_string
	(con_out, L"Missing device handle\r\n");
goto out;
}

ret = boottime->handle_protocol(loaded_image->device_handle,
			&device_path_guid,
			(void **)&device_path);
if (ret != EFI_SUCCESS) {
con_out->output_string
	(con_out, L"Missing devide path for device handle\r\n");
goto out;
}

con_out->output_string(con_out, L"Boot device: ");

ret = print_device_path(device_path, device_path_to_text);

if (ret != EFI_SUCCESS)
goto out;
con_out->output_string(con_out, L"File path: ");

ret = print_device_path(loaded_image->file_path,
device_path_to_text);

...
if (ret != EFI_SUCCESS)
goto out;
out: boottime->exit(handle, ret, 0, NULL); -- 2.29.2

Heinrich Schuchardt

1:02 p.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

On 15.01.21 05:29, AKASHI Takahiro wrote:

...

On Fri, Jan 15, 2021 at 04:12:18AM +0100, Heinrich Schuchardt wrote:

...
Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:

...
Heinrich,

On Tue, Jan 12, 2021 at 08:58:40PM +0100, Heinrich Schuchardt wrote:

...
Let helloworld.efi print the device path of the boot device and the

file

...
path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167

+++++++++++++++++++++++++++++-------

...

...
...
If this kind of information is quite useful for users, why not add that (printing) feature as an option of bootefi (or efidebug)? I'm afraid that most users who are irritated as you said won't be able to imagine such information be printed by helloworld app.

The file path is written in

https://github.com/trini/u-boot/blob/master/cmd/bootefi.c#L471

Device paths are not really user friendly.

So why do you want to print such info at helloworld?

I guess that, according to your cover letter, you have in your mind some cases where an user may get in trouble relating to the boot device. Right?

...
So I would not like to write it there.

What I meant to suggest is to add an option, -v or -h, to bootefi, which prints verbose (and helpful) information for users to identify a cause. I can easily imagine users may blindly try to add -[v|h] when they see an error message even if they don't know there is such an option:)

To me helloworld.efi is a tool for a developer to see if an EFI binary is correctly invoked.

The normal U-Boot code we want to keep as slim as possible.

According to the spec UEFI boots from the ESP and typically there is only one. So printing the file path in cmd/bootefi should be enough.

Best regards

Heinrich

AKASHI Takahiro

18 Jan 18 Jan

2:38 a.m.

New subject: [PATCH 1/3] efi_loader: print boot device and file path in helloworld

On Fri, Jan 15, 2021 at 01:02:51PM +0100, Heinrich Schuchardt wrote:

...

On 15.01.21 05:29, AKASHI Takahiro wrote:

...
On Fri, Jan 15, 2021 at 04:12:18AM +0100, Heinrich Schuchardt wrote:

...
Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:

...
Heinrich,

On Tue, Jan 12, 2021 at 08:58:40PM +0100, Heinrich Schuchardt wrote:

...
Let helloworld.efi print the device path of the boot device and the

file

...
path as provided by the loaded image protocol.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/helloworld.c | 167

+++++++++++++++++++++++++++++-------

<snip />

...
...
...
If this kind of information is quite useful for users, why not add that (printing) feature as an option of bootefi (or efidebug)? I'm afraid that most users who are irritated as you said won't be able to imagine such information be printed by helloworld app.

The file path is written in

https://github.com/trini/u-boot/blob/master/cmd/bootefi.c#L471

Device paths are not really user friendly.

So why do you want to print such info at helloworld?

I guess that, according to your cover letter, you have in your mind some cases where an user may get in trouble relating to the boot device. Right?

...
So I would not like to write it there.

What I meant to suggest is to add an option, -v or -h, to bootefi, which prints verbose (and helpful) information for users to identify a cause. I can easily imagine users may blindly try to add -[v|h] when they see an error message even if they don't know there is such an option:)

To me helloworld.efi is a tool for a developer to see if an EFI binary is correctly invoked.

My point is that most users (developers?) don't intuitively imagine such information will be printed with helloworld app.

...

The normal U-Boot code we want to keep as slim as possible.

(I doubt this in terms of UEFI)

...

According to the spec UEFI boots from the ESP and typically there is only one. So printing the file path in cmd/bootefi should be enough.

So again,

...

...
So why do you want to print such info at helloworld?

-Takahiro Akashi

...

Best regards

Heinrich

Heinrich Schuchardt

12 Jan 12 Jan

8:58 p.m.

New subject: [PATCH 2/3] efi_loader: carve out efi_check_pe()

Carve out a function to check that a buffer contains a PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 2 + lib/efi_loader/efi_image_loader.c | 80 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 0fc2255f3f..63459ea649 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -453,6 +453,8 @@ efi_status_t efi_set_watchdog(unsigned long timeout);

/* Called from places to check whether a timer expired */ void efi_timer_check(void); +/* Check if a buffer contains a PE-COFF image */ +efi_status_t efi_check_pe(void *buffer, size_t size, void **nt_header); /* PE loader implementation */ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi, size_t efi_size, diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 94f76ef6b8..d4dd9e9433 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -675,6 +675,46 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) } #endif /* CONFIG_EFI_SECURE_BOOT */

+ +/** + * efi_check_pe() - check if a memory buffer contains a PE-COFF image + * + * @buffer: buffer to check + * @size: size of buffer + * @nt_header: on return pointer to NT header of PE-COFF image + * Return: EFI_SUCCESS if the buffer contains a PE-COFF image + */ +efi_status_t efi_check_pe(void *buffer, size_t size, void **nt_header) +{ + IMAGE_DOS_HEADER *dos = buffer; + IMAGE_NT_HEADERS32 *nt; + + if (size < sizeof(*dos)) + return EFI_INVALID_PARAMETER; + + /* Check for DOS magix */ + if (dos->e_magic != IMAGE_DOS_SIGNATURE) + return EFI_INVALID_PARAMETER; + + /* + * Check if the image section header fits into the file. Knowing that at + * least one section header follows we only need to check for the length + * of the 64bit header which is longer than the 32bit header. + */ + if (size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32)) + return EFI_INVALID_PARAMETER; + nt = (IMAGE_NT_HEADERS32 *)((u8 *)buffer + dos->e_lfanew); + + /* Check for PE-COFF magic */ + if (nt->Signature != IMAGE_NT_SIGNATURE) + return EFI_INVALID_PARAMETER; + + if (nt_header) + *nt_header = nt; + + return EFI_SUCCESS; +} + /** * efi_load_pe() - relocate EFI binary * @@ -705,36 +745,10 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, int supported = 0; efi_status_t ret;

- /* Sanity check for a file header */ - if (efi_size < sizeof(*dos)) { - log_err("Truncated DOS Header\n"); - ret = EFI_LOAD_ERROR; - goto err; - } - - dos = efi; - if (dos->e_magic != IMAGE_DOS_SIGNATURE) { - log_err("Invalid DOS Signature\n"); - ret = EFI_LOAD_ERROR; - goto err; - } - - /* - * Check if the image section header fits into the file. Knowing that at - * least one section header follows we only need to check for the length - * of the 64bit header which is longer than the 32bit header. - */ - if (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS64)) { - log_err("Invalid offset for Extended Header\n"); - ret = EFI_LOAD_ERROR; - goto err; - } - - nt = (void *) ((char *)efi + dos->e_lfanew); - if (nt->Signature != IMAGE_NT_SIGNATURE) { - log_err("Invalid NT Signature\n"); - ret = EFI_LOAD_ERROR; - goto err; + ret = efi_check_pe(efi, efi_size, (void **)&nt); + if (ret != EFI_SUCCESS) { + log_err("Not a PE-COFF file\n"); + return EFI_LOAD_ERROR; }

for (i = 0; machines[i]; i++) @@ -746,8 +760,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, if (!supported) { log_err("Machine type 0x%04x is not supported\n", nt->FileHeader.Machine); - ret = EFI_LOAD_ERROR; - goto err; + return EFI_LOAD_ERROR; }

num_sections = nt->FileHeader.NumberOfSections; @@ -757,8 +770,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, if (efi_size < ((void *)sections + sizeof(sections[0]) * num_sections - efi)) { log_err("Invalid number of sections: %d\n", num_sections); - ret = EFI_LOAD_ERROR; - goto err; + return EFI_LOAD_ERROR; }

/* Authenticate an image */ -- 2.29.2

Simon Glass

14 Jan 14 Jan

4:42 p.m.

New subject: [PATCH 2/3] efi_loader: carve out efi_check_pe()

On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Carve out a function to check that a buffer contains a PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

include/efi_loader.h | 2 + lib/efi_loader/efi_image_loader.c | 80 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 34 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

Heinrich Schuchardt

12 Jan 12 Jan

8:58 p.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- cmd/bootefi.c | 136 ++++++++++++++++++++++++++----------------- doc/uefi/uefi.rst | 11 ++-- fs/fs.c | 3 +- include/efi_loader.h | 6 +- net/tftp.c | 9 ++- 5 files changed, 98 insertions(+), 67 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index c82a5bacf6..bd0d12ea9b 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -29,6 +29,78 @@ DECLARE_GLOBAL_DATA_PTR;

static struct efi_device_path *bootefi_image_path; static struct efi_device_path *bootefi_device_path; +static void *image_addr; +static size_t image_size; + +/** + * efi_clear_bootdev() - clear boot device + */ +static void efi_clear_bootdev(void) +{ + efi_free_pool(bootefi_device_path); + efi_free_pool(bootefi_image_path); + bootefi_device_path = NULL; + bootefi_image_path = NULL; + image_addr = NULL; + image_size = 0; +} + +/** + * efi_set_bootdev() - set boot device + * + * This function is called when a file is loaded, e.g. via the 'load' command. + * We use the path to this file to inform the UEFI binary about the boot device. + * + * @dev: device, e.g. "MMC" + * @devnr: number of the device, e.g. "1:2" + * @path: path to file loaded + * @buffer: buffer with file loaded + * @buffer_size: size of file loaded + */ +void efi_set_bootdev(const char *dev, const char *devnr, const char *path, + void *buffer, size_t buffer_size) +{ + struct efi_device_path *device, *image; + efi_status_t ret; + + /* Forget overwritten image */ + if (buffer + buffer_size >= image_addr && + image_addr + image_size >= buffer) + efi_clear_bootdev(); + + /* Remember only PE-COFF and FIT images */ + if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) { + if (!fit_check_format(buffer)) + return; + /* + * FIT images of type EFI_OS are started via command bootm. + * We should not use their boot device with the bootefi command. + */ + buffer = 0; + buffer_size = 0; + } + + /* efi_set_bootdev() is typically called repeatedly, recover memory */ + efi_clear_bootdev(); + + image_addr = buffer; + image_size = buffer_size; + + ret = efi_dp_from_name(dev, devnr, path, &device, &image); + if (ret == EFI_SUCCESS) { + bootefi_device_path = device; + if (image) { + /* FIXME: image should not contain device */ + struct efi_device_path *image_tmp = image; + + efi_dp_split_file_path(image, &device, &image); + efi_free_pool(image_tmp); + } + bootefi_image_path = image; + } else { + efi_clear_bootdev(); + } +}

/** * efi_env_set_load_options() - set load options from environment variable @@ -398,33 +470,28 @@ static int do_bootefi_image(const char *image_opt) { void *image_buf; unsigned long addr, size; - const char *size_str; efi_status_t ret;

#ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(image_opt, "hello")) { image_buf = __efi_helloworld_begin; size = __efi_helloworld_end - __efi_helloworld_begin; - - efi_free_pool(bootefi_device_path); - efi_free_pool(bootefi_image_path); - bootefi_device_path = NULL; - bootefi_image_path = NULL; + efi_clear_bootdev(); } else #endif { - size_str = env_get("filesize"); - if (size_str) - size = simple_strtoul(size_str, NULL, 16); - else - size = 0; - - addr = simple_strtoul(image_opt, NULL, 16); + addr = strtoul(image_opt, NULL, 16); /* Check that a numeric value was passed */ - if (!addr && *image_opt != '0') + if (!addr) return CMD_RET_USAGE;

image_buf = map_sysmem(addr, size); + + if (image_buf != image_addr) { + log_err("No UEFI binary known at %s\n", image_opt); + return CMD_RET_FAILURE; + } + size = image_size; } ret = efi_run_image(image_buf, size);

@@ -557,11 +624,8 @@ static efi_status_t bootefi_test_prepare if (ret == EFI_SUCCESS) return ret;

- efi_free_pool(bootefi_image_path); - bootefi_image_path = NULL; failure: - efi_free_pool(bootefi_device_path); - bootefi_device_path = NULL; + efi_clear_bootdev(); return ret; }

@@ -681,39 +745,3 @@ U_BOOT_CMD( "Boots an EFI payload from memory", bootefi_help_text ); - -/** - * efi_set_bootdev() - set boot device - * - * This function is called when a file is loaded, e.g. via the 'load' command. - * We use the path to this file to inform the UEFI binary about the boot device. - * - * @dev: device, e.g. "MMC" - * @devnr: number of the device, e.g. "1:2" - * @path: path to file loaded - */ -void efi_set_bootdev(const char *dev, const char *devnr, const char *path) -{ - struct efi_device_path *device, *image; - efi_status_t ret; - - /* efi_set_bootdev is typically called repeatedly, recover memory */ - efi_free_pool(bootefi_device_path); - efi_free_pool(bootefi_image_path); - - ret = efi_dp_from_name(dev, devnr, path, &device, &image); - if (ret == EFI_SUCCESS) { - bootefi_device_path = device; - if (image) { - /* FIXME: image should not contain device */ - struct efi_device_path *image_tmp = image; - - efi_dp_split_file_path(image, &device, &image); - efi_free_pool(image_tmp); - } - bootefi_image_path = image; - } else { - bootefi_device_path = NULL; - bootefi_image_path = NULL; - } -} diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst index dc930d9240..5a67737c15 100644 --- a/doc/uefi/uefi.rst +++ b/doc/uefi/uefi.rst @@ -59,13 +59,10 @@ Below you find the output of an example session starting GRUB:: 120832 bytes read in 7 ms (16.5 MiB/s) => bootefi ${kernel_addr_r} ${fdt_addr_r}

-The bootefi command uses the device, the file name, and the file size -(environment variable 'filesize') of the most recently loaded file when setting -up the binary for execution. So the UEFI binary should be loaded last. - -The environment variable 'bootargs' is passed as load options in the UEFI system -table. The Linux kernel EFI stub uses the load options as command line -arguments. +When booting from a memory location it is unknown from which file it was loaded. +Therefore the bootefi command uses the device path of the block device partition +or the network adapter and the file name of the most recently loaded PE-COFF +file when setting up the loaded image protocol.

Launching a UEFI binary from a FIT image ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/fs/fs.c b/fs/fs.c index 7a4020607a..a3989f9bdb 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -752,7 +752,8 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],

if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "", - (argc > 4) ? argv[4] : ""); + (argc > 4) ? argv[4] : "", map_sysmem(addr, 0), + len_read);

printf("%llu bytes read in %lu ms", len_read, time); if (time > 0) { diff --git a/include/efi_loader.h b/include/efi_loader.h index 63459ea649..2465a47a91 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -467,7 +467,8 @@ void efi_restore_gd(void); /* Call this to relocate the runtime section to an address space */ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map); /* Call this to set the current device name */ -void efi_set_bootdev(const char *dev, const char *devnr, const char *path); +void efi_set_bootdev(const char *dev, const char *devnr, const char *path, + void *buffer, size_t buffer_size); /* Add a new object to the object list. */ void efi_add_handle(efi_handle_t obj); /* Create handle */ @@ -829,7 +830,8 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) /* No loader configured, stub out EFI_ENTRY */ static inline void efi_restore_gd(void) { } static inline void efi_set_bootdev(const char *dev, const char *devnr, - const char *path) { } + const char *path, void *buffer, + size_t buffer_size) { } static inline void efi_net_set_dhcp_ack(void *pkt, int len) { } static inline void efi_print_image_infos(void *pc) { }

diff --git a/net/tftp.c b/net/tftp.c index 6fdb1a821a..2cfa0b1486 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -329,6 +329,12 @@ static void tftp_complete(void) time_start * 1000, "/s"); } puts("\ndone\n"); + if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) { + if (!tftp_put_active) + efi_set_bootdev("Net", "", tftp_filename, + map_sysmem(tftp_load_addr, 0), + net_boot_file_size); + } net_set_state(NETLOOP_SUCCESS); }

@@ -841,9 +847,6 @@ void tftp_start(enum proto_t protocol) printf("Load address: 0x%lx\n", tftp_load_addr); puts("Loading: *\b"); tftp_state = STATE_SEND_RRQ; -#ifdef CONFIG_CMD_BOOTEFI - efi_set_bootdev("Net", "", tftp_filename); -#endif }

time_start = get_timer(0); -- 2.29.2

Simon Glass

19 Jan 19 Jan

7:06 p.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

Hi Heinrich,

On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

cmd/bootefi.c | 136 ++++++++++++++++++++++++++----------------- doc/uefi/uefi.rst | 11 ++-- fs/fs.c | 3 +- include/efi_loader.h | 6 +- net/tftp.c | 9 ++- 5 files changed, 98 insertions(+), 67 deletions(-)

Does this need an update to the tests?

Regards, Simon

Heinrich Schuchardt

7:43 p.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

On 1/19/21 7:06 PM, Simon Glass wrote:

...

Hi Heinrich,

On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

cmd/bootefi.c | 136 ++++++++++++++++++++++++++----------------- doc/uefi/uefi.rst | 11 ++-- fs/fs.c | 3 +- include/efi_loader.h | 6 +- net/tftp.c | 9 ++- 5 files changed, 98 insertions(+), 67 deletions(-)

Does this need an update to the tests?

Our current tests still run successfully.

The observed problematic behavior is not covered by our tests. To run GRUB or helloworld.efi we just load one file and execute it.

So it would make sense to extend the coverage of our tests with a future patch.

Best regards

Heinrich

...

Regards, Simon

Simon Glass

20 Jan 20 Jan

1:15 a.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

Hi Heinrich,

On Tue, 19 Jan 2021 at 11:43, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 1/19/21 7:06 PM, Simon Glass wrote:

...
Hi Heinrich,

On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

cmd/bootefi.c | 136 ++++++++++++++++++++++++++----------------- doc/uefi/uefi.rst | 11 ++-- fs/fs.c | 3 +- include/efi_loader.h | 6 +- net/tftp.c | 9 ++- 5 files changed, 98 insertions(+), 67 deletions(-)

Does this need an update to the tests?

Our current tests still run successfully.

The observed problematic behavior is not covered by our tests. To run GRUB or helloworld.efi we just load one file and execute it.

So it would make sense to extend the coverage of our tests with a future patch.

Yes indeed.

Regards, Simon

Kyle Evans

3 Apr 3 Apr

11:08 p.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

On Tue, Jan 12, 2021 at 1:59 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Hi,

I'm only a little late on this, but may I ask what the rationale of this last part is? I'm afraid there are some real-world use cases where a compromise would be great, allowing bootefi to accept a random region of memory to boot -- in my case, I have the payload (FreeBSD's loader.efi) already in memory when U-Boot starts and it's unclear that I can come up with some other way to boot it that doesn't involve a lot of backflips.

My specific suggestion, which I can formally post to the list if you don't immediately object, is this: https://people.freebsd.org/~kevans/uboot.patch

It basically adds another form:

`bootefi addr [fdt [size]]`

If $addr isn't the last loaded PE-COFF, size must be provided. fdt can be `-` to signal EFI_FDT_USE_INTERNAL. If we provide an address that isn't the last loaded PE-COFF, it wipes out the device path and lets efi_run_image() synthesize it. Providing a size with the address of the last loaded PE-COFF is an error, but that's a bit arbitrary.

Obviously our loader doesn't get good source disk information this way and I have to drive it manually, but that's still orders of magnitude better than having to locally shuffle new loader.efis onto a flash drive and keep moving the flash drive back and forth to iterate on it.

Thans,

Kyle Evans

Heinrich Schuchardt

4 Apr 4 Apr

7:09 a.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

Am 3. April 2022 23:08:33 MESZ schrieb Kyle Evans kevans@freebsd.org:

...

On Tue, Jan 12, 2021 at 1:59 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Hi,

I'm only a little late on this, but may I ask what the rationale of this last part is? I'm afraid there are some real-world use cases where a compromise would be great, allowing bootefi to accept a random region of memory to boot -- in my case, I have the payload (FreeBSD's loader.efi) already in memory when U-Boot starts and it's unclear that

Could you, please, describe your use case in some more detail.

Why can't you load loader.efi from the ESP?

...

I can come up with some other way to boot it that doesn't involve a lot of backflips. My specific suggestion, which I can formally post to the list if you don't immediately object, is this: https://people.freebsd.org/~kevans/uboot.patch

It basically adds another form:

`bootefi addr [fdt [size]]`

What should size be used for? Both EFI binaries and device-trees provide their size in a header field.

Best regards

Heinrich

...

If $addr isn't the last loaded PE-COFF, size must be provided. fdt can be `-` to signal EFI_FDT_USE_INTERNAL. If we provide an address that isn't the last loaded PE-COFF, it wipes out the device path and lets efi_run_image() synthesize it. Providing a size with the address of the last loaded PE-COFF is an error, but that's a bit arbitrary.

Obviously our loader doesn't get good source disk information this way and I have to drive it manually, but that's still orders of magnitude better than having to locally shuffle new loader.efis onto a flash drive and keep moving the flash drive back and forth to iterate on it.

Thans,

Kyle Evans

Kyle Evans

7:40 a.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

On Mon, Apr 4, 2022 at 12:09 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Am 3. April 2022 23:08:33 MESZ schrieb Kyle Evans kevans@freebsd.org:

...
On Tue, Jan 12, 2021 at 1:59 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Hi,

I'm only a little late on this, but may I ask what the rationale of this last part is? I'm afraid there are some real-world use cases where a compromise would be great, allowing bootefi to accept a random region of memory to boot -- in my case, I have the payload (FreeBSD's loader.efi) already in memory when U-Boot starts and it's unclear that

Could you, please, describe your use case in some more detail.

Why can't you load loader.efi from the ESP?

I'm explicitly trying to override the loader.efi from ESP, because it's broken and I can't (easily) keep switching it out. This is on Apple's M1, so I can happily inject the new loader.efi from m1n1 (much like the JTAG use-case mentioned in this file) for testing new iterations.

...

...
I can come up with some other way to boot it that doesn't involve a lot of backflips. My specific suggestion, which I can formally post to the list if you don't immediately object, is this: https://people.freebsd.org/~kevans/uboot.patch

It basically adds another form:

`bootefi addr [fdt [size]]`

What should size be used for? Both EFI binaries and device-trees provide their size in a header field.

Right now it's used because efi_run_image() wants the buffer size to construct the memory-based device path. I'm not familiar enough with U-Boot to know if there's a sensible API for grabbing the size from this PE image in memory.

Heinrich Schuchardt

7:58 a.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

Am 4. April 2022 07:40:16 MESZ schrieb Kyle Evans kevans@freebsd.org:

...

On Mon, Apr 4, 2022 at 12:09 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Am 3. April 2022 23:08:33 MESZ schrieb Kyle Evans kevans@freebsd.org:

...
On Tue, Jan 12, 2021 at 1:59 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Hi,

I'm only a little late on this, but may I ask what the rationale of this last part is? I'm afraid there are some real-world use cases where a compromise would be great, allowing bootefi to accept a random region of memory to boot -- in my case, I have the payload (FreeBSD's loader.efi) already in memory when U-Boot starts and it's unclear that

Could you, please, describe your use case in some more detail.

Why can't you load loader.efi from the ESP?

I'm explicitly trying to override the loader.efi from ESP, because it's broken and I can't (easily) keep switching it out. This is on Apple's M1, so I can happily inject the new loader.efi from m1n1 (much like the JTAG use-case mentioned in this file) for testing new iterations.

You could use the loady command to load the EFI binary via the UART. This should work with an unpatched U-Boot v2022.04-rc5.

...

...
...
I can come up with some other way to boot it that doesn't involve a lot of backflips. My specific suggestion, which I can formally post to the list if you don't immediately object, is this: https://people.freebsd.org/~kevans/uboot.patch

It basically adds another form:

`bootefi addr [fdt [size]]`

What should size be used for? Both EFI binaries and device-trees provide their size in a header field.

Please, send your patch for review to the mailing list and me.

Best regards

Heinrich

...

Right now it's used because efi_run_image() wants the buffer size to construct the memory-based device path. I'm not familiar enough with U-Boot to know if there's a sensible API for grabbing the size from this PE image in memory.

Kyle Evans

4:51 p.m.

New subject: [PATCH 3/3] efi_loader: setting boot device

On Mon, Apr 4, 2022 at 12:59 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Am 4. April 2022 07:40:16 MESZ schrieb Kyle Evans kevans@freebsd.org:

...
On Mon, Apr 4, 2022 at 12:09 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Am 3. April 2022 23:08:33 MESZ schrieb Kyle Evans kevans@freebsd.org:

...
On Tue, Jan 12, 2021 at 1:59 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...
Up to now the bootefi command used the last file loaded to determine the boot partition. This has led to errors when the fdt had been loaded from another partition after the EFI binary.

Before setting the boot device from a loaded file check if it is a PE-COFF image or a FIT image.

For a PE-COFF image remember address and size, boot device and path.

For a FIT image remember boot device and path.

If the PE-COFF image is overwritten by loading another file, forget it.

Do not allow to start an image via bootefi which is not the last loaded PE-COFF image.

Hi,

I'm only a little late on this, but may I ask what the rationale of this last part is? I'm afraid there are some real-world use cases where a compromise would be great, allowing bootefi to accept a random region of memory to boot -- in my case, I have the payload (FreeBSD's loader.efi) already in memory when U-Boot starts and it's unclear that

Could you, please, describe your use case in some more detail.

Why can't you load loader.efi from the ESP?

I'm explicitly trying to override the loader.efi from ESP, because it's broken and I can't (easily) keep switching it out. This is on Apple's M1, so I can happily inject the new loader.efi from m1n1 (much like the JTAG use-case mentioned in this file) for testing new iterations.

You could use the loady command to load the EFI binary via the UART. This should work with an unpatched U-Boot v2022.04-rc5.

Indeed, thanks! It's a bit less convenient than being able to script this from the beginning, but it's a lot better than having to distribute a patched u-boot to folks working on this with me.

...

...
...
...
I can come up with some other way to boot it that doesn't involve a lot of backflips. My specific suggestion, which I can formally post to the list if you don't immediately object, is this: https://people.freebsd.org/~kevans/uboot.patch

It basically adds another form:

`bootefi addr [fdt [size]]`

What should size be used for? Both EFI binaries and device-trees provide their size in a header field.

Please, send your patch for review to the mailing list and me.

Will do- I considered dropping the size or adding a subcommand that does this instead and reading the size from the header, but I think it's grown on me as a cheap/easy way to extend the base bootefi without adding the footgun of a previously invalid command suddenly doing the wrong thing. If you just plopped it into memory, knowing the size should be an easy task

1128

Age (days ago)

1575

Last active (days ago)

List overview

Download

18 comments

4 participants

tags (0)

participants (4)

AKASHI Takahiro
Heinrich Schuchardt
Kyle Evans
Simon Glass