[PATCH RFC] Proposition of a --auto-nvram option for grub-install
Hello everyone,
I'm writing to this list since I would like to get some feedback on an
additional option to the grub-install tool we would find very
convenient to have. The diff is attached to the e-mail (also available
as a pastebin [1]).
The idea of the --auto-nvram (the name is just a proposition) flag is
a bit similar to what --no-nvram does. After providing the option
during grub-install, the tool will attempt to guess if there is access
to NVRAM variables for EFI and/or IEEE1275 and, if yes, perform the
usual variable updates. If no access to the NVRAM is available the
whole thing is handled somewhat similar to --no-nvram + a warning
message displayed. Rationale:
We would like to use this in Ubuntu for cases of dual BIOS/EFI
bootloaders installed (at the same time), helpful for the situation of
calling grub-install --target=x86_64-efi from the shim-efi package on
a BIOS legacy-mode booted machine. For this legacy-mode case when
running on a EFI-enabled device, currently this causes grub-install to
fail as obviously there is no access to the NVRAM and no --no-nvram is
given. We don't want to unconditionally append --no-nvram as this is
not what we want to happen for the case of a system that is actually
booted in EFI-mode. With this flag, we would be simply performing a
grub-install --target=x86_64-efi --auto-nvram unconditionally which
would do the right thing in both cases, allowing for a much simpler
handling of this dual-bootloader case in Ubuntu. Having it being done
inside grub-installer makes everything much cleaner.
This is of course just a proposition about which I wanted to get some
feedback from people that know the codebase the most. It's my first
time working on the grub project so apologies for any flukes or
silliness in the code or the idea itself.
Thank you!
Best regards,
[1] http://paste.ubuntu.com/p/cWR3k3NZgF/
--
Łukasz 'sil2100' Zemczak
Foundations Team
lukasz.zemc...@canonical.com
www.canonical.com
diff --git a/grub-core/osdep/basic/no_platform.c b/grub-core/osdep/basic/no_platform.c
index d76c34c14..b39e97f48 100644
--- a/grub-core/osdep/basic/no_platform.c
+++ b/grub-core/osdep/basic/no_platform.c
@@ -25,7 +25,7 @@
void
grub_install_register_ieee1275 (int is_prep, const char *install_device,
-int partno, const char *relpath)
+int partno, const char *relpath, int detect_nvram)
{
grub_util_error ("%s", _("no IEEE1275 routines are available for your platform"));
}
@@ -33,7 +33,8 @@ grub_install_register_ieee1275 (int is_prep, const char *install_device,
void
grub_install_register_efi (grub_device_t efidir_grub_dev,
const char *efifile_path,
- const char *efi_distributor)
+ const char *efi_distributor,
+ int detect_nvram)
{
grub_util_error ("%s", _("no EFI routines are available for your platform"));
}
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index ca448bc11..4eb2c11c9 100644
--- a/grub-core/osdep/unix/platform.c
+++ b/grub-core/osdep/unix/platform.c
@@ -134,7 +134,8 @@ grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
int
grub_install_register_efi (grub_device_t efidir_grub_dev,
const char *efifile_path,
- const char *efi_distributor)
+ const char *efi_distributor,
+ int detect_nvram)
{
const char * efidir_disk;
int efidir_part;
@@ -153,6 +154,21 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
#ifdef __linux__
grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
#endif
+
+ /* If requested, we try to detect if NVRAM access is available and if not,
+ warn the user and resume normal operation. */
+ if (detect_nvram)
+{
+ error = grub_util_exec_redirect_null ((const char * []){ "efibootmgr", NULL });
+ if (error == 2)
+ {
+ grub_util_warn ("%s", _("Auto-NVRAM selected and no EFI variable support detected on the system."));
+ return 0;
+ }
+ else if (error)
+ return error;
+}
+
/* Delete old entries from the same distributor. */
ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
if (ret)
@@ -178,7 +194,7 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
void
grub_install_register_ieee1275 (int is_prep, const char *install_device,
-int partno, const char *relpath)
+int partno, const char *relpath, int detect_nvram)
{
char *boot_device;
@@ -189,6 +205,14 @@ grub_install_register_ieee1275 (int is_prep, const char *install_device,
grub_util_error (_("%s: not found"), "ofpathname");
}
+ /* If requested, we try to detect if NVRAM access is available and if not,
+ warn the user and resume normal operation. */
+ if (detect_nvram && grub_util_exec_redirect_null ((const char * []){ "nvram", NULL }))
+{
+ grub_util_warn ("%s", _("Auto-NVRAM selected and no IEEE1275 variable support detected on the system."));
+ return;
+}
+
/* Get the Open Firmware device tree path
Re: Checksummed environments
On 12/04/18 10:33, Daniel Kiper wrote: > On Tue, Apr 10, 2018 at 11:09:33PM +0200, Daniel Kiper wrote: >> On Fri, Apr 06, 2018 at 03:08:23PM +0200, Kristian Amlie wrote: >>> On 06/04/18 14:35, Daniel Kiper wrote: On Fri, Apr 06, 2018 at 11:25:22AM +0200, Kristian Amlie wrote: > Hey, I work for Northern.tech, developing update software for embedded > Linux devices. > > I have a question about GRUB's environment block: This block is not I am not sure what exactly you mean by "GRUB's environment block". Could you send me some references to the code? >>> >>> Of course, sorry if the context wasn't clear. I'm talking about the >>> environment block mentioned here: >>> https://www.gnu.org/software/grub/manual/grub/grub.html#Environment-block, >>> which is used to store information from one boot to the next, using >>> save_env and load_env. >>> > checksummed, and hence I reckon it can become corrupt if power is lost > in the middle of a write. What about the other blocks? >>> >>> In theory, there is only one block, because it is written in-place, >>> directly on disk, without changing any other filesystem blocks. Is that >>> what you meant by "other blocks"? >>> > This is an important safety criterion for us, so we've been thinking of > developing environment block checksumming as an extension to the > existing save_env and load_env commands. The most likely approach will > be to grab X amount of bytes at the end of the block and use these for > the checksum. Could you tell us more about that? >>> >>> The idea comes from U-Boot [1], which uses a dedicated block on a >>> storage device to store data, and uses a CRC32 checksum to make sure it >>> is consistent. The motivation is to be able to detect that the block is >>> corrupt, rather than accepting a block of data that may have incorrect >>> data in if a write was interrupted midway by a powerloss. You can read >>> more about it in the link. >>> >>> [2] https://www.denx.de/wiki/DULG/UBootEnvVariables >> >> I am OK with the idea. However, I think that the feature should have >> a kind of switch to turn it off/on. At first sight it looks that new >> environment variable is sufficient for it. > > And/Or an argument for save_env/load_env... Yes, I'm fine with either. How about a variable that determines the default, and you can override it with flags? -- Kristian ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Checksummed environments
On Tue, Apr 10, 2018 at 11:09:33PM +0200, Daniel Kiper wrote: > On Fri, Apr 06, 2018 at 03:08:23PM +0200, Kristian Amlie wrote: > > On 06/04/18 14:35, Daniel Kiper wrote: > > > On Fri, Apr 06, 2018 at 11:25:22AM +0200, Kristian Amlie wrote: > > >> Hey, I work for Northern.tech, developing update software for embedded > > >> Linux devices. > > >> > > >> I have a question about GRUB's environment block: This block is not > > > > > > I am not sure what exactly you mean by "GRUB's environment block". > > > Could you send me some references to the code? > > > > Of course, sorry if the context wasn't clear. I'm talking about the > > environment block mentioned here: > > https://www.gnu.org/software/grub/manual/grub/grub.html#Environment-block, > > which is used to store information from one boot to the next, using > > save_env and load_env. > > > > >> checksummed, and hence I reckon it can become corrupt if power is lost > > >> in the middle of a write. > > > > > > What about the other blocks? > > > > In theory, there is only one block, because it is written in-place, > > directly on disk, without changing any other filesystem blocks. Is that > > what you meant by "other blocks"? > > > > >> This is an important safety criterion for us, so we've been thinking of > > >> developing environment block checksumming as an extension to the > > >> existing save_env and load_env commands. The most likely approach will > > >> be to grab X amount of bytes at the end of the block and use these for > > >> the checksum. > > > > > > Could you tell us more about that? > > > > The idea comes from U-Boot [1], which uses a dedicated block on a > > storage device to store data, and uses a CRC32 checksum to make sure it > > is consistent. The motivation is to be able to detect that the block is > > corrupt, rather than accepting a block of data that may have incorrect > > data in if a write was interrupted midway by a powerloss. You can read > > more about it in the link. > > > > [2] https://www.denx.de/wiki/DULG/UBootEnvVariables > > I am OK with the idea. However, I think that the feature should have > a kind of switch to turn it off/on. At first sight it looks that new > environment variable is sufficient for it. And/Or an argument for save_env/load_env... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
