Re: [PATCH] xen/device-tree: Allow exact match for overlapping regions

[Michal Orzel <michal.orzel@xxxxxxx>]

On 13/11/2024 14:50, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 06/11/2024 15:07, Michal Orzel wrote:
>>
>>
>> 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 <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
>>> Reported-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> ---
>>> 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.
> 
> How so? Is this an observation or part of a specification?
Not sure what specification you would want to see. It's all part of U-Boot and 
Linux behavior that is not documented (except for code comments).
My statement is based on the U-Boot and Linux behavior. U-Boot part only 
present for initrd:
https://github.com/u-boot/u-boot/blob/master/boot/fdt_support.c#L249

For things that Xen can be interested in, only region for ramdisk for dom0 can 
match the /memreserve/ region.
Providing a generic solution (like Luca did) would want providing an example of 
sth else that can match which I'm not aware of.

~Michal