Re: [PATCH v2] xen/device-tree: Allow region overlapping with /memreserve/ ranges

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 14.11.24 12:28, 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 overlapping with the /memreserve/
> ranges in the 'check_reserved_regions_overlap' function when a flag
> is set.
>
> In order to implement this solution, there is a distinction between
> the 'struct membanks *' handled by meminfo_overlap_check(...) that
> needs to be done, because the static shared memory banks doesn't have
> a usable bank[].type field and so it can't be accessed, hence now
> the 'struct membanks_hdr' have a 'enum region_type type' field in order
> to be able to identify static shared memory banks in meminfo_overlap_check(...).
>
> While there, set a type for the memory recorded using meminfo_add_bank()
> from efi-boot.h.
>
> Fixes: 53dc37829c31 ("xen/arm: Add DT reserve map regions to 
> bootinfo.reserved_mem")
> Reported-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> Reported-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> Changes from v1:
>   - fixed commit message
>   - used boolean to allow /memreserve/ overlap or not
>   - now 'struct membanks *' have a type, while it might seem a waste of space,
>     it might be used in the future to consolidate also the 'struct bootmodules'
>     and 'struct bootcmdlines' to use the same 'struct membanks *' interface.
>     Also the added space is limited to 3/4 enum size.
>   - The change above allowed this version to remove the "hack" when dealing with
>     static shared memory banks, that doesn't have a type.
>
> 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.
> ---
> ---
>   xen/arch/arm/efi/efi-boot.h       |  3 ++-
>   xen/arch/arm/static-shmem.c       |  2 +-
>   xen/common/device-tree/bootfdt.c  |  9 ++++++-
>   xen/common/device-tree/bootinfo.c | 39 +++++++++++++++++++++++--------
>   xen/include/xen/bootfdt.h         | 20 +++++++++++++---
>   5 files changed, 57 insertions(+), 16 deletions(-)
[...]

Dom0 started successfully with Initrd.

Tested-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>

Meminfo from boot log is below:

(XEN) Checking for initrd in /chosen
(XEN) Initrd 0000000084000040-0000000087bc5831
(XEN) RAM: 0000000048000000 - 00000000bfffffff
(XEN) RAM: 0000000480000000 - 00000004ffffffff
(XEN) RAM: 0000000600000000 - 00000006ffffffff
(XEN)
(XEN) MODULE[0]: 0000000048080000 - 00000000481ec000 Xen
(XEN) MODULE[1]: 0000000048000000 - 000000004801e080 Device Tree
(XEN) MODULE[2]: 0000000084000040 - 0000000087bc5831 Ramdisk
(XEN) MODULE[3]: 0000000048300000 - 000000004a300000 Kernel
(XEN) MODULE[4]: 0000000048070000 - 0000000048080000 XSM
(XEN)  RESVD[0]: 0000000084000040 - 0000000087bc5830
(XEN)  RESVD[1]: 0000000060000000 - 000000007fffffff
(XEN)  RESVD[2]: 00000000b0000000 - 00000000bfffffff
(XEN)  RESVD[3]: 00000000a0000000 - 00000000afffffff

Thank you,
-grygorii