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

[Mukesh Pilaniya <mpilaniy@redhat.com>]

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.

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