Right, this already came up in response to v1. I think grub is currently safe but after some further digging we might want to add support for the DT_FIXUP protocol or similar at some point and this makes things more complicated than necessary. I'll send another update.
On Mon, Jun 17, 2024 at 3:44 PM Vladimir 'phcoder' Serbinenko <phco...@gmail.com> wrote: > > Saving FDT subtly changes the behaviour. Image some command or even an > external program modifying FDT and freeing old copy. Maybe we should > call grub_efi_get_firmware_fdt every time we need fdt. > > On Mon, Jun 17, 2024 at 4:08 PM Tobias Heider > <tobias.hei...@canonical.com> wrote: > > > > The fdtdump command allows dumping arbitrary device tree properties > > and saving them to a variable similar to the smbios command. > > > > This is useful in scripts where further actions such as selecting a > > kernel or loading another device tree depend on the compatible or > > model values of the device tree provided by the firmware. > > > > For now only the root level properties of the dtb are exposed. > > > > The firmware provided device-tree is not expected to change so we > > load it once during module initialization and then use it where > > needed. > > > > Signed-off-by: Tobias Heider <tobias.hei...@canonical.com> > > --- > > docs/grub.texi | 26 ++++++++++++++++++++ > > grub-core/loader/efi/fdt.c | 49 +++++++++++++++++++++++++++++++++++++- > > 2 files changed, 74 insertions(+), 1 deletion(-) > > > > diff --git a/docs/grub.texi b/docs/grub.texi > > index f3bdc2564..a050dc0fc 100644 > > --- a/docs/grub.texi > > +++ b/docs/grub.texi > > @@ -4373,6 +4373,7 @@ you forget a command, you can run the command > > @command{help} > > * eval:: Evaluate agruments as GRUB commands > > * export:: Export an environment variable > > * false:: Do nothing, unsuccessfully > > +* fdtdump:: Retrieve device tree information > > * fwsetup:: Reboot into the firmware setup menu > > * gdbinfo:: Provide info for debugging with GDB > > * gettext:: Translate a string > > @@ -4904,6 +4905,31 @@ such as @code{if} and @code{while} > > (@pxref{Shell-like scripting}). > > @end deffn > > > > > > +@node fdtdump > > +@subsection fdtdump > > + > > +@deffn Command fdtdump @ > > + [@option{--prop} @var{prop}] @ > > + [@option{--set} @var{variable}] > > +Retrieve device tree information. > > + > > +The @command{fdtdump} command returns the value of a property in the device > > +tree provided by the firmware. The @option{--prop} option determines which > > +property to select. > > + > > +The default action is to print the value of the requested field to the > > console, > > +but a variable name can be specified with @option{--set} to store the value > > +instead of printing it. > > + > > +For example, this will store and then display the model string. > > + > > +@example > > +fdtdump --prop model --set machine_model > > +echo $machine_model > > +@end example > > +@end deffn > > + > > + > > @node fwsetup > > @subsection fwsetup > > > > diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c > > index 439964b9c..cb0422d98 100644 > > --- a/grub-core/loader/efi/fdt.c > > +++ b/grub-core/loader/efi/fdt.c > > @@ -1,6 +1,7 @@ > > /* > > * GRUB -- GRand Unified Bootloader > > * Copyright (C) 2013-2015 Free Software Foundation, Inc. > > + * Copyright (C) 2024 Canonical, Ltd. > > * > > * GRUB is free software: you can redistribute it and/or modify > > * it under the terms of the GNU General Public License as published by > > @@ -18,15 +19,18 @@ > > > > #include <grub/fdt.h> > > #include <grub/mm.h> > > +#include <grub/env.h> > > #include <grub/err.h> > > #include <grub/dl.h> > > #include <grub/command.h> > > +#include <grub/extcmd.h> > > #include <grub/file.h> > > #include <grub/efi/efi.h> > > #include <grub/efi/fdtload.h> > > #include <grub/efi/memory.h> > > #include <grub/cpu/efi/memory.h> > > > > +static void *fw_fdt; > > static void *loaded_fdt; > > static void *fdt; > > > > @@ -36,6 +40,13 @@ static void *fdt; > > sizeof (FDT_ADDR_CELLS_STRING) + \ > > sizeof (FDT_SIZE_CELLS_STRING)) > > > > +static const struct grub_arg_option options_fdtdump[] = { > > + {"prop", 'p', 0, N_("Get property."), N_("prop"), ARG_TYPE_STRING}, > > + {"set", '\0', 0, N_("Store the value in the given variable name."), > > + N_("variable"), ARG_TYPE_STRING}, > > + {0, 0, 0, 0, 0, 0} > > +}; > > + > > void * > > grub_fdt_load (grub_size_t additional_size) > > { > > @@ -51,7 +62,7 @@ grub_fdt_load (grub_size_t additional_size) > > if (loaded_fdt) > > raw_fdt = loaded_fdt; > > else > > - raw_fdt = grub_efi_get_firmware_fdt(); > > + raw_fdt = fw_fdt; > > > > if (raw_fdt) > > size = grub_fdt_get_totalsize (raw_fdt); > > @@ -167,10 +178,45 @@ out: > > return grub_errno; > > } > > > > +static grub_err_t > > +grub_cmd_fdtdump (grub_extcmd_context_t ctxt, > > + int argc __attribute__ ((unused)), > > + char **argv __attribute__ ((unused))) > > +{ > > + struct grub_arg_list *state = ctxt->state; > > + const char *value = NULL; > > + > > + if (fw_fdt == NULL) > > + return grub_error (GRUB_ERR_IO, > > + N_("No device tree found")); > > + > > + if (state[0].set) > > + value = grub_fdt_get_prop (fw_fdt, 0, state[0].arg, NULL); > > + > > + if (value == NULL) > > + return grub_error (GRUB_ERR_OUT_OF_RANGE, > > + N_("failed to retrieve the prop field")); > > + > > + if (state[1].set) > > + grub_env_set (state[1].arg, value); > > + else > > + grub_printf ("%s\n", value); > > + > > + return GRUB_ERR_NONE; > > +} > > + > > static grub_command_t cmd_devicetree; > > +static grub_extcmd_t cmd_fdtdump; > > > > GRUB_MOD_INIT (fdt) > > { > > + fw_fdt = grub_efi_get_firmware_fdt(); > > + > > + cmd_fdtdump = > > + grub_register_extcmd ("fdtdump", grub_cmd_fdtdump, 0, > > + N_("[-p] [--set variable]"), > > + N_("Retrieve device tree information."), > > + options_fdtdump); > > cmd_devicetree = > > grub_register_command_lockdown ("devicetree", grub_cmd_devicetree, 0, > > N_("Load DTB file.")); > > @@ -179,4 +225,5 @@ GRUB_MOD_INIT (fdt) > > GRUB_MOD_FINI (fdt) > > { > > grub_unregister_command (cmd_devicetree); > > + grub_unregister_extcmd (cmd_fdtdump); > > } > > -- > > 2.43.0 > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > -- > Regards > Vladimir 'phcoder' Serbinenko > > _______________________________________________ > 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