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 <sourabhj...@linux.ibm.com>
> Cc: Akhil Raj <lf32....@gmail.com>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: Aneesh Kumar K.V <aneesh.ku...@kernel.org>
> Cc: Baoquan He <b...@redhat.com>
> Cc: Borislav Petkov (AMD) <b...@alien8.de>
> Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
> Cc: Christophe Leroy <christophe.le...@csgroup.eu>
> Cc: Dave Hansen <dave.han...@linux.intel.com>
> Cc: Dave Young <dyo...@redhat.com>
> Cc: David Hildenbrand <da...@redhat.com>
> Cc: Eric DeVolder <eric.devol...@oracle.com>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Hari Bathini <hbath...@linux.ibm.com>
> Cc: Laurent Dufour <laurent.duf...@fr.ibm.com>
> Cc: Mahesh Salgaonkar <mah...@linux.ibm.com>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: Mimi Zohar <zo...@linux.ibm.com>
> Cc: Naveen N Rao <nav...@kernel.org>
> Cc: Oscar Salvador <osalva...@suse.de>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Valentin Schneider <vschn...@redhat.com>
> Cc: Vivek Goyal <vgo...@redhat.com>
> Cc: ke...@lists.infradead.org
> Cc: x...@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
>
