Hi Heinrich,
On Mon, 14 Feb 2022 at 12:02, Masahisa Kojima
<masahisa.koj...@linaro.org> wrote:
>
> On Sun, 13 Feb 2022 at 18:58, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
> >
> > On 2/10/22 08:05, Masahisa Kojima wrote:
> > > This commit adds the menu-driven UEFI Boot Variable maintenance.
> > > User can add and delete the Boot#### variable, and update the
> > > BootOrder variable through menu operation.
> > >
> > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> > > ---
> > > lib/efi_loader/efi_bootmgr.c | 720 +++++++++++++++++++++++++++++++++++
> > > 1 file changed, 720 insertions(+)
> > >
> > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > > index 013d868f23..739140f742 100644
> > > --- a/lib/efi_loader/efi_bootmgr.c
> > > +++ b/lib/efi_loader/efi_bootmgr.c
> > > @@ -32,6 +32,8 @@ static const struct efi_runtime_services *rs;
> >
> > Where is the Kconfig entry to disable this code?
>
> The patch "[PATCH 1/3] efi_loader: add menu-driven boot device selection"[*1]
> has CONFIG_EFI_BOOT_MENU to enable/disable efi bootmenu.
>
> [*1]
> https://lore.kernel.org/u-boot/cadq0-x-qenpzscso6emw52tbvg40q1yydkbjfbs2bz0xxas...@mail.gmail.com/T/#u
>
> >
> > > */
> > >
> > > #define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
> > > +#define EFI_BOOTMGR_FILE_PATH_MAX 512
> > > +#define EFI_BOOTMGR_BOOT_NAME_MAX 64
> > >
> > > typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
> > >
> > > @@ -95,12 +97,49 @@ struct efi_bootmgr_boot_selection_data {
> > >
> > > static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool
> > > *exit);
> > > static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool
> > > *exit);
> > > +static efi_status_t efi_bootmgr_process_maintenance(void *data, bool
> > > *exit);
> > > +static efi_status_t efi_bootmgr_process_add_boot_option(void *data, bool
> > > *exit);
> > > +static efi_status_t efi_bootmgr_process_delete_boot_option(void *data,
> > > bool *exit);
> > > +static efi_status_t efi_bootmgr_process_change_boot_order(void *data,
> > > bool *exit);
> > >
> > > static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
> > > {u"Boot Manager", efi_bootmgr_process_boot_selection},
> > > + {u"Boot Manager maintenance", efi_bootmgr_process_maintenance},
> > > {u"Quit", NULL},
> > > };
> > >
> > > +static struct efi_bootmgr_menu_item maintenance_menu_items[] = {
> > > + {u"Add Boot Option", efi_bootmgr_process_add_boot_option},
> > > + {u"Delete Boot Option", efi_bootmgr_process_delete_boot_option},
> > > + {u"Change Boot Order", efi_bootmgr_process_change_boot_order},
> > > + {u"Quit", NULL},
> > > +};
> > > +
> > > +struct efi_bootmgr_boot_option {
> > > + struct efi_simple_file_system_protocol *current_volume;
> > > + struct efi_device_path *dp_volume;
> > > + u16 *current_path;
> > > + u16 *boot_name;
> > > + bool file_selected;
> > > +};
> > > +
> > > +static const struct efi_device_path END = {
> > > + .type = DEVICE_PATH_TYPE_END,
> > > + .sub_type = DEVICE_PATH_SUB_TYPE_END,
> > > + .length = sizeof(END),
> > > +};
> > > +
> > > +struct efi_bootmgr_volume_entry_data {
> > > + struct efi_bootmgr_boot_option *bo;
> > > + struct efi_simple_file_system_protocol *v;
> > > + struct efi_device_path *dp;
> > > +};
> > > +
> > > +struct efi_bootmgr_file_entry_data {
> > > + struct efi_bootmgr_boot_option *bo;
> > > + struct efi_file_info *f;
> > > +};
> > > +
> > > static void efi_bootmgr_menu_print_entry(void *data)
> > > {
> > > struct efi_bootmgr_menu_entry *entry = data;
> > > @@ -558,6 +597,687 @@ static efi_status_t
> > > efi_bootmgr_process_boot_selection(void *data, bool *exit)
> > > return ret;
> > > }
> > >
> > > +static efi_status_t efi_bootmgr_volume_selected(void *data, bool *exit)
> > > +{
> > > + struct efi_bootmgr_volume_entry_data *info = data;
> > > +
> > > + *exit = true;
> > > +
> > > + if (info) {
> > > + info->bo->current_volume = info->v;
> > > + info->bo->dp_volume = info->dp;
> > > + }
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_file_selected(void *data, bool *exit)
> > > +{
> > > + struct efi_bootmgr_file_entry_data *info = data;
> > > +
> > > + *exit = true;
> > > +
> > > + if (!info)
> > > + return EFI_INVALID_PARAMETER;
> > > +
> > > + if (u16_strncmp(info->f->file_name, u".", 1) == 0 &&
> > > + u16_strlen(info->f->file_name) == 1) {
> > > + /* stay current path */
> > > + } else if (u16_strncmp(info->f->file_name, u"..", 2) == 0 &&
> > > + u16_strlen(info->f->file_name) == 2) {
> > > + u32 i;
> > > + int len = u16_strlen(info->bo->current_path);
> > > +
> > > + for (i = len - 2; i > 0; i--) {
> > > + if (info->bo->current_path[i] == u'\\')
> > > + break;
> > > + }
> > > +
> > > + if (i == 0)
> > > + info->bo->current_path[0] = u'\0';
> > > + else
> > > + info->bo->current_path[i + 1] = u'\0';
> > > + } else {
> > > + size_t new_len;
> > > +
> > > + new_len = u16_strlen(info->bo->current_path) +
> > > + u16_strlen(info->f->file_name) + 1;
> > > + if (new_len >= EFI_BOOTMGR_FILE_PATH_MAX) {
> >
> > Why do we need such an arbitrary limitation? Please, allocate a buffer
> > of adequate size.
>
> I agree, but it is better to have the maximum path size limitation.
> I currently followed efi_device_path_to_text.c implementation, there is
> following definition regarding the maximum size.
>
> #define MAX_NODE_LEN 512
>
> https://github.com/u-boot/u-boot/blob/master/lib/efi_loader/efi_device_path_to_text.c#L16
>
> In my understanding, current file path max in efi subsystem is
> defined by the above MAX_NODE_LEN.
>
> >
> > > + /* TODO: show error notification to user */
> > > + log_err("file path is too long\n");
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > + u16_strcat(info->bo->current_path, info->f->file_name);
> >
> > I would prefer to use a safe function here where the destination buffer
> > length is an argument.
>
> Yes, I will use u16_strcat_s().
>
> >
> >
> > > + if (info->f->attribute & EFI_FILE_DIRECTORY) {
> > > + if (new_len + 1 >= EFI_BOOTMGR_FILE_PATH_MAX) {
> >
> > Please, remove this duplicate test and fix the test above.
>
> I think these checks are not duplicated.
> If the selected path is directory, '\\' must be added at the last of
> the current_path[].
>
> >
> > > + log_err("file path is too long\n");
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > + u16_strcat(info->bo->current_path, u"\\");
> > > + } else {
> > > + info->bo->file_selected = true;
> > > + }
> > > + }
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_select_volume(struct
> > > efi_bootmgr_boot_option *bo)
> > > +{
> > > + u16 *name;
> > > + u32 i;
> > > + efi_status_t ret;
> > > + efi_uintn_t count;
> > > + struct efi_device_path *device_path;
> > > + efi_handle_t *volume_handles = NULL;
> > > + struct efi_simple_file_system_protocol *v;
> > > + struct efi_device_path_to_text_protocol *text;
> > > + struct efi_bootmgr_menu_item *menu_item, *iter;
> > > +
> > > + ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> > > &efi_system_partition_guid,
> >
> > We have too many EFI_CALLs. Factor out a function
> > efi_locate_handle_buffer_int() which you can call without EFI_CALL and
> > remove all of the existing EFI_CALLs.
I'm going to send v2 RFC path(sorry for not including "RFC" to this
patch series).
I newly create or expose existing xxx_int() function can be called
from inside of
U-Boot. The target functions are as follows.
- 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().
The next v2 RFC patch still contains EFI_CALLs for device_path_to_text_protocol
and text_input/output_protocol, I'm not sure I should prepare
xxx_int() functions
for these services.
>
> OK, I will newly create efi_locate_handle_buffer_int() function.
>
> >
> > > + NULL, &count,
> > > + (efi_handle_t
> > > **)&volume_handles));
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> >
> > What will you do if you get multiple results?
>
>
> If I understand this comment correctly, at the later for loop, open
> all efi system
> partitions get from efi_locate_handle_buffer() by calling
> systab.boottime->open_protocol().
>
> >
> > > +
> > > + ret =
> > > EFI_CALL(systab.boottime->locate_protocol(&efi_guid_device_path_to_text_protocol,
> > > + NULL, (void
> > > **)&text));
> >
> > This will give you a random instance of a device path and not the one
> > related to the ESP. You just called LocateHandleBuffer() for good
> > reason. Please, use efi_search_protocol().
>
> Thank you, I will use efi_search_protocol().
>
> >
> > > + if (ret != EFI_SUCCESS)
> > > + goto out1;
> > > +
> > > + menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
> > > + if (!menu_item) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out1;
> > > + }
> > > +
> > > + iter = menu_item;
> > > + for (i = 0; i < count; i++) {
> > > + struct efi_bootmgr_volume_entry_data *info;
> > > +
> > > + ret =
> > > EFI_CALL(systab.boottime->open_protocol(volume_handles[i],
> > > +
> > > &efi_simple_file_system_protocol_guid,
> > > + (void **)&v, efi_root, NULL,
> > > +
> > > EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > > + if (ret != EFI_SUCCESS)
> > > + continue;
> > > +
> > > + ret =
> > > EFI_CALL(systab.boottime->open_protocol(volume_handles[i],
> > > + &efi_guid_device_path,
> > > + (void **)&device_path,
> > > efi_root, NULL,
> > > +
> > > EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > > + if (ret != EFI_SUCCESS)
> > > + continue;
> > > +
> > > + name = text->convert_device_path_to_text(device_path, true,
> > > true);
> > > + if (!name) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out2;
> > > + }
> > > +
> > > + info = calloc(1, sizeof(struct
> > > efi_bootmgr_volume_entry_data));
> > > + if (!info) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out2;
> > > + }
> > > +
> > > + info->v = v;
> > > + info->dp = device_path;
> > > + info->bo = bo;
> > > + iter->title = name;
> > > + iter->func = efi_bootmgr_volume_selected;
> > > + iter->data = info;
> > > + iter++;
> > > + }
> > > +
> > > + iter->title = u"Quit";
> > > + iter->func = NULL;
> > > + iter->data = NULL;
> > > + count += 1;
> > > +
> > > + ret = efi_bootmgr_process_common(menu_item, count, false);
> > > +
> > > +out2:
> > > + iter = menu_item;
> > > + for (i = 0; i < count - 1; i++) {
> > > + struct efi_bootmgr_volume_entry_data *p;
> > > +
> > > + p = (struct efi_bootmgr_volume_entry_data *)(iter->data);
> > > + efi_free_pool(iter->title);
> > > + free(p);
> > > + iter++;
> > > + }
> > > +
> > > + free(menu_item);
> > > +
> > > +out1:
> > > + efi_free_pool(volume_handles);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_select_file(struct
> > > efi_bootmgr_boot_option *bo,
> > > + struct efi_file_handle *root)
> > > +{
> > > + char *buf;
> > > + u32 i;
> > > + char *dir_buf;
> > > + efi_uintn_t len;
> > > + efi_status_t ret;
> > > + efi_uintn_t size;
> > > + u32 count = 0;
> > > + struct efi_file_handle *f;
> > > + struct efi_file_info *ptr;
> > > + struct efi_bootmgr_menu_item *menu_item, *iter;
> > > +
> > > + buf = calloc(1, EFI_BOOTMGR_FILE_PATH_MAX);
> > > + if (!buf)
> > > + return EFI_OUT_OF_RESOURCES;
> > > +
> > > + while (!bo->file_selected) {
> > > + size = 0;
> > > + count = 0;
> > > +
> > > + ret = EFI_CALL(root->open(root, &f, bo->current_path,
> > > + EFI_FILE_MODE_READ, 0));
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + /* calculate directory information total size */
> > > + for (;;) {
> > > + len = EFI_BOOTMGR_FILE_PATH_MAX;
> > > + ret = EFI_CALL(f->read(f, &len, buf));
> > > + if (ret != EFI_SUCCESS || len == 0)
> > > + break;
> > > +
> > > + size += len;
> > > + count++;
> > > + }
> > > +
> > > + dir_buf = calloc(1, size);
> > > + if (!dir_buf) {
> > > + EFI_CALL(f->close(f));
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out;
> > > + }
> > > + menu_item = calloc(count + 1, sizeof(struct
> > > efi_bootmgr_menu_item));
> > > + if (!menu_item) {
> > > + EFI_CALL(f->close(f));
> > > + free(dir_buf);
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out;
> > > + }
> > > +
> > > + /* read directory and construct menu structure */
> > > + f->setpos(f, 0);
> > > + iter = menu_item;
> > > + ptr = (struct efi_file_info *)dir_buf;
> > > + for (i = 0; i < count; i++) {
> > > + int name_len;
> > > + u16 *name;
> > > + struct efi_bootmgr_file_entry_data *info;
> > > +
> > > + len = size;
> > > + ret = EFI_CALL(f->read(f, &len, ptr));
> > > + if (ret != EFI_SUCCESS || len == 0)
> > > + goto err;
> > > +
> > > + if (ptr->attribute & EFI_FILE_DIRECTORY) {
> > > + /* append u'/' at the end of directory name
> > > */
> > > + name_len = u16_strsize(ptr->file_name) +
> > > sizeof(u16);
> > > + name = calloc(1, name_len);
> > > + if (!name) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto err;
> > > + }
> > > + u16_strcpy(name, ptr->file_name);
> > > + name[u16_strlen(ptr->file_name)] = u'/';
> > > + } else {
> > > + name_len = u16_strsize(ptr->file_name);
> > > + name = calloc(1, name_len);
> > > + if (!name) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto err;
> > > + }
> > > + u16_strcpy(name, ptr->file_name);
> > > + }
> > > +
> > > + info = calloc(1, sizeof(struct
> > > efi_bootmgr_file_entry_data));
> > > + if (!info) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto err;
> > > + }
> > > + info->f = ptr;
> > > + info->bo = bo;
> > > + iter->title = name;
> > > + iter->func = efi_bootmgr_file_selected;
> > > + iter->data = info;
> > > + iter++;
> > > +
> > > + size -= len;
> > > + ptr = (struct efi_file_info *)((char *)ptr + len);
> > > + }
> > > +
> > > + /* add "Quit" entry */
> > > + iter->title = u"Quit";
> > > + iter->func = NULL;
> > > + iter->data = NULL;
> > > + count += 1;
> > > +
> > > + ret = efi_bootmgr_process_common(menu_item, count, false);
> > > +err:
> > > + EFI_CALL(f->close(f));
> > > + iter = menu_item;
> > > + for (i = 0; i < count - 1; i++, iter++) {
> > > + free(iter->title);
> > > + free(iter->data);
> > > + }
> > > +
> > > + free(dir_buf);
> > > + free(menu_item);
> > > +
> > > + if (ret != EFI_SUCCESS)
> > > + break;
> > > + }
> > > +
> > > +out:
> > > + free(buf);
> > > + return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_boot_add_enter_name(struct
> > > efi_bootmgr_boot_option *bo)
> > > +{
> > > + int c;
> > > + int len = 0;
> > > + char name[EFI_BOOTMGR_BOOT_NAME_MAX] = {0};
> > > +
> > > + puts(ANSI_CLEAR_CONSOLE);
> >
> > This does not work for all consoles.
The v2 patch still relies on ANSI_CLEAR_CONSOLE, I don't come up with
the alternative solution yet.
Thanks,
Masahisa Kojima
>
> OK.
> Is it better to use ANSI_CLEAR_LINE instead of using
> ANSI_CLEAR_CONSOLE?
>
> >
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 1, 1);
> > > + puts(ANSI_CLEAR_LINE);
> >
> > Why? You just cleared the whole screen.
>
> OK.
>
> >
> > > + printf(ANSI_CURSOR_POSITION, 2, 1);
> >
> >
> >
> > > + puts(" *** U-Boot EFI Boot Manager Menu ***");
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> >
> > Superfluous
>
> OK.
>
> >
> > > + printf(ANSI_CURSOR_POSITION, 3, 1);
> >
> > Printing '\n' is good enough to get to the next line. Just add it to the
> > previous output statement.
>
> OK.
>
> >
> > > + puts(ANSI_CLEAR_LINE);
> > > + printf(ANSI_CURSOR_POSITION, 4, 1);
> > > + puts(" enter name:");
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 8, 1);
> > > + puts(ANSI_CLEAR_LINE);
> > > + puts(" ENTER to complete, ESC/CTRL+C to quit");
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 4, 15);
> > > + puts(ANSI_CURSOR_SHOW);
> > > +
> > > + for (;;) {
> > > + while (!tstc()) {
> > > + WATCHDOG_RESET();
> > > + mdelay(10);
> > > + }
> > > +
> > > + c = getchar();
> > > +
> > > + if ((c == ' ') || (('0' <= c) && (c <= '9')) ||
> > > + (('A' <= c) && (c <= 'Z')) || (('a' <= c) && (c <=
> > > 'z'))) {
> >
> > Why shouldn't I enter 'UEFIは素晴らしいです'? Expect UTF-8 here.
>
> OK, I will support utf16 user input.
>
> >
> > Isn't there a string entry function yet in U-Boot? Anyway this whole
> > loop should be in a library function.
>
> I could only find "Yes or No" user input handling in U-Boot.
> Anyway, I will add string entry function in a library function.
>
> Thanks,
> Masahisa Kojima
>
> >
> > Best regards
> >
> > Heinrich
> >
> > > + if (len >= (EFI_BOOTMGR_BOOT_NAME_MAX - 1))
> > > + continue;
> > > +
> > > + name[len] = (char)c;
> > > + len++;
> > > + printf(ANSI_CURSOR_POSITION, 4, 15);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > + printf("%s", name);
> > > + } else if (c == '\b') {
> > > + if (len > 0)
> > > + name[--len] = '\0';
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 4, 15);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > + printf("%s", name);
> > > + } else if (c == '\r') {
> > > + u16 *p;
> > > +
> > > + name[len] = '\0';
> > > + p = bo->boot_name;
> > > + utf8_utf16_strncpy(&p, name, len);
> > > + return EFI_SUCCESS;
> > > + } else if (c == 0x3) {
> > > + return EFI_ABORTED;
> > > + } else if (c == '\e') { /* TODO: correctly handle escape
> > > sequence */
> > > + return EFI_ABORTED;
> > > + }
> > > + }
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_change_boot_order(int selected, int max,
> > > int *new)
> > > +{
> > > + int c;
> > > + int len = 0;
> > > + char new_order[6] = {0};
> > > +
> > > + puts(ANSI_CLEAR_CONSOLE);
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 1, 1);
> > > + puts(ANSI_CLEAR_LINE);
> > > + printf(ANSI_CURSOR_POSITION, 2, 1);
> > > + puts(" *** U-Boot EFI Boot Manager Menu ***");
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > + printf(ANSI_CURSOR_POSITION, 3, 1);
> > > + puts(ANSI_CLEAR_LINE);
> > > + printf(ANSI_CURSOR_POSITION, 4, 1);
> > > + printf(" current boot order : %d", selected);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 5, 1);
> > > + puts(ANSI_CLEAR_LINE);
> > > + printf(ANSI_CURSOR_POSITION, 6, 1);
> > > + printf(" new boot order(0 - %4d): ", max);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 8, 1);
> > > + puts(ANSI_CLEAR_LINE);
> > > + puts(" ENTER to complete, ESC/CTRL+C to quit");
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 6, 29);
> > > + puts(ANSI_CURSOR_SHOW);
> > > +
> > > + for (;;) {
> > > + while (!tstc()) {
> > > + WATCHDOG_RESET();
> > > + mdelay(10);
> > > + }
> > > +
> > > + c = getchar();
> > > +
> > > + if ('0' <= c && c <= '9') {
> > > + if (len >= 5)
> > > + continue;
> > > +
> > > + new_order[len] = (char)c;
> > > + len++;
> > > + printf(ANSI_CURSOR_POSITION, 6, 29);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > + printf("%s", new_order);
> > > + } else if (c == '\b') {
> > > + if (len > 0)
> > > + new_order[--len] = '\0';
> > > +
> > > + printf(ANSI_CURSOR_POSITION, 6, 29);
> > > + puts(ANSI_CLEAR_LINE_TO_END);
> > > + printf("%s", new_order);
> > > + } else if (c == '\r') {
> > > + int i;
> > > + int val = 0;
> > > +
> > > + for (i = 0; i < len; i++)
> > > + val = (val * 10) + (new_order[i] - '0');
> > > +
> > > + if (val > max) /* TODO: show error notification */
> > > + continue;
> > > +
> > > + *new = val;
> > > + return EFI_SUCCESS;
> > > + } else if (c == 0x3) {
> > > + return EFI_ABORTED;
> > > + } else if (c == '\e') { /* TODO: correctly handle escape
> > > sequence */
> >
> > If you patch is not complete, please mark it as RFC.
> >
> > You have to handle Unicode letters like は.
> >
> > > + return EFI_ABORTED;
> > > + }
> > > + }
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_select_file_handler(struct
> > > efi_bootmgr_boot_option *bo)
> > > +{
> > > + efi_status_t ret;
> > > + struct efi_file_handle *root;
> > > +
> > > + bo->file_selected = false;
> > > +
> > > + while (!bo->file_selected) {
> > > + bo->current_volume = NULL;
> > > + memset(bo->current_path, 0, sizeof(bo->current_path));
> > > +
> > > + ret = efi_bootmgr_select_volume(bo);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > +
> > > + if (!bo->current_volume)
> > > + return EFI_INVALID_PARAMETER;
> > > +
> > > + ret =
> > > EFI_CALL(bo->current_volume->open_volume(bo->current_volume, &root));
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + ret = efi_bootmgr_select_file(bo, root);
> > > +
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > + }
> > > +
> > > + ret = efi_bootmgr_boot_add_enter_name(bo);
> > > +
> > > +out:
> > > + return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_process_maintenance(void *data, bool
> > > *exit)
> > > +{
> > > + return efi_bootmgr_process_common(maintenance_menu_items,
> > > +
> > > ARRAY_SIZE(maintenance_menu_items),
> > > + false);
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_process_add_boot_option(void *data, bool
> > > *exit)
> > > +{
> > > + u32 index;
> > > + void *p = NULL;
> > > + char *buf = NULL;
> > > + efi_status_t ret;
> > > + char *iter = NULL;
> > > + u16 var_name[9];
> > > + u16 *bootorder = NULL;
> > > + u16 *new_bootorder = NULL;
> > > + struct efi_load_option lo;
> > > + efi_uintn_t dp_size, fp_size;
> > > + efi_uintn_t last, size, new_size;
> > > + struct efi_bootmgr_boot_option bo;
> > > + struct efi_device_path_file_path *fp;
> > > +
> > > + /* get unused Boot#### */
> > > + for (index = 0; index <= 0xFFFF; index++) {
> > > + size = 0;
> > > + efi_create_indexed_name(var_name, sizeof(var_name), "Boot",
> > > index);
> > > + ret = efi_get_variable_int(var_name,
> > > &efi_global_variable_guid,
> > > + NULL, &size, NULL, NULL);
> > > + if (ret == EFI_BUFFER_TOO_SMALL)
> > > + continue;
> > > + else
> > > + break;
> > > + }
> > > +
> > > + if (index >= 0xFFFF)
> > > + return EFI_OUT_OF_RESOURCES;
> > > +
> > > + efi_create_indexed_name(var_name, sizeof(var_name), "Boot", index);
> > > +
> > > + bo.current_path = calloc(1, EFI_BOOTMGR_FILE_PATH_MAX);
> > > + if (!bo.current_path)
> > > + goto out;
> > > +
> > > + bo.boot_name = calloc(1, EFI_BOOTMGR_BOOT_NAME_MAX * sizeof(u16));
> > > + if (!bo.boot_name)
> > > + goto out;
> > > +
> > > + ret = efi_bootmgr_select_file_handler(&bo);
> > > + if (ret == EFI_ABORTED)
> > > + goto out;
> > > +
> > > + dp_size = efi_dp_size(bo.dp_volume);
> > > + fp_size = sizeof(struct efi_device_path) +
> > > + ((u16_strlen(bo.current_path) + 1) * sizeof(u16));
> > > + buf = calloc(1, dp_size + fp_size + sizeof(END));
> > > + if (!buf)
> > > + goto out;
> > > +
> > > + iter = buf;
> > > + memcpy(iter, bo.dp_volume, dp_size);
> > > + iter += dp_size;
> > > +
> > > + fp = (struct efi_device_path_file_path *)iter;
> > > + fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> > > + fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
> > > + fp->dp.length = (u16)fp_size;
> > > + u16_strcpy(fp->str, bo.current_path);
> > > + iter += fp_size;
> > > + *((struct efi_device_path *)iter) = END;
> > > +
> > > + lo.file_path = (struct efi_device_path *)buf;
> > > + lo.file_path_length = efi_dp_size((struct efi_device_path *)buf) +
> > > sizeof(END);
> > > + lo.attributes = LOAD_OPTION_ACTIVE;
> > > + lo.optional_data = NULL;
> > > + lo.label = bo.boot_name;
> > > +
> > > + size = efi_serialize_load_option(&lo, (u8 **)&p);
> > > + if (!size) {
> > > + ret = EFI_INVALID_PARAMETER;
> > > + goto out;
> > > + }
> > > +
> > > + ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> > > + EFI_VARIABLE_NON_VOLATILE |
> > > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > + EFI_VARIABLE_RUNTIME_ACCESS,
> > > + size, p, false);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > +
> > > + /* append new boot option */
> > > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > &size);
> > > + last = size / sizeof(u16);
> > > + new_size = size + sizeof(u16);
> > > + new_bootorder = calloc(1, new_size);
> > > + if (!new_bootorder) {
> > > + ret = EFI_OUT_OF_RESOURCES;
> > > + goto out;
> > > + }
> > > + memcpy(new_bootorder, bootorder, size);
> > > + new_bootorder[last] = (u16)index;
> > > +
> > > + ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> > > + EFI_VARIABLE_NON_VOLATILE |
> > > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > + EFI_VARIABLE_RUNTIME_ACCESS,
> > > + new_size, new_bootorder, false);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > +
> > > +out:
> > > + free(p);
> > > + free(buf);
> > > + free(bootorder);
> > > + free(new_bootorder);
> > > + free(bo.boot_name);
> > > + free(bo.current_path);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_process_delete_boot_option(void *data,
> > > bool *exit)
> > > +{
> > > + int selected;
> > > + u16 *bootorder;
> > > + u16 var_name[9];
> > > + efi_status_t ret;
> > > + efi_uintn_t num, size;
> > > +
> > > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > &size);
> > > + if (!bootorder) {
> > > + ret = EFI_NOT_FOUND;
> > > + return ret;
> > > + }
> > > +
> > > + num = size / sizeof(u16);
> > > + ret = efi_bootmgr_show_boot_selection(bootorder, num, &selected);
> > > + if (ret == EFI_SUCCESS) {
> > > + /* delete selected boot option */
> > > + efi_create_indexed_name(var_name, sizeof(var_name),
> > > + "Boot", bootorder[selected]);
> > > + ret = efi_set_variable_int(var_name,
> > > &efi_global_variable_guid,
> > > + 0, 0, NULL, false);
> > > + if (ret != EFI_SUCCESS) {
> > > + log_err("delete boot option(%ls) failed\n",
> > > var_name);
> > > + goto out;
> > > + }
> > > +
> > > + /* update BootOrder */
> > > + memmove(&bootorder[selected], &bootorder[selected + 1],
> > > + (num - selected - 1) * sizeof(u16));
> > > + size -= sizeof(u16);
> > > + ret = efi_set_variable_int(u"BootOrder",
> > > &efi_global_variable_guid,
> > > + EFI_VARIABLE_NON_VOLATILE |
> > > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > + EFI_VARIABLE_RUNTIME_ACCESS,
> > > + size, bootorder, false);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > + }
> > > +
> > > +out:
> > > + free(bootorder);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmgr_process_change_boot_order(void *data,
> > > bool *exit)
> > > +{
> > > + int selected;
> > > + int new_order;
> > > + efi_status_t ret;
> > > + efi_uintn_t num, size;
> > > + u16 *bootorder = NULL;
> > > + u16 *new_bootorder = NULL;
> > > +
> > > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > &size);
> > > + if (!bootorder)
> > > + return EFI_NOT_FOUND;
> > > +
> > > + num = size / sizeof(u16);
> > > + ret = efi_bootmgr_show_boot_selection(bootorder, num, &selected);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > +
> > > + ret = efi_bootmgr_change_boot_order(selected, num - 1, &new_order);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > +
> > > + new_bootorder = calloc(1, size);
> > > + if (!new_bootorder)
> > > + goto out;
> > > +
> > > + memcpy(new_bootorder, bootorder, size);
> > > + if (selected > new_order) {
> > > + new_bootorder[new_order] = bootorder[selected];
> > > + memcpy(&new_bootorder[new_order + 1], &bootorder[new_order],
> > > + (selected - new_order) * sizeof(u16));
> > > + } else if (selected < new_order) {
> > > + new_bootorder[new_order] = bootorder[selected];
> > > + memcpy(&new_bootorder[selected], &bootorder[selected + 1],
> > > + (new_order - selected) * sizeof(u16));
> > > + } else {
> > > + /* nothing to change */
> >
> > You should skip SetVariable() if you aren't changing anything.
> >
> > Best regards
> >
> > Heinrich
> >
> > > + }
> > > + ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> > > + EFI_VARIABLE_NON_VOLATILE |
> > > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > + EFI_VARIABLE_RUNTIME_ACCESS,
> > > + size, new_bootorder, false);
> > > + free(new_bootorder);
> > > +out:
> > > + free(bootorder);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > /**
> > > * try_load_entry() - try to load image for boot option
> > > *
> >
