All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] (kexec-tools) arm64: add kdump support
@ 2016-09-07  4:33 AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly AKASHI Takahiro
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

My kernel patches of kdump suport on arm64 are currently under reviews.

This patchset is synced with them (v26 [1]) and provides necessary changes
for kexec-tools. It should be applied on top of Geoff's kexec-tools patches
v5[2] along with a bugfix[3].

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454588.html
[2] http://lists.infradead.org/pipermail/kexec/2016-September/017110.html
[3] http://lists.infradead.org/pipermail/kexec/2016-July/016664.html

Changes for v3:
 - rebased on Geoff's v5
 - fix a value of estimated PHYS_OFFSET
 - add a kernel code/data segment because they now reside out of linear
   mapping due to KASLR introduction
 - remove "linux,usable-memory-range" dependency, instead using
   "reserved-memory" node
 - add -mem-min/-mem-max support

Changes for v2:
 - trim a temoprary buffer in setup_2nd_dtb()
 - add patch#6("kexec: generalize and rename get_kernel_stext_sym()")
 - update patch#7 from Pratyush
   (re-worked by akashi)

AKASHI Takahiro (6):
  arm64: identify PHYS_OFFSET correctly
  arm64: kdump: identify memory regions
  arm64: kdump: add elf core header segment
  arm64: kdump: set up kernel image segment
  arm64: kdump: set up other segments
  arm64: kdump: add DT properties to crash dump kernel's dtb

Pratyush Anand (2):
  kexec: generalize and rename get_kernel_stext_sym()
  arm64: kdump: Add support for binary image files

 kexec/Makefile                       |   1 +
 kexec/arch/arm/crashdump-arm.c       |  40 +---
 kexec/arch/arm64/Makefile            |   2 +
 kexec/arch/arm64/crashdump-arm64.c   | 358 ++++++++++++++++++++++++++++++++++-
 kexec/arch/arm64/crashdump-arm64.h   |  20 +-
 kexec/arch/arm64/iomem.h             |  10 +
 kexec/arch/arm64/kexec-arm64.c       |  86 +++++++--
 kexec/arch/arm64/kexec-elf-arm64.c   |  25 ++-
 kexec/arch/arm64/kexec-image-arm64.c |  12 ++
 kexec/arch/i386/crashdump-x86.c      |  32 +---
 kexec/kexec.h                        |   2 +
 kexec/symbols.c                      |  41 ++++
 12 files changed, 534 insertions(+), 95 deletions(-)
 create mode 100644 kexec/arch/arm64/iomem.h
 create mode 100644 kexec/symbols.c

-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-09-27 15:49   ` Pratyush Anand
  2016-09-07  4:33 ` [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym() AKASHI Takahiro
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

Due to the kernel patch[1], the current code will not be able to identify
the correct value of PHYS_OFFSET if some "reserved" memory region, which
is likely to be UEFI runtime services code/data, exists at an address below
the first "System RAM" regions.

This patch fixes this issue.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/iomem.h       |  7 +++++++
 kexec/arch/arm64/kexec-arm64.c | 22 +++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)
 create mode 100644 kexec/arch/arm64/iomem.h

diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
new file mode 100644
index 0000000..7fd66eb
--- /dev/null
+++ b/kexec/arch/arm64/iomem.h
@@ -0,0 +1,7 @@
+#ifndef IOMEM_H
+#define IOMEM_H
+
+#define SYSTEM_RAM		"System RAM\n"
+#define IOMEM_RESERVED		"reserved\n"
+
+#endif
diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 7183dac..bc96c76 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -21,6 +21,7 @@
 #include "crashdump-arm64.h"
 #include "dt-ops.h"
 #include "fs2dt.h"
+#include "iomem.h"
 #include "kexec-syscall.h"
 #include "arch/options.h"
 
@@ -465,18 +466,28 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
  * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
  */
 
+static int count_memory_ranges;
+
 static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
 	unsigned long long base, unsigned long long length)
 {
 	struct memory_range *r;
 
-	if (nr >= KEXEC_SEGMENT_MAX)
+	if (count_memory_ranges >= KEXEC_SEGMENT_MAX)
 		return -1;
 
-	r = (struct memory_range *)data + nr;
-	r->type = RANGE_RAM;
+	r = (struct memory_range *)data + count_memory_ranges;
+
+	if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)))
+		r->type = RANGE_RAM;
+	else if (!strncmp(str, IOMEM_RESERVED, strlen(IOMEM_RESERVED)))
+		r->type = RANGE_RESERVED;
+	else
+		return 0;
+
 	r->start = base;
 	r->end = base + length - 1;
+	count_memory_ranges++;
 
 	set_phys_offset(r->start);
 
@@ -493,9 +504,10 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
 static int get_memory_ranges_iomem(struct memory_range *array,
 	unsigned int *count)
 {
-	*count = kexec_iomem_for_each_line("System RAM\n",
-		get_memory_ranges_iomem_cb, array);
+	count_memory_ranges = 0;
+	kexec_iomem_for_each_line(NULL, get_memory_ranges_iomem_cb, array);
 
+	*count = count_memory_ranges;
 	if (!*count) {
 		dbgprintf("%s: failed: No RAM found.\n", __func__);
 		return -EFAILED;
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-10-06 13:28   ` Matthias Brugger
  2016-09-07  4:33 ` [PATCH v3 3/8] arm64: kdump: identify memory regions AKASHI Takahiro
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

From: Pratyush Anand <panand@redhat.com>

get_kernel_stext_sym() has been defined for both arm and i386. Other
architecture might need some other kernel symbol address. Therefore rewrite
this function as generic function to get any kernel symbol address.

More over, kallsyms is not arch specific representation, therefore have
common function for all arches.

Signed-off-by: Pratyush Anand <panand@redhat.com>
[created symbols.c]
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/Makefile                  |  1 +
 kexec/arch/arm/crashdump-arm.c  | 40 +---------------------------------------
 kexec/arch/i386/crashdump-x86.c | 32 +-------------------------------
 kexec/kexec.h                   |  2 ++
 kexec/symbols.c                 | 41 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 46 insertions(+), 70 deletions(-)
 create mode 100644 kexec/symbols.c

diff --git a/kexec/Makefile b/kexec/Makefile
index 39f365f..2b4fb3d 100644
--- a/kexec/Makefile
+++ b/kexec/Makefile
@@ -26,6 +26,7 @@ KEXEC_SRCS_base += kexec/kernel_version.c
 KEXEC_SRCS_base += kexec/lzma.c
 KEXEC_SRCS_base += kexec/zlib.c
 KEXEC_SRCS_base += kexec/kexec-xen.c
+KEXEC_SRCS_base += kexec/symbols.c
 
 KEXEC_GENERATED_SRCS += $(PURGATORY_HEX_C)
 
diff --git a/kexec/arch/arm/crashdump-arm.c b/kexec/arch/arm/crashdump-arm.c
index 4a89b5e..2bc898b 100644
--- a/kexec/arch/arm/crashdump-arm.c
+++ b/kexec/arch/arm/crashdump-arm.c
@@ -73,48 +73,10 @@ static struct crash_elf_info elf_info = {
 
 extern unsigned long long user_page_offset;
 
-/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
-static unsigned long long get_kernel_stext_sym(void)
-{
-	const char *kallsyms = "/proc/kallsyms";
-	const char *stext = "_stext";
-	char sym[128];
-	char line[128];
-	FILE *fp;
-	unsigned long long vaddr = 0;
-	char type;
-
-	fp = fopen(kallsyms, "r");
-	if (!fp) {
-		fprintf(stderr, "Cannot open %s\n", kallsyms);
-		return 0;
-	}
-
-	while(fgets(line, sizeof(line), fp) != NULL) {
-		unsigned long long addr;
-
-		if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
-			continue;
-
-		if (strcmp(sym, stext) == 0) {
-			dbgprintf("kernel symbol %s vaddr = %#llx\n", stext, addr);
-			vaddr = addr;
-			break;
-		}
-	}
-
-	fclose(fp);
-
-	if (vaddr == 0)
-		fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
-
-	return vaddr;
-}
-
 static int get_kernel_page_offset(struct kexec_info *info,
 		struct crash_elf_info *elf_info)
 {
-	unsigned long long stext_sym_addr = get_kernel_stext_sym();
+	unsigned long long stext_sym_addr = get_kernel_sym("stext");
 	if (stext_sym_addr == 0) {
 		if (user_page_offset != (-1ULL)) {
 			elf_info->page_offset = user_page_offset;
diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index bbc0f35..664eb3b 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -102,36 +102,6 @@ static int get_kernel_paddr(struct kexec_info *UNUSED(info),
 	return -1;
 }
 
-/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
-static unsigned long long get_kernel_stext_sym(void)
-{
-	const char *kallsyms = "/proc/kallsyms";
-	const char *stext = "_stext";
-	char sym[128];
-	char line[128];
-	FILE *fp;
-	unsigned long long vaddr;
-	char type;
-
-	fp = fopen(kallsyms, "r");
-	if (!fp) {
-		fprintf(stderr, "Cannot open %s\n", kallsyms);
-		return 0;
-	}
-
-	while(fgets(line, sizeof(line), fp) != NULL) {
-		if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
-			continue;
-		if (strcmp(sym, stext) == 0) {
-			dbgprintf("kernel symbol %s vaddr = %16llx\n", stext, vaddr);
-			return vaddr;
-		}
-	}
-
-	fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
-	return 0;
-}
-
 /* Retrieve info regarding virtual address kernel has been compiled for and
  * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
  * from kexec-tools fails because of malformed elf notes. A kernel patch has
@@ -182,7 +152,7 @@ static int get_kernel_vaddr_and_size(struct kexec_info *UNUSED(info),
 
 	/* Traverse through the Elf headers and find the region where
 	 * _stext symbol is located in. That's where kernel is mapped */
-	stext_sym = get_kernel_stext_sym();
+	stext_sym = get_kernel_sym("stext");
 	for(phdr = ehdr.e_phdr; stext_sym && phdr != end_phdr; phdr++) {
 		if (phdr->p_type == PT_LOAD) {
 			unsigned long long saddr = phdr->p_vaddr;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 9194f1c..b4fafad 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -312,4 +312,6 @@ int xen_kexec_load(struct kexec_info *info);
 int xen_kexec_unload(uint64_t kexec_flags);
 void xen_kexec_exec(void);
 
+extern unsigned long long get_kernel_sym(const char *text);
+
 #endif /* KEXEC_H */
diff --git a/kexec/symbols.c b/kexec/symbols.c
new file mode 100644
index 0000000..ea6e327
--- /dev/null
+++ b/kexec/symbols.c
@@ -0,0 +1,41 @@
+#include <stdio.h>
+#include <string.h>
+#include "kexec.h"
+
+/* Retrieve kernel symbol virtual address from /proc/kallsyms */
+unsigned long long get_kernel_sym(const char *text)
+{
+	const char *kallsyms = "/proc/kallsyms";
+	char sym[128];
+	char line[128];
+	FILE *fp;
+	unsigned long long vaddr = 0;
+	char type;
+
+	fp = fopen(kallsyms, "r");
+	if (!fp) {
+		fprintf(stderr, "Cannot open %s\n", kallsyms);
+		return 0;
+	}
+
+	while (fgets(line, sizeof(line), fp) != NULL) {
+		unsigned long long addr;
+
+		if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
+			continue;
+
+		if (strcmp(sym, text) == 0) {
+			dbgprintf("kernel symbol %s vaddr = %#llx\n",
+								text, addr);
+			vaddr = addr;
+			break;
+		}
+	}
+
+	fclose(fp);
+
+	if (vaddr == 0)
+		fprintf(stderr, "Cannot get kernel %s symbol address\n", text);
+
+	return vaddr;
+}
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 3/8] arm64: kdump: identify memory regions
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym() AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 4/8] arm64: kdump: add elf core header segment AKASHI Takahiro
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

The following regions need to be identified for later use:
 a) memory regions which belong to the 1st kernel
 b) usable memory reserved for crash dump kernel

We go through /proc/iomem to find out a) and b) which are marked
as "System RAM" and "Crash kernel", respectively.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/Makefile          |   2 +
 kexec/arch/arm64/crashdump-arm64.c | 100 ++++++++++++++++++++++++++++++++++++-
 kexec/arch/arm64/crashdump-arm64.h |  14 +++++-
 kexec/arch/arm64/iomem.h           |   1 +
 4 files changed, 114 insertions(+), 3 deletions(-)

diff --git a/kexec/arch/arm64/Makefile b/kexec/arch/arm64/Makefile
index 37414dc..c2535c6 100644
--- a/kexec/arch/arm64/Makefile
+++ b/kexec/arch/arm64/Makefile
@@ -5,6 +5,8 @@ arm64_FS2DT_INCLUDE += -include $(srcdir)/kexec/arch/arm64/kexec-arm64.h \
 
 arm64_DT_OPS += kexec/dt-ops.c
 
+arm64_MEM_REGIONS = kexec/mem_regions.c
+
 arm64_CPPFLAGS += -I $(srcdir)/kexec/
 
 arm64_KEXEC_SRCS += \
diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
index d2272c8..dcaca43 100644
--- a/kexec/arch/arm64/crashdump-arm64.c
+++ b/kexec/arch/arm64/crashdump-arm64.c
@@ -1,5 +1,13 @@
 /*
  * ARM64 crashdump.
+ *     partly derived from arm implementation
+ *
+ * Copyright (c) 2014-2016 Linaro Limited
+ * Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
+ *
+ * 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 _GNU_SOURCE
@@ -10,12 +18,102 @@
 #include "kexec.h"
 #include "crashdump.h"
 #include "crashdump-arm64.h"
+#include "iomem.h"
 #include "kexec-arm64.h"
 #include "kexec-elf.h"
+#include "mem_regions.h"
 
-struct memory_ranges usablemem_rgns = {};
+/* memory ranges on crashed kernel */
+static struct memory_range crash_memory_ranges[CRASH_MAX_MEMORY_RANGES];
+static struct memory_ranges crash_memory_rgns = {
+	.size = 0,
+	.max_size = CRASH_MAX_MEMORY_RANGES,
+	.ranges = crash_memory_ranges,
+};
+
+/* memory range reserved for crashkernel */
+struct memory_range crash_reserved_mem;
+struct memory_ranges usablemem_rgns = {
+	.size = 0,
+	.max_size = 1,
+	.ranges = &crash_reserved_mem,
+};
+
+/*
+ * iomem_range_callback() - callback called for each iomem region
+ * @data: not used
+ * @nr: not used
+ * @str: name of the memory region
+ * @base: start address of the memory region
+ * @length: size of the memory region
+ *
+ * This function is called once for each memory region found in /proc/iomem.
+ * It locates system RAM and crashkernel reserved memory and places these to
+ * variables, respectively, crash_memory_ranges and crash_reserved_mem.
+ */
+
+static int iomem_range_callback(void *UNUSED(data), int UNUSED(nr),
+				char *str, unsigned long long base,
+				unsigned long long length)
+{
+	if (strncmp(str, CRASH_KERNEL, strlen(CRASH_KERNEL)) == 0)
+		return mem_regions_add(&usablemem_rgns,
+				       base, length, RANGE_RAM);
+	else if (strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)) == 0)
+		return mem_regions_add(&crash_memory_rgns,
+				       base, length, RANGE_RAM);
+
+	return 0;
+}
 
 int is_crashkernel_mem_reserved(void)
 {
+	if (!crash_reserved_mem.end)
+		kexec_iomem_for_each_line(NULL, iomem_range_callback, NULL);
+
+	return crash_reserved_mem.start != crash_reserved_mem.end;
+}
+
+/*
+ * crash_get_memory_ranges() - read system physical memory
+ *
+ * Function reads through system physical memory and stores found memory
+ * regions in crash_memory_ranges.
+ * Regions are sorted in ascending order.
+ *
+ * Returns 0 in case of success and -1 otherwise (errno is set).
+ */
+static int crash_get_memory_ranges(void)
+{
+	/*
+	 * First read all memory regions that can be considered as
+	 * system memory including the crash area.
+	 */
+	if (!usablemem_rgns.size)
+		kexec_iomem_for_each_line(NULL, iomem_range_callback, NULL);
+
+	/* allow only a single region for crash dump kernel */
+	if (usablemem_rgns.size != 1) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	dbgprint_mem_range("Reserved memory range", &crash_reserved_mem, 1);
+
+	if (mem_regions_exclude(&crash_memory_rgns, &crash_reserved_mem)) {
+		fprintf(stderr,
+			"Error: Number of crash memory ranges excedeed the max limit\n");
+		errno = ENOMEM;
+		return -1;
+	}
+
+	/*
+	 * Make sure that the memory regions are sorted.
+	 */
+	mem_regions_sort(&crash_memory_rgns);
+
+	dbgprint_mem_range("Coredump memory ranges",
+			   crash_memory_rgns.ranges, crash_memory_rgns.size);
+
 	return 0;
 }
diff --git a/kexec/arch/arm64/crashdump-arm64.h b/kexec/arch/arm64/crashdump-arm64.h
index f33c7a2..07a0ed0 100644
--- a/kexec/arch/arm64/crashdump-arm64.h
+++ b/kexec/arch/arm64/crashdump-arm64.h
@@ -1,12 +1,22 @@
 /*
  * ARM64 crashdump.
+ *
+ * Copyright (c) 2014-2016 Linaro Limited
+ * Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
+ *
+ * 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.
  */
 
-#if !defined(CRASHDUMP_ARM64_H)
+#ifndef CRASHDUMP_ARM64_H
 #define CRASHDUMP_ARM64_H
 
 #include "kexec.h"
 
+#define CRASH_MAX_MEMORY_RANGES	32
+
 extern struct memory_ranges usablemem_rgns;
+extern struct memory_range crash_reserved_mem;
 
-#endif
+#endif /* CRASHDUMP_ARM64_H */
diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
index 7fd66eb..20cda87 100644
--- a/kexec/arch/arm64/iomem.h
+++ b/kexec/arch/arm64/iomem.h
@@ -2,6 +2,7 @@
 #define IOMEM_H
 
 #define SYSTEM_RAM		"System RAM\n"
+#define CRASH_KERNEL		"Crash kernel\n"
 #define IOMEM_RESERVED		"reserved\n"
 
 #endif
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 4/8] arm64: kdump: add elf core header segment
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (2 preceding siblings ...)
  2016-09-07  4:33 ` [PATCH v3 3/8] arm64: kdump: identify memory regions AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 5/8] arm64: kdump: set up kernel image segment AKASHI Takahiro
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

Elf core header contains the information necessary for the coredump of
the 1st kernel, including its physcal memory layout as well as cpu register
states at the panic.
The segment is allocated inside the reserved memory of crash dump kernel.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/crashdump-arm64.c | 96 ++++++++++++++++++++++++++++++++++++++
 kexec/arch/arm64/crashdump-arm64.h |  3 ++
 kexec/arch/arm64/iomem.h           |  2 +
 kexec/arch/arm64/kexec-elf-arm64.c | 10 ++++
 4 files changed, 111 insertions(+)

diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
index dcaca43..8346131 100644
--- a/kexec/arch/arm64/crashdump-arm64.c
+++ b/kexec/arch/arm64/crashdump-arm64.c
@@ -39,6 +39,39 @@ struct memory_ranges usablemem_rgns = {
 	.ranges = &crash_reserved_mem,
 };
 
+struct memory_range elfcorehdr_mem;
+
+static struct crash_elf_info elf_info = {
+	.class		= ELFCLASS64,
+#if (__BYTE_ORDER == __LITTLE_ENDIAN)
+	.data		= ELFDATA2LSB,
+#else
+	.data		= ELFDATA2MSB,
+#endif
+	.machine	= EM_AARCH64,
+};
+
+/*
+ * Note: The returned value is correct only if !CONFIG_RANDOMIZE_BASE.
+ */
+static uint64_t get_kernel_page_offset(void)
+{
+	int i;
+
+	if (elf_info.kern_vaddr_start == UINT64_MAX)
+		return UINT64_MAX;
+
+	/* Current max virtual memory range is 48-bits. */
+	for (i = 48; i > 0; i--)
+		if (!(elf_info.kern_vaddr_start & (1UL << i)))
+			break;
+
+	if (i <= 0)
+		return UINT64_MAX;
+	else
+		return UINT64_MAX << i;
+}
+
 /*
  * iomem_range_callback() - callback called for each iomem region
  * @data: not used
@@ -62,6 +95,10 @@ static int iomem_range_callback(void *UNUSED(data), int UNUSED(nr),
 	else if (strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)) == 0)
 		return mem_regions_add(&crash_memory_rgns,
 				       base, length, RANGE_RAM);
+	else if (strncmp(str, KERNEL_CODE, strlen(KERNEL_CODE)) == 0)
+		elf_info.kern_paddr_start = base;
+	else if (strncmp(str, KERNEL_DATA, strlen(KERNEL_DATA)) == 0)
+		elf_info.kern_size = base + length - elf_info.kern_paddr_start;
 
 	return 0;
 }
@@ -115,5 +152,64 @@ static int crash_get_memory_ranges(void)
 	dbgprint_mem_range("Coredump memory ranges",
 			   crash_memory_rgns.ranges, crash_memory_rgns.size);
 
+	/*
+	 * For additional kernel code/data segment.
+	 * kern_paddr_start/kern_size are determined in iomem_range_callback
+	 */
+	elf_info.kern_vaddr_start = get_kernel_sym("_text");
+	if (!elf_info.kern_vaddr_start)
+		elf_info.kern_vaddr_start = UINT64_MAX;
+
+	return 0;
+}
+
+/*
+ * load_crashdump_segments() - load the elf core header
+ * @info: kexec info structure
+ *
+ * This function creates and loads an additional segment of elf core header
+ : which is used to construct /proc/vmcore on crash dump kernel.
+ *
+ * Return 0 in case of success and -1 in case of error.
+ */
+
+int load_crashdump_segments(struct kexec_info *info)
+{
+	unsigned long elfcorehdr;
+	unsigned long bufsz;
+	void *buf;
+	int err;
+
+	/*
+	 * First fetch all the memory (RAM) ranges that we are going to
+	 * pass to the crash dump kernel during panic.
+	 */
+
+	err = crash_get_memory_ranges();
+
+	if (err)
+		return err;
+
+	elf_info.page_offset = get_kernel_page_offset();
+	dbgprintf("%s: page_offset:   %016llx\n", __func__,
+			elf_info.page_offset);
+
+	err = crash_create_elf64_headers(info, &elf_info,
+			crash_memory_rgns.ranges, crash_memory_rgns.size,
+			&buf, &bufsz, ELF_CORE_HEADER_ALIGN);
+
+	if (err)
+		return err;
+
+	elfcorehdr = add_buffer_phys_virt(info, buf, bufsz, bufsz, 0,
+		crash_reserved_mem.start, crash_reserved_mem.end,
+		-1, 0);
+
+	elfcorehdr_mem.start = elfcorehdr;
+	elfcorehdr_mem.end = elfcorehdr + bufsz - 1;
+
+	dbgprintf("%s: elfcorehdr 0x%llx-0x%llx\n", __func__,
+			elfcorehdr_mem.start, elfcorehdr_mem.end);
+
 	return 0;
 }
diff --git a/kexec/arch/arm64/crashdump-arm64.h b/kexec/arch/arm64/crashdump-arm64.h
index 07a0ed0..da75a2d 100644
--- a/kexec/arch/arm64/crashdump-arm64.h
+++ b/kexec/arch/arm64/crashdump-arm64.h
@@ -18,5 +18,8 @@
 
 extern struct memory_ranges usablemem_rgns;
 extern struct memory_range crash_reserved_mem;
+extern struct memory_range elfcorehdr_mem;
+
+extern int load_crashdump_segments(struct kexec_info *info);
 
 #endif /* CRASHDUMP_ARM64_H */
diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
index 20cda87..d4864bb 100644
--- a/kexec/arch/arm64/iomem.h
+++ b/kexec/arch/arm64/iomem.h
@@ -2,6 +2,8 @@
 #define IOMEM_H
 
 #define SYSTEM_RAM		"System RAM\n"
+#define KERNEL_CODE		"Kernel code\n"
+#define KERNEL_DATA		"Kernel data\n"
 #define CRASH_KERNEL		"Crash kernel\n"
 #define IOMEM_RESERVED		"reserved\n"
 
diff --git a/kexec/arch/arm64/kexec-elf-arm64.c b/kexec/arch/arm64/kexec-elf-arm64.c
index daf8bf0..c70a37a 100644
--- a/kexec/arch/arm64/kexec-elf-arm64.c
+++ b/kexec/arch/arm64/kexec-elf-arm64.c
@@ -119,6 +119,16 @@ int elf_arm64_load(int argc, char **argv, const char *kernel_buf,
 	dbgprintf("%s: PE format:      %s\n", __func__,
 		(arm64_header_check_pe_sig(header) ? "yes" : "no"));
 
+	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		/* create and initialize elf core header segment */
+		result = load_crashdump_segments(info);
+		if (result) {
+			dbgprintf("%s: Creating eflcorehdr failed.\n",
+								__func__);
+			goto exit;
+		}
+	}
+
 	/* load the kernel */
 	result = elf_exec_load(&ehdr, info);
 
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 5/8] arm64: kdump: set up kernel image segment
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (3 preceding siblings ...)
  2016-09-07  4:33 ` [PATCH v3 4/8] arm64: kdump: add elf core header segment AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-09-07  4:33 ` [PATCH v3 6/8] arm64: kdump: set up other segments AKASHI Takahiro
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

On arm64, we can use the same kernel image as 1st kernel, but
we have to modify the entry point as well as segments' addresses
in the kernel's elf header in order to load them into correct places.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/crashdump-arm64.c | 23 +++++++++++++++++++++++
 kexec/arch/arm64/crashdump-arm64.h |  1 +
 kexec/arch/arm64/kexec-arm64.c     | 25 ++++++++++++++++++++-----
 kexec/arch/arm64/kexec-elf-arm64.c | 10 +++++++++-
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
index 8346131..9517329 100644
--- a/kexec/arch/arm64/crashdump-arm64.c
+++ b/kexec/arch/arm64/crashdump-arm64.c
@@ -213,3 +213,26 @@ int load_crashdump_segments(struct kexec_info *info)
 
 	return 0;
 }
+
+/*
+ * e_entry and p_paddr are actually in virtual address space.
+ * Those values will be translated to physcal addresses by
+ * using virt_to_phys().
+ * So let's get ready for later use so the memory base (phys_offset)
+ * will be correctly replaced with crash_reserved_mem.start.
+ */
+void modify_ehdr_for_crashdump(struct mem_ehdr *ehdr)
+{
+	struct mem_phdr *phdr;
+	int i;
+
+	ehdr->e_entry += - arm64_mem.phys_offset + crash_reserved_mem.start;
+
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		phdr = &ehdr->e_phdr[i];
+		if (phdr->p_type != PT_LOAD)
+			continue;
+		phdr->p_paddr +=
+			(-arm64_mem.phys_offset + crash_reserved_mem.start);
+	}
+}
diff --git a/kexec/arch/arm64/crashdump-arm64.h b/kexec/arch/arm64/crashdump-arm64.h
index da75a2d..382f571 100644
--- a/kexec/arch/arm64/crashdump-arm64.h
+++ b/kexec/arch/arm64/crashdump-arm64.h
@@ -21,5 +21,6 @@ extern struct memory_range crash_reserved_mem;
 extern struct memory_range elfcorehdr_mem;
 
 extern int load_crashdump_segments(struct kexec_info *info);
+extern void modify_ehdr_for_crashdump(struct mem_ehdr *ehdr);
 
 #endif /* CRASHDUMP_ARM64_H */
diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index bc96c76..498b218 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -305,12 +305,27 @@ unsigned long arm64_locate_kernel_segment(struct kexec_info *info)
 {
 	unsigned long hole;
 
-	hole = locate_hole(info,
-		arm64_mem.text_offset + arm64_mem.image_size,
-		MiB(2), 0, ULONG_MAX, 1);
+	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		unsigned long hole_end;
+
+		hole = (crash_reserved_mem.start < mem_min ?
+				mem_min : crash_reserved_mem.start);
+		hole = _ALIGN_UP(hole, MiB(2));
+		hole_end = hole + arm64_mem.text_offset + arm64_mem.image_size;
+
+		if ((hole_end > mem_max) ||
+		    (hole_end > crash_reserved_mem.end)) {
+			dbgprintf("%s: Crash kernel out of range\n", __func__);
+			hole = ULONG_MAX;
+		}
+	} else {
+		hole = locate_hole(info,
+			arm64_mem.text_offset + arm64_mem.image_size,
+			MiB(2), 0, ULONG_MAX, 1);
 
-	if (hole == ULONG_MAX)
-		dbgprintf("%s: locate_hole failed\n", __func__);
+		if (hole == ULONG_MAX)
+			dbgprintf("%s: locate_hole failed\n", __func__);
+	}
 
 	return hole;
 }
diff --git a/kexec/arch/arm64/kexec-elf-arm64.c b/kexec/arch/arm64/kexec-elf-arm64.c
index c70a37a..842ce21 100644
--- a/kexec/arch/arm64/kexec-elf-arm64.c
+++ b/kexec/arch/arm64/kexec-elf-arm64.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <linux/elf.h>
 
+#include "crashdump-arm64.h"
 #include "kexec-arm64.h"
 #include "kexec-elf.h"
 #include "kexec-syscall.h"
@@ -105,7 +106,8 @@ int elf_arm64_load(int argc, char **argv, const char *kernel_buf,
 	}
 
 	arm64_mem.vp_offset = _ALIGN_DOWN(ehdr.e_entry, MiB(2));
-	arm64_mem.vp_offset -= kernel_segment - get_phys_offset();
+	if (!(info->kexec_flags & KEXEC_ON_CRASH))
+		arm64_mem.vp_offset -= kernel_segment - get_phys_offset();
 
 	dbgprintf("%s: kernel_segment: %016lx\n", __func__, kernel_segment);
 	dbgprintf("%s: text_offset:    %016lx\n", __func__,
@@ -127,6 +129,12 @@ int elf_arm64_load(int argc, char **argv, const char *kernel_buf,
 								__func__);
 			goto exit;
 		}
+
+		/*
+		 * offset addresses in order to fit vmlinux
+		 * (elf_exec) into crash kernel's memory
+		 */
+		modify_ehdr_for_crashdump(&ehdr);
 	}
 
 	/* load the kernel */
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 6/8] arm64: kdump: set up other segments
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (4 preceding siblings ...)
  2016-09-07  4:33 ` [PATCH v3 5/8] arm64: kdump: set up kernel image segment AKASHI Takahiro
@ 2016-09-07  4:33 ` AKASHI Takahiro
  2016-09-07  4:34 ` [PATCH v3 7/8] arm64: kdump: add DT properties to crash dump kernel's dtb AKASHI Takahiro
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:33 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

We make sure that all the other segments, initrd and device-tree blob,
also be loaded into the reserved memory of crash dump kernel.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/kexec-arm64.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 498b218..bcb4e76 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -373,7 +373,10 @@ int arm64_load_other_segments(struct kexec_info *info,
 	/* Put the other segments after the image. */
 
 	hole_min = image_base + arm64_mem.image_size;
-	hole_max = ULONG_MAX;
+	if (info->kexec_flags & KEXEC_ON_CRASH)
+		hole_max = crash_reserved_mem.end;
+	else
+		hole_max = ULONG_MAX;
 
 	if (arm64_opts.initrd) {
 		initrd_buf = slurp_file(arm64_opts.initrd, &initrd_size);
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 7/8] arm64: kdump: add DT properties to crash dump kernel's dtb
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (5 preceding siblings ...)
  2016-09-07  4:33 ` [PATCH v3 6/8] arm64: kdump: set up other segments AKASHI Takahiro
@ 2016-09-07  4:34 ` AKASHI Takahiro
  2016-09-07  4:34 ` [PATCH v3 8/8] arm64: kdump: Add support for binary image files AKASHI Takahiro
  2016-09-29  7:52 ` [PATCH v3 0/8] (kexec-tools) arm64: add kdump support Simon Horman
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:34 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

We pass the following properties to crash dump kernel:
/chosen/linux,elfcorehdr: elf core header segment,
				same as "elfcorehdr=" as on other archs
/reserved-memory/crash_dump@xx: any memory regions to be dumped
				into /proc/vmcore

Then, we are ready to support kdump.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/crashdump-arm64.c | 141 ++++++++++++++++++++++++++++++++++++-
 kexec/arch/arm64/crashdump-arm64.h |   2 +
 kexec/arch/arm64/kexec-arm64.c     |  34 +++++++--
 kexec/arch/arm64/kexec-elf-arm64.c |   5 --
 4 files changed, 171 insertions(+), 11 deletions(-)

diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
index 9517329..01b9716 100644
--- a/kexec/arch/arm64/crashdump-arm64.c
+++ b/kexec/arch/arm64/crashdump-arm64.c
@@ -13,6 +13,7 @@
 #define _GNU_SOURCE
 
 #include <errno.h>
+#include <stdlib.h>
 #include <linux/elf.h>
 
 #include "kexec.h"
@@ -21,11 +22,12 @@
 #include "iomem.h"
 #include "kexec-arm64.h"
 #include "kexec-elf.h"
+#include "libfdt.h"
 #include "mem_regions.h"
 
 /* memory ranges on crashed kernel */
 static struct memory_range crash_memory_ranges[CRASH_MAX_MEMORY_RANGES];
-static struct memory_ranges crash_memory_rgns = {
+struct memory_ranges crash_memory_rgns = {
 	.size = 0,
 	.max_size = CRASH_MAX_MEMORY_RANGES,
 	.ranges = crash_memory_ranges,
@@ -236,3 +238,140 @@ void modify_ehdr_for_crashdump(struct mem_ehdr *ehdr)
 			(-arm64_mem.phys_offset + crash_reserved_mem.start);
 	}
 }
+
+static int dtb_add_reserved_memory(void *dtb_buf)
+{
+	int nodeoffset, rsvmem_node;
+	uint32_t cell_size;
+	uint64_t range[2];
+	char name[30];
+	int i, result = 0;
+
+	rsvmem_node = fdt_path_offset(dtb_buf, "reserved-memory");
+	if (rsvmem_node < 0) {
+		nodeoffset = fdt_path_offset(dtb_buf, "/");
+		rsvmem_node = fdt_add_subnode(dtb_buf, nodeoffset,
+						"reserved-memory");
+		if (rsvmem_node < 0) {
+			result = rsvmem_node;
+			goto on_error;
+		}
+
+		cell_size = cpu_to_fdt32(2);
+		fdt_setprop(dtb_buf, rsvmem_node, "#address-cells",
+						&cell_size, sizeof(cell_size));
+		fdt_setprop(dtb_buf, rsvmem_node, "#size-cells",
+						&cell_size, sizeof(cell_size));
+		fdt_setprop(dtb_buf, rsvmem_node, "ranges", NULL, 0);
+	}
+
+	for (i = 0; i < crash_memory_rgns.size; i++) {
+		sprintf(name, "crash_dump@%llx",
+					crash_memory_rgns.ranges[i].start);
+		nodeoffset = fdt_add_subnode(dtb_buf, rsvmem_node, name);
+		if (nodeoffset < 0) {
+			result = nodeoffset;
+			goto on_error;
+		}
+
+		range[0] = cpu_to_fdt64(crash_memory_rgns.ranges[i].start);
+		range[1] = cpu_to_fdt64(crash_memory_rgns.ranges[i].end
+					- crash_memory_rgns.ranges[i].start + 1);
+		fdt_setprop(dtb_buf, nodeoffset, "reg", &range, sizeof(range));
+		fdt_setprop(dtb_buf, nodeoffset, "no-map", NULL, 0);
+	}
+
+on_error:
+	return result;
+}
+
+/*
+ * Increased size for extra properties:
+ * - linux,elfcorehdr
+ *      linux,elfcoredhr = <base, size>;
+ * - reserved-memory node
+ *      reserved-memory {
+ *         #address-cells = <2>;
+ *         #size-cells = <2>;
+ *         ranges;
+ *         crash_dump@xx {
+ *            reg = <base, size>;
+ *            no-map;
+ *         };
+ *         ...
+ *      }
+ */
+#define DTB_ELFCOREHDR_PROP_SIZE \
+		 (sizeof(struct fdt_property)		\
+		+ FDT_TAGALIGN(sizeof(uint64_t) * 2)	\
+		+ strlen("linux,elfcorehdr") + 1)
+#define DTB_RESVMEM_NODE_SIZE \
+		 (sizeof(struct fdt_node_header)	\
+		+ strlen("reserved-memory") + 1		\
+		+ sizeof(struct fdt_property)		\
+		+ FDT_TAGALIGN(sizeof(uint32_t))	\
+		+ strlen("#address-cells") + 1		\
+		+ sizeof(struct fdt_property)		\
+		+ FDT_TAGALIGN(sizeof(uint32_t))	\
+		+ strlen("#size-cells") + 1		\
+		+ sizeof(struct fdt_property)		\
+		+ strlen("ranges") + 1)
+#define DTB_RESVMEM_SUBNODE_SIZE \
+		 (sizeof(struct fdt_node_header)	\
+		+ strlen("crash_dump@xxxxxxxxxxxxxxxx") + 1	\
+		+ sizeof(struct fdt_property)		\
+		+ FDT_TAGALIGN(sizeof(uint64_t) * 2)	\
+		+ strlen("reg") + 1			\
+		+ sizeof(struct fdt_property)	\
+		+ strlen("no-map") + 1)
+
+void *fixup_memory_properties(void *dtb_buf)
+{
+	char *new_buf;
+	int new_size;
+	uint64_t range[2];
+	int nodeoffset;
+	int result;
+
+	new_size = fdt_totalsize(dtb_buf)
+		   + DTB_ELFCOREHDR_PROP_SIZE
+		   + DTB_RESVMEM_NODE_SIZE
+		   + DTB_RESVMEM_SUBNODE_SIZE * crash_memory_rgns.size;
+
+	new_buf = xmalloc(new_size);
+	result = fdt_open_into(dtb_buf, new_buf, new_size);
+	if (result) {
+		dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
+						fdt_strerror(result));
+		result = -EFAILED;
+		goto on_error;
+	}
+
+	range[0] = cpu_to_fdt64(elfcorehdr_mem.start);
+	range[1] = cpu_to_fdt64(elfcorehdr_mem.end - elfcorehdr_mem.start + 1);
+	nodeoffset = fdt_path_offset(new_buf, "/chosen");
+	result = fdt_setprop(new_buf, nodeoffset, "linux,elfcorehdr",
+						(void *)range, sizeof(range));
+	if (result) {
+		dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
+						fdt_strerror(result));
+		goto on_error;
+	}
+
+	result = dtb_add_reserved_memory(new_buf);
+	if (result) {
+		dbgprintf("%s: adding reserved-memory failed: %s\n", __func__,
+						fdt_strerror(result));
+		result = -EFAILED;
+		goto on_error;
+	}
+
+	fdt_pack(new_buf);
+
+	return new_buf;
+
+on_error:
+	if (new_buf)
+		free(new_buf);
+	return NULL;
+}
diff --git a/kexec/arch/arm64/crashdump-arm64.h b/kexec/arch/arm64/crashdump-arm64.h
index 382f571..25889bc 100644
--- a/kexec/arch/arm64/crashdump-arm64.h
+++ b/kexec/arch/arm64/crashdump-arm64.h
@@ -16,11 +16,13 @@
 
 #define CRASH_MAX_MEMORY_RANGES	32
 
+extern struct memory_ranges crash_memory_rgns;
 extern struct memory_ranges usablemem_rgns;
 extern struct memory_range crash_reserved_mem;
 extern struct memory_range elfcorehdr_mem;
 
 extern int load_crashdump_segments(struct kexec_info *info);
 extern void modify_ehdr_for_crashdump(struct mem_ehdr *ehdr);
+extern void *fixup_memory_properties(void *dtb_buf);
 
 #endif /* CRASHDUMP_ARM64_H */
diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index bcb4e76..6f26f29 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -128,9 +128,6 @@ int arch_process_options(int argc, char **argv)
 		case OPT_INITRD:
 			arm64_opts.initrd = optarg;
 			break;
-		case OPT_PANIC:
-			die("load-panic (-p) not supported");
-			break;
 		default:
 			break; /* Ignore core and unknown options. */
 		}
@@ -283,8 +280,10 @@ on_success:
  * setup_2nd_dtb - Setup the 2nd stage kernel's dtb.
  */
 
-static int setup_2nd_dtb(struct dtb *dtb, char *command_line)
+static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 {
+	int nodeoffset;
+	void *new_buf;
 	int result;
 
 	result = fdt_check_header(dtb->buf);
@@ -296,8 +295,32 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line)
 
 	result = set_bootargs(dtb, command_line);
 
+	/*
+	 * Those properties are meaningful only for the current kernel.
+	 * So remove them anyway.
+	 */
+	nodeoffset = fdt_path_offset(dtb->buf, "/chosen");
+	fdt_delprop(dtb->buf, nodeoffset, "linux,crashkernel-base");
+	fdt_delprop(dtb->buf, nodeoffset, "linux,crashkernel-size");
+	fdt_delprop(dtb->buf, nodeoffset, "linux,elfcorehdr");
+
+	if (on_crash) {
+		new_buf = fixup_memory_properties(dtb->buf);
+		if (!new_buf)
+			goto on_error;
+
+		dtb->buf = new_buf;
+		dtb->size = fdt_totalsize(new_buf);
+	}
+
 	dump_reservemap(dtb);
 
+
+	return result;
+
+on_error:
+	fprintf(stderr, "kexec: %s failed.\n", __func__);
+
 	return result;
 }
 
@@ -365,7 +388,8 @@ int arm64_load_other_segments(struct kexec_info *info,
 		}
 	}
 
-	result = setup_2nd_dtb(&dtb, command_line);
+	result = setup_2nd_dtb(&dtb, command_line,
+			info->kexec_flags & KEXEC_ON_CRASH);
 
 	if (result)
 		return -EFAILED;
diff --git a/kexec/arch/arm64/kexec-elf-arm64.c b/kexec/arch/arm64/kexec-elf-arm64.c
index 842ce21..b17a31a 100644
--- a/kexec/arch/arm64/kexec-elf-arm64.c
+++ b/kexec/arch/arm64/kexec-elf-arm64.c
@@ -47,11 +47,6 @@ int elf_arm64_load(int argc, char **argv, const char *kernel_buf,
 	int result;
 	int i;
 
-	if (info->kexec_flags & KEXEC_ON_CRASH) {
-		fprintf(stderr, "kexec: kdump not yet supported on arm64\n");
-		return -EFAILED;
-	}
-
 	result = build_elf_exec_info(kernel_buf, kernel_size, &ehdr, 0);
 
 	if (result < 0) {
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v3 8/8] arm64: kdump: Add support for binary image files
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (6 preceding siblings ...)
  2016-09-07  4:34 ` [PATCH v3 7/8] arm64: kdump: add DT properties to crash dump kernel's dtb AKASHI Takahiro
@ 2016-09-07  4:34 ` AKASHI Takahiro
  2016-09-29  7:52 ` [PATCH v3 0/8] (kexec-tools) arm64: add kdump support Simon Horman
  8 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-07  4:34 UTC (permalink / raw)
  To: horms; +Cc: geoff, panand, kexec, AKASHI Takahiro

From: Pratyush Anand <panand@redhat.com>

This patch adds support to use binary image ie arch/arm64/boot/Image with
kdump.

Signed-off-by: Pratyush Anand <panand@redhat.com>
[takahiro.akashi@linaro.org: a bit reworked]
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 kexec/arch/arm64/kexec-image-arm64.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kexec/arch/arm64/kexec-image-arm64.c b/kexec/arch/arm64/kexec-image-arm64.c
index 960ed96..982e431 100644
--- a/kexec/arch/arm64/kexec-image-arm64.c
+++ b/kexec/arch/arm64/kexec-image-arm64.c
@@ -4,7 +4,9 @@
 
 #define _GNU_SOURCE
 
+#include "crashdump-arm64.h"
 #include "kexec-arm64.h"
+#include "kexec-syscall.h"
 #include <limits.h>
 
 int image_arm64_probe(const char *kernel_buf, off_t kernel_size)
@@ -58,6 +60,16 @@ int image_arm64_load(int argc, char **argv, const char *kernel_buf,
 	dbgprintf("%s: PE format:      %s\n", __func__,
 		(arm64_header_check_pe_sig(header) ? "yes" : "no"));
 
+	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		/* create and initialize elf core header segment */
+		result = load_crashdump_segments(info);
+		if (result) {
+			dbgprintf("%s: Creating eflcorehdr failed.\n",
+								__func__);
+			goto exit;
+		}
+	}
+
 	/* load the kernel */
 	add_segment_phys_virt(info, kernel_buf, kernel_size,
 			kernel_segment + arm64_mem.text_offset,
-- 
2.9.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly
  2016-09-07  4:33 ` [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly AKASHI Takahiro
@ 2016-09-27 15:49   ` Pratyush Anand
  2016-09-28  7:48     ` AKASHI Takahiro
  0 siblings, 1 reply; 20+ messages in thread
From: Pratyush Anand @ 2016-09-27 15:49 UTC (permalink / raw)
  To: AKASHI Takahiro
  Cc: Geoff Levand, Pratyush Anand, Simon Horman, Kexec Mailing List

On Wed, Sep 7, 2016 at 10:03 AM, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> Due to the kernel patch[1], the current code will not be able to identify

[1] is mentioned in cover letter only, not here.

> the correct value of PHYS_OFFSET if some "reserved" memory region, which
> is likely to be UEFI runtime services code/data, exists at an address below
> the first "System RAM" regions.
>
> This patch fixes this issue.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  kexec/arch/arm64/iomem.h       |  7 +++++++
>  kexec/arch/arm64/kexec-arm64.c | 22 +++++++++++++++++-----
>  2 files changed, 24 insertions(+), 5 deletions(-)
>  create mode 100644 kexec/arch/arm64/iomem.h
>
> diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
> new file mode 100644
> index 0000000..7fd66eb
> --- /dev/null
> +++ b/kexec/arch/arm64/iomem.h
> @@ -0,0 +1,7 @@
> +#ifndef IOMEM_H
> +#define IOMEM_H
> +
> +#define SYSTEM_RAM             "System RAM\n"
> +#define IOMEM_RESERVED         "reserved\n"
> +
> +#endif
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 7183dac..bc96c76 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -21,6 +21,7 @@
>  #include "crashdump-arm64.h"
>  #include "dt-ops.h"
>  #include "fs2dt.h"
> +#include "iomem.h"
>  #include "kexec-syscall.h"
>  #include "arch/options.h"
>
> @@ -465,18 +466,28 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
>   * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
>   */
>
> +static int count_memory_ranges;

IMHO improving definition of kexec_iomem_for_each_line() and
callback() for handling NULL match case would have been a better
choice than introducing a new static variable. callback() can return
-ve in case of error, 0 in case of no match and 1 in case of (one)
match.

~Pratyush

> +
>  static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>         unsigned long long base, unsigned long long length)
>  {
>         struct memory_range *r;
>
> -       if (nr >= KEXEC_SEGMENT_MAX)
> +       if (count_memory_ranges >= KEXEC_SEGMENT_MAX)
>                 return -1;
>
> -       r = (struct memory_range *)data + nr;
> -       r->type = RANGE_RAM;
> +       r = (struct memory_range *)data + count_memory_ranges;
> +
> +       if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)))
> +               r->type = RANGE_RAM;
> +       else if (!strncmp(str, IOMEM_RESERVED, strlen(IOMEM_RESERVED)))
> +               r->type = RANGE_RESERVED;
> +       else
> +               return 0;
> +
>         r->start = base;
>         r->end = base + length - 1;
> +       count_memory_ranges++;
>
>         set_phys_offset(r->start);
>
> @@ -493,9 +504,10 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>  static int get_memory_ranges_iomem(struct memory_range *array,
>         unsigned int *count)
>  {
> -       *count = kexec_iomem_for_each_line("System RAM\n",
> -               get_memory_ranges_iomem_cb, array);
> +       count_memory_ranges = 0;
> +       kexec_iomem_for_each_line(NULL, get_memory_ranges_iomem_cb, array);
>
> +       *count = count_memory_ranges;
>         if (!*count) {
>                 dbgprintf("%s: failed: No RAM found.\n", __func__);
>                 return -EFAILED;
> --
> 2.9.0
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly
  2016-09-27 15:49   ` Pratyush Anand
@ 2016-09-28  7:48     ` AKASHI Takahiro
  2016-10-24 23:02       ` Goel, Sameer
  0 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-28  7:48 UTC (permalink / raw)
  To: Pratyush Anand; +Cc: Geoff Levand, Simon Horman, Kexec Mailing List

On Tue, Sep 27, 2016 at 09:19:51PM +0530, Pratyush Anand wrote:
> On Wed, Sep 7, 2016 at 10:03 AM, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> > Due to the kernel patch[1], the current code will not be able to identify
> 
> [1] is mentioned in cover letter only, not here.

Oops, thanks.

> > the correct value of PHYS_OFFSET if some "reserved" memory region, which
> > is likely to be UEFI runtime services code/data, exists at an address below
> > the first "System RAM" regions.
> >
> > This patch fixes this issue.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  kexec/arch/arm64/iomem.h       |  7 +++++++
> >  kexec/arch/arm64/kexec-arm64.c | 22 +++++++++++++++++-----
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> >  create mode 100644 kexec/arch/arm64/iomem.h
> >
> > diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
> > new file mode 100644
> > index 0000000..7fd66eb
> > --- /dev/null
> > +++ b/kexec/arch/arm64/iomem.h
> > @@ -0,0 +1,7 @@
> > +#ifndef IOMEM_H
> > +#define IOMEM_H
> > +
> > +#define SYSTEM_RAM             "System RAM\n"
> > +#define IOMEM_RESERVED         "reserved\n"
> > +
> > +#endif
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index 7183dac..bc96c76 100644
> > --- a/kexec/arch/arm64/kexec-arm64.c
> > +++ b/kexec/arch/arm64/kexec-arm64.c
> > @@ -21,6 +21,7 @@
> >  #include "crashdump-arm64.h"
> >  #include "dt-ops.h"
> >  #include "fs2dt.h"
> > +#include "iomem.h"
> >  #include "kexec-syscall.h"
> >  #include "arch/options.h"
> >
> > @@ -465,18 +466,28 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
> >   * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
> >   */
> >
> > +static int count_memory_ranges;
> 
> IMHO improving definition of kexec_iomem_for_each_line() and
> callback() for handling NULL match case would have been a better
> choice than introducing a new static variable. callback() can return
> -ve in case of error, 0 in case of no match and 1 in case of (one)
> match.

Maybe. But we have to be careful not to change other arch's results.
Or follow arm's approach.

-Takahiro AKASHI

> ~Pratyush
> 
> > +
> >  static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
> >         unsigned long long base, unsigned long long length)
> >  {
> >         struct memory_range *r;
> >
> > -       if (nr >= KEXEC_SEGMENT_MAX)
> > +       if (count_memory_ranges >= KEXEC_SEGMENT_MAX)
> >                 return -1;
> >
> > -       r = (struct memory_range *)data + nr;
> > -       r->type = RANGE_RAM;
> > +       r = (struct memory_range *)data + count_memory_ranges;
> > +
> > +       if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)))
> > +               r->type = RANGE_RAM;
> > +       else if (!strncmp(str, IOMEM_RESERVED, strlen(IOMEM_RESERVED)))
> > +               r->type = RANGE_RESERVED;
> > +       else
> > +               return 0;
> > +
> >         r->start = base;
> >         r->end = base + length - 1;
> > +       count_memory_ranges++;
> >
> >         set_phys_offset(r->start);
> >
> > @@ -493,9 +504,10 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
> >  static int get_memory_ranges_iomem(struct memory_range *array,
> >         unsigned int *count)
> >  {
> > -       *count = kexec_iomem_for_each_line("System RAM\n",
> > -               get_memory_ranges_iomem_cb, array);
> > +       count_memory_ranges = 0;
> > +       kexec_iomem_for_each_line(NULL, get_memory_ranges_iomem_cb, array);
> >
> > +       *count = count_memory_ranges;
> >         if (!*count) {
> >                 dbgprintf("%s: failed: No RAM found.\n", __func__);
> >                 return -EFAILED;
> > --
> > 2.9.0
> >

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 0/8] (kexec-tools) arm64: add kdump support
  2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
                   ` (7 preceding siblings ...)
  2016-09-07  4:34 ` [PATCH v3 8/8] arm64: kdump: Add support for binary image files AKASHI Takahiro
@ 2016-09-29  7:52 ` Simon Horman
  2016-09-29  8:18   ` AKASHI Takahiro
  8 siblings, 1 reply; 20+ messages in thread
From: Simon Horman @ 2016-09-29  7:52 UTC (permalink / raw)
  To: AKASHI Takahiro; +Cc: geoff, panand, kexec

On Wed, Sep 07, 2016 at 01:33:53PM +0900, AKASHI Takahiro wrote:
> My kernel patches of kdump suport on arm64 are currently under reviews.
> 
> This patchset is synced with them (v26 [1]) and provides necessary changes
> for kexec-tools. It should be applied on top of Geoff's kexec-tools patches
> v5[2] along with a bugfix[3].
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454588.html
> [2] http://lists.infradead.org/pipermail/kexec/2016-September/017110.html
> [3] http://lists.infradead.org/pipermail/kexec/2016-July/016664.html

Unfortunately patch 2 does not seem to apply cleanly any more.
Could you consider rebasing if you think this series is ready to be
applied?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 0/8] (kexec-tools) arm64: add kdump support
  2016-09-29  7:52 ` [PATCH v3 0/8] (kexec-tools) arm64: add kdump support Simon Horman
@ 2016-09-29  8:18   ` AKASHI Takahiro
  2016-09-29  8:39     ` Simon Horman
  0 siblings, 1 reply; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-29  8:18 UTC (permalink / raw)
  To: Simon Horman; +Cc: geoff, panand, kexec

On Thu, Sep 29, 2016 at 09:52:00AM +0200, Simon Horman wrote:
> On Wed, Sep 07, 2016 at 01:33:53PM +0900, AKASHI Takahiro wrote:
> > My kernel patches of kdump suport on arm64 are currently under reviews.
> > 
> > This patchset is synced with them (v26 [1]) and provides necessary changes
> > for kexec-tools. It should be applied on top of Geoff's kexec-tools patches
> > v5[2] along with a bugfix[3].
> > 
> > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454588.html
> > [2] http://lists.infradead.org/pipermail/kexec/2016-September/017110.html
> > [3] http://lists.infradead.org/pipermail/kexec/2016-July/016664.html
> 
> Unfortunately patch 2 does not seem to apply cleanly any more.
> Could you consider rebasing if you think this series is ready to be
> applied?

Yes, I will as there will be some changes needed again due to the discussions
on my kernel patch.

BTW, can you give me your opinion on my question, please?

http://lists.infradead.org/pipermail/kexec/2016-September/017119.html

Thanks,
-Takahiro AKASHI

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 0/8] (kexec-tools) arm64: add kdump support
  2016-09-29  8:18   ` AKASHI Takahiro
@ 2016-09-29  8:39     ` Simon Horman
  2016-09-29 14:26       ` AKASHI Takahiro
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Horman @ 2016-09-29  8:39 UTC (permalink / raw)
  To: AKASHI Takahiro, geoff, panand, kexec

On Thu, Sep 29, 2016 at 01:18:06AM -0700, AKASHI Takahiro wrote:
> On Thu, Sep 29, 2016 at 09:52:00AM +0200, Simon Horman wrote:
> > On Wed, Sep 07, 2016 at 01:33:53PM +0900, AKASHI Takahiro wrote:
> > > My kernel patches of kdump suport on arm64 are currently under reviews.
> > > 
> > > This patchset is synced with them (v26 [1]) and provides necessary changes
> > > for kexec-tools. It should be applied on top of Geoff's kexec-tools patches
> > > v5[2] along with a bugfix[3].
> > > 
> > > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454588.html
> > > [2] http://lists.infradead.org/pipermail/kexec/2016-September/017110.html
> > > [3] http://lists.infradead.org/pipermail/kexec/2016-July/016664.html
> > 
> > Unfortunately patch 2 does not seem to apply cleanly any more.
> > Could you consider rebasing if you think this series is ready to be
> > applied?
> 
> Yes, I will as there will be some changes needed again due to the discussions
> on my kernel patch.
> 
> BTW, can you give me your opinion on my question, please?
> 
> http://lists.infradead.org/pipermail/kexec/2016-September/017119.html

I'm not particularly familiar with UEFI systems but for DT something under
chosen seems to make sense.

Regarding Mark's objections to a memmap= command line parameter.
Perhaps that could be discussed further in the context that even
though it is not particularly attractive it it being used on other
architecture(s).

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 0/8] (kexec-tools) arm64: add kdump support
  2016-09-29  8:39     ` Simon Horman
@ 2016-09-29 14:26       ` AKASHI Takahiro
  0 siblings, 0 replies; 20+ messages in thread
From: AKASHI Takahiro @ 2016-09-29 14:26 UTC (permalink / raw)
  To: Simon Horman; +Cc: geoff, panand, kexec

Simon,

On Thu, Sep 29, 2016 at 10:39:09AM +0200, Simon Horman wrote:
> On Thu, Sep 29, 2016 at 01:18:06AM -0700, AKASHI Takahiro wrote:
> > On Thu, Sep 29, 2016 at 09:52:00AM +0200, Simon Horman wrote:
> > > On Wed, Sep 07, 2016 at 01:33:53PM +0900, AKASHI Takahiro wrote:
> > > > My kernel patches of kdump suport on arm64 are currently under reviews.
> > > > 
> > > > This patchset is synced with them (v26 [1]) and provides necessary changes
> > > > for kexec-tools. It should be applied on top of Geoff's kexec-tools patches
> > > > v5[2] along with a bugfix[3].
> > > > 
> > > > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454588.html
> > > > [2] http://lists.infradead.org/pipermail/kexec/2016-September/017110.html
> > > > [3] http://lists.infradead.org/pipermail/kexec/2016-July/016664.html
> > > 
> > > Unfortunately patch 2 does not seem to apply cleanly any more.
> > > Could you consider rebasing if you think this series is ready to be
> > > applied?
> > 
> > Yes, I will as there will be some changes needed again due to the discussions
> > on my kernel patch.
> > 
> > BTW, can you give me your opinion on my question, please?
> > 
> > http://lists.infradead.org/pipermail/kexec/2016-September/017119.html
> 
> I'm not particularly familiar with UEFI systems but for DT something under
> chosen seems to make sense.
> 
> Regarding Mark's objections to a memmap= command line parameter.
> Perhaps that could be discussed further in the context that even
> though it is not particularly attractive it it being used on other
> architecture(s).


Oops, my question was not accurate here.
Instead, see the discussions:
http://lists.infradead.org/pipermail/kexec/2016-July/016686.html

The issue is the end address in a memory range can be handled
differently across various architectures.
So we are in a messy situation.

Thanks,
-Takahiro AKASHI



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()
  2016-09-07  4:33 ` [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym() AKASHI Takahiro
@ 2016-10-06 13:28   ` Matthias Brugger
  2016-10-07  4:18     ` Dave Young
  2016-10-07  6:44     ` Pratyush Anand
  0 siblings, 2 replies; 20+ messages in thread
From: Matthias Brugger @ 2016-10-06 13:28 UTC (permalink / raw)
  To: kexec
  Cc: public-panand-H+wXaHxf7aLQT0dZR+AlfA,
	public-kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	public-geoff-wEGCiKHe2LqWVfeAwA7xHQ

On 09/07/2016 06:33 AM, AKASHI Takahiro wrote:
> From: Pratyush Anand <panand@redhat.com>
>
> get_kernel_stext_sym() has been defined for both arm and i386. Other
> architecture might need some other kernel symbol address. Therefore rewrite
> this function as generic function to get any kernel symbol address.
>
> More over, kallsyms is not arch specific representation, therefore have
> common function for all arches.
>
> Signed-off-by: Pratyush Anand <panand@redhat.com>
> [created symbols.c]
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  kexec/Makefile                  |  1 +
>  kexec/arch/arm/crashdump-arm.c  | 40 +---------------------------------------
>  kexec/arch/i386/crashdump-x86.c | 32 +-------------------------------
>  kexec/kexec.h                   |  2 ++
>  kexec/symbols.c                 | 41 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 46 insertions(+), 70 deletions(-)
>  create mode 100644 kexec/symbols.c
>
> diff --git a/kexec/Makefile b/kexec/Makefile
> index 39f365f..2b4fb3d 100644
> --- a/kexec/Makefile
> +++ b/kexec/Makefile
> @@ -26,6 +26,7 @@ KEXEC_SRCS_base += kexec/kernel_version.c
>  KEXEC_SRCS_base += kexec/lzma.c
>  KEXEC_SRCS_base += kexec/zlib.c
>  KEXEC_SRCS_base += kexec/kexec-xen.c
> +KEXEC_SRCS_base += kexec/symbols.c
>
>  KEXEC_GENERATED_SRCS += $(PURGATORY_HEX_C)
>
> diff --git a/kexec/arch/arm/crashdump-arm.c b/kexec/arch/arm/crashdump-arm.c
> index 4a89b5e..2bc898b 100644
> --- a/kexec/arch/arm/crashdump-arm.c
> +++ b/kexec/arch/arm/crashdump-arm.c
> @@ -73,48 +73,10 @@ static struct crash_elf_info elf_info = {
>
>  extern unsigned long long user_page_offset;
>
> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
> -static unsigned long long get_kernel_stext_sym(void)
> -{
> -	const char *kallsyms = "/proc/kallsyms";
> -	const char *stext = "_stext";
> -	char sym[128];
> -	char line[128];
> -	FILE *fp;
> -	unsigned long long vaddr = 0;
> -	char type;
> -
> -	fp = fopen(kallsyms, "r");
> -	if (!fp) {
> -		fprintf(stderr, "Cannot open %s\n", kallsyms);
> -		return 0;
> -	}
> -
> -	while(fgets(line, sizeof(line), fp) != NULL) {
> -		unsigned long long addr;
> -
> -		if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
> -			continue;
> -
> -		if (strcmp(sym, stext) == 0) {
> -			dbgprintf("kernel symbol %s vaddr = %#llx\n", stext, addr);
> -			vaddr = addr;
> -			break;
> -		}
> -	}
> -
> -	fclose(fp);
> -
> -	if (vaddr == 0)
> -		fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
> -
> -	return vaddr;
> -}
> -
>  static int get_kernel_page_offset(struct kexec_info *info,
>  		struct crash_elf_info *elf_info)
>  {
> -	unsigned long long stext_sym_addr = get_kernel_stext_sym();
> +	unsigned long long stext_sym_addr = get_kernel_sym("stext");
>  	if (stext_sym_addr == 0) {
>  		if (user_page_offset != (-1ULL)) {
>  			elf_info->page_offset = user_page_offset;
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index bbc0f35..664eb3b 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -102,36 +102,6 @@ static int get_kernel_paddr(struct kexec_info *UNUSED(info),
>  	return -1;
>  }
>
> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
> -static unsigned long long get_kernel_stext_sym(void)
> -{
> -	const char *kallsyms = "/proc/kallsyms";
> -	const char *stext = "_stext";
> -	char sym[128];
> -	char line[128];
> -	FILE *fp;
> -	unsigned long long vaddr;
> -	char type;
> -
> -	fp = fopen(kallsyms, "r");
> -	if (!fp) {
> -		fprintf(stderr, "Cannot open %s\n", kallsyms);
> -		return 0;
> -	}
> -
> -	while(fgets(line, sizeof(line), fp) != NULL) {
> -		if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
> -			continue;
> -		if (strcmp(sym, stext) == 0) {
> -			dbgprintf("kernel symbol %s vaddr = %16llx\n", stext, vaddr);
> -			return vaddr;
> -		}
> -	}
> -
> -	fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
> -	return 0;
> -}
> -
>  /* Retrieve info regarding virtual address kernel has been compiled for and
>   * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
>   * from kexec-tools fails because of malformed elf notes. A kernel patch has
> @@ -182,7 +152,7 @@ static int get_kernel_vaddr_and_size(struct kexec_info *UNUSED(info),
>
>  	/* Traverse through the Elf headers and find the region where
>  	 * _stext symbol is located in. That's where kernel is mapped */
> -	stext_sym = get_kernel_stext_sym();
> +	stext_sym = get_kernel_sym("stext");


I think this should be get_kernel_sym("_stext");

Apart from that as Simon already mentioned, due to commit
9f62cbd ("kexec/arch/i386: Add support for KASLR memory randomization")
this patch does not apply cleanly.

Regards,
Matthias

>  	for(phdr = ehdr.e_phdr; stext_sym && phdr != end_phdr; phdr++) {
>  		if (phdr->p_type == PT_LOAD) {
>  			unsigned long long saddr = phdr->p_vaddr;
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 9194f1c..b4fafad 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -312,4 +312,6 @@ int xen_kexec_load(struct kexec_info *info);
>  int xen_kexec_unload(uint64_t kexec_flags);
>  void xen_kexec_exec(void);
>
> +extern unsigned long long get_kernel_sym(const char *text);
> +
>  #endif /* KEXEC_H */
> diff --git a/kexec/symbols.c b/kexec/symbols.c
> new file mode 100644
> index 0000000..ea6e327
> --- /dev/null
> +++ b/kexec/symbols.c
> @@ -0,0 +1,41 @@
> +#include <stdio.h>
> +#include <string.h>
> +#include "kexec.h"
> +
> +/* Retrieve kernel symbol virtual address from /proc/kallsyms */
> +unsigned long long get_kernel_sym(const char *text)
> +{
> +	const char *kallsyms = "/proc/kallsyms";
> +	char sym[128];
> +	char line[128];
> +	FILE *fp;
> +	unsigned long long vaddr = 0;
> +	char type;
> +
> +	fp = fopen(kallsyms, "r");
> +	if (!fp) {
> +		fprintf(stderr, "Cannot open %s\n", kallsyms);
> +		return 0;
> +	}
> +
> +	while (fgets(line, sizeof(line), fp) != NULL) {
> +		unsigned long long addr;
> +
> +		if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
> +			continue;
> +
> +		if (strcmp(sym, text) == 0) {
> +			dbgprintf("kernel symbol %s vaddr = %#llx\n",
> +								text, addr);
> +			vaddr = addr;
> +			break;
> +		}
> +	}
> +
> +	fclose(fp);
> +
> +	if (vaddr == 0)
> +		fprintf(stderr, "Cannot get kernel %s symbol address\n", text);
> +
> +	return vaddr;
> +}
>



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()
  2016-10-06 13:28   ` Matthias Brugger
@ 2016-10-07  4:18     ` Dave Young
  2016-10-07  6:41       ` Pratyush Anand
  2016-10-07  6:44     ` Pratyush Anand
  1 sibling, 1 reply; 20+ messages in thread
From: Dave Young @ 2016-10-07  4:18 UTC (permalink / raw)
  To: Matthias Brugger, panand
  Cc: public-panand-H+wXaHxf7aLQT0dZR+AlfA,
	public-kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, kexec,
	public-geoff-wEGCiKHe2LqWVfeAwA7xHQ

> >  	/* Traverse through the Elf headers and find the region where
> >  	 * _stext symbol is located in. That's where kernel is mapped */
> > -	stext_sym = get_kernel_stext_sym();
> > +	stext_sym = get_kernel_sym("stext");
> 
> 
> I think this should be get_kernel_sym("_stext");
> 
> Apart from that as Simon already mentioned, due to commit
> 9f62cbd ("kexec/arch/i386: Add support for KASLR memory randomization")
> this patch does not apply cleanly.

Pratyush, I had a cleanup patch below, but I did not get time to test it
so it is not ready to send out.

The basic thought is to move the page_offset_base code to
get_kernel_page_offset(), there is also another issue is for old kernel
or kernel without kaslr built-in there will be an unnecessary error
message but I have not get any idea how to fix it.

Would you like to take below cleanup patch along with your next version?
It is also fine to leave it as a future improvement to me.

Thanks
Dave

---
 kexec/arch/i386/crashdump-x86.c |  158 ++++++++++++++++++++--------------------
 1 file changed, 82 insertions(+), 76 deletions(-)

--- kexec-tools.orig/kexec/arch/i386/crashdump-x86.c
+++ kexec-tools/kexec/arch/i386/crashdump-x86.c
@@ -54,10 +54,59 @@
 
 extern struct arch_options_t arch_options;
 
+/* Retrieve kernel symbol virtual address from /proc/kallsyms */
+static unsigned long long get_kernel_sym(const char *symbol)
+{
+	const char *kallsyms = "/proc/kallsyms";
+	char sym[128];
+	char line[128];
+	FILE *fp;
+	unsigned long long vaddr;
+	char type;
+
+	fp = fopen(kallsyms, "r");
+	if (!fp) {
+		fprintf(stderr, "Cannot open %s\n", kallsyms);
+		return 0;
+	}
+
+	while(fgets(line, sizeof(line), fp) != NULL) {
+		if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
+			continue;
+		if (strcmp(sym, symbol) == 0) {
+			dbgprintf("kernel symbol %s vaddr = %16llx\n", symbol, vaddr);
+			return vaddr;
+		}
+	}
+
+	fprintf(stderr, "Cannot get kernel %s symbol address\n", symbol);
+
+	return 0;
+}
+
 static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
-				  struct crash_elf_info *elf_info)
+				  struct crash_elf_info *elf_info,
+				  struct mem_ehdr *ehdr)
 {
 	int kv;
+	struct mem_phdr *phdr, *end_phdr;
+	const unsigned long long pud_mask = ~((1 << 30) - 1);
+	unsigned long long vaddr, lowest_vaddr = 0;
+
+	end_phdr = &ehdr->e_phdr[ehdr->e_phnum];
+	/* Search for the real PAGE_OFFSET when KASLR memory randomization
+	 * is enabled */
+	if (get_kernel_sym("page_offset_base") != 0) {
+		for(phdr = ehdr->e_phdr; phdr != end_phdr; phdr++) {
+			if (phdr->p_type == PT_LOAD) {
+				vaddr = phdr->p_vaddr & pud_mask;
+				if (lowest_vaddr == 0 || lowest_vaddr > vaddr)
+					lowest_vaddr = vaddr;
+			}
+		}
+		if (lowest_vaddr != 0)
+			elf_info->page_offset = lowest_vaddr;
+	}
 
 	if (elf_info->machine == EM_X86_64) {
 		kv = kernel_version();
@@ -102,35 +151,6 @@ static int get_kernel_paddr(struct kexec
 	return -1;
 }
 
-/* Retrieve kernel symbol virtual address from /proc/kallsyms */
-static unsigned long long get_kernel_sym(const char *symbol)
-{
-	const char *kallsyms = "/proc/kallsyms";
-	char sym[128];
-	char line[128];
-	FILE *fp;
-	unsigned long long vaddr;
-	char type;
-
-	fp = fopen(kallsyms, "r");
-	if (!fp) {
-		fprintf(stderr, "Cannot open %s\n", kallsyms);
-		return 0;
-	}
-
-	while(fgets(line, sizeof(line), fp) != NULL) {
-		if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
-			continue;
-		if (strcmp(sym, symbol) == 0) {
-			dbgprintf("kernel symbol %s vaddr = %16llx\n", symbol, vaddr);
-			return vaddr;
-		}
-	}
-
-	fprintf(stderr, "Cannot get kernel %s symbol address\n", symbol);
-	return 0;
-}
-
 /* Retrieve info regarding virtual address kernel has been compiled for and
  * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
  * from kexec-tools fails because of malformed elf notes. A kernel patch has
@@ -139,19 +159,12 @@ static unsigned long long get_kernel_sym
  * we should get rid of backward compatible code. */
 
 static int get_kernel_vaddr_and_size(struct kexec_info *UNUSED(info),
-				     struct crash_elf_info *elf_info)
+				     struct crash_elf_info *elf_info,
+				     struct mem_ehdr *ehdr)
 {
-	int result;
-	const char kcore[] = "/proc/kcore";
-	char *buf;
-	struct mem_ehdr ehdr;
 	struct mem_phdr *phdr, *end_phdr;
 	int align;
-	off_t size;
-	uint32_t elf_flags = 0;
 	uint64_t stext_sym;
-	const unsigned long long pud_mask = ~((1 << 30) - 1);
-	unsigned long long vaddr, lowest_vaddr = 0;
 
 	if (elf_info->machine != EM_X86_64)
 		return 0;
@@ -160,45 +173,13 @@ static int get_kernel_vaddr_and_size(str
 		return 0;
 
 	align = getpagesize();
-	buf = slurp_file_len(kcore, KCORE_ELF_HEADERS_SIZE, &size);
-	if (!buf) {
-		fprintf(stderr, "Cannot read %s: %s\n", kcore, strerror(errno));
-		return -1;
-	}
 
-	/* Don't perform checks to make sure stated phdrs and shdrs are
-	 * actually present in the core file. It is not practical
-	 * to read the GB size file into a user space buffer, Given the
-	 * fact that we don't use any info from that.
-	 */
-	elf_flags |= ELF_SKIP_FILESZ_CHECK;
-	result = build_elf_core_info(buf, size, &ehdr, elf_flags);
-	if (result < 0) {
-		/* Perhaps KCORE_ELF_HEADERS_SIZE is too small? */
-		fprintf(stderr, "ELF core (kcore) parse failed\n");
-		return -1;
-	}
-
-	end_phdr = &ehdr.e_phdr[ehdr.e_phnum];
-
-	/* Search for the real PAGE_OFFSET when KASLR memory randomization
-	 * is enabled */
-	if (get_kernel_sym("page_offset_base") != 0) {
-		for(phdr = ehdr.e_phdr; phdr != end_phdr; phdr++) {
-			if (phdr->p_type == PT_LOAD) {
-				vaddr = phdr->p_vaddr & pud_mask;
-				if (lowest_vaddr == 0 || lowest_vaddr > vaddr)
-					lowest_vaddr = vaddr;
-			}
-		}
-		if (lowest_vaddr != 0)
-			elf_info->page_offset = lowest_vaddr;
-	}
+	end_phdr = &ehdr->e_phdr[ehdr->e_phnum];
 
 	/* Traverse through the Elf headers and find the region where
 	 * _stext symbol is located in. That's where kernel is mapped */
 	stext_sym = get_kernel_sym("_stext");
-	for(phdr = ehdr.e_phdr; stext_sym && phdr != end_phdr; phdr++) {
+	for(phdr = ehdr->e_phdr; stext_sym && phdr != end_phdr; phdr++) {
 		if (phdr->p_type == PT_LOAD) {
 			unsigned long long saddr = phdr->p_vaddr;
 			unsigned long long eaddr = phdr->p_vaddr + phdr->p_memsz;
@@ -223,7 +204,7 @@ static int get_kernel_vaddr_and_size(str
 	 * /proc/kallsyms, Traverse through the Elf headers again and
 	 * find the region where kernel is mapped using hard-coded
 	 * kernel mapping boundries */
-	for(phdr = ehdr.e_phdr; phdr != end_phdr; phdr++) {
+	for(phdr = ehdr->e_phdr; phdr != end_phdr; phdr++) {
 		if (phdr->p_type == PT_LOAD) {
 			unsigned long long saddr = phdr->p_vaddr;
 			unsigned long long eaddr = phdr->p_vaddr + phdr->p_memsz;
@@ -887,6 +868,12 @@ int load_crashdump_segments(struct kexec
 	struct memory_range *mem_range, *memmap_p;
 	struct crash_elf_info elf_info;
 	unsigned kexec_arch;
+	char *buf;
+	off_t size;
+	struct mem_ehdr ehdr;
+	uint32_t elf_flags = 0;
+	const char kcore[] = "/proc/kcore";
+	int result;
 
 	memset(&elf_info, 0x0, sizeof(elf_info));
 
@@ -943,13 +930,32 @@ int load_crashdump_segments(struct kexec
 		elf_info.class = ELFCLASS64;
 	}
 
-	if (get_kernel_page_offset(info, &elf_info))
+	buf = slurp_file_len(kcore, KCORE_ELF_HEADERS_SIZE, &size);
+	if (!buf) {
+		fprintf(stderr, "Cannot read %s: %s\n", kcore, strerror(errno));
+		return -1;
+	}
+
+	/* Don't perform checks to make sure stated phdrs and shdrs are
+	 * actually present in the core file. It is not practical
+	 * to read the GB size file into a user space buffer, Given the
+	 * fact that we don't use any info from that.
+	 */
+	elf_flags |= ELF_SKIP_FILESZ_CHECK;
+	result = build_elf_core_info(buf, size, &ehdr, elf_flags);
+	if (result < 0) {
+		/* Perhaps KCORE_ELF_HEADERS_SIZE is too small? */
+		fprintf(stderr, "ELF core (kcore) parse failed\n");
+		return -1;
+	}
+
+	if (get_kernel_page_offset(info, &elf_info, &ehdr))
 		return -1;
 
 	if (get_kernel_paddr(info, &elf_info))
 		return -1;
 
-	if (get_kernel_vaddr_and_size(info, &elf_info))
+	if (get_kernel_vaddr_and_size(info, &elf_info, &ehdr))
 		return -1;
 
 	/* Memory regions which panic kernel can safely use to boot into */

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()
  2016-10-07  4:18     ` Dave Young
@ 2016-10-07  6:41       ` Pratyush Anand
  0 siblings, 0 replies; 20+ messages in thread
From: Pratyush Anand @ 2016-10-07  6:41 UTC (permalink / raw)
  To: Dave Young
  Cc: Matthias Brugger, public-panand-H+wXaHxf7aLQT0dZR+AlfA,
	public-kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Kexec Mailing List, public-geoff-wEGCiKHe2LqWVfeAwA7xHQ

Hi Dave,

On Fri, Oct 7, 2016 at 9:48 AM, Dave Young <dyoung@redhat.com> wrote:
>> >     /* Traverse through the Elf headers and find the region where
>> >      * _stext symbol is located in. That's where kernel is mapped */
>> > -   stext_sym = get_kernel_stext_sym();
>> > +   stext_sym = get_kernel_sym("stext");
>>
>>
>> I think this should be get_kernel_sym("_stext");
>>
>> Apart from that as Simon already mentioned, due to commit
>> 9f62cbd ("kexec/arch/i386: Add support for KASLR memory randomization")
>> this patch does not apply cleanly.
>
> Pratyush, I had a cleanup patch below, but I did not get time to test it
> so it is not ready to send out.
>
> The basic thought is to move the page_offset_base code to
> get_kernel_page_offset(), there is also another issue is for old kernel
> or kernel without kaslr built-in there will be an unnecessary error
> message but I have not get any idea how to fix it.
>

Are you talking about following error message:

"Cannot get kernel %s symbol address\n"

May be it can be changes as dbgprintf() rather than fprintf(stderr)

> Would you like to take below cleanup patch along with your next version?

I think, this patch series is for ARM64 support along with any generic
modifications needed by arm64 patches. Below modification is specific
to x86. So, better to send them separately.

> It is also fine to leave it as a future improvement to me.

OK.

>
> Thanks
> Dave
>
> ---
>  kexec/arch/i386/crashdump-x86.c |  158 ++++++++++++++++++++--------------------
>  1 file changed, 82 insertions(+), 76 deletions(-)
>
> --- kexec-tools.orig/kexec/arch/i386/crashdump-x86.c
> +++ kexec-tools/kexec/arch/i386/crashdump-x86.c
> @@ -54,10 +54,59 @@
>
>  extern struct arch_options_t arch_options;
>
> +/* Retrieve kernel symbol virtual address from /proc/kallsyms */
> +static unsigned long long get_kernel_sym(const char *symbol)
> +{
> +       const char *kallsyms = "/proc/kallsyms";
> +       char sym[128];
> +       char line[128];
> +       FILE *fp;
> +       unsigned long long vaddr;
> +       char type;
> +
> +       fp = fopen(kallsyms, "r");
> +       if (!fp) {
> +               fprintf(stderr, "Cannot open %s\n", kallsyms);
> +               return 0;
> +       }
> +
> +       while(fgets(line, sizeof(line), fp) != NULL) {
> +               if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
> +                       continue;
> +               if (strcmp(sym, symbol) == 0) {
> +                       dbgprintf("kernel symbol %s vaddr = %16llx\n", symbol, vaddr);
> +                       return vaddr;
> +               }
> +       }
> +
> +       fprintf(stderr, "Cannot get kernel %s symbol address\n", symbol);
> +
> +       return 0;
> +}
> +
>  static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
> -                                 struct crash_elf_info *elf_info)
> +                                 struct crash_elf_info *elf_info,
> +                                 struct mem_ehdr *ehdr)
>  {
>         int kv;
> +       struct mem_phdr *phdr, *end_phdr;
> +       const unsigned long long pud_mask = ~((1 << 30) - 1);
> +       unsigned long long vaddr, lowest_vaddr = 0;
> +
> +       end_phdr = &ehdr->e_phdr[ehdr->e_phnum];
> +       /* Search for the real PAGE_OFFSET when KASLR memory randomization
> +        * is enabled */
> +       if (get_kernel_sym("page_offset_base") != 0) {
> +               for(phdr = ehdr->e_phdr; phdr != end_phdr; phdr++) {
> +                       if (phdr->p_type == PT_LOAD) {
> +                               vaddr = phdr->p_vaddr & pud_mask;
> +                               if (lowest_vaddr == 0 || lowest_vaddr > vaddr)

Although, it is just the copy of original code from one function to
another, but I think logic can be improved a bit here. Initialize
lowest_vaddr with ULONG_MAX and we can get rid of one check of
(lowest_vaddr == 0).

> +                                       lowest_vaddr = vaddr;
> +                       }
> +               }
> +               if (lowest_vaddr != 0)

And then here it should be (lowest_vaddr != ULONG_MAX)

> +                       elf_info->page_offset = lowest_vaddr;
> +       }
>
>         if (elf_info->machine == EM_X86_64) {
>                 kv = kernel_version();
> @@ -102,35 +151,6 @@ static int get_kernel_paddr(struct kexec
>         return -1;
>  }
>
> -/* Retrieve kernel symbol virtual address from /proc/kallsyms */
> -static unsigned long long get_kernel_sym(const char *symbol)
> -{
> -       const char *kallsyms = "/proc/kallsyms";
> -       char sym[128];
> -       char line[128];
> -       FILE *fp;
> -       unsigned long long vaddr;
> -       char type;
> -
> -       fp = fopen(kallsyms, "r");
> -       if (!fp) {
> -               fprintf(stderr, "Cannot open %s\n", kallsyms);
> -               return 0;
> -       }
> -
> -       while(fgets(line, sizeof(line), fp) != NULL) {
> -               if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
> -                       continue;
> -               if (strcmp(sym, symbol) == 0) {
> -                       dbgprintf("kernel symbol %s vaddr = %16llx\n", symbol, vaddr);
> -                       return vaddr;
> -               }
> -       }
> -
> -       fprintf(stderr, "Cannot get kernel %s symbol address\n", symbol);
> -       return 0;
> -}
> -
>  /* Retrieve info regarding virtual address kernel has been compiled for and
>   * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
>   * from kexec-tools fails because of malformed elf notes. A kernel patch has
> @@ -139,19 +159,12 @@ static unsigned long long get_kernel_sym
>   * we should get rid of backward compatible code. */
>
>  static int get_kernel_vaddr_and_size(struct kexec_info *UNUSED(info),
> -                                    struct crash_elf_info *elf_info)
> +                                    struct crash_elf_info *elf_info,
> +                                    struct mem_ehdr *ehdr)
>  {
> -       int result;
> -       const char kcore[] = "/proc/kcore";
> -       char *buf;
> -       struct mem_ehdr ehdr;
>         struct mem_phdr *phdr, *end_phdr;
>         int align;
> -       off_t size;
> -       uint32_t elf_flags = 0;
>         uint64_t stext_sym;
> -       const unsigned long long pud_mask = ~((1 << 30) - 1);
> -       unsigned long long vaddr, lowest_vaddr = 0;
>
>         if (elf_info->machine != EM_X86_64)
>                 return 0;
> @@ -160,45 +173,13 @@ static int get_kernel_vaddr_and_size(str
>                 return 0;
>
>         align = getpagesize();
> -       buf = slurp_file_len(kcore, KCORE_ELF_HEADERS_SIZE, &size);
> -       if (!buf) {
> -               fprintf(stderr, "Cannot read %s: %s\n", kcore, strerror(errno));
> -               return -1;
> -       }
>
> -       /* Don't perform checks to make sure stated phdrs and shdrs are
> -        * actually present in the core file. It is not practical
> -        * to read the GB size file into a user space buffer, Given the
> -        * fact that we don't use any info from that.
> -        */
> -       elf_flags |= ELF_SKIP_FILESZ_CHECK;
> -       result = build_elf_core_info(buf, size, &ehdr, elf_flags);
> -       if (result < 0) {
> -               /* Perhaps KCORE_ELF_HEADERS_SIZE is too small? */
> -               fprintf(stderr, "ELF core (kcore) parse failed\n");
> -               return -1;
> -       }
> -
> -       end_phdr = &ehdr.e_phdr[ehdr.e_phnum];
> -
> -       /* Search for the real PAGE_OFFSET when KASLR memory randomization
> -        * is enabled */
> -       if (get_kernel_sym("page_offset_base") != 0) {
> -               for(phdr = ehdr.e_phdr; phdr != end_phdr; phdr++) {
> -                       if (phdr->p_type == PT_LOAD) {
> -                               vaddr = phdr->p_vaddr & pud_mask;
> -                               if (lowest_vaddr == 0 || lowest_vaddr > vaddr)
> -                                       lowest_vaddr = vaddr;
> -                       }
> -               }
> -               if (lowest_vaddr != 0)
> -                       elf_info->page_offset = lowest_vaddr;
> -       }
> +       end_phdr = &ehdr->e_phdr[ehdr->e_phnum];
>
>         /* Traverse through the Elf headers and find the region where
>          * _stext symbol is located in. That's where kernel is mapped */
>         stext_sym = get_kernel_sym("_stext");
> -       for(phdr = ehdr.e_phdr; stext_sym && phdr != end_phdr; phdr++) {
> +       for(phdr = ehdr->e_phdr; stext_sym && phdr != end_phdr; phdr++) {
>                 if (phdr->p_type == PT_LOAD) {
>                         unsigned long long saddr = phdr->p_vaddr;
>                         unsigned long long eaddr = phdr->p_vaddr + phdr->p_memsz;
> @@ -223,7 +204,7 @@ static int get_kernel_vaddr_and_size(str
>          * /proc/kallsyms, Traverse through the Elf headers again and
>          * find the region where kernel is mapped using hard-coded
>          * kernel mapping boundries */
> -       for(phdr = ehdr.e_phdr; phdr != end_phdr; phdr++) {
> +       for(phdr = ehdr->e_phdr; phdr != end_phdr; phdr++) {
>                 if (phdr->p_type == PT_LOAD) {
>                         unsigned long long saddr = phdr->p_vaddr;
>                         unsigned long long eaddr = phdr->p_vaddr + phdr->p_memsz;
> @@ -887,6 +868,12 @@ int load_crashdump_segments(struct kexec
>         struct memory_range *mem_range, *memmap_p;
>         struct crash_elf_info elf_info;
>         unsigned kexec_arch;
> +       char *buf;
> +       off_t size;
> +       struct mem_ehdr ehdr;
> +       uint32_t elf_flags = 0;
> +       const char kcore[] = "/proc/kcore";
> +       int result;
>
>         memset(&elf_info, 0x0, sizeof(elf_info));
>
> @@ -943,13 +930,32 @@ int load_crashdump_segments(struct kexec
>                 elf_info.class = ELFCLASS64;
>         }
>
> -       if (get_kernel_page_offset(info, &elf_info))
> +       buf = slurp_file_len(kcore, KCORE_ELF_HEADERS_SIZE, &size);
> +       if (!buf) {
> +               fprintf(stderr, "Cannot read %s: %s\n", kcore, strerror(errno));
> +               return -1;
> +       }
> +
> +       /* Don't perform checks to make sure stated phdrs and shdrs are
> +        * actually present in the core file. It is not practical
> +        * to read the GB size file into a user space buffer, Given the
> +        * fact that we don't use any info from that.
> +        */
> +       elf_flags |= ELF_SKIP_FILESZ_CHECK;
> +       result = build_elf_core_info(buf, size, &ehdr, elf_flags);
> +       if (result < 0) {
> +               /* Perhaps KCORE_ELF_HEADERS_SIZE is too small? */
> +               fprintf(stderr, "ELF core (kcore) parse failed\n");
> +               return -1;
> +       }
> +
> +       if (get_kernel_page_offset(info, &elf_info, &ehdr))
>                 return -1;
>
>         if (get_kernel_paddr(info, &elf_info))
>                 return -1;
>
> -       if (get_kernel_vaddr_and_size(info, &elf_info))
> +       if (get_kernel_vaddr_and_size(info, &elf_info, &ehdr))
>                 return -1;
>
>         /* Memory regions which panic kernel can safely use to boot into */

~Pratyush

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym()
  2016-10-06 13:28   ` Matthias Brugger
  2016-10-07  4:18     ` Dave Young
@ 2016-10-07  6:44     ` Pratyush Anand
  1 sibling, 0 replies; 20+ messages in thread
From: Pratyush Anand @ 2016-10-07  6:44 UTC (permalink / raw)
  To: Matthias Brugger; +Cc: Kexec Mailing List

On Thu, Oct 6, 2016 at 6:58 PM, Matthias Brugger <matthias.bgg@gmail.com> wrote:
> On 09/07/2016 06:33 AM, AKASHI Takahiro wrote:
>>
>> From: Pratyush Anand <panand@redhat.com>
>>
>> get_kernel_stext_sym() has been defined for both arm and i386. Other
>> architecture might need some other kernel symbol address. Therefore
>> rewrite
>> this function as generic function to get any kernel symbol address.
>>
>> More over, kallsyms is not arch specific representation, therefore have
>> common function for all arches.
>>
>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>> [created symbols.c]
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>>  kexec/Makefile                  |  1 +
>>  kexec/arch/arm/crashdump-arm.c  | 40
>> +---------------------------------------
>>  kexec/arch/i386/crashdump-x86.c | 32 +-------------------------------
>>  kexec/kexec.h                   |  2 ++
>>  kexec/symbols.c                 | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 46 insertions(+), 70 deletions(-)
>>  create mode 100644 kexec/symbols.c
>>
>> diff --git a/kexec/Makefile b/kexec/Makefile
>> index 39f365f..2b4fb3d 100644
>> --- a/kexec/Makefile
>> +++ b/kexec/Makefile
>> @@ -26,6 +26,7 @@ KEXEC_SRCS_base += kexec/kernel_version.c
>>  KEXEC_SRCS_base += kexec/lzma.c
>>  KEXEC_SRCS_base += kexec/zlib.c
>>  KEXEC_SRCS_base += kexec/kexec-xen.c
>> +KEXEC_SRCS_base += kexec/symbols.c
>>
>>  KEXEC_GENERATED_SRCS += $(PURGATORY_HEX_C)
>>
>> diff --git a/kexec/arch/arm/crashdump-arm.c
>> b/kexec/arch/arm/crashdump-arm.c
>> index 4a89b5e..2bc898b 100644
>> --- a/kexec/arch/arm/crashdump-arm.c
>> +++ b/kexec/arch/arm/crashdump-arm.c
>> @@ -73,48 +73,10 @@ static struct crash_elf_info elf_info = {
>>
>>  extern unsigned long long user_page_offset;
>>
>> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
>> -static unsigned long long get_kernel_stext_sym(void)
>> -{
>> -       const char *kallsyms = "/proc/kallsyms";
>> -       const char *stext = "_stext";
>> -       char sym[128];
>> -       char line[128];
>> -       FILE *fp;
>> -       unsigned long long vaddr = 0;
>> -       char type;
>> -
>> -       fp = fopen(kallsyms, "r");
>> -       if (!fp) {
>> -               fprintf(stderr, "Cannot open %s\n", kallsyms);
>> -               return 0;
>> -       }
>> -
>> -       while(fgets(line, sizeof(line), fp) != NULL) {
>> -               unsigned long long addr;
>> -
>> -               if (sscanf(line, "%Lx %c %s", &addr, &type, sym) != 3)
>> -                       continue;
>> -
>> -               if (strcmp(sym, stext) == 0) {
>> -                       dbgprintf("kernel symbol %s vaddr = %#llx\n",
>> stext, addr);
>> -                       vaddr = addr;
>> -                       break;
>> -               }
>> -       }
>> -
>> -       fclose(fp);
>> -
>> -       if (vaddr == 0)
>> -               fprintf(stderr, "Cannot get kernel %s symbol address\n",
>> stext);
>> -
>> -       return vaddr;
>> -}
>> -
>>  static int get_kernel_page_offset(struct kexec_info *info,
>>                 struct crash_elf_info *elf_info)
>>  {
>> -       unsigned long long stext_sym_addr = get_kernel_stext_sym();
>> +       unsigned long long stext_sym_addr = get_kernel_sym("stext");
>>         if (stext_sym_addr == 0) {
>>                 if (user_page_offset != (-1ULL)) {
>>                         elf_info->page_offset = user_page_offset;
>> diff --git a/kexec/arch/i386/crashdump-x86.c
>> b/kexec/arch/i386/crashdump-x86.c
>> index bbc0f35..664eb3b 100644
>> --- a/kexec/arch/i386/crashdump-x86.c
>> +++ b/kexec/arch/i386/crashdump-x86.c
>> @@ -102,36 +102,6 @@ static int get_kernel_paddr(struct kexec_info
>> *UNUSED(info),
>>         return -1;
>>  }
>>
>> -/* Retrieve kernel _stext symbol virtual address from /proc/kallsyms */
>> -static unsigned long long get_kernel_stext_sym(void)
>> -{
>> -       const char *kallsyms = "/proc/kallsyms";
>> -       const char *stext = "_stext";
>> -       char sym[128];
>> -       char line[128];
>> -       FILE *fp;
>> -       unsigned long long vaddr;
>> -       char type;
>> -
>> -       fp = fopen(kallsyms, "r");
>> -       if (!fp) {
>> -               fprintf(stderr, "Cannot open %s\n", kallsyms);
>> -               return 0;
>> -       }
>> -
>> -       while(fgets(line, sizeof(line), fp) != NULL) {
>> -               if (sscanf(line, "%Lx %c %s", &vaddr, &type, sym) != 3)
>> -                       continue;
>> -               if (strcmp(sym, stext) == 0) {
>> -                       dbgprintf("kernel symbol %s vaddr = %16llx\n",
>> stext, vaddr);
>> -                       return vaddr;
>> -               }
>> -       }
>> -
>> -       fprintf(stderr, "Cannot get kernel %s symbol address\n", stext);
>> -       return 0;
>> -}
>> -
>>  /* Retrieve info regarding virtual address kernel has been compiled for
>> and
>>   * size of the kernel from /proc/kcore. Current /proc/kcore parsing from
>>   * from kexec-tools fails because of malformed elf notes. A kernel patch
>> has
>> @@ -182,7 +152,7 @@ static int get_kernel_vaddr_and_size(struct kexec_info
>> *UNUSED(info),
>>
>>         /* Traverse through the Elf headers and find the region where
>>          * _stext symbol is located in. That's where kernel is mapped */
>> -       stext_sym = get_kernel_stext_sym();
>> +       stext_sym = get_kernel_sym("stext");
>
>
>
> I think this should be get_kernel_sym("_stext");

OK.

>
> Apart from that as Simon already mentioned, due to commit
> 9f62cbd ("kexec/arch/i386: Add support for KASLR memory randomization")
> this patch does not apply cleanly.
>

Will rebase with latest.

Thanks for your review.

~Pratyush

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly
  2016-09-28  7:48     ` AKASHI Takahiro
@ 2016-10-24 23:02       ` Goel, Sameer
  0 siblings, 0 replies; 20+ messages in thread
From: Goel, Sameer @ 2016-10-24 23:02 UTC (permalink / raw)
  To: AKASHI Takahiro, Pratyush Anand, Simon Horman, Geoff Levand,
	Kexec Mailing List

We should look for the kernel code and get the physical offset by 
rounding the address down to 2M. For ARM systems if memory is not 
considered system ram then it is considered device memory. Even if a 
region is not reserved the memory blocks would be sorted.

In cases where the kernel code is not starting at offset 0, the kexec 
command will not load the kernel at the same start address as the 
running kernel. This brakes an implicit kexec assumption.

Thanks,
Sameer

On 9/28/2016 1:48 AM, AKASHI Takahiro wrote:
> On Tue, Sep 27, 2016 at 09:19:51PM +0530, Pratyush Anand wrote:
>> On Wed, Sep 7, 2016 at 10:03 AM, AKASHI Takahiro
>> <takahiro.akashi@linaro.org> wrote:
>>> Due to the kernel patch[1], the current code will not be able to identify
>>
>> [1] is mentioned in cover letter only, not here.
>
> Oops, thanks.
>
>>> the correct value of PHYS_OFFSET if some "reserved" memory region, which
>>> is likely to be UEFI runtime services code/data, exists at an address below
>>> the first "System RAM" regions.
>>>
>>> This patch fixes this issue.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>>  kexec/arch/arm64/iomem.h       |  7 +++++++
>>>  kexec/arch/arm64/kexec-arm64.c | 22 +++++++++++++++++-----
>>>  2 files changed, 24 insertions(+), 5 deletions(-)
>>>  create mode 100644 kexec/arch/arm64/iomem.h
>>>
>>> diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
>>> new file mode 100644
>>> index 0000000..7fd66eb
>>> --- /dev/null
>>> +++ b/kexec/arch/arm64/iomem.h
>>> @@ -0,0 +1,7 @@
>>> +#ifndef IOMEM_H
>>> +#define IOMEM_H
>>> +
>>> +#define SYSTEM_RAM             "System RAM\n"
>>> +#define IOMEM_RESERVED         "reserved\n"
>>> +
>>> +#endif
>>> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
>>> index 7183dac..bc96c76 100644
>>> --- a/kexec/arch/arm64/kexec-arm64.c
>>> +++ b/kexec/arch/arm64/kexec-arm64.c
>>> @@ -21,6 +21,7 @@
>>>  #include "crashdump-arm64.h"
>>>  #include "dt-ops.h"
>>>  #include "fs2dt.h"
>>> +#include "iomem.h"
>>>  #include "kexec-syscall.h"
>>>  #include "arch/options.h"
>>>
>>> @@ -465,18 +466,28 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
>>>   * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
>>>   */
>>>
>>> +static int count_memory_ranges;
>>
>> IMHO improving definition of kexec_iomem_for_each_line() and
>> callback() for handling NULL match case would have been a better
>> choice than introducing a new static variable. callback() can return
>> -ve in case of error, 0 in case of no match and 1 in case of (one)
>> match.
>
> Maybe. But we have to be careful not to change other arch's results.
> Or follow arm's approach.
>
> -Takahiro AKASHI
>
>> ~Pratyush
>>
>>> +
>>>  static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>>>         unsigned long long base, unsigned long long length)
>>>  {
>>>         struct memory_range *r;
>>>
>>> -       if (nr >= KEXEC_SEGMENT_MAX)
>>> +       if (count_memory_ranges >= KEXEC_SEGMENT_MAX)
>>>                 return -1;
>>>
>>> -       r = (struct memory_range *)data + nr;
>>> -       r->type = RANGE_RAM;
>>> +       r = (struct memory_range *)data + count_memory_ranges;
>>> +
>>> +       if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)))
>>> +               r->type = RANGE_RAM;
>>> +       else if (!strncmp(str, IOMEM_RESERVED, strlen(IOMEM_RESERVED)))
>>> +               r->type = RANGE_RESERVED;
>>> +       else
>>> +               return 0;
>>> +
>>>         r->start = base;
>>>         r->end = base + length - 1;
>>> +       count_memory_ranges++;
>>>
>>>         set_phys_offset(r->start);
>>>
>>> @@ -493,9 +504,10 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>>>  static int get_memory_ranges_iomem(struct memory_range *array,
>>>         unsigned int *count)
>>>  {
>>> -       *count = kexec_iomem_for_each_line("System RAM\n",
>>> -               get_memory_ranges_iomem_cb, array);
>>> +       count_memory_ranges = 0;
>>> +       kexec_iomem_for_each_line(NULL, get_memory_ranges_iomem_cb, array);
>>>
>>> +       *count = count_memory_ranges;
>>>         if (!*count) {
>>>                 dbgprintf("%s: failed: No RAM found.\n", __func__);
>>>                 return -EFAILED;
>>> --
>>> 2.9.0
>>>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

end of thread, other threads:[~2016-10-24 23:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07  4:33 [PATCH v3 0/8] (kexec-tools) arm64: add kdump support AKASHI Takahiro
2016-09-07  4:33 ` [PATCH v3 1/8] arm64: identify PHYS_OFFSET correctly AKASHI Takahiro
2016-09-27 15:49   ` Pratyush Anand
2016-09-28  7:48     ` AKASHI Takahiro
2016-10-24 23:02       ` Goel, Sameer
2016-09-07  4:33 ` [PATCH v3 2/8] kexec: generalize and rename get_kernel_stext_sym() AKASHI Takahiro
2016-10-06 13:28   ` Matthias Brugger
2016-10-07  4:18     ` Dave Young
2016-10-07  6:41       ` Pratyush Anand
2016-10-07  6:44     ` Pratyush Anand
2016-09-07  4:33 ` [PATCH v3 3/8] arm64: kdump: identify memory regions AKASHI Takahiro
2016-09-07  4:33 ` [PATCH v3 4/8] arm64: kdump: add elf core header segment AKASHI Takahiro
2016-09-07  4:33 ` [PATCH v3 5/8] arm64: kdump: set up kernel image segment AKASHI Takahiro
2016-09-07  4:33 ` [PATCH v3 6/8] arm64: kdump: set up other segments AKASHI Takahiro
2016-09-07  4:34 ` [PATCH v3 7/8] arm64: kdump: add DT properties to crash dump kernel's dtb AKASHI Takahiro
2016-09-07  4:34 ` [PATCH v3 8/8] arm64: kdump: Add support for binary image files AKASHI Takahiro
2016-09-29  7:52 ` [PATCH v3 0/8] (kexec-tools) arm64: add kdump support Simon Horman
2016-09-29  8:18   ` AKASHI Takahiro
2016-09-29  8:39     ` Simon Horman
2016-09-29 14:26       ` AKASHI Takahiro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.