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

[RFC v4 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

v3 -> v4:
  - Update the logic to find the additional space needed for hotadd CPUs post
    kexec load. Refer "[RFC v4 PATCH 4/5] powerpc/crash hp: add crash hotplug
    support for kexec_file_load" patch to know more about the change.
  - Fix a couple of typo.
  - Replace pr_err to pr_info_once to warn user about memory hotplug
    support.
  - In crash hotplug handle exit the for loop if FDT segment is found.
---

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_arch 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      | 154 ++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c       |  74 ++++++++++++++
 arch/powerpc/kexec/file_load_64.c |  87 -----------------
 5 files changed, 242 insertions(+), 87 deletions(-)

-- 
2.35.1

[RFC v4 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

v3 -> v4:
  - Update the logic to find the additional space needed for hotadd CPUs post
    kexec load. Refer "[RFC v4 PATCH 4/5] powerpc/crash hp: add crash hotplug
    support for kexec_file_load" patch to know more about the change.
  - Fix a couple of typo.
  - Replace pr_err to pr_info_once to warn user about memory hotplug
    support.
  - In crash hotplug handle exit the for loop if FDT segment is found.
---

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_arch 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      | 154 ++++++++++++++++++++++++++++++
 arch/powerpc/kexec/elf_64.c       |  74 ++++++++++++++
 arch/powerpc/kexec/file_load_64.c |  87 -----------------
 5 files changed, 242 insertions(+), 87 deletions(-)

-- 
2.35.1

[RFC v4 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 v4 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 v4 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>
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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
+	  kernel becomes stale and need to be updated with latest CPU data.
+	  In this method the kernel performs minimal update to only relevant
+	  kexec segments on CPU hotplug event, instead of triggering full
+	  capture kernel 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 v4 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>
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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
+	  kernel becomes stale and need to be updated with latest CPU data.
+	  In this method the kernel performs minimal update to only relevant
+	  kexec segments on CPU hotplug event, instead of triggering full
+	  capture kernel 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 v4 PATCH 3/5] powrepc/crash hp: update kimage_arch 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 v4 PATCH 3/5] powrepc/crash hp: update kimage_arch 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 v4 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.

On the kexec load path, the memsz allocation of the 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 kexec
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
the handler simply warns the user and returns.

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

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 249d2632526d..62f77cc86407 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_info_once("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..9dc774548ce4 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,6 +24,67 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include <asm/kvm_book3s.h>
+#include <asm/kvm_ppc.h>
+
+#ifdef CONFIG_CRASH_HOTPLUG
+
+/**
+ * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
+ *                     in FDT. The calculation is done based on the existing CPU
+ *                     node in unflatten device tree. Loop through all the
+ *                     properties of the very first CPU type device node found in
+ *                     unflatten device tree and returns the sum of the property
+ *                     length and property string size of all properties of a CPU
+ *                     node.
+ */
+static int get_cpu_node_sz(void) {
+	struct device_node *dn = NULL;
+	struct property *pp;
+	int cpu_node_size = 0;
+
+	dn = of_find_node_by_type(NULL, "cpu");
+
+	if (!dn) {
+		pr_warn("Unable to locate cpu device_type node.\n");
+		goto out;
+	}
+
+	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
+	 * FDT_END_NODE that takes one byte each.
+	 */
+	cpu_node_size = 2;
+
+	for_each_property_of_node(dn, pp) {
+		/* For each property add two bytes extra. One for string null
+		 * character for property name and other for FDT property start
+		 * tag FDT_PROP.
+		 */
+		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
+	}
+
+out:
+	return cpu_node_size;
+}
+
+/**
+ * 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 crash FDT
+ * post capture kenrel load due to CPU hotplug events.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt)
+{
+	int fdt_cpu_nodes_sz, offline_cpu_cnt;
+
+	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
+	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
+
+	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
+}
+#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 +184,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 v4 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.

On the kexec load path, the memsz allocation of the 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 kexec
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
the handler simply warns the user and returns.

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

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 249d2632526d..62f77cc86407 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_info_once("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..9dc774548ce4 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,6 +24,67 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include <asm/kvm_book3s.h>
+#include <asm/kvm_ppc.h>
+
+#ifdef CONFIG_CRASH_HOTPLUG
+
+/**
+ * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
+ *                     in FDT. The calculation is done based on the existing CPU
+ *                     node in unflatten device tree. Loop through all the
+ *                     properties of the very first CPU type device node found in
+ *                     unflatten device tree and returns the sum of the property
+ *                     length and property string size of all properties of a CPU
+ *                     node.
+ */
+static int get_cpu_node_sz(void) {
+	struct device_node *dn = NULL;
+	struct property *pp;
+	int cpu_node_size = 0;
+
+	dn = of_find_node_by_type(NULL, "cpu");
+
+	if (!dn) {
+		pr_warn("Unable to locate cpu device_type node.\n");
+		goto out;
+	}
+
+	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
+	 * FDT_END_NODE that takes one byte each.
+	 */
+	cpu_node_size = 2;
+
+	for_each_property_of_node(dn, pp) {
+		/* For each property add two bytes extra. One for string null
+		 * character for property name and other for FDT property start
+		 * tag FDT_PROP.
+		 */
+		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
+	}
+
+out:
+	return cpu_node_size;
+}
+
+/**
+ * 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 crash FDT
+ * post capture kenrel load due to CPU hotplug events.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt)
+{
+	int fdt_cpu_nodes_sz, offline_cpu_cnt;
+
+	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
+	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
+
+	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
+}
+#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 +184,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 v4 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load

[Sourabh Jain]

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

Now to enable crash hotplug support for the kexec_load case, the crash
hotplug handler is updated to identify the index at which the FDT segment
is present in the kexec segment array by comparing the first 32 bits of
every kexec segment with the FDT magic number.

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

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 62f77cc86407..e3f224f8eb3a 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,24 @@ 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;
+				break;
+			}
+		}
+	}
+
 	/* Must have valid FDT index */
 	if (!image->arch.fdt_index_valid) {
 		pr_err("crash hp: unable to locate FDT segment");
-- 
2.35.1

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

[Sourabh Jain]

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

Now to enable crash hotplug support for the kexec_load case, the crash
hotplug handler is updated to identify the index at which the FDT segment
is present in the kexec segment array by comparing the first 32 bits of
every kexec segment with the FDT magic number.

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

diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 62f77cc86407..e3f224f8eb3a 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,24 @@ 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;
+				break;
+			}
+		}
+	}
+
 	/* 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 v4 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Laurent Dufour]

On 11/04/2022, 10:43:56, 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.
> 
> On the kexec load path, the memsz allocation of the 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 kexec
> 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
> the handler simply warns the user and returns.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c  | 74 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..62f77cc86407 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;

I should have report since in the previous version too, why nothing is done
when CPU are removed?

> +
> +	/* 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_info_once("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..9dc774548ce4 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,67 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +#include <asm/kvm_book3s.h>
> +#include <asm/kvm_ppc.h>
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +
> +/**
> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
> + *                     in FDT. The calculation is done based on the existing CPU
> + *                     node in unflatten device tree. Loop through all the
> + *                     properties of the very first CPU type device node found in
> + *                     unflatten device tree and returns the sum of the property
> + *                     length and property string size of all properties of a CPU
> + *                     node.
> + */

I don't think this is following the indent rules.

> +static int get_cpu_node_sz(void) {
> +	struct device_node *dn = NULL;
> +	struct property *pp;
> +	int cpu_node_size = 0;
> +
> +	dn = of_find_node_by_type(NULL, "cpu");
> +
> +	if (!dn) {
> +		pr_warn("Unable to locate cpu device_type node.\n");
> +		goto out;
> +	}
> +
> +	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
> +	 * FDT_END_NODE that takes one byte each.
> +	 */
> +	cpu_node_size = 2;
> +
> +	for_each_property_of_node(dn, pp) {
> +		/* For each property add two bytes extra. One for string null
> +		 * character for property name and other for FDT property start
> +		 * tag FDT_PROP.
> +		 */

Coding style request to start with a blank comment line for multiple
comment lines.

> +		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
> +	}
> +
> +out:
> +	return cpu_node_size;
> +}
> +
> +/**
> + * 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 crash FDT
> + * post capture kenrel load due to CPU hotplug events.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	int fdt_cpu_nodes_sz, offline_cpu_cnt;
> +
> +	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
> +	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
> +
> +	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
> +}
> +#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 +184,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 v4 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Laurent Dufour]

On 11/04/2022, 10:43:56, 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.
> 
> On the kexec load path, the memsz allocation of the 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 kexec
> 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
> the handler simply warns the user and returns.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>  arch/powerpc/kexec/elf_64.c  | 74 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 249d2632526d..62f77cc86407 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;

I should have report since in the previous version too, why nothing is done
when CPU are removed?

> +
> +	/* 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_info_once("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..9dc774548ce4 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -24,6 +24,67 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +#include <asm/kvm_book3s.h>
> +#include <asm/kvm_ppc.h>
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +
> +/**
> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
> + *                     in FDT. The calculation is done based on the existing CPU
> + *                     node in unflatten device tree. Loop through all the
> + *                     properties of the very first CPU type device node found in
> + *                     unflatten device tree and returns the sum of the property
> + *                     length and property string size of all properties of a CPU
> + *                     node.
> + */

I don't think this is following the indent rules.

> +static int get_cpu_node_sz(void) {
> +	struct device_node *dn = NULL;
> +	struct property *pp;
> +	int cpu_node_size = 0;
> +
> +	dn = of_find_node_by_type(NULL, "cpu");
> +
> +	if (!dn) {
> +		pr_warn("Unable to locate cpu device_type node.\n");
> +		goto out;
> +	}
> +
> +	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
> +	 * FDT_END_NODE that takes one byte each.
> +	 */
> +	cpu_node_size = 2;
> +
> +	for_each_property_of_node(dn, pp) {
> +		/* For each property add two bytes extra. One for string null
> +		 * character for property name and other for FDT property start
> +		 * tag FDT_PROP.
> +		 */

Coding style request to start with a blank comment line for multiple
comment lines.

> +		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
> +	}
> +
> +out:
> +	return cpu_node_size;
> +}
> +
> +/**
> + * 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 crash FDT
> + * post capture kenrel load due to CPU hotplug events.
> + */
> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
> +{
> +	int fdt_cpu_nodes_sz, offline_cpu_cnt;
> +
> +	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
> +	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
> +
> +	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
> +}
> +#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 +184,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 v4 PATCH 3/5] powrepc/crash hp: update kimage_arch struct

[Laurent Dufour]

On 11/04/2022, 10:43:55, 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;
>  
#ifdef CONFIG_CRASH_HOTPLUG ?
> +	int fdt_index;
> +	bool fdt_index_valid;
#endif

It seems that this is only used when CONFIG_CRASH_HOTPLUG is defined, isn't it?

>  	unsigned long backup_start;
>  	void *backup_buf;
>  	void *fdt;

[RFC v4 PATCH 3/5] powrepc/crash hp: update kimage_arch struct

[Laurent Dufour]

On 11/04/2022, 10:43:55, 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;
>  
#ifdef CONFIG_CRASH_HOTPLUG ?
> +	int fdt_index;
> +	bool fdt_index_valid;
#endif

It seems that this is only used when CONFIG_CRASH_HOTPLUG is defined, isn't it?

>  	unsigned long backup_start;
>  	void *backup_buf;
>  	void *fdt;

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

[Laurent Dufour]

On 11/04/2022, 10:43:57, Sourabh Jain wrote:
> The kernel changes needed for crash hotplug support for kexec_load system
> calls are similar to kexec_file_load (which has already been implemented
> in earlier patches) except for finding the index of the FDT segment in the
> kexec segment array. Since the kexec segment array is prepared by the
> kexec tool in the userspace, the kernel is not aware of at which index FDT
> segment is present.
> 
> Now to enable crash hotplug support for the kexec_load case, the crash
> hotplug handler is updated to identify the index at which the FDT segment
> is present in the kexec segment array by comparing the first 32 bits of
> every kexec segment with the FDT magic number.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 62f77cc86407..e3f224f8eb3a 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,24 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>  		return;
>  	}
>  
> +	/* Sine kexec segments for kexec_load system call is prepred by
           Since

FWIW,
Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> +	 * 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;
> +				break;
> +			}
> +		}
> +	}
> +
>  	/* Must have valid FDT index */
>  	if (!image->arch.fdt_index_valid) {
>  		pr_err("crash hp: unable to locate FDT segment");

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

[Laurent Dufour]

On 11/04/2022, 10:43:57, Sourabh Jain wrote:
> The kernel changes needed for crash hotplug support for kexec_load system
> calls are similar to kexec_file_load (which has already been implemented
> in earlier patches) except for finding the index of the FDT segment in the
> kexec segment array. Since the kexec segment array is prepared by the
> kexec tool in the userspace, the kernel is not aware of at which index FDT
> segment is present.
> 
> Now to enable crash hotplug support for the kexec_load case, the crash
> hotplug handler is updated to identify the index at which the FDT segment
> is present in the kexec segment array by comparing the first 32 bits of
> every kexec segment with the FDT magic number.
> 
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/kexec/core_64.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 62f77cc86407..e3f224f8eb3a 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,24 @@ void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
>  		return;
>  	}
>  
> +	/* Sine kexec segments for kexec_load system call is prepred by
           Since

FWIW,
Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> +	 * 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;
> +				break;
> +			}
> +		}
> +	}
> +
>  	/* Must have valid FDT index */
>  	if (!image->arch.fdt_index_valid) {
>  		pr_err("crash hp: unable to locate FDT segment");

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

[Laurent Dufour]

On 11/04/2022, 10:43:54, 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>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b779603978e1..777db33f75b5 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 CPU hotplug event the kexec segments of capture
> +	  kernel becomes stale and need to be updated with latest CPU data.
> +	  In this method the kernel performs minimal update to only relevant
> +	  kexec segments on CPU hotplug event, instead of triggering full
> +	  capture kernel reload from userspace using udev rule.
> +
>  config PRESERVE_FA_DUMP
>  	bool "Preserve Firmware-assisted dump"
>  	depends on PPC64 && PPC_POWERNV && !FA_DUMP

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

[Laurent Dufour]

On 11/04/2022, 10:43:54, 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>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b779603978e1..777db33f75b5 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 CPU hotplug event the kexec segments of capture
> +	  kernel becomes stale and need to be updated with latest CPU data.
> +	  In this method the kernel performs minimal update to only relevant
> +	  kexec segments on CPU hotplug event, instead of triggering full
> +	  capture kernel reload from userspace using udev rule.
> +
>  config PRESERVE_FA_DUMP
>  	bool "Preserve Firmware-assisted dump"
>  	depends on PPC64 && PPC_POWERNV && !FA_DUMP

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

[Sourabh Jain]

On 14/04/22 22:10, Laurent Dufour wrote:
> On 11/04/2022, 10:43:54, 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>
> Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

Thanks for the review.

- Sourabh Jain

>
>> ---
>>   arch/powerpc/Kconfig | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index b779603978e1..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
>> +
>>   config PRESERVE_FA_DUMP
>>   	bool "Preserve Firmware-assisted dump"
>>   	depends on PPC64 && PPC_POWERNV && !FA_DUMP

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

[Sourabh Jain]

On 14/04/22 22:10, Laurent Dufour wrote:
> On 11/04/2022, 10:43:54, 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>
> Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

Thanks for the review.

- Sourabh Jain

>
>> ---
>>   arch/powerpc/Kconfig | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index b779603978e1..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
>> +
>>   config PRESERVE_FA_DUMP
>>   	bool "Preserve Firmware-assisted dump"
>>   	depends on PPC64 && PPC_POWERNV && !FA_DUMP

Re: [RFC v4 PATCH 3/5] powrepc/crash hp: update kimage_arch struct

[Sourabh Jain]

On 14/04/22 22:05, Laurent Dufour wrote:
> On 11/04/2022, 10:43:55, 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;
>>   
> #ifdef CONFIG_CRASH_HOTPLUG ?
>> +	int fdt_index;
>> +	bool fdt_index_valid;
> #endif
>
> It seems that this is only used when CONFIG_CRASH_HOTPLUG is defined, isn't it?

Yes it should be kept under CONFIG_CRASH_HOTPLUG config.

Thanks,
Sourabh Jain

>
>>   	unsigned long backup_start;
>>   	void *backup_buf;
>>   	void *fdt;

[RFC v4 PATCH 3/5] powrepc/crash hp: update kimage_arch struct

[Sourabh Jain]

On 14/04/22 22:05, Laurent Dufour wrote:
> On 11/04/2022, 10:43:55, 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;
>>   
> #ifdef CONFIG_CRASH_HOTPLUG ?
>> +	int fdt_index;
>> +	bool fdt_index_valid;
> #endif
>
> It seems that this is only used when CONFIG_CRASH_HOTPLUG is defined, isn't it?

Yes it should be kept under CONFIG_CRASH_HOTPLUG config.

Thanks,
Sourabh Jain

>
>>   	unsigned long backup_start;
>>   	void *backup_buf;
>>   	void *fdt;

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

[Sourabh Jain]

On 14/04/22 22:02, Laurent Dufour wrote:
> On 11/04/2022, 10:43:56, 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.
>>
>> On the kexec load path, the memsz allocation of the 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 kexec
>> 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
>> the handler simply warns the user and returns.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>>   arch/powerpc/kexec/elf_64.c  | 74 ++++++++++++++++++++++++++++++++++++
>>   2 files changed, 120 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..62f77cc86407 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;
> I should have report since in the previous version too, why nothing is done
> when CPU are removed?

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 but not needed when
CPU is removed.


Thanks
Sourabh Jain

>
>> +
>> +	/* 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_info_once("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..9dc774548ce4 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,67 @@
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   
>> +#include <asm/kvm_book3s.h>
>> +#include <asm/kvm_ppc.h>
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +
>> +/**
>> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
>> + *                     in FDT. The calculation is done based on the existing CPU
>> + *                     node in unflatten device tree. Loop through all the
>> + *                     properties of the very first CPU type device node found in
>> + *                     unflatten device tree and returns the sum of the property
>> + *                     length and property string size of all properties of a CPU
>> + *                     node.
>> + */
> I don't think this is following the indent rules.
>
>> +static int get_cpu_node_sz(void) {
>> +	struct device_node *dn = NULL;
>> +	struct property *pp;
>> +	int cpu_node_size = 0;
>> +
>> +	dn = of_find_node_by_type(NULL, "cpu");
>> +
>> +	if (!dn) {
>> +		pr_warn("Unable to locate cpu device_type node.\n");
>> +		goto out;
>> +	}
>> +
>> +	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
>> +	 * FDT_END_NODE that takes one byte each.
>> +	 */
>> +	cpu_node_size = 2;
>> +
>> +	for_each_property_of_node(dn, pp) {
>> +		/* For each property add two bytes extra. One for string null
>> +		 * character for property name and other for FDT property start
>> +		 * tag FDT_PROP.
>> +		 */
> Coding style request to start with a blank comment line for multiple
> comment lines.
>
>> +		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
>> +	}
>> +
>> +out:
>> +	return cpu_node_size;
>> +}
>> +
>> +/**
>> + * 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 crash FDT
>> + * post capture kenrel load due to CPU hotplug events.
>> + */
>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>> +{
>> +	int fdt_cpu_nodes_sz, offline_cpu_cnt;
>> +
>> +	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
>> +	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
>> +
>> +	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
>> +}
>> +#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 +184,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 v4 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Sourabh Jain]

On 14/04/22 22:02, Laurent Dufour wrote:
> On 11/04/2022, 10:43:56, 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.
>>
>> On the kexec load path, the memsz allocation of the 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 kexec
>> 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
>> the handler simply warns the user and returns.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>>   arch/powerpc/kexec/elf_64.c  | 74 ++++++++++++++++++++++++++++++++++++
>>   2 files changed, 120 insertions(+)
>>
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 249d2632526d..62f77cc86407 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;
> I should have report since in the previous version too, why nothing is done
> when CPU are removed?

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 but not needed when
CPU is removed.


Thanks
Sourabh Jain

>
>> +
>> +	/* 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_info_once("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..9dc774548ce4 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -24,6 +24,67 @@
>>   #include <linux/slab.h>
>>   #include <linux/types.h>
>>   
>> +#include <asm/kvm_book3s.h>
>> +#include <asm/kvm_ppc.h>
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +
>> +/**
>> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
>> + *                     in FDT. The calculation is done based on the existing CPU
>> + *                     node in unflatten device tree. Loop through all the
>> + *                     properties of the very first CPU type device node found in
>> + *                     unflatten device tree and returns the sum of the property
>> + *                     length and property string size of all properties of a CPU
>> + *                     node.
>> + */
> I don't think this is following the indent rules.
>
>> +static int get_cpu_node_sz(void) {
>> +	struct device_node *dn = NULL;
>> +	struct property *pp;
>> +	int cpu_node_size = 0;
>> +
>> +	dn = of_find_node_by_type(NULL, "cpu");
>> +
>> +	if (!dn) {
>> +		pr_warn("Unable to locate cpu device_type node.\n");
>> +		goto out;
>> +	}
>> +
>> +	/* Every node in FDT starts with FDT_BEGIN_NODE and ends with
>> +	 * FDT_END_NODE that takes one byte each.
>> +	 */
>> +	cpu_node_size = 2;
>> +
>> +	for_each_property_of_node(dn, pp) {
>> +		/* For each property add two bytes extra. One for string null
>> +		 * character for property name and other for FDT property start
>> +		 * tag FDT_PROP.
>> +		 */
> Coding style request to start with a blank comment line for multiple
> comment lines.
>
>> +		cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
>> +	}
>> +
>> +out:
>> +	return cpu_node_size;
>> +}
>> +
>> +/**
>> + * 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 crash FDT
>> + * post capture kenrel load due to CPU hotplug events.
>> + */
>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>> +{
>> +	int fdt_cpu_nodes_sz, offline_cpu_cnt;
>> +
>> +	offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
>> +	fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
>> +
>> +	return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
>> +}
>> +#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 +184,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 v4 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Michael Ellerman]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:
> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
> +	  kernel becomes stale and need to be updated with latest CPU data.
> +	  In this method the kernel performs minimal update to only relevant
> +	  kexec segments on CPU hotplug event, instead of triggering full
> +	  capture kernel reload from userspace using udev rule.

Why would a user ever want to turn this off?

Seems to me we should just make it always behave this way, and not have
a CONFIG option at all.

cheers

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

[Michael Ellerman]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:
> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
> +	  kernel becomes stale and need to be updated with latest CPU data.
> +	  In this method the kernel performs minimal update to only relevant
> +	  kexec segments on CPU hotplug event, instead of triggering full
> +	  capture kernel reload from userspace using udev rule.

Why would a user ever want to turn this off?

Seems to me we should just make it always behave this way, and not have
a CONFIG option at all.

cheers

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

[Eric DeVolder]

On 4/21/22 06:34, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
> 
> Why would a user ever want to turn this off?
> 
> Seems to me we should just make it always behave this way, and not have
> a CONFIG option at all.

Sourabh,

Borislav Petkov also requested I remove the config option, which will be the
case in upcoming v8.

Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.

Eric

> 
> cheers
> 

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

[Eric DeVolder]

On 4/21/22 06:34, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
> 
> Why would a user ever want to turn this off?
> 
> Seems to me we should just make it always behave this way, and not have
> a CONFIG option at all.

Sourabh,

Borislav Petkov also requested I remove the config option, which will be the
case in upcoming v8.

Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.

Eric

> 
> cheers
> 

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

[Eric DeVolder]

On 4/19/22 03:31, Sourabh Jain wrote:
> 
> On 14/04/22 22:02, Laurent Dufour wrote:
>> On 11/04/2022, 10:43:56, 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.
>>>
>>> On the kexec load path, the memsz allocation of the 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 kexec
>>> 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
>>> the handler simply warns the user and returns.
>>>
>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>> ---
>>>   arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>>>   arch/powerpc/kexec/elf_64.c  | 74 ++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 120 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>>> index 249d2632526d..62f77cc86407 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;
>> I should have report since in the previous version too, why nothing is done
>> when CPU are removed?
> 
> 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 but not needed when
> CPU is removed.
> 
> 
> Thanks
> Sourabh Jain
> 
>>
>>> +
>>> +    /* 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_info_once("crash hp: crash update is not supported with memory hotplug\n");
>>> +        return;
>>> +    }

Sourabh,

Baoquan's preference is to do away with the hp_action (and a and b) parameters to
this function. As x86_64 has no need for them, I have no reason to argue for keeping
them.

If you really need hp_action, then please respond to Baoquan's concern in the
"[PATCH v7 4/8] crash: add generic infrastructure for crash hotplug support"
thread.

Thanks,
eric


>>> +
>>> +    /* 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..9dc774548ce4 100644
>>> --- a/arch/powerpc/kexec/elf_64.c
>>> +++ b/arch/powerpc/kexec/elf_64.c
>>> @@ -24,6 +24,67 @@
>>>   #include <linux/slab.h>
>>>   #include <linux/types.h>
>>> +#include <asm/kvm_book3s.h>
>>> +#include <asm/kvm_ppc.h>
>>> +
>>> +#ifdef CONFIG_CRASH_HOTPLUG
>>> +
>>> +/**
>>> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
>>> + *                     in FDT. The calculation is done based on the existing CPU
>>> + *                     node in unflatten device tree. Loop through all the
>>> + *                     properties of the very first CPU type device node found in
>>> + *                     unflatten device tree and returns the sum of the property
>>> + *                     length and property string size of all properties of a CPU
>>> + *                     node.
>>> + */
>> I don't think this is following the indent rules.
>>
>>> +static int get_cpu_node_sz(void) {
>>> +    struct device_node *dn = NULL;
>>> +    struct property *pp;
>>> +    int cpu_node_size = 0;
>>> +
>>> +    dn = of_find_node_by_type(NULL, "cpu");
>>> +
>>> +    if (!dn) {
>>> +        pr_warn("Unable to locate cpu device_type node.\n");
>>> +        goto out;
>>> +    }
>>> +
>>> +    /* Every node in FDT starts with FDT_BEGIN_NODE and ends with
>>> +     * FDT_END_NODE that takes one byte each.
>>> +     */
>>> +    cpu_node_size = 2;
>>> +
>>> +    for_each_property_of_node(dn, pp) {
>>> +        /* For each property add two bytes extra. One for string null
>>> +         * character for property name and other for FDT property start
>>> +         * tag FDT_PROP.
>>> +         */
>> Coding style request to start with a blank comment line for multiple
>> comment lines.
>>
>>> +        cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
>>> +    }
>>> +
>>> +out:
>>> +    return cpu_node_size;
>>> +}
>>> +
>>> +/**
>>> + * 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 crash FDT
>>> + * post capture kenrel load due to CPU hotplug events.
>>> + */
>>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>>> +{
>>> +    int fdt_cpu_nodes_sz, offline_cpu_cnt;
>>> +
>>> +    offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
>>> +    fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
>>> +
>>> +    return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
>>> +}
>>> +#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 +184,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 v4 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load

[Eric DeVolder]

On 4/19/22 03:31, Sourabh Jain wrote:
> 
> On 14/04/22 22:02, Laurent Dufour wrote:
>> On 11/04/2022, 10:43:56, 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.
>>>
>>> On the kexec load path, the memsz allocation of the 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 kexec
>>> 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
>>> the handler simply warns the user and returns.
>>>
>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>> ---
>>> ? arch/powerpc/kexec/core_64.c | 46 ++++++++++++++++++++++
>>> ? arch/powerpc/kexec/elf_64.c? | 74 ++++++++++++++++++++++++++++++++++++
>>> ? 2 files changed, 120 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>>> index 249d2632526d..62f77cc86407 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;
>> I should have report since in the previous version too, why nothing is done
>> when CPU are removed?
> 
> 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 but not needed when
> CPU is removed.
> 
> 
> Thanks
> Sourabh Jain
> 
>>
>>> +
>>> +??? /* 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_info_once("crash hp: crash update is not supported with memory hotplug\n");
>>> +??????? return;
>>> +??? }

Sourabh,

Baoquan's preference is to do away with the hp_action (and a and b) parameters to
this function. As x86_64 has no need for them, I have no reason to argue for keeping
them.

If you really need hp_action, then please respond to Baoquan's concern in the
"[PATCH v7 4/8] crash: add generic infrastructure for crash hotplug support"
thread.

Thanks,
eric


>>> +
>>> +??? /* 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..9dc774548ce4 100644
>>> --- a/arch/powerpc/kexec/elf_64.c
>>> +++ b/arch/powerpc/kexec/elf_64.c
>>> @@ -24,6 +24,67 @@
>>> ? #include <linux/slab.h>
>>> ? #include <linux/types.h>
>>> +#include <asm/kvm_book3s.h>
>>> +#include <asm/kvm_ppc.h>
>>> +
>>> +#ifdef CONFIG_CRASH_HOTPLUG
>>> +
>>> +/**
>>> + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node
>>> + *???????????????????? in FDT. The calculation is done based on the existing CPU
>>> + *???????????????????? node in unflatten device tree. Loop through all the
>>> + *???????????????????? properties of the very first CPU type device node found in
>>> + *???????????????????? unflatten device tree and returns the sum of the property
>>> + *???????????????????? length and property string size of all properties of a CPU
>>> + *???????????????????? node.
>>> + */
>> I don't think this is following the indent rules.
>>
>>> +static int get_cpu_node_sz(void) {
>>> +??? struct device_node *dn = NULL;
>>> +??? struct property *pp;
>>> +??? int cpu_node_size = 0;
>>> +
>>> +??? dn = of_find_node_by_type(NULL, "cpu");
>>> +
>>> +??? if (!dn) {
>>> +??????? pr_warn("Unable to locate cpu device_type node.\n");
>>> +??????? goto out;
>>> +??? }
>>> +
>>> +??? /* Every node in FDT starts with FDT_BEGIN_NODE and ends with
>>> +???? * FDT_END_NODE that takes one byte each.
>>> +???? */
>>> +??? cpu_node_size = 2;
>>> +
>>> +??? for_each_property_of_node(dn, pp) {
>>> +??????? /* For each property add two bytes extra. One for string null
>>> +???????? * character for property name and other for FDT property start
>>> +???????? * tag FDT_PROP.
>>> +???????? */
>> Coding style request to start with a blank comment line for multiple
>> comment lines.
>>
>>> +??????? cpu_node_size = cpu_node_size + pp->length + strlen(pp->name) + 2;
>>> +??? }
>>> +
>>> +out:
>>> +??? return cpu_node_size;
>>> +}
>>> +
>>> +/**
>>> + * 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 crash FDT
>>> + * post capture kenrel load due to CPU hotplug events.
>>> + */
>>> +static unsigned int get_crash_fdt_mem_sz(void *fdt)
>>> +{
>>> +??? int fdt_cpu_nodes_sz, offline_cpu_cnt;
>>> +
>>> +??? offline_cpu_cnt = (num_possible_cpus() - num_present_cpus()) / MAX_SMT_THREADS;
>>> +??? fdt_cpu_nodes_sz = get_cpu_node_sz() * offline_cpu_cnt;
>>> +
>>> +??? return fdt_totalsize(fdt) + fdt_cpu_nodes_sz;
>>> +}
>>> +#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 +184,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 v4 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG

[Michael Ellerman]

Eric DeVolder <eric.devolder@oracle.com> writes:
> On 4/21/22 06:34, Michael Ellerman wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>>> +	  kernel becomes stale and need to be updated with latest CPU data.
>>> +	  In this method the kernel performs minimal update to only relevant
>>> +	  kexec segments on CPU hotplug event, instead of triggering full
>>> +	  capture kernel reload from userspace using udev rule.
>> 
>> Why would a user ever want to turn this off?
>> 
>> Seems to me we should just make it always behave this way, and not have
>> a CONFIG option at all.
>
> Sourabh,
>
> Borislav Petkov also requested I remove the config option, which will be the
> case in upcoming v8.
>
> Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
> CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.

If you're having to spell "CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG"
in many places then you can still add CONFIG_CRASH_HOTPLUG to represent
the sum of all the dependencies, just don't make it user-selectable.

cheers

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

[Michael Ellerman]

Eric DeVolder <eric.devolder@oracle.com> writes:
> On 4/21/22 06:34, Michael Ellerman wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>>> +	  kernel becomes stale and need to be updated with latest CPU data.
>>> +	  In this method the kernel performs minimal update to only relevant
>>> +	  kexec segments on CPU hotplug event, instead of triggering full
>>> +	  capture kernel reload from userspace using udev rule.
>> 
>> Why would a user ever want to turn this off?
>> 
>> Seems to me we should just make it always behave this way, and not have
>> a CONFIG option at all.
>
> Sourabh,
>
> Borislav Petkov also requested I remove the config option, which will be the
> case in upcoming v8.
>
> Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
> CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.

If you're having to spell "CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG"
in many places then you can still add CONFIG_CRASH_HOTPLUG to represent
the sum of all the dependencies, just don't make it user-selectable.

cheers

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

[Sourabh Jain]

On 21/04/22 17:04, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
> Why would a user ever want to turn this off?
>
> Seems to me we should just make it always behave this way, and not have
> a CONFIG option at all.


Yes, we don't need a new CONFIG option. Thanks for the suggestion.


Thanks,
Sourabh Jain

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

[Sourabh Jain]

On 21/04/22 17:04, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of capture
>> +	  kernel becomes stale and need to be updated with latest CPU data.
>> +	  In this method the kernel performs minimal update to only relevant
>> +	  kexec segments on CPU hotplug event, instead of triggering full
>> +	  capture kernel reload from userspace using udev rule.
> Why would a user ever want to turn this off?
>
> Seems to me we should just make it always behave this way, and not have
> a CONFIG option at all.


Yes, we don't need a new CONFIG option. Thanks for the suggestion.


Thanks,
Sourabh Jain

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

[Sourabh Jain]

On 21/04/22 20:59, Eric DeVolder wrote:
>
>
> On 4/21/22 06:34, Michael Ellerman wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of 
>>> capture
>>> +      kernel becomes stale and need to be updated with latest CPU 
>>> data.
>>> +      In this method the kernel performs minimal update to only 
>>> relevant
>>> +      kexec segments on CPU hotplug event, instead of triggering full
>>> +      capture kernel reload from userspace using udev rule.
>>
>> Why would a user ever want to turn this off?
>>
>> Seems to me we should just make it always behave this way, and not have
>> a CONFIG option at all.
>
> Sourabh,
>
> Borislav Petkov also requested I remove the config option, which will 
> be the
> case in upcoming v8.
>
> Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
> CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.


Yes good idea. Even Michael suggested there is no need for new CONFIG
option. I will also remove CRASH_HOTPLUG config in the next version.

Thanks,
Sourabh Jain

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

[Sourabh Jain]

On 21/04/22 20:59, Eric DeVolder wrote:
>
>
> On 4/21/22 06:34, Michael Ellerman wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>> 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..777db33f75b5 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 CPU hotplug event the kexec segments of 
>>> capture
>>> +????? kernel becomes stale and need to be updated with latest CPU 
>>> data.
>>> +????? In this method the kernel performs minimal update to only 
>>> relevant
>>> +????? kexec segments on CPU hotplug event, instead of triggering full
>>> +????? capture kernel reload from userspace using udev rule.
>>
>> Why would a user ever want to turn this off?
>>
>> Seems to me we should just make it always behave this way, and not have
>> a CONFIG option at all.
>
> Sourabh,
>
> Borislav Petkov also requested I remove the config option, which will 
> be the
> case in upcoming v8.
>
> Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the
> CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG.


Yes good idea. Even Michael suggested there is no need for new CONFIG
option. I will also remove CRASH_HOTPLUG config in the next version.

Thanks,
Sourabh Jain