[v1,2/2] zynq: Update Zynq to init the CPU in the a9mpcore device

This patch removes the initialisation of the ARM Cortex-A9
in Zynq and instead allows the a9mpcore device to init the
CPU. This also updates components that rely on the CPU
and GIC, as they are now initialised in a slightly different
way

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
Changes since RFC:
 - Rebase

 hw/arm/xilinx_zynq.c |   70 +++++++++++++++++++------------------------------
 1 files changed, 27 insertions(+), 43 deletions(-)

On Wed, Jan 21, 2015 at 5:06 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> This patch removes the initialisation of the ARM Cortex-A9
> in Zynq and instead allows the a9mpcore device to init the
> CPU. This also updates components that rely on the CPU
> and GIC, as they are now initialised in a slightly different
> way
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> Changes since RFC:
>  - Rebase
>
>  hw/arm/xilinx_zynq.c |   70 +++++++++++++++++++------------------------------
>  1 files changed, 27 insertions(+), 43 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 06e6e24..88d4473 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -26,6 +26,7 @@
>  #include "hw/loader.h"
>  #include "hw/ssi.h"
>  #include "qemu/error-report.h"
> +#include "hw/cpu/a9mpcore.h"
>
>  #define NUM_SPI_FLASHES 4
>  #define NUM_QSPI_FLASHES 2
> @@ -104,12 +105,10 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>  static void zynq_init(MachineState *machine)
>  {
>      ram_addr_t ram_size = machine->ram_size;
> -    const char *cpu_model = machine->cpu_model;
>      const char *kernel_filename = machine->kernel_filename;
>      const char *kernel_cmdline = machine->kernel_cmdline;
>      const char *initrd_filename = machine->initrd_filename;
> -    ObjectClass *cpu_oc;
> -    ARMCPU *cpu;
> +    A9MPPrivState *mpcore;
>      MemoryRegion *address_space_mem = get_system_memory();
>      MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
>      MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
> @@ -119,39 +118,8 @@ static void zynq_init(MachineState *machine)
>      Error *err = NULL;
>      int n;
>
> -    if (!cpu_model) {
> -        cpu_model = "cortex-a9";
> -    }
> -    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
> -
> -    cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
> -
> -    /* By default A9 CPUs have EL3 enabled.  This board does not
> -     * currently support EL3 so the CPU EL3 property is disabled before
> -     * realization.
> -     */
> -    if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
> -        object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
> -        if (err) {
> -            error_report("%s", error_get_pretty(err));
> -            exit(1);
> -        }
> -    }
> -
> -    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
> -    if (err) {
> -        error_report("%s", error_get_pretty(err));
> -        exit(1);
> -    }
> -
> -    object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
> -    if (err) {
> -        error_report("%s", error_get_pretty(err));
> -        exit(1);
> -    }
> -    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> -    if (err) {
> -        error_report("%s", error_get_pretty(err));
> +    if (machine->cpu_model) {
> +        error_report("Zynq does not support CPU model override!\n");
>          exit(1);
>      }
>
> @@ -186,16 +154,32 @@ static void zynq_init(MachineState *machine)
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
>
> -    dev = qdev_create(NULL, "a9mpcore_priv");
> -    qdev_prop_set_uint32(dev, "num-cpu", 1);
> -    qdev_init_nofail(dev);
> -    busdev = SYS_BUS_DEVICE(dev);
> +    mpcore = A9MPCORE_PRIV(object_new("a9mpcore_priv"));
> +    /* By default A9 CPUs have EL3 enabled.  This board does not
> +     * currently support EL3 so the CPU EL3 property is disabled before
> +     * realization.
> +     */
> +    if (object_property_find(OBJECT(mpcore), "has_el3", NULL)) {

With the defeaturing of cpu model override, I don't think this check
is needed anymore.

> +        object_property_set_bool(OBJECT(mpcore), false, "has_el3", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
> +            exit(1);
> +        }
> +    }
> +    qdev_prop_set_uint32(DEVICE(mpcore), "num-cpu", 1);

Should this be before the el3 check. Otherwise will your aliases exist?

Regards,
Peter

> +    qdev_prop_set_uint32(DEVICE(mpcore), "midr", ZYNQ_BOARD_MIDR);
> +    qdev_prop_set_uint64(DEVICE(mpcore), "reset-cbar", MPCORE_PERIPHBASE);
> +    object_property_set_bool(OBJECT(mpcore), true, "realized", &err);
> +    if (err != NULL) {
> +        error_report("Couldn't realize the Zynq A9MPCore: %s",
> +                     error_get_pretty(err));
> +        exit(1);
> +    }
> +    busdev = SYS_BUS_DEVICE(DEVICE(mpcore));
>      sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
> -    sysbus_connect_irq(busdev, 0,
> -                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
>
>      for (n = 0; n < 64; n++) {
> -        pic[n] = qdev_get_gpio_in(dev, n);
> +        pic[n] = qdev_get_gpio_in(DEVICE(mpcore), n);
>      }
>
>      zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET], false);
> --
> 1.7.1
>
>

On Tue, Feb 10, 2015 at 4:45 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Wed, Jan 21, 2015 at 5:06 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> This patch removes the initialisation of the ARM Cortex-A9
>> in Zynq and instead allows the a9mpcore device to init the
>> CPU. This also updates components that rely on the CPU
>> and GIC, as they are now initialised in a slightly different
>> way
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>> Changes since RFC:
>>  - Rebase
>>
>>  hw/arm/xilinx_zynq.c |   70 +++++++++++++++++++------------------------------
>>  1 files changed, 27 insertions(+), 43 deletions(-)
>>
>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> index 06e6e24..88d4473 100644
>> --- a/hw/arm/xilinx_zynq.c
>> +++ b/hw/arm/xilinx_zynq.c
>> @@ -26,6 +26,7 @@
>>  #include "hw/loader.h"
>>  #include "hw/ssi.h"
>>  #include "qemu/error-report.h"
>> +#include "hw/cpu/a9mpcore.h"
>>
>>  #define NUM_SPI_FLASHES 4
>>  #define NUM_QSPI_FLASHES 2
>> @@ -104,12 +105,10 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>>  static void zynq_init(MachineState *machine)
>>  {
>>      ram_addr_t ram_size = machine->ram_size;
>> -    const char *cpu_model = machine->cpu_model;
>>      const char *kernel_filename = machine->kernel_filename;
>>      const char *kernel_cmdline = machine->kernel_cmdline;
>>      const char *initrd_filename = machine->initrd_filename;
>> -    ObjectClass *cpu_oc;
>> -    ARMCPU *cpu;
>> +    A9MPPrivState *mpcore;
>>      MemoryRegion *address_space_mem = get_system_memory();
>>      MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
>>      MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
>> @@ -119,39 +118,8 @@ static void zynq_init(MachineState *machine)
>>      Error *err = NULL;
>>      int n;
>>
>> -    if (!cpu_model) {
>> -        cpu_model = "cortex-a9";
>> -    }
>> -    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
>> -
>> -    cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
>> -
>> -    /* By default A9 CPUs have EL3 enabled.  This board does not
>> -     * currently support EL3 so the CPU EL3 property is disabled before
>> -     * realization.
>> -     */
>> -    if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
>> -        object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
>> -        if (err) {
>> -            error_report("%s", error_get_pretty(err));
>> -            exit(1);
>> -        }
>> -    }
>> -
>> -    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
>> -    if (err) {
>> -        error_report("%s", error_get_pretty(err));
>> -        exit(1);
>> -    }
>> -
>> -    object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
>> -    if (err) {
>> -        error_report("%s", error_get_pretty(err));
>> -        exit(1);
>> -    }
>> -    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
>> -    if (err) {
>> -        error_report("%s", error_get_pretty(err));
>> +    if (machine->cpu_model) {
>> +        error_report("Zynq does not support CPU model override!\n");
>>          exit(1);
>>      }
>>
>> @@ -186,16 +154,32 @@ static void zynq_init(MachineState *machine)
>>      qdev_init_nofail(dev);
>>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
>>
>> -    dev = qdev_create(NULL, "a9mpcore_priv");
>> -    qdev_prop_set_uint32(dev, "num-cpu", 1);
>> -    qdev_init_nofail(dev);
>> -    busdev = SYS_BUS_DEVICE(dev);
>> +    mpcore = A9MPCORE_PRIV(object_new("a9mpcore_priv"));
>> +    /* By default A9 CPUs have EL3 enabled.  This board does not
>> +     * currently support EL3 so the CPU EL3 property is disabled before
>> +     * realization.
>> +     */
>> +    if (object_property_find(OBJECT(mpcore), "has_el3", NULL)) {
>
> With the defeaturing of cpu model override, I don't think this check
> is needed anymore.

Ok, I will remove it then

>
>> +        object_property_set_bool(OBJECT(mpcore), false, "has_el3", &err);
>> +        if (err) {
>> +            error_report("%s", error_get_pretty(err));
>> +            exit(1);
>> +        }
>> +    }
>> +    qdev_prop_set_uint32(DEVICE(mpcore), "num-cpu", 1);
>
> Should this be before the el3 check. Otherwise will your aliases exist?

I don't think it matters, but I have re-ordered the code to look nicer.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> +    qdev_prop_set_uint32(DEVICE(mpcore), "midr", ZYNQ_BOARD_MIDR);
>> +    qdev_prop_set_uint64(DEVICE(mpcore), "reset-cbar", MPCORE_PERIPHBASE);
>> +    object_property_set_bool(OBJECT(mpcore), true, "realized", &err);
>> +    if (err != NULL) {
>> +        error_report("Couldn't realize the Zynq A9MPCore: %s",
>> +                     error_get_pretty(err));
>> +        exit(1);
>> +    }
>> +    busdev = SYS_BUS_DEVICE(DEVICE(mpcore));
>>      sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
>> -    sysbus_connect_irq(busdev, 0,
>> -                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
>>
>>      for (n = 0; n < 64; n++) {
>> -        pic[n] = qdev_get_gpio_in(dev, n);
>> +        pic[n] = qdev_get_gpio_in(DEVICE(mpcore), n);
>>      }
>>
>>      zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET], false);
>> --
>> 1.7.1
>>
>>
>