Re: [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible

[Baoquan He <bhe@redhat.com>]

On 12/11/23 at 02:00pm, Sourabh Jain wrote:
> Architectures' specific functions `arch_crash_hotplug_cpu_support()` and
> `arch_crash_hotplug_memory_support()` advertise the kernel's capability
> to update the kdump image on CPU and Memory hotplug events to userspace
> via the sysfs interface. These architecture-specific functions need to
> access various attributes of the `kexec_crash_image` object to determine
> whether the kernel can update the kdump image and advertise this
> information to userspace accordingly.
> 
> As the architecture-specific code is not exposed to the APIs required to
> acquire the lock for accessing the `kexec_crash_image` object, it calls
> a generic function, `crash_check_update_elfcorehdr()`, to determine
> whether the kernel can update the kdump image or not.
> 
> The lack of access to the `kexec_crash_image` object in
> architecture-specific code restricts architectures from performing
> additional architecture-specific checks required to determine if the
> kdump image is updatable or not. For instance, on PowerPC, the kernel
> can update the kdump image only if both the elfcorehdr and FDT are
> marked as updatable for the `kexec_load` system call.
> 
> So define two generic functions, `crash_hotplug_cpu_support()` and
> `crash_hotplug_memory_support()`, which are called when userspace
> attempts to read the crash CPU and Memory hotplug support via the sysfs
> interface. These functions take the necessary locks needed to access the
> `kexec_crash_image` object and then forward it to the
> architecture-specific handler to do the rest.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Cc: Akhil Raj <lf32.dev@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Borislav Petkov (AMD) <bp@alien8.de>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Eric DeVolder <eric.devolder@oracle.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Laurent Dufour <laurent.dufour@fr.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: kexec@lists.infradead.org
> Cc: x86@kernel.org
> ---
>  arch/x86/include/asm/kexec.h |  8 ++++----
>  arch/x86/kernel/crash.c      | 20 +++++++++++++++-----
>  include/linux/kexec.h        | 13 +++++++------
>  kernel/crash_core.c          | 23 +++++++++++++++++------
>  4 files changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 9bb6607e864e..5c88d27b086d 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -212,13 +212,13 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg);
>  #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> -int arch_crash_hotplug_cpu_support(void);
> -#define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
> +int arch_crash_hotplug_cpu_support(struct kimage *image);
> +#define arch_crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_crash_hotplug_memory_support(void);
> -#define crash_hotplug_memory_support arch_crash_hotplug_memory_support
> +int arch_crash_hotplug_memory_support(struct kimage *image);
> +#define arch_crash_hotplug_memory_support arch_crash_hotplug_memory_support
>  #endif
>  
>  unsigned int arch_crash_get_elfcorehdr_size(void);
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 0d7b2657beb6..ad5941665589 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -398,18 +398,28 @@ int crash_load_segments(struct kimage *image)
>  #undef pr_fmt
>  #define pr_fmt(fmt) "crash hp: " fmt
>  
> -/* These functions provide the value for the sysfs crash_hotplug nodes */
> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)
> +static int crash_hotplug_support(struct kimage *image)
> +{
> +	if (image->file_mode)
> +		return 1;
> +
> +	return image->update_elfcorehdr;
> +}
> +#endif
> +
>  #ifdef CONFIG_HOTPLUG_CPU
> -int arch_crash_hotplug_cpu_support(void)
> +/* These functions provide the value for the sysfs crash_hotplug nodes */
> +int arch_crash_hotplug_cpu_support(struct kimage *image)
>  {
> -	return crash_check_update_elfcorehdr();
> +	return crash_hotplug_support(image);
>  }
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_crash_hotplug_memory_support(void)
> +int arch_crash_hotplug_memory_support(struct kimage *image)
>  {
> -	return crash_check_update_elfcorehdr();
> +	return crash_hotplug_support(image);
>  }
>  #endif
>  
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index ee28c09a7fb0..0f6ea35879ee 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -486,16 +486,17 @@ static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) {
>  static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
>  #endif
>  
> -int crash_check_update_elfcorehdr(void);
> -
> -#ifndef crash_hotplug_cpu_support
> -static inline int crash_hotplug_cpu_support(void) { return 0; }
> +#ifndef arch_crash_hotplug_cpu_support
> +static inline int arch_crash_hotplug_cpu_support(struct kimage *image) { return 0; }
>  #endif
>  
> -#ifndef crash_hotplug_memory_support
> -static inline int crash_hotplug_memory_support(void) { return 0; }
> +#ifndef arch_crash_hotplug_memory_support
> +static inline int arch_crash_hotplug_memory_support(struct kimage *image) { return 0; }
>  #endif
>  
> +extern int crash_hotplug_cpu_support(void);
> +extern int crash_hotplug_memory_support(void);
> +
>  #ifndef crash_get_elfcorehdr_size
>  static inline unsigned int crash_get_elfcorehdr_size(void) { return 0; }
>  #endif
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index b9190265fe52..b621f03c1104 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -892,12 +892,14 @@ DEFINE_MUTEX(__crash_hotplug_lock);
>  #define crash_hotplug_lock() mutex_lock(&__crash_hotplug_lock)
>  #define crash_hotplug_unlock() mutex_unlock(&__crash_hotplug_lock)
>  
> +#define CPU_HOTPLUG	0
> +#define MEM_HOTPLUG	1
>  /*
>   * This routine utilized when the crash_hotplug sysfs node is read.
>   * It reflects the kernel's ability/permission to update the crash
>   * elfcorehdr directly.
>   */
> -int crash_check_update_elfcorehdr(void)
> +static int crash_hotplug_support(int hotplug)

Is it possible that we rename this function to different name? e.g
crash_hotplug_check_support() or other name. We may easily mixing it up
with the one of ARCH version. personal opinion.

>  {
>  	int rc = 0;
>  
> @@ -909,18 +911,27 @@ int crash_check_update_elfcorehdr(void)
>  		return 0;
>  	}
>  	if (kexec_crash_image) {
> -		if (kexec_crash_image->file_mode)
> -			rc = 1;
> -		else
> -			rc = kexec_crash_image->update_elfcorehdr;
> +		if (hotplug == CPU_HOTPLUG)
> +			rc = arch_crash_hotplug_cpu_support(kexec_crash_image);
> +		else if (hotplug == MEM_HOTPLUG)
> +			rc = arch_crash_hotplug_memory_support(kexec_crash_image);
>  	}
>  	/* Release lock now that update complete */
>  	kexec_unlock();
>  	crash_hotplug_unlock();
> -
>  	return rc;
>  }
>  
> +int crash_hotplug_cpu_support(void)
> +{
> +	return crash_hotplug_support(CPU_HOTPLUG);
> +}
> +
> +int crash_hotplug_memory_support(void)
> +{
> +	return crash_hotplug_support(MEM_HOTPLUG);
> +}
> +
>  /*
>   * To accurately reflect hot un/plug changes of cpu and memory resources
>   * (including onling and offlining of those resources), the elfcorehdr
> -- 
> 2.41.0
>