Message ID | BLU437-SMTP338713357EB6ABB95196DBFBE0@phx.gbl |
---|---|
State | Superseded |
Delegated to: | Simon Glass |
Headers | show |
Hi Bin, On 9 June 2015 at 01:45, Bin Meng <bmeng.cn@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@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? > +{ > + /* 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
Hi Simon, On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 9 June 2015 at 01:45, Bin Meng <bmeng.cn@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@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
Hi Bin, On 11 June 2015 at 20:07, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 9 June 2015 at 01:45, Bin Meng <bmeng.cn@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@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. I worry what might happen if the device tree says 4 but there are only 2 CPUs. There should be a way to detect the number of CPUs gracefully in each arch. Can you add get_cpu_count() as a method to the UCLASS_CPU ops perhaps? Then baytrail can use it. mp_init() can make the get_cpu_count() call after finding the first CPU, and we can remove num_cpus from struct mp_params. To me that seams cleaner than creating a weak function. > >>> +{ >>> + /* 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
Hi Simon, On Sat, Jun 13, 2015 at 3:10 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 11 June 2015 at 20:07, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 9 June 2015 at 01:45, Bin Meng <bmeng.cn@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@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. > > I worry what might happen if the device tree says 4 but there are only > 2 CPUs. There should be a way to detect the number of CPUs gracefully > in each arch. I don't think there will be any problem for device tree says 4 but actually 2. U-Boot will just enable 2 CPUs and report 2 to the OS. The case that might have a problem is that the device tree says 2 but there are actually 4 CPUs. There is a check_cpu_devices() call in mp_init() which will put a debug message "Warning: Device tree does not describe all CPUs. Extra ones will not be started correctly". There are some places (like SFI) in U-Boot that look up the cpus node in device tree , and we will end up CPU#2~3 not reported to OS. But why should we create a device tree which does not describe the hardware in the first place? > Can you add get_cpu_count() as a method to the UCLASS_CPU ops perhaps? > Then baytrail can use it. > > mp_init() can make the get_cpu_count() call after finding the first > CPU, and we can remove num_cpus from struct mp_params. > > To me that seams cleaner than creating a weak function. Yes, I can try to implement that. >> >>>> +{ >>>> + /* 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; >>>> } >>>> >>>> -- Regards, Bin
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) +{ + /* 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; }
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@gmail.com> --- arch/x86/cpu/baytrail/cpu.c | 44 +----------------------------------------- arch/x86/cpu/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 43 deletions(-)