grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] loader/i386/linux: Add device tree support


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH] loader/i386/linux: Add device tree support
Date: Tue, 21 Sep 2021 21:05:31 +0200

On Tue, Sep 21, 2021 at 5:44 PM Mislav Stublić
<Mislav.Stublic@logicbricks.com> wrote:
>
> Hi,
>
> This implements device tree support for x86. Unfortunately I haven't been 
> able to test this on latest master but I have tested against 2.04 and it 
> works fine. I will test on master later but I wanted to get some initial 
> comments if this is going in the right direction. What I haven't tested is 
> firmware DTB loading support which only calls grub_efi_get_firmware_fdt from 
> kern/efi implementation as I don't have hardware to test this scenario.
>
> Mislav
>
> Signed-off-by: Mislav Stublić <mislav.stublic@logicbricks.com>
> ---
>  grub-core/Makefile.am         |   1 +
>  grub-core/Makefile.core.def   |   2 +
>  grub-core/loader/i386/linux.c | 159 
> ++++++++++++++++++++++++++++++++++++++++--
>  include/grub/efi/efi.h        |   5 +-
>  include/grub/i386/linux.h     |  21 +++++-
>  5 files changed, 181 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index ee88e44..1b9ba1f 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -186,6 +186,7 @@ KERNEL_HEADER_FILES += 
> $(top_srcdir)/include/grub/i386/tsc.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/pci.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/acpi.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/pmtimer.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/fdt.h
>  endif
>
>  if COND_ia64_efi
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 8022e1c..e499057 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -227,7 +227,9 @@ kernel = {
>    x86_64_xen = kern/x86_64/dl.c;
>    x86_64_efi = kern/x86_64/efi/callwrap.S;
>    x86_64_efi = kern/i386/efi/init.c;
> +  x86_64_efi = kern/efi/fdt.c;
>    x86_64_efi = bus/pci.c;
> +  x86_64_efi = lib/fdt.c;
Any reason to put it into kernel rather than enabling fdt module on x86?
>
>    xen = kern/i386/tsc.c;
>    xen = kern/i386/xen/tsc.c;
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 9f74a96..9c67a54 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -37,6 +37,7 @@
>  #include <grub/linux.h>
>  #include <grub/machine/kernel.h>
>  #include <grub/safemath.h>
> +#include <grub/fdt.h>
>
>  GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -77,6 +78,7 @@ static void *efi_mmap_buf;
>  static grub_size_t maximal_cmdline_size;
>  static struct linux_kernel_params linux_params;
>  static char *linux_cmdline;
> +static void *current_fdt = NULL;
>  #ifdef GRUB_MACHINE_EFI
>  static grub_efi_uintn_t efi_mmap_size;
>  #else
> @@ -398,6 +400,38 @@ grub_linux_boot_mmap_fill (grub_uint64_t addr, 
> grub_uint64_t size,
>    return 0;
>  }
>
> +struct grub_fdt_ctx
> +{
> +  grub_addr_t fdt_target;
> +  grub_memory_type_t mem_type;
> +};
> +
> +static int
> +grub_linux_boot_acpi_find (grub_uint64_t addr, grub_uint64_t size,
> +                          grub_memory_type_t type, void *data)
> +{
> +  struct grub_fdt_ctx *fdt_ctx = data;
> +  grub_uint64_t fdt_size = grub_fdt_get_totalsize(current_fdt);
> +  grub_uint64_t setup_data_header = sizeof(struct 
> linux_i386_kernel_header_setup_data);
> +
> +
> +  if (type != fdt_ctx->mem_type)
> +    return 0;
> +
> +  if (size < setup_data_header + fdt_size)
> +    return 0;
> +
> +  grub_dprintf ("fdt", "addr = 0x%lx, size = 0x%x, need_size = 0x%x, type = 
> %d\n",
> +               (unsigned long) addr,
> +               (unsigned) size,
> +               (unsigned) setup_data_header + fdt_size,
> +               fdt_ctx->mem_type);
> +
> +  fdt_ctx->fdt_target = addr;
> +
> +  return 1;
> +}
> +
>  static grub_err_t
>  grub_linux_boot (void)
>  {
> @@ -411,6 +445,10 @@ grub_linux_boot (void)
>    };
>    grub_size_t mmap_size;
>    grub_size_t cl_offset;
> +  struct grub_fdt_ctx fdt_ctx = {0};
> +  grub_uint64_t target_setup_data;
> +  struct linux_i386_kernel_header_setup_data *tmp_setup_data;
> +  grub_size_t fdt_size;
>
>  #ifdef GRUB_MACHINE_IEEE1275
>    {
> @@ -579,6 +617,57 @@ grub_linux_boot (void)
>      return grub_errno;
>    ctx.params->mmap_size = ctx.e820_num;
>
> +  if (current_fdt)
> +    {
> +      fdt_ctx.mem_type = GRUB_MEMORY_ACPI;
> +#ifdef GRUB_MACHINE_EFI
> +      grub_efi_mmap_iterate (grub_linux_boot_acpi_find, &fdt_ctx, 0);
> +#else
> +      grub_mmap_iterate (grub_linux_boot_acpi_find, &fdt_ctx);
> +#endif
> +      if (!fdt_ctx.fdt_target)
> +        {
> +          fdt_ctx.mem_type = GRUB_MEMORY_NVS;
> +#ifdef GRUB_MACHINE_EFI
> +          grub_efi_mmap_iterate (grub_linux_boot_acpi_find, &fdt_ctx, 0);
> +#else
> +          grub_mmap_iterate (grub_linux_boot_acpi_find, &fdt_ctx);
> +#endif
> +        }
> +    }
> +
> +  if (fdt_ctx.fdt_target)
> +    {
> +      grub_relocator_chunk_t ch;
> +
> +      fdt_size = grub_fdt_get_totalsize (current_fdt);
> +
> +      err = grub_relocator_alloc_chunk_addr (relocator, &ch,
> +                                            fdt_ctx.fdt_target,
> +                                            sizeof(*tmp_setup_data) +
> +                                            fdt_size);
> +      if (!err)
> +        {
> +          tmp_setup_data = grub_zalloc (sizeof (*tmp_setup_data) + fdt_size);
> +          if (tmp_setup_data)
> +            {
> +              target_setup_data = get_virtual_current_address (ch);
> +              tmp_setup_data->next = 0;
> +              tmp_setup_data->type = GRUB_SETUP_DTB;
> +              tmp_setup_data->len = fdt_size;
> +              grub_memcpy (tmp_setup_data->data, current_fdt, fdt_size);
> +              grub_memcpy (target_setup_data, tmp_setup_data,
> +                          sizeof (*tmp_setup_data) + fdt_size);
> +
> +              /* finalize FDT kernel target */
> +              ctx.params->setup_data = target_setup_data;
> +
> +              grub_free (tmp_setup_data);
> +              grub_free (current_fdt);
> +            }
> +        }
> +    }
> +
>  #ifdef GRUB_MACHINE_EFI
>    {
>      grub_efi_uintn_t efi_desc_size;
> @@ -591,9 +680,9 @@ grub_linux_boot (void)
>                                          &efi_desc_size, &efi_desc_version);
>      if (err)
>        return err;
> -
> +
Please don't include empty line changes
>      /* Note that no boot services are available from here.  */
> -    efi_mmap_target = ctx.real_mode_target
> +    efi_mmap_target = ctx.real_mode_target
>        + ((grub_uint8_t *) efi_mmap_buf - (grub_uint8_t *) real_mode_mem);
>      /* Pass EFI parameters.  */
>      if (grub_le_to_cpu16 (ctx.params->version) >= 0x0208)
> @@ -743,7 +832,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>        align = 0;
>        relocatable = 0;
>      }
> -
> +
>    if (grub_le_to_cpu16 (lh.version) >= 0x020a)
>      {
>        min_align = lh.min_alignment;
> @@ -1123,7 +1212,66 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>    return grub_errno;
>  }
>
> -static grub_command_t cmd_linux, cmd_initrd;
> +static grub_err_t
> +grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),
> +                    int argc, char *argv[])
> +{
> +  grub_file_t dtb;
> +  void *new_fdt;
> +  int size;
> +
> +  if (current_fdt)
> +    grub_free (current_fdt);
> +  current_fdt = NULL;
> +
> +#ifdef GRUB_MACHINE_EFI
> +  /* no dtb file, try firmware */
> +  if (argc == 0)
> +    {
> +      current_fdt = grub_efi_get_firmware_fdt();
You may want to have an implementation than always returns NULL on
other platforms
> +      if (current_fdt)
> +        return GRUB_ERR_NONE;
> +
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("firmware dtb not 
> found"));
> +    }
> +#endif
> +
> +  if (argc != 1)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("dtb filename expected"));
> +
> +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE_IMAGE);
> +  if (!dtb)
> +    return grub_errno;
> +
> +  size = grub_file_size (dtb);
> +  new_fdt = grub_zalloc (size);
> +  if (!new_fdt)
> +    return grub_errno;
> +
> +  grub_dprintf ("loader", "Loading device tree to %p\n",
> +               new_fdt);
> +  if (grub_file_read (dtb, new_fdt, size) < size)
> +    {
> +      grub_free (new_fdt);
> +      return grub_error (GRUB_ERR_FILE_READ_ERROR,
> +                        N_("unable to read dtb file %s"),
> +                        argv[0]);
> +    }
> +
> +  if (grub_fdt_check_header (new_fdt, size) != 0)
> +    {
> +      grub_free (new_fdt);
> +      return grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("invalid device tree"));
> +    }
> +
> +  current_fdt = new_fdt;
> +
> +  grub_file_close (dtb);
> +
> +  return grub_errno;
> +}
> +
> +static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree;
>
>  GRUB_MOD_INIT(linux)
>  {
> @@ -1131,6 +1279,9 @@ GRUB_MOD_INIT(linux)
>                                      0, N_("Load Linux."));
>    cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
>                                       0, N_("Load initrd."));
> +  cmd_devicetree = grub_register_command_lockdown ("devicetree", 
> grub_cmd_devicetree,
> +                                                  /* TRANSLATORS: DTB stands 
> for device tree blob.  */
> +                                                  0, N_("Load DTB file."));
Please use the same sentence as in another loaders
>    my_mod = mod;
>  }
>
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 83d958f..a095925 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -96,8 +96,11 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) 
> (grub_efi_handle_t hnd,
>                                                 char **device,
>                                                 char **path);
>
> -#if defined(__arm__) || defined(__aarch64__) || defined(__riscv)
> +#if defined(__arm__) || defined(__aarch64__) || defined(__riscv) || 
> defined(__x86_64__)
>  void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
> +#endif
> +
> +#if defined(__arm__) || defined(__aarch64__) || defined(__riscv)
>  grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *);
>  #include <grub/cpu/linux.h>
>  grub_err_t grub_arch_efi_linux_check_image(struct linux_arch_kernel_header 
> *lh);
> diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
> index eddf925..b66413d 100644
> --- a/include/grub/i386/linux.h
> +++ b/include/grub/i386/linux.h
> @@ -84,6 +84,23 @@ struct grub_e820_mmap
>    grub_uint32_t type;
>  } GRUB_PACKED;
>
> +/* taken from linux kernel bootparam.h */
> +#define GRUB_SETUP_NONE                      0
> +#define GRUB_SETUP_E820_EXT                  1
> +#define GRUB_SETUP_DTB                       2
> +#define GRUB_SETUP_PCI                       3
> +#define GRUB_SETUP_EFI                       4
> +#define GRUB_SETUP_APPLE_PROPERTIES          5
> +#define GRUB_SETUP_JAILHOUSE                 6
> +#define GRUB_SETUP_INDIRECT                  (1<<31)
> +
> +struct linux_i386_kernel_header_setup_data {
> +  grub_uint64_t next;
> +  grub_uint32_t type;
> +  grub_uint32_t len;
> +  grub_uint8_t data[0];
> +};
> +
>  enum
>    {
>      GRUB_VIDEO_LINUX_TYPE_TEXT = 0x01,
> @@ -134,7 +151,7 @@ struct linux_i386_kernel_header
>    grub_uint16_t heap_end_ptr;          /* Free memory after setup end */
>    grub_uint16_t pad1;                  /* Unused */
>    grub_uint32_t cmd_line_ptr;          /* Points to the kernel command line 
> */
> -  grub_uint32_t initrd_addr_max;        /* Highest address for initrd */
> +  grub_uint32_t initrd_addr_max;       /* Highest address for initrd */
>    grub_uint32_t kernel_alignment;
>    grub_uint8_t relocatable;
>    grub_uint8_t min_alignment;
> @@ -144,7 +161,7 @@ struct linux_i386_kernel_header
>    grub_uint64_t hardware_subarch_data;
>    grub_uint32_t payload_offset;
>    grub_uint32_t payload_length;
> -  grub_uint64_t setup_data;
> +  grub_uint64_t setup_data;            /* linked list of additional boot 
> parameters (E820, DTB, PCI)*/
>    grub_uint64_t pref_address;
>    grub_uint32_t init_size;
>    grub_uint32_t handover_offset;
> --
> 1.8.3.1
>
>
> ----- Disclaimer -----
> This e-mail message and its attachments may contain privileged and/or 
> confidential information. Please do not read the message if You are not its 
> designated recipient. If You have received this message by mistake, please 
> inform its sender and destroy the original message and its attachments 
> without reading or storing of any kind. Any unauthorized use, distribution, 
> reproduction or publication of this message is forbidden.
>
> ----- Pravne napomene -----
> Ova elektronicka poruka i njeni prilozi mogu sadrzavati povlastene 
> informacije i/ili povjerljive informacije. Molimo Vas da poruku ne citate ako 
> niste njen naznaceni primatelj. Ako ste ovu poruku primili greskom, molimo 
> Vas da o tome obavijestite posiljatelja i da izvornu poruku i njene privitke 
> unistite bez citanja ili bilo kakvog pohranjivanja. Svaka neovlastena 
> upotreba, distribucija, reprodukcija ili priopcavanje ove poruke zabranjena 
> je.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko



reply via email to

[Prev in Thread] Current Thread [Next in Thread]