Xen devel - Feb 2013 - [PATCH 0/4 V7] xen: arm: parse modules from DT during early boot.

Not much has changed here since v6.

Other than rebasing I''ve used the hypervisor version in
the /hypervisor/compatibility string and added an assert that we aren''t
skipping levels in xen/arch/arm/domain_build.c:write_nodes

Ian.

If a node does not have #*-cells then the parent''s value should be
used. Currently we were asssuming zero which is useless.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v7: assert we don''t skip any levels in the tree
---
 xen/arch/arm/domain_build.c   |    9 +++++++--
 xen/common/device_tree.c      |   12 ++++++++----
 xen/include/xen/device_tree.h |    3 ++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7403f1a..9ce661b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -195,11 +195,16 @@ static int write_nodes(struct domain *d, struct
kernel_info *kinfo,
             continue;
         }
 
+        /* We cannot handle descending more than one level at a time */
+        ASSERT( depth <= last_depth + 1 );
+
         while ( last_depth-- >= depth )
             fdt_end_node(kinfo->fdt);
 
-        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells");
-        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells");
+        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells",
+                                    depth > 0 ? address_cells[depth-1] : 0);
+        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells",
+                                    depth > 0 ? size_cells[depth-1] : 0);
 
         fdt_begin_node(kinfo->fdt, name);
 
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 5301d63..f3d3aa1 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -120,13 +120,14 @@ void device_tree_set_reg(u32 **cell, u32 address_cells,
u32 size_cells,
     set_val(cell, size_cells, size);
 }
 
-u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name)
+u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name,
+                        u32 dflt)
 {
     const struct fdt_property *prop;
 
     prop = fdt_get_property(fdt, node, prop_name, NULL);
     if ( !prop || prop->len < sizeof(u32) )
-        return 0; /* default to 0 */
+        return dflt;
 
     return fdt32_to_cpu(*(uint32_t*)prop->data);
 }
@@ -164,8 +165,11 @@ int device_tree_for_each_node(const void *fdt,
             continue;
         }
 
-        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells");
-        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells");
+        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells",
+                                depth > 0 ? address_cells[depth-1] : 0);
+        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells",
+                                depth > 0 ? size_cells[depth-1] : 0);
+
 
         ret = func(fdt, node, name, depth,
                    address_cells[depth-1], size_cells[depth-1], data);
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 1d04e4f..19bda98 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -66,7 +66,8 @@ void device_tree_get_reg(const u32 **cell, u32 address_cells,
u32 size_cells,
                          u64 *start, u64 *size);
 void device_tree_set_reg(u32 **cell, u32 address_cells, u32 size_cells,
                          u64 start, u64 size);
-u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name);
+u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name,
+			u32 dflt);
 bool_t device_tree_node_matches(const void *fdt, int node, const char *match);
 bool_t device_tree_node_compatible(const void *fdt, int node, const char
*match);
 int find_compatible_node(const char *compatible, int *node, int *depth,
-- 
1.7.9.1

The bootloader should populate /chosen/modules/module@<N>/ for each
module it wishes to pass to the hypervisor. The content of these nodes
is described in docs/misc/arm/device-tree/booting.txt

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
v6: Match based on compatibility instead of path
v5: Moved later in the series
v4: Use /chosen/modules/module@N
    Identify module type by compatible property not number.
v3: Use a reg = < > property for the module address/length.
v2: Reserve the zeroeth module for Xen itself (not used yet)
    Use a more idiomatic DT layout
    Document said layout
---
 docs/misc/arm/device-tree/booting.txt |   25 +++++++++++++++
 xen/common/device_tree.c              |   54 ++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletions(-)
 create mode 100644 docs/misc/arm/device-tree/booting.txt

diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
new file mode 100644
index 0000000..94cd3f1
--- /dev/null
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -0,0 +1,25 @@
+Xen is passed the dom0 kernel and initrd via a reference in the /chosen
+node of the device tree.
+
+Each node has the form /chosen/modules/module@<N> and contains the
following
+properties:
+
+- compatible
+
+	Must be:
+
+		"xen,<type>", "xen,multiboot-module"
+
+	where <type> must be one of:
+
+	- "linux-zimage" -- the dom0 kernel
+	- "linux-initrd" -- the dom0 ramdisk
+
+- reg
+
+	Specifies the physical address of the module in RAM and the
+	length of the module.
+
+- bootargs (optional)
+
+	Command line associated with this module
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index f3d3aa1..c1830a6 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -393,6 +393,48 @@ static void __init process_gic_node(const void *fdt, int
node,
     early_info.gic.gic_vcpu_addr = start;
 }
 
+static void __init process_multiboot_node(const void *fdt, int node,
+                                          const char *name,
+                                          u32 address_cells, u32 size_cells)
+{
+    const struct fdt_property *prop;
+    const u32 *cell;
+    int nr;
+    struct dt_mb_module *mod;
+    int len;
+
+    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") ==
0 )
+        nr = 1;
+    else if ( fdt_node_check_compatible(fdt, node,
"xen,linux-initrd") == 0)
+        nr = 2;
+    else
+        early_panic("%s not a known xen multiboot type\n", name);
+
+    mod = &early_info.modules.module[nr];
+
+    prop = fdt_get_property(fdt, node, "reg", NULL);
+    if ( !prop )
+        early_panic("node %s missing `reg'' property\n",
name);
+
+    cell = (const u32 *)prop->data;
+    device_tree_get_reg(&cell, address_cells, size_cells,
+                        &mod->start, &mod->size);
+
+    prop = fdt_get_property(fdt, node, "bootargs", &len);
+    if ( prop )
+    {
+        if ( len > sizeof(mod->cmdline) )
+            early_panic("module %d command line too long\n", nr);
+
+        safe_strcpy(mod->cmdline, prop->data);
+    }
+    else
+        mod->cmdline[0] = 0;
+
+    if ( nr > early_info.modules.nr_mods )
+        early_info.modules.nr_mods = nr;
+}
+
 static int __init early_scan_node(const void *fdt,
                                   int node, const char *name, int depth,
                                   u32 address_cells, u32 size_cells,
@@ -404,6 +446,8 @@ static int __init early_scan_node(const void *fdt,
         process_cpu_node(fdt, node, name, address_cells, size_cells);
     else if ( device_tree_node_compatible(fdt, node,
"arm,cortex-a15-gic") )
         process_gic_node(fdt, node, name, address_cells, size_cells);
+    else if ( device_tree_node_compatible(fdt, node,
"xen,multiboot-module" ) )
+        process_multiboot_node(fdt, node, name, address_cells, size_cells);
 
     return 0;
 }
@@ -411,12 +455,20 @@ static int __init early_scan_node(const void *fdt,
 static void __init early_print_info(void)
 {
     struct dt_mem_info *mi = &early_info.mem;
+    struct dt_module_info *mods = &early_info.modules;
     int i;
 
     for ( i = 0; i < mi->nr_banks; i++ )
-        early_printk("RAM: %016llx - %016llx\n",
+        early_printk("RAM: %"PRIpaddr" -
%"PRIpaddr"\n",
                      mi->bank[i].start,
                      mi->bank[i].start + mi->bank[i].size - 1);
+    early_printk("\n");
+    for ( i = 1 ; i < mods->nr_mods + 1; i++ )
+        early_printk("MODULE[%d]: %"PRIpaddr" -
%"PRIpaddr" %s\n",
+                     i,
+                     mods->module[i].start,
+                     mods->module[i].start + mods->module[i].size,
+                     mods->module[i].cmdline);
 }
 
 /**
-- 
1.7.9.1

These nodes are used by Xen to find the initial modules.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
v6 - filter based on compatibility node name and not path.
v4 - /chosen/modules/modules@N not /chosen/module@N
v3 - use a helper to filter out DT elements which are not for dom0.
     Better than an ad-hoc break in the middle of a loop.
---
 xen/arch/arm/domain_build.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9ce661b..1c35c82 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -172,6 +172,33 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
     return prop;
 }
 
+/* Returns the next node in fdt (starting from offset) which should be
+ * passed through to dom0.
+ */
+static int fdt_next_dom0_node(const void *fdt, int node,
+                              int *depth_out)
+{
+    int depth = *depth_out;
+
+    while ( (node = fdt_next_node(fdt, node, &depth)) &&
+            node >= 0 && depth >= 0 )
+    {
+        if ( depth >= DEVICE_TREE_MAX_DEPTH )
+            break;
+
+        /* Skip multiboot subnodes */
+        if ( fdt_node_check_compatible(fdt, node,
+                                       "xen,multiboot-module" ) == 0
)
+            continue;
+
+        /* We''ve arrived at a node which dom0 is interested in. */
+        break;
+    }
+
+    *depth_out = depth;
+    return node;
+}
+
 static int write_nodes(struct domain *d, struct kernel_info *kinfo,
                        const void *fdt)
 {
@@ -183,7 +210,7 @@ static int write_nodes(struct domain *d, struct kernel_info
*kinfo,
 
     for ( node = 0, depth = 0;
           node >= 0 && depth >= 0;
-          node = fdt_next_node(fdt, node, &depth) )
+          node = fdt_next_dom0_node(fdt, node, &depth) )
     {
         const char *name;
 
@@ -191,7 +218,8 @@ static int write_nodes(struct domain *d, struct kernel_info
*kinfo,
 
         if ( depth >= DEVICE_TREE_MAX_DEPTH )
         {
-            printk("warning: node `%s'' is nested too
deep\n", name);
+            printk("warning: node `%s'' is nested too deep
(%d)\n",
+                   name, depth);
             continue;
         }
 
-- 
1.7.9.1

I initially added hypervisor-new and confirmed via /proc/device-model
that the content is the same before changing it to drop and replace
an existing node.

NB: There is an ambiguity in the compatibility property.
linux/arch/arm/boot/dts/xenvm-4.2.dts says "xen,xen-4.2" while
Documentation/devicetree/bindings/arm/xen.txt says "xen,xen-4.3".I
have used
the actual hypervisor version as discussed in
http://marc.info/?l=xen-devel&m=135963416631423

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
v7: Use Xen version in compatibility node.
---
 xen/arch/arm/domain_build.c |   57 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1c35c82..6efc36a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1,5 +1,6 @@
 #include <xen/config.h>
 #include <xen/init.h>
+#include <xen/compile.h>
 #include <xen/lib.h>
 #include <xen/mm.h>
 #include <xen/domain_page.h>
@@ -34,7 +35,7 @@ custom_param("dom0_mem", parse_dom0_mem);
  * are added (yet) but one terminating reserve map entry (16 bytes) is
  * added.
  */
-#define DOM0_FDT_EXTRA_SIZE (sizeof(struct fdt_reserve_entry))
+#define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry))
 
 struct vcpu *__init alloc_dom0_vcpu0(void)
 {
@@ -186,6 +187,13 @@ static int fdt_next_dom0_node(const void *fdt, int node,
         if ( depth >= DEVICE_TREE_MAX_DEPTH )
             break;
 
+        /* Skip /hypervisor/ node. We will inject our own. */
+        if ( fdt_node_check_compatible(fdt, node, "xen,xen" ) == 0 )
+        {
+            printk("Device-tree contains \"xen,xen\" node.
Ignoring.\n");
+            continue;
+        }
+
         /* Skip multiboot subnodes */
         if ( fdt_node_check_compatible(fdt, node,
                                        "xen,multiboot-module" ) == 0
)
@@ -199,6 +207,48 @@ static int fdt_next_dom0_node(const void *fdt, int node,
     return node;
 }
 
+static void make_hypervisor_node(void *fdt, int addrcells, int sizecells)
+{
+    const char compat[] +       
"xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
+        "xen,xen";
+    u32 reg[4];
+    u32 intr[3];
+    u32 *cell;
+
+    /*
+     * Sanity-check address sizes, since addresses and sizes which do
+     * not take up exactly 4 or 8 bytes are not supported.
+     */
+    if ((addrcells != 1 && addrcells != 2) ||
+        (sizecells != 1 && sizecells != 2))
+        panic("Cannot cope with this size");
+
+    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
+    fdt_begin_node(fdt, "hypervisor");
+
+    /* Cannot use fdt_property_string due to embedded nulls */
+    fdt_property(fdt, "compatible", compat, sizeof(compat) + 1);
+
+    /* reg 0 is grant table space */
+    cell = &reg[0];
+    device_tree_set_reg(&cell, addrcells, sizecells, 0xb0000000, 0x20000);
+    fdt_property(fdt, "reg", reg,
+                 sizeof(reg[0]) * (addrcells + sizecells));
+
+    /*
+     * interrupts is evtchn upcall  <1 15 0xf08>
+     * See linux Documentation/devicetree/bindings/arm/gic.txt
+     */
+    intr[0] = cpu_to_fdt32(1); /* is a PPI */
+    intr[1] = cpu_to_fdt32(VGIC_IRQ_EVTCHN_CALLBACK - 16); /* PPIs start at 16
*/
+    intr[2] = cpu_to_fdt32(0xf08); /* Active-low level-sensitive */
+
+    fdt_property(fdt, "interrupts", intr, sizeof(intr[0]) * 3);
+
+    fdt_end_node(fdt);
+}
+
 static int write_nodes(struct domain *d, struct kernel_info *kinfo,
                        const void *fdt)
 {
@@ -244,9 +294,12 @@ static int write_nodes(struct domain *d, struct kernel_info
*kinfo,
         last_depth = depth;
     }
 
-    while ( last_depth-- >= 0 )
+    while ( last_depth-- >= 1 )
         fdt_end_node(kinfo->fdt);
 
+    make_hypervisor_node(kinfo->fdt, address_cells[0], size_cells[0]);
+
+    fdt_end_node(kinfo->fdt);
     return 0;
 }
 
-- 
1.7.9.1

On Mon, 18 Feb 2013, Ian Campbell wrote:
> If a node does not have #*-cells then the parent''s value should be
> used. Currently we were asssuming zero which is useless.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> v7: assert we don''t skip any levels in the tree
> ---
>  xen/arch/arm/domain_build.c   |    9 +++++++--
>  xen/common/device_tree.c      |   12 ++++++++----
>  xen/include/xen/device_tree.h |    3 ++-
>  3 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7403f1a..9ce661b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -195,11 +195,16 @@ static int write_nodes(struct domain *d, struct
kernel_info *kinfo,
>              continue;
>          }
>  
> +        /* We cannot handle descending more than one level at a time */
> +        ASSERT( depth <= last_depth + 1 );
> +
>          while ( last_depth-- >= depth )
>              fdt_end_node(kinfo->fdt);
>  
> -        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells");
> -        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells");
> +        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells",
> +                                    depth > 0 ? address_cells[depth-1]
: 0);
> +        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells",
> +                                    depth > 0 ? size_cells[depth-1] :
0);
>  
>          fdt_begin_node(kinfo->fdt, name);
>  
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 5301d63..f3d3aa1 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -120,13 +120,14 @@ void device_tree_set_reg(u32 **cell, u32
address_cells, u32 size_cells,
>      set_val(cell, size_cells, size);
>  }
>  
> -u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name)
> +u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name,
> +                        u32 dflt)
>  {
>      const struct fdt_property *prop;
>  
>      prop = fdt_get_property(fdt, node, prop_name, NULL);
>      if ( !prop || prop->len < sizeof(u32) )
> -        return 0; /* default to 0 */
> +        return dflt;
>  
>      return fdt32_to_cpu(*(uint32_t*)prop->data);
>  }
> @@ -164,8 +165,11 @@ int device_tree_for_each_node(const void *fdt,
>              continue;
>          }
>  
> -        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells");
> -        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells");
> +        address_cells[depth] = device_tree_get_u32(fdt, node,
"#address-cells",
> +                                depth > 0 ? address_cells[depth-1] :
0);
> +        size_cells[depth] = device_tree_get_u32(fdt, node,
"#size-cells",
> +                                depth > 0 ? size_cells[depth-1] : 0);
> +
>  
>          ret = func(fdt, node, name, depth,
>                     address_cells[depth-1], size_cells[depth-1], data);
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 1d04e4f..19bda98 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -66,7 +66,8 @@ void device_tree_get_reg(const u32 **cell, u32
address_cells, u32 size_cells,
>                           u64 *start, u64 *size);
>  void device_tree_set_reg(u32 **cell, u32 address_cells, u32 size_cells,
>                           u64 start, u64 size);
> -u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name);
> +u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name,
> +			u32 dflt);
>  bool_t device_tree_node_matches(const void *fdt, int node, const char
*match);
>  bool_t device_tree_node_compatible(const void *fdt, int node, const char
*match);
>  int find_compatible_node(const char *compatible, int *node, int *depth,
> -- 
> 1.7.9.1
>

On Mon, 2013-02-18 at 15:20 +0000, Ian Campbell wrote:
> Not much has changed here since v6.
> 
> Other than rebasing I''ve used the hypervisor version in
> the /hypervisor/compatibility string and added an assert that we
aren''t
> skipping levels in xen/arch/arm/domain_build.c:write_nodes
Applied with Stefano''s final Ack.

Ian.