Hello Michael,
On 22/11/23 10:47, Michael Ellerman wrote:
Aneesh Kumar K V <aneesh.ku...@linux.ibm.com> writes:
...
I am not sure whether we need to add all the complexity to enable supporting
different fadump kernel
version. Is that even a possible use case with fadump? Can't we always assume
that with fadump the
crash kernel and fadump kernel will be same version?
How sure are we of that?
Don't we go through grub when we boot into the 2nd kernel. And so
couldn't it choose to boot a different kernel, for whatever reason.
I don't think we need to support different pt_reg / cpumask sizes, but
requiring the exact same kernel version is too strict I think.
Agree.
But maybe I'm wrong. Would be good to hear what distro folks think.
How about checking fadump crash info header compatibility in the
following way?
static bool is_fadump_header_compatible(struct fadump_crash_info_header
*fdh)
{
if (fdh->magic_number == FADUMP_CRASH_INFO_MAGIC_OLD) {
pr_err("Old magic number, can't process the dump.");
return false;
}
if (fdh->magic_number != FADUMP_CRASH_INFO_MAGIC) {
pr_err("Fadump header is corrupted.");
return false;
}
/*
* If the kernel version of the first/crashed kernel and the
second/fadump
* kernel is not same, then only collect the dump if the size of all
* non-primitive type members of the fadump header is the same
across kernels.
*/
if (strcmp(fdh->kernel_version, init_uts_ns.name.release)) {
if (fdh->pt_regs_sz != sizeof(struct pt_regs) ||
fdh->cpu_mask_sz != sizeof(struct cpumask)) {
pr_err("Fadump header size mismatch.\n")
return false;
} else
pr_warn("Kernel version mismatch; dump data is unreliable.\n");
}
return true;
}
And the new fadump crash info header will be: As suggested by Hari.
/* fadump crash info structure */
struct fadump_crash_info_header {
u64 magic_number;
+ u32 version;
u32 crashing_cpu;
u64 elfcorehdr_addr;
+ u64 elfcorehdr_size;
+ u64 vmcoreinfo_raddr;
+ u64 vmcoreinfo_size;
+ u8 kernel_version[__NEW_UTS_LEN + 1];
+ u32 pt_regs_sz;
struct pt_regs regs;
+ u32 cpu_mask_sz;
struct cpumask cpu_mask;
};
Thanks,
Sourabh Jain