[Qemu-devel] [PATCH v1 0/2] Add a property to set the ARM CPU core count

[Qemu-devel] [PATCH v1 0/2] Add a property to set the ARM CPU core count

[Alistair Francis]

Add an ARM CPU property which allows us to set the ARM CPU core count.



Alistair Francis (2):
  target/arm: Add a cluster size property
  hw/arm: Set the core count for Xilinx's ZynqMP

 target/arm/cpu.h     |  5 +++++
 hw/arm/xlnx-zynqmp.c |  2 ++
 target/arm/cpu.c     |  1 +
 target/arm/cpu64.c   | 11 +++++++++--
 4 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.14.1

[Qemu-devel] [PATCH v1 1/2] target/arm: Add a cluster size property

[Alistair Francis]

The cortex A53 TRM specifices that bits 24 and 25 of the L2CTLR register
specify the number of cores present and not the number of processors. To
report this correctly on machines with multiple CPU clusters (ARM's
big.LITTLE or Xilinx's ZynqMP) we need to allow the machine to overwrite
this value. To do this let's add an optional property.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 target/arm/cpu.h   |  5 +++++
 target/arm/cpu.c   |  1 +
 target/arm/cpu64.c | 11 +++++++++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8dd6b788df..3fa8fdad21 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -745,6 +745,11 @@ struct ARMCPU {
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
+    /* Specify the number of cores in this CPU cluster. Used for the L2CTLR
+     * register.
+     */
+    int32_t core_count;
+
     /* The instance init functions for implementation-specific subclasses
      * set these fields to specify the implementation-dependent values of
      * various constant registers and reset values of non-constant
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6b77aaa445..7a17ba1418 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1765,6 +1765,7 @@ static Property arm_cpu_properties[] = {
     DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
                         mp_affinity, ARM64_AFFINITY_INVALID),
     DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
+    DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4228713b19..5e5ae44aeb 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -42,8 +42,15 @@ static inline void unset_feature(CPUARMState *env, int feature)
 #ifndef CONFIG_USER_ONLY
 static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    /* Number of processors is in [25:24]; otherwise we RAZ */
-    return (smp_cpus - 1) << 24;
+    ARMCPU *cpu = arm_env_get_cpu(env);
+
+    /* Number of cores is in [25:24]; otherwise we RAZ */
+    if (cpu->core_count == -1) {
+        /* No core_count specified, default to smp_cpus. */
+        return (smp_cpus - 1) << 24;
+    } else {
+        return (cpu->core_count - 1) << 24;
+    }
 }
 #endif
 
-- 
2.14.1

Re: [Qemu-devel] [PATCH v1 1/2] target/arm: Add a cluster size property

[Peter Maydell]

On 2 March 2018 at 17:06, Alistair Francis <alistair.francis@xilinx.com> wrote:

Subject should say "core count" rather than "cluster size" ?

> The cortex A53 TRM specifices that bits 24 and 25 of the L2CTLR register

"specifies"

> specify the number of cores present and not the number of processors. To

"the number of cores in the processor, not the total number of cores
in the system".

> report this correctly on machines with multiple CPU clusters (ARM's
> big.LITTLE or Xilinx's ZynqMP) we need to allow the machine to overwrite
> this value. To do this let's add an optional property.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  target/arm/cpu.h   |  5 +++++
>  target/arm/cpu.c   |  1 +
>  target/arm/cpu64.c | 11 +++++++++--
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 8dd6b788df..3fa8fdad21 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -745,6 +745,11 @@ struct ARMCPU {
>      /* Uniprocessor system with MP extensions */
>      bool mp_is_up;
>
> +    /* Specify the number of cores in this CPU cluster. Used for the L2CTLR
> +     * register.
> +     */
> +    int32_t core_count;
> +
>      /* The instance init functions for implementation-specific subclasses
>       * set these fields to specify the implementation-dependent values of
>       * various constant registers and reset values of non-constant
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 6b77aaa445..7a17ba1418 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1765,6 +1765,7 @@ static Property arm_cpu_properties[] = {
>      DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
>                          mp_affinity, ARM64_AFFINITY_INVALID),
>      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
> +    DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4228713b19..5e5ae44aeb 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -42,8 +42,15 @@ static inline void unset_feature(CPUARMState *env, int feature)
>  #ifndef CONFIG_USER_ONLY
>  static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
> -    /* Number of processors is in [25:24]; otherwise we RAZ */
> -    return (smp_cpus - 1) << 24;
> +    ARMCPU *cpu = arm_env_get_cpu(env);
> +
> +    /* Number of cores is in [25:24]; otherwise we RAZ */
> +    if (cpu->core_count == -1) {
> +        /* No core_count specified, default to smp_cpus. */
> +        return (smp_cpus - 1) << 24;
> +    } else {
> +        return (cpu->core_count - 1) << 24;
> +    }
>  }
>  #endif

I think having arm_cpu_realizefn do
    if (cpu->core_count == -1) {
        cpu->core_count = smp_cpus;
    }

would be neater than doing that check when the register is read.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/2] target/arm: Add a cluster size property

[Alistair Francis]

On Fri, Mar 2, 2018 at 9:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 March 2018 at 17:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
>
> Subject should say "core count" rather than "cluster size" ?

Yes, that was left over. I'll fix.

>
>> The cortex A53 TRM specifices that bits 24 and 25 of the L2CTLR register
>
> "specifies"
>
>> specify the number of cores present and not the number of processors. To
>
> "the number of cores in the processor, not the total number of cores
> in the system".

Fixed these.

>
>> report this correctly on machines with multiple CPU clusters (ARM's
>> big.LITTLE or Xilinx's ZynqMP) we need to allow the machine to overwrite
>> this value. To do this let's add an optional property.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  target/arm/cpu.h   |  5 +++++
>>  target/arm/cpu.c   |  1 +
>>  target/arm/cpu64.c | 11 +++++++++--
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 8dd6b788df..3fa8fdad21 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -745,6 +745,11 @@ struct ARMCPU {
>>      /* Uniprocessor system with MP extensions */
>>      bool mp_is_up;
>>
>> +    /* Specify the number of cores in this CPU cluster. Used for the L2CTLR
>> +     * register.
>> +     */
>> +    int32_t core_count;
>> +
>>      /* The instance init functions for implementation-specific subclasses
>>       * set these fields to specify the implementation-dependent values of
>>       * various constant registers and reset values of non-constant
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 6b77aaa445..7a17ba1418 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1765,6 +1765,7 @@ static Property arm_cpu_properties[] = {
>>      DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
>>                          mp_affinity, ARM64_AFFINITY_INVALID),
>>      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
>> +    DEFINE_PROP_INT32("core-count", ARMCPU, core_count, -1),
>>      DEFINE_PROP_END_OF_LIST()
>>  };
>>
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index 4228713b19..5e5ae44aeb 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -42,8 +42,15 @@ static inline void unset_feature(CPUARMState *env, int feature)
>>  #ifndef CONFIG_USER_ONLY
>>  static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>>  {
>> -    /* Number of processors is in [25:24]; otherwise we RAZ */
>> -    return (smp_cpus - 1) << 24;
>> +    ARMCPU *cpu = arm_env_get_cpu(env);
>> +
>> +    /* Number of cores is in [25:24]; otherwise we RAZ */
>> +    if (cpu->core_count == -1) {
>> +        /* No core_count specified, default to smp_cpus. */
>> +        return (smp_cpus - 1) << 24;
>> +    } else {
>> +        return (cpu->core_count - 1) << 24;
>> +    }
>>  }
>>  #endif
>
> I think having arm_cpu_realizefn do
>     if (cpu->core_count == -1) {
>         cpu->core_count = smp_cpus;
>     }
>
> would be neater than doing that check when the register is read.

Ok will fix. I'll send a V2 out straight away, as today is my last day
and it'd be great if I could get this done.

Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 1/2] target/arm: Add a cluster size property

[Peter Maydell]

On 2 March 2018 at 17:35, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Ok will fix. I'll send a V2 out straight away, as today is my last day
> and it'd be great if I could get this done.

Sure -- if there are any further nits in v2 I'll just fix them
up when I put it into target-arm.next.

-- PMM

[Qemu-devel] [PATCH v1 2/2] hw/arm: Set the core count for Xilinx's ZynqMP

[Alistair Francis]

Set the ARM CPU core count property for the A53's attached to the Xilnx
ZynqMP machine.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zynqmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 69227fd4c9..465796e97c 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -282,6 +282,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                                  s->virt, "has_el2", NULL);
         object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
                                 "reset-cbar", &error_abort);
+        object_property_set_int(OBJECT(&s->apu_cpu[i]), num_apus,
+                                "core-count", &error_abort);
         object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
                                  &err);
         if (err) {
-- 
2.14.1

Re: [Qemu-devel] [PATCH v1 2/2] hw/arm: Set the core count for Xilinx's ZynqMP

[Peter Maydell]

On 2 March 2018 at 17:06, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Set the ARM CPU core count property for the A53's attached to the Xilnx
> ZynqMP machine.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  hw/arm/xlnx-zynqmp.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 69227fd4c9..465796e97c 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -282,6 +282,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>                                   s->virt, "has_el2", NULL);
>          object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
>                                  "reset-cbar", &error_abort);
> +        object_property_set_int(OBJECT(&s->apu_cpu[i]), num_apus,
> +                                "core-count", &error_abort);
>          object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
>                                   &err);
>          if (err) {

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

thanks
-- PMM