[Qemu-devel] [PATCH 0/4] ARM: Set reset-cbar property for A9 and A15 boards

[Qemu-devel] [PATCH 0/4] ARM: Set reset-cbar property for A9 and A15 boards

[Peter Maydell]

These patches set the reset-cbar property for all the A9 and A15
board models which haven't already had that fix. This is necessary
for A9 so that Linux doesn't misdetect the CPU as a specific broken
A9 uniprocessor SoC; it's not strictly necessary for A15 but it is
a more accurate emulation.

The vexpress patch is a slightly rearranged version of the one
I sent out before; this one doesn't segfault for vexpress-a15.

NB: I don't have test images for realview or exynos so have only
tested those to the level of "doesn't crash on startup".

Andreas: is object_new(object_class_get_name(oc)) still the
recommended way to create an object if you have its ObjectClass * ?

Peter Maydell (4):
  vexpress: Set reset-cbar property for CPUs
  realview-pbx-a9: Set reset-cbar property for CPUs
  exynos4210: Set reset-cbar property of Cortex-A9 CPUs
  virt: Set reset-cbar on CPUs

 hw/arm/exynos4210.c |  23 ++++++++--
 hw/arm/realview.c   |  39 ++++++++++++-----
 hw/arm/vexpress.c   | 123 ++++++++++++++++++++++++++--------------------------
 hw/arm/virt.c       |  11 +++++
 4 files changed, 121 insertions(+), 75 deletions(-)

-- 
1.8.5

[Qemu-devel] [PATCH 1/4] vexpress: Set reset-cbar property for CPUs

[Peter Maydell]

Newer versions of the Linux kernel (as of commit bc41b8724 in 3.12)
now assume that if the CPU is a Cortex-A9 and the reset value of the
PERIPHBASE/CBAR register is zero then the CPU is a specific buggy
single core A9 SoC, and will not try to start other cores. Since we
now have a CPU property for the reset value of the CBAR, we can
just fix the vexpress board model to correctly set CBAR so SMP
works again. To avoid duplicate boilerplate code in both the A9
and A15 daughterboard init functions, we split out the CPU and
private memory region init to its own function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reported-by: Rob Herring <rob.herring@linaro.org>
---
 hw/arm/vexpress.c | 123 +++++++++++++++++++++++++++---------------------------
 1 file changed, 61 insertions(+), 62 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index ef1707a..67628af 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -32,6 +32,7 @@
 #include "sysemu/blockdev.h"
 #include "hw/block/flash.h"
 #include "sysemu/device_tree.h"
+#include "qemu/error-report.h"
 #include <libfdt.h>
 
 #define VEXPRESS_BOARD_ID 0x8e0
@@ -173,6 +174,64 @@ struct VEDBoardInfo {
     DBoardInitFn *init;
 };
 
+static void init_cpus(const char *cpu_model, const char *privdev,
+                      hwaddr periphbase, qemu_irq *pic)
+{
+    ObjectClass *cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
+    DeviceState *dev;
+    SysBusDevice *busdev;
+    int n;
+
+    if (!cpu_oc) {
+        fprintf(stderr, "Unable to find CPU definition\n");
+        exit(1);
+    }
+
+    /* Create the actual CPUs */
+    for (n = 0; n < smp_cpus; n++) {
+        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
+        Error *err = NULL;
+
+        object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+        object_property_set_bool(cpuobj, true, "realized", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+    }
+
+    /* Create the private peripheral devices (including the GIC);
+     * this must happen after the CPUs are created because a15mpcore_priv
+     * wires itself up to the CPU's generic_timer gpio out lines.
+     */
+    dev = qdev_create(NULL, privdev);
+    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
+    qdev_init_nofail(dev);
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, periphbase);
+
+    /* 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 there
+     * are internal interrupts 0..31).
+     */
+    for (n = 0; n < 64; n++) {
+        pic[n] = qdev_get_gpio_in(dev, n);
+    }
+
+    /* Connect the CPUs to the GIC */
+    for (n = 0; n < smp_cpus; n++) {
+        DeviceState *cpudev = DEVICE(qemu_get_cpu(n));
+
+        sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+    }
+}
+
 static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
                                   ram_addr_t ram_size,
                                   const char *cpu_model,
@@ -181,25 +240,12 @@ static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *lowram = g_new(MemoryRegion, 1);
-    DeviceState *dev;
-    SysBusDevice *busdev;
-    int n;
-    qemu_irq cpu_irq[4];
     ram_addr_t low_ram_size;
 
     if (!cpu_model) {
         cpu_model = "cortex-a9";
     }
 
-    for (n = 0; n < smp_cpus; n++) {
-        ARMCPU *cpu = cpu_arm_init(cpu_model);
-        if (!cpu) {
-            fprintf(stderr, "Unable to find CPU definition\n");
-            exit(1);
-        }
-        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
-    }
-
     if (ram_size > 0x40000000) {
         /* 1GB is the maximum the address space permits */
         fprintf(stderr, "vexpress-a9: cannot model more than 1GB RAM\n");
@@ -221,23 +267,7 @@ static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
     memory_region_add_subregion(sysmem, 0x60000000, ram);
 
     /* 0x1e000000 A9MPCore (SCU) private memory region */
-    dev = qdev_create(NULL, "a9mpcore_priv");
-    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
-    qdev_init_nofail(dev);
-    busdev = SYS_BUS_DEVICE(dev);
-    sysbus_mmio_map(busdev, 0, 0x1e000000);
-    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);
-    }
+    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic);
 
     /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
 
@@ -296,29 +326,14 @@ static void a15_daughterboard_init(const VEDBoardInfo *daughterboard,
                                    const char *cpu_model,
                                    qemu_irq *pic)
 {
-    int n;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *sram = g_new(MemoryRegion, 1);
-    qemu_irq cpu_irq[4];
-    DeviceState *dev;
-    SysBusDevice *busdev;
 
     if (!cpu_model) {
         cpu_model = "cortex-a15";
     }
 
-    for (n = 0; n < smp_cpus; n++) {
-        ARMCPU *cpu;
-
-        cpu = cpu_arm_init(cpu_model);
-        if (!cpu) {
-            fprintf(stderr, "Unable to find CPU definition\n");
-            exit(1);
-        }
-        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_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"
@@ -337,23 +352,7 @@ static void a15_daughterboard_init(const VEDBoardInfo *daughterboard,
     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]);
-    }
-    /* 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 there
-     * are internal interrupts 0..31).
-     */
-    for (n = 0; n < 64; n++) {
-        pic[n] = qdev_get_gpio_in(dev, n);
-    }
+    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic);
 
     /* A15 daughterboard peripherals: */
 
-- 
1.8.5

Re: [Qemu-devel] [PATCH 1/4] vexpress: Set reset-cbar property for CPUs

[Peter Crosthwaite]

On Tue, Feb 25, 2014 at 6:23 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Newer versions of the Linux kernel (as of commit bc41b8724 in 3.12)
> now assume that if the CPU is a Cortex-A9 and the reset value of the
> PERIPHBASE/CBAR register is zero then the CPU is a specific buggy
> single core A9 SoC, and will not try to start other cores. Since we
> now have a CPU property for the reset value of the CBAR, we can
> just fix the vexpress board model to correctly set CBAR so SMP
> works again. To avoid duplicate boilerplate code in both the A9
> and A15 daughterboard init functions, we split out the CPU and
> private memory region init to its own function.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reported-by: Rob Herring <rob.herring@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  hw/arm/vexpress.c | 123 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 61 insertions(+), 62 deletions(-)
>
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index ef1707a..67628af 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -32,6 +32,7 @@
>  #include "sysemu/blockdev.h"
>  #include "hw/block/flash.h"
>  #include "sysemu/device_tree.h"
> +#include "qemu/error-report.h"
>  #include <libfdt.h>
>
>  #define VEXPRESS_BOARD_ID 0x8e0
> @@ -173,6 +174,64 @@ struct VEDBoardInfo {
>      DBoardInitFn *init;
>  };
>
> +static void init_cpus(const char *cpu_model, const char *privdev,
> +                      hwaddr periphbase, qemu_irq *pic)
> +{
> +    ObjectClass *cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
> +    DeviceState *dev;
> +    SysBusDevice *busdev;
> +    int n;
> +
> +    if (!cpu_oc) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
> +    /* Create the actual CPUs */
> +    for (n = 0; n < smp_cpus; n++) {
> +        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> +        Error *err = NULL;
> +
> +        object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
> +            exit(1);
> +        }
> +        object_property_set_bool(cpuobj, true, "realized", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
> +            exit(1);
> +        }
> +    }
> +
> +    /* Create the private peripheral devices (including the GIC);
> +     * this must happen after the CPUs are created because a15mpcore_priv
> +     * wires itself up to the CPU's generic_timer gpio out lines.
> +     */
> +    dev = qdev_create(NULL, privdev);
> +    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
> +    qdev_init_nofail(dev);
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, periphbase);
> +
> +    /* 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 there
> +     * are internal interrupts 0..31).
> +     */
> +    for (n = 0; n < 64; n++) {
> +        pic[n] = qdev_get_gpio_in(dev, n);
> +    }
> +
> +    /* Connect the CPUs to the GIC */
> +    for (n = 0; n < smp_cpus; n++) {
> +        DeviceState *cpudev = DEVICE(qemu_get_cpu(n));
> +
> +        sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> +    }
> +}
> +
>  static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
>                                    ram_addr_t ram_size,
>                                    const char *cpu_model,
> @@ -181,25 +240,12 @@ static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *lowram = g_new(MemoryRegion, 1);
> -    DeviceState *dev;
> -    SysBusDevice *busdev;
> -    int n;
> -    qemu_irq cpu_irq[4];
>      ram_addr_t low_ram_size;
>
>      if (!cpu_model) {
>          cpu_model = "cortex-a9";
>      }
>
> -    for (n = 0; n < smp_cpus; n++) {
> -        ARMCPU *cpu = cpu_arm_init(cpu_model);
> -        if (!cpu) {
> -            fprintf(stderr, "Unable to find CPU definition\n");
> -            exit(1);
> -        }
> -        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
> -    }
> -
>      if (ram_size > 0x40000000) {
>          /* 1GB is the maximum the address space permits */
>          fprintf(stderr, "vexpress-a9: cannot model more than 1GB RAM\n");
> @@ -221,23 +267,7 @@ static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
>      memory_region_add_subregion(sysmem, 0x60000000, ram);
>
>      /* 0x1e000000 A9MPCore (SCU) private memory region */
> -    dev = qdev_create(NULL, "a9mpcore_priv");
> -    qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
> -    qdev_init_nofail(dev);
> -    busdev = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(busdev, 0, 0x1e000000);
> -    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);
> -    }
> +    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic);
>
>      /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
>
> @@ -296,29 +326,14 @@ static void a15_daughterboard_init(const VEDBoardInfo *daughterboard,
>                                     const char *cpu_model,
>                                     qemu_irq *pic)
>  {
> -    int n;
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *sram = g_new(MemoryRegion, 1);
> -    qemu_irq cpu_irq[4];
> -    DeviceState *dev;
> -    SysBusDevice *busdev;
>
>      if (!cpu_model) {
>          cpu_model = "cortex-a15";
>      }
>
> -    for (n = 0; n < smp_cpus; n++) {
> -        ARMCPU *cpu;
> -
> -        cpu = cpu_arm_init(cpu_model);
> -        if (!cpu) {
> -            fprintf(stderr, "Unable to find CPU definition\n");
> -            exit(1);
> -        }
> -        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_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"
> @@ -337,23 +352,7 @@ static void a15_daughterboard_init(const VEDBoardInfo *daughterboard,
>      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]);
> -    }
> -    /* 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 there
> -     * are internal interrupts 0..31).
> -     */
> -    for (n = 0; n < 64; n++) {
> -        pic[n] = qdev_get_gpio_in(dev, n);
> -    }
> +    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic);
>
>      /* A15 daughterboard peripherals: */
>
> --
> 1.8.5
>
>

[Qemu-devel] [PATCH 2/4] realview-pbx-a9: Set reset-cbar property for CPUs

[Peter Maydell]

If the CPU is a Cortex-A9 then we should set its reset-cbar property
so that the guest can read the correct PERIPHBASE/CBAR register value;
newer versions of the Linux kernel (as of commit bc41b8724 in 3.12)
will otherwise assume the CPU is a buggy single core A9 SoC. The
realview-pbx-a9 is the only one of the cluster of boards in realview.c
which works with the Cortex-A9 (ie which gets an a9mpcore_priv device);
make sure it also has reset-cbar set correctly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/realview.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 6ef7646..7e04e50 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -18,6 +18,7 @@
 #include "hw/i2c/i2c.h"
 #include "sysemu/blockdev.h"
 #include "exec/address-spaces.h"
+#include "qemu/error-report.h"
 
 #define SMP_BOOT_ADDR 0xe0000000
 #define SMP_BOOTREG_ADDR 0x10000030
@@ -49,6 +50,7 @@ static void realview_init(QEMUMachineInitArgs *args,
 {
     ARMCPU *cpu = NULL;
     CPUARMState *env;
+    ObjectClass *cpu_oc;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram_lo = g_new(MemoryRegion, 1);
     MemoryRegion *ram_hi = g_new(MemoryRegion, 1);
@@ -70,12 +72,14 @@ static void realview_init(QEMUMachineInitArgs *args,
     uint32_t sys_id;
     ram_addr_t low_ram_size;
     ram_addr_t ram_size = args->ram_size;
+    hwaddr periphbase = 0;
 
     switch (board_type) {
     case BOARD_EB:
         break;
     case BOARD_EB_MPCORE:
         is_mpcore = 1;
+        periphbase = 0x10100000;
         break;
     case BOARD_PB_A8:
         is_pb = 1;
@@ -83,16 +87,37 @@ static void realview_init(QEMUMachineInitArgs *args,
     case BOARD_PBX_A9:
         is_mpcore = 1;
         is_pb = 1;
+        periphbase = 0x1f000000;
         break;
     }
+
+    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, args->cpu_model);
+    if (!cpu_oc) {
+        fprintf(stderr, "Unable to find CPU definition\n");
+        exit(1);
+    }
+
     for (n = 0; n < smp_cpus; n++) {
-        cpu = cpu_arm_init(args->cpu_model);
-        if (!cpu) {
-            fprintf(stderr, "Unable to find CPU definition\n");
+        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
+        Error *err = NULL;
+
+        if (is_pb && is_mpcore) {
+            object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
+            if (err) {
+                error_report("%s", error_get_pretty(err));
+                exit(1);
+            }
+        }
+
+        object_property_set_bool(cpuobj, true, "realized", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
             exit(1);
         }
-        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
+
+        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpuobj), ARM_CPU_IRQ);
     }
+    cpu = ARM_CPU(first_cpu);
     env = &cpu->env;
     if (arm_feature(env, ARM_FEATURE_V7)) {
         if (is_mpcore) {
@@ -141,16 +166,10 @@ static void realview_init(QEMUMachineInitArgs *args,
     sysbus_mmio_map(SYS_BUS_DEVICE(sysctl), 0, 0x10000000);
 
     if (is_mpcore) {
-        hwaddr periphbase;
         dev = qdev_create(NULL, is_pb ? "a9mpcore_priv": "realview_mpcore");
         qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
         qdev_init_nofail(dev);
         busdev = SYS_BUS_DEVICE(dev);
-        if (is_pb) {
-            periphbase = 0x1f000000;
-        } else {
-            periphbase = 0x10100000;
-        }
         sysbus_mmio_map(busdev, 0, periphbase);
         for (n = 0; n < smp_cpus; n++) {
             sysbus_connect_irq(busdev, n, cpu_irq[n]);
-- 
1.8.5

Re: [Qemu-devel] [PATCH 2/4] realview-pbx-a9: Set reset-cbar property for CPUs

[Peter Crosthwaite]

On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> If the CPU is a Cortex-A9 then we should set its reset-cbar property
> so that the guest can read the correct PERIPHBASE/CBAR register value;
> newer versions of the Linux kernel (as of commit bc41b8724 in 3.12)
> will otherwise assume the CPU is a buggy single core A9 SoC. The
> realview-pbx-a9 is the only one of the cluster of boards in realview.c
> which works with the Cortex-A9 (ie which gets an a9mpcore_priv device);
> make sure it also has reset-cbar set correctly.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  hw/arm/realview.c | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 6ef7646..7e04e50 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -18,6 +18,7 @@
>  #include "hw/i2c/i2c.h"
>  #include "sysemu/blockdev.h"
>  #include "exec/address-spaces.h"
> +#include "qemu/error-report.h"
>
>  #define SMP_BOOT_ADDR 0xe0000000
>  #define SMP_BOOTREG_ADDR 0x10000030
> @@ -49,6 +50,7 @@ static void realview_init(QEMUMachineInitArgs *args,
>  {
>      ARMCPU *cpu = NULL;
>      CPUARMState *env;
> +    ObjectClass *cpu_oc;
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *ram_lo = g_new(MemoryRegion, 1);
>      MemoryRegion *ram_hi = g_new(MemoryRegion, 1);
> @@ -70,12 +72,14 @@ static void realview_init(QEMUMachineInitArgs *args,
>      uint32_t sys_id;
>      ram_addr_t low_ram_size;
>      ram_addr_t ram_size = args->ram_size;
> +    hwaddr periphbase = 0;
>
>      switch (board_type) {
>      case BOARD_EB:
>          break;
>      case BOARD_EB_MPCORE:
>          is_mpcore = 1;
> +        periphbase = 0x10100000;
>          break;
>      case BOARD_PB_A8:
>          is_pb = 1;
> @@ -83,16 +87,37 @@ static void realview_init(QEMUMachineInitArgs *args,
>      case BOARD_PBX_A9:
>          is_mpcore = 1;
>          is_pb = 1;
> +        periphbase = 0x1f000000;
>          break;
>      }
> +
> +    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, args->cpu_model);
> +    if (!cpu_oc) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
>      for (n = 0; n < smp_cpus; n++) {
> -        cpu = cpu_arm_init(args->cpu_model);
> -        if (!cpu) {
> -            fprintf(stderr, "Unable to find CPU definition\n");
> +        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> +        Error *err = NULL;
> +
> +        if (is_pb && is_mpcore) {
> +            object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
> +            if (err) {
> +                error_report("%s", error_get_pretty(err));
> +                exit(1);
> +            }
> +        }
> +
> +        object_property_set_bool(cpuobj, true, "realized", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
>              exit(1);
>          }
> -        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
> +
> +        cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpuobj), ARM_CPU_IRQ);
>      }
> +    cpu = ARM_CPU(first_cpu);
>      env = &cpu->env;
>      if (arm_feature(env, ARM_FEATURE_V7)) {
>          if (is_mpcore) {
> @@ -141,16 +166,10 @@ static void realview_init(QEMUMachineInitArgs *args,
>      sysbus_mmio_map(SYS_BUS_DEVICE(sysctl), 0, 0x10000000);
>
>      if (is_mpcore) {
> -        hwaddr periphbase;
>          dev = qdev_create(NULL, is_pb ? "a9mpcore_priv": "realview_mpcore");
>          qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
>          qdev_init_nofail(dev);
>          busdev = SYS_BUS_DEVICE(dev);
> -        if (is_pb) {
> -            periphbase = 0x1f000000;
> -        } else {
> -            periphbase = 0x10100000;
> -        }
>          sysbus_mmio_map(busdev, 0, periphbase);
>          for (n = 0; n < smp_cpus; n++) {
>              sysbus_connect_irq(busdev, n, cpu_irq[n]);
> --
> 1.8.5
>
>

[Qemu-devel] [PATCH 3/4] exynos4210: Set reset-cbar property of Cortex-A9 CPUs

[Peter Maydell]

Set the reset-cbar property of the Exynos4210 SoC's Cortex-A9
CPUs, so that Linux doesn't misrecognize them as a broken
uniprocessor SoC.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/exynos4210.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 9f137e9..a6f481a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -143,11 +143,28 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
     unsigned long mem_size;
     DeviceState *dev;
     SysBusDevice *busdev;
+    ObjectClass *cpu_oc;
+
+    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
+    if (!cpu_oc) {
+        fprintf(stderr, "Unable to find CPU definition\n");
+        exit(1);
+    }
 
     for (n = 0; n < EXYNOS4210_NCPUS; n++) {
-        s->cpu[n] = cpu_arm_init("cortex-a9");
-        if (!s->cpu[n]) {
-            fprintf(stderr, "Unable to find CPU %d definition\n", n);
+        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
+        Error *err = NULL;
+
+        s->cpu[n] = ARM_CPU(cpuobj);
+        object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
+                                "reset-cbar", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+        object_property_set_bool(cpuobj, true, "realized", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
             exit(1);
         }
     }
-- 
1.8.5

Re: [Qemu-devel] [PATCH 3/4] exynos4210: Set reset-cbar property of Cortex-A9 CPUs

[Peter Crosthwaite]

On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Set the reset-cbar property of the Exynos4210 SoC's Cortex-A9
> CPUs, so that Linux doesn't misrecognize them as a broken
> uniprocessor SoC.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

But a couple of random notes below ...

> ---
>  hw/arm/exynos4210.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 9f137e9..a6f481a 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -143,11 +143,28 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
>      unsigned long mem_size;
>      DeviceState *dev;
>      SysBusDevice *busdev;
> +    ObjectClass *cpu_oc;
> +
> +    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
> +    if (!cpu_oc) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }

Now that this code is ignoring use -cpu I wonder if this should be
promoted to an assertion. The only way this can fail is if someone
rips A9 out of QEMU (or some other nasty fatal condition).

>
>      for (n = 0; n < EXYNOS4210_NCPUS; n++) {
> -        s->cpu[n] = cpu_arm_init("cortex-a9");
> -        if (!s->cpu[n]) {
> -            fprintf(stderr, "Unable to find CPU %d definition\n", n);
> +        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> +        Error *err = NULL;
> +
> +        s->cpu[n] = ARM_CPU(cpuobj);
> +        object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
> +                                "reset-cbar", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
> +            exit(1);
> +        }

And this may even be convertible to error_abort as theres no user
accessible way to make this fail.

Regards,
Peter

> +        object_property_set_bool(cpuobj, true, "realized", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
>              exit(1);
>          }
>      }
> --
> 1.8.5
>
>

[Qemu-devel] [PATCH 4/4] virt: Set reset-cbar on CPUs

[Peter Maydell]

Set the reset-cbar property on CPUs used by the virt board,
if they have it. This isn't necessary for correct functioning
under Linux (since the A9 isn't a valid CPU for the virt board),
but it is the correct behaviour.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 517f2fe..b418b77 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -379,6 +379,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
     for (n = 0; n < smp_cpus; n++) {
         ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
         Object *cpuobj;
+        Error *err = NULL;
 
         if (!oc) {
             fprintf(stderr, "Unable to find CPU definition\n");
@@ -390,6 +391,16 @@ static void machvirt_init(QEMUMachineInitArgs *args)
         if (n > 0) {
             object_property_set_bool(cpuobj, true, "start-powered-off", NULL);
         }
+
+        if (object_property_find(cpuobj, "reset-cbar", NULL)) {
+            object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
+                                    "reset-cbar", &err);
+            if (err) {
+                error_report("%s", error_get_pretty(err));
+                exit(1);
+            }
+        }
+
         object_property_set_bool(cpuobj, true, "realized", NULL);
     }
     fdt_add_cpu_nodes(vbi);
-- 
1.8.5

Re: [Qemu-devel] [PATCH 4/4] virt: Set reset-cbar on CPUs

[Peter Crosthwaite]

On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Set the reset-cbar property on CPUs used by the virt board,
> if they have it. This isn't necessary for correct functioning
> under Linux (since the A9 isn't a valid CPU for the virt board),
> but it is the correct behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 517f2fe..b418b77 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -379,6 +379,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>      for (n = 0; n < smp_cpus; n++) {
>          ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>          Object *cpuobj;
> +        Error *err = NULL;
>
>          if (!oc) {
>              fprintf(stderr, "Unable to find CPU definition\n");
> @@ -390,6 +391,16 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>          if (n > 0) {
>              object_property_set_bool(cpuobj, true, "start-powered-off", NULL);
>          }
> +
> +        if (object_property_find(cpuobj, "reset-cbar", NULL)) {
> +            object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
> +                                    "reset-cbar", &err);
> +            if (err) {
> +                error_report("%s", error_get_pretty(err));
> +                exit(1);
> +            }

If the property exists (as enforced by the if), and you cant set it,
its probably a fatal and candidate for error_abort.

Regards,
Peter

> +        }
> +
>          object_property_set_bool(cpuobj, true, "realized", NULL);
>      }
>      fdt_add_cpu_nodes(vbi);
> --
> 1.8.5
>
>

Re: [Qemu-devel] [PATCH 4/4] virt: Set reset-cbar on CPUs

[Peter Maydell]

On 25 February 2014 08:23, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Set the reset-cbar property on CPUs used by the virt board,
>> if they have it. This isn't necessary for correct functioning
>> under Linux (since the A9 isn't a valid CPU for the virt board),
>> but it is the correct behaviour.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm/virt.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 517f2fe..b418b77 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -379,6 +379,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>>      for (n = 0; n < smp_cpus; n++) {
>>          ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>>          Object *cpuobj;
>> +        Error *err = NULL;
>>
>>          if (!oc) {
>>              fprintf(stderr, "Unable to find CPU definition\n");
>> @@ -390,6 +391,16 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>>          if (n > 0) {
>>              object_property_set_bool(cpuobj, true, "start-powered-off", NULL);
>>          }
>> +
>> +        if (object_property_find(cpuobj, "reset-cbar", NULL)) {
>> +            object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
>> +                                    "reset-cbar", &err);
>> +            if (err) {
>> +                error_report("%s", error_get_pretty(err));
>> +                exit(1);
>> +            }
>
> If the property exists (as enforced by the if), and you cant set it,
> its probably a fatal and candidate for error_abort.

Yeah. It was rather unclear to me what the right error handling
behaviour for these was.

thanks
-- PMM