Bug 231013 - [patch] efibootmgr(8): Add option to activate EFI boot entry after creating it
Summary: [patch] efibootmgr(8): Add option to activate EFI boot entry after creating it
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Rebecca Cran
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2018-08-29 21:33 UTC by Neel Chauhan
Modified: 2019-01-02 01:22 UTC (History)
5 users (show)

See Also:


Attachments
Patch (Revision 1) (4.03 KB, patch)
2018-08-29 22:10 UTC, Neel Chauhan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neel Chauhan 2018-08-29 21:33:25 UTC
Keep in mind that I am interested in making a patch for this after the HEAD thaw described here: https://www.freebsd.org/releases/12.0R/schedule.html
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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 2018-08-31 21:54:13 UTC
 https://reviews.freebsd.org/D16977
Comment 6 commit-hook freebsd_committer 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 freebsd_committer 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 freebsd_committer 2018-12-24 06:59:29 UTC
See https://reviews.freebsd.org/D18648 for one approach to fixing this.