Comment 1 Neel Chauhan 2018-08-29 21:46:28 UTC

I may make a patch now, but I do know it may not get in until after the HEAD thaw.

Comment 2 Neel Chauhan 2018-08-29 22:10:15 UTC

Created attachment 196692 [details]
Patch (Revision 1)

I have made a patch which adds an option, -m, to efibootmgr which when creating a UEFI boot entry, you can also set it active. This changes efibootmgr itself and the man page. I have tested this patch on a HP EliteBook 1040 G3.

Keep in mind that this is my first patch to src. I also know that I may need to wait until the code thaw to get this in, or that this patch may get rejected.

Comment 3 Warner Losh 2018-08-31 21:49:26 UTC

-a should already be there for that.

diff --git a/usr.sbin/efibootmgr/efibootmgr.c b/usr.sbin/efibootmgr/efibootmgr.c
index eaf7a1c1903..103c993f66d 100644
--- a/usr.sbin/efibootmgr/efibootmgr.c
+++ b/usr.sbin/efibootmgr/efibootmgr.c
@@ -622,7 +622,7 @@ create_loadopt(uint8_t *buf, size_t bufmax, uint32_t attributes, efidp dp, size_

 static int
 make_boot_var(const char *label, const char *loader, const char *kernel, const char *env, bool dry_run,
-       int bootnum)
+    int bootnum, bool activate)
 {
        struct entry *new_ent;
        uint32_t load_attrs = 0;
@@ -665,6 +665,8 @@ make_boot_var(const char *label, const char *loader, const char *kernel, const c

        /* don't make the new bootvar active by default, use the -a option later */
        load_attrs = LOAD_OPTION_CATEGORY_BOOT;
+       if (activate)
+               load_attrs |= LOAD_OPTION_ACTIVE;
        load_opt_buf = malloc(MAX_LOADOPT_LEN);
        if (load_opt_buf == NULL)
                err(1, "malloc");
@@ -693,7 +695,6 @@ make_boot_var(const char *label, const char *loader, const char *kernel, const c
        LIST_INSERT_HEAD(&efivars, new_ent, entries);
        free(load_opt_buf);
        free(dp);
-
        return 0;
 }

@@ -915,7 +916,7 @@ main(int argc, char *argv[])
                 */
                make_boot_var(opts.label ? opts.label : "",
                    opts.loader, opts.kernel, opts.env, opts.dry_run,
-                   opts.has_bootnum ? opts.bootnum : -1);
+                   opts.has_bootnum ? opts.bootnum : -1, opts.set_active);
        else if (opts.set_active || opts.set_inactive )
                handle_activity(opts.bootnum, opts.set_active);
        else if (opts.order != NULL)

Comment 4 Warner Losh 2018-08-31 21:51:34 UTC

(In reply to Warner Losh from comment #3)
ah, the next the last hunk on my diff isn't needed.

Comment 5 Warner Losh 2018-08-31 21:54:13 UTC

 https://reviews.freebsd.org/D16977

Comment 6 commit-hook 2018-09-02 18:40:47 UTC

A commit references this bug:

Author: imp
Date: Sun Sep  2 18:40:18 UTC 2018
New revision: 338432
URL: https://svnweb.freebsd.org/changeset/base/338432

Log:
  Make -a (to make the entry active) apply to creation of a new boot
  variable.

  Approved by: re@ (rgrimes)
  PR: 231013
  Differential Revision:  https://reviews.freebsd.org/D16977

Changes:
  head/usr.sbin/efibootmgr/efibootmgr.c

Comment 7 Rebecca Cran 2018-12-23 21:59:29 UTC

The commit that appears to have been supposed to fix this doesn't work, since the -a option wants a parameter.

# efibootmgr --create --label FreeBSD --loader ada0p1:/efi/freebsd/loader.efi -a
efibootmgr: option requires an argument -- a

And adding the -a option earlier in the command line causes for example the 'label' parameter to be ignored:

# efibootmgr --create -a --label "FreeBSD" --loader ada0p1:/efi/freebsd/loader.efi
...
Boot000C*
...

Comment 8 Rebecca Cran 2018-12-24 06:59:29 UTC

See https://reviews.freebsd.org/D18648 for one approach to fixing this.

Comment 9 Rebecca Cran 2019-01-02 01:22:35 UTC

Fixed in r342635.

https://svnweb.freebsd.org/base/head/usr.sbin/efibootmgr/efibootmgr.c?revision=342635&view=markup