[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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,



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.