On 06/11/2024 14:41, Luca Fancellu wrote:
>
>
> There are some cases where the device tree exposes a memory range
> in both /memreserve/ and reserved-memory node, in this case the
> current code will stop Xen to boot since it will find that the
> latter range is clashing with the already recorded /memreserve/
> ranges.
>
> Furthermore, u-boot lists boot modules ranges, such as ramdisk,
> in the /memreserve/ part and even in this case this will prevent
> Xen to boot since it will see that the module memory range that
> it is going to add in 'add_boot_module' clashes with a /memreserve/
> range.
>
> When Xen populate the data structure that tracks the memory ranges,
> it also adds a memory type described in 'enum membank_type', so
> in order to fix this behavior, allow the 'check_reserved_regions_overlap'
> function to check for exact memory range match given a specific memory
> type; allowing reserved-memory node ranges and boot modules to have an
> exact match with ranges from /memreserve/.
>
> While there, set a type for the memory recorded during ACPI boot.
>
> Fixes: 53dc37829c31 ("xen/arm: Add DT reserve map regions to
> bootinfo.reserved_mem")
> Reported-by: Shawn Anastasio <sanasta...@raptorengineering.com>
> Reported-by: Grygorii Strashko <grygorii_stras...@epam.com>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> ---
> I tested this patch adding the same range in a /memreserve/ entry and
> /reserved-memory node, and by letting u-boot pass a ramdisk.
> I've also tested that a configuration running static shared memory still works
> fine.
> ---
So we have 2 separate issues. I don't particularly like the concept of
introducing MEMBANK_NONE
and the changes below look a bit too much for me, given that for boot modules
we can only have
/memreserve/ matching initrd.
Shawn patch fixes the first issue. AFAICT the second issue can be fixed by
below simple patch:
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index 927f59c64b0d..d8bd8c44bd35 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -586,6 +586,10 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
+ ret = device_tree_for_each_node(fdt, 0, early_scan_node, NULL);
+ if ( ret )
+ panic("Early FDT parsing failed (%d)\n", ret);
+
nr_rsvd = fdt_num_mem_rsv(fdt);
if ( nr_rsvd < 0 )
panic("Parsing FDT memory reserve map failed (%d)\n", nr_rsvd);
@@ -594,10 +598,14 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t
paddr)
{
struct membank *bank;
paddr_t s, sz;
+ const struct bootmodule *mod =
boot_module_find_by_kind(BOOTMOD_RAMDISK);
if ( fdt_get_mem_rsv_paddr(device_tree_flattened, i, &s, &sz) < 0 )
continue;
+ if ( mod && (mod->start == s) && (mod->size == sz) )
+ continue;
+
if ( reserved_mem->nr_banks < reserved_mem->max_banks )
{
bank = &reserved_mem->bank[reserved_mem->nr_banks];
@@ -610,10 +618,6 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
panic("Cannot allocate reserved memory bank\n");
}
- ret = device_tree_for_each_node(fdt, 0, early_scan_node, NULL);
- if ( ret )
- panic("Early FDT parsing failed (%d)\n", ret);
-
/*
* On Arm64 setup_directmap_mappings() expects to be called with the lowest
* bank in memory first. There is no requirement that the DT will provide
I'll let other DT maintainers voice their opinion.
~Michal
