2017-09-15 10:57:46

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 00/10] arm64: kexec: add kexec_file_load() support

This is the third round of implementing kexec_file_load() support
on arm64.[1]
Most of the code is based on kexec-tools (along with some kernel code
from x86, which also came from kexec-tools).


This patch series enables us to
* load the kernel, Image, with kexec_file_load system call, and
* optionally verify its signature at load time for trusted boot.

To load the kernel via kexec_file_load system call, a small change
is also needed to kexec-tools. See [2]. This enables '-s' option.

As we discussed a long time ago, users may not be allowed to specify
device-tree file of the 2nd kernel explicitly with kexec-tools, hence
re-using the blob of the first kernel.

Regarding a signing method, we conform with x86 (or rather Microsoft?)
style of signing since the binary can also be seen as in PE format
(assuming that CONFIG_EFI is enabled).

Powerpc is also going to support extended-file-attribute-based
verification[3] with vmlinux, but arm64 doesn't for now partly
because we don't have TPM-based IMA at this moment.

Accordingly, we can use the existing command, sbsign, to sign the kernel.

$ sbsign --key ${KEY} --cert ${CERT} Image

Please note that it is totally up to the system what key/certificate is
used for signing, but one of easy ways to *try* this feature is to turn on
CONFIG_MODULE_SIG so that we can reuse certs/signing_key.pem as a signing
key, KEY and CERT above, for kernel.
(This also enables CONFIG_CRYPTO_SHA1 by default.)


Some concerns(or future works):
* Even if the kernel is configured with CONFIG_RANDOMIZE_BASE, the 2nd
kernel won't be placed at a randomized address. We will have to
add some boot code similar to efi-stub to implement the feature.
* While big-endian kernel can support kernel signing, I'm not sure that
Image can be recognized as in PE format because x86 standard only
defines little-endian-based format.
* IMA(and extended file attribute)-based kexec
* vmlinux support

[1] http://git.linaro.org/people/takahiro.akashi/linux-aarch64.git
branch:arm64/kexec_file
[2] http://git.linaro.org/people/takahiro.akashi/kexec-tools.git
branch:arm64/kexec_file
[3] http://lkml.iu.edu//hypermail/linux/kernel/1707.0/03669.html


Changes in v3 (Sep 15, 2017)
* fix kbuild test error
* factor out arch_kexec_kernel_*() & arch_kimage_file_post_load_cleanup()
* remove CONFIG_CRASH_CORE guard from kexec_file.c
* add vmapped kernel region to vmcore for gdb backtracing
(see prepare_elf64_headers())
* merge asm/kexec_file.h into asm/kexec.h
* and some cleanups

Changes in v2 (Sep 8, 2017)
* move core-header-related functions from crash_core.c to kexec_file.c
* drop hash-check code from purgatory
* modify purgatory asm to remove arch_kexec_apply_relocations_add()
* drop older kernel support
* drop vmlinux support (at least, for this series)

Patch #1 to #5 are all preparatory patches on generic side.
Patch #6 is purgatory code.
Patch #7 to #9 are common for enabling kexec_file_load.
Patch #10 is for 'Image' support.

AKASHI Takahiro (10):
include: pe.h: remove message[] from mz header definition
resource: add walk_system_ram_res_rev()
kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc
kexec_file: factor out crashdump elf header function from x86
asm-generic: add kexec_file_load system call to unistd.h
arm64: kexec_file: create purgatory
arm64: kexec_file: load initrd, device-tree and purgatory segments
arm64: kexec_file: set up for crash dump adding elf core header
arm64: enable KEXEC_FILE config
arm64: kexec_file: add Image format support

arch/arm64/Kconfig | 29 +++
arch/arm64/Makefile | 1 +
arch/arm64/include/asm/kexec.h | 93 +++++++
arch/arm64/kernel/Makefile | 4 +-
arch/arm64/kernel/kexec_image.c | 105 ++++++++
arch/arm64/kernel/machine_kexec_file.c | 365 ++++++++++++++++++++++++++++
arch/arm64/purgatory/Makefile | 24 ++
arch/arm64/purgatory/entry.S | 55 +++++
arch/powerpc/include/asm/kexec.h | 4 +
arch/powerpc/kernel/machine_kexec_file_64.c | 36 +--
arch/x86/kernel/crash.c | 324 ------------------------
arch/x86/kernel/machine_kexec_64.c | 59 +----
include/linux/ioport.h | 3 +
include/linux/kexec.h | 43 +++-
include/linux/pe.h | 2 +-
include/uapi/asm-generic/unistd.h | 4 +-
kernel/kexec_file.c | 360 ++++++++++++++++++++++++++-
kernel/kexec_internal.h | 20 ++
kernel/resource.c | 59 +++++
19 files changed, 1156 insertions(+), 434 deletions(-)
create mode 100644 arch/arm64/kernel/kexec_image.c
create mode 100644 arch/arm64/kernel/machine_kexec_file.c
create mode 100644 arch/arm64/purgatory/Makefile
create mode 100644 arch/arm64/purgatory/entry.S

--
2.14.1


2017-09-15 10:57:55

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 01/10] include: pe.h: remove message[] from mz header definition

message[] field won't be part of the definition of mz header.

This change is crucial for enabling kexec_file_load on arm64 because
arm64's "Image" binary, as in PE format, doesn't have any data for it and
accordingly the following check in pefile_parse_binary() will fail:

chkaddr(cursor, mz->peaddr, sizeof(*pe));

Signed-off-by: AKASHI Takahiro <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Cc: David Howells <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David S. Miller <[email protected]>
---
include/linux/pe.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/pe.h b/include/linux/pe.h
index 143ce75be5f0..3482b18a48b5 100644
--- a/include/linux/pe.h
+++ b/include/linux/pe.h
@@ -166,7 +166,7 @@ struct mz_hdr {
uint16_t oem_info; /* oem specific */
uint16_t reserved1[10]; /* reserved */
uint32_t peaddr; /* address of pe header */
- char message[64]; /* message to print */
+ char message[]; /* message to print */
};

struct mz_reloc {
--
2.14.1

2017-09-15 10:58:03

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 02/10] resource: add walk_system_ram_res_rev()

This function, being a variant of walk_system_ram_res() introduced in
commit 8c86e70acead ("resource: provide new functions to walk through
resources"), walks through a list of all the resources of System RAM
in reversed order, i.e., from higher to lower.

It will be used in kexec_file implementation on arm64.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
include/linux/ioport.h | 3 +++
kernel/resource.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 6230064d7f95..9a212266299f 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -271,6 +271,9 @@ extern int
walk_system_ram_res(u64 start, u64 end, void *arg,
int (*func)(u64, u64, void *));
extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(u64, u64, void *));
+extern int
walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
void *arg, int (*func)(u64, u64, void *));

diff --git a/kernel/resource.c b/kernel/resource.c
index 9b5f04404152..572f2f91ce9c 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -23,6 +23,8 @@
#include <linux/pfn.h>
#include <linux/mm.h>
#include <linux/resource_ext.h>
+#include <linux/string.h>
+#include <linux/vmalloc.h>
#include <asm/io.h>


@@ -469,6 +471,63 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
return ret;
}

+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(u64, u64, void *))
+{
+ struct resource res, *rams;
+ u64 orig_end;
+ int count, i;
+ int ret = -1;
+
+ count = 16; /* initial */
+
+ /* create a list */
+ rams = vmalloc(sizeof(struct resource) * count);
+ if (!rams)
+ return ret;
+
+ res.start = start;
+ res.end = end;
+ res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+ orig_end = res.end;
+ i = 0;
+ while ((res.start < res.end) &&
+ (!find_next_iomem_res(&res, IORES_DESC_NONE, true))) {
+ if (i >= count) {
+ /* re-alloc */
+ struct resource *rams_new;
+ int count_new;
+
+ count_new = count + 16;
+ rams_new = vmalloc(sizeof(struct resource) * count_new);
+ if (!rams_new)
+ goto out;
+
+ memcpy(rams_new, rams, count);
+ vfree(rams);
+ rams = rams_new;
+ count = count_new;
+ }
+
+ rams[i].start = res.start;
+ rams[i++].end = res.end;
+
+ res.start = res.end + 1;
+ res.end = orig_end;
+ }
+
+ /* go reverse */
+ for (i--; i >= 0; i--) {
+ ret = (*func)(rams[i].start, rams[i].end, arg);
+ if (ret)
+ break;
+ }
+
+out:
+ vfree(rams);
+ return ret;
+}
+
#if !defined(CONFIG_ARCH_HAS_WALK_MEMORY)

/*
--
2.14.1

2017-09-15 10:58:09

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

arch_kexec_kernel_*() and arch_kimage_file_post_load_cleanup can now be
duplicated among some architectures, so let's factor them out.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Dave Young <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Thiago Jung Bauermann <[email protected]>
---
arch/powerpc/include/asm/kexec.h | 4 ++
arch/powerpc/kernel/machine_kexec_file_64.c | 36 ++----------------
arch/x86/kernel/machine_kexec_64.c | 59 +----------------------------
include/linux/kexec.h | 26 ++++++++++---
kernel/kexec_file.c | 52 +++++++++++++++++++------
5 files changed, 70 insertions(+), 107 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 25668bc8cb2a..50810d24e38f 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -94,6 +94,10 @@ static inline bool kdump_in_progress(void)
#ifdef CONFIG_KEXEC_FILE
extern struct kexec_file_ops kexec_elf64_ops;

+#define arch_kexec_kernel_image_probe arch_kexec_kernel_image_probe
+int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
+ unsigned long buf_len);
+
#ifdef CONFIG_IMA_KEXEC
#define ARCH_HAS_KIMAGE_ARCH

diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c b/arch/powerpc/kernel/machine_kexec_file_64.c
index 992c0d258e5d..5b7c4a3fbb50 100644
--- a/arch/powerpc/kernel/machine_kexec_file_64.c
+++ b/arch/powerpc/kernel/machine_kexec_file_64.c
@@ -31,8 +31,9 @@

#define SLAVE_CODE_SIZE 256

-static struct kexec_file_ops *kexec_file_loaders[] = {
+struct kexec_file_ops *kexec_file_loaders[] = {
&kexec_elf64_ops,
+ NULL
};

int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
@@ -45,38 +46,7 @@ int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
if (image->type == KEXEC_TYPE_CRASH)
return -ENOTSUPP;

- for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
- fops = kexec_file_loaders[i];
- if (!fops || !fops->probe)
- continue;
-
- ret = fops->probe(buf, buf_len);
- if (!ret) {
- image->fops = fops;
- return ret;
- }
- }
-
- return ret;
-}
-
-void *arch_kexec_kernel_image_load(struct kimage *image)
-{
- if (!image->fops || !image->fops->load)
- return ERR_PTR(-ENOEXEC);
-
- return image->fops->load(image, image->kernel_buf,
- image->kernel_buf_len, image->initrd_buf,
- image->initrd_buf_len, image->cmdline_buf,
- image->cmdline_buf_len);
-}
-
-int arch_kimage_file_post_load_cleanup(struct kimage *image)
-{
- if (!image->fops || !image->fops->cleanup)
- return 0;
-
- return image->fops->cleanup(image->image_loader_data);
+ return kexec_kernel_image_probe(image, buf, buf_len);
}

/**
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index cb0a30473c23..f4df8315d001 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -30,8 +30,9 @@
#include <asm/set_memory.h>

#ifdef CONFIG_KEXEC_FILE
-static struct kexec_file_ops *kexec_file_loaders[] = {
+struct kexec_file_ops *kexec_file_loaders[] = {
&kexec_bzImage64_ops,
+ NULL
};
#endif

@@ -361,62 +362,6 @@ void arch_crash_save_vmcoreinfo(void)
/* arch-dependent functionality related to kexec file-based syscall */

#ifdef CONFIG_KEXEC_FILE
-int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
- unsigned long buf_len)
-{
- int i, ret = -ENOEXEC;
- struct kexec_file_ops *fops;
-
- for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
- fops = kexec_file_loaders[i];
- if (!fops || !fops->probe)
- continue;
-
- ret = fops->probe(buf, buf_len);
- if (!ret) {
- image->fops = fops;
- return ret;
- }
- }
-
- return ret;
-}
-
-void *arch_kexec_kernel_image_load(struct kimage *image)
-{
- vfree(image->arch.elf_headers);
- image->arch.elf_headers = NULL;
-
- if (!image->fops || !image->fops->load)
- return ERR_PTR(-ENOEXEC);
-
- return image->fops->load(image, image->kernel_buf,
- image->kernel_buf_len, image->initrd_buf,
- image->initrd_buf_len, image->cmdline_buf,
- image->cmdline_buf_len);
-}
-
-int arch_kimage_file_post_load_cleanup(struct kimage *image)
-{
- if (!image->fops || !image->fops->cleanup)
- return 0;
-
- return image->fops->cleanup(image->image_loader_data);
-}
-
-#ifdef CONFIG_KEXEC_VERIFY_SIG
-int arch_kexec_kernel_verify_sig(struct kimage *image, void *kernel,
- unsigned long kernel_len)
-{
- if (!image->fops || !image->fops->verify_sig) {
- pr_debug("kernel loader does not support signature verification.");
- return -EKEYREJECTED;
- }
-
- return image->fops->verify_sig(kernel, kernel_len);
-}
-#endif
-
/*
* Apply purgatory relocations.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index dd056fab9e35..4a2b24d94e04 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -134,6 +134,26 @@ struct kexec_file_ops {
#endif
};

+int kexec_kernel_image_probe(struct kimage *image, void *buf,
+ unsigned long buf_len);
+void *kexec_kernel_image_load(struct kimage *image);
+int kexec_kernel_post_load_cleanup(struct kimage *image);
+int kexec_kernel_verify_sig(struct kimage *image, void *buf,
+ unsigned long buf_len);
+
+#ifndef arch_kexec_kernel_image_probe
+#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
+#endif
+#ifndef arch_kexec_kernel_image_load
+#define arch_kexec_kernel_image_load kexec_kernel_image_load
+#endif
+#ifndef arch_kimage_file_post_load_cleanup
+#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
+#endif
+#ifndef arch_kexec_kernel_verify_sig
+#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
+#endif
+
/**
* struct kexec_buf - parameters for finding a place for a buffer in memory
* @image: kexec image in which memory to search.
@@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
size_t crash_get_memory_size(void);
void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);

-int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
- unsigned long buf_len);
-void * __weak arch_kexec_kernel_image_load(struct kimage *image);
-int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
-int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
- unsigned long buf_len);
int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr,
Elf_Shdr *sechdrs, unsigned int relsec);
int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 9f48f4412297..6203b54e04c5 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -26,30 +26,60 @@
#include <linux/vmalloc.h>
#include "kexec_internal.h"

+__weak struct kexec_file_ops *kexec_file_loaders[] = {NULL};
+
static int kexec_calculate_store_digests(struct kimage *image);

-/* Architectures can provide this probe function */
-int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
- unsigned long buf_len)
+int kexec_kernel_image_probe(struct kimage *image, void *buf,
+ unsigned long buf_len)
{
- return -ENOEXEC;
+ int i, ret = -ENOEXEC;
+ struct kexec_file_ops *fops;
+
+ for (i = 0; ; i++) {
+ fops = kexec_file_loaders[i];
+ if (!fops || !fops->probe)
+ break;
+
+ ret = fops->probe(buf, buf_len);
+ if (!ret) {
+ image->fops = fops;
+ return ret;
+ }
+ }
+
+ return ret;
}

-void * __weak arch_kexec_kernel_image_load(struct kimage *image)
+void *kexec_kernel_image_load(struct kimage *image)
{
- return ERR_PTR(-ENOEXEC);
+ if (!image->fops || !image->fops->load)
+ return ERR_PTR(-ENOEXEC);
+
+ return image->fops->load(image, image->kernel_buf,
+ image->kernel_buf_len, image->initrd_buf,
+ image->initrd_buf_len, image->cmdline_buf,
+ image->cmdline_buf_len);
}

-int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
+int kexec_kernel_post_load_cleanup(struct kimage *image)
{
- return -EINVAL;
+ if (!image->fops || !image->fops->cleanup)
+ return 0;
+
+ return image->fops->cleanup(image->image_loader_data);
}

#ifdef CONFIG_KEXEC_VERIFY_SIG
-int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
- unsigned long buf_len)
+int kexec_kernel_verify_sig(struct kimage *image, void *buf,
+ unsigned long buf_len)
{
- return -EKEYREJECTED;
+ if (!image->fops || !image->fops->verify_sig) {
+ pr_debug("kernel loader does not support signature verification.\n");
+ return -EKEYREJECTED;
+ }
+
+ return image->fops->verify_sig(buf, buf_len);
}
#endif

--
2.14.1

2017-09-15 10:58:14

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 04/10] kexec_file: factor out crashdump elf header function from x86

prepare_elf_headers() can also be useful for other architectures,
including arm64. So let it factored out.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Dave Young <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: Baoquan He <[email protected]>
---
arch/x86/kernel/crash.c | 324 ------------------------------------------------
include/linux/kexec.h | 17 +++
kernel/kexec_file.c | 308 +++++++++++++++++++++++++++++++++++++++++++++
kernel/kexec_internal.h | 20 +++
4 files changed, 345 insertions(+), 324 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 44404e2307bb..3c6b880f6dbf 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -21,7 +21,6 @@
#include <linux/elf.h>
#include <linux/elfcore.h>
#include <linux/export.h>
-#include <linux/slab.h>
#include <linux/vmalloc.h>

#include <asm/processor.h>
@@ -41,34 +40,6 @@
/* Alignment required for elf header segment */
#define ELF_CORE_HEADER_ALIGN 4096

-/* This primarily represents number of split ranges due to exclusion */
-#define CRASH_MAX_RANGES 16
-
-struct crash_mem_range {
- u64 start, end;
-};
-
-struct crash_mem {
- unsigned int nr_ranges;
- struct crash_mem_range ranges[CRASH_MAX_RANGES];
-};
-
-/* Misc data about ram ranges needed to prepare elf headers */
-struct crash_elf_data {
- struct kimage *image;
- /*
- * Total number of ram ranges we have after various adjustments for
- * crash reserved region, etc.
- */
- unsigned int max_nr_ranges;
-
- /* Pointer to elf header */
- void *ehdr;
- /* Pointer to next phdr */
- void *bufp;
- struct crash_mem mem;
-};
-
/* Used while preparing memory map entries for second kernel */
struct crash_memmap_data {
struct boot_params *params;
@@ -209,301 +180,6 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
}

#ifdef CONFIG_KEXEC_FILE
-static int get_nr_ram_ranges_callback(u64 start, u64 end, void *arg)
-{
- unsigned int *nr_ranges = arg;
-
- (*nr_ranges)++;
- return 0;
-}
-
-
-/* Gather all the required information to prepare elf headers for ram regions */
-static void fill_up_crash_elf_data(struct crash_elf_data *ced,
- struct kimage *image)
-{
- unsigned int nr_ranges = 0;
-
- ced->image = image;
-
- walk_system_ram_res(0, -1, &nr_ranges,
- get_nr_ram_ranges_callback);
-
- ced->max_nr_ranges = nr_ranges;
-
- /* Exclusion of crash region could split memory ranges */
- ced->max_nr_ranges++;
-
- /* If crashk_low_res is not 0, another range split possible */
- if (crashk_low_res.end)
- ced->max_nr_ranges++;
-}
-
-static int exclude_mem_range(struct crash_mem *mem,
- unsigned long long mstart, unsigned long long mend)
-{
- int i, j;
- unsigned long long start, end;
- struct crash_mem_range temp_range = {0, 0};
-
- for (i = 0; i < mem->nr_ranges; i++) {
- start = mem->ranges[i].start;
- end = mem->ranges[i].end;
-
- if (mstart > end || mend < start)
- continue;
-
- /* Truncate any area outside of range */
- if (mstart < start)
- mstart = start;
- if (mend > end)
- mend = end;
-
- /* Found completely overlapping range */
- if (mstart == start && mend == end) {
- mem->ranges[i].start = 0;
- mem->ranges[i].end = 0;
- if (i < mem->nr_ranges - 1) {
- /* Shift rest of the ranges to left */
- for (j = i; j < mem->nr_ranges - 1; j++) {
- mem->ranges[j].start =
- mem->ranges[j+1].start;
- mem->ranges[j].end =
- mem->ranges[j+1].end;
- }
- }
- mem->nr_ranges--;
- return 0;
- }
-
- if (mstart > start && mend < end) {
- /* Split original range */
- mem->ranges[i].end = mstart - 1;
- temp_range.start = mend + 1;
- temp_range.end = end;
- } else if (mstart != start)
- mem->ranges[i].end = mstart - 1;
- else
- mem->ranges[i].start = mend + 1;
- break;
- }
-
- /* If a split happend, add the split to array */
- if (!temp_range.end)
- return 0;
-
- /* Split happened */
- if (i == CRASH_MAX_RANGES - 1) {
- pr_err("Too many crash ranges after split\n");
- return -ENOMEM;
- }
-
- /* Location where new range should go */
- j = i + 1;
- if (j < mem->nr_ranges) {
- /* Move over all ranges one slot towards the end */
- for (i = mem->nr_ranges - 1; i >= j; i--)
- mem->ranges[i + 1] = mem->ranges[i];
- }
-
- mem->ranges[j].start = temp_range.start;
- mem->ranges[j].end = temp_range.end;
- mem->nr_ranges++;
- return 0;
-}
-
-/*
- * Look for any unwanted ranges between mstart, mend and remove them. This
- * might lead to split and split ranges are put in ced->mem.ranges[] array
- */
-static int elf_header_exclude_ranges(struct crash_elf_data *ced,
- unsigned long long mstart, unsigned long long mend)
-{
- struct crash_mem *cmem = &ced->mem;
- int ret = 0;
-
- memset(cmem->ranges, 0, sizeof(cmem->ranges));
-
- cmem->ranges[0].start = mstart;
- cmem->ranges[0].end = mend;
- cmem->nr_ranges = 1;
-
- /* Exclude crashkernel region */
- ret = exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
- if (ret)
- return ret;
-
- if (crashk_low_res.end) {
- ret = exclude_mem_range(cmem, crashk_low_res.start, crashk_low_res.end);
- if (ret)
- return ret;
- }
-
- return ret;
-}
-
-static int prepare_elf64_ram_headers_callback(u64 start, u64 end, void *arg)
-{
- struct crash_elf_data *ced = arg;
- Elf64_Ehdr *ehdr;
- Elf64_Phdr *phdr;
- unsigned long mstart, mend;
- struct kimage *image = ced->image;
- struct crash_mem *cmem;
- int ret, i;
-
- ehdr = ced->ehdr;
-
- /* Exclude unwanted mem ranges */
- ret = elf_header_exclude_ranges(ced, start, end);
- if (ret)
- return ret;
-
- /* Go through all the ranges in ced->mem.ranges[] and prepare phdr */
- cmem = &ced->mem;
-
- for (i = 0; i < cmem->nr_ranges; i++) {
- mstart = cmem->ranges[i].start;
- mend = cmem->ranges[i].end;
-
- phdr = ced->bufp;
- ced->bufp += sizeof(Elf64_Phdr);
-
- phdr->p_type = PT_LOAD;
- phdr->p_flags = PF_R|PF_W|PF_X;
- phdr->p_offset = mstart;
-
- /*
- * If a range matches backup region, adjust offset to backup
- * segment.
- */
- if (mstart == image->arch.backup_src_start &&
- (mend - mstart + 1) == image->arch.backup_src_sz)
- phdr->p_offset = image->arch.backup_load_addr;
-
- phdr->p_paddr = mstart;
- phdr->p_vaddr = (unsigned long long) __va(mstart);
- phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
- phdr->p_align = 0;
- ehdr->e_phnum++;
- pr_debug("Crash PT_LOAD elf header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
- phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
- ehdr->e_phnum, phdr->p_offset);
- }
-
- return ret;
-}
-
-static int prepare_elf64_headers(struct crash_elf_data *ced,
- void **addr, unsigned long *sz)
-{
- Elf64_Ehdr *ehdr;
- Elf64_Phdr *phdr;
- unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
- unsigned char *buf, *bufp;
- unsigned int cpu;
- unsigned long long notes_addr;
- int ret;
-
- /* extra phdr for vmcoreinfo elf note */
- nr_phdr = nr_cpus + 1;
- nr_phdr += ced->max_nr_ranges;
-
- /*
- * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
- * area on x86_64 (ffffffff80000000 - ffffffffa0000000).
- * I think this is required by tools like gdb. So same physical
- * memory will be mapped in two elf headers. One will contain kernel
- * text virtual addresses and other will have __va(physical) addresses.
- */
-
- nr_phdr++;
- elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
- elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
-
- buf = vzalloc(elf_sz);
- if (!buf)
- return -ENOMEM;
-
- bufp = buf;
- ehdr = (Elf64_Ehdr *)bufp;
- bufp += sizeof(Elf64_Ehdr);
- memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
- ehdr->e_ident[EI_CLASS] = ELFCLASS64;
- ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
- ehdr->e_ident[EI_VERSION] = EV_CURRENT;
- ehdr->e_ident[EI_OSABI] = ELF_OSABI;
- memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
- ehdr->e_type = ET_CORE;
- ehdr->e_machine = ELF_ARCH;
- ehdr->e_version = EV_CURRENT;
- ehdr->e_phoff = sizeof(Elf64_Ehdr);
- ehdr->e_ehsize = sizeof(Elf64_Ehdr);
- ehdr->e_phentsize = sizeof(Elf64_Phdr);
-
- /* Prepare one phdr of type PT_NOTE for each present cpu */
- for_each_present_cpu(cpu) {
- phdr = (Elf64_Phdr *)bufp;
- bufp += sizeof(Elf64_Phdr);
- phdr->p_type = PT_NOTE;
- notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
- phdr->p_offset = phdr->p_paddr = notes_addr;
- phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
- (ehdr->e_phnum)++;
- }
-
- /* Prepare one PT_NOTE header for vmcoreinfo */
- phdr = (Elf64_Phdr *)bufp;
- bufp += sizeof(Elf64_Phdr);
- phdr->p_type = PT_NOTE;
- phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
- phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
- (ehdr->e_phnum)++;
-
-#ifdef CONFIG_X86_64
- /* Prepare PT_LOAD type program header for kernel text region */
- phdr = (Elf64_Phdr *)bufp;
- bufp += sizeof(Elf64_Phdr);
- phdr->p_type = PT_LOAD;
- phdr->p_flags = PF_R|PF_W|PF_X;
- phdr->p_vaddr = (Elf64_Addr)_text;
- phdr->p_filesz = phdr->p_memsz = _end - _text;
- phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
- (ehdr->e_phnum)++;
-#endif
-
- /* Prepare PT_LOAD headers for system ram chunks. */
- ced->ehdr = ehdr;
- ced->bufp = bufp;
- ret = walk_system_ram_res(0, -1, ced,
- prepare_elf64_ram_headers_callback);
- if (ret < 0)
- return ret;
-
- *addr = buf;
- *sz = elf_sz;
- return 0;
-}
-
-/* Prepare elf headers. Return addr and size */
-static int prepare_elf_headers(struct kimage *image, void **addr,
- unsigned long *sz)
-{
- struct crash_elf_data *ced;
- int ret;
-
- ced = kzalloc(sizeof(*ced), GFP_KERNEL);
- if (!ced)
- return -ENOMEM;
-
- fill_up_crash_elf_data(ced, image);
-
- /* By default prepare 64bit headers */
- ret = prepare_elf64_headers(ced, addr, sz);
- kfree(ced);
- return ret;
-}
-
static int add_e820_entry(struct boot_params *params, struct e820_entry *entry)
{
unsigned int nr_e820_entries;
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 4a2b24d94e04..7547a3c1f94e 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -182,6 +182,23 @@ int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
int (*func)(u64, u64, void *));
extern int kexec_add_buffer(struct kexec_buf *kbuf);
int kexec_locate_mem_hole(struct kexec_buf *kbuf);
+extern int prepare_elf_headers(struct kimage *image, void **addr,
+ unsigned long *sz);
+
+/* This primarily represents number of split ranges due to exclusion */
+#define CRASH_MAX_RANGES 16
+
+struct crash_mem_range {
+ u64 start, end;
+};
+
+struct crash_mem {
+ unsigned int nr_ranges;
+ struct crash_mem_range ranges[CRASH_MAX_RANGES];
+};
+
+extern int exclude_mem_range(struct crash_mem *mem,
+ unsigned long long mstart, unsigned long long mend);
#endif /* CONFIG_KEXEC_FILE */

struct kimage {
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 6203b54e04c5..6a0e31e175af 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -22,6 +22,11 @@
#include <linux/ima.h>
#include <crypto/hash.h>
#include <crypto/sha.h>
+#include <linux/elf.h>
+#include <linux/elfcore.h>
+#include <linux/kernel.h>
+#include <linux/kexec.h>
+#include <linux/slab.h>
#include <linux/syscalls.h>
#include <linux/vmalloc.h>
#include "kexec_internal.h"
@@ -1051,3 +1056,306 @@ int kexec_purgatory_get_set_symbol(struct kimage *image, const char *name,

return 0;
}
+
+static int get_nr_ram_ranges_callback(u64 start, u64 end, void *arg)
+{
+ unsigned int *nr_ranges = arg;
+
+ (*nr_ranges)++;
+ return 0;
+}
+
+
+/* Gather all the required information to prepare elf headers for ram regions */
+static void fill_up_crash_elf_data(struct crash_elf_data *ced,
+ struct kimage *image)
+{
+ unsigned int nr_ranges = 0;
+
+ ced->image = image;
+
+ walk_system_ram_res(0, -1, &nr_ranges,
+ get_nr_ram_ranges_callback);
+
+ ced->max_nr_ranges = nr_ranges;
+
+ /* Exclusion of crash region could split memory ranges */
+ ced->max_nr_ranges++;
+
+#ifdef CONFIG_X86_64
+ /* If crashk_low_res is not 0, another range split possible */
+ if (crashk_low_res.end)
+ ced->max_nr_ranges++;
+#endif
+}
+
+int exclude_mem_range(struct crash_mem *mem,
+ unsigned long long mstart, unsigned long long mend)
+{
+ int i, j;
+ unsigned long long start, end;
+ struct crash_mem_range temp_range = {0, 0};
+
+ for (i = 0; i < mem->nr_ranges; i++) {
+ start = mem->ranges[i].start;
+ end = mem->ranges[i].end;
+
+ if (mstart > end || mend < start)
+ continue;
+
+ /* Truncate any area outside of range */
+ if (mstart < start)
+ mstart = start;
+ if (mend > end)
+ mend = end;
+
+ /* Found completely overlapping range */
+ if (mstart == start && mend == end) {
+ mem->ranges[i].start = 0;
+ mem->ranges[i].end = 0;
+ if (i < mem->nr_ranges - 1) {
+ /* Shift rest of the ranges to left */
+ for (j = i; j < mem->nr_ranges - 1; j++) {
+ mem->ranges[j].start =
+ mem->ranges[j+1].start;
+ mem->ranges[j].end =
+ mem->ranges[j+1].end;
+ }
+ }
+ mem->nr_ranges--;
+ return 0;
+ }
+
+ if (mstart > start && mend < end) {
+ /* Split original range */
+ mem->ranges[i].end = mstart - 1;
+ temp_range.start = mend + 1;
+ temp_range.end = end;
+ } else if (mstart != start)
+ mem->ranges[i].end = mstart - 1;
+ else
+ mem->ranges[i].start = mend + 1;
+ break;
+ }
+
+ /* If a split happened, add the split to array */
+ if (!temp_range.end)
+ return 0;
+
+ /* Split happened */
+ if (i == CRASH_MAX_RANGES - 1) {
+ pr_err("Too many crash ranges after split\n");
+ return -ENOMEM;
+ }
+
+ /* Location where new range should go */
+ j = i + 1;
+ if (j < mem->nr_ranges) {
+ /* Move over all ranges one slot towards the end */
+ for (i = mem->nr_ranges - 1; i >= j; i--)
+ mem->ranges[i + 1] = mem->ranges[i];
+ }
+
+ mem->ranges[j].start = temp_range.start;
+ mem->ranges[j].end = temp_range.end;
+ mem->nr_ranges++;
+ return 0;
+}
+
+/*
+ * Look for any unwanted ranges between mstart, mend and remove them. This
+ * might lead to split and split ranges are put in ced->mem.ranges[] array
+ */
+static int elf_header_exclude_ranges(struct crash_elf_data *ced,
+ unsigned long long mstart, unsigned long long mend)
+{
+ struct crash_mem *cmem = &ced->mem;
+ int ret = 0;
+
+ memset(cmem->ranges, 0, sizeof(cmem->ranges));
+
+ cmem->ranges[0].start = mstart;
+ cmem->ranges[0].end = mend;
+ cmem->nr_ranges = 1;
+
+ /* Exclude crashkernel region */
+ ret = exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
+ if (ret)
+ return ret;
+
+#ifdef CONFIG_X86_64
+ if (crashk_low_res.end) {
+ ret = exclude_mem_range(cmem, crashk_low_res.start,
+ crashk_low_res.end);
+ if (ret)
+ return ret;
+ }
+#endif
+
+ return ret;
+}
+
+static int prepare_elf64_ram_headers_callback(u64 start, u64 end, void *arg)
+{
+ struct crash_elf_data *ced = arg;
+ Elf64_Ehdr *ehdr;
+ Elf64_Phdr *phdr;
+ unsigned long mstart, mend;
+#ifdef CONFIG_X86_64
+ struct kimage *image = ced->image;
+#endif
+ struct crash_mem *cmem;
+ int ret, i;
+
+ ehdr = ced->ehdr;
+
+ /* Exclude unwanted mem ranges */
+ ret = elf_header_exclude_ranges(ced, start, end);
+ if (ret)
+ return ret;
+
+ /* Go through all the ranges in ced->mem.ranges[] and prepare phdr */
+ cmem = &ced->mem;
+
+ for (i = 0; i < cmem->nr_ranges; i++) {
+ mstart = cmem->ranges[i].start;
+ mend = cmem->ranges[i].end;
+
+ phdr = ced->bufp;
+ ced->bufp += sizeof(Elf64_Phdr);
+
+ phdr->p_type = PT_LOAD;
+ phdr->p_flags = PF_R|PF_W|PF_X;
+ phdr->p_offset = mstart;
+
+#ifdef CONFIG_X86_64
+ /*
+ * If a range matches backup region, adjust offset to backup
+ * segment.
+ */
+ if (mstart == image->arch.backup_src_start &&
+ (mend - mstart + 1) == image->arch.backup_src_sz)
+ phdr->p_offset = image->arch.backup_load_addr;
+#endif
+
+ phdr->p_paddr = mstart;
+ phdr->p_vaddr = (unsigned long long) __va(mstart);
+ phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
+ phdr->p_align = 0;
+ ehdr->e_phnum++;
+ pr_debug("Crash PT_LOAD elf header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
+ phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
+ ehdr->e_phnum, phdr->p_offset);
+ }
+
+ return ret;
+}
+
+static int prepare_elf64_headers(struct crash_elf_data *ced,
+ void **addr, unsigned long *sz)
+{
+ Elf64_Ehdr *ehdr;
+ Elf64_Phdr *phdr;
+ unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
+ unsigned char *buf, *bufp;
+ unsigned int cpu;
+ unsigned long long notes_addr;
+ int ret;
+
+ /* extra phdr for vmcoreinfo elf note */
+ nr_phdr = nr_cpus + 1;
+ nr_phdr += ced->max_nr_ranges;
+
+ /*
+ * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
+ * area on x86_64 (ffffffff80000000 - ffffffffa0000000).
+ * I think this is required by tools like gdb. So same physical
+ * memory will be mapped in two elf headers. One will contain kernel
+ * text virtual addresses and other will have __va(physical) addresses.
+ */
+
+ nr_phdr++;
+ elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
+ elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
+
+ buf = vzalloc(elf_sz);
+ if (!buf)
+ return -ENOMEM;
+
+ bufp = buf;
+ ehdr = (Elf64_Ehdr *)bufp;
+ bufp += sizeof(Elf64_Ehdr);
+ memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
+ ehdr->e_ident[EI_CLASS] = ELFCLASS64;
+ ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
+ ehdr->e_ident[EI_VERSION] = EV_CURRENT;
+ ehdr->e_ident[EI_OSABI] = ELF_OSABI;
+ memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
+ ehdr->e_type = ET_CORE;
+ ehdr->e_machine = ELF_ARCH;
+ ehdr->e_version = EV_CURRENT;
+ ehdr->e_phoff = sizeof(Elf64_Ehdr);
+ ehdr->e_ehsize = sizeof(Elf64_Ehdr);
+ ehdr->e_phentsize = sizeof(Elf64_Phdr);
+
+ /* Prepare one phdr of type PT_NOTE for each present cpu */
+ for_each_present_cpu(cpu) {
+ phdr = (Elf64_Phdr *)bufp;
+ bufp += sizeof(Elf64_Phdr);
+ phdr->p_type = PT_NOTE;
+ notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
+ phdr->p_offset = phdr->p_paddr = notes_addr;
+ phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
+ (ehdr->e_phnum)++;
+ }
+
+ /* Prepare one PT_NOTE header for vmcoreinfo */
+ phdr = (Elf64_Phdr *)bufp;
+ bufp += sizeof(Elf64_Phdr);
+ phdr->p_type = PT_NOTE;
+ phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
+ phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
+ (ehdr->e_phnum)++;
+
+#ifdef CONFIG_X86_64
+ /* Prepare PT_LOAD type program header for kernel text region */
+ phdr = (Elf64_Phdr *)bufp;
+ bufp += sizeof(Elf64_Phdr);
+ phdr->p_type = PT_LOAD;
+ phdr->p_flags = PF_R|PF_W|PF_X;
+ phdr->p_vaddr = (Elf64_Addr)_text;
+ phdr->p_filesz = phdr->p_memsz = _end - _text;
+ phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
+ (ehdr->e_phnum)++;
+#endif
+
+ /* Prepare PT_LOAD headers for system ram chunks. */
+ ced->ehdr = ehdr;
+ ced->bufp = bufp;
+ ret = walk_system_ram_res(0, -1, ced,
+ prepare_elf64_ram_headers_callback);
+ if (ret < 0)
+ return ret;
+
+ *addr = buf;
+ *sz = elf_sz;
+ return 0;
+}
+
+/* Prepare elf headers. Return addr and size */
+int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
+{
+ struct crash_elf_data *ced;
+ int ret;
+
+ ced = kzalloc(sizeof(*ced), GFP_KERNEL);
+ if (!ced)
+ return -ENOMEM;
+
+ fill_up_crash_elf_data(ced, image);
+
+ /* By default prepare 64bit headers */
+ ret = prepare_elf64_headers(ced, addr, sz);
+ kfree(ced);
+ return ret;
+}
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 50dfcb039a41..3c7a034f1fe1 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -16,6 +16,26 @@ extern struct mutex kexec_mutex;

#ifdef CONFIG_KEXEC_FILE
#include <linux/purgatory.h>
+
+/* Alignment required for elf header segment */
+#define ELF_CORE_HEADER_ALIGN 4096
+
+/* Misc data about ram ranges needed to prepare elf headers */
+struct crash_elf_data {
+ struct kimage *image;
+ /*
+ * Total number of ram ranges we have after various adjustments for
+ * crash reserved region, etc.
+ */
+ unsigned int max_nr_ranges;
+
+ /* Pointer to elf header */
+ void *ehdr;
+ /* Pointer to next phdr */
+ void *bufp;
+ struct crash_mem mem;
+};
+
void kimage_file_post_load_cleanup(struct kimage *image);
extern char kexec_purgatory[];
extern size_t kexec_purgatory_size;
--
2.14.1

2017-09-15 10:58:22

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 05/10] asm-generic: add kexec_file_load system call to unistd.h

The initial user of this system call number is arm64.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
---
include/uapi/asm-generic/unistd.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 061185a5eb51..086697fe3917 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -731,9 +731,11 @@ __SYSCALL(__NR_pkey_alloc, sys_pkey_alloc)
__SYSCALL(__NR_pkey_free, sys_pkey_free)
#define __NR_statx 291
__SYSCALL(__NR_statx, sys_statx)
+#define __NR_kexec_file_load 292
+__SYSCALL(__NR_kexec_file_load, sys_kexec_file_load)

#undef __NR_syscalls
-#define __NR_syscalls 292
+#define __NR_syscalls 293

/*
* All syscalls below here should go away really,
--
2.14.1

2017-09-15 10:58:26

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 06/10] arm64: kexec_file: create purgatory

This is a basic purgatory, or a kind of glue code between the two kernels,
for arm64.

Since purgatory is assumed to be relocatable (not executable) object by
kexec generic code, arch_kexec_apply_relocations_add() is required in
general. Arm64's purgatory, however, is a simple asm and all the references
can be resolved as local, no re-linking is needed here.

Please note that even if we don't support digest check at purgatory we
need purgatory_sha_regions and purgatory_sha256_digest as they are
referenced by generic kexec code.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Makefile | 1 +
arch/arm64/purgatory/Makefile | 24 +++++++++++++++++++
arch/arm64/purgatory/entry.S | 55 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)
create mode 100644 arch/arm64/purgatory/Makefile
create mode 100644 arch/arm64/purgatory/entry.S

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 9b41f1e3b1a0..429f60728c0a 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -105,6 +105,7 @@ core-$(CONFIG_XEN) += arch/arm64/xen/
core-$(CONFIG_CRYPTO) += arch/arm64/crypto/
libs-y := arch/arm64/lib/ $(libs-y)
core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
+core-$(CONFIG_KEXEC_FILE) += arch/arm64/purgatory/

# Default target when executing plain make
boot := arch/arm64/boot
diff --git a/arch/arm64/purgatory/Makefile b/arch/arm64/purgatory/Makefile
new file mode 100644
index 000000000000..c2127a2cbd51
--- /dev/null
+++ b/arch/arm64/purgatory/Makefile
@@ -0,0 +1,24 @@
+OBJECT_FILES_NON_STANDARD := y
+
+purgatory-y := entry.o
+
+targets += $(purgatory-y)
+PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))
+
+LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined \
+ -nostdlib -z nodefaultlib
+targets += purgatory.ro
+
+$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
+ $(call if_changed,ld)
+
+targets += kexec_purgatory.c
+
+CMD_BIN2C = $(objtree)/scripts/basic/bin2c
+quiet_cmd_bin2c = BIN2C $@
+ cmd_bin2c = $(CMD_BIN2C) kexec_purgatory < $< > $@
+
+$(obj)/kexec_purgatory.c: $(obj)/purgatory.ro FORCE
+ $(call if_changed,bin2c)
+
+obj-${CONFIG_KEXEC_FILE} += kexec_purgatory.o
diff --git a/arch/arm64/purgatory/entry.S b/arch/arm64/purgatory/entry.S
new file mode 100644
index 000000000000..fe6e968076db
--- /dev/null
+++ b/arch/arm64/purgatory/entry.S
@@ -0,0 +1,55 @@
+/*
+ * kexec core purgatory
+ */
+#include <linux/linkage.h>
+#include <uapi/linux/kexec.h>
+
+#define SHA256_DIGEST_SIZE 32 /* defined in crypto/sha.h */
+
+.text
+
+ENTRY(purgatory_start)
+ /* Start new image. */
+ ldr x17, __kernel_entry
+ ldr x0, __dtb_addr
+ mov x1, xzr
+ mov x2, xzr
+ mov x3, xzr
+ br x17
+END(purgatory_start)
+
+/*
+ * data section:
+ * kernel_entry and dtb_addr are global but also labelled as local,
+ * "__xxx:", to avoid unwanted re-linking.
+ *
+ * purgatory_sha_regions and purgatory_sha256_digest are referenced
+ * by kexec generic code and so must exist, but not actually used
+ * here because hash check is not that useful in purgatory.
+ */
+.align 3
+
+.globl kernel_entry
+kernel_entry:
+__kernel_entry:
+ .quad 0
+END(kernel_entry)
+
+.globl dtb_addr
+dtb_addr:
+__dtb_addr:
+ .quad 0
+END(dtb_addr)
+
+.globl purgatory_sha_regions
+purgatory_sha_regions:
+ .rept KEXEC_SEGMENT_MAX
+ .quad 0
+ .quad 0
+ .endr
+END(purgatory_sha_regions)
+
+.globl purgatory_sha256_digest
+purgatory_sha256_digest:
+ .skip SHA256_DIGEST_SIZE
+END(purgatory_sha256_digest)
--
2.14.1

2017-09-15 10:58:35

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 07/10] arm64: kexec_file: load initrd, device-tree and purgatory segments

load_other_segments() sets up and adds all the memory segments necessary
other than kernel, including initrd, device-tree blob and purgatory.
Most of the code was borrowed from kexec-tools' counterpart.

arch_kimage_kernel_post_load_cleanup() is meant to free arm64-specific data
allocated for loading kernel.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/kexec.h | 22 ++++
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/machine_kexec_file.c | 213 +++++++++++++++++++++++++++++++++
3 files changed, 237 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kernel/machine_kexec_file.c

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index e17f0529a882..2fadd3cbf3af 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -93,6 +93,28 @@ 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 {
+ void *dtb_buf;
+};
+
+struct kimage;
+
+#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
+extern int arch_kimage_file_post_load_cleanup(struct kimage *image);
+
+extern 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);
+extern int load_other_segments(struct kimage *image,
+ unsigned long kernel_load_addr,
+ char *initrd, unsigned long initrd_len,
+ char *cmdline, unsigned long cmdline_len);
+#endif
+
#endif /* __ASSEMBLY__ */

#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index f2b4e816b6de..5df003d6157c 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -48,8 +48,9 @@ arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL) += acpi_parking_protocol.o
arm64-obj-$(CONFIG_PARAVIRT) += paravirt.o
arm64-obj-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
arm64-obj-$(CONFIG_HIBERNATION) += hibernate.o hibernate-asm.o
-arm64-obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o \
+arm64-obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o relocate_kernel.o \
cpu-reset.o
+arm64-obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o
arm64-obj-$(CONFIG_ARM64_RELOC_TEST) += arm64-reloc-test.o
arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
arm64-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
new file mode 100644
index 000000000000..09be2674c2ab
--- /dev/null
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -0,0 +1,213 @@
+/*
+ * kexec_file for arm64
+ *
+ * Copyright (C) 2017 Linaro Limited
+ * Author: AKASHI Takahiro <[email protected]>
+ *
+ * Most code is derived from arm64 port of kexec-tools
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "kexec_file: " fmt
+
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/kexec.h>
+#include <linux/libfdt.h>
+#include <linux/memblock.h>
+#include <linux/of_fdt.h>
+
+static int __dt_root_addr_cells;
+static int __dt_root_size_cells;
+
+struct kexec_file_ops *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_kernel_post_load_cleanup(image);
+}
+
+int arch_kexec_walk_mem(struct kexec_buf *kbuf, int (*func)(u64, u64, void *))
+{
+ if (kbuf->image->type == KEXEC_TYPE_CRASH)
+ return walk_iomem_res_desc(crashk_res.desc,
+ IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
+ crashk_res.start, crashk_res.end,
+ kbuf, func);
+ else if (kbuf->top_down)
+ return walk_system_ram_res_rev(0, ULONG_MAX, kbuf, func);
+ else
+ return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
+}
+
+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 range_len;
+ int ret;
+
+ /* duplicate dt blob */
+ buf_size = fdt_totalsize(initial_boot_params);
+ range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
+
+ if (initrd_load_addr)
+ buf_size += fdt_prop_len("initrd-start", sizeof(u64))
+ + fdt_prop_len("initrd-end", sizeof(u64));
+
+ if (cmdline)
+ 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)
+ goto out_err;
+
+ nodeoffset = fdt_path_offset(buf, "/chosen");
+ if (nodeoffset < 0)
+ goto out_err;
+
+ /* add bootargs */
+ if (cmdline) {
+ ret = fdt_setprop(buf, nodeoffset, "bootargs",
+ cmdline, cmdline_len + 1);
+ if (ret)
+ goto out_err;
+ }
+
+ /* add initrd-* */
+ if (initrd_load_addr) {
+ value = cpu_to_fdt64(initrd_load_addr);
+ ret = fdt_setprop(buf, nodeoffset, "initrd-start",
+ &value, sizeof(value));
+ if (ret)
+ goto out_err;
+
+ value = cpu_to_fdt64(initrd_load_addr + initrd_len);
+ ret = fdt_setprop(buf, nodeoffset, "initrd-end",
+ &value, sizeof(value));
+ if (ret)
+ 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,
+ char *initrd, unsigned long initrd_len,
+ char *cmdline, unsigned long cmdline_len)
+{
+ struct kexec_buf kbuf;
+ unsigned long initrd_load_addr = 0;
+ unsigned long purgatory_load_addr, dtb_load_addr;
+ char *dtb = NULL;
+ unsigned long dtb_len;
+ int ret = 0;
+
+ kbuf.image = image;
+ /* not allocate anything below the kernel */
+ kbuf.buf_min = kernel_load_addr;
+
+ /* Load initrd */
+ if (initrd) {
+ kbuf.buffer = initrd;
+ kbuf.bufsz = initrd_len;
+ kbuf.memsz = initrd_len;
+ kbuf.buf_align = PAGE_SIZE;
+ /* within 1GB-aligned window of up to 32GB in size */
+ kbuf.buf_max = round_down(kernel_load_addr, SZ_1G)
+ + (unsigned long)SZ_1G * 31;
+ kbuf.top_down = 0;
+
+ 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 = 1;
+
+ ret = kexec_add_buffer(&kbuf);
+ if (ret)
+ goto out_err;
+ dtb_load_addr = kbuf.mem;
+ image->arch.dtb_buf = dtb;
+
+ pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+ dtb_load_addr, dtb_len, dtb_len);
+
+ /* Load purgatory */
+ ret = kexec_load_purgatory(image, kernel_load_addr, ULONG_MAX, 1,
+ &purgatory_load_addr);
+ if (ret) {
+ pr_err("Loading purgatory failed\n");
+ goto out_err;
+ }
+
+ ret = kexec_purgatory_get_set_symbol(image, "kernel_entry",
+ &kernel_load_addr, sizeof(kernel_load_addr), 0);
+ if (ret) {
+ pr_err("Setting symbol (kernel_entry) failed.\n");
+ goto out_err;
+ }
+
+ ret = kexec_purgatory_get_set_symbol(image, "dtb_addr",
+ &dtb_load_addr, sizeof(dtb_load_addr), 0);
+ if (ret) {
+ pr_err("Setting symbol (dtb_addr) failed.\n");
+ goto out_err;
+ }
+
+ pr_debug("Loaded purgatory at 0x%lx\n", purgatory_load_addr);
+
+ return 0;
+
+out_err:
+ vfree(dtb);
+ image->arch.dtb_buf = NULL;
+ return ret;
+}
--
2.14.1

2017-09-15 10:58:38

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 08/10] arm64: kexec_file: set up for crash dump adding elf core header

load_crashdump_segments() creates and loads a memory segment of elf core
header for crash dump.

"linux,usable-memory-range" and "linux,elfcorehdr" will add to the 2nd
kernel's device-tree blob. The logic of this cod is also from kexec-tools.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/kexec.h | 5 ++
arch/arm64/kernel/machine_kexec_file.c | 149 +++++++++++++++++++++++++++++++++
kernel/kexec_file.c | 2 +-
3 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 2fadd3cbf3af..edb702e64a8a 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -98,6 +98,10 @@ static inline void crash_post_resume(void) {}

struct kimage_arch {
void *dtb_buf;
+ /* Core ELF header buffer */
+ void *elf_headers;
+ unsigned long elf_headers_sz;
+ unsigned long elf_load_addr;
};

struct kimage;
@@ -113,6 +117,7 @@ extern int load_other_segments(struct kimage *image,
unsigned long kernel_load_addr,
char *initrd, unsigned long initrd_len,
char *cmdline, unsigned long cmdline_len);
+extern int load_crashdump_segments(struct kimage *image);
#endif

#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 09be2674c2ab..e4671610fb12 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -32,6 +32,10 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
vfree(image->arch.dtb_buf);
image->arch.dtb_buf = NULL;

+ vfree(image->arch.elf_headers);
+ image->arch.elf_headers = NULL;
+ image->arch.elf_headers_sz = 0;
+
return kexec_kernel_post_load_cleanup(image);
}

@@ -48,6 +52,77 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf, int (*func)(u64, u64, void *))
return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
}

+static int __init arch_kexec_file_init(void)
+{
+ /* Those values are used later on loading the kernel */
+ __dt_root_addr_cells = dt_root_addr_cells;
+ __dt_root_size_cells = dt_root_size_cells;
+
+ return 0;
+}
+late_initcall(arch_kexec_file_init);
+
+#define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
+#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
+
+static int fdt_prop_len(const char *prop_name, int len)
+{
+ return (strlen(prop_name) + 1) +
+ sizeof(struct fdt_property) +
+ FDT_TAGALIGN(len);
+}
+
+static bool cells_size_fitted(unsigned long base, unsigned long size)
+{
+ /* if *_cells >= 2, cells can hold 64-bit values anyway */
+ if ((__dt_root_addr_cells == 1) && (base >= (1ULL << 32)))
+ return false;
+
+ if ((__dt_root_size_cells == 1) && (size >= (1ULL << 32)))
+ return false;
+
+ return true;
+}
+
+static void fill_property(void *buf, u64 val64, int cells)
+{
+ u32 val32;
+ int i;
+
+ if (cells == 1) {
+ val32 = cpu_to_fdt32((u32)val64);
+ memcpy(buf, &val32, sizeof(val32));
+ } else {
+ for (i = 0; i < (cells * sizeof(u32) - sizeof(u64)); i++)
+ *(char *)buf++ = 0;
+
+ val64 = cpu_to_fdt64(val64);
+ memcpy(buf, &val64, sizeof(val64));
+ }
+}
+
+static int fdt_setprop_range(void *fdt, int nodeoffset, const char *name,
+ unsigned long addr, unsigned long size)
+{
+ u64 range[2];
+ void *prop;
+ size_t buf_size;
+ int result;
+
+ prop = range;
+ buf_size = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
+
+ fill_property(prop, addr, __dt_root_addr_cells);
+ prop += __dt_root_addr_cells * sizeof(u32);
+
+ fill_property(prop, size, __dt_root_size_cells);
+ prop += __dt_root_size_cells * sizeof(u32);
+
+ result = fdt_setprop(fdt, nodeoffset, name, range, buf_size);
+
+ return result;
+}
+
int setup_dtb(struct kimage *image,
unsigned long initrd_load_addr, unsigned long initrd_len,
char *cmdline, unsigned long cmdline_len,
@@ -60,10 +135,26 @@ int setup_dtb(struct kimage *image,
int range_len;
int ret;

+ /* check ranges against root's #address-cells and #size-cells */
+ if (image->type == KEXEC_TYPE_CRASH &&
+ (!cells_size_fitted(image->arch.elf_load_addr,
+ image->arch.elf_headers_sz) ||
+ !cells_size_fitted(crashk_res.start,
+ crashk_res.end - crashk_res.start + 1))) {
+ pr_err("Crash memory region doesn't fit into DT's root cell sizes.\n");
+ ret = -EINVAL;
+ goto out_err;
+ }
+
/* duplicate dt blob */
buf_size = fdt_totalsize(initial_boot_params);
range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);

+ if (image->type == KEXEC_TYPE_CRASH)
+ buf_size += fdt_prop_len("linux,elfcorehdr", range_len)
+ + fdt_prop_len("linux,usable-memory-range",
+ range_len);
+
if (initrd_load_addr)
buf_size += fdt_prop_len("initrd-start", sizeof(u64))
+ fdt_prop_len("initrd-end", sizeof(u64));
@@ -85,6 +176,23 @@ int setup_dtb(struct kimage *image,
if (nodeoffset < 0)
goto out_err;

+ if (image->type == KEXEC_TYPE_CRASH) {
+ /* add linux,elfcorehdr */
+ ret = fdt_setprop_range(buf, nodeoffset, "linux,elfcorehdr",
+ image->arch.elf_load_addr,
+ image->arch.elf_headers_sz);
+ if (ret)
+ goto out_err;
+
+ /* add linux,usable-memory-range */
+ ret = fdt_setprop_range(buf, nodeoffset,
+ "linux,usable-memory-range",
+ crashk_res.start,
+ crashk_res.end - crashk_res.start + 1);
+ if (ret)
+ goto out_err;
+ }
+
/* add bootargs */
if (cmdline) {
ret = fdt_setprop(buf, nodeoffset, "bootargs",
@@ -211,3 +319,44 @@ int load_other_segments(struct kimage *image, unsigned long kernel_load_addr,
image->arch.dtb_buf = NULL;
return ret;
}
+
+int load_crashdump_segments(struct kimage *image)
+{
+ void *elf_addr;
+ unsigned long elf_sz;
+ struct kexec_buf kbuf;
+ int ret;
+
+ if (image->type != KEXEC_TYPE_CRASH)
+ return 0;
+
+ /* Prepare elf headers and add a segment */
+ ret = prepare_elf_headers(image, &elf_addr, &elf_sz);
+ if (ret) {
+ pr_err("Preparing elf core header failed\n");
+ return ret;
+ }
+
+ kbuf.image = image;
+ kbuf.buffer = elf_addr;
+ kbuf.bufsz = elf_sz;
+ kbuf.memsz = elf_sz;
+ kbuf.buf_align = PAGE_SIZE;
+ kbuf.buf_min = crashk_res.start;
+ kbuf.buf_max = crashk_res.end + 1;
+ kbuf.top_down = 1;
+
+ ret = kexec_add_buffer(&kbuf);
+ if (ret) {
+ vfree(elf_addr);
+ return ret;
+ }
+ image->arch.elf_headers = elf_addr;
+ image->arch.elf_headers_sz = elf_sz;
+ image->arch.elf_load_addr = kbuf.mem;
+
+ pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+ image->arch.elf_load_addr, elf_sz, elf_sz);
+
+ return ret;
+}
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 6a0e31e175af..6b206883f429 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1317,7 +1317,7 @@ static int prepare_elf64_headers(struct crash_elf_data *ced,
phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
(ehdr->e_phnum)++;

-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
/* Prepare PT_LOAD type program header for kernel text region */
phdr = (Elf64_Phdr *)bufp;
bufp += sizeof(Elf64_Phdr);
--
2.14.1

2017-09-15 10:58:43

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 09/10] arm64: enable KEXEC_FILE config

Modify arm64/Kconfig and Makefile to enable kexec_file_load support.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index dfd908630631..fa7299f08cc6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -756,6 +756,28 @@ config KEXEC
but it is independent of the system firmware. And like a reboot
you can start any kernel with it, not just Linux.

+config KEXEC_FILE
+ bool "kexec file based system call"
+ select KEXEC_CORE
+ select BUILD_BIN2C
+ ---help---
+ This is new version of kexec system call. This system call is
+ file based and takes file descriptors as system call argument
+ for kernel and initramfs as opposed to list of segments as
+ accepted by previous system call.
+
+ In addition to this option, you need to enable a specific type
+ of image support.
+
+config KEXEC_VERIFY_SIG
+ bool "Verify kernel signature during kexec_file_load() syscall"
+ depends on KEXEC_FILE
+ select SYSTEM_DATA_VERIFICATION
+ ---help---
+ Select this option to verify a signature with loaded kernel
+ image. If configured, any attempt of loading a image without
+ valid signature will fail.
+
config CRASH_DUMP
bool "Build kdump crash kernel"
help
--
2.14.1

2017-09-15 10:58:49

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 10/10] arm64: kexec_file: add Image format support

The "Image" binary will be loaded at the offset of TEXT_OFFSET from
the start of system memory. TEXT_OFFSET is determined from the header
of the image.

Regarding kernel signature verification, it will be done through
verify_pefile_signature() as arm64's "Image" binary can be seen as
in PE format. This approach is consistent with x86 implementation.

we can sign an image with sbsign command.

Signed-off-by: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 7 +++
arch/arm64/include/asm/kexec.h | 66 +++++++++++++++++++++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/kexec_image.c | 105 +++++++++++++++++++++++++++++++++
arch/arm64/kernel/machine_kexec_file.c | 3 +
5 files changed, 182 insertions(+)
create mode 100644 arch/arm64/kernel/kexec_image.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fa7299f08cc6..b7f1a760434a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -769,10 +769,17 @@ config KEXEC_FILE
In addition to this option, you need to enable a specific type
of image support.

+config KEXEC_FILE_IMAGE_FMT
+ bool "Enable Image support"
+ depends on KEXEC_FILE
+ ---help---
+ Select this option to enable 'Image' kernel loading.
+
config KEXEC_VERIFY_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
select SYSTEM_DATA_VERIFICATION
+ select SIGNED_PE_FILE_VERIFICATION if KEXEC_FILE_IMAGE_FMT
---help---
Select this option to verify a signature with loaded kernel
image. If configured, any attempt of loading a image without
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index edb702e64a8a..7ec529e11a8b 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -104,6 +104,72 @@ struct kimage_arch {
unsigned long elf_load_addr;
};

+/**
+ * struct arm64_image_header - arm64 kernel image header
+ *
+ * @pe_sig: Optional PE format 'MZ' signature
+ * @branch_code: Instruction to branch to stext
+ * @text_offset: Image load offset, little endian
+ * @image_size: Effective image size, little endian
+ * @flags:
+ * Bit 0: Kernel endianness. 0=little endian, 1=big endian
+ * @reserved: Reserved
+ * @magic: Magic number, "ARM\x64"
+ * @pe_header: Optional offset to a PE format header
+ **/
+
+struct arm64_image_header {
+ u8 pe_sig[2];
+ u8 pad[2];
+ u32 branch_code;
+ u64 text_offset;
+ u64 image_size;
+ u64 flags;
+ u64 reserved[3];
+ u8 magic[4];
+ u32 pe_header;
+};
+
+static const u8 arm64_image_magic[4] = {'A', 'R', 'M', 0x64U};
+static const u8 arm64_image_pe_sig[2] = {'M', 'Z'};
+
+/**
+ * arm64_header_check_magic - Helper to check the arm64 image header.
+ *
+ * Returns non-zero if header is OK.
+ */
+
+static inline int arm64_header_check_magic(const struct arm64_image_header *h)
+{
+ if (!h)
+ return 0;
+
+ if (!h->text_offset)
+ return 0;
+
+ return (h->magic[0] == arm64_image_magic[0]
+ && h->magic[1] == arm64_image_magic[1]
+ && h->magic[2] == arm64_image_magic[2]
+ && h->magic[3] == arm64_image_magic[3]);
+}
+
+/**
+ * arm64_header_check_pe_sig - Helper to check the arm64 image header.
+ *
+ * Returns non-zero if 'MZ' signature is found.
+ */
+
+static inline int arm64_header_check_pe_sig(const struct arm64_image_header *h)
+{
+ if (!h)
+ return 0;
+
+ return (h->pe_sig[0] == arm64_image_pe_sig[0]
+ && h->pe_sig[1] == arm64_image_pe_sig[1]);
+}
+
+extern struct kexec_file_ops kexec_image_ops;
+
struct kimage;

#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 5df003d6157c..a1161bab6810 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -51,6 +51,7 @@ arm64-obj-$(CONFIG_HIBERNATION) += hibernate.o hibernate-asm.o
arm64-obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o relocate_kernel.o \
cpu-reset.o
arm64-obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o
+arm64-obj-$(CONFIG_KEXEC_FILE_IMAGE_FMT) += kexec_image.o
arm64-obj-$(CONFIG_ARM64_RELOC_TEST) += arm64-reloc-test.o
arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
arm64-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
new file mode 100644
index 000000000000..b19538af2f3d
--- /dev/null
+++ b/arch/arm64/kernel/kexec_image.c
@@ -0,0 +1,105 @@
+/*
+ * Kexec image loader
+
+ * Copyright (C) 2017 Linaro Limited
+ * Author: AKASHI Takahiro <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "kexec_file(Image): " fmt
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/kexec.h>
+#include <linux/verification.h>
+#include <asm/byteorder.h>
+#include <asm/memory.h>
+
+static int image_probe(const char *kernel_buf, unsigned long kernel_len)
+{
+ const struct arm64_image_header *h;
+
+ h = (const struct arm64_image_header *)(kernel_buf);
+
+ if ((kernel_len < sizeof(*h)) || !arm64_header_check_magic(h))
+ return -EINVAL;
+
+ pr_debug("PE format: %s\n",
+ (arm64_header_check_pe_sig(h) ? "yes" : "no"));
+
+ return 0;
+}
+
+static void *image_load(struct kimage *image, char *kernel,
+ unsigned long kernel_len, char *initrd,
+ unsigned long initrd_len, char *cmdline,
+ unsigned long cmdline_len)
+{
+ struct kexec_buf kbuf;
+ struct arm64_image_header *h = (struct arm64_image_header *)kernel;
+ unsigned long text_offset, kernel_load_addr;
+ int ret;
+
+ /* Create elf core header segment */
+ ret = load_crashdump_segments(image);
+ if (ret)
+ goto out;
+
+ /* Load the kernel */
+ kbuf.image = image;
+ if (image->type == KEXEC_TYPE_CRASH) {
+ kbuf.buf_min = crashk_res.start;
+ kbuf.buf_max = crashk_res.end + 1;
+ } else {
+ kbuf.buf_min = 0;
+ kbuf.buf_max = ULONG_MAX;
+ }
+ kbuf.top_down = 0;
+
+ kbuf.buffer = kernel;
+ kbuf.bufsz = kernel_len;
+ kbuf.memsz = le64_to_cpu(h->image_size);
+ text_offset = le64_to_cpu(h->text_offset);
+ kbuf.buf_align = SZ_2M;
+
+ /* Adjust kernel segment with TEXT_OFFSET */
+ kbuf.memsz += text_offset;
+
+ ret = kexec_add_buffer(&kbuf);
+ if (ret)
+ goto out;
+
+ image->segment[image->nr_segments - 1].mem += text_offset;
+ image->segment[image->nr_segments - 1].memsz -= text_offset;
+ kernel_load_addr = kbuf.mem + text_offset;
+
+ pr_debug("Loaded kernel at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+ kernel_load_addr, kbuf.bufsz, kbuf.memsz);
+
+ /* Load additional data */
+ ret = load_other_segments(image, kernel_load_addr,
+ initrd, initrd_len, cmdline, cmdline_len);
+
+out:
+ return ERR_PTR(ret);
+}
+
+#ifdef CONFIG_KEXEC_VERIFY_SIG
+static int image_verify_sig(const char *kernel, unsigned long kernel_len)
+{
+ return verify_pefile_signature(kernel, kernel_len, NULL,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+}
+#endif
+
+struct kexec_file_ops kexec_image_ops = {
+ .probe = image_probe,
+ .load = image_load,
+#ifdef CONFIG_KEXEC_VERIFY_SIG
+ .verify_sig = image_verify_sig,
+#endif
+};
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index e4671610fb12..fbf1cb4b58df 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -24,6 +24,9 @@ static int __dt_root_addr_cells;
static int __dt_root_size_cells;

struct kexec_file_ops *kexec_file_loaders[] = {
+#ifdef CONFIG_KEXEC_FILE_IMAGE_FMT
+ &kexec_image_ops,
+#endif
NULL
};

--
2.14.1

2017-09-21 07:35:31

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

Hi AKASHI,
On 09/15/17 at 07:59pm, AKASHI Takahiro wrote:
> arch_kexec_kernel_*() and arch_kimage_file_post_load_cleanup can now be
> duplicated among some architectures, so let's factor them out.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> Cc: Dave Young <[email protected]>
> Cc: Vivek Goyal <[email protected]>
> Cc: Baoquan He <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Thiago Jung Bauermann <[email protected]>
> ---
> arch/powerpc/include/asm/kexec.h | 4 ++
> arch/powerpc/kernel/machine_kexec_file_64.c | 36 ++----------------
> arch/x86/kernel/machine_kexec_64.c | 59 +----------------------------
> include/linux/kexec.h | 26 ++++++++++---
> kernel/kexec_file.c | 52 +++++++++++++++++++------
> 5 files changed, 70 insertions(+), 107 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 25668bc8cb2a..50810d24e38f 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -94,6 +94,10 @@ static inline bool kdump_in_progress(void)
> #ifdef CONFIG_KEXEC_FILE
> extern struct kexec_file_ops kexec_elf64_ops;
>
> +#define arch_kexec_kernel_image_probe arch_kexec_kernel_image_probe
> +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> + unsigned long buf_len);
> +
> #ifdef CONFIG_IMA_KEXEC
> #define ARCH_HAS_KIMAGE_ARCH
>
> diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c b/arch/powerpc/kernel/machine_kexec_file_64.c
> index 992c0d258e5d..5b7c4a3fbb50 100644
> --- a/arch/powerpc/kernel/machine_kexec_file_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_file_64.c
> @@ -31,8 +31,9 @@
>
> #define SLAVE_CODE_SIZE 256
>
> -static struct kexec_file_ops *kexec_file_loaders[] = {
> +struct kexec_file_ops *kexec_file_loaders[] = {
> &kexec_elf64_ops,
> + NULL
> };
>
> int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> @@ -45,38 +46,7 @@ int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> if (image->type == KEXEC_TYPE_CRASH)
> return -ENOTSUPP;
>
> - for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
> - fops = kexec_file_loaders[i];
> - if (!fops || !fops->probe)
> - continue;
> -
> - ret = fops->probe(buf, buf_len);
> - if (!ret) {
> - image->fops = fops;
> - return ret;
> - }
> - }
> -
> - return ret;
> -}
> -
> -void *arch_kexec_kernel_image_load(struct kimage *image)
> -{
> - if (!image->fops || !image->fops->load)
> - return ERR_PTR(-ENOEXEC);
> -
> - return image->fops->load(image, image->kernel_buf,
> - image->kernel_buf_len, image->initrd_buf,
> - image->initrd_buf_len, image->cmdline_buf,
> - image->cmdline_buf_len);
> -}
> -
> -int arch_kimage_file_post_load_cleanup(struct kimage *image)
> -{
> - if (!image->fops || !image->fops->cleanup)
> - return 0;
> -
> - return image->fops->cleanup(image->image_loader_data);
> + return kexec_kernel_image_probe(image, buf, buf_len);
> }
>
> /**
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index cb0a30473c23..f4df8315d001 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -30,8 +30,9 @@
> #include <asm/set_memory.h>
>
> #ifdef CONFIG_KEXEC_FILE
> -static struct kexec_file_ops *kexec_file_loaders[] = {
> +struct kexec_file_ops *kexec_file_loaders[] = {
> &kexec_bzImage64_ops,
> + NULL
> };
> #endif
>
> @@ -361,62 +362,6 @@ void arch_crash_save_vmcoreinfo(void)
> /* arch-dependent functionality related to kexec file-based syscall */
>
> #ifdef CONFIG_KEXEC_FILE
> -int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> - unsigned long buf_len)
> -{
> - int i, ret = -ENOEXEC;
> - struct kexec_file_ops *fops;
> -
> - for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
> - fops = kexec_file_loaders[i];
> - if (!fops || !fops->probe)
> - continue;
> -
> - ret = fops->probe(buf, buf_len);
> - if (!ret) {
> - image->fops = fops;
> - return ret;
> - }
> - }
> -
> - return ret;
> -}
> -
> -void *arch_kexec_kernel_image_load(struct kimage *image)
> -{
> - vfree(image->arch.elf_headers);
> - image->arch.elf_headers = NULL;
> -
> - if (!image->fops || !image->fops->load)
> - return ERR_PTR(-ENOEXEC);
> -
> - return image->fops->load(image, image->kernel_buf,
> - image->kernel_buf_len, image->initrd_buf,
> - image->initrd_buf_len, image->cmdline_buf,
> - image->cmdline_buf_len);
> -}
> -
> -int arch_kimage_file_post_load_cleanup(struct kimage *image)
> -{
> - if (!image->fops || !image->fops->cleanup)
> - return 0;
> -
> - return image->fops->cleanup(image->image_loader_data);
> -}
> -
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> -int arch_kexec_kernel_verify_sig(struct kimage *image, void *kernel,
> - unsigned long kernel_len)
> -{
> - if (!image->fops || !image->fops->verify_sig) {
> - pr_debug("kernel loader does not support signature verification.");
> - return -EKEYREJECTED;
> - }
> -
> - return image->fops->verify_sig(kernel, kernel_len);
> -}
> -#endif
> -
> /*
> * Apply purgatory relocations.
> *
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index dd056fab9e35..4a2b24d94e04 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -134,6 +134,26 @@ struct kexec_file_ops {
> #endif
> };
>
> +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> + unsigned long buf_len);
> +void *kexec_kernel_image_load(struct kimage *image);
> +int kexec_kernel_post_load_cleanup(struct kimage *image);
> +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> + unsigned long buf_len);
> +
> +#ifndef arch_kexec_kernel_image_probe
> +#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
> +#endif
> +#ifndef arch_kexec_kernel_image_load
> +#define arch_kexec_kernel_image_load kexec_kernel_image_load
> +#endif
> +#ifndef arch_kimage_file_post_load_cleanup
> +#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
> +#endif
> +#ifndef arch_kexec_kernel_verify_sig
> +#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
> +#endif
> +
> /**
> * struct kexec_buf - parameters for finding a place for a buffer in memory
> * @image: kexec image in which memory to search.
> @@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
> size_t crash_get_memory_size(void);
> void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
>
> -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> - unsigned long buf_len);
> -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
> -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> - unsigned long buf_len);

I thought we can keep using the __weak function in common code and drop
the arch functions, no need the #ifndef arch_kexec_kernel_image_probe
and the function renaming stuff. But I did not notice the powerpc
_probe function checks KEXEC_ON_CRASH, that should be checked earlier
and we can fail out early if not supported, but I have no idea
how to do it gracefully for now.

Also for x86 _load function it cleanups image->arch.elf_headers, it can
not be dropped simply.

Consider the above two issues, maybe you can keep the powerpc
version of _probe() and x86 version of _load(), and still copy the common code
to kexec_file.c weak functions. I can post patch to cleanup the x86
_load function in the future.

> int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr,
> Elf_Shdr *sechdrs, unsigned int relsec);
> int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 9f48f4412297..6203b54e04c5 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -26,30 +26,60 @@
> #include <linux/vmalloc.h>
> #include "kexec_internal.h"
>
> +__weak struct kexec_file_ops *kexec_file_loaders[] = {NULL};
> +
> static int kexec_calculate_store_digests(struct kimage *image);
>
> -/* Architectures can provide this probe function */
> -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> - unsigned long buf_len)
> +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> + unsigned long buf_len)
> {
> - return -ENOEXEC;
> + int i, ret = -ENOEXEC;
> + struct kexec_file_ops *fops;
> +
> + for (i = 0; ; i++) {
> + fops = kexec_file_loaders[i];
> + if (!fops || !fops->probe)
> + break;
> +
> + ret = fops->probe(buf, buf_len);
> + if (!ret) {
> + image->fops = fops;
> + return ret;
> + }
> + }
> +
> + return ret;
> }
>
> -void * __weak arch_kexec_kernel_image_load(struct kimage *image)
> +void *kexec_kernel_image_load(struct kimage *image)
> {
> - return ERR_PTR(-ENOEXEC);
> + if (!image->fops || !image->fops->load)
> + return ERR_PTR(-ENOEXEC);
> +
> + return image->fops->load(image, image->kernel_buf,
> + image->kernel_buf_len, image->initrd_buf,
> + image->initrd_buf_len, image->cmdline_buf,
> + image->cmdline_buf_len);
> }
>
> -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
> +int kexec_kernel_post_load_cleanup(struct kimage *image)
> {
> - return -EINVAL;
> + if (!image->fops || !image->fops->cleanup)
> + return 0;
> +
> + return image->fops->cleanup(image->image_loader_data);
> }
>
> #ifdef CONFIG_KEXEC_VERIFY_SIG
> -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> - unsigned long buf_len)
> +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> + unsigned long buf_len)
> {
> - return -EKEYREJECTED;
> + if (!image->fops || !image->fops->verify_sig) {
> + pr_debug("kernel loader does not support signature verification.\n");
> + return -EKEYREJECTED;
> + }
> +
> + return image->fops->verify_sig(buf, buf_len);
> }
> #endif
>
> --
> 2.14.1
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

2017-09-22 07:55:58

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

Hi Dave,

On Thu, Sep 21, 2017 at 03:35:16PM +0800, Dave Young wrote:
> Hi AKASHI,
> On 09/15/17 at 07:59pm, AKASHI Takahiro wrote:
> > arch_kexec_kernel_*() and arch_kimage_file_post_load_cleanup can now be
> > duplicated among some architectures, so let's factor them out.
> >
> > Signed-off-by: AKASHI Takahiro <[email protected]>
> > Cc: Dave Young <[email protected]>
> > Cc: Vivek Goyal <[email protected]>
> > Cc: Baoquan He <[email protected]>
> > Cc: Michael Ellerman <[email protected]>
> > Cc: Thiago Jung Bauermann <[email protected]>
> > ---
> > arch/powerpc/include/asm/kexec.h | 4 ++
> > arch/powerpc/kernel/machine_kexec_file_64.c | 36 ++----------------
> > arch/x86/kernel/machine_kexec_64.c | 59 +----------------------------
> > include/linux/kexec.h | 26 ++++++++++---
> > kernel/kexec_file.c | 52 +++++++++++++++++++------
> > 5 files changed, 70 insertions(+), 107 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> > index 25668bc8cb2a..50810d24e38f 100644
> > --- a/arch/powerpc/include/asm/kexec.h
> > +++ b/arch/powerpc/include/asm/kexec.h
> > @@ -94,6 +94,10 @@ static inline bool kdump_in_progress(void)
> > #ifdef CONFIG_KEXEC_FILE
> > extern struct kexec_file_ops kexec_elf64_ops;
> >
> > +#define arch_kexec_kernel_image_probe arch_kexec_kernel_image_probe
> > +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > + unsigned long buf_len);
> > +
> > #ifdef CONFIG_IMA_KEXEC
> > #define ARCH_HAS_KIMAGE_ARCH
> >
> > diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c b/arch/powerpc/kernel/machine_kexec_file_64.c
> > index 992c0d258e5d..5b7c4a3fbb50 100644
> > --- a/arch/powerpc/kernel/machine_kexec_file_64.c
> > +++ b/arch/powerpc/kernel/machine_kexec_file_64.c
> > @@ -31,8 +31,9 @@
> >
> > #define SLAVE_CODE_SIZE 256
> >
> > -static struct kexec_file_ops *kexec_file_loaders[] = {
> > +struct kexec_file_ops *kexec_file_loaders[] = {
> > &kexec_elf64_ops,
> > + NULL
> > };
> >
> > int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > @@ -45,38 +46,7 @@ int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > if (image->type == KEXEC_TYPE_CRASH)
> > return -ENOTSUPP;
> >
> > - for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
> > - fops = kexec_file_loaders[i];
> > - if (!fops || !fops->probe)
> > - continue;
> > -
> > - ret = fops->probe(buf, buf_len);
> > - if (!ret) {
> > - image->fops = fops;
> > - return ret;
> > - }
> > - }
> > -
> > - return ret;
> > -}
> > -
> > -void *arch_kexec_kernel_image_load(struct kimage *image)
> > -{
> > - if (!image->fops || !image->fops->load)
> > - return ERR_PTR(-ENOEXEC);
> > -
> > - return image->fops->load(image, image->kernel_buf,
> > - image->kernel_buf_len, image->initrd_buf,
> > - image->initrd_buf_len, image->cmdline_buf,
> > - image->cmdline_buf_len);
> > -}
> > -
> > -int arch_kimage_file_post_load_cleanup(struct kimage *image)
> > -{
> > - if (!image->fops || !image->fops->cleanup)
> > - return 0;
> > -
> > - return image->fops->cleanup(image->image_loader_data);
> > + return kexec_kernel_image_probe(image, buf, buf_len);
> > }
> >
> > /**
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index cb0a30473c23..f4df8315d001 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -30,8 +30,9 @@
> > #include <asm/set_memory.h>
> >
> > #ifdef CONFIG_KEXEC_FILE
> > -static struct kexec_file_ops *kexec_file_loaders[] = {
> > +struct kexec_file_ops *kexec_file_loaders[] = {
> > &kexec_bzImage64_ops,
> > + NULL
> > };
> > #endif
> >
> > @@ -361,62 +362,6 @@ void arch_crash_save_vmcoreinfo(void)
> > /* arch-dependent functionality related to kexec file-based syscall */
> >
> > #ifdef CONFIG_KEXEC_FILE
> > -int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > - unsigned long buf_len)
> > -{
> > - int i, ret = -ENOEXEC;
> > - struct kexec_file_ops *fops;
> > -
> > - for (i = 0; i < ARRAY_SIZE(kexec_file_loaders); i++) {
> > - fops = kexec_file_loaders[i];
> > - if (!fops || !fops->probe)
> > - continue;
> > -
> > - ret = fops->probe(buf, buf_len);
> > - if (!ret) {
> > - image->fops = fops;
> > - return ret;
> > - }
> > - }
> > -
> > - return ret;
> > -}
> > -
> > -void *arch_kexec_kernel_image_load(struct kimage *image)
> > -{
> > - vfree(image->arch.elf_headers);
> > - image->arch.elf_headers = NULL;
> > -
> > - if (!image->fops || !image->fops->load)
> > - return ERR_PTR(-ENOEXEC);
> > -
> > - return image->fops->load(image, image->kernel_buf,
> > - image->kernel_buf_len, image->initrd_buf,
> > - image->initrd_buf_len, image->cmdline_buf,
> > - image->cmdline_buf_len);
> > -}
> > -
> > -int arch_kimage_file_post_load_cleanup(struct kimage *image)
> > -{
> > - if (!image->fops || !image->fops->cleanup)
> > - return 0;
> > -
> > - return image->fops->cleanup(image->image_loader_data);
> > -}
> > -
> > -#ifdef CONFIG_KEXEC_VERIFY_SIG
> > -int arch_kexec_kernel_verify_sig(struct kimage *image, void *kernel,
> > - unsigned long kernel_len)
> > -{
> > - if (!image->fops || !image->fops->verify_sig) {
> > - pr_debug("kernel loader does not support signature verification.");
> > - return -EKEYREJECTED;
> > - }
> > -
> > - return image->fops->verify_sig(kernel, kernel_len);
> > -}
> > -#endif
> > -
> > /*
> > * Apply purgatory relocations.
> > *
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index dd056fab9e35..4a2b24d94e04 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -134,6 +134,26 @@ struct kexec_file_ops {
> > #endif
> > };
> >
> > +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> > + unsigned long buf_len);
> > +void *kexec_kernel_image_load(struct kimage *image);
> > +int kexec_kernel_post_load_cleanup(struct kimage *image);
> > +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > + unsigned long buf_len);
> > +
> > +#ifndef arch_kexec_kernel_image_probe
> > +#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
> > +#endif
> > +#ifndef arch_kexec_kernel_image_load
> > +#define arch_kexec_kernel_image_load kexec_kernel_image_load
> > +#endif
> > +#ifndef arch_kimage_file_post_load_cleanup
> > +#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
> > +#endif
> > +#ifndef arch_kexec_kernel_verify_sig
> > +#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
> > +#endif
> > +
> > /**
> > * struct kexec_buf - parameters for finding a place for a buffer in memory
> > * @image: kexec image in which memory to search.
> > @@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
> > size_t crash_get_memory_size(void);
> > void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> >
> > -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > - unsigned long buf_len);
> > -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> > -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
> > -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > - unsigned long buf_len);
>
> I thought we can keep using the __weak function in common code and drop
> the arch functions, no need the #ifndef arch_kexec_kernel_image_probe
> and the function renaming stuff. But I did not notice the powerpc
> _probe function checks KEXEC_ON_CRASH, that should be checked earlier
> and we can fail out early if not supported, but I have no idea
> how to do it gracefully for now.
>
> Also for x86 _load function it cleanups image->arch.elf_headers, it can
> not be dropped simply.

Yeah, arm64 post_load_cleanup function also has some extra stuff.
See my patch #7/8.

> Consider the above two issues, maybe you can keep the powerpc
> version of _probe() and x86 version of _load(), and still copy the common code
> to kexec_file.c weak functions.

It was exactly what I made before submitting v3, but I changed
my mind a bit. My intension is to prevent the code being duplicated
even though it has only a few lines of code.

I agree that '#ifndef arch_kexec_kernel_image_probe' in kexec.h would be
quite ugly, but similar usages can be spotted in the kernel source.

That said if you don't like it at all, I defer to you.

Thanks,
-Takahiro AKASHI

> I can post patch to cleanup the x86
> _load function in the future.

> > int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr,
> > Elf_Shdr *sechdrs, unsigned int relsec);
> > int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index 9f48f4412297..6203b54e04c5 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -26,30 +26,60 @@
> > #include <linux/vmalloc.h>
> > #include "kexec_internal.h"
> >
> > +__weak struct kexec_file_ops *kexec_file_loaders[] = {NULL};
> > +
> > static int kexec_calculate_store_digests(struct kimage *image);
> >
> > -/* Architectures can provide this probe function */
> > -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > - unsigned long buf_len)
> > +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> > + unsigned long buf_len)
> > {
> > - return -ENOEXEC;
> > + int i, ret = -ENOEXEC;
> > + struct kexec_file_ops *fops;
> > +
> > + for (i = 0; ; i++) {
> > + fops = kexec_file_loaders[i];
> > + if (!fops || !fops->probe)
> > + break;
> > +
> > + ret = fops->probe(buf, buf_len);
> > + if (!ret) {
> > + image->fops = fops;
> > + return ret;
> > + }
> > + }
> > +
> > + return ret;
> > }
> >
> > -void * __weak arch_kexec_kernel_image_load(struct kimage *image)
> > +void *kexec_kernel_image_load(struct kimage *image)
> > {
> > - return ERR_PTR(-ENOEXEC);
> > + if (!image->fops || !image->fops->load)
> > + return ERR_PTR(-ENOEXEC);
> > +
> > + return image->fops->load(image, image->kernel_buf,
> > + image->kernel_buf_len, image->initrd_buf,
> > + image->initrd_buf_len, image->cmdline_buf,
> > + image->cmdline_buf_len);
> > }
> >
> > -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
> > +int kexec_kernel_post_load_cleanup(struct kimage *image)
> > {
> > - return -EINVAL;
> > + if (!image->fops || !image->fops->cleanup)
> > + return 0;
> > +
> > + return image->fops->cleanup(image->image_loader_data);
> > }
> >
> > #ifdef CONFIG_KEXEC_VERIFY_SIG
> > -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > - unsigned long buf_len)
> > +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > + unsigned long buf_len)
> > {
> > - return -EKEYREJECTED;
> > + if (!image->fops || !image->fops->verify_sig) {
> > + pr_debug("kernel loader does not support signature verification.\n");
> > + return -EKEYREJECTED;
> > + }
> > +
> > + return image->fops->verify_sig(buf, buf_len);
> > }
> > #endif
> >
> > --
> > 2.14.1
> >
> >
> > _______________________________________________
> > kexec mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/kexec
>
> Thanks
> Dave

2017-09-25 08:03:30

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

HI AKASHI,
On 09/22/17 at 04:58pm, AKASHI Takahiro wrote:
> Hi Dave,
>
[snip]

> > > /*
> > > * Apply purgatory relocations.
> > > *
> > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > index dd056fab9e35..4a2b24d94e04 100644
> > > --- a/include/linux/kexec.h
> > > +++ b/include/linux/kexec.h
> > > @@ -134,6 +134,26 @@ struct kexec_file_ops {
> > > #endif
> > > };
> > >
> > > +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > + unsigned long buf_len);
> > > +void *kexec_kernel_image_load(struct kimage *image);
> > > +int kexec_kernel_post_load_cleanup(struct kimage *image);
> > > +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > + unsigned long buf_len);
> > > +
> > > +#ifndef arch_kexec_kernel_image_probe
> > > +#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
> > > +#endif
> > > +#ifndef arch_kexec_kernel_image_load
> > > +#define arch_kexec_kernel_image_load kexec_kernel_image_load
> > > +#endif
> > > +#ifndef arch_kimage_file_post_load_cleanup
> > > +#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
> > > +#endif
> > > +#ifndef arch_kexec_kernel_verify_sig
> > > +#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
> > > +#endif
> > > +
> > > /**
> > > * struct kexec_buf - parameters for finding a place for a buffer in memory
> > > * @image: kexec image in which memory to search.
> > > @@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
> > > size_t crash_get_memory_size(void);
> > > void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> > >
> > > -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > - unsigned long buf_len);
> > > -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> > > -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
> > > -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > - unsigned long buf_len);
> >
> > I thought we can keep using the __weak function in common code and drop
> > the arch functions, no need the #ifndef arch_kexec_kernel_image_probe
> > and the function renaming stuff. But I did not notice the powerpc
> > _probe function checks KEXEC_ON_CRASH, that should be checked earlier
> > and we can fail out early if not supported, but I have no idea
> > how to do it gracefully for now.
> >
> > Also for x86 _load function it cleanups image->arch.elf_headers, it can
> > not be dropped simply.
>
> Yeah, arm64 post_load_cleanup function also has some extra stuff.
> See my patch #7/8.

But the x86 cleanup was dropped silently, can you add it in x86
post_load_cleanup as well?

>
> > Consider the above two issues, maybe you can keep the powerpc
> > version of _probe() and x86 version of _load(), and still copy the common code
> > to kexec_file.c weak functions.
>
> It was exactly what I made before submitting v3, but I changed
> my mind a bit. My intension is to prevent the code being duplicated
> even though it has only a few lines of code.
>
> I agree that '#ifndef arch_kexec_kernel_image_probe' in kexec.h would be
> quite ugly, but similar usages can be spotted in the kernel source.
>
> That said if you don't like it at all, I defer to you.

I understand your concern, maybe still use a weak function for
arch_kexec_kernel_image_*, and they call the kexec_kernel_image_* in
kexec_file.c common code.

Like in general code:

int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
unsigned long buf_len)
{
return kexec_kernel_image_probe(image, buf, buf_len);
}

In architechture code it can add other code and call
kexec_kernel_image_*

It looks a bit better than the #ifdef way.

[snip]

>
> Thanks,
> -Takahiro AKASHI
>

Thanks
Dave

2017-09-25 08:30:55

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

On 09/25/17 at 04:03pm, Dave Young wrote:
> HI AKASHI,
> On 09/22/17 at 04:58pm, AKASHI Takahiro wrote:
> > Hi Dave,
> >
> [snip]
>
> > > > /*
> > > > * Apply purgatory relocations.
> > > > *
> > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > > index dd056fab9e35..4a2b24d94e04 100644
> > > > --- a/include/linux/kexec.h
> > > > +++ b/include/linux/kexec.h
> > > > @@ -134,6 +134,26 @@ struct kexec_file_ops {
> > > > #endif
> > > > };
> > > >
> > > > +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > > + unsigned long buf_len);
> > > > +void *kexec_kernel_image_load(struct kimage *image);
> > > > +int kexec_kernel_post_load_cleanup(struct kimage *image);
> > > > +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > > + unsigned long buf_len);
> > > > +
> > > > +#ifndef arch_kexec_kernel_image_probe
> > > > +#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
> > > > +#endif
> > > > +#ifndef arch_kexec_kernel_image_load
> > > > +#define arch_kexec_kernel_image_load kexec_kernel_image_load
> > > > +#endif
> > > > +#ifndef arch_kimage_file_post_load_cleanup
> > > > +#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
> > > > +#endif
> > > > +#ifndef arch_kexec_kernel_verify_sig
> > > > +#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
> > > > +#endif
> > > > +
> > > > /**
> > > > * struct kexec_buf - parameters for finding a place for a buffer in memory
> > > > * @image: kexec image in which memory to search.
> > > > @@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
> > > > size_t crash_get_memory_size(void);
> > > > void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> > > >
> > > > -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > > - unsigned long buf_len);
> > > > -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> > > > -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
> > > > -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > > - unsigned long buf_len);
> > >
> > > I thought we can keep using the __weak function in common code and drop
> > > the arch functions, no need the #ifndef arch_kexec_kernel_image_probe
> > > and the function renaming stuff. But I did not notice the powerpc
> > > _probe function checks KEXEC_ON_CRASH, that should be checked earlier
> > > and we can fail out early if not supported, but I have no idea
> > > how to do it gracefully for now.
> > >
> > > Also for x86 _load function it cleanups image->arch.elf_headers, it can
> > > not be dropped simply.
> >
> > Yeah, arm64 post_load_cleanup function also has some extra stuff.
> > See my patch #7/8.
>
> But the x86 cleanup was dropped silently, can you add it in x86
> post_load_cleanup as well?
>
> >
> > > Consider the above two issues, maybe you can keep the powerpc
> > > version of _probe() and x86 version of _load(), and still copy the common code
> > > to kexec_file.c weak functions.
> >
> > It was exactly what I made before submitting v3, but I changed
> > my mind a bit. My intension is to prevent the code being duplicated
> > even though it has only a few lines of code.
> >
> > I agree that '#ifndef arch_kexec_kernel_image_probe' in kexec.h would be
> > quite ugly, but similar usages can be spotted in the kernel source.
> >
> > That said if you don't like it at all, I defer to you.
>
> I understand your concern, maybe still use a weak function for
> arch_kexec_kernel_image_*, and they call the kexec_kernel_image_* in
> kexec_file.c common code.
>
> Like in general code:
>
> int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> unsigned long buf_len)
> {
> return kexec_kernel_image_probe(image, buf, buf_len);

in this way, we maybe move kexec_kernel_image_probe to
_kexec_kernel_image_probe, add a underscore prefix to mean that is used
internally.

> }
>
> In architechture code it can add other code and call
> kexec_kernel_image_*
>
> It looks a bit better than the #ifdef way.
>
> [snip]
>
> >
> > Thanks,
> > -Takahiro AKASHI
> >
>
> Thanks
> Dave
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

2017-09-25 15:36:50

by David Howells

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

AKASHI Takahiro <[email protected]> wrote:

> > > -static struct kexec_file_ops *kexec_file_loaders[] = {
> > > +struct kexec_file_ops *kexec_file_loaders[] = {
> > > &kexec_elf64_ops,
> > > + NULL
> > > };

const?

David

2017-09-25 18:15:45

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

On Mon, Sep 25, 2017 at 04:03:13PM +0800, Dave Young wrote:
> HI AKASHI,
> On 09/22/17 at 04:58pm, AKASHI Takahiro wrote:
> > Hi Dave,
> >
> [snip]
>
> > > > /*
> > > > * Apply purgatory relocations.
> > > > *
> > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > > index dd056fab9e35..4a2b24d94e04 100644
> > > > --- a/include/linux/kexec.h
> > > > +++ b/include/linux/kexec.h
> > > > @@ -134,6 +134,26 @@ struct kexec_file_ops {
> > > > #endif
> > > > };
> > > >
> > > > +int kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > > + unsigned long buf_len);
> > > > +void *kexec_kernel_image_load(struct kimage *image);
> > > > +int kexec_kernel_post_load_cleanup(struct kimage *image);
> > > > +int kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > > + unsigned long buf_len);
> > > > +
> > > > +#ifndef arch_kexec_kernel_image_probe
> > > > +#define arch_kexec_kernel_image_probe kexec_kernel_image_probe
> > > > +#endif
> > > > +#ifndef arch_kexec_kernel_image_load
> > > > +#define arch_kexec_kernel_image_load kexec_kernel_image_load
> > > > +#endif
> > > > +#ifndef arch_kimage_file_post_load_cleanup
> > > > +#define arch_kimage_file_post_load_cleanup kexec_kernel_post_load_cleanup
> > > > +#endif
> > > > +#ifndef arch_kexec_kernel_verify_sig
> > > > +#define arch_kexec_kernel_verify_sig kexec_kernel_verify_sig
> > > > +#endif
> > > > +
> > > > /**
> > > > * struct kexec_buf - parameters for finding a place for a buffer in memory
> > > > * @image: kexec image in which memory to search.
> > > > @@ -276,12 +296,6 @@ int crash_shrink_memory(unsigned long new_size);
> > > > size_t crash_get_memory_size(void);
> > > > void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> > > >
> > > > -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> > > > - unsigned long buf_len);
> > > > -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> > > > -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image);
> > > > -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> > > > - unsigned long buf_len);
> > >
> > > I thought we can keep using the __weak function in common code and drop
> > > the arch functions, no need the #ifndef arch_kexec_kernel_image_probe
> > > and the function renaming stuff. But I did not notice the powerpc
> > > _probe function checks KEXEC_ON_CRASH, that should be checked earlier
> > > and we can fail out early if not supported, but I have no idea
> > > how to do it gracefully for now.
> > >
> > > Also for x86 _load function it cleanups image->arch.elf_headers, it can
> > > not be dropped simply.
> >
> > Yeah, arm64 post_load_cleanup function also has some extra stuff.
> > See my patch #7/8.
>
> But the x86 cleanup was dropped silently, can you add it in x86
> post_load_cleanup as well?

Sure, I will do.

> >
> > > Consider the above two issues, maybe you can keep the powerpc
> > > version of _probe() and x86 version of _load(), and still copy the common code
> > > to kexec_file.c weak functions.
> >
> > It was exactly what I made before submitting v3, but I changed
> > my mind a bit. My intension is to prevent the code being duplicated
> > even though it has only a few lines of code.
> >
> > I agree that '#ifndef arch_kexec_kernel_image_probe' in kexec.h would be
> > quite ugly, but similar usages can be spotted in the kernel source.
> >
> > That said if you don't like it at all, I defer to you.
>
> I understand your concern, maybe still use a weak function for
> arch_kexec_kernel_image_*, and they call the kexec_kernel_image_* in
> kexec_file.c common code.
>
> Like in general code:
>
> int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> unsigned long buf_len)
> {
> return kexec_kernel_image_probe(image, buf, buf_len);

as you suggested,
"return _kexec_kernel_image_probe(...);

would be fine.

-Takahiro AKASHI


> }
>
> In architechture code it can add other code and call
> kexec_kernel_image_*
>
> It looks a bit better than the #ifdef way.
>
> [snip]
>
> >
> > Thanks,
> > -Takahiro AKASHI
> >
>
> Thanks
> Dave

2017-09-25 18:18:21

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

On Mon, Sep 25, 2017 at 04:36:43PM +0100, David Howells wrote:
> AKASHI Takahiro <[email protected]> wrote:
>
> > > > -static struct kexec_file_ops *kexec_file_loaders[] = {
> > > > +struct kexec_file_ops *kexec_file_loaders[] = {
> > > > &kexec_elf64_ops,
> > > > + NULL
> > > > };
>
> const?

Yes, it makes sense.

-Takahiro AKASHI

>
> David