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

[Julien Grall <julien.grall@arm.com>]

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@xilinx.com>
>> Acked-by: Julien Grall <julien.grall@arm.com>
>> ---
>> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel