All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support
@ 2014-06-26 16:45 Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 1/9] xen: arm: implement generic multiboot compatibility strings Ian Campbell
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel, Stefano Stabellini, Julien Grall, Tim Deegan
  Cc: Roy Franz, Naresh Bhat, Fu Wei

The following refactors the Xen early device tree stuff (i.e. which
walks the flattened tree directly) away from the regular device tree
stuff (i.e. the stuff which for the most part deals with the unflattened
tree). It also makes some changes to the Xen side multiboot support
which I think will make it easier to work with, both internally and for
e.g. bootloader integration.

Impact on UEFI/ACPI: Mostly I think the refactoring may be useful when
integrating the UEFI memory map and ACPI stuff (which wants early FDT,
but not unflatening etc) in to Xen.

Impact on multiboot: This could potentially simplify things on the grub
side by removing the need to guess default types for the modules in the
common case.

In the future I think it would be good to implement more probing on the
Xen side, e.g. to discover the XSM policy (similar to how it works on
x86 -- which walks all the modules looking for the policy magic nr).

The first two patches here have been posted before.

For changes since v2 see the individual patches changelog. I've dropped
"xen: arm: Drop device_tree_node_compatible" this time around (to much
yakk shaving involved).

Ian.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/9] xen: arm: implement generic multiboot compatibility strings
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 2/9] xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated Ian Campbell
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

This causes Xen to accept the more generic names specified in
http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot as of
2014-06-06.

These names are more generic than those proposed by Andre in
http://thread.gmane.org/gmane.linux.linaro.announce.boot/326 and those
used in earlier drafts of the /Multiboot wiki page.

This will allow bootloaders to not special case Xen (or at least to reduce
the amount which is required).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/common/device_tree.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index f0b17a3..48ae849 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -338,9 +338,11 @@ static void __init process_multiboot_node(const void *fdt, int node,
     struct dt_mb_module *mod;
     int len;
 
-    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
+    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
+         fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
         nr = MOD_KERNEL;
-    else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
+    else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
+              fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
         nr = MOD_INITRD;
     else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
         nr = MOD_XSM;
@@ -433,7 +435,8 @@ static int __init early_scan_node(const void *fdt,
 {
     if ( device_tree_node_matches(fdt, node, "memory") )
         process_memory_node(fdt, node, name, address_cells, size_cells);
-    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) )
+    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
+              device_tree_node_compatible(fdt, node, "multiboot,module" ))
         process_multiboot_node(fdt, node, name, address_cells, size_cells);
     else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
         process_chosen_node(fdt, node, name, address_cells, size_cells);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/9] xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 1/9] xen: arm: implement generic multiboot compatibility strings Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 3/9] xen: arm: prefer typesafe max()/min() over MAX()/MIN() Ian Campbell
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

When using a multiboot capable bootloader this is exactly the field which
should be used.

Replace the deprecation wording with a reference to the information on the
priority of the bootargs fields.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 docs/misc/arm/device-tree/booting.txt |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index a7fb52b..bfb8d01 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -25,8 +25,9 @@ Each node contains the following properties:
 
 - bootargs (optional)
 
-	Command line associated with this module. This is deprecated and should
-	be replaced by the bootargs variations described below.
+	Command line associated with this module. See below for the
+	priority of this field vs. other mechanisms of specifying the
+	bootargs for the kernel.
 
 
 Command lines
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/9] xen: arm: prefer typesafe max()/min() over MAX()/MIN()
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 1/9] xen: arm: implement generic multiboot compatibility strings Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 2/9] xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 4/9] xen: arm: rename early_info structs Ian Campbell
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/p2m.c       |    4 ++--
 xen/common/device_tree.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 9960e17..dd2c5a4 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -500,8 +500,8 @@ static int apply_p2m_changes(struct domain *d,
         unsigned long sgfn = paddr_to_pfn(start_gpaddr);
         unsigned long egfn = paddr_to_pfn(end_gpaddr);
 
-        p2m->max_mapped_gfn = MAX(p2m->max_mapped_gfn, egfn);
-        p2m->lowest_mapped_gfn = MIN(p2m->lowest_mapped_gfn, sgfn);
+        p2m->max_mapped_gfn = max(p2m->max_mapped_gfn, egfn);
+        p2m->lowest_mapped_gfn = min(p2m->lowest_mapped_gfn, sgfn);
     }
 
     rc = 0;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 48ae849..03d495a 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -425,7 +425,7 @@ static void __init process_chosen_node(const void *fdt, int node,
     mod->start = start;
     mod->size = end - start;
 
-    early_info.modules.nr_mods = MAX(MOD_INITRD, early_info.modules.nr_mods);
+    early_info.modules.nr_mods = max(MOD_INITRD, early_info.modules.nr_mods);
 }
 
 static int __init early_scan_node(const void *fdt,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/9] xen: arm: rename early_info structs
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (2 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 3/9] xen: arm: prefer typesafe max()/min() over MAX()/MIN() Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 5/9] xen: arm: move boot time fdt parsing into separate file Ian Campbell
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

There isn't really anything Device Tree specific about the early_info, we just
happen to get it from device tree (but in the future it might come e.g. from
UEFI or ACPI or something else).

Move the relevant structs out of device_tree.h and into asm/setup.h and rename to
be more neutral.

For now the code to parse the DT into the now arch specific structs remains in
common code.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c   |   13 ++++++-----
 xen/arch/arm/kernel.c         |    6 ++---
 xen/arch/arm/kernel.h         |    3 ++-
 xen/arch/arm/setup.c          |   51 ++++++++++++++++++++++-------------------
 xen/common/device_tree.c      |   38 +++++++++++++++---------------
 xen/include/asm-arm/setup.h   |   40 ++++++++++++++++++++++++++++++++
 xen/include/xen/device_tree.h |   39 -------------------------------
 7 files changed, 98 insertions(+), 92 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9d9cba9..5eef8a3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -16,6 +16,7 @@
 #include <asm/setup.h>
 #include <asm/platform.h>
 #include <asm/psci.h>
+#include <asm/setup.h>
 
 #include <asm/gic.h>
 #include <xen/irq.h>
@@ -160,9 +161,9 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
     int res = 0;
     int had_dom0_bootargs = 0;
 
-    if ( early_info.modules.nr_mods >= MOD_KERNEL &&
-         early_info.modules.module[MOD_KERNEL].cmdline[0] )
-        bootargs = &early_info.modules.module[MOD_KERNEL].cmdline[0];
+    if ( bootinfo.modules.nr_mods >= MOD_KERNEL &&
+         bootinfo.modules.module[MOD_KERNEL].cmdline[0] )
+        bootargs = &bootinfo.modules.module[MOD_KERNEL].cmdline[0];
 
     dt_for_each_property_node (node, prop)
     {
@@ -221,7 +222,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
          * If the bootloader provides an initrd, we must create a placeholder
          * for the initrd properties. The values will be replaced later.
          */
-        if ( early_info.modules.module[MOD_INITRD].size )
+        if ( bootinfo.modules.module[MOD_INITRD].size )
         {
             u64 a = 0;
             res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
@@ -976,8 +977,8 @@ static void dtb_load(struct kernel_info *kinfo)
 static void initrd_load(struct kernel_info *kinfo)
 {
     paddr_t load_addr = kinfo->initrd_paddr;
-    paddr_t paddr = early_info.modules.module[MOD_INITRD].start;
-    paddr_t len = early_info.modules.module[MOD_INITRD].size;
+    paddr_t paddr = bootinfo.modules.module[MOD_INITRD].start;
+    paddr_t len = bootinfo.modules.module[MOD_INITRD].size;
     unsigned long offs;
     int node;
     int res;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 69182ec..ce5b95a 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -69,7 +69,7 @@ static void place_modules(struct kernel_info *info,
 {
     /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */
     const paddr_t initrd_len =
-        ROUNDUP(early_info.modules.module[MOD_INITRD].size, MB(2));
+        ROUNDUP(bootinfo.modules.module[MOD_INITRD].size, MB(2));
     const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2));
     const paddr_t modsize = initrd_len + dtb_len;
 
@@ -376,8 +376,8 @@ int kernel_probe(struct kernel_info *info)
 
     paddr_t start, size;
 
-    start = early_info.modules.module[MOD_KERNEL].start;
-    size = early_info.modules.module[MOD_KERNEL].size;
+    start = bootinfo.modules.module[MOD_KERNEL].start;
+    size = bootinfo.modules.module[MOD_KERNEL].size;
 
     if ( !size )
     {
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index fd2f61d..7c7f624 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -8,6 +8,7 @@
 
 #include <xen/libelf.h>
 #include <xen/device_tree.h>
+#include <asm/setup.h>
 
 struct kernel_info {
 #ifdef CONFIG_ARM_64
@@ -16,7 +17,7 @@ struct kernel_info {
 
     void *fdt; /* flat device tree */
     paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
-    struct dt_mem_info mem;
+    struct meminfo mem;
 
     /* kernel entry point */
     paddr_t entry;
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index b9ce7a9..63f6b8e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -43,8 +43,11 @@
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
 #include <asm/procinfo.h>
+#include <asm/setup.h>
 #include <xsm/xsm.h>
 
+struct bootinfo __initdata bootinfo;
+
 struct cpuinfo_arm __read_mostly boot_cpu_data;
 
 static __used void init_done(void)
@@ -182,7 +185,7 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
 
 void __init discard_initial_modules(void)
 {
-    struct dt_module_info *mi = &early_info.modules;
+    struct bootmodules *mi = &bootinfo.modules;
     int i;
 
     for ( i = MOD_DISCARD_FIRST; i <= mi->nr_mods; i++ )
@@ -210,7 +213,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
                                        uint32_t size, paddr_t align,
                                        int first_mod)
 {
-    const struct dt_module_info *mi = &early_info.modules;
+    const struct bootmodules *mi = &bootinfo.modules;
     int i;
     int nr_rsvd;
 
@@ -275,7 +278,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e,
  */
 static paddr_t __init next_module(paddr_t s, paddr_t *end)
 {
-    struct dt_module_info *mi = &early_info.modules;
+    struct bootmodules *mi = &bootinfo.modules;
     paddr_t lowest = ~(paddr_t)0;
     int i;
 
@@ -308,7 +311,7 @@ static paddr_t __init next_module(paddr_t s, paddr_t *end)
  */
 static paddr_t __init get_xen_paddr(void)
 {
-    struct dt_mem_info *mi = &early_info.mem;
+    struct meminfo *mi = &bootinfo.mem;
     paddr_t min_size;
     paddr_t paddr = 0, last_end;
     int i;
@@ -357,8 +360,8 @@ static paddr_t __init get_xen_paddr(void)
     printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
            paddr, paddr + min_size);
 
-    early_info.modules.module[MOD_XEN].start = paddr;
-    early_info.modules.module[MOD_XEN].size = min_size;
+    bootinfo.modules.module[MOD_XEN].start = paddr;
+    bootinfo.modules.module[MOD_XEN].size = min_size;
 
     return paddr;
 }
@@ -376,7 +379,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     int i;
     void *fdt;
 
-    if ( !early_info.mem.nr_banks )
+    if ( !bootinfo.mem.nr_banks )
         panic("No memory bank");
 
     /*
@@ -393,15 +396,15 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
      * We also track the number of actual RAM pages (i.e. not counting
      * the holes).
      */
-    ram_size  = early_info.mem.bank[0].size;
+    ram_size  = bootinfo.mem.bank[0].size;
 
-    contig_start = ram_start = early_info.mem.bank[0].start;
+    contig_start = ram_start = bootinfo.mem.bank[0].start;
     contig_end   = ram_end = ram_start + ram_size;
 
-    for ( i = 1; i < early_info.mem.nr_banks; i++ )
+    for ( i = 1; i < bootinfo.mem.nr_banks; i++ )
     {
-        paddr_t bank_start = early_info.mem.bank[i].start;
-        paddr_t bank_size = early_info.mem.bank[i].size;
+        paddr_t bank_start = bootinfo.mem.bank[i].start;
+        paddr_t bank_size = bootinfo.mem.bank[i].size;
         paddr_t bank_end = bank_start + bank_size;
 
         paddr_t new_ram_size = ram_size + bank_size;
@@ -434,11 +437,11 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
         ram_end = new_ram_end;
     }
 
-    if ( i != early_info.mem.nr_banks )
+    if ( i != bootinfo.mem.nr_banks )
     {
         printk("WARNING: only using %d out of %d memory banks\n",
-               i, early_info.mem.nr_banks);
-        early_info.mem.nr_banks = i;
+               i, bootinfo.mem.nr_banks);
+        bootinfo.mem.nr_banks = i;
     }
 
     total_pages = ram_pages = ram_size >> PAGE_SHIFT;
@@ -497,10 +500,10 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     device_tree_flattened = fdt;
 
     /* Add non-xenheap memory */
-    for ( i = 0; i < early_info.mem.nr_banks; i++ )
+    for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
     {
-        paddr_t bank_start = early_info.mem.bank[i].start;
-        paddr_t bank_end = bank_start + early_info.mem.bank[i].size;
+        paddr_t bank_start = bootinfo.mem.bank[i].start;
+        paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
 
         s = bank_start;
         while ( s < bank_end )
@@ -557,10 +560,10 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     void *fdt;
 
     total_pages = 0;
-    for ( bank = 0 ; bank < early_info.mem.nr_banks; bank++ )
+    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
     {
-        paddr_t bank_start = early_info.mem.bank[bank].start;
-        paddr_t bank_size = early_info.mem.bank[bank].size;
+        paddr_t bank_start = bootinfo.mem.bank[bank].start;
+        paddr_t bank_size = bootinfo.mem.bank[bank].size;
         paddr_t bank_end = bank_start + bank_size;
         paddr_t s, e;
 
@@ -609,11 +612,11 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
         }
     }
 
-    if ( bank != early_info.mem.nr_banks )
+    if ( bank != bootinfo.mem.nr_banks )
     {
         printk("WARNING: only using %d out of %d memory banks\n",
-               bank, early_info.mem.nr_banks);
-        early_info.mem.nr_banks = bank;
+               bank, bootinfo.mem.nr_banks);
+        bootinfo.mem.nr_banks = bank;
     }
 
     total_pages += ram_size >> PAGE_SHIFT;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 03d495a..a1896d3 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -23,8 +23,8 @@
 #include <xen/cpumask.h>
 #include <xen/ctype.h>
 #include <xen/lib.h>
+#include <asm/setup.h>
 
-struct dt_early_info __initdata early_info;
 const void *device_tree_flattened;
 dt_irq_xlate_func dt_irq_xlate;
 /* Host device tree */
@@ -238,10 +238,10 @@ const char *device_tree_bootargs(const void *fdt)
     prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
     if ( prop == NULL )
     {
-        struct dt_mb_module *dom0_mod = NULL;
+        struct bootmodule *dom0_mod = NULL;
 
-        if ( early_info.modules.nr_mods >= MOD_KERNEL )
-            dom0_mod = &early_info.modules.module[MOD_KERNEL];
+        if ( bootinfo.modules.nr_mods >= MOD_KERNEL )
+            dom0_mod = &bootinfo.modules.module[MOD_KERNEL];
 
         if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
             ( dom0_mod && dom0_mod->cmdline[0] ) )
@@ -319,12 +319,12 @@ static void __init process_memory_node(const void *fdt, int node,
     cell = (const __be32 *)prop->data;
     banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
 
-    for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ )
+    for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ )
     {
         device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
-        early_info.mem.bank[early_info.mem.nr_banks].start = start;
-        early_info.mem.bank[early_info.mem.nr_banks].size = size;
-        early_info.mem.nr_banks++;
+        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
+        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
+        bootinfo.mem.nr_banks++;
     }
 }
 
@@ -335,7 +335,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
     const struct fdt_property *prop;
     const __be32 *cell;
     int nr;
-    struct dt_mb_module *mod;
+    struct bootmodule *mod;
     int len;
 
     if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
@@ -349,7 +349,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
     else
         panic("%s not a known xen multiboot type\n", name);
 
-    mod = &early_info.modules.module[nr];
+    mod = &bootinfo.modules.module[nr];
 
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
@@ -374,8 +374,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
     else
         mod->cmdline[0] = 0;
 
-    if ( nr > early_info.modules.nr_mods )
-        early_info.modules.nr_mods = nr;
+    if ( nr > bootinfo.modules.nr_mods )
+        bootinfo.modules.nr_mods = nr;
 }
 
 static void __init process_chosen_node(const void *fdt, int node,
@@ -383,7 +383,7 @@ static void __init process_chosen_node(const void *fdt, int node,
                                        u32 address_cells, u32 size_cells)
 {
     const struct fdt_property *prop;
-    struct dt_mb_module *mod = &early_info.modules.module[MOD_INITRD];
+    struct bootmodule *mod = &bootinfo.modules.module[MOD_INITRD];
     paddr_t start, end;
     int len;
 
@@ -425,7 +425,7 @@ static void __init process_chosen_node(const void *fdt, int node,
     mod->start = start;
     mod->size = end - start;
 
-    early_info.modules.nr_mods = max(MOD_INITRD, early_info.modules.nr_mods);
+    bootinfo.modules.nr_mods = max(MOD_INITRD, bootinfo.modules.nr_mods);
 }
 
 static int __init early_scan_node(const void *fdt,
@@ -446,8 +446,8 @@ 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;
+    struct meminfo *mi = &bootinfo.mem;
+    struct bootmodules *mods = &bootinfo.modules;
     int i, nr_rsvd;
 
     for ( i = 0; i < mi->nr_banks; i++ )
@@ -483,18 +483,18 @@ static void __init early_print_info(void)
  */
 size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
 {
-    struct dt_mb_module *mod;
+    struct bootmodule *mod;
     int ret;
 
     ret = fdt_check_header(fdt);
     if ( ret < 0 )
         panic("No valid device tree\n");
 
-    mod = &early_info.modules.module[MOD_FDT];
+    mod = &bootinfo.modules.module[MOD_FDT];
     mod->start = paddr;
     mod->size = fdt_totalsize(fdt);
 
-    early_info.modules.nr_mods = max(MOD_FDT, early_info.modules.nr_mods);
+    bootinfo.modules.nr_mods = max(MOD_FDT, bootinfo.modules.nr_mods);
 
     device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
     early_print_info();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index b09f688..ea0dc46 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -3,6 +3,46 @@
 
 #include <public/version.h>
 
+#define NR_MEM_BANKS 8
+
+#define MOD_XEN    0
+#define MOD_FDT    1
+#define MOD_KERNEL 2
+#define MOD_INITRD 3
+#define MOD_XSM    4
+#define NR_MODULES 5
+
+#define MOD_DISCARD_FIRST MOD_FDT
+
+struct membank {
+    paddr_t start;
+    paddr_t size;
+};
+
+struct meminfo {
+    int nr_banks;
+    struct membank bank[NR_MEM_BANKS];
+};
+
+struct bootmodule {
+    paddr_t start;
+    paddr_t size;
+    char cmdline[1024];
+};
+
+struct bootmodules {
+    int nr_mods;
+    /* Module 0 is Xen itself, followed by the provided modules-proper */
+    struct bootmodule module[NR_MODULES];
+};
+
+struct bootinfo {
+    struct meminfo mem;
+    struct bootmodules modules;
+};
+
+extern struct bootinfo bootinfo;
+
 void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 25db076..74e98f5 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -20,44 +20,6 @@
 
 #define DEVICE_TREE_MAX_DEPTH 16
 
-#define NR_MEM_BANKS 8
-
-#define MOD_XEN    0
-#define MOD_FDT    1
-#define MOD_KERNEL 2
-#define MOD_INITRD 3
-#define MOD_XSM    4
-#define NR_MODULES 5
-
-#define MOD_DISCARD_FIRST MOD_FDT
-
-struct membank {
-    paddr_t start;
-    paddr_t size;
-};
-
-struct dt_mem_info {
-    int nr_banks;
-    struct membank bank[NR_MEM_BANKS];
-};
-
-struct dt_mb_module {
-    paddr_t start;
-    paddr_t size;
-    char cmdline[1024];
-};
-
-struct dt_module_info {
-    int nr_mods;
-    /* Module 0 is Xen itself, followed by the provided modules-proper */
-    struct dt_mb_module module[NR_MODULES];
-};
-
-struct dt_early_info {
-    struct dt_mem_info mem;
-    struct dt_module_info modules;
-};
-
 /*
  * Struct used for matching a device
  */
@@ -193,7 +155,6 @@ typedef int (*device_tree_node_func)(const void *fdt,
                                      u32 address_cells, u32 size_cells,
                                      void *data);
 
-extern struct dt_early_info early_info;
 extern const void *device_tree_flattened;
 
 size_t __init device_tree_early_init(const void *fdt, paddr_t paddr);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/9] xen: arm: move boot time fdt parsing into separate file.
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (3 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 4/9] xen: arm: rename early_info structs Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 6/9] xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline Ian Campbell
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Move the early code for walking the flattened device tree out of device_tree.c.
The intention is that eventually only the proper (i.e.  unflattened) device
tree support will live in device_tree.c.

The new home is bootfdt.c to try and better reflect the purpose of the code.
Although in theory this early code could be generic in reality it is pretty ARM
specific, so place it under xen/arch/arm until a second user wants it.

As part of the move rename device_tree_early_init to boot_fdt_info. Drop
device_tree_dump, it is unused.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/Makefile         |    1 +
 xen/arch/arm/bootfdt.c        |  343 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/setup.c          |    2 +-
 xen/common/device_tree.c      |  356 -----------------------------------------
 xen/include/asm-arm/setup.h   |    2 +
 xen/include/xen/device_tree.h |    3 -
 6 files changed, 347 insertions(+), 360 deletions(-)
 create mode 100644 xen/arch/arm/bootfdt.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 63e0460..58a6714 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -21,6 +21,7 @@ obj-y += guestcopy.o
 obj-y += physdev.o
 obj-y += platform.o
 obj-y += setup.o
+obj-y += bootfdt.o
 obj-y += time.o
 obj-y += smpboot.o
 obj-y += smp.o
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
new file mode 100644
index 0000000..11182cd
--- /dev/null
+++ b/xen/arch/arm/bootfdt.c
@@ -0,0 +1,343 @@
+/*
+ * Early Device Tree
+ *
+ * Copyright (C) 2012-2014 Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/kernel.h>
+#include <xen/init.h>
+#include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
+#include <asm/setup.h>
+
+static bool_t __init device_tree_node_matches(const void *fdt, int node,
+                                              const char *match)
+{
+    const char *name;
+    size_t match_len;
+
+    name = fdt_get_name(fdt, node, NULL);
+    match_len = strlen(match);
+
+    /* Match both "match" and "match@..." patterns but not
+       "match-foo". */
+    return strncmp(name, match, match_len) == 0
+        && (name[match_len] == '@' || name[match_len] == '\0');
+}
+
+static bool_t __init device_tree_node_compatible(const void *fdt, int node,
+                                                 const char *match)
+{
+    int len, l;
+    int mlen;
+    const void *prop;
+
+    mlen = strlen(match);
+
+    prop = fdt_getprop(fdt, node, "compatible", &len);
+    if ( prop == NULL )
+        return 0;
+
+    while ( len > 0 ) {
+        if ( !dt_compat_cmp(prop, match) )
+            return 1;
+        l = strlen(prop) + 1;
+        prop += l;
+        len -= l;
+    }
+
+    return 0;
+}
+
+static void __init device_tree_get_reg(const __be32 **cell, u32 address_cells,
+                                       u32 size_cells, u64 *start, u64 *size)
+{
+    *start = dt_next_cell(address_cells, cell);
+    *size = dt_next_cell(size_cells, cell);
+}
+
+static u32 __init 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 dflt;
+
+    return fdt32_to_cpu(*(uint32_t*)prop->data);
+}
+
+/**
+ * device_tree_for_each_node - iterate over all device tree nodes
+ * @fdt: flat device tree.
+ * @func: function to call for each node.
+ * @data: data to pass to @func.
+ *
+ * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
+ *
+ * Returns 0 if all nodes were iterated over successfully.  If @func
+ * returns a value different from 0, that value is returned immediately.
+ */
+static int __init device_tree_for_each_node(const void *fdt,
+                                            device_tree_node_func func,
+                                            void *data)
+{
+    int node;
+    int depth;
+    u32 address_cells[DEVICE_TREE_MAX_DEPTH];
+    u32 size_cells[DEVICE_TREE_MAX_DEPTH];
+    int ret;
+
+    for ( node = 0, depth = 0;
+          node >=0 && depth >= 0;
+          node = fdt_next_node(fdt, node, &depth) )
+    {
+        const char *name = fdt_get_name(fdt, node, NULL);
+
+        if ( depth >= DEVICE_TREE_MAX_DEPTH )
+        {
+            printk("Warning: device tree node `%s' is nested too deep\n",
+                   name);
+            continue;
+        }
+
+        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);
+        if ( ret != 0 )
+            return ret;
+    }
+    return 0;
+}
+
+static void __init process_memory_node(const void *fdt, int node,
+                                       const char *name,
+                                       u32 address_cells, u32 size_cells)
+{
+    const struct fdt_property *prop;
+    int i;
+    int banks;
+    const __be32 *cell;
+    paddr_t start, size;
+    u32 reg_cells = address_cells + size_cells;
+
+    if ( address_cells < 1 || size_cells < 1 )
+    {
+        printk("fdt: node `%s': invalid #address-cells or #size-cells",
+               name);
+        return;
+    }
+
+    prop = fdt_get_property(fdt, node, "reg", NULL);
+    if ( !prop )
+    {
+        printk("fdt: node `%s': missing `reg' property\n", name);
+        return;
+    }
+
+    cell = (const __be32 *)prop->data;
+    banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
+
+    for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ )
+    {
+        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
+        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
+        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
+        bootinfo.mem.nr_banks++;
+    }
+}
+
+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 __be32 *cell;
+    int nr;
+    struct bootmodule *mod;
+    int len;
+
+    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
+         fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
+        nr = MOD_KERNEL;
+    else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
+              fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
+        nr = MOD_INITRD;
+    else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
+        nr = MOD_XSM;
+    else
+        panic("%s not a known xen multiboot type\n", name);
+
+    mod = &bootinfo.modules.module[nr];
+
+    prop = fdt_get_property(fdt, node, "reg", &len);
+    if ( !prop )
+        panic("node %s missing `reg' property\n", name);
+
+    if ( len < dt_cells_to_size(address_cells + size_cells) )
+        panic("fdt: node `%s': `reg` property length is too short\n",
+                    name);
+
+    cell = (const __be32 *)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) )
+            panic("module %d command line too long\n", nr);
+
+        safe_strcpy(mod->cmdline, prop->data);
+    }
+    else
+        mod->cmdline[0] = 0;
+
+    if ( nr > bootinfo.modules.nr_mods )
+        bootinfo.modules.nr_mods = nr;
+}
+
+static void __init process_chosen_node(const void *fdt, int node,
+                                       const char *name,
+                                       u32 address_cells, u32 size_cells)
+{
+    const struct fdt_property *prop;
+    struct bootmodule *mod = &bootinfo.modules.module[MOD_INITRD];
+    paddr_t start, end;
+    int len;
+
+    printk("Checking for initrd in /chosen\n");
+
+    prop = fdt_get_property(fdt, node, "linux,initrd-start", &len);
+    if ( !prop )
+        /* No initrd present. */
+        return;
+    if ( len != sizeof(u32) && len != sizeof(u64) )
+    {
+        printk("linux,initrd-start property has invalid length %d\n", len);
+        return;
+    }
+    start = dt_read_number((void *)&prop->data, dt_size_to_cells(len));
+
+    prop = fdt_get_property(fdt, node, "linux,initrd-end", &len);
+    if ( !prop )
+    {
+        printk("linux,initrd-end not present but -start was\n");
+        return;
+    }
+    if ( len != sizeof(u32) && len != sizeof(u64) )
+    {
+        printk("linux,initrd-end property has invalid length %d\n", len);
+        return;
+    }
+    end = dt_read_number((void *)&prop->data, dt_size_to_cells(len));
+
+    if ( start >= end )
+    {
+        printk("linux,initrd limits invalid: %"PRIpaddr" >= %"PRIpaddr"\n",
+                  start, end);
+        return;
+    }
+
+    printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
+
+    mod->start = start;
+    mod->size = end - start;
+
+    bootinfo.modules.nr_mods = max(MOD_INITRD, bootinfo.modules.nr_mods);
+}
+
+static int __init early_scan_node(const void *fdt,
+                                  int node, const char *name, int depth,
+                                  u32 address_cells, u32 size_cells,
+                                  void *data)
+{
+    if ( device_tree_node_matches(fdt, node, "memory") )
+        process_memory_node(fdt, node, name, address_cells, size_cells);
+    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
+              device_tree_node_compatible(fdt, node, "multiboot,module" ))
+        process_multiboot_node(fdt, node, name, address_cells, size_cells);
+    else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
+        process_chosen_node(fdt, node, name, address_cells, size_cells);
+
+    return 0;
+}
+
+static void __init early_print_info(void)
+{
+    struct meminfo *mi = &bootinfo.mem;
+    struct bootmodules *mods = &bootinfo.modules;
+    int i, nr_rsvd;
+
+    for ( i = 0; i < mi->nr_banks; i++ )
+        printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n",
+                     mi->bank[i].start,
+                     mi->bank[i].start + mi->bank[i].size - 1);
+    printk("\n");
+    for ( i = 1 ; i < mods->nr_mods + 1; i++ )
+        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
+                     i,
+                     mods->module[i].start,
+                     mods->module[i].start + mods->module[i].size,
+                     mods->module[i].cmdline);
+    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
+    for ( i = 0; i < nr_rsvd; i++ )
+    {
+        paddr_t s, e;
+        if ( fdt_get_mem_rsv(device_tree_flattened, i, &s, &e) < 0 )
+            continue;
+        /* fdt_get_mem_rsv returns length */
+        e += s;
+        printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n",
+                     i, s, e);
+    }
+    printk("\n");
+}
+
+/**
+ * boot_fdt_info - initialize bootinfo from a DTB
+ * @fdt: flattened device tree binary
+ *
+ * Returns the size of the DTB.
+ */
+size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
+{
+    struct bootmodule *mod;
+    int ret;
+
+    ret = fdt_check_header(fdt);
+    if ( ret < 0 )
+        panic("No valid device tree\n");
+
+    mod = &bootinfo.modules.module[MOD_FDT];
+    mod->start = paddr;
+    mod->size = fdt_totalsize(fdt);
+
+    bootinfo.modules.nr_mods = max(MOD_FDT, bootinfo.modules.nr_mods);
+
+    device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
+    early_print_info();
+
+    return fdt_totalsize(fdt);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 63f6b8e..4a84a32 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -678,7 +678,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     /* This is mapped by head.S */
     device_tree_flattened = (void *)BOOT_FDT_VIRT_START
         + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
-    fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
+    fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
 
     cmdline = device_tree_bootargs(device_tree_flattened);
     printk("Command line: %s\n", cmdline);
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index a1896d3..89de269 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -22,7 +22,6 @@
 #include <xen/string.h>
 #include <xen/cpumask.h>
 #include <xen/ctype.h>
-#include <xen/lib.h>
 #include <asm/setup.h>
 
 const void *device_tree_flattened;
@@ -89,52 +88,6 @@ struct dt_bus
     unsigned int (*get_flags)(const __be32 *addr);
 };
 
-static bool_t __init device_tree_node_matches(const void *fdt, int node,
-                                              const char *match)
-{
-    const char *name;
-    size_t match_len;
-
-    name = fdt_get_name(fdt, node, NULL);
-    match_len = strlen(match);
-
-    /* Match both "match" and "match@..." patterns but not
-       "match-foo". */
-    return strncmp(name, match, match_len) == 0
-        && (name[match_len] == '@' || name[match_len] == '\0');
-}
-
-static bool_t __init device_tree_node_compatible(const void *fdt, int node,
-                                                 const char *match)
-{
-    int len, l;
-    int mlen;
-    const void *prop;
-
-    mlen = strlen(match);
-
-    prop = fdt_getprop(fdt, node, "compatible", &len);
-    if ( prop == NULL )
-        return 0;
-
-    while ( len > 0 ) {
-        if ( !dt_compat_cmp(prop, match) )
-            return 1;
-        l = strlen(prop) + 1;
-        prop += l;
-        len -= l;
-    }
-
-    return 0;
-}
-
-static void __init device_tree_get_reg(const __be32 **cell, u32 address_cells,
-                                       u32 size_cells, u64 *start, u64 *size)
-{
-    *start = dt_next_cell(address_cells, cell);
-    *size = dt_next_cell(size_cells, cell);
-}
-
 void dt_get_range(const __be32 **cell, const struct dt_device_node *np,
                   u64 *address, u64 *size)
 {
@@ -162,66 +115,6 @@ void dt_set_range(__be32 **cellp, const struct dt_device_node *np,
     dt_set_cell(cellp, dt_n_size_cells(np), size);
 }
 
-static u32 __init 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 dflt;
-
-    return fdt32_to_cpu(*(uint32_t*)prop->data);
-}
-
-/**
- * device_tree_for_each_node - iterate over all device tree nodes
- * @fdt: flat device tree.
- * @func: function to call for each node.
- * @data: data to pass to @func.
- *
- * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
- *
- * Returns 0 if all nodes were iterated over successfully.  If @func
- * returns a value different from 0, that value is returned immediately.
- */
-static int __init device_tree_for_each_node(const void *fdt,
-                                            device_tree_node_func func,
-                                            void *data)
-{
-    int node;
-    int depth;
-    u32 address_cells[DEVICE_TREE_MAX_DEPTH];
-    u32 size_cells[DEVICE_TREE_MAX_DEPTH];
-    int ret;
-
-    for ( node = 0, depth = 0;
-          node >=0 && depth >= 0;
-          node = fdt_next_node(fdt, node, &depth) )
-    {
-        const char *name = fdt_get_name(fdt, node, NULL);
-
-        if ( depth >= DEVICE_TREE_MAX_DEPTH )
-        {
-            printk("Warning: device tree node `%s' is nested too deep\n",
-                   name);
-            continue;
-        }
-
-        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);
-        if ( ret != 0 )
-            return ret;
-    }
-    return 0;
-}
-
 /**
  * device_tree_bootargs - return the bootargs (the Xen command line)
  * @fdt flat device tree.
@@ -253,255 +146,6 @@ const char *device_tree_bootargs(const void *fdt)
     return prop->data;
 }
 
-static int dump_node(const void *fdt, int node, const char *name, int depth,
-                     u32 address_cells, u32 size_cells, void *data)
-{
-    char prefix[2*DEVICE_TREE_MAX_DEPTH + 1] = "";
-    int i;
-    int prop;
-
-    for ( i = 0; i < depth; i++ )
-        safe_strcat(prefix, "  ");
-
-    if ( name[0] == '\0' )
-        name = "/";
-    printk("%s%s:\n", prefix, name);
-
-    for ( prop = fdt_first_property_offset(fdt, node);
-          prop >= 0;
-          prop = fdt_next_property_offset(fdt, prop) )
-    {
-        const struct fdt_property *p;
-
-        p = fdt_get_property_by_offset(fdt, prop, NULL);
-
-        printk("%s  %s\n", prefix, fdt_string(fdt, fdt32_to_cpu(p->nameoff)));
-    }
-
-    return 0;
-}
-
-/**
- * device_tree_dump - print a text representation of a device tree
- * @fdt: flat device tree to print
- */
-void __init device_tree_dump(const void *fdt)
-{
-    device_tree_for_each_node(fdt, dump_node, NULL);
-}
-
-
-static void __init process_memory_node(const void *fdt, int node,
-                                       const char *name,
-                                       u32 address_cells, u32 size_cells)
-{
-    const struct fdt_property *prop;
-    int i;
-    int banks;
-    const __be32 *cell;
-    paddr_t start, size;
-    u32 reg_cells = address_cells + size_cells;
-
-    if ( address_cells < 1 || size_cells < 1 )
-    {
-        printk("fdt: node `%s': invalid #address-cells or #size-cells",
-               name);
-        return;
-    }
-
-    prop = fdt_get_property(fdt, node, "reg", NULL);
-    if ( !prop )
-    {
-        printk("fdt: node `%s': missing `reg' property\n", name);
-        return;
-    }
-
-    cell = (const __be32 *)prop->data;
-    banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
-
-    for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ )
-    {
-        device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
-        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
-        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
-        bootinfo.mem.nr_banks++;
-    }
-}
-
-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 __be32 *cell;
-    int nr;
-    struct bootmodule *mod;
-    int len;
-
-    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
-         fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
-        nr = MOD_KERNEL;
-    else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
-              fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
-        nr = MOD_INITRD;
-    else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
-        nr = MOD_XSM;
-    else
-        panic("%s not a known xen multiboot type\n", name);
-
-    mod = &bootinfo.modules.module[nr];
-
-    prop = fdt_get_property(fdt, node, "reg", &len);
-    if ( !prop )
-        panic("node %s missing `reg' property\n", name);
-
-    if ( len < dt_cells_to_size(address_cells + size_cells) )
-        panic("fdt: node `%s': `reg` property length is too short\n",
-                    name);
-
-    cell = (const __be32 *)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) )
-            panic("module %d command line too long\n", nr);
-
-        safe_strcpy(mod->cmdline, prop->data);
-    }
-    else
-        mod->cmdline[0] = 0;
-
-    if ( nr > bootinfo.modules.nr_mods )
-        bootinfo.modules.nr_mods = nr;
-}
-
-static void __init process_chosen_node(const void *fdt, int node,
-                                       const char *name,
-                                       u32 address_cells, u32 size_cells)
-{
-    const struct fdt_property *prop;
-    struct bootmodule *mod = &bootinfo.modules.module[MOD_INITRD];
-    paddr_t start, end;
-    int len;
-
-    printk("Checking for initrd in /chosen\n");
-
-    prop = fdt_get_property(fdt, node, "linux,initrd-start", &len);
-    if ( !prop )
-        /* No initrd present. */
-        return;
-    if ( len != sizeof(u32) && len != sizeof(u64) )
-    {
-        printk("linux,initrd-start property has invalid length %d\n", len);
-        return;
-    }
-    start = dt_read_number((void *)&prop->data, dt_size_to_cells(len));
-
-    prop = fdt_get_property(fdt, node, "linux,initrd-end", &len);
-    if ( !prop )
-    {
-        printk("linux,initrd-end not present but -start was\n");
-        return;
-    }
-    if ( len != sizeof(u32) && len != sizeof(u64) )
-    {
-        printk("linux,initrd-end property has invalid length %d\n", len);
-        return;
-    }
-    end = dt_read_number((void *)&prop->data, dt_size_to_cells(len));
-
-    if ( start >= end )
-    {
-        printk("linux,initrd limits invalid: %"PRIpaddr" >= %"PRIpaddr"\n",
-                  start, end);
-        return;
-    }
-
-    printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
-
-    mod->start = start;
-    mod->size = end - start;
-
-    bootinfo.modules.nr_mods = max(MOD_INITRD, bootinfo.modules.nr_mods);
-}
-
-static int __init early_scan_node(const void *fdt,
-                                  int node, const char *name, int depth,
-                                  u32 address_cells, u32 size_cells,
-                                  void *data)
-{
-    if ( device_tree_node_matches(fdt, node, "memory") )
-        process_memory_node(fdt, node, name, address_cells, size_cells);
-    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
-              device_tree_node_compatible(fdt, node, "multiboot,module" ))
-        process_multiboot_node(fdt, node, name, address_cells, size_cells);
-    else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
-        process_chosen_node(fdt, node, name, address_cells, size_cells);
-
-    return 0;
-}
-
-static void __init early_print_info(void)
-{
-    struct meminfo *mi = &bootinfo.mem;
-    struct bootmodules *mods = &bootinfo.modules;
-    int i, nr_rsvd;
-
-    for ( i = 0; i < mi->nr_banks; i++ )
-        printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n",
-                     mi->bank[i].start,
-                     mi->bank[i].start + mi->bank[i].size - 1);
-    printk("\n");
-    for ( i = 1 ; i < mods->nr_mods + 1; i++ )
-        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
-                     i,
-                     mods->module[i].start,
-                     mods->module[i].start + mods->module[i].size,
-                     mods->module[i].cmdline);
-    nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
-    for ( i = 0; i < nr_rsvd; i++ )
-    {
-        paddr_t s, e;
-        if ( fdt_get_mem_rsv(device_tree_flattened, i, &s, &e) < 0 )
-            continue;
-        /* fdt_get_mem_rsv returns length */
-        e += s;
-        printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n",
-                     i, s, e);
-    }
-    printk("\n");
-}
-
-/**
- * device_tree_early_init - initialize early info from a DTB
- * @fdt: flattened device tree binary
- *
- * Returns the size of the DTB.
- */
-size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
-{
-    struct bootmodule *mod;
-    int ret;
-
-    ret = fdt_check_header(fdt);
-    if ( ret < 0 )
-        panic("No valid device tree\n");
-
-    mod = &bootinfo.modules.module[MOD_FDT];
-    mod->start = paddr;
-    mod->size = fdt_totalsize(fdt);
-
-    bootinfo.modules.nr_mods = max(MOD_FDT, bootinfo.modules.nr_mods);
-
-    device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
-    early_print_info();
-
-    return fdt_totalsize(fdt);
-}
-
 static void __init *unflatten_dt_alloc(unsigned long *mem, unsigned long size,
                                        unsigned long align)
 {
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index ea0dc46..21dbcd4 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -53,6 +53,8 @@ int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
 
+size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
+
 #endif
 /*
  * Local variables:
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 74e98f5..0edec85 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -157,10 +157,7 @@ typedef int (*device_tree_node_func)(const void *fdt,
 
 extern const void *device_tree_flattened;
 
-size_t __init device_tree_early_init(const void *fdt, paddr_t paddr);
-
 const char __init *device_tree_bootargs(const void *fdt);
-void __init device_tree_dump(const void *fdt);
 
 /**
  * dt_unflatten_host_device_tree - Unflatten the host device tree
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/9] xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (4 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 5/9] xen: arm: move boot time fdt parsing into separate file Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index Ian Campbell
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/bootfdt.c        |   27 +++++++++++++++++++++++++++
 xen/arch/arm/setup.c          |    2 +-
 xen/common/device_tree.c      |   31 -------------------------------
 xen/include/asm-arm/setup.h   |    1 +
 xen/include/xen/device_tree.h |    2 --
 5 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 11182cd..e48a64b 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -333,6 +333,33 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
     return fdt_totalsize(fdt);
 }
 
+const char *boot_fdt_cmdline(const void *fdt)
+{
+    int node;
+    const struct fdt_property *prop;
+
+    node = fdt_path_offset(fdt, "/chosen");
+    if ( node < 0 )
+        return NULL;
+
+    prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
+    if ( prop == NULL )
+    {
+        struct bootmodule *dom0_mod = NULL;
+
+        if ( bootinfo.modules.nr_mods >= MOD_KERNEL )
+            dom0_mod = &bootinfo.modules.module[MOD_KERNEL];
+
+        if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
+            ( dom0_mod && dom0_mod->cmdline[0] ) )
+            prop = fdt_get_property(fdt, node, "bootargs", NULL);
+    }
+    if ( prop == NULL )
+        return NULL;
+
+    return prop->data;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 4a84a32..f1ae408 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -680,7 +680,7 @@ void __init start_xen(unsigned long boot_phys_offset,
         + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
     fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
 
-    cmdline = device_tree_bootargs(device_tree_flattened);
+    cmdline = boot_fdt_cmdline(device_tree_flattened);
     printk("Command line: %s\n", cmdline);
     cmdline_parse(cmdline);
 
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 89de269..b9deb62 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -115,37 +115,6 @@ void dt_set_range(__be32 **cellp, const struct dt_device_node *np,
     dt_set_cell(cellp, dt_n_size_cells(np), size);
 }
 
-/**
- * device_tree_bootargs - return the bootargs (the Xen command line)
- * @fdt flat device tree.
- */
-const char *device_tree_bootargs(const void *fdt)
-{
-    int node;
-    const struct fdt_property *prop;
-
-    node = fdt_path_offset(fdt, "/chosen");
-    if ( node < 0 )
-        return NULL;
-
-    prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
-    if ( prop == NULL )
-    {
-        struct bootmodule *dom0_mod = NULL;
-
-        if ( bootinfo.modules.nr_mods >= MOD_KERNEL )
-            dom0_mod = &bootinfo.modules.module[MOD_KERNEL];
-
-        if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
-            ( dom0_mod && dom0_mod->cmdline[0] ) )
-            prop = fdt_get_property(fdt, node, "bootargs", NULL);
-    }
-    if ( prop == NULL )
-        return NULL;
-
-    return prop->data;
-}
-
 static void __init *unflatten_dt_alloc(unsigned long *mem, unsigned long size,
                                        unsigned long align)
 {
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 21dbcd4..85aa866 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -54,6 +54,7 @@ int construct_dom0(struct domain *d);
 void discard_initial_modules(void);
 
 size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
+const char __init *boot_fdt_cmdline(const void *fdt);
 
 #endif
 /*
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 0edec85..b486fc6 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -157,8 +157,6 @@ typedef int (*device_tree_node_func)(const void *fdt,
 
 extern const void *device_tree_flattened;
 
-const char __init *device_tree_bootargs(const void *fdt);
-
 /**
  * dt_unflatten_host_device_tree - Unflatten the host device tree
  *
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (5 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 6/9] xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-27 10:43   ` Julien Grall
  2014-06-26 16:45 ` [PATCH v2 8/9] xen: arm: support bootmodule type detection by ordering Ian Campbell
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

This is more natural and better matches how multiboot is actually supposed to
work.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix XSM
    s/bootmodulekind/bootmodule_kind/
    Print which module we aren't adding
    Only (and explictly) preserve Xen in discard_initial_modules
---
 xen/arch/arm/bootfdt.c      |   49 +++++++++++++++----------------------------
 xen/arch/arm/domain_build.c |   20 +++++++++++-------
 xen/arch/arm/kernel.c       |   15 ++++++-------
 xen/arch/arm/setup.c        |   47 ++++++++++++++++++++++++++++++++++++-----
 xen/include/asm-arm/setup.h |   27 ++++++++++++++++--------
 xen/xsm/xsm_policy.c        |   19 ++++++++++++++---
 6 files changed, 114 insertions(+), 63 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index e48a64b..b42a789 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -165,23 +165,22 @@ static void __init process_multiboot_node(const void *fdt, int node,
 {
     const struct fdt_property *prop;
     const __be32 *cell;
-    int nr;
-    struct bootmodule *mod;
+    bootmodule_kind kind;
+    paddr_t start, size;
+    const char *cmdline;
     int len;
 
     if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 ||
          fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
-        nr = MOD_KERNEL;
+        kind = BOOTMOD_KERNEL;
     else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 ||
               fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
-        nr = MOD_INITRD;
+        kind = BOOTMOD_RAMDISK;
     else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
-        nr = MOD_XSM;
+        kind = BOOTMOD_XSM;
     else
         panic("%s not a known xen multiboot type\n", name);
 
-    mod = &bootinfo.modules.module[nr];
-
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
         panic("node %s missing `reg' property\n", name);
@@ -191,22 +190,19 @@ static void __init process_multiboot_node(const void *fdt, int node,
                     name);
 
     cell = (const __be32 *)prop->data;
-    device_tree_get_reg(&cell, address_cells, size_cells,
-                        &mod->start, &mod->size);
+    device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
 
     prop = fdt_get_property(fdt, node, "bootargs", &len);
     if ( prop )
     {
-        if ( len > sizeof(mod->cmdline) )
-            panic("module %d command line too long\n", nr);
-
-        safe_strcpy(mod->cmdline, prop->data);
+        if ( len > BOOTMOD_MAX_CMDLINE )
+            panic("module %s command line too long\n", name);
+        cmdline = prop->data;
     }
     else
-        mod->cmdline[0] = 0;
+        cmdline = NULL;
 
-    if ( nr > bootinfo.modules.nr_mods )
-        bootinfo.modules.nr_mods = nr;
+    add_boot_module(kind, start, size, cmdline);
 }
 
 static void __init process_chosen_node(const void *fdt, int node,
@@ -214,7 +210,6 @@ static void __init process_chosen_node(const void *fdt, int node,
                                        u32 address_cells, u32 size_cells)
 {
     const struct fdt_property *prop;
-    struct bootmodule *mod = &bootinfo.modules.module[MOD_INITRD];
     paddr_t start, end;
     int len;
 
@@ -253,10 +248,7 @@ static void __init process_chosen_node(const void *fdt, int node,
 
     printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
 
-    mod->start = start;
-    mod->size = end - start;
-
-    bootinfo.modules.nr_mods = max(MOD_INITRD, bootinfo.modules.nr_mods);
+    add_boot_module(BOOTMOD_RAMDISK, start, end-start, NULL);
 }
 
 static int __init early_scan_node(const void *fdt,
@@ -286,7 +278,7 @@ static void __init early_print_info(void)
                      mi->bank[i].start,
                      mi->bank[i].start + mi->bank[i].size - 1);
     printk("\n");
-    for ( i = 1 ; i < mods->nr_mods + 1; i++ )
+    for ( i = 1 ; i < mods->nr_mods; i++ )
         printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
                      i,
                      mods->module[i].start,
@@ -314,18 +306,13 @@ static void __init early_print_info(void)
  */
 size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
 {
-    struct bootmodule *mod;
     int ret;
 
     ret = fdt_check_header(fdt);
     if ( ret < 0 )
         panic("No valid device tree\n");
 
-    mod = &bootinfo.modules.module[MOD_FDT];
-    mod->start = paddr;
-    mod->size = fdt_totalsize(fdt);
-
-    bootinfo.modules.nr_mods = max(MOD_FDT, bootinfo.modules.nr_mods);
+    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), NULL);
 
     device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
     early_print_info();
@@ -345,10 +332,8 @@ const char *boot_fdt_cmdline(const void *fdt)
     prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
     if ( prop == NULL )
     {
-        struct bootmodule *dom0_mod = NULL;
-
-        if ( bootinfo.modules.nr_mods >= MOD_KERNEL )
-            dom0_mod = &bootinfo.modules.module[MOD_KERNEL];
+        struct bootmodule *dom0_mod =
+            boot_module_find_by_kind(BOOTMOD_KERNEL);
 
         if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
             ( dom0_mod && dom0_mod->cmdline[0] ) )
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5eef8a3..c1a54e5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -161,9 +161,10 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
     int res = 0;
     int had_dom0_bootargs = 0;
 
-    if ( bootinfo.modules.nr_mods >= MOD_KERNEL &&
-         bootinfo.modules.module[MOD_KERNEL].cmdline[0] )
-        bootargs = &bootinfo.modules.module[MOD_KERNEL].cmdline[0];
+    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
+
+    if ( mod && mod->cmdline[0] )
+        bootargs = &mod->cmdline[0];
 
     dt_for_each_property_node (node, prop)
     {
@@ -210,6 +211,8 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
 
     if ( dt_node_path_is_equal(node, "/chosen") )
     {
+        struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+
         if ( bootargs )
         {
             res = fdt_property(kinfo->fdt, "bootargs", bootargs,
@@ -222,7 +225,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
          * If the bootloader provides an initrd, we must create a placeholder
          * for the initrd properties. The values will be replaced later.
          */
-        if ( bootinfo.modules.module[MOD_INITRD].size )
+        if ( mod && mod->size )
         {
             u64 a = 0;
             res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
@@ -976,18 +979,21 @@ static void dtb_load(struct kernel_info *kinfo)
 
 static void initrd_load(struct kernel_info *kinfo)
 {
+    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_RAMDISK);
     paddr_t load_addr = kinfo->initrd_paddr;
-    paddr_t paddr = bootinfo.modules.module[MOD_INITRD].start;
-    paddr_t len = bootinfo.modules.module[MOD_INITRD].size;
+    paddr_t paddr, len;
     unsigned long offs;
     int node;
     int res;
     __be32 val[2];
     __be32 *cellp;
 
-    if ( !len )
+    if ( !mod || !mod->size )
         return;
 
+    paddr = mod->start;
+    len = mod->size;
+
     printk("Loading dom0 initrd from %"PRIpaddr" to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
            paddr, load_addr, load_addr + len);
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index ce5b95a..230ff8f 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -68,8 +68,8 @@ static void place_modules(struct kernel_info *info,
                           paddr_t kernbase, paddr_t kernend)
 {
     /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */
-    const paddr_t initrd_len =
-        ROUNDUP(bootinfo.modules.module[MOD_INITRD].size, MB(2));
+    const struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0, MB(2));
     const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2));
     const paddr_t modsize = initrd_len + dtb_len;
 
@@ -372,20 +372,21 @@ err:
 
 int kernel_probe(struct kernel_info *info)
 {
+    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
     int rc;
 
     paddr_t start, size;
 
-    start = bootinfo.modules.module[MOD_KERNEL].start;
-    size = bootinfo.modules.module[MOD_KERNEL].size;
-
-    if ( !size )
+    if ( !mod || !mod->size )
     {
         printk(XENLOG_ERR "Missing kernel boot module?\n");
         return -ENOENT;
     }
 
-    printk("Loading kernel from boot module %d\n", MOD_KERNEL);
+    start = mod->start;
+    size = mod->size;
+
+    printk("Loading kernel from boot module @ %"PRIpaddr"\n", start);
 
 #ifdef CONFIG_ARM_64
     rc = kernel_zimage64_probe(info, start, size);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index f1ae408..96ed13b 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -183,17 +183,56 @@ static void dt_unreserved_regions(paddr_t s, paddr_t e,
     cb(s, e);
 }
 
+void add_boot_module(bootmodule_kind kind, paddr_t start, paddr_t size,
+                     const char *cmdline)
+{
+    struct bootmodules *mods = &bootinfo.modules;
+    struct bootmodule *mod;
+
+    if ( mods->nr_mods == MAX_MODULES )
+    {
+        printk("Ignoring %s boot module at %"PRIpaddr"-%"PRIpaddr" (too many)\n",
+               boot_module_kind_as_string(kind), start, start + size);
+        return;
+    }
+
+    mod = &mods->module[mods->nr_mods++];
+    mod->kind = kind;
+    mod->start = start;
+    mod->size = size;
+    if ( cmdline )
+        safe_strcpy(mod->cmdline, cmdline);
+    else
+        mod->cmdline[0] = 0;
+
+}
+
+struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
+{
+    struct bootmodules *mods = &bootinfo.modules;
+    struct bootmodule *mod;
+    int i;
+    for (i = 0 ; i < mods->nr_mods ; i++ )
+    {
+        mod = &mods->module[i];
+        if ( mod->kind == kind )
+            return mod;
+    }
+    return NULL;
+}
+
 void __init discard_initial_modules(void)
 {
     struct bootmodules *mi = &bootinfo.modules;
     int i;
 
-    for ( i = MOD_DISCARD_FIRST; i <= mi->nr_mods; i++ )
+    for ( i = 0; i <= mi->nr_mods; i++ )
     {
         paddr_t s = mi->module[i].start;
         paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
 
-        dt_unreserved_regions(s, e, init_domheap_pages, 0);
+        if ( mi->module[i].kind != BOOTMOD_XEN )
+            dt_unreserved_regions(s, e, init_domheap_pages, 0);
     }
 
     mi->nr_mods = 0;
@@ -360,9 +399,7 @@ static paddr_t __init get_xen_paddr(void)
     printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
            paddr, paddr + min_size);
 
-    bootinfo.modules.module[MOD_XEN].start = paddr;
-    bootinfo.modules.module[MOD_XEN].size = min_size;
-
+    add_boot_module(BOOTMOD_XEN, paddr, min_size, NULL);
     return paddr;
 }
 
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 85aa866..fe4997b 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -5,14 +5,17 @@
 
 #define NR_MEM_BANKS 8
 
-#define MOD_XEN    0
-#define MOD_FDT    1
-#define MOD_KERNEL 2
-#define MOD_INITRD 3
-#define MOD_XSM    4
-#define NR_MODULES 5
+#define MAX_MODULES 5 /* Current maximum useful modules */
+
+typedef enum {
+    BOOTMOD_XEN,
+    BOOTMOD_FDT,
+    BOOTMOD_KERNEL,
+    BOOTMOD_RAMDISK,
+    BOOTMOD_XSM,
+    BOOTMOD_UNKNOWN
+}  bootmodule_kind;
 
-#define MOD_DISCARD_FIRST MOD_FDT
 
 struct membank {
     paddr_t start;
@@ -24,16 +27,18 @@ struct meminfo {
     struct membank bank[NR_MEM_BANKS];
 };
 
+#define BOOTMOD_MAX_CMDLINE 1024
 struct bootmodule {
+    bootmodule_kind kind;
     paddr_t start;
     paddr_t size;
-    char cmdline[1024];
+    char cmdline[BOOTMOD_MAX_CMDLINE];
 };
 
 struct bootmodules {
     int nr_mods;
     /* Module 0 is Xen itself, followed by the provided modules-proper */
-    struct bootmodule module[NR_MODULES];
+    struct bootmodule module[MAX_MODULES];
 };
 
 struct bootinfo {
@@ -56,6 +61,10 @@ void discard_initial_modules(void);
 size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
 const char __init *boot_fdt_cmdline(const void *fdt);
 
+void add_boot_module(bootmodule_kind kind, paddr_t start, paddr_t size,
+                     const char *cmdline);
+struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+
 #endif
 /*
  * Local variables:
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index a0dee09..6e0bb78 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -77,13 +77,16 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
 #ifdef HAS_DEVICE_TREE
 int __init xsm_dt_policy_init(void)
 {
-    paddr_t paddr = early_info.modules.module[MOD_XSM].start;
-    paddr_t len = early_info.modules.module[MOD_XSM].size;
+    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_XSM);
+    paddr_t paddr, len;
     xsm_magic_t magic;
 
-    if ( !len )
+    if ( !mod || !mod->size )
         return 0;
 
+    paddr = mod->start;
+    len = mod->size;
+
     copy_from_paddr(&magic, paddr, sizeof(magic));
 
     if ( magic != XSM_MAGIC )
@@ -106,3 +109,13 @@ int __init xsm_dt_policy_init(void)
     return 0;
 }
 #endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/9] xen: arm: support bootmodule type detection by ordering
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (6 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:45 ` [PATCH v2 9/9] xen: arm: update multiboot device tree bindings Ian Campbell
  2014-06-26 16:46 ` [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Assign modules types based on the order in which they are defined in the FDT.
This is supported only for the dom0 kernel and ramdisk when given as the first
and second modules respectively, similar to how
http://wiki.xen.org/wiki?title=Xen_ARM_with_Virtualization_Extensions/Multiboot&oldid=11824
defined the default types from the bootloader side.

This is compatible with how Xen interprets the modules with x86 multiboot and I
think simplifies things for bootloaders which now need not contain similar
guessing code if they only care about the most basic case.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Don't rely on enum ordering for guessing
---
 xen/arch/arm/bootfdt.c      |   19 ++++++++++++++++---
 xen/arch/arm/setup.c        |   14 ++++++++++++++
 xen/include/asm-arm/setup.h |    1 +
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index b42a789..7cbf0b0 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -163,6 +163,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
                                           const char *name,
                                           u32 address_cells, u32 size_cells)
 {
+    static int kind_guess = 0;
     const struct fdt_property *prop;
     const __be32 *cell;
     bootmodule_kind kind;
@@ -179,7 +180,18 @@ static void __init process_multiboot_node(const void *fdt, int node,
     else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
         kind = BOOTMOD_XSM;
     else
-        panic("%s not a known xen multiboot type\n", name);
+        kind = BOOTMOD_UNKNOWN;
+
+    /* Guess that first two unknown are kernel and ramdisk respectively. */
+    if ( kind == BOOTMOD_UNKNOWN )
+    {
+        switch ( kind_guess++ )
+        {
+        case 0: kind = BOOTMOD_KERNEL; break;
+        case 1: kind = BOOTMOD_RAMDISK; break;
+        default: break;
+        }
+    }
 
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
@@ -278,11 +290,12 @@ static void __init early_print_info(void)
                      mi->bank[i].start,
                      mi->bank[i].start + mi->bank[i].size - 1);
     printk("\n");
-    for ( i = 1 ; i < mods->nr_mods; i++ )
-        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
+    for ( i = 0 ; i < mods->nr_mods; i++ )
+        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-8s %s\n",
                      i,
                      mods->module[i].start,
                      mods->module[i].start + mods->module[i].size,
+                     boot_module_kind_as_string(mods->module[i].kind),
                      mods->module[i].cmdline);
     nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
     for ( i = 0; i < nr_rsvd; i++ )
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 96ed13b..367ad11 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -221,6 +221,20 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
     return NULL;
 }
 
+const char * __init boot_module_kind_as_string(bootmodule_kind kind)
+{
+    switch ( kind )
+    {
+    case BOOTMOD_XEN:     return "Xen";
+    case BOOTMOD_FDT:     return "FDT";
+    case BOOTMOD_KERNEL:  return "Kernel";
+    case BOOTMOD_RAMDISK: return "Ramdisk";
+    case BOOTMOD_XSM:     return "XSM";
+    case BOOTMOD_UNKNOWN: return "Unknown";
+    default: BUG();
+    }
+}
+
 void __init discard_initial_modules(void)
 {
     struct bootmodules *mi = &bootinfo.modules;
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index fe4997b..94741cf 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -64,6 +64,7 @@ const char __init *boot_fdt_cmdline(const void *fdt);
 void add_boot_module(bootmodule_kind kind, paddr_t start, paddr_t size,
                      const char *cmdline);
 struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
 #endif
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 9/9] xen: arm: update multiboot device tree bindings.
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (7 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 8/9] xen: arm: support bootmodule type detection by ordering Ian Campbell
@ 2014-06-26 16:45 ` Ian Campbell
  2014-06-26 16:46 ` [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
  9 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:45 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: (Try to) Clarify some wording, add examples to help.
---
 docs/misc/arm/device-tree/booting.txt |   54 +++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index bfb8d01..d967061 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -8,15 +8,31 @@ Each node contains the following properties:
 
 - compatible
 
-	Must be:
+	Must always include at least the generic compatiblity string:
 
-		"xen,<type>", "xen,multiboot-module"
+		"multiboot,module"
 
-	where <type> must be one of:
+	Optionally a more specific compatible string may be used in
+	addition to the above. One of:
 
-	- "linux-zimage" -- the dom0 kernel
-	- "linux-initrd" -- the dom0 ramdisk
-	- "xsm-policy"	 -- XSM policy blob
+	- "multiboot,kernel"	-- the dom0 kernel
+	- "multiboot,ramdisk"	-- the dom0 ramdisk
+	- "xen,xsm-policy"	-- XSM policy blob
+
+	It is normally recommended to include a more specific
+	compatible string (if one applies) in addition to the generic
+	string (which must always be present).
+
+        Xen 4.4 supported a different set of legacy compatible strings
+	which remain supported such that systems supporting both 4.4
+	and later can use a single DTB.
+
+	- "xen,multiboot-module" equivalent to "multiboot,module"
+	- "xen,linux-zimage"     equivalent to "multiboot,kernel"
+	- "xen,linux-initrd"     equivalent to "multiboot,ramdisk"
+
+	For compatibility with Xen 4.4 the more specific "xen,linux-*"
+	names are non-optional and must be included.
 
 - reg
 
@@ -29,6 +45,32 @@ Each node contains the following properties:
 	priority of this field vs. other mechanisms of specifying the
 	bootargs for the kernel.
 
+Examples
+========
+
+A boot module of unspecified type:
+
+	module@0xc0000000 {
+		compatible = "multiboot,module";
+		reg = <0xc0000000 0x1234>;
+		bootargs = "...";
+	};
+
+A boot module containing a ramdisk:
+
+	module@0xd0000000 {
+		compatible = "multiboot,ramdisk", "multiboot,module";
+		reg = <0xd0000000 0x5678>;
+	};
+
+The previous examples are compatible with Xen 4.5+ only.
+
+To be compatible with Xen 4.4 as well use the legacy names:
+
+	module@0xd0000000 {
+		compatible = "xen,linux-initrd", "xen,multiboot-module";
+		reg = <0xd0000000 0x5678>;
+	};
 
 Command lines
 =============
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support
  2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
                   ` (8 preceding siblings ...)
  2014-06-26 16:45 ` [PATCH v2 9/9] xen: arm: update multiboot device tree bindings Ian Campbell
@ 2014-06-26 16:46 ` Ian Campbell
  2014-06-27 11:58   ` Fu Wei
  9 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2014-06-26 16:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Naresh Bhat, Julien Grall, Tim Deegan,
	Roy Franz, Fu Wei

On Thu, 2014-06-26 at 17:45 +0100, Ian Campbell wrote:
> The following

I meant to include these git tree details too:

The following changes since commit 1c9d2acad014e997771c09d75bc071db754d2f4b:

  QEMU_TAG update (2014-06-25 15:58:02 +0100)

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/xen.git multiboot-improvements-v2

for you to fetch changes up to ea5a15a9430ee42e18453f3c825c2847d3b1afb2:

  xen: arm: update multiboot device tree bindings. (2014-06-26 17:41:47 +0100)

----------------------------------------------------------------
Ian Campbell (9):
      xen: arm: implement generic multiboot compatibility strings
      xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated
      xen: arm: prefer typesafe max()/min() over MAX()/MIN()
      xen: arm: rename early_info structs
      xen: arm: move boot time fdt parsing into separate file.
      xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline
      xen: arm: store per-boot module type instead of relying on index
      xen: arm: support bootmodule type detection by ordering
      xen: arm: update multiboot device tree bindings.

 docs/misc/arm/device-tree/booting.txt |   59 ++++-
 xen/arch/arm/Makefile                 |    1 +
 xen/arch/arm/bootfdt.c                |  368 +++++++++++++++++++++++++++++++
 xen/arch/arm/domain_build.c           |   21 +-
 xen/arch/arm/kernel.c                 |   15 +-
 xen/arch/arm/kernel.h                 |    3 +-
 xen/arch/arm/p2m.c                    |    4 +-
 xen/arch/arm/setup.c                  |  112 +++++++---
 xen/common/device_tree.c              |  386 +--------------------------------
 xen/include/asm-arm/setup.h           |   53 +++++
 xen/include/xen/device_tree.h         |   44 ----
 xen/xsm/xsm_policy.c                  |   19 +-
 12 files changed, 599 insertions(+), 486 deletions(-)
 create mode 100644 xen/arch/arm/bootfdt.c

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index
  2014-06-26 16:45 ` [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index Ian Campbell
@ 2014-06-27 10:43   ` Julien Grall
  2014-06-27 12:27     ` Ian Campbell
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2014-06-27 10:43 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: tim, stefano.stabellini

Hi Ian,

On 26/06/14 17:45, Ian Campbell wrote:
> This is more natural and better matches how multiboot is actually supposed to
> work.

I don't find any modification of consider_modules in this series. Is it 
normal? This function is badly assuming that MOD_XEN will be always the 
first one. So may end up to forget to exclude some modules during the 
memory allocation.

[..]

> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5eef8a3..c1a54e5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -161,9 +161,10 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>       int res = 0;
>       int had_dom0_bootargs = 0;
>
> -    if ( bootinfo.modules.nr_mods >= MOD_KERNEL &&
> -         bootinfo.modules.module[MOD_KERNEL].cmdline[0] )
> -        bootargs = &bootinfo.modules.module[MOD_KERNEL].cmdline[0];
> +    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);

It looks like a bit pointless to search the BOOTMOD_KERNEL everytime we 
try to write properties of a node. But it's not the purpose of this patch.

[..]

>   struct bootmodules {
>       int nr_mods;
>       /* Module 0 is Xen itself, followed by the provided modules-proper */

Does this comment still relevant?

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support
  2014-06-26 16:46 ` [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
@ 2014-06-27 11:58   ` Fu Wei
  0 siblings, 0 replies; 14+ messages in thread
From: Fu Wei @ 2014-06-27 11:58 UTC (permalink / raw)
  To: Ian Campbell, xen-devel
  Cc: Roy Franz, Naresh Bhat, Julien Grall, Tim Deegan, Stefano Stabellini

Hi Ian,
Great thanks for your update,
I have tested it with my new patch(including the new compatible streams), this branch works well. :-)
I will submit a new multiboot patch for GRUB ASAP! :-)

On 06/27/2014 12:46 AM, Ian Campbell wrote:
> On Thu, 2014-06-26 at 17:45 +0100, Ian Campbell wrote:
>> The following
> 
> I meant to include these git tree details too:
> 
> The following changes since commit 1c9d2acad014e997771c09d75bc071db754d2f4b:
> 
>   QEMU_TAG update (2014-06-25 15:58:02 +0100)
> 
> are available in the git repository at:
> 
>   git://xenbits.xen.org/people/ianc/xen.git multiboot-improvements-v2
> 
> for you to fetch changes up to ea5a15a9430ee42e18453f3c825c2847d3b1afb2:
> 
>   xen: arm: update multiboot device tree bindings. (2014-06-26 17:41:47 +0100)
> 
> ----------------------------------------------------------------
> Ian Campbell (9):
>       xen: arm: implement generic multiboot compatibility strings
>       xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated
>       xen: arm: prefer typesafe max()/min() over MAX()/MIN()
>       xen: arm: rename early_info structs
>       xen: arm: move boot time fdt parsing into separate file.
>       xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline
>       xen: arm: store per-boot module type instead of relying on index
>       xen: arm: support bootmodule type detection by ordering
>       xen: arm: update multiboot device tree bindings.
> 
>  docs/misc/arm/device-tree/booting.txt |   59 ++++-
>  xen/arch/arm/Makefile                 |    1 +
>  xen/arch/arm/bootfdt.c                |  368 +++++++++++++++++++++++++++++++
>  xen/arch/arm/domain_build.c           |   21 +-
>  xen/arch/arm/kernel.c                 |   15 +-
>  xen/arch/arm/kernel.h                 |    3 +-
>  xen/arch/arm/p2m.c                    |    4 +-
>  xen/arch/arm/setup.c                  |  112 +++++++---
>  xen/common/device_tree.c              |  386 +--------------------------------
>  xen/include/asm-arm/setup.h           |   53 +++++
>  xen/include/xen/device_tree.h         |   44 ----
>  xen/xsm/xsm_policy.c                  |   19 +-
>  12 files changed, 599 insertions(+), 486 deletions(-)
>  create mode 100644 xen/arch/arm/bootfdt.c
> 
> 


-- 
Best regards,

Fu Wei
Enterprise Server Engineer From Red Hat
LEG Team
Linaro.org | Open source software for ARM SoCs
Ph: +86 186 2020 4684 (mobile)
IRC: fuwei
Skype: tekkamanninja
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index
  2014-06-27 10:43   ` Julien Grall
@ 2014-06-27 12:27     ` Ian Campbell
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2014-06-27 12:27 UTC (permalink / raw)
  To: Julien Grall; +Cc: stefano.stabellini, tim, xen-devel

On Fri, 2014-06-27 at 11:43 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 26/06/14 17:45, Ian Campbell wrote:
> > This is more natural and better matches how multiboot is actually supposed to
> > work.
> 
> I don't find any modification of consider_modules in this series. Is it 
> normal? This function is badly assuming that MOD_XEN will be always the 
> first one. So may end up to forget to exclude some modules during the 
> memory allocation.

Cr*p, yes, I suspect you are right. I'll check.

In an early (unpublished) version of the patch I jumped through some
hoops to make sure that module 0 was always Xen. Looks like I shouldn't
have been so eager to clean that up!

> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 5eef8a3..c1a54e5 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -161,9 +161,10 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
> >       int res = 0;
> >       int had_dom0_bootargs = 0;
> >
> > -    if ( bootinfo.modules.nr_mods >= MOD_KERNEL &&
> > -         bootinfo.modules.module[MOD_KERNEL].cmdline[0] )
> > -        bootargs = &bootinfo.modules.module[MOD_KERNEL].cmdline[0];
> > +    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
> 
> It looks like a bit pointless to search the BOOTMOD_KERNEL everytime we 
> try to write properties of a node. But it's not the purpose of this patch.

Yeah, I should wrap it in a "dt_node_path_is_equal(node,
"/chosen")" (and I'll make the two existing ones into a single bool_t
foo = too while I'm there.

> [..]
> 
> >   struct bootmodules {
> >       int nr_mods;
> >       /* Module 0 is Xen itself, followed by the provided modules-proper */
> 
> Does this comment still relevant?

I suspect not.

Ian.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-06-27 12:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-26 16:45 [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
2014-06-26 16:45 ` [PATCH v2 1/9] xen: arm: implement generic multiboot compatibility strings Ian Campbell
2014-06-26 16:45 ` [PATCH v2 2/9] xen: arm: /chosen/module@N/bootargs bootprotcol node is not deprecated Ian Campbell
2014-06-26 16:45 ` [PATCH v2 3/9] xen: arm: prefer typesafe max()/min() over MAX()/MIN() Ian Campbell
2014-06-26 16:45 ` [PATCH v2 4/9] xen: arm: rename early_info structs Ian Campbell
2014-06-26 16:45 ` [PATCH v2 5/9] xen: arm: move boot time fdt parsing into separate file Ian Campbell
2014-06-26 16:45 ` [PATCH v2 6/9] xen: arm: move device_tree_bootargs to bootfdt.c, renaming to boot_fdt_cmdline Ian Campbell
2014-06-26 16:45 ` [PATCH v2 7/9] xen: arm: store per-boot module type instead of relying on index Ian Campbell
2014-06-27 10:43   ` Julien Grall
2014-06-27 12:27     ` Ian Campbell
2014-06-26 16:45 ` [PATCH v2 8/9] xen: arm: support bootmodule type detection by ordering Ian Campbell
2014-06-26 16:45 ` [PATCH v2 9/9] xen: arm: update multiboot device tree bindings Ian Campbell
2014-06-26 16:46 ` [PATCH v2 0/9] xen: arm: Refactor/improve early DT parsing and multiboot module support Ian Campbell
2014-06-27 11:58   ` Fu Wei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.