[PATCH 0/3] efi_loader: setting boot device

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/3] efi_loader: setting boot device

Heinrich Schuchardt
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

 cmd/bootefi.c                     |  14 ++-
 doc/uefi/uefi.rst                 |  11 +-
 fs/fs.c                           |   3 +-
 include/efi_loader.h              |   8 +-
 lib/efi_loader/efi_image_loader.c |  80 ++++++++------
 lib/efi_loader/helloworld.c       | 167 ++++++++++++++++++++++++------
 net/tftp.c                        |   9 +-
 7 files changed, 211 insertions(+), 81 deletions(-)

--
2.29.2

Reply | Threaded
Open this post in threaded view
|

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

Heinrich Schuchardt
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 <[hidden email]>
---
 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 <[hidden email]>
+ * Copyright 2020, Heinrich Schuchardt <[hidden email]>
  *
- * 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

Reply | Threaded
Open this post in threaded view
|

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

Heinrich Schuchardt
In reply to this post by Heinrich Schuchardt
Carve out a function to check that a buffer contains a PE-COFF image.

Signed-off-by: Heinrich Schuchardt <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/3] efi_loader: setting boot device

Heinrich Schuchardt
In reply to this post by Heinrich Schuchardt
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 <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

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

Simon Glass-3
In reply to this post by Heinrich Schuchardt
On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt <[hidden email]> 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 <[hidden email]>
> ---
>  lib/efi_loader/helloworld.c | 167 +++++++++++++++++++++++++++++-------
>  1 file changed, 137 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

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

Simon Glass-3
In reply to this post by Heinrich Schuchardt
On Tue, 12 Jan 2021 at 12:58, Heinrich Schuchardt <[hidden email]> wrote:
>
> Carve out a function to check that a buffer contains a PE-COFF image.
>
> Signed-off-by: Heinrich Schuchardt <[hidden email]>
> ---
>  include/efi_loader.h              |  2 +
>  lib/efi_loader/efi_image_loader.c | 80 ++++++++++++++++++-------------
>  2 files changed, 48 insertions(+), 34 deletions(-)
>

Reviewed-by: Simon Glass <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

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

AKASHI Takahiro
In reply to this post by Heinrich Schuchardt
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 <[hidden email]>
> ---
>  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 <[hidden email]>
> + * Copyright 2020, Heinrich Schuchardt <[hidden email]>
>   *
> - * 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
>
Reply | Threaded
Open this post in threaded view
|

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

Heinrich Schuchardt
Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro <[hidden email]>:

>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 <[hidden email]>
>> ---
>>  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 <[hidden email]>
>> + * Copyright 2020, Heinrich Schuchardt <[hidden email]>
>>   *
>> - * 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
>>

Reply | Threaded
Open this post in threaded view
|

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

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

> Am 15. Januar 2021 02:56:03 MEZ schrieb AKASHI Takahiro <[hidden email]>:
> >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 <[hidden email]>
> >> ---
> >>  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 <[hidden email]>
> >> + * Copyright 2020, Heinrich Schuchardt <[hidden email]>
> >>   *
> >> - * 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
> >>
>
Reply | Threaded
Open this post in threaded view
|

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

Heinrich Schuchardt
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 <[hidden email]>:
>>> 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 <[hidden email]>
>>>> ---
>>>>  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.

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