Re: [PATCH 2/2] Minimise writes to EFI variable storage

[Steve McIntyre <93sam@debian.org>]

First of all, thanks very much for doing this!

(For other people: I came up with a previous way of doing this years
ago, using efibootmgr to ask about the state of a variable before
maybe deleting and re-setting it.)

On Mon, Mar 11, 2019 at 03:05:46PM +0000, Colin Watson wrote:
>Some UEFI firmware is easily provoked into running out of space in its
>variable storage.  This is usually due to certain kernel drivers (e.g.
>pstore), but regardless of the cause it can cause grub-install to fail
>because it currently asks efibootmgr to delete and re-add entries, and
>the deletion often doesn't result in an immediate garbage collection.
>Writing variables frequently also increases wear on the NVRAM which may
>have limited write cycles.  For these reasons, it's desirable to find a
>way to minimise writes while still allowing grub-install to ensure that
>a suitable boot entry exists.
>
>Unfortunately, efibootmgr doesn't offer an interface that would let
>grub-install do this.  It doesn't in general make very much effort to
>minimise writes; it doesn't allow modifying an existing Boot* variable
>entry, except in certain limited ways; and current versions don't have a
>way to export the expected variable data so that grub-install can
>compare it to the current data.  While it would be possible (and perhaps
>desirable?) to add at least some of this to efibootmgr, that would still
>leave the problem that there isn't a good upstreamable way for
>grub-install to guarantee that it has a new enough version of
>efibootmgr.  In any case, it's cumbersome and slow for grub-install to
>have to fork efibootmgr to get things done.
>
>Fortunately, a few years ago Peter Jones helpfully factored out a
>substantial part of efibootmgr to the efivar and efiboot libraries, and
>so it's now possible to have grub-install use those directly.  We still
>have to use some code from efibootmgr, but much less than would
>previously have been necessary.
>
>grub-install now reuses existing boot entries where possible, and avoids
>writing to variables when the new contents are the same as the old
>contents.  In the common upgrade case where nothing needs to change, it
>no longer writes to NVRAM at all.  It's also now slightly faster, since
>using libefivar is faster than forking efibootmgr.

Awesome!

Some detailed review stuff inline below, with (maybe?) one error noted
in read_efi_variable().

I've built this code but not tested it directly on real hardware yet.

>Fixes Debian bug #891434.
>
>Signed-off-by: Colin Watson <cjwatson@ubuntu.com>
>---
> INSTALL                         |   5 +
> Makefile.util.def               |  20 ++
> configure.ac                    |  12 +
> grub-core/osdep/efivar.c        |   3 +
> grub-core/osdep/unix/efivar.c   | 503 ++++++++++++++++++++++++++++++++
> grub-core/osdep/unix/platform.c | 100 +------
> include/grub/util/install.h     |   5 +
> util/grub-install.c             |   4 +-
> 8 files changed, 557 insertions(+), 95 deletions(-)
> create mode 100644 grub-core/osdep/efivar.c
> create mode 100644 grub-core/osdep/unix/efivar.c

...

>diff --git a/grub-core/osdep/efivar.c b/grub-core/osdep/efivar.c
>new file mode 100644
>index 000000000..d2750e252
>--- /dev/null
>+++ b/grub-core/osdep/efivar.c
>@@ -0,0 +1,3 @@
>+#if !defined (__MINGW32__) && !defined (__CYGWIN__) && !defined (__AROS__)
>+#include "unix/efivar.c"
>+#endif
>diff --git a/grub-core/osdep/unix/efivar.c b/grub-core/osdep/unix/efivar.c
>new file mode 100644
>index 000000000..1df3e38f1
>--- /dev/null
>+++ b/grub-core/osdep/unix/efivar.c
>@@ -0,0 +1,503 @@
>+/*
>+ *  GRUB  --  GRand Unified Bootloader
>+ *  Copyright (C) 2013,2019 Free Software Foundation, Inc.
>+ *
>+ *  GRUB is free software: you can redistribute it and/or modify
>+ *  it under the terms of the GNU General Public License as published by
>+ *  the Free Software Foundation, either version 3 of the License, or
>+ *  (at your option) any later version.
>+ *
>+ *  GRUB is distributed in the hope that it will be useful,
>+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+ *  GNU General Public License for more details.
>+ *
>+ *  You should have received a copy of the GNU General Public License
>+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>+ */
>+
>+/* Contains portions derived from efibootmgr, licensed as follows:
>+ *
>+ *  Copyright (C) 2001-2004 Dell, Inc. <Matt_Domsch@dell.com>
>+ *  Copyright 2015-2016 Red Hat, Inc. <pjones@redhat.com>
>+ *
>+ *  This program is free software; you can redistribute it and/or modify
>+ *  it under the terms of the GNU General Public License as published by
>+ *  the Free Software Foundation; either version 2 of the License, or
>+ *  (at your option) any later version.
>+ */
>+
>+#include <config.h>
>+
>+#ifdef HAVE_EFIVAR
>+
>+#include <grub/util/install.h>
>+#include <grub/emu/hostdisk.h>
>+#include <grub/util/misc.h>
>+#include <grub/list.h>
>+#include <grub/misc.h>
>+#include <grub/emu/exec.h>
>+#include <sys/types.h>
>+#include <ctype.h>
>+#include <errno.h>
>+#include <stdlib.h>
>+#include <string.h>
>+
>+#include <efiboot.h>
>+#include <efivar.h>
>+
>+struct efi_variable {
>+  struct efi_variable *next;
>+  struct efi_variable **prev;
>+  char *name;
>+  efi_guid_t guid;
>+  uint8_t *data;
>+  size_t data_size;
>+  uint32_t attributes;
>+  int num;
>+};
>+
>+/* Boot option attributes.  */
>+#define LOAD_OPTION_ACTIVE 0x00000001
>+
>+/* GUIDs.  */
>+#define BLKX_UNKNOWN_GUID \
>+  EFI_GUID (0x47c7b225, 0xc42a, 0x11d2, 0x8e57, 0x00, 0xa0, 0xc9, 0x69, \
>+	    0x72, 0x3b)

Ugh. I'm assuming the mahic numbers here are not exposed usefully by
efivar or efiboot?

>+/* Log all errors recorded by libefivar/libefiboot.  */
>+static void
>+show_efi_errors (void)
>+{
>+  int i;
>+  int saved_errno = errno;
>+
>+  for (i = 0; ; ++i)
>+    {
>+      char *filename, *function, *message = NULL;
>+      int line, error = 0, rc;
>+
>+      rc = efi_error_get (i, &filename, &function, &line, &message, &error);
>+      if (rc < 0)
>+	/* Give up.  The caller is going to log an error anyway.  */
>+	break;
>+      if (rc == 0)
>+	/* No more errors.  */
>+	break;
>+      grub_util_warn ("%s: %s: %s", function, message, strerror (error));
>+    }
>+
>+  efi_error_clear ();
>+  errno = saved_errno;
>+}
>+
>+static struct efi_variable *
>+new_efi_variable (void)
>+{
>+  struct efi_variable *new = xmalloc (sizeof (*new));
>+  memset (new, 0, sizeof (*new));
>+  return new;
>+}
>+
>+static struct efi_variable *
>+new_boot_variable (void)
>+{
>+  struct efi_variable *new = new_efi_variable ();
>+  new->guid = EFI_GLOBAL_GUID;
>+  new->attributes = EFI_VARIABLE_NON_VOLATILE |
>+		    EFI_VARIABLE_BOOTSERVICE_ACCESS |
>+		    EFI_VARIABLE_RUNTIME_ACCESS;
>+  return new;
>+}
>+
>+static void
>+free_efi_variable (struct efi_variable *entry)
>+{
>+  if (entry)
>+    {
>+      free (entry->name);
>+      free (entry->data);
>+      free (entry);
>+    }
>+}
>+
>+static int
>+read_efi_variable (const char *name, struct efi_variable **entry)
>+{
>+  struct efi_variable *new = new_efi_variable ();
>+  int rc;
>+
>+  rc = efi_get_variable (EFI_GLOBAL_GUID, name,
>+			 &new->data, &new->data_size, &new->attributes);
>+  if (rc < 0)
>+    {
>+      free (new);
>+      new = NULL;
>+    }

So new_efi_variable() is using xmalloc() so it's safe if the
allocation fails. But what happens if efi_get_variable() fails - do
you need to free all the members by calling free_efi_variable() rather
than simply free() here?

>+
>+  if (new)
>+    {
>+      /* Latest Apple firmware sets the high bit which appears invalid
>+	 to the Linux kernel if we write it back, so let's zero it out if it
>+	 is set since it would be invalid to set it anyway.  */
>+      new->attributes = new->attributes & ~(1 << 31);
>+
>+      new->name = xstrdup (name);
>+      new->guid = EFI_GLOBAL_GUID;
>+    }
>+
>+  *entry = new;
>+  return rc;
>+}
>+
>+/* Set an EFI variable, but only if it differs from the current value.
>+   Some firmware implementations are liable to fill up flash space if we set
>+   variables unnecessarily, so try to keep write activity to a minimum. */
>+static int
>+set_efi_variable (const char *name, struct efi_variable *entry)
>+{
>+  struct efi_variable *old = NULL;
>+  int rc = 0;
>+
>+  read_efi_variable (name, &old);
>+  efi_error_clear ();
>+  if (old && old->attributes == entry->attributes &&
>+      old->data_size == entry->data_size &&
>+      memcmp (old->data, entry->data, entry->data_size) == 0)
>+    grub_util_info ("skipping unnecessary update of EFI variable %s", name);
>+  else
>+    {
>+      rc = efi_set_variable (EFI_GLOBAL_GUID, name,
>+			     entry->data, entry->data_size, entry->attributes,
>+			     0644);
>+      if (rc < 0)
>+	grub_util_warn (_("Cannot set EFI variable %s"), name);
>+    }
>+  free_efi_variable (old);
>+  return rc;
>+}
>+
>+static int
>+cmpvarbyname (const void *p1, const void *p2)
>+{
>+  const struct efi_variable *var1 = *(const struct efi_variable **)p1;
>+  const struct efi_variable *var2 = *(const struct efi_variable **)p2;
>+  return strcmp (var1->name, var2->name);
>+}
>+
>+static int
>+read_boot_variables (struct efi_variable **varlist)
>+{
>+  int rc;
>+  efi_guid_t *guid = NULL;
>+  char *name = NULL;
>+  struct efi_variable **newlist = NULL;
>+  int nentries = 0;
>+  int i;
>+
>+  while ((rc = efi_get_next_variable_name (&guid, &name)) > 0)
>+    {
>+      const char *snum = name + sizeof ("Boot") - 1;
>+      struct efi_variable *var = NULL;
>+      unsigned int num;
>+
>+      if (memcmp (guid, &efi_guid_global, sizeof (efi_guid_global)) != 0 ||
>+	  strncmp (name, "Boot", sizeof ("Boot") - 1) != 0 ||
>+	  !grub_isxdigit (snum[0]) || !grub_isxdigit (snum[1]) ||
>+	  !grub_isxdigit (snum[2]) || !grub_isxdigit (snum[3]))
>+	continue;
>+
>+      rc = read_efi_variable (name, &var);
>+      if (rc < 0)
>+	break;
>+
>+      if (sscanf (var->name, "Boot%04X-%*s", &num) == 1 && num < 65536)
>+	var->num = num;
>+
>+      newlist = xrealloc (newlist, (++nentries) * sizeof (*newlist));
>+      newlist[nentries - 1] = var;
>+    }
>+  if (rc == 0 && newlist)
>+    {
>+      qsort (newlist, nentries, sizeof (*newlist), cmpvarbyname);
>+      for (i = nentries - 1; i >= 0; --i)
>+	grub_list_push (GRUB_AS_LIST_P (varlist), GRUB_AS_LIST (newlist[i]));
>+    }
>+  else if (newlist)
>+    {
>+      for (i = 0; i < nentries; ++i)
>+	free (newlist[i]);
>+      free (newlist);
>+    }
>+  return rc;
>+}
>+
>+static void
>+remove_from_boot_order (struct efi_variable *order, uint16_t num)
>+{
>+  uint16_t *data;
>+  unsigned int old_i, new_i;
>+
>+  /* We've got an array (in order->data) of the order.  Squeeze out any
>+     instance of the entry we're deleting by shifting the remainder down.  */
>+  data = (uint16_t *) order->data;
>+
>+  for (old_i = 0, new_i = 0;
>+       old_i < order->data_size / sizeof (uint16_t);
>+       ++old_i)
>+    {
>+      if (data[old_i] != num) {
>+	if (new_i != old_i)
>+	  data[new_i] = data[old_i];
>+	new_i++;
>+      }
>+    }
>+
>+  order->data_size = sizeof (data[0]) * new_i;
>+}
>+
>+static void
>+add_to_boot_order (struct efi_variable *order, uint16_t num)
>+{
>+  uint16_t *data;
>+  int i;
>+  size_t new_data_size;
>+  uint16_t *new_data;
>+
>+  /* Check whether this entry is already in the boot order.  If it is, leave
>+     it alone.  */
>+  data = (uint16_t *) order->data;
>+  for (i = 0; i < order->data_size / sizeof (uint16_t); ++i)
>+    if (data[i] == num)
>+      return;
>+
>+  new_data_size = order->data_size + sizeof (uint16_t);
>+  new_data = xmalloc (new_data_size);
>+  new_data[0] = num;
>+  memcpy (new_data + 1, order->data, order->data_size);
>+  free (order->data);
>+  order->data = (uint8_t *) new_data;
>+  order->data_size = new_data_size;
>+}
>+
>+static int
>+find_free_boot_num (struct efi_variable *entries)
>+{
>+  int num_vars = 0, i;
>+  struct efi_variable *entry;
>+
>+  FOR_LIST_ELEMENTS (entry, entries)
>+    ++num_vars;
>+
>+  if (num_vars == 0)
>+    return 0;
>+
>+  /* O(n^2), but n is small and this is easy. */

Nod, fair point. Never seen more than a dozen or so boot entries on a
real system.

>+  for (i = 0; i < num_vars; ++i)
>+    {
>+      int found = 0;
>+      FOR_LIST_ELEMENTS (entry, entries)
>+	{
>+	  if (entry->num == i)
>+	    {
>+	      found = 1;
>+	      break;
>+	    }
>+	}
>+      if (!found)
>+	return i;
>+    }
>+
>+  return i;
>+}
>+
>+static int
>+get_edd_version (void)
>+{
>+  efi_guid_t blkx_guid = BLKX_UNKNOWN_GUID;
>+  uint8_t *data = NULL;
>+  size_t data_size = 0;
>+  uint32_t attributes;
>+  efidp_header *path;
>+  int rc;
>+
>+  rc = efi_get_variable (blkx_guid, "blk0", &data, &data_size, &attributes);
>+  if (rc < 0)
>+    return rc;
>+
>+  path = (efidp_header *) data;
>+  if (path->type == 2 && path->subtype == 1)
>+    return 3;
>+  return 1;
>+}
>+
>+static struct efi_variable *
>+make_boot_variable (int num, const char *disk, int part, const char *loader,
>+		    const char *label)
>+{
>+  struct efi_variable *entry = new_boot_variable ();
>+  uint32_t options;
>+  uint32_t edd10_devicenum;
>+  ssize_t dp_needed, loadopt_needed;
>+  efidp dp = NULL;
>+
>+  options = EFIBOOT_ABBREV_HD;
>+  switch (get_edd_version ()) {
>+    case 1:
>+      options = EFIBOOT_ABBREV_EDD10;
>+      break;
>+    case 3:
>+      options = EFIBOOT_ABBREV_NONE;
>+      break;
>+  }
>+
>+  /* This may not be the right disk; but it's probably only an issue on very
>+     old hardware anyway. */
>+  edd10_devicenum = 0x80;
>+
>+  dp_needed = efi_generate_file_device_path_from_esp (NULL, 0, disk, part,
>+						      loader, options,
>+						      edd10_devicenum);
>+  if (dp_needed < 0)
>+    goto err;
>+
>+  dp = xmalloc (dp_needed);
>+  dp_needed = efi_generate_file_device_path_from_esp ((uint8_t *) dp,
>+						      dp_needed, disk, part,
>+						      loader, options,
>+						      edd10_devicenum);
>+  if (dp_needed < 0)
>+    goto err;
>+
>+  loadopt_needed = efi_loadopt_create (NULL, 0, LOAD_OPTION_ACTIVE,
>+				       dp, dp_needed, (unsigned char *) label,
>+				       NULL, 0);
>+  if (loadopt_needed < 0)
>+    goto err;
>+  entry->data_size = loadopt_needed;
>+  entry->data = xmalloc (entry->data_size);
>+  loadopt_needed = efi_loadopt_create (entry->data, entry->data_size,
>+				       LOAD_OPTION_ACTIVE, dp, dp_needed,
>+				       (unsigned char *) label, NULL, 0);
>+  if (loadopt_needed < 0)
>+    goto err;
>+
>+  entry->name = xasprintf ("Boot%04X", num);
>+  entry->num = num;
>+
>+  return entry;
>+
>+err:
>+  free_efi_variable (entry);
>+  free (dp);
>+  return NULL;
>+}
>+
>+int
>+grub_install_efivar_register_efi (grub_device_t efidir_grub_dev,
>+				  const char *efifile_path,
>+				  const char *efi_distributor)
>+{
>+  const char *efidir_disk;
>+  int efidir_part;
>+  struct efi_variable *entries = NULL, *entry;
>+  struct efi_variable *order;
>+  int entry_num = -1;
>+  int rc;
>+
>+  efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
>+  efidir_part = efidir_grub_dev->disk->partition ? efidir_grub_dev->disk->partition->number + 1 : 1;
>+
>+#ifdef __linux__
>+  /*
>+   * Linux uses efivarfs (mounted on /sys/firmware/efi/efivars) to access the
>+   * EFI variable store. Some legacy systems may still use the deprecated
>+   * efivars interface (accessed through /sys/firmware/efi/vars). Where both
>+   * are present, libefivar will use the former in preference, so attempting
>+   * to load efivars will not interfere with later operations.
>+   */
>+  grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL },
>+			       NULL, NULL, "/dev/null");
>+#endif
>+
>+  if (!efi_variables_supported ())
>+    {
>+      grub_util_warn ("%s",
>+		      _("EFI variables are not supported on this system."));
>+      /* Let the user continue.  Perhaps they can still arrange to boot GRUB
>+         manually.  */
>+      return 0;
>+    }
>+
>+  rc = read_boot_variables (&entries);
>+  if (rc < 0)
>+    {
>+      grub_util_warn ("%s", _("Cannot read EFI Boot* variables"));
>+      goto err;
>+    }
>+  rc = read_efi_variable ("BootOrder", &order);
>+  if (rc < 0)
>+    {
>+      order = new_boot_variable ();
>+      order->name = xstrdup ("BootOrder");
>+      efi_error_clear ();
>+    }
>+
>+  /* Delete old entries from the same distributor.  */
>+  FOR_LIST_ELEMENTS (entry, entries)
>+    {
>+      efi_load_option *load_option = (efi_load_option *) entry->data;
>+      const char *label;
>+
>+      if (entry->num < 0)
>+	continue;
>+      label = (const char *) efi_loadopt_desc (load_option, entry->data_size);
>+      if (strcasecmp (label, efi_distributor) != 0)
>+	continue;
>+
>+      /* To avoid problems with some firmware implementations, reuse the first
>+         matching variable we find rather than deleting and recreating it.  */
>+      if (entry_num == -1)
>+	entry_num = entry->num;
>+      else
>+	{
>+	  grub_util_info ("deleting superfluous EFI variable %s (%s)",
>+			  entry->name, label);
>+	  rc = efi_del_variable (EFI_GLOBAL_GUID, entry->name);
>+	  if (rc < 0)
>+	    {
>+	      grub_util_warn (_("Cannot delete EFI variable %s"), entry->name);
>+	      goto err;
>+	    }
>+	}
>+
>+      remove_from_boot_order (order, (uint16_t) entry->num);
>+    }
>+
>+  if (entry_num == -1)
>+    entry_num = find_free_boot_num (entries);
>+  entry = make_boot_variable (entry_num, efidir_disk, efidir_part,
>+			      efifile_path, efi_distributor);
>+  if (!entry)
>+    goto err;
>+
>+  grub_util_info ("setting EFI variable %s", entry->name);
>+  rc = set_efi_variable (entry->name, entry);
>+  if (rc < 0)
>+    goto err;
>+
>+  add_to_boot_order (order, (uint16_t) entry_num);
>+
>+  grub_util_info ("setting EFI variable BootOrder");
>+  rc = set_efi_variable ("BootOrder", order);
>+  if (rc < 0)
>+    goto err;
>+
>+  return 0;
>+
>+err:
>+  show_efi_errors ();
>+  return errno;
>+}
>+
>+#endif /* HAVE_EFIVAR */
>diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
>index 55b8f4016..9d4530a38 100644
>--- a/grub-core/osdep/unix/platform.c
>+++ b/grub-core/osdep/unix/platform.c
>@@ -19,15 +19,12 @@
> #include <config.h>
> 
> #include <grub/util/install.h>
>-#include <grub/emu/hostdisk.h>
> #include <grub/util/misc.h>
> #include <grub/misc.h>
> #include <grub/i18n.h>
> #include <grub/emu/exec.h>
> #include <sys/types.h>
>-#include <dirent.h>
> #include <string.h>
>-#include <errno.h>
> 
> static char *
> get_ofpathname (const char *dev)
>@@ -78,102 +75,19 @@ get_ofpathname (const char *dev)
> 		   dev);
> }
> 
>-static int
>-grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
>-{
>-  int fd;
>-  pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, &fd);
>-  char *line = NULL;
>-  size_t len = 0;
>-  int rc = 0;
>-
>-  if (!pid)
>-    {
>-      grub_util_warn (_("Unable to open stream from %s: %s"),
>-		      "efibootmgr", strerror (errno));
>-      return errno;
>-    }
>-
>-  FILE *fp = fdopen (fd, "r");
>-  if (!fp)
>-    {
>-      grub_util_warn (_("Unable to open stream from %s: %s"),
>-		      "efibootmgr", strerror (errno));
>-      return errno;
>-    }
>-
>-  line = xmalloc (80);
>-  len = 80;
>-  while (1)
>-    {
>-      int ret;
>-      char *bootnum;
>-      ret = getline (&line, &len, fp);
>-      if (ret == -1)
>-	break;
>-      if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>-	  || line[sizeof ("Boot") - 1] < '0'
>-	  || line[sizeof ("Boot") - 1] > '9')
>-	continue;
>-      if (!strcasestr (line, efi_distributor))
>-	continue;
>-      bootnum = line + sizeof ("Boot") - 1;
>-      bootnum[4] = '\0';
>-      if (!verbosity)
>-	rc = grub_util_exec ((const char * []){ "efibootmgr", "-q",
>-	      "-b", bootnum,  "-B", NULL });
>-      else
>-	rc = grub_util_exec ((const char * []){ "efibootmgr",
>-	      "-b", bootnum, "-B", NULL });
>-    }
>-
>-  free (line);
>-  return rc;
>-}
>-
> int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> 			   const char *efifile_path,
> 			   const char *efi_distributor)
> {
>-  const char * efidir_disk;
>-  int efidir_part;
>-  int ret;
>-  efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
>-  efidir_part = efidir_grub_dev->disk->partition ? efidir_grub_dev->disk->partition->number + 1 : 1;
>-
>-  if (grub_util_exec_redirect_null ((const char * []){ "efibootmgr", "--version", NULL }))
>-    {
>-      /* TRANSLATORS: This message is shown when required executable `%s'
>-	 isn't found.  */
>-      grub_util_error (_("%s: not found"), "efibootmgr");
>-    }
>-
>-  /* On Linux, we need the efivars kernel modules.  */
>-#ifdef __linux__
>-  grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>+#ifdef HAVE_EFIVAR
>+  return grub_install_efivar_register_efi (efidir_grub_dev, efifile_path,
>+					   efi_distributor);
>+#else
>+  grub_util_error ("%s",
>+		   _("GRUB was not built with efivar support; "
>+		     "cannot register EFI boot entry"));
> #endif
>-  /* Delete old entries from the same distributor.  */
>-  ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
>-  if (ret)
>-    return ret;
>-
>-  char *efidir_part_str = xasprintf ("%d", efidir_part);
>-
>-  if (!verbosity)
>-    ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
>-	  "-c", "-d", efidir_disk,
>-	  "-p", efidir_part_str, "-w",
>-	  "-L", efi_distributor, "-l", 
>-	  efifile_path, NULL });
>-  else
>-    ret = grub_util_exec ((const char * []){ "efibootmgr",
>-	  "-c", "-d", efidir_disk,
>-	  "-p", efidir_part_str, "-w",
>-	  "-L", efi_distributor, "-l", 
>-	  efifile_path, NULL });
>-  free (efidir_part_str);
>-  return ret;
> }
> 
> void
>diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>index 2631b1074..dcad16ac5 100644
>--- a/include/grub/util/install.h
>+++ b/include/grub/util/install.h
>@@ -216,6 +216,11 @@ grub_install_get_default_arm_platform (void);
> const char *
> grub_install_get_default_x86_platform (void);
> 
>+int
>+grub_install_efivar_register_efi (grub_device_t efidir_grub_dev,
>+				  const char *efifile_path,
>+				  const char *efi_distributor);
>+
> int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> 			   const char *efifile_path,
>diff --git a/util/grub-install.c b/util/grub-install.c
>index 264f9ecdc..a61df056e 100644
>--- a/util/grub-install.c
>+++ b/util/grub-install.c
>@@ -1885,7 +1885,7 @@ main (int argc, char *argv[])
> 					       "\\System\\Library\\CoreServices",
> 					       efi_distributor);
> 	      if (ret)
>-	        grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
>+	        grub_util_error (_("failed to register the EFI boot entry: %s"),
> 				 strerror (ret));
> 	    }
> 
>@@ -1929,7 +1929,7 @@ main (int argc, char *argv[])
> 	  ret = grub_install_register_efi (efidir_grub_dev,
> 					   efifile_path, efi_distributor);
> 	  if (ret)
>-	    grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
>+	    grub_util_error (_("failed to register the EFI boot entry: %s"),
> 			     strerror (ret));
> 	}
>       break;
>-- 
>2.17.1
>
-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
"This dress doesn't reverse." -- Alden Spiess