Re: [PATCH v15 5/5] powerpc: add crash memory hotplug support

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 23/01/24 15:52, Hari Bathini wrote:
>
>
> On 11/01/24 4:21 pm, Sourabh Jain wrote:
>> Extend the arch crash hotplug handler, as introduced by the patch title
>> ("powerpc: add crash CPU hotplug support"), to also support memory
>> add/remove events.
>>
>> Elfcorehdr describes the memory of the crash kernel to capture the
>> kernel; hence, it needs to be updated if memory resources change due to
>> memory add/remove events. Therefore, arch_crash_handle_hotplug_event()
>> is updated to recreate the elfcorehdr and replace it with the previous
>> one on memory add/remove events.
>>
>> The memblock list is used to prepare the elfcorehdr. In the case of
>> memory hot removal, the memblock list is updated after the arch crash
>> hotplug handler is triggered, as depicted in Figure 1. Thus, the
>> hot-removed memory is explicitly removed from the crash memory ranges
>> to ensure that the memory ranges added to elfcorehdr do not include the
>> hot-removed memory.
>>
>>      Memory remove
>>            |
>>            v
>>      Offline pages
>>            |
>>            v
>>   Initiate memory notify call <----> crash hotplug handler
>>   chain for MEM_OFFLINE event
>>            |
>>            v
>>   Update memblock list
>>
>>       Figure 1
>>
>> There are two system calls, `kexec_file_load` and `kexec_load`, used to
>> load the kdump image. A few changes have been made to ensure that the
>> kernel can safely update the elfcorehdr component of the kdump image for
>> both system calls.
>>
>> For the kexec_file_load syscall, kdump image is prepared in the kernel.
>> To support an increasing number of memory regions, the elfcorehdr is
>> built with extra buffer space to ensure that it can accommodate
>> additional memory ranges in future.
>>
>> For the kexec_load syscall, the elfcorehdr is updated only if the
>> KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag is passed to the kernel by the
>> kexec tool. Passing this flag to the kernel indicates that the
>> elfcorehdr is built to accommodate additional memory ranges and the
>> elfcorehdr segment is not considered for SHA calculation, making it safe
>> to update.
>>
>> The changes related to this feature are kept under the CRASH_HOTPLUG
>> config, and it is enabled by default.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Cc: Akhil Raj <lf32.dev@gmail.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Borislav Petkov (AMD) <bp@alien8.de>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>> Cc: Laurent Dufour <laurent.dufour@fr.ibm.com>
>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Mimi Zohar <zohar@linux.ibm.com>
>> Cc: Naveen N Rao <naveen@kernel.org>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Valentin Schneider <vschneid@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: kexec@lists.infradead.org
>> Cc: x86@kernel.org
>> ---
>>   arch/powerpc/include/asm/kexec.h        |   5 +-
>>   arch/powerpc/include/asm/kexec_ranges.h |   1 +
>>   arch/powerpc/kexec/core_64.c            | 107 +++++++++++++++++++++++-
>>   arch/powerpc/kexec/file_load_64.c       |  34 +++++++-
>>   arch/powerpc/kexec/ranges.c             |  85 +++++++++++++++++++
>>   5 files changed, 225 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kexec.h 
>> b/arch/powerpc/include/asm/kexec.h
>> index 943e58eb9bff..25ff5b7f1a28 100644
>> --- a/arch/powerpc/include/asm/kexec.h
>> +++ b/arch/powerpc/include/asm/kexec.h
>> @@ -116,8 +116,11 @@ int get_crash_memory_ranges(struct crash_mem 
>> **mem_ranges);
>>   #ifdef CONFIG_CRASH_HOTPLUG
>>   void arch_crash_handle_hotplug_event(struct kimage *image, void *arg);
>>   #define arch_crash_handle_hotplug_event 
>> arch_crash_handle_hotplug_event
>> -#endif /*CONFIG_CRASH_HOTPLUG */
>>   +unsigned int arch_crash_get_elfcorehdr_size(void);
>> +#define crash_get_elfcorehdr_size arch_crash_get_elfcorehdr_size
>> +
>> +#endif /*CONFIG_CRASH_HOTPLUG */
>>   #endif /* CONFIG_PPC64 */
>>     #ifdef CONFIG_KEXEC_FILE
>> diff --git a/arch/powerpc/include/asm/kexec_ranges.h 
>> b/arch/powerpc/include/asm/kexec_ranges.h
>> index f83866a19e87..802abf580cf0 100644
>> --- a/arch/powerpc/include/asm/kexec_ranges.h
>> +++ b/arch/powerpc/include/asm/kexec_ranges.h
>> @@ -7,6 +7,7 @@
>>   void sort_memory_ranges(struct crash_mem *mrngs, bool merge);
>>   struct crash_mem *realloc_mem_ranges(struct crash_mem **mem_ranges);
>>   int add_mem_range(struct crash_mem **mem_ranges, u64 base, u64 size);
>> +int remove_mem_range(struct crash_mem **mem_ranges, u64 base, u64 
>> size);
>>   int add_tce_mem_ranges(struct crash_mem **mem_ranges);
>>   int add_initrd_mem_range(struct crash_mem **mem_ranges);
>>   #ifdef CONFIG_PPC_64S_HASH_MMU
>> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
>> index 43fcd78c2102..4673f150f973 100644
>> --- a/arch/powerpc/kexec/core_64.c
>> +++ b/arch/powerpc/kexec/core_64.c
>> @@ -19,8 +19,11 @@
>>   #include <linux/of.h>
>>   #include <linux/libfdt.h>
>>   #include <linux/memblock.h>
>> +#include <linux/memory.h>
>>     #include <asm/page.h>
>> +#include <asm/drmem.h>
>> +#include <asm/mmzone.h>
>>   #include <asm/current.h>
>>   #include <asm/machdep.h>
>>   #include <asm/cacheflush.h>
>> @@ -546,6 +549,101 @@ int update_cpus_node(void *fdt)
>>   #undef pr_fmt
>>   #define pr_fmt(fmt) "crash hp: " fmt
>>   +/*
>> + * Advertise preferred elfcorehdr size to userspace via
>> + * /sys/kernel/crash_elfcorehdr_size sysfs interface.
>> + */
>> +unsigned int arch_crash_get_elfcorehdr_size(void)
>> +{
>> +    unsigned int sz;
>> +    unsigned long elf_phdr_cnt;
>> +
>> +    /* Program header for CPU notes and vmcoreinfo */
>> +    elf_phdr_cnt = 2;
>> +    if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>> +        /* In the worst case, a Phdr is needed for every other LMB 
>> to be
>> +         * represented as an individual crash range.
>> +         */
>> +        elf_phdr_cnt += memory_hotplug_max() / (2 * drmem_lmb_size());
>> +
>> +    /* Do not cross the max limit */
>> +    if (elf_phdr_cnt > PN_XNUM)
>> +        elf_phdr_cnt = PN_XNUM;
>> +
>> +    sz = sizeof(struct elfhdr) + (elf_phdr_cnt * sizeof(Elf64_Phdr));
>> +    return sz;
>> +}
>> +
>> +/**
>> + * update_crash_elfcorehdr() - Recreate the elfcorehdr and replace 
>> it with old
>> + *                   elfcorehdr in the kexec segment array.
>> + * @image: the active struct kimage
>> + * @mn: struct memory_notify data handler
>> + */
>> +static void update_crash_elfcorehdr(struct kimage *image, struct 
>> memory_notify *mn)
>> +{
>> +    int ret;
>> +    struct crash_mem *cmem = NULL;
>> +    struct kexec_segment *ksegment;
>> +    void *ptr, *mem, *elfbuf = NULL;
>> +    unsigned long elfsz, memsz, base_addr, size;
>> +
>> +    ksegment = &image->segment[image->elfcorehdr_index];
>> +    mem = (void *) ksegment->mem;
>> +    memsz = ksegment->memsz;
>> +
>> +    ret = get_crash_memory_ranges(&cmem);
>> +    if (ret) {
>> +        pr_err("Failed to get crash mem range\n");
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * The hot unplugged memory is part of crash memory ranges,
>> +     * remove it here.
>> +     */
>> +    if (image->hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY) {
>> +        base_addr = PFN_PHYS(mn->start_pfn);
>> +        size = mn->nr_pages * PAGE_SIZE;
>> +        ret = remove_mem_range(&cmem, base_addr, size);
>> +        if (ret) {
>> +            pr_err("Failed to remove hot-unplugged from crash memory 
>> ranges.\n");
>> +            return;
>> +        }
>> +    }
>> +
>> +    ret = crash_prepare_elf64_headers(cmem, false, &elfbuf, &elfsz);
>> +    if (ret) {
>> +        pr_err("Failed to prepare elf header\n");
>> +        return;
>> +    }
>> +
>
>
>
>> +    /*
>> +     * It is unlikely that kernel hit this because elfcorehdr kexec
>> +     * segment (memsz) is built with addition space to accommodate 
>> growing
>> +     * number of crash memory ranges while loading the kdump kernel. 
>> It is
>> +     * Just to avoid any unforeseen case.
>
> nitpick..
>
> s/Just/just/

Thanks I will fix it.

>
>> +     */
>> +    if (elfsz > memsz) {
>> +        pr_err("Updated crash elfcorehdr elfsz %lu > memsz %lu", 
>> elfsz, memsz);
>> +        goto out;
>> +    }
>> +
>> +    ptr = __va(mem);
>> +    if (ptr) {
>> +        /* Temporarily invalidate the crash image while it is 
>> replaced */
>> +        xchg(&kexec_crash_image, NULL);
>> +
>> +        /* Replace the old elfcorehdr with newly prepared elfcorehdr */
>> +        memcpy((void *)ptr, elfbuf, elfsz);
>> +
>> +        /* The crash image is now valid once again */
>> +        xchg(&kexec_crash_image, image);
>> +    }
>> +out:
>> +    vfree(elfbuf);
>> +}
>> +
>>   /**
>>    * arch_crash_handle_hotplug_event - Handle crash CPU/Memory 
>> hotplug events to update the
>>    *                     necessary kexec segments based on the 
>> hotplug event.
>> @@ -556,7 +654,7 @@ int update_cpus_node(void *fdt)
>>    * CPU addition: Update the FDT segment to include the newly added 
>> CPU.
>>    * CPU removal: No action is needed, with the assumption that it's 
>> okay to have offline CPUs
>>    *        as part of the FDT.
>> - * Memory addition/removal: No action is taken as this is not yet 
>> supported.
>> + * Memory addition/removal: Recreate the elfcorehdr segment
>>    */
>>   void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>>   {
>> @@ -570,7 +668,6 @@ void arch_crash_handle_hotplug_event(struct 
>> kimage *image, void *arg)
>>           return;
>
>>       } else if (hp_action == KEXEC_CRASH_HP_ADD_CPU) {
>> -
>
>>           void *fdt, *ptr;
>>           unsigned long mem;
>>           int i, fdt_index = -1;
>> @@ -605,8 +702,10 @@ void arch_crash_handle_hotplug_event(struct 
>> kimage *image, void *arg)
>
>>       } else if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY ||
>>              hp_action == KEXEC_CRASH_HP_ADD_MEMORY) {
>
>> -        pr_info_once("Crash update is not supported for memory 
>> hotplug\n");
>> -        return;
>> +        struct memory_notify *mn;
>> +
>> +        mn = (struct memory_notify *)arg;
>> +        update_crash_elfcorehdr(image, mn);
>>       }
>
> Use switch case statement for hotplug actions for better readability..

Sure.

Thanks,
Sourabh

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec