Re: [PATCH v2 1/3] kho: walk bootmem areas in address order

[Mike Rapoport <rppt@kernel.org>]

On Fri, Nov 13, 2026 at 10:34:06PM +0530, Mukesh Pilaniya wrote:
> Hi Mike,
> 
> Thanks for the quick check. I apologies in advance if I am missing
> something obvious.
> 
> On 12/11/26 2:19 pm, Mike Rapoport wrote:
> > On Thu, Nov 12, 2026 at 12:02:05PM +0530, Mukesh Pilaniya wrote:
> >> Hi Mike,
> >>
> >> On 04/11/26 10:20 pm, Mike Rapoport (Microsoft) wrote:
> >>> kexec_add_buffer() users may depend on the order in which memory holes are
> >>> searched. For instance, arm64 places the kernel bottom-up and then
> >>> requires the DTB to be above the kernel.
> >>>
> >>> kho_walk_bootmem() walks the bootmem areas in the order they were
> >>> reserved, so if the kernel lands in a small area at the top of the memory
> >>> there is no room left for the DTB and kexec fails. For example, on an
> >>> arm64 VM with 2G of RAM and kho_bootmem=8M,64M,64M, the lowmem area is
> >>> reserved first at the top of the memory, the kernel image takes most of
> >>> it and kexec fails with
> >>>
> >>>   kexec_file(Image): Could not find any suitable kernel location!
> >>>
> >>> Sort the bootmem areas by address and walk them in ascending order, or in
> >>> descending order for top-down searches, like kexec_walk_resources() does.
> >>>
> >>> Fixes: 3bdecc3c93f9 ("kexec: add KHO support to kexec file loads")
> >>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >>> ---
> >>>  kernel/liveupdate/kexec_handover.c | 57 ++++++++++++++++++++++++++++++--------
> >>>  1 file changed, 46 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> >>> index 6f1f55fe31819..2ac5d94f33fd4 100644
> >>> --- a/kernel/liveupdate/kexec_handover.c
> >>> +++ b/kernel/liveupdate/kexec_handover.c
> >>> @@ -26,6 +26,7 @@
> >>>  #include <linux/list.h>
> >>>  #include <linux/memblock.h>
> >>>  #include <linux/page-isolation.h>
> >>> +#include <linux/sort.h>
> >>>  #include <linux/unaligned.h>
> >>>  #include <linux/vmalloc.h>
> >>>  
> >>> @@ -820,6 +821,14 @@ bool kho_bootmem_overlap(phys_addr_t phys, size_t size)
> >>>  	return false;
> >>>  }
> >>>  
> >>> +static int __init kho_bootmem_cmp(const void *_a, const void *_b)
> >>> +{
> >>> +	const struct kho_bootmem *a = _a;
> >>> +	const struct kho_bootmem *b = _b;
> >>> +
> >>> +	return cmp_int(a->addr, b->addr);
> >>> +}
> >>> +
> >>>  /**
> >>>   * kho_reserve_bootmem - Reserve a contiguous chunk of memory for kexec
> >>>   *
> >>> @@ -904,6 +913,9 @@ static void __init kho_reserve_bootmem(void)
> >>>  	 */
> >>>  	kho_bootmem_cnt = i;
> >>>  
> >>> +	sort(kho_bootmem, kho_bootmem_cnt, sizeof(*kho_bootmem),
> >>> +	     kho_bootmem_cmp, NULL);
> >> This only covers the case where this kernel allocated the areas itself.
> >> On a KHO boot the array comes from the previous kernel. A kernel with
> >> this series that is handed over from one without it therefore keeps
> >> walking the areas in reservation order for every following
> >> kexec_file_load, and the arm64 failure you describe comes back until the
> >> next cold boot and vise versa because the ABI string is still
> >> "kho-v4".
> > 
> > If we were able to kexec from a kernel with no reservation ordering, the
> > issue is not there, isn't it?
> IMHO that holds for the image that was just handed over, but maybe not
> for the next one. The producer's last kexec_file_load did place that
> exact Image, DTB and command line through the areas in that order, so I
> agree that reloading the same image from the consumer will find the same
> spot. The order is still arbitrary though, and whether it works seems to
> depend on the size of the image being loaded.
> 
> To give you the case I was looking at: on a QEMU arm64 guest with 4K
> pages, 2 GiB of RAM and kho_bootmem=44M,64M,64M, the 44 MiB lowmem area
> is reserved first, at the top of RAM (0xb8800000). The Image is about
> 38 MiB, and the DTB has to go above it on a 2 MiB boundary, which lands
> exactly on 0xbb400000, the end of that area. The load then fails with
> "kexec_file_load failed: Cannot assign requested address", and the next
> area is never tried, because the walk stops at the first area that
> cannot take the buffer. With the areas walked in address order the same
> image is placed in a lower, larger area and boots. So the first area
> decides, and a few MiB of growth in the Image or a larger DTB is enough
> to flip the result -- which is the part that makes me a bit nervous
> about the first kexec after a handover.

There's always a risk that KHO will run out of bootmem, even presuming we
fixed all our bugs :)

In your example it happens sooner then it should because there's still
free memory that kexec won't use, but it's still the same - we've run out
of bootmem.

I prefer to keep the sorting in the cold boot going forward to save a few
cycles at kexec time, so I'd rather not add sorting to the kexec'ed boot.

So the kernel from 7.5 will have it right and if there's a case when we use
an older stable kernel for the cold boot, and we see that we *really* need
sorting after kexec there we can just add such sort as a fix for stable.

BTW, in your case it's enough to switch lowmem and global sizes to gain a
few megs for future image growth, isn't it?
 
> It also does not settle down the chain, as far as I can tell. A kernel
> booted by KHO never reserves, so it never sorts: B inherits A's order,
> C inherits B's order, and only a cold boot sorts. That makes the same
> binary behave differently depending on how it was started. Sorting in
> kho_mem_retrieve as well would cover it.
> > 
> > And after a cycle of KHO boots to newer kernels we anyway can get to the
> > point when bootmem is not large enough.
> That is the same argument, and it is probably why I am not fully
> convinced that the previous successful kexec covers the ordering case
> either. The inherited state was adequate for the load that produced the
> running kernel, not necessarily for the load it is about to perform.
> 
> >  
> >>> +static int kho_bootmem_walk_top_down(struct kexec_buf *kbuf,
> >>> +				     int (*func)(struct resource *, void *))
> >>>  {
> >>>  	int ret = 0;
> >>> -	int i;
> >>>  
> >>> -	for (i = 0; i < kho_bootmem_cnt; i++) {
> >>> -		struct resource res = {
> >>> -			.start = kho_bootmem[i].addr,
> >>> -			.end = kho_bootmem[i].addr + kho_bootmem[i].size - 1,
> >>> -		};
> >>> +	for (int i = kho_bootmem_cnt - 1; i >= 0; i--) {
> >>
> >> kho_bootmem_cnt is unsigned, but the reverse loops in
> >> kho_reserve_bootmem() and kho_bootmem_walk_top_down() start an int
> >> counter at kho_bootmem_cnt - 1, which for a zero count is UINT_MAX
> >> converted to int.  That conversion is implementation-defined; with the
> >> current compilers it yields -1 and the loop body is skipped.
> > 
> > We never get here with kho_bootmem_cnt == 0.
> Unless I am misreading it, we do, via kho_bootmem=0%. The parser accepts
> it and stores the scale without a zero check, then bootmem_size_update()
> takes the explicit-size branch and assigns the zero-initialised
> bootmem_size_pernode to every node. kho_reserve_bootmem_area() returns
> early for a zero size without recording the area, so kho_bootmem_cnt
> stays 0 and kho_enable is left alone and only the error path clears it.
> 
> kho_locate_mem_hole() is guarded by kho_enable only, so with cnt == 0
> both walks return 0 and the caller turns that into -EADDRNOTAVAIL: KHO
> is enabled with no regions and every kexec_file_load fails with "Cannot
> assign requested address".
> 
> FWIW this became possible with 214f9649 commit, before it the
> zero-sized areas were recorded, so cnt was never 0. The zero-scale patch
> you posted in the v4 series [v3 1/5]thread closes this one.
> 
> >  
> >> How about using this one.
> >> 	for (unsigned int i = kho_bootmem_cnt; i-- > 0;)
> >>
> >> Reviewed-by: Mukesh Pilaniya <mpilaniy@redhat.com>
> > 
> > Thanks!
> >  
> >> -- 
> >> Regards,
> >> Mukesh Pilaniya
> >>
> > 
> 
> -- 
> Regards,
> Mukesh Pilaniya
> 

-- 
Sincerely yours,
Mike.