Re: [Xen-devel] [PATCH v7 8/8] xen/arm: add reserved-memory regions to the dom0 memory node

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

On 20/08/2019 16:28, Oleksandr wrote:
> On 19.08.19 20:43, Stefano Stabellini wrote:
> > Reserved memory regions are automatically remapped to dom0. Their device
> > tree nodes are also added to dom0 device tree. However, the dom0 memory
> > node is not currently extended to cover the reserved memory regions
> > ranges as required by the spec.  This commit fixes it.
> >
> > Change make_memory_node to take a  struct meminfo * instead of a
> > kernel_info. Call it twice for dom0, once to create the first regular
> > memory node, and the second time to create a second memory node with the
> > ranges covering reserved-memory regions.
> >
> > Also, make a small code style fix in make_memory_node.
> >
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > Acked-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> > Changes in v5:
> > - add acked-by
> >
> > Changes in v4:
> > - pass struct meminfo * to make_memory_node
> > - call make_memory_node twice for dom0, once for normal memory, once for
> >    reserved-memory regions
> > ---
> >   xen/arch/arm/domain_build.c | 22 +++++++++++++++-------
> >   1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index b4260f1fc2..306180d8cb 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -639,11 +639,11 @@ static int __init fdt_property_interrupts(void *fdt, 
> > gic_interrupt_t *intr,
> >   static int __init make_memory_node(const struct domain *d,
> >                                      void *fdt,
> >                                      int addrcells, int sizecells,
> > -                                   const struct kernel_info *kinfo)
> > +                                   struct meminfo *mem)
> >   {
> >       int res, i;
> >       int reg_size = addrcells + sizecells;
> > -    int nr_cells = reg_size*kinfo->mem.nr_banks;
> > +    int nr_cells = reg_size * mem->nr_banks;
> >       __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
> >       __be32 *cells;
> > @@ -662,10 +662,10 @@ static int __init make_memory_node(const struct domain *d,
> >           return res;
> >       cells = &reg[0];
> > -    for ( i = 0 ; i < kinfo->mem.nr_banks; i++ )
> > +    for ( i = 0 ; i < mem->nr_banks; i++ )
> >       {
> > -        u64 start = kinfo->mem.bank[i].start;
> > -        u64 size = kinfo->mem.bank[i].size;
> > +        u64 start = mem->bank[i].start;
> > +        u64 size = mem->bank[i].size;
> >           dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
> >                      i, start, start + size);
> > @@ -1486,10 +1486,18 @@ static int __init handle_node(struct domain *d, struct 
> > kernel_info *kinfo,
> >           if ( res )
> >               return res;
> > -        res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
> > +        res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, 
> > &kinfo->mem);
> >           if ( res )
> >               return res;
> > +        /*
> > +         * Create a second memory node to store the ranges covering
> > +         * reserved-memory regions.
> > +         */
> > +        res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
> > +                               &bootinfo.reserved_mem);
> > +        if ( res )
> > +            return res;
> >       }
> >       res = fdt_end_node(kinfo->fdt);
> > @@ -1745,7 +1753,7 @@ static int __init prepare_dtb_domU(struct domain *d, 
> > struct kernel_info *kinfo)
> >       if ( ret )
> >           goto err;
> > -    ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
> > +    ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem);
> >       if ( ret )
> >           goto err;
> I don't really know whether it is an issue we should worry about, but I noticed 
> that dom0 (Linux 4.14) reported the following:
> OF: Duplicate name in base, renamed to "memory#1"
Path in the device-tree should always be uniq. So it is not possible to have two 
nodes /memory as we are doing today. It looks like Linux workaround it by adding #1.
In this case, we should create name with the following format 
memory@<unit-address>. Per the DeviceTree specification, <unit-address> should 
match the base of address of the first region.
There are an example in make_cpus_node() that can be re-used here.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel