Re: [PATCH v3 09/17] ARM64 / ACPI: Parse MADT for SMP initialization

Jon Masters
2014-09-09T17:04:05Z

On 09/09/2014 12:52 PM, Lorenzo Pieralisi wrote:
> On Thu, Sep 04, 2014 at 04:29:15PM +0100, Hanjun Guo wrote:

>>>> +
>>>> + /* Get cpu_ops include the boot CPU */
>>>> + cpu_ops[cpu] = cpu_get_ops("psci");
>>>> + if (!cpu_ops[cpu])
>>>> + return -EINVAL;
>>>> +
>>>> + /* CPU 0 was already initialized */
>>>> + if (cpu) {
>>>> + if (cpu_ops[cpu]->cpu_init(NULL, cpu))
>>>> + return -EOPNOTSUPP;
>>>> +
>>>> + /* map the logical cpu id to cpu MPIDR */
>>>> + cpu_logical_map(cpu) = mpidr;
>>>> +
>>>> + set_cpu_possible(cpu, true);
>>>> + }
>>>> +
>>>> + enabled_cpus++;
>>> See above to me enabled_cpus and (num_possible_cpus() - 1) are identical.
>>
>> I think I need to remove all the CPU hotplug related code and make this function
>> as simple as possible and introduce them when needed.
>
> Yes that makes sense, even though a bit of foresight is always appreciated;
> I certainly do not want you to completely rewrite this code to support CPU
> hotplug to be 100% clear. "Disabled" CPUs is a concept that is not
> managed at the moment with DT (on ARM and ARM64), and we need to introduce it
> properly. Again, I was asking questions, to understand why you would need
> those variables.
>
> Have a look at this discussion:
>
> https://lkml.org/lkml/2013/6/6/470

There could also be other reasons to disable "CPUs". For example, in an
SMT system configured not to use SMT threads, you might (in one
instantiation) decide to simply mark such GICCs as not enabled.

Jon.