Re: [PATCH v8 2/9] eficonfig: menu-driven addition of UEFI boot option

On Tue, 12 Jul 2022 at 19:59, Masahisa Kojima masahisa.kojima@linaro.org wrote:

On Sun, 10 Jul 2022 at 18:03, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 6/19/22 06:56, Masahisa Kojima wrote:

...

This commit add the "eficonfig" command. The "eficonfig" command implements the menu-driven UEFI boot option maintenance feature. This commit implements the addition of new boot option. User can select the block device volume having efi_simple_file_system_protocol and select the file corresponding to the Boot#### variable. User can also enter the description and optional_data of the BOOT#### variable in utf8.

This commit adds "include/efi_config.h", it contains the common definition to be used from other menus such as UEFI Secure Boot key management.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v8:

• command name is change from "efimenu" to "eficonfig"
• function and struct prefixes is changed to "eficonfig"
• fix menu header string

Changes in v7:

• add "efimenu" command and uefi variable maintenance code moved into cmd/efimenu.c

• create include/efimenu.h to define the common definition for the other menu such as UEFI Secure Boot key management

• update boot option edit UI, user can select description, file, and optional_data to edit in the same menu like following.

  ** Edit Boot Option **

  Description: debian
  File: virtio 0:1/EFI\debian\grubaa64.efi
  Optional Data: test
  Save
  Quit
  
  

• remove exit parameter from efimenu_process_common()

• menu title type is changed from u16 to char

• efimenu_process_common() add menu title string

• reduce printf/puts function call for displaying the menu

• efi_console_get_u16_string() accept 0 length to allow optional_data is empty

• efi_console_get_u16_string() the "size" parameter name is changes to "count"

• efimenu is now designed to maintain the UEFI variables, remove autoboot related code

• remove one empty line before "Quit" entry

• efimenu_init() processes only the first time

Changes in v6:

• fix typos
• modify volume name to match U-Boot syntax
• compile in CONFIG_EFI_LOADER=n and CONFIG_CMD_BOOTEFI_BOOTMGR=n
• simplify u16_strncmp() usage
• support "a\b.efi" file path, use link list to handle filepath
• modify length check condition
• UEFI related menu items only appears with CONFIG_AUTOBOOT_MENU_SHOW=y

Changes in v5:

• remove forward declarations
• add const qualifier for menu items
• fix the possible unaligned access for directory info access
• split into three commit 1)add boot option 2) delete boot option 3)change boot order This commit is 1)add boot option.
• fix file name buffer allocation size, it should be EFI_BOOTMENU_FILE_PATH_MAX * sizeof(u16)
• fix wrong size checking for file selection

Chanes in v4:

• UEFI boot option maintenance menu is integrated into bootmenu
• display the simplified volume name(e.g. usb0:1, nvme1:2) for the volume selection
• instead of extending lib/efi_loader/efi_bootmgr.c, newly create lib/efi_loader/efi_bootmenu_maintenance.c and implement boot variable maintenance into it.

Changes in RFC v3: not included in v3 series

Changes in RFC v2:

• enable utf8 user input for boot option name

• create lib/efi_loader/efi_console.c::efi_console_get_u16_string() for utf8 user input handling

• use u16_strlcat instead of u16_strcat

• remove the EFI_CALLs, and newly create or expose the following xxx_int() functions. efi_locate_handle_buffer_int(), efi_open_volume_int(), efi_file_open_int(), efi_file_close_int(), efi_file_read_int() and efi_file_setpos_int(). Note that EFI_CALLs still exist for EFI_DEVICE_PATH_TO_TEXT_PROTOCOL and EFI_SIMPLE_TEXT_INPUT/OUTPUT_PROTOCOL

• use efi_search_protocol() instead of calling locate_protocol() to get the device_path_to_text_protocol interface.

• remove unnecessary puts(ANSI_CLEAR_LINE), this patch is still depends on puts(ANSI_CLEAR_CONSOLE)

• skip SetVariable() if the bootorder is not changed

  cmd/Kconfig | 7 + cmd/Makefile | 1 + cmd/eficonfig.c | 1270 +++++++++++++++++++++++++++++++++ include/efi_config.h | 91 +++ include/efi_loader.h | 40 ++ lib/efi_loader/efi_boottime.c | 52 +- lib/efi_loader/efi_console.c | 78 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- 9 files changed, 1578 insertions(+), 47 deletions(-) create mode 100644 cmd/eficonfig.c create mode 100644 include/efi_config.h

diff --git a/cmd/Kconfig b/cmd/Kconfig index 09193b61b9..bb7f1d0463 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1870,6 +1870,13 @@ config CMD_EFIDEBUG particularly for managing boot parameters as well as examining various EFI status for debugging.

+config CMD_EFICONFIG

• bool "eficonfig - provide menu-driven uefi variables maintenance interface"
  

• depends on CMD_BOOTEFI_BOOTMGR
  

• help
  

•   Enable the 'eficonfig' command which provides the menu-driven UEFI
  

•   variable maintenance interface.
  

• config CMD_EXCEPTION bool "exception - raise exception" depends on ARM || RISCV || SANDBOX || X86

diff --git a/cmd/Makefile b/cmd/Makefile index 5e43a1e022..0afa687e94 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o +obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_CMD_EROFS) += erofs.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c new file mode 100644 index 0000000000..20747db115 --- /dev/null +++ b/cmd/eficonfig.c @@ -0,0 +1,1270 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Menu-driven UEFI Variable maintenance
• • Copyright (c) 2022 Masahisa Kojima, Linaro Limited
• */

+#include <ansi.h> +#include <common.h> +#include <charset.h> +#include <efi_loader.h> +#include <efi_config.h> +#include <efi_variable.h> +#include <log.h> +#include <malloc.h> +#include <menu.h> +#include <watchdog.h> +#include <asm/unaligned.h> +#include <linux/delay.h>

+static struct efi_simple_text_input_protocol *cin; +static struct efi_simple_text_output_protocol *cout;

+#define EFICONFIG_DESCRIPTION_MAX 32 +#define EFICONFIG_OPTIONAL_DATA_MAX 64 +#define EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY 5

+#define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \

• EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \
  

•          0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde)
  

+const efi_guid_t efi_guid_bootmenu_auto_generated =

•         EFICONFIG_AUTO_GENERATED_ENTRY_GUID;
  

+struct eficonfig_boot_selection_data {

• u16 bootorder_index;
  

• int *selected;
  

+};

+struct eficonfig_filepath_info {

• u16 *name;
  

• struct list_head list;
  

+};

+/**

• • struct eficonfig_boot_option - structure to be used for uefi boot option update
• • @file_info: user selected file info
• • @boot_index: index of the uefi BootOrder variable
• • @description: pointer to the description string
• • @optional_data: pointer to the optional_data
• • @edit_completed: flag indicates edit complete
• */

+struct eficonfig_boot_option {

• struct eficonfig_select_file_info file_info;
  

• unsigned int boot_index;
  

• u16 *description;
  

• u16 *optional_data;
  

• bool edit_completed;
  

+};

+struct eficonfig_volume_entry_data {

• struct eficonfig_select_file_info *file_info;
  

• struct efi_simple_file_system_protocol *v;
  

• struct efi_device_path *dp;
  

+};

+struct eficonfig_file_entry_data {

• struct eficonfig_select_file_info *file_info;
  

• bool is_directory;
  

• u16 *file_name;
  

+};

+/**

• • eficonfig_print_msg() - print message
• • display the message to the user, user proceeds the screen
• • with ENTER key press.
• • @items: pointer to the structure of each menu entry
• • @count: the number of menu entry
• • @menu_header: pointer to the menu header string
• • Return: status code
• */

+void eficonfig_print_msg(char *msg)

None of you patches adds this function to an include. So it should be static.

OK.

My UEFI Secure Boot Key management UI series calls eficonfig_print_msg() from a different file(eficonfig_sbkey.c). So let me keep this function as global.

Thanks, Masahisa Kojima