On Tue, Jun 10, 2014 at 11:32 AM, Alistair Francis
<alistair.fran...@xilinx.com> wrote:
> This patch adds the Cortex-A9 ARM CPU to the A9MPCore. It
> first does a check to make sure no other CPUs exist and if
> they do the Cortex-A9 won't be added. This is implemented to
> maintain compatibility and can be removed once all machines
> have been updated
>
> This patch also allows the midr and reset-property to be set
>
> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
> ---
> There comments in the code explaining the reason that the CPU
> is initiated in the realize function. This is because it relies
> on the num_cpu property, which isn't yet set in the initfn
> Is this an acceptable compromise?
>
> hw/cpu/a9mpcore.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/cpu/a9mpcore.h | 4 ++++
> 2 files changed, 47 insertions(+), 0 deletions(-)
>
> diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> index c09358c..1159044 100644
> --- a/hw/cpu/a9mpcore.c
> +++ b/hw/cpu/a9mpcore.c
> @@ -21,6 +21,12 @@ static void a9mp_priv_initfn(Object *obj)
> {
> A9MPPrivState *s = A9MPCORE_PRIV(obj);
>
> + /* Ideally would init the CPUs here, but the num_cpu property has not
> been
> + * set yet. So that only works if assuming a single CPU
> + * object_initialize(&s->cpu, sizeof(s->cpu), "cortex-a9-" TYPE_ARM_CPU);
> + * object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> + */
> +
So you could add an integer property listener to init them earlier (or
even do dynamic extending/freeing or the allocated CPUs). I'm not sure
exactly what we are really supposed to do though, when the number of
child object depends on a prop like this? Andreas?
> memory_region_init(&s->container, obj, "a9mp-priv-container", 0x2000);
> sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
>
> @@ -50,6 +56,40 @@ static void a9mp_priv_realize(DeviceState *dev, Error
> **errp)
> Error *err = NULL;
> int i;
>
> + /* Just a temporary measure to not break machines that init the CPU
> + * seperatly */
"separately"
> + if (!first_cpu) {
> + s->cpu = g_malloc(sizeof(ARMCPU) * s->num_cpu);
g_new should be use to allocate arrays.
> + for (i = 0; i < s->num_cpu; i++) {
> + object_initialize((s->cpu + i), sizeof(*(s->cpu + i)),
&s->cpu[i] is more common and easier to read.
sizeof(*s->cpu) is fine.
> + "cortex-a9-" TYPE_ARM_CPU);
Use cpu_class_by_name logic like in some of the boards, rather than
the string concatenation. The specifics of the concatenation system is
(supposed to be) private to target-arm code.
> +
> + if (s->midr) {
> + object_property_set_int(OBJECT((s->cpu + i)), s->midr,
> + "midr", &err);
> + if (err) {
> + error_propagate(errp, err);
> + exit(1);
> + }
> + }
> + if (s->reset_cbar) {
> + object_property_set_int(OBJECT((s->cpu + i)), s->reset_cbar,
> + "reset-cbar", &err);
> + if (err) {
> + error_propagate(errp, err);
> + exit(1);
> + }
> + }
> + object_property_set_bool(OBJECT((s->cpu + i)), true,
> + "realized", &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> + }
> + g_free(s->cpu);
Why free the just-initialized CPUs?
> + }
> +
> scudev = DEVICE(&s->scu);
> qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
> object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
> @@ -152,6 +192,9 @@ static Property a9mp_priv_properties[] = {
> * Other boards may differ and should set this property appropriately.
> */
> DEFINE_PROP_UINT32("num-irq", A9MPPrivState, num_irq, 96),
> + /* Properties for the A9 CPU */
> + DEFINE_PROP_UINT32("midr", A9MPPrivState, midr, 0),
> + DEFINE_PROP_UINT64("reset-cbar", A9MPPrivState, reset_cbar, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
> index 5d67ca2..8e395a4 100644
> --- a/include/hw/cpu/a9mpcore.h
> +++ b/include/hw/cpu/a9mpcore.h
> @@ -29,6 +29,10 @@ typedef struct A9MPPrivState {
> MemoryRegion container;
> uint32_t num_irq;
>
> + ARMCPU *cpu;
> + uint32_t midr;
I'd preface this as "cpu_midr".
> + uint64_t reset_cbar;
MPCores refer to this as PERIPHBASE in their documentation.
Regards,
Peter
> +
> A9SCUState scu;
> GICState gic;
> A9GTimerState gtimer;
> --
> 1.7.1
>
>
