Re: [PATCH v4 12/13] s390/kexec: refactor for kernel/Kconfig.kexec
From: Alexander Gordeev
Date: Thu Jul 06 2023 - 11:59:58 EST
On Wed, Jul 05, 2023 at 08:49:58AM -0700, Nathan Chancellor wrote:
...
> I just bisected the following build failure visible with 'ARCH=s390
> allnoconfig' to this change as commit 842ce0e1dafa ("s390/kexec:
> refactor for kernel/Kconfig.kexec") in -next.
>
> arch/s390/kernel/machine_kexec.c:120:37: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
> 120 | static bool kdump_csum_valid(struct kimage *image)
> | ^~~~~~
> arch/s390/kernel/machine_kexec.c:188:34: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
> 188 | int machine_kexec_prepare(struct kimage *image)
> | ^~~~~~
> arch/s390/kernel/machine_kexec.c: In function 'machine_kexec_prepare':
> arch/s390/kernel/machine_kexec.c:192:18: error: invalid use of undefined type 'struct kimage'
> 192 | if (image->type == KEXEC_TYPE_CRASH)
> | ^~
> arch/s390/kernel/machine_kexec.c:192:28: error: 'KEXEC_TYPE_CRASH' undeclared (first use in this function); did you mean 'KEXEC_ON_CRASH'?
> 192 | if (image->type == KEXEC_TYPE_CRASH)
> | ^~~~~~~~~~~~~~~~
> | KEXEC_ON_CRASH
> arch/s390/kernel/machine_kexec.c:192:28: note: each undeclared identifier is reported only once for each function it appears in
> arch/s390/kernel/machine_kexec.c:196:18: error: invalid use of undefined type 'struct kimage'
> 196 | if (image->type != KEXEC_TYPE_DEFAULT)
> | ^~
> arch/s390/kernel/machine_kexec.c:196:28: error: 'KEXEC_TYPE_DEFAULT' undeclared (first use in this function); did you mean 'KEXEC_ARCH_DEFAULT'?
> 196 | if (image->type != KEXEC_TYPE_DEFAULT)
> | ^~~~~~~~~~~~~~~~~~
> | KEXEC_ARCH_DEFAULT
> In file included from arch/s390/include/asm/thread_info.h:31,
> from include/linux/thread_info.h:60,
> from arch/s390/include/asm/preempt.h:6,
> from include/linux/preempt.h:79,
> from arch/s390/include/asm/percpu.h:5,
> from include/linux/irqflags.h:18,
> from include/linux/rcupdate.h:26,
> from include/linux/rculist.h:11,
> from include/linux/pid.h:5,
> from include/linux/sched.h:14,
> from include/linux/ratelimit.h:6,
> from include/linux/dev_printk.h:16,
> from include/linux/device.h:15,
> from arch/s390/kernel/machine_kexec.c:9:
> arch/s390/kernel/machine_kexec.c:200:48: error: invalid use of undefined type 'struct kimage'
> 200 | reboot_code_buffer = page_to_virt(image->control_code_page);
> | ^~
> arch/s390/include/asm/page.h:186:58: note: in definition of macro '__va'
> 186 | #define __va(x) ((void *)(unsigned long)(x))
> | ^
> arch/s390/include/asm/page.h:194:38: note: in expansion of macro 'pfn_to_phys'
> 194 | #define pfn_to_virt(pfn) __va(pfn_to_phys(pfn))
> | ^~~~~~~~~~~
> arch/s390/include/asm/page.h:199:33: note: in expansion of macro 'pfn_to_virt'
> 199 | #define page_to_virt(page) pfn_to_virt(page_to_pfn(page))
> | ^~~~~~~~~~~
> include/asm-generic/memory_model.h:64:21: note: in expansion of macro '__page_to_pfn'
> 64 | #define page_to_pfn __page_to_pfn
> | ^~~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:200:30: note: in expansion of macro 'page_to_virt'
> 200 | reboot_code_buffer = page_to_virt(image->control_code_page);
> | ^~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c: At top level:
> arch/s390/kernel/machine_kexec.c:207:35: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
> 207 | void machine_kexec_cleanup(struct kimage *image)
> | ^~~~~~
> arch/s390/kernel/machine_kexec.c: In function '__do_machine_kexec':
> arch/s390/kernel/machine_kexec.c:243:40: error: invalid use of undefined type 'struct kimage'
> 243 | data_mover = page_to_phys(image->control_code_page);
> | ^~
> arch/s390/include/asm/page.h:189:35: note: in definition of macro 'pfn_to_phys'
> 189 | #define pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT)
> | ^~~
> include/asm-generic/memory_model.h:64:21: note: in expansion of macro '__page_to_pfn'
> 64 | #define page_to_pfn __page_to_pfn
> | ^~~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:243:22: note: in expansion of macro 'page_to_phys'
> 243 | data_mover = page_to_phys(image->control_code_page);
> | ^~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:244:36: error: invalid use of undefined type 'struct kimage'
> 244 | entry = virt_to_phys(&image->head);
> | ^~
> In file included from arch/s390/kernel/machine_kexec.c:27:
> arch/s390/kernel/machine_kexec.c:252:40: error: invalid use of undefined type 'struct kimage'
> 252 | unsigned long, image->start,
> | ^~
> arch/s390/include/asm/stacktrace.h:101:32: note: in definition of macro 'CALL_LARGS_2'
> 101 | long arg2 = (long)(t2)(a2)
> | ^~
> arch/s390/include/asm/stacktrace.h:216:9: note: in expansion of macro 'CALL_LARGS_3'
> 216 | CALL_LARGS_##nr(__VA_ARGS__); \
> | ^~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
> 250 | call_nodat(3, void, (relocate_kernel_t)data_mover,
> | ^~~~~~~~~~
> In file included from include/linux/irqflags.h:15:
> arch/s390/kernel/machine_kexec.c:252:40: error: invalid use of undefined type 'struct kimage'
> 252 | unsigned long, image->start,
> | ^~
> include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
> 11 | typeof(x) __dummy2; \
> | ^
> arch/s390/include/asm/stacktrace.h:136:9: note: in expansion of macro 'CALL_TYPECHECK_2'
> 136 | CALL_TYPECHECK_2(__VA_ARGS__); \
> | ^~~~~~~~~~~~~~~~
> arch/s390/include/asm/stacktrace.h:219:9: note: in expansion of macro 'CALL_TYPECHECK_3'
> 219 | CALL_TYPECHECK_##nr(__VA_ARGS__); \
> | ^~~~~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
> 250 | call_nodat(3, void, (relocate_kernel_t)data_mover,
> | ^~~~~~~~~~
> include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
> 12 | (void)(&__dummy == &__dummy2); \
> | ^~
> arch/s390/include/asm/stacktrace.h:134:9: note: in expansion of macro 'typecheck'
> 134 | typecheck(t, a)
> | ^~~~~~~~~
> arch/s390/include/asm/stacktrace.h:136:9: note: in expansion of macro 'CALL_TYPECHECK_2'
> 136 | CALL_TYPECHECK_2(__VA_ARGS__); \
> | ^~~~~~~~~~~~~~~~
> arch/s390/include/asm/stacktrace.h:219:9: note: in expansion of macro 'CALL_TYPECHECK_3'
> 219 | CALL_TYPECHECK_##nr(__VA_ARGS__); \
> | ^~~~~~~~~~~~~~~
> arch/s390/kernel/machine_kexec.c:250:9: note: in expansion of macro 'call_nodat'
> 250 | call_nodat(3, void, (relocate_kernel_t)data_mover,
> | ^~~~~~~~~~
> arch/s390/kernel/machine_kexec.c: At top level:
> arch/s390/kernel/machine_kexec.c:278:27: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
> 278 | void machine_kexec(struct kimage *image)
> | ^~~~~~
> arch/s390/kernel/machine_kexec.c: In function 'machine_kexec':
> arch/s390/kernel/machine_kexec.c:280:18: error: invalid use of undefined type 'struct kimage'
> 280 | if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
> | ^~
> arch/s390/kernel/machine_kexec.c:280:28: error: 'KEXEC_TYPE_CRASH' undeclared (first use in this function); did you mean 'KEXEC_ON_CRASH'?
> 280 | if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
> | ^~~~~~~~~~~~~~~~
> | KEXEC_ON_CRASH
> arch/s390/kernel/machine_kexec.c:280:66: error: passing argument 1 of 'kdump_csum_valid' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 280 | if (image->type == KEXEC_TYPE_CRASH && !kdump_csum_valid(image))
> | ^~~~~
> | |
> | struct kimage *
> arch/s390/kernel/machine_kexec.c:120:45: note: expected 'struct kimage *' but argument is of type 'struct kimage *'
> 120 | static bool kdump_csum_valid(struct kimage *image)
> | ~~~~~~~~~~~~~~~^~~~~
> cc1: some warnings being treated as errors
>
> I don't think this change is equivalent for s390, which had
>
> config KEXEC
> def_bool y
> select KEXEC_CORE
>
> but it is now the equivalent of
>
> config KEXEC
> bool "Enable kexec system call"
> default y
>
> which enables KEXEC by default but it also allows KEXEC to be disabled
> for s390 now, because it is a user-visible symbol, not one that is
> unconditionally enabled no matter what. If s390 can tolerate KEXEC being
> user selectable, then I assume the fix is just adjusting
> arch/s390/kernel/Makefile to only build the machine_kexec files when
> CONFIG_KEXEC_CORE is set:
>
> diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
> index 6b2a051e1f8a..a06b39da95f0 100644
> --- a/arch/s390/kernel/Makefile
> +++ b/arch/s390/kernel/Makefile
> @@ -37,10 +37,10 @@ CFLAGS_unwind_bc.o += -fno-optimize-sibling-calls
> obj-y := head64.o traps.o time.o process.o earlypgm.o early.o setup.o idle.o vtime.o
> obj-y += processor.o syscall.o ptrace.o signal.o cpcmd.o ebcdic.o nmi.o
> obj-y += debug.o irq.o ipl.o dis.o diag.o vdso.o cpufeature.o
> -obj-y += sysinfo.o lgr.o os_info.o machine_kexec.o
> +obj-y += sysinfo.o lgr.o os_info.o
> obj-y += runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
> obj-y += entry.o reipl.o relocate_kernel.o kdebugfs.o alternative.o
> -obj-y += nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
> +obj-y += nospec-branch.o ipl_vmparm.o unwind_bc.o
> obj-y += smp.o text_amode31.o stacktrace.o abs_lowcore.o
>
> extra-y += vmlinux.lds
> @@ -66,6 +66,7 @@ obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> obj-$(CONFIG_UPROBES) += uprobes.o
> obj-$(CONFIG_JUMP_LABEL) += jump_label.o
>
> +obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o machine_kexec_reloc.o
> obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o
> obj-$(CONFIG_KEXEC_FILE) += kexec_elf.o
>
>
> Otherwise, the prompt for KEXEC could be made conditional on some ARCH
> symbol so that architectures can opt out of it.
Hi Nathan,
Thanks a lot for looking into it!
With few modification the fix would looke like below.
It probably needs to be a pre-requisite for this series:
[PATCH] s390/kexec: make machine_kexec depend on CONFIG_KEXEC_CORE
Make machine_kexec.o and relocate_kernel.o depend on
CONFIG_KEXEC_CORE option as other architectures do.
Still generate machine_kexec_reloc.o unconditionally,
since arch_kexec_do_relocs() function is neded by the
decompressor.
Probably, #include <asm/kexec.h> could be be removed from
machine_kexec_reloc.c source as well, but that would revert
commit 155e6c706125 ("s390/kexec: add missing include to
machine_kexec_reloc.c").
Suggested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxxxxx>
---
arch/s390/kernel/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
index 8d7514c72bb8..0df2b88cc0da 100644
--- a/arch/s390/kernel/Makefile
+++ b/arch/s390/kernel/Makefile
@@ -37,9 +37,9 @@ CFLAGS_unwind_bc.o += -fno-optimize-sibling-calls
obj-y := head64.o traps.o time.o process.o earlypgm.o early.o setup.o idle.o vtime.o
obj-y += processor.o syscall.o ptrace.o signal.o cpcmd.o ebcdic.o nmi.o
obj-y += debug.o irq.o ipl.o dis.o diag.o vdso.o cpufeature.o
-obj-y += sysinfo.o lgr.o os_info.o machine_kexec.o
+obj-y += sysinfo.o lgr.o os_info.o
obj-y += runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
-obj-y += entry.o reipl.o relocate_kernel.o kdebugfs.o alternative.o
+obj-y += entry.o reipl.o kdebugfs.o alternative.o
obj-y += nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
obj-y += smp.o text_amode31.o stacktrace.o abs_lowcore.o
@@ -63,6 +63,7 @@ obj-$(CONFIG_RETHOOK) += rethook.o
obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o
obj-$(CONFIG_FUNCTION_TRACER) += mcount.o
obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
+obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o relocate_kernel.o
obj-$(CONFIG_UPROBES) += uprobes.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> Cheers,
> Nathan
Thanks!