Change in coreboot[master]: arch/arm: Enable FIT payloads

[Sam Lewis (Code Review)]

Sam Lewis has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly  been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 120 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44377/1

diff --git a/Documentation/lib/payloads/fit.md b/Documentation/lib/payloads/fit.md
index c6ccc7b..ef5e892 100644
--- a/Documentation/lib/payloads/fit.md
+++ b/Documentation/lib/payloads/fit.md
@@ -5,6 +5,7 @@

## Supported architectures

+* aarch32
 * aarch64
 * riscv

@@ -26,6 +27,13 @@

The FIT parser needs architecure support.

+### aarch32
+The source code can be found in `src/arch/arm/fit_payload.c`.
+
+On aarch32 the kernel (a section named 'kernel') must be in **Image**
+format and it needs a devicetree (a section named 'fdt') to boot.
+The kernel will be placed close to "*DRAMSTART*".
+
 ### aarch64
 The source code can be found in `src/arch/arm64/fit_payload.c`.

diff --git a/payloads/Kconfig b/payloads/Kconfig
index cfb28d6..627bb95 100644
--- a/payloads/Kconfig
+++ b/payloads/Kconfig
@@ -30,7 +30,7 @@

config PAYLOAD_FIT
    bool "A FIT payload"
-	depends on ARCH_ARM64 || ARCH_RISCV
+	depends on ARCH_ARM64 || ARCH_RISCV || ARCH_ARM
    select PAYLOAD_FIT_SUPPORT
    help
      Select this option if you have a payload image (a FIT file) which
@@ -97,7 +97,7 @@
    bool "FIT support"
    default n
    default y if PAYLOAD_LINUX && (ARCH_ARM || ARCH_ARM64 || ARCH_RISCV)
-	depends on ARCH_ARM64 || ARCH_RISCV
+	depends on ARCH_ARM64 || ARCH_RISCV || ARCH_ARM
    select FLATTENED_DEVICE_TREE
    help
      Select this option if your payload is of type FIT.
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index eef2650..241bfe5 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -119,6 +119,7 @@
 ramstage-y += memcpy.S
 ramstage-y += memmove.S
 ramstage-y += clock.c
+ramstage-$(CONFIG_PAYLOAD_FIT_SUPPORT) += fit_payload.c

rmodules_arm-y += memset.S
 rmodules_arm-y += memcpy.S
diff --git a/src/arch/arm/fit_payload.c b/src/arch/arm/fit_payload.c
new file mode 100644
index 0000000..39cb0f2
--- /dev/null
+++ b/src/arch/arm/fit_payload.c
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <console/console.h>
+#include <bootmem.h>
+#include <program_loading.h>
+#include <fit.h>
+
+#define MAX_KERNEL_SIZE (64 * MiB)
+
+static size_t get_kernel_size(const struct fit_image_node *node)
+{
+	/*
+	 * Since we don't have a way to determine the uncompressed size of the
+	 * kernel, we have to keep as much memory as possible free for use by
+	 * the kernel immediately after the end of the kernel image. The amount
+	 * of space required will vary depending on selected features, and is
+	 * effectively unbound.
+	 */
+
+	printk(BIOS_INFO, "FIT: Leaving additional %u MiB of free space after kernel.\n",
+	       MAX_KERNEL_SIZE >> 20);
+
+	return node->size + MAX_KERNEL_SIZE;
+}
+
+
+/**
+ * Place the region in free memory range.
+ *
+ * The caller has to set region->offset to the minimum allowed address.
+ * The region->offset is usually 0 on kernel >v4.6 and kernel_base + kernel_size
+ * on kernel <v4.6.
+ */
+static bool fit_place_mem(const struct range_entry *r, void *arg)
+{
+	struct region *region = arg;
+	resource_t start;
+
+	if (range_entry_tag(r) != BM_MEM_RAM)
+		return true;
+
+	/* Linux 4.15 doesn't like 4KiB alignment. Align to 1 MiB for now. */
+	start = ALIGN_UP(MAX(region->offset, range_entry_base(r)), 1 * MiB);
+
+	if (start + region->size < range_entry_end(r)) {
+		region->offset = (size_t)start;
+		return false;
+	}
+
+	return true;
+}
+
+
+bool fit_payload_arch(struct prog *payload, struct fit_config_node *config,
+		      struct region *kernel,
+		      struct region *fdt,
+		      struct region *initrd)
+{
+	void *arg = NULL;
+
+	kernel->size = get_kernel_size(config->kernel);
+	printk(BIOS_DEBUG, "FIT: Reserving 0x%zx bytes for kernel\n", kernel->size);
+
+	/**
+	 * The code assumes that bootmem_walk provides a sorted list of memory
+	 * regions, starting from the lowest address.
+	 * The order of the calls here doesn't matter, as the placement is
+	 * enforced in the called functions.
+	 * For details check code on top.
+	 */
+
+	kernel->offset = 0;
+	if (!bootmem_walk(fit_place_mem, kernel)) {
+		printk(BIOS_DEBUG, "Couldnt find room for kernel\n");
+		return false;
+	}
+
+	/* Mark as reserved for future allocations. */
+	bootmem_add_range(kernel->offset, kernel->size, BM_MEM_PAYLOAD);
+
+	/* Place INITRD */
+	if (config->ramdisk) {
+		initrd->offset = kernel->offset + kernel->size;
+
+		if (!bootmem_walk(fit_place_mem, initrd))
+			return false;
+
+		/* Mark as reserved for future allocations. */
+		bootmem_add_range(initrd->offset, initrd->size, BM_MEM_PAYLOAD);
+	}
+
+
+	fdt->offset = kernel->offset + kernel->size;
+
+	if (!bootmem_walk(fit_place_mem, fdt))
+		return false;
+
+	/* Mark as reserved for future allocations. */
+	bootmem_add_range(fdt->offset, fdt->size, BM_MEM_PAYLOAD);
+
+	/* Kernel expects FDT as argument */
+	arg = (void *)fdt->offset;
+
+	prog_set_entry(payload, (void *)kernel->offset, arg);
+
+	bootmem_dump_ranges();
+
+	return true;
+}

[Sam Lewis (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44377

to look at the new patch set (#2).

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly  been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 120 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44377/2

[Sam Lewis (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44377

to look at the new patch set (#3).

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly  been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 120 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44377/3

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

Very nice.

https://review.coreboot.org/c/coreboot/+/44377/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44377/2//COMMIT_MSG@11 
PS2, Line 11: mostly  been
Nit: Just one space.

[Sam Lewis (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44377

to look at the new patch set (#4).

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 120 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44377/4

[Sam Lewis (Code Review)]

Sam Lewis has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44377/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44377/2//COMMIT_MSG@11 
PS2, Line 11: mostly  been
> Nit: Just one space.

Done

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c 
File src/arch/arm/fit_payload.c:

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c@... 
PS4, Line 17: 	 * effectively unbound.
Unlike arm64, arm32 kernels are usually built with a built-in decompressor. That's why using FIT compression doesn't really make sense for them. So I think it's probably fine to just use kernel->size for the amount of space to allocate (like we're already doing for initrd for the same reason), and maybe just return an error if (kernel->compression != CBFS_COMPRESS_NONE).

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c@... 
PS4, Line 72: 	kernel->offset = 0;
Documentation/arm/booting.rst says "It is recommended that it is loaded above 32MiB in order to avoid the need to relocate prior to decompression, which will make the boot process slightly faster." so maybe start this at 32*MiB?

[Sam Lewis (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44377

to look at the new patch set (#6).

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 101 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44377/6

[Sam Lewis (Code Review)]

Sam Lewis has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

Patch Set 6:

(2 comments)

Looks like this largely duplicates https://review.coreboot.org/c/coreboot/+/35410 but I'm not sure if that's still active, so I'll keep this open.

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c 
File src/arch/arm/fit_payload.c:

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c@... 
PS4, Line 17: 	 * effectively unbound.
> Unlike arm64, arm32 kernels are usually built with a built-in decompressor. […]

Thanks for the explanation, that makes sense. I thought that this bit of "over reserving" was for a slightly different purpose: as kernel->size is the compressed size of the kernel, I thought that kernel->size was inflated so that the fdt and ramdisk wouldn't be overwritten by the kernel decompressor.

I've removed this now and changed the fdt/ramdisk placing logic to make sure they won't get overwritten by the decompressor.

https://review.coreboot.org/c/coreboot/+/44377/4/src/arch/arm/fit_payload.c@... 
PS4, Line 72: 	kernel->offset = 0;
> Documentation/arm/booting. […]

Good idea, done.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

Patch Set 6: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44377 )

Change subject: arch/arm: Enable FIT payloads
......................................................................

arch/arm: Enable FIT payloads

Implements fit_payload_arch for the arm (aarch32) architecture, so that
FIT images can be used. The implementation is very similar to the
existing implementations for arm64 and riscv, and has mostly been
lifted from these other ports.

TEST: Booted Beaglebone Black (in progress port, to be submitted soon!)
with a FIT image containing a 5.4 kernel, dtb and initramfs.

Change-Id: I6b50c6f06b83c00a5b3622b5bbafe67130b6d233
Signed-off-by: Sam Lewis 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44377
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M Documentation/lib/payloads/fit.md
M payloads/Kconfig
M src/arch/arm/Makefile.inc
A src/arch/arm/fit_payload.c
4 files changed, 101 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/Documentation/lib/payloads/fit.md b/Documentation/lib/payloads/fit.md
index c6ccc7b..ef5e892 100644
--- a/Documentation/lib/payloads/fit.md
+++ b/Documentation/lib/payloads/fit.md
@@ -5,6 +5,7 @@

## Supported architectures

+* aarch32
 * aarch64
 * riscv

@@ -26,6 +27,13 @@

The FIT parser needs architecure support.

+### aarch32
+The source code can be found in `src/arch/arm/fit_payload.c`.
+
+On aarch32 the kernel (a section named 'kernel') must be in **Image**
+format and it needs a devicetree (a section named 'fdt') to boot.
+The kernel will be placed close to "*DRAMSTART*".
+
 ### aarch64
 The source code can be found in `src/arch/arm64/fit_payload.c`.

diff --git a/payloads/Kconfig b/payloads/Kconfig
index cfb28d6..627bb95 100644
--- a/payloads/Kconfig
+++ b/payloads/Kconfig
@@ -30,7 +30,7 @@

config PAYLOAD_FIT
    bool "A FIT payload"
-	depends on ARCH_ARM64 || ARCH_RISCV
+	depends on ARCH_ARM64 || ARCH_RISCV || ARCH_ARM
    select PAYLOAD_FIT_SUPPORT
    help
      Select this option if you have a payload image (a FIT file) which
@@ -97,7 +97,7 @@
    bool "FIT support"
    default n
    default y if PAYLOAD_LINUX && (ARCH_ARM || ARCH_ARM64 || ARCH_RISCV)
-	depends on ARCH_ARM64 || ARCH_RISCV
+	depends on ARCH_ARM64 || ARCH_RISCV || ARCH_ARM
    select FLATTENED_DEVICE_TREE
    help
      Select this option if your payload is of type FIT.
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index eef2650..241bfe5 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -119,6 +119,7 @@
 ramstage-y += memcpy.S
 ramstage-y += memmove.S
 ramstage-y += clock.c
+ramstage-$(CONFIG_PAYLOAD_FIT_SUPPORT) += fit_payload.c

rmodules_arm-y += memset.S
 rmodules_arm-y += memcpy.S
diff --git a/src/arch/arm/fit_payload.c b/src/arch/arm/fit_payload.c
new file mode 100644
index 0000000..f547007
--- /dev/null
+++ b/src/arch/arm/fit_payload.c
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <console/console.h>
+#include <bootmem.h>
+#include <program_loading.h>
+#include <fit.h>
+#include <symbols.h>
+
+/**
+ * Place the region in free memory range.
+ */
+static bool fit_place_mem(const struct range_entry *r, void *arg)
+{
+	struct region *region = arg;
+	resource_t start;
+
+	if (range_entry_tag(r) != BM_MEM_RAM)
+		return true;
+
+	/* Linux 4.15 doesn't like 4KiB alignment. Align to 1 MiB for now. */
+	start = ALIGN_UP(MAX(region->offset, range_entry_base(r)), 1 * MiB);
+
+	if (start + region->size < range_entry_end(r)) {
+		region->offset = (size_t)start;
+		return false;
+	}
+
+	return true;
+}
+
+
+bool fit_payload_arch(struct prog *payload, struct fit_config_node *config,
+		      struct region *kernel,
+		      struct region *fdt,
+		      struct region *initrd)
+{
+	void *arg = NULL;
+
+	/**
+	 * The kernel ARM documentation recommends loading the kernel above 32MiB
+	 * in order to avoid the need to need to relocate prior to decompression.
+	 */
+	kernel->offset = (uintptr_t)_dram + 32 * MiB;
+
+	/**
+	 * The code assumes that bootmem_walk provides a sorted list of memory
+	 * regions, starting from the lowest address.
+	 * The order of the calls here doesn't matter, as the placement is
+	 * enforced in the called functions.
+	 * For details check code on top.
+	 */
+	if (!bootmem_walk(fit_place_mem, kernel))
+		return false;
+
+	/* Mark as reserved for future allocations. */
+	bootmem_add_range(kernel->offset, kernel->size, BM_MEM_PAYLOAD);
+
+	/**
+	 * To ensure the fdt is not overwritten by the kernel decompressor, place
+	 * the fdt above the 128 MB from the start of RAM, as recommended by the
+	 * kernel documentation.
+	 */
+	fdt->offset = (uintptr_t)_dram + 128 * MiB;
+
+	if (!bootmem_walk(fit_place_mem, fdt))
+		return false;
+
+	/* Mark as reserved for future allocations. */
+	bootmem_add_range(fdt->offset, fdt->size, BM_MEM_PAYLOAD);
+
+	/* Place INITRD */
+	if (config->ramdisk) {
+		initrd->offset = fdt->offset + fdt->size;
+
+		if (!bootmem_walk(fit_place_mem, initrd))
+			return false;
+
+		/* Mark as reserved for future allocations. */
+		bootmem_add_range(initrd->offset, initrd->size, BM_MEM_PAYLOAD);
+	}
+
+	/* Kernel expects FDT as argument */
+	arg = (void *)fdt->offset;
+
+	prog_set_entry(payload, (void *)kernel->offset, arg);
+
+	bootmem_dump_ranges();
+
+	return true;
+}