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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 Nov 2024, Julien Grall wrote:
> On 14/11/2024 12:22, Michal Orzel wrote:
> > 
> > 
> > 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)?
> 
> The spin table to bring-up CPUs.

Yes, maybe my EFI runtime services example was not ideal, but basically
any firmware "stuff" that is expected to survive the end of the
firmware/bootloader boot sequence and should not be accessed directly by
the OS could be under /memreserve/  The spin table is a good example.


> > > 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.
> 
> To give a concrete example, the /reserved-region/ can be used to reserve space
> for the VGA buffer. It would be odd that someone would put the boot module in
> the middle of the VGA buffer... If Xen ends up to use the VGA buffer (not the
> case today), then it would be a problem. Xen would need to be reworked to move
> all boot modules outside of the memory because it can access the VGA (or any
> other reserved regions).
> 
> The problem is slightly different for /memreserve/ because we don't exactly
> know what the regions are for until we parse the rest of the DT. Yes
> technically, a user could put the initrd in the wrong place. So the problem is
> the same. But this is a relexation I am more willing to accept over
> /reserved-region/ right now.

Looking at the specification, I believe we should handle
/reserved-memory and /memreserve/ differently. Note that I have not
reviewed this patch; my comments are based solely on the expected Xen
behavior at the specification level.

Given that /reserved-memory regions are designated for special driver
use, I think Xen should remap all /reserved-memory regions to Dom0.
Ideally, Xen would determine which driver requires each /reserved-memory
region and assign only the relevant region to the respective domain,
rather than assigning all regions to Dom0. However, for now, we can take
this as an initial simplification.

On the other hand, since /memreserve/ is not intended for general use
and we do not know its content, I believe /memreserve/ should not be
mapped to Dom0 or any domain. It could be included in Xen's directmap,
but Xen itself should not access it. If another device tree node
represents a range or a subset of a range also described in
/memreserve/, that node should take precedence, and the corresponding
range should be assigned to Dom0 (or another domain, such as DomU, or
Xen, as appropriate). Unfortunately, this implies Xen needs to
understand the linux,initrd node.

I think /memreserve/ is valuable for protecting certain areas from the
operating system. For instance, it could be used as a storage area for
guest EFI variables in a VM, where the guest OS should never access the
range. However, I do not believe the initrd range is a good use of
/memreserve/ and similar items, which are meant to be freed anyway and
where Linux already knows the exact range and its intended use. So I
would not change Xen to start reserving the initrd range for Linux. The
likely reason U-Boot uses /memreserve/ is that the U-Boot code
implementing this behavior predates the introduction of
/reserved-memory.