Subject: Re: [PATCH] powerpc/85xx: Add back condition for smp

From: Scott Wood <[email-masked]>
Date: Wed, 18 Apr 2012 11:23:46 -0500

On 04/18/2012 09:15 AM, Kumar Gala wrote:
> 
> On Apr 17, 2012, at 5:17 PM, Scott Wood wrote:
> 
>> On 04/17/2012 04:39 PM, York Sun wrote:
>>> The timebase synchronization is only necessary if we need to reset a
>>> separate core.  Currently only KEXEC and CPU hotplug require resetting
>>> a single core. The following code should be in the condition of
>>> CONFIG_KEXEC or CONFIG_HOTPLUG_CPU
>>>
>>>        .give_timebase  = smp_generic_give_timebase,
>>>        .take_timebase  = smp_generic_take_timebase,
>>>
>>> Signed-off-by: York Sun <[email-masked]>
>>> Acked-by: Li Yang <[email-masked]>
>>> ---
>>> arch/powerpc/platforms/85xx/smp.c |    2 ++
>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
>>> index 56942af..868c6d7 100644
>>> --- a/arch/powerpc/platforms/85xx/smp.c
>>> +++ b/arch/powerpc/platforms/85xx/smp.c
>>> @@ -192,8 +192,10 @@ struct smp_ops_t smp_85xx_ops = {
>>> 	.cpu_disable	= generic_cpu_disable,
>>> 	.cpu_die	= generic_cpu_die,
>>> #endif
>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
>>> 	.give_timebase	= smp_generic_give_timebase,
>>> 	.take_timebase	= smp_generic_take_timebase,
>>> +#endif
>>> };
>>>
>>> #ifdef CONFIG_KEXEC
>>
>> Note that this is only a temporary fix, that assumes the environments
>> where tbsync is problematic[1] (virtualization and simulation) do not
>> enable CONFIG_KEXEC or CONFIG_HOTPLUG_CPU.  Eventually the sync should
>> be done via CCSR like in U-Boot, and the decision on whether to do it
>> should be runtime.
> 
> Is the thinking with the CCSR like u-boot sync after boot would resync to some non-zero TB value?

Yes, while all timebases are stopped, the core coming out of reset will
be set to the timebase value of the other cores.

> I'm guessing the idea is doing such a rsync w/TB stopped in the
> system is quick enough that the freezing of it will not be noticed.

Quick enough is relative, but remember that the context is things like
kexec and deep sleep, which are already quite timing-intrusive.

> One should measure this and see what it looks like in core cycles and
> how it scales based on # of cores.  (could use alternate TB as a
> counter to measure).

I have a hard time imagining it being slower than the generic tbsync,
but in any case it's really the only sane option we have in cases where
we must reset individual cores -- I'm not sure how relevant such
benchmarking would be, other than "nice to know".

-Scott