Re: [Qemu-devel] [PATCH target-arm v1 8/9] arm: xlnx-zynqmp: Preface CPU variables with "A"

[Peter Crosthwaite]

On Tue, Jun 2, 2015 at 4:57 PM, Alistair Francis
<address@hidden> wrote:
> On Tue, Jun 2, 2015 at 4:04 AM, Peter Crosthwaite
> <address@hidden> wrote:
>> The CPUs currently supported by zynqmp are the APU (application
>> processing unit) CPUs. There are other CPUs in Zynqmp so unqualified
>> "cpus" in ambiguous. Preface the variables with "A" accordingly, to
>> prepare support adding the RPU (realtime processing unit) processors.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>  hw/arm/xlnx-ep108.c          |  2 +-
>>  hw/arm/xlnx-zynqmp.c         | 24 ++++++++++++------------
>>  include/hw/arm/xlnx-zynqmp.h |  4 ++--
>>  3 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>> index b924f5e..1893b9f 100644
>> --- a/hw/arm/xlnx-ep108.c
>> +++ b/hw/arm/xlnx-ep108.c
>> @@ -65,7 +65,7 @@ static void xlnx_ep108_init(MachineState *machine)
>>      xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
>>      xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;
>>      xlnx_ep108_binfo.loader_start = 0;
>> -    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
>> +    arm_load_kernel(&s->soc.acpu[0], &xlnx_ep108_binfo);
>>  }
>>
>
> Hey Peter,
>
> Why is this acpu instead of apu? APU follows the standard ZynqMP naming
> conventions, while Application Central Processing Unit (ACPU) doesn't really
> make sense.
>

So "apu" (or "rpu") doesn't work either, as each "processing unit" can
contain more than just CPUs. E.G. The GIC should actually have this
"apu" preface as well. I was trying to avoid text bloat with the short
form, but I think the correct answer is going to be:

"apu_cpu"

The PU in each does mean the same thing though :|

Regards,
Peter

> Thanks,
>
> Alistair
>
>>  static QEMUMachine xlnx_ep108_machine = {
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index 6b01965..6faa578 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -64,10 +64,10 @@ static void xlnx_zynqmp_init(Object *obj)
>>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>>      int i;
>>
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
>> -        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
>> +    for (i = 0; i < XLNX_ZYNQMP_NUM_ACPUS; i++) {
>> +        object_initialize(&s->acpu[i], sizeof(s->acpu[i]),
>>                            "cortex-a53-" TYPE_ARM_CPU);
>> -        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
>> +        object_property_add_child(obj, "acpu[*]", OBJECT(&s->acpu[i]),
>>                                    &error_abort);
>>      }
>>
>> @@ -95,7 +95,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
>> **errp)
>>
>>      qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
>>      qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
>> -    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_CPUS);
>> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_ACPUS);
>>      object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
>>      if (err) {
>>          error_propagate((errp), (err));
>> @@ -121,38 +121,38 @@ static void xlnx_zynqmp_realize(DeviceState *dev, 
>> Error **errp)
>>          }
>>      }
>>
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
>> +    for (i = 0; i < XLNX_ZYNQMP_NUM_ACPUS; i++) {
>>          qemu_irq irq;
>>
>> -        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
>> +        object_property_set_int(OBJECT(&s->acpu[i]), QEMU_PSCI_CONDUIT_SMC,
>>                                  "psci-conduit", &error_abort);
>>          if (i > 0) {
>>              /* Secondary CPUs start in PSCI powered-down state */
>> -            object_property_set_bool(OBJECT(&s->cpu[i]), true,
>> +            object_property_set_bool(OBJECT(&s->acpu[i]), true,
>>                                       "start-powered-off", &error_abort);
>>          }
>>
>> -        object_property_set_int(OBJECT(&s->cpu[i]), GIC_BASE_ADDR,
>> +        object_property_set_int(OBJECT(&s->acpu[i]), GIC_BASE_ADDR,
>>                                  "reset-cbar", &err);
>>          if (err) {
>>              error_propagate((errp), (err));
>>              return;
>>          }
>>
>> -        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", 
>> &err);
>> +        object_property_set_bool(OBJECT(&s->acpu[i]), true, "realized", 
>> &err);
>>          if (err) {
>>              error_propagate((errp), (err));
>>              return;
>>          }
>>
>>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
>> -                           qdev_get_gpio_in(DEVICE(&s->cpu[i]), 
>> ARM_CPU_IRQ));
>> +                           qdev_get_gpio_in(DEVICE(&s->acpu[i]), 
>> ARM_CPU_IRQ));
>>          irq = qdev_get_gpio_in(DEVICE(&s->gic),
>>                                 arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
>> -        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
>> +        qdev_connect_gpio_out(DEVICE(&s->acpu[i]), 0, irq);
>>          irq = qdev_get_gpio_in(DEVICE(&s->gic),
>>                                 arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
>> -        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
>> +        qdev_connect_gpio_out(DEVICE(&s->acpu[i]), 1, irq);
>>      }
>>
>>      for (i = 0; i < GIC_NUM_SPI_INTR; i++) {
>> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
>> index 79c2b0b..bb67ef6 100644
>> --- a/include/hw/arm/xlnx-zynqmp.h
>> +++ b/include/hw/arm/xlnx-zynqmp.h
>> @@ -27,7 +27,7 @@
>>  #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
>>                                         TYPE_XLNX_ZYNQMP)
>>
>> -#define XLNX_ZYNQMP_NUM_CPUS 4
>> +#define XLNX_ZYNQMP_NUM_ACPUS 4
>>  #define XLNX_ZYNQMP_NUM_GEMS 4
>>  #define XLNX_ZYNQMP_NUM_UARTS 2
>>
>> @@ -47,7 +47,7 @@ typedef struct XlnxZynqMPState {
>>      DeviceState parent_obj;
>>
>>      /*< public >*/
>> -    ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
>> +    ARMCPU acpu[XLNX_ZYNQMP_NUM_ACPUS];
>>      GICState gic;
>>      MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
>>      CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
>> --
>> 2.4.2.3.g2ffcb72
>>
>>
>