[PATCH v1] kexec-tools/s390: Improve initrd memory reservation

[PATCH v1] kexec-tools/s390: Improve initrd memory reservation

[Joseph Cathcart]

Improve memory reservation for initrd. Remove unnecessary initrd
range from memory_ranges[]. Initrd currently falls between
"System RAM" ranges, and is therefore already reserved.

Refactor so that arch_reuse_initrd() is responsible for populating
retained_initrd_base and retained_initrd_size, rather than
get_memory_ranges_s390().

Add die() case for when user uses --reuseinitrd while booting a
crash kernel.

Signed-off-by: Joseph Cathcart <cathcartj08@gmail.com>
---
 kexec/arch/s390/kexec-image.c |  8 ++++++--
 kexec/arch/s390/kexec-s390.c  | 19 ++++++++++---------
 kexec/arch/s390/kexec-s390.h  |  2 +-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index 1a59a32..a9e5442 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -79,7 +79,8 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 	}
 
 	if (reuse_initrd)
-		die("--reuseinitrd not supported with --kexec-file-syscall. Please use --kexec-syscall\n");
+		die("--reuseinitrd not supported with --kexec-file-syscall. "
+			"Please use --kexec-syscall\n");
 
 	if (ramdisk) {
 		info->initrd_fd = open(ramdisk, O_RDONLY);
@@ -143,6 +144,9 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	if (ramdisk && reuse_initrd)
 		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
 
+	if (reuse_initrd && (info->kexec_flags & KEXEC_ON_CRASH))
+		die("Can't specify --reuseinitrd while booting a crash kernel\n");
+
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (parse_iomem_single("Crash kernel\n", &crash_base,
 				       &crash_end))
@@ -173,7 +177,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 				  ramdisk_origin, ramdisk_len);
 	} else if (reuse_initrd) {
 		ramdisk_origin = retained_initrd_base;
-		ramdisk_len = retained_initrd_size;
+		ramdisk_len = (off_t)retained_initrd_size;
 	}
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (load_crashdump_segments(info, crash_base, crash_end))
diff --git a/kexec/arch/s390/kexec-s390.c b/kexec/arch/s390/kexec-s390.c
index 4e77fc4..fd0d87e 100644
--- a/kexec/arch/s390/kexec-s390.c
+++ b/kexec/arch/s390/kexec-s390.c
@@ -26,10 +26,17 @@
 
 static struct memory_range memory_range[MAX_MEMORY_RANGES];
 unsigned long long retained_initrd_base, retained_initrd_size;
-unsigned int reuse_initrd = 0;
+unsigned char reuse_initrd;
 
 void arch_reuse_initrd(void)
 {
+	uint64_t start, end;
+
+	if (parse_iomem_single("initrd\n", &start, &end))
+		die("Couldn't find initrd entry in /proc/iomem\n");
+
+	retained_initrd_base = (unsigned long long)start;
+	retained_initrd_size = (unsigned long long)end - retained_initrd_base + 1;
 	reuse_initrd = 1;
 }
 
@@ -158,7 +165,6 @@ int get_memory_ranges_s390(struct memory_range memory_range[], int *ranges,
 {
 	char crash_kernel[] = "Crash kernel\n";
 	char sys_ram[] = "System RAM\n";
-	char kernel_initrd[] = "initrd\n";
 	const char *iomem = proc_iomem();
 	FILE *fp;
 	char line[80];
@@ -182,16 +188,11 @@ int get_memory_ranges_s390(struct memory_range memory_range[], int *ranges,
 		sscanf(line,"%llx-%llx : %n", &start, &end, &cons);
 		str = line+cons;
 		if ((memcmp(str, sys_ram, strlen(sys_ram)) == 0) ||
-			((memcmp(str, kernel_initrd, strlen(kernel_initrd)) == 0) && reuse_initrd) ||
-			((memcmp(str, crash_kernel, strlen(crash_kernel)) == 0) && with_crashk)) {
+		    ((memcmp(str, crash_kernel, strlen(crash_kernel)) == 0) &&
+		     with_crashk)) {
 			memory_range[current_range].start = start;
 			memory_range[current_range].end = end;
 			memory_range[current_range].type = RANGE_RAM;
-			if (memcmp(str, kernel_initrd, strlen(kernel_initrd)) == 0) {
-				retained_initrd_base = start;
-				retained_initrd_size = end - start + 1;
-			}
-
 			current_range++;
 		}
 		else {
diff --git a/kexec/arch/s390/kexec-s390.h b/kexec/arch/s390/kexec-s390.h
index a990739..7e85976 100644
--- a/kexec/arch/s390/kexec-s390.h
+++ b/kexec/arch/s390/kexec-s390.h
@@ -35,6 +35,6 @@ extern int get_memory_ranges_s390(struct memory_range range[], int *ranges,
 				  int with_crashk);
 extern int command_line_add(struct kexec_info *info, const char *str);
 extern unsigned long long retained_initrd_base, retained_initrd_size;
-extern unsigned int reuse_initrd;
+extern unsigned char reuse_initrd;
 
 #endif /* KEXEC_S390_H */
-- 
2.55.0

base commit: 43c5cba85ad23cc69c423527d9587c59b89fa437

Re: [PATCH v1] kexec-tools/s390: Improve initrd memory reservation

[Ilya Leoshkevich]

On 8/28/26 17:22, Joseph Cathcart wrote:
> Improve memory reservation for initrd. Remove unnecessary initrd
> range from memory_ranges[]. Initrd currently falls between
> "System RAM" ranges, and is therefore already reserved.


What if this changes in the future? Isn't the current approach better then?


> Refactor so that arch_reuse_initrd() is responsible for populating
> retained_initrd_base and retained_initrd_size, rather than
> get_memory_ranges_s390().
>
> Add die() case for when user uses --reuseinitrd while booting a
> crash kernel.
>
> Signed-off-by: Joseph Cathcart <cathcartj08@gmail.com>
> ---
>   kexec/arch/s390/kexec-image.c |  8 ++++++--
>   kexec/arch/s390/kexec-s390.c  | 19 ++++++++++---------
>   kexec/arch/s390/kexec-s390.h  |  2 +-
>   3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
> index 1a59a32..a9e5442 100644
> --- a/kexec/arch/s390/kexec-image.c
> +++ b/kexec/arch/s390/kexec-image.c
> @@ -79,7 +79,8 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
>   	}
>   
>   	if (reuse_initrd)
> -		die("--reuseinitrd not supported with --kexec-file-syscall. Please use --kexec-syscall\n");
> +		die("--reuseinitrd not supported with --kexec-file-syscall. "
> +			"Please use --kexec-syscall\n");


Not sure what the kexec-tools rules there are (I see some exceptions in e.g.
fs2dt.c), but in the kernel string literals must not be split for ease of
grepping.


>   
>   	if (ramdisk) {
>   		info->initrd_fd = open(ramdisk, O_RDONLY);
> @@ -143,6 +144,9 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>   	if (ramdisk && reuse_initrd)
>   		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
>   
> +	if (reuse_initrd && (info->kexec_flags & KEXEC_ON_CRASH))
> +		die("Can't specify --reuseinitrd while booting a crash kernel\n");
> +

I agree that trying to reuse initrd for a crash kernel makes little sense on
popular distros, but in theory one can create a universal initrd. So is 
there a
technical reason to prohibit this?

>   	if (info->kexec_flags & KEXEC_ON_CRASH) {
>   		if (parse_iomem_single("Crash kernel\n", &crash_base,
>   				       &crash_end))
> @@ -173,7 +177,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>   				  ramdisk_origin, ramdisk_len);
>   	} else if (reuse_initrd) {
>   		ramdisk_origin = retained_initrd_base;
> -		ramdisk_len = retained_initrd_size;
> +		ramdisk_len = (off_t)retained_initrd_size;

I guess this is to placate -Wsign-conversion?
It would be good to have this mentioned in the commit message.

>   	}
>   	if (info->kexec_flags & KEXEC_ON_CRASH) {
>   		if (load_crashdump_segments(info, crash_base, crash_end))
> diff --git a/kexec/arch/s390/kexec-s390.c b/kexec/arch/s390/kexec-s390.c
> index 4e77fc4..fd0d87e 100644
> --- a/kexec/arch/s390/kexec-s390.c
> +++ b/kexec/arch/s390/kexec-s390.c
> @@ -26,10 +26,17 @@
>   
>   static struct memory_range memory_range[MAX_MEMORY_RANGES];
>   unsigned long long retained_initrd_base, retained_initrd_size;
> -unsigned int reuse_initrd = 0;
> +unsigned char reuse_initrd;
>   
>   void arch_reuse_initrd(void)
>   {
> +	uint64_t start, end;
> +
> +	if (parse_iomem_single("initrd\n", &start, &end))
> +		die("Couldn't find initrd entry in /proc/iomem\n");
> +
> +	retained_initrd_base = (unsigned long long)start;
> +	retained_initrd_size = (unsigned long long)end - retained_initrd_base + 1;
>   	reuse_initrd = 1;
>   }


What is the benefit of moving this here? I see that there is no 
consensus among
the other architectures.

[...]