| Message ID | CAKmqyKOcSA0wqWrsRMXMv2E+Jb+MnYkD_h16NbEmxYohVJoj-w@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2 March 2018 at 04:34, Alistair Francis <alistair23@gmail.com> wrote: > On Thu, Mar 1, 2018 at 4:20 PM, Alistair Francis > <alistair.francis@xilinx.com> wrote: >> 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. We >> have correctly been reporting the number of cores, so just fix the >> comment to match the TRM. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > > Ah! This isn't actually what I want, I want something more like this (untested): > > commit ce9d9795ebff42e1f742e7dc3786e52524807c65 > Author: Alistair Francis <alistair@alistair23.me> > Date: Thu Mar 1 20:19:23 2018 -0800 > > target/arm: Report the number of cores in the cluster > > Previously we assumed that we only has a single cluster, which meant we > could get away with reporting smp_cpus to the guest. There are cases > where we have two clusters (Xilinx's ZynqMP is a good example) so > reporting the number of smp_cpus is incorrect. Instead count the cores > in the cluster. > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 9743bdc..e7b1f3c 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -42,8 +42,24 @@ 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; > + CPUState *cpu; > + CPUState *cpu_prev = NULL; > + int num_cores = 0; > + > + /* Figure out the number of cores in the cluster */ > + for (cpu = first_cpu; cpu; cpu = CPU_NEXT(cpu)) { > + /* Only increase the core count if the CPU we are on is the same > + * class as the caller and the previous cpu. > + */ > + if ((CPU_GET_CLASS(cpu) == CPU_GET_CLASS(cpu_prev)) && > + (CPU_GET_CLASS(cpu) == CPU_GET_CLASS(CPU(env)))) { > + num_cores++; > + } > + cpu_prev = cpu; > + } > + > + /* Number of cores is in [25:24]; otherwise we RAZ */ > + return num_cores << 24; > } > #endif This seems a bit ad-hoc (it will give the wrong answer if we have two clusters both of the same kind of CPU, for instance). Maybe it would be better to have a "cluster-size" property on the CPU (with the default being number of cores in whole system) ? thanks -- PMM
On 2 March 2018 at 10:29, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 March 2018 at 04:34, Alistair Francis <alistair23@gmail.com> wrote: >> target/arm: Report the number of cores in the cluster >> >> Previously we assumed that we only has a single cluster, which meant we >> could get away with reporting smp_cpus to the guest. There are cases >> where we have two clusters (Xilinx's ZynqMP is a good example) so >> reporting the number of smp_cpus is incorrect. Instead count the cores >> in the cluster. > This seems a bit ad-hoc (it will give the wrong answer if we have > two clusters both of the same kind of CPU, for instance). > Maybe it would be better to have a "cluster-size" property on > the CPU (with the default being number of cores in whole system) ? ...though I think "cluster size" isn't quite the right name -- if you have a big.LITTLE cluster with 2xA57 and 2xA53, there are 4 cores in the cluster but I think reading the TRM that the cores will report 2 in the L2CTLR. thanks -- PMM
On Fri, Mar 2, 2018 at 2:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 March 2018 at 10:29, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 2 March 2018 at 04:34, Alistair Francis <alistair23@gmail.com> wrote: >>> target/arm: Report the number of cores in the cluster >>> >>> Previously we assumed that we only has a single cluster, which meant we >>> could get away with reporting smp_cpus to the guest. There are cases >>> where we have two clusters (Xilinx's ZynqMP is a good example) so >>> reporting the number of smp_cpus is incorrect. Instead count the cores >>> in the cluster. > >> This seems a bit ad-hoc (it will give the wrong answer if we have >> two clusters both of the same kind of CPU, for instance). >> Maybe it would be better to have a "cluster-size" property on >> the CPU (with the default being number of cores in whole system) ? > > ...though I think "cluster size" isn't quite the right name -- > if you have a big.LITTLE cluster with 2xA57 and 2xA53, there are > 4 cores in the cluster but I think reading the TRM that the cores > will report 2 in the L2CTLR. Hmmm... I would have thought cluster size is correct enough. The only other option I can think of is "processor size" or "core size"? Alistair > > thanks > -- PMM
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 9743bdc..e7b1f3c 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -42,8 +42,24 @@ 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; + CPUState *cpu; + CPUState *cpu_prev = NULL; + int num_cores = 0; + + /* Figure out the number of cores in the cluster */ + for (cpu = first_cpu; cpu; cpu = CPU_NEXT(cpu)) { + /* Only increase the core count if the CPU we are on is the same + * class as the caller and the previous cpu. + */ + if ((CPU_GET_CLASS(cpu) == CPU_GET_CLASS(cpu_prev)) && + (CPU_GET_CLASS(cpu) == CPU_GET_CLASS(CPU(env)))) { + num_cores++; + } + cpu_prev = cpu; + } + + /* Number of cores is in [25:24]; otherwise we RAZ */ + return num_cores << 24; } #endif