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 previously
So 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.stabell...@amd.com>
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
