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

[Mike Rapoport <rppt@kernel.org>]

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?

And after a cycle of KHO boots to newer kernels we anyway can get to the
point when bootmem is not large enough.
 
> > +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.
 
> 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
> 

-- 
Sincerely yours,
Mike.