[Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Peter Maydell]

This patch series adds a 'virt' platform which uses the
kernel's mach-virt (fully device-tree driven) support
to create a simple minimalist platform intended for
use for KVM VM guests.

The major change here is that I've added a PL011 UART.

Sample command line:

 qemu-system-arm -machine type=virt -display none \
  -kernel zImage \
  -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
  -cpu cortex-a15 \
  -device virtio-blk-device,drive=foo \
  -drive if=none,file=arm-wheezy.img,id=foo \
  -m 2048 -serial stdio

Note that there is no earlyprintk via the PL011 because
there's no defined device tree binding for "hey, here
is your earlyprintk UART".


*** NOTE *** to get the PL011 to work you'll need to
tweak the kernel a bit:

diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
index b184e57..2b6aceb 100644
--- a/arch/arm/mach-virt/virt.c
+++ b/arch/arm/mach-virt/virt.c
@@ -21,11 +21,13 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/smp.h>
+#include <linux/clk-provider.h>
 
 #include <asm/mach/arch.h>
 
 static void __init virt_init(void)
 {
+       of_clk_init(NULL);
        of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 

Otherwise the kernel doesn't ever add the clock to its
list, and then it refuses to probe for the PL011.
(I'm told this isn't really the right fix, though, and
ideally the call should be done in some generic location
rather than in every machine's init function.)

The alternative would be for the kernel to be fixed to
follow its own device tree binding documentation and not
require clocks/clock-names properties on the pl011 node.


Changes from John Rigby's v3->my v4:
 * renamed user-facing machine to just "virt"
 * removed the A9 support (it can't work since the A9 has no
   generic timers)
 * added virtio-mmio transports instead of random set of 'soc' devices
 * instead of updating io_base as we step through adding devices,
   define a memory map with an array (similar to vexpress)
 * folded in some minor fixes from John's aarch64-support patch
 * rather than explicitly doing endian-swapping on FDT cells,
   use fdt APIs that let us just pass in host-endian values
   and let the fdt layer take care of the swapping
 * miscellaneous minor code cleanups and style fixes
Changes v4->v5:
 * removed outdated TODO remarks from commit messages
Changes v5->v6:
 * adjusted the memory map as per Anup's review comments
   (actually made the changes this time!)
Changes v6->v7:
 * added a PL011 UART, at Alex's suggestion (and the accompanying
   fake clock dtb node that this requires)
 * added an irqmap[] in parallel with the memmap[] so that our
   assignment of devices to irq lines is neatly in one place
 * the removal of arm_pic allows us to get rid of an irritating
   array sized to the number of CPUs
 * included the "terminate dtb reservemap" patch since it's a
   dependency to get the kernel to boot

John Rigby (1):
  hw/arm/boot: Allow boards to provide an fdt blob

Peter Maydell (2):
  device_tree.c: Terminate the empty reservemap in create_device_tree()
  hw/arm: Add 'virt' platform

 device_tree.c        |    4 +
 hw/arm/Makefile.objs |    2 +-
 hw/arm/boot.c        |   32 ++--
 hw/arm/virt.c        |  419 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/arm.h |    7 +
 5 files changed, 451 insertions(+), 13 deletions(-)
 create mode 100644 hw/arm/virt.c

-- 
1.7.9.5

[Qemu-devel] [PATCH v7 1/3] device_tree.c: Terminate the empty reservemap in create_device_tree()

[Peter Maydell]

Device trees created with create_device_tree() may not have any
entries in their reservemap, because the FDT API requires that the
reservemap is completed before any FDT nodes are added, and
create_device_tree() itself creates a node.  However we were not
calling fdt_finish_reservemap(), which meant that there was no
terminator in the reservemap list and whatever happened to be at the
start of the FDT data section would end up being interpreted as
reservemap entries.  Avoid this by calling fdt_finish_reservemap()
to add the terminator.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index ffec99a..391da8c 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -41,6 +41,10 @@ void *create_device_tree(int *sizep)
     if (ret < 0) {
         goto fail;
     }
+    ret = fdt_finish_reservemap(fdt);
+    if (ret < 0) {
+        goto fail;
+    }
     ret = fdt_begin_node(fdt, "");
     if (ret < 0) {
         goto fail;
-- 
1.7.9.5

[Qemu-devel] [PATCH v7 2/3] hw/arm/boot: Allow boards to provide an fdt blob

[Peter Maydell]

From: John Rigby <john.rigby@linaro.org>

If no fdt is provided on command line and the new field
get_dtb in struct arm_boot_info is set then call it to
get a device tree blob.

Signed-off-by: John Rigby <john.rigby@linaro.org>
[PMM: minor tweaks and cleanup]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c        |   32 ++++++++++++++++++++------------
 include/hw/arm/arm.h |    7 +++++++
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e313af..967397b 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -228,23 +228,31 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
 static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
 {
     void *fdt = NULL;
-    char *filename;
     int size, rc;
     uint32_t acells, scells;
 
-    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);
-        goto fail;
-    }
+    if (binfo->dtb_filename) {
+        char *filename;
+        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);
+            goto fail;
+        }
 
-    fdt = load_device_tree(filename, &size);
-    if (!fdt) {
-        fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+        fdt = load_device_tree(filename, &size);
+        if (!fdt) {
+            fprintf(stderr, "Couldn't open dtb file %s\n", filename);
+            g_free(filename);
+            goto fail;
+        }
         g_free(filename);
-        goto fail;
+    } else if (binfo->get_dtb) {
+        fdt = binfo->get_dtb(binfo, &size);
+        if (!fdt) {
+            fprintf(stderr, "Board was unable to create a dtb blob\n");
+            goto fail;
+        }
     }
-    g_free(filename);
 
     acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
     scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
@@ -436,7 +444,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
         /* for device tree boot, we pass the DTB directly in r2. Otherwise
          * we point to the kernel args.
          */
-        if (info->dtb_filename) {
+        if (info->dtb_filename || info->get_dtb) {
             /* Place the DTB after the initrd in memory. Note that some
              * kernels will trash anything in the 4K page the initrd
              * ends in, so make sure the DTB isn't caught up in that.
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index ecbbba8..cbbf4ca 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -50,6 +50,13 @@ struct arm_boot_info {
                                  const struct arm_boot_info *info);
     void (*secondary_cpu_reset_hook)(ARMCPU *cpu,
                                      const struct arm_boot_info *info);
+    /* if a board is able to create a dtb without a dtb file then it
+     * sets get_dtb. This will only be used if no dtb file is provided
+     * by the user. On success, sets *size to the length of the created
+     * dtb, and returns a pointer to it. (The caller must free this memory
+     * with g_free() when it has finished with it.) On failure, returns NULL.
+     */
+    void *(*get_dtb)(const struct arm_boot_info *info, int *size);
     /* if a board needs to be able to modify a device tree provided by
      * the user it should implement this hook.
      */
-- 
1.7.9.5

[Qemu-devel] [PATCH v7 3/3] hw/arm: Add 'virt' platform

[Peter Maydell]

Add 'virt' platform support corresponding to arch/arm/mach-virt
in the Linux kernel tree. This has no platform-specific code but
can use any device whose kernel driver is is able to work purely
from a device tree node. We use this to instantiate a minimal
set of devices: a GIC and some virtio-mmio transports.

Signed-off-by: John Rigby <john.rigby@linaro.org>
[PMM:
 Significantly overhauled:
 * renamed user-facing machine to just "virt"
 * removed the A9 support (it can't work since the A9 has no
   generic timers)
 * added virtio-mmio transports instead of random set of 'soc' devices
   (though we retain a pl011 UART)
 * instead of updating io_base as we step through adding devices,
   define a memory map with an array (similar to vexpress)
 * similarly, define irqmap with an array
 * folded in some minor fixes from John's aarch64-support patch
 * rather than explicitly doing endian-swapping on FDT cells,
   use fdt APIs that let us just pass in host-endian values
   and let the fdt layer take care of the swapping
 * miscellaneous minor code cleanups and style fixes
]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/Makefile.objs |    2 +-
 hw/arm/virt.c        |  419 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 420 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/virt.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 3671b42..78b5614 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,7 +1,7 @@
 obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
-obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
+obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-y += omap1.o omap2.o strongarm.o
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
new file mode 100644
index 0000000..448a0e5
--- /dev/null
+++ b/hw/arm/virt.c
@@ -0,0 +1,419 @@
+/*
+ * ARM mach-virt emulation
+ *
+ * Copyright (c) 2013 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Emulate a virtual board which works by passing Linux all the information
+ * it needs about what devices are present via the device tree.
+ * There are some restrictions about what we can do here:
+ *  + we can only present devices whose Linux drivers will work based
+ *    purely on the device tree with no platform data at all
+ *  + we want to present a very stripped-down minimalist platform,
+ *    both because this reduces the security attack surface from the guest
+ *    and also because it reduces our exposure to being broken when
+ *    the kernel updates its device tree bindings and requires further
+ *    information in a device binding that we aren't providing.
+ * This is essentially the same approach kvmtool uses.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/arm/arm.h"
+#include "hw/arm/primecell.h"
+#include "hw/devices.h"
+#include "net/net.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+
+#define NUM_VIRTIO_TRANSPORTS 32
+
+/* Number of external interrupt lines to configure the GIC with */
+#define NUM_IRQS 128
+
+#define GIC_FDT_IRQ_TYPE_SPI 0
+#define GIC_FDT_IRQ_TYPE_PPI 1
+
+#define GIC_FDT_IRQ_FLAGS_EDGE_LO_HI 1
+#define GIC_FDT_IRQ_FLAGS_EDGE_HI_LO 2
+#define GIC_FDT_IRQ_FLAGS_LEVEL_HI 4
+#define GIC_FDT_IRQ_FLAGS_LEVEL_LO 8
+
+#define GIC_FDT_IRQ_PPI_CPU_START 8
+#define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
+
+enum {
+    VIRT_FLASH,
+    VIRT_MEM,
+    VIRT_CPUPERIPHS,
+    VIRT_GIC_DIST,
+    VIRT_GIC_CPU,
+    VIRT_UART,
+    VIRT_MMIO,
+};
+
+typedef struct MemMapEntry {
+    hwaddr base;
+    hwaddr size;
+} MemMapEntry;
+
+typedef struct VirtBoardInfo {
+    struct arm_boot_info bootinfo;
+    const char *cpu_model;
+    const char *cpu_compatible;
+    const char *qdevname;
+    const char *gic_compatible;
+    const MemMapEntry *memmap;
+    const int *irqmap;
+    int smp_cpus;
+    void *fdt;
+    int fdt_size;
+    uint32_t clock_phandle;
+} VirtBoardInfo;
+
+/* Addresses and sizes of our components.
+ * We leave the first 64K free for possible use later for
+ * flash (for running boot code such as UEFI); following
+ * that is I/O, and then everything else is RAM (which may
+ * happily spill over into the high memory region beyond 4GB).
+ */
+static const MemMapEntry a15memmap[] = {
+    [VIRT_FLASH] = { 0, 0x1000000 },
+    [VIRT_CPUPERIPHS] = { 0x1000000, 0x8000 },
+    /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
+    [VIRT_GIC_DIST] = { 0x1001000, 0x1000 },
+    [VIRT_GIC_CPU] = { 0x1002000, 0x1000 },
+    [VIRT_UART] = { 0x1008000, 0x1000 },
+    [VIRT_MMIO] = { 0x2000000, 0x200 },
+    /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
+    [VIRT_MEM] = { 0x8000000, 30ULL * 1024 * 1024 * 1024 },
+};
+
+static const int a15irqmap[] = {
+    [VIRT_UART] = 1,
+    [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+};
+
+static VirtBoardInfo machines[] = {
+    {
+        .cpu_model = "cortex-a15",
+        .cpu_compatible = "arm,cortex-a15",
+        .qdevname = "a15mpcore_priv",
+        .gic_compatible = "arm,cortex-a15-gic",
+        .memmap = a15memmap,
+        .irqmap = a15irqmap,
+    },
+};
+
+static VirtBoardInfo *find_machine_info(const char *cpu)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(machines); i++) {
+        if (strcmp(cpu, machines[i].cpu_model) == 0) {
+            return &machines[i];
+        }
+    }
+    return NULL;
+}
+
+static void create_fdt(VirtBoardInfo *vbi)
+{
+    void *fdt = create_device_tree(&vbi->fdt_size);
+
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    vbi->fdt = fdt;
+
+    /* Header */
+    qemu_devtree_setprop_string(fdt, "/", "compatible", "linux,dummy-virt");
+    qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 0x2);
+    qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 0x2);
+
+    /*
+     * /chosen and /memory nodes must exist for load_dtb
+     * to fill in necessary properties later
+     */
+    qemu_devtree_add_subnode(fdt, "/chosen");
+    qemu_devtree_add_subnode(fdt, "/memory");
+    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
+
+    /* Clock node, for the benefit of the UART. The kernel device tree
+     * binding documentation claims the PL011 node clock properties are
+     * optional but in practice if you omit them the kernel refuses to
+     * probe for the device.
+     */
+    vbi->clock_phandle = qemu_devtree_alloc_phandle(fdt);
+    qemu_devtree_add_subnode(fdt, "/apb-pclk");
+    qemu_devtree_setprop_string(fdt, "/apb-pclk", "compatible", "fixed-clock");
+    qemu_devtree_setprop_cell(fdt, "/apb-pclk", "#clock-cells", 0x0);
+    qemu_devtree_setprop_cell(fdt, "/apb-pclk", "clock-frequency", 24000000);
+    qemu_devtree_setprop_string(fdt, "/apb-pclk", "clock-output-names",
+                                "clk24mhz");
+    qemu_devtree_setprop_cell(fdt, "/apb-pclk", "phandle", vbi->clock_phandle);
+
+    /* No PSCI for TCG yet */
+#ifdef CONFIG_KVM
+    if (kvm_enabled()) {
+        qemu_devtree_add_subnode(fdt, "/psci");
+        qemu_devtree_setprop_string(fdt, "/psci", "compatible", "arm,psci");
+        qemu_devtree_setprop_string(fdt, "/psci", "method", "hvc");
+        qemu_devtree_setprop_cell(fdt, "/psci", "cpu_suspend",
+                                  KVM_PSCI_FN_CPU_SUSPEND);
+        qemu_devtree_setprop_cell(fdt, "/psci", "cpu_off", KVM_PSCI_FN_CPU_OFF);
+        qemu_devtree_setprop_cell(fdt, "/psci", "cpu_on", KVM_PSCI_FN_CPU_ON);
+        qemu_devtree_setprop_cell(fdt, "/psci", "migrate", KVM_PSCI_FN_MIGRATE);
+    }
+#endif
+}
+
+static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
+{
+    /* Note that on A15 h/w these interrupts are level-triggered,
+     * but for the GIC implementation provided by both QEMU and KVM
+     * they are edge-triggered.
+     */
+    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
+
+    irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
+
+    qemu_devtree_add_subnode(vbi->fdt, "/timer");
+    qemu_devtree_setprop_string(vbi->fdt, "/timer",
+                                "compatible", "arm,armv7-timer");
+    qemu_devtree_setprop_cells(vbi->fdt, "/timer", "interrupts",
+                               GIC_FDT_IRQ_TYPE_PPI, 13, irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI, 14, irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI, 11, irqflags,
+                               GIC_FDT_IRQ_TYPE_PPI, 10, irqflags);
+}
+
+static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
+{
+    int cpu;
+
+    qemu_devtree_add_subnode(vbi->fdt, "/cpus");
+    qemu_devtree_setprop_cell(vbi->fdt, "/cpus", "#address-cells", 0x1);
+    qemu_devtree_setprop_cell(vbi->fdt, "/cpus", "#size-cells", 0x0);
+
+    for (cpu = 0; cpu < vbi->smp_cpus; cpu++) {
+        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+
+        qemu_devtree_add_subnode(vbi->fdt, nodename);
+        qemu_devtree_setprop_string(vbi->fdt, nodename, "device_type", "cpu");
+        qemu_devtree_setprop_string(vbi->fdt, nodename, "compatible",
+                                    vbi->cpu_compatible);
+
+        if (vbi->smp_cpus > 1) {
+            qemu_devtree_setprop_string(vbi->fdt, nodename,
+                                        "enable-method", "psci");
+        }
+
+        qemu_devtree_setprop_cell(vbi->fdt, nodename, "reg", cpu);
+        g_free(nodename);
+    }
+}
+
+static void fdt_add_gic_node(const VirtBoardInfo *vbi)
+{
+    uint32_t gic_phandle;
+
+    gic_phandle = qemu_devtree_alloc_phandle(vbi->fdt);
+    qemu_devtree_setprop_cell(vbi->fdt, "/", "interrupt-parent", gic_phandle);
+
+    qemu_devtree_add_subnode(vbi->fdt, "/intc");
+    qemu_devtree_setprop_string(vbi->fdt, "/intc", "compatible",
+                                vbi->gic_compatible);
+    qemu_devtree_setprop_cell(vbi->fdt, "/intc", "#interrupt-cells", 3);
+    qemu_devtree_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0);
+    qemu_devtree_setprop_sized_cells(vbi->fdt, "/intc", "reg",
+                                     2, vbi->memmap[VIRT_GIC_DIST].base,
+                                     2, vbi->memmap[VIRT_GIC_DIST].size,
+                                     2, vbi->memmap[VIRT_GIC_CPU].base,
+                                     2, vbi->memmap[VIRT_GIC_CPU].size);
+    qemu_devtree_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
+}
+
+static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_UART].base;
+    hwaddr size = vbi->memmap[VIRT_UART].size;
+    int irq = vbi->irqmap[VIRT_UART];
+    const char compat[] = "arm,pl011\0arm,primecell";
+    const char clocknames[] = "uartclk\0apb_pclk";
+
+    sysbus_create_simple("pl011", base, pic[irq]);
+
+    nodename = g_strdup_printf("/pl011@%" PRIx64, base);
+    qemu_devtree_add_subnode(vbi->fdt, nodename);
+    /* Note that we can't use setprop_string because of the embedded NUL */
+    qemu_devtree_setprop(vbi->fdt, nodename, "compatible",
+                         compat, sizeof(compat));
+    qemu_devtree_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                     2, base, 2, size);
+    qemu_devtree_setprop_cells(vbi->fdt, nodename, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irq,
+                               GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+    qemu_devtree_setprop_cells(vbi->fdt, nodename, "clocks",
+                               vbi->clock_phandle, vbi->clock_phandle);
+    qemu_devtree_setprop(vbi->fdt, nodename, "clock-names",
+                         clocknames, sizeof(clocknames));
+    g_free(nodename);
+}
+
+static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    int i;
+    hwaddr base = vbi->memmap[VIRT_MMIO].base;
+    hwaddr size = vbi->memmap[VIRT_MMIO].size;
+
+    for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
+        char *nodename;
+        int irq = i + vbi->irqmap[VIRT_MMIO];
+
+        sysbus_create_simple("virtio-mmio", base, pic[irq]);
+
+        nodename = g_strdup_printf("/virtio_mmio@%" PRIx64, base);
+        qemu_devtree_add_subnode(vbi->fdt, nodename);
+        qemu_devtree_setprop_string(vbi->fdt, nodename,
+                                    "compatible", "virtio,mmio");
+        qemu_devtree_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                         2, base, 2, size);
+        qemu_devtree_setprop_cells(vbi->fdt, nodename, "interrupts",
+                                   GIC_FDT_IRQ_TYPE_SPI, irq,
+                                   GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+        g_free(nodename);
+        base += size;
+    }
+}
+
+static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+{
+    const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
+
+    *fdt_size = board->fdt_size;
+    return board->fdt;
+}
+
+static void machvirt_init(QEMUMachineInitArgs *args)
+{
+    qemu_irq pic[NUM_IRQS];
+    MemoryRegion *sysmem = get_system_memory();
+    int n;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    DeviceState *dev;
+    SysBusDevice *busdev;
+    const char *cpu_model = args->cpu_model;
+    VirtBoardInfo *vbi;
+
+    if (!cpu_model) {
+        cpu_model = "cortex-a15";
+    }
+
+    vbi = find_machine_info(cpu_model);
+
+    if (!vbi) {
+        error_report("mach-virt: CPU %s not supported", cpu_model);
+        exit(1);
+    }
+
+    vbi->smp_cpus = smp_cpus;
+
+    /*
+     * Only supported method of starting secondary CPUs is PSCI and
+     * PSCI is not yet supported with TCG, so limit smp_cpus to 1
+     * if we're not using KVM.
+     */
+    if (!kvm_enabled() && smp_cpus > 1) {
+        error_report("mach-virt: must enable KVM to use multiple CPUs");
+        exit(1);
+    }
+
+    if (args->ram_size > vbi->memmap[VIRT_MEM].size) {
+        error_report("mach-virt: cannot model more than 30GB RAM");
+        exit(1);
+    }
+
+    create_fdt(vbi);
+    fdt_add_timer_nodes(vbi);
+
+    for (n = 0; n < smp_cpus; n++) {
+        cpu_arm_init(cpu_model);
+    }
+    fdt_add_cpu_nodes(vbi);
+
+    memory_region_init_ram(ram, NULL, "mach-virt.ram", args->ram_size);
+    vmstate_register_ram_global(ram);
+    memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);
+
+    dev = qdev_create(NULL, vbi->qdevname);
+    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
+    /* Note that the num-irq property counts both internal and external
+     * interrupts; there are always 32 of the former (mandated by GIC spec).
+     */
+    qdev_prop_set_uint32(dev, "num-irq", NUM_IRQS + 32);
+    qdev_init_nofail(dev);
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, vbi->memmap[VIRT_CPUPERIPHS].base);
+    fdt_add_gic_node(vbi);
+    for (n = 0; n < smp_cpus; n++) {
+        DeviceState *cpudev = DEVICE(qemu_get_cpu(n));
+
+        sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+    }
+
+    for (n = 0; n < NUM_IRQS; n++) {
+        pic[n] = qdev_get_gpio_in(dev, n);
+    }
+
+    create_uart(vbi, pic);
+
+    /* Create mmio transports, so the user can create virtio backends
+     * (which will be automatically plugged in to the transports). If
+     * no backend is created the transport will just sit harmlessly idle.
+     */
+    create_virtio_devices(vbi, pic);
+
+    vbi->bootinfo.ram_size = args->ram_size;
+    vbi->bootinfo.kernel_filename = args->kernel_filename;
+    vbi->bootinfo.kernel_cmdline = args->kernel_cmdline;
+    vbi->bootinfo.initrd_filename = args->initrd_filename;
+    vbi->bootinfo.nb_cpus = smp_cpus;
+    vbi->bootinfo.board_id = -1;
+    vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
+    vbi->bootinfo.get_dtb = machvirt_dtb;
+    arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
+}
+
+static QEMUMachine machvirt_a15_machine = {
+    .name = "virt",
+    .desc = "ARM Virtual Machine",
+    .init = machvirt_init,
+    .max_cpus = 4,
+};
+
+static void machvirt_machine_init(void)
+{
+    qemu_register_machine(&machvirt_a15_machine);
+}
+
+machine_init(machvirt_machine_init);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Christoffer Dall]

On Thu, Sep 12, 2013 at 11:17:50AM +0100, Peter Maydell wrote:
> This patch series adds a 'virt' platform which uses the
> kernel's mach-virt (fully device-tree driven) support
> to create a simple minimalist platform intended for
> use for KVM VM guests.

looks good to me.  fwiw
Reviewed-by: Chritoffer Dall <christoffer.dall@linaro.org>

> 
> The major change here is that I've added a PL011 UART.
> 
> Sample command line:
> 
>  qemu-system-arm -machine type=virt -display none \
>   -kernel zImage \
>   -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
>   -cpu cortex-a15 \
>   -device virtio-blk-device,drive=foo \
>   -drive if=none,file=arm-wheezy.img,id=foo \
>   -m 2048 -serial stdio
> 
> Note that there is no earlyprintk via the PL011 because
> there's no defined device tree binding for "hey, here
> is your earlyprintk UART".
> 
> 
> *** NOTE *** to get the PL011 to work you'll need to
> tweak the kernel a bit:
> 
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> index b184e57..2b6aceb 100644
> --- a/arch/arm/mach-virt/virt.c
> +++ b/arch/arm/mach-virt/virt.c
> @@ -21,11 +21,13 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/smp.h>
> +#include <linux/clk-provider.h>
>  
>  #include <asm/mach/arch.h>
>  
>  static void __init virt_init(void)
>  {
> +       of_clk_init(NULL);
>         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  }
>  
> 
> Otherwise the kernel doesn't ever add the clock to its
> list, and then it refuses to probe for the PL011.
> (I'm told this isn't really the right fix, though, and
> ideally the call should be done in some generic location
> rather than in every machine's init function.)
> 
> The alternative would be for the kernel to be fixed to
> follow its own device tree binding documentation and not
> require clocks/clock-names properties on the pl011 node.

Is anyone taking a look at a proper fix as far as you know?

-Christoffer

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Peter Maydell]

On 12 September 2013 11:17, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch series adds a 'virt' platform which uses the
> kernel's mach-virt (fully device-tree driven) support
> to create a simple minimalist platform intended for
> use for KVM VM guests.
>
> The major change here is that I've added a PL011 UART.
>
> Sample command line:
>
>  qemu-system-arm -machine type=virt -display none \
>   -kernel zImage \
>   -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
>   -cpu cortex-a15 \
>   -device virtio-blk-device,drive=foo \
>   -drive if=none,file=arm-wheezy.img,id=foo \
>   -m 2048 -serial stdio
>
> Note that there is no earlyprintk via the PL011 because
> there's no defined device tree binding for "hey, here
> is your earlyprintk UART".

Last call: anybody got any comments on mach-virt?
Otherwise I'm planning to put it into a pull request
and it'll go into qemu 1.7 as an "experimental" status
platform.

-- PMM

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Peter Maydell]

On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).

Oh yes, I'd forgotten you mentioned that. Did anybody ever
track down *why* the kernel is reading the device tree
backwards?

(PS: I'd appreciate it if you didn't drop the qemu-devel cc.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Tom Sutcliffe]

On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
> 
> Oh yes, I'd forgotten you mentioned that. Did anybody ever
> track down *why* the kernel is reading the device tree
> backwards?

Not me :) I assume it's inserting into a linked list at some point by repeatedly replacing the head element (which would reverse the order) but where it's doing that, whether it's intentional, and what would break if it were changed I couldn't say.

> (PS: I'd appreciate it if you didn't drop the qemu-devel cc.)

My mistake!

Tom

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Peter Maydell]

On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>
> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
>>
>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>> track down *why* the kernel is reading the device tree
>> backwards?
>
> Not me :)

So apparently the kernel makes no guarantees at all about what
order it might process the virtio-mmio transports in. This
means that users mustn't rely on /dev/vda and /dev/vdb
corresponding to particular virtio-blk devices on QEMU's
command line -- you need to use UUIDs or something similar
instead.

I think this sucks, but that's the kernel for you.

I'll probably change QEMU anyway, just because if there's
no guarantee we might as well make qemu code do a simple
forwards loop rather than a backwards one.

-- PMM

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Tom Sutcliffe]

On 17 Oct 2013, at 15:30, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>> 
>> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>> 
>>> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
>>> 
>>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>>> track down *why* the kernel is reading the device tree
>>> backwards?
>> 
>> Not me :)
> 
> So apparently the kernel makes no guarantees at all about what
> order it might process the virtio-mmio transports in. This
> means that users mustn't rely on /dev/vda and /dev/vdb
> corresponding to particular virtio-blk devices on QEMU's
> command line -- you need to use UUIDs or something similar
> instead.
> 
> I think this sucks, but that's the kernel for you.

Oh joy. One more thing to add to the How Long Before This Blows Up In My Face list. So long as it's consistent across multiple boots of a given kernel binary, I can probably live with it for the moment.

> I'll probably change QEMU anyway, just because if there's
> no guarantee we might as well make qemu code do a simple
> forwards loop rather than a backwards one.

Sounds like the best option.


Tom

Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform

[Peter Maydell]

On 17 October 2013 15:49, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
> On 17 Oct 2013, at 15:30, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>>>> track down *why* the kernel is reading the device tree
>>>> backwards?
>>>
>>> Not me :)

So I have figured this one out. libfdt seems to always
add new subnodes at the start of the parent node's list
of subnodes. This means that if you do "add A; add B; add C"
then the resulting device tree blob lists the nodes in
the order C B A. The kernel (and dtc in decompilation mode)
read the node list forwards, so they iterate through in
reverse order to how the nodes were added by the creator.

Peter, Andreas, David: do any of you know why libfdt does
this?

Anyway, as a result the bits of QEMU that generate device
trees for PPC boards specifically make sure they add
the nodes in reverse order in the places where order
of subnodes in the tree is important. So I think we should
follow suit for ARM boards, which means leaving vexpress
as it is and making mach-virt do them in reverse order too.

>> So apparently the kernel makes no guarantees at all about what
>> order it might process the virtio-mmio transports in. This
>> means that users mustn't rely on /dev/vda and /dev/vdb
>> corresponding to particular virtio-blk devices on QEMU's
>> command line -- you need to use UUIDs or something similar
>> instead.
>>
>> I think this sucks, but that's the kernel for you.
>
> Oh joy. One more thing to add to the How Long Before This
> Blows Up In My Face list. So long as it's consistent across
> multiple boots of a given kernel binary, I can probably live
> with it for the moment.

...and I will redirect anybody who complains about
the fact that vda and vdb are the "wrong way round"
to the kernel, because as it happens the kernel probes
the two virtio-mmio transports in the "right" order
(ie in the order they appear in the device tree blob),
it just ends up assigning vda and vdb in the opposite
order.

thanks
-- PMM