[Qemu-devel] [PATCH 0/9] raspi3: various fixes for Linux booting

[Qemu-devel] [PATCH 0/9] raspi3: various fixes for Linux booting

[Peter Maydell]

I had a go at booting Debian's Linux kernel on our raspi3 model.
These patches fix a variety of bugs that were stopping that working:
 * the board was trying to boot in secure mode, which isn't
   right for AArch64 Linux kernels
 * the hw/arm/boot.c code didn't set SCR_EL3.HCE, which meant that
   the HVC instruction would UNDEF and the guest would panic trying
   to initialize KVM
 * BCM2837 uses different affinity (cluster ID) values than BCM2836;
   this really confused Linux since it didn't think the primary CPU
   existed and it ended up panicing in an obscure way.
   I opted to fix this one with a modest refactor of the bcm2836
   code into one device for each SoC, similar to how we handle the
   Aspeed SoCs.
 * the board was trying to use AArch32 code for the secondary
   CPU bootup, so secondaries just went into an infinite UNDEF
   exception loop. Using the correct instruction set and the right
   spintable protocol allows the kernel to detect all the secondaries.

Unfortunately this doesn't entirely suffice to boot, because there's
a problem with our bcm2835_sdhost device model. I've analysed the
problem, but am not sure how to fix it because as far as I can tell
there is no public documentation for this bit of the hardware :-(
Does anybody have a datasheet for it?

[Summary of the sdhost issue:

    The Linux bcm2835_sdhost driver doesn't work on QEMU, because
    our model raises spurious data interrupts. Our function
    bcm2835_sdhost_fifo_run() will flag an interrupt any time
    it is called with s->datacnt == 0, even if the host hasn't
    actually issued a data read or write command yet. This means
    that the driver gets a spurious data interrupt as soon as
    it enables IRQs and then does something else that causes
    us to call the fifo_run routine, like writing to SDHCFG.
    The driver's IRQ handler then spins forever complaining that
    there's no data and the SD controller isn't in a state where
    there's going to be any data:
    [   41.040738] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts 00000000
    [   41.042059] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts 00000000
    (continues forever).

    Without the h/w docs it's hard to know exactly when we are
    supposed to signal interrupts, though obviously what we do now
    is definitely wrong...
]

thanks
-- PMM


Peter Maydell (9):
  hw/arm/raspi: Don't do board-setup or secure-boot for raspi3
  hw/arm/boot: assert that secure_boot and secure_board_setup are false
    for AArch64
  hw/arm/boot: If booting a kernel in EL2, set SCR_EL3.HCE
  hw/arm/bcm2386: Fix parent type of bcm2386
  hw/arm/bcm2836: Rename bcm2836 type/struct to bcm283x
  hw/arm/bcm2836: Create proper bcm2837 device
  hw/arm/bcm2836: Use correct affinity values for BCM2837
  hw/arm/bcm2836: Hardcode correct CPU type
  hw/arm/raspi: Provide spin-loop code for AArch64 CPUs

 include/hw/arm/bcm2836.h | 31 ++++++++++++++----
 hw/arm/bcm2836.c         | 81 +++++++++++++++++++++++++++++++++++-------------
 hw/arm/boot.c            | 12 +++++++
 hw/arm/raspi.c           | 77 ++++++++++++++++++++++++++++++++++++---------
 4 files changed, 158 insertions(+), 43 deletions(-)

-- 
2.16.2

[Qemu-devel] [PATCH 1/9] hw/arm/raspi: Don't do board-setup or secure-boot for raspi3

[Peter Maydell]

For the rpi1 and 2 we want to boot the Linux kernel via some
custom setup code that makes sure that the SMC instruction
acts as a no-op, because it's used for cache maintenance.
The rpi3 boots AArch64 kernels, which don't need SMC for
cache maintenance and always expect to be booted non-secure.
Don't fill in the aarch32-specific parts of the binfo struct.

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

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index a37881433c..1ac0737149 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -82,10 +82,19 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     binfo.board_id = raspi_boardid[version];
     binfo.ram_size = ram_size;
     binfo.nb_cpus = smp_cpus;
-    binfo.board_setup_addr = BOARDSETUP_ADDR;
-    binfo.write_board_setup = write_board_setup;
-    binfo.secure_board_setup = true;
-    binfo.secure_boot = true;
+
+    if (version <= 2) {
+        /* The rpi1 and 2 require some custom setup code to run in Secure
+         * mode before booting a kernel (to set up the SMC vectors so
+         * that we get a no-op SMC; this is used by Linux to call the
+         * firmware for some cache maintenance operations.
+         * The rpi3 doesn't need this.
+         */
+        binfo.board_setup_addr = BOARDSETUP_ADDR;
+        binfo.write_board_setup = write_board_setup;
+        binfo.secure_board_setup = true;
+        binfo.secure_boot = true;
+    }
 
     /* Pi2 and Pi3 requires SMP setup */
     if (version >= 2) {
-- 
2.16.2

[Qemu-devel] [PATCH 2/9] hw/arm/boot: assert that secure_boot and secure_board_setup are false for AArch64

[Peter Maydell]

Add some assertions that if we're about to boot an AArch64 kernel,
the board code has not mistakenly set either secure_boot or
secure_board_setup. It doesn't make sense to set secure_boot,
because all AArch64 kernels must be booted in non-secure mode.

It might in theory make sense to set secure_board_setup, but
we don't currently support that, because only the AArch32
bootloader[] code calls this hook; bootloader_aarch64[] does not.
Since we don't have a current need for this functionality, just
assert that we don't try to use it. If it's needed we'll add
it later.

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

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 196c7fb242..e21a92f972 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -720,6 +720,13 @@ static void do_cpu_reset(void *opaque)
                     } else {
                         env->pstate = PSTATE_MODE_EL1h;
                     }
+                    /* AArch64 kernels never boot in secure mode */
+                    assert(!info->secure_boot);
+                    /* This hook is only supported for AArch32 currently:
+                     * bootloader_aarch64[] will not call the hook, and
+                     * the code above has already dropped us into EL2 or EL1.
+                     */
+                    assert(!info->secure_board_setup);
                 }
 
                 /* Set to non-secure if not a secure boot */
-- 
2.16.2

[Qemu-devel] [PATCH 3/9] hw/arm/boot: If booting a kernel in EL2, set SCR_EL3.HCE

[Peter Maydell]

If we're directly booting a Linux kernel and the CPU supports both
EL3 and EL2, we start the kernel in EL2, as it expects. We must also
set the SCR_EL3.HCE bit in this situation, so that the HVC
instruction is enabled rather than UNDEFing. Otherwise at least some
kernels will panic when trying to initialize KVM in the guest.

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

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index e21a92f972..9319b12fcd 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -729,6 +729,11 @@ static void do_cpu_reset(void *opaque)
                     assert(!info->secure_board_setup);
                 }
 
+                if (arm_feature(env, ARM_FEATURE_EL2)) {
+                    /* If we have EL2 then Linux expects the HVC insn to work */
+                    env->cp15.scr_el3 |= SCR_HCE;
+                }
+
                 /* Set to non-secure if not a secure boot */
                 if (!info->secure_boot &&
                     (cs != first_cpu || !info->secure_board_setup)) {
-- 
2.16.2

[Qemu-devel] [PATCH 4/9] hw/arm/bcm2386: Fix parent type of bcm2386

[Peter Maydell]

The TypeInfo and state struct for bcm2386 disagree about what the
parent class is -- the TypeInfo says it's TYPE_SYS_BUS_DEVICE,
but the BCM2386State struct only defines the parent_obj field
as DeviceState. This would have caused problems if anything
actually tried to treat the object as a TYPE_SYS_BUS_DEVICE.
Fix the TypeInfo to use TYPE_DEVICE as the parent, since we don't
need any of the additional functionality TYPE_SYS_BUS_DEVICE
provides.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I noticed this when I tried to make the type into one which
has its own class struct, because we hit the assert that the
child's class struct had better be bigger than the parent's.
---
 hw/arm/bcm2836.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 40e8b25a46..9266f27c14 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -165,7 +165,7 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
 
 static const TypeInfo bcm2836_type_info = {
     .name = TYPE_BCM2836,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(BCM2836State),
     .instance_init = bcm2836_init,
     .class_init = bcm2836_class_init,
-- 
2.16.2

[Qemu-devel] [PATCH 5/9] hw/arm/bcm2836: Rename bcm2836 type/struct to bcm283x

[Peter Maydell]

Our BCM2836 type is really a generic one that can be any of
the bcm283x family. Rename it accordingly. We change only
the names which are visible via the header file to the
rest of the QEMU code, leaving private function names
in bcm2836.c as they are.

This is a preliminary to making bcm283x be an abstract
parent class to specific types for the bcm2836 and bcm2837.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/bcm2836.h | 12 ++++++------
 hw/arm/bcm2836.c         | 17 +++++++++--------
 hw/arm/raspi.c           | 16 ++++++++--------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 4758b4ae54..9a10a76631 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -15,12 +15,12 @@
 #include "hw/arm/bcm2835_peripherals.h"
 #include "hw/intc/bcm2836_control.h"
 
-#define TYPE_BCM2836 "bcm2836"
-#define BCM2836(obj) OBJECT_CHECK(BCM2836State, (obj), TYPE_BCM2836)
+#define TYPE_BCM283X "bcm283x"
+#define BCM283X(obj) OBJECT_CHECK(BCM283XState, (obj), TYPE_BCM283X)
 
-#define BCM2836_NCPUS 4
+#define BCM283X_NCPUS 4
 
-typedef struct BCM2836State {
+typedef struct BCM283XState {
     /*< private >*/
     DeviceState parent_obj;
     /*< public >*/
@@ -28,9 +28,9 @@ typedef struct BCM2836State {
     char *cpu_type;
     uint32_t enabled_cpus;
 
-    ARMCPU cpus[BCM2836_NCPUS];
+    ARMCPU cpus[BCM283X_NCPUS];
     BCM2836ControlState control;
     BCM2835PeripheralState peripherals;
-} BCM2836State;
+} BCM283XState;
 
 #endif /* BCM2836_H */
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 9266f27c14..1d1908654b 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -25,7 +25,7 @@
 
 static void bcm2836_init(Object *obj)
 {
-    BCM2836State *s = BCM2836(obj);
+    BCM283XState *s = BCM283X(obj);
 
     object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
     object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
@@ -44,7 +44,7 @@ static void bcm2836_init(Object *obj)
 
 static void bcm2836_realize(DeviceState *dev, Error **errp)
 {
-    BCM2836State *s = BCM2836(dev);
+    BCM283XState *s = BCM283X(dev);
     Object *obj;
     Error *err = NULL;
     int n;
@@ -52,7 +52,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
     /* common peripherals from bcm2835 */
 
     obj = OBJECT(dev);
-    for (n = 0; n < BCM2836_NCPUS; n++) {
+    for (n = 0; n < BCM283X_NCPUS; n++) {
         object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
                           s->cpu_type);
         object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
@@ -102,7 +102,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
         qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0));
 
-    for (n = 0; n < BCM2836_NCPUS; n++) {
+    for (n = 0; n < BCM283X_NCPUS; n++) {
         /* Mirror bcm2836, which has clusterid set to 0xf
          * TODO: this should be converted to a property of ARM_CPU
          */
@@ -150,8 +150,9 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
 }
 
 static Property bcm2836_props[] = {
-    DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type),
-    DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus, BCM2836_NCPUS),
+    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
+    DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
+                       BCM283X_NCPUS),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -164,9 +165,9 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
 }
 
 static const TypeInfo bcm2836_type_info = {
-    .name = TYPE_BCM2836,
+    .name = TYPE_BCM283X,
     .parent = TYPE_DEVICE,
-    .instance_size = sizeof(BCM2836State),
+    .instance_size = sizeof(BCM283XState),
     .instance_init = bcm2836_init,
     .class_init = bcm2836_class_init,
 };
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 1ac0737149..58c6e80a17 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -32,7 +32,7 @@
 static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
 
 typedef struct RasPiState {
-    BCM2836State soc;
+    BCM283XState soc;
     MemoryRegion ram;
 } RasPiState;
 
@@ -136,7 +136,7 @@ static void raspi_init(MachineState *machine, int version)
     BusState *bus;
     DeviceState *carddev;
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM2836);
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
     object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
                               &error_abort);
 
@@ -189,9 +189,9 @@ static void raspi2_machine_init(MachineClass *mc)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
-    mc->max_cpus = BCM2836_NCPUS;
-    mc->min_cpus = BCM2836_NCPUS;
-    mc->default_cpus = BCM2836_NCPUS;
+    mc->max_cpus = BCM283X_NCPUS;
+    mc->min_cpus = BCM283X_NCPUS;
+    mc->default_cpus = BCM283X_NCPUS;
     mc->default_ram_size = 1024 * 1024 * 1024;
     mc->ignore_memory_transaction_failures = true;
 };
@@ -212,9 +212,9 @@ static void raspi3_machine_init(MachineClass *mc)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
-    mc->max_cpus = BCM2836_NCPUS;
-    mc->min_cpus = BCM2836_NCPUS;
-    mc->default_cpus = BCM2836_NCPUS;
+    mc->max_cpus = BCM283X_NCPUS;
+    mc->min_cpus = BCM283X_NCPUS;
+    mc->default_cpus = BCM283X_NCPUS;
     mc->default_ram_size = 1024 * 1024 * 1024;
 }
 DEFINE_MACHINE("raspi3", raspi3_machine_init)
-- 
2.16.2

[Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Peter Maydell]

The bcm2837 is pretty similar to the bcm2836, but it does have
some differences. Notably, the MPIDR affinity aff1 values it
sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
uses, and if this is wrong Linux will not boot.

Rather than trying to have one device with properties that
configure it differently for the two cases, create two
separate QOM devices for the two SoCs. We use the same approach
as hw/arm/aspeed_soc.c and share code and have a data table
that might differ per-SoC. For the moment the two types don't
actually have different behaviour.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/bcm2836.h | 19 +++++++++++++++++++
 hw/arm/bcm2836.c         | 37 ++++++++++++++++++++++++++++++++-----
 hw/arm/raspi.c           |  3 ++-
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 9a10a76631..93248399ba 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -20,6 +20,13 @@
 
 #define BCM283X_NCPUS 4
 
+/* These type names are for specific SoCs; other than instantiating
+ * them, code using these devices should always handle them via the
+ * BCM283x base class, so they have no BCM2836(obj) etc macros.
+ */
+#define TYPE_BCM2836 "bcm2836"
+#define TYPE_BCM2837 "bcm2837"
+
 typedef struct BCM283XState {
     /*< private >*/
     DeviceState parent_obj;
@@ -33,4 +40,16 @@ typedef struct BCM283XState {
     BCM2835PeripheralState peripherals;
 } BCM283XState;
 
+typedef struct BCM283XInfo BCM283XInfo;
+
+typedef struct BCM283XClass {
+    DeviceClass parent_class;
+    const BCM283XInfo *info;
+} BCM283XClass;
+
+#define BCM283X_CLASS(klass) \
+    OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
+#define BCM283X_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
+
 #endif /* BCM2836_H */
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 1d1908654b..07d2705f96 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -23,6 +23,19 @@
 /* "QA7" (Pi2) interrupt controller and mailboxes etc. */
 #define BCM2836_CONTROL_BASE    0x40000000
 
+struct BCM283XInfo {
+    const char *name;
+};
+
+static const BCM283XInfo bcm283x_socs[] = {
+    {
+        .name = TYPE_BCM2836,
+    },
+    {
+        .name = TYPE_BCM2837,
+    },
+};
+
 static void bcm2836_init(Object *obj)
 {
     BCM283XState *s = BCM283X(obj);
@@ -156,25 +169,39 @@ static Property bcm2836_props[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
-static void bcm2836_class_init(ObjectClass *oc, void *data)
+static void bcm283x_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
+    BCM283XClass *bc = BCM283X_CLASS(oc);
 
-    dc->props = bcm2836_props;
+    bc->info = data;
     dc->realize = bcm2836_realize;
+    dc->props = bcm2836_props;
 }
 
-static const TypeInfo bcm2836_type_info = {
+static const TypeInfo bcm283x_type_info = {
     .name = TYPE_BCM283X,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(BCM283XState),
     .instance_init = bcm2836_init,
-    .class_init = bcm2836_class_init,
+    .class_size = sizeof(BCM283XClass),
+    .abstract = true,
 };
 
 static void bcm2836_register_types(void)
 {
-    type_register_static(&bcm2836_type_info);
+    int i;
+
+    type_register_static(&bcm283x_type_info);
+    for (i = 0; i < ARRAY_SIZE(bcm283x_socs); i++) {
+        TypeInfo ti = {
+            .name = bcm283x_socs[i].name,
+            .parent = TYPE_BCM283X,
+            .class_init = bcm283x_class_init,
+            .class_data = (void *) &bcm283x_socs[i],
+        };
+        type_register(&ti);
+    }
 }
 
 type_init(bcm2836_register_types)
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 58c6e80a17..f588720138 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -136,7 +136,8 @@ static void raspi_init(MachineState *machine, int version)
     BusState *bus;
     DeviceState *carddev;
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
+    object_initialize(&s->soc, sizeof(s->soc),
+                      version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
     object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
                               &error_abort);
 
-- 
2.16.2

[Qemu-devel] [PATCH 7/9] hw/arm/bcm2836: Use correct affinity values for BCM2837

[Peter Maydell]

The BCM2837 sets the Aff1 field of the MPIDR affinity values for the
CPUs to 0, whereas the BCM2836 uses 0xf. Set this correctly, as it
is required for Linux to boot.

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

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 07d2705f96..7140257c98 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -25,14 +25,17 @@
 
 struct BCM283XInfo {
     const char *name;
+    int clusterid;
 };
 
 static const BCM283XInfo bcm283x_socs[] = {
     {
         .name = TYPE_BCM2836,
+        .clusterid = 0xf,
     },
     {
         .name = TYPE_BCM2837,
+        .clusterid = 0x0,
     },
 };
 
@@ -58,6 +61,8 @@ static void bcm2836_init(Object *obj)
 static void bcm2836_realize(DeviceState *dev, Error **errp)
 {
     BCM283XState *s = BCM283X(dev);
+    BCM283XClass *bc = BCM283X_GET_CLASS(dev);
+    const BCM283XInfo *info = bc->info;
     Object *obj;
     Error *err = NULL;
     int n;
@@ -119,7 +124,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
         /* Mirror bcm2836, which has clusterid set to 0xf
          * TODO: this should be converted to a property of ARM_CPU
          */
-        s->cpus[n].mp_affinity = 0xF00 | n;
+        s->cpus[n].mp_affinity = (info->clusterid << 8) | n;
 
         /* set periphbase/CBAR value for CPU-local registers */
         object_property_set_int(OBJECT(&s->cpus[n]),
-- 
2.16.2

[Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

Now we have separate types for BCM2386 and BCM2387, we might as well
just hard-code the CPU type they use rather than having it passed
through as an object property. This then lets us put the initialization
of the CPU object in init rather than realize.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/bcm2836.c | 22 +++++++++++++---------
 hw/arm/raspi.c   |  2 --
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 7140257c98..12f75b55a7 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -25,16 +25,19 @@
 
 struct BCM283XInfo {
     const char *name;
+    const char *cpu_type;
     int clusterid;
 };
 
 static const BCM283XInfo bcm283x_socs[] = {
     {
         .name = TYPE_BCM2836,
+        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
         .clusterid = 0xf,
     },
     {
         .name = TYPE_BCM2837,
+        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
         .clusterid = 0x0,
     },
 };
@@ -42,6 +45,16 @@ static const BCM283XInfo bcm283x_socs[] = {
 static void bcm2836_init(Object *obj)
 {
     BCM283XState *s = BCM283X(obj);
+    BCM283XClass *bc = BCM283X_GET_CLASS(obj);
+    const BCM283XInfo *info = bc->info;
+    int n;
+
+    for (n = 0; n < BCM283X_NCPUS; n++) {
+        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
+                          info->cpu_type);
+        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
+                                  &error_abort);
+    }
 
     object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
     object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
@@ -69,14 +82,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
 
     /* common peripherals from bcm2835 */
 
-    obj = OBJECT(dev);
-    for (n = 0; n < BCM283X_NCPUS; n++) {
-        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
-                          s->cpu_type);
-        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
-                                  &error_abort);
-    }
-
     obj = object_property_get_link(OBJECT(dev), "ram", &err);
     if (obj == NULL) {
         error_setg(errp, "%s: required ram link not found: %s",
@@ -168,7 +173,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
 }
 
 static Property bcm2836_props[] = {
-    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
     DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
                        BCM283X_NCPUS),
     DEFINE_PROP_END_OF_LIST()
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index f588720138..ae15997669 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int version)
     /* Setup the SOC */
     object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
                                    &error_abort);
-    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
-                            &error_abort);
     object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
                             &error_abort);
     int board_rev = version == 3 ? 0xa02082 : 0xa21041;
-- 
2.16.2

[Qemu-devel] [PATCH 9/9] hw/arm/raspi: Provide spin-loop code for AArch64 CPUs

[Peter Maydell]

The raspi3 has AArch64 CPUs, which means that our smpboot
code for keeping the secondary CPUs in a pen needs to have
a version for A64 as well as A32. Without this, the
secondary CPUs go into an infinite loop of taking undefined
instruction exceptions.

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

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index ae15997669..06f1e08ca9 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -27,6 +27,7 @@
 #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
 #define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */
 #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */
+#define SPINTABLE_ADDR  0xd8 /* Pi 3 bootloader spintable */
 
 /* Table of Linux board IDs for different Pi versions */
 static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
@@ -63,6 +64,40 @@ static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
                        info->smp_loader_start);
 }
 
+static void write_smpboot64(ARMCPU *cpu, const struct arm_boot_info *info)
+{
+    /* Unlike the AArch32 version we don't need to call the board setup hook.
+     * The mechanism for doing the spin-table is also entirely different.
+     * We must have four 64-bit fields at absolute addresses
+     * 0xd8, 0xe0, 0xe8, 0xf0 in RAM, which are the flag variables for
+     * our CPUs, and which we must ensure are zero initialized before
+     * the primary CPU goes into the kernel. We put these variables inside
+     * a rom blob, so that the reset for ROM contents zeroes them for us.
+     */
+    static const uint32_t smpboot[] = {
+        0xd2801b05, /*        mov     x5, 0xd8 */
+        0xd53800a6, /*        mrs     x6, mpidr_el1 */
+        0x924004c6, /*        and     x6, x6, #0x3 */
+        0xd503205f, /* spin:  wfe */
+        0xf86678a4, /*        ldr     x4, [x5,x6,lsl #3] */
+        0xb4ffffc4, /*        cbz     x4, spin */
+        0xd2800000, /*        mov     x0, #0x0 */
+        0xd2800001, /*        mov     x1, #0x0 */
+        0xd2800002, /*        mov     x2, #0x0 */
+        0xd2800003, /*        mov     x3, #0x0 */
+        0xd61f0080, /*        br      x4 */
+    };
+
+    static const uint64_t spintables[] = {
+        0, 0, 0, 0
+    };
+
+    rom_add_blob_fixed("raspi_smpboot", smpboot, sizeof(smpboot),
+                       info->smp_loader_start);
+    rom_add_blob_fixed("raspi_spintables", spintables, sizeof(spintables),
+                       SPINTABLE_ADDR);
+}
+
 static void write_board_setup(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     arm_write_secure_board_setup_dummy_smc(cpu, info, MVBAR_ADDR);
@@ -99,7 +134,11 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     /* Pi2 and Pi3 requires SMP setup */
     if (version >= 2) {
         binfo.smp_loader_start = SMPBOOT_ADDR;
-        binfo.write_secondary_boot = write_smpboot;
+        if (version == 2) {
+            binfo.write_secondary_boot = write_smpboot;
+        } else {
+            binfo.write_secondary_boot = write_smpboot64;
+        }
         binfo.secondary_cpu_reset_hook = reset_secondary;
     }
 
-- 
2.16.2

Re: [Qemu-devel] [PATCH 1/9] hw/arm/raspi: Don't do board-setup or secure-boot for raspi3

[Andrew Baumann]

> From: Qemu-devel <qemu-devel-
> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> For the rpi1 and 2 we want to boot the Linux kernel via some
> custom setup code that makes sure that the SMC instruction
> acts as a no-op, because it's used for cache maintenance.
> The rpi3 boots AArch64 kernels, which don't need SMC for
> cache maintenance and always expect to be booted non-secure.
> Don't fill in the aarch32-specific parts of the binfo struct.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/raspi.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index a37881433c..1ac0737149 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -82,10 +82,19 @@ static void setup_boot(MachineState *machine, int
> version, size_t ram_size)
>      binfo.board_id = raspi_boardid[version];
>      binfo.ram_size = ram_size;
>      binfo.nb_cpus = smp_cpus;
> -    binfo.board_setup_addr = BOARDSETUP_ADDR;
> -    binfo.write_board_setup = write_board_setup;
> -    binfo.secure_board_setup = true;
> -    binfo.secure_boot = true;
> +
> +    if (version <= 2) {
> +        /* The rpi1 and 2 require some custom setup code to run in Secure
> +         * mode before booting a kernel (to set up the SMC vectors so
> +         * that we get a no-op SMC; this is used by Linux to call the
> +         * firmware for some cache maintenance operations.
> +         * The rpi3 doesn't need this.
> +         */
> +        binfo.board_setup_addr = BOARDSETUP_ADDR;
> +        binfo.write_board_setup = write_board_setup;
> +        binfo.secure_board_setup = true;
> +        binfo.secure_boot = true;
> +    }
> 
>      /* Pi2 and Pi3 requires SMP setup */
>      if (version >= 2) {

Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Re: [Qemu-devel] [PATCH 4/9] hw/arm/bcm2386: Fix parent type of bcm2386

[Andrew Baumann]

> From: Qemu-devel <qemu-devel-
> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> The TypeInfo and state struct for bcm2386 disagree about what the
> parent class is -- the TypeInfo says it's TYPE_SYS_BUS_DEVICE,
> but the BCM2386State struct only defines the parent_obj field
> as DeviceState. This would have caused problems if anything
> actually tried to treat the object as a TYPE_SYS_BUS_DEVICE.
> Fix the TypeInfo to use TYPE_DEVICE as the parent, since we don't
> need any of the additional functionality TYPE_SYS_BUS_DEVICE
> provides.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I noticed this when I tried to make the type into one which
> has its own class struct, because we hit the assert that the
> child's class struct had better be bigger than the parent's.
> ---
>  hw/arm/bcm2836.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 40e8b25a46..9266f27c14 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -165,7 +165,7 @@ static void bcm2836_class_init(ObjectClass *oc, void
> *data)
> 
>  static const TypeInfo bcm2836_type_info = {
>      .name = TYPE_BCM2836,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(BCM2836State),
>      .instance_init = bcm2836_init,
>      .class_init = bcm2836_class_init,

Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Thanks for catching this -- it looks like bcm2835.c (which never got merged) has the same bug, and I copied it along.

Re: [Qemu-devel] [PATCH 5/9] hw/arm/bcm2836: Rename bcm2836 type/struct to bcm283x

[Andrew Baumann]

> From: Qemu-devel <qemu-devel-
> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> Our BCM2836 type is really a generic one that can be any of
> the bcm283x family. Rename it accordingly. We change only
> the names which are visible via the header file to the
> rest of the QEMU code, leaving private function names
> in bcm2836.c as they are.
> 
> This is a preliminary to making bcm283x be an abstract
> parent class to specific types for the bcm2836 and bcm2837.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/bcm2836.h | 12 ++++++------
>  hw/arm/bcm2836.c         | 17 +++++++++--------
>  hw/arm/raspi.c           | 16 ++++++++--------
>  3 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 4758b4ae54..9a10a76631 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -15,12 +15,12 @@
>  #include "hw/arm/bcm2835_peripherals.h"
>  #include "hw/intc/bcm2836_control.h"
> 
> -#define TYPE_BCM2836 "bcm2836"
> -#define BCM2836(obj) OBJECT_CHECK(BCM2836State, (obj), TYPE_BCM2836)
> +#define TYPE_BCM283X "bcm283x"
> +#define BCM283X(obj) OBJECT_CHECK(BCM283XState, (obj), TYPE_BCM283X)
> 
> -#define BCM2836_NCPUS 4
> +#define BCM283X_NCPUS 4
>
> 
> -typedef struct BCM2836State {
> +typedef struct BCM283XState {
>      /*< private >*/
>      DeviceState parent_obj;
>      /*< public >*/
> @@ -28,9 +28,9 @@ typedef struct BCM2836State {
>      char *cpu_type;
>      uint32_t enabled_cpus;
> 
> -    ARMCPU cpus[BCM2836_NCPUS];
> +    ARMCPU cpus[BCM283X_NCPUS];
>      BCM2836ControlState control;
>      BCM2835PeripheralState peripherals;
> -} BCM2836State;
> +} BCM283XState;
> 
>  #endif /* BCM2836_H */
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 9266f27c14..1d1908654b 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,7 +25,7 @@
> 
>  static void bcm2836_init(Object *obj)
>  {
> -    BCM2836State *s = BCM2836(obj);
> +    BCM283XState *s = BCM283X(obj);
> 
>      object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
>      object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
> @@ -44,7 +44,7 @@ static void bcm2836_init(Object *obj)
> 
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
> -    BCM2836State *s = BCM2836(dev);
> +    BCM283XState *s = BCM283X(dev);
>      Object *obj;
>      Error *err = NULL;
>      int n;
> @@ -52,7 +52,7 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>      /* common peripherals from bcm2835 */
> 
>      obj = OBJECT(dev);
> -    for (n = 0; n < BCM2836_NCPUS; n++) {
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
>          object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
>                            s->cpu_type);
>          object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> @@ -102,7 +102,7 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
>          qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0));
> 
> -    for (n = 0; n < BCM2836_NCPUS; n++) {
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
>          /* Mirror bcm2836, which has clusterid set to 0xf
>           * TODO: this should be converted to a property of ARM_CPU
>           */
> @@ -150,8 +150,9 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>  }
> 
>  static Property bcm2836_props[] = {
> -    DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type),
> -    DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus,
> BCM2836_NCPUS),
> +    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
> +    DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
> +                       BCM283X_NCPUS),
>      DEFINE_PROP_END_OF_LIST()
>  };
> 
> @@ -164,9 +165,9 @@ static void bcm2836_class_init(ObjectClass *oc, void
> *data)
>  }
> 
>  static const TypeInfo bcm2836_type_info = {
> -    .name = TYPE_BCM2836,
> +    .name = TYPE_BCM283X,
>      .parent = TYPE_DEVICE,
> -    .instance_size = sizeof(BCM2836State),
> +    .instance_size = sizeof(BCM283XState),
>      .instance_init = bcm2836_init,
>      .class_init = bcm2836_class_init,
>  };
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 1ac0737149..58c6e80a17 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -32,7 +32,7 @@
>  static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
> 
>  typedef struct RasPiState {
> -    BCM2836State soc;
> +    BCM283XState soc;
>      MemoryRegion ram;
>  } RasPiState;
> 
> @@ -136,7 +136,7 @@ static void raspi_init(MachineState *machine, int
> version)
>      BusState *bus;
>      DeviceState *carddev;
> 
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM2836);
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
> 
> @@ -189,9 +189,9 @@ static void raspi2_machine_init(MachineClass *mc)
>      mc->no_floppy = 1;
>      mc->no_cdrom = 1;
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> -    mc->max_cpus = BCM2836_NCPUS;
> -    mc->min_cpus = BCM2836_NCPUS;
> -    mc->default_cpus = BCM2836_NCPUS;
> +    mc->max_cpus = BCM283X_NCPUS;
> +    mc->min_cpus = BCM283X_NCPUS;
> +    mc->default_cpus = BCM283X_NCPUS;
>      mc->default_ram_size = 1024 * 1024 * 1024;
>      mc->ignore_memory_transaction_failures = true;
>  };
> @@ -212,9 +212,9 @@ static void raspi3_machine_init(MachineClass *mc)
>      mc->no_floppy = 1;
>      mc->no_cdrom = 1;
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
> -    mc->max_cpus = BCM2836_NCPUS;
> -    mc->min_cpus = BCM2836_NCPUS;
> -    mc->default_cpus = BCM2836_NCPUS;
> +    mc->max_cpus = BCM283X_NCPUS;
> +    mc->min_cpus = BCM283X_NCPUS;
> +    mc->default_cpus = BCM283X_NCPUS;
>      mc->default_ram_size = 1024 * 1024 * 1024;
>  }
>  DEFINE_MACHINE("raspi3", raspi3_machine_init)

Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Re: [Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Andrew Baumann]

> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, 13 March 2018 08:35
> 
> The bcm2837 is pretty similar to the bcm2836, but it does have
> some differences. Notably, the MPIDR affinity aff1 values it
> sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
> uses, and if this is wrong Linux will not boot.
> 
> Rather than trying to have one device with properties that
> configure it differently for the two cases, create two
> separate QOM devices for the two SoCs. We use the same approach
> as hw/arm/aspeed_soc.c and share code and have a data table
> that might differ per-SoC. For the moment the two types don't
> actually have different behaviour.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/bcm2836.h | 19 +++++++++++++++++++
>  hw/arm/bcm2836.c         | 37 ++++++++++++++++++++++++++++++++-----
>  hw/arm/raspi.c           |  3 ++-
>  3 files changed, 53 insertions(+), 6 deletions(-)

I haven't tried to understand enough of QOM to review this one... (It looks sane to me though.)

Andrew

Re: [Qemu-devel] [PATCH 7/9] hw/arm/bcm2836: Use correct affinity values for BCM2837

[Andrew Baumann]

> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, 13 March 2018 08:35
> 
> The BCM2837 sets the Aff1 field of the MPIDR affinity values for the
> CPUs to 0, whereas the BCM2836 uses 0xf. Set this correctly, as it
> is required for Linux to boot.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/bcm2836.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 07d2705f96..7140257c98 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,14 +25,17 @@
> 
>  struct BCM283XInfo {
>      const char *name;
> +    int clusterid;
>  };
> 
>  static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
> +        .clusterid = 0xf,
>      },
>      {
>          .name = TYPE_BCM2837,
> +        .clusterid = 0x0,
>      },
>  };
> 
> @@ -58,6 +61,8 @@ static void bcm2836_init(Object *obj)
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
>      BCM283XState *s = BCM283X(dev);
> +    BCM283XClass *bc = BCM283X_GET_CLASS(dev);
> +    const BCM283XInfo *info = bc->info;
>      Object *obj;
>      Error *err = NULL;
>      int n;
> @@ -119,7 +124,7 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>          /* Mirror bcm2836, which has clusterid set to 0xf

The first line of this comment should probably move or just be deleted. It's not relevant here.

>           * TODO: this should be converted to a property of ARM_CPU
>           */
> -        s->cpus[n].mp_affinity = 0xF00 | n;
> +        s->cpus[n].mp_affinity = (info->clusterid << 8) | n;
> 
>          /* set periphbase/CBAR value for CPU-local registers */
>          object_property_set_int(OBJECT(&s->cpus[n]),

Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Andrew Baumann]

> From: Qemu-devel <qemu-devel-
> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> Now we have separate types for BCM2386 and BCM2387, we might as well
> just hard-code the CPU type they use rather than having it passed
> through as an object property. This then lets us put the initialization
> of the CPU object in init rather than realize.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/bcm2836.c | 22 +++++++++++++---------
>  hw/arm/raspi.c   |  2 --
>  2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 7140257c98..12f75b55a7 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,16 +25,19 @@
> 
>  struct BCM283XInfo {
>      const char *name;
> +    const char *cpu_type;
>      int clusterid;
>  };
> 
>  static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),

At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?

(Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)

>          .clusterid = 0xf,
>      },
>      {
>          .name = TYPE_BCM2837,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
>          .clusterid = 0x0,
>      },
>  };
> @@ -42,6 +45,16 @@ static const BCM283XInfo bcm283x_socs[] = {
>  static void bcm2836_init(Object *obj)
>  {
>      BCM283XState *s = BCM283X(obj);
> +    BCM283XClass *bc = BCM283X_GET_CLASS(obj);
> +    const BCM283XInfo *info = bc->info;
> +    int n;
> +
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
> +        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
> +                          info->cpu_type);
> +        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> +                                  &error_abort);
> +    }
> 
>      object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
>      object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
> @@ -69,14 +82,6 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
> 
>      /* common peripherals from bcm2835 */
> 
> -    obj = OBJECT(dev);
> -    for (n = 0; n < BCM283X_NCPUS; n++) {
> -        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
> -                          s->cpu_type);
> -        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> -                                  &error_abort);
> -    }
> -
>      obj = object_property_get_link(OBJECT(dev), "ram", &err);
>      if (obj == NULL) {
>          error_setg(errp, "%s: required ram link not found: %s",
> @@ -168,7 +173,6 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>  }
> 
>  static Property bcm2836_props[] = {
> -    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
>      DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
>                         BCM283X_NCPUS),
>      DEFINE_PROP_END_OF_LIST()
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index f588720138..ae15997669 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
> version)
>      /* Setup the SOC */
>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>                                     &error_abort);
> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
> -                            &error_abort);
>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>                              &error_abort);
>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;

What about the default_cpu_type field of MachineClass set in raspi[23]_machine_init? That seems irrelevant now... Also, if anyone cares (I don't), we also just lost the ability to override the CPU type of a raspi model. 

Otherwise,
Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Cheers,
Andrew

Re: [Qemu-devel] [PATCH 7/9] hw/arm/bcm2836: Use correct affinity values for BCM2837

[Peter Maydell]

On 13 March 2018 at 16:48, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>> From: Peter Maydell <peter.maydell@linaro.org>
>> Sent: Tuesday, 13 March 2018 08:35
>>
>> The BCM2837 sets the Aff1 field of the MPIDR affinity values for the
>> CPUs to 0, whereas the BCM2836 uses 0xf. Set this correctly, as it
>> is required for Linux to boot.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> @@ -119,7 +124,7 @@ static void bcm2836_realize(DeviceState *dev, Error
>> **errp)
>>          /* Mirror bcm2836, which has clusterid set to 0xf
>
> The first line of this comment should probably move or just be deleted. It's not relevant here.

Yeah, simplest just to delete it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>> From: Qemu-devel <qemu-devel-
>> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
>> Maydell
>> Sent: Tuesday, 13 March 2018 08:35
>>
>> Now we have separate types for BCM2386 and BCM2387, we might as well
>> just hard-code the CPU type they use rather than having it passed
>> through as an object property. This then lets us put the initialization
>> of the CPU object in init rather than realize.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>>  static const BCM283XInfo bcm283x_socs[] = {
>>      {
>>          .name = TYPE_BCM2836,
>> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>
> At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?
>
> (Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)

Yeah, we should do that. I'd forgotten about that, I think
things just got lost in the shuffle of having several
patchsets that tried to change the same things at once.

I guess the simplest thing is to add a patch at the end of
the series that fixes the cpu type for bcm2836.


>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
>> version)
>>      /* Setup the SOC */
>>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>>                                     &error_abort);
>> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
>> -                            &error_abort);
>>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>>                              &error_abort);
>>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;
>
> What about the default_cpu_type field of MachineClass set in
> raspi[23]_machine_init? That seems irrelevant now...

Mmm. It doesn't hurt anything, though.

> Also, if anyone cares (I don't), we also just lost the ability
> to override the CPU type of a raspi model.

Yeah, that's deliberate -- I think that letting the user randomly
plug nonexistent combinations together just confuses people when
they don't work. I guess I should call it out in the commit message
though.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 5/9] hw/arm/bcm2836: Rename bcm2836 type/struct to bcm283x

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> Our BCM2836 type is really a generic one that can be any of
> the bcm283x family. Rename it accordingly. We change only
> the names which are visible via the header file to the
> rest of the QEMU code, leaving private function names
> in bcm2836.c as they are.
> 
> This is a preliminary to making bcm283x be an abstract
> parent class to specific types for the bcm2836 and bcm2837.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/bcm2836.h | 12 ++++++------
>  hw/arm/bcm2836.c         | 17 +++++++++--------
>  hw/arm/raspi.c           | 16 ++++++++--------
>  3 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 4758b4ae54..9a10a76631 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -15,12 +15,12 @@
>  #include "hw/arm/bcm2835_peripherals.h"
>  #include "hw/intc/bcm2836_control.h"
>  
> -#define TYPE_BCM2836 "bcm2836"
> -#define BCM2836(obj) OBJECT_CHECK(BCM2836State, (obj), TYPE_BCM2836)
> +#define TYPE_BCM283X "bcm283x"
> +#define BCM283X(obj) OBJECT_CHECK(BCM283XState, (obj), TYPE_BCM283X)
>  
> -#define BCM2836_NCPUS 4
> +#define BCM283X_NCPUS 4
>  
> -typedef struct BCM2836State {
> +typedef struct BCM283XState {
>      /*< private >*/
>      DeviceState parent_obj;
>      /*< public >*/
> @@ -28,9 +28,9 @@ typedef struct BCM2836State {
>      char *cpu_type;
>      uint32_t enabled_cpus;
>  
> -    ARMCPU cpus[BCM2836_NCPUS];
> +    ARMCPU cpus[BCM283X_NCPUS];
>      BCM2836ControlState control;
>      BCM2835PeripheralState peripherals;
> -} BCM2836State;
> +} BCM283XState;
>  
>  #endif /* BCM2836_H */
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 9266f27c14..1d1908654b 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,7 +25,7 @@
>  
>  static void bcm2836_init(Object *obj)
>  {
> -    BCM2836State *s = BCM2836(obj);
> +    BCM283XState *s = BCM283X(obj);
>  
>      object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
>      object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
> @@ -44,7 +44,7 @@ static void bcm2836_init(Object *obj)
>  
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
> -    BCM2836State *s = BCM2836(dev);
> +    BCM283XState *s = BCM283X(dev);
>      Object *obj;
>      Error *err = NULL;
>      int n;
> @@ -52,7 +52,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>      /* common peripherals from bcm2835 */
>  
>      obj = OBJECT(dev);
> -    for (n = 0; n < BCM2836_NCPUS; n++) {
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
>          object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
>                            s->cpu_type);
>          object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> @@ -102,7 +102,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
>          qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-fiq", 0));
>  
> -    for (n = 0; n < BCM2836_NCPUS; n++) {
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
>          /* Mirror bcm2836, which has clusterid set to 0xf
>           * TODO: this should be converted to a property of ARM_CPU
>           */
> @@ -150,8 +150,9 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property bcm2836_props[] = {
> -    DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type),
> -    DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus, BCM2836_NCPUS),
> +    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
> +    DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
> +                       BCM283X_NCPUS),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -164,9 +165,9 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>  }
>  
>  static const TypeInfo bcm2836_type_info = {
> -    .name = TYPE_BCM2836,
> +    .name = TYPE_BCM283X,
>      .parent = TYPE_DEVICE,
> -    .instance_size = sizeof(BCM2836State),
> +    .instance_size = sizeof(BCM283XState),
>      .instance_init = bcm2836_init,
>      .class_init = bcm2836_class_init,
>  };
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 1ac0737149..58c6e80a17 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -32,7 +32,7 @@
>  static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
>  
>  typedef struct RasPiState {
> -    BCM2836State soc;
> +    BCM283XState soc;
>      MemoryRegion ram;
>  } RasPiState;
>  
> @@ -136,7 +136,7 @@ static void raspi_init(MachineState *machine, int version)
>      BusState *bus;
>      DeviceState *carddev;
>  
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM2836);
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
>  
> @@ -189,9 +189,9 @@ static void raspi2_machine_init(MachineClass *mc)
>      mc->no_floppy = 1;
>      mc->no_cdrom = 1;
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> -    mc->max_cpus = BCM2836_NCPUS;
> -    mc->min_cpus = BCM2836_NCPUS;
> -    mc->default_cpus = BCM2836_NCPUS;
> +    mc->max_cpus = BCM283X_NCPUS;
> +    mc->min_cpus = BCM283X_NCPUS;
> +    mc->default_cpus = BCM283X_NCPUS;
>      mc->default_ram_size = 1024 * 1024 * 1024;
>      mc->ignore_memory_transaction_failures = true;
>  };
> @@ -212,9 +212,9 @@ static void raspi3_machine_init(MachineClass *mc)
>      mc->no_floppy = 1;
>      mc->no_cdrom = 1;
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
> -    mc->max_cpus = BCM2836_NCPUS;
> -    mc->min_cpus = BCM2836_NCPUS;
> -    mc->default_cpus = BCM2836_NCPUS;
> +    mc->max_cpus = BCM283X_NCPUS;
> +    mc->min_cpus = BCM283X_NCPUS;
> +    mc->default_cpus = BCM283X_NCPUS;
>      mc->default_ram_size = 1024 * 1024 * 1024;
>  }
>  DEFINE_MACHINE("raspi3", raspi3_machine_init)
> 

Re: [Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> The bcm2837 is pretty similar to the bcm2836, but it does have
> some differences. Notably, the MPIDR affinity aff1 values it
> sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
> uses, and if this is wrong Linux will not boot.
> 
> Rather than trying to have one device with properties that
> configure it differently for the two cases, create two
> separate QOM devices for the two SoCs. We use the same approach
> as hw/arm/aspeed_soc.c and share code and have a data table
> that might differ per-SoC. For the moment the two types don't
> actually have different behaviour.

:)

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/arm/bcm2836.h | 19 +++++++++++++++++++
>  hw/arm/bcm2836.c         | 37 ++++++++++++++++++++++++++++++++-----
>  hw/arm/raspi.c           |  3 ++-
>  3 files changed, 53 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 9a10a76631..93248399ba 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -20,6 +20,13 @@
>  
>  #define BCM283X_NCPUS 4
>  
> +/* These type names are for specific SoCs; other than instantiating
> + * them, code using these devices should always handle them via the
> + * BCM283x base class, so they have no BCM2836(obj) etc macros.
> + */
> +#define TYPE_BCM2836 "bcm2836"
> +#define TYPE_BCM2837 "bcm2837"
> +
>  typedef struct BCM283XState {
>      /*< private >*/
>      DeviceState parent_obj;
> @@ -33,4 +40,16 @@ typedef struct BCM283XState {
>      BCM2835PeripheralState peripherals;
>  } BCM283XState;
>  
> +typedef struct BCM283XInfo BCM283XInfo;
> +
> +typedef struct BCM283XClass {
> +    DeviceClass parent_class;
> +    const BCM283XInfo *info;
> +} BCM283XClass;
> +
> +#define BCM283X_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
> +#define BCM283X_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
> +
>  #endif /* BCM2836_H */
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 1d1908654b..07d2705f96 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -23,6 +23,19 @@
>  /* "QA7" (Pi2) interrupt controller and mailboxes etc. */
>  #define BCM2836_CONTROL_BASE    0x40000000
>  
> +struct BCM283XInfo {
> +    const char *name;
> +};
> +
> +static const BCM283XInfo bcm283x_socs[] = {
> +    {
> +        .name = TYPE_BCM2836,
> +    },
> +    {
> +        .name = TYPE_BCM2837,
> +    },
> +};
> +
>  static void bcm2836_init(Object *obj)
>  {
>      BCM283XState *s = BCM283X(obj);
> @@ -156,25 +169,39 @@ static Property bcm2836_props[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> -static void bcm2836_class_init(ObjectClass *oc, void *data)
> +static void bcm283x_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> +    BCM283XClass *bc = BCM283X_CLASS(oc);
>  
> -    dc->props = bcm2836_props;
> +    bc->info = data;
>      dc->realize = bcm2836_realize;
> +    dc->props = bcm2836_props;
>  }
>  
> -static const TypeInfo bcm2836_type_info = {
> +static const TypeInfo bcm283x_type_info = {
>      .name = TYPE_BCM283X,
>      .parent = TYPE_DEVICE,
>      .instance_size = sizeof(BCM283XState),
>      .instance_init = bcm2836_init,
> -    .class_init = bcm2836_class_init,
> +    .class_size = sizeof(BCM283XClass),
> +    .abstract = true,
>  };
>  
>  static void bcm2836_register_types(void)
>  {
> -    type_register_static(&bcm2836_type_info);
> +    int i;
> +
> +    type_register_static(&bcm283x_type_info);
> +    for (i = 0; i < ARRAY_SIZE(bcm283x_socs); i++) {
> +        TypeInfo ti = {
> +            .name = bcm283x_socs[i].name,
> +            .parent = TYPE_BCM283X,
> +            .class_init = bcm283x_class_init,
> +            .class_data = (void *) &bcm283x_socs[i],
> +        };
> +        type_register(&ti);
> +    }
>  }
>  
>  type_init(bcm2836_register_types)
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 58c6e80a17..f588720138 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -136,7 +136,8 @@ static void raspi_init(MachineState *machine, int version)
>      BusState *bus;
>      DeviceState *carddev;
>  
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
> +    object_initialize(&s->soc, sizeof(s->soc),
> +                      version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
>  
> 

Re: [Qemu-devel] [PATCH 4/9] hw/arm/bcm2386: Fix parent type of bcm2386

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> The TypeInfo and state struct for bcm2386 disagree about what the
> parent class is -- the TypeInfo says it's TYPE_SYS_BUS_DEVICE,
> but the BCM2386State struct only defines the parent_obj field
> as DeviceState. This would have caused problems if anything
> actually tried to treat the object as a TYPE_SYS_BUS_DEVICE.
> Fix the TypeInfo to use TYPE_DEVICE as the parent, since we don't
> need any of the additional functionality TYPE_SYS_BUS_DEVICE
> provides.

I once wondered if we can dump the whole devices hierarchy (xml format)
and check consistency, or generate hyperlink doc and graph for the wiki...

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> I noticed this when I tried to make the type into one which
> has its own class struct, because we hit the assert that the
> child's class struct had better be bigger than the parent's.

Yeah once you understand this obscure assert(), it is VERY useful.

> ---
>  hw/arm/bcm2836.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 40e8b25a46..9266f27c14 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -165,7 +165,7 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>  
>  static const TypeInfo bcm2836_type_info = {
>      .name = TYPE_BCM2836,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(BCM2836State),
>      .instance_init = bcm2836_init,
>      .class_init = bcm2836_class_init,
> 

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> Now we have separate types for BCM2386 and BCM2387, we might as well
> just hard-code the CPU type they use rather than having it passed
> through as an object property. This then lets us put the initialization
> of the CPU object in init rather than realize.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/bcm2836.c | 22 +++++++++++++---------
>  hw/arm/raspi.c   |  2 --
>  2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 7140257c98..12f75b55a7 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,16 +25,19 @@
>  
>  struct BCM283XInfo {
>      const char *name;
> +    const char *cpu_type;
>      int clusterid;
>  };
>  
>  static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>          .clusterid = 0xf,
>      },
>      {
>          .name = TYPE_BCM2837,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
>          .clusterid = 0x0,
>      },
>  };
> @@ -42,6 +45,16 @@ static const BCM283XInfo bcm283x_socs[] = {
>  static void bcm2836_init(Object *obj)
>  {
>      BCM283XState *s = BCM283X(obj);
> +    BCM283XClass *bc = BCM283X_GET_CLASS(obj);
> +    const BCM283XInfo *info = bc->info;
> +    int n;
> +
> +    for (n = 0; n < BCM283X_NCPUS; n++) {
> +        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
> +                          info->cpu_type);
> +        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> +                                  &error_abort);
> +    }
>  
>      object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
>      object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
> @@ -69,14 +82,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>  
>      /* common peripherals from bcm2835 */
>  
> -    obj = OBJECT(dev);
> -    for (n = 0; n < BCM283X_NCPUS; n++) {
> -        object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
> -                          s->cpu_type);
> -        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
> -                                  &error_abort);
> -    }
> -
>      obj = object_property_get_link(OBJECT(dev), "ram", &err);
>      if (obj == NULL) {
>          error_setg(errp, "%s: required ram link not found: %s",
> @@ -168,7 +173,6 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property bcm2836_props[] = {
> -    DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
>      DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
>                         BCM283X_NCPUS),
>      DEFINE_PROP_END_OF_LIST()
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index f588720138..ae15997669 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int version)
>      /* Setup the SOC */
>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>                                     &error_abort);
> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
> -                            &error_abort);
>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>                              &error_abort);
>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;
> 

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Philippe Mathieu-Daudé]

On 03/13/2018 06:09 PM, Peter Maydell wrote:
> On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>>> From: Qemu-devel <qemu-devel-
>>> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
>>> Maydell
>>> Sent: Tuesday, 13 March 2018 08:35
>>>
>>> Now we have separate types for BCM2386 and BCM2387, we might as well
>>> just hard-code the CPU type they use rather than having it passed
>>> through as an object property. This then lets us put the initialization
>>> of the CPU object in init rather than realize.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
>>>  static const BCM283XInfo bcm283x_socs[] = {
>>>      {
>>>          .name = TYPE_BCM2836,
>>> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>>
>> At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?
>>
>> (Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)
> 
> Yeah, we should do that. I'd forgotten about that, I think
> things just got lost in the shuffle of having several
> patchsets that tried to change the same things at once.
> 
> I guess the simplest thing is to add a patch at the end of
> the series that fixes the cpu type for bcm2836.

Peter, here is the patch Andrew remembered (maybe can be applied at the
end):
http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg04286.html

> 
> 
>>> --- a/hw/arm/raspi.c
>>> +++ b/hw/arm/raspi.c
>>> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
>>> version)
>>>      /* Setup the SOC */
>>>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>>>                                     &error_abort);
>>> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
>>> -                            &error_abort);
>>>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>>>                              &error_abort);
>>>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;
>>
>> What about the default_cpu_type field of MachineClass set in
>> raspi[23]_machine_init? That seems irrelevant now...
> 
> Mmm. It doesn't hurt anything, though.
> 
>> Also, if anyone cares (I don't), we also just lost the ability
>> to override the CPU type of a raspi model.
> 
> Yeah, that's deliberate -- I think that letting the user randomly
> plug nonexistent combinations together just confuses people when
> they don't work. I guess I should call it out in the commit message
> though.
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Philippe Mathieu-Daudé]

On 03/13/2018 06:09 PM, Peter Maydell wrote:
> On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>>> From: Qemu-devel <qemu-devel-
>>> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
>>> Maydell
>>> Sent: Tuesday, 13 March 2018 08:35
>>>
>>> Now we have separate types for BCM2386 and BCM2387, we might as well
>>> just hard-code the CPU type they use rather than having it passed
>>> through as an object property. This then lets us put the initialization
>>> of the CPU object in init rather than realize.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
>>>  static const BCM283XInfo bcm283x_socs[] = {
>>>      {
>>>          .name = TYPE_BCM2836,
>>> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>>
>> At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?
>>
>> (Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)
> 
> Yeah, we should do that. I'd forgotten about that, I think
> things just got lost in the shuffle of having several
> patchsets that tried to change the same things at once.
> 
> I guess the simplest thing is to add a patch at the end of
> the series that fixes the cpu type for bcm2836.

Peter, here is the patch Andrew remembered (maybe can be applied at the
end):
http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg04286.html

> 
> 
>>> --- a/hw/arm/raspi.c
>>> +++ b/hw/arm/raspi.c
>>> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
>>> version)
>>>      /* Setup the SOC */
>>>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>>>                                     &error_abort);
>>> -    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
>>> -                            &error_abort);
>>>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>>>                              &error_abort);
>>>      int board_rev = version == 3 ? 0xa02082 : 0xa21041;
>>
>> What about the default_cpu_type field of MachineClass set in
>> raspi[23]_machine_init? That seems irrelevant now...
> 
> Mmm. It doesn't hurt anything, though.
> 
>> Also, if anyone cares (I don't), we also just lost the ability
>> to override the CPU type of a raspi model.
> 
> Yeah, that's deliberate -- I think that letting the user randomly
> plug nonexistent combinations together just confuses people when
> they don't work. I guess I should call it out in the commit message
> though.
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/9] hw/arm/raspi: Don't do board-setup or secure-boot for raspi3

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> For the rpi1 and 2 we want to boot the Linux kernel via some
> custom setup code that makes sure that the SMC instruction
> acts as a no-op, because it's used for cache maintenance.
> The rpi3 boots AArch64 kernels, which don't need SMC for
> cache maintenance and always expect to be booted non-secure.
> Don't fill in the aarch32-specific parts of the binfo struct.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/raspi.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index a37881433c..1ac0737149 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -82,10 +82,19 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>      binfo.board_id = raspi_boardid[version];
>      binfo.ram_size = ram_size;
>      binfo.nb_cpus = smp_cpus;
> -    binfo.board_setup_addr = BOARDSETUP_ADDR;
> -    binfo.write_board_setup = write_board_setup;
> -    binfo.secure_board_setup = true;
> -    binfo.secure_boot = true;
> +
> +    if (version <= 2) {
> +        /* The rpi1 and 2 require some custom setup code to run in Secure
> +         * mode before booting a kernel (to set up the SMC vectors so
> +         * that we get a no-op SMC; this is used by Linux to call the
> +         * firmware for some cache maintenance operations.
> +         * The rpi3 doesn't need this.
> +         */

I was expecting a much complicated fix... neat.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +        binfo.board_setup_addr = BOARDSETUP_ADDR;
> +        binfo.write_board_setup = write_board_setup;
> +        binfo.secure_board_setup = true;
> +        binfo.secure_boot = true;
> +    }
>  
>      /* Pi2 and Pi3 requires SMP setup */
>      if (version >= 2) {
> 

Re: [Qemu-devel] [PATCH 2/9] hw/arm/boot: assert that secure_boot and secure_board_setup are false for AArch64

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> Add some assertions that if we're about to boot an AArch64 kernel,
> the board code has not mistakenly set either secure_boot or
> secure_board_setup. It doesn't make sense to set secure_boot,
> because all AArch64 kernels must be booted in non-secure mode.
> 
> It might in theory make sense to set secure_board_setup, but
> we don't currently support that, because only the AArch32
> bootloader[] code calls this hook; bootloader_aarch64[] does not.
> Since we don't have a current need for this functionality, just
> assert that we don't try to use it. If it's needed we'll add
> it later.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/boot.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 196c7fb242..e21a92f972 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -720,6 +720,13 @@ static void do_cpu_reset(void *opaque)
>                      } else {
>                          env->pstate = PSTATE_MODE_EL1h;
>                      }
> +                    /* AArch64 kernels never boot in secure mode */
> +                    assert(!info->secure_boot);
> +                    /* This hook is only supported for AArch32 currently:
> +                     * bootloader_aarch64[] will not call the hook, and
> +                     * the code above has already dropped us into EL2 or EL1.
> +                     */
> +                    assert(!info->secure_board_setup);
>                  }
>  
>                  /* Set to non-secure if not a secure boot */
> 

Re: [Qemu-devel] [PATCH 7/9] hw/arm/bcm2836: Use correct affinity values for BCM2837

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> The BCM2837 sets the Aff1 field of the MPIDR affinity values for the
> CPUs to 0, whereas the BCM2836 uses 0xf. Set this correctly, as it
> is required for Linux to boot.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I don't have the datasheet for this SoC,
but checked Linux dts ("ARM CPUs bindings -> cpus and cpu node bindings
definition" from doc/Documentation/devicetree/bindings/arm/cpus.txt,
then arch/arm/boot/dts/bcm283[67].dtsi). This might be useful to add in
the commit description.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/bcm2836.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 07d2705f96..7140257c98 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,14 +25,17 @@
>  
>  struct BCM283XInfo {
>      const char *name;
> +    int clusterid;
>  };
>  
>  static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
> +        .clusterid = 0xf,
>      },
>      {
>          .name = TYPE_BCM2837,
> +        .clusterid = 0x0,
>      },
>  };
>  
> @@ -58,6 +61,8 @@ static void bcm2836_init(Object *obj)
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
>      BCM283XState *s = BCM283X(dev);
> +    BCM283XClass *bc = BCM283X_GET_CLASS(dev);
> +    const BCM283XInfo *info = bc->info;
>      Object *obj;
>      Error *err = NULL;
>      int n;
> @@ -119,7 +124,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>          /* Mirror bcm2836, which has clusterid set to 0xf
>           * TODO: this should be converted to a property of ARM_CPU
>           */
> -        s->cpus[n].mp_affinity = 0xF00 | n;
> +        s->cpus[n].mp_affinity = (info->clusterid << 8) | n;
>  
>          /* set periphbase/CBAR value for CPU-local registers */
>          object_property_set_int(OBJECT(&s->cpus[n]),
> 

Re: [Qemu-devel] [PATCH 9/9] hw/arm/raspi: Provide spin-loop code for AArch64 CPUs

[Philippe Mathieu-Daudé]

On 03/13/2018 04:34 PM, Peter Maydell wrote:
> The raspi3 has AArch64 CPUs, which means that our smpboot
> code for keeping the secondary CPUs in a pen needs to have
> a version for A64 as well as A32. Without this, the
> secondary CPUs go into an infinite loop of taking undefined
> instruction exceptions.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/raspi.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index ae15997669..06f1e08ca9 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -27,6 +27,7 @@
>  #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
>  #define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */
>  #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */
> +#define SPINTABLE_ADDR  0xd8 /* Pi 3 bootloader spintable */
>  
>  /* Table of Linux board IDs for different Pi versions */
>  static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
> @@ -63,6 +64,40 @@ static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
>                         info->smp_loader_start);
>  }
>  
> +static void write_smpboot64(ARMCPU *cpu, const struct arm_boot_info *info)
> +{
> +    /* Unlike the AArch32 version we don't need to call the board setup hook.
> +     * The mechanism for doing the spin-table is also entirely different.
> +     * We must have four 64-bit fields at absolute addresses
> +     * 0xd8, 0xe0, 0xe8, 0xf0 in RAM, which are the flag variables for
> +     * our CPUs, and which we must ensure are zero initialized before
> +     * the primary CPU goes into the kernel. We put these variables inside
> +     * a rom blob, so that the reset for ROM contents zeroes them for us.
> +     */

Checking with Linux doc/Documentation/arm64/booting.txt and
arch/arm/boot/dts/bcm2837.dtsi.

> +    static const uint32_t smpboot[] = {
> +        0xd2801b05, /*        mov     x5, 0xd8 */

x5 = &spintable;

> +        0xd53800a6, /*        mrs     x6, mpidr_el1 */
> +        0x924004c6, /*        and     x6, x6, #0x3 */

x6 = clusterid;

> +        0xd503205f, /* spin:  wfe */
> +        0xf86678a4, /*        ldr     x4, [x5,x6,lsl #3] */

x4 = &spintable[clusterid];

> +        0xb4ffffc4, /*        cbz     x4, spin */

loop if !x4 ...

> +        0xd2800000, /*        mov     x0, #0x0 */
> +        0xd2800001, /*        mov     x1, #0x0 */
> +        0xd2800002, /*        mov     x2, #0x0 */
> +        0xd2800003, /*        mov     x3, #0x0 */
> +        0xd61f0080, /*        br      x4 */

... else jump()

> +    };
> +
> +    static const uint64_t spintables[] = {
> +        0, 0, 0, 0

the "naturally-aligned 64-bit zero-initalised memory"

> +    };
> +
> +    rom_add_blob_fixed("raspi_smpboot", smpboot, sizeof(smpboot),
> +                       info->smp_loader_start);
> +    rom_add_blob_fixed("raspi_spintables", spintables, sizeof(spintables),
> +                       SPINTABLE_ADDR);

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +}
> +
>  static void write_board_setup(ARMCPU *cpu, const struct arm_boot_info *info)
>  {
>      arm_write_secure_board_setup_dummy_smc(cpu, info, MVBAR_ADDR);
> @@ -99,7 +134,11 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>      /* Pi2 and Pi3 requires SMP setup */
>      if (version >= 2) {
>          binfo.smp_loader_start = SMPBOOT_ADDR;
> -        binfo.write_secondary_boot = write_smpboot;
> +        if (version == 2) {
> +            binfo.write_secondary_boot = write_smpboot;
> +        } else {
> +            binfo.write_secondary_boot = write_smpboot64;
> +        }
>          binfo.secondary_cpu_reset_hook = reset_secondary;
>      }
>  
> 

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>> From: Qemu-devel <qemu-devel-
>> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
>> Maydell
>> Sent: Tuesday, 13 March 2018 08:35
>>
>> Now we have separate types for BCM2386 and BCM2387, we might as well
>> just hard-code the CPU type they use rather than having it passed
>> through as an object property. This then lets us put the initialization
>> of the CPU object in init rather than realize.

> What about the default_cpu_type field of MachineClass set in
> raspi[23]_machine_init? That seems irrelevant now...

Igor, can you help with this question? For a board that always
has one CPU type (because the real hardware only ever has
one SoC, and that SoC QOM object hard codes the CPU type)
does it still need to set the mc->default_cpu_type field in
its MachineClass, or does that become pointless ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 13 March 2018 at 23:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 03/13/2018 06:09 PM, Peter Maydell wrote:
>> On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
>>> At some point I remember seeing a patch to change this to cortex-a7. Is there a reason we didn't make that change?
>>>
>>> (Background: the real Pi2 has an A7. When I first implemented the machine model there was no A7 emulation in QEMU, so I used the A15 which was the closest equivalent.)
>>
>> Yeah, we should do that. I'd forgotten about that, I think
>> things just got lost in the shuffle of having several
>> patchsets that tried to change the same things at once.
>>
>> I guess the simplest thing is to add a patch at the end of
>> the series that fixes the cpu type for bcm2836.
>
> Peter, here is the patch Andrew remembered (maybe can be applied at the
> end):
> http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg04286.html

Thanks for finding that. It doesn't apply after this refactoring,
but I'll send out a patch which does the equivalent thing in the
new code. In the meantime I'm going to apply this patchset to
target-arm.next since I'm going to send out a pullreq with bugfixes
for rc0 today.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 13 March 2018 at 15:34, Peter Maydell <peter.maydell@linaro.org> wrote:
> Now we have separate types for BCM2386 and BCM2387, we might as well
> just hard-code the CPU type they use rather than having it passed
> through as an object property. This then lets us put the initialization
> of the CPU object in init rather than realize.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/bcm2836.c | 22 +++++++++++++---------
>  hw/arm/raspi.c   |  2 --
>  2 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 7140257c98..12f75b55a7 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,16 +25,19 @@
>
>  struct BCM283XInfo {
>      const char *name;
> +    const char *cpu_type;
>      int clusterid;
>  };
>
>  static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
>          .clusterid = 0xf,
>      },
>      {
>          .name = TYPE_BCM2837,
> +        .cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
>          .clusterid = 0x0,
>      },
>  };

With this change we need to also add
#ifdef TARGET_AARCH64
#endif

around the entry in the array for bcm2837. Otherwise the
device-introspection tests in 'make check' will fail trying
to create the bcm2837 in qemu-system-arm, because there there
is no cortex-a53 device.

I'll just squash that into this patch...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Igor Mammedov]

On Thu, 15 Mar 2018 17:13:03 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 13 March 2018 at 16:55, Andrew Baumann <Andrew.Baumann@microsoft.com> wrote:
> >> From: Qemu-devel <qemu-devel-
> >> bounces+andrew.baumann=microsoft.com@nongnu.org> On Behalf Of Peter
> >> Maydell
> >> Sent: Tuesday, 13 March 2018 08:35
> >>
> >> Now we have separate types for BCM2386 and BCM2387, we might as well
> >> just hard-code the CPU type they use rather than having it passed
> >> through as an object property. This then lets us put the initialization
> >> of the CPU object in init rather than realize.
> 
> > What about the default_cpu_type field of MachineClass set in
> > raspi[23]_machine_init? That seems irrelevant now...
> 
> Igor, can you help with this question? For a board that always
> has one CPU type (because the real hardware only ever has
> one SoC, and that SoC QOM object hard codes the CPU type)
> does it still need to set the mc->default_cpu_type field in
> its MachineClass, or does that become pointless ?

Since board ignores whatever were specified on '-cpu' and 
there aren't any checks in board code for current_machine->cpu_type,
removing mc->default_cpu_type won't really affect anything.

With current code in vl.c and with default_cpu_type set, user will
get error if he specified non existing cpu_model with -cpu.
If default_cpu_type were NULL, -cpu is completely ignored.

But once http://patchwork.ozlabs.org/patch/870297/ is applied
it will error out the same way as if default_cpu_type were
set since vl.c won't rely on it anymore for parsing cpu_model.

So in short it's ok to remove mc->default_cpu_type here.

> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Peter Maydell]

On 19 March 2018 at 14:40, Igor Mammedov <imammedo@redhat.com> wrote:
> On Thu, 15 Mar 2018 17:13:03 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> Igor, can you help with this question? For a board that always
>> has one CPU type (because the real hardware only ever has
>> one SoC, and that SoC QOM object hard codes the CPU type)
>> does it still need to set the mc->default_cpu_type field in
>> its MachineClass, or does that become pointless ?
>
> Since board ignores whatever were specified on '-cpu' and
> there aren't any checks in board code for current_machine->cpu_type,
> removing mc->default_cpu_type won't really affect anything.
>
> With current code in vl.c and with default_cpu_type set, user will
> get error if he specified non existing cpu_model with -cpu.
> If default_cpu_type were NULL, -cpu is completely ignored.
>
> But once http://patchwork.ozlabs.org/patch/870297/ is applied
> it will error out the same way as if default_cpu_type were
> set since vl.c won't rely on it anymore for parsing cpu_model.
>
> So in short it's ok to remove mc->default_cpu_type here.

Thanks for the explanation. I think what I'll do is put these
in as-is, and then have a followup patch that cleans up the
default_cpu_types from raspi.c once that patch 870297 is in.

-- PMM

Re: [Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Thomas Huth]

On 13.03.2018 16:34, Peter Maydell wrote:
> The bcm2837 is pretty similar to the bcm2836, but it does have
> some differences. Notably, the MPIDR affinity aff1 values it
> sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
> uses, and if this is wrong Linux will not boot.
> 
> Rather than trying to have one device with properties that
> configure it differently for the two cases, create two
> separate QOM devices for the two SoCs. We use the same approach
> as hw/arm/aspeed_soc.c and share code and have a data table
> that might differ per-SoC. For the moment the two types don't
> actually have different behaviour.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/arm/bcm2836.h | 19 +++++++++++++++++++
>  hw/arm/bcm2836.c         | 37 ++++++++++++++++++++++++++++++++-----
>  hw/arm/raspi.c           |  3 ++-
>  3 files changed, 53 insertions(+), 6 deletions(-)

 Hi Peter,

if I've bisected it right, this patch introduced a new way to crash QEMU:

$ aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest \
    -nographic
QEMU 2.11.50 monitor - type 'help' for more information
(qemu) device_add bcm2837
Device 'bcm2837' can not be hotplugged on this machine
(qemu) device_add corgi-ssp
Segmentation fault (core dumped)

Not sure why this is happening, though, I can't see anything obvious
wrong with your patch.

 Thomas



> diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
> index 9a10a76631..93248399ba 100644
> --- a/include/hw/arm/bcm2836.h
> +++ b/include/hw/arm/bcm2836.h
> @@ -20,6 +20,13 @@
>  
>  #define BCM283X_NCPUS 4
>  
> +/* These type names are for specific SoCs; other than instantiating
> + * them, code using these devices should always handle them via the
> + * BCM283x base class, so they have no BCM2836(obj) etc macros.
> + */
> +#define TYPE_BCM2836 "bcm2836"
> +#define TYPE_BCM2837 "bcm2837"
> +
>  typedef struct BCM283XState {
>      /*< private >*/
>      DeviceState parent_obj;
> @@ -33,4 +40,16 @@ typedef struct BCM283XState {
>      BCM2835PeripheralState peripherals;
>  } BCM283XState;
>  
> +typedef struct BCM283XInfo BCM283XInfo;
> +
> +typedef struct BCM283XClass {
> +    DeviceClass parent_class;
> +    const BCM283XInfo *info;
> +} BCM283XClass;
> +
> +#define BCM283X_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
> +#define BCM283X_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
> +
>  #endif /* BCM2836_H */
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 1d1908654b..07d2705f96 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -23,6 +23,19 @@
>  /* "QA7" (Pi2) interrupt controller and mailboxes etc. */
>  #define BCM2836_CONTROL_BASE    0x40000000
>  
> +struct BCM283XInfo {
> +    const char *name;
> +};
> +
> +static const BCM283XInfo bcm283x_socs[] = {
> +    {
> +        .name = TYPE_BCM2836,
> +    },
> +    {
> +        .name = TYPE_BCM2837,
> +    },
> +};
> +
>  static void bcm2836_init(Object *obj)
>  {
>      BCM283XState *s = BCM283X(obj);
> @@ -156,25 +169,39 @@ static Property bcm2836_props[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> -static void bcm2836_class_init(ObjectClass *oc, void *data)
> +static void bcm283x_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> +    BCM283XClass *bc = BCM283X_CLASS(oc);
>  
> -    dc->props = bcm2836_props;
> +    bc->info = data;
>      dc->realize = bcm2836_realize;
> +    dc->props = bcm2836_props;
>  }
>  
> -static const TypeInfo bcm2836_type_info = {
> +static const TypeInfo bcm283x_type_info = {
>      .name = TYPE_BCM283X,
>      .parent = TYPE_DEVICE,
>      .instance_size = sizeof(BCM283XState),
>      .instance_init = bcm2836_init,
> -    .class_init = bcm2836_class_init,
> +    .class_size = sizeof(BCM283XClass),
> +    .abstract = true,
>  };
>  
>  static void bcm2836_register_types(void)
>  {
> -    type_register_static(&bcm2836_type_info);
> +    int i;
> +
> +    type_register_static(&bcm283x_type_info);
> +    for (i = 0; i < ARRAY_SIZE(bcm283x_socs); i++) {
> +        TypeInfo ti = {
> +            .name = bcm283x_socs[i].name,
> +            .parent = TYPE_BCM283X,
> +            .class_init = bcm283x_class_init,
> +            .class_data = (void *) &bcm283x_socs[i],
> +        };
> +        type_register(&ti);
> +    }
>  }
>  
>  type_init(bcm2836_register_types)
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 58c6e80a17..f588720138 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -136,7 +136,8 @@ static void raspi_init(MachineState *machine, int version)
>      BusState *bus;
>      DeviceState *carddev;
>  
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM283X);
> +    object_initialize(&s->soc, sizeof(s->soc),
> +                      version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
>  
>