| Message ID | 1357777251-13541-7-git-send-email-nicolas.pitre@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: > Now that the b.L power API is in place, we can use it for SMP secondary > bringup and CPU hotplug in a generic fashion. > > Signed-off-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/common/Makefile | 2 +- > arch/arm/common/bL_platsmp.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 80 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/common/bL_platsmp.c > > diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile > index 894c2ddf9b..59b36db7cc 100644 > --- a/arch/arm/common/Makefile > +++ b/arch/arm/common/Makefile > @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o > obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o > obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o > obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o > -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o > +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o vlock.o > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c > new file mode 100644 > index 0000000000..0acb9f4685 > --- /dev/null > +++ b/arch/arm/common/bL_platsmp.c > @@ -0,0 +1,79 @@ > +/* > + * linux/arch/arm/mach-vexpress/bL_platsmp.c > + * > + * Created by: Nicolas Pitre, November 2012 > + * Copyright: (C) 2012 Linaro Limited > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Code to handle secondary CPU bringup and hotplug for the bL power API. > + */ > + > +#include <linux/init.h> > +#include <linux/smp.h> > + > +#include <asm/bL_entry.h> > +#include <asm/smp_plat.h> > +#include <asm/hardware/gic.h> > + > +static void __init simple_smp_init_cpus(void) > +{ > + set_smp_cross_call(gic_raise_softirq); > +} > + > +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + unsigned int pcpu, pcluster, ret; > + extern void secondary_startup(void); > + > + pcpu = cpu_logical_map(cpu) & 0xff; > + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; > + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", > + __func__, cpu, pcpu, pcluster); > + > + bL_set_entry_vector(pcpu, pcluster, NULL); > + ret = bL_cpu_power_up(pcpu, pcluster); > + if (ret) > + return ret; > + bL_set_entry_vector(pcpu, pcluster, secondary_startup); > + gic_raise_softirq(cpumask_of(cpu), 0); > + sev(); softirq() should be enough to break a CPU if it is in standby with wfe state. Is that additional sev() needed here ? Regards, Santosh
On Thu, Jan 10, 2013 at 12:20:41AM +0000, Nicolas Pitre wrote: > Now that the b.L power API is in place, we can use it for SMP secondary > bringup and CPU hotplug in a generic fashion. [...] > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c > new file mode 100644 > index 0000000000..0acb9f4685 > --- /dev/null > +++ b/arch/arm/common/bL_platsmp.c > @@ -0,0 +1,79 @@ > +/* > + * linux/arch/arm/mach-vexpress/bL_platsmp.c > + * > + * Created by: Nicolas Pitre, November 2012 > + * Copyright: (C) 2012 Linaro Limited > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Code to handle secondary CPU bringup and hotplug for the bL power API. > + */ > + > +#include <linux/init.h> > +#include <linux/smp.h> > + > +#include <asm/bL_entry.h> > +#include <asm/smp_plat.h> > +#include <asm/hardware/gic.h> > + > +static void __init simple_smp_init_cpus(void) > +{ > + set_smp_cross_call(gic_raise_softirq); > +} > + > +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + unsigned int pcpu, pcluster, ret; > + extern void secondary_startup(void); > + > + pcpu = cpu_logical_map(cpu) & 0xff; > + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; Again, you can probably use Lorenzo's helpers here. > + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", > + __func__, cpu, pcpu, pcluster); > + > + bL_set_entry_vector(pcpu, pcluster, NULL); Now that you don't have a barrier in this function, you need one here. > + ret = bL_cpu_power_up(pcpu, pcluster); > + if (ret) > + return ret; and here, although I confess to not understanding why you write NULL the first time. > + bL_set_entry_vector(pcpu, pcluster, secondary_startup); > + gic_raise_softirq(cpumask_of(cpu), 0); > + sev(); This relise on the event register being able to be set if the target is in a low-power (wfi) state. I'd feel safer with a dsb before the sev... Will
On Mon, 14 Jan 2013, Will Deacon wrote: > On Thu, Jan 10, 2013 at 12:20:41AM +0000, Nicolas Pitre wrote: > > Now that the b.L power API is in place, we can use it for SMP secondary > > bringup and CPU hotplug in a generic fashion. > > [...] > > > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c > > new file mode 100644 > > index 0000000000..0acb9f4685 > > --- /dev/null > > +++ b/arch/arm/common/bL_platsmp.c > > @@ -0,0 +1,79 @@ > > +/* > > + * linux/arch/arm/mach-vexpress/bL_platsmp.c > > + * > > + * Created by: Nicolas Pitre, November 2012 > > + * Copyright: (C) 2012 Linaro Limited > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + * Code to handle secondary CPU bringup and hotplug for the bL power API. > > + */ > > + > > +#include <linux/init.h> > > +#include <linux/smp.h> > > + > > +#include <asm/bL_entry.h> > > +#include <asm/smp_plat.h> > > +#include <asm/hardware/gic.h> > > + > > +static void __init simple_smp_init_cpus(void) > > +{ > > + set_smp_cross_call(gic_raise_softirq); > > +} > > + > > +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle) > > +{ > > + unsigned int pcpu, pcluster, ret; > > + extern void secondary_startup(void); > > + > > + pcpu = cpu_logical_map(cpu) & 0xff; > > + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; > > Again, you can probably use Lorenzo's helpers here. Yes, that goes for the whole series. > > + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", > > + __func__, cpu, pcpu, pcluster); > > + > > + bL_set_entry_vector(pcpu, pcluster, NULL); > > Now that you don't have a barrier in this function, you need one here. Hmmm... Why? > > + ret = bL_cpu_power_up(pcpu, pcluster); > > + if (ret) > > + return ret; > > and here, although I confess to not understanding why you write NULL the > first time. If for some reasons the bL_cpu_power_up() call fails, I don't want this CPU to suddenly decide to enter the kernel if it wakes up at a later time when secondary_startup is not ready to deal with it anymore. > > + bL_set_entry_vector(pcpu, pcluster, secondary_startup); > > + gic_raise_softirq(cpumask_of(cpu), 0); > > + sev(); > > This relise on the event register being able to be set if the target is in a > low-power (wfi) state. I'd feel safer with a dsb before the sev... Sure. Nicolas
Hi Santosh, On Fri, Jan 11, 2013 at 6:02 PM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: >> >> Now that the b.L power API is in place, we can use it for SMP secondary >> bringup and CPU hotplug in a generic fashion. >> >> Signed-off-by: Nicolas Pitre <nico@linaro.org> >> --- >> arch/arm/common/Makefile | 2 +- >> arch/arm/common/bL_platsmp.c | 79 >> ++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 80 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm/common/bL_platsmp.c >> >> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile >> index 894c2ddf9b..59b36db7cc 100644 >> --- a/arch/arm/common/Makefile >> +++ b/arch/arm/common/Makefile >> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o >> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o >> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o >> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o >> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o >> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o >> vlock.o >> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c >> new file mode 100644 >> index 0000000000..0acb9f4685 >> --- /dev/null >> +++ b/arch/arm/common/bL_platsmp.c >> @@ -0,0 +1,79 @@ >> +/* >> + * linux/arch/arm/mach-vexpress/bL_platsmp.c >> + * >> + * Created by: Nicolas Pitre, November 2012 >> + * Copyright: (C) 2012 Linaro Limited >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * Code to handle secondary CPU bringup and hotplug for the bL power API. >> + */ >> + >> +#include <linux/init.h> >> +#include <linux/smp.h> >> + >> +#include <asm/bL_entry.h> >> +#include <asm/smp_plat.h> >> +#include <asm/hardware/gic.h> >> + >> +static void __init simple_smp_init_cpus(void) >> +{ >> + set_smp_cross_call(gic_raise_softirq); >> +} >> + >> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct >> task_struct *idle) >> +{ >> + unsigned int pcpu, pcluster, ret; >> + extern void secondary_startup(void); >> + >> + pcpu = cpu_logical_map(cpu) & 0xff; >> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; >> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", >> + __func__, cpu, pcpu, pcluster); >> + >> + bL_set_entry_vector(pcpu, pcluster, NULL); >> + ret = bL_cpu_power_up(pcpu, pcluster); >> + if (ret) >> + return ret; >> + bL_set_entry_vector(pcpu, pcluster, secondary_startup); >> + gic_raise_softirq(cpumask_of(cpu), 0); >> + sev(); > > softirq() should be enough to break a CPU if it is in standby with > wfe state. Is that additional sev() needed here ? Not if the target cpu has its I & F bits disabled and that would be the case with a secondary waiting to be woken up thanks, Achin > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Monday 14 January 2013 11:35 PM, Achin Gupta wrote: > Hi Santosh, > > On Fri, Jan 11, 2013 at 6:02 PM, Santosh Shilimkar > <santosh.shilimkar@ti.com> wrote: >> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: >>> >>> Now that the b.L power API is in place, we can use it for SMP secondary >>> bringup and CPU hotplug in a generic fashion. >>> >>> Signed-off-by: Nicolas Pitre <nico@linaro.org> >>> --- >>> arch/arm/common/Makefile | 2 +- >>> arch/arm/common/bL_platsmp.c | 79 >>> ++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 80 insertions(+), 1 deletion(-) >>> create mode 100644 arch/arm/common/bL_platsmp.c >>> >>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile >>> index 894c2ddf9b..59b36db7cc 100644 >>> --- a/arch/arm/common/Makefile >>> +++ b/arch/arm/common/Makefile >>> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o >>> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o >>> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o >>> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o >>> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o >>> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o >>> vlock.o >>> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c >>> new file mode 100644 >>> index 0000000000..0acb9f4685 >>> --- /dev/null >>> +++ b/arch/arm/common/bL_platsmp.c >>> @@ -0,0 +1,79 @@ >>> +/* >>> + * linux/arch/arm/mach-vexpress/bL_platsmp.c >>> + * >>> + * Created by: Nicolas Pitre, November 2012 >>> + * Copyright: (C) 2012 Linaro Limited >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + * >>> + * Code to handle secondary CPU bringup and hotplug for the bL power API. >>> + */ >>> + >>> +#include <linux/init.h> >>> +#include <linux/smp.h> >>> + >>> +#include <asm/bL_entry.h> >>> +#include <asm/smp_plat.h> >>> +#include <asm/hardware/gic.h> >>> + >>> +static void __init simple_smp_init_cpus(void) >>> +{ >>> + set_smp_cross_call(gic_raise_softirq); >>> +} >>> + >>> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct >>> task_struct *idle) >>> +{ >>> + unsigned int pcpu, pcluster, ret; >>> + extern void secondary_startup(void); >>> + >>> + pcpu = cpu_logical_map(cpu) & 0xff; >>> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; >>> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", >>> + __func__, cpu, pcpu, pcluster); >>> + >>> + bL_set_entry_vector(pcpu, pcluster, NULL); >>> + ret = bL_cpu_power_up(pcpu, pcluster); >>> + if (ret) >>> + return ret; >>> + bL_set_entry_vector(pcpu, pcluster, secondary_startup); >>> + gic_raise_softirq(cpumask_of(cpu), 0); >>> + sev(); >> >> softirq() should be enough to break a CPU if it is in standby with >> wfe state. Is that additional sev() needed here ? > > Not if the target cpu has its I & F bits disabled and that would be the > case with a secondary waiting to be woken up > This is interesting since CPU is actually in standby state and this was not my understanding so far. Your statement at least contradicts the ARM ARM (B1.8.12 Wait For Interrupt) ----------------------- The processor can remain in the WFI low-power state until it is reset, or it detects one of the following WFI wake-up events: • a physical IRQ interrupt, regardless of the value of the CPSR.I bit • a physical FIQ interrupt, regardless of the value of the CPSR.F bit ---------------------------------- Are you referring to some new behavior on latest ARMv7 CPUs ? Regards, Santosh
Hi Santosh, On Tue, Jan 15, 2013 at 6:32 AM, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > On Monday 14 January 2013 11:35 PM, Achin Gupta wrote: >> >> Hi Santosh, >> >> On Fri, Jan 11, 2013 at 6:02 PM, Santosh Shilimkar >> <santosh.shilimkar@ti.com> wrote: >>> >>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: >>>> >>>> >>>> Now that the b.L power API is in place, we can use it for SMP secondary >>>> bringup and CPU hotplug in a generic fashion. >>>> >>>> Signed-off-by: Nicolas Pitre <nico@linaro.org> >>>> --- >>>> arch/arm/common/Makefile | 2 +- >>>> arch/arm/common/bL_platsmp.c | 79 >>>> ++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 80 insertions(+), 1 deletion(-) >>>> create mode 100644 arch/arm/common/bL_platsmp.c >>>> >>>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile >>>> index 894c2ddf9b..59b36db7cc 100644 >>>> --- a/arch/arm/common/Makefile >>>> +++ b/arch/arm/common/Makefile >>>> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o >>>> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o >>>> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o >>>> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o >>>> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o >>>> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o >>>> vlock.o >>>> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c >>>> new file mode 100644 >>>> index 0000000000..0acb9f4685 >>>> --- /dev/null >>>> +++ b/arch/arm/common/bL_platsmp.c >>>> @@ -0,0 +1,79 @@ >>>> +/* >>>> + * linux/arch/arm/mach-vexpress/bL_platsmp.c >>>> + * >>>> + * Created by: Nicolas Pitre, November 2012 >>>> + * Copyright: (C) 2012 Linaro Limited >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License version 2 as >>>> + * published by the Free Software Foundation. >>>> + * >>>> + * Code to handle secondary CPU bringup and hotplug for the bL power >>>> API. >>>> + */ >>>> + >>>> +#include <linux/init.h> >>>> +#include <linux/smp.h> >>>> + >>>> +#include <asm/bL_entry.h> >>>> +#include <asm/smp_plat.h> >>>> +#include <asm/hardware/gic.h> >>>> + >>>> +static void __init simple_smp_init_cpus(void) >>>> +{ >>>> + set_smp_cross_call(gic_raise_softirq); >>>> +} >>>> + >>>> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct >>>> task_struct *idle) >>>> +{ >>>> + unsigned int pcpu, pcluster, ret; >>>> + extern void secondary_startup(void); >>>> + >>>> + pcpu = cpu_logical_map(cpu) & 0xff; >>>> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; >>>> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", >>>> + __func__, cpu, pcpu, pcluster); >>>> + >>>> + bL_set_entry_vector(pcpu, pcluster, NULL); >>>> + ret = bL_cpu_power_up(pcpu, pcluster); >>>> + if (ret) >>>> + return ret; >>>> + bL_set_entry_vector(pcpu, pcluster, secondary_startup); >>>> + gic_raise_softirq(cpumask_of(cpu), 0); >>>> + sev(); >>> >>> >>> softirq() should be enough to break a CPU if it is in standby with >>> wfe state. Is that additional sev() needed here ? >> >> >> Not if the target cpu has its I & F bits disabled and that would be the >> case with a secondary waiting to be woken up >> > This is interesting since CPU is actually in standby state and this > was not my understanding so far. Your statement at least contradicts > the ARM ARM (B1.8.12 Wait For Interrupt) > ----------------------- > The processor can remain in the WFI low-power state until it is reset, or it > detects one of the following WFI wake-up > events: > • a physical IRQ interrupt, regardless of the value of the CPSR.I bit > • a physical FIQ interrupt, regardless of the value of the CPSR.F bit > ---------------------------------- > > Are you referring to some new behavior on latest ARMv7 CPUs ? You are abs right about the 'wfi' behaviour. I was talking about the effect of interrupts on a cpu thats in 'wfe'. The power up process takes place in two steps. The first step involves sending an ipi which will either: a. cause the power controller to bring the processor out of reset b. cause the processor to exit from wfi (most probably in the bootloader code) The cpu then enters Linux (bL_entry_point) and after doing any cluster setup waits in 'wfe' if its 'bL_entry_vector' has not been set as yet. The 'sev' is meant to poke the cpu once this has been done. Its not required in this case as we have already set 'bL_entry_vector' , issued a barrier & flushed the cache line. So if the incoming cpu sees a 0 in its vector then that would be a symptom of a different problem. Thanks, Achin > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tuesday 15 January 2013 04:48 PM, Achin Gupta wrote: > Hi Santosh, > > On Tue, Jan 15, 2013 at 6:32 AM, Santosh Shilimkar > <santosh.shilimkar@ti.com> wrote: >> On Monday 14 January 2013 11:35 PM, Achin Gupta wrote: >>> >>> Hi Santosh, >>> >>> On Fri, Jan 11, 2013 at 6:02 PM, Santosh Shilimkar >>> <santosh.shilimkar@ti.com> wrote: >>>> >>>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: >>>>> >>>>> >>>>> Now that the b.L power API is in place, we can use it for SMP secondary >>>>> bringup and CPU hotplug in a generic fashion. >>>>> >>>>> Signed-off-by: Nicolas Pitre <nico@linaro.org> >>>>> --- >>>>> arch/arm/common/Makefile | 2 +- >>>>> arch/arm/common/bL_platsmp.c | 79 >>>>> ++++++++++++++++++++++++++++++++++++++++++++ >>>>> 2 files changed, 80 insertions(+), 1 deletion(-) >>>>> create mode 100644 arch/arm/common/bL_platsmp.c >>>>> >>>>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile >>>>> index 894c2ddf9b..59b36db7cc 100644 >>>>> --- a/arch/arm/common/Makefile >>>>> +++ b/arch/arm/common/Makefile >>>>> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o >>>>> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o >>>>> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o >>>>> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o >>>>> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o >>>>> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o >>>>> vlock.o >>>>> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c >>>>> new file mode 100644 >>>>> index 0000000000..0acb9f4685 >>>>> --- /dev/null >>>>> +++ b/arch/arm/common/bL_platsmp.c >>>>> @@ -0,0 +1,79 @@ >>>>> +/* >>>>> + * linux/arch/arm/mach-vexpress/bL_platsmp.c >>>>> + * >>>>> + * Created by: Nicolas Pitre, November 2012 >>>>> + * Copyright: (C) 2012 Linaro Limited >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or modify >>>>> + * it under the terms of the GNU General Public License version 2 as >>>>> + * published by the Free Software Foundation. >>>>> + * >>>>> + * Code to handle secondary CPU bringup and hotplug for the bL power >>>>> API. >>>>> + */ >>>>> + >>>>> +#include <linux/init.h> >>>>> +#include <linux/smp.h> >>>>> + >>>>> +#include <asm/bL_entry.h> >>>>> +#include <asm/smp_plat.h> >>>>> +#include <asm/hardware/gic.h> >>>>> + >>>>> +static void __init simple_smp_init_cpus(void) >>>>> +{ >>>>> + set_smp_cross_call(gic_raise_softirq); >>>>> +} >>>>> + >>>>> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct >>>>> task_struct *idle) >>>>> +{ >>>>> + unsigned int pcpu, pcluster, ret; >>>>> + extern void secondary_startup(void); >>>>> + >>>>> + pcpu = cpu_logical_map(cpu) & 0xff; >>>>> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; >>>>> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", >>>>> + __func__, cpu, pcpu, pcluster); >>>>> + >>>>> + bL_set_entry_vector(pcpu, pcluster, NULL); >>>>> + ret = bL_cpu_power_up(pcpu, pcluster); >>>>> + if (ret) >>>>> + return ret; >>>>> + bL_set_entry_vector(pcpu, pcluster, secondary_startup); >>>>> + gic_raise_softirq(cpumask_of(cpu), 0); >>>>> + sev(); >>>> >>>> >>>> softirq() should be enough to break a CPU if it is in standby with >>>> wfe state. Is that additional sev() needed here ? >>> >>> >>> Not if the target cpu has its I & F bits disabled and that would be the >>> case with a secondary waiting to be woken up >>> >> This is interesting since CPU is actually in standby state and this >> was not my understanding so far. Your statement at least contradicts >> the ARM ARM (B1.8.12 Wait For Interrupt) >> ----------------------- >> The processor can remain in the WFI low-power state until it is reset, or it >> detects one of the following WFI wake-up >> events: >> • a physical IRQ interrupt, regardless of the value of the CPSR.I bit >> • a physical FIQ interrupt, regardless of the value of the CPSR.F bit >> ---------------------------------- >> >> Are you referring to some new behavior on latest ARMv7 CPUs ? > > You are abs right about the 'wfi' behaviour. I was talking about the effect > of interrupts on a cpu thats in 'wfe'. > > The power up process takes place in two steps. The first step involves > sending an ipi which will either: > > a. cause the power controller to bring the processor out of reset > b. cause the processor to exit from wfi (most probably in the bootloader code) > > The cpu then enters Linux (bL_entry_point) and after doing any cluster setup > waits in 'wfe' if its 'bL_entry_vector' has not been set as yet. The > 'sev' is meant > to poke the cpu once this has been done. > Thanks for additional information. Its clear to me now. Regards Santosh
On Tue, Jan 15, 2013 at 11:18:44AM +0000, Achin Gupta wrote: > Hi Santosh, > > On Tue, Jan 15, 2013 at 6:32 AM, Santosh Shilimkar > <santosh.shilimkar@ti.com> wrote: > > On Monday 14 January 2013 11:35 PM, Achin Gupta wrote: > >> > >> Hi Santosh, > >> > >> On Fri, Jan 11, 2013 at 6:02 PM, Santosh Shilimkar > >> <santosh.shilimkar@ti.com> wrote: > >>> > >>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote: > >>>> > >>>> > >>>> Now that the b.L power API is in place, we can use it for SMP secondary > >>>> bringup and CPU hotplug in a generic fashion. > >>>> > >>>> Signed-off-by: Nicolas Pitre <nico@linaro.org> > >>>> --- > >>>> arch/arm/common/Makefile | 2 +- > >>>> arch/arm/common/bL_platsmp.c | 79 > >>>> ++++++++++++++++++++++++++++++++++++++++++++ > >>>> 2 files changed, 80 insertions(+), 1 deletion(-) > >>>> create mode 100644 arch/arm/common/bL_platsmp.c > >>>> > >>>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile > >>>> index 894c2ddf9b..59b36db7cc 100644 > >>>> --- a/arch/arm/common/Makefile > >>>> +++ b/arch/arm/common/Makefile > >>>> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o > >>>> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o > >>>> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o > >>>> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o > >>>> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o > >>>> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o > >>>> vlock.o > >>>> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c > >>>> new file mode 100644 > >>>> index 0000000000..0acb9f4685 > >>>> --- /dev/null > >>>> +++ b/arch/arm/common/bL_platsmp.c > >>>> @@ -0,0 +1,79 @@ > >>>> +/* > >>>> + * linux/arch/arm/mach-vexpress/bL_platsmp.c > >>>> + * > >>>> + * Created by: Nicolas Pitre, November 2012 > >>>> + * Copyright: (C) 2012 Linaro Limited > >>>> + * > >>>> + * This program is free software; you can redistribute it and/or modify > >>>> + * it under the terms of the GNU General Public License version 2 as > >>>> + * published by the Free Software Foundation. > >>>> + * > >>>> + * Code to handle secondary CPU bringup and hotplug for the bL power > >>>> API. > >>>> + */ > >>>> + > >>>> +#include <linux/init.h> > >>>> +#include <linux/smp.h> > >>>> + > >>>> +#include <asm/bL_entry.h> > >>>> +#include <asm/smp_plat.h> > >>>> +#include <asm/hardware/gic.h> > >>>> + > >>>> +static void __init simple_smp_init_cpus(void) > >>>> +{ > >>>> + set_smp_cross_call(gic_raise_softirq); > >>>> +} > >>>> + > >>>> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct > >>>> task_struct *idle) > >>>> +{ > >>>> + unsigned int pcpu, pcluster, ret; > >>>> + extern void secondary_startup(void); > >>>> + > >>>> + pcpu = cpu_logical_map(cpu) & 0xff; > >>>> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; > >>>> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", > >>>> + __func__, cpu, pcpu, pcluster); > >>>> + > >>>> + bL_set_entry_vector(pcpu, pcluster, NULL); > >>>> + ret = bL_cpu_power_up(pcpu, pcluster); > >>>> + if (ret) > >>>> + return ret; > >>>> + bL_set_entry_vector(pcpu, pcluster, secondary_startup); > >>>> + gic_raise_softirq(cpumask_of(cpu), 0); > >>>> + sev(); > >>> > >>> > >>> softirq() should be enough to break a CPU if it is in standby with > >>> wfe state. Is that additional sev() needed here ? > >> > >> > >> Not if the target cpu has its I & F bits disabled and that would be the > >> case with a secondary waiting to be woken up > >> > > This is interesting since CPU is actually in standby state and this > > was not my understanding so far. Your statement at least contradicts > > the ARM ARM (B1.8.12 Wait For Interrupt) > > ----------------------- > > The processor can remain in the WFI low-power state until it is reset, or it > > detects one of the following WFI wake-up > > events: > > • a physical IRQ interrupt, regardless of the value of the CPSR.I bit > > • a physical FIQ interrupt, regardless of the value of the CPSR.F bit > > ---------------------------------- > > > > Are you referring to some new behavior on latest ARMv7 CPUs ? > > You are abs right about the 'wfi' behaviour. I was talking about the effect > of interrupts on a cpu thats in 'wfe'. > > The power up process takes place in two steps. The first step involves > sending an ipi which will either: > > a. cause the power controller to bring the processor out of reset > b. cause the processor to exit from wfi (most probably in the bootloader code) > > The cpu then enters Linux (bL_entry_point) and after doing any cluster setup > waits in 'wfe' if its 'bL_entry_vector' has not been set as yet. The > 'sev' is meant > to poke the cpu once this has been done. > > Its not required in this case as we have already set 'bL_entry_vector' , issued > a barrier & flushed the cache line. So if the incoming cpu sees a 0 in > its vector > then that would be a symptom of a different problem. Perhaps this could be made a bit clearer by defining two helpers, bL_entry_close_gate() and bL_entry_open_gate(). The sev() is only applicable for opening the gate, and could be buried in bL_entry_open_gate(). This is pretty specialised low-level code though, so I'm not sure that the abstraction is worth it. Cheers ---Dave
On Mon, Jan 14, 2013 at 11:51:11AM -0500, Nicolas Pitre wrote: > On Mon, 14 Jan 2013, Will Deacon wrote: > > > On Thu, Jan 10, 2013 at 12:20:41AM +0000, Nicolas Pitre wrote: > > > Now that the b.L power API is in place, we can use it for SMP secondary > > > bringup and CPU hotplug in a generic fashion. > > > > [...] > > > > > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c > > > new file mode 100644 > > > index 0000000000..0acb9f4685 > > > --- /dev/null > > > +++ b/arch/arm/common/bL_platsmp.c > > > @@ -0,0 +1,79 @@ > > > +/* > > > + * linux/arch/arm/mach-vexpress/bL_platsmp.c > > > + * > > > + * Created by: Nicolas Pitre, November 2012 > > > + * Copyright: (C) 2012 Linaro Limited > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License version 2 as > > > + * published by the Free Software Foundation. > > > + * > > > + * Code to handle secondary CPU bringup and hotplug for the bL power API. > > > + */ > > > + > > > +#include <linux/init.h> > > > +#include <linux/smp.h> > > > + > > > +#include <asm/bL_entry.h> > > > +#include <asm/smp_plat.h> > > > +#include <asm/hardware/gic.h> > > > + > > > +static void __init simple_smp_init_cpus(void) > > > +{ > > > + set_smp_cross_call(gic_raise_softirq); > > > +} > > > + > > > +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle) > > > +{ > > > + unsigned int pcpu, pcluster, ret; > > > + extern void secondary_startup(void); > > > + > > > + pcpu = cpu_logical_map(cpu) & 0xff; > > > + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; > > > > Again, you can probably use Lorenzo's helpers here. > > Yes, that goes for the whole series. > > > > + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", > > > + __func__, cpu, pcpu, pcluster); > > > + > > > + bL_set_entry_vector(pcpu, pcluster, NULL); > > > > Now that you don't have a barrier in this function, you need one here. > > Hmmm... Why? In effect, we are entering a critical section here: that's precisely why we close the gate. We need a barrier after bL_set_entry_vector() to make sure that no operations from the critical section leak outside from the perspective of the target CPU. Similarly, we need a barrier before bL_set_entry_vector() when opening the gate. The corresponding barrier required in bL_head.S at bL_entry_gated is added by my recent barriers and tidyups series. Closing and opening the gate are a bit like taking and releasing a lock -- which is one reason for having simple wrapper functions to make these roles more obvious. > > > > + ret = bL_cpu_power_up(pcpu, pcluster); > > > + if (ret) > > > + return ret; > > > > and here, although I confess to not understanding why you write NULL the > > first time. > > If for some reasons the bL_cpu_power_up() call fails, I don't want this > CPU to suddenly decide to enter the kernel if it wakes up at a later > time when secondary_startup is not ready to deal with it anymore. > > > > + bL_set_entry_vector(pcpu, pcluster, secondary_startup); > > > + gic_raise_softirq(cpumask_of(cpu), 0); > > > + sev(); > > > > This relise on the event register being able to be set if the target is in a > > low-power (wfi) state. I'd feel safer with a dsb before the sev... The sev() signals the update to the entry vector, which has already been DSB'd by the flushing in bL_set_entry_vector(). Also, the relative order of gic_raise_softirq() and sev() here is not important, provided they both follow bL_set_entry_vector(). There are no circumstances under which we could know whether the IRQ or SEV arrives first at the destination CPU anyway. A DSB is insufficient since the store may still not have arrived at the GIC; but even doing a readback from the GIC isn't enough, because the relationships and relative speed of the underlying interrupt and SEV signalling mechanisms are not architecturally visible. The crucial thing is that the SEV does not arrive before the destination CPU has observed the modification to the entry vector -- that could cause the target CPU to stall in WFE. If the CPU powers up after missing the SEV, that's fine, because the observability of the non-NULL entry vector is guaranteed by the flushing which precedes gic_raise_softirq(). SEV is only important if the CPU powers up early, observes a NULL entry vector and goes into WFE. Because this code only applies with the multiprocessing extensions, we know that the writes associated with gic_raise_softirq() will drain and take effect eventually, but we don't care when. We can't know the results for sure until the target CPU re-enters the kernel. Cheers ---Dave
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile index 894c2ddf9b..59b36db7cc 100644 --- a/arch/arm/common/Makefile +++ b/arch/arm/common/Makefile @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o vlock.o diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c new file mode 100644 index 0000000000..0acb9f4685 --- /dev/null +++ b/arch/arm/common/bL_platsmp.c @@ -0,0 +1,79 @@ +/* + * linux/arch/arm/mach-vexpress/bL_platsmp.c + * + * Created by: Nicolas Pitre, November 2012 + * Copyright: (C) 2012 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Code to handle secondary CPU bringup and hotplug for the bL power API. + */ + +#include <linux/init.h> +#include <linux/smp.h> + +#include <asm/bL_entry.h> +#include <asm/smp_plat.h> +#include <asm/hardware/gic.h> + +static void __init simple_smp_init_cpus(void) +{ + set_smp_cross_call(gic_raise_softirq); +} + +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle) +{ + unsigned int pcpu, pcluster, ret; + extern void secondary_startup(void); + + pcpu = cpu_logical_map(cpu) & 0xff; + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff; + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n", + __func__, cpu, pcpu, pcluster); + + bL_set_entry_vector(pcpu, pcluster, NULL); + ret = bL_cpu_power_up(pcpu, pcluster); + if (ret) + return ret; + bL_set_entry_vector(pcpu, pcluster, secondary_startup); + gic_raise_softirq(cpumask_of(cpu), 0); + sev(); + return 0; +} + +static void __cpuinit bL_secondary_init(unsigned int cpu) +{ + bL_cpu_powered_up(); + gic_secondary_init(0); +} + +#ifdef CONFIG_HOTPLUG_CPU + +static int bL_cpu_disable(unsigned int cpu) +{ + /* + * We assume all CPUs may be shut down. + * This would be the hook to use for eventual Secure + * OS migration requests. + */ + return 0; +} + +static void __ref bL_cpu_die(unsigned int cpu) +{ + bL_cpu_power_down(); +} + +#endif + +struct smp_operations __initdata bL_smp_ops = { + .smp_init_cpus = simple_smp_init_cpus, + .smp_boot_secondary = bL_boot_secondary, + .smp_secondary_init = bL_secondary_init, +#ifdef CONFIG_HOTPLUG_CPU + .cpu_disable = bL_cpu_disable, + .cpu_die = bL_cpu_die, +#endif +};
Now that the b.L power API is in place, we can use it for SMP secondary bringup and CPU hotplug in a generic fashion. Signed-off-by: Nicolas Pitre <nico@linaro.org> --- arch/arm/common/Makefile | 2 +- arch/arm/common/bL_platsmp.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 arch/arm/common/bL_platsmp.c