| Message ID | 1435240970-30869-1-git-send-email-vitalya@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: > This commit add cpu_die implementation using psci api I don't understand. If you have a PSCI implementation, it should be sufficient to have a PSCI node (and enable-method) in your DT, and the generic code will be used. Nothing should be required in your board code. You should also use CPU_ON to bring secondaries online rather than mixing up PSCI and platform-specific mechanisms. > > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > --- > arch/arm/mach-keystone/platsmp.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/arm/mach-keystone/platsmp.c b/arch/arm/mach-keystone/platsmp.c > index 5f46a7c..2c40cc0 100644 > --- a/arch/arm/mach-keystone/platsmp.c > +++ b/arch/arm/mach-keystone/platsmp.c > @@ -20,6 +20,7 @@ > #include <asm/prom.h> > #include <asm/tlbflush.h> > #include <asm/pgtable.h> > +#include <asm/psci.h> > > #include "keystone.h" > > @@ -51,7 +52,38 @@ static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) > {} > #endif > > + > +#ifdef CONFIG_HOTPLUG_CPU > +static void keystone_cpu_die(unsigned int cpu) > +{ > +#ifdef CONFIG_ARM_PSCI > + struct psci_power_state pwr_state = {0, 0, 0}; > + > + pr_info("keystone_cpu_die(%d) from %d using PSCI\n", cpu, > + smp_processor_id()); > + > + if (psci_ops.cpu_off) > + psci_ops.cpu_off(pwr_state); > +#else > + /* > + * We may want to add here a direct smc call to monitor > + * if the kernel doesn't support PSCI API > + */ > +#endif You should determine this from your DT. Your FW/bootloader can patch in the relevant nodes and properties when support is present, so the presence of such nodes should guarantee that PSCI is available. > + > + /* > + * we shouldn't come here. But in case something went > + * wrong the code below prevents kernel from crush > + */ > + while (1) > + cpu_do_idle(); This will make kexec appear to work, yet it won't. Thanks, Mark. > +} > +#endif > + > struct smp_operations keystone_smp_ops __initdata = { > .smp_boot_secondary = keystone_smp_boot_secondary, > .smp_secondary_init = keystone_smp_secondary_initmem, > +#ifdef CONFIG_HOTPLUG_CPU > + .cpu_die = keystone_cpu_die, > +#endif > }; > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 6/25/2015 7:02 AM, Vitaly Andrianov wrote: > This commit add cpu_die implementation using psci api > > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > --- > arch/arm/mach-keystone/platsmp.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/arm/mach-keystone/platsmp.c b/arch/arm/mach-keystone/platsmp.c > index 5f46a7c..2c40cc0 100644 > --- a/arch/arm/mach-keystone/platsmp.c > +++ b/arch/arm/mach-keystone/platsmp.c > @@ -20,6 +20,7 @@ > #include <asm/prom.h> > #include <asm/tlbflush.h> > #include <asm/pgtable.h> > +#include <asm/psci.h> > > #include "keystone.h" > > @@ -51,7 +52,38 @@ static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) > {} > #endif > > + > +#ifdef CONFIG_HOTPLUG_CPU > +static void keystone_cpu_die(unsigned int cpu) > +{ > +#ifdef CONFIG_ARM_PSCI > + struct psci_power_state pwr_state = {0, 0, 0}; > + > + pr_info("keystone_cpu_die(%d) from %d using PSCI\n", cpu, > + smp_processor_id()); > + > + if (psci_ops.cpu_off) > + psci_ops.cpu_off(pwr_state); > +#else > + /* > + * We may want to add here a direct smc call to monitor > + * if the kernel doesn't support PSCI API > + */ > +#endif I don't see much value adding the SMC. I mean CPU_HOTPLUG works with PSCI o.w doesn't is good enough. If you still like to add it, just abstract above into two functions, one with PSCI and other with SMC. That way you can avoid that ugly #defines in the middle of the code. Regards, Santosh
On 6/25/2015 7:45 AM, Mark Rutland wrote: > Hi, > > On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: >> This commit add cpu_die implementation using psci api > > I don't understand. If you have a PSCI implementation, it should be > sufficient to have a PSCI node (and enable-method) in your DT, and the > generic code will be used. Nothing should be required in your board > code. > > You should also use CPU_ON to bring secondaries online rather than > mixing up PSCI and platform-specific mechanisms. > Good point about CPU_ON. We need that as well. >> >> Signed-off-by: Vitaly Andrianov <vitalya@ti.com> >> --- >> arch/arm/mach-keystone/platsmp.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/arch/arm/mach-keystone/platsmp.c b/arch/arm/mach-keystone/platsmp.c >> index 5f46a7c..2c40cc0 100644 >> --- a/arch/arm/mach-keystone/platsmp.c >> +++ b/arch/arm/mach-keystone/platsmp.c >> @@ -20,6 +20,7 @@ >> #include <asm/prom.h> >> #include <asm/tlbflush.h> >> #include <asm/pgtable.h> >> +#include <asm/psci.h> >> >> #include "keystone.h" >> >> @@ -51,7 +52,38 @@ static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) >> {} >> #endif >> >> + >> +#ifdef CONFIG_HOTPLUG_CPU >> +static void keystone_cpu_die(unsigned int cpu) >> +{ >> +#ifdef CONFIG_ARM_PSCI >> + struct psci_power_state pwr_state = {0, 0, 0}; >> + >> + pr_info("keystone_cpu_die(%d) from %d using PSCI\n", cpu, >> + smp_processor_id()); >> + >> + if (psci_ops.cpu_off) >> + psci_ops.cpu_off(pwr_state); >> +#else >> + /* >> + * We may want to add here a direct smc call to monitor >> + * if the kernel doesn't support PSCI API >> + */ >> +#endif > > You should determine this from your DT. Your FW/bootloader can patch in > the relevant nodes and properties when support is present, so the > presence of such nodes should guarantee that PSCI is available. > That will be a nice trick. FW already does some tweaks of dts for LPAE/non_LPAE tweaks. Regards, Santosh
On 06/25/2015 10:59 AM, santosh shilimkar wrote: > On 6/25/2015 7:45 AM, Mark Rutland wrote: >> Hi, >> >> On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: >>> This commit add cpu_die implementation using psci api >> >> I don't understand. If you have a PSCI implementation, it should be >> sufficient to have a PSCI node (and enable-method) in your DT, and the >> generic code will be used. Nothing should be required in your board >> code. >> >> You should also use CPU_ON to bring secondaries online rather than >> mixing up PSCI and platform-specific mechanisms. >> > Good point about CPU_ON. We need that as well. > Does it mean that keystone_defconfig must always have CONFIG_HOTPLUG_CPU and CONFIG_ARM_PSCI enabled? What is if someone doesn't want to have HOTPLUG_CPU? How he can boot secondary CPU w/o platform-specific mechanizm? >>> >>> Signed-off-by: Vitaly Andrianov <vitalya@ti.com> >>> --- >>> arch/arm/mach-keystone/platsmp.c | 32 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> diff --git a/arch/arm/mach-keystone/platsmp.c >>> b/arch/arm/mach-keystone/platsmp.c >>> index 5f46a7c..2c40cc0 100644 >>> --- a/arch/arm/mach-keystone/platsmp.c >>> +++ b/arch/arm/mach-keystone/platsmp.c >>> @@ -20,6 +20,7 @@ >>> #include <asm/prom.h> >>> #include <asm/tlbflush.h> >>> #include <asm/pgtable.h> >>> +#include <asm/psci.h> >>> >>> #include "keystone.h" >>> >>> @@ -51,7 +52,38 @@ static inline void __cpuinit >>> keystone_smp_secondary_initmem(unsigned int cpu) >>> {} >>> #endif >>> >>> + >>> +#ifdef CONFIG_HOTPLUG_CPU >>> +static void keystone_cpu_die(unsigned int cpu) >>> +{ >>> +#ifdef CONFIG_ARM_PSCI >>> + struct psci_power_state pwr_state = {0, 0, 0}; >>> + >>> + pr_info("keystone_cpu_die(%d) from %d using PSCI\n", cpu, >>> + smp_processor_id()); >>> + >>> + if (psci_ops.cpu_off) >>> + psci_ops.cpu_off(pwr_state); >>> +#else >>> + /* >>> + * We may want to add here a direct smc call to monitor >>> + * if the kernel doesn't support PSCI API >>> + */ >>> +#endif >> >> You should determine this from your DT. Your FW/bootloader can patch in >> the relevant nodes and properties when support is present, so the >> presence of such nodes should guarantee that PSCI is available. >> > That will be a nice trick. FW already does some tweaks of dts for > LPAE/non_LPAE tweaks. > > Regards, > Santosh Thanks, -Vitaly
On Thu, Jun 25, 2015 at 05:01:36PM +0100, Vitaly Andrianov wrote: > > > On 06/25/2015 10:59 AM, santosh shilimkar wrote: > > On 6/25/2015 7:45 AM, Mark Rutland wrote: > >> Hi, > >> > >> On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: > >>> This commit add cpu_die implementation using psci api > >> > >> I don't understand. If you have a PSCI implementation, it should be > >> sufficient to have a PSCI node (and enable-method) in your DT, and the > >> generic code will be used. Nothing should be required in your board > >> code. > >> > >> You should also use CPU_ON to bring secondaries online rather than > >> mixing up PSCI and platform-specific mechanisms. > >> > > Good point about CPU_ON. We need that as well. > > > Does it mean that keystone_defconfig must always have CONFIG_HOTPLUG_CPU > and CONFIG_ARM_PSCI enabled? No. You need CONFIG_ARM_PSCI for PSCI CPU_ON. Without CONFIG_HOTPLUG_CPU, you'll get PSCI CPU_ON but not PSCI CPU_OFF. > What is if someone doesn't want to have HOTPLUG_CPU? Then you don't get PSCI CPU_OFF, but can still have PSCI CPU_ON. Only the portions of the PSCI code required for turning CPUs off are dependent on HOTPLUG_CPU. > How he can boot secondary CPU w/o platform-specific mechanizm? By using PSCI CPU_ON. Thanks, Mark
On 06/25/2015 12:13 PM, Mark Rutland wrote: > On Thu, Jun 25, 2015 at 05:01:36PM +0100, Vitaly Andrianov wrote: >> >> >> On 06/25/2015 10:59 AM, santosh shilimkar wrote: >>> On 6/25/2015 7:45 AM, Mark Rutland wrote: >>>> Hi, >>>> >>>> On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: >>>>> This commit add cpu_die implementation using psci api >>>> >>>> I don't understand. If you have a PSCI implementation, it should be >>>> sufficient to have a PSCI node (and enable-method) in your DT, and the >>>> generic code will be used. Nothing should be required in your board >>>> code. >>>> >>>> You should also use CPU_ON to bring secondaries online rather than >>>> mixing up PSCI and platform-specific mechanisms. >>>> >>> Good point about CPU_ON. We need that as well. >>> >> Does it mean that keystone_defconfig must always have CONFIG_HOTPLUG_CPU >> and CONFIG_ARM_PSCI enabled? > > No. > > You need CONFIG_ARM_PSCI for PSCI CPU_ON. Without CONFIG_HOTPLUG_CPU, > you'll get PSCI CPU_ON but not PSCI CPU_OFF. > >> What is if someone doesn't want to have HOTPLUG_CPU? > > Then you don't get PSCI CPU_OFF, but can still have PSCI CPU_ON. > > Only the portions of the PSCI code required for turning CPUs off are > dependent on HOTPLUG_CPU. > >> How he can boot secondary CPU w/o platform-specific mechanizm? > > By using PSCI CPU_ON. > > Thanks, > Mark > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > Thanks Mark, I need rework and re-test the patch. One more question. Shall I post the dts related commit, which add PSCI command together with this commit? Or it may be posted later independently? Thanks, Vitaly
On Thu, Jun 25, 2015 at 05:55:40PM +0100, Vitaly Andrianov wrote: > > > On 06/25/2015 12:13 PM, Mark Rutland wrote: > > On Thu, Jun 25, 2015 at 05:01:36PM +0100, Vitaly Andrianov wrote: > >> > >> > >> On 06/25/2015 10:59 AM, santosh shilimkar wrote: > >>> On 6/25/2015 7:45 AM, Mark Rutland wrote: > >>>> Hi, > >>>> > >>>> On Thu, Jun 25, 2015 at 03:02:50PM +0100, Vitaly Andrianov wrote: > >>>>> This commit add cpu_die implementation using psci api > >>>> > >>>> I don't understand. If you have a PSCI implementation, it should be > >>>> sufficient to have a PSCI node (and enable-method) in your DT, and the > >>>> generic code will be used. Nothing should be required in your board > >>>> code. > >>>> > >>>> You should also use CPU_ON to bring secondaries online rather than > >>>> mixing up PSCI and platform-specific mechanisms. > >>>> > >>> Good point about CPU_ON. We need that as well. > >>> > >> Does it mean that keystone_defconfig must always have CONFIG_HOTPLUG_CPU > >> and CONFIG_ARM_PSCI enabled? > > > > No. > > > > You need CONFIG_ARM_PSCI for PSCI CPU_ON. Without CONFIG_HOTPLUG_CPU, > > you'll get PSCI CPU_ON but not PSCI CPU_OFF. > > > >> What is if someone doesn't want to have HOTPLUG_CPU? > > > > Then you don't get PSCI CPU_OFF, but can still have PSCI CPU_ON. > > > > Only the portions of the PSCI code required for turning CPUs off are > > dependent on HOTPLUG_CPU. > > > >> How he can boot secondary CPU w/o platform-specific mechanizm? > > > > By using PSCI CPU_ON. > > > > Thanks, > > Mark > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > Thanks Mark, > > I need rework and re-test the patch. > One more question. Shall I post the dts related commit, which add PSCI > command together with this commit? Or it may be posted later independently? The DTS and Kconfig changes can be seaprate patches, but they'll need to go through at the same time. You shouldn't need to add any platform code, but you may need to add a some guards to support existing (non-PSCI) systems and PSCI systems with the same kernel if your board code unconditionally pokes hardware that the PSCI implementation will be in charge of (with PSCI the kernel should no nothing about said hardware). Thanks, Mark.
> > I need rework and re-test the patch. > > One more question. Shall I post the dts related commit, which add PSCI > > command together with this commit? Or it may be posted later independently? > > The DTS and Kconfig changes can be seaprate patches, but they'll need to > go through at the same time. If your bootloader patches the DTB then you don't even need a dts update. That should make things less confusing for existing users... Thanks, Mark.
On 6/25/2015 10:20 AM, Mark Rutland wrote: >>> I need rework and re-test the patch. >>> One more question. Shall I post the dts related commit, which add PSCI >>> command together with this commit? Or it may be posted later independently? >> >> The DTS and Kconfig changes can be seaprate patches, but they'll need to >> go through at the same time. > > If your bootloader patches the DTB then you don't even need a dts > update. That should make things less confusing for existing users... > More than confusing we need to keep existing DTB binding work with updated kernel at least for as basic as booting all the CPUs. Regards, Santosh
On 06/25/2015 02:42 PM, santosh shilimkar wrote: > On 6/25/2015 10:20 AM, Mark Rutland wrote: >>>> I need rework and re-test the patch. >>>> One more question. Shall I post the dts related commit, which add PSCI >>>> command together with this commit? Or it may be posted later >>>> independently? >>> >>> The DTS and Kconfig changes can be seaprate patches, but they'll need to >>> go through at the same time. >> >> If your bootloader patches the DTB then you don't even need a dts >> update. That should make things less confusing for existing users... >> > More than confusing we need to keep existing DTB binding work with > updated kernel at least for as basic as booting all the CPUs. > > Regards, > Santosh > > OK. Now I'm confused :) We may have several different configurations here: 1) CONFIG_HOTPLUG_CPU and CONFIG_ARM_PSCI are not set. In this case keystone arch needs to have keystone_smp_boot_secondary(); 2) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI is not set. keystone_smp_boot_secondary() is required and non PSCI implementation of keystone_cpu_die() is also required. 3) CONFIG_HOTPLUG_CPU is not set and CONFIG_ARM_PSCI=y 4) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI=y How do I boot secondary CPUs in cases of 3 and 4? Do I need to implement PSCI version of the keystone_smp_boot_secondary() of adding PSCI commands to DTB is enough? Do I need to implement keystone_cpu_die() if PSCI commands are added to DTB? Thanks, Vitaly
> OK. Now I'm confused :) We may have several different configurations here: > > 1) CONFIG_HOTPLUG_CPU and CONFIG_ARM_PSCI are not set. > In this case keystone arch needs to have > keystone_smp_boot_secondary(); > > 2) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI is not set. > keystone_smp_boot_secondary() is required and non PSCI > implementation of keystone_cpu_die() is also required. > > 3) CONFIG_HOTPLUG_CPU is not set and CONFIG_ARM_PSCI=y > 4) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI=y > > How do I boot secondary CPUs in cases of 3 and 4? > Do I need to implement PSCI version of the > keystone_smp_boot_secondary() of adding PSCI commands to DTB is > enough? The DTB additions alone _should_ be sufficient. > Do I need to implement keystone_cpu_die() if PSCI commands are > added to DTB? You _should not_ need to add any platform code to use PSCI. However, if you have existing platform code which pokes the hardware logically owned by your FW PSCI implementation, you need to ensure that this code doesn't poke that hardware when PSCI is in use. Thanks, Mark.
Hi, On 06/26/2015 07:57 PM, Vitaly Andrianov wrote: > On 06/25/2015 02:42 PM, santosh shilimkar wrote: >> On 6/25/2015 10:20 AM, Mark Rutland wrote: >>>>> I need rework and re-test the patch. >>>>> One more question. Shall I post the dts related commit, which add PSCI >>>>> command together with this commit? Or it may be posted later >>>>> independently? >>>> >>>> The DTS and Kconfig changes can be seaprate patches, but they'll >>>> need to >>>> go through at the same time. >>> >>> If your bootloader patches the DTB then you don't even need a dts >>> update. That should make things less confusing for existing users... >>> >> More than confusing we need to keep existing DTB binding work with >> updated kernel at least for as basic as booting all the CPUs. >> >> Regards, >> Santosh >> >> > OK. Now I'm confused :) We may have several different configurations here: > > 1) CONFIG_HOTPLUG_CPU and CONFIG_ARM_PSCI are not set. > In this case keystone arch needs to have > keystone_smp_boot_secondary(); > > 2) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI is not set. > keystone_smp_boot_secondary() is required and non PSCI > implementation of keystone_cpu_die() is also required. > > 3) CONFIG_HOTPLUG_CPU is not set and CONFIG_ARM_PSCI=y > 4) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI=y > > How do I boot secondary CPUs in cases of 3 and 4? > Do I need to implement PSCI version of the > keystone_smp_boot_secondary() of adding PSCI commands to DTB is > enough? > > Do I need to implement keystone_cpu_die() if PSCI commands are > added to DTB? Things are more or less simple here :) 1) to support psci you need to have DT entry like below: psci { compatible = "arm,psci"; method = "smc"; cpu_off = <0x84000002>; cpu_on = <0x84000003>; }; and CONFIG_ARM_PSCI=y in this case Kernel will ignore mach.smp = smp_ops(keystone_smp_ops), and will use PSCU interface (see setup_arch()0 2) if don't have PSCI DT entry, but still have custom smp_operations - they will be used. So, question here is not about CONFIG_HOTPLUG_CPU, but rather will you support "PSCI only" or "PSCI and legacy boot". For the last case You should keep mach specific code in mach-kestone/platsmp.c. For the first case "PSCI only" - above code can be removed. 3) if you'd like CONFIG_HOTPLUG_CPU in legacy mode - platsmp.c can be updated as below, without using psci: +#define KEYSTONE_MON_CPU_DOWN_IDX 0x01 +#ifdef CONFIG_HOTPLUG_CPU +static void __ref keystone_smp_cpu_die(unsigned int cpu) +{ + int error; + + error = keystone_cpu_smc(KEYSTONE_MON_CPU_DOWN_IDX, cpu, 0); + if (error) + pr_err("CPU %u->%u down failed with %d\n", + smp_processor_id(), cpu, error); + + cpu_do_idle(); +} +#endif Another question is how well current PSCI implementation supports keystone2/LPAE !? - It seems, at least below hack should be applied :( +++ b/arch/arm/kernel/psci_smp.c @@ -51,7 +51,7 @@ static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle) { if (psci_ops.cpu_on) return psci_ops.cpu_on(cpu_logical_map(cpu), - __pa(secondary_startup)); + virt_to_idmap(&secondary_startup)); return -ENODEV; } - and what to do with code in keystone_smp_secondary_initmem() ?: static void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) { pgd_t *pgd0 = pgd_offset_k(0); cpu_set_ttbr(1, __pa(pgd0) + TTBR1_OFFSET); local_flush_tlb_all(); }
On 06/26/2015 01:47 PM, Grygorii Strashko wrote: > Hi, > > On 06/26/2015 07:57 PM, Vitaly Andrianov wrote: >> On 06/25/2015 02:42 PM, santosh shilimkar wrote: >>> On 6/25/2015 10:20 AM, Mark Rutland wrote: >>>>>> I need rework and re-test the patch. >>>>>> One more question. Shall I post the dts related commit, which add PSCI >>>>>> command together with this commit? Or it may be posted later >>>>>> independently? >>>>> >>>>> The DTS and Kconfig changes can be seaprate patches, but they'll >>>>> need to >>>>> go through at the same time. >>>> >>>> If your bootloader patches the DTB then you don't even need a dts >>>> update. That should make things less confusing for existing users... >>>> >>> More than confusing we need to keep existing DTB binding work with >>> updated kernel at least for as basic as booting all the CPUs. >>> >>> Regards, >>> Santosh >>> >>> >> OK. Now I'm confused :) We may have several different configurations here: >> >> 1) CONFIG_HOTPLUG_CPU and CONFIG_ARM_PSCI are not set. >> In this case keystone arch needs to have >> keystone_smp_boot_secondary(); >> >> 2) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI is not set. >> keystone_smp_boot_secondary() is required and non PSCI >> implementation of keystone_cpu_die() is also required. >> >> 3) CONFIG_HOTPLUG_CPU is not set and CONFIG_ARM_PSCI=y >> 4) CONFIG_HOTPLUG_CPU=y and CONFIG_ARM_PSCI=y >> >> How do I boot secondary CPUs in cases of 3 and 4? >> Do I need to implement PSCI version of the >> keystone_smp_boot_secondary() of adding PSCI commands to DTB is >> enough? >> >> Do I need to implement keystone_cpu_die() if PSCI commands are >> added to DTB? > > Things are more or less simple here :) > 1) to support psci you need to have DT entry like below: > psci { > compatible = "arm,psci"; > method = "smc"; > cpu_off = <0x84000002>; > cpu_on = <0x84000003>; > }; > and CONFIG_ARM_PSCI=y > > in this case Kernel will ignore mach.smp = smp_ops(keystone_smp_ops), > and will use PSCU interface (see setup_arch()0 > > 2) if don't have PSCI DT entry, but still have custom smp_operations - > they will be used. > > So, question here is not about CONFIG_HOTPLUG_CPU, but rather > will you support "PSCI only" or "PSCI and legacy boot". > > For the last case You should keep mach specific code in mach-kestone/platsmp.c. > For the first case "PSCI only" - above code can be removed. > > 3) if you'd like CONFIG_HOTPLUG_CPU in legacy mode - platsmp.c can be updated as below, > without using psci: > > > +#define KEYSTONE_MON_CPU_DOWN_IDX 0x01 > +#ifdef CONFIG_HOTPLUG_CPU > +static void __ref keystone_smp_cpu_die(unsigned int cpu) > +{ > + int error; > + > + error = keystone_cpu_smc(KEYSTONE_MON_CPU_DOWN_IDX, cpu, 0); > + if (error) > + pr_err("CPU %u->%u down failed with %d\n", > + smp_processor_id(), cpu, error); > + > + cpu_do_idle(); > +} > +#endif > > Another question is how well current PSCI implementation supports keystone2/LPAE !? > That is exactly what I'm working on now :) > - It seems, at least below hack should be applied :( > > +++ b/arch/arm/kernel/psci_smp.c > @@ -51,7 +51,7 @@ static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle) > { > if (psci_ops.cpu_on) > return psci_ops.cpu_on(cpu_logical_map(cpu), > - __pa(secondary_startup)); > + virt_to_idmap(&secondary_startup)); > return -ENODEV; > } > > - and what to do with code in keystone_smp_secondary_initmem() ?: > > static void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) > { > pgd_t *pgd0 = pgd_offset_k(0); > cpu_set_ttbr(1, __pa(pgd0) + TTBR1_OFFSET); > local_flush_tlb_all(); > } > > Thanks, Vitaly
On 6/26/2015 10:47 AM, Grygorii Strashko wrote: > Hi, > > On 06/26/2015 07:57 PM, Vitaly Andrianov wrote: >> On 06/25/2015 02:42 PM, santosh shilimkar wrote: > Another question is how well current PSCI implementation supports keystone2/LPAE !? > > - It seems, at least below hack should be applied :( > > +++ b/arch/arm/kernel/psci_smp.c > @@ -51,7 +51,7 @@ static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle) > { > if (psci_ops.cpu_on) > return psci_ops.cpu_on(cpu_logical_map(cpu), > - __pa(secondary_startup)); > + virt_to_idmap(&secondary_startup)); This is legitimate change should be there irrespectively. I think you should post this as a proper patch. Regards, Santosh
diff --git a/arch/arm/mach-keystone/platsmp.c b/arch/arm/mach-keystone/platsmp.c index 5f46a7c..2c40cc0 100644 --- a/arch/arm/mach-keystone/platsmp.c +++ b/arch/arm/mach-keystone/platsmp.c @@ -20,6 +20,7 @@ #include <asm/prom.h> #include <asm/tlbflush.h> #include <asm/pgtable.h> +#include <asm/psci.h> #include "keystone.h" @@ -51,7 +52,38 @@ static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu) {} #endif + +#ifdef CONFIG_HOTPLUG_CPU +static void keystone_cpu_die(unsigned int cpu) +{ +#ifdef CONFIG_ARM_PSCI + struct psci_power_state pwr_state = {0, 0, 0}; + + pr_info("keystone_cpu_die(%d) from %d using PSCI\n", cpu, + smp_processor_id()); + + if (psci_ops.cpu_off) + psci_ops.cpu_off(pwr_state); +#else + /* + * We may want to add here a direct smc call to monitor + * if the kernel doesn't support PSCI API + */ +#endif + + /* + * we shouldn't come here. But in case something went + * wrong the code below prevents kernel from crush + */ + while (1) + cpu_do_idle(); +} +#endif + struct smp_operations keystone_smp_ops __initdata = { .smp_boot_secondary = keystone_smp_boot_secondary, .smp_secondary_init = keystone_smp_secondary_initmem, +#ifdef CONFIG_HOTPLUG_CPU + .cpu_die = keystone_cpu_die, +#endif };
This commit add cpu_die implementation using psci api Signed-off-by: Vitaly Andrianov <vitalya@ti.com> --- arch/arm/mach-keystone/platsmp.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)