[PATCH v14 3/6] crash: add a new kexec flag for FDT update

[Sourabh Jain]

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 at linux.ibm.com>
>> Cc: Akhil Raj <lf32.dev at gmail.com>
>> Cc: Andrew Morton <akpm at linux-foundation.org>
>> Cc: Aneesh Kumar K.V <aneesh.kumar at kernel.org>
>> Cc: Baoquan He <bhe at redhat.com>
>> Cc: Borislav Petkov (AMD) <bp at alien8.de>
>> Cc: Boris Ostrovsky <boris.ostrovsky at oracle.com>
>> Cc: Christophe Leroy <christophe.leroy at csgroup.eu>
>> Cc: Dave Hansen <dave.hansen at linux.intel.com>
>> Cc: Dave Young <dyoung at redhat.com>
>> Cc: David Hildenbrand <david at redhat.com>
>> Cc: Eric DeVolder <eric.devolder at oracle.com>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: Hari Bathini <hbathini at linux.ibm.com>
>> Cc: Laurent Dufour <laurent.dufour at fr.ibm.com>
>> Cc: Mahesh Salgaonkar <mahesh at linux.ibm.com>
>> Cc: Michael Ellerman <mpe at ellerman.id.au>
>> Cc: Mimi Zohar <zohar at linux.ibm.com>
>> Cc: Naveen N Rao <naveen at kernel.org>
>> Cc: Oscar Salvador <osalvador at suse.de>
>> Cc: Thomas Gleixner <tglx at linutronix.de>
>> Cc: Valentin Schneider <vschneid at redhat.com>
>> Cc: Vivek Goyal <vgoyal at redhat.com>
>> Cc: kexec at lists.infradead.org
>> Cc: x86 at 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
>>