Message-ID: <de4c557a-c592-d9f7-d516-bc746d153fa1@oracle.com>
Date:   Mon, 12 Jun 2023 12:32:27 -0500
From:   Eric DeVolder <eric.devolder@...cle.com>
To:     linux@...linux.org.uk, catalin.marinas@....com, will@...nel.org,
        chenhuacai@...nel.org, geert@...ux-m68k.org,
        tsbogend@...ha.franken.de, James.Bottomley@...senPartnership.com,
        deller@....de, ysato@...rs.sourceforge.jp, dalias@...c.org,
        glaubitz@...sik.fu-berlin.de, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, 86@...nel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-ia64@...r.kernel.org, loongarch@...ts.linux.dev,
        linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org
Cc:     kernel@...0n.name, mpe@...erman.id.au, npiggin@...il.com,
        christophe.leroy@...roup.eu, paul.walmsley@...ive.com,
        palmer@...belt.com, aou@...s.berkeley.edu, hca@...ux.ibm.com,
        gor@...ux.ibm.com, agordeev@...ux.ibm.com,
        borntraeger@...ux.ibm.com, svens@...ux.ibm.com, hpa@...or.com,
        keescook@...omium.org, paulmck@...nel.org, peterz@...radead.org,
        frederic@...nel.org, akpm@...ux-foundation.org, ardb@...nel.org,
        samitolvanen@...gle.com, juerg.haefliger@...onical.com,
        arnd@...db.de, rmk+kernel@...linux.org.uk,
        linus.walleij@...aro.org, sebastian.reichel@...labora.com,
        rppt@...nel.org, kirill.shutemov@...ux.intel.com,
        anshuman.khandual@....com, ziy@...dia.com, masahiroy@...nel.org,
        ndesaulniers@...gle.com, mhiramat@...nel.org, ojeda@...nel.org,
        thunder.leizhen@...wei.com, xin3.li@...el.com, tj@...nel.org,
        gregkh@...uxfoundation.org, tsi@...oix.net, bhe@...hat.com,
        hbathini@...ux.ibm.com, sourabhjain@...ux.ibm.com,
        boris.ostrovsky@...cle.com, konrad.wilk@...cle.com
Subject: Re: [PATCH v1 00/21] refactor Kconfig to consolidate KEXEC and CRASH
 options

My apologies, but this patch series is 13 patches, not 21. The last patch is "PATCH v1 13/21 
sh/kexec: refactor for kernel/Kconfig.kexec"
I'll correct for v2.
eric

On 6/12/23 12:27, Eric DeVolder wrote:
> The Kconfig is refactored to consolidate KEXEC and CRASH options from
> various arch/<arch>/Kconfig files into new file kernel/Kconfig.kexec.
> 
> The Kconfig.kexec is now a submenu titled "Kexec and crash features"
> located under "General Setup".
> 
> The following options are impacted:
> 
>   - KEXEC
>   - KEXEC_FILE
>   - KEXEC_SIG
>   - KEXEC_SIG_FORCE
>   - KEXEC_BZIMAGE_VERIFY_SIG
>   - KEXEC_JUMP
>   - CRASH_DUMP
> 
> Over time, these options have been copied between Kconfig files and
> are very similar to one another, but with slight differences.
> 
> The following architectures are impacted by the refactor (because of
> use of one or more KEXEC/CRASH options):
> 
>   - arm
>   - arm64
>   - ia64
>   - loongarch
>   - m68k
>   - mips
>   - parisc
>   - powerpc
>   - riscv
>   - s390
>   - sh
>   - x86
> 
> More information:
> 
> In the patch series "crash: Kernel handling of CPU and memory hot
> un/plug"
> 
>   https://lore.kernel.org/lkml/20230503224145.7405-1-eric.devolder@oracle.com/
> 
> the new kernel feature introduces the config option CRASH_HOTPLUG.
> 
> In reviewing, Thomas Gleixner requested that the new config option
> not be placed in x86 Kconfig. Rather the option needs a generic/common
> home. To Thomas' point, the KEXEC and CRASH options have largely been
> duplicated in the various arch/<arch>/Kconfig files, with minor
> differences. This kind of proliferation is to be avoid/stopped.
> 
>   https://lore.kernel.org/lkml/875y91yv63.ffs@tglx/
> 
> To that end, I have refactored the arch Kconfigs so as to consolidate
> the various KEXEC and CRASH options. Generally speaking, this work has
> the following themes:
> 
> - KEXEC and CRASH options are moved into new file kernel/Kconfig.kexec
>    - These items from arch/Kconfig:
>        CRASH_CORE KEXEC_CORE KEXEC_ELF HAVE_IMA_KEXEC
>    - These items from arch/x86/Kconfig form the common options:
>        KEXEC KEXEC_FILE KEXEC_SIG KEXEC_SIG_FORCE
>        KEXEC_BZIMAGE_VERIFY_SIG KEXEC_JUMP CRASH_DUMP
>    - The crash hotplug series appends CRASH_HOTPLUG to Kconfig.kexec
>    NOTE: PHYSICAL_START could be argued to be included in this series.
> - The Kconfig.kexec is now a submenu titled "Kexec and crash features"
> - The Kconfig.kexec is now listed in "General Setup" submenu from
>    init/Kconfig
> - To control the main common options, new options ARCH_HAS_KEXEC,
>    ARCH_HAS_KEXEC_FILE and ARCH_HAS_CRASH_DUMP are introduced.
>    NOTE: I went with ARCH_HAS_ due to the existing ARCH_HAS_KEXEC_PURGATORY.
> - To account for the slight differences, new options ARCH_SUPPORTS_KEXEC,
>    ARCH_SUPPORTS_KEXEC_FILE and ARCH_SUPPORTS_CRASH_DUMP are used to
>    elicit the same side effects as the original arch/<arch>/Kconfig
>    files for KEXEC and CRASH options.
>    NOTE: I'm open to a better name than 'ARCH_SUPPORTS', perhaps
>    ARCH_CUSTOMIZE ?
> 
> An example, 'make menuconfig' illustrating the submenu:
> 
>    > General setup > Kexec and crash features
>    [*] Enable kexec system call
>    [*] Enable kexec file based system call
>    [*]   Verify kernel signature during kexec_file_load() syscall
>    [ ]     Require a valid signature in kexec_file_load() syscall
>    [ ]     Enable bzImage signature verification support
>    [*] kexec jump
>    [*] kernel crash dumps
>    [*]   Update the crash elfcorehdr on system configuration changes
> 
> The three main options are KEXEC, KEXEC_FILE and CRASH_DUMP. In the
> process of consolidating these options, I encountered slight differences
> in the coding of these options in several of the architectures. As a
> result, I settled on the following solution:
> 
> - Each of three main options has a 'depends on ARCH_HAS_<option>'
>    statement: ARCH_HAS_KEXEC, ARCH_HAS_KEXEC_FILE, ARCH_HAS_CRASH_DUMP.
> 
>    For example, the KEXEC_FILE option has a 'depends on
>    ARCH_HAS_KEXEC_FILE' statement.
> 
> - The boolean ARCH_HAS_<option> in effect allows the arch to determine
>    when the feature is allowed.  Archs which don't have the feature
>    simply do not provide the corresponding ARCH_HAS_<option>.
>    For each arch, where there previously were KEXEC and/or CRASH
>    options, these have been replaced with the corresponding boolean
>    ARCH_HAS_<option>, and an appropriate def_bool statement.
> 
>    For example, if the arch supports KEXEC_FILE, then the
>    ARCH_HAS_KEXEC_FILE simply has a 'def_bool y'. This permits the
>    KEXEC_FILE option to be available.
> 
>    If the arch has a 'depends on' statement in its original coding
>    of the option, then that expression becomes part of the def_bool
>    expression. For example, arm64 had:
> 
>    config KEXEC
>      depends on PM_SLEEP_SMP
> 
>    and in this solution, this converts to:
> 
>    config ARCH_HAS_KEXEC
>      def_bool PM_SLEEP_SMP
> 
> 
> - In order to account for the differences in the config coding for
>    the three common options, the ARCH_SUPPORTS_<option> is used.
>    This options has a 'depends on <option>' statement to couple it
>    to the main option, and from there can insert the differences
>    from the common option and the arch original coding of that option.
> 
>    For example, a few archs enable CRYPTO and CRYTPO_SHA256 for
>    KEXEC_FILE. These require a ARCH_SUPPORTS_KEXEC_FILE and
>    'select CRYPTO' and 'select CRYPTO_SHA256' statements.
> 
> Illustrating the option relationships:
> 
> For KEXEC:
>   ARCH_HAS_KEXEC <- KEXEC <- ARCH_SUPPORTS_KEXEC
> 
>   KEXEC                      # in Kconfig.kexec
>   ARCH_HAS_KEXEC             # in arch/<arch>/Kconfig, as needed
>   ARCH_SUPPORTS_KEXEC        # in arch/<arch>/Kconfig, as needed
> 
> 
> For KEXEC_FILE:
>   ARCH_HAS_KEXEC_FILE <- KEXEC_FILE <- ARCH_SUPPORTS_KEXEC_FILE
> 
>   KEXEC_FILE                 # in Kconfig.kexec
>   ARCH_HAS_KEXEC_FILE        # in arch/<arch>/Kconfig, as needed
>   ARCH_SUPPORTS_KEXEC_FILE   # in arch/<arch>/Kconfig, as needed
> 
> 
> For CRASH:
>   ARCH_HAS_CRASH_DUMP <- CRASH_DUMP <- ARCH_SUPPORTS_CRASH_DUMP
> 
>   CRASH_DUMP                 # in Kconfig.kexec
>   ARCH_HAS_CRASH_DUMP        # in arch/<arch>/Kconfig, as needed
>   ARCH_SUPPORTS_CRASH_DUMP   # in arch/<arch>/Kconfig, as needed
> 
> To summarize, the ARCH_HAS_<option> permits the <option> to be
> enabled, and the ARCH_SUPPORTS_<option> handles side effects (ie.
> select statements).
> 
> Examples:
> A few examples to show the new strategy in action:
> 
> ===== x86 (minus the help section) =====
> Original:
>   config KEXEC
>      bool "kexec system call"
>      select KEXEC_CORE
> 
>   config KEXEC_FILE
>      bool "kexec file based system call"
>      select KEXEC_CORE
>      select HAVE_IMA_KEXEC if IMA
>      depends on X86_64
>      depends on CRYPTO=y
>      depends on CRYPTO_SHA256=y
> 
>   config ARCH_HAS_KEXEC_PURGATORY
>      def_bool KEXEC_FILE
> 
>   config KEXEC_SIG
>      bool "Verify kernel signature during kexec_file_load() syscall"
>      depends on KEXEC_FILE
> 
>   config KEXEC_SIG_FORCE
>      bool "Require a valid signature in kexec_file_load() syscall"
>      depends on KEXEC_SIG
> 
>   config KEXEC_BZIMAGE_VERIFY_SIG
>      bool "Enable bzImage signature verification support"
>      depends on KEXEC_SIG
>      depends on SIGNED_PE_FILE_VERIFICATION
>      select SYSTEM_TRUSTED_KEYRING
> 
>   config CRASH_DUMP
>      bool "kernel crash dumps"
>      depends on X86_64 || (X86_32 && HIGHMEM)
> 
>   config KEXEC_JUMP
>      bool "kexec jump"
>      depends on KEXEC && HIBERNATION
>      help
> 
> becomes...
> New:
>   config ARCH_HAS_KEXEC
>      def_bool y
> 
>   config ARCH_HAS_KEXEC_FILE
>      def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> 
>   config ARCH_SUPPORTS_KEXEC_FILE
>      def_bool y
>      depends on KEXEC_FILE
>      select HAVE_IMA_KEXEC if IMA
> 
>   config ARCH_HAS_KEXEC_PURGATORY
>      def_bool KEXEC_FILE
> 
>   config ARCH_HAS_KEXEC_JUMP
>      def_bool y
> 
>   config ARCH_HAS_CRASH_DUMP
>      def_bool X86_64 || (X86_32 && HIGHMEM)
> 
> 
> ===== powerpc (minus the help section) =====
> Original:
>   config KEXEC
>      bool "kexec system call"
>      depends on PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>      select KEXEC_CORE
> 
>   config KEXEC_FILE
>      bool "kexec file based system call"
>      select KEXEC_CORE
>      select HAVE_IMA_KEXEC if IMA
>      select KEXEC_ELF
>      depends on PPC64
>      depends on CRYPTO=y
>      depends on CRYPTO_SHA256=y
> 
>   config ARCH_HAS_KEXEC_PURGATORY
>      def_bool KEXEC_FILE
> 
>   config CRASH_DUMP
>      bool "Build a dump capture kernel"
>      depends on PPC64 || PPC_BOOK3S_32 || PPC_85xx || (44x && !SMP)
>      select RELOCATABLE if PPC64 || 44x || PPC_85xx
> 
> becomes...
> New:
> config ARCH_HAS_KEXEC
>      def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
> 
> config ARCH_HAS_KEXEC_FILE
>      def_bool PPC64 && CRYPTO && CRYPTO_SHA256
> 
> config ARCH_HAS_KEXEC_PURGATORY
>      def_bool KEXEC_FILE
> 
> config ARCH_SUPPORTS_KEXEC_FILE
>      def_bool y
>      depends on KEXEC_FILE
>      select KEXEC_ELF
>      select HAVE_IMA_KEXEC if IMA
> 
> config ARCH_HAS_CRASH_DUMP
>      def_bool PPC64 || PPC_BOOK3S_32 || PPC_85xx || (44x && !SMP)
> 
> config ARCH_SUPPORTS_CRASH_DUMP
>      def_bool y
>      depends on CRASH_DUMP
>      select RELOCATABLE if PPC64 || 44x || PPC_85xx
> 
> 
> Testing Approach and Results
> 
> There are 388 config files in the arch/<arch>/configs directories.
> For each of these config files, a .config is generated both before and
> after this Kconfig series, and checked for equivalence. This approach
> allows for a rather rapid check of all architectures and a wide
> variety of configs wrt/ KEXEC and CRASH, and avoids requiring
> compiling for all architectures and running kernels and run-time
> testing.
> 
> As such, I developed the following script to compare the before and
> after of 'make olddefconfig'. The new symbols introduced by this
> series are filtered out, but otherwise the config files are PASS
> only if they were equivalent, and FAIL otherwise.
> 
> The script performs the test by doing the following:
> 
>   # Obtain the "golden" .config output for given config file
>   # Reset test sandbox
>   git checkout master
>   git branch -D test_Kconfig
>   git checkout -B test_Kconfig master
>   make distclean
>   # Write out updated config
>   cp -f <config file> .config
>   make ARCH=<arch> olddefconfig
>   # Track each item in .config, LHSB is "golden"
>   scoreboard .config
> 
>   # Obtain the "changed" .config output for given config file
>   # Reset test sandbox
>   make distclean
>   # Apply this Kconfig series
>   git am <this Kconfig series>
>   # Write out updated config
>   cp -f <config file> .config
>   make ARCH=<arch> olddefconfig
>   # Track each item in .config, RHSB is "changed"
>   scoreboard .config
> 
>   # Determine test result
>   # Filter-out new symbols introduced by this series
>   # Filter-out symbol=n which not in either scoreboard
>   # Compare LHSB "golden" and RHSB "changed" scoreboards and issue PASS/FAIL
> 
> The script was instrumental during the refactoring of Kconfig as it
> continually revealed problems. The end result being that the solution
> presented in this series passes all configs as checked by the script.
> 
> Regards,
> eric
> 
> 
> ---
> v1: 12jun2023
>   - Initial
>   - Based on 6.4.0-rc6
> 
> ---
> Eric DeVolder (21):
>    kexec: consolidate kexec and crash options into kernel/Kconfig.kexec
>    x86/kexec: refactor for kernel/Kconfig.kexec
>    arm/kexec: refactor for kernel/Kconfig.kexec
>    ia64/kexec: refactor for kernel/Kconfig.kexec
>    arm64/kexec: refactor for kernel/Kconfig.kexec
>    loongarch/kexec: refactor for kernel/Kconfig.kexec
>    m68k/kexec: refactor for kernel/Kconfig.kexec
>    mips/kexec: refactor for kernel/Kconfig.kexec
>    parisc/kexec: refactor for kernel/Kconfig.kexec
>    powerpc/kexec: refactor for kernel/Kconfig.kexec
>    riscv/kexec: refactor for kernel/Kconfig.kexec
>    s390/kexec: refactor for kernel/Kconfig.kexec
>    sh/kexec: refactor for kernel/Kconfig.kexec
>    crash: move a few code bits to setup support of crash hotplug
>    crash: add generic infrastructure for crash hotplug support
>    kexec: exclude elfcorehdr from the segment digest
>    crash: memory and CPU hotplug sysfs attributes
>    x86/crash: add x86 crash hotplug support
>    crash: hotplug support for kexec_load()
>    crash: change crash_prepare_elf64_headers() to for_each_possible_cpu()
>    x86/crash: optimize CPU changes
> 
>   .../admin-guide/mm/memory-hotplug.rst         |   8 +
>   Documentation/core-api/cpu_hotplug.rst        |  18 +
>   arch/Kconfig                                  |  13 -
>   arch/arm/Kconfig                              |  29 +-
>   arch/arm64/Kconfig                            |  61 +--
>   arch/ia64/Kconfig                             |  28 +-
>   arch/loongarch/Kconfig                        |  26 +-
>   arch/m68k/Kconfig                             |  19 +-
>   arch/mips/Kconfig                             |  32 +-
>   arch/parisc/Kconfig                           |  34 +-
>   arch/powerpc/Kconfig                          |  55 +--
>   arch/riscv/Kconfig                            |  48 +--
>   arch/s390/Kconfig                             |  65 +---
>   arch/sh/Kconfig                               |  46 +--
>   arch/x86/Kconfig                              |  90 +----
>   arch/x86/include/asm/kexec.h                  |  18 +
>   arch/x86/kernel/crash.c                       | 140 ++++++-
>   drivers/base/cpu.c                            |  14 +
>   drivers/base/memory.c                         |  13 +
>   include/linux/crash_core.h                    |   9 +
>   include/linux/kexec.h                         |  63 +++-
>   include/uapi/linux/kexec.h                    |   1 +
>   init/Kconfig                                  |   2 +
>   kernel/Kconfig.kexec                          | 134 +++++++
>   kernel/crash_core.c                           | 355 ++++++++++++++++++
>   kernel/kexec.c                                |   5 +
>   kernel/kexec_core.c                           |   6 +
>   kernel/kexec_file.c                           | 187 +--------
>   kernel/ksysfs.c                               |  15 +
>   29 files changed, 900 insertions(+), 634 deletions(-)
>   create mode 100644 kernel/Kconfig.kexec
> 

Powered by blists - more mailing lists