[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