On 15.10.24 10:41, David Hildenbrand wrote:
On 15.10.24 10:30, Heiko Carstens wrote:
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.
It obviously was too late in the evening for me :) Thanks!
+ * 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.
My concern is that we'll now have
bool is_kdump_kernel(void)
{
return oldmem_data.start && !is_ipl_type_dump();
}
Which matches 3), but if 2) is also called "kdump", then should it
actually be
bool is_kdump_kernel(void)
{
return oldmem_data.start;
}
?
When I wrote that code I was rather convinced that the variant in this
patch is the right thing to do.
I think we can do some follow up cleanups, assuming is_kdump_kernel() here is correct:
diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c
index cca1827d3d2e..fbc5de66d03b 100644
--- a/arch/s390/kernel/crash_dump.c
+++ b/arch/s390/kernel/crash_dump.c
@@ -609,7 +609,7 @@ int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size)
u64 hdr_off;
/* If we are not in kdump or zfcp/nvme dump mode return */
- if (!oldmem_data.start && !is_ipl_type_dump())
+ if (!dump_available())
return 0;
/* If we cannot get HSA size for zfcp/nvme dump return error */
if (is_ipl_type_dump() && !sclp.hsa_size)
diff --git a/arch/s390/kernel/os_info.c b/arch/s390/kernel/os_info.c
index b695f980bbde..09578f400ef7 100644
--- a/arch/s390/kernel/os_info.c
+++ b/arch/s390/kernel/os_info.c
@@ -148,7 +148,7 @@ static void os_info_old_init(void)
if (os_info_init)
return;
- if (!oldmem_data.start && !is_ipl_type_dump())
+ if (!dump_available())
goto fail;
if (copy_oldmem_kernel(&addr, __LC_OS_INFO, sizeof(addr)))
goto fail;
diff --git a/drivers/s390/char/zcore.c b/drivers/s390/char/zcore.c
index 33cebb91b933..6a194b4f6ba5 100644
--- a/drivers/s390/char/zcore.c
+++ b/drivers/s390/char/zcore.c
@@ -300,9 +300,7 @@ static int __init zcore_init(void)
unsigned char arch;
int rc;
- if (!is_ipl_type_dump())
- return -ENODATA;
- if (oldmem_data.start)
+ if (is_kdump_kernel())
return -ENODATA;
zcore_dbf = debug_register("zcore", 4, 1, 4 * sizeof(long));
--
Cheers,
David / dhildenb
