[Qemu-devel] Add minimal support for Cortex A15 for ARM KVM

[Qemu-devel] Add minimal support for Cortex A15 for ARM KVM

[bill4carson]

This patch adds minimal support for Cortex A15 to run under ARM KVM as guest
os, and architecture timer is NOT supported yet. 

Qemu based on:
git://git.linaro.org/qemu/qemu-linaro.git


Makefile.target     |    2 +-
hw/a15mpcore.c      |  118 +++++++++++++++++++++++++++++++
hw/vexpress.c       |  192 +++++++++++++++++++++++++++++++++++++++++++++++++++
target-arm/cpu.h    |    1 +
target-arm/helper.c |   31 ++++++++
5 files changed, 343 insertions(+), 1 deletions(-)
create mode 100644 hw/a15mpcore.c

[Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[bill4carson]

From: Bill Carson <bill4carson@gmail.com>

This patch adds minimal codes to support A15  which enables ARM KVM could
run Guest OS build with Versatile Express Cortex-A15x4 tile.

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 Makefile.target     |    2 +-
 hw/a15mpcore.c      |  118 +++++++++++++++++++++++++++++++
 hw/vexpress.c       |  192 +++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h    |    1 +
 target-arm/helper.c |   31 ++++++++
 5 files changed, 343 insertions(+), 1 deletions(-)
 create mode 100644 hw/a15mpcore.c

diff --git a/Makefile.target b/Makefile.target
index cce3554..69f0c4f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -343,7 +343,7 @@ endif
 obj-arm-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o
 obj-arm-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o
 obj-arm-y += versatile_pci.o
-obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
+obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o a15mpcore.o
 obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o
 obj-arm-y += pl061.o
 obj-arm-y += arm-semi.o
diff --git a/hw/a15mpcore.c b/hw/a15mpcore.c
new file mode 100644
index 0000000..2518c17
--- /dev/null
+++ b/hw/a15mpcore.c
@@ -0,0 +1,118 @@
+/*
+ * ARM A15MPCore internal peripheral emulation (common code).
+ *
+ * Based on mpcore.c written by Paul Brook
+ * This code is licensed under the GPL.
+ */
+
+#include "sysbus.h"
+#include "qemu-timer.h"
+
+/* 64 external IRQ lines */
+#define GIC_NIRQ 96
+
+#define NCPU 4
+
+static inline int
+gic_get_current_cpu(void)
+{
+  return cpu_single_env->cpu_index;
+}
+
+#include "arm_gic.c"
+
+/* MPCore private memory region.  */
+typedef struct a15mpcore_priv_state {
+    gic_state gic;
+    uint32_t scu_control;
+    int iomemtype;
+    uint32_t num_cpu;
+    MemoryRegion iomem;
+    MemoryRegion container;
+} a15mpcore_priv_state;
+
+/* TODO:Per-CPU Timers.  */
+
+
+/* Per-CPU private memory mapped IO.  */
+static uint64_t a15mpcore_priv_read(void *opaque, target_phys_addr_t offset,
+                                 unsigned size)
+{
+    a15mpcore_priv_state *s = (a15mpcore_priv_state *)opaque;
+    int id;
+
+    offset &= 0xfff;
+    /* Interrupt controller.  */
+    if (offset < 0x200) {
+    	id = gic_get_current_cpu();
+	} else {
+        id = (offset - 0x200) >> 8;
+        if (id >= s->num_cpu) {
+                return 0;
+        }
+    }
+	return gic_cpu_read(&s->gic, id, offset & 0xff);
+}
+
+static void a15mpcore_priv_write(void *opaque, target_phys_addr_t offset,
+                              uint64_t value, unsigned size)
+{
+    a15mpcore_priv_state *s = (a15mpcore_priv_state *)opaque;
+    int id;
+	
+    offset &= 0xfff;
+    /* Interrupt controller.  */
+    if (offset < 0x200) {
+    	id = gic_get_current_cpu();
+	} else {
+        id = (offset - 0x200) >> 8;
+        if (id >= s->num_cpu) {
+                return 0;
+        }
+    }
+	return gic_cpu_write(&s->gic, id, offset & 0xff, value);
+}
+
+static const MemoryRegionOps mpcore_priv_ops = {
+    .read = a15mpcore_priv_read,
+    .write = a15mpcore_priv_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void a15mpcore_priv_map_setup(a15mpcore_priv_state *s)
+{
+    memory_region_init(&s->container, "mpcode-priv-container", 0x3000);
+    memory_region_init_io(&s->iomem, &mpcore_priv_ops, s, "mpcode-priv",
+                          0x1000);
+    memory_region_add_subregion(&s->container, 0x2000, &s->iomem);
+    memory_region_add_subregion(&s->container, 0x1000, &s->gic.iomem);
+}
+
+static int a15mpcore_priv_init(SysBusDevice *dev)
+{
+    a15mpcore_priv_state *s = FROM_SYSBUSGIC(a15mpcore_priv_state, dev);
+    int i;
+
+    gic_init(&s->gic, s->num_cpu);
+    a15mpcore_priv_map_setup(s);
+    sysbus_init_mmio_region(dev, &s->container);
+    return 0;
+}
+
+static SysBusDeviceInfo a15mpcore_priv_info = {
+	.init = a15mpcore_priv_init,
+	.qdev.name  = "a15mpcore_priv",
+	.qdev.size  = sizeof(a15mpcore_priv_state),
+	.qdev.props = (Property[]) {
+		DEFINE_PROP_UINT32("num-cpu", a15mpcore_priv_state, num_cpu, 1),
+		DEFINE_PROP_END_OF_LIST(),
+	}
+};
+
+static void a15mpcore_register_devices(void)
+{
+    sysbus_register_withprop(&a15mpcore_priv_info);
+}
+
+device_init(a15mpcore_register_devices)
+
diff --git a/hw/vexpress.c b/hw/vexpress.c
index 0940a26..46a3c76 100644
--- a/hw/vexpress.c
+++ b/hw/vexpress.c
@@ -34,6 +34,10 @@ static struct arm_boot_info vexpress_binfo = {
     .smp_loader_start = SMP_BOOT_ADDR,
 };
 
+static struct arm_boot_info vexpress_a15_binfo = {
+    .smp_loader_start = SMP_BOOT_ADDR,
+};
+
 static void vexpress_a9_init(ram_addr_t ram_size,
                      const char *boot_device,
                      const char *kernel_filename, const char *kernel_cmdline,
@@ -221,9 +225,197 @@ static QEMUMachine vexpress_a9_machine = {
     .max_cpus = 4,
 };
 
+static void vexpress_a15_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)
+{
+    CPUState *env = NULL;
+    ram_addr_t ram_offset, vram_offset, sram_offset;
+    DeviceState *dev, *sysctl, *pl041;
+    SysBusDevice *busdev;
+    qemu_irq *irqp;
+    qemu_irq pic[64];
+    int n;
+    qemu_irq cpu_irq[4];
+    uint32_t proc_id;
+    uint32_t sys_id;
+    ram_addr_t low_ram_size, vram_size, sram_size;
+
+    if (!cpu_model) {
+        cpu_model = "cortex-a15";
+    }
+
+    for (n = 0; n < smp_cpus; n++) {
+        env = cpu_init(cpu_model);
+        if (!env) {
+            fprintf(stderr, "Unable to find CPU definition\n");
+            exit(1);
+        }
+        irqp = arm_pic_init_cpu(env);
+        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
+    }
+
+    if (ram_size > 0x40000000) {
+        /* 1GB is the maximum the address space permits */
+        fprintf(stderr, "vexpress: cannot model more than 1GB RAM\n");
+        exit(1);
+    }
+
+    ram_offset = qemu_ram_alloc(NULL, "vexpress.highmem", ram_size);
+    low_ram_size = ram_size;
+    if (low_ram_size > 0x4000000) {
+        low_ram_size = 0x4000000;
+    }
+    /* RAM is from 0x80000000 upwards. The bottom 64MB of the
+     * address space should in theory be remappable to various
+     * things including ROM or RAM; we always map the RAM there.
+     */
+    cpu_register_physical_memory(0x0, low_ram_size, ram_offset | IO_MEM_RAM);
+    cpu_register_physical_memory(0x80000000, ram_size,
+                                 ram_offset | IO_MEM_RAM);
+
+    /* 0x2c000000 A15MPCore (SCU) private memory region */
+    dev = qdev_create(NULL, "a15mpcore_priv");
+    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
+    qdev_init_nofail(dev);
+    busdev = sysbus_from_qdev(dev);
+    vexpress_a15_binfo.smp_priv_base = 0x2c000000;
+    sysbus_mmio_map(busdev, 0, vexpress_a15_binfo.smp_priv_base);
+    for (n = 0; n < smp_cpus; n++) {
+        sysbus_connect_irq(busdev, n, cpu_irq[n]);
+    }
+    /* Interrupts [42:0] are from the motherboard;
+     * [47:43] are reserved; [63:48] are daughterboard
+     * peripherals. Note that some documentation numbers
+     * external interrupts starting from 32 (because the
+     * A9MP has internal interrupts 0..31).
+     */
+    for (n = 0; n < 64; n++) {
+        pic[n] = qdev_get_gpio_in(dev, n);
+    }
+
+    /* Motherboard peripherals CS7 : 0x10000000 .. 0x10020000 */
+    sys_id =  0x1190f500;
+    proc_id = 0x0c000000; /*depend on Fast module*/
+
+    /* 0x10000000 System registers */
+    sysctl = qdev_create(NULL, "realview_sysctl");
+    qdev_prop_set_uint32(sysctl, "sys_id", sys_id);
+    qdev_init_nofail(sysctl);
+    qdev_prop_set_uint32(sysctl, "proc_id", proc_id);
+	
+	sysbus_mmio_map(sysbus_from_qdev(sysctl), 0, 0x1c010000);
+
+    /* 0x10001000 SP810 system control */
+    /* 0x10002000 serial bus PCI */
+    /* 0x10004000 PL041 audio */
+    pl041 = qdev_create(NULL, "pl041");
+    qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
+    qdev_init_nofail(pl041);
+    sysbus_mmio_map(sysbus_from_qdev(pl041), 0, 0x10004000);
+    sysbus_connect_irq(sysbus_from_qdev(pl041), 0, pic[11]);
+    dev = sysbus_create_varargs("pl181", 0x10005000, pic[9], pic[10], NULL);
+    /* Wire up MMC card detect and read-only signals */
+    qdev_connect_gpio_out(dev, 0,
+                          qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT));
+    qdev_connect_gpio_out(dev, 1,
+                          qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN));
+
+    sysbus_create_simple("pl050_keyboard", 0x1c060000, pic[12]);
+    sysbus_create_simple("pl050_mouse", 0x1c070000, pic[13]);
+
+	sysbus_create_simple("pl011", 0x1c090000, pic[5]);
+	sysbus_create_simple("pl011", 0x1c0a0000, pic[6]);
+    sysbus_create_simple("pl011", 0x1c0b0000, pic[7]);
+    sysbus_create_simple("pl011", 0x1c0c0000, pic[8]);
+
+    /* 0x1000f000 SP805 WDT */
+
+    sysbus_create_simple("sp804", 0x1c110000, pic[2]);
+    sysbus_create_simple("sp804", 0x1c120000, pic[3]);
+
+    /* 0x10016000 Serial Bus DVI */
+
+    sysbus_create_simple("pl031", 0x1c170000, pic[36]); /* RTC */
+
+    /* 0x1001a000 Compact Flash */
+
+    /* 0x1001f000 PL111 CLCD (motherboard) */
+
+    /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
+
+    /* 0x10020000 PL111 CLCD (daughterboard) */
+    sysbus_create_simple("pl111", 0x10020000, pic[44]);
+
+    /* 0x10060000 AXI RAM */
+    /* 0x100e0000 PL341 Dynamic Memory Controller */
+    /* 0x100e1000 PL354 Static Memory Controller */
+    /* 0x100e2000 System Configuration Controller */
+
+    sysbus_create_simple("sp804", 0x100e4000, pic[48]);
+    /* 0x100e5000 SP805 Watchdog module */
+    /* 0x100e6000 BP147 TrustZone Protection Controller */
+    /* 0x100e9000 PL301 'Fast' AXI matrix */
+    /* 0x100ea000 PL301 'Slow' AXI matrix */
+    /* 0x100ec000 TrustZone Address Space Controller */
+    /* 0x10200000 CoreSight debug APB */
+    /* 0x1e00a000 PL310 L2 Cache Controller */
+
+    /* CS0: NOR0 flash          : 0x40000000 .. 0x44000000 */
+    /* CS4: NOR1 flash          : 0x44000000 .. 0x48000000 */
+    /* CS2: SRAM                : 0x48000000 .. 0x4a000000 */
+    sram_size = 0x2000000;
+    sram_offset = qemu_ram_alloc(NULL, "vexpress.sram", sram_size);
+    cpu_register_physical_memory(0x48000000, sram_size,
+                                 sram_offset | IO_MEM_RAM);
+
+    /* CS3: USB, ethernet, VRAM : 0x4c000000 .. 0x50000000 */
+
+    /* 0x4c000000 Video RAM */
+    vram_size = 0x2000000; /* 32MB */
+    vram_offset = qemu_ram_alloc(NULL, "vexpress.vram", vram_size);
+    cpu_register_physical_memory(0x18000000, vram_size,
+                                 vram_offset | IO_MEM_RAM);
+
+    /* 0x4e000000 LAN9118 Ethernet */
+    if (nd_table[0].vlan) {
+        lan9118_init(&nd_table[0], 0x4e000000, pic[15]);
+    }
+
+    /* 0x4f000000 ISP1761 USB */
+
+    /* ??? Hack to map an additional page of ram for the secondary CPU
+       startup code.  I guess this works on real hardware because the
+       BootROM happens to be in ROM/flash or in memory that isn't clobbered
+       until after Linux boots the secondary CPUs.  */
+    ram_offset = qemu_ram_alloc(NULL, "vexpress.hack", 0x1000);
+    cpu_register_physical_memory(SMP_BOOT_ADDR, 0x1000,
+                                 ram_offset | IO_MEM_RAM);
+
+    vexpress_a15_binfo.ram_size = ram_size;
+    vexpress_a15_binfo.kernel_filename = kernel_filename;
+    vexpress_a15_binfo.kernel_cmdline = kernel_cmdline;
+    vexpress_a15_binfo.initrd_filename = initrd_filename;
+    vexpress_a15_binfo.nb_cpus = smp_cpus;
+    vexpress_a15_binfo.board_id = VEXPRESS_BOARD_ID;
+    vexpress_a15_binfo.loader_start = 0x80000000;
+    arm_load_kernel(first_cpu, &vexpress_a15_binfo);
+}
+
+
+static QEMUMachine vexpress_a15_machine = {
+    .name = "vexpress-a15",
+    .desc = "ARM Versatile Express for Cortex-A15",
+    .init = vexpress_a15_init,
+    .use_scsi = 1,
+    .max_cpus = 4,
+};
+
 static void vexpress_machine_init(void)
 {
     qemu_register_machine(&vexpress_a9_machine);
+    qemu_register_machine(&vexpress_a15_machine);
 }
 
 machine_init(vexpress_machine_init);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7e0a667..52334c5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -438,6 +438,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_CORTEXA8    0x410fc080
 #define ARM_CPUID_CORTEXA8_R2 0x412fc083
 #define ARM_CPUID_CORTEXA9    0x410fc090
+#define ARM_CPUID_CORTEXA15   0x410fc0f0
 #define ARM_CPUID_CORTEXM3    0x410fc231
 #define ARM_CPUID_ANY         0xffffffff
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e712554..6a2c89d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -217,6 +217,36 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
         env->cp15.c1_sys = 0x00c50078;
         break;
+    case ARM_CPUID_CORTEXA15: /* most same as A9 */
+        set_feature(env, ARM_FEATURE_V4T);
+        set_feature(env, ARM_FEATURE_V5);
+        set_feature(env, ARM_FEATURE_V6);
+        set_feature(env, ARM_FEATURE_V6K);
+        set_feature(env, ARM_FEATURE_V7);
+        set_feature(env, ARM_FEATURE_AUXCR);
+        set_feature(env, ARM_FEATURE_THUMB2);
+        set_feature(env, ARM_FEATURE_VFP);
+        set_feature(env, ARM_FEATURE_VFP3);
+        set_feature(env, ARM_FEATURE_VFP_FP16);
+        set_feature(env, ARM_FEATURE_NEON);
+        set_feature(env, ARM_FEATURE_THUMB2EE);
+        /* Note that A9 supports the MP extensions even for
+         * A9UP and single-core A9MP (which are both different
+         * and valid configurations; we don't model A9UP).
+         */
+        set_feature(env, ARM_FEATURE_V7MP);
+        env->vfp.xregs[ARM_VFP_FPSID] = 0x41034000; /* Guess */
+        env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
+        env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
+        memcpy(env->cp15.c0_c1, cortexa9_cp15_c0_c1, 8 * sizeof(uint32_t));
+        memcpy(env->cp15.c0_c2, cortexa9_cp15_c0_c2, 8 * sizeof(uint32_t));
+        env->cp15.c0_cachetype = 0x80038003;
+        env->cp15.c0_clid = (1 << 27) | (1 << 24) | 3;
+        env->cp15.c0_ccsid[0] = 0xe00fe015; /* 16k L1 dcache. */
+        env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
+        env->cp15.c1_sys = 0x00c50078;
+        break;
+		
     case ARM_CPUID_CORTEXM3:
         set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
@@ -460,6 +490,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_CORTEXA8, "cortex-a8"},
     { ARM_CPUID_CORTEXA8_R2, "cortex-a8-r2"},
     { ARM_CPUID_CORTEXA9, "cortex-a9"},
+    { ARM_CPUID_CORTEXA15, "cortex-a15"},
     { ARM_CPUID_TI925T, "ti925t" },
     { ARM_CPUID_PXA250, "pxa250" },
     { ARM_CPUID_SA1100,    "sa1100" },
-- 
1.7.1

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 1 December 2011 01:37,  <bill4carson@gmail.com> wrote:
> Qemu based on:
> git://git.linaro.org/qemu/qemu-linaro.git

Please don't post patches to upstream which are based on qemu-linaro.
This work should be entirely doable against upstream qemu.

You don't say whether you've tested this with TCG -- done correctly
it should be entirely possible to run an A15 kernel in QEMU system
mode without using KVM.

> From: Bill Carson <bill4carson@gmail.com>
>
> This patch adds minimal codes to support A15  which enables ARM KVM could
> run Guest OS build with Versatile Express Cortex-A15x4 tile.
>
> Signed-off-by: Bill Carson <bill4carson@gmail.com>

Patch format etiquette:
 * for second and later rounds of a patch please put [PATCH v2] (v3,v4...)
   in the subject lines of patches and cover letters rather than just [PATCH]
 * include (in the cover letter for patch series, below the "---" line for
   a single patch) a brief summary of changes since previous versions of
   the patch

This patch should be split up into a series, something like:
 1/3 add the A15 private peripherals
 2/3 add the A15 cpu support itself
 3/3 add the vexpress A15 board support


> ---
>  Makefile.target     |    2 +-
>  hw/a15mpcore.c      |  118 +++++++++++++++++++++++++++++++
>  hw/vexpress.c       |  192 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/cpu.h    |    1 +
>  target-arm/helper.c |   31 ++++++++
>  5 files changed, 343 insertions(+), 1 deletions(-)
>  create mode 100644 hw/a15mpcore.c
>
> diff --git a/Makefile.target b/Makefile.target
> index cce3554..69f0c4f 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -343,7 +343,7 @@ endif
>  obj-arm-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o
>  obj-arm-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o
>  obj-arm-y += versatile_pci.o
> -obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
> +obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o a15mpcore.o
>  obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o
>  obj-arm-y += pl061.o
>  obj-arm-y += arm-semi.o
> diff --git a/hw/a15mpcore.c b/hw/a15mpcore.c
> new file mode 100644
> index 0000000..2518c17
> --- /dev/null
> +++ b/hw/a15mpcore.c
> @@ -0,0 +1,118 @@
> +/*
> + * ARM A15MPCore internal peripheral emulation (common code).
> + *
> + * Based on mpcore.c written by Paul Brook
> + * This code is licensed under the GPL.

Missing copyright notice. Say which version(s) of the GPL.

> + */
> +
> +#include "sysbus.h"
> +#include "qemu-timer.h"
> +
> +/* 64 external IRQ lines */
> +#define GIC_NIRQ 96

64 or 96?

> +
> +#define NCPU 4
> +
> +static inline int
> +gic_get_current_cpu(void)
> +{
> +  return cpu_single_env->cpu_index;
> +}
> +
> +#include "arm_gic.c"
> +
> +/* MPCore private memory region.  */
> +typedef struct a15mpcore_priv_state {
> +    gic_state gic;
> +    uint32_t scu_control;
> +    int iomemtype;
> +    uint32_t num_cpu;
> +    MemoryRegion iomem;
> +    MemoryRegion container;
> +} a15mpcore_priv_state;

We don't seem to use scu_control.

> +/* TODO:Per-CPU Timers.  */

A15 generic timers aren't memory mapped so they won't be in this file.

> +
> +
> +/* Per-CPU private memory mapped IO.  */
> +static uint64_t a15mpcore_priv_read(void *opaque, target_phys_addr_t offset,
> +                                 unsigned size)
> +{
> +    a15mpcore_priv_state *s = (a15mpcore_priv_state *)opaque;
> +    int id;
> +
> +    offset &= 0xfff;
> +    /* Interrupt controller.  */
> +    if (offset < 0x200) {
> +       id = gic_get_current_cpu();
> +       } else {
> +        id = (offset - 0x200) >> 8;
> +        if (id >= s->num_cpu) {
> +                return 0;
> +        }
> +    }
> +       return gic_cpu_read(&s->gic, id, offset & 0xff);
> +}

This is not what the A15 GIC looks like. Please implement against
the TRM; don't just copy old code from the A9/11MPCore QEMU code.
The cores are not the same.

> +
> +static void a15mpcore_priv_map_setup(a15mpcore_priv_state *s)
> +{
> +    memory_region_init(&s->container, "mpcode-priv-container", 0x3000);
> +    memory_region_init_io(&s->iomem, &mpcore_priv_ops, s, "mpcode-priv",
> +                          0x1000);
> +    memory_region_add_subregion(&s->container, 0x2000, &s->iomem);
> +    memory_region_add_subregion(&s->container, 0x1000, &s->gic.iomem);

This is wrong too. The GIC CPU interface region is 0x2000..0x3fff,
not 0x2000..0x2fff.

Since the A15 doesn't squash multiple different things into one page
it might be simpler to have the GIC just expose more memory regions
to be mapped rather than having an indirect through the mpcore_priv_ops
here.

> +
> +static SysBusDeviceInfo a15mpcore_priv_info = {
> +       .init = a15mpcore_priv_init,
> +       .qdev.name  = "a15mpcore_priv",
> +       .qdev.size  = sizeof(a15mpcore_priv_state),
> +       .qdev.props = (Property[]) {
> +               DEFINE_PROP_UINT32("num-cpu", a15mpcore_priv_state, num_cpu, 1),
> +               DEFINE_PROP_END_OF_LIST(),
> +       }
> +};
> +
> +static void a15mpcore_register_devices(void)
> +{
> +    sysbus_register_withprop(&a15mpcore_priv_info);
> +}
> +
> +device_init(a15mpcore_register_devices)
> +
> diff --git a/hw/vexpress.c b/hw/vexpress.c
> index 0940a26..46a3c76 100644
> --- a/hw/vexpress.c
> +++ b/hw/vexpress.c
> @@ -34,6 +34,10 @@ static struct arm_boot_info vexpress_binfo = {
>     .smp_loader_start = SMP_BOOT_ADDR,
>  };
>
> +static struct arm_boot_info vexpress_a15_binfo = {
> +    .smp_loader_start = SMP_BOOT_ADDR,
> +};
> +

This is identical to vexpress_binfo so we don't need it.

>  static void vexpress_a9_init(ram_addr_t ram_size,
>                      const char *boot_device,
>                      const char *kernel_filename, const char *kernel_cmdline,
> @@ -221,9 +225,197 @@ static QEMUMachine vexpress_a9_machine = {
>     .max_cpus = 4,
>  };
>
> +static void vexpress_a15_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)
> +{
> +    CPUState *env = NULL;
> +    ram_addr_t ram_offset, vram_offset, sram_offset;
> +    DeviceState *dev, *sysctl, *pl041;
> +    SysBusDevice *busdev;
> +    qemu_irq *irqp;
> +    qemu_irq pic[64];
> +    int n;
> +    qemu_irq cpu_irq[4];
> +    uint32_t proc_id;
> +    uint32_t sys_id;
> +    ram_addr_t low_ram_size, vram_size, sram_size;
> +
> +    if (!cpu_model) {
> +        cpu_model = "cortex-a15";
> +    }
> +
> +    for (n = 0; n < smp_cpus; n++) {
> +        env = cpu_init(cpu_model);
> +        if (!env) {
> +            fprintf(stderr, "Unable to find CPU definition\n");
> +            exit(1);
> +        }
> +        irqp = arm_pic_init_cpu(env);
> +        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
> +    }
> +
> +    if (ram_size > 0x40000000) {
> +        /* 1GB is the maximum the address space permits */
> +        fprintf(stderr, "vexpress: cannot model more than 1GB RAM\n");
> +        exit(1);
> +    }

This is wrong. The whole point of the A15 vexpress having a different
memory map is that it can support more RAM.

> +
> +    ram_offset = qemu_ram_alloc(NULL, "vexpress.highmem", ram_size);
> +    low_ram_size = ram_size;
> +    if (low_ram_size > 0x4000000) {
> +        low_ram_size = 0x4000000;
> +    }
> +    /* RAM is from 0x80000000 upwards. The bottom 64MB of the
> +     * address space should in theory be remappable to various
> +     * things including ROM or RAM; we always map the RAM there.
> +     */
> +    cpu_register_physical_memory(0x0, low_ram_size, ram_offset | IO_MEM_RAM);
> +    cpu_register_physical_memory(0x80000000, ram_size,
> +                                 ram_offset | IO_MEM_RAM);
> +
> +    /* 0x2c000000 A15MPCore (SCU) private memory region */
> +    dev = qdev_create(NULL, "a15mpcore_priv");
> +    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
> +    qdev_init_nofail(dev);
> +    busdev = sysbus_from_qdev(dev);
> +    vexpress_a15_binfo.smp_priv_base = 0x2c000000;
> +    sysbus_mmio_map(busdev, 0, vexpress_a15_binfo.smp_priv_base);
> +    for (n = 0; n < smp_cpus; n++) {
> +        sysbus_connect_irq(busdev, n, cpu_irq[n]);
> +    }
> +    /* Interrupts [42:0] are from the motherboard;
> +     * [47:43] are reserved; [63:48] are daughterboard
> +     * peripherals. Note that some documentation numbers
> +     * external interrupts starting from 32 (because the
> +     * A9MP has internal interrupts 0..31).
> +     */
> +    for (n = 0; n < 64; n++) {
> +        pic[n] = qdev_get_gpio_in(dev, n);
> +    }
> +
> +    /* Motherboard peripherals CS7 : 0x10000000 .. 0x10020000 */
> +    sys_id =  0x1190f500;
> +    proc_id = 0x0c000000; /*depend on Fast module*/

This is definitely wrong -- that 0x0c says "Cortex-A9".
Also the comment is excessively cryptic.

> +
> +    /* 0x10000000 System registers */
> +    sysctl = qdev_create(NULL, "realview_sysctl");
> +    qdev_prop_set_uint32(sysctl, "sys_id", sys_id);
> +    qdev_init_nofail(sysctl);
> +    qdev_prop_set_uint32(sysctl, "proc_id", proc_id);
> +
> +       sysbus_mmio_map(sysbus_from_qdev(sysctl), 0, 0x1c010000);
> +
> +    /* 0x10001000 SP810 system control */
> +    /* 0x10002000 serial bus PCI */
> +    /* 0x10004000 PL041 audio */

This is all wrong as well. These devices don't live in the same place
in the A15 board memory map. All the rest of this looks wrong too.

Consider whether you can share code between the vexpress-a9 and
vexpress-a15 board init functions -- the motherboard CSx regions
are shuffled around a bit but I think they are have the same contents.

> +    pl041 = qdev_create(NULL, "pl041");
> +    qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
> +    qdev_init_nofail(pl041);
> +    sysbus_mmio_map(sysbus_from_qdev(pl041), 0, 0x10004000);
> +    sysbus_connect_irq(sysbus_from_qdev(pl041), 0, pic[11]);
> +    dev = sysbus_create_varargs("pl181", 0x10005000, pic[9], pic[10], NULL);
> +    /* Wire up MMC card detect and read-only signals */
> +    qdev_connect_gpio_out(dev, 0,
> +                          qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT));
> +    qdev_connect_gpio_out(dev, 1,
> +                          qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN));
> +
> +    sysbus_create_simple("pl050_keyboard", 0x1c060000, pic[12]);
> +    sysbus_create_simple("pl050_mouse", 0x1c070000, pic[13]);
> +
> +       sysbus_create_simple("pl011", 0x1c090000, pic[5]);
> +       sysbus_create_simple("pl011", 0x1c0a0000, pic[6]);
> +    sysbus_create_simple("pl011", 0x1c0b0000, pic[7]);
> +    sysbus_create_simple("pl011", 0x1c0c0000, pic[8]);
> +
> +    /* 0x1000f000 SP805 WDT */
> +
> +    sysbus_create_simple("sp804", 0x1c110000, pic[2]);
> +    sysbus_create_simple("sp804", 0x1c120000, pic[3]);
> +
> +    /* 0x10016000 Serial Bus DVI */
> +
> +    sysbus_create_simple("pl031", 0x1c170000, pic[36]); /* RTC */
> +
> +    /* 0x1001a000 Compact Flash */
> +
> +    /* 0x1001f000 PL111 CLCD (motherboard) */
> +
> +    /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
> +
> +    /* 0x10020000 PL111 CLCD (daughterboard) */
> +    sysbus_create_simple("pl111", 0x10020000, pic[44]);
> +
> +    /* 0x10060000 AXI RAM */
> +    /* 0x100e0000 PL341 Dynamic Memory Controller */
> +    /* 0x100e1000 PL354 Static Memory Controller */
> +    /* 0x100e2000 System Configuration Controller */
> +
> +    sysbus_create_simple("sp804", 0x100e4000, pic[48]);
> +    /* 0x100e5000 SP805 Watchdog module */
> +    /* 0x100e6000 BP147 TrustZone Protection Controller */
> +    /* 0x100e9000 PL301 'Fast' AXI matrix */
> +    /* 0x100ea000 PL301 'Slow' AXI matrix */
> +    /* 0x100ec000 TrustZone Address Space Controller */
> +    /* 0x10200000 CoreSight debug APB */
> +    /* 0x1e00a000 PL310 L2 Cache Controller */
> +
> +    /* CS0: NOR0 flash          : 0x40000000 .. 0x44000000 */
> +    /* CS4: NOR1 flash          : 0x44000000 .. 0x48000000 */
> +    /* CS2: SRAM                : 0x48000000 .. 0x4a000000 */
> +    sram_size = 0x2000000;
> +    sram_offset = qemu_ram_alloc(NULL, "vexpress.sram", sram_size);
> +    cpu_register_physical_memory(0x48000000, sram_size,
> +                                 sram_offset | IO_MEM_RAM);
> +
> +    /* CS3: USB, ethernet, VRAM : 0x4c000000 .. 0x50000000 */
> +
> +    /* 0x4c000000 Video RAM */
> +    vram_size = 0x2000000; /* 32MB */
> +    vram_offset = qemu_ram_alloc(NULL, "vexpress.vram", vram_size);
> +    cpu_register_physical_memory(0x18000000, vram_size,
> +                                 vram_offset | IO_MEM_RAM);
> +
> +    /* 0x4e000000 LAN9118 Ethernet */
> +    if (nd_table[0].vlan) {
> +        lan9118_init(&nd_table[0], 0x4e000000, pic[15]);
> +    }
> +
> +    /* 0x4f000000 ISP1761 USB */
> +
> +    /* ??? Hack to map an additional page of ram for the secondary CPU
> +       startup code.  I guess this works on real hardware because the
> +       BootROM happens to be in ROM/flash or in memory that isn't clobbered
> +       until after Linux boots the secondary CPUs.  */
> +    ram_offset = qemu_ram_alloc(NULL, "vexpress.hack", 0x1000);
> +    cpu_register_physical_memory(SMP_BOOT_ADDR, 0x1000,
> +                                 ram_offset | IO_MEM_RAM);
> +
> +    vexpress_a15_binfo.ram_size = ram_size;
> +    vexpress_a15_binfo.kernel_filename = kernel_filename;
> +    vexpress_a15_binfo.kernel_cmdline = kernel_cmdline;
> +    vexpress_a15_binfo.initrd_filename = initrd_filename;
> +    vexpress_a15_binfo.nb_cpus = smp_cpus;
> +    vexpress_a15_binfo.board_id = VEXPRESS_BOARD_ID;

Are you sure the board ID is the same?

> +    vexpress_a15_binfo.loader_start = 0x80000000;
> +    arm_load_kernel(first_cpu, &vexpress_a15_binfo);
> +}
> +
> +
> +static QEMUMachine vexpress_a15_machine = {
> +    .name = "vexpress-a15",
> +    .desc = "ARM Versatile Express for Cortex-A15",
> +    .init = vexpress_a15_init,
> +    .use_scsi = 1,
> +    .max_cpus = 4,
> +};
> +
>  static void vexpress_machine_init(void)
>  {
>     qemu_register_machine(&vexpress_a9_machine);
> +    qemu_register_machine(&vexpress_a15_machine);
>  }
>
>  machine_init(vexpress_machine_init);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7e0a667..52334c5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -438,6 +438,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>  #define ARM_CPUID_CORTEXA8    0x410fc080
>  #define ARM_CPUID_CORTEXA8_R2 0x412fc083
>  #define ARM_CPUID_CORTEXA9    0x410fc090
> +#define ARM_CPUID_CORTEXA15   0x410fc0f0

This claims we're an r0p0. r2p0 is the variant for which the TRM
is available and which we should be implementing.

>  #define ARM_CPUID_CORTEXM3    0x410fc231
>  #define ARM_CPUID_ANY         0xffffffff
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e712554..6a2c89d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -217,6 +217,36 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
>         env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
>         env->cp15.c1_sys = 0x00c50078;
>         break;
> +    case ARM_CPUID_CORTEXA15: /* most same as A9 */

Don't say that, it's irrelevant.

> +        set_feature(env, ARM_FEATURE_V4T);
> +        set_feature(env, ARM_FEATURE_V5);
> +        set_feature(env, ARM_FEATURE_V6);
> +        set_feature(env, ARM_FEATURE_V6K);
> +        set_feature(env, ARM_FEATURE_V7);
> +        set_feature(env, ARM_FEATURE_AUXCR);
> +        set_feature(env, ARM_FEATURE_THUMB2);
> +        set_feature(env, ARM_FEATURE_VFP);
> +        set_feature(env, ARM_FEATURE_VFP3);
> +        set_feature(env, ARM_FEATURE_VFP_FP16);
> +        set_feature(env, ARM_FEATURE_NEON);
> +        set_feature(env, ARM_FEATURE_THUMB2EE);
> +        /* Note that A9 supports the MP extensions even for
> +         * A9UP and single-core A9MP (which are both different
> +         * and valid configurations; we don't model A9UP).
> +         */
> +        set_feature(env, ARM_FEATURE_V7MP);

Missing ARM_FEATURE_TRUSTZONE, ARM_FEATURE_ARM_DIV, ARM_FEATURE_VFP4.

> +        env->vfp.xregs[ARM_VFP_FPSID] = 0x41034000; /* Guess */

Don't guess! Check against the TRM. All these ID registers need
cross-checking against the TRM.

> +        env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
> +        env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
> +        memcpy(env->cp15.c0_c1, cortexa9_cp15_c0_c1, 8 * sizeof(uint32_t));
> +        memcpy(env->cp15.c0_c2, cortexa9_cp15_c0_c2, 8 * sizeof(uint32_t));
> +        env->cp15.c0_cachetype = 0x80038003;
> +        env->cp15.c0_clid = (1 << 27) | (1 << 24) | 3;
> +        env->cp15.c0_ccsid[0] = 0xe00fe015; /* 16k L1 dcache. */
> +        env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
> +        env->cp15.c1_sys = 0x00c50078;
> +        break;
> +
>     case ARM_CPUID_CORTEXM3:
>         set_feature(env, ARM_FEATURE_V4T);
>         set_feature(env, ARM_FEATURE_V5);
> @@ -460,6 +490,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
>     { ARM_CPUID_CORTEXA8, "cortex-a8"},
>     { ARM_CPUID_CORTEXA8_R2, "cortex-a8-r2"},
>     { ARM_CPUID_CORTEXA9, "cortex-a9"},
> +    { ARM_CPUID_CORTEXA15, "cortex-a15"},
>     { ARM_CPUID_TI925T, "ti925t" },
>     { ARM_CPUID_PXA250, "pxa250" },
>     { ARM_CPUID_SA1100,    "sa1100" },
> --
> 1.7.1

-- PMM

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 1 December 2011 11:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 1 December 2011 01:37,  <bill4carson@gmail.com> wrote:
>> Qemu based on:
>> git://git.linaro.org/qemu/qemu-linaro.git
>
> Please don't post patches to upstream which are based on qemu-linaro.
> This work should be entirely doable against upstream qemu.

To clarify that a little, having noticed the cc list here:
we can do kvm-specific work against qemu-linaro, but where possible
we should do the non-kvm-specific bits against qemu upstream and
bring them in that way. A15 board support is definitely not kvm
specific and we should be aiming to upstream it first.

I'm not sure what the best thing to do with work-in-progress or
intended-for-kvm-qemu-linaro patches is; I think I'd suggest that
those be posted to android-virt and linaro-toolchain but not
qemu-devel.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Andreas Färber]

Am 01.12.2011 02:37, schrieb bill4carson@gmail.com:
> From: Bill Carson <bill4carson@gmail.com>
> 
> This patch adds minimal codes to support A15  which enables ARM KVM could
> run Guest OS build with Versatile Express Cortex-A15x4 tile.
> 
> Signed-off-by: Bill Carson <bill4carson@gmail.com>
> ---

> diff --git a/hw/a15mpcore.c b/hw/a15mpcore.c
> new file mode 100644
> index 0000000..2518c17
> --- /dev/null
> +++ b/hw/a15mpcore.c
> @@ -0,0 +1,118 @@
> +/*
> + * ARM A15MPCore internal peripheral emulation (common code).

ARM Cortex-A15 MPCore, please. No need to repeat the short file name.

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e712554..6a2c89d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -217,6 +217,36 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
>          env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
>          env->cp15.c1_sys = 0x00c50078;
>          break;
> +    case ARM_CPUID_CORTEXA15: /* most same as A9 */
> +        set_feature(env, ARM_FEATURE_V4T);
> +        set_feature(env, ARM_FEATURE_V5);
> +        set_feature(env, ARM_FEATURE_V6);
> +        set_feature(env, ARM_FEATURE_V6K);
> +        set_feature(env, ARM_FEATURE_V7);
> +        set_feature(env, ARM_FEATURE_AUXCR);
> +        set_feature(env, ARM_FEATURE_THUMB2);
> +        set_feature(env, ARM_FEATURE_VFP);
> +        set_feature(env, ARM_FEATURE_VFP3);
> +        set_feature(env, ARM_FEATURE_VFP_FP16);
> +        set_feature(env, ARM_FEATURE_NEON);
> +        set_feature(env, ARM_FEATURE_THUMB2EE);
> +        /* Note that A9 supports the MP extensions even for
> +         * A9UP and single-core A9MP (which are both different
> +         * and valid configurations; we don't model A9UP).
> +         */
> +        set_feature(env, ARM_FEATURE_V7MP);

Peter, this calls for my feature inference series.

Now that 1.0 is out I'll have another go at it later tonight, adding
your suggested rules. Feel free to pick up the initial ones if you like.

Andreas

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 5 December 2011 17:59, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 01.12.2011 02:37, schrieb bill4carson@gmail.com:
>> + * ARM A15MPCore internal peripheral emulation (common code).
>
> ARM Cortex-A15 MPCore, please. No need to repeat the short file name.

(also it's not "common code" with anything.)

The private-peripheral parts of this patch will need to be tweaked
to follow the mpcore-cleanup series I just posted, I'm afraid.

> Peter, this calls for my feature inference series.
>
> Now that 1.0 is out I'll have another go at it later tonight, adding
> your suggested rules. Feel free to pick up the initial ones if you like.

Yes, we should get those feature inference patches in. I think it
will be easiest if you just do a v2 series with the initial patches
plus the new ones, and I'll take the whole lot at once.

-- PMM

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Avi Kivity]

On 12/01/2011 03:37 AM, bill4carson@gmail.com wrote:
> From: Bill Carson <bill4carson@gmail.com>
>
> This patch adds minimal codes to support A15  which enables ARM KVM could
> run Guest OS build with Versatile Express Cortex-A15x4 tile.
>
> +
> +static inline int
> +gic_get_current_cpu(void)
> +{
> +  return cpu_single_env->cpu_index;
> +}

Bad indents.

> +/* Per-CPU private memory mapped IO.  */
> +static uint64_t a15mpcore_priv_read(void *opaque, target_phys_addr_t offset,
> +                                 unsigned size)
> +{
> +    a15mpcore_priv_state *s = (a15mpcore_priv_state *)opaque;
> +    int id;
> +
> +    offset &= 0xfff;
> +    /* Interrupt controller.  */
> +    if (offset < 0x200) {
> +    	id = gic_get_current_cpu();
> +	} else {
> +        id = (offset - 0x200) >> 8;
> +        if (id >= s->num_cpu) {
> +                return 0;
> +        }
> +    }
> +	return gic_cpu_read(&s->gic, id, offset & 0xff);
> +}

Very bad indents.  Detab your files.

> +
> +static void a15mpcore_priv_write(void *opaque, target_phys_addr_t offset,
> +                              uint64_t value, unsigned size)
> +{
> +    a15mpcore_priv_state *s = (a15mpcore_priv_state *)opaque;
> +    int id;
> +	
> +    offset &= 0xfff;
> +    /* Interrupt controller.  */
> +    if (offset < 0x200) {
> +    	id = gic_get_current_cpu();
> +	} else {
> +        id = (offset - 0x200) >> 8;
> +        if (id >= s->num_cpu) {
> +                return 0;
> +        }
> +    }
> +	return gic_cpu_write(&s->gic, id, offset & 0xff, value);
> +}

Here, too.

> +static void a15mpcore_priv_map_setup(a15mpcore_priv_state *s)
> +{
> +    memory_region_init(&s->container, "mpcode-priv-container", 0x3000);
> +    memory_region_init_io(&s->iomem, &mpcore_priv_ops, s, "mpcode-priv",
> +                          0x1000);
> +    memory_region_add_subregion(&s->container, 0x2000, &s->iomem);
> +    memory_region_add_subregion(&s->container, 0x1000, &s->gic.iomem);
> +}

mpcode or mpcore?

> +    /* RAM is from 0x80000000 upwards. The bottom 64MB of the
> +     * address space should in theory be remappable to various
> +     * things including ROM or RAM; we always map the RAM there.
> +     */
> +    cpu_register_physical_memory(0x0, low_ram_size, ram_offset | IO_MEM_RAM);
> +    cpu_register_physical_memory(0x80000000, ram_size,
> +                                 ram_offset | IO_MEM_RAM);
> +

Use memory_region_init_ram()/memory_region_add_subregion() instead.

> +
> +    /* ??? Hack to map an additional page of ram for the secondary CPU
> +       startup code.  I guess this works on real hardware because the
> +       BootROM happens to be in ROM/flash or in memory that isn't clobbered
> +       until after Linux boots the secondary CPUs.  */
> +    ram_offset = qemu_ram_alloc(NULL, "vexpress.hack", 0x1000);
> +    cpu_register_physical_memory(SMP_BOOT_ADDR, 0x1000,
> +                                 ram_offset | IO_MEM_RAM);

Here, too.

It would be better to unhack this; short-term hacks tend to remain in
the long term, and even after they're fixed we keep them for backwards
compatibility.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 6 December 2011 12:28, Avi Kivity <avi@redhat.com> wrote:
> On 12/01/2011 03:37 AM, bill4carson@gmail.com wrote:
>> +    /* ??? Hack to map an additional page of ram for the secondary CPU
>> +       startup code.  I guess this works on real hardware because the
>> +       BootROM happens to be in ROM/flash or in memory that isn't clobbered
>> +       until after Linux boots the secondary CPUs.  */
>> +    ram_offset = qemu_ram_alloc(NULL, "vexpress.hack", 0x1000);
>> +    cpu_register_physical_memory(SMP_BOOT_ADDR, 0x1000,
>> +                                 ram_offset | IO_MEM_RAM);

> It would be better to unhack this; short-term hacks tend to remain in
> the long term, and even after they're fixed we keep them for backwards
> compatibility.

Do you have a better suggestion in this case? We've had the same
code in the realview board since 2007 when ARM SMP support was first
added...

There's no particular back-compat implication here as far as I know:
the location of the secondary CPU holding pen code is irrelevant to
the actual guest being run. (On a real system it will be somewhere
inside the boot ROM.)

-- PMM

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Avi Kivity]

On 12/06/2011 02:35 PM, Peter Maydell wrote:
> On 6 December 2011 12:28, Avi Kivity <avi@redhat.com> wrote:
> > On 12/01/2011 03:37 AM, bill4carson@gmail.com wrote:
> >> +    /* ??? Hack to map an additional page of ram for the secondary CPU
> >> +       startup code.  I guess this works on real hardware because the
> >> +       BootROM happens to be in ROM/flash or in memory that isn't clobbered
> >> +       until after Linux boots the secondary CPUs.  */
> >> +    ram_offset = qemu_ram_alloc(NULL, "vexpress.hack", 0x1000);
> >> +    cpu_register_physical_memory(SMP_BOOT_ADDR, 0x1000,
> >> +                                 ram_offset | IO_MEM_RAM);
>
> > It would be better to unhack this; short-term hacks tend to remain in
> > the long term, and even after they're fixed we keep them for backwards
> > compatibility.
>
> Do you have a better suggestion in this case? We've had the same
> code in the realview board since 2007 when ARM SMP support was first
> added...

No idea really since I don't fully understand what's going on.  It's
just a knee-jerk reaction to the word 'hack'.

Can't we just do what real hardware does?

> There's no particular back-compat implication here as far as I know:
> the location of the secondary CPU holding pen code is irrelevant to
> the actual guest being run. (On a real system it will be somewhere
> inside the boot ROM.)

Suppose you live migrate when the code is running there?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 6 December 2011 12:39, Avi Kivity <avi@redhat.com> wrote:
> On 12/06/2011 02:35 PM, Peter Maydell wrote:
>> On 6 December 2011 12:28, Avi Kivity <avi@redhat.com> wrote:
>> Do you have a better suggestion in this case? We've had the same
>> code in the realview board since 2007 when ARM SMP support was first
>> added...
>
> No idea really since I don't fully understand what's going on.  It's
> just a knee-jerk reaction to the word 'hack'.
>
> Can't we just do what real hardware does?

Real hardware runs the boot ROM. The boot ROM is an unredistributable
binary blob, and in any case we almost certainly don't implement
the hardware well enough to pass the boot ROM's self tests and init
code.

We could probably put the pen code in a QEMU-specific bit of ROM
code in the same place the hardware boot ROM actually lives.

Longer term, I'm toying with the idea of having QEMU run UEFI
(for vexpress UEFI can boot the platform from bare metal, and it's
open source so we can (a) redistribute it and (b) add in qemu-specific
code if we need to). That would look a bit more like x86 qemu.

>> There's no particular back-compat implication here as far as I know:
>> the location of the secondary CPU holding pen code is irrelevant to
>> the actual guest being run. (On a real system it will be somewhere
>> inside the boot ROM.)
>
> Suppose you live migrate when the code is running there?

Currently for ARM we permit changes which would break live migration,
because it's not supported to start with.

(Does migration copy across rom contents and sizes?)

-- PMM

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Avi Kivity]

On 12/06/2011 02:48 PM, Peter Maydell wrote:
> On 6 December 2011 12:39, Avi Kivity <avi@redhat.com> wrote:
> > On 12/06/2011 02:35 PM, Peter Maydell wrote:
> >> On 6 December 2011 12:28, Avi Kivity <avi@redhat.com> wrote:
> >> Do you have a better suggestion in this case? We've had the same
> >> code in the realview board since 2007 when ARM SMP support was first
> >> added...
> >
> > No idea really since I don't fully understand what's going on.  It's
> > just a knee-jerk reaction to the word 'hack'.
> >
> > Can't we just do what real hardware does?
>
> Real hardware runs the boot ROM. The boot ROM is an unredistributable
> binary blob, and in any case we almost certainly don't implement
> the hardware well enough to pass the boot ROM's self tests and init
> code.
>
> We could probably put the pen code in a QEMU-specific bit of ROM
> code in the same place the hardware boot ROM actually lives.

That would be an improvement.

> Longer term, I'm toying with the idea of having QEMU run UEFI
> (for vexpress UEFI can boot the platform from bare metal, and it's
> open source so we can (a) redistribute it and (b) add in qemu-specific
> code if we need to). That would look a bit more like x86 qemu.

Right.

> >> There's no particular back-compat implication here as far as I know:
> >> the location of the secondary CPU holding pen code is irrelevant to
> >> the actual guest being run. (On a real system it will be somewhere
> >> inside the boot ROM.)
> >
> > Suppose you live migrate when the code is running there?
>
> Currently for ARM we permit changes which would break live migration,
> because it's not supported to start with.

Okay, so we should remove the hack before enabling live migration.

In any case, I understand you currently have to boot with -kernel? 
That's even a stronger reason to fix this thing.

> (Does migration copy across rom contents and sizes?)

Contents, yes, sizes, no.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Add minimal Vexpress Cortex A15 support

[Peter Maydell]

On 6 December 2011 13:12, Avi Kivity <avi@redhat.com> wrote:
> On 12/06/2011 02:48 PM, Peter Maydell wrote:
>> Real hardware runs the boot ROM. The boot ROM is an unredistributable
>> binary blob, and in any case we almost certainly don't implement
>> the hardware well enough to pass the boot ROM's self tests and init
>> code.
>>
>> We could probably put the pen code in a QEMU-specific bit of ROM
>> code in the same place the hardware boot ROM actually lives.
>
> That would be an improvement.

Feel free to submit a patch :-)

Anyway, I don't intend to make cleaning this up be a blocker for
adding A15 system support.

-- PMM