[PATCH v4 00/10] crash: Kernel handling of CPU and memory hot un/plug

[PATCH v4 00/10] crash: Kernel handling of CPU and memory hot un/plug

[Eric DeVolder]

When the kdump service is loaded, if a CPU or memory is hot
un/plugged, the crash elfcorehdr (for x86), which describes the CPUs
and memory in the system, must also be updated, else the resulting
vmcore is inaccurate (eg. missing either CPU context or memory
regions).

The current solution utilizes udev to initiate an unload-then-reload
of the kdump image (e. kernel, initrd, boot_params, puratory and
elfcorehdr) by the userspace kexec utility. In previous posts I have
outlined the significant performance problems related to offloading
this activity to userspace.

This patchset introduces a generic crash hot un/plug handler that
registers with the CPU and memory notifiers. Upon CPU or memory
changes, this generic handler is invoked and performs important
housekeeping, for example obtaining the appropriate lock, and then
invokes an architecture specific handler to do the appropriate
updates.

In the case of x86_64, the arch specific handler generates a new
elfcorehdr, and overwrites the old one in memory. No involvement
with userspace needed.

To realize the benefits/test this patchset, one must make a couple
of minor changes to userspace:

 - Disable the udev rule for updating kdump on hot un/plug changes
   Eg. on RHEL: rm -f /usr/lib/udev/rules.d/98-kexec.rules
   or other technique to neuter the rule.

 - Change to the kexec_file_load for loading the kdump kernel:
   Eg. on RHEL: in /usr/bin/kdumpctl, change to:
    standard_kexec_args="-p -d -s"
   which adds the -s to select kexec_file_load syscall.

This patchset supports kexec_load with a modified kexec userspace
utility, and a working changeset to the kexec userspace utility
is provided here (and to use, the above change to standard_kexec_args
would be, for example, to append --hotplug-size=262144 instead of -s).

 diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
 index 9826f6d..06adb7e 100644
 --- a/kexec/arch/i386/crashdump-x86.c
 +++ b/kexec/arch/i386/crashdump-x86.c
 @@ -48,6 +48,7 @@
  #include <x86/x86-linux.h>
  
  extern struct arch_options_t arch_options;
 +extern unsigned long long hotplug_size;
  
  static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
  				  struct crash_elf_info *elf_info)
 @@ -975,6 +976,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
  	} else {
  		memsz = bufsz;
  	}
 +
 +    /* If hotplug support enabled, use that size */
 +    if (hotplug_size) {
 +        memsz = hotplug_size;
 +    }
 +
 +    info->elfcorehdr =
  	elfcorehdr = add_buffer(info, tmp, bufsz, memsz, align, min_base,
  							max_addr, -1);
  	dbgprintf("Created elf header segment at 0x%lx\n", elfcorehdr);
 diff --git a/kexec/kexec.c b/kexec/kexec.c
 index f63b36b..9569d9a 100644
 --- a/kexec/kexec.c
 +++ b/kexec/kexec.c
 @@ -58,6 +58,7 @@
  
  unsigned long long mem_min = 0;
  unsigned long long mem_max = ULONG_MAX;
 +unsigned long long hotplug_size = 0;
  static unsigned long kexec_flags = 0;
  /* Flags for kexec file (fd) based syscall */
  static unsigned long kexec_file_flags = 0;
 @@ -672,6 +673,12 @@ static void update_purgatory(struct kexec_info *info)
  		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
  			continue;
  		}
 +        /* Don't include elfcorehdr in the checksum, if hotplug
 +         * support enabled.
 +         */
 +        if (hotplug_size && (info->segment[i].mem == (void *)info->elfcorehdr)) {
 +			continue;
 +		}
  		sha256_update(&ctx, info->segment[i].buf,
  			      info->segment[i].bufsz);
  		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
 @@ -1504,6 +1511,17 @@ int main(int argc, char *argv[])
  		case OPT_PRINT_CKR_SIZE:
  			print_crashkernel_region_size();
  			return 0;
 +		case OPT_HOTPLUG_SIZE:
 +            /* Reserved the specified size for hotplug growth */
 +			hotplug_size = strtoul(optarg, &endptr, 0);
 +			if (*endptr) {
 +				fprintf(stderr,
 +					"Bad option value in --hotplug-size=%s\n",
 +					optarg);
 +				usage();
 +				return 1;
 +			}
 +			break;
  		default:
  			break;
  		}
 diff --git a/kexec/kexec.h b/kexec/kexec.h
 index 595dd68..b30dda4 100644
 --- a/kexec/kexec.h
 +++ b/kexec/kexec.h
 @@ -169,6 +169,7 @@ struct kexec_info {
  	int command_line_len;
  
  	int skip_checks;
 +    unsigned long elfcorehdr;
   };
  
  struct arch_map_entry {
 @@ -231,7 +232,8 @@ extern int file_types;
  #define OPT_PRINT_CKR_SIZE	262
  #define OPT_LOAD_LIVE_UPDATE	263
  #define OPT_EXEC_LIVE_UPDATE	264
 -#define OPT_MAX			265
 +#define OPT_HOTPLUG_SIZE	265
 +#define OPT_MAX			266
  #define KEXEC_OPTIONS \
  	{ "help",		0, 0, OPT_HELP }, \
  	{ "version",		0, 0, OPT_VERSION }, \
 @@ -258,6 +260,7 @@ extern int file_types;
  	{ "debug",		0, 0, OPT_DEBUG }, \
  	{ "status",		0, 0, OPT_STATUS }, \
  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 +	{ "hotplug-size",     2, 0, OPT_HOTPLUG_SIZE }, \
  
  #define KEXEC_OPT_STR "h?vdfixyluet:pscaS"
 

Regards,
eric
---
v4: 9feb2022
 - Refactored patches per Baoquan suggestsions.
 - A few corrections, per Baoquan.

v3: 10jan2022
 https://lkml.org/lkml/2022/1/10/1212
 - Rebasing per Baoquan He request.
 - Changed memory notifier per David Hildenbrand.
 - Providing example kexec userspace change in cover letter.

RFC v2: 7dec2021
 https://lkml.org/lkml/2021/12/7/1088
 - Acting upon Baoquan He suggestion of removing elfcorehdr from
   the purgatory list of segments, removed purgatory code from
   patchset, and it is signficiantly simpler now.

RFC v1: 18nov2021
 https://lkml.org/lkml/2021/11/18/845
 - working patchset demonstrating kernel handling of hotplug
   updates to x86 elfcorehdr for kexec_file_load

RFC: 14dec2020
 https://lkml.org/lkml/2020/12/14/532
 - proposed concept of allowing kernel to handle hotplug update
   of elfcorehdr
---

Eric DeVolder (10):
  crash: fix minor typo/bug in debug message
  crash hp: Introduce CRASH_HOTPLUG configuration options
  crash hp: definitions and prototype changes
  crash hp: prototype change for crash_prepare_elf64_headers
  crash hp: introduce helper functions un/map_crash_pages
  crash hp: generic crash hotplug support infrastructure
  crash hp: exclude elfcorehdr from the segment digest
  crash hp: exclude hot remove cpu from elfcorehdr notes
  crash hp: Add x86 crash hotplug support for kexec_file_load
  crash hp: Add x86 crash hotplug support for kexec_load

 arch/arm64/kernel/machine_kexec_file.c |   6 +-
 arch/powerpc/kexec/file_load_64.c      |   2 +-
 arch/x86/Kconfig                       |  26 +++++
 arch/x86/kernel/crash.c                | 123 ++++++++++++++++++++-
 include/linux/kexec.h                  |  23 +++-
 kernel/crash_core.c                    | 146 +++++++++++++++++++++++++
 kernel/kexec_file.c                    |  15 ++-
 7 files changed, 331 insertions(+), 10 deletions(-)

-- 
2.27.0

[PATCH v4 00/10] crash: Kernel handling of CPU and memory hot un/plug

[Eric DeVolder]

When the kdump service is loaded, if a CPU or memory is hot
un/plugged, the crash elfcorehdr (for x86), which describes the CPUs
and memory in the system, must also be updated, else the resulting
vmcore is inaccurate (eg. missing either CPU context or memory
regions).

The current solution utilizes udev to initiate an unload-then-reload
of the kdump image (e. kernel, initrd, boot_params, puratory and
elfcorehdr) by the userspace kexec utility. In previous posts I have
outlined the significant performance problems related to offloading
this activity to userspace.

This patchset introduces a generic crash hot un/plug handler that
registers with the CPU and memory notifiers. Upon CPU or memory
changes, this generic handler is invoked and performs important
housekeeping, for example obtaining the appropriate lock, and then
invokes an architecture specific handler to do the appropriate
updates.

In the case of x86_64, the arch specific handler generates a new
elfcorehdr, and overwrites the old one in memory. No involvement
with userspace needed.

To realize the benefits/test this patchset, one must make a couple
of minor changes to userspace:

 - Disable the udev rule for updating kdump on hot un/plug changes
   Eg. on RHEL: rm -f /usr/lib/udev/rules.d/98-kexec.rules
   or other technique to neuter the rule.

 - Change to the kexec_file_load for loading the kdump kernel:
   Eg. on RHEL: in /usr/bin/kdumpctl, change to:
    standard_kexec_args="-p -d -s"
   which adds the -s to select kexec_file_load syscall.

This patchset supports kexec_load with a modified kexec userspace
utility, and a working changeset to the kexec userspace utility
is provided here (and to use, the above change to standard_kexec_args
would be, for example, to append --hotplug-size=262144 instead of -s).

 diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
 index 9826f6d..06adb7e 100644
 --- a/kexec/arch/i386/crashdump-x86.c
 +++ b/kexec/arch/i386/crashdump-x86.c
 @@ -48,6 +48,7 @@
  #include <x86/x86-linux.h>
  
  extern struct arch_options_t arch_options;
 +extern unsigned long long hotplug_size;
  
  static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
  				  struct crash_elf_info *elf_info)
 @@ -975,6 +976,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
  	} else {
  		memsz = bufsz;
  	}
 +
 +    /* If hotplug support enabled, use that size */
 +    if (hotplug_size) {
 +        memsz = hotplug_size;
 +    }
 +
 +    info->elfcorehdr =
  	elfcorehdr = add_buffer(info, tmp, bufsz, memsz, align, min_base,
  							max_addr, -1);
  	dbgprintf("Created elf header segment at 0x%lx\n", elfcorehdr);
 diff --git a/kexec/kexec.c b/kexec/kexec.c
 index f63b36b..9569d9a 100644
 --- a/kexec/kexec.c
 +++ b/kexec/kexec.c
 @@ -58,6 +58,7 @@
  
  unsigned long long mem_min = 0;
  unsigned long long mem_max = ULONG_MAX;
 +unsigned long long hotplug_size = 0;
  static unsigned long kexec_flags = 0;
  /* Flags for kexec file (fd) based syscall */
  static unsigned long kexec_file_flags = 0;
 @@ -672,6 +673,12 @@ static void update_purgatory(struct kexec_info *info)
  		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
  			continue;
  		}
 +        /* Don't include elfcorehdr in the checksum, if hotplug
 +         * support enabled.
 +         */
 +        if (hotplug_size && (info->segment[i].mem == (void *)info->elfcorehdr)) {
 +			continue;
 +		}
  		sha256_update(&ctx, info->segment[i].buf,
  			      info->segment[i].bufsz);
  		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
 @@ -1504,6 +1511,17 @@ int main(int argc, char *argv[])
  		case OPT_PRINT_CKR_SIZE:
  			print_crashkernel_region_size();
  			return 0;
 +		case OPT_HOTPLUG_SIZE:
 +            /* Reserved the specified size for hotplug growth */
 +			hotplug_size = strtoul(optarg, &endptr, 0);
 +			if (*endptr) {
 +				fprintf(stderr,
 +					"Bad option value in --hotplug-size=%s\n",
 +					optarg);
 +				usage();
 +				return 1;
 +			}
 +			break;
  		default:
  			break;
  		}
 diff --git a/kexec/kexec.h b/kexec/kexec.h
 index 595dd68..b30dda4 100644
 --- a/kexec/kexec.h
 +++ b/kexec/kexec.h
 @@ -169,6 +169,7 @@ struct kexec_info {
  	int command_line_len;
  
  	int skip_checks;
 +    unsigned long elfcorehdr;
   };
  
  struct arch_map_entry {
 @@ -231,7 +232,8 @@ extern int file_types;
  #define OPT_PRINT_CKR_SIZE	262
  #define OPT_LOAD_LIVE_UPDATE	263
  #define OPT_EXEC_LIVE_UPDATE	264
 -#define OPT_MAX			265
 +#define OPT_HOTPLUG_SIZE	265
 +#define OPT_MAX			266
  #define KEXEC_OPTIONS \
  	{ "help",		0, 0, OPT_HELP }, \
  	{ "version",		0, 0, OPT_VERSION }, \
 @@ -258,6 +260,7 @@ extern int file_types;
  	{ "debug",		0, 0, OPT_DEBUG }, \
  	{ "status",		0, 0, OPT_STATUS }, \
  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 +	{ "hotplug-size",     2, 0, OPT_HOTPLUG_SIZE }, \
  
  #define KEXEC_OPT_STR "h?vdfixyluet:pscaS"
 

Regards,
eric
---
v4: 9feb2022
 - Refactored patches per Baoquan suggestsions.
 - A few corrections, per Baoquan.

v3: 10jan2022
 https://lkml.org/lkml/2022/1/10/1212
 - Rebasing per Baoquan He request.
 - Changed memory notifier per David Hildenbrand.
 - Providing example kexec userspace change in cover letter.

RFC v2: 7dec2021
 https://lkml.org/lkml/2021/12/7/1088
 - Acting upon Baoquan He suggestion of removing elfcorehdr from
   the purgatory list of segments, removed purgatory code from
   patchset, and it is signficiantly simpler now.

RFC v1: 18nov2021
 https://lkml.org/lkml/2021/11/18/845
 - working patchset demonstrating kernel handling of hotplug
   updates to x86 elfcorehdr for kexec_file_load

RFC: 14dec2020
 https://lkml.org/lkml/2020/12/14/532
 - proposed concept of allowing kernel to handle hotplug update
   of elfcorehdr
---

Eric DeVolder (10):
  crash: fix minor typo/bug in debug message
  crash hp: Introduce CRASH_HOTPLUG configuration options
  crash hp: definitions and prototype changes
  crash hp: prototype change for crash_prepare_elf64_headers
  crash hp: introduce helper functions un/map_crash_pages
  crash hp: generic crash hotplug support infrastructure
  crash hp: exclude elfcorehdr from the segment digest
  crash hp: exclude hot remove cpu from elfcorehdr notes
  crash hp: Add x86 crash hotplug support for kexec_file_load
  crash hp: Add x86 crash hotplug support for kexec_load

 arch/arm64/kernel/machine_kexec_file.c |   6 +-
 arch/powerpc/kexec/file_load_64.c      |   2 +-
 arch/x86/Kconfig                       |  26 +++++
 arch/x86/kernel/crash.c                | 123 ++++++++++++++++++++-
 include/linux/kexec.h                  |  23 +++-
 kernel/crash_core.c                    | 146 +++++++++++++++++++++++++
 kernel/kexec_file.c                    |  15 ++-
 7 files changed, 331 insertions(+), 10 deletions(-)

-- 
2.27.0

[PATCH v4 01/10] crash: fix minor typo/bug in debug message

[Eric DeVolder]

The pr_debug() intends to display the memsz member, but the
parameter is actually the bufsz member (which is already
displayed). Correct this to display memsz value.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/kernel/crash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index e8326a8d1c5d..9730c88530fc 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -407,7 +407,7 @@ int crash_load_segments(struct kimage *image)
 	}
 	image->elf_load_addr = kbuf.mem;
 	pr_debug("Loaded ELF headers at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
-		 image->elf_load_addr, kbuf.bufsz, kbuf.bufsz);
+		 image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
 
 	return ret;
 }
-- 
2.27.0

[PATCH v4 01/10] crash: fix minor typo/bug in debug message

[Eric DeVolder]

The pr_debug() intends to display the memsz member, but the
parameter is actually the bufsz member (which is already
displayed). Correct this to display memsz value.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/kernel/crash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index e8326a8d1c5d..9730c88530fc 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -407,7 +407,7 @@ int crash_load_segments(struct kimage *image)
 	}
 	image->elf_load_addr = kbuf.mem;
 	pr_debug("Loaded ELF headers at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
-		 image->elf_load_addr, kbuf.bufsz, kbuf.bufsz);
+		 image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
 
 	return ret;
 }
-- 
2.27.0

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

Support for CPU and memory hotplug for crash is controlled by the
CRASH_HOTPLUG configuration option, introduced by this patch.

The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
also introduced with this patch.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ebe8fc76949a..4e3374edab02 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2060,6 +2060,32 @@ config CRASH_DUMP
 	  (CONFIG_RELOCATABLE=y).
 	  For more details see Documentation/admin-guide/kdump/kdump.rst
 
+config CRASH_HOTPLUG
+	bool "kernel updates of crash elfcorehdr"
+	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
+	help
+	  Enable the kernel to update the crash elfcorehdr (which contains
+	  the list of CPUs and memory regions) directly when hot plug/unplug
+	  of CPUs or memory. Otherwise userspace must monitor these hot
+	  plug/unplug change notifications via udev in order to
+	  unload-then-reload the crash kernel so that the list of CPUs and
+	  memory regions is kept up-to-date. Note that the udev CPU and
+	  memory change notifications still occur (however, userspace is not
+	  required to monitor for crash dump purposes).
+
+config CRASH_HOTPLUG_ELFCOREHDR_SZ
+	depends on CRASH_HOTPLUG
+	int
+	default 131072
+	help
+	  Specify the maximum size of the elfcorehdr buffer/segment.
+	  The 128KiB default is sized so that it can accommodate 2048
+	  Elf64_Phdr, where each Phdr represents either a CPU or a
+	  region of memory.
+	  For example, this size can accommodate hotplugging a machine
+	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
+	  with 1024 1GiB memory DIMMs).
+
 config KEXEC_JUMP
 	bool "kexec jump"
 	depends on KEXEC && HIBERNATION
-- 
2.27.0

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

Support for CPU and memory hotplug for crash is controlled by the
CRASH_HOTPLUG configuration option, introduced by this patch.

The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
also introduced with this patch.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ebe8fc76949a..4e3374edab02 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2060,6 +2060,32 @@ config CRASH_DUMP
 	  (CONFIG_RELOCATABLE=y).
 	  For more details see Documentation/admin-guide/kdump/kdump.rst
 
+config CRASH_HOTPLUG
+	bool "kernel updates of crash elfcorehdr"
+	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
+	help
+	  Enable the kernel to update the crash elfcorehdr (which contains
+	  the list of CPUs and memory regions) directly when hot plug/unplug
+	  of CPUs or memory. Otherwise userspace must monitor these hot
+	  plug/unplug change notifications via udev in order to
+	  unload-then-reload the crash kernel so that the list of CPUs and
+	  memory regions is kept up-to-date. Note that the udev CPU and
+	  memory change notifications still occur (however, userspace is not
+	  required to monitor for crash dump purposes).
+
+config CRASH_HOTPLUG_ELFCOREHDR_SZ
+	depends on CRASH_HOTPLUG
+	int
+	default 131072
+	help
+	  Specify the maximum size of the elfcorehdr buffer/segment.
+	  The 128KiB default is sized so that it can accommodate 2048
+	  Elf64_Phdr, where each Phdr represents either a CPU or a
+	  region of memory.
+	  For example, this size can accommodate hotplugging a machine
+	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
+	  with 1024 1GiB memory DIMMs).
+
 config KEXEC_JUMP
 	bool "kexec jump"
 	depends on KEXEC && HIBERNATION
-- 
2.27.0

[PATCH v4 03/10] crash hp: definitions and prototype changes

[Eric DeVolder]

This change adds members to struct kimage to facilitate crash
hotplug support.

This change also defines crash hotplug events and associated
prototypes.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 include/linux/kexec.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0c994ae37729..13c238c2da48 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -299,6 +299,13 @@ struct kimage {
 
 	/* Information for loading purgatory */
 	struct purgatory_info purgatory_info;
+
+#ifdef CONFIG_CRASH_HOTPLUG
+	bool hotplug_event;
+	int offlinecpu;
+	bool elf_index_valid;
+	int elf_index;
+#endif
 #endif
 
 #ifdef CONFIG_IMA_KEXEC
@@ -315,6 +322,15 @@ struct kimage {
 	unsigned long elf_load_addr;
 };
 
+#ifdef CONFIG_CRASH_HOTPLUG
+void arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b);
+#define KEXEC_CRASH_HP_REMOVE_CPU   0
+#define KEXEC_CRASH_HP_ADD_CPU      1
+#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
+#define KEXEC_CRASH_HP_ADD_MEMORY   3
+#endif /* CONFIG_CRASH_HOTPLUG */
+
 /* kexec interface functions */
 extern void machine_kexec(struct kimage *image);
 extern int machine_kexec_prepare(struct kimage *image);
-- 
2.27.0

[PATCH v4 03/10] crash hp: definitions and prototype changes

[Eric DeVolder]

This change adds members to struct kimage to facilitate crash
hotplug support.

This change also defines crash hotplug events and associated
prototypes.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 include/linux/kexec.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0c994ae37729..13c238c2da48 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -299,6 +299,13 @@ struct kimage {
 
 	/* Information for loading purgatory */
 	struct purgatory_info purgatory_info;
+
+#ifdef CONFIG_CRASH_HOTPLUG
+	bool hotplug_event;
+	int offlinecpu;
+	bool elf_index_valid;
+	int elf_index;
+#endif
 #endif
 
 #ifdef CONFIG_IMA_KEXEC
@@ -315,6 +322,15 @@ struct kimage {
 	unsigned long elf_load_addr;
 };
 
+#ifdef CONFIG_CRASH_HOTPLUG
+void arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b);
+#define KEXEC_CRASH_HP_REMOVE_CPU   0
+#define KEXEC_CRASH_HP_ADD_CPU      1
+#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
+#define KEXEC_CRASH_HP_ADD_MEMORY   3
+#endif /* CONFIG_CRASH_HOTPLUG */
+
 /* kexec interface functions */
 extern void machine_kexec(struct kimage *image);
 extern int machine_kexec_prepare(struct kimage *image);
-- 
2.27.0

[PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Eric DeVolder]

From within crash_prepare_elf64_headers() there is a need to
reference the struct kimage hotplug members. As such, this
change passes the struct kimage as a parameter to the
crash_prepare_elf64_headers().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/arm64/kernel/machine_kexec_file.c | 6 +++---
 arch/powerpc/kexec/file_load_64.c      | 2 +-
 arch/x86/kernel/crash.c                | 3 ++-
 include/linux/kexec.h                  | 5 +++--
 kernel/kexec_file.c                    | 4 ++--
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 59c648d51848..7dbafb42ecf2 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
 	return kexec_image_post_load_cleanup_default(image);
 }
 
-static int prepare_elf_headers(void **addr, unsigned long *sz)
+static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
 {
 	struct crash_mem *cmem;
 	unsigned int nr_ranges;
@@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
 	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
 
 	if (!ret)
-		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
+		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
 
 	kfree(cmem);
 	return ret;
@@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
 
 	/* load elf core header */
 	if (image->type == KEXEC_TYPE_CRASH) {
-		ret = prepare_elf_headers(&headers, &headers_sz);
+		ret = prepare_elf_headers(image, &headers, &headers_sz);
 		if (ret) {
 			pr_err("Preparing elf core header failed\n");
 			goto out_err;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index b4981b651d9a..07da6bf1cf24 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
 		goto out;
 
 	/* Setup elfcorehdr segment */
-	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
+	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
 	if (ret) {
 		pr_err("Failed to prepare elf headers for the core\n");
 		goto out;
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9730c88530fc..9db41cce8d97 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
 		goto out;
 
 	/* By default prepare 64bit headers */
-	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
+	ret =  crash_prepare_elf64_headers(image, cmem,
+				IS_ENABLED(CONFIG_X86_64), addr, sz);
 
 out:
 	vfree(cmem);
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 13c238c2da48..b11d75a6b2bc 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -221,8 +221,9 @@ struct crash_mem {
 extern int crash_exclude_mem_range(struct crash_mem *mem,
 				   unsigned long long mstart,
 				   unsigned long long mend);
-extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
-				       void **addr, unsigned long *sz);
+extern int crash_prepare_elf64_headers(struct kimage *image,
+	struct crash_mem *mem, int kernel_map,
+	void **addr, unsigned long *sz);
 #endif /* CONFIG_KEXEC_FILE */
 
 #ifdef CONFIG_KEXEC_ELF
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 8347fc158d2b..801d0d0a5012 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
 	return 0;
 }
 
-int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
-			  void **addr, unsigned long *sz)
+int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
+	int kernel_map, void **addr, unsigned long *sz)
 {
 	Elf64_Ehdr *ehdr;
 	Elf64_Phdr *phdr;
-- 
2.27.0

[PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Eric DeVolder]

From within crash_prepare_elf64_headers() there is a need to
reference the struct kimage hotplug members. As such, this
change passes the struct kimage as a parameter to the
crash_prepare_elf64_headers().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/arm64/kernel/machine_kexec_file.c | 6 +++---
 arch/powerpc/kexec/file_load_64.c      | 2 +-
 arch/x86/kernel/crash.c                | 3 ++-
 include/linux/kexec.h                  | 5 +++--
 kernel/kexec_file.c                    | 4 ++--
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 59c648d51848..7dbafb42ecf2 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
 	return kexec_image_post_load_cleanup_default(image);
 }
 
-static int prepare_elf_headers(void **addr, unsigned long *sz)
+static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
 {
 	struct crash_mem *cmem;
 	unsigned int nr_ranges;
@@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
 	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
 
 	if (!ret)
-		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
+		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
 
 	kfree(cmem);
 	return ret;
@@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
 
 	/* load elf core header */
 	if (image->type == KEXEC_TYPE_CRASH) {
-		ret = prepare_elf_headers(&headers, &headers_sz);
+		ret = prepare_elf_headers(image, &headers, &headers_sz);
 		if (ret) {
 			pr_err("Preparing elf core header failed\n");
 			goto out_err;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index b4981b651d9a..07da6bf1cf24 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
 		goto out;
 
 	/* Setup elfcorehdr segment */
-	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
+	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
 	if (ret) {
 		pr_err("Failed to prepare elf headers for the core\n");
 		goto out;
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9730c88530fc..9db41cce8d97 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
 		goto out;
 
 	/* By default prepare 64bit headers */
-	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
+	ret =  crash_prepare_elf64_headers(image, cmem,
+				IS_ENABLED(CONFIG_X86_64), addr, sz);
 
 out:
 	vfree(cmem);
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 13c238c2da48..b11d75a6b2bc 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -221,8 +221,9 @@ struct crash_mem {
 extern int crash_exclude_mem_range(struct crash_mem *mem,
 				   unsigned long long mstart,
 				   unsigned long long mend);
-extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
-				       void **addr, unsigned long *sz);
+extern int crash_prepare_elf64_headers(struct kimage *image,
+	struct crash_mem *mem, int kernel_map,
+	void **addr, unsigned long *sz);
 #endif /* CONFIG_KEXEC_FILE */
 
 #ifdef CONFIG_KEXEC_ELF
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 8347fc158d2b..801d0d0a5012 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
 	return 0;
 }
 
-int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
-			  void **addr, unsigned long *sz)
+int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
+	int kernel_map, void **addr, unsigned long *sz)
 {
 	Elf64_Ehdr *ehdr;
 	Elf64_Phdr *phdr;
-- 
2.27.0

[PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Eric DeVolder]

This change introduces two new functions un/map_crash_pages()
which are used to enable/disable access to the segments in the
crash memory region. (Upon loading of a crash kernel, the
crash memory regions are made inaccessible for integrity purposes.)

For example, on x86_64, one of the segments is the elfcorehdr,
which contains the list of CPUs and memories. This segment
needs to be modified in response to hotplug events. These functions
are used to obtain (and subsequenntly release) access to the crash
memory region in order to make the modifications.

QUESTION: These might need to be in arch/x86 as I'm not certain
the implementatin is valid for all archs?

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 include/linux/kexec.h |  2 ++
 kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b11d75a6b2bc..e00c373c4095 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -324,6 +324,8 @@ struct kimage {
 };
 
 #ifdef CONFIG_CRASH_HOTPLUG
+void *map_crash_pages(unsigned long paddr, unsigned long size);
+void unmap_crash_pages(void **ptr);
 void arch_crash_hotplug_handler(struct kimage *image,
 	unsigned int hp_action, unsigned long a, unsigned long b);
 #define KEXEC_CRASH_HP_REMOVE_CPU   0
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..0ff06d0698ad 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
+#include <linux/highmem.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
@@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
 }
 
 subsys_initcall(crash_save_vmcoreinfo_init);
+
+#ifdef CONFIG_CRASH_HOTPLUG
+void *map_crash_pages(unsigned long paddr, unsigned long size)
+{
+	/*
+	 * NOTE: The addresses and sizes passed to this routine have
+	 * already been fully aligned on page boundaries. There is no
+	 * need for massaging the address or size.
+	 */
+	void *ptr = NULL;
+
+	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
+	if (size > 0) {
+		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
+
+		ptr = kmap(page);
+	}
+
+	return ptr;
+}
+
+void unmap_crash_pages(void **ptr)
+{
+	if (ptr) {
+		if (*ptr)
+			kunmap(*ptr);
+		*ptr = NULL;
+	}
+}
+#endif
+
-- 
2.27.0

[PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Eric DeVolder]

This change introduces two new functions un/map_crash_pages()
which are used to enable/disable access to the segments in the
crash memory region. (Upon loading of a crash kernel, the
crash memory regions are made inaccessible for integrity purposes.)

For example, on x86_64, one of the segments is the elfcorehdr,
which contains the list of CPUs and memories. This segment
needs to be modified in response to hotplug events. These functions
are used to obtain (and subsequenntly release) access to the crash
memory region in order to make the modifications.

QUESTION: These might need to be in arch/x86 as I'm not certain
the implementatin is valid for all archs?

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 include/linux/kexec.h |  2 ++
 kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b11d75a6b2bc..e00c373c4095 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -324,6 +324,8 @@ struct kimage {
 };
 
 #ifdef CONFIG_CRASH_HOTPLUG
+void *map_crash_pages(unsigned long paddr, unsigned long size);
+void unmap_crash_pages(void **ptr);
 void arch_crash_hotplug_handler(struct kimage *image,
 	unsigned int hp_action, unsigned long a, unsigned long b);
 #define KEXEC_CRASH_HP_REMOVE_CPU   0
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..0ff06d0698ad 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
+#include <linux/highmem.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
@@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
 }
 
 subsys_initcall(crash_save_vmcoreinfo_init);
+
+#ifdef CONFIG_CRASH_HOTPLUG
+void *map_crash_pages(unsigned long paddr, unsigned long size)
+{
+	/*
+	 * NOTE: The addresses and sizes passed to this routine have
+	 * already been fully aligned on page boundaries. There is no
+	 * need for massaging the address or size.
+	 */
+	void *ptr = NULL;
+
+	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
+	if (size > 0) {
+		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
+
+		ptr = kmap(page);
+	}
+
+	return ptr;
+}
+
+void unmap_crash_pages(void **ptr)
+{
+	if (ptr) {
+		if (*ptr)
+			kunmap(*ptr);
+		*ptr = NULL;
+	}
+}
+#endif
+
-- 
2.27.0

[PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Eric DeVolder]

This patch introduces a generic crash hot plug/unplug infrastructure
for CPU and memory changes. Upon CPU and memory changes, a generic
crash_hotplug_handler() obtains the appropriate lock, does some
important house keeping and then dispatches the hot plug/unplug event
to the architecture specific arch_crash_hotplug_handler(), and when
that handler returns, the lock is released.

This patch modifies crash_core.c to implement a subsys_initcall()
function that installs handlers for hot plug/unplug events. If CPU
hotplug is enabled, then cpuhp_setup_state() is invoked to register a
handler for CPU changes. Similarly, if memory hotplug is enabled, then
register_memory_notifier() is invoked to install a handler for memory
changes. These handlers in turn invoke the common generic handler
crash_hotplug_handler().

On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
the CPU still shows up in foreach_present_cpu() during the regeneration
of the new CPU list, thus the need to explicitly check and exclude the
soon-to-be offlined CPU in crash_prepare_elf64_headers().

On the memory side, each un/plugged memory block passes through the
handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
memory events, one for each 128MiB memblock.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 0ff06d0698ad..524470e18883 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -10,12 +10,16 @@
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
 #include <linux/highmem.h>
+#include <linux/memory.h>
+#include <linux/cpuhotplug.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
 
 #include <crypto/sha1.h>
 
+#include "kexec_internal.h"
+
 /* vmcoreinfo stuff */
 unsigned char *vmcoreinfo_data;
 size_t vmcoreinfo_size;
@@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
 		*ptr = NULL;
 	}
 }
+
+void __weak arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b)
+{
+	pr_warn("crash hp: %s not implemented", __func__);
+}
+
+static void crash_hotplug_handler(unsigned int hp_action,
+	unsigned long a, unsigned long b)
+{
+	/* Obtain lock while changing crash information */
+	if (!mutex_trylock(&kexec_mutex))
+		return;
+
+	/* Check kdump is loaded */
+	if (kexec_crash_image) {
+		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
+			a, b);
+
+		/* Needed in order for the segments to be updated */
+		arch_kexec_unprotect_crashkres();
+
+		/* Flag to differentiate between normal load and hotplug */
+		kexec_crash_image->hotplug_event = true;
+
+		/* Now invoke arch-specific update handler */
+		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
+
+		/* No longer handling a hotplug event */
+		kexec_crash_image->hotplug_event = false;
+
+		/* Change back to read-only */
+		arch_kexec_protect_crashkres();
+	}
+
+	/* Release lock now that update complete */
+	mutex_unlock(&kexec_mutex);
+}
+
+#if defined(CONFIG_MEMORY_HOTPLUG)
+static int crash_memhp_notifier(struct notifier_block *nb,
+	unsigned long val, void *v)
+{
+	struct memory_notify *mhp = v;
+	unsigned long start, end;
+
+	start = mhp->start_pfn << PAGE_SHIFT;
+	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
+
+	switch (val) {
+	case MEM_ONLINE:
+		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
+			start, end-start);
+		break;
+
+	case MEM_OFFLINE:
+		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
+			start, end-start);
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block crash_memhp_nb = {
+	.notifier_call = crash_memhp_notifier,
+	.priority = 0
+};
 #endif
 
+#if defined(CONFIG_HOTPLUG_CPU)
+static int crash_cpuhp_online(unsigned int cpu)
+{
+	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
+	return 0;
+}
+
+static int crash_cpuhp_offline(unsigned int cpu)
+{
+	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
+	return 0;
+}
+#endif
+
+static int __init crash_hotplug_init(void)
+{
+	int result = 0;
+
+#if defined(CONFIG_MEMORY_HOTPLUG)
+	register_memory_notifier(&crash_memhp_nb);
+#endif
+
+#if defined(CONFIG_HOTPLUG_CPU)
+	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
+				"crash/cpuhp",
+				crash_cpuhp_online, crash_cpuhp_offline);
+#endif
+
+	return result;
+}
+
+subsys_initcall(crash_hotplug_init);
+#endif /* CONFIG_CRASH_HOTPLUG */
-- 
2.27.0

[PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Eric DeVolder]

This patch introduces a generic crash hot plug/unplug infrastructure
for CPU and memory changes. Upon CPU and memory changes, a generic
crash_hotplug_handler() obtains the appropriate lock, does some
important house keeping and then dispatches the hot plug/unplug event
to the architecture specific arch_crash_hotplug_handler(), and when
that handler returns, the lock is released.

This patch modifies crash_core.c to implement a subsys_initcall()
function that installs handlers for hot plug/unplug events. If CPU
hotplug is enabled, then cpuhp_setup_state() is invoked to register a
handler for CPU changes. Similarly, if memory hotplug is enabled, then
register_memory_notifier() is invoked to install a handler for memory
changes. These handlers in turn invoke the common generic handler
crash_hotplug_handler().

On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
the CPU still shows up in foreach_present_cpu() during the regeneration
of the new CPU list, thus the need to explicitly check and exclude the
soon-to-be offlined CPU in crash_prepare_elf64_headers().

On the memory side, each un/plugged memory block passes through the
handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
memory events, one for each 128MiB memblock.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 0ff06d0698ad..524470e18883 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -10,12 +10,16 @@
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
 #include <linux/highmem.h>
+#include <linux/memory.h>
+#include <linux/cpuhotplug.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
 
 #include <crypto/sha1.h>
 
+#include "kexec_internal.h"
+
 /* vmcoreinfo stuff */
 unsigned char *vmcoreinfo_data;
 size_t vmcoreinfo_size;
@@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
 		*ptr = NULL;
 	}
 }
+
+void __weak arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b)
+{
+	pr_warn("crash hp: %s not implemented", __func__);
+}
+
+static void crash_hotplug_handler(unsigned int hp_action,
+	unsigned long a, unsigned long b)
+{
+	/* Obtain lock while changing crash information */
+	if (!mutex_trylock(&kexec_mutex))
+		return;
+
+	/* Check kdump is loaded */
+	if (kexec_crash_image) {
+		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
+			a, b);
+
+		/* Needed in order for the segments to be updated */
+		arch_kexec_unprotect_crashkres();
+
+		/* Flag to differentiate between normal load and hotplug */
+		kexec_crash_image->hotplug_event = true;
+
+		/* Now invoke arch-specific update handler */
+		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
+
+		/* No longer handling a hotplug event */
+		kexec_crash_image->hotplug_event = false;
+
+		/* Change back to read-only */
+		arch_kexec_protect_crashkres();
+	}
+
+	/* Release lock now that update complete */
+	mutex_unlock(&kexec_mutex);
+}
+
+#if defined(CONFIG_MEMORY_HOTPLUG)
+static int crash_memhp_notifier(struct notifier_block *nb,
+	unsigned long val, void *v)
+{
+	struct memory_notify *mhp = v;
+	unsigned long start, end;
+
+	start = mhp->start_pfn << PAGE_SHIFT;
+	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
+
+	switch (val) {
+	case MEM_ONLINE:
+		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
+			start, end-start);
+		break;
+
+	case MEM_OFFLINE:
+		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
+			start, end-start);
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block crash_memhp_nb = {
+	.notifier_call = crash_memhp_notifier,
+	.priority = 0
+};
 #endif
 
+#if defined(CONFIG_HOTPLUG_CPU)
+static int crash_cpuhp_online(unsigned int cpu)
+{
+	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
+	return 0;
+}
+
+static int crash_cpuhp_offline(unsigned int cpu)
+{
+	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
+	return 0;
+}
+#endif
+
+static int __init crash_hotplug_init(void)
+{
+	int result = 0;
+
+#if defined(CONFIG_MEMORY_HOTPLUG)
+	register_memory_notifier(&crash_memhp_nb);
+#endif
+
+#if defined(CONFIG_HOTPLUG_CPU)
+	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
+				"crash/cpuhp",
+				crash_cpuhp_online, crash_cpuhp_offline);
+#endif
+
+	return result;
+}
+
+subsys_initcall(crash_hotplug_init);
+#endif /* CONFIG_CRASH_HOTPLUG */
-- 
2.27.0

[PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Eric DeVolder]

When a crash kernel is loaded via the kexec_file_load syscall, the
kernel places the various segments (ie crash kernel, crash initrd,
boot_params, elfcorehdr, purgatory, etc) in memory. For those
architectures that utilize purgatory, a hash digest of the segments
is calculated for integrity checking. This digest is embedded into
the purgatory image prior to placing purgatory in memory.

Since hotplug events cause changes to the elfcorehdr, purgatory
integrity checking fails (at crash time, and no kdump created).
As a result, this change explicitly excludes the elfcorehdr segment
from the list of segments used to create the digest. By doing so,
this permits changes to the elfcorehdr in response to hotplug events,
without having to also reload purgatory due to the change to the
digest.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/kexec_file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 801d0d0a5012..93bb0c0ce66c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
 	for (j = i = 0; i < image->nr_segments; i++) {
 		struct kexec_segment *ksegment;
 
+#ifdef CONFIG_CRASH_HOTPLUG
+		/* This segment excluded to allow future changes via hotplug */
+		if (image->elf_index_valid && (j == image->elf_index))
+			continue;
+#endif
+
 		ksegment = &image->segment[i];
 		/*
 		 * Skip purgatory as it will be modified once we put digest
-- 
2.27.0

[PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Eric DeVolder]

When a crash kernel is loaded via the kexec_file_load syscall, the
kernel places the various segments (ie crash kernel, crash initrd,
boot_params, elfcorehdr, purgatory, etc) in memory. For those
architectures that utilize purgatory, a hash digest of the segments
is calculated for integrity checking. This digest is embedded into
the purgatory image prior to placing purgatory in memory.

Since hotplug events cause changes to the elfcorehdr, purgatory
integrity checking fails (at crash time, and no kdump created).
As a result, this change explicitly excludes the elfcorehdr segment
from the list of segments used to create the digest. By doing so,
this permits changes to the elfcorehdr in response to hotplug events,
without having to also reload purgatory due to the change to the
digest.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/kexec_file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 801d0d0a5012..93bb0c0ce66c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
 	for (j = i = 0; i < image->nr_segments; i++) {
 		struct kexec_segment *ksegment;
 
+#ifdef CONFIG_CRASH_HOTPLUG
+		/* This segment excluded to allow future changes via hotplug */
+		if (image->elf_index_valid && (j == image->elf_index))
+			continue;
+#endif
+
 		ksegment = &image->segment[i];
 		/*
 		 * Skip purgatory as it will be modified once we put digest
-- 
2.27.0

[PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Eric DeVolder]

Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
still in the for_each_present_cpu() list when within the
crash_hotplug_handler(). Thus the CPU must be explicitly excluded
when building the new list of CPUs.

This change identifies in crash_hotplug_handler() the CPU to be
excluded, and the check for excluding the CPU in
crash_prepare_elf64_headers().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/crash_core.c | 10 ++++++++++
 kernel/kexec_file.c |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 524470e18883..b6942e04a188 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
 		/* Flag to differentiate between normal load and hotplug */
 		kexec_crash_image->hotplug_event = true;
 
+		/*
+		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
+		 * this callback, the CPU is still in the for_each_present_cpu()
+		 * list. Must explicitly look to exclude this CPU when building
+		 * new list.
+		 */
+		kexec_crash_image->offlinecpu =
+			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
+				(unsigned int)a : ~0U;
+
 		/* Now invoke arch-specific update handler */
 		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
 
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 93bb0c0ce66c..339995d42169 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
 
 	/* Prepare one phdr of type PT_NOTE for each present CPU */
 	for_each_present_cpu(cpu) {
+#ifdef CONFIG_CRASH_HOTPLUG
+		/* Skip the soon-to-be offlined cpu */
+		if (image->hotplug_event && (cpu == image->offlinecpu))
+			continue;
+#endif
 		phdr->p_type = PT_NOTE;
 		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
 		phdr->p_offset = phdr->p_paddr = notes_addr;
-- 
2.27.0

[PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Eric DeVolder]

Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
still in the for_each_present_cpu() list when within the
crash_hotplug_handler(). Thus the CPU must be explicitly excluded
when building the new list of CPUs.

This change identifies in crash_hotplug_handler() the CPU to be
excluded, and the check for excluding the CPU in
crash_prepare_elf64_headers().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 kernel/crash_core.c | 10 ++++++++++
 kernel/kexec_file.c |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 524470e18883..b6942e04a188 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
 		/* Flag to differentiate between normal load and hotplug */
 		kexec_crash_image->hotplug_event = true;
 
+		/*
+		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
+		 * this callback, the CPU is still in the for_each_present_cpu()
+		 * list. Must explicitly look to exclude this CPU when building
+		 * new list.
+		 */
+		kexec_crash_image->offlinecpu =
+			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
+				(unsigned int)a : ~0U;
+
 		/* Now invoke arch-specific update handler */
 		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
 
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 93bb0c0ce66c..339995d42169 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
 
 	/* Prepare one phdr of type PT_NOTE for each present CPU */
 	for_each_present_cpu(cpu) {
+#ifdef CONFIG_CRASH_HOTPLUG
+		/* Skip the soon-to-be offlined cpu */
+		if (image->hotplug_event && (cpu == image->offlinecpu))
+			continue;
+#endif
 		phdr->p_type = PT_NOTE;
 		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
 		phdr->p_offset = phdr->p_paddr = notes_addr;
-- 
2.27.0

[PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Eric DeVolder]

For x86_64, when CPU or memory is hot un/plugged, the crash
elfcorehdr, which describes the CPUs and memory in the system,
must also be updated.

To update the elfcorehdr for x86_64, a new elfcorehdr must be
generated from the available CPUs and memory. The new elfcorehdr
is prepared into a buffer, and if no errors occur, it is
installed over the top of the existing elfcorehdr.

In the patch 'crash hp: exclude elfcorehdr from the segment digest'
the need to update purgatory due to the change in elfcorehdr was
eliminated.  As a result, no changes to purgatory or boot_params
(as the elfcorehdr= kernel command line parameter pointer
remains unchanged and correct) are needed, just elfcorehdr.

To accommodate a growing number of resources via hotplug, the
elfcorehdr segment must be sufficiently large enough to accommodate
changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.

With this change, the kexec_file_load syscall (not kexec_load)
is supported. When loading the crash kernel via kexec_file_load,
the elfcorehdr is identified at load time in crash_load_segments().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9db41cce8d97..6215c51919db 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -25,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/memblock.h>
+#include <linux/highmem.h>
 
 #include <asm/processor.h>
 #include <asm/hardirq.h>
@@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
 	image->elf_headers = kbuf.buffer;
 	image->elf_headers_sz = kbuf.bufsz;
 
+#ifdef CONFIG_CRASH_HOTPLUG
+	/* Ensure elfcorehdr segment large enough for hotplug changes */
+	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
+	/* For marking as usable to crash kernel */
+	image->elf_headers_sz = kbuf.memsz;
+	/* Record the index of the elfcorehdr segment */
+	image->elf_index = image->nr_segments;
+	image->elf_index_valid = true;
+#else
 	kbuf.memsz = kbuf.bufsz;
+#endif
 	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
 	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 	ret = kexec_add_buffer(&kbuf);
@@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
 	return ret;
 }
 #endif /* CONFIG_KEXEC_FILE */
+
+#ifdef CONFIG_CRASH_HOTPLUG
+/**
+ * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
+ * is passed to the crash kernel via the elfcorehdr= parameter)
+ * must be updated with the new list of CPUs and memories. The new
+ * elfcorehdr is prepared in a kernel buffer, and if no errors,
+ * then it is written on top of the existing/old elfcorehdr.
+ *
+ * For hotplug changes to elfcorehdr to work, two conditions are
+ * needed:
+ * First, the segment containing the elfcorehdr must be large enough
+ * to permit a growing number of resources. See
+ * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
+ * Second, purgatory must explicitly exclude the elfcorehdr from the
+ * list of segments it checks (since the elfcorehdr changes and thus
+ * would require an update to purgatory itself to update the digest).
+ *
+ */
+void arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b)
+{
+	struct kexec_segment *ksegment;
+	unsigned char *ptr = NULL;
+	unsigned long elfsz = 0;
+	void *elfbuf = NULL;
+	unsigned long mem, memsz;
+
+	/* Must have valid elfcorehdr index */
+	if (!image->elf_index_valid) {
+		pr_err("crash hp: unable to locate elfcorehdr segment");
+		goto out;
+	}
+
+	ksegment = &image->segment[image->elf_index];
+	mem = ksegment->mem;
+	memsz = ksegment->memsz;
+
+	/*
+	 * Create the new elfcorehdr reflecting the changes to CPU and/or
+	 * memory resources. The elfcorehdr segment memsz must be
+	 * sufficiently large to accommodate increases due to hotplug
+	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
+	 */
+	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
+		pr_err("crash hp: unable to prepare elfcore headers");
+		goto out;
+	}
+	if (elfsz > memsz) {
+		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
+			elfsz, memsz);
+		goto out;
+	}
+
+	/*
+	 * At this point, we are all but assured of success.
+	 * Copy new elfcorehdr into destination.
+	 */
+	ptr = map_crash_pages(mem, memsz);
+	if (ptr) {
+		/* Temporarily invalidate the crash image while it is replaced */
+		xchg(&kexec_crash_image, NULL);
+		/* Write the new elfcorehdr into memory */
+		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
+		/* The crash image is now valid once again */
+		xchg(&kexec_crash_image, image);
+	}
+	unmap_crash_pages((void **)&ptr);
+	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
+
+out:
+	if (elfbuf)
+		vfree(elfbuf);
+}
+#endif /* CONFIG_CRASH_HOTPLUG */
-- 
2.27.0

[PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Eric DeVolder]

For x86_64, when CPU or memory is hot un/plugged, the crash
elfcorehdr, which describes the CPUs and memory in the system,
must also be updated.

To update the elfcorehdr for x86_64, a new elfcorehdr must be
generated from the available CPUs and memory. The new elfcorehdr
is prepared into a buffer, and if no errors occur, it is
installed over the top of the existing elfcorehdr.

In the patch 'crash hp: exclude elfcorehdr from the segment digest'
the need to update purgatory due to the change in elfcorehdr was
eliminated.  As a result, no changes to purgatory or boot_params
(as the elfcorehdr= kernel command line parameter pointer
remains unchanged and correct) are needed, just elfcorehdr.

To accommodate a growing number of resources via hotplug, the
elfcorehdr segment must be sufficiently large enough to accommodate
changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.

With this change, the kexec_file_load syscall (not kexec_load)
is supported. When loading the crash kernel via kexec_file_load,
the elfcorehdr is identified at load time in crash_load_segments().

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9db41cce8d97..6215c51919db 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -25,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/memblock.h>
+#include <linux/highmem.h>
 
 #include <asm/processor.h>
 #include <asm/hardirq.h>
@@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
 	image->elf_headers = kbuf.buffer;
 	image->elf_headers_sz = kbuf.bufsz;
 
+#ifdef CONFIG_CRASH_HOTPLUG
+	/* Ensure elfcorehdr segment large enough for hotplug changes */
+	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
+	/* For marking as usable to crash kernel */
+	image->elf_headers_sz = kbuf.memsz;
+	/* Record the index of the elfcorehdr segment */
+	image->elf_index = image->nr_segments;
+	image->elf_index_valid = true;
+#else
 	kbuf.memsz = kbuf.bufsz;
+#endif
 	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
 	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 	ret = kexec_add_buffer(&kbuf);
@@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
 	return ret;
 }
 #endif /* CONFIG_KEXEC_FILE */
+
+#ifdef CONFIG_CRASH_HOTPLUG
+/**
+ * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
+ * is passed to the crash kernel via the elfcorehdr= parameter)
+ * must be updated with the new list of CPUs and memories. The new
+ * elfcorehdr is prepared in a kernel buffer, and if no errors,
+ * then it is written on top of the existing/old elfcorehdr.
+ *
+ * For hotplug changes to elfcorehdr to work, two conditions are
+ * needed:
+ * First, the segment containing the elfcorehdr must be large enough
+ * to permit a growing number of resources. See
+ * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
+ * Second, purgatory must explicitly exclude the elfcorehdr from the
+ * list of segments it checks (since the elfcorehdr changes and thus
+ * would require an update to purgatory itself to update the digest).
+ *
+ */
+void arch_crash_hotplug_handler(struct kimage *image,
+	unsigned int hp_action, unsigned long a, unsigned long b)
+{
+	struct kexec_segment *ksegment;
+	unsigned char *ptr = NULL;
+	unsigned long elfsz = 0;
+	void *elfbuf = NULL;
+	unsigned long mem, memsz;
+
+	/* Must have valid elfcorehdr index */
+	if (!image->elf_index_valid) {
+		pr_err("crash hp: unable to locate elfcorehdr segment");
+		goto out;
+	}
+
+	ksegment = &image->segment[image->elf_index];
+	mem = ksegment->mem;
+	memsz = ksegment->memsz;
+
+	/*
+	 * Create the new elfcorehdr reflecting the changes to CPU and/or
+	 * memory resources. The elfcorehdr segment memsz must be
+	 * sufficiently large to accommodate increases due to hotplug
+	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
+	 */
+	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
+		pr_err("crash hp: unable to prepare elfcore headers");
+		goto out;
+	}
+	if (elfsz > memsz) {
+		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
+			elfsz, memsz);
+		goto out;
+	}
+
+	/*
+	 * At this point, we are all but assured of success.
+	 * Copy new elfcorehdr into destination.
+	 */
+	ptr = map_crash_pages(mem, memsz);
+	if (ptr) {
+		/* Temporarily invalidate the crash image while it is replaced */
+		xchg(&kexec_crash_image, NULL);
+		/* Write the new elfcorehdr into memory */
+		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
+		/* The crash image is now valid once again */
+		xchg(&kexec_crash_image, image);
+	}
+	unmap_crash_pages((void **)&ptr);
+	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
+
+out:
+	if (elfbuf)
+		vfree(elfbuf);
+}
+#endif /* CONFIG_CRASH_HOTPLUG */
-- 
2.27.0

[PATCH v4 10/10] crash hp: Add x86 crash hotplug support for kexec_load

[Eric DeVolder]

For kexec_file_load support, the loading of the crash kernel occurs
entirely within the kernel, and as such the elfcorehdr is readily
identified (so that it can be modified upon hotplug events).

This change enables support for kexec_load by identifying the
elfcorehdr segment in the arch_crash_hotplug_handler(), if it has
not already been identified.

In general, support for kexec_load requires corresponding changes
to the userspace kexec-tools utility. It is the responsibility of
the userspace kexec utility to ensure that:
 - the elfcorehdr segment is sufficiently large enough to accommodate
   hotplug changes, ala CRASH_HOTPLUG_ELFCOREHDR_SZ.
 - provides a purgatory that excludes the elfcorehdr from its list of
   run-time segments to check.
These changes to the userspace kexec utility are available, but not
yet accepted upstream.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/kernel/crash.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 6215c51919db..1b3e5e4fb965 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -458,6 +458,32 @@ void arch_crash_hotplug_handler(struct kimage *image,
 	void *elfbuf = NULL;
 	unsigned long mem, memsz;
 
+	/*
+	 * When the struct kimage is alloced, it is wiped to zero, so
+	 * the elf_index_valid defaults to false. It is set on the
+	 * kexec_file_load path, or here for kexec_load.
+	 */
+	if (!image->elf_index_valid) {
+		unsigned int n;
+
+		for (n = 0; n < image->nr_segments; n++) {
+			mem = image->segment[n].mem;
+			memsz = image->segment[n].memsz;
+			ptr = map_crash_pages(mem, memsz);
+			if (ptr) {
+				/* The segment containing elfcorehdr */
+				if ((ptr[0] == 0x7F) &&
+					(ptr[1] == 'E') &&
+					(ptr[2] == 'L') &&
+					(ptr[3] == 'F')) {
+					image->elf_index = (int)n;
+					image->elf_index_valid = true;
+				}
+			}
+			unmap_crash_pages((void **)&ptr);
+		}
+	}
+
 	/* Must have valid elfcorehdr index */
 	if (!image->elf_index_valid) {
 		pr_err("crash hp: unable to locate elfcorehdr segment");
-- 
2.27.0

[PATCH v4 10/10] crash hp: Add x86 crash hotplug support for kexec_load

[Eric DeVolder]

For kexec_file_load support, the loading of the crash kernel occurs
entirely within the kernel, and as such the elfcorehdr is readily
identified (so that it can be modified upon hotplug events).

This change enables support for kexec_load by identifying the
elfcorehdr segment in the arch_crash_hotplug_handler(), if it has
not already been identified.

In general, support for kexec_load requires corresponding changes
to the userspace kexec-tools utility. It is the responsibility of
the userspace kexec utility to ensure that:
 - the elfcorehdr segment is sufficiently large enough to accommodate
   hotplug changes, ala CRASH_HOTPLUG_ELFCOREHDR_SZ.
 - provides a purgatory that excludes the elfcorehdr from its list of
   run-time segments to check.
These changes to the userspace kexec utility are available, but not
yet accepted upstream.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 arch/x86/kernel/crash.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 6215c51919db..1b3e5e4fb965 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -458,6 +458,32 @@ void arch_crash_hotplug_handler(struct kimage *image,
 	void *elfbuf = NULL;
 	unsigned long mem, memsz;
 
+	/*
+	 * When the struct kimage is alloced, it is wiped to zero, so
+	 * the elf_index_valid defaults to false. It is set on the
+	 * kexec_file_load path, or here for kexec_load.
+	 */
+	if (!image->elf_index_valid) {
+		unsigned int n;
+
+		for (n = 0; n < image->nr_segments; n++) {
+			mem = image->segment[n].mem;
+			memsz = image->segment[n].memsz;
+			ptr = map_crash_pages(mem, memsz);
+			if (ptr) {
+				/* The segment containing elfcorehdr */
+				if ((ptr[0] == 0x7F) &&
+					(ptr[1] == 'E') &&
+					(ptr[2] == 'L') &&
+					(ptr[3] == 'F')) {
+					image->elf_index = (int)n;
+					image->elf_index_valid = true;
+				}
+			}
+			unmap_crash_pages((void **)&ptr);
+		}
+	}
+
 	/* Must have valid elfcorehdr index */
 	if (!image->elf_index_valid) {
 		pr_err("crash hp: unable to locate elfcorehdr segment");
-- 
2.27.0

Re: [PATCH v4 00/10] crash: Kernel handling of CPU and memory hot un/plug

[Baoquan He]

Hi Eric,

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> When the kdump service is loaded, if a CPU or memory is hot
> un/plugged, the crash elfcorehdr (for x86), which describes the CPUs
> and memory in the system, must also be updated, else the resulting
> vmcore is inaccurate (eg. missing either CPU context or memory
> regions).

This version looks much better. I am gonna to review this week and
give it a shot on a machine. Can you share your test steps? I have
knowledge about mem hotplug, but haven't tried cpu hotplug yet.

Thanks
Baoquan

> 
> The current solution utilizes udev to initiate an unload-then-reload
> of the kdump image (e. kernel, initrd, boot_params, puratory and
> elfcorehdr) by the userspace kexec utility. In previous posts I have
> outlined the significant performance problems related to offloading
> this activity to userspace.
> 
> This patchset introduces a generic crash hot un/plug handler that
> registers with the CPU and memory notifiers. Upon CPU or memory
> changes, this generic handler is invoked and performs important
> housekeeping, for example obtaining the appropriate lock, and then
> invokes an architecture specific handler to do the appropriate
> updates.
> 
> In the case of x86_64, the arch specific handler generates a new
> elfcorehdr, and overwrites the old one in memory. No involvement
> with userspace needed.
> 
> To realize the benefits/test this patchset, one must make a couple
> of minor changes to userspace:
> 
>  - Disable the udev rule for updating kdump on hot un/plug changes
>    Eg. on RHEL: rm -f /usr/lib/udev/rules.d/98-kexec.rules
>    or other technique to neuter the rule.
> 
>  - Change to the kexec_file_load for loading the kdump kernel:
>    Eg. on RHEL: in /usr/bin/kdumpctl, change to:
>     standard_kexec_args="-p -d -s"
>    which adds the -s to select kexec_file_load syscall.
> 
> This patchset supports kexec_load with a modified kexec userspace
> utility, and a working changeset to the kexec userspace utility
> is provided here (and to use, the above change to standard_kexec_args
> would be, for example, to append --hotplug-size=262144 instead of -s).
> 
>  diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
>  index 9826f6d..06adb7e 100644
>  --- a/kexec/arch/i386/crashdump-x86.c
>  +++ b/kexec/arch/i386/crashdump-x86.c
>  @@ -48,6 +48,7 @@
>   #include <x86/x86-linux.h>
>   
>   extern struct arch_options_t arch_options;
>  +extern unsigned long long hotplug_size;
>   
>   static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
>   				  struct crash_elf_info *elf_info)
>  @@ -975,6 +976,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
>   	} else {
>   		memsz = bufsz;
>   	}
>  +
>  +    /* If hotplug support enabled, use that size */
>  +    if (hotplug_size) {
>  +        memsz = hotplug_size;
>  +    }
>  +
>  +    info->elfcorehdr =
>   	elfcorehdr = add_buffer(info, tmp, bufsz, memsz, align, min_base,
>   							max_addr, -1);
>   	dbgprintf("Created elf header segment at 0x%lx\n", elfcorehdr);
>  diff --git a/kexec/kexec.c b/kexec/kexec.c
>  index f63b36b..9569d9a 100644
>  --- a/kexec/kexec.c
>  +++ b/kexec/kexec.c
>  @@ -58,6 +58,7 @@
>   
>   unsigned long long mem_min = 0;
>   unsigned long long mem_max = ULONG_MAX;
>  +unsigned long long hotplug_size = 0;
>   static unsigned long kexec_flags = 0;
>   /* Flags for kexec file (fd) based syscall */
>   static unsigned long kexec_file_flags = 0;
>  @@ -672,6 +673,12 @@ static void update_purgatory(struct kexec_info *info)
>   		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>   			continue;
>   		}
>  +        /* Don't include elfcorehdr in the checksum, if hotplug
>  +         * support enabled.
>  +         */
>  +        if (hotplug_size && (info->segment[i].mem == (void *)info->elfcorehdr)) {
>  +			continue;
>  +		}
>   		sha256_update(&ctx, info->segment[i].buf,
>   			      info->segment[i].bufsz);
>   		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
>  @@ -1504,6 +1511,17 @@ int main(int argc, char *argv[])
>   		case OPT_PRINT_CKR_SIZE:
>   			print_crashkernel_region_size();
>   			return 0;
>  +		case OPT_HOTPLUG_SIZE:
>  +            /* Reserved the specified size for hotplug growth */
>  +			hotplug_size = strtoul(optarg, &endptr, 0);
>  +			if (*endptr) {
>  +				fprintf(stderr,
>  +					"Bad option value in --hotplug-size=%s\n",
>  +					optarg);
>  +				usage();
>  +				return 1;
>  +			}
>  +			break;
>   		default:
>   			break;
>   		}
>  diff --git a/kexec/kexec.h b/kexec/kexec.h
>  index 595dd68..b30dda4 100644
>  --- a/kexec/kexec.h
>  +++ b/kexec/kexec.h
>  @@ -169,6 +169,7 @@ struct kexec_info {
>   	int command_line_len;
>   
>   	int skip_checks;
>  +    unsigned long elfcorehdr;
>    };
>   
>   struct arch_map_entry {
>  @@ -231,7 +232,8 @@ extern int file_types;
>   #define OPT_PRINT_CKR_SIZE	262
>   #define OPT_LOAD_LIVE_UPDATE	263
>   #define OPT_EXEC_LIVE_UPDATE	264
>  -#define OPT_MAX			265
>  +#define OPT_HOTPLUG_SIZE	265
>  +#define OPT_MAX			266
>   #define KEXEC_OPTIONS \
>   	{ "help",		0, 0, OPT_HELP }, \
>   	{ "version",		0, 0, OPT_VERSION }, \
>  @@ -258,6 +260,7 @@ extern int file_types;
>   	{ "debug",		0, 0, OPT_DEBUG }, \
>   	{ "status",		0, 0, OPT_STATUS }, \
>   	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  +	{ "hotplug-size",     2, 0, OPT_HOTPLUG_SIZE }, \
>   
>   #define KEXEC_OPT_STR "h?vdfixyluet:pscaS"
>  
> 
> Regards,
> eric
> ---
> v4: 9feb2022
>  - Refactored patches per Baoquan suggestsions.
>  - A few corrections, per Baoquan.
> 
> v3: 10jan2022
>  https://lkml.org/lkml/2022/1/10/1212
>  - Rebasing per Baoquan He request.
>  - Changed memory notifier per David Hildenbrand.
>  - Providing example kexec userspace change in cover letter.
> 
> RFC v2: 7dec2021
>  https://lkml.org/lkml/2021/12/7/1088
>  - Acting upon Baoquan He suggestion of removing elfcorehdr from
>    the purgatory list of segments, removed purgatory code from
>    patchset, and it is signficiantly simpler now.
> 
> RFC v1: 18nov2021
>  https://lkml.org/lkml/2021/11/18/845
>  - working patchset demonstrating kernel handling of hotplug
>    updates to x86 elfcorehdr for kexec_file_load
> 
> RFC: 14dec2020
>  https://lkml.org/lkml/2020/12/14/532
>  - proposed concept of allowing kernel to handle hotplug update
>    of elfcorehdr
> ---
> 
> Eric DeVolder (10):
>   crash: fix minor typo/bug in debug message
>   crash hp: Introduce CRASH_HOTPLUG configuration options
>   crash hp: definitions and prototype changes
>   crash hp: prototype change for crash_prepare_elf64_headers
>   crash hp: introduce helper functions un/map_crash_pages
>   crash hp: generic crash hotplug support infrastructure
>   crash hp: exclude elfcorehdr from the segment digest
>   crash hp: exclude hot remove cpu from elfcorehdr notes
>   crash hp: Add x86 crash hotplug support for kexec_file_load
>   crash hp: Add x86 crash hotplug support for kexec_load
> 
>  arch/arm64/kernel/machine_kexec_file.c |   6 +-
>  arch/powerpc/kexec/file_load_64.c      |   2 +-
>  arch/x86/Kconfig                       |  26 +++++
>  arch/x86/kernel/crash.c                | 123 ++++++++++++++++++++-
>  include/linux/kexec.h                  |  23 +++-
>  kernel/crash_core.c                    | 146 +++++++++++++++++++++++++
>  kernel/kexec_file.c                    |  15 ++-
>  7 files changed, 331 insertions(+), 10 deletions(-)
> 
> -- 
> 2.27.0
> 

[PATCH v4 00/10] crash: Kernel handling of CPU and memory hot un/plug

[Baoquan He]

Hi Eric,

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> When the kdump service is loaded, if a CPU or memory is hot
> un/plugged, the crash elfcorehdr (for x86), which describes the CPUs
> and memory in the system, must also be updated, else the resulting
> vmcore is inaccurate (eg. missing either CPU context or memory
> regions).

This version looks much better. I am gonna to review this week and
give it a shot on a machine. Can you share your test steps? I have
knowledge about mem hotplug, but haven't tried cpu hotplug yet.

Thanks
Baoquan

> 
> The current solution utilizes udev to initiate an unload-then-reload
> of the kdump image (e. kernel, initrd, boot_params, puratory and
> elfcorehdr) by the userspace kexec utility. In previous posts I have
> outlined the significant performance problems related to offloading
> this activity to userspace.
> 
> This patchset introduces a generic crash hot un/plug handler that
> registers with the CPU and memory notifiers. Upon CPU or memory
> changes, this generic handler is invoked and performs important
> housekeeping, for example obtaining the appropriate lock, and then
> invokes an architecture specific handler to do the appropriate
> updates.
> 
> In the case of x86_64, the arch specific handler generates a new
> elfcorehdr, and overwrites the old one in memory. No involvement
> with userspace needed.
> 
> To realize the benefits/test this patchset, one must make a couple
> of minor changes to userspace:
> 
>  - Disable the udev rule for updating kdump on hot un/plug changes
>    Eg. on RHEL: rm -f /usr/lib/udev/rules.d/98-kexec.rules
>    or other technique to neuter the rule.
> 
>  - Change to the kexec_file_load for loading the kdump kernel:
>    Eg. on RHEL: in /usr/bin/kdumpctl, change to:
>     standard_kexec_args="-p -d -s"
>    which adds the -s to select kexec_file_load syscall.
> 
> This patchset supports kexec_load with a modified kexec userspace
> utility, and a working changeset to the kexec userspace utility
> is provided here (and to use, the above change to standard_kexec_args
> would be, for example, to append --hotplug-size=262144 instead of -s).
> 
>  diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
>  index 9826f6d..06adb7e 100644
>  --- a/kexec/arch/i386/crashdump-x86.c
>  +++ b/kexec/arch/i386/crashdump-x86.c
>  @@ -48,6 +48,7 @@
>   #include <x86/x86-linux.h>
>   
>   extern struct arch_options_t arch_options;
>  +extern unsigned long long hotplug_size;
>   
>   static int get_kernel_page_offset(struct kexec_info *UNUSED(info),
>   				  struct crash_elf_info *elf_info)
>  @@ -975,6 +976,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
>   	} else {
>   		memsz = bufsz;
>   	}
>  +
>  +    /* If hotplug support enabled, use that size */
>  +    if (hotplug_size) {
>  +        memsz = hotplug_size;
>  +    }
>  +
>  +    info->elfcorehdr =
>   	elfcorehdr = add_buffer(info, tmp, bufsz, memsz, align, min_base,
>   							max_addr, -1);
>   	dbgprintf("Created elf header segment at 0x%lx\n", elfcorehdr);
>  diff --git a/kexec/kexec.c b/kexec/kexec.c
>  index f63b36b..9569d9a 100644
>  --- a/kexec/kexec.c
>  +++ b/kexec/kexec.c
>  @@ -58,6 +58,7 @@
>   
>   unsigned long long mem_min = 0;
>   unsigned long long mem_max = ULONG_MAX;
>  +unsigned long long hotplug_size = 0;
>   static unsigned long kexec_flags = 0;
>   /* Flags for kexec file (fd) based syscall */
>   static unsigned long kexec_file_flags = 0;
>  @@ -672,6 +673,12 @@ static void update_purgatory(struct kexec_info *info)
>   		if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
>   			continue;
>   		}
>  +        /* Don't include elfcorehdr in the checksum, if hotplug
>  +         * support enabled.
>  +         */
>  +        if (hotplug_size && (info->segment[i].mem == (void *)info->elfcorehdr)) {
>  +			continue;
>  +		}
>   		sha256_update(&ctx, info->segment[i].buf,
>   			      info->segment[i].bufsz);
>   		nullsz = info->segment[i].memsz - info->segment[i].bufsz;
>  @@ -1504,6 +1511,17 @@ int main(int argc, char *argv[])
>   		case OPT_PRINT_CKR_SIZE:
>   			print_crashkernel_region_size();
>   			return 0;
>  +		case OPT_HOTPLUG_SIZE:
>  +            /* Reserved the specified size for hotplug growth */
>  +			hotplug_size = strtoul(optarg, &endptr, 0);
>  +			if (*endptr) {
>  +				fprintf(stderr,
>  +					"Bad option value in --hotplug-size=%s\n",
>  +					optarg);
>  +				usage();
>  +				return 1;
>  +			}
>  +			break;
>   		default:
>   			break;
>   		}
>  diff --git a/kexec/kexec.h b/kexec/kexec.h
>  index 595dd68..b30dda4 100644
>  --- a/kexec/kexec.h
>  +++ b/kexec/kexec.h
>  @@ -169,6 +169,7 @@ struct kexec_info {
>   	int command_line_len;
>   
>   	int skip_checks;
>  +    unsigned long elfcorehdr;
>    };
>   
>   struct arch_map_entry {
>  @@ -231,7 +232,8 @@ extern int file_types;
>   #define OPT_PRINT_CKR_SIZE	262
>   #define OPT_LOAD_LIVE_UPDATE	263
>   #define OPT_EXEC_LIVE_UPDATE	264
>  -#define OPT_MAX			265
>  +#define OPT_HOTPLUG_SIZE	265
>  +#define OPT_MAX			266
>   #define KEXEC_OPTIONS \
>   	{ "help",		0, 0, OPT_HELP }, \
>   	{ "version",		0, 0, OPT_VERSION }, \
>  @@ -258,6 +260,7 @@ extern int file_types;
>   	{ "debug",		0, 0, OPT_DEBUG }, \
>   	{ "status",		0, 0, OPT_STATUS }, \
>   	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  +	{ "hotplug-size",     2, 0, OPT_HOTPLUG_SIZE }, \
>   
>   #define KEXEC_OPT_STR "h?vdfixyluet:pscaS"
>  
> 
> Regards,
> eric
> ---
> v4: 9feb2022
>  - Refactored patches per Baoquan suggestsions.
>  - A few corrections, per Baoquan.
> 
> v3: 10jan2022
>  https://lkml.org/lkml/2022/1/10/1212
>  - Rebasing per Baoquan He request.
>  - Changed memory notifier per David Hildenbrand.
>  - Providing example kexec userspace change in cover letter.
> 
> RFC v2: 7dec2021
>  https://lkml.org/lkml/2021/12/7/1088
>  - Acting upon Baoquan He suggestion of removing elfcorehdr from
>    the purgatory list of segments, removed purgatory code from
>    patchset, and it is signficiantly simpler now.
> 
> RFC v1: 18nov2021
>  https://lkml.org/lkml/2021/11/18/845
>  - working patchset demonstrating kernel handling of hotplug
>    updates to x86 elfcorehdr for kexec_file_load
> 
> RFC: 14dec2020
>  https://lkml.org/lkml/2020/12/14/532
>  - proposed concept of allowing kernel to handle hotplug update
>    of elfcorehdr
> ---
> 
> Eric DeVolder (10):
>   crash: fix minor typo/bug in debug message
>   crash hp: Introduce CRASH_HOTPLUG configuration options
>   crash hp: definitions and prototype changes
>   crash hp: prototype change for crash_prepare_elf64_headers
>   crash hp: introduce helper functions un/map_crash_pages
>   crash hp: generic crash hotplug support infrastructure
>   crash hp: exclude elfcorehdr from the segment digest
>   crash hp: exclude hot remove cpu from elfcorehdr notes
>   crash hp: Add x86 crash hotplug support for kexec_file_load
>   crash hp: Add x86 crash hotplug support for kexec_load
> 
>  arch/arm64/kernel/machine_kexec_file.c |   6 +-
>  arch/powerpc/kexec/file_load_64.c      |   2 +-
>  arch/x86/Kconfig                       |  26 +++++
>  arch/x86/kernel/crash.c                | 123 ++++++++++++++++++++-
>  include/linux/kexec.h                  |  23 +++-
>  kernel/crash_core.c                    | 146 +++++++++++++++++++++++++
>  kernel/kexec_file.c                    |  15 ++-
>  7 files changed, 331 insertions(+), 10 deletions(-)
> 
> -- 
> 2.27.0
> 

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> Support for CPU and memory hotplug for crash is controlled by the
> CRASH_HOTPLUG configuration option, introduced by this patch.
> 
> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> also introduced with this patch.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ebe8fc76949a..4e3374edab02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>  	  (CONFIG_RELOCATABLE=y).
>  	  For more details see Documentation/admin-guide/kdump/kdump.rst
>  
> +config CRASH_HOTPLUG
> +	bool "kernel updates of crash elfcorehdr"
> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> +	help
> +	  Enable the kernel to update the crash elfcorehdr (which contains
> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> +	  plug/unplug change notifications via udev in order to
> +	  unload-then-reload the crash kernel so that the list of CPUs and
> +	  memory regions is kept up-to-date. Note that the udev CPU and
> +	  memory change notifications still occur (however, userspace is not
> +	  required to monitor for crash dump purposes).
> +
> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> +	depends on CRASH_HOTPLUG
> +	int
> +	default 131072
> +	help
> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> +	  The 128KiB default is sized so that it can accommodate 2048
> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> +	  region of memory.
> +	  For example, this size can accommodate hotplugging a machine
> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> +	  with 1024 1GiB memory DIMMs).

This example of memory could be a little misleading. The memory regions
may not be related to memory DIMMs. System could split them into many
smaller regions during bootup.

Otherwise, this patch looks good to me.

> +
>  config KEXEC_JUMP
>  	bool "kexec jump"
>  	depends on KEXEC && HIBERNATION
> -- 
> 2.27.0
> 

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> Support for CPU and memory hotplug for crash is controlled by the
> CRASH_HOTPLUG configuration option, introduced by this patch.
> 
> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> also introduced with this patch.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ebe8fc76949a..4e3374edab02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>  	  (CONFIG_RELOCATABLE=y).
>  	  For more details see Documentation/admin-guide/kdump/kdump.rst
>  
> +config CRASH_HOTPLUG
> +	bool "kernel updates of crash elfcorehdr"
> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> +	help
> +	  Enable the kernel to update the crash elfcorehdr (which contains
> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> +	  plug/unplug change notifications via udev in order to
> +	  unload-then-reload the crash kernel so that the list of CPUs and
> +	  memory regions is kept up-to-date. Note that the udev CPU and
> +	  memory change notifications still occur (however, userspace is not
> +	  required to monitor for crash dump purposes).
> +
> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> +	depends on CRASH_HOTPLUG
> +	int
> +	default 131072
> +	help
> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> +	  The 128KiB default is sized so that it can accommodate 2048
> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> +	  region of memory.
> +	  For example, this size can accommodate hotplugging a machine
> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> +	  with 1024 1GiB memory DIMMs).

This example of memory could be a little misleading. The memory regions
may not be related to memory DIMMs. System could split them into many
smaller regions during bootup.

Otherwise, this patch looks good to me.

> +
>  config KEXEC_JUMP
>  	bool "kexec jump"
>  	depends on KEXEC && HIBERNATION
> -- 
> 2.27.0
> 

Re: [PATCH v4 03/10] crash hp: definitions and prototype changes

[Baoquan He]

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> This change adds members to struct kimage to facilitate crash
> hotplug support.
> 
> This change also defines crash hotplug events and associated
> prototypes.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  include/linux/kexec.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 0c994ae37729..13c238c2da48 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -299,6 +299,13 @@ struct kimage {
>  
>  	/* Information for loading purgatory */
>  	struct purgatory_info purgatory_info;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	bool hotplug_event;
> +	int offlinecpu;
> +	bool elf_index_valid;
> +	int elf_index;
> +#endif
>  #endif
>  
>  #ifdef CONFIG_IMA_KEXEC
> @@ -315,6 +322,15 @@ struct kimage {
>  	unsigned long elf_load_addr;
>  };
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +void arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b);

Seems this patch can be merged with patch 6/10. I don't see prototype
change in this patch.

> +#define KEXEC_CRASH_HP_REMOVE_CPU   0
> +#define KEXEC_CRASH_HP_ADD_CPU      1
> +#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
> +#define KEXEC_CRASH_HP_ADD_MEMORY   3
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>  /* kexec interface functions */
>  extern void machine_kexec(struct kimage *image);
>  extern int machine_kexec_prepare(struct kimage *image);
> -- 
> 2.27.0
> 

[PATCH v4 03/10] crash hp: definitions and prototype changes

[Baoquan He]

On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> This change adds members to struct kimage to facilitate crash
> hotplug support.
> 
> This change also defines crash hotplug events and associated
> prototypes.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  include/linux/kexec.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 0c994ae37729..13c238c2da48 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -299,6 +299,13 @@ struct kimage {
>  
>  	/* Information for loading purgatory */
>  	struct purgatory_info purgatory_info;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	bool hotplug_event;
> +	int offlinecpu;
> +	bool elf_index_valid;
> +	int elf_index;
> +#endif
>  #endif
>  
>  #ifdef CONFIG_IMA_KEXEC
> @@ -315,6 +322,15 @@ struct kimage {
>  	unsigned long elf_load_addr;
>  };
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +void arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b);

Seems this patch can be merged with patch 6/10. I don't see prototype
change in this patch.

> +#define KEXEC_CRASH_HP_REMOVE_CPU   0
> +#define KEXEC_CRASH_HP_ADD_CPU      1
> +#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
> +#define KEXEC_CRASH_HP_ADD_MEMORY   3
> +#endif /* CONFIG_CRASH_HOTPLUG */
> +
>  /* kexec interface functions */
>  extern void machine_kexec(struct kimage *image);
>  extern int machine_kexec_prepare(struct kimage *image);
> -- 
> 2.27.0
> 

Re: [PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> From within crash_prepare_elf64_headers() there is a need to
> reference the struct kimage hotplug members. As such, this
> change passes the struct kimage as a parameter to the
> crash_prepare_elf64_headers().

One more sentence can be added"

"This is preparation for later patch, no functionality change."

Otherwise, this patch looks good to me.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/arm64/kernel/machine_kexec_file.c | 6 +++---
>  arch/powerpc/kexec/file_load_64.c      | 2 +-
>  arch/x86/kernel/crash.c                | 3 ++-
>  include/linux/kexec.h                  | 5 +++--
>  kernel/kexec_file.c                    | 4 ++--
>  5 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 59c648d51848..7dbafb42ecf2 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>  	return kexec_image_post_load_cleanup_default(image);
>  }
>  
> -static int prepare_elf_headers(void **addr, unsigned long *sz)
> +static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
>  {
>  	struct crash_mem *cmem;
>  	unsigned int nr_ranges;
> @@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>  	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
>  
>  	if (!ret)
> -		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
> +		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
>  
>  	kfree(cmem);
>  	return ret;
> @@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
>  
>  	/* load elf core header */
>  	if (image->type == KEXEC_TYPE_CRASH) {
> -		ret = prepare_elf_headers(&headers, &headers_sz);
> +		ret = prepare_elf_headers(image, &headers, &headers_sz);
>  		if (ret) {
>  			pr_err("Preparing elf core header failed\n");
>  			goto out_err;
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index b4981b651d9a..07da6bf1cf24 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>  		goto out;
>  
>  	/* Setup elfcorehdr segment */
> -	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
> +	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
>  	if (ret) {
>  		pr_err("Failed to prepare elf headers for the core\n");
>  		goto out;
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9730c88530fc..9db41cce8d97 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
>  		goto out;
>  
>  	/* By default prepare 64bit headers */
> -	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
> +	ret =  crash_prepare_elf64_headers(image, cmem,
> +				IS_ENABLED(CONFIG_X86_64), addr, sz);
>  
>  out:
>  	vfree(cmem);
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 13c238c2da48..b11d75a6b2bc 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -221,8 +221,9 @@ struct crash_mem {
>  extern int crash_exclude_mem_range(struct crash_mem *mem,
>  				   unsigned long long mstart,
>  				   unsigned long long mend);
> -extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> -				       void **addr, unsigned long *sz);
> +extern int crash_prepare_elf64_headers(struct kimage *image,
> +	struct crash_mem *mem, int kernel_map,
> +	void **addr, unsigned long *sz);
>  #endif /* CONFIG_KEXEC_FILE */
>  
>  #ifdef CONFIG_KEXEC_ELF
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 8347fc158d2b..801d0d0a5012 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
>  	return 0;
>  }
>  
> -int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> -			  void **addr, unsigned long *sz)
> +int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
> +	int kernel_map, void **addr, unsigned long *sz)
>  {
>  	Elf64_Ehdr *ehdr;
>  	Elf64_Phdr *phdr;
> -- 
> 2.27.0
> 

[PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> From within crash_prepare_elf64_headers() there is a need to
> reference the struct kimage hotplug members. As such, this
> change passes the struct kimage as a parameter to the
> crash_prepare_elf64_headers().

One more sentence can be added"

"This is preparation for later patch, no functionality change."

Otherwise, this patch looks good to me.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/arm64/kernel/machine_kexec_file.c | 6 +++---
>  arch/powerpc/kexec/file_load_64.c      | 2 +-
>  arch/x86/kernel/crash.c                | 3 ++-
>  include/linux/kexec.h                  | 5 +++--
>  kernel/kexec_file.c                    | 4 ++--
>  5 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 59c648d51848..7dbafb42ecf2 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>  	return kexec_image_post_load_cleanup_default(image);
>  }
>  
> -static int prepare_elf_headers(void **addr, unsigned long *sz)
> +static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
>  {
>  	struct crash_mem *cmem;
>  	unsigned int nr_ranges;
> @@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>  	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
>  
>  	if (!ret)
> -		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
> +		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
>  
>  	kfree(cmem);
>  	return ret;
> @@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
>  
>  	/* load elf core header */
>  	if (image->type == KEXEC_TYPE_CRASH) {
> -		ret = prepare_elf_headers(&headers, &headers_sz);
> +		ret = prepare_elf_headers(image, &headers, &headers_sz);
>  		if (ret) {
>  			pr_err("Preparing elf core header failed\n");
>  			goto out_err;
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index b4981b651d9a..07da6bf1cf24 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>  		goto out;
>  
>  	/* Setup elfcorehdr segment */
> -	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
> +	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
>  	if (ret) {
>  		pr_err("Failed to prepare elf headers for the core\n");
>  		goto out;
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9730c88530fc..9db41cce8d97 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
>  		goto out;
>  
>  	/* By default prepare 64bit headers */
> -	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
> +	ret =  crash_prepare_elf64_headers(image, cmem,
> +				IS_ENABLED(CONFIG_X86_64), addr, sz);
>  
>  out:
>  	vfree(cmem);
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 13c238c2da48..b11d75a6b2bc 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -221,8 +221,9 @@ struct crash_mem {
>  extern int crash_exclude_mem_range(struct crash_mem *mem,
>  				   unsigned long long mstart,
>  				   unsigned long long mend);
> -extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> -				       void **addr, unsigned long *sz);
> +extern int crash_prepare_elf64_headers(struct kimage *image,
> +	struct crash_mem *mem, int kernel_map,
> +	void **addr, unsigned long *sz);
>  #endif /* CONFIG_KEXEC_FILE */
>  
>  #ifdef CONFIG_KEXEC_ELF
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 8347fc158d2b..801d0d0a5012 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
>  	return 0;
>  }
>  
> -int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> -			  void **addr, unsigned long *sz)
> +int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
> +	int kernel_map, void **addr, unsigned long *sz)
>  {
>  	Elf64_Ehdr *ehdr;
>  	Elf64_Phdr *phdr;
> -- 
> 2.27.0
> 

Re: [PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> This change introduces two new functions un/map_crash_pages()
> which are used to enable/disable access to the segments in the
> crash memory region. (Upon loading of a crash kernel, the
> crash memory regions are made inaccessible for integrity purposes.)
> 
> For example, on x86_64, one of the segments is the elfcorehdr,
> which contains the list of CPUs and memories. This segment
> needs to be modified in response to hotplug events. These functions
> are used to obtain (and subsequenntly release) access to the crash
> memory region in order to make the modifications.
> 
> QUESTION: These might need to be in arch/x86 as I'm not certain
> the implementatin is valid for all archs?

Since only x86_64 uses them, I would suggest putting them into x86_64,
near the caller.

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  include/linux/kexec.h |  2 ++
>  kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index b11d75a6b2bc..e00c373c4095 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -324,6 +324,8 @@ struct kimage {
>  };
>  
>  #ifdef CONFIG_CRASH_HOTPLUG
> +void *map_crash_pages(unsigned long paddr, unsigned long size);
> +void unmap_crash_pages(void **ptr);
>  void arch_crash_hotplug_handler(struct kimage *image,
>  	unsigned int hp_action, unsigned long a, unsigned long b);
>  #define KEXEC_CRASH_HP_REMOVE_CPU   0
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..0ff06d0698ad 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -9,6 +9,7 @@
>  #include <linux/init.h>
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
> +#include <linux/highmem.h>
>  
>  #include <asm/page.h>
>  #include <asm/sections.h>
> @@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
>  }
>  
>  subsys_initcall(crash_save_vmcoreinfo_init);
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +void *map_crash_pages(unsigned long paddr, unsigned long size)
> +{
> +	/*
> +	 * NOTE: The addresses and sizes passed to this routine have
> +	 * already been fully aligned on page boundaries. There is no
> +	 * need for massaging the address or size.
> +	 */
> +	void *ptr = NULL;
> +
> +	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
> +	if (size > 0) {
> +		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
> +
> +		ptr = kmap(page);
> +	}
> +
> +	return ptr;
> +}
> +
> +void unmap_crash_pages(void **ptr)
> +{
> +	if (ptr) {
> +		if (*ptr)
> +			kunmap(*ptr);
> +		*ptr = NULL;
> +	}
> +}
> +#endif
> +
> -- 
> 2.27.0
> 

[PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> This change introduces two new functions un/map_crash_pages()
> which are used to enable/disable access to the segments in the
> crash memory region. (Upon loading of a crash kernel, the
> crash memory regions are made inaccessible for integrity purposes.)
> 
> For example, on x86_64, one of the segments is the elfcorehdr,
> which contains the list of CPUs and memories. This segment
> needs to be modified in response to hotplug events. These functions
> are used to obtain (and subsequenntly release) access to the crash
> memory region in order to make the modifications.
> 
> QUESTION: These might need to be in arch/x86 as I'm not certain
> the implementatin is valid for all archs?

Since only x86_64 uses them, I would suggest putting them into x86_64,
near the caller.

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  include/linux/kexec.h |  2 ++
>  kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index b11d75a6b2bc..e00c373c4095 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -324,6 +324,8 @@ struct kimage {
>  };
>  
>  #ifdef CONFIG_CRASH_HOTPLUG
> +void *map_crash_pages(unsigned long paddr, unsigned long size);
> +void unmap_crash_pages(void **ptr);
>  void arch_crash_hotplug_handler(struct kimage *image,
>  	unsigned int hp_action, unsigned long a, unsigned long b);
>  #define KEXEC_CRASH_HP_REMOVE_CPU   0
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..0ff06d0698ad 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -9,6 +9,7 @@
>  #include <linux/init.h>
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
> +#include <linux/highmem.h>
>  
>  #include <asm/page.h>
>  #include <asm/sections.h>
> @@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
>  }
>  
>  subsys_initcall(crash_save_vmcoreinfo_init);
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +void *map_crash_pages(unsigned long paddr, unsigned long size)
> +{
> +	/*
> +	 * NOTE: The addresses and sizes passed to this routine have
> +	 * already been fully aligned on page boundaries. There is no
> +	 * need for massaging the address or size.
> +	 */
> +	void *ptr = NULL;
> +
> +	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
> +	if (size > 0) {
> +		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
> +
> +		ptr = kmap(page);
> +	}
> +
> +	return ptr;
> +}
> +
> +void unmap_crash_pages(void **ptr)
> +{
> +	if (ptr) {
> +		if (*ptr)
> +			kunmap(*ptr);
> +		*ptr = NULL;
> +	}
> +}
> +#endif
> +
> -- 
> 2.27.0
> 

Re: [PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> This patch introduces a generic crash hot plug/unplug infrastructure
> for CPU and memory changes. Upon CPU and memory changes, a generic
> crash_hotplug_handler() obtains the appropriate lock, does some
> important house keeping and then dispatches the hot plug/unplug event
> to the architecture specific arch_crash_hotplug_handler(), and when
> that handler returns, the lock is released.
> 
> This patch modifies crash_core.c to implement a subsys_initcall()
> function that installs handlers for hot plug/unplug events. If CPU
> hotplug is enabled, then cpuhp_setup_state() is invoked to register a
> handler for CPU changes. Similarly, if memory hotplug is enabled, then
> register_memory_notifier() is invoked to install a handler for memory
> changes. These handlers in turn invoke the common generic handler
> crash_hotplug_handler().
> 
> On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
> CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
> the CPU still shows up in foreach_present_cpu() during the regeneration
> of the new CPU list, thus the need to explicitly check and exclude the
> soon-to-be offlined CPU in crash_prepare_elf64_headers().
> 
> On the memory side, each un/plugged memory block passes through the
> handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
> memory events, one for each 128MiB memblock.

This looks good to me. As I commented in patch 3, these both may need be
merged.

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 0ff06d0698ad..524470e18883 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -10,12 +10,16 @@
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
>  #include <linux/highmem.h>
> +#include <linux/memory.h>
> +#include <linux/cpuhotplug.h>
>  
>  #include <asm/page.h>
>  #include <asm/sections.h>
>  
>  #include <crypto/sha1.h>
>  
> +#include "kexec_internal.h"
> +
>  /* vmcoreinfo stuff */
>  unsigned char *vmcoreinfo_data;
>  size_t vmcoreinfo_size;
> @@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
>  		*ptr = NULL;
>  	}
>  }
> +
> +void __weak arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b)
> +{
> +	pr_warn("crash hp: %s not implemented", __func__);
> +}
> +
> +static void crash_hotplug_handler(unsigned int hp_action,
> +	unsigned long a, unsigned long b)
> +{
> +	/* Obtain lock while changing crash information */
> +	if (!mutex_trylock(&kexec_mutex))
> +		return;
> +
> +	/* Check kdump is loaded */
> +	if (kexec_crash_image) {
> +		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
> +			a, b);
> +
> +		/* Needed in order for the segments to be updated */
> +		arch_kexec_unprotect_crashkres();
> +
> +		/* Flag to differentiate between normal load and hotplug */
> +		kexec_crash_image->hotplug_event = true;
> +
> +		/* Now invoke arch-specific update handler */
> +		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
> +
> +		/* No longer handling a hotplug event */
> +		kexec_crash_image->hotplug_event = false;
> +
> +		/* Change back to read-only */
> +		arch_kexec_protect_crashkres();
> +	}
> +
> +	/* Release lock now that update complete */
> +	mutex_unlock(&kexec_mutex);
> +}
> +
> +#if defined(CONFIG_MEMORY_HOTPLUG)
> +static int crash_memhp_notifier(struct notifier_block *nb,
> +	unsigned long val, void *v)
> +{
> +	struct memory_notify *mhp = v;
> +	unsigned long start, end;
> +
> +	start = mhp->start_pfn << PAGE_SHIFT;
> +	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
> +
> +	switch (val) {
> +	case MEM_ONLINE:
> +		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
> +			start, end-start);
> +		break;
> +
> +	case MEM_OFFLINE:
> +		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
> +			start, end-start);
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block crash_memhp_nb = {
> +	.notifier_call = crash_memhp_notifier,
> +	.priority = 0
> +};
>  #endif
>  
> +#if defined(CONFIG_HOTPLUG_CPU)
> +static int crash_cpuhp_online(unsigned int cpu)
> +{
> +	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
> +	return 0;
> +}
> +
> +static int crash_cpuhp_offline(unsigned int cpu)
> +{
> +	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
> +	return 0;
> +}
> +#endif
> +
> +static int __init crash_hotplug_init(void)
> +{
> +	int result = 0;
> +
> +#if defined(CONFIG_MEMORY_HOTPLUG)
> +	register_memory_notifier(&crash_memhp_nb);
> +#endif
> +
> +#if defined(CONFIG_HOTPLUG_CPU)
> +	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> +				"crash/cpuhp",
> +				crash_cpuhp_online, crash_cpuhp_offline);
> +#endif
> +
> +	return result;
> +}
> +
> +subsys_initcall(crash_hotplug_init);
> +#endif /* CONFIG_CRASH_HOTPLUG */
> -- 
> 2.27.0
> 

[PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> This patch introduces a generic crash hot plug/unplug infrastructure
> for CPU and memory changes. Upon CPU and memory changes, a generic
> crash_hotplug_handler() obtains the appropriate lock, does some
> important house keeping and then dispatches the hot plug/unplug event
> to the architecture specific arch_crash_hotplug_handler(), and when
> that handler returns, the lock is released.
> 
> This patch modifies crash_core.c to implement a subsys_initcall()
> function that installs handlers for hot plug/unplug events. If CPU
> hotplug is enabled, then cpuhp_setup_state() is invoked to register a
> handler for CPU changes. Similarly, if memory hotplug is enabled, then
> register_memory_notifier() is invoked to install a handler for memory
> changes. These handlers in turn invoke the common generic handler
> crash_hotplug_handler().
> 
> On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
> CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
> the CPU still shows up in foreach_present_cpu() during the regeneration
> of the new CPU list, thus the need to explicitly check and exclude the
> soon-to-be offlined CPU in crash_prepare_elf64_headers().
> 
> On the memory side, each un/plugged memory block passes through the
> handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
> memory events, one for each 128MiB memblock.

This looks good to me. As I commented in patch 3, these both may need be
merged.

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 0ff06d0698ad..524470e18883 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -10,12 +10,16 @@
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
>  #include <linux/highmem.h>
> +#include <linux/memory.h>
> +#include <linux/cpuhotplug.h>
>  
>  #include <asm/page.h>
>  #include <asm/sections.h>
>  
>  #include <crypto/sha1.h>
>  
> +#include "kexec_internal.h"
> +
>  /* vmcoreinfo stuff */
>  unsigned char *vmcoreinfo_data;
>  size_t vmcoreinfo_size;
> @@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
>  		*ptr = NULL;
>  	}
>  }
> +
> +void __weak arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b)
> +{
> +	pr_warn("crash hp: %s not implemented", __func__);
> +}
> +
> +static void crash_hotplug_handler(unsigned int hp_action,
> +	unsigned long a, unsigned long b)
> +{
> +	/* Obtain lock while changing crash information */
> +	if (!mutex_trylock(&kexec_mutex))
> +		return;
> +
> +	/* Check kdump is loaded */
> +	if (kexec_crash_image) {
> +		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
> +			a, b);
> +
> +		/* Needed in order for the segments to be updated */
> +		arch_kexec_unprotect_crashkres();
> +
> +		/* Flag to differentiate between normal load and hotplug */
> +		kexec_crash_image->hotplug_event = true;
> +
> +		/* Now invoke arch-specific update handler */
> +		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
> +
> +		/* No longer handling a hotplug event */
> +		kexec_crash_image->hotplug_event = false;
> +
> +		/* Change back to read-only */
> +		arch_kexec_protect_crashkres();
> +	}
> +
> +	/* Release lock now that update complete */
> +	mutex_unlock(&kexec_mutex);
> +}
> +
> +#if defined(CONFIG_MEMORY_HOTPLUG)
> +static int crash_memhp_notifier(struct notifier_block *nb,
> +	unsigned long val, void *v)
> +{
> +	struct memory_notify *mhp = v;
> +	unsigned long start, end;
> +
> +	start = mhp->start_pfn << PAGE_SHIFT;
> +	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
> +
> +	switch (val) {
> +	case MEM_ONLINE:
> +		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
> +			start, end-start);
> +		break;
> +
> +	case MEM_OFFLINE:
> +		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
> +			start, end-start);
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block crash_memhp_nb = {
> +	.notifier_call = crash_memhp_notifier,
> +	.priority = 0
> +};
>  #endif
>  
> +#if defined(CONFIG_HOTPLUG_CPU)
> +static int crash_cpuhp_online(unsigned int cpu)
> +{
> +	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
> +	return 0;
> +}
> +
> +static int crash_cpuhp_offline(unsigned int cpu)
> +{
> +	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
> +	return 0;
> +}
> +#endif
> +
> +static int __init crash_hotplug_init(void)
> +{
> +	int result = 0;
> +
> +#if defined(CONFIG_MEMORY_HOTPLUG)
> +	register_memory_notifier(&crash_memhp_nb);
> +#endif
> +
> +#if defined(CONFIG_HOTPLUG_CPU)
> +	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> +				"crash/cpuhp",
> +				crash_cpuhp_online, crash_cpuhp_offline);
> +#endif
> +
> +	return result;
> +}
> +
> +subsys_initcall(crash_hotplug_init);
> +#endif /* CONFIG_CRASH_HOTPLUG */
> -- 
> 2.27.0
> 

Re: [PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> When a crash kernel is loaded via the kexec_file_load syscall, the
> kernel places the various segments (ie crash kernel, crash initrd,
> boot_params, elfcorehdr, purgatory, etc) in memory. For those
> architectures that utilize purgatory, a hash digest of the segments
> is calculated for integrity checking. This digest is embedded into
> the purgatory image prior to placing purgatory in memory.
> 
> Since hotplug events cause changes to the elfcorehdr, purgatory
> integrity checking fails (at crash time, and no kdump created).
> As a result, this change explicitly excludes the elfcorehdr segment
> from the list of segments used to create the digest. By doing so,
> this permits changes to the elfcorehdr in response to hotplug events,
> without having to also reload purgatory due to the change to the
> digest.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/kexec_file.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 801d0d0a5012..93bb0c0ce66c 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  	for (j = i = 0; i < image->nr_segments; i++) {
>  		struct kexec_segment *ksegment;
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +		/* This segment excluded to allow future changes via hotplug */
> +		if (image->elf_index_valid && (j == image->elf_index))
> +			continue;
> +#endif
> +
>  		ksegment = &image->segment[i];
>  		/*
>  		 * Skip purgatory as it will be modified once we put digest
> -- 
> 2.27.0
> 

[PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> When a crash kernel is loaded via the kexec_file_load syscall, the
> kernel places the various segments (ie crash kernel, crash initrd,
> boot_params, elfcorehdr, purgatory, etc) in memory. For those
> architectures that utilize purgatory, a hash digest of the segments
> is calculated for integrity checking. This digest is embedded into
> the purgatory image prior to placing purgatory in memory.
> 
> Since hotplug events cause changes to the elfcorehdr, purgatory
> integrity checking fails (at crash time, and no kdump created).
> As a result, this change explicitly excludes the elfcorehdr segment
> from the list of segments used to create the digest. By doing so,
> this permits changes to the elfcorehdr in response to hotplug events,
> without having to also reload purgatory due to the change to the
> digest.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/kexec_file.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 801d0d0a5012..93bb0c0ce66c 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  	for (j = i = 0; i < image->nr_segments; i++) {
>  		struct kexec_segment *ksegment;
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +		/* This segment excluded to allow future changes via hotplug */
> +		if (image->elf_index_valid && (j == image->elf_index))
> +			continue;
> +#endif
> +
>  		ksegment = &image->segment[i];
>  		/*
>  		 * Skip purgatory as it will be modified once we put digest
> -- 
> 2.27.0
> 

Re: [PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
> still in the for_each_present_cpu() list when within the
> crash_hotplug_handler(). Thus the CPU must be explicitly excluded
> when building the new list of CPUs.
> 
> This change identifies in crash_hotplug_handler() the CPU to be
> excluded, and the check for excluding the CPU in
> crash_prepare_elf64_headers().

This looks good to me, and the particularity of cpu hotplug can be
understood well by other code reader. Thanks.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/crash_core.c | 10 ++++++++++
>  kernel/kexec_file.c |  5 +++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 524470e18883..b6942e04a188 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
>  		/* Flag to differentiate between normal load and hotplug */
>  		kexec_crash_image->hotplug_event = true;
>  
> +		/*
> +		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
> +		 * this callback, the CPU is still in the for_each_present_cpu()
> +		 * list. Must explicitly look to exclude this CPU when building
> +		 * new list.
> +		 */
> +		kexec_crash_image->offlinecpu =
> +			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
> +				(unsigned int)a : ~0U;
> +
>  		/* Now invoke arch-specific update handler */
>  		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>  
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 93bb0c0ce66c..339995d42169 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>  
>  	/* Prepare one phdr of type PT_NOTE for each present CPU */
>  	for_each_present_cpu(cpu) {
> +#ifdef CONFIG_CRASH_HOTPLUG
> +		/* Skip the soon-to-be offlined cpu */
> +		if (image->hotplug_event && (cpu == image->offlinecpu))
> +			continue;
> +#endif
>  		phdr->p_type = PT_NOTE;
>  		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>  		phdr->p_offset = phdr->p_paddr = notes_addr;
> -- 
> 2.27.0
> 

[PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
> still in the for_each_present_cpu() list when within the
> crash_hotplug_handler(). Thus the CPU must be explicitly excluded
> when building the new list of CPUs.
> 
> This change identifies in crash_hotplug_handler() the CPU to be
> excluded, and the check for excluding the CPU in
> crash_prepare_elf64_headers().

This looks good to me, and the particularity of cpu hotplug can be
understood well by other code reader. Thanks.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  kernel/crash_core.c | 10 ++++++++++
>  kernel/kexec_file.c |  5 +++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 524470e18883..b6942e04a188 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
>  		/* Flag to differentiate between normal load and hotplug */
>  		kexec_crash_image->hotplug_event = true;
>  
> +		/*
> +		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
> +		 * this callback, the CPU is still in the for_each_present_cpu()
> +		 * list. Must explicitly look to exclude this CPU when building
> +		 * new list.
> +		 */
> +		kexec_crash_image->offlinecpu =
> +			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
> +				(unsigned int)a : ~0U;
> +
>  		/* Now invoke arch-specific update handler */
>  		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>  
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 93bb0c0ce66c..339995d42169 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>  
>  	/* Prepare one phdr of type PT_NOTE for each present CPU */
>  	for_each_present_cpu(cpu) {
> +#ifdef CONFIG_CRASH_HOTPLUG
> +		/* Skip the soon-to-be offlined cpu */
> +		if (image->hotplug_event && (cpu == image->offlinecpu))
> +			continue;
> +#endif
>  		phdr->p_type = PT_NOTE;
>  		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>  		phdr->p_offset = phdr->p_paddr = notes_addr;
> -- 
> 2.27.0
> 

Re: [PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> For x86_64, when CPU or memory is hot un/plugged, the crash
> elfcorehdr, which describes the CPUs and memory in the system,
> must also be updated.

Please mark the patch as X86 related by adding:

x86/kexec: Add x86 crash hotplug support for kexec_file_load

And also update other subject to reflect under which component the
changes are made, crash hp is the feature name.

Otherwise, this looks good to me.

> 
> To update the elfcorehdr for x86_64, a new elfcorehdr must be
> generated from the available CPUs and memory. The new elfcorehdr
> is prepared into a buffer, and if no errors occur, it is
> installed over the top of the existing elfcorehdr.
> 
> In the patch 'crash hp: exclude elfcorehdr from the segment digest'
> the need to update purgatory due to the change in elfcorehdr was
> eliminated.  As a result, no changes to purgatory or boot_params
> (as the elfcorehdr= kernel command line parameter pointer
> remains unchanged and correct) are needed, just elfcorehdr.
> 
> To accommodate a growing number of resources via hotplug, the
> elfcorehdr segment must be sufficiently large enough to accommodate
> changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
> 
> With this change, the kexec_file_load syscall (not kexec_load)
> is supported. When loading the crash kernel via kexec_file_load,
> the elfcorehdr is identified at load time in crash_load_segments().
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9db41cce8d97..6215c51919db 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -25,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  #include <linux/memblock.h>
> +#include <linux/highmem.h>
>  
>  #include <asm/processor.h>
>  #include <asm/hardirq.h>
> @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
>  	image->elf_headers = kbuf.buffer;
>  	image->elf_headers_sz = kbuf.bufsz;
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	/* Ensure elfcorehdr segment large enough for hotplug changes */
> +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
> +	/* For marking as usable to crash kernel */
> +	image->elf_headers_sz = kbuf.memsz;
> +	/* Record the index of the elfcorehdr segment */
> +	image->elf_index = image->nr_segments;
> +	image->elf_index_valid = true;
> +#else
>  	kbuf.memsz = kbuf.bufsz;
> +#endif
>  	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
>  	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>  	ret = kexec_add_buffer(&kbuf);
> @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
>  	return ret;
>  }
>  #endif /* CONFIG_KEXEC_FILE */
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
> + * is passed to the crash kernel via the elfcorehdr= parameter)
> + * must be updated with the new list of CPUs and memories. The new
> + * elfcorehdr is prepared in a kernel buffer, and if no errors,
> + * then it is written on top of the existing/old elfcorehdr.
> + *
> + * For hotplug changes to elfcorehdr to work, two conditions are
> + * needed:
> + * First, the segment containing the elfcorehdr must be large enough
> + * to permit a growing number of resources. See
> + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
> + * Second, purgatory must explicitly exclude the elfcorehdr from the
> + * list of segments it checks (since the elfcorehdr changes and thus
> + * would require an update to purgatory itself to update the digest).
> + *
> + */
> +void arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b)
> +{
> +	struct kexec_segment *ksegment;
> +	unsigned char *ptr = NULL;
> +	unsigned long elfsz = 0;
> +	void *elfbuf = NULL;
> +	unsigned long mem, memsz;
> +
> +	/* Must have valid elfcorehdr index */
> +	if (!image->elf_index_valid) {
> +		pr_err("crash hp: unable to locate elfcorehdr segment");
> +		goto out;
> +	}
> +
> +	ksegment = &image->segment[image->elf_index];
> +	mem = ksegment->mem;
> +	memsz = ksegment->memsz;
> +
> +	/*
> +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
> +	 * memory resources. The elfcorehdr segment memsz must be
> +	 * sufficiently large to accommodate increases due to hotplug
> +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
> +	 */
> +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
> +		pr_err("crash hp: unable to prepare elfcore headers");
> +		goto out;
> +	}
> +	if (elfsz > memsz) {
> +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
> +			elfsz, memsz);
> +		goto out;
> +	}
> +
> +	/*
> +	 * At this point, we are all but assured of success.
> +	 * Copy new elfcorehdr into destination.
> +	 */
> +	ptr = map_crash_pages(mem, memsz);
> +	if (ptr) {
> +		/* Temporarily invalidate the crash image while it is replaced */
> +		xchg(&kexec_crash_image, NULL);
> +		/* Write the new elfcorehdr into memory */
> +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
> +		/* The crash image is now valid once again */
> +		xchg(&kexec_crash_image, image);
> +	}
> +	unmap_crash_pages((void **)&ptr);
> +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
> +
> +out:
> +	if (elfbuf)
> +		vfree(elfbuf);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> -- 
> 2.27.0
> 

[PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Baoquan He]

On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> For x86_64, when CPU or memory is hot un/plugged, the crash
> elfcorehdr, which describes the CPUs and memory in the system,
> must also be updated.

Please mark the patch as X86 related by adding:

x86/kexec: Add x86 crash hotplug support for kexec_file_load

And also update other subject to reflect under which component the
changes are made, crash hp is the feature name.

Otherwise, this looks good to me.

> 
> To update the elfcorehdr for x86_64, a new elfcorehdr must be
> generated from the available CPUs and memory. The new elfcorehdr
> is prepared into a buffer, and if no errors occur, it is
> installed over the top of the existing elfcorehdr.
> 
> In the patch 'crash hp: exclude elfcorehdr from the segment digest'
> the need to update purgatory due to the change in elfcorehdr was
> eliminated.  As a result, no changes to purgatory or boot_params
> (as the elfcorehdr= kernel command line parameter pointer
> remains unchanged and correct) are needed, just elfcorehdr.
> 
> To accommodate a growing number of resources via hotplug, the
> elfcorehdr segment must be sufficiently large enough to accommodate
> changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
> 
> With this change, the kexec_file_load syscall (not kexec_load)
> is supported. When loading the crash kernel via kexec_file_load,
> the elfcorehdr is identified at load time in crash_load_segments().
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>  arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9db41cce8d97..6215c51919db 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -25,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  #include <linux/memblock.h>
> +#include <linux/highmem.h>
>  
>  #include <asm/processor.h>
>  #include <asm/hardirq.h>
> @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
>  	image->elf_headers = kbuf.buffer;
>  	image->elf_headers_sz = kbuf.bufsz;
>  
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	/* Ensure elfcorehdr segment large enough for hotplug changes */
> +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
> +	/* For marking as usable to crash kernel */
> +	image->elf_headers_sz = kbuf.memsz;
> +	/* Record the index of the elfcorehdr segment */
> +	image->elf_index = image->nr_segments;
> +	image->elf_index_valid = true;
> +#else
>  	kbuf.memsz = kbuf.bufsz;
> +#endif
>  	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
>  	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>  	ret = kexec_add_buffer(&kbuf);
> @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
>  	return ret;
>  }
>  #endif /* CONFIG_KEXEC_FILE */
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +/**
> + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
> + * is passed to the crash kernel via the elfcorehdr= parameter)
> + * must be updated with the new list of CPUs and memories. The new
> + * elfcorehdr is prepared in a kernel buffer, and if no errors,
> + * then it is written on top of the existing/old elfcorehdr.
> + *
> + * For hotplug changes to elfcorehdr to work, two conditions are
> + * needed:
> + * First, the segment containing the elfcorehdr must be large enough
> + * to permit a growing number of resources. See
> + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
> + * Second, purgatory must explicitly exclude the elfcorehdr from the
> + * list of segments it checks (since the elfcorehdr changes and thus
> + * would require an update to purgatory itself to update the digest).
> + *
> + */
> +void arch_crash_hotplug_handler(struct kimage *image,
> +	unsigned int hp_action, unsigned long a, unsigned long b)
> +{
> +	struct kexec_segment *ksegment;
> +	unsigned char *ptr = NULL;
> +	unsigned long elfsz = 0;
> +	void *elfbuf = NULL;
> +	unsigned long mem, memsz;
> +
> +	/* Must have valid elfcorehdr index */
> +	if (!image->elf_index_valid) {
> +		pr_err("crash hp: unable to locate elfcorehdr segment");
> +		goto out;
> +	}
> +
> +	ksegment = &image->segment[image->elf_index];
> +	mem = ksegment->mem;
> +	memsz = ksegment->memsz;
> +
> +	/*
> +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
> +	 * memory resources. The elfcorehdr segment memsz must be
> +	 * sufficiently large to accommodate increases due to hotplug
> +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
> +	 */
> +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
> +		pr_err("crash hp: unable to prepare elfcore headers");
> +		goto out;
> +	}
> +	if (elfsz > memsz) {
> +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
> +			elfsz, memsz);
> +		goto out;
> +	}
> +
> +	/*
> +	 * At this point, we are all but assured of success.
> +	 * Copy new elfcorehdr into destination.
> +	 */
> +	ptr = map_crash_pages(mem, memsz);
> +	if (ptr) {
> +		/* Temporarily invalidate the crash image while it is replaced */
> +		xchg(&kexec_crash_image, NULL);
> +		/* Write the new elfcorehdr into memory */
> +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
> +		/* The crash image is now valid once again */
> +		xchg(&kexec_crash_image, image);
> +	}
> +	unmap_crash_pages((void **)&ptr);
> +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
> +
> +out:
> +	if (elfbuf)
> +		vfree(elfbuf);
> +}
> +#endif /* CONFIG_CRASH_HOTPLUG */
> -- 
> 2.27.0
> 

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

On 2/22/22 21:25, Baoquan He wrote:
> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>> Support for CPU and memory hotplug for crash is controlled by the
>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>
>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>> also introduced with this patch.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index ebe8fc76949a..4e3374edab02 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>   	  (CONFIG_RELOCATABLE=y).
>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>   
>> +config CRASH_HOTPLUG
>> +	bool "kernel updates of crash elfcorehdr"
>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>> +	help
>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>> +	  plug/unplug change notifications via udev in order to
>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>> +	  memory change notifications still occur (however, userspace is not
>> +	  required to monitor for crash dump purposes).
>> +
>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>> +	depends on CRASH_HOTPLUG
>> +	int
>> +	default 131072
>> +	help
>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>> +	  The 128KiB default is sized so that it can accommodate 2048
>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>> +	  region of memory.
>> +	  For example, this size can accommodate hotplugging a machine
>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>> +	  with 1024 1GiB memory DIMMs).
> 
> This example of memory could be a little misleading. The memory regions
> may not be related to memory DIMMs. System could split them into many
> smaller regions during bootup.

I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
eric

> 
> Otherwise, this patch looks good to me.
> 
>> +
>>   config KEXEC_JUMP
>>   	bool "kexec jump"
>>   	depends on KEXEC && HIBERNATION
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

On 2/22/22 21:25, Baoquan He wrote:
> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>> Support for CPU and memory hotplug for crash is controlled by the
>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>
>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>> also introduced with this patch.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index ebe8fc76949a..4e3374edab02 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>   	  (CONFIG_RELOCATABLE=y).
>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>   
>> +config CRASH_HOTPLUG
>> +	bool "kernel updates of crash elfcorehdr"
>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>> +	help
>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>> +	  plug/unplug change notifications via udev in order to
>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>> +	  memory change notifications still occur (however, userspace is not
>> +	  required to monitor for crash dump purposes).
>> +
>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>> +	depends on CRASH_HOTPLUG
>> +	int
>> +	default 131072
>> +	help
>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>> +	  The 128KiB default is sized so that it can accommodate 2048
>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>> +	  region of memory.
>> +	  For example, this size can accommodate hotplugging a machine
>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>> +	  with 1024 1GiB memory DIMMs).
> 
> This example of memory could be a little misleading. The memory regions
> may not be related to memory DIMMs. System could split them into many
> smaller regions during bootup.

I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
eric

> 
> Otherwise, this patch looks good to me.
> 
>> +
>>   config KEXEC_JUMP
>>   	bool "kexec jump"
>>   	depends on KEXEC && HIBERNATION
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 03/10] crash hp: definitions and prototype changes

[Eric DeVolder]

On 2/22/22 21:43, Baoquan He wrote:
> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>> This change adds members to struct kimage to facilitate crash
>> hotplug support.
>>
>> This change also defines crash hotplug events and associated
>> prototypes.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   include/linux/kexec.h | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 0c994ae37729..13c238c2da48 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -299,6 +299,13 @@ struct kimage {
>>   
>>   	/* Information for loading purgatory */
>>   	struct purgatory_info purgatory_info;
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +	bool hotplug_event;
>> +	int offlinecpu;
>> +	bool elf_index_valid;
>> +	int elf_index;
>> +#endif
>>   #endif
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>> @@ -315,6 +322,15 @@ struct kimage {
>>   	unsigned long elf_load_addr;
>>   };
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +void arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b);
> 
> Seems this patch can be merged with patch 6/10. I don't see prototype
> change in this patch.

Ok, done.
eric

> 
>> +#define KEXEC_CRASH_HP_REMOVE_CPU   0
>> +#define KEXEC_CRASH_HP_ADD_CPU      1
>> +#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
>> +#define KEXEC_CRASH_HP_ADD_MEMORY   3
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>>   /* kexec interface functions */
>>   extern void machine_kexec(struct kimage *image);
>>   extern int machine_kexec_prepare(struct kimage *image);
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 03/10] crash hp: definitions and prototype changes

[Eric DeVolder]

On 2/22/22 21:43, Baoquan He wrote:
> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>> This change adds members to struct kimage to facilitate crash
>> hotplug support.
>>
>> This change also defines crash hotplug events and associated
>> prototypes.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   include/linux/kexec.h | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 0c994ae37729..13c238c2da48 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -299,6 +299,13 @@ struct kimage {
>>   
>>   	/* Information for loading purgatory */
>>   	struct purgatory_info purgatory_info;
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +	bool hotplug_event;
>> +	int offlinecpu;
>> +	bool elf_index_valid;
>> +	int elf_index;
>> +#endif
>>   #endif
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>> @@ -315,6 +322,15 @@ struct kimage {
>>   	unsigned long elf_load_addr;
>>   };
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +void arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b);
> 
> Seems this patch can be merged with patch 6/10. I don't see prototype
> change in this patch.

Ok, done.
eric

> 
>> +#define KEXEC_CRASH_HP_REMOVE_CPU   0
>> +#define KEXEC_CRASH_HP_ADD_CPU      1
>> +#define KEXEC_CRASH_HP_REMOVE_MEMORY 2
>> +#define KEXEC_CRASH_HP_ADD_MEMORY   3
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> +
>>   /* kexec interface functions */
>>   extern void machine_kexec(struct kimage *image);
>>   extern int machine_kexec_prepare(struct kimage *image);
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Eric DeVolder]

On 2/22/22 21:46, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>>  From within crash_prepare_elf64_headers() there is a need to
>> reference the struct kimage hotplug members. As such, this
>> change passes the struct kimage as a parameter to the
>> crash_prepare_elf64_headers().
> 
> One more sentence can be added"
> 
> "This is preparation for later patch, no functionality change."
OK, done!

> 
> Otherwise, this patch looks good to me.
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Thank you!
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/arm64/kernel/machine_kexec_file.c | 6 +++---
>>   arch/powerpc/kexec/file_load_64.c      | 2 +-
>>   arch/x86/kernel/crash.c                | 3 ++-
>>   include/linux/kexec.h                  | 5 +++--
>>   kernel/kexec_file.c                    | 4 ++--
>>   5 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index 59c648d51848..7dbafb42ecf2 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>>   	return kexec_image_post_load_cleanup_default(image);
>>   }
>>   
>> -static int prepare_elf_headers(void **addr, unsigned long *sz)
>> +static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
>>   {
>>   	struct crash_mem *cmem;
>>   	unsigned int nr_ranges;
>> @@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>>   	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
>>   
>>   	if (!ret)
>> -		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
>> +		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
>>   
>>   	kfree(cmem);
>>   	return ret;
>> @@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
>>   
>>   	/* load elf core header */
>>   	if (image->type == KEXEC_TYPE_CRASH) {
>> -		ret = prepare_elf_headers(&headers, &headers_sz);
>> +		ret = prepare_elf_headers(image, &headers, &headers_sz);
>>   		if (ret) {
>>   			pr_err("Preparing elf core header failed\n");
>>   			goto out_err;
>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>> index b4981b651d9a..07da6bf1cf24 100644
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>>   		goto out;
>>   
>>   	/* Setup elfcorehdr segment */
>> -	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
>> +	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
>>   	if (ret) {
>>   		pr_err("Failed to prepare elf headers for the core\n");
>>   		goto out;
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 9730c88530fc..9db41cce8d97 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
>>   		goto out;
>>   
>>   	/* By default prepare 64bit headers */
>> -	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
>> +	ret =  crash_prepare_elf64_headers(image, cmem,
>> +				IS_ENABLED(CONFIG_X86_64), addr, sz);
>>   
>>   out:
>>   	vfree(cmem);
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 13c238c2da48..b11d75a6b2bc 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -221,8 +221,9 @@ struct crash_mem {
>>   extern int crash_exclude_mem_range(struct crash_mem *mem,
>>   				   unsigned long long mstart,
>>   				   unsigned long long mend);
>> -extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> -				       void **addr, unsigned long *sz);
>> +extern int crash_prepare_elf64_headers(struct kimage *image,
>> +	struct crash_mem *mem, int kernel_map,
>> +	void **addr, unsigned long *sz);
>>   #endif /* CONFIG_KEXEC_FILE */
>>   
>>   #ifdef CONFIG_KEXEC_ELF
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 8347fc158d2b..801d0d0a5012 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
>>   	return 0;
>>   }
>>   
>> -int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> -			  void **addr, unsigned long *sz)
>> +int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>> +	int kernel_map, void **addr, unsigned long *sz)
>>   {
>>   	Elf64_Ehdr *ehdr;
>>   	Elf64_Phdr *phdr;
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 04/10] crash hp: prototype change for crash_prepare_elf64_headers

[Eric DeVolder]

On 2/22/22 21:46, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>>  From within crash_prepare_elf64_headers() there is a need to
>> reference the struct kimage hotplug members. As such, this
>> change passes the struct kimage as a parameter to the
>> crash_prepare_elf64_headers().
> 
> One more sentence can be added"
> 
> "This is preparation for later patch, no functionality change."
OK, done!

> 
> Otherwise, this patch looks good to me.
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Thank you!
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/arm64/kernel/machine_kexec_file.c | 6 +++---
>>   arch/powerpc/kexec/file_load_64.c      | 2 +-
>>   arch/x86/kernel/crash.c                | 3 ++-
>>   include/linux/kexec.h                  | 5 +++--
>>   kernel/kexec_file.c                    | 4 ++--
>>   5 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index 59c648d51848..7dbafb42ecf2 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -39,7 +39,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>>   	return kexec_image_post_load_cleanup_default(image);
>>   }
>>   
>> -static int prepare_elf_headers(void **addr, unsigned long *sz)
>> +static int prepare_elf_headers(struct kimage *image, void **addr, unsigned long *sz)
>>   {
>>   	struct crash_mem *cmem;
>>   	unsigned int nr_ranges;
>> @@ -67,7 +67,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>>   	ret = crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end);
>>   
>>   	if (!ret)
>> -		ret =  crash_prepare_elf64_headers(cmem, true, addr, sz);
>> +		ret =  crash_prepare_elf64_headers(image, cmem, true, addr, sz);
>>   
>>   	kfree(cmem);
>>   	return ret;
>> @@ -96,7 +96,7 @@ int load_other_segments(struct kimage *image,
>>   
>>   	/* load elf core header */
>>   	if (image->type == KEXEC_TYPE_CRASH) {
>> -		ret = prepare_elf_headers(&headers, &headers_sz);
>> +		ret = prepare_elf_headers(image, &headers, &headers_sz);
>>   		if (ret) {
>>   			pr_err("Preparing elf core header failed\n");
>>   			goto out_err;
>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>> index b4981b651d9a..07da6bf1cf24 100644
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -797,7 +797,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
>>   		goto out;
>>   
>>   	/* Setup elfcorehdr segment */
>> -	ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
>> +	ret = crash_prepare_elf64_headers(image, cmem, false, &headers, &headers_sz);
>>   	if (ret) {
>>   		pr_err("Failed to prepare elf headers for the core\n");
>>   		goto out;
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 9730c88530fc..9db41cce8d97 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -265,7 +265,8 @@ static int prepare_elf_headers(struct kimage *image, void **addr,
>>   		goto out;
>>   
>>   	/* By default prepare 64bit headers */
>> -	ret =  crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
>> +	ret =  crash_prepare_elf64_headers(image, cmem,
>> +				IS_ENABLED(CONFIG_X86_64), addr, sz);
>>   
>>   out:
>>   	vfree(cmem);
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 13c238c2da48..b11d75a6b2bc 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -221,8 +221,9 @@ struct crash_mem {
>>   extern int crash_exclude_mem_range(struct crash_mem *mem,
>>   				   unsigned long long mstart,
>>   				   unsigned long long mend);
>> -extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> -				       void **addr, unsigned long *sz);
>> +extern int crash_prepare_elf64_headers(struct kimage *image,
>> +	struct crash_mem *mem, int kernel_map,
>> +	void **addr, unsigned long *sz);
>>   #endif /* CONFIG_KEXEC_FILE */
>>   
>>   #ifdef CONFIG_KEXEC_ELF
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 8347fc158d2b..801d0d0a5012 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -1260,8 +1260,8 @@ int crash_exclude_mem_range(struct crash_mem *mem,
>>   	return 0;
>>   }
>>   
>> -int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> -			  void **addr, unsigned long *sz)
>> +int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>> +	int kernel_map, void **addr, unsigned long *sz)
>>   {
>>   	Elf64_Ehdr *ehdr;
>>   	Elf64_Phdr *phdr;
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Eric DeVolder]

On 2/22/22 21:58, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> This change introduces two new functions un/map_crash_pages()
>> which are used to enable/disable access to the segments in the
>> crash memory region. (Upon loading of a crash kernel, the
>> crash memory regions are made inaccessible for integrity purposes.)
>>
>> For example, on x86_64, one of the segments is the elfcorehdr,
>> which contains the list of CPUs and memories. This segment
>> needs to be modified in response to hotplug events. These functions
>> are used to obtain (and subsequenntly release) access to the crash
>> memory region in order to make the modifications.
>>
>> QUESTION: These might need to be in arch/x86 as I'm not certain
>> the implementatin is valid for all archs?
> 
> Since only x86_64 uses them, I would suggest putting them into x86_64,
> near the caller.

I've moved these to arch/x86/kernel/crash.c within the #ifdef CONFIG_CRASH_HOTPLUG.
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   include/linux/kexec.h |  2 ++
>>   kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index b11d75a6b2bc..e00c373c4095 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -324,6 +324,8 @@ struct kimage {
>>   };
>>   
>>   #ifdef CONFIG_CRASH_HOTPLUG
>> +void *map_crash_pages(unsigned long paddr, unsigned long size);
>> +void unmap_crash_pages(void **ptr);
>>   void arch_crash_hotplug_handler(struct kimage *image,
>>   	unsigned int hp_action, unsigned long a, unsigned long b);
>>   #define KEXEC_CRASH_HP_REMOVE_CPU   0
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 256cf6db573c..0ff06d0698ad 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/init.h>
>>   #include <linux/utsname.h>
>>   #include <linux/vmalloc.h>
>> +#include <linux/highmem.h>
>>   
>>   #include <asm/page.h>
>>   #include <asm/sections.h>
>> @@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
>>   }
>>   
>>   subsys_initcall(crash_save_vmcoreinfo_init);
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +void *map_crash_pages(unsigned long paddr, unsigned long size)
>> +{
>> +	/*
>> +	 * NOTE: The addresses and sizes passed to this routine have
>> +	 * already been fully aligned on page boundaries. There is no
>> +	 * need for massaging the address or size.
>> +	 */
>> +	void *ptr = NULL;
>> +
>> +	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
>> +	if (size > 0) {
>> +		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
>> +
>> +		ptr = kmap(page);
>> +	}
>> +
>> +	return ptr;
>> +}
>> +
>> +void unmap_crash_pages(void **ptr)
>> +{
>> +	if (ptr) {
>> +		if (*ptr)
>> +			kunmap(*ptr);
>> +		*ptr = NULL;
>> +	}
>> +}
>> +#endif
>> +
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 05/10] crash hp: introduce helper functions un/map_crash_pages

[Eric DeVolder]

On 2/22/22 21:58, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> This change introduces two new functions un/map_crash_pages()
>> which are used to enable/disable access to the segments in the
>> crash memory region. (Upon loading of a crash kernel, the
>> crash memory regions are made inaccessible for integrity purposes.)
>>
>> For example, on x86_64, one of the segments is the elfcorehdr,
>> which contains the list of CPUs and memories. This segment
>> needs to be modified in response to hotplug events. These functions
>> are used to obtain (and subsequenntly release) access to the crash
>> memory region in order to make the modifications.
>>
>> QUESTION: These might need to be in arch/x86 as I'm not certain
>> the implementatin is valid for all archs?
> 
> Since only x86_64 uses them, I would suggest putting them into x86_64,
> near the caller.

I've moved these to arch/x86/kernel/crash.c within the #ifdef CONFIG_CRASH_HOTPLUG.
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   include/linux/kexec.h |  2 ++
>>   kernel/crash_core.c   | 32 ++++++++++++++++++++++++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index b11d75a6b2bc..e00c373c4095 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -324,6 +324,8 @@ struct kimage {
>>   };
>>   
>>   #ifdef CONFIG_CRASH_HOTPLUG
>> +void *map_crash_pages(unsigned long paddr, unsigned long size);
>> +void unmap_crash_pages(void **ptr);
>>   void arch_crash_hotplug_handler(struct kimage *image,
>>   	unsigned int hp_action, unsigned long a, unsigned long b);
>>   #define KEXEC_CRASH_HP_REMOVE_CPU   0
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 256cf6db573c..0ff06d0698ad 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/init.h>
>>   #include <linux/utsname.h>
>>   #include <linux/vmalloc.h>
>> +#include <linux/highmem.h>
>>   
>>   #include <asm/page.h>
>>   #include <asm/sections.h>
>> @@ -491,3 +492,34 @@ static int __init crash_save_vmcoreinfo_init(void)
>>   }
>>   
>>   subsys_initcall(crash_save_vmcoreinfo_init);
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +void *map_crash_pages(unsigned long paddr, unsigned long size)
>> +{
>> +	/*
>> +	 * NOTE: The addresses and sizes passed to this routine have
>> +	 * already been fully aligned on page boundaries. There is no
>> +	 * need for massaging the address or size.
>> +	 */
>> +	void *ptr = NULL;
>> +
>> +	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write access */
>> +	if (size > 0) {
>> +		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
>> +
>> +		ptr = kmap(page);
>> +	}
>> +
>> +	return ptr;
>> +}
>> +
>> +void unmap_crash_pages(void **ptr)
>> +{
>> +	if (ptr) {
>> +		if (*ptr)
>> +			kunmap(*ptr);
>> +		*ptr = NULL;
>> +	}
>> +}
>> +#endif
>> +
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Eric DeVolder]

On 2/22/22 21:59, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> This patch introduces a generic crash hot plug/unplug infrastructure
>> for CPU and memory changes. Upon CPU and memory changes, a generic
>> crash_hotplug_handler() obtains the appropriate lock, does some
>> important house keeping and then dispatches the hot plug/unplug event
>> to the architecture specific arch_crash_hotplug_handler(), and when
>> that handler returns, the lock is released.
>>
>> This patch modifies crash_core.c to implement a subsys_initcall()
>> function that installs handlers for hot plug/unplug events. If CPU
>> hotplug is enabled, then cpuhp_setup_state() is invoked to register a
>> handler for CPU changes. Similarly, if memory hotplug is enabled, then
>> register_memory_notifier() is invoked to install a handler for memory
>> changes. These handlers in turn invoke the common generic handler
>> crash_hotplug_handler().
>>
>> On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
>> CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
>> the CPU still shows up in foreach_present_cpu() during the regeneration
>> of the new CPU list, thus the need to explicitly check and exclude the
>> soon-to-be offlined CPU in crash_prepare_elf64_headers().
>>
>> On the memory side, each un/plugged memory block passes through the
>> handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
>> memory events, one for each 128MiB memblock.
> 
> This looks good to me. As I commented in patch 3, these both may need be
> merged.
OK, merged with 3.
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 104 insertions(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 0ff06d0698ad..524470e18883 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -10,12 +10,16 @@
>>   #include <linux/utsname.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/highmem.h>
>> +#include <linux/memory.h>
>> +#include <linux/cpuhotplug.h>
>>   
>>   #include <asm/page.h>
>>   #include <asm/sections.h>
>>   
>>   #include <crypto/sha1.h>
>>   
>> +#include "kexec_internal.h"
>> +
>>   /* vmcoreinfo stuff */
>>   unsigned char *vmcoreinfo_data;
>>   size_t vmcoreinfo_size;
>> @@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
>>   		*ptr = NULL;
>>   	}
>>   }
>> +
>> +void __weak arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b)
>> +{
>> +	pr_warn("crash hp: %s not implemented", __func__);
>> +}
>> +
>> +static void crash_hotplug_handler(unsigned int hp_action,
>> +	unsigned long a, unsigned long b)
>> +{
>> +	/* Obtain lock while changing crash information */
>> +	if (!mutex_trylock(&kexec_mutex))
>> +		return;
>> +
>> +	/* Check kdump is loaded */
>> +	if (kexec_crash_image) {
>> +		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
>> +			a, b);
>> +
>> +		/* Needed in order for the segments to be updated */
>> +		arch_kexec_unprotect_crashkres();
>> +
>> +		/* Flag to differentiate between normal load and hotplug */
>> +		kexec_crash_image->hotplug_event = true;
>> +
>> +		/* Now invoke arch-specific update handler */
>> +		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>> +
>> +		/* No longer handling a hotplug event */
>> +		kexec_crash_image->hotplug_event = false;
>> +
>> +		/* Change back to read-only */
>> +		arch_kexec_protect_crashkres();
>> +	}
>> +
>> +	/* Release lock now that update complete */
>> +	mutex_unlock(&kexec_mutex);
>> +}
>> +
>> +#if defined(CONFIG_MEMORY_HOTPLUG)
>> +static int crash_memhp_notifier(struct notifier_block *nb,
>> +	unsigned long val, void *v)
>> +{
>> +	struct memory_notify *mhp = v;
>> +	unsigned long start, end;
>> +
>> +	start = mhp->start_pfn << PAGE_SHIFT;
>> +	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
>> +
>> +	switch (val) {
>> +	case MEM_ONLINE:
>> +		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
>> +			start, end-start);
>> +		break;
>> +
>> +	case MEM_OFFLINE:
>> +		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
>> +			start, end-start);
>> +		break;
>> +	}
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static struct notifier_block crash_memhp_nb = {
>> +	.notifier_call = crash_memhp_notifier,
>> +	.priority = 0
>> +};
>>   #endif
>>   
>> +#if defined(CONFIG_HOTPLUG_CPU)
>> +static int crash_cpuhp_online(unsigned int cpu)
>> +{
>> +	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
>> +	return 0;
>> +}
>> +
>> +static int crash_cpuhp_offline(unsigned int cpu)
>> +{
>> +	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static int __init crash_hotplug_init(void)
>> +{
>> +	int result = 0;
>> +
>> +#if defined(CONFIG_MEMORY_HOTPLUG)
>> +	register_memory_notifier(&crash_memhp_nb);
>> +#endif
>> +
>> +#if defined(CONFIG_HOTPLUG_CPU)
>> +	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
>> +				"crash/cpuhp",
>> +				crash_cpuhp_online, crash_cpuhp_offline);
>> +#endif
>> +
>> +	return result;
>> +}
>> +
>> +subsys_initcall(crash_hotplug_init);
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 06/10] crash hp: generic crash hotplug support infrastructure

[Eric DeVolder]

On 2/22/22 21:59, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> This patch introduces a generic crash hot plug/unplug infrastructure
>> for CPU and memory changes. Upon CPU and memory changes, a generic
>> crash_hotplug_handler() obtains the appropriate lock, does some
>> important house keeping and then dispatches the hot plug/unplug event
>> to the architecture specific arch_crash_hotplug_handler(), and when
>> that handler returns, the lock is released.
>>
>> This patch modifies crash_core.c to implement a subsys_initcall()
>> function that installs handlers for hot plug/unplug events. If CPU
>> hotplug is enabled, then cpuhp_setup_state() is invoked to register a
>> handler for CPU changes. Similarly, if memory hotplug is enabled, then
>> register_memory_notifier() is invoked to install a handler for memory
>> changes. These handlers in turn invoke the common generic handler
>> crash_hotplug_handler().
>>
>> On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
>> CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
>> the CPU still shows up in foreach_present_cpu() during the regeneration
>> of the new CPU list, thus the need to explicitly check and exclude the
>> soon-to-be offlined CPU in crash_prepare_elf64_headers().
>>
>> On the memory side, each un/plugged memory block passes through the
>> handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
>> memory events, one for each 128MiB memblock.
> 
> This looks good to me. As I commented in patch 3, these both may need be
> merged.
OK, merged with 3.
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/crash_core.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 104 insertions(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 0ff06d0698ad..524470e18883 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -10,12 +10,16 @@
>>   #include <linux/utsname.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/highmem.h>
>> +#include <linux/memory.h>
>> +#include <linux/cpuhotplug.h>
>>   
>>   #include <asm/page.h>
>>   #include <asm/sections.h>
>>   
>>   #include <crypto/sha1.h>
>>   
>> +#include "kexec_internal.h"
>> +
>>   /* vmcoreinfo stuff */
>>   unsigned char *vmcoreinfo_data;
>>   size_t vmcoreinfo_size;
>> @@ -521,5 +525,105 @@ void unmap_crash_pages(void **ptr)
>>   		*ptr = NULL;
>>   	}
>>   }
>> +
>> +void __weak arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b)
>> +{
>> +	pr_warn("crash hp: %s not implemented", __func__);
>> +}
>> +
>> +static void crash_hotplug_handler(unsigned int hp_action,
>> +	unsigned long a, unsigned long b)
>> +{
>> +	/* Obtain lock while changing crash information */
>> +	if (!mutex_trylock(&kexec_mutex))
>> +		return;
>> +
>> +	/* Check kdump is loaded */
>> +	if (kexec_crash_image) {
>> +		pr_debug("crash hp: hp_action %u, a %lu, b %lu", hp_action,
>> +			a, b);
>> +
>> +		/* Needed in order for the segments to be updated */
>> +		arch_kexec_unprotect_crashkres();
>> +
>> +		/* Flag to differentiate between normal load and hotplug */
>> +		kexec_crash_image->hotplug_event = true;
>> +
>> +		/* Now invoke arch-specific update handler */
>> +		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>> +
>> +		/* No longer handling a hotplug event */
>> +		kexec_crash_image->hotplug_event = false;
>> +
>> +		/* Change back to read-only */
>> +		arch_kexec_protect_crashkres();
>> +	}
>> +
>> +	/* Release lock now that update complete */
>> +	mutex_unlock(&kexec_mutex);
>> +}
>> +
>> +#if defined(CONFIG_MEMORY_HOTPLUG)
>> +static int crash_memhp_notifier(struct notifier_block *nb,
>> +	unsigned long val, void *v)
>> +{
>> +	struct memory_notify *mhp = v;
>> +	unsigned long start, end;
>> +
>> +	start = mhp->start_pfn << PAGE_SHIFT;
>> +	end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
>> +
>> +	switch (val) {
>> +	case MEM_ONLINE:
>> +		crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
>> +			start, end-start);
>> +		break;
>> +
>> +	case MEM_OFFLINE:
>> +		crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
>> +			start, end-start);
>> +		break;
>> +	}
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static struct notifier_block crash_memhp_nb = {
>> +	.notifier_call = crash_memhp_notifier,
>> +	.priority = 0
>> +};
>>   #endif
>>   
>> +#if defined(CONFIG_HOTPLUG_CPU)
>> +static int crash_cpuhp_online(unsigned int cpu)
>> +{
>> +	crash_hotplug_handler(KEXEC_CRASH_HP_ADD_CPU, cpu, 0);
>> +	return 0;
>> +}
>> +
>> +static int crash_cpuhp_offline(unsigned int cpu)
>> +{
>> +	crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_CPU, cpu, 0);
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static int __init crash_hotplug_init(void)
>> +{
>> +	int result = 0;
>> +
>> +#if defined(CONFIG_MEMORY_HOTPLUG)
>> +	register_memory_notifier(&crash_memhp_nb);
>> +#endif
>> +
>> +#if defined(CONFIG_HOTPLUG_CPU)
>> +	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
>> +				"crash/cpuhp",
>> +				crash_cpuhp_online, crash_cpuhp_offline);
>> +#endif
>> +
>> +	return result;
>> +}
>> +
>> +subsys_initcall(crash_hotplug_init);
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Eric DeVolder]

On 2/22/22 22:00, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> When a crash kernel is loaded via the kexec_file_load syscall, the
>> kernel places the various segments (ie crash kernel, crash initrd,
>> boot_params, elfcorehdr, purgatory, etc) in memory. For those
>> architectures that utilize purgatory, a hash digest of the segments
>> is calculated for integrity checking. This digest is embedded into
>> the purgatory image prior to placing purgatory in memory.
>>
>> Since hotplug events cause changes to the elfcorehdr, purgatory
>> integrity checking fails (at crash time, and no kdump created).
>> As a result, this change explicitly excludes the elfcorehdr segment
>> from the list of segments used to create the digest. By doing so,
>> this permits changes to the elfcorehdr in response to hotplug events,
>> without having to also reload purgatory due to the change to the
>> digest.
> 
> Acked-by: Baoquan He <bhe@redhat.com>
Thank you!
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/kexec_file.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 801d0d0a5012..93bb0c0ce66c 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
>>   	for (j = i = 0; i < image->nr_segments; i++) {
>>   		struct kexec_segment *ksegment;
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +		/* This segment excluded to allow future changes via hotplug */
>> +		if (image->elf_index_valid && (j == image->elf_index))
>> +			continue;
>> +#endif
>> +
>>   		ksegment = &image->segment[i];
>>   		/*
>>   		 * Skip purgatory as it will be modified once we put digest
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 07/10] crash hp: exclude elfcorehdr from the segment digest

[Eric DeVolder]

On 2/22/22 22:00, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> When a crash kernel is loaded via the kexec_file_load syscall, the
>> kernel places the various segments (ie crash kernel, crash initrd,
>> boot_params, elfcorehdr, purgatory, etc) in memory. For those
>> architectures that utilize purgatory, a hash digest of the segments
>> is calculated for integrity checking. This digest is embedded into
>> the purgatory image prior to placing purgatory in memory.
>>
>> Since hotplug events cause changes to the elfcorehdr, purgatory
>> integrity checking fails (at crash time, and no kdump created).
>> As a result, this change explicitly excludes the elfcorehdr segment
>> from the list of segments used to create the digest. By doing so,
>> this permits changes to the elfcorehdr in response to hotplug events,
>> without having to also reload purgatory due to the change to the
>> digest.
> 
> Acked-by: Baoquan He <bhe@redhat.com>
Thank you!
eric

> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/kexec_file.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 801d0d0a5012..93bb0c0ce66c 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -765,6 +765,12 @@ static int kexec_calculate_store_digests(struct kimage *image)
>>   	for (j = i = 0; i < image->nr_segments; i++) {
>>   		struct kexec_segment *ksegment;
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +		/* This segment excluded to allow future changes via hotplug */
>> +		if (image->elf_index_valid && (j == image->elf_index))
>> +			continue;
>> +#endif
>> +
>>   		ksegment = &image->segment[i];
>>   		/*
>>   		 * Skip purgatory as it will be modified once we put digest
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Eric DeVolder]

On 2/22/22 22:04, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
>> still in the for_each_present_cpu() list when within the
>> crash_hotplug_handler(). Thus the CPU must be explicitly excluded
>> when building the new list of CPUs.
>>
>> This change identifies in crash_hotplug_handler() the CPU to be
>> excluded, and the check for excluding the CPU in
>> crash_prepare_elf64_headers().
> 
> This looks good to me, and the particularity of cpu hotplug can be
> understood well by other code reader. Thanks.
> 
> Acked-by: Baoquan He <bhe@redhat.com>
Thank you!
eric
> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/crash_core.c | 10 ++++++++++
>>   kernel/kexec_file.c |  5 +++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 524470e18883..b6942e04a188 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
>>   		/* Flag to differentiate between normal load and hotplug */
>>   		kexec_crash_image->hotplug_event = true;
>>   
>> +		/*
>> +		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
>> +		 * this callback, the CPU is still in the for_each_present_cpu()
>> +		 * list. Must explicitly look to exclude this CPU when building
>> +		 * new list.
>> +		 */
>> +		kexec_crash_image->offlinecpu =
>> +			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
>> +				(unsigned int)a : ~0U;
>> +
>>   		/* Now invoke arch-specific update handler */
>>   		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>>   
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 93bb0c0ce66c..339995d42169 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>>   
>>   	/* Prepare one phdr of type PT_NOTE for each present CPU */
>>   	for_each_present_cpu(cpu) {
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +		/* Skip the soon-to-be offlined cpu */
>> +		if (image->hotplug_event && (cpu == image->offlinecpu))
>> +			continue;
>> +#endif
>>   		phdr->p_type = PT_NOTE;
>>   		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>>   		phdr->p_offset = phdr->p_paddr = notes_addr;
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 08/10] crash hp: exclude hot remove cpu from elfcorehdr notes

[Eric DeVolder]

On 2/22/22 22:04, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> Due to use of CPUHP_AP_ONLINE_DYN, upon CPU unplug, the CPU is
>> still in the for_each_present_cpu() list when within the
>> crash_hotplug_handler(). Thus the CPU must be explicitly excluded
>> when building the new list of CPUs.
>>
>> This change identifies in crash_hotplug_handler() the CPU to be
>> excluded, and the check for excluding the CPU in
>> crash_prepare_elf64_headers().
> 
> This looks good to me, and the particularity of cpu hotplug can be
> understood well by other code reader. Thanks.
> 
> Acked-by: Baoquan He <bhe@redhat.com>
Thank you!
eric
> 
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   kernel/crash_core.c | 10 ++++++++++
>>   kernel/kexec_file.c |  5 +++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 524470e18883..b6942e04a188 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -550,6 +550,16 @@ static void crash_hotplug_handler(unsigned int hp_action,
>>   		/* Flag to differentiate between normal load and hotplug */
>>   		kexec_crash_image->hotplug_event = true;
>>   
>> +		/*
>> +		 * Due to use of CPUHP_AP_ONLINE_DYN, upon unplug and during
>> +		 * this callback, the CPU is still in the for_each_present_cpu()
>> +		 * list. Must explicitly look to exclude this CPU when building
>> +		 * new list.
>> +		 */
>> +		kexec_crash_image->offlinecpu =
>> +			(hp_action == KEXEC_CRASH_HP_REMOVE_CPU) ?
>> +				(unsigned int)a : ~0U;
>> +
>>   		/* Now invoke arch-specific update handler */
>>   		arch_crash_hotplug_handler(kexec_crash_image, hp_action, a, b);
>>   
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 93bb0c0ce66c..339995d42169 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -1314,6 +1314,11 @@ int crash_prepare_elf64_headers(struct kimage *image, struct crash_mem *mem,
>>   
>>   	/* Prepare one phdr of type PT_NOTE for each present CPU */
>>   	for_each_present_cpu(cpu) {
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +		/* Skip the soon-to-be offlined cpu */
>> +		if (image->hotplug_event && (cpu == image->offlinecpu))
>> +			continue;
>> +#endif
>>   		phdr->p_type = PT_NOTE;
>>   		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>>   		phdr->p_offset = phdr->p_paddr = notes_addr;
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Eric DeVolder]

On 2/22/22 22:10, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> For x86_64, when CPU or memory is hot un/plugged, the crash
>> elfcorehdr, which describes the CPUs and memory in the system,
>> must also be updated.
> 
> Please mark the patch as X86 related by adding:
> 
> x86/kexec: Add x86 crash hotplug support for kexec_file_load
Done!
> 
> And also update other subject to reflect under which component the
> changes are made, crash hp is the feature name.
Here's what I have at the moment, is this matching expectation?

pick 891fcff41a8f crash: fix minor typo/bug in debug message
pick 8f44e0d695be crash hp: Introduce CRASH_HOTPLUG configuration options
pick bae60e9db6ad crash: prototype change for crash_prepare_elf64_headers
pick cb98d1d923c8 crash: generic crash hotplug support infrastructure
pick 40e4434abad4 kexec: exclude elfcorehdr from the segment digest
pick 75039922aedc kexec: exclude hot remove cpu from elfcorehdr notes
pick 0be483dadeb8 x86/kexec: Add x86 crash hotplug support for kexec_file_load
pick 388bb5e4bf9a x86/kexec: Add x86 crash hotplug support for kexec_load

> 
> Otherwise, this looks good to me.
Thanks!
eric

> 
>>
>> To update the elfcorehdr for x86_64, a new elfcorehdr must be
>> generated from the available CPUs and memory. The new elfcorehdr
>> is prepared into a buffer, and if no errors occur, it is
>> installed over the top of the existing elfcorehdr.
>>
>> In the patch 'crash hp: exclude elfcorehdr from the segment digest'
>> the need to update purgatory due to the change in elfcorehdr was
>> eliminated.  As a result, no changes to purgatory or boot_params
>> (as the elfcorehdr= kernel command line parameter pointer
>> remains unchanged and correct) are needed, just elfcorehdr.
>>
>> To accommodate a growing number of resources via hotplug, the
>> elfcorehdr segment must be sufficiently large enough to accommodate
>> changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
>>
>> With this change, the kexec_file_load syscall (not kexec_load)
>> is supported. When loading the crash kernel via kexec_file_load,
>> the elfcorehdr is identified at load time in crash_load_segments().
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 92 insertions(+)
>>
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 9db41cce8d97..6215c51919db 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/memblock.h>
>> +#include <linux/highmem.h>
>>   
>>   #include <asm/processor.h>
>>   #include <asm/hardirq.h>
>> @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
>>   	image->elf_headers = kbuf.buffer;
>>   	image->elf_headers_sz = kbuf.bufsz;
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +	/* Ensure elfcorehdr segment large enough for hotplug changes */
>> +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
>> +	/* For marking as usable to crash kernel */
>> +	image->elf_headers_sz = kbuf.memsz;
>> +	/* Record the index of the elfcorehdr segment */
>> +	image->elf_index = image->nr_segments;
>> +	image->elf_index_valid = true;
>> +#else
>>   	kbuf.memsz = kbuf.bufsz;
>> +#endif
>>   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
>>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>   	ret = kexec_add_buffer(&kbuf);
>> @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
>>   	return ret;
>>   }
>>   #endif /* CONFIG_KEXEC_FILE */
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
>> + * is passed to the crash kernel via the elfcorehdr= parameter)
>> + * must be updated with the new list of CPUs and memories. The new
>> + * elfcorehdr is prepared in a kernel buffer, and if no errors,
>> + * then it is written on top of the existing/old elfcorehdr.
>> + *
>> + * For hotplug changes to elfcorehdr to work, two conditions are
>> + * needed:
>> + * First, the segment containing the elfcorehdr must be large enough
>> + * to permit a growing number of resources. See
>> + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
>> + * Second, purgatory must explicitly exclude the elfcorehdr from the
>> + * list of segments it checks (since the elfcorehdr changes and thus
>> + * would require an update to purgatory itself to update the digest).
>> + *
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b)
>> +{
>> +	struct kexec_segment *ksegment;
>> +	unsigned char *ptr = NULL;
>> +	unsigned long elfsz = 0;
>> +	void *elfbuf = NULL;
>> +	unsigned long mem, memsz;
>> +
>> +	/* Must have valid elfcorehdr index */
>> +	if (!image->elf_index_valid) {
>> +		pr_err("crash hp: unable to locate elfcorehdr segment");
>> +		goto out;
>> +	}
>> +
>> +	ksegment = &image->segment[image->elf_index];
>> +	mem = ksegment->mem;
>> +	memsz = ksegment->memsz;
>> +
>> +	/*
>> +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
>> +	 * memory resources. The elfcorehdr segment memsz must be
>> +	 * sufficiently large to accommodate increases due to hotplug
>> +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
>> +	 */
>> +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
>> +		pr_err("crash hp: unable to prepare elfcore headers");
>> +		goto out;
>> +	}
>> +	if (elfsz > memsz) {
>> +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
>> +			elfsz, memsz);
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * At this point, we are all but assured of success.
>> +	 * Copy new elfcorehdr into destination.
>> +	 */
>> +	ptr = map_crash_pages(mem, memsz);
>> +	if (ptr) {
>> +		/* Temporarily invalidate the crash image while it is replaced */
>> +		xchg(&kexec_crash_image, NULL);
>> +		/* Write the new elfcorehdr into memory */
>> +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
>> +		/* The crash image is now valid once again */
>> +		xchg(&kexec_crash_image, image);
>> +	}
>> +	unmap_crash_pages((void **)&ptr);
>> +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
>> +
>> +out:
>> +	if (elfbuf)
>> +		vfree(elfbuf);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> -- 
>> 2.27.0
>>
> 

[PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Eric DeVolder]

On 2/22/22 22:10, Baoquan He wrote:
> On 02/09/22 at 02:57pm, Eric DeVolder wrote:
>> For x86_64, when CPU or memory is hot un/plugged, the crash
>> elfcorehdr, which describes the CPUs and memory in the system,
>> must also be updated.
> 
> Please mark the patch as X86 related by adding:
> 
> x86/kexec: Add x86 crash hotplug support for kexec_file_load
Done!
> 
> And also update other subject to reflect under which component the
> changes are made, crash hp is the feature name.
Here's what I have at the moment, is this matching expectation?

pick 891fcff41a8f crash: fix minor typo/bug in debug message
pick 8f44e0d695be crash hp: Introduce CRASH_HOTPLUG configuration options
pick bae60e9db6ad crash: prototype change for crash_prepare_elf64_headers
pick cb98d1d923c8 crash: generic crash hotplug support infrastructure
pick 40e4434abad4 kexec: exclude elfcorehdr from the segment digest
pick 75039922aedc kexec: exclude hot remove cpu from elfcorehdr notes
pick 0be483dadeb8 x86/kexec: Add x86 crash hotplug support for kexec_file_load
pick 388bb5e4bf9a x86/kexec: Add x86 crash hotplug support for kexec_load

> 
> Otherwise, this looks good to me.
Thanks!
eric

> 
>>
>> To update the elfcorehdr for x86_64, a new elfcorehdr must be
>> generated from the available CPUs and memory. The new elfcorehdr
>> is prepared into a buffer, and if no errors occur, it is
>> installed over the top of the existing elfcorehdr.
>>
>> In the patch 'crash hp: exclude elfcorehdr from the segment digest'
>> the need to update purgatory due to the change in elfcorehdr was
>> eliminated.  As a result, no changes to purgatory or boot_params
>> (as the elfcorehdr= kernel command line parameter pointer
>> remains unchanged and correct) are needed, just elfcorehdr.
>>
>> To accommodate a growing number of resources via hotplug, the
>> elfcorehdr segment must be sufficiently large enough to accommodate
>> changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
>>
>> With this change, the kexec_file_load syscall (not kexec_load)
>> is supported. When loading the crash kernel via kexec_file_load,
>> the elfcorehdr is identified at load time in crash_load_segments().
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 92 insertions(+)
>>
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 9db41cce8d97..6215c51919db 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/memblock.h>
>> +#include <linux/highmem.h>
>>   
>>   #include <asm/processor.h>
>>   #include <asm/hardirq.h>
>> @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
>>   	image->elf_headers = kbuf.buffer;
>>   	image->elf_headers_sz = kbuf.bufsz;
>>   
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +	/* Ensure elfcorehdr segment large enough for hotplug changes */
>> +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
>> +	/* For marking as usable to crash kernel */
>> +	image->elf_headers_sz = kbuf.memsz;
>> +	/* Record the index of the elfcorehdr segment */
>> +	image->elf_index = image->nr_segments;
>> +	image->elf_index_valid = true;
>> +#else
>>   	kbuf.memsz = kbuf.bufsz;
>> +#endif
>>   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
>>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>   	ret = kexec_add_buffer(&kbuf);
>> @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
>>   	return ret;
>>   }
>>   #endif /* CONFIG_KEXEC_FILE */
>> +
>> +#ifdef CONFIG_CRASH_HOTPLUG
>> +/**
>> + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
>> + * is passed to the crash kernel via the elfcorehdr= parameter)
>> + * must be updated with the new list of CPUs and memories. The new
>> + * elfcorehdr is prepared in a kernel buffer, and if no errors,
>> + * then it is written on top of the existing/old elfcorehdr.
>> + *
>> + * For hotplug changes to elfcorehdr to work, two conditions are
>> + * needed:
>> + * First, the segment containing the elfcorehdr must be large enough
>> + * to permit a growing number of resources. See
>> + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
>> + * Second, purgatory must explicitly exclude the elfcorehdr from the
>> + * list of segments it checks (since the elfcorehdr changes and thus
>> + * would require an update to purgatory itself to update the digest).
>> + *
>> + */
>> +void arch_crash_hotplug_handler(struct kimage *image,
>> +	unsigned int hp_action, unsigned long a, unsigned long b)
>> +{
>> +	struct kexec_segment *ksegment;
>> +	unsigned char *ptr = NULL;
>> +	unsigned long elfsz = 0;
>> +	void *elfbuf = NULL;
>> +	unsigned long mem, memsz;
>> +
>> +	/* Must have valid elfcorehdr index */
>> +	if (!image->elf_index_valid) {
>> +		pr_err("crash hp: unable to locate elfcorehdr segment");
>> +		goto out;
>> +	}
>> +
>> +	ksegment = &image->segment[image->elf_index];
>> +	mem = ksegment->mem;
>> +	memsz = ksegment->memsz;
>> +
>> +	/*
>> +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
>> +	 * memory resources. The elfcorehdr segment memsz must be
>> +	 * sufficiently large to accommodate increases due to hotplug
>> +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
>> +	 */
>> +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
>> +		pr_err("crash hp: unable to prepare elfcore headers");
>> +		goto out;
>> +	}
>> +	if (elfsz > memsz) {
>> +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
>> +			elfsz, memsz);
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * At this point, we are all but assured of success.
>> +	 * Copy new elfcorehdr into destination.
>> +	 */
>> +	ptr = map_crash_pages(mem, memsz);
>> +	if (ptr) {
>> +		/* Temporarily invalidate the crash image while it is replaced */
>> +		xchg(&kexec_crash_image, NULL);
>> +		/* Write the new elfcorehdr into memory */
>> +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
>> +		/* The crash image is now valid once again */
>> +		xchg(&kexec_crash_image, image);
>> +	}
>> +	unmap_crash_pages((void **)&ptr);
>> +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
>> +
>> +out:
>> +	if (elfbuf)
>> +		vfree(elfbuf);
>> +}
>> +#endif /* CONFIG_CRASH_HOTPLUG */
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[David Hildenbrand]

On 01.03.22 21:04, Eric DeVolder wrote:
> 
> 
> On 2/22/22 21:25, Baoquan He wrote:
>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>> Support for CPU and memory hotplug for crash is controlled by the
>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>
>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>> also introduced with this patch.
>>>
>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>> ---
>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>> index ebe8fc76949a..4e3374edab02 100644
>>> --- a/arch/x86/Kconfig
>>> +++ b/arch/x86/Kconfig
>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>   	  (CONFIG_RELOCATABLE=y).
>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>   
>>> +config CRASH_HOTPLUG
>>> +	bool "kernel updates of crash elfcorehdr"
>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>> +	help
>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>> +	  plug/unplug change notifications via udev in order to
>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>> +	  memory change notifications still occur (however, userspace is not
>>> +	  required to monitor for crash dump purposes).
>>> +
>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>> +	depends on CRASH_HOTPLUG
>>> +	int
>>> +	default 131072
>>> +	help
>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>> +	  region of memory.
>>> +	  For example, this size can accommodate hotplugging a machine
>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>> +	  with 1024 1GiB memory DIMMs).
>>
>> This example of memory could be a little misleading. The memory regions
>> may not be related to memory DIMMs. System could split them into many
>> smaller regions during bootup.
> 
> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> eric

It's still not quite precise. Essentially it's the individual "System
RAM" entries in /proc/iomem

Boot memory (i.e., a single DIMM) might be represented by multiple
entries due to rearranged holes (by the BIOS).

While hoplugged DIMMs (under virt!) are usually represented using a
single range, it can be different on physical machines. Last but not
least, dax/kmem and virtio-mem behave in a different way.

-- 
Thanks,

David / dhildenb

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[David Hildenbrand]

On 01.03.22 21:04, Eric DeVolder wrote:
> 
> 
> On 2/22/22 21:25, Baoquan He wrote:
>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>> Support for CPU and memory hotplug for crash is controlled by the
>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>
>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>> also introduced with this patch.
>>>
>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>> ---
>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>> index ebe8fc76949a..4e3374edab02 100644
>>> --- a/arch/x86/Kconfig
>>> +++ b/arch/x86/Kconfig
>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>   	  (CONFIG_RELOCATABLE=y).
>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>   
>>> +config CRASH_HOTPLUG
>>> +	bool "kernel updates of crash elfcorehdr"
>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>> +	help
>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>> +	  plug/unplug change notifications via udev in order to
>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>> +	  memory change notifications still occur (however, userspace is not
>>> +	  required to monitor for crash dump purposes).
>>> +
>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>> +	depends on CRASH_HOTPLUG
>>> +	int
>>> +	default 131072
>>> +	help
>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>> +	  region of memory.
>>> +	  For example, this size can accommodate hotplugging a machine
>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>> +	  with 1024 1GiB memory DIMMs).
>>
>> This example of memory could be a little misleading. The memory regions
>> may not be related to memory DIMMs. System could split them into many
>> smaller regions during bootup.
> 
> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> eric

It's still not quite precise. Essentially it's the individual "System
RAM" entries in /proc/iomem

Boot memory (i.e., a single DIMM) might be represented by multiple
entries due to rearranged holes (by the BIOS).

While hoplugged DIMMs (under virt!) are usually represented using a
single range, it can be different on physical machines. Last but not
least, dax/kmem and virtio-mem behave in a different way.

-- 
Thanks,

David / dhildenb

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 03/02/22 at 10:20am, David Hildenbrand wrote:
> On 01.03.22 21:04, Eric DeVolder wrote:
> > 
> > 
> > On 2/22/22 21:25, Baoquan He wrote:
> >> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> >>> Support for CPU and memory hotplug for crash is controlled by the
> >>> CRASH_HOTPLUG configuration option, introduced by this patch.
> >>>
> >>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> >>> also introduced with this patch.
> >>>
> >>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> >>> ---
> >>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
> >>>   1 file changed, 26 insertions(+)
> >>>
> >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >>> index ebe8fc76949a..4e3374edab02 100644
> >>> --- a/arch/x86/Kconfig
> >>> +++ b/arch/x86/Kconfig
> >>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
> >>>   	  (CONFIG_RELOCATABLE=y).
> >>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
> >>>   
> >>> +config CRASH_HOTPLUG
> >>> +	bool "kernel updates of crash elfcorehdr"
> >>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> >>> +	help
> >>> +	  Enable the kernel to update the crash elfcorehdr (which contains
> >>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> >>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> >>> +	  plug/unplug change notifications via udev in order to
> >>> +	  unload-then-reload the crash kernel so that the list of CPUs and
> >>> +	  memory regions is kept up-to-date. Note that the udev CPU and
> >>> +	  memory change notifications still occur (however, userspace is not
> >>> +	  required to monitor for crash dump purposes).
> >>> +
> >>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> >>> +	depends on CRASH_HOTPLUG
> >>> +	int
> >>> +	default 131072
> >>> +	help
> >>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> >>> +	  The 128KiB default is sized so that it can accommodate 2048
> >>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> >>> +	  region of memory.
> >>> +	  For example, this size can accommodate hotplugging a machine
> >>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> >>> +	  with 1024 1GiB memory DIMMs).
> >>
> >> This example of memory could be a little misleading. The memory regions
> >> may not be related to memory DIMMs. System could split them into many
> >> smaller regions during bootup.
> > 
> > I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> > eric
> 
> It's still not quite precise. Essentially it's the individual "System
> RAM" entries in /proc/iomem
> 
> Boot memory (i.e., a single DIMM) might be represented by multiple
> entries due to rearranged holes (by the BIOS).
> 
> While hoplugged DIMMs (under virt!) are usually represented using a
> single range, it can be different on physical machines. Last but not
> least, dax/kmem and virtio-mem behave in a different way.

Right. How about only mentioning the 'System RAM' entries in /proc/iomem
as below? It's just giving an example, talking about the details of
memory regions from each type may not be necessry here. People
interested can refer to code or document related to get it.


+ default 131072
+ help
+   Specify the maximum size of the elfcorehdr buffer/segment.
+   The 128KiB default is sized so that it can accommodate 2048
+   Elf64_Phdr, where each Phdr represents either a CPU or a
+   region of memory.
+   For example, this size can accommodate hotplugging a machine
+   with up to 1024 CPUs and up to 1024 memory regions which are
    represented by 'System RAM' entries in /proc/iomem.

> 
> -- 
> Thanks,
> 
> David / dhildenb
> 

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 03/02/22 at 10:20am, David Hildenbrand wrote:
> On 01.03.22 21:04, Eric DeVolder wrote:
> > 
> > 
> > On 2/22/22 21:25, Baoquan He wrote:
> >> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> >>> Support for CPU and memory hotplug for crash is controlled by the
> >>> CRASH_HOTPLUG configuration option, introduced by this patch.
> >>>
> >>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> >>> also introduced with this patch.
> >>>
> >>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> >>> ---
> >>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
> >>>   1 file changed, 26 insertions(+)
> >>>
> >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >>> index ebe8fc76949a..4e3374edab02 100644
> >>> --- a/arch/x86/Kconfig
> >>> +++ b/arch/x86/Kconfig
> >>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
> >>>   	  (CONFIG_RELOCATABLE=y).
> >>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
> >>>   
> >>> +config CRASH_HOTPLUG
> >>> +	bool "kernel updates of crash elfcorehdr"
> >>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> >>> +	help
> >>> +	  Enable the kernel to update the crash elfcorehdr (which contains
> >>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> >>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> >>> +	  plug/unplug change notifications via udev in order to
> >>> +	  unload-then-reload the crash kernel so that the list of CPUs and
> >>> +	  memory regions is kept up-to-date. Note that the udev CPU and
> >>> +	  memory change notifications still occur (however, userspace is not
> >>> +	  required to monitor for crash dump purposes).
> >>> +
> >>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> >>> +	depends on CRASH_HOTPLUG
> >>> +	int
> >>> +	default 131072
> >>> +	help
> >>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> >>> +	  The 128KiB default is sized so that it can accommodate 2048
> >>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> >>> +	  region of memory.
> >>> +	  For example, this size can accommodate hotplugging a machine
> >>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> >>> +	  with 1024 1GiB memory DIMMs).
> >>
> >> This example of memory could be a little misleading. The memory regions
> >> may not be related to memory DIMMs. System could split them into many
> >> smaller regions during bootup.
> > 
> > I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> > eric
> 
> It's still not quite precise. Essentially it's the individual "System
> RAM" entries in /proc/iomem
> 
> Boot memory (i.e., a single DIMM) might be represented by multiple
> entries due to rearranged holes (by the BIOS).
> 
> While hoplugged DIMMs (under virt!) are usually represented using a
> single range, it can be different on physical machines. Last but not
> least, dax/kmem and virtio-mem behave in a different way.

Right. How about only mentioning the 'System RAM' entries in /proc/iomem
as below? It's just giving an example, talking about the details of
memory regions from each type may not be necessry here. People
interested can refer to code or document related to get it.


+ default 131072
+ help
+   Specify the maximum size of the elfcorehdr buffer/segment.
+   The 128KiB default is sized so that it can accommodate 2048
+   Elf64_Phdr, where each Phdr represents either a CPU or a
+   region of memory.
+   For example, this size can accommodate hotplugging a machine
+   with up to 1024 CPUs and up to 1024 memory regions which are
    represented by 'System RAM' entries in /proc/iomem.

> 
> -- 
> Thanks,
> 
> David / dhildenb
> 

Re: [PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Baoquan He]

On 03/01/22 at 02:12pm, Eric DeVolder wrote:
> 
> 
> On 2/22/22 22:10, Baoquan He wrote:
> > On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> > > For x86_64, when CPU or memory is hot un/plugged, the crash
> > > elfcorehdr, which describes the CPUs and memory in the system,
> > > must also be updated.
> > 
> > Please mark the patch as X86 related by adding:
> > 
> > x86/kexec: Add x86 crash hotplug support for kexec_file_load
> Done!
> > 
> > And also update other subject to reflect under which component the
> > changes are made, crash hp is the feature name.
> Here's what I have at the moment, is this matching expectation?
> 
> pick 891fcff41a8f crash: fix minor typo/bug in debug message

It's x86 code change, shoud be:
  x86/crash: fix minor typo/bug in debug message

Others are similar.

> pick 8f44e0d695be crash hp: Introduce CRASH_HOTPLUG configuration options
> pick bae60e9db6ad crash: prototype change for crash_prepare_elf64_headers
> pick cb98d1d923c8 crash: generic crash hotplug support infrastructure
> pick 40e4434abad4 kexec: exclude elfcorehdr from the segment digest
> pick 75039922aedc kexec: exclude hot remove cpu from elfcorehdr notes
> pick 0be483dadeb8 x86/kexec: Add x86 crash hotplug support for kexec_file_load
> pick 388bb5e4bf9a x86/kexec: Add x86 crash hotplug support for kexec_load
> 
> > 
> > Otherwise, this looks good to me.
> Thanks!
> eric
> 
> > 
> > > 
> > > To update the elfcorehdr for x86_64, a new elfcorehdr must be
> > > generated from the available CPUs and memory. The new elfcorehdr
> > > is prepared into a buffer, and if no errors occur, it is
> > > installed over the top of the existing elfcorehdr.
> > > 
> > > In the patch 'crash hp: exclude elfcorehdr from the segment digest'
> > > the need to update purgatory due to the change in elfcorehdr was
> > > eliminated.  As a result, no changes to purgatory or boot_params
> > > (as the elfcorehdr= kernel command line parameter pointer
> > > remains unchanged and correct) are needed, just elfcorehdr.
> > > 
> > > To accommodate a growing number of resources via hotplug, the
> > > elfcorehdr segment must be sufficiently large enough to accommodate
> > > changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
> > > 
> > > With this change, the kexec_file_load syscall (not kexec_load)
> > > is supported. When loading the crash kernel via kexec_file_load,
> > > the elfcorehdr is identified at load time in crash_load_segments().
> > > 
> > > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> > > ---
> > >   arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 92 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > > index 9db41cce8d97..6215c51919db 100644
> > > --- a/arch/x86/kernel/crash.c
> > > +++ b/arch/x86/kernel/crash.c
> > > @@ -25,6 +25,7 @@
> > >   #include <linux/slab.h>
> > >   #include <linux/vmalloc.h>
> > >   #include <linux/memblock.h>
> > > +#include <linux/highmem.h>
> > >   #include <asm/processor.h>
> > >   #include <asm/hardirq.h>
> > > @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
> > >   	image->elf_headers = kbuf.buffer;
> > >   	image->elf_headers_sz = kbuf.bufsz;
> > > +#ifdef CONFIG_CRASH_HOTPLUG
> > > +	/* Ensure elfcorehdr segment large enough for hotplug changes */
> > > +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
> > > +	/* For marking as usable to crash kernel */
> > > +	image->elf_headers_sz = kbuf.memsz;
> > > +	/* Record the index of the elfcorehdr segment */
> > > +	image->elf_index = image->nr_segments;
> > > +	image->elf_index_valid = true;
> > > +#else
> > >   	kbuf.memsz = kbuf.bufsz;
> > > +#endif
> > >   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> > >   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > >   	ret = kexec_add_buffer(&kbuf);
> > > @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
> > >   	return ret;
> > >   }
> > >   #endif /* CONFIG_KEXEC_FILE */
> > > +
> > > +#ifdef CONFIG_CRASH_HOTPLUG
> > > +/**
> > > + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
> > > + * is passed to the crash kernel via the elfcorehdr= parameter)
> > > + * must be updated with the new list of CPUs and memories. The new
> > > + * elfcorehdr is prepared in a kernel buffer, and if no errors,
> > > + * then it is written on top of the existing/old elfcorehdr.
> > > + *
> > > + * For hotplug changes to elfcorehdr to work, two conditions are
> > > + * needed:
> > > + * First, the segment containing the elfcorehdr must be large enough
> > > + * to permit a growing number of resources. See
> > > + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
> > > + * Second, purgatory must explicitly exclude the elfcorehdr from the
> > > + * list of segments it checks (since the elfcorehdr changes and thus
> > > + * would require an update to purgatory itself to update the digest).
> > > + *
> > > + */
> > > +void arch_crash_hotplug_handler(struct kimage *image,
> > > +	unsigned int hp_action, unsigned long a, unsigned long b)
> > > +{
> > > +	struct kexec_segment *ksegment;
> > > +	unsigned char *ptr = NULL;
> > > +	unsigned long elfsz = 0;
> > > +	void *elfbuf = NULL;
> > > +	unsigned long mem, memsz;
> > > +
> > > +	/* Must have valid elfcorehdr index */
> > > +	if (!image->elf_index_valid) {
> > > +		pr_err("crash hp: unable to locate elfcorehdr segment");
> > > +		goto out;
> > > +	}
> > > +
> > > +	ksegment = &image->segment[image->elf_index];
> > > +	mem = ksegment->mem;
> > > +	memsz = ksegment->memsz;
> > > +
> > > +	/*
> > > +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
> > > +	 * memory resources. The elfcorehdr segment memsz must be
> > > +	 * sufficiently large to accommodate increases due to hotplug
> > > +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
> > > +	 */
> > > +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
> > > +		pr_err("crash hp: unable to prepare elfcore headers");
> > > +		goto out;
> > > +	}
> > > +	if (elfsz > memsz) {
> > > +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
> > > +			elfsz, memsz);
> > > +		goto out;
> > > +	}
> > > +
> > > +	/*
> > > +	 * At this point, we are all but assured of success.
> > > +	 * Copy new elfcorehdr into destination.
> > > +	 */
> > > +	ptr = map_crash_pages(mem, memsz);
> > > +	if (ptr) {
> > > +		/* Temporarily invalidate the crash image while it is replaced */
> > > +		xchg(&kexec_crash_image, NULL);
> > > +		/* Write the new elfcorehdr into memory */
> > > +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
> > > +		/* The crash image is now valid once again */
> > > +		xchg(&kexec_crash_image, image);
> > > +	}
> > > +	unmap_crash_pages((void **)&ptr);
> > > +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
> > > +
> > > +out:
> > > +	if (elfbuf)
> > > +		vfree(elfbuf);
> > > +}
> > > +#endif /* CONFIG_CRASH_HOTPLUG */
> > > -- 
> > > 2.27.0
> > > 
> > 
> 

[PATCH v4 09/10] crash hp: Add x86 crash hotplug support for kexec_file_load

[Baoquan He]

On 03/01/22 at 02:12pm, Eric DeVolder wrote:
> 
> 
> On 2/22/22 22:10, Baoquan He wrote:
> > On 02/09/22 at 02:57pm, Eric DeVolder wrote:
> > > For x86_64, when CPU or memory is hot un/plugged, the crash
> > > elfcorehdr, which describes the CPUs and memory in the system,
> > > must also be updated.
> > 
> > Please mark the patch as X86 related by adding:
> > 
> > x86/kexec: Add x86 crash hotplug support for kexec_file_load
> Done!
> > 
> > And also update other subject to reflect under which component the
> > changes are made, crash hp is the feature name.
> Here's what I have at the moment, is this matching expectation?
> 
> pick 891fcff41a8f crash: fix minor typo/bug in debug message

It's x86 code change, shoud be:
  x86/crash: fix minor typo/bug in debug message

Others are similar.

> pick 8f44e0d695be crash hp: Introduce CRASH_HOTPLUG configuration options
> pick bae60e9db6ad crash: prototype change for crash_prepare_elf64_headers
> pick cb98d1d923c8 crash: generic crash hotplug support infrastructure
> pick 40e4434abad4 kexec: exclude elfcorehdr from the segment digest
> pick 75039922aedc kexec: exclude hot remove cpu from elfcorehdr notes
> pick 0be483dadeb8 x86/kexec: Add x86 crash hotplug support for kexec_file_load
> pick 388bb5e4bf9a x86/kexec: Add x86 crash hotplug support for kexec_load
> 
> > 
> > Otherwise, this looks good to me.
> Thanks!
> eric
> 
> > 
> > > 
> > > To update the elfcorehdr for x86_64, a new elfcorehdr must be
> > > generated from the available CPUs and memory. The new elfcorehdr
> > > is prepared into a buffer, and if no errors occur, it is
> > > installed over the top of the existing elfcorehdr.
> > > 
> > > In the patch 'crash hp: exclude elfcorehdr from the segment digest'
> > > the need to update purgatory due to the change in elfcorehdr was
> > > eliminated.  As a result, no changes to purgatory or boot_params
> > > (as the elfcorehdr= kernel command line parameter pointer
> > > remains unchanged and correct) are needed, just elfcorehdr.
> > > 
> > > To accommodate a growing number of resources via hotplug, the
> > > elfcorehdr segment must be sufficiently large enough to accommodate
> > > changes, see the CRASH_HOTPLUG_ELFCOREHDR_SZ configure item.
> > > 
> > > With this change, the kexec_file_load syscall (not kexec_load)
> > > is supported. When loading the crash kernel via kexec_file_load,
> > > the elfcorehdr is identified at load time in crash_load_segments().
> > > 
> > > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> > > ---
> > >   arch/x86/kernel/crash.c | 92 +++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 92 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > > index 9db41cce8d97..6215c51919db 100644
> > > --- a/arch/x86/kernel/crash.c
> > > +++ b/arch/x86/kernel/crash.c
> > > @@ -25,6 +25,7 @@
> > >   #include <linux/slab.h>
> > >   #include <linux/vmalloc.h>
> > >   #include <linux/memblock.h>
> > > +#include <linux/highmem.h>
> > >   #include <asm/processor.h>
> > >   #include <asm/hardirq.h>
> > > @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
> > >   	image->elf_headers = kbuf.buffer;
> > >   	image->elf_headers_sz = kbuf.bufsz;
> > > +#ifdef CONFIG_CRASH_HOTPLUG
> > > +	/* Ensure elfcorehdr segment large enough for hotplug changes */
> > > +	kbuf.memsz = CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ;
> > > +	/* For marking as usable to crash kernel */
> > > +	image->elf_headers_sz = kbuf.memsz;
> > > +	/* Record the index of the elfcorehdr segment */
> > > +	image->elf_index = image->nr_segments;
> > > +	image->elf_index_valid = true;
> > > +#else
> > >   	kbuf.memsz = kbuf.bufsz;
> > > +#endif
> > >   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> > >   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > >   	ret = kexec_add_buffer(&kbuf);
> > > @@ -413,3 +424,84 @@ int crash_load_segments(struct kimage *image)
> > >   	return ret;
> > >   }
> > >   #endif /* CONFIG_KEXEC_FILE */
> > > +
> > > +#ifdef CONFIG_CRASH_HOTPLUG
> > > +/**
> > > + * arch_crash_hotplug_handler() - Handle hotplug elfcorehdr 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 hot un/plug changes, the elfcorehdr (which
> > > + * is passed to the crash kernel via the elfcorehdr= parameter)
> > > + * must be updated with the new list of CPUs and memories. The new
> > > + * elfcorehdr is prepared in a kernel buffer, and if no errors,
> > > + * then it is written on top of the existing/old elfcorehdr.
> > > + *
> > > + * For hotplug changes to elfcorehdr to work, two conditions are
> > > + * needed:
> > > + * First, the segment containing the elfcorehdr must be large enough
> > > + * to permit a growing number of resources. See
> > > + * CONFIG_CRASH_HOTPLUG_ELFCOREHDR_SZ.
> > > + * Second, purgatory must explicitly exclude the elfcorehdr from the
> > > + * list of segments it checks (since the elfcorehdr changes and thus
> > > + * would require an update to purgatory itself to update the digest).
> > > + *
> > > + */
> > > +void arch_crash_hotplug_handler(struct kimage *image,
> > > +	unsigned int hp_action, unsigned long a, unsigned long b)
> > > +{
> > > +	struct kexec_segment *ksegment;
> > > +	unsigned char *ptr = NULL;
> > > +	unsigned long elfsz = 0;
> > > +	void *elfbuf = NULL;
> > > +	unsigned long mem, memsz;
> > > +
> > > +	/* Must have valid elfcorehdr index */
> > > +	if (!image->elf_index_valid) {
> > > +		pr_err("crash hp: unable to locate elfcorehdr segment");
> > > +		goto out;
> > > +	}
> > > +
> > > +	ksegment = &image->segment[image->elf_index];
> > > +	mem = ksegment->mem;
> > > +	memsz = ksegment->memsz;
> > > +
> > > +	/*
> > > +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
> > > +	 * memory resources. The elfcorehdr segment memsz must be
> > > +	 * sufficiently large to accommodate increases due to hotplug
> > > +	 * activity. See CRASH_HOTPLUG_ELFCOREHDR_SZ.
> > > +	 */
> > > +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
> > > +		pr_err("crash hp: unable to prepare elfcore headers");
> > > +		goto out;
> > > +	}
> > > +	if (elfsz > memsz) {
> > > +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz %lu",
> > > +			elfsz, memsz);
> > > +		goto out;
> > > +	}
> > > +
> > > +	/*
> > > +	 * At this point, we are all but assured of success.
> > > +	 * Copy new elfcorehdr into destination.
> > > +	 */
> > > +	ptr = map_crash_pages(mem, memsz);
> > > +	if (ptr) {
> > > +		/* Temporarily invalidate the crash image while it is replaced */
> > > +		xchg(&kexec_crash_image, NULL);
> > > +		/* Write the new elfcorehdr into memory */
> > > +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
> > > +		/* The crash image is now valid once again */
> > > +		xchg(&kexec_crash_image, image);
> > > +	}
> > > +	unmap_crash_pages((void **)&ptr);
> > > +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
> > > +
> > > +out:
> > > +	if (elfbuf)
> > > +		vfree(elfbuf);
> > > +}
> > > +#endif /* CONFIG_CRASH_HOTPLUG */
> > > -- 
> > > 2.27.0
> > > 
> > 
> 

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[David Hildenbrand]

On 03.03.22 11:22, Baoquan He wrote:
> On 03/02/22 at 10:20am, David Hildenbrand wrote:
>> On 01.03.22 21:04, Eric DeVolder wrote:
>>>
>>>
>>> On 2/22/22 21:25, Baoquan He wrote:
>>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>>>> Support for CPU and memory hotplug for crash is controlled by the
>>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>>>
>>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>>>> also introduced with this patch.
>>>>>
>>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>>>> ---
>>>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>>>   1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>>> index ebe8fc76949a..4e3374edab02 100644
>>>>> --- a/arch/x86/Kconfig
>>>>> +++ b/arch/x86/Kconfig
>>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>>>   	  (CONFIG_RELOCATABLE=y).
>>>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>>>   
>>>>> +config CRASH_HOTPLUG
>>>>> +	bool "kernel updates of crash elfcorehdr"
>>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>>>> +	help
>>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>>>> +	  plug/unplug change notifications via udev in order to
>>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>>>> +	  memory change notifications still occur (however, userspace is not
>>>>> +	  required to monitor for crash dump purposes).
>>>>> +
>>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>>>> +	depends on CRASH_HOTPLUG
>>>>> +	int
>>>>> +	default 131072
>>>>> +	help
>>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>>>> +	  region of memory.
>>>>> +	  For example, this size can accommodate hotplugging a machine
>>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>>>> +	  with 1024 1GiB memory DIMMs).
>>>>
>>>> This example of memory could be a little misleading. The memory regions
>>>> may not be related to memory DIMMs. System could split them into many
>>>> smaller regions during bootup.
>>>
>>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
>>> eric
>>
>> It's still not quite precise. Essentially it's the individual "System
>> RAM" entries in /proc/iomem
>>
>> Boot memory (i.e., a single DIMM) might be represented by multiple
>> entries due to rearranged holes (by the BIOS).
>>
>> While hoplugged DIMMs (under virt!) are usually represented using a
>> single range, it can be different on physical machines. Last but not
>> least, dax/kmem and virtio-mem behave in a different way.
> 
> Right. How about only mentioning the 'System RAM' entries in /proc/iomem
> as below? It's just giving an example, talking about the details of
> memory regions from each type may not be necessry here. People
> interested can refer to code or document related to get it.
> 
> 
> + default 131072
> + help
> +   Specify the maximum size of the elfcorehdr buffer/segment.
> +   The 128KiB default is sized so that it can accommodate 2048
> +   Elf64_Phdr, where each Phdr represents either a CPU or a
> +   region of memory.
> +   For example, this size can accommodate hotplugging a machine
> +   with up to 1024 CPUs and up to 1024 memory regions which are
>     represented by 'System RAM' entries in /proc/iomem.

Maybe changing the last paragraph to:

"For example, this size can accommodate a machine with up to 1024 CPUs
and up to 1024 memory regions, for example, as represented by 'System
RAM' entries in /proc/iomem."


-- 
Thanks,

David / dhildenb

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[David Hildenbrand]

On 03.03.22 11:22, Baoquan He wrote:
> On 03/02/22 at 10:20am, David Hildenbrand wrote:
>> On 01.03.22 21:04, Eric DeVolder wrote:
>>>
>>>
>>> On 2/22/22 21:25, Baoquan He wrote:
>>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>>>> Support for CPU and memory hotplug for crash is controlled by the
>>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>>>
>>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>>>> also introduced with this patch.
>>>>>
>>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>>>> ---
>>>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>>>   1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>>> index ebe8fc76949a..4e3374edab02 100644
>>>>> --- a/arch/x86/Kconfig
>>>>> +++ b/arch/x86/Kconfig
>>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>>>   	  (CONFIG_RELOCATABLE=y).
>>>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>>>   
>>>>> +config CRASH_HOTPLUG
>>>>> +	bool "kernel updates of crash elfcorehdr"
>>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>>>> +	help
>>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>>>> +	  plug/unplug change notifications via udev in order to
>>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>>>> +	  memory change notifications still occur (however, userspace is not
>>>>> +	  required to monitor for crash dump purposes).
>>>>> +
>>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>>>> +	depends on CRASH_HOTPLUG
>>>>> +	int
>>>>> +	default 131072
>>>>> +	help
>>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>>>> +	  region of memory.
>>>>> +	  For example, this size can accommodate hotplugging a machine
>>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>>>> +	  with 1024 1GiB memory DIMMs).
>>>>
>>>> This example of memory could be a little misleading. The memory regions
>>>> may not be related to memory DIMMs. System could split them into many
>>>> smaller regions during bootup.
>>>
>>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
>>> eric
>>
>> It's still not quite precise. Essentially it's the individual "System
>> RAM" entries in /proc/iomem
>>
>> Boot memory (i.e., a single DIMM) might be represented by multiple
>> entries due to rearranged holes (by the BIOS).
>>
>> While hoplugged DIMMs (under virt!) are usually represented using a
>> single range, it can be different on physical machines. Last but not
>> least, dax/kmem and virtio-mem behave in a different way.
> 
> Right. How about only mentioning the 'System RAM' entries in /proc/iomem
> as below? It's just giving an example, talking about the details of
> memory regions from each type may not be necessry here. People
> interested can refer to code or document related to get it.
> 
> 
> + default 131072
> + help
> +   Specify the maximum size of the elfcorehdr buffer/segment.
> +   The 128KiB default is sized so that it can accommodate 2048
> +   Elf64_Phdr, where each Phdr represents either a CPU or a
> +   region of memory.
> +   For example, this size can accommodate hotplugging a machine
> +   with up to 1024 CPUs and up to 1024 memory regions which are
>     represented by 'System RAM' entries in /proc/iomem.

Maybe changing the last paragraph to:

"For example, this size can accommodate a machine with up to 1024 CPUs
and up to 1024 memory regions, for example, as represented by 'System
RAM' entries in /proc/iomem."


-- 
Thanks,

David / dhildenb

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 03/03/22 at 12:36pm, David Hildenbrand wrote:
> On 03.03.22 11:22, Baoquan He wrote:
> > On 03/02/22 at 10:20am, David Hildenbrand wrote:
> >> On 01.03.22 21:04, Eric DeVolder wrote:
> >>>
> >>>
> >>> On 2/22/22 21:25, Baoquan He wrote:
> >>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> >>>>> Support for CPU and memory hotplug for crash is controlled by the
> >>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
> >>>>>
> >>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> >>>>> also introduced with this patch.
> >>>>>
> >>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> >>>>> ---
> >>>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
> >>>>>   1 file changed, 26 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >>>>> index ebe8fc76949a..4e3374edab02 100644
> >>>>> --- a/arch/x86/Kconfig
> >>>>> +++ b/arch/x86/Kconfig
> >>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
> >>>>>   	  (CONFIG_RELOCATABLE=y).
> >>>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
> >>>>>   
> >>>>> +config CRASH_HOTPLUG
> >>>>> +	bool "kernel updates of crash elfcorehdr"
> >>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> >>>>> +	help
> >>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
> >>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> >>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> >>>>> +	  plug/unplug change notifications via udev in order to
> >>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
> >>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
> >>>>> +	  memory change notifications still occur (however, userspace is not
> >>>>> +	  required to monitor for crash dump purposes).
> >>>>> +
> >>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> >>>>> +	depends on CRASH_HOTPLUG
> >>>>> +	int
> >>>>> +	default 131072
> >>>>> +	help
> >>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> >>>>> +	  The 128KiB default is sized so that it can accommodate 2048
> >>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> >>>>> +	  region of memory.
> >>>>> +	  For example, this size can accommodate hotplugging a machine
> >>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> >>>>> +	  with 1024 1GiB memory DIMMs).
> >>>>
> >>>> This example of memory could be a little misleading. The memory regions
> >>>> may not be related to memory DIMMs. System could split them into many
> >>>> smaller regions during bootup.
> >>>
> >>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> >>> eric
> >>
> >> It's still not quite precise. Essentially it's the individual "System
> >> RAM" entries in /proc/iomem
> >>
> >> Boot memory (i.e., a single DIMM) might be represented by multiple
> >> entries due to rearranged holes (by the BIOS).
> >>
> >> While hoplugged DIMMs (under virt!) are usually represented using a
> >> single range, it can be different on physical machines. Last but not
> >> least, dax/kmem and virtio-mem behave in a different way.
> > 
> > Right. How about only mentioning the 'System RAM' entries in /proc/iomem
> > as below? It's just giving an example, talking about the details of
> > memory regions from each type may not be necessry here. People
> > interested can refer to code or document related to get it.
> > 
> > 
> > + default 131072
> > + help
> > +   Specify the maximum size of the elfcorehdr buffer/segment.
> > +   The 128KiB default is sized so that it can accommodate 2048
> > +   Elf64_Phdr, where each Phdr represents either a CPU or a
> > +   region of memory.
> > +   For example, this size can accommodate hotplugging a machine
> > +   with up to 1024 CPUs and up to 1024 memory regions which are
> >     represented by 'System RAM' entries in /proc/iomem.
> 
> Maybe changing the last paragraph to:
> 
> "For example, this size can accommodate a machine with up to 1024 CPUs
> and up to 1024 memory regions, for example, as represented by 'System
> RAM' entries in /proc/iomem."

Yeah, this looks good. Can the 2nd 'for example' be removed or replaced
with 'e.g'? Please ignore it if it's normal to have two 'for example' in
one sentence, just gentlely ask.

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Baoquan He]

On 03/03/22 at 12:36pm, David Hildenbrand wrote:
> On 03.03.22 11:22, Baoquan He wrote:
> > On 03/02/22 at 10:20am, David Hildenbrand wrote:
> >> On 01.03.22 21:04, Eric DeVolder wrote:
> >>>
> >>>
> >>> On 2/22/22 21:25, Baoquan He wrote:
> >>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
> >>>>> Support for CPU and memory hotplug for crash is controlled by the
> >>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
> >>>>>
> >>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
> >>>>> also introduced with this patch.
> >>>>>
> >>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> >>>>> ---
> >>>>>   arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
> >>>>>   1 file changed, 26 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >>>>> index ebe8fc76949a..4e3374edab02 100644
> >>>>> --- a/arch/x86/Kconfig
> >>>>> +++ b/arch/x86/Kconfig
> >>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
> >>>>>   	  (CONFIG_RELOCATABLE=y).
> >>>>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
> >>>>>   
> >>>>> +config CRASH_HOTPLUG
> >>>>> +	bool "kernel updates of crash elfcorehdr"
> >>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
> >>>>> +	help
> >>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
> >>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
> >>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
> >>>>> +	  plug/unplug change notifications via udev in order to
> >>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
> >>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
> >>>>> +	  memory change notifications still occur (however, userspace is not
> >>>>> +	  required to monitor for crash dump purposes).
> >>>>> +
> >>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
> >>>>> +	depends on CRASH_HOTPLUG
> >>>>> +	int
> >>>>> +	default 131072
> >>>>> +	help
> >>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
> >>>>> +	  The 128KiB default is sized so that it can accommodate 2048
> >>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
> >>>>> +	  region of memory.
> >>>>> +	  For example, this size can accommodate hotplugging a machine
> >>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
> >>>>> +	  with 1024 1GiB memory DIMMs).
> >>>>
> >>>> This example of memory could be a little misleading. The memory regions
> >>>> may not be related to memory DIMMs. System could split them into many
> >>>> smaller regions during bootup.
> >>>
> >>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
> >>> eric
> >>
> >> It's still not quite precise. Essentially it's the individual "System
> >> RAM" entries in /proc/iomem
> >>
> >> Boot memory (i.e., a single DIMM) might be represented by multiple
> >> entries due to rearranged holes (by the BIOS).
> >>
> >> While hoplugged DIMMs (under virt!) are usually represented using a
> >> single range, it can be different on physical machines. Last but not
> >> least, dax/kmem and virtio-mem behave in a different way.
> > 
> > Right. How about only mentioning the 'System RAM' entries in /proc/iomem
> > as below? It's just giving an example, talking about the details of
> > memory regions from each type may not be necessry here. People
> > interested can refer to code or document related to get it.
> > 
> > 
> > + default 131072
> > + help
> > +   Specify the maximum size of the elfcorehdr buffer/segment.
> > +   The 128KiB default is sized so that it can accommodate 2048
> > +   Elf64_Phdr, where each Phdr represents either a CPU or a
> > +   region of memory.
> > +   For example, this size can accommodate hotplugging a machine
> > +   with up to 1024 CPUs and up to 1024 memory regions which are
> >     represented by 'System RAM' entries in /proc/iomem.
> 
> Maybe changing the last paragraph to:
> 
> "For example, this size can accommodate a machine with up to 1024 CPUs
> and up to 1024 memory regions, for example, as represented by 'System
> RAM' entries in /proc/iomem."

Yeah, this looks good. Can the 2nd 'for example' be removed or replaced
with 'e.g'? Please ignore it if it's normal to have two 'for example' in
one sentence, just gentlely ask.

Re: [PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

On 3/3/22 06:08, Baoquan He wrote:
> On 03/03/22 at 12:36pm, David Hildenbrand wrote:
>> On 03.03.22 11:22, Baoquan He wrote:
>>> On 03/02/22 at 10:20am, David Hildenbrand wrote:
>>>> On 01.03.22 21:04, Eric DeVolder wrote:
>>>>>
>>>>>
>>>>> On 2/22/22 21:25, Baoquan He wrote:
>>>>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>>>>>> Support for CPU and memory hotplug for crash is controlled by the
>>>>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>>>>>
>>>>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>>>>>> also introduced with this patch.
>>>>>>>
>>>>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>>>>>> ---
>>>>>>>    arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>>>>>    1 file changed, 26 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>>>>> index ebe8fc76949a..4e3374edab02 100644
>>>>>>> --- a/arch/x86/Kconfig
>>>>>>> +++ b/arch/x86/Kconfig
>>>>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>>>>>    	  (CONFIG_RELOCATABLE=y).
>>>>>>>    	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>>>>>    
>>>>>>> +config CRASH_HOTPLUG
>>>>>>> +	bool "kernel updates of crash elfcorehdr"
>>>>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>>>>>> +	help
>>>>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>>>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>>>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>>>>>> +	  plug/unplug change notifications via udev in order to
>>>>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>>>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>>>>>> +	  memory change notifications still occur (however, userspace is not
>>>>>>> +	  required to monitor for crash dump purposes).
>>>>>>> +
>>>>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>>>>>> +	depends on CRASH_HOTPLUG
>>>>>>> +	int
>>>>>>> +	default 131072
>>>>>>> +	help
>>>>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>>>>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>>>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>>>>>> +	  region of memory.
>>>>>>> +	  For example, this size can accommodate hotplugging a machine
>>>>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>>>>>> +	  with 1024 1GiB memory DIMMs).
>>>>>>
>>>>>> This example of memory could be a little misleading. The memory regions
>>>>>> may not be related to memory DIMMs. System could split them into many
>>>>>> smaller regions during bootup.
>>>>>
>>>>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
>>>>> eric
>>>>
>>>> It's still not quite precise. Essentially it's the individual "System
>>>> RAM" entries in /proc/iomem
>>>>
>>>> Boot memory (i.e., a single DIMM) might be represented by multiple
>>>> entries due to rearranged holes (by the BIOS).
>>>>
>>>> While hoplugged DIMMs (under virt!) are usually represented using a
>>>> single range, it can be different on physical machines. Last but not
>>>> least, dax/kmem and virtio-mem behave in a different way.
>>>
>>> Right. How about only mentioning the 'System RAM' entries in /proc/iomem
>>> as below? It's just giving an example, talking about the details of
>>> memory regions from each type may not be necessry here. People
>>> interested can refer to code or document related to get it.
>>>
>>>
>>> + default 131072
>>> + help
>>> +   Specify the maximum size of the elfcorehdr buffer/segment.
>>> +   The 128KiB default is sized so that it can accommodate 2048
>>> +   Elf64_Phdr, where each Phdr represents either a CPU or a
>>> +   region of memory.
>>> +   For example, this size can accommodate hotplugging a machine
>>> +   with up to 1024 CPUs and up to 1024 memory regions which are
>>>      represented by 'System RAM' entries in /proc/iomem.
>>
>> Maybe changing the last paragraph to:
>>
>> "For example, this size can accommodate a machine with up to 1024 CPUs
>> and up to 1024 memory regions, for example, as represented by 'System
>> RAM' entries in /proc/iomem."
> 
> Yeah, this looks good. Can the 2nd 'for example' be removed or replaced
> with 'e.g'? Please ignore it if it's normal to have two 'for example' in
> one sentence, just gentlely ask.
> 
Great, I will make the change to the text as agreed upon here!
eric

[PATCH v4 02/10] crash hp: Introduce CRASH_HOTPLUG configuration options

[Eric DeVolder]

On 3/3/22 06:08, Baoquan He wrote:
> On 03/03/22 at 12:36pm, David Hildenbrand wrote:
>> On 03.03.22 11:22, Baoquan He wrote:
>>> On 03/02/22 at 10:20am, David Hildenbrand wrote:
>>>> On 01.03.22 21:04, Eric DeVolder wrote:
>>>>>
>>>>>
>>>>> On 2/22/22 21:25, Baoquan He wrote:
>>>>>> On 02/09/22 at 02:56pm, Eric DeVolder wrote:
>>>>>>> Support for CPU and memory hotplug for crash is controlled by the
>>>>>>> CRASH_HOTPLUG configuration option, introduced by this patch.
>>>>>>>
>>>>>>> The CRASH_HOTPLUG_ELFCOREHDR_SZ related configuration option is
>>>>>>> also introduced with this patch.
>>>>>>>
>>>>>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>>>>>> ---
>>>>>>>    arch/x86/Kconfig | 26 ++++++++++++++++++++++++++
>>>>>>>    1 file changed, 26 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>>>>> index ebe8fc76949a..4e3374edab02 100644
>>>>>>> --- a/arch/x86/Kconfig
>>>>>>> +++ b/arch/x86/Kconfig
>>>>>>> @@ -2060,6 +2060,32 @@ config CRASH_DUMP
>>>>>>>    	  (CONFIG_RELOCATABLE=y).
>>>>>>>    	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>>>>>>    
>>>>>>> +config CRASH_HOTPLUG
>>>>>>> +	bool "kernel updates of crash elfcorehdr"
>>>>>>> +	depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG) && KEXEC_FILE
>>>>>>> +	help
>>>>>>> +	  Enable the kernel to update the crash elfcorehdr (which contains
>>>>>>> +	  the list of CPUs and memory regions) directly when hot plug/unplug
>>>>>>> +	  of CPUs or memory. Otherwise userspace must monitor these hot
>>>>>>> +	  plug/unplug change notifications via udev in order to
>>>>>>> +	  unload-then-reload the crash kernel so that the list of CPUs and
>>>>>>> +	  memory regions is kept up-to-date. Note that the udev CPU and
>>>>>>> +	  memory change notifications still occur (however, userspace is not
>>>>>>> +	  required to monitor for crash dump purposes).
>>>>>>> +
>>>>>>> +config CRASH_HOTPLUG_ELFCOREHDR_SZ
>>>>>>> +	depends on CRASH_HOTPLUG
>>>>>>> +	int
>>>>>>> +	default 131072
>>>>>>> +	help
>>>>>>> +	  Specify the maximum size of the elfcorehdr buffer/segment.
>>>>>>> +	  The 128KiB default is sized so that it can accommodate 2048
>>>>>>> +	  Elf64_Phdr, where each Phdr represents either a CPU or a
>>>>>>> +	  region of memory.
>>>>>>> +	  For example, this size can accommodate hotplugging a machine
>>>>>>> +	  with up to 1024 CPUs and up to 1024 memory regions (e.g. 1TiB
>>>>>>> +	  with 1024 1GiB memory DIMMs).
>>>>>>
>>>>>> This example of memory could be a little misleading. The memory regions
>>>>>> may not be related to memory DIMMs. System could split them into many
>>>>>> smaller regions during bootup.
>>>>>
>>>>> I changed "with 1024 1GiB memory DIMMs" to "with 1024 1GiB hotplug memories".
>>>>> eric
>>>>
>>>> It's still not quite precise. Essentially it's the individual "System
>>>> RAM" entries in /proc/iomem
>>>>
>>>> Boot memory (i.e., a single DIMM) might be represented by multiple
>>>> entries due to rearranged holes (by the BIOS).
>>>>
>>>> While hoplugged DIMMs (under virt!) are usually represented using a
>>>> single range, it can be different on physical machines. Last but not
>>>> least, dax/kmem and virtio-mem behave in a different way.
>>>
>>> Right. How about only mentioning the 'System RAM' entries in /proc/iomem
>>> as below? It's just giving an example, talking about the details of
>>> memory regions from each type may not be necessry here. People
>>> interested can refer to code or document related to get it.
>>>
>>>
>>> + default 131072
>>> + help
>>> +   Specify the maximum size of the elfcorehdr buffer/segment.
>>> +   The 128KiB default is sized so that it can accommodate 2048
>>> +   Elf64_Phdr, where each Phdr represents either a CPU or a
>>> +   region of memory.
>>> +   For example, this size can accommodate hotplugging a machine
>>> +   with up to 1024 CPUs and up to 1024 memory regions which are
>>>      represented by 'System RAM' entries in /proc/iomem.
>>
>> Maybe changing the last paragraph to:
>>
>> "For example, this size can accommodate a machine with up to 1024 CPUs
>> and up to 1024 memory regions, for example, as represented by 'System
>> RAM' entries in /proc/iomem."
> 
> Yeah, this looks good. Can the 2nd 'for example' be removed or replaced
> with 'e.g'? Please ignore it if it's normal to have two 'for example' in
> one sentence, just gentlely ask.
> 
Great, I will make the change to the text as agreed upon here!
eric