On Fri, Mar 2, 2018 at 9:31 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 2 March 2018 at 17:06, Alistair Francis <alistair.fran...@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.fran...@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
