At the outcome of this patch set, the crash_prepare_elf64_headers()
is utilized on both the kexec_file_load() and kexec_load() paths. As
such, need to move this function out of kexec_file.c and into a
common location crash_core.c.
No functionality change.
Signed-off-by: Eric DeVolder <[email protected]>
Acked-by: Baoquan He <[email protected]>
---
kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
kernel/kexec_file.c | 99 -------------------------------------------
2 files changed, 100 insertions(+), 99 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index a0eb4d5cf557..46c160d14045 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -10,6 +10,7 @@
#include <linux/utsname.h>
#include <linux/vmalloc.h>
#include <linux/sizes.h>
+#include <linux/kexec.h>
#include <asm/page.h>
#include <asm/sections.h>
@@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
}
early_param("crashkernel", parse_crashkernel_dummy);
+int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
+ void **addr, unsigned long *sz)
+{
+ Elf64_Ehdr *ehdr;
+ Elf64_Phdr *phdr;
+ unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
+ unsigned char *buf;
+ unsigned int cpu, i;
+ unsigned long long notes_addr;
+ unsigned long mstart, mend;
+
+ /* extra phdr for vmcoreinfo ELF note */
+ nr_phdr = nr_cpus + 1;
+ nr_phdr += mem->nr_ranges;
+
+ /*
+ * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
+ * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
+ * I think this is required by tools like gdb. So same physical
+ * memory will be mapped in two ELF headers. One will contain kernel
+ * text virtual addresses and other will have __va(physical) addresses.
+ */
+
+ nr_phdr++;
+ elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
+ elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
+
+ buf = vzalloc(elf_sz);
+ if (!buf)
+ return -ENOMEM;
+
+ ehdr = (Elf64_Ehdr *)buf;
+ phdr = (Elf64_Phdr *)(ehdr + 1);
+ memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
+ ehdr->e_ident[EI_CLASS] = ELFCLASS64;
+ ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
+ ehdr->e_ident[EI_VERSION] = EV_CURRENT;
+ ehdr->e_ident[EI_OSABI] = ELF_OSABI;
+ memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
+ ehdr->e_type = ET_CORE;
+ ehdr->e_machine = ELF_ARCH;
+ ehdr->e_version = EV_CURRENT;
+ ehdr->e_phoff = sizeof(Elf64_Ehdr);
+ ehdr->e_ehsize = sizeof(Elf64_Ehdr);
+ ehdr->e_phentsize = sizeof(Elf64_Phdr);
+
+ /* Prepare one phdr of type PT_NOTE for each present CPU */
+ for_each_present_cpu(cpu) {
+ 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;
+ phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
+ (ehdr->e_phnum)++;
+ phdr++;
+ }
+
+ /* Prepare one PT_NOTE header for vmcoreinfo */
+ phdr->p_type = PT_NOTE;
+ phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
+ phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
+ (ehdr->e_phnum)++;
+ phdr++;
+
+ /* Prepare PT_LOAD type program header for kernel text region */
+ if (need_kernel_map) {
+ phdr->p_type = PT_LOAD;
+ phdr->p_flags = PF_R|PF_W|PF_X;
+ phdr->p_vaddr = (unsigned long) _text;
+ phdr->p_filesz = phdr->p_memsz = _end - _text;
+ phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
+ ehdr->e_phnum++;
+ phdr++;
+ }
+
+ /* Go through all the ranges in mem->ranges[] and prepare phdr */
+ for (i = 0; i < mem->nr_ranges; i++) {
+ mstart = mem->ranges[i].start;
+ mend = mem->ranges[i].end;
+
+ phdr->p_type = PT_LOAD;
+ phdr->p_flags = PF_R|PF_W|PF_X;
+ phdr->p_offset = mstart;
+
+ phdr->p_paddr = mstart;
+ phdr->p_vaddr = (unsigned long) __va(mstart);
+ phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
+ phdr->p_align = 0;
+ ehdr->e_phnum++;
+ pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
+ phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
+ ehdr->e_phnum, phdr->p_offset);
+ phdr++;
+ }
+
+ *addr = buf;
+ *sz = elf_sz;
+ return 0;
+}
+
Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
void *data, size_t data_len)
{
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 45637511e0de..f98d1742872b 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -1217,102 +1217,3 @@ int crash_exclude_mem_range(struct crash_mem *mem,
mem->nr_ranges++;
return 0;
}
-
-int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
- void **addr, unsigned long *sz)
-{
- Elf64_Ehdr *ehdr;
- Elf64_Phdr *phdr;
- unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
- unsigned char *buf;
- unsigned int cpu, i;
- unsigned long long notes_addr;
- unsigned long mstart, mend;
-
- /* extra phdr for vmcoreinfo ELF note */
- nr_phdr = nr_cpus + 1;
- nr_phdr += mem->nr_ranges;
-
- /*
- * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
- * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
- * I think this is required by tools like gdb. So same physical
- * memory will be mapped in two ELF headers. One will contain kernel
- * text virtual addresses and other will have __va(physical) addresses.
- */
-
- nr_phdr++;
- elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
- elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
-
- buf = vzalloc(elf_sz);
- if (!buf)
- return -ENOMEM;
-
- ehdr = (Elf64_Ehdr *)buf;
- phdr = (Elf64_Phdr *)(ehdr + 1);
- memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
- ehdr->e_ident[EI_CLASS] = ELFCLASS64;
- ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
- ehdr->e_ident[EI_VERSION] = EV_CURRENT;
- ehdr->e_ident[EI_OSABI] = ELF_OSABI;
- memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
- ehdr->e_type = ET_CORE;
- ehdr->e_machine = ELF_ARCH;
- ehdr->e_version = EV_CURRENT;
- ehdr->e_phoff = sizeof(Elf64_Ehdr);
- ehdr->e_ehsize = sizeof(Elf64_Ehdr);
- ehdr->e_phentsize = sizeof(Elf64_Phdr);
-
- /* Prepare one phdr of type PT_NOTE for each present CPU */
- for_each_present_cpu(cpu) {
- 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;
- phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
- (ehdr->e_phnum)++;
- phdr++;
- }
-
- /* Prepare one PT_NOTE header for vmcoreinfo */
- phdr->p_type = PT_NOTE;
- phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
- phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
- (ehdr->e_phnum)++;
- phdr++;
-
- /* Prepare PT_LOAD type program header for kernel text region */
- if (need_kernel_map) {
- phdr->p_type = PT_LOAD;
- phdr->p_flags = PF_R|PF_W|PF_X;
- phdr->p_vaddr = (unsigned long) _text;
- phdr->p_filesz = phdr->p_memsz = _end - _text;
- phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
- ehdr->e_phnum++;
- phdr++;
- }
-
- /* Go through all the ranges in mem->ranges[] and prepare phdr */
- for (i = 0; i < mem->nr_ranges; i++) {
- mstart = mem->ranges[i].start;
- mend = mem->ranges[i].end;
-
- phdr->p_type = PT_LOAD;
- phdr->p_flags = PF_R|PF_W|PF_X;
- phdr->p_offset = mstart;
-
- phdr->p_paddr = mstart;
- phdr->p_vaddr = (unsigned long) __va(mstart);
- phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
- phdr->p_align = 0;
- ehdr->e_phnum++;
- pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
- phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
- ehdr->e_phnum, phdr->p_offset);
- phdr++;
- }
-
- *addr = buf;
- *sz = elf_sz;
- return 0;
-}
--
2.31.1
On 09/12/22 21:06, Eric DeVolder wrote:
> At the outcome of this patch set, the crash_prepare_elf64_headers()
> is utilized on both the kexec_file_load() and kexec_load() paths. As
> such, need to move this function out of kexec_file.c and into a
> common location crash_core.c.
>
> No functionality change.
>
> Signed-off-by: Eric DeVolder <[email protected]>
> Acked-by: Baoquan He <[email protected]>
> ---
> kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
> kernel/kexec_file.c | 99 -------------------------------------------
> 2 files changed, 100 insertions(+), 99 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index a0eb4d5cf557..46c160d14045 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -10,6 +10,7 @@
> #include <linux/utsname.h>
> #include <linux/vmalloc.h>
> #include <linux/sizes.h>
> +#include <linux/kexec.h>
>
> #include <asm/page.h>
> #include <asm/sections.h>
> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
> }
> early_param("crashkernel", parse_crashkernel_dummy);
>
> +int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
> + void **addr, unsigned long *sz)
> +{
> + Elf64_Ehdr *ehdr;
> + Elf64_Phdr *phdr;
> + unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
> + unsigned char *buf;
> + unsigned int cpu, i;
> + unsigned long long notes_addr;
> + unsigned long mstart, mend;
> +
> + /* extra phdr for vmcoreinfo ELF note */
> + nr_phdr = nr_cpus + 1;
> + nr_phdr += mem->nr_ranges;
> +
> + /*
> + * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
> + * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
> + * I think this is required by tools like gdb. So same physical
> + * memory will be mapped in two ELF headers. One will contain kernel
> + * text virtual addresses and other will have __va(physical) addresses.
> + */
> +
> + nr_phdr++;
> + elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
> + elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
Seems like above function is out of CONFIG_KEXEC_FILE but some of the
structure/attributes like crash_mem and ELF_CORE_HEADER_ALIGN are
still defined under CONFIG_KEXEC_FILE (look for include/linux/kexec.h).
This leads to kernel build issue when CONFIG_KEXEC_FILE is disabled.
Thanks,
Sourabh Jain
On 1/8/23 23:05, Sourabh Jain wrote:
>
> On 09/12/22 21:06, Eric DeVolder wrote:
>> At the outcome of this patch set, the crash_prepare_elf64_headers()
>> is utilized on both the kexec_file_load() and kexec_load() paths. As
>> such, need to move this function out of kexec_file.c and into a
>> common location crash_core.c.
>>
>> No functionality change.
>>
>> Signed-off-by: Eric DeVolder <[email protected]>
>> Acked-by: Baoquan He <[email protected]>
>> ---
>> kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
>> kernel/kexec_file.c | 99 -------------------------------------------
>> 2 files changed, 100 insertions(+), 99 deletions(-)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index a0eb4d5cf557..46c160d14045 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -10,6 +10,7 @@
>> #include <linux/utsname.h>
>> #include <linux/vmalloc.h>
>> #include <linux/sizes.h>
>> +#include <linux/kexec.h>
>> #include <asm/page.h>
>> #include <asm/sections.h>
>> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
>> }
>> early_param("crashkernel", parse_crashkernel_dummy);
>> +int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>> + void **addr, unsigned long *sz)
>> +{
>> + Elf64_Ehdr *ehdr;
>> + Elf64_Phdr *phdr;
>> + unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>> + unsigned char *buf;
>> + unsigned int cpu, i;
>> + unsigned long long notes_addr;
>> + unsigned long mstart, mend;
>> +
>> + /* extra phdr for vmcoreinfo ELF note */
>> + nr_phdr = nr_cpus + 1;
>> + nr_phdr += mem->nr_ranges;
>> +
>> + /*
>> + * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
>> + * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
>> + * I think this is required by tools like gdb. So same physical
>> + * memory will be mapped in two ELF headers. One will contain kernel
>> + * text virtual addresses and other will have __va(physical) addresses.
>> + */
>> +
>> + nr_phdr++;
>> + elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>> + elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
> Seems like above function is out of CONFIG_KEXEC_FILE but some of the
> structure/attributes like crash_mem and ELF_CORE_HEADER_ALIGN are
> still defined under CONFIG_KEXEC_FILE (look for include/linux/kexec.h).
>
> This leads to kernel build issue when CONFIG_KEXEC_FILE is disabled.
>
> Thanks,
> Sourabh Jain
Thanks Sourabh, I'll look into this further.
eric
On 1/8/23 23:05, Sourabh Jain wrote:
>
> On 09/12/22 21:06, Eric DeVolder wrote:
>> At the outcome of this patch set, the crash_prepare_elf64_headers()
>> is utilized on both the kexec_file_load() and kexec_load() paths. As
>> such, need to move this function out of kexec_file.c and into a
>> common location crash_core.c.
>>
>> No functionality change.
>>
>> Signed-off-by: Eric DeVolder <[email protected]>
>> Acked-by: Baoquan He <[email protected]>
>> ---
>> kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
>> kernel/kexec_file.c | 99 -------------------------------------------
>> 2 files changed, 100 insertions(+), 99 deletions(-)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index a0eb4d5cf557..46c160d14045 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -10,6 +10,7 @@
>> #include <linux/utsname.h>
>> #include <linux/vmalloc.h>
>> #include <linux/sizes.h>
>> +#include <linux/kexec.h>
>> #include <asm/page.h>
>> #include <asm/sections.h>
>> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
>> }
>> early_param("crashkernel", parse_crashkernel_dummy);
>> +int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>> + void **addr, unsigned long *sz)
>> +{
>> + Elf64_Ehdr *ehdr;
>> + Elf64_Phdr *phdr;
>> + unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>> + unsigned char *buf;
>> + unsigned int cpu, i;
>> + unsigned long long notes_addr;
>> + unsigned long mstart, mend;
>> +
>> + /* extra phdr for vmcoreinfo ELF note */
>> + nr_phdr = nr_cpus + 1;
>> + nr_phdr += mem->nr_ranges;
>> +
>> + /*
>> + * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
>> + * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
>> + * I think this is required by tools like gdb. So same physical
>> + * memory will be mapped in two ELF headers. One will contain kernel
>> + * text virtual addresses and other will have __va(physical) addresses.
>> + */
>> +
>> + nr_phdr++;
>> + elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>> + elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
> Seems like above function is out of CONFIG_KEXEC_FILE but some of the
> structure/attributes like crash_mem and ELF_CORE_HEADER_ALIGN are
> still defined under CONFIG_KEXEC_FILE (look for include/linux/kexec.h).
>
> This leads to kernel build issue when CONFIG_KEXEC_FILE is disabled.
>
> Thanks,
> Sourabh Jain
After looking into this for a bit, to allow hotplug without kexec_file would require quite a bit of
code movement. Why? Because hotplug is basically built on top of (part of) the infrastructure that
was needed for kexec_file.
I'd be inclined to suggest that KEXEC_FILE be a required dependency for CRASH_HOTPLUG, ie:
config CRASH_HOTPLUG
bool "Update the crash elfcorehdr on system configuration changes"
default n
- depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG)
+ depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU || MEMORY_HOTPLUG)
If that isn't feasible, then it would appear quite a bit of surgery is needed to properly separate
out the items hotplug needs from kexec_file.
Thoughts?
eric
Hi Eric,
On Thu, 12 Jan 2023 11:23:18 -0600
Eric DeVolder <[email protected]> wrote:
>[...]
> After looking into this for a bit, to allow hotplug without kexec_file would require quite a bit of
> code movement. Why? Because hotplug is basically built on top of (part of) the infrastructure that
> was needed for kexec_file.
>
> I'd be inclined to suggest that KEXEC_FILE be a required dependency for CRASH_HOTPLUG, ie:
>
> config CRASH_HOTPLUG
> bool "Update the crash elfcorehdr on system configuration changes"
> default n
> - depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG)
> + depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU || MEMORY_HOTPLUG)
>
>
> If that isn't feasible, then it would appear quite a bit of surgery is needed to properly separate
> out the items hotplug needs from kexec_file.
>
> Thoughts?
I would have thought that CPU hotplug can be handled in the kernel only
if the crash image was loaded by the kernel with kexec_file_load(2).
When the image is loaded with kexec_load(2), then all data structures
are prepared by the user-space utility kexec(8), and the kernel
generally has no idea how to handle them.
In short, I believe that by definition there must be this dependency of
CRASH_HOTPLUG on KEXEC_FILE.
Petr T
On 1/12/23 11:23, Eric DeVolder wrote:
>
>
> On 1/8/23 23:05, Sourabh Jain wrote:
>>
>> On 09/12/22 21:06, Eric DeVolder wrote:
>>> At the outcome of this patch set, the crash_prepare_elf64_headers()
>>> is utilized on both the kexec_file_load() and kexec_load() paths. As
>>> such, need to move this function out of kexec_file.c and into a
>>> common location crash_core.c.
>>>
>>> No functionality change.
>>>
>>> Signed-off-by: Eric DeVolder <[email protected]>
>>> Acked-by: Baoquan He <[email protected]>
>>> ---
>>> kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
>>> kernel/kexec_file.c | 99 -------------------------------------------
>>> 2 files changed, 100 insertions(+), 99 deletions(-)
>>>
>>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>>> index a0eb4d5cf557..46c160d14045 100644
>>> --- a/kernel/crash_core.c
>>> +++ b/kernel/crash_core.c
>>> @@ -10,6 +10,7 @@
>>> #include <linux/utsname.h>
>>> #include <linux/vmalloc.h>
>>> #include <linux/sizes.h>
>>> +#include <linux/kexec.h>
>>> #include <asm/page.h>
>>> #include <asm/sections.h>
>>> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
>>> }
>>> early_param("crashkernel", parse_crashkernel_dummy);
>>> +int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>>> + void **addr, unsigned long *sz)
>>> +{
>>> + Elf64_Ehdr *ehdr;
>>> + Elf64_Phdr *phdr;
>>> + unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>>> + unsigned char *buf;
>>> + unsigned int cpu, i;
>>> + unsigned long long notes_addr;
>>> + unsigned long mstart, mend;
>>> +
>>> + /* extra phdr for vmcoreinfo ELF note */
>>> + nr_phdr = nr_cpus + 1;
>>> + nr_phdr += mem->nr_ranges;
>>> +
>>> + /*
>>> + * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
>>> + * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
>>> + * I think this is required by tools like gdb. So same physical
>>> + * memory will be mapped in two ELF headers. One will contain kernel
>>> + * text virtual addresses and other will have __va(physical) addresses.
>>> + */
>>> +
>>> + nr_phdr++;
>>> + elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>>> + elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
>> Seems like above function is out of CONFIG_KEXEC_FILE but some of the
>> structure/attributes like crash_mem and ELF_CORE_HEADER_ALIGN are
>> still defined under CONFIG_KEXEC_FILE (look for include/linux/kexec.h).
>>
>> This leads to kernel build issue when CONFIG_KEXEC_FILE is disabled.
>>
>> Thanks,
>> Sourabh Jain
>
> After looking into this for a bit, to allow hotplug without kexec_file would require quite a bit of
> code movement. Why? Because hotplug is basically built on top of (part of) the infrastructure that
> was needed for kexec_file.
>
> I'd be inclined to suggest that KEXEC_FILE be a required dependency for CRASH_HOTPLUG, ie:
>
> config CRASH_HOTPLUG
> bool "Update the crash elfcorehdr on system configuration changes"
> default n
> - depends on CRASH_DUMP && (HOTPLUG_CPU || MEMORY_HOTPLUG)
> + depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU || MEMORY_HOTPLUG)
>
>
> If that isn't feasible, then it would appear quite a bit of surgery is needed to properly separate
> out the items hotplug needs from kexec_file.
>
> Thoughts?
> eric
I completed the changes necessary to move code around to allow this to work outside of KEXEC_FILE;
in the end it wasn't too bad. I'll include these changes in the next version.
eric
On 12/01/23 22:53, Eric DeVolder wrote:
>
>
> On 1/8/23 23:05, Sourabh Jain wrote:
>>
>> On 09/12/22 21:06, Eric DeVolder wrote:
>>> At the outcome of this patch set, the crash_prepare_elf64_headers()
>>> is utilized on both the kexec_file_load() and kexec_load() paths. As
>>> such, need to move this function out of kexec_file.c and into a
>>> common location crash_core.c.
>>>
>>> No functionality change.
>>>
>>> Signed-off-by: Eric DeVolder <[email protected]>
>>> Acked-by: Baoquan He <[email protected]>
>>> ---
>>> kernel/crash_core.c | 100
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>> kernel/kexec_file.c | 99 -------------------------------------------
>>> 2 files changed, 100 insertions(+), 99 deletions(-)
>>>
>>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>>> index a0eb4d5cf557..46c160d14045 100644
>>> --- a/kernel/crash_core.c
>>> +++ b/kernel/crash_core.c
>>> @@ -10,6 +10,7 @@
>>> #include <linux/utsname.h>
>>> #include <linux/vmalloc.h>
>>> #include <linux/sizes.h>
>>> +#include <linux/kexec.h>
>>> #include <asm/page.h>
>>> #include <asm/sections.h>
>>> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char
>>> *arg)
>>> }
>>> early_param("crashkernel", parse_crashkernel_dummy);
>>> +int crash_prepare_elf64_headers(struct crash_mem *mem, int
>>> need_kernel_map,
>>> + void **addr, unsigned long *sz)
>>> +{
>>> + Elf64_Ehdr *ehdr;
>>> + Elf64_Phdr *phdr;
>>> + unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>>> + unsigned char *buf;
>>> + unsigned int cpu, i;
>>> + unsigned long long notes_addr;
>>> + unsigned long mstart, mend;
>>> +
>>> + /* extra phdr for vmcoreinfo ELF note */
>>> + nr_phdr = nr_cpus + 1;
>>> + nr_phdr += mem->nr_ranges;
>>> +
>>> + /*
>>> + * kexec-tools creates an extra PT_LOAD phdr for kernel text
>>> mapping
>>> + * area (for example, ffffffff80000000 - ffffffffa0000000 on
>>> x86_64).
>>> + * I think this is required by tools like gdb. So same physical
>>> + * memory will be mapped in two ELF headers. One will contain
>>> kernel
>>> + * text virtual addresses and other will have __va(physical)
>>> addresses.
>>> + */
>>> +
>>> + nr_phdr++;
>>> + elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>>> + elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
>> Seems like above function is out of CONFIG_KEXEC_FILE but some of the
>> structure/attributes like crash_mem and ELF_CORE_HEADER_ALIGN are
>> still defined under CONFIG_KEXEC_FILE (look for include/linux/kexec.h).
>>
>> This leads to kernel build issue when CONFIG_KEXEC_FILE is disabled.
>>
>> Thanks,
>> Sourabh Jain
>
> After looking into this for a bit, to allow hotplug without kexec_file
> would require quite a bit of code movement. Why? Because hotplug is
> basically built on top of (part of) the infrastructure that was needed
> for kexec_file.
>
> I'd be inclined to suggest that KEXEC_FILE be a required dependency
> for CRASH_HOTPLUG, ie:
Since kexec_load is deprecated I don't see any harm in doing that.
- Sourabh Jain