Re: [PATCH 01/10] xen/arm: introduce domain on Static Allocation

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

Hi,

On Thu, 3 Jun 2021 at 22:33, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> On Thu, 3 Jun 2021, Julien Grall wrote:
> > On 02/06/2021 11:09, Penny Zheng wrote:
> > > I could not think a way to fix it properly in codes, do you have any
> > > suggestion? Or we just put a warning in doc/commits.
> >
> > The correct approach is to find the parent of staticmemdomU1 (i.e.
> > reserved-memory) and use the #address-cells and #size-cells from there.
>
> Julien is right about how to parse the static-memory.
>
> But I have a suggestion on the new binding. The /reserved-memory node is
> a weird node: it is one of the very few node (the only node aside from
> /chosen) which is about software configurations rather than hardware
> description.
>
> For this reason, in a device tree with multiple domains /reserved-memory
> doesn't make a lot of sense: for which domain is the memory reserved?

IHMO, /reserved-memory refers to the memory that the hypervisor should
not touch. It is just a coincidence that most of the domains are then
passed through to dom0.

This also matches the fact that the GIC, /memory is consumed by the
hypervisor directly and not the domain..

>
> This was one of the first points raised by Rob Herring in reviewing
> system device tree.
>
> So the solution we went for is the following: if there is a default
> domain /reserved-memory applies to the default domain. Otherwise, each
> domain is going to have its own reserved-memory. Example:
>
>         domU1 {
>             compatible = "xen,domain";
>             #address-cells = <0x1>;
>             #size-cells = <0x1>;
>             cpus = <2>;
>
>             reserved-memory {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>
>                 static-memory@0x30000000 {
>                     compatible = "xen,static-memory-domain";
>                     reg = <0x0 0x30000000 0x0 0x20000000>;
>                 };
>             };
>         };

This sounds wrong to me because the memory is reserved from the
hypervisor PoV not from the domain. IOW, when I read this, I think the
memory will be reserved in the domain.

>
> So I don't think we want to use reserved-memory for this, either
> /reserved-memory or /chosen/domU1/reserved-memory. Instead it would be
> good to align it with system device tree and define it as a new property
> under domU1.

Do you have any formal documentation of the system device-tree?

>
> In system device tree we would use a property called "memory" to specify
> one or more ranges, e.g.:
>
>     domU1 {
>         memory = <0x0 0x500000 0x0 0x7fb00000>;
>
> Unfortunately for xen,domains we have already defined "memory" to
> specify an amount, rather than a range. That's too bad because the most
> natural way to do this would be:
>
>     domU1 {
>         size = <amount>;
>         memory = <ranges>;
>
> When we'll introduce native system device tree support in Xen we'll be
> able to do that. For now, we need to come up with a different property.
> For instance: "static-memory" (other names are welcome if you have a
> better suggestion).
>
> We use a new property called "static-memory" together with
> #static-memory-address-cells and #static-memory-size-cells to define how
> many cells to use for address and size.
>
> Example:
>
>     domU1 {
>         #static-memory-address-cells = <2>;
>         #static-memory-size-cells = <2>;
>         static-memory = <0x0 0x500000 0x0 0x7fb00000>;

This is pretty similar to what Penny suggested. But I dislike it
because of the amount of code that needs to be duplicated with the
reserved memory.

Cheers,