| Message ID | 20180711074203.3019-10-takahiro.akashi@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 --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; +}
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(+)