[U-Boot] [PATCH 4/9] x86: Move MP initialization codes into a common place

[Bin Meng]

Hi Simon,

On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 9 June 2015 at 01:45, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Most of the MP initialization codes in arch/x86/cpu/baytrail/cpu.c is
>> common to all x86 processors, except detect_num_cpus() which varies
>> from cpu to cpu. Move these to arch/x86/cpu/cpu.c and declare a weak
>> detect_num_cpus() which just returns 2 which is minimally required.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>  arch/x86/cpu/baytrail/cpu.c | 44 +-----------------------------------------
>>  arch/x86/cpu/cpu.c          | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 48 insertions(+), 43 deletions(-)
>>
>> diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c
>> index 05156a5..7805056 100644
>> --- a/arch/x86/cpu/baytrail/cpu.c
>> +++ b/arch/x86/cpu/baytrail/cpu.c
>> @@ -12,23 +12,11 @@
>>  #include <asm/cpu.h>
>>  #include <asm/cpu_x86.h>
>>  #include <asm/lapic.h>
>> -#include <asm/mp.h>
>>  #include <asm/msr.h>
>>  #include <asm/turbo.h>
>>
>>  #ifdef CONFIG_SMP
>> -static int enable_smis(struct udevice *cpu, void *unused)
>> -{
>> -       return 0;
>> -}
>> -
>> -static struct mp_flight_record mp_steps[] = {
>> -       MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
>> -       /* Wait for APs to finish initialization before proceeding. */
>> -       MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),
>> -};
>> -
>> -static int detect_num_cpus(void)
>> +int detect_num_cpus(void)
>>  {
>>         int ecx = 0;
>>
>> @@ -52,38 +40,8 @@ static int detect_num_cpus(void)
>>                 ecx++;
>>         }
>>  }
>> -
>> -static int baytrail_init_cpus(void)
>> -{
>> -       struct mp_params mp_params;
>> -
>> -       lapic_setup();
>> -
>> -       mp_params.num_cpus = detect_num_cpus();
>> -       mp_params.parallel_microcode_load = 0,
>> -       mp_params.flight_plan = &mp_steps[0];
>> -       mp_params.num_records = ARRAY_SIZE(mp_steps);
>> -       mp_params.microcode_pointer = 0;
>> -
>> -       if (mp_init(&mp_params)) {
>> -               printf("Warning: MP init failure\n");
>> -               return -EIO;
>> -       }
>> -
>> -       return 0;
>> -}
>>  #endif
>>
>> -int x86_init_cpus(void)
>> -{
>> -#ifdef CONFIG_SMP
>> -       debug("Init additional CPUs\n");
>> -       baytrail_init_cpus();
>> -#endif
>> -
>> -       return 0;
>> -}
>> -
>>  static void set_max_freq(void)
>>  {
>>         msr_t perf_ctl;
>> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
>> index ffb6e43..ddc7dc3 100644
>> --- a/arch/x86/cpu/cpu.c
>> +++ b/arch/x86/cpu/cpu.c
>> @@ -21,10 +21,13 @@
>>
>>  #include <common.h>
>>  #include <command.h>
>> +#include <dm.h>
>>  #include <errno.h>
>>  #include <malloc.h>
>>  #include <asm/control_regs.h>
>>  #include <asm/cpu.h>
>> +#include <asm/lapic.h>
>> +#include <asm/mp.h>
>>  #include <asm/post.h>
>>  #include <asm/processor.h>
>>  #include <asm/processor-flags.h>
>> @@ -601,8 +604,52 @@ int last_stage_init(void)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_SMP
>> +static int enable_smis(struct udevice *cpu, void *unused)
>> +{
>> +       return 0;
>> +}
>> +
>> +static struct mp_flight_record mp_steps[] = {
>> +       MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
>> +       /* Wait for APs to finish initialization before proceeding */
>> +       MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),
>> +};
>> +
>> +__weak int detect_num_cpus(void)
>
> Does this need to be weak? We could perhaps require that the function exists?
>

Yes, since I don't see there is a common way to detect number of cpu
cores. Or maybe I am not aware of one. If we want to remove the weak,
maybe we can just switch to count the number of cpu nodes in /cpus
from device tree, which should be generic for all cases. I believe we
discussed this during the review when you added MP stuff.

>> +{
>> +       /* We need at least 2 cores to perform mp_init() */
>> +       return 2;
>> +}
>> +
>> +static int x86_mp_init(void)
>> +{
>> +       struct mp_params mp_params;
>> +
>> +       lapic_setup();
>> +
>> +       mp_params.num_cpus = detect_num_cpus();
>> +       mp_params.parallel_microcode_load = 0,
>> +       mp_params.flight_plan = &mp_steps[0];
>> +       mp_params.num_records = ARRAY_SIZE(mp_steps);
>> +       mp_params.microcode_pointer = 0;
>> +
>> +       if (mp_init(&mp_params)) {
>> +               printf("Warning: MP init failure\n");
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>>  __weak int x86_init_cpus(void)
>>  {
>> +#ifdef CONFIG_SMP
>> +       debug("Init additional CPUs\n");
>> +       x86_mp_init();
>> +#endif
>> +
>>         return 0;
>>  }
>>
>> --
>> 1.8.2.1
>>
>
> Regards,
> Simon

Regards,
Bin