* [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug
@ 2023-12-11 8:30 Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 1/6] crash: forward memory_notify arg to arch crash hotplug handler Sourabh Jain
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
Commit 247262756121 ("crash: add generic infrastructure for crash
hotplug support") added a generic infrastructure that allows
architectures to selectively update the kdump image component during CPU
or memory add/remove events within the kernel itself.
This patch series adds crash hotplug handler for PowerPC and enable
support to update the kdump image on CPU/Memory add/remove events.
Among the 6 patches in this series, the first three patches make changes
to the generic crash hotplug handler to assist PowerPC in adding support
for this feature. The last three patches add support for this feature.
The following section outlines the problem addressed by this patch
series, along with the current solution, its shortcomings, and the
proposed resolution.
Problem:
========
Due to CPU/Memory hotplug or online/offline events the elfcorehdr
(which describes the CPUs and memory of the crashed kernel) and FDT
(Flattened Device Tree) of kdump image becomes outdated. Consequently,
attempting dump collection with an outdated elfcorehdr or FDT can lead
to failed or inaccurate dump collection.
Going forward CPU hotplug or online/offline events are referred as
CPU/Memory add/remove events.
Existing solution and its shortcoming:
======================================
The current solution to address the above issue involves monitoring the
CPU/memory add/remove events in userspace using udev rules and whenever
there are changes in CPU and memory resources, the entire kdump image
is loaded again. The kdump image includes kernel, initrd, elfcorehdr,
FDT, purgatory. Given that only elfcorehdr and FDT get outdated due to
CPU/Memory add/remove events, reloading the entire kdump image is
inefficient. More importantly, kdump remains inactive for a substantial
amount of time until the kdump reload completes.
Proposed solution:
==================
Instead of initiating a full kdump image reload from userspace on
CPU/Memory hotplug and online/offline events, the proposed solution aims
to update only the necessary kdump image component within the kernel
itself.
Git tree for testing:
=====================
Git tree rebased on top of v6.7-rc4:
https://github.com/sourabhjains/linux/tree/kdump-in-kernel-crash-update-v14
To realize this feature, the kdump udev rule must be updated. On RHEL,
add the following two lines at the top of the
"/usr/lib/udev/rules.d/98-kexec.rules" file.
SUBSYSTEM=="cpu", ATTRS{crash_hotplug}=="1", GOTO="kdump_reload_end"
SUBSYSTEM=="memory", ATTRS{crash_hotplug}=="1", GOTO="kdump_reload_end"
With the above change to the kdump udev rule, kdump reload is avoided
during CPU/Memory add/remove events if this feature is enabled in the
kernel.
Note: only kexec_file_load syscall will work. For kexec_load minor changes
are required in kexec tool.
Changelog:
----------
v14:
- Fix build warnings by including necessary header files
- Rebase to v6.7-rc5
v13:
- Fix a build warning, take ranges.c out of CONFIG_KEXEC_FILE
- Rebase to v6.7-rc4
v12:
- A patch to add new kexec flags to support this feature on kexec_load
system call
- Change in the way this feature is advertise to userspace for both
kexec_load syscall
- Rebase to v6.6-rc7
v11:
- Rebase to v6.4-rc6
- The patch that introduced CONFIG_CRASH_HOTPLUG for PowerPC has been
removed. The config is now part of common configuration:
https://lore.kernel.org/all/87ilbpflsk.fsf@mail.lhotse/
v10:
- Drop the patch that adds fdt_index attribute to struct kimage_arch
Find the fdt segment index when needed.
- Added more details into commits messages.
- Rebased onto 6.3.0-rc5
v9:
- Removed patch to prepare elfcorehdr crash notes for possible CPUs.
The patch is moved to generic patch series that introduces generic
infrastructure for in kernel crash update.
- Removed patch to pass the hotplug action type to the arch crash
hotplug handler function. The generic patch series has introduced
the hotplug action type in kimage struct.
- Add detail commit message for better understanding.
v8:
- Restrict fdt_index initialization to machine_kexec_post_load
it work for both kexec_load and kexec_file_load.[3/8] Laurent Dufour
- Updated the logic to find the number of offline core. [6/8]
- Changed the logic to find the elfcore program header to accommodate
future memory ranges due memory hotplug events. [8/8]
v7
- added a new config to configure this feature
- pass hotplug action type to arch specific handler
v6
- Added crash memory hotplug support
v5:
- Replace COFNIG_CRASH_HOTPLUG with CONFIG_HOTPLUG_CPU.
- Move fdt segment identification for kexec_load case to load path
instead of crash hotplug handler
- Keep new attribute defined under kimage_arch to track FDT segment
under CONFIG_HOTPLUG_CPU config.
v4:
- Update the logic to find the additional space needed for hotadd CPUs
post kexec load. Refer "[RFC v4 PATCH 4/5] powerpc/crash hp: add crash
hotplug support for kexec_file_load" patch to know more about the
change.
- Fix a couple of typo.
- Replace pr_err to pr_info_once to warn user about memory hotplug
support.
- In crash hotplug handle exit the for loop if FDT segment is found.
v3
- Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
- Rebase patche on top of
https://lore.kernel.org/lkml/20220303162725.49640-1-eric.devolder@oracle.com/
- Fixed warning reported by checpatch script
v2:
- Use generic hotplug handler introduced by
https://lore.kernel.org/lkml/20220209195706.51522-1-eric.devolder@oracle.com/
a significant change from v1.
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
Sourabh Jain (6):
crash: forward memory_notify arg to arch crash hotplug handler
crash: make CPU and Memory hotplug support reporting flexible
crash: add a new kexec flag for FDT update
powerpc/kexec: turn some static helper functions public
powerpc: add crash CPU hotplug support
powerpc: add crash memory hotplug support
arch/powerpc/Kconfig | 4 +
arch/powerpc/include/asm/kexec.h | 25 ++
arch/powerpc/include/asm/kexec_ranges.h | 1 +
arch/powerpc/kexec/Makefile | 4 +-
arch/powerpc/kexec/core_64.c | 369 ++++++++++++++++++++++++
arch/powerpc/kexec/elf_64.c | 12 +-
arch/powerpc/kexec/file_load_64.c | 211 +++-----------
arch/powerpc/kexec/ranges.c | 85 ++++++
arch/x86/include/asm/kexec.h | 10 +-
arch/x86/kernel/crash.c | 23 +-
include/linux/kexec.h | 21 +-
include/uapi/linux/kexec.h | 1 +
kernel/crash_core.c | 37 ++-
kernel/kexec.c | 2 +
14 files changed, 605 insertions(+), 200 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v14 1/6] crash: forward memory_notify arg to arch crash hotplug handler
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible Sourabh Jain
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
In the event of memory hotplug or online/offline events, the crash
memory hotplug notifier `crash_memhp_notifier()` receives a
`memory_notify` object but doesn't forward that object to the
generic and architecture-specific crash hotplug handler.
The `memory_notify` object contains the starting PFN (Page Frame Number)
and the number of pages in the hot-removed memory. This information is
necessary for architectures like PowerPC to update/recreate the kdump
image, specifically `elfcorehdr`.
So update the function signature of `crash_handle_hotplug_event()` and
`arch_crash_handle_hotplug_event()` to accept the `memory_notify` object
as an argument from crash memory hotplug notifier.
Since no such object is available in the case of CPU hotplug event, the
crash CPU hotplug notifier `crash_cpuhp_online()` passes NULL to the
crash hotplug handler.
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 | 2 +-
arch/x86/kernel/crash.c | 3 ++-
include/linux/kexec.h | 2 +-
kernel/crash_core.c | 14 +++++++-------
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index c9f6a6c5de3c..9bb6607e864e 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -208,7 +208,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image);
extern void kdump_nmi_shootdown_cpus(void);
#ifdef CONFIG_CRASH_HOTPLUG
-void arch_crash_handle_hotplug_event(struct kimage *image);
+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
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index c92d88680dbf..0d7b2657beb6 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -428,10 +428,11 @@ unsigned int arch_crash_get_elfcorehdr_size(void)
/**
* arch_crash_handle_hotplug_event() - Handle hotplug elfcorehdr changes
* @image: a pointer to kexec_crash_image
+ * @arg: struct memory_notify handler for memory hotplug case and NULL for CPU hotplug case.
*
* Prepare the new elfcorehdr and replace the existing elfcorehdr.
*/
-void arch_crash_handle_hotplug_event(struct kimage *image)
+void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
{
void *elfbuf = NULL, *old_elfcorehdr;
unsigned long nr_mem_ranges;
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8227455192b7..ee28c09a7fb0 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -483,7 +483,7 @@ static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) {
#endif
#ifndef arch_crash_handle_hotplug_event
-static inline void arch_crash_handle_hotplug_event(struct kimage *image) { }
+static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
#endif
int crash_check_update_elfcorehdr(void);
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index efe87d501c8c..b9190265fe52 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -935,7 +935,7 @@ int crash_check_update_elfcorehdr(void)
* list of segments it checks (since the elfcorehdr changes and thus
* would require an update to purgatory itself to update the digest).
*/
-static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
+static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu, void *arg)
{
struct kimage *image;
@@ -997,7 +997,7 @@ static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
image->hp_action = hp_action;
/* Now invoke arch-specific update handler */
- arch_crash_handle_hotplug_event(image);
+ arch_crash_handle_hotplug_event(image, arg);
/* No longer handling a hotplug event */
image->hp_action = KEXEC_CRASH_HP_NONE;
@@ -1013,17 +1013,17 @@ static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
crash_hotplug_unlock();
}
-static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, void *v)
+static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, void *arg)
{
switch (val) {
case MEM_ONLINE:
crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY,
- KEXEC_CRASH_HP_INVALID_CPU);
+ KEXEC_CRASH_HP_INVALID_CPU, arg);
break;
case MEM_OFFLINE:
crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY,
- KEXEC_CRASH_HP_INVALID_CPU);
+ KEXEC_CRASH_HP_INVALID_CPU, arg);
break;
}
return NOTIFY_OK;
@@ -1036,13 +1036,13 @@ static struct notifier_block crash_memhp_nb = {
static int crash_cpuhp_online(unsigned int cpu)
{
- crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu);
+ crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu, NULL);
return 0;
}
static int crash_cpuhp_offline(unsigned int cpu)
{
- crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu);
+ crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu, NULL);
return 0;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 1/6] crash: forward memory_notify arg to arch crash hotplug handler Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-14 14:13 ` Baoquan He
2023-12-11 8:30 ` [PATCH v14 3/6] crash: add a new kexec flag for FDT update Sourabh Jain
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
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)
{
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 1/6] crash: forward memory_notify arg to arch crash hotplug handler Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-15 2:28 ` Baoquan He
2023-12-11 8:30 ` [PATCH v14 4/6] powerpc/kexec: turn some static helper functions public Sourabh Jain
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
The commit a72bbec70da2 ("crash: hotplug support for kexec_load()")
introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses
this flag to indicate kernel that it is safe to modify the elfcorehdr
of kdump image loaded using kexec_load system call.
Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump
component named FDT (Flatten Device Tree). Architectures like PowerPC
need to update FDT kdump image component on CPU hotplug events. Kexec
tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT
segment is not part of SHA calculation hence it is safe to update it.
With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags,
crash hotplug support can be added to PowerPC for the kexec_load syscall
while maintaining the backward compatibility with older kexec tools that
do not have these newly introduced flags.
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
---
include/linux/kexec.h | 6 ++++--
include/uapi/linux/kexec.h | 1 +
kernel/kexec.c | 2 ++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0f6ea35879ee..bcedb7625b1f 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -319,6 +319,7 @@ struct kimage {
#ifdef CONFIG_CRASH_HOTPLUG
/* If set, allow changes to elfcorehdr of kexec_load'd image */
unsigned int update_elfcorehdr:1;
+ unsigned int update_fdt:1;
#endif
#ifdef ARCH_HAS_KIMAGE_ARCH
@@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
/* List of defined/legal kexec flags */
#ifndef CONFIG_KEXEC_JUMP
-#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
+#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
#else
-#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
+#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
+ KEXEC_UPDATE_FDT)
#endif
/* List of defined/legal kexec file flags */
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 01766dd839b0..3d5b3d757bed 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -13,6 +13,7 @@
#define KEXEC_ON_CRASH 0x00000001
#define KEXEC_PRESERVE_CONTEXT 0x00000002
#define KEXEC_UPDATE_ELFCOREHDR 0x00000004
+#define KEXEC_UPDATE_FDT 0x00000008
#define KEXEC_ARCH_MASK 0xffff0000
/*
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 8f35a5a42af8..97eb151cd931 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
#ifdef CONFIG_CRASH_HOTPLUG
if (flags & KEXEC_UPDATE_ELFCOREHDR)
image->update_elfcorehdr = 1;
+ if (flags & KEXEC_UPDATE_FDT)
+ image->update_fdt = 1;
#endif
ret = machine_kexec_prepare(image);
--
2.41.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v14 4/6] powerpc/kexec: turn some static helper functions public
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
` (2 preceding siblings ...)
2023-12-11 8:30 ` [PATCH v14 3/6] crash: add a new kexec flag for FDT update Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 5/6] powerpc: add crash CPU hotplug support Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 6/6] powerpc: add crash memory " Sourabh Jain
5 siblings, 0 replies; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
Move the functions update_cpus_node and get_crash_memory_ranges from
kexec/file_load_64.c to kexec/core_64.c to make these functions usable
by other kexec components.
get_crash_memory_ranges uses functions defined in ranges.c, so take
ranges.c out of CONFIG_KEXEC_FILE.
Later in the series, these functions are utilized for in-kernel updates
to kdump image during CPU/Memory hotplug or online/offline events for
both kexec_load and kexec_file_load syscalls.
There is no intended functional change.
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reviewed-by: Laurent Dufour <laurent.dufour@fr.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: 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/powerpc/include/asm/kexec.h | 6 ++
arch/powerpc/kexec/Makefile | 4 +-
arch/powerpc/kexec/core_64.c | 166 ++++++++++++++++++++++++++++++
arch/powerpc/kexec/file_load_64.c | 162 -----------------------------
4 files changed, 174 insertions(+), 164 deletions(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index e1b43aa12175..562e1bb689da 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -108,6 +108,12 @@ void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
#endif /* CONFIG_PPC_RTAS */
#endif /* CONFIG_CRASH_DUMP */
+#ifdef CONFIG_PPC64
+struct crash_mem;
+int update_cpus_node(void *fdt);
+int get_crash_memory_ranges(struct crash_mem **mem_ranges);
+#endif /* CONFIG_PPC64 */
+
#ifdef CONFIG_KEXEC_FILE
extern const struct kexec_file_ops kexec_elf64_ops;
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 0c2abe7f9908..f2ed5b85b912 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,11 +3,11 @@
# Makefile for the linux kernel.
#
-obj-y += core.o crash.o core_$(BITS).o
+obj-y += core.o crash.o ranges.o core_$(BITS).o
obj-$(CONFIG_PPC32) += relocate_32.o
-obj-$(CONFIG_KEXEC_FILE) += file_load.o ranges.o file_load_$(BITS).o elf_$(BITS).o
+obj-$(CONFIG_KEXEC_FILE) += file_load.o file_load_$(BITS).o elf_$(BITS).o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_core_$(BITS).o := n
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 0bee7ca9a77c..9966b51d9aa8 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -17,6 +17,8 @@
#include <linux/cpu.h>
#include <linux/hardirq.h>
#include <linux/of.h>
+#include <linux/libfdt.h>
+#include <linux/memblock.h>
#include <asm/page.h>
#include <asm/current.h>
@@ -30,6 +32,8 @@
#include <asm/hw_breakpoint.h>
#include <asm/svm.h>
#include <asm/ultravisor.h>
+#include <asm/kexec_ranges.h>
+#include <asm/crashdump-ppc64.h>
int machine_kexec_prepare(struct kimage *image)
{
@@ -377,6 +381,168 @@ void default_machine_kexec(struct kimage *image)
/* NOTREACHED */
}
+/**
+ * get_crash_memory_ranges - Get crash memory ranges. This list includes
+ * first/crashing kernel's memory regions that
+ * would be exported via an elfcore.
+ * @mem_ranges: Range list to add the memory ranges to.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+int get_crash_memory_ranges(struct crash_mem **mem_ranges)
+{
+ phys_addr_t base, end;
+ struct crash_mem *tmem;
+ u64 i;
+ int ret;
+
+ for_each_mem_range(i, &base, &end) {
+ u64 size = end - base;
+
+ /* Skip backup memory region, which needs a separate entry */
+ if (base == BACKUP_SRC_START) {
+ if (size > BACKUP_SRC_SIZE) {
+ base = BACKUP_SRC_END + 1;
+ size -= BACKUP_SRC_SIZE;
+ } else
+ continue;
+ }
+
+ ret = add_mem_range(mem_ranges, base, size);
+ if (ret)
+ goto out;
+
+ /* Try merging adjacent ranges before reallocation attempt */
+ if ((*mem_ranges)->nr_ranges == (*mem_ranges)->max_nr_ranges)
+ sort_memory_ranges(*mem_ranges, true);
+ }
+
+ /* Reallocate memory ranges if there is no space to split ranges */
+ tmem = *mem_ranges;
+ if (tmem && (tmem->nr_ranges == tmem->max_nr_ranges)) {
+ tmem = realloc_mem_ranges(mem_ranges);
+ if (!tmem)
+ goto out;
+ }
+
+ /* Exclude crashkernel region */
+ ret = crash_exclude_mem_range(tmem, crashk_res.start, crashk_res.end);
+ if (ret)
+ goto out;
+
+ /*
+ * FIXME: For now, stay in parity with kexec-tools but if RTAS/OPAL
+ * regions are exported to save their context at the time of
+ * crash, they should actually be backed up just like the
+ * first 64K bytes of memory.
+ */
+ ret = add_rtas_mem_range(mem_ranges);
+ if (ret)
+ goto out;
+
+ ret = add_opal_mem_range(mem_ranges);
+ if (ret)
+ goto out;
+
+ /* create a separate program header for the backup region */
+ ret = add_mem_range(mem_ranges, BACKUP_SRC_START, BACKUP_SRC_SIZE);
+ if (ret)
+ goto out;
+
+ sort_memory_ranges(*mem_ranges, false);
+out:
+ if (ret)
+ pr_err("Failed to setup crash memory ranges\n");
+ return ret;
+}
+
+/**
+ * add_node_props - Reads node properties from device node structure and add
+ * them to fdt.
+ * @fdt: Flattened device tree of the kernel
+ * @node_offset: offset of the node to add a property at
+ * @dn: device node pointer
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int add_node_props(void *fdt, int node_offset, const struct device_node *dn)
+{
+ int ret = 0;
+ struct property *pp;
+
+ if (!dn)
+ return -EINVAL;
+
+ for_each_property_of_node(dn, pp) {
+ ret = fdt_setprop(fdt, node_offset, pp->name, pp->value, pp->length);
+ if (ret < 0) {
+ pr_err("Unable to add %s property: %s\n", pp->name, fdt_strerror(ret));
+ return ret;
+ }
+ }
+ return ret;
+}
+
+/**
+ * update_cpus_node - Update cpus node of flattened device tree using of_root
+ * device node.
+ * @fdt: Flattened device tree of the kernel.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+int update_cpus_node(void *fdt)
+{
+ struct device_node *cpus_node, *dn;
+ int cpus_offset, cpus_subnode_offset, ret = 0;
+
+ cpus_offset = fdt_path_offset(fdt, "/cpus");
+ if (cpus_offset < 0 && cpus_offset != -FDT_ERR_NOTFOUND) {
+ pr_err("Malformed device tree: error reading /cpus node: %s\n",
+ fdt_strerror(cpus_offset));
+ return cpus_offset;
+ }
+
+ if (cpus_offset > 0) {
+ ret = fdt_del_node(fdt, cpus_offset);
+ if (ret < 0) {
+ pr_err("Error deleting /cpus node: %s\n", fdt_strerror(ret));
+ return -EINVAL;
+ }
+ }
+
+ /* Add cpus node to fdt */
+ cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
+ if (cpus_offset < 0) {
+ pr_err("Error creating /cpus node: %s\n", fdt_strerror(cpus_offset));
+ return -EINVAL;
+ }
+
+ /* Add cpus node properties */
+ cpus_node = of_find_node_by_path("/cpus");
+ ret = add_node_props(fdt, cpus_offset, cpus_node);
+ of_node_put(cpus_node);
+ if (ret < 0)
+ return ret;
+
+ /* Loop through all subnodes of cpus and add them to fdt */
+ for_each_node_by_type(dn, "cpu") {
+ cpus_subnode_offset = fdt_add_subnode(fdt, cpus_offset, dn->full_name);
+ if (cpus_subnode_offset < 0) {
+ pr_err("Unable to add %s subnode: %s\n", dn->full_name,
+ fdt_strerror(cpus_subnode_offset));
+ ret = cpus_subnode_offset;
+ goto out;
+ }
+
+ ret = add_node_props(fdt, cpus_subnode_offset, dn);
+ if (ret < 0)
+ goto out;
+ }
+out:
+ of_node_put(dn);
+ return ret;
+}
+
#ifdef CONFIG_PPC_64S_HASH_MMU
/* Values we need to export to the second kernel via the device tree. */
static __be64 htab_base;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 961a6dd67365..9c29f913112d 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -134,81 +134,6 @@ static int get_usable_memory_ranges(struct crash_mem **mem_ranges)
return ret;
}
-/**
- * get_crash_memory_ranges - Get crash memory ranges. This list includes
- * first/crashing kernel's memory regions that
- * would be exported via an elfcore.
- * @mem_ranges: Range list to add the memory ranges to.
- *
- * Returns 0 on success, negative errno on error.
- */
-static int get_crash_memory_ranges(struct crash_mem **mem_ranges)
-{
- phys_addr_t base, end;
- struct crash_mem *tmem;
- u64 i;
- int ret;
-
- for_each_mem_range(i, &base, &end) {
- u64 size = end - base;
-
- /* Skip backup memory region, which needs a separate entry */
- if (base == BACKUP_SRC_START) {
- if (size > BACKUP_SRC_SIZE) {
- base = BACKUP_SRC_END + 1;
- size -= BACKUP_SRC_SIZE;
- } else
- continue;
- }
-
- ret = add_mem_range(mem_ranges, base, size);
- if (ret)
- goto out;
-
- /* Try merging adjacent ranges before reallocation attempt */
- if ((*mem_ranges)->nr_ranges == (*mem_ranges)->max_nr_ranges)
- sort_memory_ranges(*mem_ranges, true);
- }
-
- /* Reallocate memory ranges if there is no space to split ranges */
- tmem = *mem_ranges;
- if (tmem && (tmem->nr_ranges == tmem->max_nr_ranges)) {
- tmem = realloc_mem_ranges(mem_ranges);
- if (!tmem)
- goto out;
- }
-
- /* Exclude crashkernel region */
- ret = crash_exclude_mem_range(tmem, crashk_res.start, crashk_res.end);
- if (ret)
- goto out;
-
- /*
- * FIXME: For now, stay in parity with kexec-tools but if RTAS/OPAL
- * regions are exported to save their context at the time of
- * crash, they should actually be backed up just like the
- * first 64K bytes of memory.
- */
- ret = add_rtas_mem_range(mem_ranges);
- if (ret)
- goto out;
-
- ret = add_opal_mem_range(mem_ranges);
- if (ret)
- goto out;
-
- /* create a separate program header for the backup region */
- ret = add_mem_range(mem_ranges, BACKUP_SRC_START, BACKUP_SRC_SIZE);
- if (ret)
- goto out;
-
- sort_memory_ranges(*mem_ranges, false);
-out:
- if (ret)
- pr_err("Failed to setup crash memory ranges\n");
- return ret;
-}
-
/**
* get_reserved_memory_ranges - Get reserve memory ranges. This list includes
* memory regions that should be added to the
@@ -1019,93 +944,6 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
return extra_size;
}
-/**
- * add_node_props - Reads node properties from device node structure and add
- * them to fdt.
- * @fdt: Flattened device tree of the kernel
- * @node_offset: offset of the node to add a property at
- * @dn: device node pointer
- *
- * Returns 0 on success, negative errno on error.
- */
-static int add_node_props(void *fdt, int node_offset, const struct device_node *dn)
-{
- int ret = 0;
- struct property *pp;
-
- if (!dn)
- return -EINVAL;
-
- for_each_property_of_node(dn, pp) {
- ret = fdt_setprop(fdt, node_offset, pp->name, pp->value, pp->length);
- if (ret < 0) {
- pr_err("Unable to add %s property: %s\n", pp->name, fdt_strerror(ret));
- return ret;
- }
- }
- return ret;
-}
-
-/**
- * update_cpus_node - Update cpus node of flattened device tree using of_root
- * device node.
- * @fdt: Flattened device tree of the kernel.
- *
- * Returns 0 on success, negative errno on error.
- */
-static int update_cpus_node(void *fdt)
-{
- struct device_node *cpus_node, *dn;
- int cpus_offset, cpus_subnode_offset, ret = 0;
-
- cpus_offset = fdt_path_offset(fdt, "/cpus");
- if (cpus_offset < 0 && cpus_offset != -FDT_ERR_NOTFOUND) {
- pr_err("Malformed device tree: error reading /cpus node: %s\n",
- fdt_strerror(cpus_offset));
- return cpus_offset;
- }
-
- if (cpus_offset > 0) {
- ret = fdt_del_node(fdt, cpus_offset);
- if (ret < 0) {
- pr_err("Error deleting /cpus node: %s\n", fdt_strerror(ret));
- return -EINVAL;
- }
- }
-
- /* Add cpus node to fdt */
- cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
- if (cpus_offset < 0) {
- pr_err("Error creating /cpus node: %s\n", fdt_strerror(cpus_offset));
- return -EINVAL;
- }
-
- /* Add cpus node properties */
- cpus_node = of_find_node_by_path("/cpus");
- ret = add_node_props(fdt, cpus_offset, cpus_node);
- of_node_put(cpus_node);
- if (ret < 0)
- return ret;
-
- /* Loop through all subnodes of cpus and add them to fdt */
- for_each_node_by_type(dn, "cpu") {
- cpus_subnode_offset = fdt_add_subnode(fdt, cpus_offset, dn->full_name);
- if (cpus_subnode_offset < 0) {
- pr_err("Unable to add %s subnode: %s\n", dn->full_name,
- fdt_strerror(cpus_subnode_offset));
- ret = cpus_subnode_offset;
- goto out;
- }
-
- ret = add_node_props(fdt, cpus_subnode_offset, dn);
- if (ret < 0)
- goto out;
- }
-out:
- of_node_put(dn);
- return ret;
-}
-
static int copy_property(void *fdt, int node_offset, const struct device_node *dn,
const char *propname)
{
--
2.41.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v14 5/6] powerpc: add crash CPU hotplug support
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
` (3 preceding siblings ...)
2023-12-11 8:30 ` [PATCH v14 4/6] powerpc/kexec: turn some static helper functions public Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-19 10:35 ` Hari Bathini
2023-12-11 8:30 ` [PATCH v14 6/6] powerpc: add crash memory " Sourabh Jain
5 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
Due to CPU/Memory hotplug or online/offline events the elfcorehdr
(which describes the CPUs and memory of the crashed kernel) and FDT
(Flattened Device Tree) of kdump image becomes outdated. Consequently,
attempting dump collection with an outdated elfcorehdr or FDT can lead
to failed or inaccurate dump collection.
Going forward CPU hotplug or online/offlice events are referred as
CPU/Memory add/remvoe events.
The current solution to address the above issue involves monitoring the
CPU/memory add/remove events in userspace using udev rules and whenever
there are changes in CPU and memory resources, the entire kdump image
is loaded again. The kdump image includes kernel, initrd, elfcorehdr,
FDT, purgatory. Given that only elfcorehdr and FDT get outdated due to
CPU/Memory add/remove events, reloading the entire kdump image is
inefficient. More importantly, kdump remains inactive for a substantial
amount of time until the kdump reload completes.
To address the aforementioned issue, commit 247262756121 ("crash: add
generic infrastructure for crash hotplug support") added a generic
infrastructure that allows architectures to selectively update the kdump
image component during CPU or memory add/remove events within the kernel
itself.
In the event of a CPU or memory add/remove event, the generic crash
hotplug event handler, `crash_handle_hotplug_event()`, is triggered. It
then acquires the necessary locks to update the kdump image and invokes
the architecture-specific crash hotplug handler,
`arch_crash_handle_hotplug_event()`, to update the required kdump image
components.
This patch adds crash hotplug handler for PowerPC and enable support to
update the kdump image on CPU add/remove events. Support for memory
add/remove events is added in a subsequent patch with the title
"powerpc: add crash memory hotplug support."
As mentioned earlier, only the elfcorehdr and FDT kdump image components
need to be updated in the event of CPU or memory add/remove events.
However, the PowerPC architecture crash hotplug handler only updates the
FDT to enable crash hotplug support for CPU add/remove events. Here's
why.
The Elfcorehdr on PowerPC is built with possible CPUs, and thus, it does
not need an update on CPU add/remove events. On the other hand, the FDT
needs to be updated on CPU add events to include the newly added CPU. If
the FDT is not updated and the kernel crashes on a newly added CPU, the
kdump kernel will fail to boot due to the unavailability of the crashing
CPU in the FDT. During the early boot, it is expected that the boot CPU
must be a part of the FDT; otherwise, the kernel will raise a BUG and
fail to boot. For more information, refer to commit 36ae37e3436b0
("powerpc: Make boot_cpuid common between 32 and 64-bit"). Since it is
okay to have an offline CPU in the kdump FDT, no action is taken in case
of CPU removal.
There are two system calls, `kexec_file_load` and `kexec_load`, used to
load the kdump image. Few changes have been made to ensure kernel can
safely update the kdump FDT for both system calls.
For kexec_file_load syscall the kdump image is prepared in kernel. So to
support an increasing number of CPUs, the FDT is constructed with extra
buffer space to ensure it can accommodate a possible number of CPU
nodes. Additionally, a call to fdt_pack (which trims the unused space
once the FDT is prepared) is avoided for kdump image loading if this
feature is enabled.
For the kexec_load syscall, the FDT is updated only if both the
KEXEC_UPDATE_FDT and KEXEC_UPDATE_ELFCOREHDR kexec flags are passed to
the kernel by the kexec tool. Passing these flags to the kernel
indicates that the FDT is built to accommodate possible CPUs, and the
FDT segment is not considered for SHA calculation, making it safe to
update the FDT.
Commit 88a6f8994421 ("crash: memory and CPU hotplug sysfs attributes")
added a sysfs interface to indicate userspace (kdump udev rule) that
kernel will update the kdump image on CPU hotplug events, so kdump
reload can be avoided. Implement arch specific function
`arch_crash_hotplug_cpu_support()` to correctly advertise kernel
capability to update kdump image.
This feature is advertised to userspace when the following conditions
are met:
1. Kdump image is loaded using kexec_file_load system call.
2. Kdump image is loaded using kexec_load system and both
KEXEC_UPATE_ELFCOREHDR and KEXEC_UPDATE_FDT kexec flags are
passed to kernel.
The changes related to this feature are kept under the CRASH_HOTPLUG
config, and it is enabled by default.
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/powerpc/Kconfig | 4 ++
arch/powerpc/include/asm/kexec.h | 11 ++++
arch/powerpc/kexec/core_64.c | 91 +++++++++++++++++++++++++++++++
arch/powerpc/kexec/elf_64.c | 12 +++-
arch/powerpc/kexec/file_load_64.c | 15 +++++
5 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..29949c0d301e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -681,6 +681,10 @@ config RELOCATABLE_TEST
config ARCH_SUPPORTS_CRASH_DUMP
def_bool PPC64 || PPC_BOOK3S_32 || PPC_85xx || (44x && !SMP)
+config ARCH_SUPPORTS_CRASH_HOTPLUG
+ def_bool y
+ depends on PPC64
+
config ARCH_SELECTS_CRASH_DUMP
def_bool y
depends on CRASH_DUMP
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 562e1bb689da..7823ab10d323 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -112,6 +112,17 @@ void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
struct crash_mem;
int update_cpus_node(void *fdt);
int get_crash_memory_ranges(struct crash_mem **mem_ranges);
+
+#ifdef CONFIG_CRASH_HOTPLUG
+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(struct kimage *image);
+#define arch_crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
+#endif
+
+#endif /*CONFIG_CRASH_HOTPLUG */
#endif /* CONFIG_PPC64 */
#ifdef CONFIG_KEXEC_FILE
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 9966b51d9aa8..9932793cd64b 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -543,6 +543,97 @@ int update_cpus_node(void *fdt)
return ret;
}
+#ifdef CONFIG_CRASH_HOTPLUG
+#undef pr_fmt
+#define pr_fmt(fmt) "crash hp: " fmt
+
+#ifdef CONFIG_HOTPLUG_CPU
+ /* Provides the value for the sysfs crash_hotplug nodes */
+int arch_crash_hotplug_cpu_support(struct kimage *image)
+{
+ if (image->file_mode)
+ return 1;
+
+ /*
+ * Crash CPU hotplug support is not available for kdump image loaded
+ * using the kexec_load system and the required attributes are not set.
+ */
+ return image->update_elfcorehdr && image->update_fdt;
+}
+#endif
+
+/**
+ * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
+ * necessary kexec segments based on the hotplug event.
+ * @image: a pointer to kexec_crash_image
+ * @arg: struct memory_notify handler for memory hotplug case and NULL for CPU hotplug case.
+ *
+ * Update the kdump image based on the type of hotplug event, represented by image->hp_action.
+ * CPU addition: Update the FDT segment to include the newly added CPU.
+ * CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
+ * as part of the FDT.
+ * Memory addition/removal: No action is taken as this is not yet supported.
+ */
+void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
+{
+ unsigned int hp_action = image->hp_action;
+
+ /*
+ * Check if it is safe to update the FDT for the kexec_load syscall.
+ * There's no need to check for the elfcorehdr; the generic crash
+ * hotplug handler does that before calling this function.
+ */
+ if (!(image->file_mode || image->update_fdt))
+ return;
+ /*
+ * Since the hot-unplugged CPU is already part of crash FDT,
+ * no action is needed for CPU remove case.
+ */
+ if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU) {
+ return;
+
+ } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
+
+ void *fdt, *ptr;
+ unsigned long mem;
+ int i, fdt_index = -1;
+
+ /* Find the FDT segment index in kexec segment array. */
+ for (i = 0; i < image->nr_segments; i++) {
+ mem = image->segment[i].mem;
+ ptr = __va(mem);
+
+ if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
+ fdt_index = i;
+ break;
+ }
+ }
+
+ if (fdt_index < 0) {
+ pr_err("Unable to locate FDT segment.\n");
+ return;
+ }
+
+ fdt = __va((void *)image->segment[fdt_index].mem);
+
+ /* Temporarily invalidate the crash image while it is replaced */
+ xchg(&kexec_crash_image, NULL);
+
+ /* update FDT to refelect changes in CPU resrouces */
+ if (update_cpus_node(fdt))
+ pr_err("Failed to update crash FDT");
+
+ /* The crash image is now valid once again */
+ xchg(&kexec_crash_image, image);
+
+ } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
+ hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
+ pr_info_once("Crash update is not supported for memory hotplug\n");
+ return;
+ }
+}
+#endif
+
#ifdef CONFIG_PPC_64S_HASH_MMU
/* Values we need to export to the second kernel via the device tree. */
static __be64 htab_base;
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..b072f589b142 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -30,6 +30,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len)
{
int ret;
+ bool do_pack_fdt = true;
unsigned long kernel_load_addr;
unsigned long initrd_load_addr = 0, fdt_load_addr;
void *fdt;
@@ -116,7 +117,16 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
if (ret)
goto out_free_fdt;
- fdt_pack(fdt);
+#ifdef CONFIG_CRASH_HOTPLG
+ /*
+ * Do not pack FDT, additional space is reserved to accommodate
+ * possible CPU nodes which are not yet present in the system yet.
+ */
+ if (image->type == KEXEC_TYPE_CRASH)
+ do_pack_fdt = false;
+#endif
+ if (do_pack_fdt)
+ fdt_pack(fdt);
kbuf.buffer = fdt;
kbuf.bufsz = kbuf.memsz = fdt_totalsize(fdt);
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 9c29f913112d..e2148a009701 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -22,6 +22,7 @@
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <asm/setup.h>
+#include <asm/cputhreads.h>
#include <asm/drmem.h>
#include <asm/firmware.h>
#include <asm/kexec_ranges.h>
@@ -909,6 +910,9 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
unsigned int cpu_nodes, extra_size = 0;
struct device_node *dn;
u64 usm_entries;
+#ifdef CONFIG_CRASH_HOTPLUG
+ unsigned int possible_cpu_nodes;
+#endif
// Budget some space for the password blob. There's already extra space
// for the key name
@@ -941,6 +945,17 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
if (cpu_nodes > boot_cpu_node_count)
extra_size += (cpu_nodes - boot_cpu_node_count) * cpu_node_size();
+#ifdef CONFIG_CRASH_HOTPLUG
+ /*
+ * Make sure enough space is reserved to accommodate possible CPU nodes
+ * in the crash FDT. This allows packing possible CPU nodes which are
+ * not yet present in the system without regenerating the entire FDT.
+ */
+ possible_cpu_nodes = num_possible_cpus() / threads_per_core;
+ if (image->type == KEXEC_TYPE_CRASH && possible_cpu_nodes > cpu_nodes)
+ extra_size += (possible_cpu_nodes - cpu_nodes) * cpu_node_size();
+#endif
+
return extra_size;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v14 6/6] powerpc: add crash memory hotplug support
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
` (4 preceding siblings ...)
2023-12-11 8:30 ` [PATCH v14 5/6] powerpc: add crash CPU hotplug support Sourabh Jain
@ 2023-12-11 8:30 ` Sourabh Jain
2023-12-15 1:23 ` Baoquan He
5 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-11 8:30 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Sourabh Jain, Akhil Raj, Andrew Morton
Extend the arch crash hotplug handler, as introduced by the patch title
("powerpc: add crash CPU hotplug support"), to also support memory
add/remove events.
Elfcorehdr describes the memory of the crash kernel to capture the
kernel; hence, it needs to be updated if memory resources change due to
memory add/remove events. Therefore, arch_crash_handle_hotplug_event()
is updated to recreate the elfcorehdr and replace it with the previous
one on memory add/remove events.
The memblock list is used to prepare the elfcorehdr. In the case of
memory hot removal, the memblock list is updated after the arch crash
hotplug handler is triggered, as depicted in Figure 1. Thus, the
hot-removed memory is explicitly removed from the crash memory ranges
to ensure that the memory ranges added to elfcorehdr do not include the
hot-removed memory.
Memory remove
|
v
Offline pages
|
v
Initiate memory notify call <----> crash hotplug handler
chain for MEM_OFFLINE event
|
v
Update memblock list
Figure 1
There are two system calls, `kexec_file_load` and `kexec_load`, used to
load the kdump image. A few changes have been made to ensure that the
kernel can safely update the elfcorehdr component of the kdump image for
both system calls.
For the kexec_file_load syscall, kdump image is prepared in the kernel.
To support an increasing number of memory regions, the elfcorehdr is
built with extra buffer space to ensure that it can accommodate
additional memory ranges in future.
For the kexec_load syscall, the elfcorehdr is updated only if both the
KEXEC_UPDATE_FDT and KEXEC_UPDATE_ELFCOREHDR kexec flags are passed to
the kernel by the kexec tool. Passing these flags to the kernel
indicates that the elfcorehdr is built to accommodate additional memory
ranges and the elfcorehdr segment is not considered for SHA calculation,
making it safe to update it.
Commit 88a6f8994421 ("crash: memory and CPU hotplug sysfs attributes")
added a sysfs interface to indicate userspace (kdump udev rule) that
kernel will update the kdump image on memory hotplug events, so kdump
reload can be avoided. Implement arch specific function
`arch_crash_hotplug_memory_support()` to correctly advertise kernel
capability to update kdump image.
This feature is advertised to usersapce when following conditions are
met:
1. Kdump image is loaded using kexec_file_load system call.
2. Kdump image is loaded using kexec_load system and both
KEXEC_UPATE_ELFCOREHDR and KEXEC_UPDATE_FDT kexec flags are passed
to kernel.
The changes related to this feature are kept under the CRASH_HOTPLUG
config, and it is enabled by default.
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/powerpc/include/asm/kexec.h | 8 ++
arch/powerpc/include/asm/kexec_ranges.h | 1 +
arch/powerpc/kexec/core_64.c | 126 ++++++++++++++++++++++--
arch/powerpc/kexec/file_load_64.c | 34 ++++++-
arch/powerpc/kexec/ranges.c | 85 ++++++++++++++++
5 files changed, 245 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 7823ab10d323..c8d6cfda523c 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -122,6 +122,14 @@ 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(struct kimage *image);
+#define arch_crash_hotplug_memory_support arch_crash_hotplug_memory_support
+#endif
+
+unsigned int arch_crash_get_elfcorehdr_size(void);
+#define crash_get_elfcorehdr_size arch_crash_get_elfcorehdr_size
+
#endif /*CONFIG_CRASH_HOTPLUG */
#endif /* CONFIG_PPC64 */
diff --git a/arch/powerpc/include/asm/kexec_ranges.h b/arch/powerpc/include/asm/kexec_ranges.h
index f83866a19e87..802abf580cf0 100644
--- a/arch/powerpc/include/asm/kexec_ranges.h
+++ b/arch/powerpc/include/asm/kexec_ranges.h
@@ -7,6 +7,7 @@
void sort_memory_ranges(struct crash_mem *mrngs, bool merge);
struct crash_mem *realloc_mem_ranges(struct crash_mem **mem_ranges);
int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
+int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
int add_tce_mem_ranges(struct crash_mem **mem_ranges);
int add_initrd_mem_range(struct crash_mem **mem_ranges);
#ifdef CONFIG_PPC_64S_HASH_MMU
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 9932793cd64b..5be30659172f 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -19,8 +19,11 @@
#include <linux/of.h>
#include <linux/libfdt.h>
#include <linux/memblock.h>
+#include <linux/memory.h>
#include <asm/page.h>
+#include <asm/drmem.h>
+#include <asm/mmzone.h>
#include <asm/current.h>
#include <asm/machdep.h>
#include <asm/cacheflush.h>
@@ -547,9 +550,7 @@ int update_cpus_node(void *fdt)
#undef pr_fmt
#define pr_fmt(fmt) "crash hp: " fmt
-#ifdef CONFIG_HOTPLUG_CPU
- /* Provides the value for the sysfs crash_hotplug nodes */
-int arch_crash_hotplug_cpu_support(struct kimage *image)
+static int crash_hotplug_support(struct kimage *image)
{
if (image->file_mode)
return 1;
@@ -560,8 +561,118 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
*/
return image->update_elfcorehdr && image->update_fdt;
}
+
+#ifdef CONFIG_HOTPLUG_CPU
+ /* Provides the value for the sysfs crash_hotplug nodes */
+int arch_crash_hotplug_cpu_support(struct kimage *image)
+{
+ return crash_hotplug_support(image);
+}
+#endif
+
+#ifdef CONFIG_MEMORY_HOTPLUG
+ /* Provides the value for the sysfs memory_hotplug nodes */
+int arch_crash_hotplug_memory_support(struct kimage *image)
+{
+ return crash_hotplug_support(image);
+}
#endif
+/*
+ * Advertise preferred elfcorehdr size to userspace via
+ * /sys/kernel/crash_elfcorehdr_size sysfs interface.
+ */
+unsigned int arch_crash_get_elfcorehdr_size(void)
+{
+ unsigned int sz;
+ unsigned long elf_phdr_cnt;
+
+ /* Program header for CPU notes and vmcoreinfo */
+ elf_phdr_cnt = 2;
+ if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
+ /* In the worst case, a Phdr is needed for every other LMB to be
+ * represented as an individual crash range.
+ */
+ elf_phdr_cnt += memory_hotplug_max() / (2 * drmem_lmb_size());
+
+ /* Do not cross the max limit */
+ if (elf_phdr_cnt > PN_XNUM)
+ elf_phdr_cnt = PN_XNUM;
+
+ sz = sizeof(struct elfhdr) + (elf_phdr_cnt * sizeof(Elf64_Phdr));
+ return sz;
+}
+
+/**
+ * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace it with old
+ * elfcorehdr in the kexec segment array.
+ * @image: the active struct kimage
+ * @mn: struct memory_notify data handler
+ */
+static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *mn)
+{
+ int ret;
+ struct crash_mem *cmem = NULL;
+ struct kexec_segment *ksegment;
+ void *ptr, *mem, *elfbuf = NULL;
+ unsigned long elfsz, memsz, base_addr, size;
+
+ ksegment = &image->segment[image->elfcorehdr_index];
+ mem = (void *) ksegment->mem;
+ memsz = ksegment->memsz;
+
+ ret = get_crash_memory_ranges(&cmem);
+ if (ret) {
+ pr_err("Failed to get crash mem range\n");
+ return;
+ }
+
+ /*
+ * The hot unplugged memory is part of crash memory ranges,
+ * remove it here.
+ */
+ if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
+ base_addr = PFN_PHYS(mn->start_pfn);
+ size = mn->nr_pages * PAGE_SIZE;
+ ret = remove_mem_range(&cmem, base_addr, size);
+ if (ret) {
+ pr_err("Failed to remove hot-unplugged from crash memory ranges.\n");
+ return;
+ }
+ }
+
+ ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
+ if (ret) {
+ pr_err("Failed to prepare elf header\n");
+ return;
+ }
+
+ /*
+ * It is unlikely that kernel hit this because elfcorehdr kexec
+ * segment (memsz) is built with addition space to accommodate growing
+ * number of crash memory ranges while loading the kdump kernel. It is
+ * Just to avoid any unforeseen case.
+ */
+ if (elfsz > memsz) {
+ pr_err("Updated crash elfcorehdr elfsz %lu > memsz %lu", elfsz, memsz);
+ goto out;
+ }
+
+ ptr = __va(mem);
+ if (ptr) {
+ /* Temporarily invalidate the crash image while it is replaced */
+ xchg(&kexec_crash_image, NULL);
+
+ /* Replace the old elfcorehdr with newly prepared elfcorehdr */
+ memcpy((void *)ptr, elfbuf, elfsz);
+
+ /* The crash image is now valid once again */
+ xchg(&kexec_crash_image, image);
+ }
+out:
+ vfree(elfbuf);
+}
+
/**
* arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
* necessary kexec segments based on the hotplug event.
@@ -572,7 +683,7 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
* CPU addition: Update the FDT segment to include the newly added CPU.
* CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
* as part of the FDT.
- * Memory addition/removal: No action is taken as this is not yet supported.
+ * Memory addition/removal: Recreate the elfcorehdr segment
*/
void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
{
@@ -593,7 +704,6 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
return;
} else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
-
void *fdt, *ptr;
unsigned long mem;
int i, fdt_index = -1;
@@ -628,8 +738,10 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
} else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
- pr_info_once("Crash update is not supported for memory hotplug\n");
- return;
+ struct memory_notify *mn;
+
+ mn = (struct memory_notify *)arg;
+ update_crash_elfcorehdr(image, mn);
}
}
#endif
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index e2148a009701..2457d7ec2075 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -21,6 +21,8 @@
#include <linux/memblock.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
+#include <linux/elf.h>
+
#include <asm/setup.h>
#include <asm/cputhreads.h>
#include <asm/drmem.h>
@@ -740,7 +742,35 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
kbuf->buffer = headers;
kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
- kbuf->bufsz = kbuf->memsz = headers_sz;
+ kbuf->bufsz = headers_sz;
+#if defined(CONFIG_CRASH_HOTPLUG) && defined(CONFIG_MEMORY_HOTPLUG)
+ /* Adjust the elfcorehdr segment size to accommodate
+ * future crash memory ranges.
+ */
+ int max_lmb;
+ unsigned long pnum;
+
+ /* In the worst case, a Phdr is needed for every other LMB to be
+ * represented as an individual crash range.
+ */
+ max_lmb = memory_hotplug_max() / (2 * drmem_lmb_size());
+
+ /* Do not cross the Phdr max limit of the elf header.
+ * Avoid counting Phdr for crash ranges (cmem->nr_ranges)
+ * which are already part of elfcorehdr.
+ */
+ if (max_lmb > PN_XNUM)
+ pnum = PN_XNUM - cmem->nr_ranges;
+ else
+ pnum = max_lmb - cmem->nr_ranges;
+
+ /* Additional buffer space for elfcorehdr to accommodate
+ * future memory ranges.
+ */
+ kbuf->memsz = headers_sz + pnum * sizeof(Elf64_Phdr);
+#else
+ kbuf->memsz = headers_sz;
+#endif
kbuf->top_down = false;
ret = kexec_add_buffer(kbuf);
@@ -750,7 +780,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
}
image->elf_load_addr = kbuf->mem;
- image->elf_headers_sz = headers_sz;
+ image->elf_headers_sz = kbuf->memsz;
image->elf_headers = headers;
out:
kfree(cmem);
diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
index fb3e12f15214..4fd0c5d5607b 100644
--- a/arch/powerpc/kexec/ranges.c
+++ b/arch/powerpc/kexec/ranges.c
@@ -234,6 +234,91 @@ int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
return __add_mem_range(mem_ranges, base, size);
}
+/**
+ * remove_mem_range - Removes the given memory range from the range list.
+ * @mem_ranges: Range list to remove the memory range to.
+ * @base: Base address of the range to remove.
+ * @size: Size of the memory range to remove.
+ *
+ * (Re)allocates memory, if needed.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
+{
+ u64 end;
+ int ret = 0;
+ unsigned int i;
+ u64 mstart, mend;
+ struct crash_mem *mem_rngs = *mem_ranges;
+
+ if (!size)
+ return 0;
+
+ /*
+ * Memory range are stored as start and end address, use
+ * the same format to do remove operation.
+ */
+ end = base + size - 1;
+
+ for (i = 0; i < mem_rngs->nr_ranges; i++) {
+ mstart = mem_rngs->ranges[i].start;
+ mend = mem_rngs->ranges[i].end;
+
+ /*
+ * Memory range to remove is not part of this range entry
+ * in the memory range list
+ */
+ if (!(base >= mstart && end <= mend))
+ continue;
+
+ /*
+ * Memory range to remove is equivalent to this entry in the
+ * memory range list. Remove the range entry from the list.
+ */
+ if (base == mstart && end == mend) {
+ for (; i < mem_rngs->nr_ranges - 1; i++) {
+ mem_rngs->ranges[i].start = mem_rngs->ranges[i+1].start;
+ mem_rngs->ranges[i].end = mem_rngs->ranges[i+1].end;
+ }
+ mem_rngs->nr_ranges--;
+ goto out;
+ }
+ /*
+ * Start address of the memory range to remove and the
+ * current memory range entry in the list is same. Just
+ * move the start address of the current memory range
+ * entry in the list to end + 1.
+ */
+ else if (base == mstart) {
+ mem_rngs->ranges[i].start = end + 1;
+ goto out;
+ }
+ /*
+ * End address of the memory range to remove and the
+ * current memory range entry in the list is same.
+ * Just move the end address of the current memory
+ * range entry in the list to base - 1.
+ */
+ else if (end == mend) {
+ mem_rngs->ranges[i].end = base - 1;
+ goto out;
+ }
+ /*
+ * Memory range to remove is not at the edge of current
+ * memory range entry. Split the current memory entry into
+ * two half.
+ */
+ else {
+ mem_rngs->ranges[i].end = base - 1;
+ size = mem_rngs->ranges[i].end - end;
+ ret = add_mem_range(mem_ranges, end + 1, size);
+ }
+ }
+out:
+ return ret;
+}
+
/**
* add_tce_mem_ranges - Adds tce-table range to the given memory ranges list.
* @mem_ranges: Range list to add the memory range(s) to.
--
2.41.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible
2023-12-11 8:30 ` [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible Sourabh Jain
@ 2023-12-14 14:13 ` Baoquan He
2023-12-15 5:46 ` Sourabh Jain
0 siblings, 1 reply; 21+ messages in thread
From: Baoquan He @ 2023-12-14 14:13 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 6/6] powerpc: add crash memory hotplug support
2023-12-11 8:30 ` [PATCH v14 6/6] powerpc: add crash memory " Sourabh Jain
@ 2023-12-15 1:23 ` Baoquan He
2023-12-15 5:59 ` Sourabh Jain
0 siblings, 1 reply; 21+ messages in thread
From: Baoquan He @ 2023-12-15 1:23 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/11/23 at 02:00pm, Sourabh Jain wrote:
......
> diff --git a/arch/powerpc/include/asm/kexec_ranges.h b/arch/powerpc/include/asm/kexec_ranges.h
> index f83866a19e87..802abf580cf0 100644
> --- a/arch/powerpc/include/asm/kexec_ranges.h
> +++ b/arch/powerpc/include/asm/kexec_ranges.h
> @@ -7,6 +7,7 @@
> void sort_memory_ranges(struct crash_mem *mrngs, bool merge);
> struct crash_mem *realloc_mem_ranges(struct crash_mem **mem_ranges);
> int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
> +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
> int add_tce_mem_ranges(struct crash_mem **mem_ranges);
> int add_initrd_mem_range(struct crash_mem **mem_ranges);
> #ifdef CONFIG_PPC_64S_HASH_MMU
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 9932793cd64b..5be30659172f 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -19,8 +19,11 @@
> #include <linux/of.h>
> #include <linux/libfdt.h>
> #include <linux/memblock.h>
> +#include <linux/memory.h>
>
> #include <asm/page.h>
> +#include <asm/drmem.h>
> +#include <asm/mmzone.h>
> #include <asm/current.h>
> #include <asm/machdep.h>
> #include <asm/cacheflush.h>
> @@ -547,9 +550,7 @@ int update_cpus_node(void *fdt)
> #undef pr_fmt
> #define pr_fmt(fmt) "crash hp: " fmt
>
> -#ifdef CONFIG_HOTPLUG_CPU
> - /* Provides the value for the sysfs crash_hotplug nodes */
> -int arch_crash_hotplug_cpu_support(struct kimage *image)
> +static int crash_hotplug_support(struct kimage *image)
> {
> if (image->file_mode)
> return 1;
> @@ -560,8 +561,118 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
> */
> return image->update_elfcorehdr && image->update_fdt;
> }
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> + /* Provides the value for the sysfs crash_hotplug nodes */
> +int arch_crash_hotplug_cpu_support(struct kimage *image)
> +{
> + return crash_hotplug_support(image);
> +}
> +#endif
> +
> +#ifdef CONFIG_MEMORY_HOTPLUG
> + /* Provides the value for the sysfs memory_hotplug nodes */
> +int arch_crash_hotplug_memory_support(struct kimage *image)
> +{
> + return crash_hotplug_support(image);
> +}
> #endif
>
> +/*
> + * Advertise preferred elfcorehdr size to userspace via
> + * /sys/kernel/crash_elfcorehdr_size sysfs interface.
> + */
> +unsigned int arch_crash_get_elfcorehdr_size(void)
> +{
> + unsigned int sz;
> + unsigned long elf_phdr_cnt;
> +
> + /* Program header for CPU notes and vmcoreinfo */
> + elf_phdr_cnt = 2;
> + if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
> + /* In the worst case, a Phdr is needed for every other LMB to be
> + * represented as an individual crash range.
> + */
> + elf_phdr_cnt += memory_hotplug_max() / (2 * drmem_lmb_size());
> +
> + /* Do not cross the max limit */
> + if (elf_phdr_cnt > PN_XNUM)
> + elf_phdr_cnt = PN_XNUM;
> +
> + sz = sizeof(struct elfhdr) + (elf_phdr_cnt * sizeof(Elf64_Phdr));
> + return sz;
> +}
> +
> +/**
> + * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace it with old
> + * elfcorehdr in the kexec segment array.
> + * @image: the active struct kimage
> + * @mn: struct memory_notify data handler
> + */
> +static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *mn)
> +{
> + int ret;
> + struct crash_mem *cmem = NULL;
> + struct kexec_segment *ksegment;
> + void *ptr, *mem, *elfbuf = NULL;
> + unsigned long elfsz, memsz, base_addr, size;
> +
> + ksegment = &image->segment[image->elfcorehdr_index];
> + mem = (void *) ksegment->mem;
> + memsz = ksegment->memsz;
> +
> + ret = get_crash_memory_ranges(&cmem);
> + if (ret) {
> + pr_err("Failed to get crash mem range\n");
> + return;
> + }
> +
> + /*
> + * The hot unplugged memory is part of crash memory ranges,
> + * remove it here.
> + */
> + if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
> + base_addr = PFN_PHYS(mn->start_pfn);
> + size = mn->nr_pages * PAGE_SIZE;
> + ret = remove_mem_range(&cmem, base_addr, size);
Althouth this is ppc specific, I don't understand. Why don't you
recreate the elfcorehdr, but take removing the removed region. Comparing the
remove_mem_range() implementation with recreating, I don't see too much
benefit from that, and it makes your code more complicated. Just
curious, surely ppc people can decide what should be taken.
> + if (ret) {
> + pr_err("Failed to remove hot-unplugged from crash memory ranges.\n");
> + return;
> + }
> + }
> +
> + ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
> + if (ret) {
> + pr_err("Failed to prepare elf header\n");
> + return;
> + }
> +
> + /*
> + * It is unlikely that kernel hit this because elfcorehdr kexec
> + * segment (memsz) is built with addition space to accommodate growing
> + * number of crash memory ranges while loading the kdump kernel. It is
> + * Just to avoid any unforeseen case.
> + */
> + if (elfsz > memsz) {
> + pr_err("Updated crash elfcorehdr elfsz %lu > memsz %lu", elfsz, memsz);
> + goto out;
> + }
> +
> + ptr = __va(mem);
> + if (ptr) {
> + /* Temporarily invalidate the crash image while it is replaced */
> + xchg(&kexec_crash_image, NULL);
> +
> + /* Replace the old elfcorehdr with newly prepared elfcorehdr */
> + memcpy((void *)ptr, elfbuf, elfsz);
> +
> + /* The crash image is now valid once again */
> + xchg(&kexec_crash_image, image);
> + }
> +out:
> + vfree(elfbuf);
> +}
> +
> /**
> * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
> * necessary kexec segments based on the hotplug event.
> @@ -572,7 +683,7 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
> * CPU addition: Update the FDT segment to include the newly added CPU.
> * CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
> * as part of the FDT.
> - * Memory addition/removal: No action is taken as this is not yet supported.
> + * Memory addition/removal: Recreate the elfcorehdr segment
> */
> void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> {
> @@ -593,7 +704,6 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> return;
>
> } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
> -
> void *fdt, *ptr;
> unsigned long mem;
> int i, fdt_index = -1;
> @@ -628,8 +738,10 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>
> } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
> hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> - pr_info_once("Crash update is not supported for memory hotplug\n");
> - return;
> + struct memory_notify *mn;
> +
> + mn = (struct memory_notify *)arg;
> + update_crash_elfcorehdr(image, mn);
> }
> }
> #endif
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index e2148a009701..2457d7ec2075 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -21,6 +21,8 @@
> #include <linux/memblock.h>
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
> +#include <linux/elf.h>
> +
> #include <asm/setup.h>
> #include <asm/cputhreads.h>
> #include <asm/drmem.h>
> @@ -740,7 +742,35 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>
> kbuf->buffer = headers;
> kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
> - kbuf->bufsz = kbuf->memsz = headers_sz;
> + kbuf->bufsz = headers_sz;
> +#if defined(CONFIG_CRASH_HOTPLUG) && defined(CONFIG_MEMORY_HOTPLUG)
> + /* Adjust the elfcorehdr segment size to accommodate
> + * future crash memory ranges.
> + */
> + int max_lmb;
> + unsigned long pnum;
> +
> + /* In the worst case, a Phdr is needed for every other LMB to be
> + * represented as an individual crash range.
> + */
> + max_lmb = memory_hotplug_max() / (2 * drmem_lmb_size());
> +
> + /* Do not cross the Phdr max limit of the elf header.
> + * Avoid counting Phdr for crash ranges (cmem->nr_ranges)
> + * which are already part of elfcorehdr.
> + */
> + if (max_lmb > PN_XNUM)
> + pnum = PN_XNUM - cmem->nr_ranges;
> + else
> + pnum = max_lmb - cmem->nr_ranges;
> +
> + /* Additional buffer space for elfcorehdr to accommodate
> + * future memory ranges.
> + */
> + kbuf->memsz = headers_sz + pnum * sizeof(Elf64_Phdr);
> +#else
> + kbuf->memsz = headers_sz;
> +#endif
> kbuf->top_down = false;
>
> ret = kexec_add_buffer(kbuf);
> @@ -750,7 +780,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
> }
>
> image->elf_load_addr = kbuf->mem;
> - image->elf_headers_sz = headers_sz;
> + image->elf_headers_sz = kbuf->memsz;
> image->elf_headers = headers;
> out:
> kfree(cmem);
> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
> index fb3e12f15214..4fd0c5d5607b 100644
> --- a/arch/powerpc/kexec/ranges.c
> +++ b/arch/powerpc/kexec/ranges.c
> @@ -234,6 +234,91 @@ int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
> return __add_mem_range(mem_ranges, base, size);
> }
>
> +/**
> + * remove_mem_range - Removes the given memory range from the range list.
> + * @mem_ranges: Range list to remove the memory range to.
> + * @base: Base address of the range to remove.
> + * @size: Size of the memory range to remove.
> + *
> + * (Re)allocates memory, if needed.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
> +{
> + u64 end;
> + int ret = 0;
> + unsigned int i;
> + u64 mstart, mend;
> + struct crash_mem *mem_rngs = *mem_ranges;
> +
> + if (!size)
> + return 0;
> +
> + /*
> + * Memory range are stored as start and end address, use
> + * the same format to do remove operation.
> + */
> + end = base + size - 1;
> +
> + for (i = 0; i < mem_rngs->nr_ranges; i++) {
> + mstart = mem_rngs->ranges[i].start;
> + mend = mem_rngs->ranges[i].end;
> +
> + /*
> + * Memory range to remove is not part of this range entry
> + * in the memory range list
> + */
> + if (!(base >= mstart && end <= mend))
> + continue;
> +
> + /*
> + * Memory range to remove is equivalent to this entry in the
> + * memory range list. Remove the range entry from the list.
> + */
> + if (base == mstart && end == mend) {
> + for (; i < mem_rngs->nr_ranges - 1; i++) {
> + mem_rngs->ranges[i].start = mem_rngs->ranges[i+1].start;
> + mem_rngs->ranges[i].end = mem_rngs->ranges[i+1].end;
> + }
> + mem_rngs->nr_ranges--;
> + goto out;
> + }
> + /*
> + * Start address of the memory range to remove and the
> + * current memory range entry in the list is same. Just
> + * move the start address of the current memory range
> + * entry in the list to end + 1.
> + */
> + else if (base == mstart) {
> + mem_rngs->ranges[i].start = end + 1;
> + goto out;
> + }
> + /*
> + * End address of the memory range to remove and the
> + * current memory range entry in the list is same.
> + * Just move the end address of the current memory
> + * range entry in the list to base - 1.
> + */
> + else if (end == mend) {
> + mem_rngs->ranges[i].end = base - 1;
> + goto out;
> + }
> + /*
> + * Memory range to remove is not at the edge of current
> + * memory range entry. Split the current memory entry into
> + * two half.
> + */
> + else {
> + mem_rngs->ranges[i].end = base - 1;
> + size = mem_rngs->ranges[i].end - end;
> + ret = add_mem_range(mem_ranges, end + 1, size);
> + }
> + }
> +out:
> + return ret;
> +}
> +
> /**
> * add_tce_mem_ranges - Adds tce-table range to the given memory ranges list.
> * @mem_ranges: Range list to add the memory range(s) to.
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-11 8:30 ` [PATCH v14 3/6] crash: add a new kexec flag for FDT update Sourabh Jain
@ 2023-12-15 2:28 ` Baoquan He
2023-12-15 6:47 ` Sourabh Jain
0 siblings, 1 reply; 21+ messages in thread
From: Baoquan He @ 2023-12-15 2:28 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/11/23 at 02:00pm, Sourabh Jain wrote:
> The commit a72bbec70da2 ("crash: hotplug support for kexec_load()")
> introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses
> this flag to indicate kernel that it is safe to modify the elfcorehdr
> of kdump image loaded using kexec_load system call.
>
> Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump
> component named FDT (Flatten Device Tree). Architectures like PowerPC
> need to update FDT kdump image component on CPU hotplug events. Kexec
> tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT
> segment is not part of SHA calculation hence it is safe to update it.
>
> With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags,
> crash hotplug support can be added to PowerPC for the kexec_load syscall
> while maintaining the backward compatibility with older kexec tools that
> do not have these newly introduced flags.
>
> 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
> ---
> include/linux/kexec.h | 6 ++++--
> include/uapi/linux/kexec.h | 1 +
> kernel/kexec.c | 2 ++
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 0f6ea35879ee..bcedb7625b1f 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -319,6 +319,7 @@ struct kimage {
> #ifdef CONFIG_CRASH_HOTPLUG
> /* If set, allow changes to elfcorehdr of kexec_load'd image */
> unsigned int update_elfcorehdr:1;
> + unsigned int update_fdt:1;
Can we unify this to one flag, e.g hotplug_update?
With this, on x86_64, we will skip the sha calculation for elfcorehdr.
On ppc, we will skip the sha calculation for elfcorehdr and fdt.
> #endif
>
> #ifdef ARCH_HAS_KIMAGE_ARCH
> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
>
> /* List of defined/legal kexec flags */
> #ifndef CONFIG_KEXEC_JUMP
> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
> #else
> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
> + KEXEC_UPDATE_FDT)
> #endif
>
> /* List of defined/legal kexec file flags */
> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> index 01766dd839b0..3d5b3d757bed 100644
> --- a/include/uapi/linux/kexec.h
> +++ b/include/uapi/linux/kexec.h
> @@ -13,6 +13,7 @@
> #define KEXEC_ON_CRASH 0x00000001
> #define KEXEC_PRESERVE_CONTEXT 0x00000002
> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> +#define KEXEC_UPDATE_FDT 0x00000008
> #define KEXEC_ARCH_MASK 0xffff0000
>
> /*
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 8f35a5a42af8..97eb151cd931 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> #ifdef CONFIG_CRASH_HOTPLUG
> if (flags & KEXEC_UPDATE_ELFCOREHDR)
> image->update_elfcorehdr = 1;
> + if (flags & KEXEC_UPDATE_FDT)
> + image->update_fdt = 1;
> #endif
>
> ret = machine_kexec_prepare(image);
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible
2023-12-14 14:13 ` Baoquan He
@ 2023-12-15 5:46 ` Sourabh Jain
0 siblings, 0 replies; 21+ messages in thread
From: Sourabh Jain @ 2023-12-15 5:46 UTC (permalink / raw)
To: Baoquan He
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
Hello Baoquan,
On 14/12/23 19:43, Baoquan He wrote:
> 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
Sure I will rename the common function to crash_hotplug_check_support()
and arch-specific function to arch_crash_hotplug_check_support.
Thanks,
Sourabh
>
>> {
>> 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
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 6/6] powerpc: add crash memory hotplug support
2023-12-15 1:23 ` Baoquan He
@ 2023-12-15 5:59 ` Sourabh Jain
2023-12-16 3:11 ` Baoquan He
0 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-15 5:59 UTC (permalink / raw)
To: Baoquan He
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 15/12/23 06:53, Baoquan He wrote:
> On 12/11/23 at 02:00pm, Sourabh Jain wrote:
> ......
>> diff --git a/arch/powerpc/include/asm/kexec_ranges.h b/arch/powerpc/include/asm/kexec_ranges.h
>> index f83866a19e87..802abf580cf0 100644
>> --- a/arch/powerpc/include/asm/kexec_ranges.h
>> +++ b/arch/powerpc/include/asm/kexec_ranges.h
>> @@ -7,6 +7,7 @@
>> void sort_memory_ranges(struct crash_mem *mrngs, bool merge);
>> struct crash_mem *realloc_mem_ranges(struct crash_mem **mem_ranges);
>> int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
>> +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
>> int add_tce_mem_ranges(struct crash_mem **mem_ranges);
>> int add_initrd_mem_range(struct crash_mem **mem_ranges);
>> #ifdef CONFIG_PPC_64S_HASH_MMU
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 9932793cd64b..5be30659172f 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -19,8 +19,11 @@
>> #include <linux/of.h>
>> #include <linux/libfdt.h>
>> #include <linux/memblock.h>
>> +#include <linux/memory.h>
>>
>> #include <asm/page.h>
>> +#include <asm/drmem.h>
>> +#include <asm/mmzone.h>
>> #include <asm/current.h>
>> #include <asm/machdep.h>
>> #include <asm/cacheflush.h>
>> @@ -547,9 +550,7 @@ int update_cpus_node(void *fdt)
>> #undef pr_fmt
>> #define pr_fmt(fmt) "crash hp: " fmt
>>
>> -#ifdef CONFIG_HOTPLUG_CPU
>> - /* Provides the value for the sysfs crash_hotplug nodes */
>> -int arch_crash_hotplug_cpu_support(struct kimage *image)
>> +static int crash_hotplug_support(struct kimage *image)
>> {
>> if (image->file_mode)
>> return 1;
>> @@ -560,8 +561,118 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
>> */
>> return image->update_elfcorehdr && image->update_fdt;
>> }
>> +
>> +#ifdef CONFIG_HOTPLUG_CPU
>> + /* Provides the value for the sysfs crash_hotplug nodes */
>> +int arch_crash_hotplug_cpu_support(struct kimage *image)
>> +{
>> + return crash_hotplug_support(image);
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> + /* Provides the value for the sysfs memory_hotplug nodes */
>> +int arch_crash_hotplug_memory_support(struct kimage *image)
>> +{
>> + return crash_hotplug_support(image);
>> +}
>> #endif
>>
>> +/*
>> + * Advertise preferred elfcorehdr size to userspace via
>> + * /sys/kernel/crash_elfcorehdr_size sysfs interface.
>> + */
>> +unsigned int arch_crash_get_elfcorehdr_size(void)
>> +{
>> + unsigned int sz;
>> + unsigned long elf_phdr_cnt;
>> +
>> + /* Program header for CPU notes and vmcoreinfo */
>> + elf_phdr_cnt = 2;
>> + if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>> + /* In the worst case, a Phdr is needed for every other LMB to be
>> + * represented as an individual crash range.
>> + */
>> + elf_phdr_cnt += memory_hotplug_max() / (2 * drmem_lmb_size());
>> +
>> + /* Do not cross the max limit */
>> + if (elf_phdr_cnt > PN_XNUM)
>> + elf_phdr_cnt = PN_XNUM;
>> +
>> + sz = sizeof(struct elfhdr) + (elf_phdr_cnt * sizeof(Elf64_Phdr));
>> + return sz;
>> +}
>> +
>> +/**
>> + * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace it with old
>> + * elfcorehdr in the kexec segment array.
>> + * @image: the active struct kimage
>> + * @mn: struct memory_notify data handler
>> + */
>> +static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *mn)
>> +{
>> + int ret;
>> + struct crash_mem *cmem = NULL;
>> + struct kexec_segment *ksegment;
>> + void *ptr, *mem, *elfbuf = NULL;
>> + unsigned long elfsz, memsz, base_addr, size;
>> +
>> + ksegment = &image->segment[image->elfcorehdr_index];
>> + mem = (void *) ksegment->mem;
>> + memsz = ksegment->memsz;
>> +
>> + ret = get_crash_memory_ranges(&cmem);
>> + if (ret) {
>> + pr_err("Failed to get crash mem range\n");
>> + return;
>> + }
>> +
>> + /*
>> + * The hot unplugged memory is part of crash memory ranges,
>> + * remove it here.
>> + */
>> + if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
>> + base_addr = PFN_PHYS(mn->start_pfn);
>> + size = mn->nr_pages * PAGE_SIZE;
>> + ret = remove_mem_range(&cmem, base_addr, size);
> Althouth this is ppc specific, I don't understand. Why don't you
> recreate the elfcorehdr, but take removing the removed region. Comparing the
> remove_mem_range() implementation with recreating, I don't see too much
> benefit from that, and it makes your code more complicated. Just
> curious, surely ppc people can decide what should be taken.
I am recreating `elfcorehdr` by calling `crash_prepare_elf64_headers()`
below.
This complexity is necessary to avoid adding hot-removed memory to the
new `elfcorehdr`.
On powerpc, the memblock list is utilized to prepare the `elfcorehdr`.
In the
case of memory hot removal, the memblock list is updated after the arch
crash hotplug handler is triggered. Thus, the hot-removed memory is
explicitly
removed from the crash memory ranges to ensure that the memory ranges
added to `elfcorehdr` do not include the hot-removed memory.
Thanks,
Sourabh Jain
>
>> + if (ret) {
>> + pr_err("Failed to remove hot-unplugged from crash memory ranges.\n");
>> + return;
>> + }
>> + }
>> +
>> + ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
>> + if (ret) {
>> + pr_err("Failed to prepare elf header\n");
>> + return;
>> + }
>> +
>> + /*
>> + * It is unlikely that kernel hit this because elfcorehdr kexec
>> + * segment (memsz) is built with addition space to accommodate growing
>> + * number of crash memory ranges while loading the kdump kernel. It is
>> + * Just to avoid any unforeseen case.
>> + */
>> + if (elfsz > memsz) {
>> + pr_err("Updated crash elfcorehdr elfsz %lu > memsz %lu", elfsz, memsz);
>> + goto out;
>> + }
>> +
>> + ptr = __va(mem);
>> + if (ptr) {
>> + /* Temporarily invalidate the crash image while it is replaced */
>> + xchg(&kexec_crash_image, NULL);
>> +
>> + /* Replace the old elfcorehdr with newly prepared elfcorehdr */
>> + memcpy((void *)ptr, elfbuf, elfsz);
>> +
>> + /* The crash image is now valid once again */
>> + xchg(&kexec_crash_image, image);
>> + }
>> +out:
>> + vfree(elfbuf);
>> +}
>> +
>> /**
>> * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
>> * necessary kexec segments based on the hotplug event.
>> @@ -572,7 +683,7 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
>> * CPU addition: Update the FDT segment to include the newly added CPU.
>> * CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
>> * as part of the FDT.
>> - * Memory addition/removal: No action is taken as this is not yet supported.
>> + * Memory addition/removal: Recreate the elfcorehdr segment
>> */
>> void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>> {
>> @@ -593,7 +704,6 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>> return;
>>
>> } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
>> -
>> void *fdt, *ptr;
>> unsigned long mem;
>> int i, fdt_index = -1;
>> @@ -628,8 +738,10 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>>
>> } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
>> hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
>> - pr_info_once("Crash update is not supported for memory hotplug\n");
>> - return;
>> + struct memory_notify *mn;
>> +
>> + mn = (struct memory_notify *)arg;
>> + update_crash_elfcorehdr(image, mn);
>> }
>> }
>> #endif
>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>> index e2148a009701..2457d7ec2075 100644
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -21,6 +21,8 @@
>> #include <linux/memblock.h>
>> #include <linux/slab.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/elf.h>
>> +
>> #include <asm/setup.h>
>> #include <asm/cputhreads.h>
>> #include <asm/drmem.h>
>> @@ -740,7 +742,35 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>>
>> kbuf->buffer = headers;
>> kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
>> - kbuf->bufsz = kbuf->memsz = headers_sz;
>> + kbuf->bufsz = headers_sz;
>> +#if defined(CONFIG_CRASH_HOTPLUG) && defined(CONFIG_MEMORY_HOTPLUG)
>> + /* Adjust the elfcorehdr segment size to accommodate
>> + * future crash memory ranges.
>> + */
>> + int max_lmb;
>> + unsigned long pnum;
>> +
>> + /* In the worst case, a Phdr is needed for every other LMB to be
>> + * represented as an individual crash range.
>> + */
>> + max_lmb = memory_hotplug_max() / (2 * drmem_lmb_size());
>> +
>> + /* Do not cross the Phdr max limit of the elf header.
>> + * Avoid counting Phdr for crash ranges (cmem->nr_ranges)
>> + * which are already part of elfcorehdr.
>> + */
>> + if (max_lmb > PN_XNUM)
>> + pnum = PN_XNUM - cmem->nr_ranges;
>> + else
>> + pnum = max_lmb - cmem->nr_ranges;
>> +
>> + /* Additional buffer space for elfcorehdr to accommodate
>> + * future memory ranges.
>> + */
>> + kbuf->memsz = headers_sz + pnum * sizeof(Elf64_Phdr);
>> +#else
>> + kbuf->memsz = headers_sz;
>> +#endif
>> kbuf->top_down = false;
>>
>> ret = kexec_add_buffer(kbuf);
>> @@ -750,7 +780,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>> }
>>
>> image->elf_load_addr = kbuf->mem;
>> - image->elf_headers_sz = headers_sz;
>> + image->elf_headers_sz = kbuf->memsz;
>> image->elf_headers = headers;
>> out:
>> kfree(cmem);
>> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
>> index fb3e12f15214..4fd0c5d5607b 100644
>> --- a/arch/powerpc/kexec/ranges.c
>> +++ b/arch/powerpc/kexec/ranges.c
>> @@ -234,6 +234,91 @@ int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
>> return __add_mem_range(mem_ranges, base, size);
>> }
>>
>> +/**
>> + * remove_mem_range - Removes the given memory range from the range list.
>> + * @mem_ranges: Range list to remove the memory range to.
>> + * @base: Base address of the range to remove.
>> + * @size: Size of the memory range to remove.
>> + *
>> + * (Re)allocates memory, if needed.
>> + *
>> + * Returns 0 on success, negative errno on error.
>> + */
>> +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
>> +{
>> + u64 end;
>> + int ret = 0;
>> + unsigned int i;
>> + u64 mstart, mend;
>> + struct crash_mem *mem_rngs = *mem_ranges;
>> +
>> + if (!size)
>> + return 0;
>> +
>> + /*
>> + * Memory range are stored as start and end address, use
>> + * the same format to do remove operation.
>> + */
>> + end = base + size - 1;
>> +
>> + for (i = 0; i < mem_rngs->nr_ranges; i++) {
>> + mstart = mem_rngs->ranges[i].start;
>> + mend = mem_rngs->ranges[i].end;
>> +
>> + /*
>> + * Memory range to remove is not part of this range entry
>> + * in the memory range list
>> + */
>> + if (!(base >= mstart && end <= mend))
>> + continue;
>> +
>> + /*
>> + * Memory range to remove is equivalent to this entry in the
>> + * memory range list. Remove the range entry from the list.
>> + */
>> + if (base == mstart && end == mend) {
>> + for (; i < mem_rngs->nr_ranges - 1; i++) {
>> + mem_rngs->ranges[i].start = mem_rngs->ranges[i+1].start;
>> + mem_rngs->ranges[i].end = mem_rngs->ranges[i+1].end;
>> + }
>> + mem_rngs->nr_ranges--;
>> + goto out;
>> + }
>> + /*
>> + * Start address of the memory range to remove and the
>> + * current memory range entry in the list is same. Just
>> + * move the start address of the current memory range
>> + * entry in the list to end + 1.
>> + */
>> + else if (base == mstart) {
>> + mem_rngs->ranges[i].start = end + 1;
>> + goto out;
>> + }
>> + /*
>> + * End address of the memory range to remove and the
>> + * current memory range entry in the list is same.
>> + * Just move the end address of the current memory
>> + * range entry in the list to base - 1.
>> + */
>> + else if (end == mend) {
>> + mem_rngs->ranges[i].end = base - 1;
>> + goto out;
>> + }
>> + /*
>> + * Memory range to remove is not at the edge of current
>> + * memory range entry. Split the current memory entry into
>> + * two half.
>> + */
>> + else {
>> + mem_rngs->ranges[i].end = base - 1;
>> + size = mem_rngs->ranges[i].end - end;
>> + ret = add_mem_range(mem_ranges, end + 1, size);
>> + }
>> + }
>> +out:
>> + return ret;
>> +}
>> +
>> /**
>> * add_tce_mem_ranges - Adds tce-table range to the given memory ranges list.
>> * @mem_ranges: Range list to add the memory range(s) to.
>> --
>> 2.41.0
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-15 2:28 ` Baoquan He
@ 2023-12-15 6:47 ` Sourabh Jain
2023-12-16 9:41 ` Baoquan He
0 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-15 6:47 UTC (permalink / raw)
To: Baoquan He
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
Hello Baoquan,
On 15/12/23 07:58, Baoquan He wrote:
> On 12/11/23 at 02:00pm, Sourabh Jain wrote:
>> The commit a72bbec70da2 ("crash: hotplug support for kexec_load()")
>> introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses
>> this flag to indicate kernel that it is safe to modify the elfcorehdr
>> of kdump image loaded using kexec_load system call.
>>
>> Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump
>> component named FDT (Flatten Device Tree). Architectures like PowerPC
>> need to update FDT kdump image component on CPU hotplug events. Kexec
>> tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT
>> segment is not part of SHA calculation hence it is safe to update it.
>>
>> With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags,
>> crash hotplug support can be added to PowerPC for the kexec_load syscall
>> while maintaining the backward compatibility with older kexec tools that
>> do not have these newly introduced flags.
>>
>> 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
>> ---
>> include/linux/kexec.h | 6 ++++--
>> include/uapi/linux/kexec.h | 1 +
>> kernel/kexec.c | 2 ++
>> 3 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 0f6ea35879ee..bcedb7625b1f 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -319,6 +319,7 @@ struct kimage {
>> #ifdef CONFIG_CRASH_HOTPLUG
>> /* If set, allow changes to elfcorehdr of kexec_load'd image */
>> unsigned int update_elfcorehdr:1;
>> + unsigned int update_fdt:1;
> Can we unify this to one flag, e.g hotplug_update?
>
> With this, on x86_64, we will skip the sha calculation for elfcorehdr.
> On ppc, we will skip the sha calculation for elfcorehdr and fdt.
Yeah, that's what I suggested to Eric. I can do that, but I see one
problem with powerpc or other platforms that need to skip SHA
for more kexec segments in addition to elfcorehdr.
`update_elfcorehdr` is set when the kexec tool sends the
`KEXEC_UPDATE_ELFCOREHDR`
flag to the kernel for the `kexec_load` system call.
Given that the kexec tool has already been updated to send the
`KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
SHA verification in generic code, now it would be tricky for
architectures to
determine whether kexec has skipped SHA verification for just elfcorehdr
or all segments needed on the platform with the same flag.
Code snippet from the kexec tool:
main() {
...
/* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */
if (do_hotplug) {
...
/* Indicate to the kernel it is ok to modify the elfcorehdr */
kexec_flags |= KEXEC_UPDATE_ELFCOREHDR;
}
...
}
Any suggestion how to handle this with just one kexec flag?
Thanks for the review.
Thanks,
Sourabh Jain
>
>> #endif
>>
>> #ifdef ARCH_HAS_KIMAGE_ARCH
>> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
>>
>> /* List of defined/legal kexec flags */
>> #ifndef CONFIG_KEXEC_JUMP
>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
>> #else
>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
>> + KEXEC_UPDATE_FDT)
>> #endif
>>
>> /* List of defined/legal kexec file flags */
>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
>> index 01766dd839b0..3d5b3d757bed 100644
>> --- a/include/uapi/linux/kexec.h
>> +++ b/include/uapi/linux/kexec.h
>> @@ -13,6 +13,7 @@
>> #define KEXEC_ON_CRASH 0x00000001
>> #define KEXEC_PRESERVE_CONTEXT 0x00000002
>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
>> +#define KEXEC_UPDATE_FDT 0x00000008
>> #define KEXEC_ARCH_MASK 0xffff0000
>>
>> /*
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>> index 8f35a5a42af8..97eb151cd931 100644
>> --- a/kernel/kexec.c
>> +++ b/kernel/kexec.c
>> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>> #ifdef CONFIG_CRASH_HOTPLUG
>> if (flags & KEXEC_UPDATE_ELFCOREHDR)
>> image->update_elfcorehdr = 1;
>> + if (flags & KEXEC_UPDATE_FDT)
>> + image->update_fdt = 1;
>> #endif
>>
>> ret = machine_kexec_prepare(image);
>> --
>> 2.41.0
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 6/6] powerpc: add crash memory hotplug support
2023-12-15 5:59 ` Sourabh Jain
@ 2023-12-16 3:11 ` Baoquan He
0 siblings, 0 replies; 21+ messages in thread
From: Baoquan He @ 2023-12-16 3:11 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/15/23 at 11:29am, Sourabh Jain wrote:
......
> > > +static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *mn)
> > > +{
> > > + int ret;
> > > + struct crash_mem *cmem = NULL;
> > > + struct kexec_segment *ksegment;
> > > + void *ptr, *mem, *elfbuf = NULL;
> > > + unsigned long elfsz, memsz, base_addr, size;
> > > +
> > > + ksegment = &image->segment[image->elfcorehdr_index];
> > > + mem = (void *) ksegment->mem;
> > > + memsz = ksegment->memsz;
> > > +
> > > + ret = get_crash_memory_ranges(&cmem);
> > > + if (ret) {
> > > + pr_err("Failed to get crash mem range\n");
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * The hot unplugged memory is part of crash memory ranges,
> > > + * remove it here.
> > > + */
> > > + if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
> > > + base_addr = PFN_PHYS(mn->start_pfn);
> > > + size = mn->nr_pages * PAGE_SIZE;
> > > + ret = remove_mem_range(&cmem, base_addr, size);
> > Althouth this is ppc specific, I don't understand. Why don't you
> > recreate the elfcorehdr, but take removing the removed region. Comparing the
> > remove_mem_range() implementation with recreating, I don't see too much
> > benefit from that, and it makes your code more complicated. Just
> > curious, surely ppc people can decide what should be taken.
>
> I am recreating `elfcorehdr` by calling `crash_prepare_elf64_headers()`
> below.
>
> This complexity is necessary to avoid adding hot-removed memory to the
> new `elfcorehdr`.
>
> On powerpc, the memblock list is utilized to prepare the `elfcorehdr`. In
> the
> case of memory hot removal, the memblock list is updated after the arch
> crash hotplug handler is triggered. Thus, the hot-removed memory is
> explicitly
> removed from the crash memory ranges to ensure that the memory ranges
> added to `elfcorehdr` do not include the hot-removed memory.
Ah, I see. Thanks for the explanation. Then please ignore this one.
>
>
> >
> > > + if (ret) {
> > > + pr_err("Failed to remove hot-unplugged from crash memory ranges.\n");
> > > + return;
> > > + }
> > > + }
> > > +
> > > + ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
> > > + if (ret) {
> > > + pr_err("Failed to prepare elf header\n");
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * It is unlikely that kernel hit this because elfcorehdr kexec
> > > + * segment (memsz) is built with addition space to accommodate growing
> > > + * number of crash memory ranges while loading the kdump kernel. It is
> > > + * Just to avoid any unforeseen case.
> > > + */
> > > + if (elfsz > memsz) {
> > > + pr_err("Updated crash elfcorehdr elfsz %lu > memsz %lu", elfsz, memsz);
> > > + goto out;
> > > + }
> > > +
> > > + ptr = __va(mem);
> > > + if (ptr) {
> > > + /* Temporarily invalidate the crash image while it is replaced */
> > > + xchg(&kexec_crash_image, NULL);
> > > +
> > > + /* Replace the old elfcorehdr with newly prepared elfcorehdr */
> > > + memcpy((void *)ptr, elfbuf, elfsz);
> > > +
> > > + /* The crash image is now valid once again */
> > > + xchg(&kexec_crash_image, image);
> > > + }
> > > +out:
> > > + vfree(elfbuf);
> > > +}
> > > +
> > > /**
> > > * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
> > > * necessary kexec segments based on the hotplug event.
> > > @@ -572,7 +683,7 @@ int arch_crash_hotplug_cpu_support(struct kimage *image)
> > > * CPU addition: Update the FDT segment to include the newly added CPU.
> > > * CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
> > > * as part of the FDT.
> > > - * Memory addition/removal: No action is taken as this is not yet supported.
> > > + * Memory addition/removal: Recreate the elfcorehdr segment
> > > */
> > > void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> > > {
> > > @@ -593,7 +704,6 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> > > return;
> > > } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
> > > -
> > > void *fdt, *ptr;
> > > unsigned long mem;
> > > int i, fdt_index = -1;
> > > @@ -628,8 +738,10 @@ void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> > > } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
> > > hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> > > - pr_info_once("Crash update is not supported for memory hotplug\n");
> > > - return;
> > > + struct memory_notify *mn;
> > > +
> > > + mn = (struct memory_notify *)arg;
> > > + update_crash_elfcorehdr(image, mn);
> > > }
> > > }
> > > #endif
> > > diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> > > index e2148a009701..2457d7ec2075 100644
> > > --- a/arch/powerpc/kexec/file_load_64.c
> > > +++ b/arch/powerpc/kexec/file_load_64.c
> > > @@ -21,6 +21,8 @@
> > > #include <linux/memblock.h>
> > > #include <linux/slab.h>
> > > #include <linux/vmalloc.h>
> > > +#include <linux/elf.h>
> > > +
> > > #include <asm/setup.h>
> > > #include <asm/cputhreads.h>
> > > #include <asm/drmem.h>
> > > @@ -740,7 +742,35 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
> > > kbuf->buffer = headers;
> > > kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
> > > - kbuf->bufsz = kbuf->memsz = headers_sz;
> > > + kbuf->bufsz = headers_sz;
> > > +#if defined(CONFIG_CRASH_HOTPLUG) && defined(CONFIG_MEMORY_HOTPLUG)
> > > + /* Adjust the elfcorehdr segment size to accommodate
> > > + * future crash memory ranges.
> > > + */
> > > + int max_lmb;
> > > + unsigned long pnum;
> > > +
> > > + /* In the worst case, a Phdr is needed for every other LMB to be
> > > + * represented as an individual crash range.
> > > + */
> > > + max_lmb = memory_hotplug_max() / (2 * drmem_lmb_size());
> > > +
> > > + /* Do not cross the Phdr max limit of the elf header.
> > > + * Avoid counting Phdr for crash ranges (cmem->nr_ranges)
> > > + * which are already part of elfcorehdr.
> > > + */
> > > + if (max_lmb > PN_XNUM)
> > > + pnum = PN_XNUM - cmem->nr_ranges;
> > > + else
> > > + pnum = max_lmb - cmem->nr_ranges;
> > > +
> > > + /* Additional buffer space for elfcorehdr to accommodate
> > > + * future memory ranges.
> > > + */
> > > + kbuf->memsz = headers_sz + pnum * sizeof(Elf64_Phdr);
> > > +#else
> > > + kbuf->memsz = headers_sz;
> > > +#endif
> > > kbuf->top_down = false;
> > > ret = kexec_add_buffer(kbuf);
> > > @@ -750,7 +780,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
> > > }
> > > image->elf_load_addr = kbuf->mem;
> > > - image->elf_headers_sz = headers_sz;
> > > + image->elf_headers_sz = kbuf->memsz;
> > > image->elf_headers = headers;
> > > out:
> > > kfree(cmem);
> > > diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
> > > index fb3e12f15214..4fd0c5d5607b 100644
> > > --- a/arch/powerpc/kexec/ranges.c
> > > +++ b/arch/powerpc/kexec/ranges.c
> > > @@ -234,6 +234,91 @@ int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
> > > return __add_mem_range(mem_ranges, base, size);
> > > }
> > > +/**
> > > + * remove_mem_range - Removes the given memory range from the range list.
> > > + * @mem_ranges: Range list to remove the memory range to.
> > > + * @base: Base address of the range to remove.
> > > + * @size: Size of the memory range to remove.
> > > + *
> > > + * (Re)allocates memory, if needed.
> > > + *
> > > + * Returns 0 on success, negative errno on error.
> > > + */
> > > +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size)
> > > +{
> > > + u64 end;
> > > + int ret = 0;
> > > + unsigned int i;
> > > + u64 mstart, mend;
> > > + struct crash_mem *mem_rngs = *mem_ranges;
> > > +
> > > + if (!size)
> > > + return 0;
> > > +
> > > + /*
> > > + * Memory range are stored as start and end address, use
> > > + * the same format to do remove operation.
> > > + */
> > > + end = base + size - 1;
> > > +
> > > + for (i = 0; i < mem_rngs->nr_ranges; i++) {
> > > + mstart = mem_rngs->ranges[i].start;
> > > + mend = mem_rngs->ranges[i].end;
> > > +
> > > + /*
> > > + * Memory range to remove is not part of this range entry
> > > + * in the memory range list
> > > + */
> > > + if (!(base >= mstart && end <= mend))
> > > + continue;
> > > +
> > > + /*
> > > + * Memory range to remove is equivalent to this entry in the
> > > + * memory range list. Remove the range entry from the list.
> > > + */
> > > + if (base == mstart && end == mend) {
> > > + for (; i < mem_rngs->nr_ranges - 1; i++) {
> > > + mem_rngs->ranges[i].start = mem_rngs->ranges[i+1].start;
> > > + mem_rngs->ranges[i].end = mem_rngs->ranges[i+1].end;
> > > + }
> > > + mem_rngs->nr_ranges--;
> > > + goto out;
> > > + }
> > > + /*
> > > + * Start address of the memory range to remove and the
> > > + * current memory range entry in the list is same. Just
> > > + * move the start address of the current memory range
> > > + * entry in the list to end + 1.
> > > + */
> > > + else if (base == mstart) {
> > > + mem_rngs->ranges[i].start = end + 1;
> > > + goto out;
> > > + }
> > > + /*
> > > + * End address of the memory range to remove and the
> > > + * current memory range entry in the list is same.
> > > + * Just move the end address of the current memory
> > > + * range entry in the list to base - 1.
> > > + */
> > > + else if (end == mend) {
> > > + mem_rngs->ranges[i].end = base - 1;
> > > + goto out;
> > > + }
> > > + /*
> > > + * Memory range to remove is not at the edge of current
> > > + * memory range entry. Split the current memory entry into
> > > + * two half.
> > > + */
> > > + else {
> > > + mem_rngs->ranges[i].end = base - 1;
> > > + size = mem_rngs->ranges[i].end - end;
> > > + ret = add_mem_range(mem_ranges, end + 1, size);
> > > + }
> > > + }
> > > +out:
> > > + return ret;
> > > +}
> > > +
> > > /**
> > > * add_tce_mem_ranges - Adds tce-table range to the given memory ranges list.
> > > * @mem_ranges: Range list to add the memory range(s) to.
> > > --
> > > 2.41.0
> > >
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-15 6:47 ` Sourabh Jain
@ 2023-12-16 9:41 ` Baoquan He
2023-12-16 18:57 ` Sourabh Jain
0 siblings, 1 reply; 21+ messages in thread
From: Baoquan He @ 2023-12-16 9:41 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/15/23 at 12:17pm, Sourabh Jain wrote:
......
> > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > index 0f6ea35879ee..bcedb7625b1f 100644
> > > --- a/include/linux/kexec.h
> > > +++ b/include/linux/kexec.h
> > > @@ -319,6 +319,7 @@ struct kimage {
> > > #ifdef CONFIG_CRASH_HOTPLUG
> > > /* If set, allow changes to elfcorehdr of kexec_load'd image */
> > > unsigned int update_elfcorehdr:1;
> > > + unsigned int update_fdt:1;
> > Can we unify this to one flag, e.g hotplug_update?
> >
> > With this, on x86_64, we will skip the sha calculation for elfcorehdr.
> > On ppc, we will skip the sha calculation for elfcorehdr and fdt.
> Yeah, that's what I suggested to Eric. I can do that, but I see one
> problem with powerpc or other platforms that need to skip SHA
> for more kexec segments in addition to elfcorehdr.
>
> `update_elfcorehdr` is set when the kexec tool sends the
> `KEXEC_UPDATE_ELFCOREHDR`
> flag to the kernel for the `kexec_load` system call.
>
> Given that the kexec tool has already been updated to send the
> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
> SHA verification in generic code, now it would be tricky for architectures
> to
> determine whether kexec has skipped SHA verification for just elfcorehdr
> or all segments needed on the platform with the same flag.
In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr
segment. I am wondering how you skip the fdt segment when calculating
and verifying sha, only saw the update_fdt mark.
About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro,
but still use the same value, could you think of what problem could be
caused between kernel and kexec-tools utility, the old and new version
compatibility?
If it's about the new header files installed on older kernel, we can
change it like below? Fortunately only one release, 6.6 passed.
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 3d5b3d757bed..df6a6505e267 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -13,7 +13,7 @@
#define KEXEC_ON_CRASH 0x00000001
#define KEXEC_PRESERVE_CONTEXT 0x00000002
-#define KEXEC_UPDATE_FDT 0x00000008
+#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
#define KEXEC_UPDATE_ELFCOREHDR 0x00000004
#define KEXEC_ARCH_MASK 0xffff0000
/*
With my understanding, the kexec flag should be indicating the action,
the mem/cpu hotplug, but not relating to any detail. Imagine later
another segment need be skipped on one ARCH again, then another flag
need be added, this sounds not reasonable.
>
> Code snippet from the kexec tool:
>
> main() {
> ...
> /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */
> if (do_hotplug) {
> ...
>
> /* Indicate to the kernel it is ok to modify the elfcorehdr */
> kexec_flags |= KEXEC_UPDATE_ELFCOREHDR;
> }
> ...
> }
>
> Any suggestion how to handle this with just one kexec flag?
>
> Thanks for the review.
>
> Thanks,
> Sourabh Jain
>
> >
> > > #endif
> > > #ifdef ARCH_HAS_KIMAGE_ARCH
> > > @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
> > > /* List of defined/legal kexec flags */
> > > #ifndef CONFIG_KEXEC_JUMP
> > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
> > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
> > > #else
> > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
> > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
> > > + KEXEC_UPDATE_FDT)
> > > #endif
> > > /* List of defined/legal kexec file flags */
> > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> > > index 01766dd839b0..3d5b3d757bed 100644
> > > --- a/include/uapi/linux/kexec.h
> > > +++ b/include/uapi/linux/kexec.h
> > > @@ -13,6 +13,7 @@
> > > #define KEXEC_ON_CRASH 0x00000001
> > > #define KEXEC_PRESERVE_CONTEXT 0x00000002
> > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> > > +#define KEXEC_UPDATE_FDT 0x00000008
> > > #define KEXEC_ARCH_MASK 0xffff0000
> > > /*
> > > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > > index 8f35a5a42af8..97eb151cd931 100644
> > > --- a/kernel/kexec.c
> > > +++ b/kernel/kexec.c
> > > @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > > #ifdef CONFIG_CRASH_HOTPLUG
> > > if (flags & KEXEC_UPDATE_ELFCOREHDR)
> > > image->update_elfcorehdr = 1;
> > > + if (flags & KEXEC_UPDATE_FDT)
> > > + image->update_fdt = 1;
> > > #endif
> > > ret = machine_kexec_prepare(image);
> > > --
> > > 2.41.0
> > >
>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-16 9:41 ` Baoquan He
@ 2023-12-16 18:57 ` Sourabh Jain
2023-12-17 0:59 ` Baoquan He
0 siblings, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-16 18:57 UTC (permalink / raw)
To: Baoquan He
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 16/12/23 15:11, Baoquan He wrote:
> On 12/15/23 at 12:17pm, Sourabh Jain wrote:
> ......
>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>>> index 0f6ea35879ee..bcedb7625b1f 100644
>>>> --- a/include/linux/kexec.h
>>>> +++ b/include/linux/kexec.h
>>>> @@ -319,6 +319,7 @@ struct kimage {
>>>> #ifdef CONFIG_CRASH_HOTPLUG
>>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */
>>>> unsigned int update_elfcorehdr:1;
>>>> + unsigned int update_fdt:1;
>>> Can we unify this to one flag, e.g hotplug_update?
>>>
>>> With this, on x86_64, we will skip the sha calculation for elfcorehdr.
>>> On ppc, we will skip the sha calculation for elfcorehdr and fdt.
>> Yeah, that's what I suggested to Eric. I can do that, but I see one
>> problem with powerpc or other platforms that need to skip SHA
>> for more kexec segments in addition to elfcorehdr.
>>
>> `update_elfcorehdr` is set when the kexec tool sends the
>> `KEXEC_UPDATE_ELFCOREHDR`
>> flag to the kernel for the `kexec_load` system call.
>>
>> Given that the kexec tool has already been updated to send the
>> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
>> SHA verification in generic code, now it would be tricky for architectures
>> to
>> determine whether kexec has skipped SHA verification for just elfcorehdr
>> or all segments needed on the platform with the same flag.
> In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr
> segment. I am wondering how you skip the fdt segment when calculating
> and verifying sha, only saw the update_fdt mark.
In the kexec tool where we loop through all the kexec segments to calculate
the SHA, there will be a arch call made to determine whether the segment
needs
to be excluded from SHA or not.
Now in the arch function if decide a specific segment needs to excluded then
corresponding flag is also set by arch function to communicate same with the
kernel.
>
> About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro,
> but still use the same value, could you think of what problem could be
> caused between kernel and kexec-tools utility, the old and new version
> compatibility?
Just changing the macro name will NOT help because the current kexec tool
enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit
if
the command argument --hotplug is passed to the kexec
and
the /sys/kernel/crash_elfcorehdr_size file exists in the system.
Now, let's say an architecture enables this feature in the kernel with
the assumption
that the 0x00000004 kexec flag bit is passed from the kexec tool when
all the required
kexec segments are skipped from SHA calculation. In this case, the
current kexec tool,
which passes the 0x00000004 kexec flag bit only when the elfcorehdr is
skipped, will
cause issues for architectures.
>
> If it's about the new header files installed on older kernel, we can
> change it like below? Fortunately only one release, 6.6 passed.
>
> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> index 3d5b3d757bed..df6a6505e267 100644
> --- a/include/uapi/linux/kexec.h
> +++ b/include/uapi/linux/kexec.h
> @@ -13,7 +13,7 @@
> #define KEXEC_ON_CRASH 0x00000001
> #define KEXEC_PRESERVE_CONTEXT 0x00000002
> -#define KEXEC_UPDATE_FDT 0x00000008
> +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> #define KEXEC_ARCH_MASK 0xffff0000
>
> /*
>
> With my understanding, the kexec flag should be indicating the action,
> the mem/cpu hotplug, but not relating to any detail. Imagine later
> another segment need be skipped on one ARCH again, then another flag
> need be added, this sounds not reasonable.
I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag
should be sufficient to inform the kernel that the kexec tool has been
updated
to support CPU/Memory hotplug for the kexec_load system call. Unfortunately,
we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE
at the moment.
What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag?
I am aware that we are utilizing two kexec flag bits (0x00000004 and
0x00000008)
for the same feature, but what other options do we have?
Thanks,
Sourabh
>> Code snippet from the kexec tool:
>>
>> main() {
>> ...
>> /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */
>> if (do_hotplug) {
>> ...
>>
>> /* Indicate to the kernel it is ok to modify the elfcorehdr */
>> kexec_flags |= KEXEC_UPDATE_ELFCOREHDR;
>> }
>> ...
>> }
>>
>> Any suggestion how to handle this with just one kexec flag?
>>
>> Thanks for the review.
>>
>> Thanks,
>> Sourabh Jain
>>
>>>> #endif
>>>> #ifdef ARCH_HAS_KIMAGE_ARCH
>>>> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
>>>> /* List of defined/legal kexec flags */
>>>> #ifndef CONFIG_KEXEC_JUMP
>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
>>>> #else
>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
>>>> + KEXEC_UPDATE_FDT)
>>>> #endif
>>>> /* List of defined/legal kexec file flags */
>>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
>>>> index 01766dd839b0..3d5b3d757bed 100644
>>>> --- a/include/uapi/linux/kexec.h
>>>> +++ b/include/uapi/linux/kexec.h
>>>> @@ -13,6 +13,7 @@
>>>> #define KEXEC_ON_CRASH 0x00000001
>>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002
>>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
>>>> +#define KEXEC_UPDATE_FDT 0x00000008
>>>> #define KEXEC_ARCH_MASK 0xffff0000
>>>> /*
>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>>> index 8f35a5a42af8..97eb151cd931 100644
>>>> --- a/kernel/kexec.c
>>>> +++ b/kernel/kexec.c
>>>> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>> #ifdef CONFIG_CRASH_HOTPLUG
>>>> if (flags & KEXEC_UPDATE_ELFCOREHDR)
>>>> image->update_elfcorehdr = 1;
>>>> + if (flags & KEXEC_UPDATE_FDT)
>>>> + image->update_fdt = 1;
>>>> #endif
>>>> ret = machine_kexec_prepare(image);
>>>> --
>>>> 2.41.0
>>>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-16 18:57 ` Sourabh Jain
@ 2023-12-17 0:59 ` Baoquan He
2023-12-17 15:50 ` Sourabh Jain
2023-12-21 6:06 ` Sourabh Jain
0 siblings, 2 replies; 21+ messages in thread
From: Baoquan He @ 2023-12-17 0:59 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/17/23 at 12:27am, Sourabh Jain wrote:
>
>
> On 16/12/23 15:11, Baoquan He wrote:
> > On 12/15/23 at 12:17pm, Sourabh Jain wrote:
> > ......
> > > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > > > index 0f6ea35879ee..bcedb7625b1f 100644
> > > > > --- a/include/linux/kexec.h
> > > > > +++ b/include/linux/kexec.h
> > > > > @@ -319,6 +319,7 @@ struct kimage {
> > > > > #ifdef CONFIG_CRASH_HOTPLUG
> > > > > /* If set, allow changes to elfcorehdr of kexec_load'd image */
> > > > > unsigned int update_elfcorehdr:1;
> > > > > + unsigned int update_fdt:1;
> > > > Can we unify this to one flag, e.g hotplug_update?
> > > >
> > > > With this, on x86_64, we will skip the sha calculation for elfcorehdr.
> > > > On ppc, we will skip the sha calculation for elfcorehdr and fdt.
> > > Yeah, that's what I suggested to Eric. I can do that, but I see one
> > > problem with powerpc or other platforms that need to skip SHA
> > > for more kexec segments in addition to elfcorehdr.
> > >
> > > `update_elfcorehdr` is set when the kexec tool sends the
> > > `KEXEC_UPDATE_ELFCOREHDR`
> > > flag to the kernel for the `kexec_load` system call.
> > >
> > > Given that the kexec tool has already been updated to send the
> > > `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
> > > SHA verification in generic code, now it would be tricky for architectures
> > > to
> > > determine whether kexec has skipped SHA verification for just elfcorehdr
> > > or all segments needed on the platform with the same flag.
> > In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr
> > segment. I am wondering how you skip the fdt segment when calculating
> > and verifying sha, only saw the update_fdt mark.
>
> In the kexec tool where we loop through all the kexec segments to calculate
> the SHA, there will be a arch call made to determine whether the segment
> needs
> to be excluded from SHA or not.
OK, a arch call will be added to exclude segments in the ARCH. And the
elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later
may not want to exclude elfcorehdr.
>
> Now in the arch function if decide a specific segment needs to excluded then
> corresponding flag is also set by arch function to communicate same with the
> kernel.
But I don't see how you exclude elfcorehdr and fdt in kernel for
kexec_file codes. It's not happening in kexec-tools.
>
> >
> > About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro,
> > but still use the same value, could you think of what problem could be
> > caused between kernel and kexec-tools utility, the old and new version
> > compatibility?
>
> Just changing the macro name will NOT help because the current kexec tool
> enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit
> if
> the command argument --hotplug is passed to the kexec
> and
> the /sys/kernel/crash_elfcorehdr_size file exists in the system.
As we have discussed, excluding will be done in each ARCH's function
when doing sha calculation in kexec-tools, isn't it?
diff --git a/kexec/kexec.c b/kexec/kexec.c
index b5393e3b20aa..0095aeec988a 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info)
continue;
}
- /* Don't include elfcorehdr in the checksum, if hotplug
+ /* Don't include unwanted segments in the checksum, if hotplug
* support enabled.
- */
- if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) {
+ if (do_hotplug)
+ arch_exclude_segments(info, &i)
continue;
}
>
> Now, let's say an architecture enables this feature in the kernel with the
> assumption
> that the 0x00000004 kexec flag bit is passed from the kexec tool when all
> the required
> kexec segments are skipped from SHA calculation. In this case, the current
> kexec tool,
> which passes the 0x00000004 kexec flag bit only when the elfcorehdr is
> skipped, will
> cause issues for architectures.
>
> >
> > If it's about the new header files installed on older kernel, we can
> > change it like below? Fortunately only one release, 6.6 passed.
> >
> > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> > index 3d5b3d757bed..df6a6505e267 100644
> > --- a/include/uapi/linux/kexec.h
> > +++ b/include/uapi/linux/kexec.h
> > @@ -13,7 +13,7 @@
> > #define KEXEC_ON_CRASH 0x00000001
> > #define KEXEC_PRESERVE_CONTEXT 0x00000002
> > -#define KEXEC_UPDATE_FDT 0x00000008
> > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
> > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> > #define KEXEC_ARCH_MASK 0xffff0000
> > /*
> >
> > With my understanding, the kexec flag should be indicating the action,
> > the mem/cpu hotplug, but not relating to any detail. Imagine later
> > another segment need be skipped on one ARCH again, then another flag
> > need be added, this sounds not reasonable.
> I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag
> should be sufficient to inform the kernel that the kexec tool has been
> updated
> to support CPU/Memory hotplug for the kexec_load system call. Unfortunately,
> we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE
> at the moment.
I am fine with 0x00000008 and a new flag, it has the same effect as
#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
I am worried about the header file incompatiblity.
>
> What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag?
>
> I am aware that we are utilizing two kexec flag bits (0x00000004 and
> 0x00000008)
> for the same feature, but what other options do we have?
>
> Thanks,
> Sourabh
> > > Code snippet from the kexec tool:
> > >
> > > main() {
> > > ...
> > > /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */
> > > if (do_hotplug) {
> > > ...
> > >
> > > /* Indicate to the kernel it is ok to modify the elfcorehdr */
> > > kexec_flags |= KEXEC_UPDATE_ELFCOREHDR;
> > > }
> > > ...
> > > }
> > >
> > > Any suggestion how to handle this with just one kexec flag?
> > >
> > > Thanks for the review.
> > >
> > > Thanks,
> > > Sourabh Jain
> > >
> > > > > #endif
> > > > > #ifdef ARCH_HAS_KIMAGE_ARCH
> > > > > @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
> > > > > /* List of defined/legal kexec flags */
> > > > > #ifndef CONFIG_KEXEC_JUMP
> > > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
> > > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
> > > > > #else
> > > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
> > > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
> > > > > + KEXEC_UPDATE_FDT)
> > > > > #endif
> > > > > /* List of defined/legal kexec file flags */
> > > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> > > > > index 01766dd839b0..3d5b3d757bed 100644
> > > > > --- a/include/uapi/linux/kexec.h
> > > > > +++ b/include/uapi/linux/kexec.h
> > > > > @@ -13,6 +13,7 @@
> > > > > #define KEXEC_ON_CRASH 0x00000001
> > > > > #define KEXEC_PRESERVE_CONTEXT 0x00000002
> > > > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> > > > > +#define KEXEC_UPDATE_FDT 0x00000008
> > > > > #define KEXEC_ARCH_MASK 0xffff0000
> > > > > /*
> > > > > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > > > > index 8f35a5a42af8..97eb151cd931 100644
> > > > > --- a/kernel/kexec.c
> > > > > +++ b/kernel/kexec.c
> > > > > @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > > > > #ifdef CONFIG_CRASH_HOTPLUG
> > > > > if (flags & KEXEC_UPDATE_ELFCOREHDR)
> > > > > image->update_elfcorehdr = 1;
> > > > > + if (flags & KEXEC_UPDATE_FDT)
> > > > > + image->update_fdt = 1;
> > > > > #endif
> > > > > ret = machine_kexec_prepare(image);
> > > > > --
> > > > > 2.41.0
> > > > >
>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-17 0:59 ` Baoquan He
@ 2023-12-17 15:50 ` Sourabh Jain
2023-12-21 6:06 ` Sourabh Jain
1 sibling, 0 replies; 21+ messages in thread
From: Sourabh Jain @ 2023-12-17 15:50 UTC (permalink / raw)
To: linuxppc-dev
On 17/12/23 06:29, Baoquan He wrote:
> On 12/17/23 at 12:27am, Sourabh Jain wrote:
>>
>> On 16/12/23 15:11, Baoquan He wrote:
>>> On 12/15/23 at 12:17pm, Sourabh Jain wrote:
>>> ......
>>>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>>>>> index 0f6ea35879ee..bcedb7625b1f 100644
>>>>>> --- a/include/linux/kexec.h
>>>>>> +++ b/include/linux/kexec.h
>>>>>> @@ -319,6 +319,7 @@ struct kimage {
>>>>>> #ifdef CONFIG_CRASH_HOTPLUG
>>>>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */
>>>>>> unsigned int update_elfcorehdr:1;
>>>>>> + unsigned int update_fdt:1;
>>>>> Can we unify this to one flag, e.g hotplug_update?
>>>>>
>>>>> With this, on x86_64, we will skip the sha calculation for elfcorehdr.
>>>>> On ppc, we will skip the sha calculation for elfcorehdr and fdt.
>>>> Yeah, that's what I suggested to Eric. I can do that, but I see one
>>>> problem with powerpc or other platforms that need to skip SHA
>>>> for more kexec segments in addition to elfcorehdr.
>>>>
>>>> `update_elfcorehdr` is set when the kexec tool sends the
>>>> `KEXEC_UPDATE_ELFCOREHDR`
>>>> flag to the kernel for the `kexec_load` system call.
>>>>
>>>> Given that the kexec tool has already been updated to send the
>>>> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
>>>> SHA verification in generic code, now it would be tricky for architectures
>>>> to
>>>> determine whether kexec has skipped SHA verification for just elfcorehdr
>>>> or all segments needed on the platform with the same flag.
>>> In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr
>>> segment. I am wondering how you skip the fdt segment when calculating
>>> and verifying sha, only saw the update_fdt mark.
>> In the kexec tool where we loop through all the kexec segments to calculate
>> the SHA, there will be a arch call made to determine whether the segment
>> needs
>> to be excluded from SHA or not.
> OK, a arch call will be added to exclude segments in the ARCH. And the
> elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later
> may not want to exclude elfcorehdr.
Yes, Arch can choose which segment to exclude.
>
>> Now in the arch function if decide a specific segment needs to excluded then
>> corresponding flag is also set by arch function to communicate same with the
>> kernel.
> But I don't see how you exclude elfcorehdr and fdt in kernel for
> kexec_file codes. It's not happening in kexec-tools.
On PowerPC, SHA verification is NOT performed for the kexec_file_load
case; hence, you
won't find any code changes in my patch series to exclude FDT in the
kernel code.
However, let's consider a scenario where it gets added in the future, or
other architectures
need to skip the kexec segment, in addition to elfcorehdr. In that case,
we can use the
same setup as you suggested below. For each kexec segment, there should
be an
architecture-specific function call to decide whether the segment needs
to be excluded or not.
>
>>> About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro,
>>> but still use the same value, could you think of what problem could be
>>> caused between kernel and kexec-tools utility, the old and new version
>>> compatibility?
>> Just changing the macro name will NOT help because the current kexec tool
>> enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit
>> if
>> the command argument --hotplug is passed to the kexec
>> and
>> the /sys/kernel/crash_elfcorehdr_size file exists in the system.
> As we have discussed, excluding will be done in each ARCH's function
> when doing sha calculation in kexec-tools, isn't it?
>
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index b5393e3b20aa..0095aeec988a 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info)
> continue;
> }
>
> - /* Don't include elfcorehdr in the checksum, if hotplug
> + /* Don't include unwanted segments in the checksum, if hotplug
> * support enabled.
> - */
> - if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) {
> + if (do_hotplug)
> + arch_exclude_segments(info, &i)
> continue;
> }
Yes, something like the above should work.
>
>
>> Now, let's say an architecture enables this feature in the kernel with the
>> assumption
>> that the 0x00000004 kexec flag bit is passed from the kexec tool when all
>> the required
>> kexec segments are skipped from SHA calculation. In this case, the current
>> kexec tool,
>> which passes the 0x00000004 kexec flag bit only when the elfcorehdr is
>> skipped, will
>> cause issues for architectures.
>>
>>> If it's about the new header files installed on older kernel, we can
>>> change it like below? Fortunately only one release, 6.6 passed.
>>>
>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
>>> index 3d5b3d757bed..df6a6505e267 100644
>>> --- a/include/uapi/linux/kexec.h
>>> +++ b/include/uapi/linux/kexec.h
>>> @@ -13,7 +13,7 @@
>>> #define KEXEC_ON_CRASH 0x00000001
>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002
>>> -#define KEXEC_UPDATE_FDT 0x00000008
>>> +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
>>> #define KEXEC_ARCH_MASK 0xffff0000
>>> /*
>>>
>>> With my understanding, the kexec flag should be indicating the action,
>>> the mem/cpu hotplug, but not relating to any detail. Imagine later
>>> another segment need be skipped on one ARCH again, then another flag
>>> need be added, this sounds not reasonable.
>> I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag
>> should be sufficient to inform the kernel that the kexec tool has been
>> updated
>> to support CPU/Memory hotplug for the kexec_load system call. Unfortunately,
>> we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE
>> at the moment.
> I am fine with 0x00000008 and a new flag, it has the same effect as
> #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
>
> I am worried about the header file incompatiblity.
If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit
to introduce CPU/Memory hotplug feature for kexec_load syscall, we will
not have
compatibility issue.
Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE
0x00000008
as new flag bit and show how it will be handled. I will also share kexec
code for clarity.
Thanks,
Sourabh
>
>> What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag?
>>
>> I am aware that we are utilizing two kexec flag bits (0x00000004 and
>> 0x00000008)
>> for the same feature, but what other options do we have?
>>
>> Thanks,
>> Sourabh
>>>> Code snippet from the kexec tool:
>>>>
>>>> main() {
>>>> ...
>>>> /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */
>>>> if (do_hotplug) {
>>>> ...
>>>>
>>>> /* Indicate to the kernel it is ok to modify the elfcorehdr */
>>>> kexec_flags |= KEXEC_UPDATE_ELFCOREHDR;
>>>> }
>>>> ...
>>>> }
>>>>
>>>> Any suggestion how to handle this with just one kexec flag?
>>>>
>>>> Thanks for the review.
>>>>
>>>> Thanks,
>>>> Sourabh Jain
>>>>
>>>>>> #endif
>>>>>> #ifdef ARCH_HAS_KIMAGE_ARCH
>>>>>> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type);
>>>>>> /* List of defined/legal kexec flags */
>>>>>> #ifndef CONFIG_KEXEC_JUMP
>>>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
>>>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT)
>>>>>> #else
>>>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR)
>>>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \
>>>>>> + KEXEC_UPDATE_FDT)
>>>>>> #endif
>>>>>> /* List of defined/legal kexec file flags */
>>>>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
>>>>>> index 01766dd839b0..3d5b3d757bed 100644
>>>>>> --- a/include/uapi/linux/kexec.h
>>>>>> +++ b/include/uapi/linux/kexec.h
>>>>>> @@ -13,6 +13,7 @@
>>>>>> #define KEXEC_ON_CRASH 0x00000001
>>>>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002
>>>>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
>>>>>> +#define KEXEC_UPDATE_FDT 0x00000008
>>>>>> #define KEXEC_ARCH_MASK 0xffff0000
>>>>>> /*
>>>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>>>>> index 8f35a5a42af8..97eb151cd931 100644
>>>>>> --- a/kernel/kexec.c
>>>>>> +++ b/kernel/kexec.c
>>>>>> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>>>> #ifdef CONFIG_CRASH_HOTPLUG
>>>>>> if (flags & KEXEC_UPDATE_ELFCOREHDR)
>>>>>> image->update_elfcorehdr = 1;
>>>>>> + if (flags & KEXEC_UPDATE_FDT)
>>>>>> + image->update_fdt = 1;
>>>>>> #endif
>>>>>> ret = machine_kexec_prepare(image);
>>>>>> --
>>>>>> 2.41.0
>>>>>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 5/6] powerpc: add crash CPU hotplug support
2023-12-11 8:30 ` [PATCH v14 5/6] powerpc: add crash CPU hotplug support Sourabh Jain
@ 2023-12-19 10:35 ` Hari Bathini
0 siblings, 0 replies; 21+ messages in thread
From: Hari Bathini @ 2023-12-19 10:35 UTC (permalink / raw)
To: Sourabh Jain, linuxppc-dev
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, Eric DeVolder,
Boris Ostrovsky, Valentin Schneider, Baoquan He, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Oscar Salvador,
Greg Kroah-Hartman, kexec, Mahesh Salgaonkar, Akhil Raj,
Andrew Morton
Hi Sourabh
On 11/12/23 2:00 pm, Sourabh Jain wrote:
> Due to CPU/Memory hotplug or online/offline events the elfcorehdr
> (which describes the CPUs and memory of the crashed kernel) and FDT
> (Flattened Device Tree) of kdump image becomes outdated. Consequently,
> attempting dump collection with an outdated elfcorehdr or FDT can lead
> to failed or inaccurate dump collection.
>
> Going forward CPU hotplug or online/offlice events are referred as
s/offlice/offline/
> CPU/Memory add/remvoe events.
s/remvoe/remove/
> The current solution to address the above issue involves monitoring the
> CPU/memory add/remove events in userspace using udev rules and whenever
> there are changes in CPU and memory resources, the entire kdump image
> is loaded again. The kdump image includes kernel, initrd, elfcorehdr,
> FDT, purgatory. Given that only elfcorehdr and FDT get outdated due to
> CPU/Memory add/remove events, reloading the entire kdump image is
> inefficient. More importantly, kdump remains inactive for a substantial
> amount of time until the kdump reload completes.
>
> To address the aforementioned issue, commit 247262756121 ("crash: add
> generic infrastructure for crash hotplug support") added a generic
> infrastructure that allows architectures to selectively update the kdump
> image component during CPU or memory add/remove events within the kernel
> itself.
>
> In the event of a CPU or memory add/remove event, the generic crash
> hotplug event handler, `crash_handle_hotplug_event()`, is triggered. It
> then acquires the necessary locks to update the kdump image and invokes
> the architecture-specific crash hotplug handler,
> `arch_crash_handle_hotplug_event()`, to update the required kdump image
> components.
>
> This patch adds crash hotplug handler for PowerPC and enable support to
> update the kdump image on CPU add/remove events. Support for memory
> add/remove events is added in a subsequent patch with the title
> "powerpc: add crash memory hotplug support."
>
> As mentioned earlier, only the elfcorehdr and FDT kdump image components
> need to be updated in the event of CPU or memory add/remove events.
> However, the PowerPC architecture crash hotplug handler only updates the
> FDT to enable crash hotplug support for CPU add/remove events. Here's
> why.
>
> The Elfcorehdr on PowerPC is built with possible CPUs, and thus, it does
> not need an update on CPU add/remove events. On the other hand, the FDT
> needs to be updated on CPU add events to include the newly added CPU. If
> the FDT is not updated and the kernel crashes on a newly added CPU, the
> kdump kernel will fail to boot due to the unavailability of the crashing
> CPU in the FDT. During the early boot, it is expected that the boot CPU
> must be a part of the FDT; otherwise, the kernel will raise a BUG and
> fail to boot. For more information, refer to commit 36ae37e3436b0
> ("powerpc: Make boot_cpuid common between 32 and 64-bit"). Since it is
> okay to have an offline CPU in the kdump FDT, no action is taken in case
> of CPU removal.
>
> There are two system calls, `kexec_file_load` and `kexec_load`, used to
> load the kdump image. Few changes have been made to ensure kernel can
> safely update the kdump FDT for both system calls.
>
> For kexec_file_load syscall the kdump image is prepared in kernel. So to
> support an increasing number of CPUs, the FDT is constructed with extra
> buffer space to ensure it can accommodate a possible number of CPU
> nodes. Additionally, a call to fdt_pack (which trims the unused space
> once the FDT is prepared) is avoided for kdump image loading if this
> feature is enabled.
>
> For the kexec_load syscall, the FDT is updated only if both the
> KEXEC_UPDATE_FDT and KEXEC_UPDATE_ELFCOREHDR kexec flags are passed to
> the kernel by the kexec tool. Passing these flags to the kernel
> indicates that the FDT is built to accommodate possible CPUs, and the
> FDT segment is not considered for SHA calculation, making it safe to
> update the FDT.
>
> Commit 88a6f8994421 ("crash: memory and CPU hotplug sysfs attributes")
> added a sysfs interface to indicate userspace (kdump udev rule) that
> kernel will update the kdump image on CPU hotplug events, so kdump
> reload can be avoided. Implement arch specific function
> `arch_crash_hotplug_cpu_support()` to correctly advertise kernel
> capability to update kdump image.
>
> This feature is advertised to userspace when the following conditions
> are met:
>
> 1. Kdump image is loaded using kexec_file_load system call.
> 2. Kdump image is loaded using kexec_load system and both
> KEXEC_UPATE_ELFCOREHDR and KEXEC_UPDATE_FDT kexec flags are
> passed to kernel.
>
> The changes related to this feature are kept under the CRASH_HOTPLUG
> config, and it is enabled by default.
>
> 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/powerpc/Kconfig | 4 ++
> arch/powerpc/include/asm/kexec.h | 11 ++++
> arch/powerpc/kexec/core_64.c | 91 +++++++++++++++++++++++++++++++
> arch/powerpc/kexec/elf_64.c | 12 +++-
> arch/powerpc/kexec/file_load_64.c | 15 +++++
> 5 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 6f105ee4f3cf..29949c0d301e 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -681,6 +681,10 @@ config RELOCATABLE_TEST
> config ARCH_SUPPORTS_CRASH_DUMP
> def_bool PPC64 || PPC_BOOK3S_32 || PPC_85xx || (44x && !SMP)
>
> +config ARCH_SUPPORTS_CRASH_HOTPLUG
> + def_bool y
> + depends on PPC64
> +
> config ARCH_SELECTS_CRASH_DUMP
> def_bool y
> depends on CRASH_DUMP
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 562e1bb689da..7823ab10d323 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -112,6 +112,17 @@ void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
> struct crash_mem;
> int update_cpus_node(void *fdt);
> int get_crash_memory_ranges(struct crash_mem **mem_ranges);
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +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(struct kimage *image);
> +#define arch_crash_hotplug_cpu_support arch_crash_hotplug_cpu_support
> +#endif
> +
> +#endif /*CONFIG_CRASH_HOTPLUG */
> #endif /* CONFIG_PPC64 */
>
> #ifdef CONFIG_KEXEC_FILE
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 9966b51d9aa8..9932793cd64b 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -543,6 +543,97 @@ int update_cpus_node(void *fdt)
> return ret;
> }
>
> +#ifdef CONFIG_CRASH_HOTPLUG
> +#undef pr_fmt
> +#define pr_fmt(fmt) "crash hp: " fmt
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> + /* Provides the value for the sysfs crash_hotplug nodes */
> +int arch_crash_hotplug_cpu_support(struct kimage *image)
> +{
> + if (image->file_mode)
> + return 1;
> +
> + /*
> + * Crash CPU hotplug support is not available for kdump image loaded
> + * using the kexec_load system and the required attributes are not set.
This comment is confusing. I guess, you mean "Crash CPU hotplug support
is available for kexec_load system call only if the syscall is made
with certain update flags set" or some such..
> + */
> + return image->update_elfcorehdr && image->update_fdt;
> +}
> +#endif
> +
> +/**
> + * arch_crash_handle_hotplug_event - Handle crash CPU/Memory hotplug events to update the
> + * necessary kexec segments based on the hotplug event.
> + * @image: a pointer to kexec_crash_image
> + * @arg: struct memory_notify handler for memory hotplug case and NULL for CPU hotplug case.
> + *
> + * Update the kdump image based on the type of hotplug event, represented by image->hp_action.
> + * CPU addition: Update the FDT segment to include the newly added CPU.
> + * CPU removal: No action is needed, with the assumption that it's okay to have offline CPUs
> + * as part of the FDT.
> + * Memory addition/removal: No action is taken as this is not yet supported.
> + */
> +void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
> +{
> + unsigned int hp_action = image->hp_action;
> +
> + /*
> + * Check if it is safe to update the FDT for the kexec_load syscall.
> + * There's no need to check for the elfcorehdr; the generic crash
> + * hotplug handler does that before calling this function.
> + */
> + if (!(image->file_mode || image->update_fdt))
> + return;
> + /*
> + * Since the hot-unplugged CPU is already part of crash FDT,
> + * no action is needed for CPU remove case.
> + */
> + if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU) {
> + return;
> +
> + } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
> +
> + void *fdt, *ptr;
> + unsigned long mem;
> + int i, fdt_index = -1;
> +
> + /* Find the FDT segment index in kexec segment array. */
> + for (i = 0; i < image->nr_segments; i++) {
> + mem = image->segment[i].mem;
> + ptr = __va(mem);
> +
> + if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
> + fdt_index = i;
> + break;
> + }
> + }
> +
> + if (fdt_index < 0) {
> + pr_err("Unable to locate FDT segment.\n");
> + return;
> + }
> +
> + fdt = __va((void *)image->segment[fdt_index].mem);
> +
> + /* Temporarily invalidate the crash image while it is replaced */
> + xchg(&kexec_crash_image, NULL);
> +
> + /* update FDT to refelect changes in CPU resrouces */
> + if (update_cpus_node(fdt))
> + pr_err("Failed to update crash FDT");
> +
> + /* The crash image is now valid once again */
> + xchg(&kexec_crash_image, image);
> +
> + } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
> + hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> + pr_info_once("Crash update is not supported for memory hotplug\n");
> + return;
> + }
> +}
> +#endif
> +
> #ifdef CONFIG_PPC_64S_HASH_MMU
> /* Values we need to export to the second kernel via the device tree. */
> static __be64 htab_base;
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..b072f589b142 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -30,6 +30,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> unsigned long cmdline_len)
> {
> int ret;
> + bool do_pack_fdt = true;
> unsigned long kernel_load_addr;
> unsigned long initrd_load_addr = 0, fdt_load_addr;
> void *fdt;
> @@ -116,7 +117,16 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> if (ret)
> goto out_free_fdt;
>
> - fdt_pack(fdt);
> +#ifdef CONFIG_CRASH_HOTPLG
Fix the typo (HOTPLG) lest this be dead code..
> + /*
> + * Do not pack FDT, additional space is reserved to accommodate
> + * possible CPU nodes which are not yet present in the system yet.
> + */
> + if (image->type == KEXEC_TYPE_CRASH)
> + do_pack_fdt = false;
> +#endif
Thanks
Hari
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-17 0:59 ` Baoquan He
2023-12-17 15:50 ` Sourabh Jain
@ 2023-12-21 6:06 ` Sourabh Jain
2023-12-22 0:28 ` Baoquan He
1 sibling, 1 reply; 21+ messages in thread
From: Sourabh Jain @ 2023-12-21 6:06 UTC (permalink / raw)
To: Baoquan He
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
Hello Baoquan,
While replying to this email earlier, I mistakenly pressed "Reply to List"
instead of "Reply to All." Consequently, my response was sent only to
powerpc
mailing list.
On 17/12/23 06:29, Baoquan He wrote:
> On 12/17/23 at 12:27am, Sourabh Jain wrote:
>> On 16/12/23 15:11, Baoquan He wrote:
>>> On 12/15/23 at 12:17pm, Sourabh Jain wrote:
>>> ......
>>>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>>>>> index 0f6ea35879ee..bcedb7625b1f 100644
>>>>>> --- a/include/linux/kexec.h
>>>>>> +++ b/include/linux/kexec.h
>>>>>> @@ -319,6 +319,7 @@ struct kimage {
>>>>>> #ifdef CONFIG_CRASH_HOTPLUG
>>>>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */
>>>>>> unsigned int update_elfcorehdr:1;
>>>>>> + unsigned int update_fdt:1;
>>>>> Can we unify this to one flag, e.g hotplug_update?
>>>>>
>>>>> With this, on x86_64, we will skip the sha calculation for elfcorehdr.
>>>>> On ppc, we will skip the sha calculation for elfcorehdr and fdt.
>>>> Yeah, that's what I suggested to Eric. I can do that, but I see one
>>>> problem with powerpc or other platforms that need to skip SHA
>>>> for more kexec segments in addition to elfcorehdr.
>>>>
>>>> `update_elfcorehdr` is set when the kexec tool sends the
>>>> `KEXEC_UPDATE_ELFCOREHDR`
>>>> flag to the kernel for the `kexec_load` system call.
>>>>
>>>> Given that the kexec tool has already been updated to send the
>>>> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from
>>>> SHA verification in generic code, now it would be tricky for architectures
>>>> to
>>>> determine whether kexec has skipped SHA verification for just elfcorehdr
>>>> or all segments needed on the platform with the same flag.
>>> In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr
>>> segment. I am wondering how you skip the fdt segment when calculating
>>> and verifying sha, only saw the update_fdt mark.
>> In the kexec tool where we loop through all the kexec segments to calculate
>> the SHA, there will be a arch call made to determine whether the segment
>> needs
>> to be excluded from SHA or not.
> OK, a arch call will be added to exclude segments in the ARCH. And the
> elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later
> may not want to exclude elfcorehdr.
Yes, Arch can choose which segment to exclude.
>> Now in the arch function if decide a specific segment needs to excluded then
>> corresponding flag is also set by arch function to communicate same with the
>> kernel.
> But I don't see how you exclude elfcorehdr and fdt in kernel for
> kexec_file codes. It's not happening in kexec-tools.
On PowerPC, SHA verification is NOT performed for the kexec_file_load
case; hence, you
won't find any code changes in my patch series to exclude FDT in the
kernel code.
However, let's consider a scenario where it gets added in the future, or
other architectures
need to skip the kexec segment, in addition to elfcorehdr. In that case,
we can use the
same setup as you suggested below. For each kexec segment, there should
be an
architecture-specific function call to decide whether the segment needs
to be excluded or not.
>>> About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro,
>>> but still use the same value, could you think of what problem could be
>>> caused between kernel and kexec-tools utility, the old and new version
>>> compatibility?
>> Just changing the macro name will NOT help because the current kexec tool
>> enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit
>> if
>> the command argument --hotplug is passed to the kexec
>> and
>> the /sys/kernel/crash_elfcorehdr_size file exists in the system.
> As we have discussed, excluding will be done in each ARCH's function
> when doing sha calculation in kexec-tools, isn't it?
>
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index b5393e3b20aa..0095aeec988a 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info)
> continue;
> }
>
> - /* Don't include elfcorehdr in the checksum, if hotplug
> + /* Don't include unwanted segments in the checksum, if hotplug
> * support enabled.
> - */
> - if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) {
> + if (do_hotplug)
> + arch_exclude_segments(info, &i)
> continue;
> }
Yes, something like the above should work.
>> Now, let's say an architecture enables this feature in the kernel with the
>> assumption
>> that the 0x00000004 kexec flag bit is passed from the kexec tool when all
>> the required
>> kexec segments are skipped from SHA calculation. In this case, the current
>> kexec tool,
>> which passes the 0x00000004 kexec flag bit only when the elfcorehdr is
>> skipped, will
>> cause issues for architectures.
>>
>>> If it's about the new header files installed on older kernel, we can
>>> change it like below? Fortunately only one release, 6.6 passed.
>>>
>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
>>> index 3d5b3d757bed..df6a6505e267 100644
>>> --- a/include/uapi/linux/kexec.h
>>> +++ b/include/uapi/linux/kexec.h
>>> @@ -13,7 +13,7 @@
>>> #define KEXEC_ON_CRASH 0x00000001
>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002
>>> -#define KEXEC_UPDATE_FDT 0x00000008
>>> +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
>>> #define KEXEC_ARCH_MASK 0xffff0000
>>> /*
>>>
>>> With my understanding, the kexec flag should be indicating the action,
>>> the mem/cpu hotplug, but not relating to any detail. Imagine later
>>> another segment need be skipped on one ARCH again, then another flag
>>> need be added, this sounds not reasonable.
>> I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag
>> should be sufficient to inform the kernel that the kexec tool has been
>> updated
>> to support CPU/Memory hotplug for the kexec_load system call. Unfortunately,
>> we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE
>> at the moment.
> I am fine with 0x00000008 and a new flag, it has the same effect as
> #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
>
> I am worried about the header file incompatiblity.
If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit
to introduce CPU/Memory hotplug feature for kexec_load syscall, we will
not have
compatibility issue.
Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE
0x00000008
as new flag bit and show how it will be handled. I will also share kexec
code for clarity.
Thanks,
Sourabh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
2023-12-21 6:06 ` Sourabh Jain
@ 2023-12-22 0:28 ` Baoquan He
0 siblings, 0 replies; 21+ messages in thread
From: Baoquan He @ 2023-12-22 0:28 UTC (permalink / raw)
To: Sourabh Jain
Cc: David Hildenbrand, Dave Hansen, Mimi Zohar, linuxppc-dev,
Eric DeVolder, Boris Ostrovsky, Valentin Schneider, x86,
Aneesh Kumar K . V, Laurent Dufour, Dave Young, Vivek Goyal,
Naveen N Rao, Borislav Petkov, Thomas Gleixner, Hari Bathini,
Oscar Salvador, Greg Kroah-Hartman, kexec, Mahesh Salgaonkar,
Akhil Raj, Andrew Morton
On 12/21/23 at 11:36am, Sourabh Jain wrote:
......
> > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
> > > > index 3d5b3d757bed..df6a6505e267 100644
> > > > --- a/include/uapi/linux/kexec.h
> > > > +++ b/include/uapi/linux/kexec.h
> > > > @@ -13,7 +13,7 @@
> > > > #define KEXEC_ON_CRASH 0x00000001
> > > > #define KEXEC_PRESERVE_CONTEXT 0x00000002
> > > > -#define KEXEC_UPDATE_FDT 0x00000008
> > > > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
> > > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004
> > > > #define KEXEC_ARCH_MASK 0xffff0000
> > > > /*
> > > >
> > > > With my understanding, the kexec flag should be indicating the action,
> > > > the mem/cpu hotplug, but not relating to any detail. Imagine later
> > > > another segment need be skipped on one ARCH again, then another flag
> > > > need be added, this sounds not reasonable.
> > > I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag
> > > should be sufficient to inform the kernel that the kexec tool has been
> > > updated
> > > to support CPU/Memory hotplug for the kexec_load system call. Unfortunately,
> > > we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE
> > > at the moment.
> > I am fine with 0x00000008 and a new flag, it has the same effect as
> > #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004
> >
> > I am worried about the header file incompatiblity.
>
> If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit
> to introduce CPU/Memory hotplug feature for kexec_load syscall, we will not
> have
> compatibility issue.
>
> Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE
> 0x00000008
> as new flag bit and show how it will be handled. I will also share kexec
> code for clarity.
It's great we are in the same page about segments excluding done in arch
function. While It's a little unclear to me why we can't reuse 0x00000004
flag value. Then KEXEC_UPDATE_ELFCOREHDR will only exist in v6.6 kernel,
and that bit won't be used in v6.7 and future version.
Except of the existence in kexec-tools utility for XEN, do you see other
barrier? I would like to know so that one day I can explain
KEXEC_UPDATE_ELFCOREHDR to someone else if asked.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-12-22 0:29 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 8:30 [PATCH v14 0/6] powerpc/crash: Kernel handling of CPU and memory hotplug Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 1/6] crash: forward memory_notify arg to arch crash hotplug handler Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 2/6] crash: make CPU and Memory hotplug support reporting flexible Sourabh Jain
2023-12-14 14:13 ` Baoquan He
2023-12-15 5:46 ` Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 3/6] crash: add a new kexec flag for FDT update Sourabh Jain
2023-12-15 2:28 ` Baoquan He
2023-12-15 6:47 ` Sourabh Jain
2023-12-16 9:41 ` Baoquan He
2023-12-16 18:57 ` Sourabh Jain
2023-12-17 0:59 ` Baoquan He
2023-12-17 15:50 ` Sourabh Jain
2023-12-21 6:06 ` Sourabh Jain
2023-12-22 0:28 ` Baoquan He
2023-12-11 8:30 ` [PATCH v14 4/6] powerpc/kexec: turn some static helper functions public Sourabh Jain
2023-12-11 8:30 ` [PATCH v14 5/6] powerpc: add crash CPU hotplug support Sourabh Jain
2023-12-19 10:35 ` Hari Bathini
2023-12-11 8:30 ` [PATCH v14 6/6] powerpc: add crash memory " Sourabh Jain
2023-12-15 1:23 ` Baoquan He
2023-12-15 5:59 ` Sourabh Jain
2023-12-16 3:11 ` Baoquan He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).