[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 <takahiro.akashi at 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 at 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 at chromium.org>
> >> + * Copyright 2020, Heinrich Schuchardt <xypron.glpk at 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
> >> 
>