diff mbox series

[v7,8/8] xen/arm: add reserved-memory regions to the dom0 memory node

Message ID 20190819174338.10466-8-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v7,1/8] xen/arm: pass node to device_tree_for_each_node | expand

Commit Message

Stefano Stabellini Aug. 19, 2019, 5:43 p.m. UTC 
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(-)

Comments

Oleksandr Tyshchenko Aug. 20, 2019, 3:28 p.m. UTC | #1 
On 19.08.19 20:43, Stefano Stabellini wrote:

Hi Stefano

> 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"

----------

When I was trying to retrieve resulting dom0 device tree on a target I 
got an error:

dtc -I fs -O dts -o dom0.dts /sys/firmware/devicetree/base
dom0.dts: ERROR (name_properties): "name" property in /memory#1 is 
incorrect ("memory" instead of base node name)
ERROR: Input tree has errors, aborting (use -f to force output)
Julien Grall Aug. 20, 2019, 5:49 p.m. UTC | #2 
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,
diff mbox series

Patch

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;