[PATCH] loader/i386/linux: Add device tree support
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ć
---
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;
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
#include
#include
+#include
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
Re: [SPECIFICATION RFC v3] The firmware and bootloader log specification
Alec Brown wrote: > Below is how the layout of these logs would store their data. > > bf_log_header: >+---+ > u32| version | > u32| size | > u8[64] | producer | > u8[64] | log_format| > u64| flags | > u64| next_bflh_addr| > u64| log_addr | > u32| log_size | >+---+ I suggest to include a .magic at least in bf_log_header and an .xor_checksum or .crc32 only in bf_log_header. .magic doubles as endianess indicator when the structures are stored on movable media. (Pick an asymmetric magic bit pattern!) I suggest renaming .next_bflh_addr to .next_log_header and .log_addr to .log_buffer_addr. I suggest to remove .size and .log_size: The rationale for .size is "to allow for backward compatibility" but that seems redundant thanks to .version. .log_size can be calculated from the subordinate data and is thus mostly an unneccessary source of potential inconsistency. > bf_log_buffer: >+---+ > u32| version | > u32| size | > u8[64] | producer | > u32| next_msg_off | > bf_log_msg[l] | msgs | >+---+ I suggest replacing .size and .next_msg_off with .messages containing l: .size can then be calculated from .messages; again, reliably avoiding inconsistency between .size and .next_msg_off. Allocated size doesn't seem useful if writers must anyway maintain state containing the starting address. If writers must be allowed to be completely stateless then maybe at least rename .size to .allocated_size and see below for discovery. Having .messages also eliminates the need for an end-of-messages marker when the allocated space is not yet filled. > bf_log_msg: >+---+ > u32| size | > u64| ts_nsec | > u32| level | > u32| facility | > u32| msg_off | > u8[n] | type | > u8[m] | msg | >+---+ It seems inconsistent that log_header.size and log_msg.size cover only the respective struct itself while log_buffer.size also covers all subordinate messages. Skipping all .size in this version fixes that. And log_msg.size is not very useful since both .type and .msg have variable length; it's not possible to access .msg without scanning .type. Please at a minimum add .type_size but better yet replace .size with .type_size and .msg_size. > There is still the outstanding issue of how the logs will be sent to the OS. > If > UEFI is used, we can use config tables. If ACPI or Device Tree is used, we can > use bf_log_header.next_bflh_addr to present the logs. If none of these > platforms > are used, it becomes a lot trickier to solve this issue. > > Any suggestions are much appreciated and will be taken into consideration. Having bf_log_header.magic and some bf_log_header.$checksum, a strict rule for bf_log_header start address granularity and a strict maximum offset for the first header from top and/or bottom of memory allows to quickly discover a log in memory without explicit handover. > LPC System Boot and Security Micro-conference on the 22nd of September > at 7:50 AM PDT (14:50 UTC). Have fun! :) Heinrich Schuchardt wrote: > We already the EFI_TCG2_PROTOCOL and RFC 5424 (The syslog protocol). > Why do we need to start from scratch? That's a good question. I guess noone wants to settle for a standard from somewhere else. ;) I wouldn't mind if log_msg was a syslog transport, but I can understand if that's rejected because syslog messages require a lot of parsing for presentation while Alec's proposal seems focused on efficiency and simplicity. It's also nice to be able to strictly mandate UTF-8 for all fields. (RFC 5424 allows MSG to be anything.) Kind regards //Peter ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 08/10] grub-shell: Boot PowerPC using PMU instead of CUDA for power management
Dear Glenn,
Thank you. I noticed two small typos.
Am 18.09.21 um 01:04 schrieb Glenn Washburn:
At some point it looks like the defualt machine for qemu-system-ppc started
default
using CUDA as a backend for power management. This causes the machine to
throw an exception and not actually power down the VM[1]. Switching to PMU
allows power downs and reboots to work as expceted.
expected
[1] https://gitlab.com/qemu-project/qemu/-/issues/624
Thank you for reporting this upstream.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 93e9f5148..5354d8678 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -84,6 +84,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
serial_null="-serial null"
netbootext=elf
trim=1
+ qemuopts="-M mac99,via=pmu $qemuopts"
;;
sparc64-ieee1275)
Reviewed-by: Paul Menzel
Kind regards,
Paul
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] loader/i386/linux: Add device tree support
On Tue, Sep 21, 2021 at 5:44 PM Mislav Stublić
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ć
> ---
> 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
> #include
> #include
> +#include
>
> 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) +
> +
Re: [SPECIFICATION RFC v3] The firmware and bootloader log specification
> Since it doesn't seem possible to have each boot component using the same log > format, we added a log_format and log_phys_addr fields to give flexibility in > how logs are stored. An example of a different log format that can be used is > the cbmem_console log format used by coreboot: I am not exactly sure how you expect this interoperability you seem to be suggesting here to work. Are you saying that your bf_log_header can sometimes point to the bf_log_buffer structure you define, and sometimes to a coreboot CBMEM console buffer? But that's a completely different format that requires a different reader implementation, how is that supposed to work? If this proposal is just "we define a wrapper structure that points to everyone's custom firmware log implementation", then I don't really see the point (the only benefit still left then might be discovery of the log buffer, but that's the part you leave open in your design while all those other implementations already have working discovery mechanisms of their own anyway). For the other structures you have defined, the same feedback that I think was already mentioned on the last iteration of this thread still applies: it seems incredibly bloated for a simple firmware logging mechanism. You have a whooping 24+n bytes of overhead *per line* which probably comes out to somewhere between 30-50% of total space wasted on overhead for the average log buffer. I guess there are just fundamentally different opinions on how featureful a firmware log mechanism needs to be so we're probably not gonna find a format that makes everyone happy here, but at least for the coreboot project I see little reason for us to implement something like this when we already have a well-working existing solution with tooling and wideranged support. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
