diff for duplicates of <1427973405260.57477@freescale.com> diff --git a/a/1.txt b/N1/1.txt index e3cbcbf..160ae5a 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -4,8 +4,7 @@ ________________________________________ From: Wood Scott-B07421 Sent: Tuesday, March 31, 2015 10:07 To: Zhao Chenhui-B35336 -Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel= -@vger.kernel.org; Jin Zhengxiong-R64188 +Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Jin Zhengxiong-R64188 Subject: Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500 On Thu, Mar 26, 2015 at 06:18:14PM +0800, chenhui zhao wrote: @@ -26,8 +25,7 @@ On Thu, Mar 26, 2015 at 06:18:14PM +0800, chenhui zhao wrote: > arch/powerpc/include/asm/smp.h | 2 + > arch/powerpc/kernel/head_64.S | 20 +++-- > arch/powerpc/kernel/smp.c | 5 ++ -> arch/powerpc/platforms/85xx/smp.c | 182 +++++++++++++++++++++++++++++---= ------- +> arch/powerpc/platforms/85xx/smp.c | 182 +++++++++++++++++++++++++++++--------- > arch/powerpc/sysdev/fsl_rcpm.c | 56 ++++++++++++ > 7 files changed, 220 insertions(+), 51 deletions(-) @@ -50,8 +48,7 @@ explanations. > ---help--- > Say Y here to be able to disable and re-enable individual > CPUs at runtime on SMP machines. -> diff --git a/arch/powerpc/include/asm/fsl_pm.h b/arch/powerpc/include/asm= -/fsl_pm.h +> diff --git a/arch/powerpc/include/asm/fsl_pm.h b/arch/powerpc/include/asm/fsl_pm.h > index bbe6089..579f495 100644 > --- a/arch/powerpc/include/asm/fsl_pm.h > +++ b/arch/powerpc/include/asm/fsl_pm.h @@ -111,8 +108,7 @@ If __cur_boot_cpu is meant to be the PIR of the currently booting CPU, it's a misleading. It looks like it's supposed to have something to do with the boot cpu (not "booting"). -[chenhui] I mean the PIR of the currently booting CPU. Change to "booting_c= -pu_hwid". +[chenhui] I mean the PIR of the currently booting CPU. Change to "booting_cpu_hwid". Also please don't put leading underscores on symbols just because the adjacent symbols have them. @@ -121,7 +117,7 @@ adjacent symbols have them. > +#ifdef CONFIG_PPC_E500MC > +static void qoriq_cpu_wait_die(void) > +{ -> + unsigned int cpu =3D smp_processor_id(); +> + unsigned int cpu = smp_processor_id(); > + > + hard_irq_disable(); > + /* mask all irqs to prevent cpu wakeup */ @@ -145,29 +141,28 @@ Indent the ; [chenhui] OK -> @@ -174,17 +232,29 @@ static inline u32 read_spin_table_addr_l(void *spin= -_table) +> @@ -174,17 +232,29 @@ static inline u32 read_spin_table_addr_l(void *spin_table) > static void wake_hw_thread(void *info) > { > void fsl_secondary_thread_init(void); > - unsigned long imsr1, inia1; > + unsigned long imsr, inia; -> int nr =3D *(const int *)info; +> int nr = *(const int *)info; > - -> - imsr1 =3D MSR_KERNEL; -> - inia1 =3D *(unsigned long *)fsl_secondary_thread_init; +> - imsr1 = MSR_KERNEL; +> - inia1 = *(unsigned long *)fsl_secondary_thread_init; > - > - mttmr(TMRN_IMSR1, imsr1); > - mttmr(TMRN_INIA1, inia1); > - mtspr(SPRN_TENS, TEN_THREAD(1)); -> + int hw_cpu =3D get_hard_smp_processor_id(nr); -> + int thread_idx =3D cpu_thread_in_core(hw_cpu); +> + int hw_cpu = get_hard_smp_processor_id(nr); +> + int thread_idx = cpu_thread_in_core(hw_cpu); > + -> + __cur_boot_cpu =3D (u32)hw_cpu; -> + imsr =3D MSR_KERNEL; -> + inia =3D *(unsigned long *)fsl_secondary_thread_init; +> + __cur_boot_cpu = (u32)hw_cpu; +> + imsr = MSR_KERNEL; +> + inia = *(unsigned long *)fsl_secondary_thread_init; > + smp_mb(); -> + if (thread_idx =3D=3D 0) { +> + if (thread_idx == 0) { > + mttmr(TMRN_IMSR0, imsr); > + mttmr(TMRN_INIA0, inia); > + } else { @@ -193,7 +188,7 @@ it's really needed. In general, this patch tries to do too much at once. > pr_debug("smp_85xx_kick_cpu: kick CPU #%d\n", nr); > > +#ifdef CONFIG_HOTPLUG_CPU -> + sync_tb =3D 0; +> + sync_tb = 0; > + smp_mb(); > +#endif @@ -201,14 +196,14 @@ Timebase synchronization should also be separate. > #ifdef CONFIG_PPC64 > - /* Threads don't use the spin table */ -> - if (cpu_thread_in_core(nr) !=3D 0) { +> - if (cpu_thread_in_core(nr) != 0) { > + if (threads_per_core > 1) { -> int primary =3D cpu_first_thread_sibling(nr); +> int primary = cpu_first_thread_sibling(nr); > > if (WARN_ON_ONCE(!cpu_has_feature(CPU_FTR_SMT))) > return -ENOENT; > -> - if (cpu_thread_in_core(nr) !=3D 1) { +> - if (cpu_thread_in_core(nr) != 1) { > - pr_err("%s: cpu %d: invalid hw thread %d\n", > - __func__, nr, cpu_thread_in_core(nr)); > - return -ENOENT; @@ -247,24 +242,21 @@ Where does this reset occur? [chenhui] Reset occurs in the function mpic_reset_core(). -> + if (hw_cpu !=3D cpu_first_thread_sibling(hw_cpu)) { +> + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) { > + int hw_cpu1, hw_cpu2; > + -> + hw_cpu1 =3D get_hard_smp_processor_id(primary); -> + hw_cpu2 =3D get_hard_smp_processor_id(primary + 1); +> + hw_cpu1 = get_hard_smp_processor_id(primary); +> + hw_cpu2 = get_hard_smp_processor_id(primary + 1); > + set_hard_smp_processor_id(primary, hw_cpu2); > + set_hard_smp_processor_id(primary + 1, hw_cpu1); > + /* get new physical cpu id */ -> + hw_cpu =3D get_hard_smp_processor_id(nr); +> + hw_cpu = get_hard_smp_processor_id(nr); Why are you swapping the hard smp ids? -[chenhui] For example, Core1 has two threads, Thread0 and Thread1. In norma= -l boot, Thread0 is CPU2, and Thread1 is CPU3. -But, if CPU2 and CPU3 are all off, user wants CPU3 up first. we need to cal= -l Thread0 as CPU3 and Thead1 as CPU2, considering -the limitation, after core is reset, only Thread0 is up, then Thread0 kicks= - up Thread1. +[chenhui] For example, Core1 has two threads, Thread0 and Thread1. In normal boot, Thread0 is CPU2, and Thread1 is CPU3. +But, if CPU2 and CPU3 are all off, user wants CPU3 up first. we need to call Thread0 as CPU3 and Thead1 as CPU2, considering +the limitation, after core is reset, only Thread0 is up, then Thread0 kicks up Thread1. > } > - @@ -274,7 +266,7 @@ the limitation, after core is reset, only Thread0 is up, then Thread0 kicks= > #endif > > @@ -252,11 +340,7 @@ static int smp_85xx_kick_cpu(int nr) -> spin_table =3D phys_to_virt(*cpu_rel_addr); +> spin_table = phys_to_virt(*cpu_rel_addr); > > local_irq_save(flags); > -#ifdef CONFIG_PPC32 @@ -287,7 +279,7 @@ Why did you move this? [chenhui] It would be better to set this after CPU is really up. -> if (system_state =3D=3D SYSTEM_RUNNING) { +> if (system_state == SYSTEM_RUNNING) { > /* > * To keep it compatible with old boot program which uses > @@ -269,11 +353,16 @@ static int smp_85xx_kick_cpu(int nr) @@ -305,22 +297,22 @@ doesn't see SMT). > __func__); > return; > } -> - smp_85xx_ops.give_timebase =3D mpc85xx_give_timebase; -> - smp_85xx_ops.take_timebase =3D mpc85xx_take_timebase; +> - smp_85xx_ops.give_timebase = mpc85xx_give_timebase; +> - smp_85xx_ops.take_timebase = mpc85xx_take_timebase; > -#ifdef CONFIG_HOTPLUG_CPU -> - ppc_md.cpu_die =3D smp_85xx_mach_cpu_die; +> - ppc_md.cpu_die = smp_85xx_mach_cpu_die; > -#endif You're moving this from a place that only runs when guts is found... > } > -> + smp_85xx_ops.cpu_die =3D generic_cpu_die; -> + ppc_md.cpu_die =3D smp_85xx_mach_cpu_die; +> + smp_85xx_ops.cpu_die = generic_cpu_die; +> + ppc_md.cpu_die = smp_85xx_mach_cpu_die; > +#endif -> + smp_85xx_ops.give_timebase =3D mpc85xx_give_timebase; -> + smp_85xx_ops.take_timebase =3D mpc85xx_take_timebase; -> + smp_85xx_ops.cpu_disable =3D generic_cpu_disable; +> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase; +> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase; +> + smp_85xx_ops.cpu_disable = generic_cpu_disable; > +#endif /* CONFIG_HOTPLUG_CPU */ ...to a place that runs unconditionally. Again, you're breaking VM @@ -328,4 +320,4 @@ guests. -Scott -[chenhui] Will correct it.= +[chenhui] Will correct it. diff --git a/a/content_digest b/N1/content_digest index 5c72a0c..bffe171 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -4,8 +4,8 @@ "Subject\0Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500\0" "Date\0Thu, 2 Apr 2015 11:16:45 +0000\0" "To\0Scott Wood <scottwood@freescale.com>\0" - "Cc\0devicetree@vger.kernel.org <devicetree@vger.kernel.org>" - linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org> + "Cc\0linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>" + devicetree@vger.kernel.org <devicetree@vger.kernel.org> linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> " Jason.Jin@freescale.com <Jason.Jin@freescale.com>\0" "\00:1\0" @@ -16,8 +16,7 @@ "From: Wood Scott-B07421\n" "Sent: Tuesday, March 31, 2015 10:07\n" "To: Zhao Chenhui-B35336\n" - "Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel=\n" - "@vger.kernel.org; Jin Zhengxiong-R64188\n" + "Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Jin Zhengxiong-R64188\n" "Subject: Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500\n" "\n" "On Thu, Mar 26, 2015 at 06:18:14PM +0800, chenhui zhao wrote:\n" @@ -38,8 +37,7 @@ "> arch/powerpc/include/asm/smp.h | 2 +\n" "> arch/powerpc/kernel/head_64.S | 20 +++--\n" "> arch/powerpc/kernel/smp.c | 5 ++\n" - "> arch/powerpc/platforms/85xx/smp.c | 182 +++++++++++++++++++++++++++++---=\n" - "------\n" + "> arch/powerpc/platforms/85xx/smp.c | 182 +++++++++++++++++++++++++++++---------\n" "> arch/powerpc/sysdev/fsl_rcpm.c | 56 ++++++++++++\n" "> 7 files changed, 220 insertions(+), 51 deletions(-)\n" "\n" @@ -62,8 +60,7 @@ "> ---help---\n" "> Say Y here to be able to disable and re-enable individual\n" "> CPUs at runtime on SMP machines.\n" - "> diff --git a/arch/powerpc/include/asm/fsl_pm.h b/arch/powerpc/include/asm=\n" - "/fsl_pm.h\n" + "> diff --git a/arch/powerpc/include/asm/fsl_pm.h b/arch/powerpc/include/asm/fsl_pm.h\n" "> index bbe6089..579f495 100644\n" "> --- a/arch/powerpc/include/asm/fsl_pm.h\n" "> +++ b/arch/powerpc/include/asm/fsl_pm.h\n" @@ -123,8 +120,7 @@ "it's a misleading. It looks like it's supposed to have something to do\n" "with the boot cpu (not \"booting\").\n" "\n" - "[chenhui] I mean the PIR of the currently booting CPU. Change to \"booting_c=\n" - "pu_hwid\".\n" + "[chenhui] I mean the PIR of the currently booting CPU. Change to \"booting_cpu_hwid\".\n" "\n" "Also please don't put leading underscores on symbols just because the\n" "adjacent symbols have them.\n" @@ -133,7 +129,7 @@ "> +#ifdef CONFIG_PPC_E500MC\n" "> +static void qoriq_cpu_wait_die(void)\n" "> +{\n" - "> + unsigned int cpu =3D smp_processor_id();\n" + "> + unsigned int cpu = smp_processor_id();\n" "> +\n" "> + hard_irq_disable();\n" "> + /* mask all irqs to prevent cpu wakeup */\n" @@ -157,29 +153,28 @@ "\n" "[chenhui] OK\n" "\n" - "> @@ -174,17 +232,29 @@ static inline u32 read_spin_table_addr_l(void *spin=\n" - "_table)\n" + "> @@ -174,17 +232,29 @@ static inline u32 read_spin_table_addr_l(void *spin_table)\n" "> static void wake_hw_thread(void *info)\n" "> {\n" "> void fsl_secondary_thread_init(void);\n" "> - unsigned long imsr1, inia1;\n" "> + unsigned long imsr, inia;\n" - "> int nr =3D *(const int *)info;\n" + "> int nr = *(const int *)info;\n" "> -\n" - "> - imsr1 =3D MSR_KERNEL;\n" - "> - inia1 =3D *(unsigned long *)fsl_secondary_thread_init;\n" + "> - imsr1 = MSR_KERNEL;\n" + "> - inia1 = *(unsigned long *)fsl_secondary_thread_init;\n" "> -\n" "> - mttmr(TMRN_IMSR1, imsr1);\n" "> - mttmr(TMRN_INIA1, inia1);\n" "> - mtspr(SPRN_TENS, TEN_THREAD(1));\n" - "> + int hw_cpu =3D get_hard_smp_processor_id(nr);\n" - "> + int thread_idx =3D cpu_thread_in_core(hw_cpu);\n" + "> + int hw_cpu = get_hard_smp_processor_id(nr);\n" + "> + int thread_idx = cpu_thread_in_core(hw_cpu);\n" "> +\n" - "> + __cur_boot_cpu =3D (u32)hw_cpu;\n" - "> + imsr =3D MSR_KERNEL;\n" - "> + inia =3D *(unsigned long *)fsl_secondary_thread_init;\n" + "> + __cur_boot_cpu = (u32)hw_cpu;\n" + "> + imsr = MSR_KERNEL;\n" + "> + inia = *(unsigned long *)fsl_secondary_thread_init;\n" "> + smp_mb();\n" - "> + if (thread_idx =3D=3D 0) {\n" + "> + if (thread_idx == 0) {\n" "> + mttmr(TMRN_IMSR0, imsr);\n" "> + mttmr(TMRN_INIA0, inia);\n" "> + } else {\n" @@ -205,7 +200,7 @@ "> pr_debug(\"smp_85xx_kick_cpu: kick CPU #%d\\n\", nr);\n" ">\n" "> +#ifdef CONFIG_HOTPLUG_CPU\n" - "> + sync_tb =3D 0;\n" + "> + sync_tb = 0;\n" "> + smp_mb();\n" "> +#endif\n" "\n" @@ -213,14 +208,14 @@ "\n" "> #ifdef CONFIG_PPC64\n" "> - /* Threads don't use the spin table */\n" - "> - if (cpu_thread_in_core(nr) !=3D 0) {\n" + "> - if (cpu_thread_in_core(nr) != 0) {\n" "> + if (threads_per_core > 1) {\n" - "> int primary =3D cpu_first_thread_sibling(nr);\n" + "> int primary = cpu_first_thread_sibling(nr);\n" ">\n" "> if (WARN_ON_ONCE(!cpu_has_feature(CPU_FTR_SMT)))\n" "> return -ENOENT;\n" ">\n" - "> - if (cpu_thread_in_core(nr) !=3D 1) {\n" + "> - if (cpu_thread_in_core(nr) != 1) {\n" "> - pr_err(\"%s: cpu %d: invalid hw thread %d\\n\",\n" "> - __func__, nr, cpu_thread_in_core(nr));\n" "> - return -ENOENT;\n" @@ -259,24 +254,21 @@ "\n" "[chenhui] Reset occurs in the function mpic_reset_core().\n" "\n" - "> + if (hw_cpu !=3D cpu_first_thread_sibling(hw_cpu)) {\n" + "> + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {\n" "> + int hw_cpu1, hw_cpu2;\n" "> +\n" - "> + hw_cpu1 =3D get_hard_smp_processor_id(primary);\n" - "> + hw_cpu2 =3D get_hard_smp_processor_id(primary + 1);\n" + "> + hw_cpu1 = get_hard_smp_processor_id(primary);\n" + "> + hw_cpu2 = get_hard_smp_processor_id(primary + 1);\n" "> + set_hard_smp_processor_id(primary, hw_cpu2);\n" "> + set_hard_smp_processor_id(primary + 1, hw_cpu1);\n" "> + /* get new physical cpu id */\n" - "> + hw_cpu =3D get_hard_smp_processor_id(nr);\n" + "> + hw_cpu = get_hard_smp_processor_id(nr);\n" "\n" "Why are you swapping the hard smp ids?\n" "\n" - "[chenhui] For example, Core1 has two threads, Thread0 and Thread1. In norma=\n" - "l boot, Thread0 is CPU2, and Thread1 is CPU3.\n" - "But, if CPU2 and CPU3 are all off, user wants CPU3 up first. we need to cal=\n" - "l Thread0 as CPU3 and Thead1 as CPU2, considering\n" - "the limitation, after core is reset, only Thread0 is up, then Thread0 kicks=\n" - " up Thread1.\n" + "[chenhui] For example, Core1 has two threads, Thread0 and Thread1. In normal boot, Thread0 is CPU2, and Thread1 is CPU3.\n" + "But, if CPU2 and CPU3 are all off, user wants CPU3 up first. we need to call Thread0 as CPU3 and Thead1 as CPU2, considering\n" + "the limitation, after core is reset, only Thread0 is up, then Thread0 kicks up Thread1.\n" "\n" "> }\n" "> -\n" @@ -286,7 +278,7 @@ "> #endif\n" ">\n" "> @@ -252,11 +340,7 @@ static int smp_85xx_kick_cpu(int nr)\n" - "> spin_table =3D phys_to_virt(*cpu_rel_addr);\n" + "> spin_table = phys_to_virt(*cpu_rel_addr);\n" ">\n" "> local_irq_save(flags);\n" "> -#ifdef CONFIG_PPC32\n" @@ -299,7 +291,7 @@ "\n" "[chenhui] It would be better to set this after CPU is really up.\n" "\n" - "> if (system_state =3D=3D SYSTEM_RUNNING) {\n" + "> if (system_state == SYSTEM_RUNNING) {\n" "> /*\n" "> * To keep it compatible with old boot program which uses\n" "> @@ -269,11 +353,16 @@ static int smp_85xx_kick_cpu(int nr)\n" @@ -317,22 +309,22 @@ "> __func__);\n" "> return;\n" "> }\n" - "> - smp_85xx_ops.give_timebase =3D mpc85xx_give_timebase;\n" - "> - smp_85xx_ops.take_timebase =3D mpc85xx_take_timebase;\n" + "> - smp_85xx_ops.give_timebase = mpc85xx_give_timebase;\n" + "> - smp_85xx_ops.take_timebase = mpc85xx_take_timebase;\n" "> -#ifdef CONFIG_HOTPLUG_CPU\n" - "> - ppc_md.cpu_die =3D smp_85xx_mach_cpu_die;\n" + "> - ppc_md.cpu_die = smp_85xx_mach_cpu_die;\n" "> -#endif\n" "\n" "You're moving this from a place that only runs when guts is found...\n" "\n" "> }\n" ">\n" - "> + smp_85xx_ops.cpu_die =3D generic_cpu_die;\n" - "> + ppc_md.cpu_die =3D smp_85xx_mach_cpu_die;\n" + "> + smp_85xx_ops.cpu_die = generic_cpu_die;\n" + "> + ppc_md.cpu_die = smp_85xx_mach_cpu_die;\n" "> +#endif\n" - "> + smp_85xx_ops.give_timebase =3D mpc85xx_give_timebase;\n" - "> + smp_85xx_ops.take_timebase =3D mpc85xx_take_timebase;\n" - "> + smp_85xx_ops.cpu_disable =3D generic_cpu_disable;\n" + "> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;\n" + "> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;\n" + "> + smp_85xx_ops.cpu_disable = generic_cpu_disable;\n" "> +#endif /* CONFIG_HOTPLUG_CPU */\n" "\n" "...to a place that runs unconditionally. Again, you're breaking VM\n" @@ -340,6 +332,6 @@ "\n" "-Scott\n" "\n" - [chenhui] Will correct it.= + [chenhui] Will correct it. -e228472dd6c87238e0c84fe16cdebffb27a674bc509f7bb0355f344fc3d2aa31 +1da3ba4d805fb7ddf470eef150e4686ce6c11f64e1050e6ee41e18d8de3b1574
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.