2013-05-24 11:29:50

by Michael Holzheu

[permalink] [raw]
Subject: [PATCH v4 0/3] kdump: Allow ELF header creation in new kernel

Hello Vivek,

Here the 4th version of the patch set with the rebase to version 8 of
the mmap patches.

ChangeLog
=========
v3 => v4)

- Rebase to 3.10-rc2 + vmcore mmap patches v8

v2 => v3)

- Get rid of ELFCORE_ADDR_NEWMEM
- Make read_from_crash_header() only read from kernel
- Move read_from_crash_header() to weak function arch_read_from_crash_header()
- Implement read_from_crash_header() strong function for s390
- Set elfcorehdr_addr to address of new ELF header

v1 => v2)

- Rebase 3.10-rc2 + vmcore mmap patches
- Introduced arch_get/free_crash_header() and ELFCORE_ADDR_NEWMEM

Feature Description
===================
For s390 we want to use /proc/vmcore for our SCSI stand-alone
dump (zfcpdump). We have support where the first HSA_SIZE bytes are
saved into a hypervisor owned memory area (HSA) before the kdump
kernel is booted. When the kdump kernel starts, it is restricted
to use only HSA_SIZE bytes.

The advantages of this mechanism are:

* No crashkernel memory has to be defined in the old kernel.
* Early boot problems (before kexec_load has been done) can be dumped
* Non-Linux systems can be dumped.

We modify the s390 copy_oldmem_page() function to read from the HSA memory
if memory below HSA_SIZE bytes is requested.

Since we cannot use the kexec tool to load the kernel in this scenario,
we have to build the ELF header in the 2nd (kdump/new) kernel.

So with the following patch set we would like to introduce the new
function that the ELF header for /proc/vmcore can be created in the 2nd
kernel memory.

The following steps are done during zfcpdump execution:

1. Production system crashes
2. User boots a SCSI disk that has been prepared with the zfcpdump tool
3. Hypervisor saves CPU state of boot CPU and HSA_SIZE bytes of memory into HSA
4. Boot loader loads kernel into low memory area
5. Kernel boots and uses only HSA_SIZE bytes of memory
6. Kernel saves registers of non-boot CPUs
7. Kernel does memory detection for dump memory map
8. Kernel creates ELF header for /proc/vmcore
9. /proc/vmcore uses this header for initialization
10. The zfcpdump user space reads /proc/vmcore to write dump to SCSI disk
- copy_oldmem_page() copies from HSA for memory below HSA_SIZE
- copy_oldmem_page() copies from real memory for memory above HSA_SIZE

---
Jan Willeke (1):
s390/kdump: Use vmcore for zfcpdump

Michael Holzheu (2):
kdump: Introduce ELF header in new memory feature
s390/kdump: Use ELF header in new memory feature

arch/s390/Kconfig | 3 +-
arch/s390/include/asm/sclp.h | 1 +
arch/s390/kernel/crash_dump.c | 138 +++++++++++++++++++++++++++++++-----------
drivers/s390/char/zcore.c | 6 +-
fs/proc/vmcore.c | 58 +++++++++++++-----
include/linux/crash_dump.h | 4 ++
6 files changed, 157 insertions(+), 53 deletions(-)

--
1.8.1.6


2013-05-24 11:29:49

by Michael Holzheu

[permalink] [raw]
Subject: [PATCH v4 1/3] kdump: Introduce ELF header in new memory feature

Currently for s390 we create the ELF core header in the 2nd kernel
with a small trick. We relocate the addresses in the ELF header in
a way that for the /proc/vmcore code it seems to be in the 1st kernel
(old) memory and the read_from_oldmem() returns the correct data.
This allows the /proc/vmcore code to use the ELF header in the
2nd kernel.

This patch now exchanges the old mechanism with the new and much
cleaner function call override feature that now offcially allows to
create the ELF core header in the 2nd kernel.

To use the new feature the following has to be done by the architecture
backend code:

* Override arch_get_crash_header() to return the address of the ELF
header in new memory.
* Override arch_free_crash_header() to free the memory of the ELF
header in new memory.
* Override arch_read_from_crash_header() to read from the ELF header
in new memory
* Set elfcorehdr_addr to the address of ELF header in new memory

Signed-off-by: Michael Holzheu <[email protected]>
---
fs/proc/vmcore.c | 58 ++++++++++++++++++++++++++++++++++------------
include/linux/crash_dump.h | 4 ++++
2 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 80221d7..eb47346 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -123,6 +123,28 @@ static ssize_t read_from_oldmem(char *buf, size_t count,
return read;
}

+/*
+ * Architetures may override this function to read header data
+ */
+ssize_t __weak arch_read_from_crash_header(char *buf, size_t count, u64 *ppos)
+{
+ return read_from_oldmem(buf, count, ppos, 0);
+}
+
+/*
+ * Architetures may override this function to get header address
+ */
+unsigned long long __weak arch_get_crash_header(void)
+{
+ return elfcorehdr_addr;
+}
+
+/*
+ * Architetures may override this function to free header
+ */
+void __weak arch_free_crash_header(void)
+{}
+
/* Read from the ELF header and then the crash dump. On error, negative value is
* returned otherwise number of bytes read are returned.
*/
@@ -335,7 +357,8 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
notes_section = kmalloc(max_sz, GFP_KERNEL);
if (!notes_section)
return -ENOMEM;
- rc = read_from_oldmem(notes_section, max_sz, &offset, 0);
+ rc = arch_read_from_crash_header(notes_section, max_sz,
+ &offset);
if (rc < 0) {
kfree(notes_section);
return rc;
@@ -422,7 +445,8 @@ static int __init copy_notes_elf64(const Elf64_Ehdr *ehdr_ptr, char *notes_buf)
if (phdr_ptr->p_type != PT_NOTE)
continue;
offset = phdr_ptr->p_offset;
- rc = read_from_oldmem(notes_buf, phdr_ptr->p_memsz, &offset, 0);
+ rc = arch_read_from_crash_header(notes_buf, phdr_ptr->p_memsz,
+ &offset, 0);
if (rc < 0)
return rc;
notes_buf += phdr_ptr->p_memsz;
@@ -521,7 +545,8 @@ static int __init update_note_header_size_elf32(const Elf32_Ehdr *ehdr_ptr)
notes_section = kmalloc(max_sz, GFP_KERNEL);
if (!notes_section)
return -ENOMEM;
- rc = read_from_oldmem(notes_section, max_sz, &offset, 0);
+ rc = arch_read_from_crash_header(notes_section, max_sz,
+ &offset);
if (rc < 0) {
kfree(notes_section);
return rc;
@@ -608,7 +633,8 @@ static int __init copy_notes_elf32(const Elf32_Ehdr *ehdr_ptr, char *notes_buf)
if (phdr_ptr->p_type != PT_NOTE)
continue;
offset = phdr_ptr->p_offset;
- rc = read_from_oldmem(notes_buf, phdr_ptr->p_memsz, &offset, 0);
+ rc = arch_read_from_crash_header(notes_buf, phdr_ptr->p_memsz,
+ &offset, 0);
if (rc < 0)
return rc;
notes_buf += phdr_ptr->p_memsz;
@@ -791,10 +817,11 @@ static int __init parse_crash_elf64_headers(void)
Elf64_Ehdr ehdr;
u64 addr;

- addr = elfcorehdr_addr;
+ addr = arch_get_crash_header();

/* Read Elf header */
- rc = read_from_oldmem((char*)&ehdr, sizeof(Elf64_Ehdr), &addr, 0);
+ rc = arch_read_from_crash_header((char *)&ehdr, sizeof(Elf64_Ehdr),
+ &addr);
if (rc < 0)
return rc;

@@ -819,8 +846,8 @@ static int __init parse_crash_elf64_headers(void)
get_order(elfcorebuf_sz_orig));
if (!elfcorebuf)
return -ENOMEM;
- addr = elfcorehdr_addr;
- rc = read_from_oldmem(elfcorebuf, elfcorebuf_sz_orig, &addr, 0);
+ addr = arch_get_crash_header();
+ rc = arch_read_from_crash_header(elfcorebuf, elfcorebuf_sz_orig, &addr);
if (rc < 0) {
free_pages((unsigned long)elfcorebuf,
get_order(elfcorebuf_sz_orig));
@@ -853,10 +880,11 @@ static int __init parse_crash_elf32_headers(void)
Elf32_Ehdr ehdr;
u64 addr;

- addr = elfcorehdr_addr;
+ addr = arch_get_crash_header();

/* Read Elf header */
- rc = read_from_oldmem((char*)&ehdr, sizeof(Elf32_Ehdr), &addr, 0);
+ rc = arch_read_from_crash_header((char *)&ehdr, sizeof(Elf32_Ehdr),
+ &addr);
if (rc < 0)
return rc;

@@ -881,8 +909,8 @@ static int __init parse_crash_elf32_headers(void)
get_order(elfcorebuf_sz_orig));
if (!elfcorebuf)
return -ENOMEM;
- addr = elfcorehdr_addr;
- rc = read_from_oldmem(elfcorebuf, elfcorebuf_sz_orig, &addr, 0);
+ addr = arch_get_crash_header();
+ rc = arch_read_from_crash_header(elfcorebuf, elfcorebuf_sz_orig, &addr);
if (rc < 0) {
free_pages((unsigned long)elfcorebuf,
get_order(elfcorebuf_sz_orig));
@@ -915,8 +943,8 @@ static int __init parse_crash_elf_headers(void)
u64 addr;
int rc=0;

- addr = elfcorehdr_addr;
- rc = read_from_oldmem(e_ident, EI_NIDENT, &addr, 0);
+ addr = arch_get_crash_header();
+ rc = arch_read_from_crash_header(e_ident, EI_NIDENT, &addr);
if (rc < 0)
return rc;
if (memcmp(e_ident, ELFMAG, SELFMAG) != 0) {
@@ -940,7 +968,7 @@ static int __init parse_crash_elf_headers(void)
/* Determine vmcore size. */
vmcore_size = get_vmcore_size(elfcorebuf_sz, elfnotes_sz,
&vmcore_list);
-
+ arch_free_crash_header();
return 0;
}

diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 37e4f8d..c66da41 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,6 +14,10 @@ extern unsigned long long elfcorehdr_size;

extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);
+extern unsigned long long __weak arch_get_crash_header(void);
+extern void __weak arch_free_crash_header(void);
+extern ssize_t __weak arch_read_from_crash_header(char *buf, size_t count,
+ u64 *ppos);

/* Architecture code defines this if there are other possible ELF
* machine types, e.g. on bi-arch capable hardware. */
--
1.8.1.6

2013-05-24 11:30:20

by Michael Holzheu

[permalink] [raw]
Subject: [PATCH v4 3/3] s390/kdump: Use vmcore for zfcpdump

From: Jan Willeke <[email protected]>

This patch modifies the s390 copy_oldmem_page() function for zfcpdump
to read from the HSA memory if memory below HSA_SIZE bytes is requested.
Otherwise from real memory is read.

Signed-off-by: Jan Willeke <[email protected]>
Signed-off-by: Michael Holzheu <[email protected]>
---
arch/s390/Kconfig | 3 +-
arch/s390/include/asm/sclp.h | 1 +
arch/s390/kernel/crash_dump.c | 74 ++++++++++++++++++++++++++++++++++---------
drivers/s390/char/zcore.c | 6 ++--
4 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2c9789d..a0d78f1 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -516,6 +516,7 @@ config CRASH_DUMP
bool "kernel crash dumps"
depends on 64BIT && SMP
select KEXEC
+ select ZFCPDUMP
help
Generate crash dump after being started by kexec.
Crash dump kernels are loaded in the main kernel with kexec-tools
@@ -526,7 +527,7 @@ config CRASH_DUMP
config ZFCPDUMP
def_bool n
prompt "zfcpdump support"
- select SMP
+ depends on SMP
help
Select this option if you want to build an zfcpdump enabled kernel.
Refer to <file:Documentation/s390/zfcpdump.txt> for more details on this.
diff --git a/arch/s390/include/asm/sclp.h b/arch/s390/include/asm/sclp.h
index 06a1361..7dc7f9c 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -56,5 +56,6 @@ bool sclp_has_linemode(void);
bool sclp_has_vt220(void);
int sclp_pci_configure(u32 fid);
int sclp_pci_deconfigure(u32 fid);
+int memcpy_hsa(void *dest, unsigned long src, size_t count, int mode);

#endif /* _ASM_S390_SCLP_H */
diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
index aeb1207..fd0dc60 100644
--- a/arch/s390/kernel/crash_dump.c
+++ b/arch/s390/kernel/crash_dump.c
@@ -16,6 +16,7 @@
#include <asm/os_info.h>
#include <asm/elf.h>
#include <asm/ipl.h>
+#include <asm/sclp.h>

#define PTR_ADD(x, y) (((char *) (x)) + ((unsigned long) (y)))
#define PTR_SUB(x, y) (((char *) (x)) - ((unsigned long) (y)))
@@ -24,6 +25,25 @@
static size_t elfcorebuf_sz;
static char *elfcorebuf;

+/* For pages below ZFCPDUMP_HSA_SIZE memory from the HSA is copied. Otherwise
+ * real memory copy is used.
+ */
+static ssize_t copy_oldmem_page_zfcpdump(char *buf, size_t csize,
+ unsigned long src, int userbuf)
+{
+ if (src < ZFCPDUMP_HSA_SIZE) {
+ if (memcpy_hsa(buf, src, csize, userbuf) < 0)
+ return -EINVAL;
+ } else {
+ if (userbuf)
+ copy_to_user_real((void __force __user *) buf,
+ (void *) src, csize);
+ else
+ memcpy_real(buf, (void *) src, csize);
+ }
+ return csize;
+}
+
/*
* Copy one page from "oldmem"
*
@@ -31,15 +51,10 @@ static char *elfcorebuf;
* - [OLDMEM_BASE - OLDMEM_BASE + OLDMEM_SIZE] is mapped to [0 - OLDMEM_SIZE].
* - [0 - OLDMEM_SIZE] is mapped to [OLDMEM_BASE - OLDMEM_BASE + OLDMEM_SIZE]
*/
-ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
- size_t csize, unsigned long offset, int userbuf)
-{
- unsigned long src;
+static ssize_t copy_oldmem_page_kdump(char *buf, size_t csize,
+ unsigned long src, int userbuf)

- if (!csize)
- return 0;
-
- src = (pfn << PAGE_SHIFT) + offset;
+{
if (src < OLDMEM_SIZE)
src += OLDMEM_BASE;
else if (src > OLDMEM_BASE &&
@@ -54,6 +69,23 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
}

/*
+ * Copy one page from "oldmem"
+ */
+ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
+ unsigned long offset, int userbuf)
+{
+ unsigned long src;
+ if (!csize)
+ return 0;
+ src = (pfn << PAGE_SHIFT) + offset;
+
+ if (OLDMEM_BASE)
+ return copy_oldmem_page_kdump(buf, csize, src, userbuf);
+ else
+ return copy_oldmem_page_zfcpdump(buf, csize, src, userbuf);
+}
+
+/*
* Copy memory from old kernel
*/
int copy_from_oldmem(void *dest, void *src, size_t count)
@@ -61,11 +93,21 @@ int copy_from_oldmem(void *dest, void *src, size_t count)
unsigned long copied = 0;
int rc;

- if ((unsigned long) src < OLDMEM_SIZE) {
- copied = min(count, OLDMEM_SIZE - (unsigned long) src);
- rc = memcpy_real(dest, src + OLDMEM_BASE, copied);
- if (rc)
- return rc;
+ if (OLDMEM_BASE) {
+ if ((unsigned long) src < OLDMEM_SIZE) {
+ copied = min(count, OLDMEM_SIZE - (unsigned long) src);
+ rc = memcpy_real(dest, src + OLDMEM_BASE, copied);
+ if (rc)
+ return rc;
+ }
+ } else {
+ if ((unsigned long) src < ZFCPDUMP_HSA_SIZE) {
+ copied = min(count,
+ ZFCPDUMP_HSA_SIZE - (unsigned long) src);
+ rc = memcpy_hsa(dest, (unsigned long) src, copied, 0);
+ if (rc)
+ return rc;
+ }
}
return memcpy_real(dest + copied, src + copied, count - copied);
}
@@ -458,7 +500,9 @@ ssize_t arch_read_from_crash_header(char *buf, size_t count, u64 *ppos)
*/
static int setup_kdump_elfcorehdr(void)
{
- if (!OLDMEM_BASE || is_kdump_kernel())
+ if (!OLDMEM_BASE && (ipl_info.type != IPL_TYPE_FCP_DUMP))
+ return -EINVAL;
+ if (is_kdump_kernel())
return -EINVAL;
s390_elf_corehdr_create(&elfcorebuf, &elfcorebuf_sz);
elfcorehdr_addr = __pa(elfcorebuf);
@@ -466,4 +510,4 @@ static int setup_kdump_elfcorehdr(void)
return 0;
}

-subsys_initcall(setup_kdump_elfcorehdr);
+subsys_initcall_sync(setup_kdump_elfcorehdr);
diff --git a/drivers/s390/char/zcore.c b/drivers/s390/char/zcore.c
index 9e5e146..794820a 100644
--- a/drivers/s390/char/zcore.c
+++ b/drivers/s390/char/zcore.c
@@ -30,8 +30,8 @@

#define TRACE(x...) debug_sprintf_event(zcore_dbf, 1, x)

-#define TO_USER 0
-#define TO_KERNEL 1
+#define TO_USER 1
+#define TO_KERNEL 0
#define CHUNK_INFO_SIZE 34 /* 2 16-byte char, each followed by blank */

enum arch_id {
@@ -73,7 +73,7 @@ static struct ipl_parameter_block *ipl_block;
* @count: Size of buffer, which should be copied
* @mode: Either TO_KERNEL or TO_USER
*/
-static int memcpy_hsa(void *dest, unsigned long src, size_t count, int mode)
+int memcpy_hsa(void *dest, unsigned long src, size_t count, int mode)
{
int offs, blk_num;
static char buf[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
--
1.8.1.6

2013-05-24 11:30:19

by Michael Holzheu

[permalink] [raw]
Subject: [PATCH v4 2/3] s390/kdump: Use ELF header in new memory feature

This patch now exchanges the old relocate mechanism with the new
arch function call override mechanism that allows to create the ELF
core header in the 2nd kernel.

Signed-off-by: Michael Holzheu <[email protected]>
---
arch/s390/kernel/crash_dump.c | 64 ++++++++++++++++++++++++++++++-------------
1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
index f703d91..aeb1207 100644
--- a/arch/s390/kernel/crash_dump.c
+++ b/arch/s390/kernel/crash_dump.c
@@ -21,6 +21,9 @@
#define PTR_SUB(x, y) (((char *) (x)) - ((unsigned long) (y)))
#define PTR_DIFF(x, y) ((unsigned long)(((char *) (x)) - ((unsigned long) (y))))

+static size_t elfcorebuf_sz;
+static char *elfcorebuf;
+
/*
* Copy one page from "oldmem"
*
@@ -325,14 +328,6 @@ static int get_mem_chunk_cnt(void)
}

/*
- * Relocate pointer in order to allow vmcore code access the data
- */
-static inline unsigned long relocate(unsigned long addr)
-{
- return OLDMEM_BASE + addr;
-}
-
-/*
* Initialize ELF loads (new kernel)
*/
static int loads_init(Elf64_Phdr *phdr, u64 loads_offset)
@@ -383,7 +378,7 @@ static void *notes_init(Elf64_Phdr *phdr, void *ptr, u64 notes_offset)
ptr = nt_vmcoreinfo(ptr);
memset(phdr, 0, sizeof(*phdr));
phdr->p_type = PT_NOTE;
- phdr->p_offset = relocate(notes_offset);
+ phdr->p_offset = notes_offset;
phdr->p_filesz = (unsigned long) PTR_SUB(ptr, ptr_start);
phdr->p_memsz = phdr->p_filesz;
return ptr;
@@ -392,7 +387,7 @@ static void *notes_init(Elf64_Phdr *phdr, void *ptr, u64 notes_offset)
/*
* Create ELF core header (new kernel)
*/
-static void s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
+static int s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
{
Elf64_Phdr *phdr_notes, *phdr_loads;
int mem_chunk_cnt;
@@ -414,28 +409,59 @@ static void s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
ptr = PTR_ADD(ptr, sizeof(Elf64_Phdr) * mem_chunk_cnt);
/* Init notes */
hdr_off = PTR_DIFF(ptr, hdr);
- ptr = notes_init(phdr_notes, ptr, ((unsigned long) hdr) + hdr_off);
+ ptr = notes_init(phdr_notes, ptr, (unsigned long) hdr + hdr_off);
/* Init loads */
hdr_off = PTR_DIFF(ptr, hdr);
- loads_init(phdr_loads, ((unsigned long) hdr) + hdr_off);
+ loads_init(phdr_loads, hdr_off);
*elfcorebuf_sz = hdr_off;
- *elfcorebuf = (void *) relocate((unsigned long) hdr);
+ *elfcorebuf = hdr;
BUG_ON(*elfcorebuf_sz > alloc_size);
+ return 0;
}

/*
- * Create kdump ELF core header in new kernel, if it has not been passed via
- * the "elfcorehdr" kernel parameter
+ * Return address of ELF core header (new or old memory)
*/
-static int setup_kdump_elfcorehdr(void)
+unsigned long long arch_get_crash_header(void)
{
- size_t elfcorebuf_sz;
- char *elfcorebuf;
+ if (elfcorebuf)
+ return elfcorehdr_addr;
+ else
+ return __pa(elfcorebuf);
+}

+/*
+ * Free crash header
+ */
+void arch_free_crash_header(void)
+{
+ kfree(elfcorebuf);
+ elfcorebuf = 0;
+}
+
+/*
+ * Read from crash header (new or old memory)
+ */
+ssize_t arch_read_from_crash_header(char *buf, size_t count, u64 *ppos)
+{
+ if (elfcorebuf)
+ memcpy(buf, (void *)*ppos, count);
+ else
+ copy_from_oldmem(buf, (void *)*ppos, count);
+ *ppos += count;
+ return count;
+}
+
+/*
+ * Create kdump ELF core header in new kernel if it has not been passed via
+ * the "elfcorehdr=" kernel parameter
+ */
+static int setup_kdump_elfcorehdr(void)
+{
if (!OLDMEM_BASE || is_kdump_kernel())
return -EINVAL;
s390_elf_corehdr_create(&elfcorebuf, &elfcorebuf_sz);
- elfcorehdr_addr = (unsigned long long) elfcorebuf;
+ elfcorehdr_addr = __pa(elfcorebuf);
elfcorehdr_size = elfcorebuf_sz;
return 0;
}
--
1.8.1.6

2013-05-30 14:50:28

by Vivek Goyal

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] kdump: Introduce ELF header in new memory feature

On Fri, May 24, 2013 at 11:18:56AM -0400, Vivek Goyal wrote:
> On Fri, May 24, 2013 at 01:29:41PM +0200, Michael Holzheu wrote:
> > Currently for s390 we create the ELF core header in the 2nd kernel
> > with a small trick. We relocate the addresses in the ELF header in
> > a way that for the /proc/vmcore code it seems to be in the 1st kernel
> > (old) memory and the read_from_oldmem() returns the correct data.
> > This allows the /proc/vmcore code to use the ELF header in the
> > 2nd kernel.
> >
> > This patch now exchanges the old mechanism with the new and much
> > cleaner function call override feature that now offcially allows to
> > create the ELF core header in the 2nd kernel.
> >
> > To use the new feature the following has to be done by the architecture
> > backend code:
> >
> > * Override arch_get_crash_header() to return the address of the ELF
> > header in new memory.
> > * Override arch_free_crash_header() to free the memory of the ELF
> > header in new memory.
> > * Override arch_read_from_crash_header() to read from the ELF header
> > in new memory
>
> I think above function is not clear in definition. What happens to reading
> of data pointed by ELF headers. For sections of data pointed by PT_NOTE
> we are calling arch_read_from_crash_header() but for accessing data
> pointed by PT_LOAD, we must be calling regular functions.
>
> Not very sure what to do about this. Just that it looks little ugly
> right now.

Hi Michael,

How about if we introduce another function to read notes, say
arch_read_crash_notes(). So arch_read_from_crash_header() is meant
to get just ELF headers (ELF header and PT_LOAD, PT_NOTE type elf headers)
and arch_read_crash_notes() gets the data as pointed by PT_NOTE elf
header.

Thanks
Vivek

2013-05-30 14:58:09

by Vivek Goyal

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] s390/kdump: Use ELF header in new memory feature

On Fri, May 24, 2013 at 01:29:42PM +0200, Michael Holzheu wrote:
> This patch now exchanges the old relocate mechanism with the new
> arch function call override mechanism that allows to create the ELF
> core header in the 2nd kernel.
>
> Signed-off-by: Michael Holzheu <[email protected]>
> ---
> arch/s390/kernel/crash_dump.c | 64 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
> index f703d91..aeb1207 100644
> --- a/arch/s390/kernel/crash_dump.c
> +++ b/arch/s390/kernel/crash_dump.c
> @@ -21,6 +21,9 @@
> #define PTR_SUB(x, y) (((char *) (x)) - ((unsigned long) (y)))
> #define PTR_DIFF(x, y) ((unsigned long)(((char *) (x)) - ((unsigned long) (y))))
>
> +static size_t elfcorebuf_sz;
> +static char *elfcorebuf;
> +
> /*
> * Copy one page from "oldmem"
> *
> @@ -325,14 +328,6 @@ static int get_mem_chunk_cnt(void)
> }
>
> /*
> - * Relocate pointer in order to allow vmcore code access the data
> - */
> -static inline unsigned long relocate(unsigned long addr)
> -{
> - return OLDMEM_BASE + addr;
> -}
> -
> -/*
> * Initialize ELF loads (new kernel)
> */
> static int loads_init(Elf64_Phdr *phdr, u64 loads_offset)
> @@ -383,7 +378,7 @@ static void *notes_init(Elf64_Phdr *phdr, void *ptr, u64 notes_offset)
> ptr = nt_vmcoreinfo(ptr);
> memset(phdr, 0, sizeof(*phdr));
> phdr->p_type = PT_NOTE;
> - phdr->p_offset = relocate(notes_offset);
> + phdr->p_offset = notes_offset;
> phdr->p_filesz = (unsigned long) PTR_SUB(ptr, ptr_start);
> phdr->p_memsz = phdr->p_filesz;
> return ptr;
> @@ -392,7 +387,7 @@ static void *notes_init(Elf64_Phdr *phdr, void *ptr, u64 notes_offset)
> /*
> * Create ELF core header (new kernel)
> */
> -static void s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
> +static int s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
> {
> Elf64_Phdr *phdr_notes, *phdr_loads;
> int mem_chunk_cnt;
> @@ -414,28 +409,59 @@ static void s390_elf_corehdr_create(char **elfcorebuf, size_t *elfcorebuf_sz)
> ptr = PTR_ADD(ptr, sizeof(Elf64_Phdr) * mem_chunk_cnt);
> /* Init notes */
> hdr_off = PTR_DIFF(ptr, hdr);
> - ptr = notes_init(phdr_notes, ptr, ((unsigned long) hdr) + hdr_off);
> + ptr = notes_init(phdr_notes, ptr, (unsigned long) hdr + hdr_off);
> /* Init loads */
> hdr_off = PTR_DIFF(ptr, hdr);
> - loads_init(phdr_loads, ((unsigned long) hdr) + hdr_off);
> + loads_init(phdr_loads, hdr_off);
> *elfcorebuf_sz = hdr_off;
> - *elfcorebuf = (void *) relocate((unsigned long) hdr);
> + *elfcorebuf = hdr;
> BUG_ON(*elfcorebuf_sz > alloc_size);
> + return 0;
> }
>
> /*
> - * Create kdump ELF core header in new kernel, if it has not been passed via
> - * the "elfcorehdr" kernel parameter
> + * Return address of ELF core header (new or old memory)
> */
> -static int setup_kdump_elfcorehdr(void)
> +unsigned long long arch_get_crash_header(void)
> {
> - size_t elfcorebuf_sz;
> - char *elfcorebuf;
> + if (elfcorebuf)
> + return elfcorehdr_addr;
> + else
> + return __pa(elfcorebuf);
> +}
>
> +/*
> + * Free crash header
> + */
> +void arch_free_crash_header(void)
> +{
> + kfree(elfcorebuf);
> + elfcorebuf = 0;
> +}
> +
> +/*
> + * Read from crash header (new or old memory)
> + */
> +ssize_t arch_read_from_crash_header(char *buf, size_t count, u64 *ppos)
> +{
> + if (elfcorebuf)
> + memcpy(buf, (void *)*ppos, count);

This is ugly. It assumes that generic code will always free headers before
reading any PT_LOAD data. It can be easily broken.

Why arch_read_from_crash_header() should not always read new memory for
s390?

Vivek