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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 14/11/2024 13:04, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 14/11/2024 11:48, Michal Orzel wrote:
>>
>>
>> On 14/11/2024 11:31, Julien Grall wrote:
>>> Looking at the code, I think /memreserve/ and /reserved-memory are not
>>> mapped in Xen and everything in /reserved-memory is mapped to dom0.
>> Why do we forward /reserved-memory to dom0 fdt but /memreserve/ not?
> 
> I was wondering the same. The main issue I can think of with
> /memreserve/ is some of the regions will likely be for Xen own usage. So
Can you give example of regions defined as reserved for Xen usage (other than 
static-mem)?

> we would need to have a way to exclude them from dom0.
> 
>  >  From the discussion> we're having it seems like we should treat them
> equally. Also, looking at Luca patch,
>> we seem to special case /memreserve/ and only allow for overlap /memresrve/ 
>> with boot modules
>> and not /reserved-memory/ with boot modules. If we are going to claim that 
>> all the boot modules
>> can be marked as reserved by the bootloader, then I think we should treat 
>> them equally providing
>> the same experience to dom0.
> 
> In my mind, /memreserved/ and /reserved-memory/ are different. The
> former doesn't say what the region is for, but the latter will indicate it.
In the context of this patch, I don't agree. We're discussing overlap, and if a 
region A
from /memreserve/ overlaps fully with a module A, we know what is the purpose 
of it.
Today it's initrd, but as you say we cannot rule out other modules as well.

> 
> So I am not 100% sure how the bootmodule could end up in
> /reserved-memory/ because they are described as part of the multiboot
> modules. Do you have a scenario?
I don't same as I don't have scenario for /memreserve/ overlapping with sth 
else than initrd.
All of these comes from my validation of u-boot, grub, barebox code. I have a 
feeling that due to
U-Boot trick that is not present in any other *known* bootloader, we are trying 
to over-engineer the problem :)
But as Stefano and you wrote, we should follow the spec and for me we should 
therefore treat them equally.

> 
> Regardless that, if we decide to allow boot modules in /reserved-memory/
> then we would need need to rework how the reserved-regions are mapped
> because we don't want the boot modules to be exposed to dom0.
+1

> 
>>
>> Last thing I wanted to ask (for my clarity) is that if bootloader loads 
>> initrd at region A and marks
>> it as reserved (either in /memreserve/ or /reserved-memory/), and later on 
>> Xen copies initrd from region
>> A to B, shouldn't the reserved memory region be updated to cover new region 
>> for initrd?
> 
> If we mark the region has a reserved, then we are telling the OS it
> can't use the region. But I am not sure why it would be needed as Xen
Well, in the context of initrd, kernel uses it even though it is reserved. This 
is because
of the second part of the spec where other bindings come into play.

> doesn't care how the regions is going to be used by the domain. From a
> domain side, do you see any reason why we would want to mark again the
> region as reserved?
TBH I don't same as I still don't know why U-Boot does that trick. It comes 
from a very
old code and my initial understanding is that it is done purely for U-boot 
bookkeeping.

> 
> If we didn't copy the initrd, then I would have directly agreed that
> they should be marked as /memreserve/.
> 
> Cheers,
> 
> --
> Julien Grall
> 

~Michal