From: Baoquan He <b...@redhat.com>  Sent: Tuesday, January 23, 2024 9:13 PM
> 
> Now crash codes under kernel/ folder has been split out from kexec
> code, crash dumping can be separated from kexec reboot in config
> items on x86 with some adjustments.
> 
> Here, also change some ifdefs or IS_ENABLED() check to more appropriate
> ones, e,g
>  - #ifdef CONFIG_KEXEC_CORE -> #ifdef CONFIG_CRASH_DUMP
>  - (!IS_ENABLED(CONFIG_KEXEC_CORE)) - >
> (!IS_ENABLED(CONFIG_CRASH_RESERVE))
> 
> Signed-off-by: Baoquan He <b...@redhat.com>
> ---
>  arch/x86/kernel/Makefile           | 4 ++--
>  arch/x86/kernel/cpu/mshyperv.c     | 4 ++++
>  arch/x86/kernel/kexec-bzimage64.c  | 4 ++++
>  arch/x86/kernel/kvm.c              | 4 ++--
>  arch/x86/kernel/machine_kexec_64.c | 3 +++
>  arch/x86/kernel/reboot.c           | 2 +-
>  arch/x86/kernel/setup.c            | 2 +-
>  arch/x86/kernel/smp.c              | 2 +-
>  arch/x86/xen/enlighten_hvm.c       | 4 ++++
>  9 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 913d4022131e..3668b1edef2d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -100,9 +100,9 @@ obj-$(CONFIG_TRACING)             += trace.o
>  obj-$(CONFIG_RETHOOK)                += rethook.o
>  obj-$(CONFIG_VMCORE_INFO)    += vmcore_info_$(BITS).o
>  obj-$(CONFIG_KEXEC_CORE)     += machine_kexec_$(BITS).o
> -obj-$(CONFIG_KEXEC_CORE)     += relocate_kernel_$(BITS).o crash.o
> +obj-$(CONFIG_KEXEC_CORE)     += relocate_kernel_$(BITS).o
>  obj-$(CONFIG_KEXEC_FILE)     += kexec-bzimage64.o
> -obj-$(CONFIG_CRASH_DUMP)     += crash_dump_$(BITS).o
> +obj-$(CONFIG_CRASH_DUMP)     += crash_dump_$(BITS).o crash.o
>  obj-y                                += kprobes/
>  obj-$(CONFIG_MODULES)                += module.o
>  obj-$(CONFIG_X86_32)         += doublefault_32.o
> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> index 01fa06dd06b6..f8163a59026b 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -210,6 +210,7 @@ static void hv_machine_shutdown(void)
>               hyperv_cleanup();
>  }
> 
> +#ifdef CONFIG_CRASH_DUMP
>  static void hv_machine_crash_shutdown(struct pt_regs *regs)
>  {
>       if (hv_crash_handler)
> @@ -221,6 +222,7 @@ static void hv_machine_crash_shutdown(struct
> pt_regs *regs)
>       /* Disable the hypercall page when there is only 1 active CPU. */
>       hyperv_cleanup();
>  }
> +#endif
>  #endif /* CONFIG_KEXEC_CORE */

Note that the #ifdef CONFIG_CRASH_DUMP is nested inside
#ifdef CONFIG_KEXEC_CODE here, and in the other Hyper-V code
just below.   It's also nested in xen_hvm_guest_init() at the bottom
of this patch.  But the KVM case of setting crash_shutdown is
not nested -- you changed #ifdef CONFIG_KEXEC_CORE to #ifdef
CONFIG_CRASH_DUMP.

I think both approaches work because CONFIG_CRASH_DUMP implies
CONFIG_KEXEC_CORE, but I wonder if it would be better to *not* nest
in all cases.  I'd like to see the cases be consistent so in the future
someone doesn't wonder why there's a difference (unless there's
a reason for the difference that I missed).

>  #endif /* CONFIG_HYPERV */
> 
> @@ -497,7 +499,9 @@ static void __init ms_hyperv_init_platform(void)
> 
>  #if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
>       machine_ops.shutdown = hv_machine_shutdown;
> +#ifdef CONFIG_CRASH_DUMP
>       machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> +#endif
>  #endif
>       if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
>               /*
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-
> bzimage64.c
> index 2a422e00ed4b..b55737b83a84 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -263,11 +263,13 @@ setup_boot_parameters(struct kimage *image,
> struct boot_params *params,
>       memset(&params->hd0_info, 0, sizeof(params->hd0_info));
>       memset(&params->hd1_info, 0, sizeof(params->hd1_info));
> 
> +#ifdef CONFIG_CRASH_DUMP
>       if (image->type == KEXEC_TYPE_CRASH) {
>               ret = crash_setup_memmap_entries(image, params);
>               if (ret)
>                       return ret;
>       } else
> +#endif
>               setup_e820_entries(params);
> 
>       nr_e820_entries = params->e820_entries;
> @@ -433,12 +435,14 @@ static void *bzImage64_load(struct kimage *image, char 
> *kernel,
>               return ERR_PTR(-EINVAL);
>       }
> 
> +#ifdef CONFIG_CRASH_DUMP
>       /* Allocate and load backup region */
>       if (image->type == KEXEC_TYPE_CRASH) {
>               ret = crash_load_segments(image);
>               if (ret)
>                       return ERR_PTR(ret);
>       }
> +#endif
> 
>       /*
>        * Load purgatory. For 64bit entry point, purgatory  code can be
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index dfe9945b9bec..acfc2d3183bc 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -769,7 +769,7 @@ static struct notifier_block kvm_pv_reboot_nb = {
>   * won't be valid. In cases like kexec, in which you install a new kernel, 
> this
>   * means a random memory location will be kept being written.
>   */
> -#ifdef CONFIG_KEXEC_CORE
> +#ifdef CONFIG_CRASH_DUMP
>  static void kvm_crash_shutdown(struct pt_regs *regs)
>  {
>       kvm_guest_cpu_offline(true);
> @@ -852,7 +852,7 @@ static void __init kvm_guest_init(void)
>       kvm_guest_cpu_init();
>  #endif
> 
> -#ifdef CONFIG_KEXEC_CORE
> +#ifdef CONFIG_CRASH_DUMP
>       machine_ops.crash_shutdown = kvm_crash_shutdown;
>  #endif
> 
> diff --git a/arch/x86/kernel/machine_kexec_64.c
> b/arch/x86/kernel/machine_kexec_64.c
> index bc0a5348b4a6..b180d8e497c3 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -508,6 +508,8 @@ int arch_kimage_file_post_load_cleanup(struct
> kimage *image)
>  }
>  #endif /* CONFIG_KEXEC_FILE */
> 
> +#ifdef CONFIG_CRASH_DUMP
> +
>  static int
>  kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>  {
> @@ -552,6 +554,7 @@ void arch_kexec_unprotect_crashkres(void)
>  {
>       kexec_mark_crashkres(false);
>  }
> +#endif
> 
>  /*
>   * During a traditional boot under SME, SME will encrypt the kernel,
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 830425e6d38e..1287b0d5962f 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -796,7 +796,7 @@ struct machine_ops machine_ops __ro_after_init = {
>       .emergency_restart = native_machine_emergency_restart,
>       .restart = native_machine_restart,
>       .halt = native_machine_halt,
> -#ifdef CONFIG_KEXEC_CORE
> +#ifdef CONFIG_CRASH_DUMP
>       .crash_shutdown = native_machine_crash_shutdown,
>  #endif
>  };

Also in arch/x86/kernel/reboot.c, should the function
machine_crash_shutdown() be updated with
#ifdef CONFIG_CRASH_SHUTDOWN instead of #ifdef
CONFIG_KEXEC_CORE?

Michael

> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 84201071dfac..899d839a2954 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -471,7 +471,7 @@ static void __init arch_reserve_crashkernel(void)
>       bool high = false;
>       int ret;
> 
> -     if (!IS_ENABLED(CONFIG_KEXEC_CORE))
> +     if (!IS_ENABLED(CONFIG_CRASH_RESERVE))
>               return;
> 
>       ret = parse_crashkernel(cmdline, memblock_phys_mem_size(),
> diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
> index 2908e063d7d8..18266cc3d98c 100644
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -286,7 +286,7 @@ struct smp_ops smp_ops = {
>       .smp_cpus_done          = native_smp_cpus_done,
> 
>       .stop_other_cpus        = native_stop_other_cpus,
> -#if defined(CONFIG_KEXEC_CORE)
> +#if defined(CONFIG_CRASH_DUMP)
>       .crash_stop_other_cpus  = kdump_nmi_shootdown_cpus,
>  #endif
>       .smp_send_reschedule    = native_smp_send_reschedule,
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index 3f8c34707c50..09e3db7ff990 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -149,12 +149,14 @@ static void xen_hvm_shutdown(void)
>               xen_reboot(SHUTDOWN_soft_reset);
>  }
> 
> +#ifdef CONFIG_CRASH_DUMP
>  static void xen_hvm_crash_shutdown(struct pt_regs *regs)
>  {
>       native_machine_crash_shutdown(regs);
>       xen_reboot(SHUTDOWN_soft_reset);
>  }
>  #endif
> +#endif
> 
>  static int xen_cpu_up_prepare_hvm(unsigned int cpu)
>  {
> @@ -236,8 +238,10 @@ static void __init xen_hvm_guest_init(void)
> 
>  #ifdef CONFIG_KEXEC_CORE
>       machine_ops.shutdown = xen_hvm_shutdown;
> +#ifdef CONFIG_CRASH_DUMP
>       machine_ops.crash_shutdown = xen_hvm_crash_shutdown;
>  #endif
> +#endif
>  }
> 
>  static __init int xen_parse_nopv(char *arg)
> --
> 2.41.0
> 

Reply via email to