[PATCH V4 02/10] xen/arm: introduce new helper device_tree_get_meminfo

Penny Zheng posted 10 patches 4 years, 6 months ago
There is a newer version of this series
[PATCH V4 02/10] xen/arm: introduce new helper device_tree_get_meminfo
Posted by Penny Zheng 4 years, 6 months ago
A few functions iterate over the device tree property to get memory info,
like "reg" or "xen,static-mem", so this commit creates a new helper
device_tree_get_meminfo to extract the
common codes.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
 xen/arch/arm/bootfdt.c | 104 +++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 62 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index d2714446e1..04210684c9 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -63,6 +63,44 @@ void __init device_tree_get_reg(const __be32 **cell, u32 address_cells,
     *size = dt_next_cell(size_cells, cell);
 }
 
+static int __init device_tree_get_meminfo(const void *fdt, int node,
+                                          const char *prop_name,
+                                          u32 address_cells, u32 size_cells,
+                                          void *data)
+{
+    const struct fdt_property *prop;
+    unsigned int i, banks;
+    const __be32 *cell;
+    u32 reg_cells = address_cells + size_cells;
+    paddr_t start, size;
+    struct meminfo *mem = data;
+
+    prop = fdt_get_property(fdt, node, prop_name, NULL);
+    if ( !prop )
+        return -ENOENT;
+
+    cell = (const __be32 *)prop->data;
+    banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
+
+    for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
+    {
+        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
+        /* Some DT may describe empty bank, ignore them */
+        if ( !size )
+            continue;
+        mem->bank[mem->nr_banks].start = start;
+        mem->bank[mem->nr_banks].size = size;
+        mem->nr_banks++;
+    }
+
+    if ( i < banks )
+    {
+        printk("Warning: Max number of supported memory regions reached.\n");
+        return -ENOSPC;
+    }
+    return 0;
+}
+
 u32 __init device_tree_get_u32(const void *fdt, int node,
                                const char *prop_name, u32 dflt)
 {
@@ -139,14 +177,6 @@ static int __init process_memory_node(const void *fdt, int node,
                                       u32 address_cells, u32 size_cells,
                                       void *data)
 {
-    const struct fdt_property *prop;
-    int i;
-    int banks;
-    const __be32 *cell;
-    paddr_t start, size;
-    u32 reg_cells = address_cells + size_cells;
-    struct meminfo *mem = data;
-
     if ( address_cells < 1 || size_cells < 1 )
     {
         printk("fdt: node `%s': invalid #address-cells or #size-cells",
@@ -154,27 +184,7 @@ static int __init process_memory_node(const void *fdt, int node,
         return -EINVAL;
     }
 
-    prop = fdt_get_property(fdt, node, "reg", NULL);
-    if ( !prop )
-        return -ENOENT;
-
-    cell = (const __be32 *)prop->data;
-    banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
-
-    for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
-    {
-        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
-        /* Some DT may describe empty bank, ignore them */
-        if ( !size )
-            continue;
-        mem->bank[mem->nr_banks].start = start;
-        mem->bank[mem->nr_banks].size = size;
-        mem->nr_banks++;
-    }
-
-    if ( i < banks )
-        return -ENOSPC;
-    return 0;
+    return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, data);
 }
 
 static int __init process_reserved_memory_node(const void *fdt, int node,
@@ -195,13 +205,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node,
 
 static int __init process_static_memory(const void *fdt, int node, void *data)
 {
-    int i = 0, banks;
-    const __be32 *cell;
-    paddr_t start, size;
-    u32 address_cells, size_cells, reg_cells;
-    struct meminfo *mem = data;
-    const struct fdt_property *prop;
-
+    u32 address_cells, size_cells;
 
     address_cells = device_tree_get_u32(fdt, node,
                                         "#xen,static-mem-address-cells", 0);
@@ -213,33 +217,9 @@ static int __init process_static_memory(const void *fdt, int node, void *data)
                  "\"#xen,static-mem-address-cell\".\n");
          return -EINVAL;
     }
-    reg_cells = address_cells + size_cells;
-
-    prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
-    /*
-     * Static memory shall belong to a specific domain, that is,
-     * its node `domUx` has compatible string "xen,domain".
-     */
-    if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
-    {
-        printk("xen,static-mem property can only be located under /domUx node.\n");
-        return -EINVAL;
-    }
-
-    cell = (const __be32 *)prop->data;
-    banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
-
-    for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
-    {
-        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
-        mem->bank[mem->nr_banks].start = start;
-        mem->bank[mem->nr_banks].size = size;
-        mem->nr_banks++;
-    }
 
-    if ( i < banks )
-        return -ENOSPC;
-    return 0;
+    return device_tree_get_meminfo(fdt, node, "xen,static-mem", address_cells,
+                                   size_cells, data);
 }
 
 static int __init process_reserved_memory(const void *fdt, int node,
-- 
2.25.1


Re: [PATCH V4 02/10] xen/arm: introduce new helper device_tree_get_meminfo
Posted by Julien Grall 4 years, 6 months ago
Hi Penny,

On 28/07/2021 11:27, Penny Zheng wrote:
> A few functions iterate over the device tree property to get memory info,
> like "reg" or "xen,static-mem", so this commit creates a new helper
> device_tree_get_meminfo to extract the
> common codes.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Ah, this is where you did the consolidation. Sorry, I didn't notice this 
patch.

In general, we are avoiding to introduce code and then rework it in the 
same series. Instead, the rework is done first and then the function is 
used.

So can you move this patch first?

Cheers,

-- 
Julien Grall

RE: [PATCH V4 02/10] xen/arm: introduce new helper device_tree_get_meminfo
Posted by Penny Zheng 4 years, 5 months ago
Hi Julien

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Wednesday, August 11, 2021 9:35 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> <Wei.Chen@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH V4 02/10] xen/arm: introduce new helper
> device_tree_get_meminfo
> 
> Hi Penny,
> 
> On 28/07/2021 11:27, Penny Zheng wrote:
> > A few functions iterate over the device tree property to get memory
> > info, like "reg" or "xen,static-mem", so this commit creates a new
> > helper device_tree_get_meminfo to extract the common codes.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> 
> Ah, this is where you did the consolidation. Sorry, I didn't notice this patch.
> 
> In general, we are avoiding to introduce code and then rework it in the same
> series. Instead, the rework is done first and then the function is used.
> 
> So can you move this patch first?
> 

Thx for the explanation. I'll reorganize. ;)

> Cheers,
> 
> --

Cheers,

Penny


> Julien Grall