On Tue, Feb 21, 2023 at 4:39 PM Daniel Kiper <dki...@net-space.pl> wrote:
> On Mon, Feb 20, 2023 at 08:15:35AM -0800, Oliver Steffen wrote: > > Thank you for the comments, Daniel. > > > > Quoting Daniel Kiper (2023-02-15 19:27:03) > > > On Mon, Jan 16, 2023 at 12:40:53PM +0100, Oliver Steffen wrote: > > > > Add a new module named boot_loader_interface, which provides a > command > > > > > > I would prefer something shorter than boot_loader_interface. bli? > > > > Then bli it is. > > Cool! Thanks! > > > > > with the same name. It implements a small but quite useful part of > the > > > > Boot Loader Interface [0]. This interface uses EFI variables for > > > > communication between the boot loader and the operating system. > > > > > > > > This module sets two EFI variables under the vendor GUID > > > > 4a67b082-0a4c-41cf-b6c7-440b29bb8c4f: > > > > > > > > - LoaderInfo: contains GRUB + <version number>. > > > > This allows the running operating system to identify the boot > loader > > > > used during boot. > > > > > > > > - LoaderDevicePartUUID: contains the partition UUID of the > > > > EFI System Partition (ESP). This is used by > > > > systemd-gpt-auto-generator [1] to find the root partitions (and > others > > > > too), via partition type IDs [2]. > > > > > > > > This module is only available on EFI platforms. > > > > > > > > [0] https://systemd.io/BOOT_LOADER_INTERFACE/ > > > > [1] > https://www.freedesktop.org/software/systemd/man/systemd-gpt-auto-generator.html > > > > [2] > https://uapi-group.org/specifications/specs/discoverable_partitions_specification/ > > > > > > > > Signed-off-by: Oliver Steffen <ostef...@redhat.com> > > > > --- > > > > grub-core/Makefile.core.def | 6 + > > > > grub-core/commands/boot_loader_interface.c | 217 > +++++++++++++++++++++ > > > > 2 files changed, 223 insertions(+) > > > > create mode 100644 grub-core/commands/boot_loader_interface.c > > > > > > > > diff --git a/grub-core/Makefile.core.def > b/grub-core/Makefile.core.def > > > > index ba967aac8..23455fb71 100644 > > > > --- a/grub-core/Makefile.core.def > > > > +++ b/grub-core/Makefile.core.def > > > > @@ -2547,3 +2547,9 @@ module = { > > > > common = commands/i386/wrmsr.c; > > > > enable = x86; > > > > }; > > > > + > > > > +module = { > > > > + name = boot_loader_interface; > > > > > > s/boot_loader_interface/bli/? > > > > > > > + efi = commands/boot_loader_interface.c; > > > > > > Ditto and below if needed... > > > > > > > + enable = efi; > > > > +}; > > > > diff --git a/grub-core/commands/boot_loader_interface.c > b/grub-core/commands/boot_loader_interface.c > > > > new file mode 100644 > > > > index 000000000..ccd7fa3d9 > > > > --- /dev/null > > > > +++ b/grub-core/commands/boot_loader_interface.c > > > > @@ -0,0 +1,217 @@ > > > > +/*-*- Mode: C; c-basic-offset: 2; indent-tabs-mode: t -*-*/ > > > > > > Could you move this to the end of the file? Good example you can find > in > > > the Xen project [1]. > > > > I personally do not care about this, it was just part of the file I used > > as a template. Is this wanted, or can we just drop it? > > If you do not need it please drop it. > > [...] > > > > > + guid = &entry.guid; > > > > + *part_uuid = grub_xasprintf ( > > > > + "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", > > > > + grub_le_to_cpu32 (guid->data1), grub_le_to_cpu16 > (guid->data2), > > > > + grub_le_to_cpu16 (guid->data3), guid->data4[0], > guid->data4[1], > > > > + guid->data4[2], guid->data4[3], guid->data4[4], > guid->data4[5], > > > > + guid->data4[6], guid->data4[7]); > > > > > > I think this is generic thing and can be converted into a function. > > > Similar code is in the grub-core/commands/probe.c and > util/grub-probe.c. > > > It seems to me it is at least easy to make one function for the GRUB > > > core code. Please do that. Of course in separate patch. > > > > There are multiple representations of a GUID: > > - grub_gpt_part_guid (gpt_partition.h) > > - grub_efi_guid (efi/api.h) > > - grub_efi_packed_guid (efi/api.h) > > > > I would add a function for the first kind to gpt_partition.h and > > partmap/gpt.c that prints into a string buffer (length checked). > > > > It would be nice to add a format specifier for GUIDs to the printf > > implementation, > > but that only makes much sense (I think) if the GUID repesentations > > could be unified into one first, in a central place. > > Could you do the unification and introduce format specifier for GUIDs? > Assume there is a common GUID struct. Then we have the following options (AFAIK): - add a format specifier for it, lets say '%g' or '%y'. This causes problems with the -Wformat checks. We could disable it, but that's a bad idea. - use an additional qualifier for %p, for example %p{GUID}. This way gcc will not complain (%p takes void *). But it is ugly if one wants to print a '{' after a GUID. - Do not use a custom format specifier at all; just provide a "to-string" function. To print a guid one needs a temporary buffer (less convenient, but still better than repeating the raw printf statement for GUID everywhere, like it is now). Pick your poison... :-) Or does anybody have a better idea? Thanks, Oliver
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
