[Qemu-devel] [PATCH] arm: add device tree support

[Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

If compiled with CONFIG_FDT, allow user to specify a device tree file using
the -dtb argument.  If the machine supports it then the dtb will be loaded
into memory and passed to the kernel on boot.

This version of the patch only adds support to the arm versatile and versatile
express platforms.  Other platforms can easily be added.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
[grant.likely: Changed from platform specifying dtb to user option]
[grant.likely: Squashed in initrd and memory property manipulation patches]
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Brook <paul@codesourcery.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
I've been working with this patch for quite a while now.  It works well for
me.  It should be easy to extend to add support for other architectures too.

Note: I've done something ugly here for passing the dtb filename into the
machine->init() function by using a global variable.  I did it this way to
avoid changing the ->init() function prototype which would mean a lot of
functions need to be changed, but I'm not happy with it.

Alternately, I could change the prototype to accept an arguments structure
which can easily be extended in the future to support more machine arguments
without requires every machine to be changed.

g.

 Makefile.target  |    1 +
 configure        |    3 ++
 hw/arm-misc.h    |    1 +
 hw/arm_boot.c    |   95 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/boards.h      |    1 +
 hw/versatilepb.c |   20 +++++++++++
 hw/vexpress.c    |    1 +
 qemu-options.hx  |    9 +++++
 vl.c             |   11 ++++++
 9 files changed, 133 insertions(+), 9 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 68481a3..5e465ec 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -363,6 +363,7 @@ obj-arm-y += vexpress.o
 obj-arm-y += strongarm.o
 obj-arm-y += collie.o
 obj-arm-y += pl041.o lm4549.o
+obj-arm-$(CONFIG_FDT) += device_tree.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/configure b/configure
index f69e08f..0c2deab 100755
--- a/configure
+++ b/configure
@@ -3411,6 +3411,9 @@ case "$target_arch2" in
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
     target_phys_bits=32
     target_llong_alignment=4
+    if test "$fdt" = "yes" ; then
+      target_libs_softmmu="$fdt_libs"
+    fi
   ;;
   cris)
     target_nptl="yes"
diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 5e5204b..4b55fb8 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -29,6 +29,7 @@ struct arm_boot_info {
     const char *kernel_filename;
     const char *kernel_cmdline;
     const char *initrd_filename;
+    const char *dtb_filename;
     target_phys_addr_t loader_start;
     /* multicore boards that use the default secondary core boot functions
      * need to put the address of the secondary boot code, the boot reg,
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 5f163fd..93edeb1 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -7,11 +7,13 @@
  * This code is licensed under the GPL.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "arm-misc.h"
 #include "sysemu.h"
 #include "loader.h"
 #include "elf.h"
+#include "device_tree.h"
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
@@ -207,6 +209,66 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
     }
 }
 
+static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
+{
+#ifdef CONFIG_FDT
+    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
+                                    cpu_to_be32(binfo->ram_size) };
+    void *fdt = NULL;
+    char *filename;
+    int size, rc;
+
+    if (!binfo->dtb_filename)
+        return 0;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
+    if (!filename) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
+        return -1;
+    }
+
+    fdt = load_device_tree(filename, &size);
+    if (!fdt) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        g_free(filename);
+        return -1;
+    }
+    g_free(filename);
+
+    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (rc < 0)
+        fprintf(stderr, "couldn't set /memory/reg\n");
+
+    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                      binfo->kernel_cmdline);
+    if (rc < 0)
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+
+    if (binfo->initrd_size) {
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                binfo->loader_start + INITRD_LOAD_ADDR);
+        if (rc < 0)
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                    binfo->loader_start +INITRD_LOAD_ADDR +
+                    binfo->initrd_size);
+        if (rc < 0)
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+    }
+
+    cpu_physical_memory_write (addr, fdt, size);
+
+    return 0;
+
+#else
+    fprintf(stderr, "Platform requested a device tree, "
+                "but qemu was compiled without fdt support\n");
+    return -1;
+#endif
+}
+
 static void do_cpu_reset(void *opaque)
 {
     CPUState *env = opaque;
@@ -221,12 +283,14 @@ static void do_cpu_reset(void *opaque)
         } else {
             if (env == first_cpu) {
                 env->regs[15] = info->loader_start;
-                if (old_param) {
-                    set_kernel_args_old(info, info->initrd_size,
+                if (!info->dtb_filename) {
+                    if (old_param) {
+                        set_kernel_args_old(info, info->initrd_size,
+                                            info->loader_start);
+                    } else {
+                        set_kernel_args(info, info->initrd_size,
                                         info->loader_start);
-                } else {
-                    set_kernel_args(info, info->initrd_size,
-                                    info->loader_start);
+                    }
                 }
             } else {
                 info->secondary_cpu_reset_hook(env, info);
@@ -239,10 +303,10 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
 {
     int kernel_size;
     int initrd_size;
-    int n;
+    int n, rc;
     int is_linux = 0;
     uint64_t elf_entry;
-    target_phys_addr_t entry;
+    target_phys_addr_t entry, dtb_start;
     int big_endian;
 
     /* Load the kernel.  */
@@ -301,8 +365,22 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         } else {
             initrd_size = 0;
         }
+        info->initrd_size = initrd_size;
+
+        /* Place the DTB after the initrd in memory */
+        dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR +
+                                      initrd_size);
+        rc = load_dtb(dtb_start, info);
+        if (rc)
+            exit(1);
+
         bootloader[4] = info->board_id;
-        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        /* for device tree boot, we pass the DTB directly in r2. Otherwise
+         * we point to the kernel args */
+        if (info->dtb_filename)
+            bootloader[5] = dtb_start;
+        else
+            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
         bootloader[6] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
@@ -312,7 +390,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(env, info);
         }
-        info->initrd_size = initrd_size;
     }
     info->is_linux = is_linux;
 
diff --git a/hw/boards.h b/hw/boards.h
index f6d3784..d06776c 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -34,5 +34,6 @@ typedef struct QEMUMachine {
 int qemu_register_machine(QEMUMachine *m);
 
 extern QEMUMachine *current_machine;
+extern const char *current_dtb_filename;
 
 #endif
diff --git a/hw/versatilepb.c b/hw/versatilepb.c
index 3f7490c..7549c24 100644
--- a/hw/versatilepb.c
+++ b/hw/versatilepb.c
@@ -317,6 +317,7 @@ static void versatile_init(ram_addr_t ram_size,
     versatile_binfo.kernel_filename = kernel_filename;
     versatile_binfo.kernel_cmdline = kernel_cmdline;
     versatile_binfo.initrd_filename = initrd_filename;
+    versatile_binfo.dtb_filename = current_dtb_filename;
     versatile_binfo.board_id = board_id;
     arm_load_kernel(env, &versatile_binfo);
 }
@@ -343,6 +344,17 @@ static void vab_init(ram_addr_t ram_size,
                    initrd_filename, cpu_model, 0x25e);
 }
 
+static void vdt_init(ram_addr_t ram_size,
+                     const char *boot_device,
+                     const char *kernel_filename, const char *kernel_cmdline,
+                     const char *initrd_filename, const char *cpu_model)
+{
+    versatile_init(ram_size,
+                   boot_device,
+                   kernel_filename, kernel_cmdline,
+                   initrd_filename, cpu_model, 0xffffffff);
+}
+
 static QEMUMachine versatilepb_machine = {
     .name = "versatilepb",
     .desc = "ARM Versatile/PB (ARM926EJ-S)",
@@ -357,10 +369,18 @@ static QEMUMachine versatileab_machine = {
     .use_scsi = 1,
 };
 
+static QEMUMachine versatiledt_machine = {
+    .name = "versatiledt",
+    .desc = "ARM Versatile (device tree)",
+    .init = vdt_init,
+    .use_scsi = 1,
+};
+
 static void versatile_machine_init(void)
 {
     qemu_register_machine(&versatilepb_machine);
     qemu_register_machine(&versatileab_machine);
+    qemu_register_machine(&versatiledt_machine);
 }
 
 machine_init(versatile_machine_init);
diff --git a/hw/vexpress.c b/hw/vexpress.c
index 64fab45..ecfb585 100644
--- a/hw/vexpress.c
+++ b/hw/vexpress.c
@@ -219,6 +219,7 @@ static void vexpress_a9_init(ram_addr_t ram_size,
     vexpress_binfo.kernel_filename = kernel_filename;
     vexpress_binfo.kernel_cmdline = kernel_cmdline;
     vexpress_binfo.initrd_filename = initrd_filename;
+    vexpress_binfo.dtb_filename = current_dtb_filename;
     vexpress_binfo.nb_cpus = smp_cpus;
     vexpress_binfo.board_id = VEXPRESS_BOARD_ID;
     vexpress_binfo.loader_start = 0x60000000;
diff --git a/qemu-options.hx b/qemu-options.hx
index 3a07ae8..0a01baa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
 first module.
 ETEXI
 
+DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
+    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)
+STEXI
+@item -dtb @var{file}
+@findex -dtb
+Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
+on boot.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index d88a18c..f42f1fc 100644
--- a/vl.c
+++ b/vl.c
@@ -1154,6 +1154,7 @@ void pcmcia_info(Monitor *mon)
 
 static QEMUMachine *first_machine = NULL;
 QEMUMachine *current_machine = NULL;
+const char *current_dtb_filename = NULL;
 
 int qemu_register_machine(QEMUMachine *m)
 {
@@ -2166,6 +2167,7 @@ int main(int argc, char **argv, char **envp)
     int snapshot, linux_boot;
     const char *icount_option = NULL;
     const char *initrd_filename;
+    const char *dtb_filename = NULL;
     const char *kernel_filename, *kernel_cmdline;
     char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
     DisplayState *ds;
@@ -2304,6 +2306,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_initrd:
                 initrd_filename = optarg;
                 break;
+            case QEMU_OPTION_dtb:
+                dtb_filename = optarg;
+                break;
             case QEMU_OPTION_hda:
                 {
                     char buf[256];
@@ -3241,6 +3246,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (!linux_boot && dtb_filename != NULL) {
+        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        exit(1);
+    }
+
     os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {
@@ -3374,6 +3384,7 @@ int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
+    current_dtb_filename = dtb_filename;
     machine->init(ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Paul Brook]

> If compiled with CONFIG_FDT, allow user to specify a device tree file using
> the -dtb argument.  If the machine supports it then the dtb will be loaded
> into memory and passed to the kernel on boot.

Adding annother machine feels wrong. Why does the board specific code need to 
know about this at all? You already going it via a global variable, so can't 
this be entirely contained within arm_boot.c?  If the board file is involved, 
why is it asking the user?

> +    versatile_init(ram_size,
> +                   boot_device,
> +                   kernel_filename, kernel_cmdline,
> +                   initrd_filename, cpu_model, 0xffffffff);

This only works because we're currently too dumb to emulate the differences 
between the two board variants.


What we probably want to be doing is shipping/constructing device trees for 
the boards we implement, with an option to turn this on/off.  Requiring a user 
to invent their own seems deeply sub-optimal given we know exactly what 
hardware we're emulating.  A user that needs to provide their own FDT seems 
like a fairly rare corner case.

This gets slightly more interesting when you have custom machine variants 
(i.e. once we fix the object model, and have proper dynamic machine 
construction).  Even then I'd expect the FDT to be derived from/specificed by 
the machine description, not a separate option.

Paul

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Fri, Jan 27, 2012 at 10:34:01PM +0000, Paul Brook wrote:
> > If compiled with CONFIG_FDT, allow user to specify a device tree file using
> > the -dtb argument.  If the machine supports it then the dtb will be loaded
> > into memory and passed to the kernel on boot.
> 
> Adding annother machine feels wrong. Why does the board specific code need to 
> know about this at all? You already going it via a global variable, so can't 
> this be entirely contained within arm_boot.c?

Mostly because the infrastructure isn't yet in place to pass the .dtb file
through to the arm_boot code (or maybe it is; what is the best way to pass
command line data through to the arm_boot.c code (or similar for other
architectures)?

> If the board file is involved, why is it asking the user?

There is a lot of configuration in the .dts file that the QEMU user may want
to manipulate; particularly when using QEMU for testing embedded platforms.
The direction I want to go is to select the machine model based on the top
level DT compatible property (making -M optional), and then also allow a lot
of the machine layout to be driven by DT data.  ie. populate emulated devices
from DT data.

I believe this is how Edgar is using the microblaze model, but I don't
think those patches have been upstreamed yet.  I hope that microblaze,
ARM and powerpc can all use the same model here.

> 
> > +    versatile_init(ram_size,
> > +                   boot_device,
> > +                   kernel_filename, kernel_cmdline,
> > +                   initrd_filename, cpu_model, 0xffffffff);
> 
> This only works because we're currently too dumb to emulate the differences 
> between the two board variants.

Yeah, this is a hack so I could play with forcing the machine id.
I'll remove it.

> What we probably want to be doing is shipping/constructing device trees for 
> the boards we implement, with an option to turn this on/off.  Requiring a user 
> to invent their own seems deeply sub-optimal given we know exactly what 
> hardware we're emulating.  A user that needs to provide their own FDT seems 
> like a fairly rare corner case.

I disagree.  QEMU may want to ship stock .dts files, but it will
absolutely be a common use case for embedded developers to pass in
their own .dtb file.

> This gets slightly more interesting when you have custom machine variants 
> (i.e. once we fix the object model, and have proper dynamic machine 
> construction).  Even then I'd expect the FDT to be derived from/specificed by 
> the machine description, not a separate option.

I started with going down that route, but switched to this model after
playing with it and noticing that it doesn't seem to fit as well for
embedded development as providing a .dtb file and having QEMU
construct a machine that matches the data.

g.

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Peter Crosthwaite]

[-- Attachment #1: Type: text/plain, Size: 3568 bytes --]

Hi Grant,

The patch series for dts driven machine creation we (myself and Edgar) use
that you are referring to was rejected a few months ago on the grounds that
it conflicted with QOM:

http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02953.html

I am maintaining it our of tree, although I have not fast forwarded that
branch for a while, so I cant recreate that series quickly. But now that
QOM is immenent, can we get another review on the original series, and
comments on what needs to happen to make this play nice with QOM? There is
nothing in this series that is target specific, Adaption for arm should be
fairly trivial.

Peter

On Fri, Jan 27, 2012 at 10:34:01PM +0000, Paul Brook wrote:
> > > If compiled with CONFIG_FDT, allow user to specify a device tree file
> using
> > > the -dtb argument.  If the machine supports it then the dtb will be
> loaded
> > > into memory and passed to the kernel on boot.
> >
> > Adding annother machine feels wrong. Why does the board specific code
> need to
> > know about this at all? You already going it via a global variable, so
> can't
> > this be entirely contained within arm_boot.c?
>
> Mostly because the infrastructure isn't yet in place to pass the .dtb file
> through to the arm_boot code (or maybe it is; what is the best way to pass
> command line data through to the arm_boot.c code (or similar for other
> architectures)?
>
> > If the board file is involved, why is it asking the user?
>
> There is a lot of configuration in the .dts file that the QEMU user may
> want
> to manipulate; particularly when using QEMU for testing embedded platforms.
> The direction I want to go is to select the machine model based on the top
> level DT compatible property (making -M optional), and then also allow a
> lot
> of the machine layout to be driven by DT data.  ie. populate emulated
> devices
> from DT data.
>
> I believe this is how Edgar is using the microblaze model, but I don't
> think those patches have been upstreamed yet.  I hope that microblaze,
> ARM and powerpc can all use the same model here.
>
> >
> > > +    versatile_init(ram_size,
> > > +                   boot_device,
> > > +                   kernel_filename, kernel_cmdline,
> > > +                   initrd_filename, cpu_model, 0xffffffff);
> >
> > This only works because we're currently too dumb to emulate the
> differences
> > between the two board variants.
>
> Yeah, this is a hack so I could play with forcing the machine id.
> I'll remove it.
>
> > What we probably want to be doing is shipping/constructing device trees
> for
> > the boards we implement, with an option to turn this on/off.  Requiring
> a user
> > to invent their own seems deeply sub-optimal given we know exactly what
> > hardware we're emulating.  A user that needs to provide their own FDT
> seems
> > like a fairly rare corner case.
>
> I disagree.  QEMU may want to ship stock .dts files, but it will
> absolutely be a common use case for embedded developers to pass in
> their own .dtb file.
>
> > This gets slightly more interesting when you have custom machine variants
> > (i.e. once we fix the object model, and have proper dynamic machine
> > construction).  Even then I'd expect the FDT to be derived
> from/specificed by
> > the machine description, not a separate option.
>
> I started with going down that route, but switched to this model after
> playing with it and noticing that it doesn't seem to fit as well for
> embedded development as providing a .dtb file and having QEMU
> construct a machine that matches the data.
>
> g.
>
>

[-- Attachment #2: Type: text/html, Size: 4325 bytes --]

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Paul Brook]

> There is a lot of configuration in the .dts file that the QEMU user may
> want to manipulate; particularly when using QEMU for testing embedded
> platforms. The direction I want to go is to select the machine model based
> on the top level DT compatible property (making -M optional), and then
> also allow a lot of the machine layout to be driven by DT data.  ie.
> populate emulated devices from DT data.

I want to remove -M, and the associated board specific knowledge altogether.  
A system that can only create limited variants of hardcoded boards is IMO not 
a viable medium/long term solution.

> > This gets slightly more interesting when you have custom machine variants
> > (i.e. once we fix the object model, and have proper dynamic machine
> > construction).  Even then I'd expect the FDT to be derived
> > from/specificed by the machine description, not a separate option.
> 
> I started with going down that route, but switched to this model after
> playing with it and noticing that it doesn't seem to fit as well for
> embedded development as providing a .dtb file and having QEMU
> construct a machine that matches the data.

Problem is that a FDT doesn't really tell you everything you need to know in 
order to construct the machine.  If you look back in the archives you should 
find that I tried something similar when I first added the qdev code.  While 
it appears to be a nice idea at first, it doesn't actually work do well in 
practice.  It's maybe OK for simple embedded systems with a single system bus, 
but when you start looking at more sophisticated machines it doesn't fit so 
well.  In particular when you look at things like PCI, devices connected via 
multiple busses (a sound codec with both I2C [command] and I2S [data] 
connections), and some of the stranger bus topographies (some ppc boards have 
lots of weirdness in the FDT) the requirements of qemu and linux start to 
diverge.

Clearly we need to have some sort of FDT support.  However I'm unconvinced 
that it's the correct format for the primary data structure.  For one thing 
it's a hierarchical tree structure, and in my experience describing links 
outside that structure gets hairy.  One of the things we're doing with QOM is 
move from a hierarchical tree/bus to a more general connected web of links 
between devices.

Paul

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Sun, Jan 29, 2012 at 11:15:42AM +0000, Paul Brook wrote:
> Clearly we need to have some sort of FDT support.  However I'm unconvinced 
> that it's the correct format for the primary data structure.  For one thing 
> it's a hierarchical tree structure, and in my experience describing links 
> outside that structure gets hairy.  One of the things we're doing with QOM is 
> move from a hierarchical tree/bus to a more general connected web of links 
> between devices.

Okay, well at least let's start with this.  Here is an updated version of the
patch that doesn't touch the board code.  It makes the -dtb option available
to all ARM platforms.

Even if dtb is not the primary structure, we absolutely need this option.
Developers using qemu to simulate real hardware are going to still want
to pass in the exact same .dtb file to qemu as they use for the real target,
and those .dtb files are maintained completely separately from qemu.

g.

---
diff --git a/Makefile.target b/Makefile.target
index 68481a3..5e465ec 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -363,6 +363,7 @@ obj-arm-y += vexpress.o
 obj-arm-y += strongarm.o
 obj-arm-y += collie.o
 obj-arm-y += pl041.o lm4549.o
+obj-arm-$(CONFIG_FDT) += device_tree.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/configure b/configure
index f69e08f..0c2deab 100755
--- a/configure
+++ b/configure
@@ -3411,6 +3411,9 @@ case "$target_arch2" in
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
     target_phys_bits=32
     target_llong_alignment=4
+    if test "$fdt" = "yes" ; then
+      target_libs_softmmu="$fdt_libs"
+    fi
   ;;
   cris)
     target_nptl="yes"
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 5f163fd..35bfa62 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -7,11 +7,14 @@
  * This code is licensed under the GPL.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "arm-misc.h"
 #include "sysemu.h"
+#include "boards.h"
 #include "loader.h"
 #include "elf.h"
+#include "device_tree.h"
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
@@ -207,6 +210,66 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
     }
 }
 
+static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
+{
+#ifdef CONFIG_FDT
+    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
+                                    cpu_to_be32(binfo->ram_size) };
+    void *fdt = NULL;
+    char *filename;
+    int size, rc;
+
+    if (!current_dtb_filename)
+        return 0;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename);
+    if (!filename) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename);
+        return -1;
+    }
+
+    fdt = load_device_tree(filename, &size);
+    if (!fdt) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        g_free(filename);
+        return -1;
+    }
+    g_free(filename);
+
+    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (rc < 0)
+        fprintf(stderr, "couldn't set /memory/reg\n");
+
+    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                      binfo->kernel_cmdline);
+    if (rc < 0)
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+
+    if (binfo->initrd_size) {
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                binfo->loader_start + INITRD_LOAD_ADDR);
+        if (rc < 0)
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                    binfo->loader_start +INITRD_LOAD_ADDR +
+                    binfo->initrd_size);
+        if (rc < 0)
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+    }
+
+    cpu_physical_memory_write (addr, fdt, size);
+
+    return 0;
+
+#else
+    fprintf(stderr, "Platform requested a device tree, "
+                "but qemu was compiled without fdt support\n");
+    return -1;
+#endif
+}
+
 static void do_cpu_reset(void *opaque)
 {
     CPUState *env = opaque;
@@ -221,12 +284,14 @@ static void do_cpu_reset(void *opaque)
         } else {
             if (env == first_cpu) {
                 env->regs[15] = info->loader_start;
-                if (old_param) {
-                    set_kernel_args_old(info, info->initrd_size,
+                if (!current_dtb_filename) {
+                    if (old_param) {
+                        set_kernel_args_old(info, info->initrd_size,
+                                            info->loader_start);
+                    } else {
+                        set_kernel_args(info, info->initrd_size,
                                         info->loader_start);
-                } else {
-                    set_kernel_args(info, info->initrd_size,
-                                    info->loader_start);
+                    }
                 }
             } else {
                 info->secondary_cpu_reset_hook(env, info);
@@ -239,10 +304,10 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
 {
     int kernel_size;
     int initrd_size;
-    int n;
+    int n, rc;
     int is_linux = 0;
     uint64_t elf_entry;
-    target_phys_addr_t entry;
+    target_phys_addr_t entry, dtb_start;
     int big_endian;
 
     /* Load the kernel.  */
@@ -301,8 +366,22 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         } else {
             initrd_size = 0;
         }
+        info->initrd_size = initrd_size;
+
+        /* Place the DTB after the initrd in memory */
+        dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR +
+                                      initrd_size);
+        rc = load_dtb(dtb_start, info);
+        if (rc)
+            exit(1);
+
         bootloader[4] = info->board_id;
-        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        /* for device tree boot, we pass the DTB directly in r2. Otherwise
+         * we point to the kernel args */
+        if (current_dtb_filename)
+            bootloader[5] = dtb_start;
+        else
+            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
         bootloader[6] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
@@ -312,7 +391,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(env, info);
         }
-        info->initrd_size = initrd_size;
     }
     info->is_linux = is_linux;
 
diff --git a/hw/boards.h b/hw/boards.h
index f6d3784..d06776c 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -34,5 +34,6 @@ typedef struct QEMUMachine {
 int qemu_register_machine(QEMUMachine *m);
 
 extern QEMUMachine *current_machine;
+extern const char *current_dtb_filename;
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 3a07ae8..0a01baa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
 first module.
 ETEXI
 
+DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
+    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)
+STEXI
+@item -dtb @var{file}
+@findex -dtb
+Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
+on boot.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index d88a18c..b22eae3 100644
--- a/vl.c
+++ b/vl.c
@@ -1154,6 +1154,7 @@ void pcmcia_info(Monitor *mon)
 
 static QEMUMachine *first_machine = NULL;
 QEMUMachine *current_machine = NULL;
+const char *current_dtb_filename = NULL;
 
 int qemu_register_machine(QEMUMachine *m)
 {
@@ -2304,6 +2305,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_initrd:
                 initrd_filename = optarg;
                 break;
+            case QEMU_OPTION_dtb:
+                current_dtb_filename = optarg;
+                break;
             case QEMU_OPTION_hda:
                 {
                     char buf[256];
@@ -3241,6 +3245,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (!linux_boot && current_dtb_filename != NULL) {
+        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        exit(1);
+    }
+
     os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Andreas Färber]

Am 29.01.2012 17:01, schrieb Grant Likely:
> Okay, well at least let's start with this.  Here is an updated version of the
> patch that doesn't touch the board code.  It makes the -dtb option available
> to all ARM platforms.
> 
> Even if dtb is not the primary structure, we absolutely need this option.

Well, the option itself is what has been the most controversial here.
Yours is at least the third attempt, so far we've seen discussed -dtb,
-kern-dtb, -kernel-dtb, -kernel dtb=, etc. Also how to pass around the
data obtained that way - whether as global variable or via struct passed
to machine init or in a few weeks via QOM.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Peter Maydell]

On 29 January 2012 16:01, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Sun, Jan 29, 2012 at 11:15:42AM +0000, Paul Brook wrote:
>> Clearly we need to have some sort of FDT support.  However I'm unconvinced
>> that it's the correct format for the primary data structure.  For one thing
>> it's a hierarchical tree structure, and in my experience describing links
>> outside that structure gets hairy.  One of the things we're doing with QOM is
>> move from a hierarchical tree/bus to a more general connected web of links
>> between devices.
>
> Okay, well at least let's start with this.  Here is an updated version of the
> patch that doesn't touch the board code.  It makes the -dtb option available
> to all ARM platforms.
>
> Even if dtb is not the primary structure, we absolutely need this option.
> Developers using qemu to simulate real hardware are going to still want
> to pass in the exact same .dtb file to qemu as they use for the real target,
> and those .dtb files are maintained completely separately from qemu.

I agree with Paul that trying to use the dtb as the primary structure
for describing a machine model to qemu is decidedly problematic. Let's
start with the basic "we're a bootloader, provide minimal support for
passing in a device tree blob"...

Incidentally, it would probably be useful to sort out the problems with
libfdt not being very widely provided (cf previous discussion,
http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02252.html).
I see Aurelien has suggested a patch to the debian package to provide
it (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=477565) which
ought to help there.

Some more detailed-level code review follows on the assumption that
this patch is a reasonable direction to go in (which it seems like to me).

> diff --git a/Makefile.target b/Makefile.target
> index 68481a3..5e465ec 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -363,6 +363,7 @@ obj-arm-y += vexpress.o
>  obj-arm-y += strongarm.o
>  obj-arm-y += collie.o
>  obj-arm-y += pl041.o lm4549.o
> +obj-arm-$(CONFIG_FDT) += device_tree.o
>
>  obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
>  obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
> diff --git a/configure b/configure
> index f69e08f..0c2deab 100755
> --- a/configure
> +++ b/configure
> @@ -3411,6 +3411,9 @@ case "$target_arch2" in
>     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
>     target_phys_bits=32
>     target_llong_alignment=4
> +    if test "$fdt" = "yes" ; then
> +      target_libs_softmmu="$fdt_libs"
> +    fi

This doesn't need to be conditional -- compare the similar stanzas
for other fdt-using architectures.

>   ;;
>   cris)
>     target_nptl="yes"
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 5f163fd..35bfa62 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -7,11 +7,14 @@
>  * This code is licensed under the GPL.
>  */
>
> +#include "config.h"
>  #include "hw.h"
>  #include "arm-misc.h"
>  #include "sysemu.h"
> +#include "boards.h"
>  #include "loader.h"
>  #include "elf.h"
> +#include "device_tree.h"
>
>  #define KERNEL_ARGS_ADDR 0x100
>  #define KERNEL_LOAD_ADDR 0x00010000
> @@ -207,6 +210,66 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
>     }
>  }
>
> +static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
> +{
> +#ifdef CONFIG_FDT
> +    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
> +                                    cpu_to_be32(binfo->ram_size) };
> +    void *fdt = NULL;
> +    char *filename;
> +    int size, rc;
> +
> +    if (!current_dtb_filename)
> +        return 0;

scripts/checkpatch.pl complains about this and other style issues...

> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename);
> +    if (!filename) {
> +        fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename);
> +        return -1;
> +    }
> +
> +    fdt = load_device_tree(filename, &size);
> +    if (!fdt) {
> +        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
> +        g_free(filename);
> +        return -1;
> +    }
> +    g_free(filename);
> +
> +    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
> +                               sizeof(mem_reg_property));
> +    if (rc < 0)
> +        fprintf(stderr, "couldn't set /memory/reg\n");
> +
> +    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
> +                                      binfo->kernel_cmdline);
> +    if (rc < 0)
> +        fprintf(stderr, "couldn't set /chosen/bootargs\n");

This seems kind of weird -- if you're not trying to use 'rc' as
a running "something failed" flag to return to the caller then
why not just have "if (function() < 0) { fprintf(...); }" ?

> +
> +    if (binfo->initrd_size) {
> +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> +                binfo->loader_start + INITRD_LOAD_ADDR);
> +        if (rc < 0)
> +            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> +
> +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +                    binfo->loader_start +INITRD_LOAD_ADDR +
> +                    binfo->initrd_size);
> +        if (rc < 0)
> +            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> +    }
> +
> +    cpu_physical_memory_write (addr, fdt, size);
> +
> +    return 0;
> +
> +#else
> +    fprintf(stderr, "Platform requested a device tree, "
> +                "but qemu was compiled without fdt support\n");
> +    return -1;
> +#endif
> +}
> +
>  static void do_cpu_reset(void *opaque)
>  {
>     CPUState *env = opaque;
> @@ -221,12 +284,14 @@ static void do_cpu_reset(void *opaque)
>         } else {
>             if (env == first_cpu) {
>                 env->regs[15] = info->loader_start;
> -                if (old_param) {
> -                    set_kernel_args_old(info, info->initrd_size,
> +                if (!current_dtb_filename) {
> +                    if (old_param) {
> +                        set_kernel_args_old(info, info->initrd_size,
> +                                            info->loader_start);
> +                    } else {
> +                        set_kernel_args(info, info->initrd_size,
>                                         info->loader_start);
> -                } else {
> -                    set_kernel_args(info, info->initrd_size,
> -                                    info->loader_start);
> +                    }
>                 }
>             } else {
>                 info->secondary_cpu_reset_hook(env, info);
> @@ -239,10 +304,10 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>  {
>     int kernel_size;
>     int initrd_size;
> -    int n;
> +    int n, rc;
>     int is_linux = 0;
>     uint64_t elf_entry;
> -    target_phys_addr_t entry;
> +    target_phys_addr_t entry, dtb_start;
>     int big_endian;
>
>     /* Load the kernel.  */
> @@ -301,8 +366,22 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>         } else {
>             initrd_size = 0;
>         }
> +        info->initrd_size = initrd_size;
> +
> +        /* Place the DTB after the initrd in memory */
> +        dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR +
> +                                      initrd_size);
> +        rc = load_dtb(dtb_start, info);
> +        if (rc)
> +            exit(1);

The rc variable is pretty obviously unnecessary here.

> +
>         bootloader[4] = info->board_id;
> -        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> +        /* for device tree boot, we pass the DTB directly in r2. Otherwise
> +         * we point to the kernel args */
> +        if (current_dtb_filename)
> +            bootloader[5] = dtb_start;
> +        else
> +            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
>         bootloader[6] = entry;
>         for (n = 0; n < sizeof(bootloader) / 4; n++) {
>             bootloader[n] = tswap32(bootloader[n]);
> @@ -312,7 +391,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>         if (info->nb_cpus > 1) {
>             info->write_secondary_boot(env, info);
>         }
> -        info->initrd_size = initrd_size;
>     }
>     info->is_linux = is_linux;
>
> diff --git a/hw/boards.h b/hw/boards.h
> index f6d3784..d06776c 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -34,5 +34,6 @@ typedef struct QEMUMachine {
>  int qemu_register_machine(QEMUMachine *m);
>
>  extern QEMUMachine *current_machine;
> +extern const char *current_dtb_filename;

The suggestion on IRC for the long-term Right Thing for passing
dtb filename/kernel/etc to arm_boot is that the arm_boot code ought
to turn into a QOM object, and then the top level code can just
search the QOM tree the machine model instantiates for a QOM
object of the right type and pass filenames to it directly by
setting properties on it. So this new global is ok as it's
likely to go away again eventually.

>  #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 3a07ae8..0a01baa 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
>  first module.
>  ETEXI
>
> +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
> +    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)

Needs more spaces to make it line up right in -help output:
Linux/Multiboot boot specific:
-kernel bzImage use 'bzImage' as kernel image
-append cmdline use 'cmdline' as kernel command line
-initrd file    use 'file' as initial ram disk
-dtb file use 'file' as a device tree image

> +STEXI
> +@item -dtb @var{file}
> +@findex -dtb
> +Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
> +on boot.
> +ETEXI
> +
>  STEXI
>  @end table
>  ETEXI
> diff --git a/vl.c b/vl.c
> index d88a18c..b22eae3 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1154,6 +1154,7 @@ void pcmcia_info(Monitor *mon)
>
>  static QEMUMachine *first_machine = NULL;
>  QEMUMachine *current_machine = NULL;
> +const char *current_dtb_filename = NULL;
>
>  int qemu_register_machine(QEMUMachine *m)
>  {
> @@ -2304,6 +2305,9 @@ int main(int argc, char **argv, char **envp)
>             case QEMU_OPTION_initrd:
>                 initrd_filename = optarg;
>                 break;
> +            case QEMU_OPTION_dtb:
> +                current_dtb_filename = optarg;
> +                break;
>             case QEMU_OPTION_hda:
>                 {
>                     char buf[256];
> @@ -3241,6 +3245,11 @@ int main(int argc, char **argv, char **envp)
>         exit(1);
>     }
>
> +    if (!linux_boot && current_dtb_filename != NULL) {
> +        fprintf(stderr, "-dtb only allowed with -kernel option\n");
> +        exit(1);
> +    }
> +
>     os_set_line_buffering();
>
>     if (init_timer_alarm() < 0) {


-- PMM

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Sun, Jan 29, 2012 at 07:13:55PM +0000, Peter Maydell wrote:
> On 29 January 2012 16:01, Grant Likely <grant.likely@secretlab.ca> wrote:
> > diff --git a/configure b/configure
> > index f69e08f..0c2deab 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3411,6 +3411,9 @@ case "$target_arch2" in
> >     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
> >     target_phys_bits=32
> >     target_llong_alignment=4
> > +    if test "$fdt" = "yes" ; then
> > +      target_libs_softmmu="$fdt_libs"
> > +    fi
> 
> This doesn't need to be conditional -- compare the similar stanzas
> for other fdt-using architectures.

Okay.

> >   ;;
> >   cris)
> >     target_nptl="yes"
> > diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> > index 5f163fd..35bfa62 100644
> > --- a/hw/arm_boot.c
> > +++ b/hw/arm_boot.c
> > @@ -7,11 +7,14 @@
> >  * This code is licensed under the GPL.
> >  */
> >
> > +#include "config.h"
> >  #include "hw.h"
> >  #include "arm-misc.h"
> >  #include "sysemu.h"
> > +#include "boards.h"
> >  #include "loader.h"
> >  #include "elf.h"
> > +#include "device_tree.h"
> >
> >  #define KERNEL_ARGS_ADDR 0x100
> >  #define KERNEL_LOAD_ADDR 0x00010000
> > @@ -207,6 +210,66 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
> >     }
> >  }
> >
> > +static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
> > +{
> > +#ifdef CONFIG_FDT
> > +    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
> > +                                    cpu_to_be32(binfo->ram_size) };
> > +    void *fdt = NULL;
> > +    char *filename;
> > +    int size, rc;
> > +
> > +    if (!current_dtb_filename)
> > +        return 0;
> 
> scripts/checkpatch.pl complains about this and other style issues...
> 

Fixed.

> > +
> > +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename);
> > +    if (!filename) {
> > +        fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename);
> > +        return -1;
> > +    }
> > +
> > +    fdt = load_device_tree(filename, &size);
> > +    if (!fdt) {
> > +        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
> > +        g_free(filename);
> > +        return -1;
> > +    }
> > +    g_free(filename);
> > +
> > +    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
> > +                               sizeof(mem_reg_property));
> > +    if (rc < 0)
> > +        fprintf(stderr, "couldn't set /memory/reg\n");
> > +
> > +    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
> > +                                      binfo->kernel_cmdline);
> > +    if (rc < 0)
> > +        fprintf(stderr, "couldn't set /chosen/bootargs\n");
> 
> This seems kind of weird -- if you're not trying to use 'rc' as
> a running "something failed" flag to return to the caller then
> why not just have "if (function() < 0) { fprintf(...); }" ?

Mostly because it looked ugly when done that way.  There is already a
line break to deal with the long arguments, it looked worse to me when
also put inside an if() block.  If you prefer, then I will change it.

> 
> > +
> > +    if (binfo->initrd_size) {
> > +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> > +                binfo->loader_start + INITRD_LOAD_ADDR);
> > +        if (rc < 0)
> > +            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> > +
> > +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> > +                    binfo->loader_start +INITRD_LOAD_ADDR +
> > +                    binfo->initrd_size);
> > +        if (rc < 0)
> > +            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> > +    }
> > +
> > +    cpu_physical_memory_write (addr, fdt, size);
> > +
> > +    return 0;
> > +
> > +#else
> > +    fprintf(stderr, "Platform requested a device tree, "
> > +                "but qemu was compiled without fdt support\n");
> > +    return -1;
> > +#endif
> > +}
> > +
> >  static void do_cpu_reset(void *opaque)
> >  {
> >     CPUState *env = opaque;
> > @@ -221,12 +284,14 @@ static void do_cpu_reset(void *opaque)
> >         } else {
> >             if (env == first_cpu) {
> >                 env->regs[15] = info->loader_start;
> > -                if (old_param) {
> > -                    set_kernel_args_old(info, info->initrd_size,
> > +                if (!current_dtb_filename) {
> > +                    if (old_param) {
> > +                        set_kernel_args_old(info, info->initrd_size,
> > +                                            info->loader_start);
> > +                    } else {
> > +                        set_kernel_args(info, info->initrd_size,
> >                                         info->loader_start);
> > -                } else {
> > -                    set_kernel_args(info, info->initrd_size,
> > -                                    info->loader_start);
> > +                    }
> >                 }
> >             } else {
> >                 info->secondary_cpu_reset_hook(env, info);
> > @@ -239,10 +304,10 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
> >  {
> >     int kernel_size;
> >     int initrd_size;
> > -    int n;
> > +    int n, rc;
> >     int is_linux = 0;
> >     uint64_t elf_entry;
> > -    target_phys_addr_t entry;
> > +    target_phys_addr_t entry, dtb_start;
> >     int big_endian;
> >
> >     /* Load the kernel.  */
> > @@ -301,8 +366,22 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
> >         } else {
> >             initrd_size = 0;
> >         }
> > +        info->initrd_size = initrd_size;
> > +
> > +        /* Place the DTB after the initrd in memory */
> > +        dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR +
> > +                                      initrd_size);
> > +        rc = load_dtb(dtb_start, info);
> > +        if (rc)
> > +            exit(1);
> 
> The rc variable is pretty obviously unnecessary here.

Fixed

> 
> > +
> >         bootloader[4] = info->board_id;
> > -        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> > +        /* for device tree boot, we pass the DTB directly in r2. Otherwise
> > +         * we point to the kernel args */
> > +        if (current_dtb_filename)
> > +            bootloader[5] = dtb_start;
> > +        else
> > +            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> >         bootloader[6] = entry;
> >         for (n = 0; n < sizeof(bootloader) / 4; n++) {
> >             bootloader[n] = tswap32(bootloader[n]);
> > @@ -312,7 +391,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
> >         if (info->nb_cpus > 1) {
> >             info->write_secondary_boot(env, info);
> >         }
> > -        info->initrd_size = initrd_size;
> >     }
> >     info->is_linux = is_linux;
> >
> > diff --git a/hw/boards.h b/hw/boards.h
> > index f6d3784..d06776c 100644
> > --- a/hw/boards.h
> > +++ b/hw/boards.h
> > @@ -34,5 +34,6 @@ typedef struct QEMUMachine {
> >  int qemu_register_machine(QEMUMachine *m);
> >
> >  extern QEMUMachine *current_machine;
> > +extern const char *current_dtb_filename;
> 
> The suggestion on IRC for the long-term Right Thing for passing
> dtb filename/kernel/etc to arm_boot is that the arm_boot code ought
> to turn into a QOM object, and then the top level code can just
> search the QOM tree the machine model instantiates for a QOM
> object of the right type and pass filenames to it directly by
> setting properties on it. So this new global is ok as it's
> likely to go away again eventually.

Okay, good.  I had assumed that this would be a temporary solution until
the needed infrastructure is in place.

> 
> >  #endif
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 3a07ae8..0a01baa 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
> >  first module.
> >  ETEXI
> >
> > +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
> > +    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)
> 
> Needs more spaces to make it line up right in -help output:
> Linux/Multiboot boot specific:
> -kernel bzImage use 'bzImage' as kernel image
> -append cmdline use 'cmdline' as kernel command line
> -initrd file    use 'file' as initial ram disk
> -dtb file use 'file' as a device tree image

Fixed.

New patch below...

---
diff --git a/Makefile.target b/Makefile.target
index 68481a3..5e465ec 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -363,6 +363,7 @@ obj-arm-y += vexpress.o
 obj-arm-y += strongarm.o
 obj-arm-y += collie.o
 obj-arm-y += pl041.o lm4549.o
+obj-arm-$(CONFIG_FDT) += device_tree.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/configure b/configure
index f69e08f..2856897 100755
--- a/configure
+++ b/configure
@@ -3411,6 +3411,7 @@ case "$target_arch2" in
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
     target_phys_bits=32
     target_llong_alignment=4
+    target_libs_softmmu="$fdt_libs"
   ;;
   cris)
     target_nptl="yes"
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 5f163fd..377d202 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -7,11 +7,14 @@
  * This code is licensed under the GPL.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "arm-misc.h"
 #include "sysemu.h"
+#include "boards.h"
 #include "loader.h"
 #include "elf.h"
+#include "device_tree.h"
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
@@ -207,6 +210,71 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
     }
 }
 
+static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
+{
+#ifdef CONFIG_FDT
+    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
+                                    cpu_to_be32(binfo->ram_size) };
+    void *fdt = NULL;
+    char *filename;
+    int size, rc;
+
+    if (!current_dtb_filename) {
+        return 0;
+    }
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename);
+    if (!filename) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename);
+        return -1;
+    }
+
+    fdt = load_device_tree(filename, &size);
+    if (!fdt) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        g_free(filename);
+        return -1;
+    }
+    g_free(filename);
+
+    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /memory/reg\n");
+    }
+
+    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                      binfo->kernel_cmdline);
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    }
+
+    if (binfo->initrd_size) {
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                binfo->loader_start + INITRD_LOAD_ADDR);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+        }
+
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                    binfo->loader_start + INITRD_LOAD_ADDR +
+                    binfo->initrd_size);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+        }
+    }
+
+    cpu_physical_memory_write(addr, fdt, size);
+
+    return 0;
+
+#else
+    fprintf(stderr, "Platform requested a device tree, "
+                "but qemu was compiled without fdt support\n");
+    return -1;
+#endif
+}
+
 static void do_cpu_reset(void *opaque)
 {
     CPUState *env = opaque;
@@ -221,12 +289,14 @@ static void do_cpu_reset(void *opaque)
         } else {
             if (env == first_cpu) {
                 env->regs[15] = info->loader_start;
-                if (old_param) {
-                    set_kernel_args_old(info, info->initrd_size,
+                if (!current_dtb_filename) {
+                    if (old_param) {
+                        set_kernel_args_old(info, info->initrd_size,
+                                            info->loader_start);
+                    } else {
+                        set_kernel_args(info, info->initrd_size,
                                         info->loader_start);
-                } else {
-                    set_kernel_args(info, info->initrd_size,
-                                    info->loader_start);
+                    }
                 }
             } else {
                 info->secondary_cpu_reset_hook(env, info);
@@ -242,7 +312,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     int n;
     int is_linux = 0;
     uint64_t elf_entry;
-    target_phys_addr_t entry;
+    target_phys_addr_t entry, dtb_start;
     int big_endian;
 
     /* Load the kernel.  */
@@ -301,8 +371,23 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         } else {
             initrd_size = 0;
         }
+        info->initrd_size = initrd_size;
+
+        /* Place the DTB after the initrd in memory */
+        dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR +
+                                      initrd_size);
+        if (load_dtb(dtb_start, info)) {
+            exit(1);
+        }
+
         bootloader[4] = info->board_id;
-        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        /* for device tree boot, we pass the DTB directly in r2. Otherwise
+         * we point to the kernel args */
+        if (current_dtb_filename) {
+            bootloader[5] = dtb_start;
+        } else {
+            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        }
         bootloader[6] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
@@ -312,7 +397,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(env, info);
         }
-        info->initrd_size = initrd_size;
     }
     info->is_linux = is_linux;
 
diff --git a/hw/boards.h b/hw/boards.h
index f6d3784..d06776c 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -34,5 +34,6 @@ typedef struct QEMUMachine {
 int qemu_register_machine(QEMUMachine *m);
 
 extern QEMUMachine *current_machine;
+extern const char *current_dtb_filename;
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 3a07ae8..309a403 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
 first module.
 ETEXI
 
+DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
+    "-dtb    file    use 'file' as device tree image\n", QEMU_ARCH_ARM)
+STEXI
+@item -dtb @var{file}
+@findex -dtb
+Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
+on boot.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index d88a18c..b22eae3 100644
--- a/vl.c
+++ b/vl.c
@@ -1154,6 +1154,7 @@ void pcmcia_info(Monitor *mon)
 
 static QEMUMachine *first_machine = NULL;
 QEMUMachine *current_machine = NULL;
+const char *current_dtb_filename = NULL;
 
 int qemu_register_machine(QEMUMachine *m)
 {
@@ -2304,6 +2305,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_initrd:
                 initrd_filename = optarg;
                 break;
+            case QEMU_OPTION_dtb:
+                current_dtb_filename = optarg;
+                break;
             case QEMU_OPTION_hda:
                 {
                     char buf[256];
@@ -3241,6 +3245,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (!linux_boot && current_dtb_filename != NULL) {
+        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        exit(1);
+    }
+
     os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Edgar E. Iglesias]

On Sat, Jan 28, 2012 at 11:48:37AM -0700, Grant Likely wrote:
> On Fri, Jan 27, 2012 at 10:34:01PM +0000, Paul Brook wrote:
> > > If compiled with CONFIG_FDT, allow user to specify a device tree file using
> > > the -dtb argument.  If the machine supports it then the dtb will be loaded
> > > into memory and passed to the kernel on boot.
> > 
> > Adding annother machine feels wrong. Why does the board specific code need to 
> > know about this at all? You already going it via a global variable, so can't 
> > this be entirely contained within arm_boot.c?
> 
> Mostly because the infrastructure isn't yet in place to pass the .dtb file
> through to the arm_boot code (or maybe it is; what is the best way to pass
> command line data through to the arm_boot.c code (or similar for other
> architectures)?
> 
> > If the board file is involved, why is it asking the user?
> 
> There is a lot of configuration in the .dts file that the QEMU user may want
> to manipulate; particularly when using QEMU for testing embedded platforms.
> The direction I want to go is to select the machine model based on the top
> level DT compatible property (making -M optional), and then also allow a lot
> of the machine layout to be driven by DT data.  ie. populate emulated devices
> from DT data.
> 
> I believe this is how Edgar is using the microblaze model, but I don't
> think those patches have been upstreamed yet.  I hope that microblaze,
> ARM and powerpc can all use the same model here.

Yes, that's right.

> > 
> > > +    versatile_init(ram_size,
> > > +                   boot_device,
> > > +                   kernel_filename, kernel_cmdline,
> > > +                   initrd_filename, cpu_model, 0xffffffff);
> > 
> > This only works because we're currently too dumb to emulate the differences 
> > between the two board variants.
> 
> Yeah, this is a hack so I could play with forcing the machine id.
> I'll remove it.
> 
> > What we probably want to be doing is shipping/constructing device trees for 
> > the boards we implement, with an option to turn this on/off.  Requiring a user 
> > to invent their own seems deeply sub-optimal given we know exactly what 
> > hardware we're emulating.  A user that needs to provide their own FDT seems 
> > like a fairly rare corner case.
> 
> I disagree.  QEMU may want to ship stock .dts files, but it will
> absolutely be a common use case for embedded developers to pass in
> their own .dtb file.
>
> > This gets slightly more interesting when you have custom machine variants 
> > (i.e. once we fix the object model, and have proper dynamic machine 
> > construction).  Even then I'd expect the FDT to be derived from/specificed by 
> > the machine description, not a separate option.
> 
> I started with going down that route, but switched to this model after
> playing with it and noticing that it doesn't seem to fit as well for
> embedded development as providing a .dtb file and having QEMU
> construct a machine that matches the data.

That's the way I see it too. Our use-case is to build the virtual machine
from the dtb. We then run the exact same software that runs on real hw.
I don't know the details on how practical dtb's are for describing more
complex bus hierarchies, maybe Peter has more info on that.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Peter Maydell]

On 29 January 2012 18:48, Andreas Färber <afaerber@suse.de> wrote:
> Am 29.01.2012 17:01, schrieb Grant Likely:
>> Okay, well at least let's start with this.  Here is an updated version of the
>> patch that doesn't touch the board code.  It makes the -dtb option available
>> to all ARM platforms.
>>
>> Even if dtb is not the primary structure, we absolutely need this option.
>
> Well, the option itself is what has been the most controversial here.
> Yours is at least the third attempt, so far we've seen discussed -dtb,
> -kern-dtb, -kernel-dtb, -kernel dtb=, etc.

FWIW, my vote is for '-dtb' because it parallels the existing -initrd
and -append. (Or -devicetree if we think -dtb is too cryptic; but anyway
not something with 'kernel' in it.)

-- PMM

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Peter Crosthwaite]

[-- Attachment #1: Type: text/plain, Size: 5214 bytes --]

On Mon, Jan 30, 2012 at 6:42 AM, Edgar E. Iglesias <edgar.iglesias@gmail.com
> wrote:

> On Sat, Jan 28, 2012 at 11:48:37AM -0700, Grant Likely wrote:
> > On Fri, Jan 27, 2012 at 10:34:01PM +0000, Paul Brook wrote:
> > > > If compiled with CONFIG_FDT, allow user to specify a device tree
> file using
> > > > the -dtb argument.  If the machine supports it then the dtb will be
> loaded
> > > > into memory and passed to the kernel on boot.
> > >
> > > Adding annother machine feels wrong. Why does the board specific code
> need to
> > > know about this at all? You already going it via a global variable, so
> can't
> > > this be entirely contained within arm_boot.c?
> >
> > Mostly because the infrastructure isn't yet in place to pass the .dtb
> file
> > through to the arm_boot code (or maybe it is; what is the best way to
> pass
> > command line data through to the arm_boot.c code (or similar for other
> > architectures)?
> >
> > > If the board file is involved, why is it asking the user?
> >
> > There is a lot of configuration in the .dts file that the QEMU user may
> want
> > to manipulate; particularly when using QEMU for testing embedded
> platforms.
> > The direction I want to go is to select the machine model based on the
> top
> > level DT compatible property (making -M optional), and then also allow a
> lot
> > of the machine layout to be driven by DT data.  ie. populate emulated
> devices
> > from DT data.
> >
> > I believe this is how Edgar is using the microblaze model, but I don't
> > think those patches have been upstreamed yet.  I hope that microblaze,
> > ARM and powerpc can all use the same model here.
>
> Yes, that's right.
>
> > >
> > > > +    versatile_init(ram_size,
> > > > +                   boot_device,
> > > > +                   kernel_filename, kernel_cmdline,
> > > > +                   initrd_filename, cpu_model, 0xffffffff);
> > >
> > > This only works because we're currently too dumb to emulate the
> differences
> > > between the two board variants.
> >
> > Yeah, this is a hack so I could play with forcing the machine id.
> > I'll remove it.
> >
> > > What we probably want to be doing is shipping/constructing device
> trees for
> > > the boards we implement, with an option to turn this on/off.
>  Requiring a user
> > > to invent their own seems deeply sub-optimal given we know exactly what
> > > hardware we're emulating.  A user that needs to provide their own FDT
> seems
> > > like a fairly rare corner case.
>
>
> > I disagree.  QEMU may want to ship stock .dts files, but it will
> > absolutely be a common use case for embedded developers to pass in
> > their own .dtb file.
> >
> > > This gets slightly more interesting when you have custom machine
> variants
> > > (i.e. once we fix the object model, and have proper dynamic machine
> > > construction).  Even then I'd expect the FDT to be derived
> from/specificed by
> > > the machine description, not a separate option.
>
>
> > I started with going down that route, but switched to this model after
> > playing with it and noticing that it doesn't seem to fit as well for
> > embedded development as providing a .dtb file and having QEMU
> > construct a machine that matches the data.
>

Another major issue with that is DTBs have little bits of software in them
as well, such as the "chosen" node, which would need to be populated before
passing off to software. Coding these dtb properties into machine models
would be sub-optimal, you would need some way of parameterizing certain dtb
properties at boot time. The easiest approach to this is just passing in
your own dtb file.

>
> That's the way I see it too. Our use-case is to build the virtual machine
> from the dtb. We then run the exact same software that runs on real hw.
> I don't know the details on how practical dtb's are for describing more
> complex bus hierarchies, maybe Peter has more info on that.
>
> Dtbs are capable of describing pretty much anything you want, the main
issue is ambiguity on how the dts data structure is used to describe
complex or irregular bus heirachies. Im our approach, device models
themselves describe how dts syntax is interpreted wrt to machine creation.
This means different device models can interpret dts node handles or
subnode relationships in any arbitrary way, which would proivde you a
backend with which you could implement complex bus heirachies. For example,
a device tree node with compatibility "simple-bus" will instantiate all its
subnodes as elements on the parent bus (or system bus in the case of the
dtb root node). There is however a software hook which would allow you to
implement more complex behaviours such as address range remapping for bus
bridges (based on the ranges property). We didnt do this, as the memory API
wasnt adequate at the time, but recent updates would now make this
possible. As another example, a USB controller could interpret its children
as USB devices and create the corresponding USB device model interconnects.
We successfully prototyped this idea out of tree for SPI busses, but the
mechanism is there to do USB, PCI, SD and the other usual suspects. Just
needs to be implemented and the framework reworked for QOM.

Cheers,
> Edgar
>

[-- Attachment #2: Type: text/html, Size: 6575 bytes --]

Re: [Qemu-devel] [PATCH] arm: add device tree support

[John Williams]

On Mon, Jan 30, 2012 at 5:13 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 29 January 2012 16:01, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Sun, Jan 29, 2012 at 11:15:42AM +0000, Paul Brook wrote:
>>> Clearly we need to have some sort of FDT support.  However I'm unconvinced
>>> that it's the correct format for the primary data structure.  For one thing
>>> it's a hierarchical tree structure, and in my experience describing links
>>> outside that structure gets hairy.  One of the things we're doing with QOM is
>>> move from a hierarchical tree/bus to a more general connected web of links
>>> between devices.
>>
>> Okay, well at least let's start with this.  Here is an updated version of the
>> patch that doesn't touch the board code.  It makes the -dtb option available
>> to all ARM platforms.
>>
>> Even if dtb is not the primary structure, we absolutely need this option.
>> Developers using qemu to simulate real hardware are going to still want
>> to pass in the exact same .dtb file to qemu as they use for the real target,
>> and those .dtb files are maintained completely separately from qemu.
>
> I agree with Paul that trying to use the dtb as the primary structure
> for describing a machine model to qemu is decidedly problematic. Let's
> start with the basic "we're a bootloader, provide minimal support for
> passing in a device tree blob"...

We've never advocated DTB as primary structure, but rather just one
very useful, simple and generic approach.

Among the the sea of fixed machine models that is QEMU today, having a
DTB-driven machine in there would be an excellent proving ground for
QOM conversion and steps towards fully data driven machines which
should be the ultimate objective anyway.

As Peter C says elsewhere in this thread, through its link syntax
foo=<&ref> you can describe any sort of topology you like in a device
tree.

Regards,

John
-- 
John Williams, PhD, B. Eng, B. IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com  p: +61-7-30090663  f: +61-7-30090663

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Andreas Färber]

Am 29.01.2012 21:36, schrieb Grant Likely:
> On Sun, Jan 29, 2012 at 07:13:55PM +0000, Peter Maydell wrote:
>> On 29 January 2012 16:01, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
>>> +    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)
>>
>> Needs more spaces to make it line up right in -help output:
>> Linux/Multiboot boot specific:
>> -kernel bzImage use 'bzImage' as kernel image
>> -append cmdline use 'cmdline' as kernel command line
>> -initrd file    use 'file' as initial ram disk
>> -dtb file use 'file' as a device tree image
> 
> Fixed.
> 
> New patch below...
> 
> ---
> diff --git a/Makefile.target b/Makefile.target
> index 68481a3..5e465ec 100644
> --- a/Makefile.target
> +++ b/Makefile.target
[snip]

Please always post patches using git-send-email and indicate the version
number as [PATCH vX] (opinions are divided whether as reply or as
top-level). The changes should be described below the commit message (or
in a cover letter).

http://wiki.qemu.org/Contribute/SubmitAPatch

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Mon, Jan 30, 2012 at 12:36:40PM +0100, Andreas Färber wrote:
> Am 29.01.2012 21:36, schrieb Grant Likely:
> > On Sun, Jan 29, 2012 at 07:13:55PM +0000, Peter Maydell wrote:
> >> On 29 January 2012 16:01, Grant Likely <grant.likely@secretlab.ca> wrote:
> >>> +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
> >>> +    "-dtb file use 'file' as a device tree image\n", QEMU_ARCH_ARM)
> >>
> >> Needs more spaces to make it line up right in -help output:
> >> Linux/Multiboot boot specific:
> >> -kernel bzImage use 'bzImage' as kernel image
> >> -append cmdline use 'cmdline' as kernel command line
> >> -initrd file    use 'file' as initial ram disk
> >> -dtb file use 'file' as a device tree image
> > 
> > Fixed.
> > 
> > New patch below...
> > 
> > ---
> > diff --git a/Makefile.target b/Makefile.target
> > index 68481a3..5e465ec 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> [snip]
> 
> Please always post patches using git-send-email and indicate the version
> number as [PATCH vX] (opinions are divided whether as reply or as
> top-level). The changes should be described below the commit message (or
> in a cover letter).
> 
> http://wiki.qemu.org/Contribute/SubmitAPatch
> 
> Andreas

I was really only including the new versions in my replies to shorten
up the review cycle.  I'll repost correctly when it looks like all
comments are resolved.

g.

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Sun, Jan 29, 2012 at 09:29:58PM +0000, Peter Maydell wrote:
> On 29 January 2012 18:48, Andreas Färber <afaerber@suse.de> wrote:
> > Am 29.01.2012 17:01, schrieb Grant Likely:
> >> Okay, well at least let's start with this.  Here is an updated version of the
> >> patch that doesn't touch the board code.  It makes the -dtb option available
> >> to all ARM platforms.
> >>
> >> Even if dtb is not the primary structure, we absolutely need this option.
> >
> > Well, the option itself is what has been the most controversial here.
> > Yours is at least the third attempt, so far we've seen discussed -dtb,
> > -kern-dtb, -kernel-dtb, -kernel dtb=, etc.
> 
> FWIW, my vote is for '-dtb' because it parallels the existing -initrd
> and -append. (Or -devicetree if we think -dtb is too cryptic; but anyway
> not something with 'kernel' in it.)

That was my reasoning too.  I'm not stuck on any particular flag though as
long as the feature gets merged.  :-)

g.

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Grant Likely]

On Mon, Jan 30, 2012 at 09:54:45AM +1000, Peter Crosthwaite wrote:
> On Mon, Jan 30, 2012 at 6:42 AM, Edgar E. Iglesias <edgar.iglesias@gmail.com
> > On Sat, Jan 28, 2012 at 11:48:37AM -0700, Grant Likely wrote:
> > > On Fri, Jan 27, 2012 at 10:34:01PM +0000, Paul Brook wrote:
> Another major issue with that is DTBs have little bits of software in them
> as well, such as the "chosen" node, which would need to be populated before
> passing off to software. Coding these dtb properties into machine models
> would be sub-optimal, you would need some way of parameterizing certain dtb
> properties at boot time. The easiest approach to this is just passing in
> your own dtb file.

I wouldn't say it that way.  dtbs don't contain any sort of executable
code.  It is only data.  However, it is true that the bootloader is expected
to update properties in the /chosen node for the kernel command line and
initrd base address, to update the /memory node, and on some systems to
set the Ethernet mac address properties if that is not discoverable from
the simulated HW model.

Since that data is knowledge held by QEMU internally, it is appropriate
for QEMU to modify the dtb with that data, and this patch does exactly that.

g.

[Qemu-devel] [PATCH] arm: add device tree support

[Peter Maydell]

From: Grant Likely <grant.likely@secretlab.ca>

If compiled with CONFIG_FDT, allow user to specify a device tree file using
the -dtb argument.  If the machine supports it then the dtb will be loaded
into memory and passed to the kernel on boot.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
[Peter Maydell: Use machine opt rather than global to pass dtb filename]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
As http://patchwork.ozlabs.org/patch/140066/ (4/4 in that series) but
 * rebased on current master
 * fixed a bug I accidentally introduced that meant we never passed the
   initrd to the kernel if using device tree

Anthony and Andrzej between them have applied patches 1-3 of that series
so as Anthony suggests I'll put this one in my next arm-devs pullreq...

 Makefile.target |    1 +
 configure       |    1 +
 hw/arm-misc.h   |    1 +
 hw/arm_boot.c   |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 qemu-config.c   |    4 ++
 qemu-options.hx |    9 +++++
 vl.c            |    9 +++++
 7 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index d5eb70d..f7c1d51 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -372,6 +372,7 @@ obj-arm-y += vexpress.o
 obj-arm-y += strongarm.o
 obj-arm-y += collie.o
 obj-arm-y += pl041.o lm4549.o
+obj-arm-$(CONFIG_FDT) += device_tree.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/configure b/configure
index 037f7f7..1a5c8c7 100755
--- a/configure
+++ b/configure
@@ -3457,6 +3457,7 @@ case "$target_arch2" in
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
     target_phys_bits=32
     target_llong_alignment=4
+    target_libs_softmmu="$fdt_libs"
   ;;
   cris)
     target_nptl="yes"
diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 306013a..734bd82 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -29,6 +29,7 @@ struct arm_boot_info {
     const char *kernel_filename;
     const char *kernel_cmdline;
     const char *initrd_filename;
+    const char *dtb_filename;
     target_phys_addr_t loader_start;
     /* multicore boards that use the default secondary core boot functions
      * need to put the address of the secondary boot code, the boot reg,
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 2ef25ca..6705882 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -7,11 +7,14 @@
  * This code is licensed under the GPL.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "arm-misc.h"
 #include "sysemu.h"
+#include "boards.h"
 #include "loader.h"
 #include "elf.h"
+#include "device_tree.h"
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
@@ -208,6 +211,67 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
     }
 }
 
+static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
+{
+#ifdef CONFIG_FDT
+    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
+                                    cpu_to_be32(binfo->ram_size) };
+    void *fdt = NULL;
+    char *filename;
+    int size, rc;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
+    if (!filename) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
+        return -1;
+    }
+
+    fdt = load_device_tree(filename, &size);
+    if (!fdt) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        g_free(filename);
+        return -1;
+    }
+    g_free(filename);
+
+    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /memory/reg\n");
+    }
+
+    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                      binfo->kernel_cmdline);
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    }
+
+    if (binfo->initrd_size) {
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                binfo->loader_start + INITRD_LOAD_ADDR);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+        }
+
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                    binfo->loader_start + INITRD_LOAD_ADDR +
+                    binfo->initrd_size);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+        }
+    }
+
+    cpu_physical_memory_write(addr, fdt, size);
+
+    return 0;
+
+#else
+    fprintf(stderr, "Device tree requested, "
+                "but qemu was compiled without fdt support\n");
+    return -1;
+#endif
+}
+
 static void do_cpu_reset(void *opaque)
 {
     CPUState *env = opaque;
@@ -222,10 +286,12 @@ static void do_cpu_reset(void *opaque)
         } else {
             if (env == first_cpu) {
                 env->regs[15] = info->loader_start;
-                if (old_param) {
-                    set_kernel_args_old(info);
-                } else {
-                    set_kernel_args(info);
+                if (!info->dtb_filename) {
+                    if (old_param) {
+                        set_kernel_args_old(info);
+                    } else {
+                        set_kernel_args(info);
+                    }
                 }
             } else {
                 info->secondary_cpu_reset_hook(env, info);
@@ -250,6 +316,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         exit(1);
     }
 
+    info->dtb_filename = qemu_opt_get(qemu_opts_find(qemu_find_opts("machine"),
+                                                     0), "dtb");
+
     if (!info->secondary_cpu_reset_hook) {
         info->secondary_cpu_reset_hook = default_reset_secondary;
     }
@@ -300,8 +369,25 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         } else {
             initrd_size = 0;
         }
+        info->initrd_size = initrd_size;
+
         bootloader[4] = info->board_id;
-        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+
+        /* for device tree boot, we pass the DTB directly in r2. Otherwise
+         * we point to the kernel args.
+         */
+        if (info->dtb_filename) {
+            /* Place the DTB after the initrd in memory */
+            target_phys_addr_t dtb_start = TARGET_PAGE_ALIGN(info->loader_start
+                                                             + INITRD_LOAD_ADDR
+                                                             + initrd_size);
+            if (load_dtb(dtb_start, info)) {
+                exit(1);
+            }
+            bootloader[5] = dtb_start;
+        } else {
+            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        }
         bootloader[6] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
@@ -311,7 +397,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(env, info);
         }
-        info->initrd_size = initrd_size;
     }
     info->is_linux = is_linux;
 
diff --git a/qemu-config.c b/qemu-config.c
index 7d9da78..be84a03 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -578,6 +578,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "append",
             .type = QEMU_OPT_STRING,
             .help = "Linux kernel command line",
+        }, {
+            .name = "dtb",
+            .type = QEMU_OPT_STRING,
+            .help = "Linux kernel device tree file",
         },
         { /* End of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index b129996..e38799c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2037,6 +2037,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
 first module.
 ETEXI
 
+DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
+    "-dtb    file    use 'file' as device tree image\n", QEMU_ARCH_ARM)
+STEXI
+@item -dtb @var{file}
+@findex -dtb
+Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
+on boot.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index 7a8cc08..050f22e 100644
--- a/vl.c
+++ b/vl.c
@@ -2453,6 +2453,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_append:
                 qemu_opts_set(qemu_find_opts("machine"), 0, "append", optarg);
                 break;
+            case QEMU_OPTION_dtb:
+                qemu_opts_set(qemu_find_opts("machine"), 0, "dtb", optarg);
+                break;
             case QEMU_OPTION_cdrom:
                 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
                 break;
@@ -3269,6 +3272,12 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (!linux_boot
+        && qemu_opt_get(qemu_opts_find(qemu_find_opts("machine"), 0), "dtb")) {
+        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        exit(1);
+    }
+
     os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {
-- 
1.7.1

[Qemu-devel] [PATCH] arm: add device tree support

[Peter Maydell]

From: Grant Likely <grant.likely@secretlab.ca>

If compiled with CONFIG_FDT, allow user to specify a device tree file using
the -dtb argument.  If the machine supports it then the dtb will be loaded
into memory and passed to the kernel on boot.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
[Peter Maydell: Use machine opt rather than global to pass dtb filename]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2: fix potential NULL deref if qemu_opts_find() returns NULL

 Makefile.target |    1 +
 configure       |    1 +
 hw/arm-misc.h   |    1 +
 hw/arm_boot.c   |  102 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 qemu-config.c   |    4 ++
 qemu-options.hx |    9 +++++
 vl.c            |    8 ++++
 7 files changed, 120 insertions(+), 6 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 68a5641..6a56b3e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -373,6 +373,7 @@ obj-arm-y += vexpress.o
 obj-arm-y += strongarm.o
 obj-arm-y += collie.o
 obj-arm-y += pl041.o lm4549.o
+obj-arm-$(CONFIG_FDT) += device_tree.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/configure b/configure
index f9d5330..7d509e8 100755
--- a/configure
+++ b/configure
@@ -3472,6 +3472,7 @@ case "$target_arch2" in
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
     target_phys_bits=32
     target_llong_alignment=4
+    target_libs_softmmu="$fdt_libs"
   ;;
   cris)
     target_nptl="yes"
diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 306013a..734bd82 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -29,6 +29,7 @@ struct arm_boot_info {
     const char *kernel_filename;
     const char *kernel_cmdline;
     const char *initrd_filename;
+    const char *dtb_filename;
     target_phys_addr_t loader_start;
     /* multicore boards that use the default secondary core boot functions
      * need to put the address of the secondary boot code, the boot reg,
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 2ef25ca..fc66910 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -7,11 +7,14 @@
  * This code is licensed under the GPL.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "arm-misc.h"
 #include "sysemu.h"
+#include "boards.h"
 #include "loader.h"
 #include "elf.h"
+#include "device_tree.h"
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
@@ -208,6 +211,67 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
     }
 }
 
+static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
+{
+#ifdef CONFIG_FDT
+    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
+                                    cpu_to_be32(binfo->ram_size) };
+    void *fdt = NULL;
+    char *filename;
+    int size, rc;
+
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
+    if (!filename) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
+        return -1;
+    }
+
+    fdt = load_device_tree(filename, &size);
+    if (!fdt) {
+        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        g_free(filename);
+        return -1;
+    }
+    g_free(filename);
+
+    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                               sizeof(mem_reg_property));
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /memory/reg\n");
+    }
+
+    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                      binfo->kernel_cmdline);
+    if (rc < 0) {
+        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    }
+
+    if (binfo->initrd_size) {
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                binfo->loader_start + INITRD_LOAD_ADDR);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+        }
+
+        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                    binfo->loader_start + INITRD_LOAD_ADDR +
+                    binfo->initrd_size);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+        }
+    }
+
+    cpu_physical_memory_write(addr, fdt, size);
+
+    return 0;
+
+#else
+    fprintf(stderr, "Device tree requested, "
+                "but qemu was compiled without fdt support\n");
+    return -1;
+#endif
+}
+
 static void do_cpu_reset(void *opaque)
 {
     CPUState *env = opaque;
@@ -222,10 +286,12 @@ static void do_cpu_reset(void *opaque)
         } else {
             if (env == first_cpu) {
                 env->regs[15] = info->loader_start;
-                if (old_param) {
-                    set_kernel_args_old(info);
-                } else {
-                    set_kernel_args(info);
+                if (!info->dtb_filename) {
+                    if (old_param) {
+                        set_kernel_args_old(info);
+                    } else {
+                        set_kernel_args(info);
+                    }
                 }
             } else {
                 info->secondary_cpu_reset_hook(env, info);
@@ -243,6 +309,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     uint64_t elf_entry;
     target_phys_addr_t entry;
     int big_endian;
+    QemuOpts *machine_opts;
 
     /* Load the kernel.  */
     if (!info->kernel_filename) {
@@ -250,6 +317,13 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         exit(1);
     }
 
+    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+    if (machine_opts) {
+        info->dtb_filename = qemu_opt_get(machine_opts, "dtb");
+    } else {
+        info->dtb_filename = NULL;
+    }
+
     if (!info->secondary_cpu_reset_hook) {
         info->secondary_cpu_reset_hook = default_reset_secondary;
     }
@@ -300,8 +374,25 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         } else {
             initrd_size = 0;
         }
+        info->initrd_size = initrd_size;
+
         bootloader[4] = info->board_id;
-        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+
+        /* for device tree boot, we pass the DTB directly in r2. Otherwise
+         * we point to the kernel args.
+         */
+        if (info->dtb_filename) {
+            /* Place the DTB after the initrd in memory */
+            target_phys_addr_t dtb_start = TARGET_PAGE_ALIGN(info->loader_start
+                                                             + INITRD_LOAD_ADDR
+                                                             + initrd_size);
+            if (load_dtb(dtb_start, info)) {
+                exit(1);
+            }
+            bootloader[5] = dtb_start;
+        } else {
+            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
+        }
         bootloader[6] = entry;
         for (n = 0; n < sizeof(bootloader) / 4; n++) {
             bootloader[n] = tswap32(bootloader[n]);
@@ -311,7 +402,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
         if (info->nb_cpus > 1) {
             info->write_secondary_boot(env, info);
         }
-        info->initrd_size = initrd_size;
     }
     info->is_linux = is_linux;
 
diff --git a/qemu-config.c b/qemu-config.c
index 7d9da78..be84a03 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -578,6 +578,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "append",
             .type = QEMU_OPT_STRING,
             .help = "Linux kernel command line",
+        }, {
+            .name = "dtb",
+            .type = QEMU_OPT_STRING,
+            .help = "Linux kernel device tree file",
         },
         { /* End of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index b129996..e38799c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2037,6 +2037,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
 first module.
 ETEXI
 
+DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
+    "-dtb    file    use 'file' as device tree image\n", QEMU_ARCH_ARM)
+STEXI
+@item -dtb @var{file}
+@findex -dtb
+Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
+on boot.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/vl.c b/vl.c
index 1d4c350..2d3347d 100644
--- a/vl.c
+++ b/vl.c
@@ -2526,6 +2526,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_append:
                 qemu_opts_set(qemu_find_opts("machine"), 0, "append", optarg);
                 break;
+            case QEMU_OPTION_dtb:
+                qemu_opts_set(qemu_find_opts("machine"), 0, "dtb", optarg);
+                break;
             case QEMU_OPTION_cdrom:
                 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
                 break;
@@ -3345,6 +3348,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (!linux_boot && machine_opts && qemu_opt_get(machine_opts, "dtb")) {
+        fprintf(stderr, "-dtb only allowed with -kernel option\n");
+        exit(1);
+    }
+
     os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {
-- 
1.7.1

Re: [Qemu-devel] [PATCH] arm: add device tree support

[Anthony Liguori]

On 02/27/2012 11:38 AM, Peter Maydell wrote:
> From: Grant Likely<grant.likely@secretlab.ca>
>
> If compiled with CONFIG_FDT, allow user to specify a device tree file using
> the -dtb argument.  If the machine supports it then the dtb will be loaded
> into memory and passed to the kernel on boot.
>
> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
> Signed-off-by: Grant Likely<grant.likely@secretlab.ca>
> [Peter Maydell: Use machine opt rather than global to pass dtb filename]
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Thanks for doing the work to wrap this in QemuOpts.  This was a nice improvement.

Regards,

Anthony Liguori

> ---
> v1->v2: fix potential NULL deref if qemu_opts_find() returns NULL
>
>   Makefile.target |    1 +
>   configure       |    1 +
>   hw/arm-misc.h   |    1 +
>   hw/arm_boot.c   |  102 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>   qemu-config.c   |    4 ++
>   qemu-options.hx |    9 +++++
>   vl.c            |    8 ++++
>   7 files changed, 120 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile.target b/Makefile.target
> index 68a5641..6a56b3e 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -373,6 +373,7 @@ obj-arm-y += vexpress.o
>   obj-arm-y += strongarm.o
>   obj-arm-y += collie.o
>   obj-arm-y += pl041.o lm4549.o
> +obj-arm-$(CONFIG_FDT) += device_tree.o
>
>   obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
>   obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
> diff --git a/configure b/configure
> index f9d5330..7d509e8 100755
> --- a/configure
> +++ b/configure
> @@ -3472,6 +3472,7 @@ case "$target_arch2" in
>       gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
>       target_phys_bits=32
>       target_llong_alignment=4
> +    target_libs_softmmu="$fdt_libs"
>     ;;
>     cris)
>       target_nptl="yes"
> diff --git a/hw/arm-misc.h b/hw/arm-misc.h
> index 306013a..734bd82 100644
> --- a/hw/arm-misc.h
> +++ b/hw/arm-misc.h
> @@ -29,6 +29,7 @@ struct arm_boot_info {
>       const char *kernel_filename;
>       const char *kernel_cmdline;
>       const char *initrd_filename;
> +    const char *dtb_filename;
>       target_phys_addr_t loader_start;
>       /* multicore boards that use the default secondary core boot functions
>        * need to put the address of the secondary boot code, the boot reg,
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 2ef25ca..fc66910 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -7,11 +7,14 @@
>    * This code is licensed under the GPL.
>    */
>
> +#include "config.h"
>   #include "hw.h"
>   #include "arm-misc.h"
>   #include "sysemu.h"
> +#include "boards.h"
>   #include "loader.h"
>   #include "elf.h"
> +#include "device_tree.h"
>
>   #define KERNEL_ARGS_ADDR 0x100
>   #define KERNEL_LOAD_ADDR 0x00010000
> @@ -208,6 +211,67 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>       }
>   }
>
> +static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
> +{
> +#ifdef CONFIG_FDT
> +    uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start),
> +                                    cpu_to_be32(binfo->ram_size) };
> +    void *fdt = NULL;
> +    char *filename;
> +    int size, rc;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
> +    if (!filename) {
> +        fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
> +        return -1;
> +    }
> +
> +    fdt = load_device_tree(filename,&size);
> +    if (!fdt) {
> +        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
> +        g_free(filename);
> +        return -1;
> +    }
> +    g_free(filename);
> +
> +    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
> +                               sizeof(mem_reg_property));
> +    if (rc<  0) {
> +        fprintf(stderr, "couldn't set /memory/reg\n");
> +    }
> +
> +    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
> +                                      binfo->kernel_cmdline);
> +    if (rc<  0) {
> +        fprintf(stderr, "couldn't set /chosen/bootargs\n");
> +    }
> +
> +    if (binfo->initrd_size) {
> +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> +                binfo->loader_start + INITRD_LOAD_ADDR);
> +        if (rc<  0) {
> +            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> +        }
> +
> +        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +                    binfo->loader_start + INITRD_LOAD_ADDR +
> +                    binfo->initrd_size);
> +        if (rc<  0) {
> +            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> +        }
> +    }
> +
> +    cpu_physical_memory_write(addr, fdt, size);
> +
> +    return 0;
> +
> +#else
> +    fprintf(stderr, "Device tree requested, "
> +                "but qemu was compiled without fdt support\n");
> +    return -1;
> +#endif
> +}
> +
>   static void do_cpu_reset(void *opaque)
>   {
>       CPUState *env = opaque;
> @@ -222,10 +286,12 @@ static void do_cpu_reset(void *opaque)
>           } else {
>               if (env == first_cpu) {
>                   env->regs[15] = info->loader_start;
> -                if (old_param) {
> -                    set_kernel_args_old(info);
> -                } else {
> -                    set_kernel_args(info);
> +                if (!info->dtb_filename) {
> +                    if (old_param) {
> +                        set_kernel_args_old(info);
> +                    } else {
> +                        set_kernel_args(info);
> +                    }
>                   }
>               } else {
>                   info->secondary_cpu_reset_hook(env, info);
> @@ -243,6 +309,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>       uint64_t elf_entry;
>       target_phys_addr_t entry;
>       int big_endian;
> +    QemuOpts *machine_opts;
>
>       /* Load the kernel.  */
>       if (!info->kernel_filename) {
> @@ -250,6 +317,13 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>           exit(1);
>       }
>
> +    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> +    if (machine_opts) {
> +        info->dtb_filename = qemu_opt_get(machine_opts, "dtb");
> +    } else {
> +        info->dtb_filename = NULL;
> +    }
> +
>       if (!info->secondary_cpu_reset_hook) {
>           info->secondary_cpu_reset_hook = default_reset_secondary;
>       }
> @@ -300,8 +374,25 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>           } else {
>               initrd_size = 0;
>           }
> +        info->initrd_size = initrd_size;
> +
>           bootloader[4] = info->board_id;
> -        bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> +
> +        /* for device tree boot, we pass the DTB directly in r2. Otherwise
> +         * we point to the kernel args.
> +         */
> +        if (info->dtb_filename) {
> +            /* Place the DTB after the initrd in memory */
> +            target_phys_addr_t dtb_start = TARGET_PAGE_ALIGN(info->loader_start
> +                                                             + INITRD_LOAD_ADDR
> +                                                             + initrd_size);
> +            if (load_dtb(dtb_start, info)) {
> +                exit(1);
> +            }
> +            bootloader[5] = dtb_start;
> +        } else {
> +            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> +        }
>           bootloader[6] = entry;
>           for (n = 0; n<  sizeof(bootloader) / 4; n++) {
>               bootloader[n] = tswap32(bootloader[n]);
> @@ -311,7 +402,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>           if (info->nb_cpus>  1) {
>               info->write_secondary_boot(env, info);
>           }
> -        info->initrd_size = initrd_size;
>       }
>       info->is_linux = is_linux;
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 7d9da78..be84a03 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -578,6 +578,10 @@ static QemuOptsList qemu_machine_opts = {
>               .name = "append",
>               .type = QEMU_OPT_STRING,
>               .help = "Linux kernel command line",
> +        }, {
> +            .name = "dtb",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Linux kernel device tree file",
>           },
>           { /* End of list */ }
>       },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b129996..e38799c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2037,6 +2037,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the
>   first module.
>   ETEXI
>
> +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \
> +    "-dtb    file    use 'file' as device tree image\n", QEMU_ARCH_ARM)
> +STEXI
> +@item -dtb @var{file}
> +@findex -dtb
> +Use @var{file} as a device tree binary (dtb) image and pass it to the kernel
> +on boot.
> +ETEXI
> +
>   STEXI
>   @end table
>   ETEXI
> diff --git a/vl.c b/vl.c
> index 1d4c350..2d3347d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2526,6 +2526,9 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_append:
>                   qemu_opts_set(qemu_find_opts("machine"), 0, "append", optarg);
>                   break;
> +            case QEMU_OPTION_dtb:
> +                qemu_opts_set(qemu_find_opts("machine"), 0, "dtb", optarg);
> +                break;
>               case QEMU_OPTION_cdrom:
>                   drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
>                   break;
> @@ -3345,6 +3348,11 @@ int main(int argc, char **argv, char **envp)
>           exit(1);
>       }
>
> +    if (!linux_boot&&  machine_opts&&  qemu_opt_get(machine_opts, "dtb")) {
> +        fprintf(stderr, "-dtb only allowed with -kernel option\n");
> +        exit(1);
> +    }
> +
>       os_set_line_buffering();
>
>       if (init_timer_alarm()<  0) {