[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

This patch series implements the crash hotplug handler on PowerPC introduced
by https://lkml.org/lkml/2022/3/3/674 patch series.


The Problem:
============
Post hotplug/DLPAR events the capture kernel holds stale information about the
system. Dump collection with stale capture kernel might end up in dump capture
failure or an inaccurate dump collection.


Existing solution:
==================
The existing solution to keep the capture kernel up-to-date is observe the
hotplug event via udev rule and trigger a full capture kernel reload post
hotplug event. 

Shortcomings:
------------------------------------------------
- Leaves a window where kernel crash might not lead to successful dump
  collection.
- Reloading all kexec components for each hotplug is inefficient. Since only
  one or two kexec components need to be updated due to hotplug event reloading
  all kexec component is redundant.
- udev rules are prone to races if hotplug events are frequent.

More about issues with an existing solution is posted here:
 - https://lkml.org/lkml/2020/12/14/532
 - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html

Proposed Solution:
==================
Instead of reloading all kexec segments on hotplug event, this patch series
focuses on updating only the relevant kexec segment. Once the kexec
segments are loaded in the kernel reserved area then an arch-specific hotplug handler
will update the relevant kexec segment based on hotplug event type.

As mentioned above this patch series implemented a PowerPC crash hotplug
handler for the CPU. The crash hotplug handler memory is in our TODO list.


A couple of minor changes are required to realize the benefit of the patch
series:

- disalble the udev rule:

  comment out the below line in kdump udev rule file:
  RHEL: /usr/lib/udev/rules.d/98-kexec.rules
  # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"

- kexec tool needs to be updated with patch for kexec_load system call
  to work (not needed if -s option is used during kexec panic load):

---
diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
index 695b8b0..1dc6490 100644
--- a/kexec/arch/ppc64/kexec-elf-ppc64.c
+++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
@@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
 unsigned char reuse_initrd = 0;
 const char *ramdisk;
 
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt) {
+	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+
 int elf_ppc64_probe(const char *buf, off_t len)
 {
 	struct mem_ehdr ehdr;
@@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
 	uint64_t max_addr, hole_addr;
 	char *seg_buf = NULL;
 	off_t seg_size = 0;
+	unsigned int mem_sz = 0;
 	struct mem_phdr *phdr;
 	size_t size;
 #ifdef NEED_RESERVE_DTB
@@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
 	if (result < 0)
 		return result;
 
-	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
+	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
+		fdt_set_totalsize(seg_buf, mem_sz);
+		info->fdt_index = info->nr_segments;
+	}
+
+	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
 				0, 0, max_addr, -1);
 
 #ifdef NEED_RESERVE_DTB
diff --git a/kexec/kexec.c b/kexec/kexec.c
index f63b36b..846b1a8 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
 		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
 			continue;
 		}
+		if (info->fdt_index == i)
+			continue;
+
 		sha256_update(&ctx, info->segment[i].buf,
 			      info->segment[i].bufsz);
 		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 595dd68..0906a1b 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -169,6 +169,7 @@ struct kexec_info {
 	int command_line_len;
 
 	int skip_checks;
+       // Given that we might need to update mutliple kexec segments
+       // then having array to keep indexes of all hotplug kexec segments
+       // will be helpful.
+	unsigned int fdt_index;
 };
 
 struct arch_map_entry {
---

---
Changelog:

v1 -> v2:
  - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
    significant change from v1.

v2 -> v3
  - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
  - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
  - Fixed warning reported by checpatch script
---

Sourabh Jain (5):
  powerpc/kexec: make update_cpus_node non-static
  powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
  powrepc/crash hp: update kimage struct
  powerpc/crash hp: add crash hotplug support for kexec_file_load
  powerpc/crash hp: add crash hotplug support for kexec_load

 arch/powerpc/Kconfig              |  11 +++
 arch/powerpc/include/asm/kexec.h  |   3 +
 arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c       |  40 ++++++++
 arch/powerpc/kexec/file_load_64.c |  87 -----------------
 5 files changed, 207 insertions(+), 87 deletions(-)

-- 
2.35.1

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

This patch series implements the crash hotplug handler on PowerPC introduced
by https://lkml.org/lkml/2022/3/3/674 patch series.


The Problem:
============
Post hotplug/DLPAR events the capture kernel holds stale information about the
system. Dump collection with stale capture kernel might end up in dump capture
failure or an inaccurate dump collection.


Existing solution:
==================
The existing solution to keep the capture kernel up-to-date is observe the
hotplug event via udev rule and trigger a full capture kernel reload post
hotplug event. 

Shortcomings:
------------------------------------------------
- Leaves a window where kernel crash might not lead to successful dump
  collection.
- Reloading all kexec components for each hotplug is inefficient. Since only
  one or two kexec components need to be updated due to hotplug event reloading
  all kexec component is redundant.
- udev rules are prone to races if hotplug events are frequent.

More about issues with an existing solution is posted here:
 - https://lkml.org/lkml/2020/12/14/532
 - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html

Proposed Solution:
==================
Instead of reloading all kexec segments on hotplug event, this patch series
focuses on updating only the relevant kexec segment. Once the kexec
segments are loaded in the kernel reserved area then an arch-specific hotplug handler
will update the relevant kexec segment based on hotplug event type.

As mentioned above this patch series implemented a PowerPC crash hotplug
handler for the CPU. The crash hotplug handler memory is in our TODO list.


A couple of minor changes are required to realize the benefit of the patch
series:

- disalble the udev rule:

  comment out the below line in kdump udev rule file:
  RHEL: /usr/lib/udev/rules.d/98-kexec.rules
  # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"

- kexec tool needs to be updated with patch for kexec_load system call
  to work (not needed if -s option is used during kexec panic load):

---
diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
index 695b8b0..1dc6490 100644
--- a/kexec/arch/ppc64/kexec-elf-ppc64.c
+++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
@@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
 unsigned char reuse_initrd = 0;
 const char *ramdisk;
 
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt) {
+	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+
 int elf_ppc64_probe(const char *buf, off_t len)
 {
 	struct mem_ehdr ehdr;
@@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
 	uint64_t max_addr, hole_addr;
 	char *seg_buf = NULL;
 	off_t seg_size = 0;
+	unsigned int mem_sz = 0;
 	struct mem_phdr *phdr;
 	size_t size;
 #ifdef NEED_RESERVE_DTB
@@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
 	if (result < 0)
 		return result;
 
-	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
+	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
+		fdt_set_totalsize(seg_buf, mem_sz);
+		info->fdt_index = info->nr_segments;
+	}
+
+	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
 				0, 0, max_addr, -1);
 
 #ifdef NEED_RESERVE_DTB
diff --git a/kexec/kexec.c b/kexec/kexec.c
index f63b36b..846b1a8 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
 		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
 			continue;
 		}
+		if (info->fdt_index == i)
+			continue;
+
 		sha256_update(&ctx, info->segment[i].buf,
 			      info->segment[i].bufsz);
 		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 595dd68..0906a1b 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -169,6 +169,7 @@ struct kexec_info {
 	int command_line_len;
 
 	int skip_checks;
+       // Given that we might need to update mutliple kexec segments
+       // then having array to keep indexes of all hotplug kexec segments
+       // will be helpful.
+	unsigned int fdt_index;
 };
 
 struct arch_map_entry {
---

---
Changelog:

v1 -> v2:
  - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
    significant change from v1.

v2 -> v3
  - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
  - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
  - Fixed warning reported by checpatch script
---

Sourabh Jain (5):
  powerpc/kexec: make update_cpus_node non-static
  powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
  powrepc/crash hp: update kimage struct
  powerpc/crash hp: add crash hotplug support for kexec_file_load
  powerpc/crash hp: add crash hotplug support for kexec_load

 arch/powerpc/Kconfig              |  11 +++
 arch/powerpc/include/asm/kexec.h  |   3 +
 arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c       |  40 ++++++++
 arch/powerpc/kexec/file_load_64.c |  87 -----------------
 5 files changed, 207 insertions(+), 87 deletions(-)

-- 
2.35.1

[RFC v3 PATCH 1/5] powerpc/kexec: make update_cpus_node non-static

[Sourabh Jain]

Make the update_cpus_node function non-static and export it for
usage in other kexec components.

The update_cpus_node definition is moved to core_64.c so that it
can be used with both kexec_load and kexec_file_load system calls.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/kexec.h  |  1 +
 arch/powerpc/kexec/core_64.c      | 88 +++++++++++++++++++++++++++++++
 arch/powerpc/kexec/file_load_64.c | 87 ------------------------------
 3 files changed, 89 insertions(+), 87 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 8ebdd23d987c..e1288826e22e 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -127,6 +127,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
 int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
 			unsigned long initrd_load_addr,
 			unsigned long initrd_len, const char *cmdline);
+int update_cpus_node(void *fdt);
 #endif /* CONFIG_PPC64 */
 
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 635b5fc30b53..249d2632526d 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/cpu.h>
 #include <linux/hardirq.h>
+#include <linux/libfdt.h>
 
 #include <asm/page.h>
 #include <asm/current.h>
@@ -378,6 +379,93 @@ void default_machine_kexec(struct kimage *image)
 	/* NOTREACHED */
 }
 
+/**
+ * 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 unsigned long htab_base;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 07da6bf1cf24..57f991b0a9da 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -951,93 +951,6 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
 	return (unsigned int)(usm_entries * sizeof(u64));
 }
 
-/**
- * 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;
-}
-
 /**
  * setup_new_fdt_ppc64 - Update the flattend device-tree of the kernel
  *                       being loaded.
-- 
2.35.1

[RFC v3 PATCH 1/5] powerpc/kexec: make update_cpus_node non-static

[Sourabh Jain]

Make the update_cpus_node function non-static and export it for
usage in other kexec components.

The update_cpus_node definition is moved to core_64.c so that it
can be used with both kexec_load and kexec_file_load system calls.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/kexec.h  |  1 +
 arch/powerpc/kexec/core_64.c      | 88 +++++++++++++++++++++++++++++++
 arch/powerpc/kexec/file_load_64.c | 87 ------------------------------
 3 files changed, 89 insertions(+), 87 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 8ebdd23d987c..e1288826e22e 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -127,6 +127,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
 int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
 			unsigned long initrd_load_addr,
 			unsigned long initrd_len, const char *cmdline);
+int update_cpus_node(void *fdt);
 #endif /* CONFIG_PPC64 */
 
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 635b5fc30b53..249d2632526d 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/cpu.h>
 #include <linux/hardirq.h>
+#include <linux/libfdt.h>
 
 #include <asm/page.h>
 #include <asm/current.h>
@@ -378,6 +379,93 @@ void default_machine_kexec(struct kimage *image)
 	/* NOTREACHED */
 }
 
+/**
+ * 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 unsigned long htab_base;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 07da6bf1cf24..57f991b0a9da 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -951,93 +951,6 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
 	return (unsigned int)(usm_entries * sizeof(u64));
 }
 
-/**
- * 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;
-}
-
 /**
  * setup_new_fdt_ppc64 - Update the flattend device-tree of the kernel
  *                       being loaded.
-- 
2.35.1

[RFC v3 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Sourabh Jain]

The option CRASH_HOTPLUG enables, in kernel update to kexec segments on
hotplug events.

All the updates needed on the capture kernel load path in the kernel for
both kexec_load and kexec_file_load system will be kept under this config.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..b816339ef8c7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -623,6 +623,17 @@ config FA_DUMP
 	  If unsure, say "y". Only special kernels like petitboot may
 	  need to say "N" here.
 
+config CRASH_HOTPLUG
+	bool "kernel updates of crash kexec segments"
+	depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE
+	help
+	  An efficient way to keep the capture kernel up-to-date with CPU
+	  hotplug events. On hotplug event (CPU/memory) the kexec segments
+	  of capture kernel becomes stale and need to be updated with latest
+	  CPU and memory regions. In this method the kernel performs minimal
+	  update to only relevant kexec segments on CPU hotplug event, instead
+	  of triggering full capture reload from userspace using udev rule.
+
 config PRESERVE_FA_DUMP
 	bool "Preserve Firmware-assisted dump"
 	depends on PPC64 && PPC_POWERNV && !FA_DUMP
-- 
2.35.1

[RFC v3 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Sourabh Jain]

The option CRASH_HOTPLUG enables, in kernel update to kexec segments on
hotplug events.

All the updates needed on the capture kernel load path in the kernel for
both kexec_load and kexec_file_load system will be kept under this config.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..b816339ef8c7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -623,6 +623,17 @@ config FA_DUMP
 	  If unsure, say "y". Only special kernels like petitboot may
 	  need to say "N" here.
 
+config CRASH_HOTPLUG
+	bool "kernel updates of crash kexec segments"
+	depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE
+	help
+	  An efficient way to keep the capture kernel up-to-date with CPU
+	  hotplug events. On hotplug event (CPU/memory) the kexec segments
+	  of capture kernel becomes stale and need to be updated with latest
+	  CPU and memory regions. In this method the kernel performs minimal
+	  update to only relevant kexec segments on CPU hotplug event, instead
+	  of triggering full capture reload from userspace using udev rule.
+
 config PRESERVE_FA_DUMP
 	bool "Preserve Firmware-assisted dump"
 	depends on PPC64 && PPC_POWERNV && !FA_DUMP
-- 
2.35.1

[RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Sourabh Jain]

Two new members fdt_index and fdt_index_valid are added in kimage_arch
struct to track the FDT kexec segment. These new members of kimage_arch
struct will help the crash hotplug handler to easily access the FDT
segment from the kexec segment array. Otherwise, we have to loop through
all kexec segments to find the FDT segments.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/kexec.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index e1288826e22e..19c2cab6a880 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
 struct kimage_arch {
 	struct crash_mem *exclude_ranges;
 
+	int fdt_index;
+	bool fdt_index_valid;
 	unsigned long backup_start;
 	void *backup_buf;
 	void *fdt;
-- 
2.35.1

[RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Sourabh Jain]

Two new members fdt_index and fdt_index_valid are added in kimage_arch
struct to track the FDT kexec segment. These new members of kimage_arch
struct will help the crash hotplug handler to easily access the FDT
segment from the kexec segment array. Otherwise, we have to loop through
all kexec segments to find the FDT segments.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/kexec.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index e1288826e22e..19c2cab6a880 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
 struct kimage_arch {
 	struct crash_mem *exclude_ranges;
 
+	int fdt_index;
+	bool fdt_index_valid;
 	unsigned long backup_start;
 	void *backup_buf;
 	void *fdt;
-- 
2.35.1

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Two major changes are done to enable the crash CPU hotplug handler.
Firstly, updated the kexec_load path to prepare kimage for hotplug
changes and secondly, implemented the crash hotplug handler itself.

On the kexec load path, memsz allocation of crash FDT segment is
updated to ensure that it has sufficient buffer space to accommodate
future hot add CPUs. Initialized the kimage members to track the FDT
segment.

The crash hotplug handler updates the cpus node of crash FDT. While
we update crash FDT the kexec_crash_image is marked invalid and restored
after FDT update to avoid race.

Since memory crash hotplug support is not there yet the crash hotplug
handler simply warn the user and return.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 249d2632526d..a470fe6904e3 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
 	return ret;
 }
 
+#ifdef CONFIG_CRASH_HOTPLUG
+/**
+ * arch_crash_hotplug_handler() - Handle hotplug FDT changes
+ * @image: the active struct kimage
+ * @hp_action: the hot un/plug action being handled
+ * @a: first parameter dependent upon hp_action
+ * @b: first parameter dependent upon hp_action
+ *
+ * To accurately reflect CPU hot un/plug changes, the FDT
+ * must be updated with the new list of CPUs and memories.
+ */
+void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
+				unsigned long a, unsigned long b)
+{
+	void *fdt;
+
+	/* No action needed for CPU hot-unplug */
+	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+		return;
+
+	/* crash update on memory hotplug is not support yet */
+	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
+		pr_err("crash hp: crash update is not supported with memory hotplug\n");
+		return;
+	}
+
+	/* Must have valid FDT index */
+	if (!image->arch.fdt_index_valid) {
+		pr_err("crash hp: unable to locate FDT segment");
+		return;
+	}
+
+	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
+
+	/* Temporarily invalidate the crash image while it is replaced */
+	xchg(&kexec_crash_image, NULL);
+
+	/* update FDT to refelect changes to CPU resrouces */
+	if (update_cpus_node(fdt))
+		pr_err("crash hp: failed to update crash FDT");
+
+	/* The crash image is now valid once again */
+	xchg(&kexec_crash_image, image);
+}
+#endif /* CONFIG_CRASH_HOTPLUG */
+
 #ifdef CONFIG_PPC_64S_HASH_MMU
 /* Values we need to export to the second kernel via the device tree. */
 static unsigned long htab_base;
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..2ffe6d69e186 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,6 +24,33 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+
+#ifdef CONFIG_CRASH_HOTPLUG
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt)
+{
+	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+#endif
+
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
 			unsigned long initrd_len, char *cmdline,
@@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	kbuf.buf_align = PAGE_SIZE;
 	kbuf.top_down = true;
 	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+
+#ifdef CONFIG_CRASH_HOTPLUG
+	if (image->type == KEXEC_TYPE_CRASH) {
+		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
+		fdt_set_totalsize(fdt, kbuf.memsz);
+		image->arch.fdt_index = image->nr_segments;
+		image->arch.fdt_index_valid = true;
+	} else
+#endif
+	{
+		kbuf.memsz = fdt_totalsize(fdt);
+	}
+
 	ret = kexec_add_buffer(&kbuf);
 	if (ret)
 		goto out_free_fdt;
-- 
2.35.1

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Two major changes are done to enable the crash CPU hotplug handler.
Firstly, updated the kexec_load path to prepare kimage for hotplug
changes and secondly, implemented the crash hotplug handler itself.

On the kexec load path, memsz allocation of crash FDT segment is
updated to ensure that it has sufficient buffer space to accommodate
future hot add CPUs. Initialized the kimage members to track the FDT
segment.

The crash hotplug handler updates the cpus node of crash FDT. While
we update crash FDT the kexec_crash_image is marked invalid and restored
after FDT update to avoid race.

Since memory crash hotplug support is not there yet the crash hotplug
handler simply warn the user and return.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 249d2632526d..a470fe6904e3 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
 	return ret;
 }
 
+#ifdef CONFIG_CRASH_HOTPLUG
+/**
+ * arch_crash_hotplug_handler() - Handle hotplug FDT changes
+ * @image: the active struct kimage
+ * @hp_action: the hot un/plug action being handled
+ * @a: first parameter dependent upon hp_action
+ * @b: first parameter dependent upon hp_action
+ *
+ * To accurately reflect CPU hot un/plug changes, the FDT
+ * must be updated with the new list of CPUs and memories.
+ */
+void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
+				unsigned long a, unsigned long b)
+{
+	void *fdt;
+
+	/* No action needed for CPU hot-unplug */
+	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+		return;
+
+	/* crash update on memory hotplug is not support yet */
+	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
+		pr_err("crash hp: crash update is not supported with memory hotplug\n");
+		return;
+	}
+
+	/* Must have valid FDT index */
+	if (!image->arch.fdt_index_valid) {
+		pr_err("crash hp: unable to locate FDT segment");
+		return;
+	}
+
+	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
+
+	/* Temporarily invalidate the crash image while it is replaced */
+	xchg(&kexec_crash_image, NULL);
+
+	/* update FDT to refelect changes to CPU resrouces */
+	if (update_cpus_node(fdt))
+		pr_err("crash hp: failed to update crash FDT");
+
+	/* The crash image is now valid once again */
+	xchg(&kexec_crash_image, image);
+}
+#endif /* CONFIG_CRASH_HOTPLUG */
+
 #ifdef CONFIG_PPC_64S_HASH_MMU
 /* Values we need to export to the second kernel via the device tree. */
 static unsigned long htab_base;
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..2ffe6d69e186 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,6 +24,33 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+
+#ifdef CONFIG_CRASH_HOTPLUG
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt)
+{
+	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+#endif
+
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
 			unsigned long initrd_len, char *cmdline,
@@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	kbuf.buf_align = PAGE_SIZE;
 	kbuf.top_down = true;
 	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+
+#ifdef CONFIG_CRASH_HOTPLUG
+	if (image->type == KEXEC_TYPE_CRASH) {
+		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
+		fdt_set_totalsize(fdt, kbuf.memsz);
+		image->arch.fdt_index = image->nr_segments;
+		image->arch.fdt_index_valid = true;
+	} else
+#endif
+	{
+		kbuf.memsz = fdt_totalsize(fdt);
+	}
+
 	ret = kexec_add_buffer(&kbuf);
 	if (ret)
 		goto out_free_fdt;
-- 
2.35.1

[RFC v3 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load

[Sourabh Jain]

The kernel changes needed to add support for crash hotplug support for
kexec_load system calls are similar to kexec_file_load (which has already
been implemented in earlier patches). Since kexec segment array is
prepared by kexec tool in the userspace, the kernel does aware of which
index FDT segment is present.

The only change was done to enabled crash hotplug support for kexec_load is
updated the crash hotplug handler to identify the FDT segment from kexec
segment array.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/kexec/core_64.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index a470fe6904e3..2c248dfb169b 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -480,7 +480,9 @@ int update_cpus_node(void *fdt)
 void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
 				unsigned long a, unsigned long b)
 {
-	void *fdt;
+	void *fdt, *ptr;
+	unsigned int n;
+	unsigned long mem, memsz;
 
 	/* No action needed for CPU hot-unplug */
 	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
@@ -492,6 +494,23 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
 		return;
 	}
 
+	/* Sine kexec segments for kexec_load system call is prepred by
+	 * kexec tool in userspace we need loop through all the segments
+	 * to find out segment index corresponds FDT segment. In case of
+	 * kexec_file_load it is discovered during the load itself.
+	 */
+	if (!image->arch.fdt_index_valid) {
+		for (n = 0; n < image->nr_segments; n++) {
+			mem = image->segment[n].mem;
+			memsz = image->segment[n].memsz;
+			ptr = __va(mem);
+			if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
+				image->arch.fdt_index = n;
+				image->arch.fdt_index_valid = true;
+			}
+		}
+	}
+
 	/* Must have valid FDT index */
 	if (!image->arch.fdt_index_valid) {
 		pr_err("crash hp: unable to locate FDT segment");
-- 
2.35.1

[RFC v3 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load

[Sourabh Jain]

The kernel changes needed to add support for crash hotplug support for
kexec_load system calls are similar to kexec_file_load (which has already
been implemented in earlier patches). Since kexec segment array is
prepared by kexec tool in the userspace, the kernel does aware of which
index FDT segment is present.

The only change was done to enabled crash hotplug support for kexec_load is
updated the crash hotplug handler to identify the FDT segment from kexec
segment array.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/kexec/core_64.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index a470fe6904e3..2c248dfb169b 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -480,7 +480,9 @@ int update_cpus_node(void *fdt)
 void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
 				unsigned long a, unsigned long b)
 {
-	void *fdt;
+	void *fdt, *ptr;
+	unsigned int n;
+	unsigned long mem, memsz;
 
 	/* No action needed for CPU hot-unplug */
 	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
@@ -492,6 +494,23 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
 		return;
 	}
 
+	/* Sine kexec segments for kexec_load system call is prepred by
+	 * kexec tool in userspace we need loop through all the segments
+	 * to find out segment index corresponds FDT segment. In case of
+	 * kexec_file_load it is discovered during the load itself.
+	 */
+	if (!image->arch.fdt_index_valid) {
+		for (n = 0; n < image->nr_segments; n++) {
+			mem = image->segment[n].mem;
+			memsz = image->segment[n].memsz;
+			ptr = __va(mem);
+			if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
+				image->arch.fdt_index = n;
+				image->arch.fdt_index_valid = true;
+			}
+		}
+	}
+
 	/* Must have valid FDT index */
 	if (!image->arch.fdt_index_valid) {
 		pr_err("crash hp: unable to locate FDT segment");
-- 
2.35.1

Re: [RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> This patch series implements the crash hotplug handler on PowerPC introduced
> by https://lkml.org/lkml/2022/3/3/674 patch series.
> 
> 
> The Problem:
> ============
> Post hotplug/DLPAR events the capture kernel holds stale information about the
> system. Dump collection with stale capture kernel might end up in dump capture
> failure or an inaccurate dump collection.
> 
> 
> Existing solution:
> ==================
> The existing solution to keep the capture kernel up-to-date is observe the
> hotplug event via udev rule and trigger a full capture kernel reload post
> hotplug event.
> 
> Shortcomings:
> ------------------------------------------------
> - Leaves a window where kernel crash might not lead to successful dump
>    collection.
> - Reloading all kexec components for each hotplug is inefficient. Since only
>    one or two kexec components need to be updated due to hotplug event reloading
>    all kexec component is redundant.
> - udev rules are prone to races if hotplug events are frequent.
> 
> More about issues with an existing solution is posted here:
>   - https://lkml.org/lkml/2020/12/14/532
>   - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
> 
> Proposed Solution:
> ==================
> Instead of reloading all kexec segments on hotplug event, this patch series
> focuses on updating only the relevant kexec segment. Once the kexec
> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
> will update the relevant kexec segment based on hotplug event type.
> 
> As mentioned above this patch series implemented a PowerPC crash hotplug
> handler for the CPU. The crash hotplug handler memory is in our TODO list.
> 
> 
> A couple of minor changes are required to realize the benefit of the patch
> series:
> 
> - disalble the udev rule:
> 
>    comment out the below line in kdump udev rule file:
fwiw, this will need to be conditionalized on arch, ie to skip for ppc64. I'm doing the same for x86_64.
>    RHEL: /usr/lib/udev/rules.d/98-kexec.rules
>    # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"
> 
> - kexec tool needs to be updated with patch for kexec_load system call
>    to work (not needed if -s option is used during kexec panic load):
> 
> ---
> diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
> index 695b8b0..1dc6490 100644
> --- a/kexec/arch/ppc64/kexec-elf-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
> @@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
>   unsigned char reuse_initrd = 0;
>   const char *ramdisk;
>   
> +#define MAX_CORE 256
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt) {
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +
>   int elf_ppc64_probe(const char *buf, off_t len)
>   {
>   	struct mem_ehdr ehdr;
> @@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>   	uint64_t max_addr, hole_addr;
>   	char *seg_buf = NULL;
>   	off_t seg_size = 0;
> +	unsigned int mem_sz = 0;
>   	struct mem_phdr *phdr;
>   	size_t size;
>   #ifdef NEED_RESERVE_DTB
> @@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>   	if (result < 0)
>   		return result;
>   
> -	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
> +	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
> +		fdt_set_totalsize(seg_buf, mem_sz);
> +		info->fdt_index = info->nr_segments;
> +	}
> +
> +	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
>   				0, 0, max_addr, -1);
>   
>   #ifdef NEED_RESERVE_DTB
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f63b36b..846b1a8 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
>   		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>   			continue;
>   		}
> +		if (info->fdt_index == i)
> +			continue;
> +
>   		sha256_update(&ctx, info->segment[i].buf,
>   			      info->segment[i].bufsz);
>   		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 595dd68..0906a1b 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -169,6 +169,7 @@ struct kexec_info {
>   	int command_line_len;
>   
>   	int skip_checks;
> +       // Given that we might need to update mutliple kexec segments
> +       // then having array to keep indexes of all hotplug kexec segments
> +       // will be helpful.
> +	unsigned int fdt_index;
>   };
>   
>   struct arch_map_entry {
> ---
> 
> ---
> Changelog:
> 
> v1 -> v2:
>    - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
>      significant change from v1.

Generally speaking, this looks good to me!
eric

> 
> v2 -> v3
>    - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
>    - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
>    - Fixed warning reported by checpatch script
> ---
> 

> Sourabh Jain (5):
>    powerpc/kexec: make update_cpus_node non-static
>    powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
>    powrepc/crash hp: update kimage struct
>    powerpc/crash hp: add crash hotplug support for kexec_file_load
>    powerpc/crash hp: add crash hotplug support for kexec_load
> 
>   arch/powerpc/Kconfig              |  11 +++
>   arch/powerpc/include/asm/kexec.h  |   3 +
>   arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
>   arch/powerpc/kexec/elf_64.c       |  40 ++++++++
>   arch/powerpc/kexec/file_load_64.c |  87 -----------------
>   5 files changed, 207 insertions(+), 87 deletions(-)
> 

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> This patch series implements the crash hotplug handler on PowerPC introduced
> by https://lkml.org/lkml/2022/3/3/674 patch series.
> 
> 
> The Problem:
> ============
> Post hotplug/DLPAR events the capture kernel holds stale information about the
> system. Dump collection with stale capture kernel might end up in dump capture
> failure or an inaccurate dump collection.
> 
> 
> Existing solution:
> ==================
> The existing solution to keep the capture kernel up-to-date is observe the
> hotplug event via udev rule and trigger a full capture kernel reload post
> hotplug event.
> 
> Shortcomings:
> ------------------------------------------------
> - Leaves a window where kernel crash might not lead to successful dump
>    collection.
> - Reloading all kexec components for each hotplug is inefficient. Since only
>    one or two kexec components need to be updated due to hotplug event reloading
>    all kexec component is redundant.
> - udev rules are prone to races if hotplug events are frequent.
> 
> More about issues with an existing solution is posted here:
>   - https://lkml.org/lkml/2020/12/14/532
>   - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
> 
> Proposed Solution:
> ==================
> Instead of reloading all kexec segments on hotplug event, this patch series
> focuses on updating only the relevant kexec segment. Once the kexec
> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
> will update the relevant kexec segment based on hotplug event type.
> 
> As mentioned above this patch series implemented a PowerPC crash hotplug
> handler for the CPU. The crash hotplug handler memory is in our TODO list.
> 
> 
> A couple of minor changes are required to realize the benefit of the patch
> series:
> 
> - disalble the udev rule:
> 
>    comment out the below line in kdump udev rule file:
fwiw, this will need to be conditionalized on arch, ie to skip for ppc64. I'm doing the same for x86_64.
>    RHEL: /usr/lib/udev/rules.d/98-kexec.rules
>    # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"
> 
> - kexec tool needs to be updated with patch for kexec_load system call
>    to work (not needed if -s option is used during kexec panic load):
> 
> ---
> diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
> index 695b8b0..1dc6490 100644
> --- a/kexec/arch/ppc64/kexec-elf-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
> @@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
>   unsigned char reuse_initrd = 0;
>   const char *ramdisk;
>   
> +#define MAX_CORE 256
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt) {
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +
>   int elf_ppc64_probe(const char *buf, off_t len)
>   {
>   	struct mem_ehdr ehdr;
> @@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>   	uint64_t max_addr, hole_addr;
>   	char *seg_buf = NULL;
>   	off_t seg_size = 0;
> +	unsigned int mem_sz = 0;
>   	struct mem_phdr *phdr;
>   	size_t size;
>   #ifdef NEED_RESERVE_DTB
> @@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>   	if (result < 0)
>   		return result;
>   
> -	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
> +	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
> +		fdt_set_totalsize(seg_buf, mem_sz);
> +		info->fdt_index = info->nr_segments;
> +	}
> +
> +	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
>   				0, 0, max_addr, -1);
>   
>   #ifdef NEED_RESERVE_DTB
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f63b36b..846b1a8 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
>   		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>   			continue;
>   		}
> +		if (info->fdt_index == i)
> +			continue;
> +
>   		sha256_update(&ctx, info->segment[i].buf,
>   			      info->segment[i].bufsz);
>   		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 595dd68..0906a1b 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -169,6 +169,7 @@ struct kexec_info {
>   	int command_line_len;
>   
>   	int skip_checks;
> +       // Given that we might need to update mutliple kexec segments
> +       // then having array to keep indexes of all hotplug kexec segments
> +       // will be helpful.
> +	unsigned int fdt_index;
>   };
>   
>   struct arch_map_entry {
> ---
> 
> ---
> Changelog:
> 
> v1 -> v2:
>    - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
>      significant change from v1.

Generally speaking, this looks good to me!
eric

> 
> v2 -> v3
>    - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
>    - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
>    - Fixed warning reported by checpatch script
> ---
> 

> Sourabh Jain (5):
>    powerpc/kexec: make update_cpus_node non-static
>    powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
>    powrepc/crash hp: update kimage struct
>    powerpc/crash hp: add crash hotplug support for kexec_file_load
>    powerpc/crash hp: add crash hotplug support for kexec_load
> 
>   arch/powerpc/Kconfig              |  11 +++
>   arch/powerpc/include/asm/kexec.h  |   3 +
>   arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
>   arch/powerpc/kexec/elf_64.c       |  40 ++++++++
>   arch/powerpc/kexec/file_load_64.c |  87 -----------------
>   5 files changed, 207 insertions(+), 87 deletions(-)
> 

Re: [RFC v3 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> The option CRASH_HOTPLUG enables, in kernel update to kexec segments on
> hotplug events.
> 
> All the updates needed on the capture kernel load path in the kernel for
> both kexec_load and kexec_file_load system will be kept under this config.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Reviewed-by: Eric DeVolder <eric.devolder@oracle.com>

> ---
>   arch/powerpc/Kconfig | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b779603978e1..b816339ef8c7 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -623,6 +623,17 @@ config FA_DUMP
>   	  If unsure, say "y". Only special kernels like petitboot may
>   	  need to say "N" here.
>   
> +config CRASH_HOTPLUG
> +	bool "kernel updates of crash kexec segments"
> +	depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE
> +	help
> +	  An efficient way to keep the capture kernel up-to-date with CPU
> +	  hotplug events. On hotplug event (CPU/memory) the kexec segments
> +	  of capture kernel becomes stale and need to be updated with latest
> +	  CPU and memory regions. In this method the kernel performs minimal
> +	  update to only relevant kexec segments on CPU hotplug event, instead
> +	  of triggering full capture reload from userspace using udev rule.
> +
>   config PRESERVE_FA_DUMP
>   	bool "Preserve Firmware-assisted dump"
>   	depends on PPC64 && PPC_POWERNV && !FA_DUMP
> 

[RFC v3 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> The option CRASH_HOTPLUG enables, in kernel update to kexec segments on
> hotplug events.
> 
> All the updates needed on the capture kernel load path in the kernel for
> both kexec_load and kexec_file_load system will be kept under this config.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Reviewed-by: Eric DeVolder <eric.devolder@oracle.com>

> ---
>   arch/powerpc/Kconfig | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b779603978e1..b816339ef8c7 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -623,6 +623,17 @@ config FA_DUMP
>   	  If unsure, say "y". Only special kernels like petitboot may
>   	  need to say "N" here.
>   
> +config CRASH_HOTPLUG
> +	bool "kernel updates of crash kexec segments"
> +	depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE
> +	help
> +	  An efficient way to keep the capture kernel up-to-date with CPU
> +	  hotplug events. On hotplug event (CPU/memory) the kexec segments
> +	  of capture kernel becomes stale and need to be updated with latest
> +	  CPU and memory regions. In this method the kernel performs minimal
> +	  update to only relevant kexec segments on CPU hotplug event, instead
> +	  of triggering full capture reload from userspace using udev rule.
> +
>   config PRESERVE_FA_DUMP
>   	bool "Preserve Firmware-assisted dump"
>   	depends on PPC64 && PPC_POWERNV && !FA_DUMP
> 

Re: [RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> Two new members fdt_index and fdt_index_valid are added in kimage_arch
> struct to track the FDT kexec segment. These new members of kimage_arch
> struct will help the crash hotplug handler to easily access the FDT
> segment from the kexec segment array. Otherwise, we have to loop through
> all kexec segments to find the FDT segments.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>

> ---
>   arch/powerpc/include/asm/kexec.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index e1288826e22e..19c2cab6a880 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
>   struct kimage_arch {
>   	struct crash_mem *exclude_ranges;
>   
> +	int fdt_index;
> +	bool fdt_index_valid;
>   	unsigned long backup_start;
>   	void *backup_buf;
>   	void *fdt;
> 

Question, for the kexec_file_load scenario, is there a need to have the fdt_index segment excluded 
by kexec_calculate_store_digests() ?

[RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Eric DeVolder]

On 3/21/22 03:04, Sourabh Jain wrote:
> Two new members fdt_index and fdt_index_valid are added in kimage_arch
> struct to track the FDT kexec segment. These new members of kimage_arch
> struct will help the crash hotplug handler to easily access the FDT
> segment from the kexec segment array. Otherwise, we have to loop through
> all kexec segments to find the FDT segments.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>

> ---
>   arch/powerpc/include/asm/kexec.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index e1288826e22e..19c2cab6a880 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
>   struct kimage_arch {
>   	struct crash_mem *exclude_ranges;
>   
> +	int fdt_index;
> +	bool fdt_index_valid;
>   	unsigned long backup_start;
>   	void *backup_buf;
>   	void *fdt;
> 

Question, for the kexec_file_load scenario, is there a need to have the fdt_index segment excluded 
by kexec_calculate_store_digests() ?

Re: [RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Eric DeVolder]

Notes below.
eric

On 3/21/22 03:04, Sourabh Jain wrote:
> Two major changes are done to enable the crash CPU hotplug handler.
> Firstly, updated the kexec_load path to prepare kimage for hotplug
> changes and secondly, implemented the crash hotplug handler itself.
> 
> On the kexec load path, memsz allocation of crash FDT segment is
> updated to ensure that it has sufficient buffer space to accommodate
> future hot add CPUs. Initialized the kimage members to track the FDT
> segment.
> 
> The crash hotplug handler updates the cpus node of crash FDT. While
> we update crash FDT the kexec_crash_image is marked invalid and restored
> after FDT update to avoid race.
> 
> Since memory crash hotplug support is not there yet the crash hotplug
> handler simply warn the user and return.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>   arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>   2 files changed, 86 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..a470fe6904e3 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>   	return ret;
>   }
>   
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
> + * @image: the active struct kimage
> + * @hp_action: the hot un/plug action being handled
> + * @a: first parameter dependent upon hp_action
> + * @b: first parameter dependent upon hp_action
> + *
> + * To accurately reflect CPU hot un/plug changes, the FDT
> + * must be updated with the new list of CPUs and memories.
> + */
> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
> +				unsigned long a, unsigned long b)
> +{
> +	void *fdt;
> +
> +	/* No action needed for CPU hot-unplug */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> +		return;
Just curious why no action is needed on cpu remove?

> +
> +	/* crash update on memory hotplug is not support yet */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
> +		return;
> +	}
> +
> +	/* Must have valid FDT index */
> +	if (!image->arch.fdt_index_valid) {
> +		pr_err("crash hp: unable to locate FDT segment");
> +		return;
> +	}
> +
> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
> +
> +	/* Temporarily invalidate the crash image while it is replaced */
> +	xchg(&kexec_crash_image, NULL);
> +
> +	/* update FDT to refelect changes to CPU resrouces */
> +	if (update_cpus_node(fdt))
> +		pr_err("crash hp: failed to update crash FDT");
> +
> +	/* The crash image is now valid once again */
> +	xchg(&kexec_crash_image, image);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>   #ifdef CONFIG_PPC_64S_HASH_MMU
>   /* Values we need to export to the second kernel via the device tree. */
>   static unsigned long htab_base;
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..2ffe6d69e186 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,33 @@
>   #include <linux/slab.h>
>   #include <linux/types.h>
>   
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +#define MAX_CORE 256
Is there a better config option to tie this value too?
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +#endif
> +
>   static void *elf64_load(struct kimage *image, char *kernel_buf,
>   			unsigned long kernel_len, char *initrd,
>   			unsigned long initrd_len, char *cmdline,
> @@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	kbuf.buf_align = PAGE_SIZE;
>   	kbuf.top_down = true;
>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	if (image->type == KEXEC_TYPE_CRASH) {
> +		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
> +		fdt_set_totalsize(fdt, kbuf.memsz);
> +		image->arch.fdt_index = image->nr_segments;
> +		image->arch.fdt_index_valid = true;
> +	} else
> +#endif
> +	{
> +		kbuf.memsz = fdt_totalsize(fdt);
> +	}
> +
>   	ret = kexec_add_buffer(&kbuf);
>   	if (ret)
>   		goto out_free_fdt;
> 

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Eric DeVolder]

Notes below.
eric

On 3/21/22 03:04, Sourabh Jain wrote:
> Two major changes are done to enable the crash CPU hotplug handler.
> Firstly, updated the kexec_load path to prepare kimage for hotplug
> changes and secondly, implemented the crash hotplug handler itself.
> 
> On the kexec load path, memsz allocation of crash FDT segment is
> updated to ensure that it has sufficient buffer space to accommodate
> future hot add CPUs. Initialized the kimage members to track the FDT
> segment.
> 
> The crash hotplug handler updates the cpus node of crash FDT. While
> we update crash FDT the kexec_crash_image is marked invalid and restored
> after FDT update to avoid race.
> 
> Since memory crash hotplug support is not there yet the crash hotplug
> handler simply warn the user and return.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>   arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>   2 files changed, 86 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..a470fe6904e3 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>   	return ret;
>   }
>   
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
> + * @image: the active struct kimage
> + * @hp_action: the hot un/plug action being handled
> + * @a: first parameter dependent upon hp_action
> + * @b: first parameter dependent upon hp_action
> + *
> + * To accurately reflect CPU hot un/plug changes, the FDT
> + * must be updated with the new list of CPUs and memories.
> + */
> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
> +				unsigned long a, unsigned long b)
> +{
> +	void *fdt;
> +
> +	/* No action needed for CPU hot-unplug */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> +		return;
Just curious why no action is needed on cpu remove?

> +
> +	/* crash update on memory hotplug is not support yet */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
> +		return;
> +	}
> +
> +	/* Must have valid FDT index */
> +	if (!image->arch.fdt_index_valid) {
> +		pr_err("crash hp: unable to locate FDT segment");
> +		return;
> +	}
> +
> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
> +
> +	/* Temporarily invalidate the crash image while it is replaced */
> +	xchg(&kexec_crash_image, NULL);
> +
> +	/* update FDT to refelect changes to CPU resrouces */
> +	if (update_cpus_node(fdt))
> +		pr_err("crash hp: failed to update crash FDT");
> +
> +	/* The crash image is now valid once again */
> +	xchg(&kexec_crash_image, image);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>   #ifdef CONFIG_PPC_64S_HASH_MMU
>   /* Values we need to export to the second kernel via the device tree. */
>   static unsigned long htab_base;
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..2ffe6d69e186 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,33 @@
>   #include <linux/slab.h>
>   #include <linux/types.h>
>   
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +#define MAX_CORE 256
Is there a better config option to tie this value too?
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +#endif
> +
>   static void *elf64_load(struct kimage *image, char *kernel_buf,
>   			unsigned long kernel_len, char *initrd,
>   			unsigned long initrd_len, char *cmdline,
> @@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	kbuf.buf_align = PAGE_SIZE;
>   	kbuf.top_down = true;
>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	if (image->type == KEXEC_TYPE_CRASH) {
> +		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
> +		fdt_set_totalsize(fdt, kbuf.memsz);
> +		image->arch.fdt_index = image->nr_segments;
> +		image->arch.fdt_index_valid = true;
> +	} else
> +#endif
> +	{
> +		kbuf.memsz = fdt_totalsize(fdt);
> +	}
> +
>   	ret = kexec_add_buffer(&kbuf);
>   	if (ret)
>   		goto out_free_fdt;
> 

Re: [RFC v3 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load

[Eric DeVolder]

Some minor nits below.
eric

On 3/21/22 03:04, Sourabh Jain wrote:
> The kernel changes needed to add support for crash hotplug support for
> kexec_load system calls are similar to kexec_file_load (which has already
> been implemented in earlier patches). Since kexec segment array is
> prepared by kexec tool in the userspace, the kernel does aware of which
s/kernel does aware/ kernel is not aware/ ?

> index FDT segment is present.
> 
> The only change was done to enabled crash hotplug support for kexec_load is
s/was done to enabled/to enable/ ?
> updated the crash hotplug handler to identify the FDT segment from kexec
s/updated/ to update/ ?
> segment array.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   arch/powerpc/kexec/core_64.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index a470fe6904e3..2c248dfb169b 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -480,7 +480,9 @@ int update_cpus_node(void *fdt)
>   void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>   				unsigned long a, unsigned long b)
>   {
> -	void *fdt;
> +	void *fdt, *ptr;
> +	unsigned int n;
> +	unsigned long mem, memsz;
>   
>   	/* No action needed for CPU hot-unplug */
>   	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> @@ -492,6 +494,23 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>   		return;
>   	}
>   
> +	/* Sine kexec segments for kexec_load system call is prepred by
s/Sine/Since/
s/is prepred/are prepared/
> +	 * kexec tool in userspace we need loop through all the segments
> +	 * to find out segment index corresponds FDT segment. In case of
> +	 * kexec_file_load it is discovered during the load itself.
> +	 */
> +	if (!image->arch.fdt_index_valid) {
> +		for (n = 0; n < image->nr_segments; n++) {
> +			mem = image->segment[n].mem;
> +			memsz = image->segment[n].memsz;
> +			ptr = __va(mem);
> +			if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
> +				image->arch.fdt_index = n;
> +				image->arch.fdt_index_valid = true;
How about adding a break; statement to early exit?
> +			}
> +		}
> +	}
> +
>   	/* Must have valid FDT index */
>   	if (!image->arch.fdt_index_valid) {
>   		pr_err("crash hp: unable to locate FDT segment");
> 

[RFC v3 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load

[Eric DeVolder]

Some minor nits below.
eric

On 3/21/22 03:04, Sourabh Jain wrote:
> The kernel changes needed to add support for crash hotplug support for
> kexec_load system calls are similar to kexec_file_load (which has already
> been implemented in earlier patches). Since kexec segment array is
> prepared by kexec tool in the userspace, the kernel does aware of which
s/kernel does aware/ kernel is not aware/ ?

> index FDT segment is present.
> 
> The only change was done to enabled crash hotplug support for kexec_load is
s/was done to enabled/to enable/ ?
> updated the crash hotplug handler to identify the FDT segment from kexec
s/updated/ to update/ ?
> segment array.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   arch/powerpc/kexec/core_64.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index a470fe6904e3..2c248dfb169b 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -480,7 +480,9 @@ int update_cpus_node(void *fdt)
>   void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>   				unsigned long a, unsigned long b)
>   {
> -	void *fdt;
> +	void *fdt, *ptr;
> +	unsigned int n;
> +	unsigned long mem, memsz;
>   
>   	/* No action needed for CPU hot-unplug */
>   	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> @@ -492,6 +494,23 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>   		return;
>   	}
>   
> +	/* Sine kexec segments for kexec_load system call is prepred by
s/Sine/Since/
s/is prepred/are prepared/
> +	 * kexec tool in userspace we need loop through all the segments
> +	 * to find out segment index corresponds FDT segment. In case of
> +	 * kexec_file_load it is discovered during the load itself.
> +	 */
> +	if (!image->arch.fdt_index_valid) {
> +		for (n = 0; n < image->nr_segments; n++) {
> +			mem = image->segment[n].mem;
> +			memsz = image->segment[n].memsz;
> +			ptr = __va(mem);
> +			if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
> +				image->arch.fdt_index = n;
> +				image->arch.fdt_index_valid = true;
How about adding a break; statement to early exit?
> +			}
> +		}
> +	}
> +
>   	/* Must have valid FDT index */
>   	if (!image->arch.fdt_index_valid) {
>   		pr_err("crash hp: unable to locate FDT segment");
> 

Re: [RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Sourabh Jain]

Hello Eric,

On 24/03/22 00:02, Eric DeVolder wrote:
>
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> Two new members fdt_index and fdt_index_valid are added in kimage_arch
>> struct to track the FDT kexec segment. These new members of kimage_arch
>> struct will help the crash hotplug handler to easily access the FDT
>> segment from the kexec segment array. Otherwise, we have to loop through
>> all kexec segments to find the FDT segments.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>
>> ---
>>   arch/powerpc/include/asm/kexec.h | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/kexec.h 
>> b/arch/powerpc/include/asm/kexec.h
>> index e1288826e22e..19c2cab6a880 100644
>> --- a/arch/powerpc/include/asm/kexec.h
>> +++ b/arch/powerpc/include/asm/kexec.h
>> @@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
>>   struct kimage_arch {
>>       struct crash_mem *exclude_ranges;
>>   +    int fdt_index;
>> +    bool fdt_index_valid;
>>       unsigned long backup_start;
>>       void *backup_buf;
>>       void *fdt;
>>
>
> Question, for the kexec_file_load scenario, is there a need to have 
> the fdt_index segment excluded by kexec_calculate_store_digests() ?

On PowerPC, SHA verification is not done for kexec_file_load system 
call, so we might not need to exclude the FDT segment.

Thanks for the review.

- Sourabh Jain

[RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct

[Sourabh Jain]

Hello Eric,

On 24/03/22 00:02, Eric DeVolder wrote:
>
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> Two new members fdt_index and fdt_index_valid are added in kimage_arch
>> struct to track the FDT kexec segment. These new members of kimage_arch
>> struct will help the crash hotplug handler to easily access the FDT
>> segment from the kexec segment array. Otherwise, we have to loop through
>> all kexec segments to find the FDT segments.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>
>> ---
>> ? arch/powerpc/include/asm/kexec.h | 2 ++
>> ? 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/kexec.h 
>> b/arch/powerpc/include/asm/kexec.h
>> index e1288826e22e..19c2cab6a880 100644
>> --- a/arch/powerpc/include/asm/kexec.h
>> +++ b/arch/powerpc/include/asm/kexec.h
>> @@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
>> ? struct kimage_arch {
>> ????? struct crash_mem *exclude_ranges;
>> ? +??? int fdt_index;
>> +??? bool fdt_index_valid;
>> ????? unsigned long backup_start;
>> ????? void *backup_buf;
>> ????? void *fdt;
>>
>
> Question, for the kexec_file_load scenario, is there a need to have 
> the fdt_index segment excluded by kexec_calculate_store_digests() ?

On PowerPC, SHA verification is not done for kexec_file_load system 
call, so we might not need to exclude the FDT segment.

Thanks for the review.

- Sourabh Jain

Re: [RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

On 24/03/22 00:02, Eric DeVolder wrote:
>
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> This patch series implements the crash hotplug handler on PowerPC 
>> introduced
>> by https://lkml.org/lkml/2022/3/3/674 patch series.
>>
>>
>> The Problem:
>> ============
>> Post hotplug/DLPAR events the capture kernel holds stale information 
>> about the
>> system. Dump collection with stale capture kernel might end up in 
>> dump capture
>> failure or an inaccurate dump collection.
>>
>>
>> Existing solution:
>> ==================
>> The existing solution to keep the capture kernel up-to-date is 
>> observe the
>> hotplug event via udev rule and trigger a full capture kernel reload 
>> post
>> hotplug event.
>>
>> Shortcomings:
>> ------------------------------------------------
>> - Leaves a window where kernel crash might not lead to successful dump
>>    collection.
>> - Reloading all kexec components for each hotplug is inefficient. 
>> Since only
>>    one or two kexec components need to be updated due to hotplug 
>> event reloading
>>    all kexec component is redundant.
>> - udev rules are prone to races if hotplug events are frequent.
>>
>> More about issues with an existing solution is posted here:
>>   - https://lkml.org/lkml/2020/12/14/532
>>   - 
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
>>
>> Proposed Solution:
>> ==================
>> Instead of reloading all kexec segments on hotplug event, this patch 
>> series
>> focuses on updating only the relevant kexec segment. Once the kexec
>> segments are loaded in the kernel reserved area then an arch-specific 
>> hotplug handler
>> will update the relevant kexec segment based on hotplug event type.
>>
>> As mentioned above this patch series implemented a PowerPC crash hotplug
>> handler for the CPU. The crash hotplug handler memory is in our TODO 
>> list.
>>
>>
>> A couple of minor changes are required to realize the benefit of the 
>> patch
>> series:
>>
>> - disalble the udev rule:
>>
>>    comment out the below line in kdump udev rule file:
> fwiw, this will need to be conditionalized on arch, ie to skip for 
> ppc64. I'm doing the same for x86_64.


I think kexec-tools maintains a separate udev rule file for PowerPC. I 
will ensure CPU rules gets removed from kdump udev rules once this 
feature is upstream.

Thanks,
- Sourabh Jain

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

On 24/03/22 00:02, Eric DeVolder wrote:
>
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> This patch series implements the crash hotplug handler on PowerPC 
>> introduced
>> by https://lkml.org/lkml/2022/3/3/674 patch series.
>>
>>
>> The Problem:
>> ============
>> Post hotplug/DLPAR events the capture kernel holds stale information 
>> about the
>> system. Dump collection with stale capture kernel might end up in 
>> dump capture
>> failure or an inaccurate dump collection.
>>
>>
>> Existing solution:
>> ==================
>> The existing solution to keep the capture kernel up-to-date is 
>> observe the
>> hotplug event via udev rule and trigger a full capture kernel reload 
>> post
>> hotplug event.
>>
>> Shortcomings:
>> ------------------------------------------------
>> - Leaves a window where kernel crash might not lead to successful dump
>> ?? collection.
>> - Reloading all kexec components for each hotplug is inefficient. 
>> Since only
>> ?? one or two kexec components need to be updated due to hotplug 
>> event reloading
>> ?? all kexec component is redundant.
>> - udev rules are prone to races if hotplug events are frequent.
>>
>> More about issues with an existing solution is posted here:
>> ? - https://lkml.org/lkml/2020/12/14/532
>> ? - 
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
>>
>> Proposed Solution:
>> ==================
>> Instead of reloading all kexec segments on hotplug event, this patch 
>> series
>> focuses on updating only the relevant kexec segment. Once the kexec
>> segments are loaded in the kernel reserved area then an arch-specific 
>> hotplug handler
>> will update the relevant kexec segment based on hotplug event type.
>>
>> As mentioned above this patch series implemented a PowerPC crash hotplug
>> handler for the CPU. The crash hotplug handler memory is in our TODO 
>> list.
>>
>>
>> A couple of minor changes are required to realize the benefit of the 
>> patch
>> series:
>>
>> - disalble the udev rule:
>>
>> ?? comment out the below line in kdump udev rule file:
> fwiw, this will need to be conditionalized on arch, ie to skip for 
> ppc64. I'm doing the same for x86_64.


I think kexec-tools maintains a separate udev rule file for PowerPC. I 
will ensure CPU rules gets removed from kdump udev rules once this 
feature is upstream.

Thanks,
- Sourabh Jain

Re: [RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Hello Eric,

On 24/03/22 00:02, Eric DeVolder wrote:
> Notes below.
> eric
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> Two major changes are done to enable the crash CPU hotplug handler.
>> Firstly, updated the kexec_load path to prepare kimage for hotplug
>> changes and secondly, implemented the crash hotplug handler itself.
>>
>> On the kexec load path, memsz allocation of crash FDT segment is
>> updated to ensure that it has sufficient buffer space to accommodate
>> future hot add CPUs. Initialized the kimage members to track the FDT
>> segment.
>>
>> The crash hotplug handler updates the cpus node of crash FDT. While
>> we update crash FDT the kexec_crash_image is marked invalid and restored
>> after FDT update to avoid race.
>>
>> Since memory crash hotplug support is not there yet the crash hotplug
>> handler simply warn the user and return.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>>   arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>>   2 files changed, 86 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..a470fe6904e3 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>>       return ret;
>>   }
>>   +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
>> + * @image: the active struct kimage
>> + * @hp_action: the hot un/plug action being handled
>> + * @a: first parameter dependent upon hp_action
>> + * @b: first parameter dependent upon hp_action
>> + *
>> + * To accurately reflect CPU hot un/plug changes, the FDT
>> + * must be updated with the new list of CPUs and memories.
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int 
>> hp_action,
>> +                unsigned long a, unsigned long b)
>> +{
>> +    void *fdt;
>> +
>> +    /* No action needed for CPU hot-unplug */
>> +    if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
>> +        return;
> Just curious why no action is needed on cpu remove?


Since CPU note addresses are already available for all possible CPUs 
(regardless they are online or not) the
PHDR is allocated for all possible CPUs during elfcorehdr creation. At 
least for the kexec_load system call.

Now on the crash path, the crashing CPU initiates an IPI call to update 
the CPU data of all online CPUs and
jumps to the purgatory. Hence no action is needed for the remove case.

With the above logic, we shouldn't be taking any action for the CPU add 
case too, right? But on PowerPC early
boot path there is validation that checks the boot CPU is part of the 
Flattened Device Tree (FDT) or not. If the
boot CPU is not found in FDT the boot fails. Hence FDT needs to be 
updated for every new CPU added to the
system.

>
>> +
>> +    /* crash update on memory hotplug is not support yet */
>> +    if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == 
>> KEXEC_CRASH_HP_ADD_MEMORY) {
>> +        pr_err("crash hp: crash update is not supported with memory 
>> hotplug\n");
>> +        return;
>> +    }
>> +
>> +    /* Must have valid FDT index */
>> +    if (!image->arch.fdt_index_valid) {
>> +        pr_err("crash hp: unable to locate FDT segment");
>> +        return;
>> +    }
>> +
>> +    fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
>> +
>> +    /* Temporarily invalidate the crash image while it is replaced */
>> +    xchg(&kexec_crash_image, NULL);
>> +
>> +    /* update FDT to refelect changes to CPU resrouces */
>> +    if (update_cpus_node(fdt))
>> +        pr_err("crash hp: failed to update crash FDT");
>> +
>> +    /* The crash image is now valid once again */
>> +    xchg(&kexec_crash_image, image);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>>   #ifdef CONFIG_PPC_64S_HASH_MMU
>>   /* Values we need to export to the second kernel via the device 
>> tree. */
>>   static unsigned long htab_base;
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index eeb258002d1e..2ffe6d69e186 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,33 @@
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +#define MAX_CORE 256
> Is there a better config option to tie this value too?

The above #defines are just placeholders. Eventually, we will replace 
the MAX_CORE with possible CPUs to calculate the FDT size. Maybe in the 
next version, the above #define will be removed.

>> +#define PER_CORE_NODE_SIZE 1500
>> +
>>

Thanks for the review Eric.

- Sourabh Jain

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Hello Eric,

On 24/03/22 00:02, Eric DeVolder wrote:
> Notes below.
> eric
>
> On 3/21/22 03:04, Sourabh Jain wrote:
>> Two major changes are done to enable the crash CPU hotplug handler.
>> Firstly, updated the kexec_load path to prepare kimage for hotplug
>> changes and secondly, implemented the crash hotplug handler itself.
>>
>> On the kexec load path, memsz allocation of crash FDT segment is
>> updated to ensure that it has sufficient buffer space to accommodate
>> future hot add CPUs. Initialized the kimage members to track the FDT
>> segment.
>>
>> The crash hotplug handler updates the cpus node of crash FDT. While
>> we update crash FDT the kexec_crash_image is marked invalid and restored
>> after FDT update to avoid race.
>>
>> Since memory crash hotplug support is not there yet the crash hotplug
>> handler simply warn the user and return.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>> ? arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>> ? arch/powerpc/kexec/elf_64.c? | 40 +++++++++++++++++++++++++++++++
>> ? 2 files changed, 86 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..a470fe6904e3 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>> ????? return ret;
>> ? }
>> ? +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
>> + * @image: the active struct kimage
>> + * @hp_action: the hot un/plug action being handled
>> + * @a: first parameter dependent upon hp_action
>> + * @b: first parameter dependent upon hp_action
>> + *
>> + * To accurately reflect CPU hot un/plug changes, the FDT
>> + * must be updated with the new list of CPUs and memories.
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int 
>> hp_action,
>> +??????????????? unsigned long a, unsigned long b)
>> +{
>> +??? void *fdt;
>> +
>> +??? /* No action needed for CPU hot-unplug */
>> +??? if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
>> +??????? return;
> Just curious why no action is needed on cpu remove?


Since CPU note addresses are already available for all possible CPUs 
(regardless they are online or not) the
PHDR is allocated for all possible CPUs during elfcorehdr creation. At 
least for the kexec_load system call.

Now on the crash path, the crashing CPU initiates an IPI call to update 
the CPU data of all online CPUs and
jumps to the purgatory. Hence no action is needed for the remove case.

With the above logic, we shouldn't be taking any action for the CPU add 
case too, right? But on PowerPC early
boot path there is validation that checks the boot CPU is part of the 
Flattened Device Tree (FDT) or not. If the
boot CPU is not found in FDT the boot fails. Hence FDT needs to be 
updated for every new CPU added to the
system.

>
>> +
>> +??? /* crash update on memory hotplug is not support yet */
>> +??? if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == 
>> KEXEC_CRASH_HP_ADD_MEMORY) {
>> +??????? pr_err("crash hp: crash update is not supported with memory 
>> hotplug\n");
>> +??????? return;
>> +??? }
>> +
>> +??? /* Must have valid FDT index */
>> +??? if (!image->arch.fdt_index_valid) {
>> +??????? pr_err("crash hp: unable to locate FDT segment");
>> +??????? return;
>> +??? }
>> +
>> +??? fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
>> +
>> +??? /* Temporarily invalidate the crash image while it is replaced */
>> +??? xchg(&kexec_crash_image, NULL);
>> +
>> +??? /* update FDT to refelect changes to CPU resrouces */
>> +??? if (update_cpus_node(fdt))
>> +??????? pr_err("crash hp: failed to update crash FDT");
>> +
>> +??? /* The crash image is now valid once again */
>> +??? xchg(&kexec_crash_image, image);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>> ? #ifdef CONFIG_PPC_64S_HASH_MMU
>> ? /* Values we need to export to the second kernel via the device 
>> tree. */
>> ? static unsigned long htab_base;
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index eeb258002d1e..2ffe6d69e186 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,33 @@
>> ? #include <linux/slab.h>
>> ? #include <linux/types.h>
>> ? +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +#define MAX_CORE 256
> Is there a better config option to tie this value too?

The above #defines are just placeholders. Eventually, we will replace 
the MAX_CORE with possible CPUs to calculate the FDT size. Maybe in the 
next version, the above #define will be removed.

>> +#define PER_CORE_NODE_SIZE 1500
>> +
>>

Thanks for the review Eric.

- Sourabh Jain

Re: [RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Laurent Dufour]

On 21/03/2022, 09:04:17, Sourabh Jain wrote:
> This patch series implements the crash hotplug handler on PowerPC introduced
> by https://lkml.org/lkml/2022/3/3/674 patch series.

Hi Sourabh,

That's a great idea!

> 
> The Problem:
> ============
> Post hotplug/DLPAR events the capture kernel holds stale information about the
> system. Dump collection with stale capture kernel might end up in dump capture
> failure or an inaccurate dump collection.
> 
> 
> Existing solution:
> ==================
> The existing solution to keep the capture kernel up-to-date is observe the
> hotplug event via udev rule and trigger a full capture kernel reload post
> hotplug event. 
> 
> Shortcomings:
> ------------------------------------------------
> - Leaves a window where kernel crash might not lead to successful dump
>   collection.
> - Reloading all kexec components for each hotplug is inefficient. Since only
>   one or two kexec components need to be updated due to hotplug event reloading
>   all kexec component is redundant.
> - udev rules are prone to races if hotplug events are frequent.
> 
> More about issues with an existing solution is posted here:
>  - https://lkml.org/lkml/2020/12/14/532
>  - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
> 
> Proposed Solution:
> ==================
> Instead of reloading all kexec segments on hotplug event, this patch series
> focuses on updating only the relevant kexec segment. Once the kexec
> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
> will update the relevant kexec segment based on hotplug event type.
> 
> As mentioned above this patch series implemented a PowerPC crash hotplug
> handler for the CPU. The crash hotplug handler memory is in our TODO list.

If I understand corrrectly, and based on the change in the patch 4/5,
memory hotplug operations are ignored. Does this means that once this
series is applied, the capture kenrel will not be able to work correctly on
this hot plug/unplugged memory areas?

Thanks,
Laurent.
> 
> A couple of minor changes are required to realize the benefit of the patch
> series:
> 
> - disalble the udev rule:
> 
>   comment out the below line in kdump udev rule file:
>   RHEL: /usr/lib/udev/rules.d/98-kexec.rules
>   # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"
> 
> - kexec tool needs to be updated with patch for kexec_load system call
>   to work (not needed if -s option is used during kexec panic load):
> 
> ---
> diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
> index 695b8b0..1dc6490 100644
> --- a/kexec/arch/ppc64/kexec-elf-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
> @@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
>  unsigned char reuse_initrd = 0;
>  const char *ramdisk;
>  
> +#define MAX_CORE 256
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt) {
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +
>  int elf_ppc64_probe(const char *buf, off_t len)
>  {
>  	struct mem_ehdr ehdr;
> @@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>  	uint64_t max_addr, hole_addr;
>  	char *seg_buf = NULL;
>  	off_t seg_size = 0;
> +	unsigned int mem_sz = 0;
>  	struct mem_phdr *phdr;
>  	size_t size;
>  #ifdef NEED_RESERVE_DTB
> @@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>  	if (result < 0)
>  		return result;
>  
> -	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
> +	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
> +		fdt_set_totalsize(seg_buf, mem_sz);
> +		info->fdt_index = info->nr_segments;
> +	}
> +
> +	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
>  				0, 0, max_addr, -1);
>  
>  #ifdef NEED_RESERVE_DTB
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f63b36b..846b1a8 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
>  		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>  			continue;
>  		}
> +		if (info->fdt_index == i)
> +			continue;
> +
>  		sha256_update(&ctx, info->segment[i].buf,
>  			      info->segment[i].bufsz);
>  		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 595dd68..0906a1b 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -169,6 +169,7 @@ struct kexec_info {
>  	int command_line_len;
>  
>  	int skip_checks;
> +       // Given that we might need to update mutliple kexec segments
> +       // then having array to keep indexes of all hotplug kexec segments
> +       // will be helpful.
> +	unsigned int fdt_index;
>  };
>  
>  struct arch_map_entry {
> ---
> 
> ---
> Changelog:
> 
> v1 -> v2:
>   - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
>     significant change from v1.
> 
> v2 -> v3
>   - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
>   - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
>   - Fixed warning reported by checpatch script
> ---
> 
> Sourabh Jain (5):
>   powerpc/kexec: make update_cpus_node non-static
>   powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
>   powrepc/crash hp: update kimage struct
>   powerpc/crash hp: add crash hotplug support for kexec_file_load
>   powerpc/crash hp: add crash hotplug support for kexec_load
> 
>  arch/powerpc/Kconfig              |  11 +++
>  arch/powerpc/include/asm/kexec.h  |   3 +
>  arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c       |  40 ++++++++
>  arch/powerpc/kexec/file_load_64.c |  87 -----------------
>  5 files changed, 207 insertions(+), 87 deletions(-)
> 

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Laurent Dufour]

On 21/03/2022, 09:04:17, Sourabh Jain wrote:
> This patch series implements the crash hotplug handler on PowerPC introduced
> by https://lkml.org/lkml/2022/3/3/674 patch series.

Hi Sourabh,

That's a great idea!

> 
> The Problem:
> ============
> Post hotplug/DLPAR events the capture kernel holds stale information about the
> system. Dump collection with stale capture kernel might end up in dump capture
> failure or an inaccurate dump collection.
> 
> 
> Existing solution:
> ==================
> The existing solution to keep the capture kernel up-to-date is observe the
> hotplug event via udev rule and trigger a full capture kernel reload post
> hotplug event. 
> 
> Shortcomings:
> ------------------------------------------------
> - Leaves a window where kernel crash might not lead to successful dump
>   collection.
> - Reloading all kexec components for each hotplug is inefficient. Since only
>   one or two kexec components need to be updated due to hotplug event reloading
>   all kexec component is redundant.
> - udev rules are prone to races if hotplug events are frequent.
> 
> More about issues with an existing solution is posted here:
>  - https://lkml.org/lkml/2020/12/14/532
>  - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
> 
> Proposed Solution:
> ==================
> Instead of reloading all kexec segments on hotplug event, this patch series
> focuses on updating only the relevant kexec segment. Once the kexec
> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
> will update the relevant kexec segment based on hotplug event type.
> 
> As mentioned above this patch series implemented a PowerPC crash hotplug
> handler for the CPU. The crash hotplug handler memory is in our TODO list.

If I understand corrrectly, and based on the change in the patch 4/5,
memory hotplug operations are ignored. Does this means that once this
series is applied, the capture kenrel will not be able to work correctly on
this hot plug/unplugged memory areas?

Thanks,
Laurent.
> 
> A couple of minor changes are required to realize the benefit of the patch
> series:
> 
> - disalble the udev rule:
> 
>   comment out the below line in kdump udev rule file:
>   RHEL: /usr/lib/udev/rules.d/98-kexec.rules
>   # SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"
> 
> - kexec tool needs to be updated with patch for kexec_load system call
>   to work (not needed if -s option is used during kexec panic load):
> 
> ---
> diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
> index 695b8b0..1dc6490 100644
> --- a/kexec/arch/ppc64/kexec-elf-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
> @@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
>  unsigned char reuse_initrd = 0;
>  const char *ramdisk;
>  
> +#define MAX_CORE 256
> +#define PER_CORE_NODE_SIZE 1500
> +
> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt) {
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> +}
> +
>  int elf_ppc64_probe(const char *buf, off_t len)
>  {
>  	struct mem_ehdr ehdr;
> @@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>  	uint64_t max_addr, hole_addr;
>  	char *seg_buf = NULL;
>  	off_t seg_size = 0;
> +	unsigned int mem_sz = 0;
>  	struct mem_phdr *phdr;
>  	size_t size;
>  #ifdef NEED_RESERVE_DTB
> @@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>  	if (result < 0)
>  		return result;
>  
> -	my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
> +	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
> +		fdt_set_totalsize(seg_buf, mem_sz);
> +		info->fdt_index = info->nr_segments;
> +	}
> +
> +	my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
>  				0, 0, max_addr, -1);
>  
>  #ifdef NEED_RESERVE_DTB
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f63b36b..846b1a8 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
>  		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>  			continue;
>  		}
> +		if (info->fdt_index == i)
> +			continue;
> +
>  		sha256_update(&ctx, info->segment[i].buf,
>  			      info->segment[i].bufsz);
>  		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 595dd68..0906a1b 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -169,6 +169,7 @@ struct kexec_info {
>  	int command_line_len;
>  
>  	int skip_checks;
> +       // Given that we might need to update mutliple kexec segments
> +       // then having array to keep indexes of all hotplug kexec segments
> +       // will be helpful.
> +	unsigned int fdt_index;
>  };
>  
>  struct arch_map_entry {
> ---
> 
> ---
> Changelog:
> 
> v1 -> v2:
>   - Use generic hotplug handler introduced by https://lkml.org/lkml/2022/2/9/1406, a
>     significant change from v1.
> 
> v2 -> v3
>   - Move fdt_index and fdt_index_vaild variables to kimage_arch struct.
>   - Rebase patche on top of https://lkml.org/lkml/2022/3/3/674 [v5]
>   - Fixed warning reported by checpatch script
> ---
> 
> Sourabh Jain (5):
>   powerpc/kexec: make update_cpus_node non-static
>   powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
>   powrepc/crash hp: update kimage struct
>   powerpc/crash hp: add crash hotplug support for kexec_file_load
>   powerpc/crash hp: add crash hotplug support for kexec_load
> 
>  arch/powerpc/Kconfig              |  11 +++
>  arch/powerpc/include/asm/kexec.h  |   3 +
>  arch/powerpc/kexec/core_64.c      | 153 ++++++++++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c       |  40 ++++++++
>  arch/powerpc/kexec/file_load_64.c |  87 -----------------
>  5 files changed, 207 insertions(+), 87 deletions(-)
> 

Re: [RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Laurent Dufour]

On 21/03/2022, 09:04:21, Sourabh Jain wrote:
> Two major changes are done to enable the crash CPU hotplug handler.
> Firstly, updated the kexec_load path to prepare kimage for hotplug
> changes and secondly, implemented the crash hotplug handler itself.
> 
> On the kexec load path, memsz allocation of crash FDT segment is
> updated to ensure that it has sufficient buffer space to accommodate
> future hot add CPUs. Initialized the kimage members to track the FDT
> segment.
> 
> The crash hotplug handler updates the cpus node of crash FDT. While
> we update crash FDT the kexec_crash_image is marked invalid and restored
> after FDT update to avoid race.
> 
> Since memory crash hotplug support is not there yet the crash hotplug
> handler simply warn the user and return.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..a470fe6904e3 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
> + * @image: the active struct kimage
> + * @hp_action: the hot un/plug action being handled
> + * @a: first parameter dependent upon hp_action
> + * @b: first parameter dependent upon hp_action
> + *
> + * To accurately reflect CPU hot un/plug changes, the FDT
> + * must be updated with the new list of CPUs and memories.
> + */
> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
> +				unsigned long a, unsigned long b)
> +{
> +	void *fdt;
> +
> +	/* No action needed for CPU hot-unplug */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> +		return;
> +
> +	/* crash update on memory hotplug is not support yet */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
		
May be pr_info_once() that's not really an error ?

> +		return;
> +	}
> +
> +	/* Must have valid FDT index */
> +	if (!image->arch.fdt_index_valid) {
> +		pr_err("crash hp: unable to locate FDT segment");
> +		return;
> +	}
> +
> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
> +
> +	/* Temporarily invalidate the crash image while it is replaced */
> +	xchg(&kexec_crash_image, NULL);
> +
> +	/* update FDT to refelect changes to CPU resrouces */
> +	if (update_cpus_node(fdt))
> +		pr_err("crash hp: failed to update crash FDT");
> +
> +	/* The crash image is now valid once again */
> +	xchg(&kexec_crash_image, image);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>  #ifdef CONFIG_PPC_64S_HASH_MMU
>  /* Values we need to export to the second kernel via the device tree. */
>  static unsigned long htab_base;
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..2ffe6d69e186 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,33 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +#define MAX_CORE 256

Why not computing something based on nr_cpus_ids and threads_per_core instead?

> +#define PER_CORE_NODE_SIZE 1500

Is that size function of threadsd_per_core too?

> +> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
              kernel
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.

See above.

> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);

I guess fdt_totalsize() is already taken in account the online CPUs, isn't it?
If that's the case, you should add the remaining needed part only.

> +}
> +#endif
> +
>  static void *elf64_load(struct kimage *image, char *kernel_buf,
>  			unsigned long kernel_len, char *initrd,
>  			unsigned long initrd_len, char *cmdline,
> @@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  	kbuf.buf_align = PAGE_SIZE;
>  	kbuf.top_down = true;
>  	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	if (image->type == KEXEC_TYPE_CRASH) {
> +		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
> +		fdt_set_totalsize(fdt, kbuf.memsz);
> +		image->arch.fdt_index = image->nr_segments;
> +		image->arch.fdt_index_valid = true;
> +	} else
> +#endif
> +	{
> +		kbuf.memsz = fdt_totalsize(fdt);
> +	}
> +
>  	ret = kexec_add_buffer(&kbuf);
>  	if (ret)
>  		goto out_free_fdt;

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Laurent Dufour]

On 21/03/2022, 09:04:21, Sourabh Jain wrote:
> Two major changes are done to enable the crash CPU hotplug handler.
> Firstly, updated the kexec_load path to prepare kimage for hotplug
> changes and secondly, implemented the crash hotplug handler itself.
> 
> On the kexec load path, memsz allocation of crash FDT segment is
> updated to ensure that it has sufficient buffer space to accommodate
> future hot add CPUs. Initialized the kimage members to track the FDT
> segment.
> 
> The crash hotplug handler updates the cpus node of crash FDT. While
> we update crash FDT the kexec_crash_image is marked invalid and restored
> after FDT update to avoid race.
> 
> Since memory crash hotplug support is not there yet the crash hotplug
> handler simply warn the user and return.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..a470fe6904e3 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
> + * @image: the active struct kimage
> + * @hp_action: the hot un/plug action being handled
> + * @a: first parameter dependent upon hp_action
> + * @b: first parameter dependent upon hp_action
> + *
> + * To accurately reflect CPU hot un/plug changes, the FDT
> + * must be updated with the new list of CPUs and memories.
> + */
> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
> +				unsigned long a, unsigned long b)
> +{
> +	void *fdt;
> +
> +	/* No action needed for CPU hot-unplug */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
> +		return;
> +
> +	/* crash update on memory hotplug is not support yet */
> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
		
May be pr_info_once() that's not really an error ?

> +		return;
> +	}
> +
> +	/* Must have valid FDT index */
> +	if (!image->arch.fdt_index_valid) {
> +		pr_err("crash hp: unable to locate FDT segment");
> +		return;
> +	}
> +
> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
> +
> +	/* Temporarily invalidate the crash image while it is replaced */
> +	xchg(&kexec_crash_image, NULL);
> +
> +	/* update FDT to refelect changes to CPU resrouces */
> +	if (update_cpus_node(fdt))
> +		pr_err("crash hp: failed to update crash FDT");
> +
> +	/* The crash image is now valid once again */
> +	xchg(&kexec_crash_image, image);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>  #ifdef CONFIG_PPC_64S_HASH_MMU
>  /* Values we need to export to the second kernel via the device tree. */
>  static unsigned long htab_base;
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..2ffe6d69e186 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,33 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +#define MAX_CORE 256

Why not computing something based on nr_cpus_ids and threads_per_core instead?

> +#define PER_CORE_NODE_SIZE 1500

Is that size function of threadsd_per_core too?

> +> +/**
> + * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
> + * @fdt: pointer to crash kernel FDT
> + *
> + * Calculate the buffer space needed to add more CPU nodes in FDT after
> + * capture kenrel load due to hot add events.
              kernel
> + *
> + * Some assumption are made to calculate the additional buffer size needed
> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
> + * in the system would not go beyond MAX_CORE and memory needed to store per core
> + * date in FDT is PER_CORE_NODE_SIZE.
> + *
> + * Certainly MAX_CORE count can be replaced with possible core count and
> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
> + * core size value on Power9 LPAR.

See above.

> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);

I guess fdt_totalsize() is already taken in account the online CPUs, isn't it?
If that's the case, you should add the remaining needed part only.

> +}
> +#endif
> +
>  static void *elf64_load(struct kimage *image, char *kernel_buf,
>  			unsigned long kernel_len, char *initrd,
>  			unsigned long initrd_len, char *cmdline,
> @@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  	kbuf.buf_align = PAGE_SIZE;
>  	kbuf.top_down = true;
>  	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	if (image->type == KEXEC_TYPE_CRASH) {
> +		kbuf.memsz = get_crash_fdt_mem_sz(fdt);
> +		fdt_set_totalsize(fdt, kbuf.memsz);
> +		image->arch.fdt_index = image->nr_segments;
> +		image->arch.fdt_index_valid = true;
> +	} else
> +#endif
> +	{
> +		kbuf.memsz = fdt_totalsize(fdt);
> +	}
> +
>  	ret = kexec_add_buffer(&kbuf);
>  	if (ret)
>  		goto out_free_fdt;

Re: [RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Hi Laurent,

On 25/03/22 23:33, Laurent Dufour wrote:
> On 21/03/2022, 09:04:21, Sourabh Jain wrote:
>> Two major changes are done to enable the crash CPU hotplug handler.
>> Firstly, updated the kexec_load path to prepare kimage for hotplug
>> changes and secondly, implemented the crash hotplug handler itself.
>>
>> On the kexec load path, memsz allocation of crash FDT segment is
>> updated to ensure that it has sufficient buffer space to accommodate
>> future hot add CPUs. Initialized the kimage members to track the FDT
>> segment.
>>
>> The crash hotplug handler updates the cpus node of crash FDT. While
>> we update crash FDT the kexec_crash_image is marked invalid and restored
>> after FDT update to avoid race.
>>
>> Since memory crash hotplug support is not there yet the crash hotplug
>> handler simply warn the user and return.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>>   arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>>   2 files changed, 86 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..a470fe6904e3 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>>   	return ret;
>>   }
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
>> + * @image: the active struct kimage
>> + * @hp_action: the hot un/plug action being handled
>> + * @a: first parameter dependent upon hp_action
>> + * @b: first parameter dependent upon hp_action
>> + *
>> + * To accurately reflect CPU hot un/plug changes, the FDT
>> + * must be updated with the new list of CPUs and memories.
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>> +				unsigned long a, unsigned long b)
>> +{
>> +	void *fdt;
>> +
>> +	/* No action needed for CPU hot-unplug */
>> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
>> +		return;
>> +
>> +	/* crash update on memory hotplug is not support yet */
>> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
>> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
> 		
> May be pr_info_once() that's not really an error ?


Ye not an error.

>
>> +		return;
>> +	}
>> +
>> +	/* Must have valid FDT index */
>> +	if (!image->arch.fdt_index_valid) {
>> +		pr_err("crash hp: unable to locate FDT segment");
>> +		return;
>> +	}
>> +
>> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
>> +
>> +	/* Temporarily invalidate the crash image while it is replaced */
>> +	xchg(&kexec_crash_image, NULL);
>> +
>> +	/* update FDT to refelect changes to CPU resrouces */
>> +	if (update_cpus_node(fdt))
>> +		pr_err("crash hp: failed to update crash FDT");
>> +
>> +	/* The crash image is now valid once again */
>> +	xchg(&kexec_crash_image, image);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>>   #ifdef CONFIG_PPC_64S_HASH_MMU
>>   /* Values we need to export to the second kernel via the device tree. */
>>   static unsigned long htab_base;
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index eeb258002d1e..2ffe6d69e186 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,33 @@
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +#define MAX_CORE 256
> Why not computing something based on nr_cpus_ids and threads_per_core instead?


Yes in the next version I will change this to nr_cpus.

>
>> +#define PER_CORE_NODE_SIZE 1500
> Is that size function of threadsd_per_core too?


It is per core basis. I noticed that the size of FDT get increased by
approx 1.5 KB after adding a core.

In next version this patch series, we will be computing FDT buffer
size needed of hotplug CPUs based on the existing core size.
While loading the kdump kernel we will go through all attributes
of the currently online CPU and use them to calculate the buffer
space needed for hotplug CPUs.

>> + *
>> + * Some assumption are made to calculate the additional buffer size needed
>> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
>> + * in the system would not go beyond MAX_CORE and memory needed to store per core
>> + * date in FDT is PER_CORE_NODE_SIZE.
>> + *
>> + * Certainly MAX_CORE count can be replaced with possible core count and
>> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
>> + * core size value on Power9 LPAR.
> See above.
>
>> + */
>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>> +{
>> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> I guess fdt_totalsize() is already taken in account the online CPUs, isn't it?
> If that's the case, you should add the remaining needed part only.

Agree, I will take of that in next series.


Thanks for the review.

- Sourabh Jain

[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

Hi Laurent,

On 25/03/22 23:33, Laurent Dufour wrote:
> On 21/03/2022, 09:04:21, Sourabh Jain wrote:
>> Two major changes are done to enable the crash CPU hotplug handler.
>> Firstly, updated the kexec_load path to prepare kimage for hotplug
>> changes and secondly, implemented the crash hotplug handler itself.
>>
>> On the kexec load path, memsz allocation of crash FDT segment is
>> updated to ensure that it has sufficient buffer space to accommodate
>> future hot add CPUs. Initialized the kimage members to track the FDT
>> segment.
>>
>> The crash hotplug handler updates the cpus node of crash FDT. While
>> we update crash FDT the kexec_crash_image is marked invalid and restored
>> after FDT update to avoid race.
>>
>> Since memory crash hotplug support is not there yet the crash hotplug
>> handler simply warn the user and return.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++++++++++++++++
>>   arch/powerpc/kexec/elf_64.c  | 40 +++++++++++++++++++++++++++++++
>>   2 files changed, 86 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..a470fe6904e3 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
>>   	return ret;
>>   }
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug FDT changes
>> + * @image: the active struct kimage
>> + * @hp_action: the hot un/plug action being handled
>> + * @a: first parameter dependent upon hp_action
>> + * @b: first parameter dependent upon hp_action
>> + *
>> + * To accurately reflect CPU hot un/plug changes, the FDT
>> + * must be updated with the new list of CPUs and memories.
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>> +				unsigned long a, unsigned long b)
>> +{
>> +	void *fdt;
>> +
>> +	/* No action needed for CPU hot-unplug */
>> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
>> +		return;
>> +
>> +	/* crash update on memory hotplug is not support yet */
>> +	if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
>> +		pr_err("crash hp: crash update is not supported with memory hotplug\n");
> 		
> May be pr_info_once() that's not really an error ?


Ye not an error.

>
>> +		return;
>> +	}
>> +
>> +	/* Must have valid FDT index */
>> +	if (!image->arch.fdt_index_valid) {
>> +		pr_err("crash hp: unable to locate FDT segment");
>> +		return;
>> +	}
>> +
>> +	fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
>> +
>> +	/* Temporarily invalidate the crash image while it is replaced */
>> +	xchg(&kexec_crash_image, NULL);
>> +
>> +	/* update FDT to refelect changes to CPU resrouces */
>> +	if (update_cpus_node(fdt))
>> +		pr_err("crash hp: failed to update crash FDT");
>> +
>> +	/* The crash image is now valid once again */
>> +	xchg(&kexec_crash_image, image);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>>   #ifdef CONFIG_PPC_64S_HASH_MMU
>>   /* Values we need to export to the second kernel via the device tree. */
>>   static unsigned long htab_base;
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index eeb258002d1e..2ffe6d69e186 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,33 @@
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +#define MAX_CORE 256
> Why not computing something based on nr_cpus_ids and threads_per_core instead?


Yes in the next version I will change this to nr_cpus.

>
>> +#define PER_CORE_NODE_SIZE 1500
> Is that size function of threadsd_per_core too?


It is per core basis. I noticed that the size of FDT get increased by
approx 1.5 KB after adding a core.

In next version this patch series, we will be computing FDT buffer
size needed of hotplug CPUs based on the existing core size.
While loading the kdump kernel we will go through all attributes
of the currently online CPU and use them to calculate the buffer
space needed for hotplug CPUs.

>> + *
>> + * Some assumption are made to calculate the additional buffer size needed
>> + * to accommodate future hot add CPUs to the crash FDT. The maximum core count
>> + * in the system would not go beyond MAX_CORE and memory needed to store per core
>> + * date in FDT is PER_CORE_NODE_SIZE.
>> + *
>> + * Certainly MAX_CORE count can be replaced with possible core count and
>> + * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
>> + * core size value on Power9 LPAR.
> See above.
>
>> + */
>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>> +{
>> +	return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
> I guess fdt_totalsize() is already taken in account the online CPUs, isn't it?
> If that's the case, you should add the remaining needed part only.

Agree, I will take of that in next series.


Thanks for the review.

- Sourabh Jain

Re: [RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

On 25/03/22 22:34, Laurent Dufour wrote:
> On 21/03/2022, 09:04:17, Sourabh Jain wrote:
>> This patch series implements the crash hotplug handler on PowerPC introduced
>> by https://lkml.org/lkml/2022/3/3/674 patch series.
> Hi Sourabh,
>
> That's a great idea!
>
>> The Problem:
>> ============
>> Post hotplug/DLPAR events the capture kernel holds stale information about the
>> system. Dump collection with stale capture kernel might end up in dump capture
>> failure or an inaccurate dump collection.
>>
>>
>> Existing solution:
>> ==================
>> The existing solution to keep the capture kernel up-to-date is observe the
>> hotplug event via udev rule and trigger a full capture kernel reload post
>> hotplug event.
>>
>> Shortcomings:
>> ------------------------------------------------
>> - Leaves a window where kernel crash might not lead to successful dump
>>    collection.
>> - Reloading all kexec components for each hotplug is inefficient. Since only
>>    one or two kexec components need to be updated due to hotplug event reloading
>>    all kexec component is redundant.
>> - udev rules are prone to races if hotplug events are frequent.
>>
>> More about issues with an existing solution is posted here:
>>   - https://lkml.org/lkml/2020/12/14/532
>>   - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
>>
>> Proposed Solution:
>> ==================
>> Instead of reloading all kexec segments on hotplug event, this patch series
>> focuses on updating only the relevant kexec segment. Once the kexec
>> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
>> will update the relevant kexec segment based on hotplug event type.
>>
>> As mentioned above this patch series implemented a PowerPC crash hotplug
>> handler for the CPU. The crash hotplug handler memory is in our TODO list.
> If I understand corrrectly, and based on the change in the patch 4/5,
> memory hotplug operations are ignored. Does this means that once this
> series is applied, the capture kenrel will not be able to work correctly on
> this hot plug/unplugged memory areas?
It will work because we will not remove the kdump udev rule to restart the
kdump service on memory hotplug until that feature is available in the 
kernel.


Thanks,
Sourabh Jain

[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel

[Sourabh Jain]

On 25/03/22 22:34, Laurent Dufour wrote:
> On 21/03/2022, 09:04:17, Sourabh Jain wrote:
>> This patch series implements the crash hotplug handler on PowerPC introduced
>> by https://lkml.org/lkml/2022/3/3/674 patch series.
> Hi Sourabh,
>
> That's a great idea!
>
>> The Problem:
>> ============
>> Post hotplug/DLPAR events the capture kernel holds stale information about the
>> system. Dump collection with stale capture kernel might end up in dump capture
>> failure or an inaccurate dump collection.
>>
>>
>> Existing solution:
>> ==================
>> The existing solution to keep the capture kernel up-to-date is observe the
>> hotplug event via udev rule and trigger a full capture kernel reload post
>> hotplug event.
>>
>> Shortcomings:
>> ------------------------------------------------
>> - Leaves a window where kernel crash might not lead to successful dump
>>    collection.
>> - Reloading all kexec components for each hotplug is inefficient. Since only
>>    one or two kexec components need to be updated due to hotplug event reloading
>>    all kexec component is redundant.
>> - udev rules are prone to races if hotplug events are frequent.
>>
>> More about issues with an existing solution is posted here:
>>   - https://lkml.org/lkml/2020/12/14/532
>>   - https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
>>
>> Proposed Solution:
>> ==================
>> Instead of reloading all kexec segments on hotplug event, this patch series
>> focuses on updating only the relevant kexec segment. Once the kexec
>> segments are loaded in the kernel reserved area then an arch-specific hotplug handler
>> will update the relevant kexec segment based on hotplug event type.
>>
>> As mentioned above this patch series implemented a PowerPC crash hotplug
>> handler for the CPU. The crash hotplug handler memory is in our TODO list.
> If I understand corrrectly, and based on the change in the patch 4/5,
> memory hotplug operations are ignored. Does this means that once this
> series is applied, the capture kenrel will not be able to work correctly on
> this hot plug/unplugged memory areas?
It will work because we will not remove the kdump udev rule to restart the
kdump service on memory hotplug until that feature is available in the 
kernel.


Thanks,
Sourabh Jain