devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v16 00/16] arm64: kexec: add kexec_file_load() support
@ 2018-11-15  5:52 AKASHI Takahiro
  2018-11-15  5:52 ` [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property AKASHI Takahiro
  2018-11-15  5:52 ` [PATCH v16 12/16] arm64: kexec_file: add crash dump support AKASHI Takahiro
  0 siblings, 2 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-11-15  5:52 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, dhowells, vgoyal, herbert, davem,
	dyoung, bhe, arnd, schwidefsky, heiko.carstens, robh+dt,
	frowand.list
  Cc: prudo, ard.biesheuvel, james.morse, bhsharma, kexec,
	linux-arm-kernel, linux-kernel, AKASHI Takahiro, devicetree

This is the sixteenth round of implementing kexec_file_load() support
on arm64.[1] (See "Changes" below)
Most of the code is based on kexec-tools.

This patch series enables us to
  * load the kernel by specifying its file descriptor, instead of user-
    filled buffer, at kexec_file_load() system call, and
  * optionally verify its signature at load time for trusted boot.
Kernel virtual address randomization is also supported since v9.

Contrary to kexec_load() system call, as we discussed a long time ago,
users may not be allowed to provide a device tree to the 2nd kernel
explicitly, hence enforcing a dt blob of the first kernel to be re-used
internally.

To use kexec_file_load() system call, instead of kexec_load(), at kexec
command, '-s' option must be specified. See [2] for a necessary patch for
kexec-tools.

To analyze a generated crash dump file, use the latest master branch of
crash utility[3]. I always try to submit patches to fix any inconsistencies
introduced in the latest kernel.

Regarding a kernel image verification, a signature must be presented
along with the binary itself. A signature is basically a hash value
calculated against the whole binary data and encrypted by a key which
will be authenticated by one of the system's trusted certificates.
Any attempt to read and load a to-be-kexec-ed kernel image through
a system call will be checked and blocked if the binary's hash value
doesn't match its associated signature.

There are two methods available now:
1. implementing arch-specific verification hook of kexec_file_load()
2. utilizing IMA(Integrity Measurement Architecture)[4] appraisal framework

Before my v7, I believed that my patch only supports (1) but am now
confident that (2) comes free if IMA is enabled and properly configured.


(1) Arch-specific verification hook
If CONFIG_KEXEC_VERIFY_SIG is enabled, kexec_file_load() invokes an arch-
defined (and hence file-format-specific) hook function to check for the
validity of kernel binary.

On x86, a signature is embedded into a PE file (Microsoft's format) header
of binary. Since arm64's "Image" can also be seen as a PE file as far as
CONFIG_EFI is enabled, we adopt this format for kernel signing.  

As in the case of UEFI applications, we can create a signed kernel image:
    $ sbsign --key ${KEY} --cert ${CERT} Image

You may want to use certs/signing_key.pem, which is intended to be used
for module signing (CONFIG_MODULE_SIG), as ${KEY} and ${CERT} for test
purpose.


(2) IMA appraisal-based
IMA was first introduced in linux in order to meet TCG (Trusted Computing
Group) requirement that all the sensitive files be *measured* before
reading/executing them to detect any untrusted changes/modification.
Then appraisal feature, which allows us to ensure the integrity of
files and even prevent them from reading/executing, was added later.

Meanwhile, kexec_file_load() has been merged since v3.17 and evolved to
enable IMA-appraisal type verification by the commit b804defe4297 ("kexec:
replace call to copy_file_from_fd() with kernel version").

In this scheme, a signature will be stored in a extended file attribute,
"security.ima" while a decryption key is hold in a dedicated keyring,
".ima" or "_ima".  All the necessary process of verification is confined
in a secure API, kernel_read_file_from_fd(), called by kexec_file_load().

    Please note that powerpc is one of the two architectures now
    supporting KEXEC_FILE, and that it wishes to extend IMA,
    where a signature may be appended to "vmlinux" file[5], like module
    signing, instead of using an extended file attribute.

While IMA meant to be used with TPM (Trusted Platform Module) on secure
platform, IMA is still usable without TPM. Here is an example procedure
about how we can give it a try to run the feature using a self-signed
root ca for demo/test purposes:

 1) Generate needed keys and certificates, following "Generate trusted
    keys" section in README of ima-evm-utils[6].

 2) Build the kernel with the following kernel configurations, specifying
    "ima-local-ca.pem" for CONFIG_SYSTEM_TRUSTED_KEYS:
	CONFIG_EXT4_FS_SECURITY
	CONFIG_INTEGRITY_SIGNATURE
	CONFIG_INTEGRITY_ASYMMETRIC_KEYS
	CONFIG_INTEGRITY_TRUSTED_KEYRING
	CONFIG_IMA
	CONFIG_IMA_WRITE_POLICY
	CONFIG_IMA_READ_POLICY
	CONFIG_IMA_APPRAISE
	CONFIG_IMA_APPRAISE_BOOTPARAM
	CONFIG_SYSTEM_TRUSTED_KEYS
    Please note that CONFIG_KEXEC_VERIFY_SIG is not, actually should
    not be, enabled.

 3) Sign(label) a kernel image binary to be kexec-ed on target filesystem:
    $ evmctl ima_sign --key /path/to/private_key.pem /your/Image

 4) Add a command line parameter and boot the kernel:
    ima_appraise=enforce

 On live system,
 5) Set a security policy:
    $ mount -t securityfs none /sys/kernel/security
    $ echo "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig" \
      > /sys/kernel/security/ima/policy

 6) Add a key for ima:
    $ keyctl padd asymmetric my_ima_key %:.ima < /path/to/x509_ima.der
    (or evmctl import /path/to/x509_ima.der <ima_keyring_id>)

 7) Then try kexec as normal.


Concerns(or future works):
* Support for physical address randomization
* Signature verification of big endian kernel with CONFIG_KEXEC_VERIFY_SIG
  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.
* Support for vminux loading

  [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://github.com/crash-utility/crash.git
  [4] https://sourceforge.net/p/linux-ima/wiki/Home/
  [5] http://lkml.iu.edu//hypermail/linux/kernel/1707.0/03669.html
  [6] https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/master/tree/


Changes in v16 (Nov 15, 2018)
* rebased to v4.20-rc
* remove CONFIG_HAVE_MEMBLOCK dependency from kexec_locate_mem_hole()
* drop fdt_prop_len()
* rename fill_property() to cpu64_to_fdt_cells(), adding range check to it
* add lib/fdt_addresses.c which comes form libfdt
* move fdt_setprop_reg() from drivers/of/fdt.c to lib/fdt_addresses.c
  (Patch #6 was originally sent out under the name, "of/fdt: add helper
  functions for handling properties")
* modify struct arm64_image_header definition, moving it and related
  HEAD_FLAG_* definitions to new asm/image.h
* rework setup_dtb(), which is renamed to create_dtb(), not to reply on
  fdt_prop_len()
* some cleanup changes in image_load()

Changes in v15 (Sep 28, 2018)
* rework fdt helper functions, utilizing new fdt_address_cells() and
  fdt_size_cells() from libfdt after Frank's comment
  This change requires the following patches to be applied as well:
  [7] http://lkml.iu.edu//hypermail/linux/kernel/1809.1/04800.html
  [8] https://www.spinics.net/lists/devicetree/msg250653.html

Changes in v14 (Sep 7, 2018)
* rebased to v4.19-rc
* define default KEXEC_BUF_MEM_UNKNOWN which indicates that,
  if kexec_buf.mem does match it, kexec_locate_mem_hole() should allocate
  free memory for arch

Changes in v13 (July 31, 2018)
* rebased to arm64/for-next/core
* system call number changed to 294
  (kexec-tools must be updated as well.)
* rename arch_kexec_walk_mem() to kexec_walk_resources()
* define string macros for dtb properties' names in setup_dtb()
* use MEMBLOCK_NONE rather than 0 at for_each_mem_range[_reverse]()
* rename dtb_buf to dtb in struct kexec_arch at right place
* provide "kaslr-seed" only if random number generator is available

Changes in v12 (July 24, 2018)
 (mostly addressing James' comments)
* unify all the variants of arch_kexec_walk_mem(), including s390's, into
  common code, leaving arch_kexec_walk_mem() static (i.e. no longer
  replaceable)
* always initialize kbuf.mem to zero to align with a change above
* set kbuf.buf_min/buf_max consistently between kexec and kdump
* try to consistently use "unsigned long" for physical (kexec-time)
  address, and "void *" for virtual (runtime) address in
  load_other_segments() with a couple of variables renamed for readability
* fix a 'sparse' warning against arch_kimage_file_post_load_cleanup()
* fix a calculation of string length of "ARM64_MAGIC"
* set kernel image alignment to MIN_KIMG_ALIGN rather than SZ_2M
* set elf header alignment to SZ_64K rather than SZ_4K


Changes in v11 (July 11, 2018)
* split v10's patch#3, a refactoring stuff, into two parts, "just move"
  and modify
* remove selecting BUILD_BIN2C from KEXEC_FILE config
* modify setup_dtb()
   * to correct a return value on failure of fdt_xyz() call,
   * to always remove existing bootargs and initrd-start/end properties,
     if any, when copying current system's dtb into new dtb
   * to use fdt_setprop_string() for bootargs (I'm now sure that
     kimage->cmdline_buf is a null-terminated string.)
* revise a warning comment in case of KEXEC_VERIFY_SIG but
  !(EFI && SIGNED_PE_FILE_VERIFICATION)

Changes in v10 (June 23, 2018)
* rebased to v4.18-rc
* change syscall number of kexec_file_load from 292 to 293
* factor out memblock-based arch_kexec_walk_mem() from powerpc and
  merge it into generic one
* move generic fdt helper functions from arm64 dir to drivers/of
  (dt_root_[addr|size]_cells are no longer __initdata.)
* modify fill_property() to use 'while' loop
* modify fdt_setprop_reg() to allocate a buffer on stack
* modify setup_dtb() to use fdt_setprop_u64()
* pass kernel_load_addr/size directly as arguments, instead of via
  kimage_arch.kern_segment, at load_other_segments()
* refuse loading an image which cannot be supported in image loader,
  adding cpu-feature(MMFR0) helper functions
* modify prepare_elf_headers() to use kmalloc() instead of vmalloc()
* always pass arch.dtb_mem as the fourth argument to cpu_soft_restart()
  in machine_kexec() while dtb_mem will be zero in kexec case

Changes in v9 (April 25, 2018)
* rebased to v4.17-rc
* remove preparatory patches on generic/x86/ppc code
  They have now been merged in v4.17-rc1.
* allocate memory based on memblock list instead of system resources
  This will prevent reserved regions, particularly UEFI/ACPI data,
  from being corrupted.
* correct dt property names, linux,initrd-*, in newly-created dtb
  "linux," was missing.
* remove alignment requirement for initrd loading
* add kaslr (kernel virtual address randomization) support
* misc code clean-up
* revise commit messages

Changes in v8 (Feb 22, 2018)
* introduce ARCH_HAS_KEXEC_PURGATORY so that arm64 will be able to skip
  purgatory
* remove "ifdef CONFIG_X86_64" stuffs from a re-factored function,
  prepare_elf64_headers(), making its interface more generic
  (The original patch was split into two for easier reviews.)
* modify cpu_soft_restart() so as to let the 2nd kernel jump into its entry
  code directly without requiring purgatory in case of kexec_file_load
* remove CONFIG_KEXEC_FILE_IMAGE_FMT and introduce
  CONFIG_KEXEC_IMAGE_VERIFY_SIG, much similar to x86 but quite redundant
  for now.
* In addition, update/modify dependencies of KEXEC_IMAGE_VERIFY_SIG

Changes in v7 (Dec 4, 2017)
* rebased to v4.15-rc2
* re-organize the patch set to separate KEXEC_FILE_VERIFY_SIG-related
  code from the others
* revamp factored-out code in kernel/kexec_file.c due to the changes
  in original x86 code
* redefine walk_sys_ram_res_rev() prototype due to change of callback
  type in the counterpart, walk_sys_ram_res()
* make KEXEC_FILE_IMAGE_FMT default on if KEXEC_FILE selected

Changes in v6 (Oct 24, 2017)
* fix a for-loop bug in _kexec_kernel_image_probe() per Julien

Changes in v5 (Oct 10, 2017)
* fix kbuild errors around patch #3
per Julien's comments,
* fix a bug in walk_system_ram_res_rev() with some cleanup
* modify fdt_setprop_range() to use vmalloc()
* modify fill_property() to use memset()

Changes in v4 (Oct 2, 2017)
* reinstate x86's arch_kexec_kernel_image_load()
* rename weak arch_kexec_kernel_xxx() to _kexec_kernel_xxx() for
  better re-use
* constify kexec_file_loaders[]

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 #10 are essential part for KEXEC_FILE support
(additionally allowing for IMA-based verification):
  Patch #1 to #6 are all preparatory patches on generic side.
  Patch #7 to #11 are to enable kexec_file_load on arm64.

Patch #12 to #13 are for KEXEC_VERIFY_SIG (arch-specific verification)
support
AKASHI Takahiro (16):
  asm-generic: add kexec_file_load system call to unistd.h
  kexec_file: make kexec_image_post_load_cleanup_default() global
  s390, kexec_file: drop arch_kexec_mem_walk()
  powerpc, kexec_file: factor out memblock-based arch_kexec_walk_mem()
  kexec_file: kexec_walk_memblock() only walks a dedicated region at
    kdump
  lib: fdt: add a helper function for handling memory range property
  arm64: add image head flag definitions
  arm64: cpufeature: add MMFR0 helper functions
  arm64: enable KEXEC_FILE config
  arm64: kexec_file: load initrd and device-tree
  arm64: kexec_file: allow for loading Image-format kernel
  arm64: kexec_file: add crash dump support
  arm64: kexec_file: invoke the kernel without purgatory
  include: pe.h: remove message[] from mz header definition
  arm64: kexec_file: add kernel signature verification support
  arm64: kexec_file: add kaslr support

 arch/arm64/Kconfig                          |  33 ++
 arch/arm64/include/asm/cpufeature.h         |  48 +++
 arch/arm64/include/asm/image.h              |  59 ++++
 arch/arm64/include/asm/kexec.h              |  23 ++
 arch/arm64/kernel/Makefile                  |   3 +-
 arch/arm64/kernel/cpu-reset.S               |   8 +-
 arch/arm64/kernel/head.S                    |   3 +-
 arch/arm64/kernel/image.h                   |  21 +-
 arch/arm64/kernel/kexec_image.c             | 130 ++++++++
 arch/arm64/kernel/machine_kexec.c           |  12 +-
 arch/arm64/kernel/machine_kexec_file.c      | 337 ++++++++++++++++++++
 arch/arm64/kernel/relocate_kernel.S         |   3 +-
 arch/powerpc/kernel/machine_kexec_file_64.c |  54 ----
 arch/s390/kernel/machine_kexec_file.c       |  10 -
 include/linux/kexec.h                       |  11 +-
 include/linux/libfdt.h                      |  26 ++
 include/linux/pe.h                          |   2 +-
 include/uapi/asm-generic/unistd.h           |   4 +-
 kernel/kexec_file.c                         |  70 +++-
 lib/Makefile                                |   2 +-
 lib/fdt_addresses.c                         |  56 ++++
 21 files changed, 824 insertions(+), 91 deletions(-)
 create mode 100644 arch/arm64/include/asm/image.h
 create mode 100644 arch/arm64/kernel/kexec_image.c
 create mode 100644 arch/arm64/kernel/machine_kexec_file.c
 create mode 100644 lib/fdt_addresses.c

-- 
2.19.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-11-15  5:52 [PATCH v16 00/16] arm64: kexec: add kexec_file_load() support AKASHI Takahiro
@ 2018-11-15  5:52 ` AKASHI Takahiro
  2018-11-30 13:21   ` Will Deacon
  2018-12-06 14:47   ` Rob Herring
  2018-11-15  5:52 ` [PATCH v16 12/16] arm64: kexec_file: add crash dump support AKASHI Takahiro
  1 sibling, 2 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-11-15  5:52 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, dhowells, vgoyal, herbert, davem,
	dyoung, bhe, arnd, schwidefsky, heiko.carstens
  Cc: prudo, ard.biesheuvel, james.morse, bhsharma, kexec,
	linux-arm-kernel, linux-kernel, AKASHI Takahiro, Rob Herring,
	Frank Rowand, devicetree

Added function, fdt_setprop_reg(), will be used later to handle
kexec-specific property in arm64's kexec_file implementation.
It will possibly be merged into libfdt in the future.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
---
 include/linux/libfdt.h | 26 ++++++++++++++++++++
 lib/Makefile           |  2 +-
 lib/fdt_addresses.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 lib/fdt_addresses.c

diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h
index 90ed4ebfa692..47c4dc9e135c 100644
--- a/include/linux/libfdt.h
+++ b/include/linux/libfdt.h
@@ -5,4 +5,30 @@
 #include <linux/libfdt_env.h>
 #include "../../scripts/dtc/libfdt/libfdt.h"
 
+/**
+ * fdt_setprop_reg - add/set a memory region property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node to add a property at
+ * @name: name of property
+ * @addr: physical start address
+ * @size: size of region
+ *
+ * returns:
+ *	0, on success
+ *      -FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid
+ *		#address-cells property
+ *      -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADVALUE, addr or size doesn't fit to respective cells size
+ *      -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ *              contain a new property
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
+					       u64 addr, u64 size);
+
 #endif /* _INCLUDE_LIBFDT_H_ */
diff --git a/lib/Makefile b/lib/Makefile
index db06d1237898..2a96cb05e15d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -205,7 +205,7 @@ KASAN_SANITIZE_stackdepot.o := n
 KCOV_INSTRUMENT_stackdepot.o := n
 
 libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
-	       fdt_empty_tree.o
+	       fdt_empty_tree.o fdt_addresses.o
 $(foreach file, $(libfdt_files), \
 	$(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt))
 lib-$(CONFIG_LIBFDT) += $(libfdt_files)
diff --git a/lib/fdt_addresses.c b/lib/fdt_addresses.c
new file mode 100644
index 000000000000..97ddd5a5cc10
--- /dev/null
+++ b/lib/fdt_addresses.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/libfdt_env.h>
+#include <linux/types.h>
+#include "../scripts/dtc/libfdt/fdt_addresses.c"
+
+/*
+ * helper functions for arm64 kexec
+ * Those functions may be merged into libfdt in the future.
+ */
+
+/* This function assumes that cells is 1 or 2 */
+static void cpu64_to_fdt_cells(void *buf, u64 val64, int cells)
+{
+	__be32 val32;
+
+	while (cells) {
+		val32 = cpu_to_fdt32(val64 >> (32 * (--cells)));
+		memcpy(buf, &val32, sizeof(val32));
+		buf += sizeof(val32);
+	}
+}
+
+int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
+						u64 addr, u64 size)
+{
+	int addr_cells, size_cells;
+	char buf[sizeof(__be32) * 2 * 2];
+		/* assume dt_root_[addr|size]_cells <= 2 */
+	void *prop;
+	size_t buf_size;
+
+	addr_cells = fdt_address_cells(fdt, 0);
+	if (addr_cells < 0)
+		return addr_cells;
+	size_cells = fdt_size_cells(fdt, 0);
+	if (size_cells < 0)
+		return size_cells;
+
+	/* if *_cells >= 2, cells can hold 64-bit values anyway */
+	if ((addr_cells == 1) && ((addr > U32_MAX) ||
+				  ((addr + size) > U32_MAX)))
+		return -FDT_ERR_BADVALUE;
+
+	if ((size_cells == 1) && (size > U32_MAX))
+		return -FDT_ERR_BADVALUE;
+
+	buf_size = (addr_cells + size_cells) * sizeof(u32);
+	prop = buf;
+
+	cpu64_to_fdt_cells(prop, addr, addr_cells);
+	prop += addr_cells * sizeof(u32);
+
+	cpu64_to_fdt_cells(prop, size, size_cells);
+
+	return fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
+}
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v16 12/16] arm64: kexec_file: add crash dump support
  2018-11-15  5:52 [PATCH v16 00/16] arm64: kexec: add kexec_file_load() support AKASHI Takahiro
  2018-11-15  5:52 ` [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property AKASHI Takahiro
@ 2018-11-15  5:52 ` AKASHI Takahiro
  1 sibling, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-11-15  5:52 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, dhowells, vgoyal, herbert, davem,
	dyoung, bhe, arnd, schwidefsky, heiko.carstens
  Cc: prudo, ard.biesheuvel, james.morse, bhsharma, kexec,
	linux-arm-kernel, linux-kernel, AKASHI Takahiro, Rob Herring,
	Frank Rowand, devicetree

Enabling crash dump (kdump) includes
* prepare contents of ELF header of a core dump file, /proc/vmcore,
  using crash_prepare_elf64_headers(), and
* add two device tree properties, "linux,usable-memory-range" and
  "linux,elfcorehdr", which represent respectively a memory range
  to be used by crash dump kernel and the header's location

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Reviewed-by: James Morse <james.morse@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
---
 arch/arm64/include/asm/kexec.h         |  4 ++
 arch/arm64/kernel/machine_kexec_file.c | 97 +++++++++++++++++++++++++-
 2 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 67e4cb75d1fd..660705515e33 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -99,6 +99,10 @@ static inline void crash_post_resume(void) {}
 struct kimage_arch {
 	void *dtb;
 	unsigned long dtb_mem;
+	/* Core ELF header buffer */
+	void *elf_headers;
+	unsigned long elf_headers_mem;
+	unsigned long elf_headers_sz;
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 7e9d5ed3e238..ab296b98d633 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -16,11 +16,15 @@
 #include <linux/libfdt.h>
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
+#include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/vmalloc.h>
 #include <asm/byteorder.h>
 
 /* relevant device tree properties */
+#define FDT_PSTR_KEXEC_ELFHDR	"linux,elfcorehdr"
+#define FDT_PSTR_MEM_RANGE	"linux,usable-memory-range"
 #define FDT_PSTR_INITRD_STA	"linux,initrd-start"
 #define FDT_PSTR_INITRD_END	"linux,initrd-end"
 #define FDT_PSTR_BOOTARGS	"bootargs"
@@ -35,6 +39,10 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
 	vfree(image->arch.dtb);
 	image->arch.dtb = NULL;
 
+	vfree(image->arch.elf_headers);
+	image->arch.elf_headers = NULL;
+	image->arch.elf_headers_sz = 0;
+
 	return kexec_image_post_load_cleanup_default(image);
 }
 
@@ -49,6 +57,22 @@ static int setup_dtb(struct kimage *image,
 	if (nodeoffset < 0)
 		return -EINVAL;
 
+	if (image->type == KEXEC_TYPE_CRASH) {
+		/* add linux,elfcorehdr */
+		ret = fdt_setprop_reg(dtb, nodeoffset, FDT_PSTR_KEXEC_ELFHDR,
+				image->arch.elf_headers_mem,
+				image->arch.elf_headers_sz);
+		if (ret)
+			return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
+
+		/* add linux,usable-memory-range */
+		ret = fdt_setprop_reg(dtb, nodeoffset, FDT_PSTR_MEM_RANGE,
+				crashk_res.start,
+				crashk_res.end - crashk_res.start + 1);
+		if (ret)
+			return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
+	}
+
 	/* add bootargs */
 	if (cmdline) {
 		ret = fdt_setprop_string(dtb, nodeoffset, FDT_PSTR_BOOTARGS,
@@ -86,7 +110,8 @@ static int setup_dtb(struct kimage *image,
 }
 
 /*
- * More space needed so that we can add initrd and bootargs.
+ * More space needed so that we can add initrd, bootargs,
+ * userable-memory-range and elfcorehdr.
  */
 #define DTB_EXTRA_SPACE 0x1000
 
@@ -132,6 +157,43 @@ static int create_dtb(struct kimage *image,
 	}
 }
 
+static int prepare_elf_headers(void **addr, unsigned long *sz)
+{
+	struct crash_mem *cmem;
+	unsigned int nr_ranges;
+	int ret;
+	u64 i;
+	phys_addr_t start, end;
+
+	nr_ranges = 1; /* for exclusion of crashkernel region */
+	for_each_mem_range(i, &memblock.memory, NULL, NUMA_NO_NODE,
+					MEMBLOCK_NONE, &start, &end, NULL)
+		nr_ranges++;
+
+	cmem = kmalloc(sizeof(struct crash_mem) +
+			sizeof(struct crash_mem_range) * nr_ranges, GFP_KERNEL);
+	if (!cmem)
+		return -ENOMEM;
+
+	cmem->max_nr_ranges = nr_ranges;
+	cmem->nr_ranges = 0;
+	for_each_mem_range(i, &memblock.memory, NULL, NUMA_NO_NODE,
+					MEMBLOCK_NONE, &start, &end, NULL) {
+		cmem->ranges[cmem->nr_ranges].start = start;
+		cmem->ranges[cmem->nr_ranges].end = end - 1;
+		cmem->nr_ranges++;
+	}
+
+	/* Exclude crashkernel region */
+	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
+
+	if (!ret)
+		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
+
+	kfree(cmem);
+	return ret;
+}
+
 int load_other_segments(struct kimage *image,
 			unsigned long kernel_load_addr,
 			unsigned long kernel_size,
@@ -139,14 +201,43 @@ int load_other_segments(struct kimage *image,
 			char *cmdline)
 {
 	struct kexec_buf kbuf;
-	void *dtb = NULL;
-	unsigned long initrd_load_addr = 0, dtb_len;
+	void *headers, *dtb = NULL;
+	unsigned long headers_sz, initrd_load_addr = 0, dtb_len;
 	int ret = 0;
 
 	kbuf.image = image;
 	/* not allocate anything below the kernel */
 	kbuf.buf_min = kernel_load_addr + kernel_size;
 
+	/* load elf core header */
+	if (image->type == KEXEC_TYPE_CRASH) {
+		ret = prepare_elf_headers(&headers, &headers_sz);
+		if (ret) {
+			pr_err("Preparing elf core header failed\n");
+			goto out_err;
+		}
+
+		kbuf.buffer = headers;
+		kbuf.bufsz = headers_sz;
+		kbuf.mem = 0;
+		kbuf.memsz = headers_sz;
+		kbuf.buf_align = SZ_64K; /* largest supported page size */
+		kbuf.buf_max = ULONG_MAX;
+		kbuf.top_down = true;
+
+		ret = kexec_add_buffer(&kbuf);
+		if (ret) {
+			vfree(headers);
+			goto out_err;
+		}
+		image->arch.elf_headers = headers;
+		image->arch.elf_headers_mem = kbuf.mem;
+		image->arch.elf_headers_sz = headers_sz;
+
+		pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+			 image->arch.elf_headers_mem, headers_sz, headers_sz);
+	}
+
 	/* load initrd */
 	if (initrd) {
 		kbuf.buffer = initrd;
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-11-15  5:52 ` [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property AKASHI Takahiro
@ 2018-11-30 13:21   ` Will Deacon
  2018-12-06 14:47   ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Will Deacon @ 2018-11-30 13:21 UTC (permalink / raw)
  To: AKASHI Takahiro, mark.rutland, robh+dt, frowand.list
  Cc: catalin.marinas, dhowells, vgoyal, herbert, davem, dyoung, bhe,
	arnd, schwidefsky, heiko.carstens, prudo, ard.biesheuvel,
	james.morse, bhsharma, kexec, linux-arm-kernel, linux-kernel,
	devicetree

[moving some DT people to TO:]

On Thu, Nov 15, 2018 at 02:52:45PM +0900, AKASHI Takahiro wrote:
> Added function, fdt_setprop_reg(), will be used later to handle
> kexec-specific property in arm64's kexec_file implementation.
> It will possibly be merged into libfdt in the future.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> ---
>  include/linux/libfdt.h | 26 ++++++++++++++++++++
>  lib/Makefile           |  2 +-
>  lib/fdt_addresses.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 lib/fdt_addresses.c

We need an ack from the DT side before we can merge this series, please.

Will

> diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h
> index 90ed4ebfa692..47c4dc9e135c 100644
> --- a/include/linux/libfdt.h
> +++ b/include/linux/libfdt.h
> @@ -5,4 +5,30 @@
>  #include <linux/libfdt_env.h>
>  #include "../../scripts/dtc/libfdt/libfdt.h"
>  
> +/**
> + * fdt_setprop_reg - add/set a memory region property
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node to add a property at
> + * @name: name of property
> + * @addr: physical start address
> + * @size: size of region
> + *
> + * returns:
> + *	0, on success
> + *      -FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid
> + *		#address-cells property
> + *      -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADVALUE, addr or size doesn't fit to respective cells size
> + *      -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
> + *              contain a new property
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
> +					       u64 addr, u64 size);
> +
>  #endif /* _INCLUDE_LIBFDT_H_ */
> diff --git a/lib/Makefile b/lib/Makefile
> index db06d1237898..2a96cb05e15d 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -205,7 +205,7 @@ KASAN_SANITIZE_stackdepot.o := n
>  KCOV_INSTRUMENT_stackdepot.o := n
>  
>  libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
> -	       fdt_empty_tree.o
> +	       fdt_empty_tree.o fdt_addresses.o
>  $(foreach file, $(libfdt_files), \
>  	$(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt))
>  lib-$(CONFIG_LIBFDT) += $(libfdt_files)
> diff --git a/lib/fdt_addresses.c b/lib/fdt_addresses.c
> new file mode 100644
> index 000000000000..97ddd5a5cc10
> --- /dev/null
> +++ b/lib/fdt_addresses.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/libfdt_env.h>
> +#include <linux/types.h>
> +#include "../scripts/dtc/libfdt/fdt_addresses.c"
> +
> +/*
> + * helper functions for arm64 kexec
> + * Those functions may be merged into libfdt in the future.
> + */
> +
> +/* This function assumes that cells is 1 or 2 */
> +static void cpu64_to_fdt_cells(void *buf, u64 val64, int cells)
> +{
> +	__be32 val32;
> +
> +	while (cells) {
> +		val32 = cpu_to_fdt32(val64 >> (32 * (--cells)));
> +		memcpy(buf, &val32, sizeof(val32));
> +		buf += sizeof(val32);
> +	}
> +}
> +
> +int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
> +						u64 addr, u64 size)
> +{
> +	int addr_cells, size_cells;
> +	char buf[sizeof(__be32) * 2 * 2];
> +		/* assume dt_root_[addr|size]_cells <= 2 */
> +	void *prop;
> +	size_t buf_size;
> +
> +	addr_cells = fdt_address_cells(fdt, 0);
> +	if (addr_cells < 0)
> +		return addr_cells;
> +	size_cells = fdt_size_cells(fdt, 0);
> +	if (size_cells < 0)
> +		return size_cells;
> +
> +	/* if *_cells >= 2, cells can hold 64-bit values anyway */
> +	if ((addr_cells == 1) && ((addr > U32_MAX) ||
> +				  ((addr + size) > U32_MAX)))
> +		return -FDT_ERR_BADVALUE;
> +
> +	if ((size_cells == 1) && (size > U32_MAX))
> +		return -FDT_ERR_BADVALUE;
> +
> +	buf_size = (addr_cells + size_cells) * sizeof(u32);
> +	prop = buf;
> +
> +	cpu64_to_fdt_cells(prop, addr, addr_cells);
> +	prop += addr_cells * sizeof(u32);
> +
> +	cpu64_to_fdt_cells(prop, size, size_cells);
> +
> +	return fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
> +}
> -- 
> 2.19.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-11-15  5:52 ` [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property AKASHI Takahiro
  2018-11-30 13:21   ` Will Deacon
@ 2018-12-06 14:47   ` Rob Herring
  2018-12-06 15:54     ` Will Deacon
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2018-12-06 14:47 UTC (permalink / raw)
  To: AKASHI, Takahiro
  Cc: Catalin Marinas, Will Deacon, David Howells, Vivek Goyal,
	Herbert Xu, David Miller, dyoung, Baoquan He, Arnd Bergmann,
	Martin Schwidefsky, Heiko Carstens, prudo, Ard Biesheuvel,
	James Morse, bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree

On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Added function, fdt_setprop_reg(), will be used later to handle
> kexec-specific property in arm64's kexec_file implementation.
> It will possibly be merged into libfdt in the future.

You generally can't modify libfdt files. Any changes will be blown
away with the next dtc sync (there's one in -next now). Though here
you are creating a new location with fdt code. lib/ is just a shim to
the actual libfdt code. Don't put any implementation there. You can
add this to drivers/of/fdt_address.c for the short term, but it still
needs to go upstream.

Otherwise, the implementation looks fine to me.

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> ---
>  include/linux/libfdt.h | 26 ++++++++++++++++++++
>  lib/Makefile           |  2 +-
>  lib/fdt_addresses.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 lib/fdt_addresses.c

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-12-06 14:47   ` Rob Herring
@ 2018-12-06 15:54     ` Will Deacon
  2018-12-07 10:12       ` James Morse
  0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2018-12-06 15:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: AKASHI, Takahiro, Catalin Marinas, David Howells, Vivek Goyal,
	Herbert Xu, David Miller, dyoung, Baoquan He, Arnd Bergmann,
	Martin Schwidefsky, Heiko Carstens, prudo, Ard Biesheuvel,
	James Morse, bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree@

Hi Rob,

Thanks for reviewing this.

On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote:
> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Added function, fdt_setprop_reg(), will be used later to handle
> > kexec-specific property in arm64's kexec_file implementation.
> > It will possibly be merged into libfdt in the future.
> 
> You generally can't modify libfdt files. Any changes will be blown
> away with the next dtc sync (there's one in -next now). Though here
> you are creating a new location with fdt code. lib/ is just a shim to
> the actual libfdt code. Don't put any implementation there. You can
> add this to drivers/of/fdt_address.c for the short term, but it still
> needs to go upstream.
> 
> Otherwise, the implementation looks fine to me.

I agree, but I don't think there's a real need for us to hack
drivers/of/fdt_address.c in the meantime -- let's just target upstream
and not carry this in the kernel.

Akashi -- for now, I'll drop the kdump parts of this series which rely
on this helper. The majority of the series is actually independent and
can go in as-is.

I've pushed out a kexec branch to the arm64 tree for you to take a look
at:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec

Thanks,

Will

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-12-06 15:54     ` Will Deacon
@ 2018-12-07 10:12       ` James Morse
  2018-12-11  6:17         ` AKASHI, Takahiro
  0 siblings, 1 reply; 10+ messages in thread
From: James Morse @ 2018-12-07 10:12 UTC (permalink / raw)
  To: Will Deacon, AKASHI, Takahiro
  Cc: Rob Herring, Catalin Marinas, David Howells, Vivek Goyal,
	Herbert Xu, David Miller, dyoung, Baoquan He, Arnd Bergmann,
	Martin Schwidefsky, Heiko Carstens, prudo, Ard Biesheuvel,
	bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree

Hi Akashi, Will,

On 06/12/2018 15:54, Will Deacon wrote:
> On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote:
>> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
>> <takahiro.akashi@linaro.org> wrote:
>>>
>>> Added function, fdt_setprop_reg(), will be used later to handle
>>> kexec-specific property in arm64's kexec_file implementation.
>>> It will possibly be merged into libfdt in the future.
>>
>> You generally can't modify libfdt files. Any changes will be blown
>> away with the next dtc sync (there's one in -next now). Though here
>> you are creating a new location with fdt code. lib/ is just a shim to
>> the actual libfdt code. Don't put any implementation there. You can
>> add this to drivers/of/fdt_address.c for the short term, but it still
>> needs to go upstream.
>>
>> Otherwise, the implementation looks fine to me.
> 
> I agree, but I don't think there's a real need for us to hack
> drivers/of/fdt_address.c in the meantime -- let's just target upstream
> and not carry this in the kernel.
> 
> Akashi -- for now, I'll drop the kdump parts of this series which rely
> on this helper. The majority of the series is actually independent and
> can go in as-is.
> 
> I've pushed out a kexec branch to the arm64 tree for you to take a look
> at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec

I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs
to explicitly forbid kdump via kexec_file_load. (like powerpc does already).
Without this kdump works, but the second kernel overwrites the first as those DT
properties are missing.

I'll post a patch momentarily,


Thanks,

James

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-12-07 10:12       ` James Morse
@ 2018-12-11  6:17         ` AKASHI, Takahiro
  2018-12-11 10:09           ` James Morse
  0 siblings, 1 reply; 10+ messages in thread
From: AKASHI, Takahiro @ 2018-12-11  6:17 UTC (permalink / raw)
  To: James Morse
  Cc: Will Deacon, Rob Herring, Catalin Marinas, David Howells,
	Vivek Goyal, Herbert Xu, David Miller, dyoung, Baoquan He,
	Arnd Bergmann, Martin Schwidefsky, Heiko Carstens, prudo,
	Ard Biesheuvel, bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree

James,

On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote:
> Hi Akashi, Will,
> 
> On 06/12/2018 15:54, Will Deacon wrote:
> > On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote:
> >> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
> >> <takahiro.akashi@linaro.org> wrote:
> >>>
> >>> Added function, fdt_setprop_reg(), will be used later to handle
> >>> kexec-specific property in arm64's kexec_file implementation.
> >>> It will possibly be merged into libfdt in the future.
> >>
> >> You generally can't modify libfdt files. Any changes will be blown
> >> away with the next dtc sync (there's one in -next now). Though here
> >> you are creating a new location with fdt code. lib/ is just a shim to
> >> the actual libfdt code. Don't put any implementation there. You can
> >> add this to drivers/of/fdt_address.c for the short term, but it still
> >> needs to go upstream.
> >>
> >> Otherwise, the implementation looks fine to me.
> > 
> > I agree, but I don't think there's a real need for us to hack
> > drivers/of/fdt_address.c in the meantime -- let's just target upstream
> > and not carry this in the kernel.
> > 
> > Akashi -- for now, I'll drop the kdump parts of this series which rely
> > on this helper. The majority of the series is actually independent and
> > can go in as-is.
> > 
> > I've pushed out a kexec branch to the arm64 tree for you to take a look
> > at:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec
> 
> I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs
> to explicitly forbid kdump via kexec_file_load. (like powerpc does already).

Thank you for pointing this out.

> Without this kdump works, but the second kernel overwrites the first as those DT
> properties are missing.
> 
> I'll post a patch momentarily,

Fine, but anyhow I'm going to submit a new version (*without* kdump),
I will fix the issue along with others.

-Takahiro Akashi


> 
> Thanks,
> 
> James
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-12-11  6:17         ` AKASHI, Takahiro
@ 2018-12-11 10:09           ` James Morse
  2018-12-12  1:28             ` AKASHI, Takahiro
  0 siblings, 1 reply; 10+ messages in thread
From: James Morse @ 2018-12-11 10:09 UTC (permalink / raw)
  To: AKASHI, Takahiro, Will Deacon
  Cc: Rob Herring, Catalin Marinas, David Howells, Vivek Goyal,
	Herbert Xu, David Miller, dyoung, Baoquan He, Arnd Bergmann,
	Martin Schwidefsky, Heiko Carstens, prudo, Ard Biesheuvel,
	bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree

Hi Akashi,

On 11/12/2018 06:17, AKASHI, Takahiro wrote:
> On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote:
>> On 06/12/2018 15:54, Will Deacon wrote:
>>> On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote:
>>>> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
>>>> <takahiro.akashi@linaro.org> wrote:
>>>>>
>>>>> Added function, fdt_setprop_reg(), will be used later to handle
>>>>> kexec-specific property in arm64's kexec_file implementation.
>>>>> It will possibly be merged into libfdt in the future.
>>>>
>>>> You generally can't modify libfdt files. Any changes will be blown
>>>> away with the next dtc sync (there's one in -next now). Though here
>>>> you are creating a new location with fdt code. lib/ is just a shim to
>>>> the actual libfdt code. Don't put any implementation there. You can
>>>> add this to drivers/of/fdt_address.c for the short term, but it still
>>>> needs to go upstream.
>>>>
>>>> Otherwise, the implementation looks fine to me.
>>>
>>> I agree, but I don't think there's a real need for us to hack
>>> drivers/of/fdt_address.c in the meantime -- let's just target upstream
>>> and not carry this in the kernel.
>>>
>>> Akashi -- for now, I'll drop the kdump parts of this series which rely
>>> on this helper. The majority of the series is actually independent and
>>> can go in as-is.
>>>
>>> I've pushed out a kexec branch to the arm64 tree for you to take a look
>>> at:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec
>>
>> I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs
>> to explicitly forbid kdump via kexec_file_load. (like powerpc does already).
> 
> Thank you for pointing this out.
> 
>> Without this kdump works, but the second kernel overwrites the first as those DT
>> properties are missing.
>>
>> I'll post a patch momentarily,
> 
> Fine, but anyhow I'm going to submit a new version (*without* kdump),
> I will fix the issue along with others.

I had a quick look at the arm64 for-next/core branch. Will has queued the
non-kdump parts of this series.

If you have changes, they need to be against the arm64 tree.


Thanks,

James

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property
  2018-12-11 10:09           ` James Morse
@ 2018-12-12  1:28             ` AKASHI, Takahiro
  0 siblings, 0 replies; 10+ messages in thread
From: AKASHI, Takahiro @ 2018-12-12  1:28 UTC (permalink / raw)
  To: James Morse
  Cc: Will Deacon, Rob Herring, Catalin Marinas, David Howells,
	Vivek Goyal, Herbert Xu, David Miller, dyoung, Baoquan He,
	Arnd Bergmann, Martin Schwidefsky, Heiko Carstens, prudo,
	Ard Biesheuvel, bhsharma, kexec,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, Frank Rowand, devicetree

On Tue, Dec 11, 2018 at 10:09:07AM +0000, James Morse wrote:
> Hi Akashi,
> 
> On 11/12/2018 06:17, AKASHI, Takahiro wrote:
> > On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote:
> >> On 06/12/2018 15:54, Will Deacon wrote:
> >>> On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote:
> >>>> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro
> >>>> <takahiro.akashi@linaro.org> wrote:
> >>>>>
> >>>>> Added function, fdt_setprop_reg(), will be used later to handle
> >>>>> kexec-specific property in arm64's kexec_file implementation.
> >>>>> It will possibly be merged into libfdt in the future.
> >>>>
> >>>> You generally can't modify libfdt files. Any changes will be blown
> >>>> away with the next dtc sync (there's one in -next now). Though here
> >>>> you are creating a new location with fdt code. lib/ is just a shim to
> >>>> the actual libfdt code. Don't put any implementation there. You can
> >>>> add this to drivers/of/fdt_address.c for the short term, but it still
> >>>> needs to go upstream.
> >>>>
> >>>> Otherwise, the implementation looks fine to me.
> >>>
> >>> I agree, but I don't think there's a real need for us to hack
> >>> drivers/of/fdt_address.c in the meantime -- let's just target upstream
> >>> and not carry this in the kernel.
> >>>
> >>> Akashi -- for now, I'll drop the kdump parts of this series which rely
> >>> on this helper. The majority of the series is actually independent and
> >>> can go in as-is.
> >>>
> >>> I've pushed out a kexec branch to the arm64 tree for you to take a look
> >>> at:
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec
> >>
> >> I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs
> >> to explicitly forbid kdump via kexec_file_load. (like powerpc does already).
> > 
> > Thank you for pointing this out.
> > 
> >> Without this kdump works, but the second kernel overwrites the first as those DT
> >> properties are missing.
> >>
> >> I'll post a patch momentarily,
> > 
> > Fine, but anyhow I'm going to submit a new version (*without* kdump),
> > I will fix the issue along with others.
> 
> I had a quick look at the arm64 for-next/core branch. Will has queued the
> non-kdump parts of this series.
> 
> If you have changes, they need to be against the arm64 tree.

Okay!

-Takahiro Akashi

> 
> Thanks,
> 
> James

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-12-12  1:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-15  5:52 [PATCH v16 00/16] arm64: kexec: add kexec_file_load() support AKASHI Takahiro
2018-11-15  5:52 ` [PATCH v16 06/16] lib: fdt: add a helper function for handling memory range property AKASHI Takahiro
2018-11-30 13:21   ` Will Deacon
2018-12-06 14:47   ` Rob Herring
2018-12-06 15:54     ` Will Deacon
2018-12-07 10:12       ` James Morse
2018-12-11  6:17         ` AKASHI, Takahiro
2018-12-11 10:09           ` James Morse
2018-12-12  1:28             ` AKASHI, Takahiro
2018-11-15  5:52 ` [PATCH v16 12/16] arm64: kexec_file: add crash dump support AKASHI Takahiro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).