diff mbox series

[v11,09/15] arm64: kexec_file: load initrd and device-tree

Message ID 20180711074203.3019-10-takahiro.akashi@linaro.org
State Superseded
Headers show
Series subject: arm64: kexec: add kexec_file_load() support | expand

Commit Message

AKASHI Takahiro July 11, 2018, 7:41 a.m. UTC 
load_other_segments() is expected to allocate and place all the necessary
memory segments other than kernel, including initrd and device-tree
blob (and elf core header for crash).
While most of the code was borrowed from kexec-tools' counterpart,
users may not be allowed to specify dtb explicitly, instead, the dtb
presented by the original boot loader is reused.

arch_kimage_kernel_post_load_cleanup() is responsible for freeing arm64-
specific data allocated in load_other_segments().

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/kexec.h         |  16 +++
 arch/arm64/kernel/machine_kexec_file.c | 184 +++++++++++++++++++++++++
 2 files changed, 200 insertions(+)

-- 
2.17.0

Comments

James Morse July 17, 2018, 4:57 p.m. UTC | #1 
Hi Akashi,

On 11/07/18 08:41, AKASHI Takahiro wrote:
> load_other_segments() is expected to allocate and place all the necessary

> memory segments other than kernel, including initrd and device-tree

> blob (and elf core header for crash).

> While most of the code was borrowed from kexec-tools' counterpart,

> users may not be allowed to specify dtb explicitly, instead, the dtb

> presented by the original boot loader is reused.

> 

> arch_kimage_kernel_post_load_cleanup() is responsible for freeing arm64-

> specific data allocated in load_other_segments().


> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c

> index c38a8048ed00..ca00681c25c6 100644

> --- a/arch/arm64/kernel/machine_kexec_file.c

> +++ b/arch/arm64/kernel/machine_kexec_file.c


> +int arch_kimage_file_post_load_cleanup(struct kimage *image)

> +{

> +	vfree(image->arch.dtb_buf);

> +	image->arch.dtb_buf = NULL;

> +

> +	return kexec_image_post_load_cleanup_default(image);

> +}


A nit from sparse:
| warning: symbol 'arch_kimage_file_post_load_cleanup' was not declared

Can we add a definition for this to a header file somewhere. asm/kexec.h is
probably the best bet.


> +static int setup_dtb(struct kimage *image,

> +		unsigned long initrd_load_addr, unsigned long initrd_len,

> +		char *cmdline, unsigned long cmdline_len,

> +		char **dtb_buf, size_t *dtb_buf_len)

> +{


> +	/* add initrd-* */

> +	if (initrd_load_addr) {

> +		value = cpu_to_fdt64(initrd_load_addr);

> +		ret = fdt_setprop_u64(buf, nodeoffset, "linux,initrd-start",

> +							value);


fdt_setprop_u64() already does the endian conversion.

From scripts/dtc/libfdt/libfdt.h, its implemented as:
| 	fdt64_t tmp = cpu_to_fdt64(val);
| 	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));

(I think you were using setprop directly in an older version)


This leads to:
| ------------[ cut here ]------------
| initrd not fully accessible via the linear mapping -- please check your
| bootloader ...
| WARNING: CPU: 0 PID: 0 at ../arch/arm64/mm/init.c:429
| arm64_memblock_init+0x150/0x3d8
| Modules linked in:
| CPU: 0 PID: 0 Comm: swapper Not tainted 4.18.0-rc5-00015-g95b5c843d0da #10150
| Hardware name: AMD Seattle (Rev.B0) Development Board (Overdrive) (DT)
| pstate: 60000085 (nZCv daIf -PAN -UAO)
| pc : arm64_memblock_init+0x150/0x3d8
| lr : arm64_memblock_init+0x150/0x3d8

| Call trace:
| arm64_memblock_init+0x150/0x3d8
| setup_arch+0x1c0/0x510
| start_kernel+0x80/0x418
| random: get_random_bytes called from print_oops_end_marker+0x4c/0x68 with
| crng_init=0
| ---[ end trace 0000000000000000 ]---


Which is caused by the values being miles outside ram due to the extra byte
swapping:
| morse@frikadeller:~$ sudo dtc -I dtb -O dts /sys/firmware/fdt  | grep initrd
|                 linux,initrd-end = <0x900b6c05 0x80000000>;
|                 linux,initrd-start = <0x906a04 0x80000000>;


With the two extra cpu_to_fdt64() calls removed:
Reviewed-by: James Morse <james.morse@arm.com>



Thanks,

James
AKASHI Takahiro July 18, 2018, 5:56 a.m. UTC | #2 
James,

On Tue, Jul 17, 2018 at 05:57:06PM +0100, James Morse wrote:
> Hi Akashi,

> 

> On 11/07/18 08:41, AKASHI Takahiro wrote:

> > load_other_segments() is expected to allocate and place all the necessary

> > memory segments other than kernel, including initrd and device-tree

> > blob (and elf core header for crash).

> > While most of the code was borrowed from kexec-tools' counterpart,

> > users may not be allowed to specify dtb explicitly, instead, the dtb

> > presented by the original boot loader is reused.

> > 

> > arch_kimage_kernel_post_load_cleanup() is responsible for freeing arm64-

> > specific data allocated in load_other_segments().

> 

> > diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c

> > index c38a8048ed00..ca00681c25c6 100644

> > --- a/arch/arm64/kernel/machine_kexec_file.c

> > +++ b/arch/arm64/kernel/machine_kexec_file.c

> 

> > +int arch_kimage_file_post_load_cleanup(struct kimage *image)

> > +{

> > +	vfree(image->arch.dtb_buf);

> > +	image->arch.dtb_buf = NULL;

> > +

> > +	return kexec_image_post_load_cleanup_default(image);

> > +}

> 

> A nit from sparse:

> | warning: symbol 'arch_kimage_file_post_load_cleanup' was not declared

> 

> Can we add a definition for this to a header file somewhere. asm/kexec.h is

> probably the best bet.


Sparse! Ok, I will fix it.

> > +static int setup_dtb(struct kimage *image,

> > +		unsigned long initrd_load_addr, unsigned long initrd_len,

> > +		char *cmdline, unsigned long cmdline_len,

> > +		char **dtb_buf, size_t *dtb_buf_len)

> > +{

> 

> > +	/* add initrd-* */

> > +	if (initrd_load_addr) {

> > +		value = cpu_to_fdt64(initrd_load_addr);

> > +		ret = fdt_setprop_u64(buf, nodeoffset, "linux,initrd-start",

> > +							value);

> 

> fdt_setprop_u64() already does the endian conversion.

> 

> From scripts/dtc/libfdt/libfdt.h, its implemented as:

> | 	fdt64_t tmp = cpu_to_fdt64(val);

> | 	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));

> 

> (I think you were using setprop directly in an older version)


Indeed.

> This leads to:

> | ------------[ cut here ]------------

> | initrd not fully accessible via the linear mapping -- please check your

> | bootloader ...

> | WARNING: CPU: 0 PID: 0 at ../arch/arm64/mm/init.c:429

> | arm64_memblock_init+0x150/0x3d8

> | Modules linked in:

> | CPU: 0 PID: 0 Comm: swapper Not tainted 4.18.0-rc5-00015-g95b5c843d0da #10150

> | Hardware name: AMD Seattle (Rev.B0) Development Board (Overdrive) (DT)

> | pstate: 60000085 (nZCv daIf -PAN -UAO)

> | pc : arm64_memblock_init+0x150/0x3d8

> | lr : arm64_memblock_init+0x150/0x3d8

> 

> | Call trace:

> | arm64_memblock_init+0x150/0x3d8

> | setup_arch+0x1c0/0x510

> | start_kernel+0x80/0x418

> | random: get_random_bytes called from print_oops_end_marker+0x4c/0x68 with

> | crng_init=0

> | ---[ end trace 0000000000000000 ]---

> 

> 

> Which is caused by the values being miles outside ram due to the extra byte

> swapping:


So it is in little endian.

> | morse@frikadeller:~$ sudo dtc -I dtb -O dts /sys/firmware/fdt  | grep initrd

> |                 linux,initrd-end = <0x900b6c05 0x80000000>;

> |                 linux,initrd-start = <0x906a04 0x80000000>;

> 

> 

> With the two extra cpu_to_fdt64() calls removed:

> Reviewed-by: James Morse <james.morse@arm.com>


Thank you for your review.

-Takahiro AKASHI

> 

> Thanks,

> 

> James
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index e17f0529a882..01bbf6cebf12 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -93,6 +93,22 @@  static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#ifdef CONFIG_KEXEC_FILE
+#define ARCH_HAS_KIMAGE_ARCH
+
+struct kimage_arch {
+	phys_addr_t dtb_mem;
+	void *dtb_buf;
+};
+
+struct kimage;
+
+extern int load_other_segments(struct kimage *image,
+		unsigned long kernel_load_addr, unsigned long kernel_size,
+		char *initrd, unsigned long initrd_len,
+		char *cmdline, unsigned long cmdline_len);
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index c38a8048ed00..ca00681c25c6 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -5,12 +5,196 @@ 
  * Copyright (C) 2018 Linaro Limited
  * Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
  *
+ * Most code is derived from arm64 port of kexec-tools
  */
 
 #define pr_fmt(fmt) "kexec_file: " fmt
 
+#include <linux/ioport.h>
+#include <linux/kernel.h>
 #include <linux/kexec.h>
+#include <linux/libfdt.h>
+#include <linux/memblock.h>
+#include <linux/of_fdt.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
 
 const struct kexec_file_ops * const kexec_file_loaders[] = {
 	NULL
 };
+
+int arch_kimage_file_post_load_cleanup(struct kimage *image)
+{
+	vfree(image->arch.dtb_buf);
+	image->arch.dtb_buf = NULL;
+
+	return kexec_image_post_load_cleanup_default(image);
+}
+
+static int setup_dtb(struct kimage *image,
+		unsigned long initrd_load_addr, unsigned long initrd_len,
+		char *cmdline, unsigned long cmdline_len,
+		char **dtb_buf, size_t *dtb_buf_len)
+{
+	char *buf = NULL;
+	size_t buf_size;
+	int nodeoffset;
+	u64 value;
+	int ret;
+
+	/* duplicate dt blob */
+	buf_size = fdt_totalsize(initial_boot_params);
+
+	if (initrd_load_addr) {
+		/* can be redundant, but trimmed at the end */
+		buf_size += fdt_prop_len("linux,initrd-start", sizeof(u64));
+		buf_size += fdt_prop_len("linux,initrd-end", sizeof(u64));
+	}
+
+	if (cmdline)
+		/* can be redundant, but trimmed at the end */
+		buf_size += fdt_prop_len("bootargs", cmdline_len + 1);
+
+	buf = vmalloc(buf_size);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	ret = fdt_open_into(initial_boot_params, buf, buf_size);
+	if (ret) {
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	nodeoffset = fdt_path_offset(buf, "/chosen");
+	if (nodeoffset < 0) {
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	/* add bootargs */
+	if (cmdline) {
+		ret = fdt_setprop_string(buf, nodeoffset, "bootargs", cmdline);
+		if (ret) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+	} else {
+		ret = fdt_delprop(buf, nodeoffset, "bootargs");
+		if (ret && (ret != -FDT_ERR_NOTFOUND)) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+	}
+
+	/* add initrd-* */
+	if (initrd_load_addr) {
+		value = cpu_to_fdt64(initrd_load_addr);
+		ret = fdt_setprop_u64(buf, nodeoffset, "linux,initrd-start",
+							value);
+		if (ret) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+
+		value = cpu_to_fdt64(initrd_load_addr + initrd_len);
+		ret = fdt_setprop_u64(buf, nodeoffset, "linux,initrd-end",
+							value);
+		if (ret) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+	} else {
+		ret = fdt_delprop(buf, nodeoffset, "linux,initrd-start");
+		if (ret && (ret != -FDT_ERR_NOTFOUND)) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+
+		ret = fdt_delprop(buf, nodeoffset, "linux,initrd-end");
+		if (ret && (ret != -FDT_ERR_NOTFOUND)) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+	}
+
+	/* trim a buffer */
+	fdt_pack(buf);
+	*dtb_buf = buf;
+	*dtb_buf_len = fdt_totalsize(buf);
+
+	return 0;
+
+out_err:
+	vfree(buf);
+	return ret;
+}
+
+int load_other_segments(struct kimage *image,
+			unsigned long kernel_load_addr,
+			unsigned long kernel_size,
+			char *initrd, unsigned long initrd_len,
+			char *cmdline, unsigned long cmdline_len)
+{
+	struct kexec_buf kbuf;
+	unsigned long initrd_load_addr = 0;
+	char *dtb = NULL;
+	unsigned long dtb_len = 0;
+	int ret = 0;
+
+	kbuf.image = image;
+	/* not allocate anything below the kernel */
+	kbuf.buf_min = kernel_load_addr + kernel_size;
+
+	/* load initrd */
+	if (initrd) {
+		kbuf.buffer = initrd;
+		kbuf.bufsz = initrd_len;
+		kbuf.memsz = initrd_len;
+		kbuf.buf_align = 0;
+		/* within 1GB-aligned window of up to 32GB in size */
+		kbuf.buf_max = round_down(kernel_load_addr, SZ_1G)
+						+ (unsigned long)SZ_1G * 32;
+		kbuf.top_down = false;
+
+		ret = kexec_add_buffer(&kbuf);
+		if (ret)
+			goto out_err;
+		initrd_load_addr = kbuf.mem;
+
+		pr_debug("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+				initrd_load_addr, initrd_len, initrd_len);
+	}
+
+	/* load dtb blob */
+	ret = setup_dtb(image, initrd_load_addr, initrd_len,
+				cmdline, cmdline_len, &dtb, &dtb_len);
+	if (ret) {
+		pr_err("Preparing for new dtb failed\n");
+		goto out_err;
+	}
+
+	kbuf.buffer = dtb;
+	kbuf.bufsz = dtb_len;
+	kbuf.memsz = dtb_len;
+	/* not across 2MB boundary */
+	kbuf.buf_align = SZ_2M;
+	kbuf.buf_max = ULONG_MAX;
+	kbuf.top_down = true;
+
+	ret = kexec_add_buffer(&kbuf);
+	if (ret)
+		goto out_err;
+	image->arch.dtb_mem = kbuf.mem;
+	image->arch.dtb_buf = dtb;
+
+	pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+			kbuf.mem, dtb_len, dtb_len);
+
+	return 0;
+
+out_err:
+	vfree(dtb);
+	return ret;
+}