[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/arm/efi: merge neighboring banks
Hi Stefano, On 21/03/2025 21:14, Stefano Stabellini wrote: When booting from U-Boot bootefi, there can be a high number of neighboring RAM banks. See for example: (XEN) RAM: 0000000000000000 - 0000000000bfffff (XEN) RAM: 0000000000c00000 - 0000000000c00fff (XEN) RAM: 0000000000c01000 - 0000000000dfffff (XEN) RAM: 0000000000e00000 - 000000000279dfff (XEN) RAM: 000000000279e000 - 00000000029fffff (XEN) RAM: 0000000002a00000 - 0000000008379fff (XEN) RAM: 000000000837a000 - 00000000083fffff (XEN) RAM: 0000000008400000 - 0000000008518fff (XEN) RAM: 0000000008519000 - 00000000085fffff (XEN) RAM: 0000000008600000 - 0000000008613fff (XEN) RAM: 0000000008614000 - 00000000097fffff (XEN) RAM: 0000000009800000 - 00000000098a7fff (XEN) RAM: 00000000098a8000 - 0000000009dfffff (XEN) RAM: 0000000009e00000 - 0000000009ea7fff (XEN) RAM: 0000000009ea8000 - 000000001fffffff (XEN) RAM: 0000000020000000 - 000000002007ffff (XEN) RAM: 0000000020080000 - 0000000077b17fff (XEN) RAM: 0000000077b19000 - 0000000077b2bfff (XEN) RAM: 0000000077b2c000 - 0000000077c8dfff (XEN) RAM: 0000000077c8e000 - 0000000077c91fff (XEN) RAM: 0000000077ca7000 - 0000000077caafff (XEN) RAM: 0000000077cac000 - 0000000077caefff (XEN) RAM: 0000000077cd0000 - 0000000077cd2fff (XEN) RAM: 0000000077cd4000 - 0000000077cd7fff (XEN) RAM: 0000000077cd8000 - 000000007bd07fff (XEN) RAM: 000000007bd09000 - 000000007fd5ffff (XEN) RAM: 000000007fd70000 - 000000007fefffff (XEN) RAM: 0000000800000000 - 000000087fffffff It is undesirable to keep them separate, as this approach consumes more resources. What resources? This is pre-allocated static array in the binary. They are also dropped after init. The more interesting argument is... Additionally, Xen does not currently support boot modules that span multiple banks: at least one of the regions get freed twice. The first time from setup_mm->populate_boot_allocator, then again from discard_initial_modules->fw_unreserved_regions. With a high number of banks, it can be difficult to arrange the boot modules in a way that avoids spanning across multiple banks. ... this one. Although, I find weird that U-boot would create tons of banks. Have you considered to ask them what's going on? Also, what about the cases where U-boot is not booting Xen without UEFI? Why is this not a problem? Asking just in case the merge should happen in code common rather than in UEFI. This small patch merges neighboring regions, to make dealing with them more efficient, and to make it easier to load boot modules. While I understand the value for this, I ... The patch also takes the opportunity to check and discard duplicates. don't understand why we need to check for duplicates. This also doesn't properly work for a few reasons: * This doesn't cover partial overlap * This would not work if an entry was merged with another previouslySo I think the code to check duplicates should be removed. If you are concerned about overlap, then it would be better to enable check_reserved just drop the code. If you are concerned about to detect and warn/panic. Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index a80a5a7ab3..f6f23ed5b2 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -163,6 +163,20 @@ static bool __init meminfo_add_bank(struct membanks *mem, struct membank *bank; paddr_t start = desc->PhysicalStart; paddr_t size = desc->NumberOfPages * EFI_PAGE_SIZE; + int j; nr_banks is an "unsigned int". So this should be the same type. + + for ( j = 0; j < mem->nr_banks; j++ ) + { + if ( mem->bank[j].start == start && mem->bank[j].size == size ) + { + return true; + } + else if ( mem->bank[j].start + mem->bank[j].size == start ) Please add some parentheses to make it more obvious that you checking (a + b) == size. + { + mem->bank[j].size += size; + return true; + } + }if ( mem->nr_banks >= mem->max_banks )return false; Cheers, -- Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |