Re: [PATCH v2 1/7] s390/kdump: implement is_kdump_kernel()

[Heiko Carstens]

On Mon, Oct 14, 2024 at 09:26:03PM +0200, David Hildenbrand wrote:
> On 14.10.24 20:20, Heiko Carstens wrote:
> > Looks like this could work. But the comment in smp.c above
> > dump_available() needs to be updated.
> 
> A right, I remember that there was some outdated documentation.
> 
> > 
> > Are you willing to do that, or should I provide an addon patch?
> > 
> 
> I can squash the following:
> 
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index 4df56fdb2488..a4f538876462 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -587,16 +587,16 @@ int smp_store_status(int cpu)
>   *    with sigp stop-and-store-status. The firmware or the boot-loader
>   *    stored the registers of the boot CPU in the absolute lowcore in the
>   *    memory of the old system.
> - * 3) kdump and the old kernel did not store the CPU state,
> - *    or stand-alone kdump for DASD
> - *    condition: OLDMEM_BASE != NULL && !is_kdump_kernel()
> + * 3) kdump or stand-alone kdump for DASD
> + *    condition: OLDMEM_BASE != NULL && !is_ipl_type_dump() == false
>   *    The state for all CPUs except the boot CPU needs to be collected
>   *    with sigp stop-and-store-status. The kexec code or the boot-loader
>   *    stored the registers of the boot CPU in the memory of the old system.
> - * 4) kdump and the old kernel stored the CPU state
> - *    condition: OLDMEM_BASE != NULL && is_kdump_kernel()
> - *    This case does not exist for s390 anymore, setup_arch explicitly
> - *    deactivates the elfcorehdr= kernel parameter
> + *
> + * Note that the old Kdump mode where the old kernel stored the CPU state

To be consistent with the rest of the comment, please write kdump in
all lower case characters, please.

> + * does no longer exist: setup_arch explicitly deactivates the elfcorehdr=
> + * kernel parameter. The is_kudmp_kernel() implementation on s390 is independent

Typo: kudmp.

> Does that sound reasonable? I'm not so sure about the "2) stand-alone kdump for
> SCSI/NVMe (zfcp/nvme dump with swapped memory)": is that really "kdump" ?

Yes, it is some sort of kdump, even though a bit odd. But the comment
as it is doesn't need to be changed. Only at the very top, please also
change: "There are four cases" into "There are three cases".

Then it all looks good. Thanks a lot!