diff mbox series

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

Message ID 20230705142004.3605799-13-eric.devolder@oracle.com (mailing list archive)
State Superseded
Headers show
Series refactor Kconfig to consolidate KEXEC and CRASH options | expand

Commit Message

Eric DeVolder July 5, 2023, 2:20 p.m. UTC
The kexec and crash kernel options are provided in the common
kernel/Kconfig.kexec. Utilize the common options and provide
the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
equivalent set of KEXEC and CRASH options.

NOTE: The original Kconfig has a KEXEC_SIG which depends on
MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
dependency (using the strategy outlined in this series, and other
techniques) results in 'error: recursive dependency detected'
on CRYPTO.

Per Alexander Gordeev <agordeev@linux.ibm.com>: "the MODULE_SIG_FORMAT
dependency was introduced with [git commit below] and in fact was not
necessary, since s390 did/does not use mod_check_sig() anyway.

 commit c8424e776b09 ("MODSIGN: Export module signature definitions")

MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION. But
SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so dropping
MODULE_SIG_FORMAT does not hurt."

Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
from KEXEC_SIG. Still results in equivalent .config files for s390.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>
---
 arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 46 deletions(-)

Comments

Nathan Chancellor July 5, 2023, 3:49 p.m. UTC | #1
Hi Eric,

On Wed, Jul 05, 2023 at 10:20:03AM -0400, Eric DeVolder wrote:
> The kexec and crash kernel options are provided in the common
> kernel/Kconfig.kexec. Utilize the common options and provide
> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
> equivalent set of KEXEC and CRASH options.
> 
> NOTE: The original Kconfig has a KEXEC_SIG which depends on
> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
> dependency (using the strategy outlined in this series, and other
> techniques) results in 'error: recursive dependency detected'
> on CRYPTO.
> 
> Per Alexander Gordeev <agordeev@linux.ibm.com>: "the MODULE_SIG_FORMAT
> dependency was introduced with [git commit below] and in fact was not
> necessary, since s390 did/does not use mod_check_sig() anyway.
> 
>  commit c8424e776b09 ("MODSIGN: Export module signature definitions")
> 
> MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION. But
> SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so dropping
> MODULE_SIG_FORMAT does not hurt."
> 
> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
> from KEXEC_SIG. Still results in equivalent .config files for s390.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
>  arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
>  1 file changed, 19 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 5b39918b7042..5d4fbbfdd1cd 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -244,6 +244,25 @@ config PGTABLE_LEVELS
>  
>  source "kernel/livepatch/Kconfig"
>  
> +config ARCH_DEFAULT_KEXEC
> +	def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC
> +	def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC_FILE
> +	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +
> +config ARCH_HAS_KEXEC_PURGATORY
> +	def_bool KEXEC_FILE
> +
> +config ARCH_SUPPORTS_CRASH_DUMP
> +	def_bool y
> +	help
> +	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
> +	  This option also enables s390 zfcpdump.
> +	  See also <file:Documentation/s390/zfcpdump.rst>
> +
>  menu "Processor type and features"
>  
>  config HAVE_MARCH_Z10_FEATURES
> @@ -482,36 +501,6 @@ config SCHED_TOPOLOGY
>  
>  source "kernel/Kconfig.hz"
>  
> -config KEXEC
> -	def_bool y
> -	select KEXEC_CORE
> -
> -config KEXEC_FILE
> -	bool "kexec file based system call"
> -	select KEXEC_CORE
> -	depends on CRYPTO
> -	depends on CRYPTO_SHA256
> -	depends on CRYPTO_SHA256_S390
> -	help
> -	  Enable the kexec file based system call. In contrast to the normal
> -	  kexec system call this system call takes file descriptors for the
> -	  kernel and initramfs as arguments.
> -
> -config ARCH_HAS_KEXEC_PURGATORY
> -	def_bool y
> -	depends on KEXEC_FILE
> -
> -config KEXEC_SIG
> -	bool "Verify kernel signature during kexec_file_load() syscall"
> -	depends on KEXEC_FILE && MODULE_SIG_FORMAT
> -	help
> -	  This option makes kernel signature verification mandatory for
> -	  the kexec_file_load() syscall.
> -
> -	  In addition to that option, you need to enable signature
> -	  verification for the corresponding kernel image type being
> -	  loaded in order for this to work.
> -
>  config KERNEL_NOBP
>  	def_bool n
>  	prompt "Enable modified branch prediction for the kernel by default"
> @@ -733,22 +722,6 @@ config VFIO_AP
>  
>  endmenu
>  
> -menu "Dump support"
> -
> -config CRASH_DUMP
> -	bool "kernel crash dumps"
> -	select KEXEC
> -	help
> -	  Generate crash dump after being started by kexec.
> -	  Crash dump kernels are loaded in the main kernel with kexec-tools
> -	  into a specially reserved region and then later executed after
> -	  a crash by kdump/kexec.
> -	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
> -	  This option also enables s390 zfcpdump.
> -	  See also <file:Documentation/s390/zfcpdump.rst>
> -
> -endmenu
> -
>  config CCW
>  	def_bool y
>  
> -- 
> 2.31.1
> 

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.

As an aside, is this intended for the 6.5 merge window? If not, it
shouldn't be in -next at this point, I was surprised to see new broken
builds.

Cheers,
Nathan
Eric DeVolder July 5, 2023, 4:23 p.m. UTC | #2
On 7/5/23 10:49, Nathan Chancellor wrote:
> Hi Eric,
> 
> On Wed, Jul 05, 2023 at 10:20:03AM -0400, Eric DeVolder wrote:
>> The kexec and crash kernel options are provided in the common
>> kernel/Kconfig.kexec. Utilize the common options and provide
>> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
>> equivalent set of KEXEC and CRASH options.
>>
>> NOTE: The original Kconfig has a KEXEC_SIG which depends on
>> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
>> dependency (using the strategy outlined in this series, and other
>> techniques) results in 'error: recursive dependency detected'
>> on CRYPTO.
>>
>> Per Alexander Gordeev <agordeev@linux.ibm.com>: "the MODULE_SIG_FORMAT
>> dependency was introduced with [git commit below] and in fact was not
>> necessary, since s390 did/does not use mod_check_sig() anyway.
>>
>>   commit c8424e776b09 ("MODSIGN: Export module signature definitions")
>>
>> MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION. But
>> SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so dropping
>> MODULE_SIG_FORMAT does not hurt."
>>
>> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
>> from KEXEC_SIG. Still results in equivalent .config files for s390.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>
>> ---
>>   arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
>>   1 file changed, 19 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>> index 5b39918b7042..5d4fbbfdd1cd 100644
>> --- a/arch/s390/Kconfig
>> +++ b/arch/s390/Kconfig
>> @@ -244,6 +244,25 @@ config PGTABLE_LEVELS
>>   
>>   source "kernel/livepatch/Kconfig"
>>   
>> +config ARCH_DEFAULT_KEXEC
>> +	def_bool y
>> +
>> +config ARCH_SUPPORTS_KEXEC
>> +	def_bool y
>> +
>> +config ARCH_SUPPORTS_KEXEC_FILE
>> +	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
>> +
>> +config ARCH_HAS_KEXEC_PURGATORY
>> +	def_bool KEXEC_FILE
>> +
>> +config ARCH_SUPPORTS_CRASH_DUMP
>> +	def_bool y
>> +	help
>> +	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>> +	  This option also enables s390 zfcpdump.
>> +	  See also <file:Documentation/s390/zfcpdump.rst>
>> +
>>   menu "Processor type and features"
>>   
>>   config HAVE_MARCH_Z10_FEATURES
>> @@ -482,36 +501,6 @@ config SCHED_TOPOLOGY
>>   
>>   source "kernel/Kconfig.hz"
>>   
>> -config KEXEC
>> -	def_bool y
>> -	select KEXEC_CORE
>> -
>> -config KEXEC_FILE
>> -	bool "kexec file based system call"
>> -	select KEXEC_CORE
>> -	depends on CRYPTO
>> -	depends on CRYPTO_SHA256
>> -	depends on CRYPTO_SHA256_S390
>> -	help
>> -	  Enable the kexec file based system call. In contrast to the normal
>> -	  kexec system call this system call takes file descriptors for the
>> -	  kernel and initramfs as arguments.
>> -
>> -config ARCH_HAS_KEXEC_PURGATORY
>> -	def_bool y
>> -	depends on KEXEC_FILE
>> -
>> -config KEXEC_SIG
>> -	bool "Verify kernel signature during kexec_file_load() syscall"
>> -	depends on KEXEC_FILE && MODULE_SIG_FORMAT
>> -	help
>> -	  This option makes kernel signature verification mandatory for
>> -	  the kexec_file_load() syscall.
>> -
>> -	  In addition to that option, you need to enable signature
>> -	  verification for the corresponding kernel image type being
>> -	  loaded in order for this to work.
>> -
>>   config KERNEL_NOBP
>>   	def_bool n
>>   	prompt "Enable modified branch prediction for the kernel by default"
>> @@ -733,22 +722,6 @@ config VFIO_AP
>>   
>>   endmenu
>>   
>> -menu "Dump support"
>> -
>> -config CRASH_DUMP
>> -	bool "kernel crash dumps"
>> -	select KEXEC
>> -	help
>> -	  Generate crash dump after being started by kexec.
>> -	  Crash dump kernels are loaded in the main kernel with kexec-tools
>> -	  into a specially reserved region and then later executed after
>> -	  a crash by kdump/kexec.
>> -	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>> -	  This option also enables s390 zfcpdump.
>> -	  See also <file:Documentation/s390/zfcpdump.rst>
>> -
>> -endmenu
>> -
>>   config CCW
>>   	def_bool y
>>   
>> -- 
>> 2.31.1
>>
> 
> 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.

Nathan,
Thanks for looking at this! I've been receiving broken build info from Andrew's
machinery. I've investigated and learned that CRASH_DUMP can be specified without
KEXEC. I also realized that s390 originally had this right, but in the conversion
I did, I got it wrong.

This v4 series corrects that by having CRASH_DUMP select KEXEC.

The new KEXEC looks like:

config KEXEC
     bool "Enable kexec system call"
     default ARCH_DEFAULT_KEXEC
     depends on ARCH_SUPPORTS_KEXEC
     select KEXEC_CORE

which appears to be equivalent, I think the CRASH_DUMP issue is the root problem.

In a separate thread with Arnd Bergmann wrt/ arm build issues, he identifies
similar problems&solutions as you did, but points out that CRASH_DUMP might still
need some refinement; I'm looking into that.

The goal really is to make this series to result in equivalent configs as before,
but there are small problems in the conversion that are showing up that I'm working.


> 
> As an aside, is this intended for the 6.5 merge window? If not, it
> shouldn't be in -next at this point, I was surprised to see new broken
> builds.
> 
> Cheers,
> Nathan
I'm not going to pretend I know when this will make it...
eric
Eric DeVolder July 5, 2023, 7:44 p.m. UTC | #3
On 7/5/23 11:23, Eric DeVolder wrote:
> 
> 
> On 7/5/23 10:49, Nathan Chancellor wrote:
>> Hi Eric,
>>
>> On Wed, Jul 05, 2023 at 10:20:03AM -0400, Eric DeVolder wrote:
>>> The kexec and crash kernel options are provided in the common
>>> kernel/Kconfig.kexec. Utilize the common options and provide
>>> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
>>> equivalent set of KEXEC and CRASH options.
>>>
>>> NOTE: The original Kconfig has a KEXEC_SIG which depends on
>>> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
>>> dependency (using the strategy outlined in this series, and other
>>> techniques) results in 'error: recursive dependency detected'
>>> on CRYPTO.
>>>
>>> Per Alexander Gordeev <agordeev@linux.ibm.com>: "the MODULE_SIG_FORMAT
>>> dependency was introduced with [git commit below] and in fact was not
>>> necessary, since s390 did/does not use mod_check_sig() anyway.
>>>
>>>   commit c8424e776b09 ("MODSIGN: Export module signature definitions")
>>>
>>> MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION. But
>>> SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so dropping
>>> MODULE_SIG_FORMAT does not hurt."
>>>
>>> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
>>> from KEXEC_SIG. Still results in equivalent .config files for s390.
>>>
>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>> Acked-by: Alexander Gordeev <agordeev@linux.ibm.com>
>>> ---
>>>   arch/s390/Kconfig | 65 ++++++++++++++---------------------------------
>>>   1 file changed, 19 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>>> index 5b39918b7042..5d4fbbfdd1cd 100644
>>> --- a/arch/s390/Kconfig
>>> +++ b/arch/s390/Kconfig
>>> @@ -244,6 +244,25 @@ config PGTABLE_LEVELS
>>>   source "kernel/livepatch/Kconfig"
>>> +config ARCH_DEFAULT_KEXEC
>>> +    def_bool y
>>> +
>>> +config ARCH_SUPPORTS_KEXEC
>>> +    def_bool y
>>> +
>>> +config ARCH_SUPPORTS_KEXEC_FILE
>>> +    def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
>>> +
>>> +config ARCH_HAS_KEXEC_PURGATORY
>>> +    def_bool KEXEC_FILE
>>> +
>>> +config ARCH_SUPPORTS_CRASH_DUMP
>>> +    def_bool y
>>> +    help
>>> +      Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>>> +      This option also enables s390 zfcpdump.
>>> +      See also <file:Documentation/s390/zfcpdump.rst>
>>> +
>>>   menu "Processor type and features"
>>>   config HAVE_MARCH_Z10_FEATURES
>>> @@ -482,36 +501,6 @@ config SCHED_TOPOLOGY
>>>   source "kernel/Kconfig.hz"
>>> -config KEXEC
>>> -    def_bool y
>>> -    select KEXEC_CORE
>>> -
>>> -config KEXEC_FILE
>>> -    bool "kexec file based system call"
>>> -    select KEXEC_CORE
>>> -    depends on CRYPTO
>>> -    depends on CRYPTO_SHA256
>>> -    depends on CRYPTO_SHA256_S390
>>> -    help
>>> -      Enable the kexec file based system call. In contrast to the normal
>>> -      kexec system call this system call takes file descriptors for the
>>> -      kernel and initramfs as arguments.
>>> -
>>> -config ARCH_HAS_KEXEC_PURGATORY
>>> -    def_bool y
>>> -    depends on KEXEC_FILE
>>> -
>>> -config KEXEC_SIG
>>> -    bool "Verify kernel signature during kexec_file_load() syscall"
>>> -    depends on KEXEC_FILE && MODULE_SIG_FORMAT
>>> -    help
>>> -      This option makes kernel signature verification mandatory for
>>> -      the kexec_file_load() syscall.
>>> -
>>> -      In addition to that option, you need to enable signature
>>> -      verification for the corresponding kernel image type being
>>> -      loaded in order for this to work.
>>> -
>>>   config KERNEL_NOBP
>>>       def_bool n
>>>       prompt "Enable modified branch prediction for the kernel by default"
>>> @@ -733,22 +722,6 @@ config VFIO_AP
>>>   endmenu
>>> -menu "Dump support"
>>> -
>>> -config CRASH_DUMP
>>> -    bool "kernel crash dumps"
>>> -    select KEXEC
>>> -    help
>>> -      Generate crash dump after being started by kexec.
>>> -      Crash dump kernels are loaded in the main kernel with kexec-tools
>>> -      into a specially reserved region and then later executed after
>>> -      a crash by kdump/kexec.
>>> -      Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
>>> -      This option also enables s390 zfcpdump.
>>> -      See also <file:Documentation/s390/zfcpdump.rst>
>>> -
>>> -endmenu
>>> -
>>>   config CCW
>>>       def_bool y
>>> -- 
>>> 2.31.1
>>>
>>
>> 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.
> 
> Nathan,
> Thanks for looking at this! I've been receiving broken build info from Andrew's
> machinery. I've investigated and learned that CRASH_DUMP can be specified without
> KEXEC. I also realized that s390 originally had this right, but in the conversion
> I did, I got it wrong.
> 
> This v4 series corrects that by having CRASH_DUMP select KEXEC.
> 
> The new KEXEC looks like:
> 
> config KEXEC
>      bool "Enable kexec system call"
>      default ARCH_DEFAULT_KEXEC
>      depends on ARCH_SUPPORTS_KEXEC
>      select KEXEC_CORE
> 
> which appears to be equivalent, I think the CRASH_DUMP issue is the root problem.
> 
> In a separate thread with Arnd Bergmann wrt/ arm build issues, he identifies
> similar problems&solutions as you did, but points out that CRASH_DUMP might still
> need some refinement; I'm looking into that.
> 
> The goal really is to make this series to result in equivalent configs as before,
> but there are small problems in the conversion that are showing up that I'm working.

Nathan,
Thanks again for looking at this! Your feedback made me realize that relying on
'olddefconfig' for equivalence has proven to be inadequate. As a result, I've
retooled my testing environment to use olddefconfig, allnoconfig and allyesconfig.
That enhancement has revealed a small number of issues, which I have now fixed.

I'll be posting a v5 once I've worked through all architecture results.

Thanks again!
eric

> 
> 
>>
>> As an aside, is this intended for the 6.5 merge window? If not, it
>> shouldn't be in -next at this point, I was surprised to see new broken
>> builds.
>>
>> Cheers,
>> Nathan
> I'm not going to pretend I know when this will make it...
> eric
Alexander Gordeev July 6, 2023, 3:58 p.m. UTC | #4
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@kernel.org>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
---
 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!
Eric DeVolder July 6, 2023, 4:07 p.m. UTC | #5
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@kernel.org>
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
>   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
diff mbox series

Patch

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 5b39918b7042..5d4fbbfdd1cd 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -244,6 +244,25 @@  config PGTABLE_LEVELS
 
 source "kernel/livepatch/Kconfig"
 
+config ARCH_DEFAULT_KEXEC
+	def_bool y
+
+config ARCH_SUPPORTS_KEXEC
+	def_bool y
+
+config ARCH_SUPPORTS_KEXEC_FILE
+	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+
+config ARCH_HAS_KEXEC_PURGATORY
+	def_bool KEXEC_FILE
+
+config ARCH_SUPPORTS_CRASH_DUMP
+	def_bool y
+	help
+	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
+	  This option also enables s390 zfcpdump.
+	  See also <file:Documentation/s390/zfcpdump.rst>
+
 menu "Processor type and features"
 
 config HAVE_MARCH_Z10_FEATURES
@@ -482,36 +501,6 @@  config SCHED_TOPOLOGY
 
 source "kernel/Kconfig.hz"
 
-config KEXEC
-	def_bool y
-	select KEXEC_CORE
-
-config KEXEC_FILE
-	bool "kexec file based system call"
-	select KEXEC_CORE
-	depends on CRYPTO
-	depends on CRYPTO_SHA256
-	depends on CRYPTO_SHA256_S390
-	help
-	  Enable the kexec file based system call. In contrast to the normal
-	  kexec system call this system call takes file descriptors for the
-	  kernel and initramfs as arguments.
-
-config ARCH_HAS_KEXEC_PURGATORY
-	def_bool y
-	depends on KEXEC_FILE
-
-config KEXEC_SIG
-	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
-	help
-	  This option makes kernel signature verification mandatory for
-	  the kexec_file_load() syscall.
-
-	  In addition to that option, you need to enable signature
-	  verification for the corresponding kernel image type being
-	  loaded in order for this to work.
-
 config KERNEL_NOBP
 	def_bool n
 	prompt "Enable modified branch prediction for the kernel by default"
@@ -733,22 +722,6 @@  config VFIO_AP
 
 endmenu
 
-menu "Dump support"
-
-config CRASH_DUMP
-	bool "kernel crash dumps"
-	select KEXEC
-	help
-	  Generate crash dump after being started by kexec.
-	  Crash dump kernels are loaded in the main kernel with kexec-tools
-	  into a specially reserved region and then later executed after
-	  a crash by kdump/kexec.
-	  Refer to <file:Documentation/s390/zfcpdump.rst> for more details on this.
-	  This option also enables s390 zfcpdump.
-	  See also <file:Documentation/s390/zfcpdump.rst>
-
-endmenu
-
 config CCW
 	def_bool y