[PATCH 0/3] kho: fixes and cleanups

[PATCH 0/3] kho: fixes and cleanups

[Mike Rapoport]

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

Hi,

These are small KHO and KHO test fixes and cleanups

Mike Rapoport (Microsoft) (3):
  kho: allow scratch areas with zero size
  lib/test_kho: fixes for error handling
  selftest/kho: update generation of initrd

 kernel/kexec_handover.c               |  7 +++-
 lib/test_kho.c                        | 52 +++++++++++++++++----------
 tools/testing/selftests/kho/init.c    | 13 +++----
 tools/testing/selftests/kho/vmtest.sh | 28 ++++++++-------
 4 files changed, 58 insertions(+), 42 deletions(-)


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.47.2

[PATCH 1/3] kho: allow scratch areas with zero size

[Mike Rapoport]

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

Parsing of kho_scratch parameter treats zero size as an invalid value,
although it should be fine for user to request zero sized scratch area
for some types if scratch memory, when for example there is no need to
create scratch area in the low memory.

Treat zero as a valid value for a scratch area size but reject
kho_scratch parameter that defines no scratch memory at all.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 kernel/kexec_handover.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
index e49743ae52c5..c6ac5a5e51cb 100644
--- a/kernel/kexec_handover.c
+++ b/kernel/kexec_handover.c
@@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
 {
 	size_t len;
 	unsigned long sizes[3];
+	size_t total_size = 0;
 	int i;
 
 	if (!p)
@@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
 		}
 
 		sizes[i] = memparse(p, &endp);
-		if (!sizes[i] || endp == p)
+		if (endp == p)
 			return -EINVAL;
 		p = endp;
+		total_size += sizes[i];
 	}
 
+	if (!total_size)
+		return -EINVAL;
+
 	scratch_size_lowmem = sizes[0];
 	scratch_size_global = sizes[1];
 	scratch_size_pernode = sizes[2];
-- 
2.47.2

[PATCH 2/3] lib/test_kho: fixes for error handling

[Mike Rapoport]

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

* Update kho_test_save() so that folios array won't be freed when
  returning from the function and the fdt will be freed on error
* Reset state->nr_folios to 0 in  kho_test_generate_data() on error
* Simplify allocation of folios info in fdt.

Reported-by: Pratyush Yadav <pratyush@kernel.org>
Closes: https://lore.kernel.org/all/mafs0zfcjcepf.fsf@kernel.org
Fixes: b753522bed0b ("kho: add test for kexec handover")
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 lib/test_kho.c | 52 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/lib/test_kho.c b/lib/test_kho.c
index c2eb899c3b45..fe8504e3407b 100644
--- a/lib/test_kho.c
+++ b/lib/test_kho.c
@@ -67,13 +67,20 @@ static struct notifier_block kho_test_nb = {
 
 static int kho_test_save_data(struct kho_test_state *state, void *fdt)
 {
-	phys_addr_t *folios_info __free(kvfree) = NULL;
+	phys_addr_t *folios_info;
 	int err = 0;
 
-	folios_info = kvmalloc_array(state->nr_folios, sizeof(*folios_info),
-				     GFP_KERNEL);
-	if (!folios_info)
-		return -ENOMEM;
+	err |= fdt_begin_node(fdt, "data");
+	err |= fdt_property(fdt, "nr_folios", &state->nr_folios,
+			    sizeof(state->nr_folios));
+	err |= fdt_property_placeholder(fdt, "folios_info",
+					state->nr_folios * sizeof(*folios_info),
+					(void **)&folios_info);
+	err |= fdt_property(fdt, "csum", &state->csum, sizeof(state->csum));
+	err |= fdt_end_node(fdt);
+
+	if (err)
+		return err;
 
 	for (int i = 0; i < state->nr_folios; i++) {
 		struct folio *folio = state->folios[i];
@@ -83,17 +90,9 @@ static int kho_test_save_data(struct kho_test_state *state, void *fdt)
 
 		err = kho_preserve_folio(folio);
 		if (err)
-			return err;
+			break;
 	}
 
-	err |= fdt_begin_node(fdt, "data");
-	err |= fdt_property(fdt, "nr_folios", &state->nr_folios,
-			    sizeof(state->nr_folios));
-	err |= fdt_property(fdt, "folios_info", folios_info,
-			    state->nr_folios * sizeof(*folios_info));
-	err |= fdt_property(fdt, "csum", &state->csum, sizeof(state->csum));
-	err |= fdt_end_node(fdt);
-
 	return err;
 }
 
@@ -140,7 +139,10 @@ static int kho_test_generate_data(struct kho_test_state *state)
 		unsigned int size;
 		void *addr;
 
-		/* cap allocation so that we won't exceed max_mem */
+		/*
+		 * Since get_order() rounds up, make sure that actual
+		 * allocation is smaller so that we won't exceed max_mem
+		 */
 		if (alloc_size + (PAGE_SIZE << order) > max_mem) {
 			order = get_order(max_mem - alloc_size);
 			if (order)
@@ -165,13 +167,14 @@ static int kho_test_generate_data(struct kho_test_state *state)
 err_free_folios:
 	for (int i = 0; i < state->nr_folios; i++)
 		folio_put(state->folios[i]);
+	state->nr_folios = 0;
 	return -ENOMEM;
 }
 
 static int kho_test_save(void)
 {
 	struct kho_test_state *state = &kho_test_state;
-	struct folio **folios __free(kvfree) = NULL;
+	struct folio **folios;
 	unsigned long max_nr;
 	int err;
 
@@ -185,13 +188,23 @@ static int kho_test_save(void)
 
 	err = kho_test_generate_data(state);
 	if (err)
-		return err;
+		goto err_free_folios;
 
 	err = kho_test_prepare_fdt(state);
 	if (err)
-		return err;
+		goto err_free_folios;
 
-	return register_kho_notifier(&kho_test_nb);
+	err = register_kho_notifier(&kho_test_nb);
+	if (err)
+		goto err_free_fdt;
+
+	return 0;
+
+err_free_fdt:
+	folio_put(state->fdt);
+err_free_folios:
+	kvfree(folios);
+	return err;
 }
 
 static int kho_test_restore_data(const void *fdt, int node)
@@ -291,6 +304,7 @@ static void kho_test_cleanup(void)
 		folio_put(kho_test_state.folios[i]);
 
 	kvfree(kho_test_state.folios);
+	folio_put(kho_test_state.fdt);
 }
 
 static void __exit kho_test_exit(void)
-- 
2.47.2

[PATCH 3/3] selftest/kho: update generation of initrd

[Mike Rapoport]

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

Use nolibc include directory rather than include a cumulative nolibc.h
on the compiler command line and replace use of 'sudo cpio' with
usr/gen_init_cpio.

While on it fix spelling of KHO_FINALIZE

Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 tools/testing/selftests/kho/init.c    | 13 ++++---------
 tools/testing/selftests/kho/vmtest.sh | 28 ++++++++++++++-------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/kho/init.c b/tools/testing/selftests/kho/init.c
index 8034e24c6bf6..6d9e91d55d68 100644
--- a/tools/testing/selftests/kho/init.c
+++ b/tools/testing/selftests/kho/init.c
@@ -1,22 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0
 
-#ifndef NOLIBC
-#include <errno.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <fcntl.h>
-#include <syscall.h>
+#include <sys/syscall.h>
 #include <sys/mount.h>
 #include <sys/reboot.h>
-#endif
+#include <linux/kexec.h>
 
 /* from arch/x86/include/asm/setup.h */
 #define COMMAND_LINE_SIZE	2048
 
-/* from include/linux/kexex.h */
-#define KEXEC_FILE_NO_INITRAMFS	0x00000004
-
-#define KHO_FINILIZE "/debugfs/kho/out/finalize"
+#define KHO_FINALIZE "/debugfs/kho/out/finalize"
 #define KERNEL_IMAGE "/kernel"
 
 static int mount_filesystems(void)
@@ -32,7 +27,7 @@ static int kho_enable(void)
 	const char enable[] = "1";
 	int fd;
 
-	fd = open(KHO_FINILIZE, O_RDWR);
+	fd = open(KHO_FINALIZE, O_RDWR);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/testing/selftests/kho/vmtest.sh b/tools/testing/selftests/kho/vmtest.sh
index ec70a17bd476..3f6c17166846 100755
--- a/tools/testing/selftests/kho/vmtest.sh
+++ b/tools/testing/selftests/kho/vmtest.sh
@@ -10,7 +10,6 @@ kernel_dir=$(realpath "$test_dir/../../../..")
 
 tmp_dir=$(mktemp -d /tmp/kho-test.XXXXXXXX)
 headers_dir="$tmp_dir/usr"
-initrd_dir="$tmp_dir/initrd"
 initrd="$tmp_dir/initrd.cpio"
 
 source "$test_dir/../kselftest/ktap_helpers.sh"
@@ -81,19 +80,22 @@ EOF
 function mkinitrd() {
 	local kernel=$1
 
-	mkdir -p "$initrd_dir"/{dev,debugfs,proc}
-	sudo mknod "$initrd_dir/dev/console" c 5 1
-
-	"$CROSS_COMPILE"gcc -s -static -Os -nostdinc -I"$headers_dir/include" \
-			-fno-asynchronous-unwind-tables -fno-ident -nostdlib \
-			-include "$test_dir/../../../include/nolibc/nolibc.h" \
-			-o "$initrd_dir/init" "$test_dir/init.c" \
-
-	cp "$kernel" "$initrd_dir/kernel"
+	"$CROSS_COMPILE"gcc -s -static -Os -nostdinc -nostdlib \
+			-fno-asynchronous-unwind-tables -fno-ident \
+			-I "$headers_dir/include" \
+			-I "$kernel_dir/tools/include/nolibc" \
+			-o "$tmp_dir/init" "$test_dir/init.c"
+
+	cat > "$tmp_dir/cpio_list" <<EOF
+dir /dev 0755 0 0
+dir /proc 0755 0 0
+dir /debugfs 0755 0 0
+nod /dev/console 0600 0 0 c 5 1
+file /init $tmp_dir/init 0755 0 0
+file /kernel $kernel 0644 0 0
+EOF
 
-	pushd "$initrd_dir" &>/dev/null
-	find . | cpio -H newc --create > "$initrd" 2>/dev/null
-	popd &>/dev/null
+	"$build_dir/usr/gen_init_cpio" "$tmp_dir/cpio_list" > "$initrd"
 }
 
 function run_qemu() {
-- 
2.47.2

Re: [PATCH 1/3] kho: allow scratch areas with zero size

[Pratyush Yadav]

On Mon, Aug 11 2025, Mike Rapoport wrote:

> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> Parsing of kho_scratch parameter treats zero size as an invalid value,
> although it should be fine for user to request zero sized scratch area
> for some types if scratch memory, when for example there is no need to
> create scratch area in the low memory.

Can the system boot with 0 per-node memory? If not, then perhaps we
should only allow lowmem scratch to be zero?

>
> Treat zero as a valid value for a scratch area size but reject
> kho_scratch parameter that defines no scratch memory at all.
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  kernel/kexec_handover.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> index e49743ae52c5..c6ac5a5e51cb 100644
> --- a/kernel/kexec_handover.c
> +++ b/kernel/kexec_handover.c
> @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
>  {
>  	size_t len;
>  	unsigned long sizes[3];
> +	size_t total_size = 0;
>  	int i;
>  
>  	if (!p)
> @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
>  		}
>  
>  		sizes[i] = memparse(p, &endp);
> -		if (!sizes[i] || endp == p)
> +		if (endp == p)
>  			return -EINVAL;
>  		p = endp;
> +		total_size += sizes[i];
>  	}
>  
> +	if (!total_size)
> +		return -EINVAL;
> +

Looks good. BTW, unrelated to this patch, but should we also check that
p == '\0' here to make sure the whole argument was consumed?


>  	scratch_size_lowmem = sizes[0];
>  	scratch_size_global = sizes[1];
>  	scratch_size_pernode = sizes[2];

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/3] lib/test_kho: fixes for error handling

[Pratyush Yadav]

On Mon, Aug 11 2025, Mike Rapoport wrote:

> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> * Update kho_test_save() so that folios array won't be freed when
>   returning from the function and the fdt will be freed on error
> * Reset state->nr_folios to 0 in  kho_test_generate_data() on error
> * Simplify allocation of folios info in fdt.
>
> Reported-by: Pratyush Yadav <pratyush@kernel.org>
> Closes: https://lore.kernel.org/all/mafs0zfcjcepf.fsf@kernel.org
> Fixes: b753522bed0b ("kho: add test for kexec handover")
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

Re: [PATCH 1/3] kho: allow scratch areas with zero size

[Mike Rapoport]

On Wed, Aug 13, 2025 at 03:45:29PM +0200, Pratyush Yadav wrote:
> On Mon, Aug 11 2025, Mike Rapoport wrote:
> 
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > Parsing of kho_scratch parameter treats zero size as an invalid value,
> > although it should be fine for user to request zero sized scratch area
> > for some types if scratch memory, when for example there is no need to
> > create scratch area in the low memory.
> 
> Can the system boot with 0 per-node memory? If not, then perhaps we
> should only allow lowmem scratch to be zero?

In most cases yes because most of boot time allocations have fallback to
"any node".
And there's also an option to omit the "global" scratch and boot with only
per-node scratch areas, so I'd keep the possibility of setting any of these
to 0.

> > Treat zero as a valid value for a scratch area size but reject
> > kho_scratch parameter that defines no scratch memory at all.
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> >  kernel/kexec_handover.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> > index e49743ae52c5..c6ac5a5e51cb 100644
> > --- a/kernel/kexec_handover.c
> > +++ b/kernel/kexec_handover.c
> > @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
> >  {
> >  	size_t len;
> >  	unsigned long sizes[3];
> > +	size_t total_size = 0;
> >  	int i;
> >  
> >  	if (!p)
> > @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
> >  		}
> >  
> >  		sizes[i] = memparse(p, &endp);
> > -		if (!sizes[i] || endp == p)
> > +		if (endp == p)
> >  			return -EINVAL;
> >  		p = endp;
> > +		total_size += sizes[i];
> >  	}
> >  
> > +	if (!total_size)
> > +		return -EINVAL;
> > +
> 
> Looks good. BTW, unrelated to this patch, but should we also check that
> p == '\0' here to make sure the whole argument was consumed?

Care to send a patch? ;-) 
 
> >  	scratch_size_lowmem = sizes[0];
> >  	scratch_size_global = sizes[1];
> >  	scratch_size_pernode = sizes[2];
> 
> -- 
> Regards,
> Pratyush Yadav

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/3] kho: allow scratch areas with zero size

[Pratyush Yadav]

On Wed, Aug 13 2025, Mike Rapoport wrote:

> On Wed, Aug 13, 2025 at 03:45:29PM +0200, Pratyush Yadav wrote:
>> On Mon, Aug 11 2025, Mike Rapoport wrote:
>> 
>> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>> >
>> > Parsing of kho_scratch parameter treats zero size as an invalid value,
>> > although it should be fine for user to request zero sized scratch area
>> > for some types if scratch memory, when for example there is no need to
>> > create scratch area in the low memory.
>> 
>> Can the system boot with 0 per-node memory? If not, then perhaps we
>> should only allow lowmem scratch to be zero?
>
> In most cases yes because most of boot time allocations have fallback to
> "any node".
> And there's also an option to omit the "global" scratch and boot with only
> per-node scratch areas, so I'd keep the possibility of setting any of these
> to 0.

Makes sense. In that case,

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

>
>> > Treat zero as a valid value for a scratch area size but reject
>> > kho_scratch parameter that defines no scratch memory at all.
>> >
>> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>> > ---
>> >  kernel/kexec_handover.c | 7 ++++++-
>> >  1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
>> > index e49743ae52c5..c6ac5a5e51cb 100644
>> > --- a/kernel/kexec_handover.c
>> > +++ b/kernel/kexec_handover.c
>> > @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
>> >  {
>> >  	size_t len;
>> >  	unsigned long sizes[3];
>> > +	size_t total_size = 0;
>> >  	int i;
>> >  
>> >  	if (!p)
>> > @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
>> >  		}
>> >  
>> >  		sizes[i] = memparse(p, &endp);
>> > -		if (!sizes[i] || endp == p)
>> > +		if (endp == p)
>> >  			return -EINVAL;
>> >  		p = endp;
>> > +		total_size += sizes[i];
>> >  	}
>> >  
>> > +	if (!total_size)
>> > +		return -EINVAL;
>> > +
>> 
>> Looks good. BTW, unrelated to this patch, but should we also check that
>> p == '\0' here to make sure the whole argument was consumed?
>
> Care to send a patch? ;-) 

Will do :-)

>  
>> >  	scratch_size_lowmem = sizes[0];
>> >  	scratch_size_global = sizes[1];
>> >  	scratch_size_pernode = sizes[2];
>> 
>> -- 
>> Regards,
>> Pratyush Yadav

-- 
Regards,
Pratyush Yadav