Re: [PATCH v2] loader/efi/fdt: Add fdtdump command to access device tree

[Tobias Heider]

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
<phcoder@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.heider@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.heider@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