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

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

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
>>