Re: [PATCH v4 12/13] s390/kexec: refactor for kernel/Kconfig.kexec

Eric DeVolder <eric.devolder@xxxxxxxxxx> · Thu, 6 Jul 2023 11:07:13 -0500

On 7/6/23 10:58, Alexander Gordeev wrote:
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!

A bit of additional information. I've corrected the problem with s390 and now all config files pass 
with the olddefconfig, allyesconfig and allnoconfig targets (using approach outlined in the cover 
letter). What I did to resolve the last s390 problem is that I realized that KEXEC was 
unconditionally set, so I did the same by adding 'select KEXEC' to the config S390 section.

I'm not saying you shouldn't do the above changes, but wanted to make you aware that we're attacking 
the same problem...

I'm running my final regression run here (takes about 8 hours to get through all 385 now) and then I 
hope to post v5 within a day.

Thanks,
eric






