Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores
On Thu, 2015-08-06 at 12:32 +0800, Chenhui Zhao wrote:
> On Thu, Aug 6, 2015 at 11:16 AM, Scott Wood
> wrote:
> > On Wed, 2015-08-05 at 19:08 +0800, Chenhui Zhao wrote:
> > > On Sat, Aug 1, 2015 at 8:22 AM, Scott Wood
> > > wrote:
> > > > On Fri, 2015-07-31 at 17:20 +0800, b29983@freescale.comwrote:
> > > > > + /*
> > > > > + * If both threads are offline, reset core to
> > > start.
> > > > > + * When core is up, Thread 0 always gets up
> > > first,
> > > > > + * so bind the current logical cpu with Thread 0.
> > > > > + */
> > > > > + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
> > > > > + int hw_cpu1, hw_cpu2;
> > > > > +
> > > > > + 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 = get_hard_smp_processor_id(nr);
> > > >
> > > > NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
> > > >
> > > > -Scott
> > >
> > > You said,
> > >
> > > There's no need for this. I have booting from a thread1, and
> > > having
> > > it
> > > kick its thread0, working locally without messing with the
> > > hwid/cpu
> > > mapping.
> > >
> > > I still have questions here. After a core reset, how can you boot
> > > Thread1
> > > of the core first. As I know, Thread0 boots up first by default.
> >
> > So the issue isn't that thread1 comes up first, but that you *want*
> > thread1
> > to come up first and it won't. I don't think this remapping is an
> > acceptable
> > answer, though. Instead, if you need only thread1 to come up, start
> > the
> > core, have thread0 start thread1, and then send thread0 into whatever
> > waiting
> > state it would be in if thread1 had never been offlined.
> >
> > -Scott
>
> Remapping is a concise solution. what's the harm of it?
> Keeping things simple is good in my opinion.
Remapping is not simple. Remapping will make debugging more complicated (I
see an oops on CPU , which CPU's registers do I dump in the debugger?),
could expose bugs where smp_processor_id() is used where
hard_smp_processor_id() is needed, etc.
Having thread0 start thread1 and then go wherever it would have gone if
thread1 were up the whole time is much more straightforward.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores
On Thu, Aug 6, 2015 at 11:16 AM, Scott Wood
wrote:
On Wed, 2015-08-05 at 19:08 +0800, Chenhui Zhao wrote:
On Sat, Aug 1, 2015 at 8:22 AM, Scott Wood
wrote:
> On Fri, 2015-07-31 at 17:20 +0800, b29983@freescale.comwrote:
> > + /*
> > + * If both threads are offline, reset core to
start.
> > + * When core is up, Thread 0 always gets up
first,
> > + * so bind the current logical cpu with Thread 0.
> > + */
> > + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
> > + int hw_cpu1, hw_cpu2;
> > +
> > + 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 = get_hard_smp_processor_id(nr);
>
> NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
>
> -Scott
You said,
There's no need for this. I have booting from a thread1, and
having
it
kick its thread0, working locally without messing with the
hwid/cpu
mapping.
I still have questions here. After a core reset, how can you boot
Thread1
of the core first. As I know, Thread0 boots up first by default.
So the issue isn't that thread1 comes up first, but that you *want*
thread1
to come up first and it won't. I don't think this remapping is an
acceptable
answer, though. Instead, if you need only thread1 to come up, start
the
core, have thread0 start thread1, and then send thread0 into whatever
waiting
state it would be in if thread1 had never been offlined.
-Scott
Remapping is a concise solution. what's the harm of it?
Keeping things simple is good in my opinion.
-Chenhui
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores
On Wed, 2015-08-05 at 19:08 +0800, Chenhui Zhao wrote:
> On Sat, Aug 1, 2015 at 8:22 AM, Scott Wood
> wrote:
> > On Fri, 2015-07-31 at 17:20 +0800, b29983@freescale.comwrote:
> > > + /*
> > > + * If both threads are offline, reset core to start.
> > > + * When core is up, Thread 0 always gets up first,
> > > + * so bind the current logical cpu with Thread 0.
> > > + */
> > > + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
> > > + int hw_cpu1, hw_cpu2;
> > > +
> > > + 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 = get_hard_smp_processor_id(nr);
> >
> > NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
> >
> > -Scott
>
> You said,
>
> There's no need for this. I have booting from a thread1, and having
> it
> kick its thread0, working locally without messing with the hwid/cpu
> mapping.
>
> I still have questions here. After a core reset, how can you boot
> Thread1
> of the core first. As I know, Thread0 boots up first by default.
So the issue isn't that thread1 comes up first, but that you *want* thread1
to come up first and it won't. I don't think this remapping is an acceptable
answer, though. Instead, if you need only thread1 to come up, start the
core, have thread0 start thread1, and then send thread0 into whatever waiting
state it would be in if thread1 had never been offlined.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores
On Sat, Aug 1, 2015 at 8:22 AM, Scott Wood
wrote:
On Fri, 2015-07-31 at 17:20 +0800, b29...@freescale.com wrote:
diff --git a/arch/powerpc/platforms/85xx/smp.c
b/arch/powerpc/platforms/85xx/smp.c
index 7f0dadb..8652a49 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -189,15 +189,22 @@ 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 = *(const int *)info;
-
- 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 = get_hard_smp_processor_id(nr);
+ int thread_idx = cpu_thread_in_core(hw_cpu);
+
+ booting_cpu_hwid = (u32)hw_cpu;
Unnecessary cast. Please explain why you need booting_cpu_hwid.
+ imsr = MSR_KERNEL;
+ inia = *(unsigned long *)fsl_secondary_thread_init;
+ if (thread_idx == 0) {
+ mttmr(TMRN_IMSR0, imsr);
+ mttmr(TMRN_INIA0, inia);
+ } else {
+ mttmr(TMRN_IMSR1, imsr);
+ mttmr(TMRN_INIA1, inia);
+ }
+ mtspr(SPRN_TENS, TEN_THREAD(thread_idx));
Please rebase this on top of http://patchwork.ozlabs.org/patch/496952/
OK.
+ /*
+ * If both threads are offline, reset core to start.
+ * When core is up, Thread 0 always gets up first,
+ * so bind the current logical cpu with Thread 0.
+ */
+ if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
+ int hw_cpu1, hw_cpu2;
+
+ 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 = get_hard_smp_processor_id(nr);
NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
-Scott
You said,
There's no need for this. I have booting from a thread1, and having
it
kick its thread0, working locally without messing with the hwid/cpu
mapping.
I still have questions here. After a core reset, how can you boot
Thread1
of the core first. As I know, Thread0 boots up first by default.
-Chenhui
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores
On Fri, 2015-07-31 at 17:20 +0800, b29...@freescale.com wrote:
> diff --git a/arch/powerpc/platforms/85xx/smp.c
> b/arch/powerpc/platforms/85xx/smp.c
> index 7f0dadb..8652a49 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -189,15 +189,22 @@ 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 = *(const int *)info;
> -
> - 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 = get_hard_smp_processor_id(nr);
> + int thread_idx = cpu_thread_in_core(hw_cpu);
> +
> + booting_cpu_hwid = (u32)hw_cpu;
Unnecessary cast. Please explain why you need booting_cpu_hwid.
> + imsr = MSR_KERNEL;
> + inia = *(unsigned long *)fsl_secondary_thread_init;
> + if (thread_idx == 0) {
> + mttmr(TMRN_IMSR0, imsr);
> + mttmr(TMRN_INIA0, inia);
> + } else {
> + mttmr(TMRN_IMSR1, imsr);
> + mttmr(TMRN_INIA1, inia);
> + }
> + mtspr(SPRN_TENS, TEN_THREAD(thread_idx));
Please rebase this on top of http://patchwork.ozlabs.org/patch/496952/
> + /*
> + * If both threads are offline, reset core to start.
> + * When core is up, Thread 0 always gets up first,
> + * so bind the current logical cpu with Thread 0.
> + */
> + if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
> + int hw_cpu1, hw_cpu2;
> +
> + 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 = get_hard_smp_processor_id(nr);
NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
