Re: ZFS grubenv write support

2020-02-21 Thread Daniel Kiper
On Fri, Feb 21, 2020 at 11:14:25AM +0100, Javier Martinez Canillas wrote:
> On 1/17/20 1:10 PM, Daniel Kiper wrote:
> > Hi Paul,
> >
>
> [snip]
>
> >
> > In general I like the idea. Though I would like to have this
> > configurable by user if possible (some FSes may support traditional
> > grubenv and special regions for boot loaders). I would also take into
> > account Luiz's comments. UEFI variables? Seems interesting at first
> > sight. Though it may have some limitations.
> >
>
> Peter wrote support for this and we carry in our downstream patch-set. It
> is very useful for debugging purposes or when doing network booting from
> an EFI machine that has no grubenv file.
>
> The grubenv is stored in a GRUB_ENV-91376aff-cba6-42be-949d-06fde81128e8
> EFI variable, can be loaded with an efi-load-env command and the current
> grubenv exported to an EFI var with the efi-export-env command.
>
> The patches are [0] and [1]. If there's interest for this I can post to
> the list properly.

Yes, please do...

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: ZFS grubenv write support

2020-02-21 Thread Javier Martinez Canillas
On 1/17/20 1:10 PM, Daniel Kiper wrote:
> Hi Paul,
> 

[snip]

> 
> In general I like the idea. Though I would like to have this
> configurable by user if possible (some FSes may support traditional
> grubenv and special regions for boot loaders). I would also take into
> account Luiz's comments. UEFI variables? Seems interesting at first
> sight. Though it may have some limitations.
> 

Peter wrote support for this and we carry in our downstream patch-set. It
is very useful for debugging purposes or when doing network booting from
an EFI machine that has no grubenv file.

The grubenv is stored in a GRUB_ENV-91376aff-cba6-42be-949d-06fde81128e8
EFI variable, can be loaded with an efi-load-env command and the current
grubenv exported to an EFI var with the efi-export-env command.

The patches are [0] and [1]. If there's interest for this I can post to
the list properly.

[0]:
From eb6884bcff0e911e8543a28b5534d2bfe6b4dd4a Mon Sep 17 00:00:00 2001
From: Peter Jones 
Date: Mon, 7 Dec 2015 14:20:49 -0500
Subject: [PATCH] Make efi machines load an env block from a variable

Signed-off-by: Peter Jones 
---
 grub-core/Makefile.core.def |  1 +
 grub-core/kern/efi/init.c   | 34 +-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 6b10e814edd..0c7222710d9 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -203,6 +203,7 @@ kernel = {
   efi = term/efi/console.c;
   efi = kern/acpi.c;
   efi = kern/efi/acpi.c;
+  efi = lib/envblk.c;
   i386_coreboot = kern/i386/pc/acpi.c;
   i386_multiboot = kern/i386/pc/acpi.c;
   i386_coreboot = kern/acpi.c;
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index 3dfdf2d22b0..71d2279a0c1 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -25,9 +25,40 @@
 #include 
 #include 
 #include 
+#include 
 
 grub_addr_t grub_modbase;
 
+#define GRUB_EFI_GRUB_VARIABLE_GUID \
+  { 0x91376aff, 0xcba6, 0x42be, \
+{ 0x94, 0x9d, 0x06, 0xfd, 0xe8, 0x11, 0x28, 0xe8 } \
+  }
+
+/* Helper for grub_efi_env_init */
+static int
+set_var (const char *name, const char *value,
+void *whitelist __attribute__((__unused__)))
+{
+  grub_env_set (name, value);
+  return 0;
+}
+
+static void
+grub_efi_env_init (void)
+{
+  grub_efi_guid_t efi_grub_guid = GRUB_EFI_GRUB_VARIABLE_GUID;
+  struct grub_envblk envblk_s = { NULL, 0 };
+  grub_envblk_t envblk = _s;
+
+  envblk_s.buf = grub_efi_get_variable ("GRUB_ENV", _grub_guid,
+   _s.size);
+  if (!envblk_s.buf || envblk_s.size < 1)
+return;
+
+  grub_envblk_iterate (envblk, NULL, set_var);
+  grub_free (envblk_s.buf);
+}
+
 void
 grub_efi_init (void)
 {
@@ -42,10 +73,11 @@ grub_efi_init (void)
   efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
  0, 0, 0, NULL);
 
+  grub_efi_env_init ();
   grub_efidisk_init ();
 }
 
-void (*grub_efi_net_config) (grub_efi_handle_t hnd, 
+void (*grub_efi_net_config) (grub_efi_handle_t hnd,
 char **device,
 char **path);
 
-- 
2.24.1

[1]:
From 30d5a92f398d10b5c9beb05bab0656cf14f2fac2 Mon Sep 17 00:00:00 2001
From: Peter Jones 
Date: Wed, 16 Jan 2019 13:21:46 -0500
Subject: [PATCH] Add efi-export-env and efi-load-env commands

This adds "efi-export-env VARIABLE" and "efi-load-env", which manipulate the
environment block stored in the EFI variable
GRUB_ENV-91376aff-cba6-42be-949d-06fde81128e8.

Signed-off-by: Peter Jones 
---
 grub-core/Makefile.core.def  |   6 ++
 grub-core/commands/efi/env.c | 168 +++
 grub-core/kern/efi/efi.c |   3 +
 grub-core/kern/efi/init.c|   5 --
 grub-core/lib/envblk.c   |  43 +
 include/grub/efi/efi.h   |   5 ++
 include/grub/lib/envblk.h|   3 +
 util/grub-set-bootflag.c |   1 +
 8 files changed, 229 insertions(+), 5 deletions(-)
 create mode 100644 grub-core/commands/efi/env.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index c6ba4809ef8..8cf4be8e306 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -817,6 +817,12 @@ module = {
   enable = efi;
 };
 
+module = {
+  name = efienv;
+  common = commands/efi/env.c;
+  enable = efi;
+};
+
 module = {
   name = efifwsetup;
   efi = commands/efi/efifwsetup.c;
diff --git a/grub-core/commands/efi/env.c b/grub-core/commands/efi/env.c
new file mode 100644
index 000..a69079786aa
--- /dev/null
+++ b/grub-core/commands/efi/env.c
@@ -0,0 +1,168 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2012  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 

Re: ZFS grubenv write support

2020-01-17 Thread Daniel Kiper
Hi Paul,

On Mon, Jan 13, 2020 at 04:03:34PM -0800, Paul Dagnelie wrote:
> Hey all,
>
> I'm interested in trying to enable support for boot-time modification of
> the grubenv file on ZFS and other non-standard filesystems that don't
> necessarily work today. I have a proposed design, and I was wondering if
> you all would be interested in it. I'd be implementing it and testing it
> myself, my main goal was to try to make sure that after doing so, the
> community would be interested in integrating it. This has the dual
> advantage of allowing everyone using ZFS and other filesystems to benefit
> from it, and also making it so I don't have to maintain a fork indefinitely.
>
> The proposal is that rather than always opening the grubenv file and
> reading it using the normal file API, we can add a function (or functions)
> to the fs struct that allows a filesystem to advertise that it needs to use
> special logic to handle writable files. Then the grubenv file can be stored
> in a special location (probably one of that padding areas in the label,
> which is where the FreeBSD loader stores some boot data in zfs) where it
> can be written safely by the bootloader without harming the checksum tree
> in ZFS. The GRUB code would need to change to invoke the special "Please
> let the fs handle this writable file" logic (which would have open, read,
> and write functions) for filesystems that have them available, but this
> should be extensible to other CoW or sparse filesystems like BTRFS. If a
> filesystem doesn't advertise that logic, GRUB would fall back to using the
> currently file-based logic.

In general I like the idea. Though I would like to have this
configurable by user if possible (some FSes may support traditional
grubenv and special regions for boot loaders). I would also take into
account Luiz's comments. UEFI variables? Seems interesting at first
sight. Though it may have some limitations.

> An alternative proposal that I found slightly less elegant but would
> involve fewer invasive changes is to just modify the zfs grub logic to
> check if it's opening the grubenv file, and if so divert to special read
> and open logic that would access this padding area. This would involve
> hardcoding the grubenv file name in the ZFS code, but I figured I would
> mention it as a possibility in case it was preferred for other reasons.

Please do not do that.

And please CC relevant FS people if you post patches.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: ZFS grubenv write support

2020-01-13 Thread Luiz Angelo Daros de Luca
Hello,

I'm not a grub Dev but I would like to give my two cents.

There is already a patch similar to what you proposed for btrfs:
https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/grub2-grubenv-in-btrfs-header.patch?expand=1

However, I still prefer to look for a place independent of any fs. A
possible place for grubenv might be after grub1.5 modules. It would require
the same write access needed to install grub.

There is also UEFI vars, but it would only work on UEFI devices. However,
we don't need one solution for every single case. We just need more options.

We normally have billions of kbytes available and grub can't find a couple
of kbytes for its use?

Regards,

Em seg, 13 de jan de 2020 21:04, Paul Dagnelie  escreveu:

> Hey all,
>
> I'm interested in trying to enable support for boot-time modification of
> the grubenv file on ZFS and other non-standard filesystems that don't
> necessarily work today. I have a proposed design, and I was wondering if
> you all would be interested in it. I'd be implementing it and testing it
> myself, my main goal was to try to make sure that after doing so, the
> community would be interested in integrating it. This has the dual
> advantage of allowing everyone using ZFS and other filesystems to benefit
> from it, and also making it so I don't have to maintain a fork indefinitely.
>
> The proposal is that rather than always opening the grubenv file and
> reading it using the normal file API, we can add a function (or functions)
> to the fs struct that allows a filesystem to advertise that it needs to use
> special logic to handle writable files. Then the grubenv file can be stored
> in a special location (probably one of that padding areas in the label,
> which is where the FreeBSD loader stores some boot data in zfs) where it
> can be written safely by the bootloader without harming the checksum tree
> in ZFS. The GRUB code would need to change to invoke the special "Please
> let the fs handle this writable file" logic (which would have open, read,
> and write functions) for filesystems that have them available, but this
> should be extensible to other CoW or sparse filesystems like BTRFS. If a
> filesystem doesn't advertise that logic, GRUB would fall back to using the
> currently file-based logic.
>
> An alternative proposal that I found slightly less elegant but would
> involve fewer invasive changes is to just modify the zfs grub logic to
> check if it's opening the grubenv file, and if so divert to special read
> and open logic that would access this padding area. This would involve
> hardcoding the grubenv file name in the ZFS code, but I figured I would
> mention it as a possibility in case it was preferred for other reasons.
>
> Do people have thoughts or opinions about this concept and this design? I
> would greatly appreciate any feedback or thoughts on alternative
> approaches. Thank you for your time and attention!
>
> --
> Paul Dagnelie
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


ZFS grubenv write support

2020-01-13 Thread Paul Dagnelie
Hey all,

I'm interested in trying to enable support for boot-time modification of
the grubenv file on ZFS and other non-standard filesystems that don't
necessarily work today. I have a proposed design, and I was wondering if
you all would be interested in it. I'd be implementing it and testing it
myself, my main goal was to try to make sure that after doing so, the
community would be interested in integrating it. This has the dual
advantage of allowing everyone using ZFS and other filesystems to benefit
from it, and also making it so I don't have to maintain a fork indefinitely.

The proposal is that rather than always opening the grubenv file and
reading it using the normal file API, we can add a function (or functions)
to the fs struct that allows a filesystem to advertise that it needs to use
special logic to handle writable files. Then the grubenv file can be stored
in a special location (probably one of that padding areas in the label,
which is where the FreeBSD loader stores some boot data in zfs) where it
can be written safely by the bootloader without harming the checksum tree
in ZFS. The GRUB code would need to change to invoke the special "Please
let the fs handle this writable file" logic (which would have open, read,
and write functions) for filesystems that have them available, but this
should be extensible to other CoW or sparse filesystems like BTRFS. If a
filesystem doesn't advertise that logic, GRUB would fall back to using the
currently file-based logic.

An alternative proposal that I found slightly less elegant but would
involve fewer invasive changes is to just modify the zfs grub logic to
check if it's opening the grubenv file, and if so divert to special read
and open logic that would access this padding area. This would involve
hardcoding the grubenv file name in the ZFS code, but I figured I would
mention it as a possibility in case it was preferred for other reasons.

Do people have thoughts or opinions about this concept and this design? I
would greatly appreciate any feedback or thoughts on alternative
approaches. Thank you for your time and attention!

-- 
Paul Dagnelie
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel