[Qemu-devel] [PATCH v2 0/2] Add mach-virt platform

[Qemu-devel] [PATCH v2 0/2] Add mach-virt platform

[John Rigby]

First modify arm fdt handling so boards can provide a blob
rather than requiring one on command line.

Next add mach-virt platform that creates fdt blob from scratch
describing its minimal hw which is a pl011 uart and a sp804 timer.

Tested on amd64 host with full system emulation and also using kvm
on an arndale board

v2 changes: remove bogus cruft from mach-virt.c that was leftover
from checkpatch fixing

John Rigby (2):
  ARM: Allow boards to provide an fdt blob
  ARM: Add mach-virt platform

 hw/arm/Makefile.objs |    2 +-
 hw/arm/boot.c        |   31 +++--
 hw/arm/mach-virt.c   |  337 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/arm.h |    6 +
 4 files changed, 364 insertions(+), 12 deletions(-)
 create mode 100644 hw/arm/mach-virt.c

-- 
1.7.9.5

[Qemu-devel] [PATCH v2 1/2] ARM: Allow boards to provide an fdt blob

[John Rigby]

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.

Also allow dumping of device tree by calling qemu_devtree_dumpdtb
near the end of load_dtb.

Signed-off-by: John Rigby <john.rigby@linaro.org>
---
 hw/arm/boot.c        |   31 ++++++++++++++++++++-----------
 include/hw/arm/arm.h |    6 ++++++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index f451529..de71edf 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -235,19 +235,27 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
     int size, rc;
     uint32_t acells, scells, hival;
 
-    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;
-    }
+    if (binfo->dtb_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);
+            return -1;
+        }
 
-    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);
+            return -1;
+        }
         g_free(filename);
-        return -1;
+    } else if (binfo->get_dtb) {
+        fdt = binfo->get_dtb(addr, binfo, &size);
+        if (!fdt) {
+            fprintf(stderr, "Couldn't get dtb blob from board func\n");
+            return -1;
+        }
     }
-    g_free(filename);
 
     acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
     scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
@@ -304,6 +312,7 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
             fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
         }
     }
+    qemu_devtree_dumpdtb(fdt, size);
 
     cpu_physical_memory_write(addr, fdt, size);
 
@@ -440,7 +449,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 7b2b02d..4c56a1b 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -31,6 +31,10 @@ struct arm_boot_info {
     const char *kernel_cmdline;
     const char *initrd_filename;
     const char *dtb_filename;
+    /* 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.
+     */
+    void *(*get_dtb)(hwaddr addr, const struct arm_boot_info *binfo, int *size);
     hwaddr 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,
@@ -59,6 +63,8 @@ struct arm_boot_info {
     int is_linux;
     hwaddr initrd_start;
     hwaddr initrd_size;
+    void *dtb_blob;
+    int dtb_blob_size;
     hwaddr entry;
 };
 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 2/2] ARM: Add mach-virt platform

[John Rigby]

Add mach-virt platform support cooresponding to
/arch/arm/mach-virt in kernel tree.

For now it is not virtual but instantiates a pl011 uart
and and sp804 timer.  The uart is need for a console
the timer is needed when running without kvm and there
is no arch timer.

Signed-off-by: John Rigby <john.rigby@linaro.org>
---
 hw/arm/Makefile.objs |    2 +-
 hw/arm/mach-virt.c   |  337 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 338 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/mach-virt.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 9e3a06f..0789352 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,5 +1,5 @@
 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 += integratorcp.o kzm.o mach-virt.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o pic_cpu.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
 
diff --git a/hw/arm/mach-virt.c b/hw/arm/mach-virt.c
new file mode 100644
index 0000000..b55e4d1
--- /dev/null
+++ b/hw/arm/mach-virt.c
@@ -0,0 +1,337 @@
+/*
+ * ARM mach-virt emulation
+ *
+ * Copyright (c) 2013 Linaro
+ *
+ * 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/>.
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/arm-misc.h"
+#include "hw/primecell.h"
+#include "hw/devices.h"
+#include "net/net.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "hw/flash.h"
+#include "libfdt_env.h"
+
+#define ARM_GIC_DIST_BASE 0x2c001000
+#define ARM_GIC_DIST_SIZE 0x1000
+#define ARM_GIC_CPUI_BASE 0x2c002000
+#define ARM_GIC_CPUI_SIZE 0x1000
+
+#define GIC_FDT_IRQ_NUM_CELLS 3
+
+#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_SHIFT 8
+#define GIC_FDT_IRQ_PPI_CPU_MASK (0xff << GIC_FDT_IRQ_PPI_CPU_SHIFT)
+
+#define CPU_NAME_MAX_LEN 16
+static void add_cpu_nodes(void *fdt, const struct arm_boot_info *binfo)
+{
+    int cpu;
+
+    qemu_devtree_add_subnode(fdt, "/cpus");
+    qemu_devtree_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
+    qemu_devtree_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
+
+    for (cpu = 0; cpu < binfo->nb_cpus; ++cpu) {
+        char cpu_name[CPU_NAME_MAX_LEN];
+
+        snprintf(cpu_name, CPU_NAME_MAX_LEN, "/cpus/cpu@%d", cpu);
+
+        qemu_devtree_add_subnode(fdt, cpu_name);
+        qemu_devtree_setprop_string(fdt, cpu_name, "device_type", "cpu");
+        qemu_devtree_setprop_string(fdt, cpu_name, "compatible",
+            "arm,cortex-a15");
+
+        if (binfo->nb_cpus > 1) {
+            qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "psci");
+        }
+
+        qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu);
+    }
+}
+
+static void add_gic_nodes(void *fdt, uint32_t phandle)
+{
+    uint64_t reg_prop[] = {
+        cpu_to_fdt64(ARM_GIC_DIST_BASE), cpu_to_fdt64(ARM_GIC_DIST_SIZE),
+        cpu_to_fdt64(ARM_GIC_CPUI_BASE), cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
+    };
+
+    qemu_devtree_add_subnode(fdt, "/intc");
+    qemu_devtree_setprop_string(fdt, "/intc", "compatible",
+            "arm,cortex-a15-gic");
+    qemu_devtree_setprop_cell(fdt, "/intc", "#interrupt-cells",
+            GIC_FDT_IRQ_NUM_CELLS);
+    qemu_devtree_setprop(fdt, "/intc", "interrupt-controller", NULL, 0);
+    qemu_devtree_setprop(fdt, "/intc", "reg", reg_prop, sizeof(reg_prop));
+    qemu_devtree_setprop_cell(fdt, "/intc", "phandle", phandle);
+}
+
+static void add_timer_nodes(void *fdt, const struct arm_boot_info *binfo)
+{
+    uint32_t cpu_mask =
+        (((1 << binfo->nb_cpus) - 1) << GIC_FDT_IRQ_PPI_CPU_SHIFT)
+                        & GIC_FDT_IRQ_PPI_CPU_MASK;
+    cpu_mask = 0xf00;
+    uint32_t irq_prop[] = {
+        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
+        cpu_to_fdt32(13),
+        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
+
+        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
+        cpu_to_fdt32(14),
+        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
+
+        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
+        cpu_to_fdt32(11),
+        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
+
+        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
+        cpu_to_fdt32(10),
+        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
+    };
+
+    qemu_devtree_add_subnode(fdt, "/timer");
+    qemu_devtree_setprop_string(fdt, "/timer", "compatible", "arm,armv7-timer");
+    qemu_devtree_setprop(fdt, "/timer", "interrupts", irq_prop,
+            sizeof(irq_prop));
+}
+
+#define NRMAPIRQS 43
+
+static void add_interrupt_map(void *fdt, const char *soc, uint32_t gic)
+{
+    int i, irq;
+    int len;
+    uint32_t *map;
+
+    qemu_devtree_setprop_cell(fdt, soc, "#interrupt-cells", 0x1);
+    qemu_devtree_setprop_cells(fdt, soc, "interrupt-map-mask", 0, 63);
+
+    len = NRMAPIRQS * 6 * sizeof(uint32_t);
+    map = g_malloc(len);
+
+    for (i = 0, irq = 0; irq < NRMAPIRQS; irq++) {
+        map[i++] = cpu_to_be32(0x0);
+        map[i++] = cpu_to_be32(irq);
+        map[i++] = cpu_to_be32(gic);
+        map[i++] = cpu_to_be32(0x0);
+        map[i++] = cpu_to_be32(irq);
+        map[i++] = cpu_to_be32(4);
+    }
+    qemu_devtree_setprop(fdt, soc, "interrupt-map", map, len);
+    g_free(map);
+}
+
+static void *machvirt_dtb(hwaddr addr, const struct arm_boot_info *binfo,
+        int *fdt_size)
+{
+    void *fdt;
+    uint32_t gic_phandle;
+    uint32_t clock_phandle;
+    char compatible_sb[] = "simple-bus\0arm,amba-bus";
+    char compatible_uart[] = "arm,pl011\0arm,primecell";
+    char compatible_timer[] = "arm,sp804\0arm,primecell";
+    char clock_names_uart[] = "uartclk\0apb_pclk";
+    char clock_names_timer[] = "timerclk\0apb_pclk";
+
+    fdt = create_device_tree(fdt_size);
+    if (fdt == NULL) {
+        goto out;
+    }
+    gic_phandle = qemu_devtree_alloc_phandle(fdt);
+
+    /* Header */
+    qemu_devtree_setprop_cell(fdt, "/", "interrupt-parent", gic_phandle);
+    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
+     * only add properties are not added later in arm_boot
+     */
+    qemu_devtree_add_subnode(fdt, "/chosen");
+    qemu_devtree_add_subnode(fdt, "/memory");
+    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
+
+    /* CPU and peripherals (interrupt controller, timers, etc) */
+    add_cpu_nodes(fdt, binfo);
+    add_gic_nodes(fdt, gic_phandle);
+    add_timer_nodes(fdt, binfo);
+
+    qemu_devtree_add_subnode(fdt, "/soc");
+    qemu_devtree_setprop(fdt, "/soc", "compatible", compatible_sb,
+            sizeof(compatible_sb));
+    qemu_devtree_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
+    qemu_devtree_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
+    qemu_devtree_setprop_cells(fdt, "/soc", "ranges", 0x10000000, 0x0,
+            0x10000000, 0x10000000);
+    qemu_devtree_setprop_cell(fdt, "/soc", "#interrupt-cells", 0x1);
+
+    add_interrupt_map(fdt, "/soc", gic_phandle);
+
+    clock_phandle = qemu_devtree_alloc_phandle(fdt);
+    qemu_devtree_add_subnode(fdt, "/soc/clock");
+    qemu_devtree_setprop_string(fdt, "/soc/clock", "compatible", "fixed-clock");
+    qemu_devtree_setprop_cell(fdt, "/soc/clock", "#clock-cells", 0x0);
+    qemu_devtree_setprop_cell(fdt, "/soc/clock", "clock-frequency", 24000000);
+    qemu_devtree_setprop_string(fdt, "/soc/clock", "clock-output-names",
+            "clk24mhz");
+    qemu_devtree_setprop_cell(fdt, "/soc/clock", "phandle", clock_phandle);
+
+    qemu_devtree_add_subnode(fdt, "/soc/uart");
+    qemu_devtree_setprop(fdt, "/soc/uart", "compatible", compatible_uart,
+            sizeof(compatible_uart));
+    qemu_devtree_setprop_cells(fdt, "/soc/uart", "reg", 0x1c090000, 0x1000);
+    qemu_devtree_setprop_cell(fdt, "/soc/uart", "interrupts", 0x5);
+    qemu_devtree_setprop_cells(fdt, "/soc/uart", "clocks", clock_phandle,
+            clock_phandle);
+    qemu_devtree_setprop(fdt, "/soc/uart", "clock-names", clock_names_uart,
+            sizeof(clock_names_uart));
+
+    qemu_devtree_add_subnode(fdt, "/soc/timer");
+    qemu_devtree_setprop(fdt, "/soc/timer", "compatible", compatible_timer,
+            sizeof(compatible_timer));
+    qemu_devtree_setprop_cells(fdt, "/soc/timer", "reg", 0x1c110000, 0x1000);
+    qemu_devtree_setprop_cell(fdt, "/soc/timer", "interrupts", 0x2);
+    qemu_devtree_setprop_cells(fdt, "/soc/timer", "clocks", clock_phandle,
+            clock_phandle);
+    qemu_devtree_setprop(fdt, "/soc/timer", "clock-names", clock_names_timer,
+            sizeof(clock_names_timer));
+#if 0
+    /* PSCI firmware */
+    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
+
+out:
+    return fdt;
+}
+
+
+static struct arm_boot_info machvirt_binfo;
+
+static void machvirt_init(QEMUMachineInitArgs *args)
+{
+    qemu_irq pic[64];
+    MemoryRegion *sysmem = get_system_memory();
+    int n;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    qemu_irq cpu_irq[4];
+    DeviceState *dev;
+    SysBusDevice *busdev;
+    const char *cpu_model = args->cpu_model;
+
+    if (!cpu_model) {
+        cpu_model = "cortex-a15";
+    }
+
+    for (n = 0; n < smp_cpus; n++) {
+        ARMCPU *cpu;
+        qemu_irq *irqp;
+
+        cpu = cpu_arm_init(cpu_model);
+        if (!cpu) {
+            fprintf(stderr, "Unable to find CPU definition %s\n", cpu_model);
+            exit(1);
+        }
+        irqp = arm_pic_init_cpu(cpu);
+        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
+    }
+
+    {
+        /* We have to use a separate 64 bit variable here to avoid the gcc
+         * "comparison is always false due to limited range of data type"
+         * warning if we are on a host where ram_addr_t is 32 bits.
+         */
+        uint64_t rsz = ram_size;
+        if (rsz > (30ULL * 1024 * 1024 * 1024)) {
+            fprintf(stderr, "mach-virt: cannot model more than 30GB RAM\n");
+            exit(1);
+        }
+    }
+
+    memory_region_init_ram(ram, "mach-virt.ram", ram_size);
+    vmstate_register_ram_global(ram);
+    /* RAM is from 0x80000000 upwards; there is no low-memory alias for it. */
+    memory_region_add_subregion(sysmem, 0x80000000, ram);
+
+    /* 0x2c000000 A15MPCore private memory region (GIC) */
+    dev = qdev_create(NULL, "a15mpcore_priv");
+    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
+    qdev_init_nofail(dev);
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, 0x2c000000);
+    for (n = 0; n < smp_cpus; n++) {
+        sysbus_connect_irq(busdev, n, cpu_irq[n]);
+    }
+
+    for (n = 0; n < 64; n++) {
+        pic[n] = qdev_get_gpio_in(dev, n);
+    }
+
+    /*
+     * Not completely virt yet, we do have one uart
+     * .. and a timer...
+     */
+    sysbus_create_simple("pl011", 0x1c090000, pic[5]);
+    sysbus_create_simple("sp804", 0x1c110000, pic[2]);
+
+    machvirt_binfo.ram_size = args->ram_size;
+    machvirt_binfo.kernel_filename = args->kernel_filename;
+    machvirt_binfo.kernel_cmdline = args->kernel_cmdline;
+    machvirt_binfo.initrd_filename = args->initrd_filename;
+    machvirt_binfo.nb_cpus = smp_cpus;
+    machvirt_binfo.board_id = -1;
+    machvirt_binfo.loader_start = 0x80000000;
+    machvirt_binfo.smp_bootreg_addr = 0x1c010000 + 0x30;
+    machvirt_binfo.gic_cpu_if_addr = 0x2c002000;
+    machvirt_binfo.get_dtb = machvirt_dtb;
+    arm_load_kernel(arm_env_get_cpu(first_cpu), &machvirt_binfo);
+}
+
+static QEMUMachine machvirt_a15_machine = {
+    .name = "machvirt-a15",
+    .desc = "ARM Virtual Cortex-A15",
+    .init = machvirt_init,
+    .max_cpus = 4,
+    DEFAULT_MACHINE_OPTIONS,
+};
+
+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 v2 1/2] ARM: Allow boards to provide an fdt blob

[Peter Maydell]

On 30 April 2013 17:04, John Rigby <john.rigby@linaro.org> wrote:
> 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.
>
> Also allow dumping of device tree by calling qemu_devtree_dumpdtb
> near the end of load_dtb.

"Also ..." in a commit message is usually a clue that you
should split the patch :-)

> Signed-off-by: John Rigby <john.rigby@linaro.org>
> ---
>  hw/arm/boot.c        |   31 ++++++++++++++++++++-----------
>  include/hw/arm/arm.h |    6 ++++++
>  2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index f451529..de71edf 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -235,19 +235,27 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>      int size, rc;
>      uint32_t acells, scells, hival;
>
> -    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;
> -    }
> +    if (binfo->dtb_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);
> +            return -1;
> +        }
>
> -    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);
> +            return -1;
> +        }
>          g_free(filename);
> -        return -1;
> +    } else if (binfo->get_dtb) {
> +        fdt = binfo->get_dtb(addr, binfo, &size);
> +        if (!fdt) {
> +            fprintf(stderr, "Couldn't get dtb blob from board func\n");

I think it's better to avoid being too abbreviated in error messages;
"Attempt to create dtb blob for this board model failed\n",
perhaps?

> +            return -1;
> +        }
>      }
> -    g_free(filename);
>
>      acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
>      scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
> @@ -304,6 +312,7 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
>          }
>      }
> +    qemu_devtree_dumpdtb(fdt, size);
>
>      cpu_physical_memory_write(addr, fdt, size);
>
> @@ -440,7 +449,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 7b2b02d..4c56a1b 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -31,6 +31,10 @@ struct arm_boot_info {
>      const char *kernel_cmdline;
>      const char *initrd_filename;
>      const char *dtb_filename;
> +    /* 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.
> +     */
> +    void *(*get_dtb)(hwaddr addr, const struct arm_boot_info *binfo, int *size);
>      hwaddr 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,
> @@ -59,6 +63,8 @@ struct arm_boot_info {
>      int is_linux;
>      hwaddr initrd_start;
>      hwaddr initrd_size;
> +    void *dtb_blob;
> +    int dtb_blob_size;

The get_dtb function returns the blob and its size via
the hook's arguments, so what are these extra fields for?

>      hwaddr entry;
>  };
>  void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
> --
> 1.7.9.5
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] ARM: Add mach-virt platform

[Peter Maydell]

On 30 April 2013 17:04, John Rigby <john.rigby@linaro.org> wrote:
> Add mach-virt platform support cooresponding to

"corresponding"

I wonder if we should just call this "virt" ?

> /arch/arm/mach-virt in kernel tree.
>
> For now it is not virtual but instantiates a pl011 uart
> and and sp804 timer.  The uart is need for a console
> the timer is needed when running without kvm and there
> is no arch timer.
>
> Signed-off-by: John Rigby <john.rigby@linaro.org>
> ---
>  hw/arm/Makefile.objs |    2 +-
>  hw/arm/mach-virt.c   |  337 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 338 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/mach-virt.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 9e3a06f..0789352 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,5 +1,5 @@
>  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 += integratorcp.o kzm.o mach-virt.o mainstone.o musicpal.o nseries.o
>  obj-y += omap_sx1.o palm.o pic_cpu.o realview.o spitz.o stellaris.o
>  obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>
> diff --git a/hw/arm/mach-virt.c b/hw/arm/mach-virt.c
> new file mode 100644
> index 0000000..b55e4d1
> --- /dev/null
> +++ b/hw/arm/mach-virt.c
> @@ -0,0 +1,337 @@
> +/*
> + * ARM mach-virt emulation
> + *
> + * Copyright (c) 2013 Linaro
> + *
> + * 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/>.

This comment could also use a brief description of what this
is emulating.

> + *
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm-misc.h"
> +#include "hw/primecell.h"
> +#include "hw/devices.h"
> +#include "net/net.h"
> +#include "sysemu/device_tree.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/boards.h"
> +#include "exec/address-spaces.h"
> +#include "hw/flash.h"
> +#include "libfdt_env.h"
> +
> +#define ARM_GIC_DIST_BASE 0x2c001000
> +#define ARM_GIC_DIST_SIZE 0x1000
> +#define ARM_GIC_CPUI_BASE 0x2c002000
> +#define ARM_GIC_CPUI_SIZE 0x1000
> +
> +#define GIC_FDT_IRQ_NUM_CELLS 3
> +
> +#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_SHIFT 8
> +#define GIC_FDT_IRQ_PPI_CPU_MASK (0xff << GIC_FDT_IRQ_PPI_CPU_SHIFT)
> +
> +#define CPU_NAME_MAX_LEN 16
> +static void add_cpu_nodes(void *fdt, const struct arm_boot_info *binfo)
> +{
> +    int cpu;
> +
> +    qemu_devtree_add_subnode(fdt, "/cpus");
> +    qemu_devtree_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
> +    qemu_devtree_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
> +
> +    for (cpu = 0; cpu < binfo->nb_cpus; ++cpu) {
> +        char cpu_name[CPU_NAME_MAX_LEN];
> +
> +        snprintf(cpu_name, CPU_NAME_MAX_LEN, "/cpus/cpu@%d", cpu);
> +
> +        qemu_devtree_add_subnode(fdt, cpu_name);
> +        qemu_devtree_setprop_string(fdt, cpu_name, "device_type", "cpu");
> +        qemu_devtree_setprop_string(fdt, cpu_name, "compatible",
> +            "arm,cortex-a15");
> +
> +        if (binfo->nb_cpus > 1) {
> +            qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "psci");

Does this make sense for the non-KVM case? Should we implement
PSCI for TCG too?

> +        }
> +
> +        qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu);
> +    }
> +}
> +
> +static void add_gic_nodes(void *fdt, uint32_t phandle)
> +{
> +    uint64_t reg_prop[] = {
> +        cpu_to_fdt64(ARM_GIC_DIST_BASE), cpu_to_fdt64(ARM_GIC_DIST_SIZE),
> +        cpu_to_fdt64(ARM_GIC_CPUI_BASE), cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
> +    };
> +
> +    qemu_devtree_add_subnode(fdt, "/intc");
> +    qemu_devtree_setprop_string(fdt, "/intc", "compatible",
> +            "arm,cortex-a15-gic");
> +    qemu_devtree_setprop_cell(fdt, "/intc", "#interrupt-cells",
> +            GIC_FDT_IRQ_NUM_CELLS);
> +    qemu_devtree_setprop(fdt, "/intc", "interrupt-controller", NULL, 0);
> +    qemu_devtree_setprop(fdt, "/intc", "reg", reg_prop, sizeof(reg_prop));
> +    qemu_devtree_setprop_cell(fdt, "/intc", "phandle", phandle);
> +}
> +
> +static void add_timer_nodes(void *fdt, const struct arm_boot_info *binfo)
> +{
> +    uint32_t cpu_mask =
> +        (((1 << binfo->nb_cpus) - 1) << GIC_FDT_IRQ_PPI_CPU_SHIFT)
> +                        & GIC_FDT_IRQ_PPI_CPU_MASK;
> +    cpu_mask = 0xf00;
> +    uint32_t irq_prop[] = {
> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
> +        cpu_to_fdt32(13),
> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +
> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
> +        cpu_to_fdt32(14),
> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +
> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
> +        cpu_to_fdt32(11),
> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +
> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
> +        cpu_to_fdt32(10),
> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +    };
> +
> +    qemu_devtree_add_subnode(fdt, "/timer");
> +    qemu_devtree_setprop_string(fdt, "/timer", "compatible", "arm,armv7-timer");
> +    qemu_devtree_setprop(fdt, "/timer", "interrupts", irq_prop,
> +            sizeof(irq_prop));

TCG doesn't implement these timers either (though we probably
ought to).

> +}
> +
> +#define NRMAPIRQS 43
> +
> +static void add_interrupt_map(void *fdt, const char *soc, uint32_t gic)
> +{
> +    int i, irq;
> +    int len;
> +    uint32_t *map;
> +
> +    qemu_devtree_setprop_cell(fdt, soc, "#interrupt-cells", 0x1);
> +    qemu_devtree_setprop_cells(fdt, soc, "interrupt-map-mask", 0, 63);
> +
> +    len = NRMAPIRQS * 6 * sizeof(uint32_t);
> +    map = g_malloc(len);
> +
> +    for (i = 0, irq = 0; irq < NRMAPIRQS; irq++) {
> +        map[i++] = cpu_to_be32(0x0);
> +        map[i++] = cpu_to_be32(irq);
> +        map[i++] = cpu_to_be32(gic);
> +        map[i++] = cpu_to_be32(0x0);
> +        map[i++] = cpu_to_be32(irq);
> +        map[i++] = cpu_to_be32(4);
> +    }
> +    qemu_devtree_setprop(fdt, soc, "interrupt-map", map, len);
> +    g_free(map);
> +}
> +
> +static void *machvirt_dtb(hwaddr addr, const struct arm_boot_info *binfo,
> +        int *fdt_size)
> +{
> +    void *fdt;
> +    uint32_t gic_phandle;
> +    uint32_t clock_phandle;
> +    char compatible_sb[] = "simple-bus\0arm,amba-bus";
> +    char compatible_uart[] = "arm,pl011\0arm,primecell";
> +    char compatible_timer[] = "arm,sp804\0arm,primecell";
> +    char clock_names_uart[] = "uartclk\0apb_pclk";
> +    char clock_names_timer[] = "timerclk\0apb_pclk";
> +
> +    fdt = create_device_tree(fdt_size);
> +    if (fdt == NULL) {
> +        goto out;

This is the only use of the 'out' label I think so you might
as well just 'return NULL;'.

> +    }
> +    gic_phandle = qemu_devtree_alloc_phandle(fdt);
> +
> +    /* Header */
> +    qemu_devtree_setprop_cell(fdt, "/", "interrupt-parent", gic_phandle);
> +    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
> +     * only add properties are not added later in arm_boot

I can't parse this comment...

> +     */
> +    qemu_devtree_add_subnode(fdt, "/chosen");
> +    qemu_devtree_add_subnode(fdt, "/memory");
> +    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
> +
> +    /* CPU and peripherals (interrupt controller, timers, etc) */
> +    add_cpu_nodes(fdt, binfo);
> +    add_gic_nodes(fdt, gic_phandle);
> +    add_timer_nodes(fdt, binfo);
> +
> +    qemu_devtree_add_subnode(fdt, "/soc");
> +    qemu_devtree_setprop(fdt, "/soc", "compatible", compatible_sb,
> +            sizeof(compatible_sb));
> +    qemu_devtree_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
> +    qemu_devtree_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
> +    qemu_devtree_setprop_cells(fdt, "/soc", "ranges", 0x10000000, 0x0,
> +            0x10000000, 0x10000000);
> +    qemu_devtree_setprop_cell(fdt, "/soc", "#interrupt-cells", 0x1);
> +
> +    add_interrupt_map(fdt, "/soc", gic_phandle);
> +
> +    clock_phandle = qemu_devtree_alloc_phandle(fdt);
> +    qemu_devtree_add_subnode(fdt, "/soc/clock");
> +    qemu_devtree_setprop_string(fdt, "/soc/clock", "compatible", "fixed-clock");
> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "#clock-cells", 0x0);
> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "clock-frequency", 24000000);
> +    qemu_devtree_setprop_string(fdt, "/soc/clock", "clock-output-names",
> +            "clk24mhz");
> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "phandle", clock_phandle);
> +
> +    qemu_devtree_add_subnode(fdt, "/soc/uart");
> +    qemu_devtree_setprop(fdt, "/soc/uart", "compatible", compatible_uart,
> +            sizeof(compatible_uart));
> +    qemu_devtree_setprop_cells(fdt, "/soc/uart", "reg", 0x1c090000, 0x1000);
> +    qemu_devtree_setprop_cell(fdt, "/soc/uart", "interrupts", 0x5);
> +    qemu_devtree_setprop_cells(fdt, "/soc/uart", "clocks", clock_phandle,
> +            clock_phandle);
> +    qemu_devtree_setprop(fdt, "/soc/uart", "clock-names", clock_names_uart,
> +            sizeof(clock_names_uart));

Having this all hardcoded here means that we have two different
bits of code (this dtb writing code and the code below which actually
instantiates qemu devices) which have to stay in sync. Can you find
an abstraction (data tables or something??) which avoids the
duplication, please?

> +
> +    qemu_devtree_add_subnode(fdt, "/soc/timer");
> +    qemu_devtree_setprop(fdt, "/soc/timer", "compatible", compatible_timer,
> +            sizeof(compatible_timer));
> +    qemu_devtree_setprop_cells(fdt, "/soc/timer", "reg", 0x1c110000, 0x1000);
> +    qemu_devtree_setprop_cell(fdt, "/soc/timer", "interrupts", 0x2);
> +    qemu_devtree_setprop_cells(fdt, "/soc/timer", "clocks", clock_phandle,
> +            clock_phandle);
> +    qemu_devtree_setprop(fdt, "/soc/timer", "clock-names", clock_names_timer,
> +            sizeof(clock_names_timer));
> +#if 0
> +    /* PSCI firmware */
> +    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

Don't include #if-0'd out code, please.

> +
> +out:
> +    return fdt;
> +}
> +
> +
> +static struct arm_boot_info machvirt_binfo;
> +
> +static void machvirt_init(QEMUMachineInitArgs *args)
> +{
> +    qemu_irq pic[64];
> +    MemoryRegion *sysmem = get_system_memory();
> +    int n;
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +    qemu_irq cpu_irq[4];
> +    DeviceState *dev;
> +    SysBusDevice *busdev;
> +    const char *cpu_model = args->cpu_model;
> +
> +    if (!cpu_model) {
> +        cpu_model = "cortex-a15";
> +    }
> +
> +    for (n = 0; n < smp_cpus; n++) {
> +        ARMCPU *cpu;
> +        qemu_irq *irqp;
> +
> +        cpu = cpu_arm_init(cpu_model);
> +        if (!cpu) {
> +            fprintf(stderr, "Unable to find CPU definition %s\n", cpu_model);
> +            exit(1);
> +        }
> +        irqp = arm_pic_init_cpu(cpu);
> +        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
> +    }
> +
> +    {
> +        /* We have to use a separate 64 bit variable here to avoid the gcc
> +         * "comparison is always false due to limited range of data type"
> +         * warning if we are on a host where ram_addr_t is 32 bits.
> +         */
> +        uint64_t rsz = ram_size;
> +        if (rsz > (30ULL * 1024 * 1024 * 1024)) {
> +            fprintf(stderr, "mach-virt: cannot model more than 30GB RAM\n");
> +            exit(1);
> +        }
> +    }
> +
> +    memory_region_init_ram(ram, "mach-virt.ram", ram_size);
> +    vmstate_register_ram_global(ram);
> +    /* RAM is from 0x80000000 upwards; there is no low-memory alias for it. */
> +    memory_region_add_subregion(sysmem, 0x80000000, ram);

This is a pure-virtual machine, any reason to start the RAM
so high in the address space?

Similarly you could put the peripherals at some usefully
rationalised addresses; no need to have them where vexpress
happens to put them. Ditto IRQ line assignments I suppose.

> +
> +    /* 0x2c000000 A15MPCore private memory region (GIC) */
> +    dev = qdev_create(NULL, "a15mpcore_priv");

If we always create the A15 private peripherals we should
probably fail with an error message if the user tries to
pass us some other CPU type.

> +    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
> +    qdev_init_nofail(dev);
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, 0x2c000000);
> +    for (n = 0; n < smp_cpus; n++) {
> +        sysbus_connect_irq(busdev, n, cpu_irq[n]);
> +    }
> +
> +    for (n = 0; n < 64; n++) {
> +        pic[n] = qdev_get_gpio_in(dev, n);
> +    }
> +
> +    /*
> +     * Not completely virt yet, we do have one uart
> +     * .. and a timer...
> +     */
> +    sysbus_create_simple("pl011", 0x1c090000, pic[5]);
> +    sysbus_create_simple("sp804", 0x1c110000, pic[2]);
> +
> +    machvirt_binfo.ram_size = args->ram_size;
> +    machvirt_binfo.kernel_filename = args->kernel_filename;
> +    machvirt_binfo.kernel_cmdline = args->kernel_cmdline;
> +    machvirt_binfo.initrd_filename = args->initrd_filename;
> +    machvirt_binfo.nb_cpus = smp_cpus;
> +    machvirt_binfo.board_id = -1;
> +    machvirt_binfo.loader_start = 0x80000000;
> +    machvirt_binfo.smp_bootreg_addr = 0x1c010000 + 0x30;

This doesn't make sense -- it's borrowed from vexpress-a15,
but on that board this address is in the realview_sysctl
device. You haven't put anything at this address at all.

I think the expectation for mach-virt is that secondary
CPUs are started via PSCI, which probably means we want
to implement that for TCG and not to use our standard
secondary-cpu-boot-stub code from boot.c.

> +    machvirt_binfo.gic_cpu_if_addr = 0x2c002000;
> +    machvirt_binfo.get_dtb = machvirt_dtb;
> +    arm_load_kernel(arm_env_get_cpu(first_cpu), &machvirt_binfo);
> +}
> +
> +static QEMUMachine machvirt_a15_machine = {
> +    .name = "machvirt-a15",
> +    .desc = "ARM Virtual Cortex-A15",

I don't think the machine name or description should include
the CPU type. We might limit it to A15 for the moment, but
we should leave ourselves the freedom to support also
different CPU types in future (by the user passing -cpu
and the init function handling it appropriately.)

> +    .init = machvirt_init,
> +    .max_cpus = 4,
> +    DEFAULT_MACHINE_OPTIONS,
> +};
> +
> +static void machvirt_machine_init(void)
> +{
> +    qemu_register_machine(&machvirt_a15_machine);
> +}
> +
> +machine_init(machvirt_machine_init);
> --
> 1.7.9.5

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] ARM: Add mach-virt platform

[John Rigby]

On Fri, May 3, 2013 at 8:29 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 April 2013 17:04, John Rigby <john.rigby@linaro.org> wrote:
>> Add mach-virt platform support cooresponding to
>
> "corresponding"
>
> I wonder if we should just call this "virt" ?
>
>> /arch/arm/mach-virt in kernel tree.
>>
>> For now it is not virtual but instantiates a pl011 uart
>> and and sp804 timer.  The uart is need for a console
>> the timer is needed when running without kvm and there
>> is no arch timer.
>>
>> Signed-off-by: John Rigby <john.rigby@linaro.org>
>> ---
>>  hw/arm/Makefile.objs |    2 +-
>>  hw/arm/mach-virt.c   |  337 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 338 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/arm/mach-virt.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 9e3a06f..0789352 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -1,5 +1,5 @@
>>  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 += integratorcp.o kzm.o mach-virt.o mainstone.o musicpal.o nseries.o
>>  obj-y += omap_sx1.o palm.o pic_cpu.o realview.o spitz.o stellaris.o
>>  obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>>
>> diff --git a/hw/arm/mach-virt.c b/hw/arm/mach-virt.c
>> new file mode 100644
>> index 0000000..b55e4d1
>> --- /dev/null
>> +++ b/hw/arm/mach-virt.c
>> @@ -0,0 +1,337 @@
>> +/*
>> + * ARM mach-virt emulation
>> + *
>> + * Copyright (c) 2013 Linaro
>> + *
>> + * 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/>.
>
> This comment could also use a brief description of what this
> is emulating.
>
ok
>> + *
>> + */
>> +
>> +#include "hw/sysbus.h"
>> +#include "hw/arm-misc.h"
>> +#include "hw/primecell.h"
>> +#include "hw/devices.h"
>> +#include "net/net.h"
>> +#include "sysemu/device_tree.h"
>> +#include "sysemu/sysemu.h"
>> +#include "hw/boards.h"
>> +#include "exec/address-spaces.h"
>> +#include "hw/flash.h"
>> +#include "libfdt_env.h"
>> +
>> +#define ARM_GIC_DIST_BASE 0x2c001000
>> +#define ARM_GIC_DIST_SIZE 0x1000
>> +#define ARM_GIC_CPUI_BASE 0x2c002000
>> +#define ARM_GIC_CPUI_SIZE 0x1000
>> +
>> +#define GIC_FDT_IRQ_NUM_CELLS 3
>> +
>> +#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_SHIFT 8
>> +#define GIC_FDT_IRQ_PPI_CPU_MASK (0xff << GIC_FDT_IRQ_PPI_CPU_SHIFT)
>> +
>> +#define CPU_NAME_MAX_LEN 16
>> +static void add_cpu_nodes(void *fdt, const struct arm_boot_info *binfo)
>> +{
>> +    int cpu;
>> +
>> +    qemu_devtree_add_subnode(fdt, "/cpus");
>> +    qemu_devtree_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
>> +    qemu_devtree_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
>> +
>> +    for (cpu = 0; cpu < binfo->nb_cpus; ++cpu) {
>> +        char cpu_name[CPU_NAME_MAX_LEN];
>> +
>> +        snprintf(cpu_name, CPU_NAME_MAX_LEN, "/cpus/cpu@%d", cpu);
>> +
>> +        qemu_devtree_add_subnode(fdt, cpu_name);
>> +        qemu_devtree_setprop_string(fdt, cpu_name, "device_type", "cpu");
>> +        qemu_devtree_setprop_string(fdt, cpu_name, "compatible",
>> +            "arm,cortex-a15");
>> +
>> +        if (binfo->nb_cpus > 1) {
>> +            qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "psci");
>
> Does this make sense for the non-KVM case? Should we implement
> PSCI for TCG too?
>
Wrapping parts like this in if (feature_to_be_in_tcg_someday()) would
mean adding stubs for each yet to be implemented feature.  Or I could
just use if (kvm_enabled()) for now?
>> +        }
>> +
>> +        qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu);
>> +    }
>> +}
>> +
>> +static void add_gic_nodes(void *fdt, uint32_t phandle)
>> +{
>> +    uint64_t reg_prop[] = {
>> +        cpu_to_fdt64(ARM_GIC_DIST_BASE), cpu_to_fdt64(ARM_GIC_DIST_SIZE),
>> +        cpu_to_fdt64(ARM_GIC_CPUI_BASE), cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
>> +    };
>> +
>> +    qemu_devtree_add_subnode(fdt, "/intc");
>> +    qemu_devtree_setprop_string(fdt, "/intc", "compatible",
>> +            "arm,cortex-a15-gic");
>> +    qemu_devtree_setprop_cell(fdt, "/intc", "#interrupt-cells",
>> +            GIC_FDT_IRQ_NUM_CELLS);
>> +    qemu_devtree_setprop(fdt, "/intc", "interrupt-controller", NULL, 0);
>> +    qemu_devtree_setprop(fdt, "/intc", "reg", reg_prop, sizeof(reg_prop));
>> +    qemu_devtree_setprop_cell(fdt, "/intc", "phandle", phandle);
>> +}
>> +
>> +static void add_timer_nodes(void *fdt, const struct arm_boot_info *binfo)
>> +{
>> +    uint32_t cpu_mask =
>> +        (((1 << binfo->nb_cpus) - 1) << GIC_FDT_IRQ_PPI_CPU_SHIFT)
>> +                        & GIC_FDT_IRQ_PPI_CPU_MASK;
>> +    cpu_mask = 0xf00;
>> +    uint32_t irq_prop[] = {
>> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>> +        cpu_to_fdt32(13),
>> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
>> +
>> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>> +        cpu_to_fdt32(14),
>> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
>> +
>> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>> +        cpu_to_fdt32(11),
>> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
>> +
>> +        cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>> +        cpu_to_fdt32(10),
>> +        cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
>> +    };
>> +
>> +    qemu_devtree_add_subnode(fdt, "/timer");
>> +    qemu_devtree_setprop_string(fdt, "/timer", "compatible", "arm,armv7-timer");
>> +    qemu_devtree_setprop(fdt, "/timer", "interrupts", irq_prop,
>> +            sizeof(irq_prop));
>
> TCG doesn't implement these timers either (though we probably
> ought to).
>
same as above, I can add a stub has_armv7_timer() or just use kvm_enabled
>> +}
>> +
>> +#define NRMAPIRQS 43
>> +
>> +static void add_interrupt_map(void *fdt, const char *soc, uint32_t gic)
>> +{
>> +    int i, irq;
>> +    int len;
>> +    uint32_t *map;
>> +
>> +    qemu_devtree_setprop_cell(fdt, soc, "#interrupt-cells", 0x1);
>> +    qemu_devtree_setprop_cells(fdt, soc, "interrupt-map-mask", 0, 63);
>> +
>> +    len = NRMAPIRQS * 6 * sizeof(uint32_t);
>> +    map = g_malloc(len);
>> +
>> +    for (i = 0, irq = 0; irq < NRMAPIRQS; irq++) {
>> +        map[i++] = cpu_to_be32(0x0);
>> +        map[i++] = cpu_to_be32(irq);
>> +        map[i++] = cpu_to_be32(gic);
>> +        map[i++] = cpu_to_be32(0x0);
>> +        map[i++] = cpu_to_be32(irq);
>> +        map[i++] = cpu_to_be32(4);
>> +    }
>> +    qemu_devtree_setprop(fdt, soc, "interrupt-map", map, len);
>> +    g_free(map);
>> +}
>> +
>> +static void *machvirt_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>> +        int *fdt_size)
>> +{
>> +    void *fdt;
>> +    uint32_t gic_phandle;
>> +    uint32_t clock_phandle;
>> +    char compatible_sb[] = "simple-bus\0arm,amba-bus";
>> +    char compatible_uart[] = "arm,pl011\0arm,primecell";
>> +    char compatible_timer[] = "arm,sp804\0arm,primecell";
>> +    char clock_names_uart[] = "uartclk\0apb_pclk";
>> +    char clock_names_timer[] = "timerclk\0apb_pclk";
>> +
>> +    fdt = create_device_tree(fdt_size);
>> +    if (fdt == NULL) {
>> +        goto out;
>
> This is the only use of the 'out' label I think so you might
> as well just 'return NULL;'.
ok
>
>> +    }
>> +    gic_phandle = qemu_devtree_alloc_phandle(fdt);
>> +
>> +    /* Header */
>> +    qemu_devtree_setprop_cell(fdt, "/", "interrupt-parent", gic_phandle);
>> +    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
>> +     * only add properties are not added later in arm_boot
>
> I can't parse this comment...
oops, the arm boot code will add properties to exiting nodes in the
fdt blob but will not add the actual nodes so empty nodes must be
created here
>
>> +     */
>> +    qemu_devtree_add_subnode(fdt, "/chosen");
>> +    qemu_devtree_add_subnode(fdt, "/memory");
>> +    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
>> +
>> +    /* CPU and peripherals (interrupt controller, timers, etc) */
>> +    add_cpu_nodes(fdt, binfo);
>> +    add_gic_nodes(fdt, gic_phandle);
>> +    add_timer_nodes(fdt, binfo);
>> +
>> +    qemu_devtree_add_subnode(fdt, "/soc");
>> +    qemu_devtree_setprop(fdt, "/soc", "compatible", compatible_sb,
>> +            sizeof(compatible_sb));
>> +    qemu_devtree_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
>> +    qemu_devtree_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
>> +    qemu_devtree_setprop_cells(fdt, "/soc", "ranges", 0x10000000, 0x0,
>> +            0x10000000, 0x10000000);
>> +    qemu_devtree_setprop_cell(fdt, "/soc", "#interrupt-cells", 0x1);
>> +
>> +    add_interrupt_map(fdt, "/soc", gic_phandle);
>> +
>> +    clock_phandle = qemu_devtree_alloc_phandle(fdt);
>> +    qemu_devtree_add_subnode(fdt, "/soc/clock");
>> +    qemu_devtree_setprop_string(fdt, "/soc/clock", "compatible", "fixed-clock");
>> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "#clock-cells", 0x0);
>> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "clock-frequency", 24000000);
>> +    qemu_devtree_setprop_string(fdt, "/soc/clock", "clock-output-names",
>> +            "clk24mhz");
>> +    qemu_devtree_setprop_cell(fdt, "/soc/clock", "phandle", clock_phandle);
>> +
>> +    qemu_devtree_add_subnode(fdt, "/soc/uart");
>> +    qemu_devtree_setprop(fdt, "/soc/uart", "compatible", compatible_uart,
>> +            sizeof(compatible_uart));
>> +    qemu_devtree_setprop_cells(fdt, "/soc/uart", "reg", 0x1c090000, 0x1000);
>> +    qemu_devtree_setprop_cell(fdt, "/soc/uart", "interrupts", 0x5);
>> +    qemu_devtree_setprop_cells(fdt, "/soc/uart", "clocks", clock_phandle,
>> +            clock_phandle);
>> +    qemu_devtree_setprop(fdt, "/soc/uart", "clock-names", clock_names_uart,
>> +            sizeof(clock_names_uart));
>
> Having this all hardcoded here means that we have two different
> bits of code (this dtb writing code and the code below which actually
> instantiates qemu devices) which have to stay in sync. Can you find
> an abstraction (data tables or something??) which avoids the
> duplication, please?
I'll come up with something.
>
>> +
>> +    qemu_devtree_add_subnode(fdt, "/soc/timer");
>> +    qemu_devtree_setprop(fdt, "/soc/timer", "compatible", compatible_timer,
>> +            sizeof(compatible_timer));
>> +    qemu_devtree_setprop_cells(fdt, "/soc/timer", "reg", 0x1c110000, 0x1000);
>> +    qemu_devtree_setprop_cell(fdt, "/soc/timer", "interrupts", 0x2);
>> +    qemu_devtree_setprop_cells(fdt, "/soc/timer", "clocks", clock_phandle,
>> +            clock_phandle);
>> +    qemu_devtree_setprop(fdt, "/soc/timer", "clock-names", clock_names_timer,
>> +            sizeof(clock_names_timer));
>> +#if 0
>> +    /* PSCI firmware */
>> +    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
>
> Don't include #if-0'd out code, please.
ok
>
>> +
>> +out:
>> +    return fdt;
>> +}
>> +
>> +
>> +static struct arm_boot_info machvirt_binfo;
>> +
>> +static void machvirt_init(QEMUMachineInitArgs *args)
>> +{
>> +    qemu_irq pic[64];
>> +    MemoryRegion *sysmem = get_system_memory();
>> +    int n;
>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>> +    qemu_irq cpu_irq[4];
>> +    DeviceState *dev;
>> +    SysBusDevice *busdev;
>> +    const char *cpu_model = args->cpu_model;
>> +
>> +    if (!cpu_model) {
>> +        cpu_model = "cortex-a15";
>> +    }
>> +
>> +    for (n = 0; n < smp_cpus; n++) {
>> +        ARMCPU *cpu;
>> +        qemu_irq *irqp;
>> +
>> +        cpu = cpu_arm_init(cpu_model);
>> +        if (!cpu) {
>> +            fprintf(stderr, "Unable to find CPU definition %s\n", cpu_model);
>> +            exit(1);
>> +        }
>> +        irqp = arm_pic_init_cpu(cpu);
>> +        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
>> +    }
>> +
>> +    {
>> +        /* We have to use a separate 64 bit variable here to avoid the gcc
>> +         * "comparison is always false due to limited range of data type"
>> +         * warning if we are on a host where ram_addr_t is 32 bits.
>> +         */
>> +        uint64_t rsz = ram_size;
>> +        if (rsz > (30ULL * 1024 * 1024 * 1024)) {
>> +            fprintf(stderr, "mach-virt: cannot model more than 30GB RAM\n");
>> +            exit(1);
>> +        }
>> +    }
>> +
>> +    memory_region_init_ram(ram, "mach-virt.ram", ram_size);
>> +    vmstate_register_ram_global(ram);
>> +    /* RAM is from 0x80000000 upwards; there is no low-memory alias for it. */
>> +    memory_region_add_subregion(sysmem, 0x80000000, ram);
>
> This is a pure-virtual machine, any reason to start the RAM
> so high in the address space?

mach-virt started out as a copy of vexpress so that is one reason.
Another is that kvmtool uses this same address so I thought is was ok.
>
> Similarly you could put the peripherals at some usefully
> rationalised addresses; no need to have them where vexpress
> happens to put them. Ditto IRQ line assignments I suppose.
>
>> +
>> +    /* 0x2c000000 A15MPCore private memory region (GIC) */
>> +    dev = qdev_create(NULL, "a15mpcore_priv");
>
> If we always create the A15 private peripherals we should
> probably fail with an error message if the user tries to
> pass us some other CPU type.
k
>
>> +    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
>> +    qdev_init_nofail(dev);
>> +    busdev = SYS_BUS_DEVICE(dev);
>> +    sysbus_mmio_map(busdev, 0, 0x2c000000);
>> +    for (n = 0; n < smp_cpus; n++) {
>> +        sysbus_connect_irq(busdev, n, cpu_irq[n]);
>> +    }
>> +
>> +    for (n = 0; n < 64; n++) {
>> +        pic[n] = qdev_get_gpio_in(dev, n);
>> +    }
>> +
>> +    /*
>> +     * Not completely virt yet, we do have one uart
>> +     * .. and a timer...
>> +     */
>> +    sysbus_create_simple("pl011", 0x1c090000, pic[5]);
>> +    sysbus_create_simple("sp804", 0x1c110000, pic[2]);
>> +
>> +    machvirt_binfo.ram_size = args->ram_size;
>> +    machvirt_binfo.kernel_filename = args->kernel_filename;
>> +    machvirt_binfo.kernel_cmdline = args->kernel_cmdline;
>> +    machvirt_binfo.initrd_filename = args->initrd_filename;
>> +    machvirt_binfo.nb_cpus = smp_cpus;
>> +    machvirt_binfo.board_id = -1;
>> +    machvirt_binfo.loader_start = 0x80000000;
>> +    machvirt_binfo.smp_bootreg_addr = 0x1c010000 + 0x30;
>
> This doesn't make sense -- it's borrowed from vexpress-a15,
> but on that board this address is in the realview_sysctl
> device. You haven't put anything at this address at all.
this is more residue from vexpress, I will remove it
>
> I think the expectation for mach-virt is that secondary
> CPUs are started via PSCI, which probably means we want
> to implement that for TCG and not to use our standard
> secondary-cpu-boot-stub code from boot.c.
ok, if (!kvm_enabled()) smp_cpus better be 1 until we have psci
implemented in TCG
>
>> +    machvirt_binfo.gic_cpu_if_addr = 0x2c002000;
>> +    machvirt_binfo.get_dtb = machvirt_dtb;
>> +    arm_load_kernel(arm_env_get_cpu(first_cpu), &machvirt_binfo);
>> +}
>> +
>> +static QEMUMachine machvirt_a15_machine = {
>> +    .name = "machvirt-a15",
>> +    .desc = "ARM Virtual Cortex-A15",
>
> I don't think the machine name or description should include
> the CPU type. We might limit it to A15 for the moment, but
> we should leave ourselves the freedom to support also
> different CPU types in future (by the user passing -cpu
> and the init function handling it appropriately.)
ok, if no -cpu then default to A15 if -cpu is not A15 then error for now
>
>> +    .init = machvirt_init,
>> +    .max_cpus = 4,
>> +    DEFAULT_MACHINE_OPTIONS,
>> +};
>> +
>> +static void machvirt_machine_init(void)
>> +{
>> +    qemu_register_machine(&machvirt_a15_machine);
>> +}
>> +
>> +machine_init(machvirt_machine_init);
>> --
>> 1.7.9.5
>
> thanks
> -- PMM