Re: [PATCH v27 2/8] crash: add generic infrastructure for crash hotplug support

From: Eric DeVolder
Date: Tue Aug 15 2023 - 10:42:40 EST




On 8/12/23 05:47, Sourabh Jain wrote:
Hello Eric,

On 11/08/23 22:36, Eric DeVolder wrote:
To support crash hotplug, a mechanism is needed to update the crash
elfcorehdr upon CPU or memory changes (eg. hot un/plug or off/
onlining). The crash elfcorehdr describes the CPUs and memory to
be written into the vmcore.

To track CPU changes, callbacks are registered with the cpuhp
mechanism via cpuhp_setup_state_nocalls(CPUHP_BP_PREPARE_DYN). The
crash hotplug elfcorehdr update has no explicit ordering requirement
(relative to other cpuhp states), so meets the criteria for
utilizing CPUHP_BP_PREPARE_DYN. CPUHP_BP_PREPARE_DYN is a dynamic
state and avoids the need to introduce a new state for crash
hotplug. Also, CPUHP_BP_PREPARE_DYN is the last state in the PREPARE
group, just prior to the STARTING group, which is very close to the
CPU starting up in a plug/online situation, or stopping in a unplug/
offline situation. This minimizes the window of time during an
actual plug/online or unplug/offline situation in which the
elfcorehdr would be inaccurate. Note that for a CPU being unplugged
or offlined, the CPU will still be present in the list of CPUs
generated by crash_prepare_elf64_headers(). However, there is no
need to explicitly omit the CPU, see justification in
'crash: change crash_prepare_elf64_headers() to for_each_possible_cpu()'.

To track memory changes, a notifier is registered to capture the
memblock MEM_ONLINE and MEM_OFFLINE events via register_memory_notifier().

The CPU callbacks and memory notifiers invoke crash_handle_hotplug_event()
which performs needed tasks and then dispatches the event to the
architecture specific arch_crash_handle_hotplug_event() to update the
elfcorehdr with the current state of CPUs and memory. During the
process, the kexec_lock is held.

Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
Reviewed-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
Acked-by: Hari Bathini <hbathini@xxxxxxxxxxxxx>
Acked-by: Baoquan He <bhe@xxxxxxxxxx>
---
  include/linux/crash_core.h |   9 +++
  include/linux/kexec.h      |  11 +++
  kernel/Kconfig.kexec       |  31 ++++++++
  kernel/crash_core.c        | 142 +++++++++++++++++++++++++++++++++++++
  kernel/kexec_core.c        |   6 ++
  5 files changed, 199 insertions(+)

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index de62a722431e..e14345cc7a22 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -84,4 +84,13 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
  int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
          unsigned long long *crash_size, unsigned long long *crash_base);
+#define KEXEC_CRASH_HP_NONE            0
+#define KEXEC_CRASH_HP_ADD_CPU            1
+#define KEXEC_CRASH_HP_REMOVE_CPU        2
+#define KEXEC_CRASH_HP_ADD_MEMORY        3
+#define KEXEC_CRASH_HP_REMOVE_MEMORY        4
+#define KEXEC_CRASH_HP_INVALID_CPU        -1U
+
+struct kimage;
+
  #endif /* LINUX_CRASH_CORE_H */
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 811a90e09698..b9903dd48e24 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -33,6 +33,7 @@ extern note_buf_t __percpu *crash_notes;
  #include <linux/compat.h>
  #include <linux/ioport.h>
  #include <linux/module.h>
+#include <linux/highmem.h>
  #include <asm/kexec.h>
  /* Verify architecture specific macros are defined */
@@ -360,6 +361,12 @@ struct kimage {
      struct purgatory_info purgatory_info;
  #endif
+#ifdef CONFIG_CRASH_HOTPLUG
+    int hp_action;
+    int elfcorehdr_index;
+    bool elfcorehdr_updated;
+#endif
+
  #ifdef CONFIG_IMA_KEXEC
      /* Virtual address of IMA measurement buffer for kexec syscall */
      void *ima_buffer;
@@ -490,6 +497,10 @@ static inline int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, g
  static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) { }
  #endif
+#ifndef arch_crash_handle_hotplug_event
+static inline void arch_crash_handle_hotplug_event(struct kimage *image) { }
+#endif
+

Isn't the above function should be declare under CONFIG_CRASH_HOTPLUG?

Thanks,
Sourabh

There are no compiler warnings/errors, due to the nature of being declared static inline.
And most of the other functions defined in a similar way in this file are not guard banded
by CONFIG ifdefs. I'm inclined to leave it this way.
Thanks!
eric

  #else /* !CONFIG_KEXEC_CORE */
  struct pt_regs;
  struct task_struct;
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index ff72e45cfaef..d0a9a5392035 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -113,4 +113,35 @@ config CRASH_DUMP
        For s390, this option also enables zfcpdump.
        See also <file:Documentation/s390/zfcpdump.rst>
+config CRASH_HOTPLUG
+    bool "Update the crash elfcorehdr on system configuration changes"
+    default y
+    depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG)
+    depends on ARCH_SUPPORTS_CRASH_HOTPLUG
+    help
+      Enable direct update to the crash elfcorehdr (which contains
+      the list of CPUs and memory regions to be dumped upon a crash)
+      in response to hot plug/unplug or online/offline of CPUs or
+      memory. This is a much more advanced approach than userspace
+      attempting that.
+
+      If unsure, say Y.
+
+config CRASH_MAX_MEMORY_RANGES
+    int "Specify the maximum number of memory regions for the elfcorehdr"
+    default 8192
+    depends on CRASH_HOTPLUG
+    help
+      For the kexec_file_load() syscall path, specify the maximum number of
+      memory regions that the elfcorehdr buffer/segment can accommodate.
+      These regions are obtained via walk_system_ram_res(); eg. the
+      'System RAM' entries in /proc/iomem.
+      This value is combined with NR_CPUS_DEFAULT and multiplied by
+      sizeof(Elf64_Phdr) to determine the final elfcorehdr memory buffer/
+      segment size.
+      The value 8192, for example, covers a (sparsely populated) 1TiB system
+      consisting of 128MiB memblocks, while resulting in an elfcorehdr
+      memory buffer/segment size under 1MiB. This represents a sane choice
+      to accommodate both baremetal and virtual machine configurations.
+
  endmenu
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index b7c30b748a16..53d211c690a1 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -11,6 +11,8 @@
  #include <linux/vmalloc.h>
  #include <linux/sizes.h>
  #include <linux/kexec.h>
+#include <linux/memory.h>
+#include <linux/cpuhotplug.h>
  #include <asm/page.h>
  #include <asm/sections.h>
@@ -18,6 +20,7 @@
  #include <crypto/sha1.h>
  #include "kallsyms_internal.h"
+#include "kexec_internal.h"
  /* vmcoreinfo stuff */
  unsigned char *vmcoreinfo_data;
@@ -697,3 +700,142 @@ static int __init crash_save_vmcoreinfo_init(void)
  }
  subsys_initcall(crash_save_vmcoreinfo_init);
+
+#ifdef CONFIG_CRASH_HOTPLUG
+#undef pr_fmt
+#define pr_fmt(fmt) "crash hp: " fmt
+/*
+ * To accurately reflect hot un/plug changes of cpu and memory resources
+ * (including onling and offlining of those resources), the elfcorehdr
+ * (which is passed to the crash kernel via the elfcorehdr= parameter)
+ * must be updated with the new list of CPUs and memories.
+ *
+ * In order to make changes to elfcorehdr, two conditions are needed:
+ * First, the segment containing the elfcorehdr must be large enough
+ * to permit a growing number of resources; the elfcorehdr memory size
+ * is based on NR_CPUS_DEFAULT and CRASH_MAX_MEMORY_RANGES.
+ * 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).
+ */
+static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
+{
+    struct kimage *image;
+
+    /* Obtain lock while changing crash information */
+    if (!kexec_trylock()) {
+        pr_info("kexec_trylock() failed, elfcorehdr may be inaccurate\n");
+        return;
+    }
+
+    /* Check kdump is not loaded */
+    if (!kexec_crash_image)
+        goto out;
+
+    image = kexec_crash_image;
+
+    if (hp_action == KEXEC_CRASH_HP_ADD_CPU ||
+        hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+        pr_debug("hp_action %u, cpu %u\n", hp_action, cpu);
+    else
+        pr_debug("hp_action %u\n", hp_action);
+
+    /*
+     * The elfcorehdr_index is set to -1 when the struct kimage
+     * is allocated. Find the segment containing the elfcorehdr,
+     * if not already found.
+     */
+    if (image->elfcorehdr_index < 0) {
+        unsigned long mem;
+        unsigned char *ptr;
+        unsigned int n;
+
+        for (n = 0; n < image->nr_segments; n++) {
+            mem = image->segment[n].mem;
+            ptr = kmap_local_page(pfn_to_page(mem >> PAGE_SHIFT));
+            if (ptr) {
+                /* The segment containing elfcorehdr */
+                if (memcmp(ptr, ELFMAG, SELFMAG) == 0)
+                    image->elfcorehdr_index = (int)n;
+                kunmap_local(ptr);
+            }
+        }
+    }
+
+    if (image->elfcorehdr_index < 0) {
+        pr_err("unable to locate elfcorehdr segment");
+        goto out;
+    }
+
+    /* Needed in order for the segments to be updated */
+    arch_kexec_unprotect_crashkres();
+
+    /* Differentiate between normal load and hotplug update */
+    image->hp_action = hp_action;
+
+    /* Now invoke arch-specific update handler */
+    arch_crash_handle_hotplug_event(image);
+
+    /* No longer handling a hotplug event */
+    image->hp_action = KEXEC_CRASH_HP_NONE;
+    image->elfcorehdr_updated = true;
+
+    /* Change back to read-only */
+    arch_kexec_protect_crashkres();
+
+    /* Errors in the callback is not a reason to rollback state */
+out:
+    /* Release lock now that update complete */
+    kexec_unlock();
+}
+
+static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, void *v)
+{
+    switch (val) {
+    case MEM_ONLINE:
+        crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY,
+            KEXEC_CRASH_HP_INVALID_CPU);
+        break;
+
+    case MEM_OFFLINE:
+        crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY,
+            KEXEC_CRASH_HP_INVALID_CPU);
+        break;
+    }
+    return NOTIFY_OK;
+}
+
+static struct notifier_block crash_memhp_nb = {
+    .notifier_call = crash_memhp_notifier,
+    .priority = 0
+};
+
+static int crash_cpuhp_online(unsigned int cpu)
+{
+    crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_CPU, cpu);
+    return 0;
+}
+
+static int crash_cpuhp_offline(unsigned int cpu)
+{
+    crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_CPU, cpu);
+    return 0;
+}
+
+static int __init crash_hotplug_init(void)
+{
+    int result = 0;
+
+    if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
+        register_memory_notifier(&crash_memhp_nb);
+
+    if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
+        result = cpuhp_setup_state_nocalls(CPUHP_BP_PREPARE_DYN,
+            "crash/cpuhp", crash_cpuhp_online, crash_cpuhp_offline);
+    }
+
+    return result;
+}
+
+subsys_initcall(crash_hotplug_init);
+#endif
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index e2f2574d8b74..5d323255862a 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -277,6 +277,12 @@ struct kimage *do_kimage_alloc_init(void)
      /* Initialize the list of unusable pages */
      INIT_LIST_HEAD(&image->unusable_pages);
+#ifdef CONFIG_CRASH_HOTPLUG
+    image->hp_action = KEXEC_CRASH_HP_NONE;
+    image->elfcorehdr_index = -1;
+    image->elfcorehdr_updated = false;
+#endif
+
      return image;
  }