Discussion:
[PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
Xunlei Pang
2017-06-09 02:17:05 UTC
Permalink
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
is now defined as follows:
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.

This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.

Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
Cc: Dave Young <***@redhat.com>
Cc: Dave Anderson <***@redhat.com>
Cc: Hari Bathini <***@linux.vnet.ibm.com>
Cc: Gustavo Luiz Duarte <***@linux.vnet.ibm.com>
Signed-off-by: Xunlei Pang <***@redhat.com>
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)

+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif

#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@

#if !defined(__ASSEMBLY__)

-#include <linux/crash_core.h>
#include <asm/io.h>

#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>

/* Verify architecture specific macros are defined */

@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME

/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
* This structure is used to hold the arguments that are used when loading
* kernel binaries.
*/
--
1.8.3.1
Dave Young
2017-06-09 02:29:04 UTC
Permalink
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif
#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
#if !defined(__ASSEMBLY__)
-#include <linux/crash_core.h>
#include <asm/io.h>
#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>
/* Verify architecture specific macros are defined */
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
It is still not clear how does s390 use the crash_notes except this macro.
But from code point of view we do need to update this as well after the
crash_core splitting.

Acked-by: Dave Young <***@redhat.com>

Thanks
Dave
Dave Young
2017-06-09 07:45:29 UTC
Permalink
Post by Dave Young
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
I found that in mainline since below commit, above define should be
useless, but if distribution with older kernel does need your fix, so in
mainline the right fix should be dropping the s390 part about these
macros usage.

Anyway this need a comment from Michael.

commit 8a07dd02d7615d91d65d6235f7232e3f9b5d347f
Author: Martin Schwidefsky <***@de.ibm.com>
Date: Wed Oct 14 15:53:06 2015 +0200

s390/kdump: remove code to create ELF notes in the crashed system

The s390 architecture can store the CPU registers of the crashed
system
after the kdump kernel has been started and this is the preferred
way.
Remove the remaining code fragments that deal with storing CPU
registers
while the crashed system is still active.
Post by Dave Young
Post by Xunlei Pang
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif
#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
#if !defined(__ASSEMBLY__)
-#include <linux/crash_core.h>
#include <asm/io.h>
#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>
/* Verify architecture specific macros are defined */
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
It is still not clear how does s390 use the crash_notes except this macro.
But from code point of view we do need to update this as well after the
crash_core splitting.
Hold on the ack because of the new findings, wait for Michael's
feedback.

Thanks
Dave
Xunlei Pang
2017-06-11 08:43:23 UTC
Permalink
Post by Dave Young
Post by Dave Young
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
I found that in mainline since below commit, above define should be
useless, but if distribution with older kernel does need your fix, so in
mainline the right fix should be dropping the s390 part about these
macros usage.
Indeed, then I think we can remove this special definition of S390 to avoid confusion.

Regards,
Xunlei
Post by Dave Young
Anyway this need a comment from Michael.
commit 8a07dd02d7615d91d65d6235f7232e3f9b5d347f
Date: Wed Oct 14 15:53:06 2015 +0200
s390/kdump: remove code to create ELF notes in the crashed system
The s390 architecture can store the CPU registers of the crashed
system
after the kdump kernel has been started and this is the preferred
way.
Remove the remaining code fragments that deal with storing CPU
registers
while the crashed system is still active.
Post by Dave Young
Post by Xunlei Pang
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif
#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
#if !defined(__ASSEMBLY__)
-#include <linux/crash_core.h>
#include <asm/io.h>
#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>
/* Verify architecture specific macros are defined */
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
It is still not clear how does s390 use the crash_notes except this macro.
But from code point of view we do need to update this as well after the
crash_core splitting.
Hold on the ack because of the new findings, wait for Michael's
feedback.
Thanks
Dave
Hari Bathini
2017-06-09 04:12:10 UTC
Permalink
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif
#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
#if !defined(__ASSEMBLY__)
-#include <linux/crash_core.h>
#include <asm/io.h>
#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>
/* Verify architecture specific macros are defined */
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
* This structure is used to hold the arguments that are used when loading
* kernel binaries.
*/
Looks good to me. Thanks for the fix..

- Hari
kbuild test robot
2017-06-11 19:46:54 UTC
Permalink
Hi Xunlei,

[auto build test ERROR on next-20170605]
[also build test ERROR on v4.12-rc4]
[cannot apply to linus/master linux/master s390/features v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Xunlei-Pang/s390-crash-Fix-KEXEC_NOTE_BYTES-definition/20170610-173047
config: arm-sa1100 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All error/warnings (new ones prefixed by >>):

In file included from arch/arm/include/asm/memory.h:378:0,
from arch/arm/include/asm/page.h:165,
from arch/arm/include/asm/thread_info.h:17,
from include/linux/thread_info.h:37,
from include/asm-generic/preempt.h:4,
from ./arch/arm/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:80,
from include/linux/spinlock.h:50,
from include/linux/seqlock.h:35,
from include/linux/time.h:5,
from include/linux/efi.h:16,
include/asm-generic/memory_model.h:63:14: error: implicit declaration of function 'page_to_section' [-Werror=implicit-function-declaration]
int __sec = page_to_section(__pg); \
^
include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
#define page_to_pfn __page_to_pfn
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
^~~~~~~~~~~
In file included from include/linux/pid_namespace.h:6:0,
from include/linux/ptrace.h:9,
from include/uapi/linux/elfcore.h:7,
from include/linux/elfcore.h:9,
from include/linux/crash_core.h:5,
from include/linux/kexec.h:27,
include/linux/mm.h:979:29: error: conflicting types for 'page_to_section'
static inline unsigned long page_to_section(const struct page *page)
^~~~~~~~~~~~~~~
In file included from arch/arm/include/asm/memory.h:378:0,
from arch/arm/include/asm/page.h:165,
from arch/arm/include/asm/thread_info.h:17,
from include/linux/thread_info.h:37,
from include/asm-generic/preempt.h:4,
from ./arch/arm/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:80,
from include/linux/spinlock.h:50,
from include/linux/seqlock.h:35,
from include/linux/time.h:5,
from include/linux/efi.h:16,
from arch/arm/kernel/setup.c:10:
include/asm-generic/memory_model.h:63:14: note: previous implicit declaration of 'page_to_section' was here
int __sec = page_to_section(__pg); \
^
include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
#define page_to_pfn __page_to_pfn
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
^~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from arch/arm/include/asm/memory.h:378:0,
from arch/arm/include/asm/page.h:165,
from arch/arm/include/asm/thread_info.h:17,
from include/linux/thread_info.h:37,
from include/asm-generic/preempt.h:4,
from ./arch/arm/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:80,
from include/linux/spinlock.h:50,
include/asm-generic/memory_model.h:63:14: error: implicit declaration of function 'page_to_section' [-Werror=implicit-function-declaration]
int __sec = page_to_section(__pg); \
^
include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
#define page_to_pfn __page_to_pfn
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
^~~~~~~~~~~
include/asm-generic/memory_model.h:64:23: error: arithmetic on pointer to an incomplete type
(unsigned long)(__pg - __section_mem_map_addr(__nr_to_section(__sec))); \
^
include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
#define page_to_pfn __page_to_pfn
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
^~~~~~~~~~~
include/asm-generic/memory_model.h:69:9: error: invalid use of undefined type 'struct page'
struct mem_section *__sec = __pfn_to_section(__pfn); \
^
include/asm-generic/memory_model.h:81:21: note: in expansion of macro '__pfn_to_page'
#define pfn_to_page __pfn_to_page
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:76:9: note: in expansion of macro 'pfn_to_page'
return pfn_to_page(boot_pfn - (arch_phys_to_idmap_offset >> PAGE_SHIFT));
^~~~~~~~~~~
In file included from include/linux/pid_namespace.h:6:0,
from include/linux/ptrace.h:9,
from include/uapi/linux/elfcore.h:7,
from include/linux/elfcore.h:9,
from include/linux/crash_core.h:5,
from include/linux/kexec.h:27,
include/linux/mm.h:979:29: error: conflicting types for 'page_to_section'
static inline unsigned long page_to_section(const struct page *page)
^~~~~~~~~~~~~~~
In file included from arch/arm/include/asm/memory.h:378:0,
from arch/arm/include/asm/page.h:165,
from arch/arm/include/asm/thread_info.h:17,
from include/linux/thread_info.h:37,
from include/asm-generic/preempt.h:4,
from ./arch/arm/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:80,
from include/linux/spinlock.h:50,
from arch/arm/kernel/traps.c:18:
include/asm-generic/memory_model.h:63:14: note: previous implicit declaration of 'page_to_section' was here
int __sec = page_to_section(__pg); \
^
include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
#define page_to_pfn __page_to_pfn
^~~~~~~~~~~~~
arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
^~~~~~~~~~~
cc1: some warnings being treated as errors

vim +/pfn_to_page +76 arch/arm/include/asm/kexec.h

0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 60 #define phys_to_boot_phys phys_to_boot_phys
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 61
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 62 static inline phys_addr_t boot_phys_to_phys(unsigned long entry)
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 63 {
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 64 return idmap_to_phys(entry);
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 65 }
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 66 #define boot_phys_to_phys boot_phys_to_phys
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 67
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 68 static inline unsigned long page_to_boot_pfn(struct page *page)
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 69 {
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 70 return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 71 }
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 72 #define page_to_boot_pfn page_to_boot_pfn
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 73
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 74 static inline struct page *boot_pfn_to_page(unsigned long boot_pfn)
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 75 {
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 @76 return pfn_to_page(boot_pfn - (arch_phys_to_idmap_offset >> PAGE_SHIFT));
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 77 }
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 78 #define boot_pfn_to_page boot_pfn_to_page
0719392a arch/arm/include/asm/kexec.h Russell King 2016-08-02 79
c587e4a6 include/asm-arm/kexec.h Richard Purdie 2007-02-06 80 #endif /* __ASSEMBLY__ */
c587e4a6 include/asm-arm/kexec.h Richard Purdie 2007-02-06 81
c587e4a6 include/asm-arm/kexec.h Richard Purdie 2007-02-06 82 #endif /* CONFIG_KEXEC */
c587e4a6 include/asm-arm/kexec.h Richard Purdie 2007-02-06 83
c587e4a6 include/asm-arm/kexec.h Richard Purdie 2007-02-06 84 #endif /* _ARM_KEXEC_H */

:::::: The code at line 76 was first introduced by commit
:::::: 0719392a61a9dbc2c850bc7bd1a17efba953fcf5 ARM: kexec: fix kexec for Keystone 2

:::::: TO: Russell King <rmk+***@arm.linux.org.uk>
:::::: CC: Linus Torvalds <***@linux-foundation.org>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Michael Holzheu
2017-06-21 09:00:31 UTC
Permalink
Hi Xunlei,

Sorry for the late reply - I was on vacation up to now.
Give us some time to look into this issue.

Michael

Am Fri, 9 Jun 2017 10:17:05 +0800
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
---
arch/s390/include/asm/kexec.h | 2 +-
include/linux/crash_core.h | 7 +++++++
include/linux/kexec.h | 11 +----------
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
* Seven notes plus zero note at the end: prstatus, fpregset, timer,
* tod_cmp, tod_reg, control regs, and prefix
*/
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
(ALIGN(sizeof(struct elf_note), 4) * 8 + \
ALIGN(sizeof("CORE"), 4) * 7 + \
ALIGN(sizeof(struct elf_prstatus), 4) + \
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
+#endif
#define VMCOREINFO_BYTES PAGE_SIZE
#define VMCOREINFO_NOTE_NAME "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
#if !defined(__ASSEMBLY__)
-#include <linux/crash_core.h>
#include <asm/io.h>
#include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/module.h>
#include <asm/kexec.h>
+#include <linux/crash_core.h>
/* Verify architecture specific macros are defined */
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
* This structure is used to hold the arguments that are used when loading
* kernel binaries.
*/
Michael Holzheu
2017-06-21 17:44:23 UTC
Permalink
Am Fri, 9 Jun 2017 10:17:05 +0800
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
Hello Xunlei,

As you already know on s390 we create the ELF header in the new kernel.
Therefore we don't use the per-cpu buffers for ELF notes to store
the register state.

For RHEL7 we still store the registers in machine_kexec.c:add_elf_notes().
Though we also use the ELF header from new kernel ...

We assume your original problem with the "kmem -s" failure
was caused by the memory overwrite due to the invalid size of the
"crash_notes" per-cpu buffers.

Therefore your patch looks good for RHEL7 but for upstream we propose the
patch below.
---
[PATCH] s390/crash: Remove unused KEXEC_NOTE_BYTES

After commmit 692f66f26a4c19 ("crash: move crashkernel parsing and vmcore
related code under CONFIG_CRASH_CORE") the KEXEC_NOTE_BYTES macro is not
used anymore and for s390 we create the ELF header in the new kernel
anyway. Therefore remove the macro.

Reported-by: Xunlei Pang <***@redhat.com>
Reviewed-by: Mikhail Zaslonko <***@linux.vnet.ibm.com>
Signed-off-by: Michael Holzheu <***@linux.vnet.ibm.com>
---
arch/s390/include/asm/kexec.h | 18 ------------------
include/linux/crash_core.h | 5 +++++
include/linux/kexec.h | 9 ---------
3 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc30e35..dccf24ee26d3 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -41,24 +41,6 @@
/* The native architecture */
#define KEXEC_ARCH KEXEC_ARCH_S390

-/*
- * Size for s390x ELF notes per CPU
- *
- * Seven notes plus zero note at the end: prstatus, fpregset, timer,
- * tod_cmp, tod_reg, control regs, and prefix
- */
-#define KEXEC_NOTE_BYTES \
- (ALIGN(sizeof(struct elf_note), 4) * 8 + \
- ALIGN(sizeof("CORE"), 4) * 7 + \
- ALIGN(sizeof(struct elf_prstatus), 4) + \
- ALIGN(sizeof(elf_fpregset_t), 4) + \
- ALIGN(sizeof(u64), 4) + \
- ALIGN(sizeof(u64), 4) + \
- ALIGN(sizeof(u32), 4) + \
- ALIGN(sizeof(u64) * 16, 4) + \
- ALIGN(sizeof(u32), 4) \
- )
-
/* Provide a dummy definition to avoid build failures. */
static inline void crash_setup_regs(struct pt_regs *newregs,
struct pt_regs *oldregs) { }
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 541a197ba4a2..4090a42578a8 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,6 +10,11 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)

+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index c9481ebcbc0c..65888418fb69 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME

/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
* This structure is used to hold the arguments that are used when loading
* kernel binaries.
*/
--
2.11.2
Xunlei Pang
2017-06-22 09:12:07 UTC
Permalink
Post by Michael Holzheu
Am Fri, 9 Jun 2017 10:17:05 +0800
Post by Xunlei Pang
S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.
This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
Hello Xunlei,
As you already know on s390 we create the ELF header in the new kernel.
Therefore we don't use the per-cpu buffers for ELF notes to store
the register state.
For RHEL7 we still store the registers in machine_kexec.c:add_elf_notes().
Though we also use the ELF header from new kernel ...
We assume your original problem with the "kmem -s" failure
was caused by the memory overwrite due to the invalid size of the
"crash_notes" per-cpu buffers.
Therefore your patch looks good for RHEL7 but for upstream we propose the
patch below.
Hi Michael,

Yes, we already did this way.
Thanks for the confirmation, the patch below looks good to me.

Regards,
Xunlei
Post by Michael Holzheu
---
[PATCH] s390/crash: Remove unused KEXEC_NOTE_BYTES
After commmit 692f66f26a4c19 ("crash: move crashkernel parsing and vmcore
related code under CONFIG_CRASH_CORE") the KEXEC_NOTE_BYTES macro is not
used anymore and for s390 we create the ELF header in the new kernel
anyway. Therefore remove the macro.
---
arch/s390/include/asm/kexec.h | 18 ------------------
include/linux/crash_core.h | 5 +++++
include/linux/kexec.h | 9 ---------
3 files changed, 5 insertions(+), 27 deletions(-)
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc30e35..dccf24ee26d3 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -41,24 +41,6 @@
/* The native architecture */
#define KEXEC_ARCH KEXEC_ARCH_S390
-/*
- * Size for s390x ELF notes per CPU
- *
- * Seven notes plus zero note at the end: prstatus, fpregset, timer,
- * tod_cmp, tod_reg, control regs, and prefix
- */
-#define KEXEC_NOTE_BYTES \
- (ALIGN(sizeof(struct elf_note), 4) * 8 + \
- ALIGN(sizeof("CORE"), 4) * 7 + \
- ALIGN(sizeof(struct elf_prstatus), 4) + \
- ALIGN(sizeof(elf_fpregset_t), 4) + \
- ALIGN(sizeof(u64), 4) + \
- ALIGN(sizeof(u64), 4) + \
- ALIGN(sizeof(u32), 4) + \
- ALIGN(sizeof(u64) * 16, 4) + \
- ALIGN(sizeof(u32), 4) \
- )
-
/* Provide a dummy definition to avoid build failures. */
static inline void crash_setup_regs(struct pt_regs *newregs,
struct pt_regs *oldregs) { }
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 541a197ba4a2..4090a42578a8 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,6 +10,11 @@
#define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
#define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header. For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
#define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \
CRASH_CORE_NOTE_NAME_BYTES + \
CRASH_CORE_NOTE_DESC_BYTES)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index c9481ebcbc0c..65888418fb69 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -63,15 +63,6 @@
#define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
/*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header. For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
* This structure is used to hold the arguments that are used when loading
* kernel binaries.
*/
Loading...