Skip to contentSkip to conversation

RISC-V: Add kexec/kdump support

PATCH In review Nick Kossifidis 5 participants 14 messages Apr 19, 2021 – Oct 30, 2023

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v4 cover RISC-V: Add kexec/kdump support

This patch series adds kexec/kdump and crash kernel support on RISC-V. For testing the patches a patched version of kexec-tools is needed (still a work in progress) which can be found at:

https://riscv.ics.forth.gr/kexec-tools-patched.tar.xz

v4:
* Rebase on top of "fixes" branch
* Resolve Alex's comments

v3:
* Rebase on newer kernel tree
* Minor cleanups
* Split UAPI changes to a separate patch
* Improve / cleanup init_resources
* Resolve Palmer's comments

v2:
* Rebase on newer kernel tree
* Minor cleanups
* Properly populate the ioresources tre, so that it can be used later on for implementing strict /dev/mem
* Use linux,usable-memory on /memory instead of a new binding
* USe a reserved-memory node for ELF core header

Nick Kossifidis (5):
RISC-V: Add EM_RISCV to kexec UAPI header
RISC-V: Add kexec support
RISC-V: Improve init_resources
RISC-V: Add kdump support
RISC-V: Add crash kernel support

 arch/riscv/Kconfig                  |  25 ++++
 arch/riscv/include/asm/elf.h        |   6 +
 arch/riscv/include/asm/kexec.h      |  56 +++++++
 arch/riscv/kernel/Makefile          |   6 +
 arch/riscv/kernel/crash_dump.c      |  46 ++++++
 arch/riscv/kernel/crash_save_regs.S |  56 +++++++
 arch/riscv/kernel/kexec_relocate.S  | 223 ++++++++++++++++++++++++++++
 arch/riscv/kernel/machine_kexec.c   | 193 ++++++++++++++++++++++++
 arch/riscv/kernel/setup.c           | 114 ++++++++------
 arch/riscv/mm/init.c                | 104 +++++++++++++
 include/uapi/linux/kexec.h          |   1 +
 11 files changed, 784 insertions(+), 46 deletions(-)
 create mode 100644 arch/riscv/include/asm/kexec.h
 create mode 100644 arch/riscv/kernel/crash_dump.c
 create mode 100644 arch/riscv/kernel/crash_save_regs.S
 create mode 100644 arch/riscv/kernel/kexec_relocate.S
 create mode 100644 arch/riscv/kernel/machine_kexec.c
Signature · 1 line
2.26.2
#1 lore raw

PATCH v4 1/5 RISC-V: Add EM_RISCV to kexec UAPI header

Add RISC-V to the list of supported kexec architectures, we need to add the definition early-on so that later patches can use it.

EM_RISCV is 243 as per ELF psABI specification here:
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md

---
 include/uapi/linux/kexec.h | 1 +
 1 file changed, 1 insertion(+)
include/uapi/linux/kexec.h +1 -0
index 05669c87a..778dc191c 100644
@@ -42,6 +42,7 @@
 #define KEXEC_ARCH_MIPS_LE (10 << 16)
 #define KEXEC_ARCH_MIPS    ( 8 << 16)
 #define KEXEC_ARCH_AARCH64 (183 << 16)
+#define KEXEC_ARCH_RISCV   (243 << 16)
 
 /* The artificial cap on the number of segments passed to kexec_load. */
 #define KEXEC_SEGMENT_MAX 16
Signature · 1 line
2.26.2
#2 lore raw

PATCH v4 2/5 RISC-V: Add kexec support

In reply to Nick Kossifidis

This patch adds support for kexec on RISC-V. On SMP systems it depends on HOTPLUG_CPU in order to be able to bring up all harts after kexec. It also needs a recent OpenSBI version that supports the HSM extension. I tested it on riscv64 QEMU on both an smp and a non-smp system.

v6:
* Re-based on top of "fixes" branch
* Use PAGE_SIZE and CSR_* macros where possible
* Small cleanups

v5:
* For now depend on MMU, further changes needed for NOMMU support
* Make sure stvec is aligned
* Cleanup some unneeded fences
* Verify control code's buffer size
* Compile kexec_relocate.S with medany and norelax

v4:
* No functional changes, just re-based

v3:
* Use the new smp_shutdown_nonboot_cpus() call.
* Move riscv_kexec_relocate to .rodata

v2:
* Pass needed parameters as arguments to riscv_kexec_relocate instead of using global variables.
* Use kimage_arch to hold the fdt address of the included fdt.
* Use SYM_* macros on kexec_relocate.S.
* Compatibility with STRICT_KERNEL_RWX.
* Compatibility with HOTPLUG_CPU for SMP
* Small cleanups

---
 arch/riscv/Kconfig                 |  15 +++
 arch/riscv/include/asm/kexec.h     |  49 ++++++++
 arch/riscv/kernel/Makefile         |   5 +
 arch/riscv/kernel/kexec_relocate.S | 157 ++++++++++++++++++++++++
 arch/riscv/kernel/machine_kexec.c  | 186 +++++++++++++++++++++++++++++
 5 files changed, 412 insertions(+)
 create mode 100644 arch/riscv/include/asm/kexec.h
 create mode 100644 arch/riscv/kernel/kexec_relocate.S
 create mode 100644 arch/riscv/kernel/machine_kexec.c
arch/riscv/Kconfig +15 -0
index 4515a10c5..10cc19be0 100644
@@ -386,6 +386,21 @@ config RISCV_SBI_V01
 	help
 	  This config allows kernel to use SBI v0.1 APIs. This will be
 	  deprecated in future once legacy M-mode software are no longer in use.
+
+config KEXEC
+	bool "Kexec system call"
+	select KEXEC_CORE
+	select HOTPLUG_CPU if SMP
+	depends on MMU
+	help
+	  kexec is a system call that implements the ability to shutdown your
+	  current kernel, and to start another kernel. It is like a reboot
+	  but it is independent of the system firmware. And like a reboot
+	  you can start any kernel with it, not just Linux.
+
+	  The name comes from the similarity to the exec system call.
+
+
 endmenu
 
 menu "Boot options"
arch/riscv/include/asm/kexec.h new file +49 -0

49 lines not on this page · raw message

arch/riscv/kernel/Makefile +5 -0
index 647a47f54..211ef87de 100644
@@ -10,6 +10,10 @@ CFLAGS_REMOVE_sbi.o	= $(CC_FLAGS_FTRACE)
 endif
 CFLAGS_syscall_table.o	+= $(call cc-option,-Wno-override-init,)
 
+ifdef CONFIG_KEXEC
+AFLAGS_kexec_relocate.o := -mcmodel=medany -mno-relax
+endif
+
 extra-y += head.o
 extra-y += vmlinux.lds
 
@@ -55,6 +59,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o
 endif
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu-hotplug.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
+obj-$(CONFIG_KEXEC)		+= kexec_relocate.o machine_kexec.o
 
 obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
 
arch/riscv/kernel/kexec_relocate.S new file +157 -0

157 lines not on this page · raw message

arch/riscv/kernel/machine_kexec.c new file +186 -0

186 lines not on this page · raw message

Signature · 1 line
2.26.2
#3 lore raw

PATCH v4 3/5 RISC-V: Improve init_resources

In reply to Nick Kossifidis

* Kernel region is always present and we know where it is, no need to look for it inside the loop, just ignore it like the rest of the reserved regions within system's memory.

* Don't call memblock_free inside the loop, if called it'll split the region of pre-allocated resources in two parts, messing things up, just re-use the previous pre-allocated resource and free any unused resources after both loops finish.

---
 arch/riscv/kernel/setup.c | 91 ++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 45 deletions(-)
arch/riscv/kernel/setup.c +46 -45
index f8f15332c..030554bab 100644
@@ -60,6 +60,7 @@ static DEFINE_PER_CPU(struct cpu, cpu_devices);
  * also add "System RAM" regions for compatibility with other
  * archs, and the rest of the known regions for completeness.
  */
+static struct resource kimage_res = { .name = "Kernel image", };
 static struct resource code_res = { .name = "Kernel code", };
 static struct resource data_res = { .name = "Kernel data", };
 static struct resource rodata_res = { .name = "Kernel rodata", };
@@ -80,45 +81,54 @@ static int __init add_resource(struct resource *parent,
 	return 1;
 }
 
-static int __init add_kernel_resources(struct resource *res)
+static int __init add_kernel_resources(void)
 {
 	int ret = 0;
 
 	/*
 	 * The memory region of the kernel image is continuous and
-	 * was reserved on setup_bootmem, find it here and register
-	 * it as a resource, then register the various segments of
-	 * the image as child nodes
+	 * was reserved on setup_bootmem, register it here as a
+	 * resource, with the various segments of the image as
+	 * child nodes.
 	 */
-	if (!(res->start <= code_res.start && res->end >= data_res.end))
-		return 0;
 
-	res->name = "Kernel image";
-	res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+	code_res.start = __pa_symbol(_text);
+	code_res.end = __pa_symbol(_etext) - 1;
+	code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
 
-	/*
-	 * We removed a part of this region on setup_bootmem so
-	 * we need to expand the resource for the bss to fit in.
-	 */
-	res->end = bss_res.end;
+	rodata_res.start = __pa_symbol(__start_rodata);
+	rodata_res.end = __pa_symbol(__end_rodata) - 1;
+	rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	data_res.start = __pa_symbol(_data);
+	data_res.end = __pa_symbol(_edata) - 1;
+	data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	bss_res.start = __pa_symbol(__bss_start);
+	bss_res.end = __pa_symbol(__bss_stop) - 1;
+	bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	kimage_res.start = code_res.start;
+	kimage_res.end = bss_res.end;
+	kimage_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
 
-	ret = add_resource(&iomem_resource, res);
+	ret = add_resource(&iomem_resource, &kimage_res);
 	if (ret < 0)
 		return ret;
 
-	ret = add_resource(res, &code_res);
+	ret = add_resource(&kimage_res, &code_res);
 	if (ret < 0)
 		return ret;
 
-	ret = add_resource(res, &rodata_res);
+	ret = add_resource(&kimage_res, &rodata_res);
 	if (ret < 0)
 		return ret;
 
-	ret = add_resource(res, &data_res);
+	ret = add_resource(&kimage_res, &data_res);
 	if (ret < 0)
 		return ret;
 
-	ret = add_resource(res, &bss_res);
+	ret = add_resource(&kimage_res, &bss_res);
 
 	return ret;
 }
@@ -129,54 +139,42 @@ static void __init init_resources(void)
 	struct resource *res = NULL;
 	struct resource *mem_res = NULL;
 	size_t mem_res_sz = 0;
-	int ret = 0, i = 0;
-
-	code_res.start = __pa_symbol(_text);
-	code_res.end = __pa_symbol(_etext) - 1;
-	code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
-
-	rodata_res.start = __pa_symbol(__start_rodata);
-	rodata_res.end = __pa_symbol(__end_rodata) - 1;
-	rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
-
-	data_res.start = __pa_symbol(_data);
-	data_res.end = __pa_symbol(_edata) - 1;
-	data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
-
-	bss_res.start = __pa_symbol(__bss_start);
-	bss_res.end = __pa_symbol(__bss_stop) - 1;
-	bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+	int num_resources = 0, res_idx = 0;
+	int ret = 0;
 
 	/* + 1 as memblock_alloc() might increase memblock.reserved.cnt */
-	mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt + 1) * sizeof(*mem_res);
+	num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1;
+	res_idx = num_resources - 1;
+
+	mem_res_sz = num_resources * sizeof(*mem_res);
 	mem_res = memblock_alloc(mem_res_sz, SMP_CACHE_BYTES);
 	if (!mem_res)
 		panic("%s: Failed to allocate %zu bytes\n", __func__, mem_res_sz);
+
 	/*
 	 * Start by adding the reserved regions, if they overlap
 	 * with /memory regions, insert_resource later on will take
 	 * care of it.
 	 */
+	ret = add_kernel_resources();
+	if (ret < 0)
+		goto error;
+
 	for_each_reserved_mem_region(region) {
-		res = &mem_res[i++];
+		res = &mem_res[res_idx--];
 
 		res->name = "Reserved";
 		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 		res->start = __pfn_to_phys(memblock_region_reserved_base_pfn(region));
 		res->end = __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1;
 
-		ret = add_kernel_resources(res);
-		if (ret < 0)
-			goto error;
-		else if (ret)
-			continue;
-
 		/*
 		 * Ignore any other reserved regions within
 		 * system memory.
 		 */
 		if (memblock_is_memory(res->start)) {
-			memblock_free((phys_addr_t) res, sizeof(struct resource));
+			/* Re-use this pre-allocated resource */
+			res_idx++;
 			continue;
 		}
 
@@ -187,7 +185,7 @@ static void __init init_resources(void)
 
 	/* Add /memory regions to the resource tree */
 	for_each_mem_region(region) {
-		res = &mem_res[i++];
+		res = &mem_res[res_idx--];
 
 		if (unlikely(memblock_is_nomap(region))) {
 			res->name = "Reserved";
@@ -205,6 +203,9 @@ static void __init init_resources(void)
 			goto error;
 	}
 
+	/* Clean-up any unused pre-allocated resources */
+	mem_res_sz = (num_resources - res_idx + 1) * sizeof(*mem_res);
+	memblock_free((phys_addr_t) mem_res, mem_res_sz);
 	return;
 
  error:
Signature · 1 line
2.26.2
#4 lore raw

PATCH v4 4/5 RISC-V: Add kdump support

In reply to Nick Kossifidis

This patch adds support for kdump, the kernel will reserve a region for the crash kernel and jump there on panic. In order for userspace tools (kexec-tools) to prepare the crash kernel kexec image, we also need to expose some information on /proc/iomem for the memory regions used by the kernel and for the region reserved for crash kernel. Note that on userspace the device tree is used to determine the system's memory layout so the "System RAM" on /proc/iomem is ignored.

I tested this on riscv64 qemu and works as expected, you may test it by triggering a crash through /proc/sysrq_trigger:

echo c > /proc/sysrq_trigger

v4:
* Re-base on top of "fixes" branch
* Use CSR_* macros and PMD_SIZE where possible

v3:
* Move ELF_CORE_COPY_REGS to asm/elf.h instead of uapi/asm/elf.h
* Set stvec when disabling MMU
* Minor cleanups and re-base

v2:
* Properly populate the ioresources tree, so that it can be used later on for implementing strict /dev/mem.
* Minor cleanups and re-base

---
 arch/riscv/include/asm/elf.h        |  6 +++
 arch/riscv/include/asm/kexec.h      | 19 +++++---
 arch/riscv/kernel/Makefile          |  2 +-
 arch/riscv/kernel/crash_save_regs.S | 56 +++++++++++++++++++++++
 arch/riscv/kernel/kexec_relocate.S  | 68 ++++++++++++++++++++++++++-
 arch/riscv/kernel/machine_kexec.c   | 43 +++++++++--------
 arch/riscv/kernel/setup.c           | 11 ++++-
 arch/riscv/mm/init.c                | 71 +++++++++++++++++++++++++++++
 8 files changed, 249 insertions(+), 27 deletions(-)
 create mode 100644 arch/riscv/kernel/crash_save_regs.S
arch/riscv/include/asm/elf.h +6 -0
index 5c725e1df..f4b490cd0 100644
@@ -81,4 +81,10 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm,
 	int uses_interp);
 #endif /* CONFIG_MMU */
 
+#define ELF_CORE_COPY_REGS(dest, regs)			\
+do {							\
+	*(struct user_regs_struct *)&(dest) =		\
+		*(struct user_regs_struct *)regs;	\
+} while (0);
+
 #endif /* _ASM_RISCV_ELF_H */
arch/riscv/include/asm/kexec.h +13 -6
index 86e6e4922..1e9541019 100644
@@ -23,11 +23,16 @@
 
 #define KEXEC_ARCH KEXEC_ARCH_RISCV
 
+extern void riscv_crash_save_regs(struct pt_regs *newregs);
+
 static inline void
 crash_setup_regs(struct pt_regs *newregs,
 		 struct pt_regs *oldregs)
 {
-	/* Dummy implementation for now */
+	if (oldregs)
+		memcpy(newregs, oldregs, sizeof(struct pt_regs));
+	else
+		riscv_crash_save_regs(newregs);
 }
 
 
@@ -40,10 +45,12 @@ struct kimage_arch {
 const extern unsigned char riscv_kexec_relocate[];
 const extern unsigned int riscv_kexec_relocate_size;
 
-typedef void (*riscv_kexec_do_relocate)(unsigned long first_ind_entry,
-					unsigned long jump_addr,
-					unsigned long fdt_addr,
-					unsigned long hartid,
-					unsigned long va_pa_off);
+typedef void (*riscv_kexec_method)(unsigned long first_ind_entry,
+				   unsigned long jump_addr,
+				   unsigned long fdt_addr,
+				   unsigned long hartid,
+				   unsigned long va_pa_off);
+
+extern riscv_kexec_method riscv_kexec_norelocate;
 
 #endif
arch/riscv/kernel/Makefile +1 -1
index 211ef87de..41a1469b2 100644
@@ -59,7 +59,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o
 endif
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu-hotplug.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
-obj-$(CONFIG_KEXEC)		+= kexec_relocate.o machine_kexec.o
+obj-$(CONFIG_KEXEC)		+= kexec_relocate.o crash_save_regs.o machine_kexec.o
 
 obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
 
arch/riscv/kernel/crash_save_regs.S new file +56 -0

56 lines not on this page · raw message

arch/riscv/kernel/kexec_relocate.S +67 -1

74 lines not on this page · raw message

arch/riscv/kernel/machine_kexec.c +25 -18

73 lines not on this page · raw message

arch/riscv/kernel/setup.c +10 -1
index 030554bab..31866dce9 100644
@@ -20,6 +20,7 @@
 #include <linux/swiotlb.h>
 #include <linux/smp.h>
 #include <linux/efi.h>
+#include <linux/crash_dump.h>
 
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
@@ -160,6 +161,14 @@ static void __init init_resources(void)
 	if (ret < 0)
 		goto error;
 
+#ifdef CONFIG_KEXEC_CORE
+	if (crashk_res.start != crashk_res.end) {
+		ret = add_resource(&iomem_resource, &crashk_res);
+		if (ret < 0)
+			goto error;
+	}
+#endif
+
 	for_each_reserved_mem_region(region) {
 		res = &mem_res[res_idx--];
 
@@ -252,7 +261,6 @@ void __init setup_arch(char **cmdline_p)
 	efi_init();
 	setup_bootmem();
 	paging_init();
-	init_resources();
 #if IS_ENABLED(CONFIG_BUILTIN_DTB)
 	unflatten_and_copy_device_tree();
 #else
@@ -263,6 +271,7 @@ void __init setup_arch(char **cmdline_p)
 #endif
 	misc_mem_init();
 
+	init_resources();
 	sbi_init();
 
 	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
arch/riscv/mm/init.c +71 -0

95 lines not on this page · raw message

Signature · 1 line
2.26.2
#5 lore raw

PATCH v4 5/5 RISC-V: Add crash kernel support

In reply to Nick Kossifidis

This patch allows Linux to act as a crash kernel for use with kdump. Userspace will let the crash kernel know about the memory region it can use through linux,usable-memory property on the /memory node (overriding its reg property), and about the memory region where the elf core header of the previous kernel is saved, through a reserved-memory node with a compatible string of "linux,elfcorehdr". This approach is the least invasive and re-uses functionality already present.

I tested this on riscv64 qemu and it works as expected, you may test it by retrieving the dmesg of the previous kernel through /proc/vmcore, using the vmcore-dmesg utility from kexec-tools.

v4:
* Rebase on top of "fixes" branch

v3:
* Rebase

v2:
* Use linux,usable-memory on /memory instead of a new binding
* Use a reserved-memory node for ELF core header

---
 arch/riscv/Kconfig             | 10 ++++++++
 arch/riscv/kernel/Makefile     |  1 +
 arch/riscv/kernel/crash_dump.c | 46 ++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/setup.c      | 12 +++++++++
 arch/riscv/mm/init.c           | 33 ++++++++++++++++++++++++
 5 files changed, 102 insertions(+)
 create mode 100644 arch/riscv/kernel/crash_dump.c
arch/riscv/Kconfig +10 -0
index 10cc19be0..39aa18ef4 100644
@@ -400,6 +400,16 @@ config KEXEC
 
 	  The name comes from the similarity to the exec system call.
 
+config CRASH_DUMP
+	bool "Build kdump crash kernel"
+	help
+	  Generate crash dump after being started by kexec. This should
+	  be normally only set in special crash dump kernels which are
+	  loaded in the main kernel with kexec-tools into a specially
+	  reserved region and then later executed after a crash by
+	  kdump/kexec.
+
+	  For more details see Documentation/admin-guide/kdump/kdump.rst
 
 endmenu
 
arch/riscv/kernel/Makefile +1 -0
index 41a1469b2..d3081e4d9 100644
@@ -60,6 +60,7 @@ endif
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu-hotplug.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_KEXEC)		+= kexec_relocate.o crash_save_regs.o machine_kexec.o
+obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 
 obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
 
arch/riscv/kernel/crash_dump.c new file +46 -0

46 lines not on this page · raw message

arch/riscv/kernel/setup.c +12 -0
index 31866dce9..ff398a3d8 100644
@@ -66,6 +66,9 @@ static struct resource code_res = { .name = "Kernel code", };
 static struct resource data_res = { .name = "Kernel data", };
 static struct resource rodata_res = { .name = "Kernel rodata", };
 static struct resource bss_res = { .name = "Kernel bss", };
+#ifdef CONFIG_CRASH_DUMP
+static struct resource elfcorehdr_res = { .name = "ELF Core hdr", };
+#endif
 
 static int __init add_resource(struct resource *parent,
 				struct resource *res)
@@ -169,6 +172,15 @@ static void __init init_resources(void)
 	}
 #endif
 
+#ifdef CONFIG_CRASH_DUMP
+	if (elfcorehdr_size > 0) {
+		elfcorehdr_res.start = elfcorehdr_addr;
+		elfcorehdr_res.end = elfcorehdr_addr + elfcorehdr_size - 1;
+		elfcorehdr_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+		add_resource(&iomem_resource, &elfcorehdr_res);
+	}
+#endif
+
 	for_each_reserved_mem_region(region) {
 		res = &mem_res[res_idx--];
 
arch/riscv/mm/init.c +33 -0
index 8b2b85a57..9c048fccb 100644
@@ -13,6 +13,7 @@
 #include <linux/swap.h>
 #include <linux/sizes.h>
 #include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/libfdt.h>
 #include <linux/set_memory.h>
 #include <linux/dma-map-ops.h>
@@ -605,6 +606,18 @@ static void __init reserve_crashkernel(void)
 
 	int ret = 0;
 
+	/*
+	 * Don't reserve a region for a crash kernel on a crash kernel
+	 * since it doesn't make much sense and we have limited memory
+	 * resources.
+	 */
+#ifdef CONFIG_CRASH_DUMP
+	if (is_kdump_kernel()) {
+		pr_info("crashkernel: ignoring reservation request\n");
+		return;
+	}
+#endif
+
 	ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
 				&crash_size, &crash_base);
 	if (ret || !crash_size)
@@ -653,6 +666,26 @@ static void __init reserve_crashkernel(void)
 }
 #endif /* CONFIG_KEXEC_CORE */
 
+#ifdef CONFIG_CRASH_DUMP
+/*
+ * We keep track of the ELF core header of the crashed
+ * kernel with a reserved-memory region with compatible
+ * string "linux,elfcorehdr". Here we register a callback
+ * to populate elfcorehdr_addr/size when this region is
+ * present. Note that this region will be marked as
+ * reserved once we call early_init_fdt_scan_reserved_mem()
+ * later on.
+ */
+static int elfcore_hdr_setup(struct reserved_mem *rmem)
+{
+	elfcorehdr_addr = rmem->base;
+	elfcorehdr_size = rmem->size;
+	return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(elfcorehdr, "linux,elfcorehdr", elfcore_hdr_setup);
+#endif
+
 void __init paging_init(void)
 {
 	setup_vm_final();
Signature · 1 line
2.26.2
#6 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hi Nick,

CC rob, dt

Thanks for your patch, which is now commit 5640975003d0234d ("RISC-V:
Add crash kernel support") in v5.13-rc1.

Quoted 8 lines · Nick Kossifidis

This patch allows Linux to act as a crash kernel for use with kdump. Userspace will let the crash kernel know about the memory region it can use through linux,usable-memory property on the /memory node (overriding its reg property), and about the memory region where the elf core header of the previous kernel is saved, through a reserved-memory node with a compatible string of "linux,elfcorehdr". This approach is the least invasive and re-uses functionality already present.

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory range, the address and the size, of the elf core header which mainly describes the panicked kernel\'s memory layout as PT_LOAD segments of elf format.

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

Quoted 5 lines

I tested this on riscv64 qemu and it works as expected, you may test it by retrieving the dmesg of the previous kernel through /proc/vmcore, using the vmcore-dmesg utility from kexec-tools.

Quoted 8 lines

v4:
* Rebase on top of "fixes" branch

v3:
* Rebase

v2:
* Use linux,usable-memory on /memory instead of a new binding

This part seems to have been removed in v3 and later? Note that "linux,usable-memory-range" should be a property of the /chosen node, too, cfr.
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85

Quoted 35 lines

* Use a reserved-memory node for ELF core header

arch/riscv/mm/init.c +20 -1
@@ -653,6 +666,26 @@ static void __init reserve_crashkernel(void)
 }
 #endif /* CONFIG_KEXEC_CORE */
 
+#ifdef CONFIG_CRASH_DUMP
+/*
+ * We keep track of the ELF core header of the crashed
+ * kernel with a reserved-memory region with compatible
+ * string "linux,elfcorehdr". Here we register a callback
+ * to populate elfcorehdr_addr/size when this region is
+ * present. Note that this region will be marked as
+ * reserved once we call early_init_fdt_scan_reserved_mem()
+ * later on.
+ */
+static int elfcore_hdr_setup(struct reserved_mem *rmem)
+{
+       elfcorehdr_addr = rmem->base;
+       elfcorehdr_size = rmem->size;
+       return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(elfcorehdr, "linux,elfcorehdr", elfcore_hdr_setup);
+#endif
+
 void __init paging_init(void)
 {
        setup_vm_final();
--
Quoted 1 line

2.26.2

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
#7 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hello Geert,

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 14 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Quoted 3 lines

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also on arm64. Instead the reserved-region binding I used is a standard binding, if you don't like the name used for the compatible string because it overlaps with that property we can change it. I want to use a reserved-region for this because we'll have to reserve it anyway so using a property on /chosen and then using that property to reserve the region seemed suboptimal.

Quoted 8 lines
Quoted 2 lines

v2:
* Use linux,usable-memory on /memory instead of a new binding

This part seems to have been removed in v3 and later? Note that "linux,usable-memory-range" should be a property of the /chosen node, too, cfr.
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85

No special handling is needed when using linux,usable-memory on /memory, limiting the available memory is handled by generic code at drivers/of/fdt.c

Regards,
Nick

#8 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hi Nick,

Quoted 14 lines · Nick Kossifidis

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 13 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Quoted 6 lines
Quoted 3 lines

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also

That doesn't mean it must not be used on other architectures ;-) Arm64 was just the first one to use it...

Quoted 16 lines

on arm64. Instead the reserved-region binding I used is a standard binding, if you don't like the name used for the compatible string because it overlaps with that property we can change it. I want to use a reserved-region for this because we'll have to reserve it anyway so using a property on /chosen and then using that property to reserve the region seemed suboptimal.

Quoted 8 lines
Quoted 2 lines

v2:
* Use linux,usable-memory on /memory instead of a new binding

This part seems to have been removed in v3 and later? Note that "linux,usable-memory-range" should be a property of the /chosen node, too, cfr.
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85

Quoted 3 lines

No special handling is needed when using linux,usable-memory on /memory, limiting the available memory is handled by generic code at drivers/of/fdt.c

It was my understanding both properties under /chosen are the recommended methods for new platforms... Let's see what Rob has to say...

Anyway, I sent a patch series to switch to generic "linux,elfcorehdr" handling
https://lore.kernel.org/r/[email protected]/

Thanks!

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
#9 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Quoted 22 lines · Geert Uytterhoeven

Hi Nick,

Quoted 18 lines · Nick Kossifidis

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 16 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

Quoted 5 lines
Quoted 2 lines

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also

That doesn't mean it must not be used on other architectures ;-) Arm64 was just the first one to use it...

It is used on arm64 because memory is often passed by UEFI tables and not with /memory node. As riscv is also supporting EFI, I'd think they would do the same.

Quoted 40 lines
Quoted 16 lines

on arm64. Instead the reserved-region binding I used is a standard binding, if you don't like the name used for the compatible string because it overlaps with that property we can change it. I want to use a reserved-region for this because we'll have to reserve it anyway so using a property on /chosen and then using that property to reserve the region seemed suboptimal.

Quoted 8 lines
Quoted 2 lines

v2:
* Use linux,usable-memory on /memory instead of a new binding

This part seems to have been removed in v3 and later? Note that "linux,usable-memory-range" should be a property of the /chosen node, too, cfr.
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85

Quoted 3 lines

No special handling is needed when using linux,usable-memory on /memory, limiting the available memory is handled by generic code at drivers/of/fdt.c

It was my understanding both properties under /chosen are the recommended methods for new platforms... Let's see what Rob has to say...

Anyway, I sent a patch series to switch to generic "linux,elfcorehdr" handling
https://lore.kernel.org/r/[email protected]/

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#10 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Στις 2021-06-15 22:21, Rob Herring έγραψε:

Quoted 27 lines
Quoted 25 lines · Geert Uytterhoeven

Hi Nick,

Quoted 20 lines · Nick Kossifidis

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 13 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Quoted 3 lines

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also

Quoted 7 lines
Quoted 2 lines

That doesn't mean it must not be used on other architectures ;-) Arm64 was just the first one to use it...

It is used on arm64 because memory is often passed by UEFI tables and not with /memory node. As riscv is also supporting EFI, I'd think they would do the same.

We've had this discussion before, riscv uses /memory for now and even if we switched to getting memory from ACPI/UEFI tables, the elf core header is passed from the crashed kernel to the kdump kernel, it has nothing to do with UEFI since the bootloader is the kernel itself. Am I missing something ?

Regards,
Nick

#11 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

+Ard

Quoted 38 lines · Nick Kossifidis

Στις 2021-06-15 22:21, Rob Herring έγραψε:

Quoted 28 lines
Quoted 26 lines · Geert Uytterhoeven

Hi Nick,

Quoted 20 lines · Nick Kossifidis

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 13 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Quoted 3 lines

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also

Quoted 7 lines
Quoted 2 lines

That doesn't mean it must not be used on other architectures ;-) Arm64 was just the first one to use it...

It is used on arm64 because memory is often passed by UEFI tables and not with /memory node. As riscv is also supporting EFI, I'd think they would do the same.

Quoted 5 lines

We've had this discussion before, riscv uses /memory for now and even if we switched to getting memory from ACPI/UEFI tables, the elf core header is passed from the crashed kernel to the kdump kernel, it has nothing to do with UEFI since the bootloader is the kernel itself. Am I missing something ?

I believe if we originally booted using UEFI tables, then those are passed the kdump kernel as well. The original DT may have had a /memory node, but it's possible it didn't match what was in the UEFI tables. So using the DT /memory nodes for kdump could give surprising results. I think reserved regions also come from UEFI. Ard can probably comment better.

Rob

#12 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Quoted 48 lines · Rob Herring

+Ard

Quoted 38 lines · Nick Kossifidis

Στις 2021-06-15 22:21, Rob Herring έγραψε:

Quoted 28 lines
Quoted 26 lines · Geert Uytterhoeven

Hi Nick,

Quoted 20 lines · Nick Kossifidis

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:

Quoted 13 lines

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory
range,
the address and the size, of the elf core header which mainly
describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf
format.

Quoted 3 lines

Hence "linux,elfcorehdr" should be a property of the /chosen node, instead of a memory node with a compatible value of "linux,elfcorehdr".

That's a binding for a property on the /chosen node, that as the text says it's defined for arm64 only and the code that handled it was also

Quoted 7 lines
Quoted 2 lines

That doesn't mean it must not be used on other architectures ;-) Arm64 was just the first one to use it...

It is used on arm64 because memory is often passed by UEFI tables and not with /memory node. As riscv is also supporting EFI, I'd think they would do the same.

Quoted 5 lines

We've had this discussion before, riscv uses /memory for now and even if we switched to getting memory from ACPI/UEFI tables, the elf core header is passed from the crashed kernel to the kdump kernel, it has nothing to do with UEFI since the bootloader is the kernel itself. Am I missing something ?

Quoted 7 lines

I believe if we originally booted using UEFI tables, then those are passed the kdump kernel as well. The original DT may have had a /memory node, but it's possible it didn't match what was in the UEFI tables. So using the DT /memory nodes for kdump could give surprising results. I think reserved regions also come from UEFI. Ard can probably comment better.

Anything that executes in the context of the UEFI boot firmware (loaders, drivers, etc) may use the UEFI memory allocation routines to allocate memory, and these allocations are communicated via the UEFI memory map, not via the /memory node.

So it depends whether it matters if the kexec kernel tramples over those regions. For kdump scenarios, it might be reasonable, but in the general case, we should really respect what UEFI tells us about the memory map when booting via UEFI.

#13 lore raw

Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

In reply to Nick Kossifidis
Quoted 6 lines · Nick Kossifidis
arch/riscv/mm/init.c +2 -0
@@ -605,6 +606,18 @@ static void __init reserve_crashkernel(void)
 
 	int ret = 0;
 
+	/*
+	 * Don't reserve a region for a crash kernel on a crash kernel

arch/riscv/mm/init.c static void __init reserve_crashkernel(void) · line 620

Quoted 10 lines
arch/riscv/mm/init.c +10 -0
+	 * since it doesn't make much sense and we have limited memory
+	 * resources.
+	 */
+#ifdef CONFIG_CRASH_DUMP
+	if (is_kdump_kernel()) {
+		pr_info("crashkernel: ignoring reservation request\n");
+		return;
+	}
+#endif
+

Why is this necessary? It breaks
https://github.com/openSUSE/kdump/tree/master/calibrate which wants to run the kernel with both elfcorehdr and crashkernel.

Signature · 3 lines
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."