Re: [PATCH v5 09/17] bootmenu: add UEFI boot entry into bootmenu

2022-05-09 Thread Masahisa Kojima
Hi Heinrich,

On Mon, 2 May 2022 at 06:44, Heinrich Schuchardt  wrote:
>
> On 4/28/22 10:09, Masahisa Kojima wrote:
> > This commit adds the UEFI related menu entries
> > into the bootmenu.
> >
> > User can select which UEFI "Boot" option to execute
> > from bootmenu, then bootmenu sets the "BootNext" UEFI
> > variable and invoke efi bootmgr. The efi bootmgr
> > will handle the "BootNext" UEFI variable.
> >
> > If the "BootNext" UEFI variable is preset and efi bootmgr is enabled,
> > bootmenu invokes efi bootmgr to handle "BootNext" as first priority.
> >
> > The UEFI boot entry has the "UEFI BOOT" prefix as below.
>
> This prefix provides no value.
>
> >
> >*** U-Boot Boot Menu ***
> >
> >   UEFI BOOT : debian
> >   UEFI BOOT0001 : ubuntu
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> > Changes in v5:
> > - split into the separate patch
> > - add function description comment
> > - remove non-volatile attribute for BootNext variable to minimize
> >the access to the non-volatile storage
> >
> >   cmd/bootmenu.c | 155 -
> >   1 file changed, 154 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > index 15ad621c9f..da688e6213 100644
> > --- a/cmd/bootmenu.c
> > +++ b/cmd/bootmenu.c
> > @@ -7,6 +7,8 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -28,6 +30,7 @@
> >   enum boot_type {
> >   BOOTMENU_TYPE_NONE = 0,
> >   BOOTMENU_TYPE_BOOTMENU,
> > + BOOTMENU_TYPE_UEFI_BOOT_OPTION,
> >   };
> >
> >   struct bootmenu_entry {
> > @@ -85,6 +88,8 @@ static void bootmenu_print_entry(void *data)
> >
> >   if (entry->type == BOOTMENU_TYPE_BOOTMENU)
> >   printf("bootmenu_%02d   : %ls", entry->bootorder, 
> > entry->title);
> > + else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
> > + printf("UEFI BOOT%04X : %ls", entry->bootorder, entry->title);
>
> Please, remove this hunk.

Thank you for requesting to merge this patch into u-boot/master.
You have removed the "UEFI BOOT" prefix from
the bootmenu title, but the commit message still says that there is
"UEFI BOOT" prefix for UEFI entries.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> >   else
> >   printf("%ls", entry->title);
> >
> > @@ -371,6 +376,95 @@ static int prepare_bootmenu_entry(struct bootmenu_data 
> > *menu,
> >   return 1;
> >   }
> >
> > +/**
> > + * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries
> > + *
> > + * This function read the "BootOrder" UEFI variable
> > + * and generate the bootmenu entries in the order of "BootOrder".
> > + *
> > + * @menu:pointer to the bootmenu structure
> > + * @current: pointer to the last bootmenu entry list
> > + * @index:   pointer to the index of the last bootmenu entry,
> > + *   the number of uefi entry is added by this function
> > + * Return:   1 on success, negative value on error
> > + */
> > +static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
> > + struct bootmenu_entry **current,
> > + unsigned short int *index)
> > +{
> > + u16 *bootorder;
> > + efi_status_t ret;
> > + unsigned short j;
> > + efi_uintn_t num, size;
> > + void *load_option;
> > + struct efi_load_option lo;
> > + u16 varname[] = u"Boot";
> > + unsigned short int i = *index;
> > + struct bootmenu_entry *entry = NULL;
> > + struct bootmenu_entry *iter = *current;
> > +
> > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, 
> > &size);
> > + if (!bootorder)
> > + return -ENOENT;
> > +
> > + num = size / sizeof(u16);
> > + for (j = 0; j < num; j++) {
> > + entry = malloc(sizeof(struct bootmenu_entry));
> > + if (!entry)
> > + return -ENOMEM;
> > +
> > + efi_create_indexed_name(varname, sizeof(varname),
> > + "Boot", bootorder[j]);
> > + load_option = efi_get_var(varname, &efi_global_variable_guid, 
> > &size);
> > + if (!load_option)
> > + continue;
> > +
> > + ret = efi_deserialize_load_option(&lo, load_option, &size);
> > + if (ret != EFI_SUCCESS) {
> > + log_warning("Invalid load option for %ls\n", varname);
> > + free(load_option);
> > + free(entry);
> > + continue;
> > + }
> > +
> > + if (lo.attributes & LOAD_OPTION_ACTIVE) {
> > + entry->title = u16_strdup(lo.label);
> > + if (!entry->title) {
> > + free(load_option);
> > + free(entry);
> > + free(bootorder);
> > + 

Re: [PATCH v5 09/17] bootmenu: add UEFI boot entry into bootmenu

2022-05-01 Thread Heinrich Schuchardt

On 4/28/22 10:09, Masahisa Kojima wrote:

This commit adds the UEFI related menu entries
into the bootmenu.

User can select which UEFI "Boot" option to execute
from bootmenu, then bootmenu sets the "BootNext" UEFI
variable and invoke efi bootmgr. The efi bootmgr
will handle the "BootNext" UEFI variable.

If the "BootNext" UEFI variable is preset and efi bootmgr is enabled,
bootmenu invokes efi bootmgr to handle "BootNext" as first priority.

The UEFI boot entry has the "UEFI BOOT" prefix as below.


This prefix provides no value.



   *** U-Boot Boot Menu ***

  UEFI BOOT : debian
  UEFI BOOT0001 : ubuntu

Signed-off-by: Masahisa Kojima 
---
Changes in v5:
- split into the separate patch
- add function description comment
- remove non-volatile attribute for BootNext variable to minimize
   the access to the non-volatile storage

  cmd/bootmenu.c | 155 -
  1 file changed, 154 insertions(+), 1 deletion(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 15ad621c9f..da688e6213 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -7,6 +7,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  #include 
  #include 
  #include 
@@ -28,6 +30,7 @@
  enum boot_type {
BOOTMENU_TYPE_NONE = 0,
BOOTMENU_TYPE_BOOTMENU,
+   BOOTMENU_TYPE_UEFI_BOOT_OPTION,
  };

  struct bootmenu_entry {
@@ -85,6 +88,8 @@ static void bootmenu_print_entry(void *data)

if (entry->type == BOOTMENU_TYPE_BOOTMENU)
printf("bootmenu_%02d   : %ls", entry->bootorder, entry->title);
+   else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
+   printf("UEFI BOOT%04X : %ls", entry->bootorder, entry->title);


Please, remove this hunk.

Best regards

Heinrich


else
printf("%ls", entry->title);

@@ -371,6 +376,95 @@ static int prepare_bootmenu_entry(struct bootmenu_data 
*menu,
return 1;
  }

+/**
+ * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries
+ *
+ * This function read the "BootOrder" UEFI variable
+ * and generate the bootmenu entries in the order of "BootOrder".
+ *
+ * @menu:  pointer to the bootmenu structure
+ * @current:   pointer to the last bootmenu entry list
+ * @index: pointer to the index of the last bootmenu entry,
+ * the number of uefi entry is added by this function
+ * Return: 1 on success, negative value on error
+ */
+static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
+   struct bootmenu_entry **current,
+   unsigned short int *index)
+{
+   u16 *bootorder;
+   efi_status_t ret;
+   unsigned short j;
+   efi_uintn_t num, size;
+   void *load_option;
+   struct efi_load_option lo;
+   u16 varname[] = u"Boot";
+   unsigned short int i = *index;
+   struct bootmenu_entry *entry = NULL;
+   struct bootmenu_entry *iter = *current;
+
+   bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+   if (!bootorder)
+   return -ENOENT;
+
+   num = size / sizeof(u16);
+   for (j = 0; j < num; j++) {
+   entry = malloc(sizeof(struct bootmenu_entry));
+   if (!entry)
+   return -ENOMEM;
+
+   efi_create_indexed_name(varname, sizeof(varname),
+   "Boot", bootorder[j]);
+   load_option = efi_get_var(varname, &efi_global_variable_guid, 
&size);
+   if (!load_option)
+   continue;
+
+   ret = efi_deserialize_load_option(&lo, load_option, &size);
+   if (ret != EFI_SUCCESS) {
+   log_warning("Invalid load option for %ls\n", varname);
+   free(load_option);
+   free(entry);
+   continue;
+   }
+
+   if (lo.attributes & LOAD_OPTION_ACTIVE) {
+   entry->title = u16_strdup(lo.label);
+   if (!entry->title) {
+   free(load_option);
+   free(entry);
+   free(bootorder);
+   return -ENOMEM;
+   }
+   entry->command = strdup("bootefi bootmgr");
+   sprintf(entry->key, "%d", i);
+   entry->num = i;
+   entry->menu = menu;
+   entry->type = BOOTMENU_TYPE_UEFI_BOOT_OPTION;
+   entry->bootorder = bootorder[j];
+   entry->next = NULL;
+
+   if (!iter)
+   menu->first = entry;
+   else
+   iter->next = entry;
+
+   iter = entry;
+   i++;
+   }
+
+   

[PATCH v5 09/17] bootmenu: add UEFI boot entry into bootmenu

2022-04-28 Thread Masahisa Kojima
This commit adds the UEFI related menu entries
into the bootmenu.

User can select which UEFI "Boot" option to execute
from bootmenu, then bootmenu sets the "BootNext" UEFI
variable and invoke efi bootmgr. The efi bootmgr
will handle the "BootNext" UEFI variable.

If the "BootNext" UEFI variable is preset and efi bootmgr is enabled,
bootmenu invokes efi bootmgr to handle "BootNext" as first priority.

The UEFI boot entry has the "UEFI BOOT" prefix as below.

  *** U-Boot Boot Menu ***

 UEFI BOOT : debian
 UEFI BOOT0001 : ubuntu

Signed-off-by: Masahisa Kojima 
---
Changes in v5:
- split into the separate patch
- add function description comment
- remove non-volatile attribute for BootNext variable to minimize
  the access to the non-volatile storage

 cmd/bootmenu.c | 155 -
 1 file changed, 154 insertions(+), 1 deletion(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 15ad621c9f..da688e6213 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -7,6 +7,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -28,6 +30,7 @@
 enum boot_type {
BOOTMENU_TYPE_NONE = 0,
BOOTMENU_TYPE_BOOTMENU,
+   BOOTMENU_TYPE_UEFI_BOOT_OPTION,
 };
 
 struct bootmenu_entry {
@@ -85,6 +88,8 @@ static void bootmenu_print_entry(void *data)
 
if (entry->type == BOOTMENU_TYPE_BOOTMENU)
printf("bootmenu_%02d   : %ls", entry->bootorder, entry->title);
+   else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
+   printf("UEFI BOOT%04X : %ls", entry->bootorder, entry->title);
else
printf("%ls", entry->title);
 
@@ -371,6 +376,95 @@ static int prepare_bootmenu_entry(struct bootmenu_data 
*menu,
return 1;
 }
 
+/**
+ * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries
+ *
+ * This function read the "BootOrder" UEFI variable
+ * and generate the bootmenu entries in the order of "BootOrder".
+ *
+ * @menu:  pointer to the bootmenu structure
+ * @current:   pointer to the last bootmenu entry list
+ * @index: pointer to the index of the last bootmenu entry,
+ * the number of uefi entry is added by this function
+ * Return: 1 on success, negative value on error
+ */
+static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
+   struct bootmenu_entry **current,
+   unsigned short int *index)
+{
+   u16 *bootorder;
+   efi_status_t ret;
+   unsigned short j;
+   efi_uintn_t num, size;
+   void *load_option;
+   struct efi_load_option lo;
+   u16 varname[] = u"Boot";
+   unsigned short int i = *index;
+   struct bootmenu_entry *entry = NULL;
+   struct bootmenu_entry *iter = *current;
+
+   bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+   if (!bootorder)
+   return -ENOENT;
+
+   num = size / sizeof(u16);
+   for (j = 0; j < num; j++) {
+   entry = malloc(sizeof(struct bootmenu_entry));
+   if (!entry)
+   return -ENOMEM;
+
+   efi_create_indexed_name(varname, sizeof(varname),
+   "Boot", bootorder[j]);
+   load_option = efi_get_var(varname, &efi_global_variable_guid, 
&size);
+   if (!load_option)
+   continue;
+
+   ret = efi_deserialize_load_option(&lo, load_option, &size);
+   if (ret != EFI_SUCCESS) {
+   log_warning("Invalid load option for %ls\n", varname);
+   free(load_option);
+   free(entry);
+   continue;
+   }
+
+   if (lo.attributes & LOAD_OPTION_ACTIVE) {
+   entry->title = u16_strdup(lo.label);
+   if (!entry->title) {
+   free(load_option);
+   free(entry);
+   free(bootorder);
+   return -ENOMEM;
+   }
+   entry->command = strdup("bootefi bootmgr");
+   sprintf(entry->key, "%d", i);
+   entry->num = i;
+   entry->menu = menu;
+   entry->type = BOOTMENU_TYPE_UEFI_BOOT_OPTION;
+   entry->bootorder = bootorder[j];
+   entry->next = NULL;
+
+   if (!iter)
+   menu->first = entry;
+   else
+   iter->next = entry;
+
+   iter = entry;
+   i++;
+   }
+
+   free(load_option);
+
+   if (i == MAX_COUNT - 1)
+   break;
+   }
+
+   free(bootorder);
+   *in