| Message ID | 20231211083056.340404-4-sourabhjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/crash: Kernel handling of CPU and memory hotplug | expand |
On 12/11/23 at 02:00pm, Sourabh Jain wrote: > The commit a72bbec70da2 ("crash: hotplug support for kexec_load()") > introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses > this flag to indicate kernel that it is safe to modify the elfcorehdr > of kdump image loaded using kexec_load system call. > > Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump > component named FDT (Flatten Device Tree). Architectures like PowerPC > need to update FDT kdump image component on CPU hotplug events. Kexec > tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT > segment is not part of SHA calculation hence it is safe to update it. > > With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags, > crash hotplug support can be added to PowerPC for the kexec_load syscall > while maintaining the backward compatibility with older kexec tools that > do not have these newly introduced flags. > > 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: Eric DeVolder <eric.devolder@oracle.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 > --- > include/linux/kexec.h | 6 ++++-- > include/uapi/linux/kexec.h | 1 + > kernel/kexec.c | 2 ++ > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 0f6ea35879ee..bcedb7625b1f 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -319,6 +319,7 @@ struct kimage { > #ifdef CONFIG_CRASH_HOTPLUG > /* If set, allow changes to elfcorehdr of kexec_load'd image */ > unsigned int update_elfcorehdr:1; > + unsigned int update_fdt:1; Can we unify this to one flag, e.g hotplug_update? With this, on x86_64, we will skip the sha calculation for elfcorehdr. On ppc, we will skip the sha calculation for elfcorehdr and fdt. > #endif > > #ifdef ARCH_HAS_KIMAGE_ARCH > @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); > > /* List of defined/legal kexec flags */ > #ifndef CONFIG_KEXEC_JUMP > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) > #else > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ > + KEXEC_UPDATE_FDT) > #endif > > /* List of defined/legal kexec file flags */ > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > index 01766dd839b0..3d5b3d757bed 100644 > --- a/include/uapi/linux/kexec.h > +++ b/include/uapi/linux/kexec.h > @@ -13,6 +13,7 @@ > #define KEXEC_ON_CRASH 0x00000001 > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > +#define KEXEC_UPDATE_FDT 0x00000008 > #define KEXEC_ARCH_MASK 0xffff0000 > > /* > diff --git a/kernel/kexec.c b/kernel/kexec.c > index 8f35a5a42af8..97eb151cd931 100644 > --- a/kernel/kexec.c > +++ b/kernel/kexec.c > @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, > #ifdef CONFIG_CRASH_HOTPLUG > if (flags & KEXEC_UPDATE_ELFCOREHDR) > image->update_elfcorehdr = 1; > + if (flags & KEXEC_UPDATE_FDT) > + image->update_fdt = 1; > #endif > > ret = machine_kexec_prepare(image); > -- > 2.41.0 >
Hello Baoquan, On 15/12/23 07:58, Baoquan He wrote: > On 12/11/23 at 02:00pm, Sourabh Jain wrote: >> The commit a72bbec70da2 ("crash: hotplug support for kexec_load()") >> introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses >> this flag to indicate kernel that it is safe to modify the elfcorehdr >> of kdump image loaded using kexec_load system call. >> >> Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump >> component named FDT (Flatten Device Tree). Architectures like PowerPC >> need to update FDT kdump image component on CPU hotplug events. Kexec >> tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT >> segment is not part of SHA calculation hence it is safe to update it. >> >> With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags, >> crash hotplug support can be added to PowerPC for the kexec_load syscall >> while maintaining the backward compatibility with older kexec tools that >> do not have these newly introduced flags. >> >> 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: Eric DeVolder <eric.devolder@oracle.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 >> --- >> include/linux/kexec.h | 6 ++++-- >> include/uapi/linux/kexec.h | 1 + >> kernel/kexec.c | 2 ++ >> 3 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >> index 0f6ea35879ee..bcedb7625b1f 100644 >> --- a/include/linux/kexec.h >> +++ b/include/linux/kexec.h >> @@ -319,6 +319,7 @@ struct kimage { >> #ifdef CONFIG_CRASH_HOTPLUG >> /* If set, allow changes to elfcorehdr of kexec_load'd image */ >> unsigned int update_elfcorehdr:1; >> + unsigned int update_fdt:1; > Can we unify this to one flag, e.g hotplug_update? > > With this, on x86_64, we will skip the sha calculation for elfcorehdr. > On ppc, we will skip the sha calculation for elfcorehdr and fdt. Yeah, that's what I suggested to Eric. I can do that, but I see one problem with powerpc or other platforms that need to skip SHA for more kexec segments in addition to elfcorehdr. `update_elfcorehdr` is set when the kexec tool sends the `KEXEC_UPDATE_ELFCOREHDR` flag to the kernel for the `kexec_load` system call. Given that the kexec tool has already been updated to send the `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from SHA verification in generic code, now it would be tricky for architectures to determine whether kexec has skipped SHA verification for just elfcorehdr or all segments needed on the platform with the same flag. Code snippet from the kexec tool: main() { ... /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */ if (do_hotplug) { ... /* Indicate to the kernel it is ok to modify the elfcorehdr */ kexec_flags |= KEXEC_UPDATE_ELFCOREHDR; } ... } Any suggestion how to handle this with just one kexec flag? Thanks for the review. Thanks, Sourabh Jain > >> #endif >> >> #ifdef ARCH_HAS_KIMAGE_ARCH >> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); >> >> /* List of defined/legal kexec flags */ >> #ifndef CONFIG_KEXEC_JUMP >> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) >> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) >> #else >> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) >> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ >> + KEXEC_UPDATE_FDT) >> #endif >> >> /* List of defined/legal kexec file flags */ >> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h >> index 01766dd839b0..3d5b3d757bed 100644 >> --- a/include/uapi/linux/kexec.h >> +++ b/include/uapi/linux/kexec.h >> @@ -13,6 +13,7 @@ >> #define KEXEC_ON_CRASH 0x00000001 >> #define KEXEC_PRESERVE_CONTEXT 0x00000002 >> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 >> +#define KEXEC_UPDATE_FDT 0x00000008 >> #define KEXEC_ARCH_MASK 0xffff0000 >> >> /* >> diff --git a/kernel/kexec.c b/kernel/kexec.c >> index 8f35a5a42af8..97eb151cd931 100644 >> --- a/kernel/kexec.c >> +++ b/kernel/kexec.c >> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, >> #ifdef CONFIG_CRASH_HOTPLUG >> if (flags & KEXEC_UPDATE_ELFCOREHDR) >> image->update_elfcorehdr = 1; >> + if (flags & KEXEC_UPDATE_FDT) >> + image->update_fdt = 1; >> #endif >> >> ret = machine_kexec_prepare(image); >> -- >> 2.41.0 >>
On 12/15/23 at 12:17pm, Sourabh Jain wrote: ...... > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > > > index 0f6ea35879ee..bcedb7625b1f 100644 > > > --- a/include/linux/kexec.h > > > +++ b/include/linux/kexec.h > > > @@ -319,6 +319,7 @@ struct kimage { > > > #ifdef CONFIG_CRASH_HOTPLUG > > > /* If set, allow changes to elfcorehdr of kexec_load'd image */ > > > unsigned int update_elfcorehdr:1; > > > + unsigned int update_fdt:1; > > Can we unify this to one flag, e.g hotplug_update? > > > > With this, on x86_64, we will skip the sha calculation for elfcorehdr. > > On ppc, we will skip the sha calculation for elfcorehdr and fdt. > Yeah, that's what I suggested to Eric. I can do that, but I see one > problem with powerpc or other platforms that need to skip SHA > for more kexec segments in addition to elfcorehdr. > > `update_elfcorehdr` is set when the kexec tool sends the > `KEXEC_UPDATE_ELFCOREHDR` > flag to the kernel for the `kexec_load` system call. > > Given that the kexec tool has already been updated to send the > `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from > SHA verification in generic code, now it would be tricky for architectures > to > determine whether kexec has skipped SHA verification for just elfcorehdr > or all segments needed on the platform with the same flag. In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr segment. I am wondering how you skip the fdt segment when calculating and verifying sha, only saw the update_fdt mark. About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro, but still use the same value, could you think of what problem could be caused between kernel and kexec-tools utility, the old and new version compatibility? If it's about the new header files installed on older kernel, we can change it like below? Fortunately only one release, 6.6 passed. diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h index 3d5b3d757bed..df6a6505e267 100644 --- a/include/uapi/linux/kexec.h +++ b/include/uapi/linux/kexec.h @@ -13,7 +13,7 @@ #define KEXEC_ON_CRASH 0x00000001 #define KEXEC_PRESERVE_CONTEXT 0x00000002 -#define KEXEC_UPDATE_FDT 0x00000008 +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 #define KEXEC_ARCH_MASK 0xffff0000 /* With my understanding, the kexec flag should be indicating the action, the mem/cpu hotplug, but not relating to any detail. Imagine later another segment need be skipped on one ARCH again, then another flag need be added, this sounds not reasonable. > > Code snippet from the kexec tool: > > main() { > ... > /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */ > if (do_hotplug) { > ... > > /* Indicate to the kernel it is ok to modify the elfcorehdr */ > kexec_flags |= KEXEC_UPDATE_ELFCOREHDR; > } > ... > } > > Any suggestion how to handle this with just one kexec flag? > > Thanks for the review. > > Thanks, > Sourabh Jain > > > > > > #endif > > > #ifdef ARCH_HAS_KIMAGE_ARCH > > > @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); > > > /* List of defined/legal kexec flags */ > > > #ifndef CONFIG_KEXEC_JUMP > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) > > > #else > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ > > > + KEXEC_UPDATE_FDT) > > > #endif > > > /* List of defined/legal kexec file flags */ > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > > > index 01766dd839b0..3d5b3d757bed 100644 > > > --- a/include/uapi/linux/kexec.h > > > +++ b/include/uapi/linux/kexec.h > > > @@ -13,6 +13,7 @@ > > > #define KEXEC_ON_CRASH 0x00000001 > > > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > > > +#define KEXEC_UPDATE_FDT 0x00000008 > > > #define KEXEC_ARCH_MASK 0xffff0000 > > > /* > > > diff --git a/kernel/kexec.c b/kernel/kexec.c > > > index 8f35a5a42af8..97eb151cd931 100644 > > > --- a/kernel/kexec.c > > > +++ b/kernel/kexec.c > > > @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, > > > #ifdef CONFIG_CRASH_HOTPLUG > > > if (flags & KEXEC_UPDATE_ELFCOREHDR) > > > image->update_elfcorehdr = 1; > > > + if (flags & KEXEC_UPDATE_FDT) > > > + image->update_fdt = 1; > > > #endif > > > ret = machine_kexec_prepare(image); > > > -- > > > 2.41.0 > > > >
On 16/12/23 15:11, Baoquan He wrote: > On 12/15/23 at 12:17pm, Sourabh Jain wrote: > ...... >>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >>>> index 0f6ea35879ee..bcedb7625b1f 100644 >>>> --- a/include/linux/kexec.h >>>> +++ b/include/linux/kexec.h >>>> @@ -319,6 +319,7 @@ struct kimage { >>>> #ifdef CONFIG_CRASH_HOTPLUG >>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */ >>>> unsigned int update_elfcorehdr:1; >>>> + unsigned int update_fdt:1; >>> Can we unify this to one flag, e.g hotplug_update? >>> >>> With this, on x86_64, we will skip the sha calculation for elfcorehdr. >>> On ppc, we will skip the sha calculation for elfcorehdr and fdt. >> Yeah, that's what I suggested to Eric. I can do that, but I see one >> problem with powerpc or other platforms that need to skip SHA >> for more kexec segments in addition to elfcorehdr. >> >> `update_elfcorehdr` is set when the kexec tool sends the >> `KEXEC_UPDATE_ELFCOREHDR` >> flag to the kernel for the `kexec_load` system call. >> >> Given that the kexec tool has already been updated to send the >> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from >> SHA verification in generic code, now it would be tricky for architectures >> to >> determine whether kexec has skipped SHA verification for just elfcorehdr >> or all segments needed on the platform with the same flag. > In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr > segment. I am wondering how you skip the fdt segment when calculating > and verifying sha, only saw the update_fdt mark. In the kexec tool where we loop through all the kexec segments to calculate the SHA, there will be a arch call made to determine whether the segment needs to be excluded from SHA or not. Now in the arch function if decide a specific segment needs to excluded then corresponding flag is also set by arch function to communicate same with the kernel. > > About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro, > but still use the same value, could you think of what problem could be > caused between kernel and kexec-tools utility, the old and new version > compatibility? Just changing the macro name will NOT help because the current kexec tool enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit if the command argument --hotplug is passed to the kexec and the /sys/kernel/crash_elfcorehdr_size file exists in the system. Now, let's say an architecture enables this feature in the kernel with the assumption that the 0x00000004 kexec flag bit is passed from the kexec tool when all the required kexec segments are skipped from SHA calculation. In this case, the current kexec tool, which passes the 0x00000004 kexec flag bit only when the elfcorehdr is skipped, will cause issues for architectures. > > If it's about the new header files installed on older kernel, we can > change it like below? Fortunately only one release, 6.6 passed. > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > index 3d5b3d757bed..df6a6505e267 100644 > --- a/include/uapi/linux/kexec.h > +++ b/include/uapi/linux/kexec.h > @@ -13,7 +13,7 @@ > #define KEXEC_ON_CRASH 0x00000001 > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > -#define KEXEC_UPDATE_FDT 0x00000008 > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > #define KEXEC_ARCH_MASK 0xffff0000 > > /* > > With my understanding, the kexec flag should be indicating the action, > the mem/cpu hotplug, but not relating to any detail. Imagine later > another segment need be skipped on one ARCH again, then another flag > need be added, this sounds not reasonable. I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag should be sufficient to inform the kernel that the kexec tool has been updated to support CPU/Memory hotplug for the kexec_load system call. Unfortunately, we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE at the moment. What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag? I am aware that we are utilizing two kexec flag bits (0x00000004 and 0x00000008) for the same feature, but what other options do we have? Thanks, Sourabh >> Code snippet from the kexec tool: >> >> main() { >> ... >> /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */ >> if (do_hotplug) { >> ... >> >> /* Indicate to the kernel it is ok to modify the elfcorehdr */ >> kexec_flags |= KEXEC_UPDATE_ELFCOREHDR; >> } >> ... >> } >> >> Any suggestion how to handle this with just one kexec flag? >> >> Thanks for the review. >> >> Thanks, >> Sourabh Jain >> >>>> #endif >>>> #ifdef ARCH_HAS_KIMAGE_ARCH >>>> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); >>>> /* List of defined/legal kexec flags */ >>>> #ifndef CONFIG_KEXEC_JUMP >>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) >>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) >>>> #else >>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) >>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ >>>> + KEXEC_UPDATE_FDT) >>>> #endif >>>> /* List of defined/legal kexec file flags */ >>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h >>>> index 01766dd839b0..3d5b3d757bed 100644 >>>> --- a/include/uapi/linux/kexec.h >>>> +++ b/include/uapi/linux/kexec.h >>>> @@ -13,6 +13,7 @@ >>>> #define KEXEC_ON_CRASH 0x00000001 >>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002 >>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 >>>> +#define KEXEC_UPDATE_FDT 0x00000008 >>>> #define KEXEC_ARCH_MASK 0xffff0000 >>>> /* >>>> diff --git a/kernel/kexec.c b/kernel/kexec.c >>>> index 8f35a5a42af8..97eb151cd931 100644 >>>> --- a/kernel/kexec.c >>>> +++ b/kernel/kexec.c >>>> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, >>>> #ifdef CONFIG_CRASH_HOTPLUG >>>> if (flags & KEXEC_UPDATE_ELFCOREHDR) >>>> image->update_elfcorehdr = 1; >>>> + if (flags & KEXEC_UPDATE_FDT) >>>> + image->update_fdt = 1; >>>> #endif >>>> ret = machine_kexec_prepare(image); >>>> -- >>>> 2.41.0 >>>>
On 12/17/23 at 12:27am, Sourabh Jain wrote: > > > On 16/12/23 15:11, Baoquan He wrote: > > On 12/15/23 at 12:17pm, Sourabh Jain wrote: > > ...... > > > > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > > > > > index 0f6ea35879ee..bcedb7625b1f 100644 > > > > > --- a/include/linux/kexec.h > > > > > +++ b/include/linux/kexec.h > > > > > @@ -319,6 +319,7 @@ struct kimage { > > > > > #ifdef CONFIG_CRASH_HOTPLUG > > > > > /* If set, allow changes to elfcorehdr of kexec_load'd image */ > > > > > unsigned int update_elfcorehdr:1; > > > > > + unsigned int update_fdt:1; > > > > Can we unify this to one flag, e.g hotplug_update? > > > > > > > > With this, on x86_64, we will skip the sha calculation for elfcorehdr. > > > > On ppc, we will skip the sha calculation for elfcorehdr and fdt. > > > Yeah, that's what I suggested to Eric. I can do that, but I see one > > > problem with powerpc or other platforms that need to skip SHA > > > for more kexec segments in addition to elfcorehdr. > > > > > > `update_elfcorehdr` is set when the kexec tool sends the > > > `KEXEC_UPDATE_ELFCOREHDR` > > > flag to the kernel for the `kexec_load` system call. > > > > > > Given that the kexec tool has already been updated to send the > > > `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from > > > SHA verification in generic code, now it would be tricky for architectures > > > to > > > determine whether kexec has skipped SHA verification for just elfcorehdr > > > or all segments needed on the platform with the same flag. > > In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr > > segment. I am wondering how you skip the fdt segment when calculating > > and verifying sha, only saw the update_fdt mark. > > In the kexec tool where we loop through all the kexec segments to calculate > the SHA, there will be a arch call made to determine whether the segment > needs > to be excluded from SHA or not. OK, a arch call will be added to exclude segments in the ARCH. And the elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later may not want to exclude elfcorehdr. > > Now in the arch function if decide a specific segment needs to excluded then > corresponding flag is also set by arch function to communicate same with the > kernel. But I don't see how you exclude elfcorehdr and fdt in kernel for kexec_file codes. It's not happening in kexec-tools. > > > > > About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro, > > but still use the same value, could you think of what problem could be > > caused between kernel and kexec-tools utility, the old and new version > > compatibility? > > Just changing the macro name will NOT help because the current kexec tool > enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit > if > the command argument --hotplug is passed to the kexec > and > the /sys/kernel/crash_elfcorehdr_size file exists in the system. As we have discussed, excluding will be done in each ARCH's function when doing sha calculation in kexec-tools, isn't it? diff --git a/kexec/kexec.c b/kexec/kexec.c index b5393e3b20aa..0095aeec988a 100644 --- a/kexec/kexec.c +++ b/kexec/kexec.c @@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info) continue; } - /* Don't include elfcorehdr in the checksum, if hotplug + /* Don't include unwanted segments in the checksum, if hotplug * support enabled. - */ - if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) { + if (do_hotplug) + arch_exclude_segments(info, &i) continue; } > > Now, let's say an architecture enables this feature in the kernel with the > assumption > that the 0x00000004 kexec flag bit is passed from the kexec tool when all > the required > kexec segments are skipped from SHA calculation. In this case, the current > kexec tool, > which passes the 0x00000004 kexec flag bit only when the elfcorehdr is > skipped, will > cause issues for architectures. > > > > > If it's about the new header files installed on older kernel, we can > > change it like below? Fortunately only one release, 6.6 passed. > > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > > index 3d5b3d757bed..df6a6505e267 100644 > > --- a/include/uapi/linux/kexec.h > > +++ b/include/uapi/linux/kexec.h > > @@ -13,7 +13,7 @@ > > #define KEXEC_ON_CRASH 0x00000001 > > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > > -#define KEXEC_UPDATE_FDT 0x00000008 > > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > > #define KEXEC_ARCH_MASK 0xffff0000 > > /* > > > > With my understanding, the kexec flag should be indicating the action, > > the mem/cpu hotplug, but not relating to any detail. Imagine later > > another segment need be skipped on one ARCH again, then another flag > > need be added, this sounds not reasonable. > I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag > should be sufficient to inform the kernel that the kexec tool has been > updated > to support CPU/Memory hotplug for the kexec_load system call. Unfortunately, > we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE > at the moment. I am fine with 0x00000008 and a new flag, it has the same effect as #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 I am worried about the header file incompatiblity. > > What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag? > > I am aware that we are utilizing two kexec flag bits (0x00000004 and > 0x00000008) > for the same feature, but what other options do we have? > > Thanks, > Sourabh > > > Code snippet from the kexec tool: > > > > > > main() { > > > ... > > > /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */ > > > if (do_hotplug) { > > > ... > > > > > > /* Indicate to the kernel it is ok to modify the elfcorehdr */ > > > kexec_flags |= KEXEC_UPDATE_ELFCOREHDR; > > > } > > > ... > > > } > > > > > > Any suggestion how to handle this with just one kexec flag? > > > > > > Thanks for the review. > > > > > > Thanks, > > > Sourabh Jain > > > > > > > > #endif > > > > > #ifdef ARCH_HAS_KIMAGE_ARCH > > > > > @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); > > > > > /* List of defined/legal kexec flags */ > > > > > #ifndef CONFIG_KEXEC_JUMP > > > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) > > > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) > > > > > #else > > > > > -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) > > > > > +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ > > > > > + KEXEC_UPDATE_FDT) > > > > > #endif > > > > > /* List of defined/legal kexec file flags */ > > > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > > > > > index 01766dd839b0..3d5b3d757bed 100644 > > > > > --- a/include/uapi/linux/kexec.h > > > > > +++ b/include/uapi/linux/kexec.h > > > > > @@ -13,6 +13,7 @@ > > > > > #define KEXEC_ON_CRASH 0x00000001 > > > > > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > > > > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > > > > > +#define KEXEC_UPDATE_FDT 0x00000008 > > > > > #define KEXEC_ARCH_MASK 0xffff0000 > > > > > /* > > > > > diff --git a/kernel/kexec.c b/kernel/kexec.c > > > > > index 8f35a5a42af8..97eb151cd931 100644 > > > > > --- a/kernel/kexec.c > > > > > +++ b/kernel/kexec.c > > > > > @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, > > > > > #ifdef CONFIG_CRASH_HOTPLUG > > > > > if (flags & KEXEC_UPDATE_ELFCOREHDR) > > > > > image->update_elfcorehdr = 1; > > > > > + if (flags & KEXEC_UPDATE_FDT) > > > > > + image->update_fdt = 1; > > > > > #endif > > > > > ret = machine_kexec_prepare(image); > > > > > -- > > > > > 2.41.0 > > > > > >
On 17/12/23 06:29, Baoquan He wrote: > On 12/17/23 at 12:27am, Sourabh Jain wrote: >> >> On 16/12/23 15:11, Baoquan He wrote: >>> On 12/15/23 at 12:17pm, Sourabh Jain wrote: >>> ...... >>>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >>>>>> index 0f6ea35879ee..bcedb7625b1f 100644 >>>>>> --- a/include/linux/kexec.h >>>>>> +++ b/include/linux/kexec.h >>>>>> @@ -319,6 +319,7 @@ struct kimage { >>>>>> #ifdef CONFIG_CRASH_HOTPLUG >>>>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */ >>>>>> unsigned int update_elfcorehdr:1; >>>>>> + unsigned int update_fdt:1; >>>>> Can we unify this to one flag, e.g hotplug_update? >>>>> >>>>> With this, on x86_64, we will skip the sha calculation for elfcorehdr. >>>>> On ppc, we will skip the sha calculation for elfcorehdr and fdt. >>>> Yeah, that's what I suggested to Eric. I can do that, but I see one >>>> problem with powerpc or other platforms that need to skip SHA >>>> for more kexec segments in addition to elfcorehdr. >>>> >>>> `update_elfcorehdr` is set when the kexec tool sends the >>>> `KEXEC_UPDATE_ELFCOREHDR` >>>> flag to the kernel for the `kexec_load` system call. >>>> >>>> Given that the kexec tool has already been updated to send the >>>> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from >>>> SHA verification in generic code, now it would be tricky for architectures >>>> to >>>> determine whether kexec has skipped SHA verification for just elfcorehdr >>>> or all segments needed on the platform with the same flag. >>> In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr >>> segment. I am wondering how you skip the fdt segment when calculating >>> and verifying sha, only saw the update_fdt mark. >> In the kexec tool where we loop through all the kexec segments to calculate >> the SHA, there will be a arch call made to determine whether the segment >> needs >> to be excluded from SHA or not. > OK, a arch call will be added to exclude segments in the ARCH. And the > elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later > may not want to exclude elfcorehdr. Yes, Arch can choose which segment to exclude. > >> Now in the arch function if decide a specific segment needs to excluded then >> corresponding flag is also set by arch function to communicate same with the >> kernel. > But I don't see how you exclude elfcorehdr and fdt in kernel for > kexec_file codes. It's not happening in kexec-tools. On PowerPC, SHA verification is NOT performed for the kexec_file_load case; hence, you won't find any code changes in my patch series to exclude FDT in the kernel code. However, let's consider a scenario where it gets added in the future, or other architectures need to skip the kexec segment, in addition to elfcorehdr. In that case, we can use the same setup as you suggested below. For each kexec segment, there should be an architecture-specific function call to decide whether the segment needs to be excluded or not. > >>> About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro, >>> but still use the same value, could you think of what problem could be >>> caused between kernel and kexec-tools utility, the old and new version >>> compatibility? >> Just changing the macro name will NOT help because the current kexec tool >> enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit >> if >> the command argument --hotplug is passed to the kexec >> and >> the /sys/kernel/crash_elfcorehdr_size file exists in the system. > As we have discussed, excluding will be done in each ARCH's function > when doing sha calculation in kexec-tools, isn't it? > > diff --git a/kexec/kexec.c b/kexec/kexec.c > index b5393e3b20aa..0095aeec988a 100644 > --- a/kexec/kexec.c > +++ b/kexec/kexec.c > @@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info) > continue; > } > > - /* Don't include elfcorehdr in the checksum, if hotplug > + /* Don't include unwanted segments in the checksum, if hotplug > * support enabled. > - */ > - if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) { > + if (do_hotplug) > + arch_exclude_segments(info, &i) > continue; > } Yes, something like the above should work. > > >> Now, let's say an architecture enables this feature in the kernel with the >> assumption >> that the 0x00000004 kexec flag bit is passed from the kexec tool when all >> the required >> kexec segments are skipped from SHA calculation. In this case, the current >> kexec tool, >> which passes the 0x00000004 kexec flag bit only when the elfcorehdr is >> skipped, will >> cause issues for architectures. >> >>> If it's about the new header files installed on older kernel, we can >>> change it like below? Fortunately only one release, 6.6 passed. >>> >>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h >>> index 3d5b3d757bed..df6a6505e267 100644 >>> --- a/include/uapi/linux/kexec.h >>> +++ b/include/uapi/linux/kexec.h >>> @@ -13,7 +13,7 @@ >>> #define KEXEC_ON_CRASH 0x00000001 >>> #define KEXEC_PRESERVE_CONTEXT 0x00000002 >>> -#define KEXEC_UPDATE_FDT 0x00000008 >>> +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 >>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 >>> #define KEXEC_ARCH_MASK 0xffff0000 >>> /* >>> >>> With my understanding, the kexec flag should be indicating the action, >>> the mem/cpu hotplug, but not relating to any detail. Imagine later >>> another segment need be skipped on one ARCH again, then another flag >>> need be added, this sounds not reasonable. >> I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag >> should be sufficient to inform the kernel that the kexec tool has been >> updated >> to support CPU/Memory hotplug for the kexec_load system call. Unfortunately, >> we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE >> at the moment. > I am fine with 0x00000008 and a new flag, it has the same effect as > #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > > I am worried about the header file incompatiblity. If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit to introduce CPU/Memory hotplug feature for kexec_load syscall, we will not have compatibility issue. Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new flag bit and show how it will be handled. I will also share kexec code for clarity. Thanks, Sourabh > >> What about using 0x00000008 for the KEXEC_CRASH_HOTPLUG_UPDATE flag? >> >> I am aware that we are utilizing two kexec flag bits (0x00000004 and >> 0x00000008) >> for the same feature, but what other options do we have? >> >> Thanks, >> Sourabh >>>> Code snippet from the kexec tool: >>>> >>>> main() { >>>> ... >>>> /* NOTE: Xen KEXEC_LIVE_UPDATE and KEXEC_UPDATE_ELFCOREHDR collide */ >>>> if (do_hotplug) { >>>> ... >>>> >>>> /* Indicate to the kernel it is ok to modify the elfcorehdr */ >>>> kexec_flags |= KEXEC_UPDATE_ELFCOREHDR; >>>> } >>>> ... >>>> } >>>> >>>> Any suggestion how to handle this with just one kexec flag? >>>> >>>> Thanks for the review. >>>> >>>> Thanks, >>>> Sourabh Jain >>>> >>>>>> #endif >>>>>> #ifdef ARCH_HAS_KIMAGE_ARCH >>>>>> @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); >>>>>> /* List of defined/legal kexec flags */ >>>>>> #ifndef CONFIG_KEXEC_JUMP >>>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) >>>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) >>>>>> #else >>>>>> -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) >>>>>> +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ >>>>>> + KEXEC_UPDATE_FDT) >>>>>> #endif >>>>>> /* List of defined/legal kexec file flags */ >>>>>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h >>>>>> index 01766dd839b0..3d5b3d757bed 100644 >>>>>> --- a/include/uapi/linux/kexec.h >>>>>> +++ b/include/uapi/linux/kexec.h >>>>>> @@ -13,6 +13,7 @@ >>>>>> #define KEXEC_ON_CRASH 0x00000001 >>>>>> #define KEXEC_PRESERVE_CONTEXT 0x00000002 >>>>>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 >>>>>> +#define KEXEC_UPDATE_FDT 0x00000008 >>>>>> #define KEXEC_ARCH_MASK 0xffff0000 >>>>>> /* >>>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c >>>>>> index 8f35a5a42af8..97eb151cd931 100644 >>>>>> --- a/kernel/kexec.c >>>>>> +++ b/kernel/kexec.c >>>>>> @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, >>>>>> #ifdef CONFIG_CRASH_HOTPLUG >>>>>> if (flags & KEXEC_UPDATE_ELFCOREHDR) >>>>>> image->update_elfcorehdr = 1; >>>>>> + if (flags & KEXEC_UPDATE_FDT) >>>>>> + image->update_fdt = 1; >>>>>> #endif >>>>>> ret = machine_kexec_prepare(image); >>>>>> -- >>>>>> 2.41.0 >>>>>>
Hello Baoquan, While replying to this email earlier, I mistakenly pressed "Reply to List" instead of "Reply to All." Consequently, my response was sent only to powerpc mailing list. On 17/12/23 06:29, Baoquan He wrote: > On 12/17/23 at 12:27am, Sourabh Jain wrote: >> On 16/12/23 15:11, Baoquan He wrote: >>> On 12/15/23 at 12:17pm, Sourabh Jain wrote: >>> ...... >>>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >>>>>> index 0f6ea35879ee..bcedb7625b1f 100644 >>>>>> --- a/include/linux/kexec.h >>>>>> +++ b/include/linux/kexec.h >>>>>> @@ -319,6 +319,7 @@ struct kimage { >>>>>> #ifdef CONFIG_CRASH_HOTPLUG >>>>>> /* If set, allow changes to elfcorehdr of kexec_load'd image */ >>>>>> unsigned int update_elfcorehdr:1; >>>>>> + unsigned int update_fdt:1; >>>>> Can we unify this to one flag, e.g hotplug_update? >>>>> >>>>> With this, on x86_64, we will skip the sha calculation for elfcorehdr. >>>>> On ppc, we will skip the sha calculation for elfcorehdr and fdt. >>>> Yeah, that's what I suggested to Eric. I can do that, but I see one >>>> problem with powerpc or other platforms that need to skip SHA >>>> for more kexec segments in addition to elfcorehdr. >>>> >>>> `update_elfcorehdr` is set when the kexec tool sends the >>>> `KEXEC_UPDATE_ELFCOREHDR` >>>> flag to the kernel for the `kexec_load` system call. >>>> >>>> Given that the kexec tool has already been updated to send the >>>> `KEXEC_UPDATE_ELFCOREHDR` flag only when elfcorehdr is skipped from >>>> SHA verification in generic code, now it would be tricky for architectures >>>> to >>>> determine whether kexec has skipped SHA verification for just elfcorehdr >>>> or all segments needed on the platform with the same flag. >>> In kexec-tools, it's judged by do_hotplug to skip the elfcorehdr >>> segment. I am wondering how you skip the fdt segment when calculating >>> and verifying sha, only saw the update_fdt mark. >> In the kexec tool where we loop through all the kexec segments to calculate >> the SHA, there will be a arch call made to determine whether the segment >> needs >> to be excluded from SHA or not. > OK, a arch call will be added to exclude segments in the ARCH. And the > elfcorehdr segment need be excluded in x86 ARCH in case other ARCH later > may not want to exclude elfcorehdr. Yes, Arch can choose which segment to exclude. >> Now in the arch function if decide a specific segment needs to excluded then >> corresponding flag is also set by arch function to communicate same with the >> kernel. > But I don't see how you exclude elfcorehdr and fdt in kernel for > kexec_file codes. It's not happening in kexec-tools. On PowerPC, SHA verification is NOT performed for the kexec_file_load case; hence, you won't find any code changes in my patch series to exclude FDT in the kernel code. However, let's consider a scenario where it gets added in the future, or other architectures need to skip the kexec segment, in addition to elfcorehdr. In that case, we can use the same setup as you suggested below. For each kexec segment, there should be an architecture-specific function call to decide whether the segment needs to be excluded or not. >>> About the existing KEXEC_UPDATE_ELFCOREHDR, we only rename the macro, >>> but still use the same value, could you think of what problem could be >>> caused between kernel and kexec-tools utility, the old and new version >>> compatibility? >> Just changing the macro name will NOT help because the current kexec tool >> enables the KEXEC_UPDATE_ELFCOREHDR = 0x00000004 kexec flag bit >> if >> the command argument --hotplug is passed to the kexec >> and >> the /sys/kernel/crash_elfcorehdr_size file exists in the system. > As we have discussed, excluding will be done in each ARCH's function > when doing sha calculation in kexec-tools, isn't it? > > diff --git a/kexec/kexec.c b/kexec/kexec.c > index b5393e3b20aa..0095aeec988a 100644 > --- a/kexec/kexec.c > +++ b/kexec/kexec.c > @@ -701,10 +701,10 @@ static void update_purgatory(struct kexec_info *info) > continue; > } > > - /* Don't include elfcorehdr in the checksum, if hotplug > + /* Don't include unwanted segments in the checksum, if hotplug > * support enabled. > - */ > - if (do_hotplug && (info->segment[i].mem == (void *)info->elfcorehdr)) { > + if (do_hotplug) > + arch_exclude_segments(info, &i) > continue; > } Yes, something like the above should work. >> Now, let's say an architecture enables this feature in the kernel with the >> assumption >> that the 0x00000004 kexec flag bit is passed from the kexec tool when all >> the required >> kexec segments are skipped from SHA calculation. In this case, the current >> kexec tool, >> which passes the 0x00000004 kexec flag bit only when the elfcorehdr is >> skipped, will >> cause issues for architectures. >> >>> If it's about the new header files installed on older kernel, we can >>> change it like below? Fortunately only one release, 6.6 passed. >>> >>> diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h >>> index 3d5b3d757bed..df6a6505e267 100644 >>> --- a/include/uapi/linux/kexec.h >>> +++ b/include/uapi/linux/kexec.h >>> @@ -13,7 +13,7 @@ >>> #define KEXEC_ON_CRASH 0x00000001 >>> #define KEXEC_PRESERVE_CONTEXT 0x00000002 >>> -#define KEXEC_UPDATE_FDT 0x00000008 >>> +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 >>> #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 >>> #define KEXEC_ARCH_MASK 0xffff0000 >>> /* >>> >>> With my understanding, the kexec flag should be indicating the action, >>> the mem/cpu hotplug, but not relating to any detail. Imagine later >>> another segment need be skipped on one ARCH again, then another flag >>> need be added, this sounds not reasonable. >> I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag >> should be sufficient to inform the kernel that the kexec tool has been >> updated >> to support CPU/Memory hotplug for the kexec_load system call. Unfortunately, >> we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE >> at the moment. > I am fine with 0x00000008 and a new flag, it has the same effect as > #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > > I am worried about the header file incompatiblity. If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit to introduce CPU/Memory hotplug feature for kexec_load syscall, we will not have compatibility issue. Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new flag bit and show how it will be handled. I will also share kexec code for clarity. Thanks, Sourabh
On 12/21/23 at 11:36am, Sourabh Jain wrote: ...... > > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > > > > index 3d5b3d757bed..df6a6505e267 100644 > > > > --- a/include/uapi/linux/kexec.h > > > > +++ b/include/uapi/linux/kexec.h > > > > @@ -13,7 +13,7 @@ > > > > #define KEXEC_ON_CRASH 0x00000001 > > > > #define KEXEC_PRESERVE_CONTEXT 0x00000002 > > > > -#define KEXEC_UPDATE_FDT 0x00000008 > > > > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > > > > #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 > > > > #define KEXEC_ARCH_MASK 0xffff0000 > > > > /* > > > > > > > > With my understanding, the kexec flag should be indicating the action, > > > > the mem/cpu hotplug, but not relating to any detail. Imagine later > > > > another segment need be skipped on one ARCH again, then another flag > > > > need be added, this sounds not reasonable. > > > I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag > > > should be sufficient to inform the kernel that the kexec tool has been > > > updated > > > to support CPU/Memory hotplug for the kexec_load system call. Unfortunately, > > > we cannot use the 0x00000004 kexec flags bit for KEXEC_CRASH_HOTPLUG_UPDATE > > > at the moment. > > I am fine with 0x00000008 and a new flag, it has the same effect as > > #define KEXEC_CRASH_HOTPLUG_UPDATE 0x00000004 > > > > I am worried about the header file incompatiblity. > > If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x00000008 as new bit > to introduce CPU/Memory hotplug feature for kexec_load syscall, we will not > have > compatibility issue. > > Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE > 0x00000008 > as new flag bit and show how it will be handled. I will also share kexec > code for clarity. It's great we are in the same page about segments excluding done in arch function. While It's a little unclear to me why we can't reuse 0x00000004 flag value. Then KEXEC_UPDATE_ELFCOREHDR will only exist in v6.6 kernel, and that bit won't be used in v6.7 and future version. Except of the existence in kexec-tools utility for XEN, do you see other barrier? I would like to know so that one day I can explain KEXEC_UPDATE_ELFCOREHDR to someone else if asked.
diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 0f6ea35879ee..bcedb7625b1f 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -319,6 +319,7 @@ struct kimage { #ifdef CONFIG_CRASH_HOTPLUG /* If set, allow changes to elfcorehdr of kexec_load'd image */ unsigned int update_elfcorehdr:1; + unsigned int update_fdt:1; #endif #ifdef ARCH_HAS_KIMAGE_ARCH @@ -396,9 +397,10 @@ bool kexec_load_permitted(int kexec_image_type); /* List of defined/legal kexec flags */ #ifndef CONFIG_KEXEC_JUMP -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR) +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | KEXEC_UPDATE_FDT) #else -#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR) +#define KEXEC_FLAGS (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | KEXEC_UPDATE_ELFCOREHDR | \ + KEXEC_UPDATE_FDT) #endif /* List of defined/legal kexec file flags */ diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h index 01766dd839b0..3d5b3d757bed 100644 --- a/include/uapi/linux/kexec.h +++ b/include/uapi/linux/kexec.h @@ -13,6 +13,7 @@ #define KEXEC_ON_CRASH 0x00000001 #define KEXEC_PRESERVE_CONTEXT 0x00000002 #define KEXEC_UPDATE_ELFCOREHDR 0x00000004 +#define KEXEC_UPDATE_FDT 0x00000008 #define KEXEC_ARCH_MASK 0xffff0000 /* diff --git a/kernel/kexec.c b/kernel/kexec.c index 8f35a5a42af8..97eb151cd931 100644 --- a/kernel/kexec.c +++ b/kernel/kexec.c @@ -132,6 +132,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments, #ifdef CONFIG_CRASH_HOTPLUG if (flags & KEXEC_UPDATE_ELFCOREHDR) image->update_elfcorehdr = 1; + if (flags & KEXEC_UPDATE_FDT) + image->update_fdt = 1; #endif ret = machine_kexec_prepare(image);
The commit a72bbec70da2 ("crash: hotplug support for kexec_load()") introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses this flag to indicate kernel that it is safe to modify the elfcorehdr of kdump image loaded using kexec_load system call. Similarly, add a new kexec flag, `KEXEC_UPDATE_FDT`, for another kdump component named FDT (Flatten Device Tree). Architectures like PowerPC need to update FDT kdump image component on CPU hotplug events. Kexec tool passing `KEXEC_UPDATE_FDT` will be an indication to kernel that FDT segment is not part of SHA calculation hence it is safe to update it. With the `KEXEC_UPDATE_ELFCOREHDR` and `KEXEC_UPDATE_FDT` kexec flags, crash hotplug support can be added to PowerPC for the kexec_load syscall while maintaining the backward compatibility with older kexec tools that do not have these newly introduced flags. 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: Eric DeVolder <eric.devolder@oracle.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 --- include/linux/kexec.h | 6 ++++-- include/uapi/linux/kexec.h | 1 + kernel/kexec.c | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-)