[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Marc C]

Hello,

I have a question regarding the interaction with the ARM Generic (or
architected) timer, high-resolution timer support, and the WFI instruction.

This commit [1] sets up the "C3STOP" flag on the arch_timer. According
to the commit log, an ARM core which implements the ARM Generic Timer
may power-down and thus invalidate some registers within the timer.
Therefore, flagging the timer with C3STOP will ensure that an
appropriate broadcast timer will be used whenever the CPU goes into idle.

However, according to this article [2], there is a difference in
implementation between the Cortex-A7 and Cortex-A15, wherein the A7 will
power-down on idle, and the A15 will not.

Now, given the 2 citations, can someone explain why the C3STOP flag is
used for _all_ implementations? Is it not the case that the A15-based
arch_timer can be used as a broadcast timer, even if the kernel is
configured to call WFI and put the CPU into low-power mode?

There have been some recent commits which conditionally setup the C3STOP
flag, depending on the CONFIG_CPU_IDLE flag. However, my concern is that
there are some ARM implementations that can actually go into low-power
mode and still use the arch_timer reliably.

Thank you,
Marc C

[1] -
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=27a5569dc66ecce06cb532542ddcd0b6da8783f6
[2] -
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka16107.html

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Lorenzo Pieralisi]

On Thu, Nov 21, 2013 at 07:58:04AM +0000, Marc C wrote:
> Hello,
> 
> I have a question regarding the interaction with the ARM Generic (or
> architected) timer, high-resolution timer support, and the WFI instruction.
> 
> This commit [1] sets up the "C3STOP" flag on the arch_timer. According
> to the commit log, an ARM core which implements the ARM Generic Timer
> may power-down and thus invalidate some registers within the timer.
> Therefore, flagging the timer with C3STOP will ensure that an
> appropriate broadcast timer will be used whenever the CPU goes into idle.
> 
> However, according to this article [2], there is a difference in
> implementation between the Cortex-A7 and Cortex-A15, wherein the A7 will
> power-down on idle, and the A15 will not.

There is a difference in implementation, which implies that A15 local
timers might still be running when a CPU is power gated since the
local timer voltage domain is different from the vcpu, voltage to the core.

It is an A15 oddity. On A7 and newer cores, the local timer will be
held in reset/power down when a single core is shutdown.

See, point is, even if on A15 the timer is still running when a single
core is shutdown, this might not be true when the A15 cluster goes down.

At that point in time, when the last CPU running in a cluster is about
to call a cluster shutdown, how can you offload the local timer of _another_
CPU to an always-on timer acting as broadcast ? Wake up that CPU up just
to enter broadcast mode ? This is not really power efficient and we add
complexity in the kernel. There might be a way to solve this by using
the clock event API but I would rule it out.

It is basically the same reason why we save/restore the GIC CPU IF
configuration whenever a single core goes down (but the GIC stays on),
because there is no way to read it from other CPUs when the GIC is shutdown
for good (NB this is just an example and is likely to change with future GIC
implementations).

Since it is an A15 oddity, I think we should not care and keep things as
they are in the kernel.

> Now, given the 2 citations, can someone explain why the C3STOP flag is
> used for _all_ implementations? Is it not the case that the A15-based
> arch_timer can be used as a broadcast timer, even if the kernel is
> configured to call WFI and put the CPU into low-power mode?
> 
> There have been some recent commits which conditionally setup the C3STOP
> flag, depending on the CONFIG_CPU_IDLE flag. However, my concern is that
> there are some ARM implementations that can actually go into low-power
> mode and still use the arch_timer reliably.

It is an A15 oddity and we should not care, given that this behaviour is
platform specific and likely to fail in most common A15 implementations.

Thank you for pointing that out though.

Lorenzo

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Marc C]

Hello Lorenzo,

> It is an A15 oddity and we should not care, given that this behaviour
> is platform specific and likely to fail in most common A15
> implementations.

I'm supporting a platform where this "oddity" is actually a relied-upon
feature. Our ARMv7-compliant MPCore implements the ARM Generic Timer per
spec. Our implementation isn't a constituent of a big.LITTLE
arrangement, and we'll never completely power-off all cores (we just use
WFI).

According to the documentation that currently exists, it seems that the
behavior of the ARM Generic Timer on cores like the A15 is really just
an attribute of that specific implementation. As you've alluded to,
there may be other implementations that are also usable when the CPUs
enter WFI. That said, do you object to having an optional boolean
property in the arch_timer DT binding [1] which allows users to override
the C3STOP flag? The default behavior would be as is currently
implemented, and for the odd machines we can add the new property to the DT.

Regards,
Marc

[1]
https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/arch_timer.txt

On 11/21/13, 4:00 AM, Lorenzo Pieralisi wrote:
> On Thu, Nov 21, 2013 at 07:58:04AM +0000, Marc C wrote:
>> Hello,
>>
>> I have a question regarding the interaction with the ARM Generic (or
>> architected) timer, high-resolution timer support, and the WFI instruction.
>>
>> This commit [1] sets up the "C3STOP" flag on the arch_timer. According
>> to the commit log, an ARM core which implements the ARM Generic Timer
>> may power-down and thus invalidate some registers within the timer.
>> Therefore, flagging the timer with C3STOP will ensure that an
>> appropriate broadcast timer will be used whenever the CPU goes into idle.
>>
>> However, according to this article [2], there is a difference in
>> implementation between the Cortex-A7 and Cortex-A15, wherein the A7 will
>> power-down on idle, and the A15 will not.
> 
> There is a difference in implementation, which implies that A15 local
> timers might still be running when a CPU is power gated since the
> local timer voltage domain is different from the vcpu, voltage to the core.
> 
> It is an A15 oddity. On A7 and newer cores, the local timer will be
> held in reset/power down when a single core is shutdown.
> 
> See, point is, even if on A15 the timer is still running when a single
> core is shutdown, this might not be true when the A15 cluster goes down.
> 
> At that point in time, when the last CPU running in a cluster is about
> to call a cluster shutdown, how can you offload the local timer of _another_
> CPU to an always-on timer acting as broadcast ? Wake up that CPU up just
> to enter broadcast mode ? This is not really power efficient and we add
> complexity in the kernel. There might be a way to solve this by using
> the clock event API but I would rule it out.
> 
> It is basically the same reason why we save/restore the GIC CPU IF
> configuration whenever a single core goes down (but the GIC stays on),
> because there is no way to read it from other CPUs when the GIC is shutdown
> for good (NB this is just an example and is likely to change with future GIC
> implementations).
> 
> Since it is an A15 oddity, I think we should not care and keep things as
> they are in the kernel.
> 
>> Now, given the 2 citations, can someone explain why the C3STOP flag is
>> used for _all_ implementations? Is it not the case that the A15-based
>> arch_timer can be used as a broadcast timer, even if the kernel is
>> configured to call WFI and put the CPU into low-power mode?
>>
>> There have been some recent commits which conditionally setup the C3STOP
>> flag, depending on the CONFIG_CPU_IDLE flag. However, my concern is that
>> there are some ARM implementations that can actually go into low-power
>> mode and still use the arch_timer reliably.
> 
> It is an A15 oddity and we should not care, given that this behaviour is
> platform specific and likely to fail in most common A15 implementations.
> 
> Thank you for pointing that out though.
> 
> Lorenzo
> 

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Lorenzo Pieralisi]

Hi Marc,

On Tue, Nov 26, 2013 at 06:42:03AM +0000, Marc C wrote:
> Hello Lorenzo,
> 
> > It is an A15 oddity and we should not care, given that this behaviour
> > is platform specific and likely to fail in most common A15
> > implementations.
> 
> I'm supporting a platform where this "oddity" is actually a relied-upon
> feature. Our ARMv7-compliant MPCore implements the ARM Generic Timer per
> spec. Our implementation isn't a constituent of a big.LITTLE
> arrangement, and we'll never completely power-off all cores (we just use
> WFI).

So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
to the C-state flags and you are all set, or I am missing something here ?

> According to the documentation that currently exists, it seems that the
> behavior of the ARM Generic Timer on cores like the A15 is really just
> an attribute of that specific implementation. As you've alluded to,
> there may be other implementations that are also usable when the CPUs
> enter WFI. That said, do you object to having an optional boolean
> property in the arch_timer DT binding [1] which allows users to override
> the C3STOP flag? The default behavior would be as is currently
> implemented, and for the odd machines we can add the new property to the DT.

Yes I do object. Timer binding is global in the DT and do not want to
override the flag for all local timers when, as I mentioned, A15 behaviour is
just an exception. If you really need that, please write an idle driver that
does not enter broadcast mode on C-state entry (see above).

Thanks,
Lorenzo

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Marc C]

Hi Lorenzo,

> So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
> to the C-state flags and you are all set, or I am missing something
> here ?

The C3STOP flag prevents the kernel from using the timer as a
high-resolution clock source. There are some patches that remove the
C3STOP flag [1] in order for the timer to function for use with hrtimer.
I think something similar could be manageable as a DT property on the
armv7-timer binding.

> Yes I do object. Timer binding is global in the DT and do not want to
> override the flag for all local timers when, as I mentioned, A15
> behaviour is just an exception. If you really need that, please write
> an idle driver that does not enter broadcast mode on C-state entry
> (see above).

So what you're saying is that you'll outright disapprove of any patch
that would otherwise help ensure the kernel would run on any/all
variations of armv7-timer? I would imagine that we'd want things to be
more inclusive, and since there are quite a few SoCs with the timer that
behave in this manner.

I'm not trying to be a thorn in your side. I just want to make sure
everyone that has an implementation similar to ours is covered, too.

I appreciate your feedback.

Thanks,
Marc

[1] https://lkml.org/lkml/2013/6/16/245

[1] https://lkml.org/lkml/2013/6/16/245
On 11/26/13, 2:10 AM, Lorenzo Pieralisi wrote:
> Hi Marc,
> 
> On Tue, Nov 26, 2013 at 06:42:03AM +0000, Marc C wrote:
>> Hello Lorenzo,
>>
>>> It is an A15 oddity and we should not care, given that this behaviour
>>> is platform specific and likely to fail in most common A15
>>> implementations.
>>
>> I'm supporting a platform where this "oddity" is actually a relied-upon
>> feature. Our ARMv7-compliant MPCore implements the ARM Generic Timer per
>> spec. Our implementation isn't a constituent of a big.LITTLE
>> arrangement, and we'll never completely power-off all cores (we just use
>> WFI).
> 
> So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
> to the C-state flags and you are all set, or I am missing something here ?
> 
>> According to the documentation that currently exists, it seems that the
>> behavior of the ARM Generic Timer on cores like the A15 is really just
>> an attribute of that specific implementation. As you've alluded to,
>> there may be other implementations that are also usable when the CPUs
>> enter WFI. That said, do you object to having an optional boolean
>> property in the arch_timer DT binding [1] which allows users to override
>> the C3STOP flag? The default behavior would be as is currently
>> implemented, and for the odd machines we can add the new property to the DT.
> 
> Yes I do object. Timer binding is global in the DT and do not want to
> override the flag for all local timers when, as I mentioned, A15 behaviour is
> just an exception. If you really need that, please write an idle driver that
> does not enter broadcast mode on C-state entry (see above).
> 
> Thanks,
> Lorenzo
> 

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Lorenzo Pieralisi]

[adding Mark in CC to make him aware of this thread]

On Tue, Nov 26, 2013 at 11:03:22AM +0000, Marc C wrote:
> Hi Lorenzo,
> 
> > So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
> > to the C-state flags and you are all set, or I am missing something
> > here ?
> 
> The C3STOP flag prevents the kernel from using the timer as a
> high-resolution clock source. There are some patches that remove the
> C3STOP flag [1] in order for the timer to function for use with hrtimer.
> I think something similar could be manageable as a DT property on the
> armv7-timer binding.
> 
> > Yes I do object. Timer binding is global in the DT and do not want to
> > override the flag for all local timers when, as I mentioned, A15
> > behaviour is just an exception. If you really need that, please write
> > an idle driver that does not enter broadcast mode on C-state entry
> > (see above).
> 
> So what you're saying is that you'll outright disapprove of any patch
> that would otherwise help ensure the kernel would run on any/all
> variations of armv7-timer? I would imagine that we'd want things to be
> more inclusive, and since there are quite a few SoCs with the timer that
> behave in this manner.
> 
> I'm not trying to be a thorn in your side. I just want to make sure
> everyone that has an implementation similar to ours is covered, too.

Ok, I think I got it now. The platform in question has just local timers
(no global timer), and they are not shut down on idle entry, actually
the platform has no PM capability at all (apart from wfi).

Given that arch timers are C3STOP, kernel does not enter hr mode.

Problem is more complex than I thought and deserves some time to sort it
out properly, I am also working on C-states binding for ARM, I will take
this into account.

> I appreciate your feedback.

Sorry for misunderstanding the problem at first.

Thanks,
Lorenzo

> Thanks,
> Marc
> 
> [1] https://lkml.org/lkml/2013/6/16/245
> 
> [1] https://lkml.org/lkml/2013/6/16/245
> On 11/26/13, 2:10 AM, Lorenzo Pieralisi wrote:
> > Hi Marc,
> > 
> > On Tue, Nov 26, 2013 at 06:42:03AM +0000, Marc C wrote:
> >> Hello Lorenzo,
> >>
> >>> It is an A15 oddity and we should not care, given that this behaviour
> >>> is platform specific and likely to fail in most common A15
> >>> implementations.
> >>
> >> I'm supporting a platform where this "oddity" is actually a relied-upon
> >> feature. Our ARMv7-compliant MPCore implements the ARM Generic Timer per
> >> spec. Our implementation isn't a constituent of a big.LITTLE
> >> arrangement, and we'll never completely power-off all cores (we just use
> >> WFI).
> > 
> > So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
> > to the C-state flags and you are all set, or I am missing something here ?
> > 
> >> According to the documentation that currently exists, it seems that the
> >> behavior of the ARM Generic Timer on cores like the A15 is really just
> >> an attribute of that specific implementation. As you've alluded to,
> >> there may be other implementations that are also usable when the CPUs
> >> enter WFI. That said, do you object to having an optional boolean
> >> property in the arch_timer DT binding [1] which allows users to override
> >> the C3STOP flag? The default behavior would be as is currently
> >> implemented, and for the odd machines we can add the new property to the DT.
> > 
> > Yes I do object. Timer binding is global in the DT and do not want to
> > override the flag for all local timers when, as I mentioned, A15 behaviour is
> > just an exception. If you really need that, please write an idle driver that
> > does not enter broadcast mode on C-state entry (see above).
> > 
> > Thanks,
> > Lorenzo
> > 
> 

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Santosh Shilimkar]

On Tuesday 26 November 2013 07:28 AM, Lorenzo Pieralisi wrote:
> [adding Mark in CC to make him aware of this thread]
> 
> On Tue, Nov 26, 2013 at 11:03:22AM +0000, Marc C wrote:
>> Hi Lorenzo,
>>
>>> So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
>>> to the C-state flags and you are all set, or I am missing something
>>> here ?
>>
>> The C3STOP flag prevents the kernel from using the timer as a
>> high-resolution clock source. There are some patches that remove the
>> C3STOP flag [1] in order for the timer to function for use with hrtimer.
>> I think something similar could be manageable as a DT property on the
>> armv7-timer binding.
>>
>>> Yes I do object. Timer binding is global in the DT and do not want to
>>> override the flag for all local timers when, as I mentioned, A15
>>> behaviour is just an exception. If you really need that, please write
>>> an idle driver that does not enter broadcast mode on C-state entry
>>> (see above).
>>
>> So what you're saying is that you'll outright disapprove of any patch
>> that would otherwise help ensure the kernel would run on any/all
>> variations of armv7-timer? I would imagine that we'd want things to be
>> more inclusive, and since there are quite a few SoCs with the timer that
>> behave in this manner.
>>
>> I'm not trying to be a thorn in your side. I just want to make sure
>> everyone that has an implementation similar to ours is covered, too.
> 
> Ok, I think I got it now. The platform in question has just local timers
> (no global timer), and they are not shut down on idle entry, actually
> the platform has no PM capability at all (apart from wfi).
> 
> Given that arch timers are C3STOP, kernel does not enter hr mode.
> 
> Problem is more complex than I thought and deserves some time to sort it
> out properly, I am also working on C-states binding for ARM, I will take
> this into account.
> 
Indeed its worth looking at considering power domain partitioning and PM
support on SOCs dictate the C3-STOP presence.

Regards,
Santosh

[RFC] ARM Generic Timer + Interaction with WFI on Cortex-A15

[Lorenzo Pieralisi]

Hi Marc,

On Tue, Nov 26, 2013 at 11:03:22AM +0000, Marc C wrote:
> Hi Lorenzo,
> 
> > So what's the problem then ? Just avoid adding CPUIDLE_FLAG_TIMER_STOP
> > to the C-state flags and you are all set, or I am missing something
> > here ?
> 
> The C3STOP flag prevents the kernel from using the timer as a
> high-resolution clock source. There are some patches that remove the
> C3STOP flag [1] in order for the timer to function for use with hrtimer.
> I think something similar could be manageable as a DT property on the
> armv7-timer binding.
> 
> > Yes I do object. Timer binding is global in the DT and do not want to
> > override the flag for all local timers when, as I mentioned, A15
> > behaviour is just an exception. If you really need that, please write
> > an idle driver that does not enter broadcast mode on C-state entry
> > (see above).
> 
> So what you're saying is that you'll outright disapprove of any patch
> that would otherwise help ensure the kernel would run on any/all
> variations of armv7-timer? I would imagine that we'd want things to be
> more inclusive, and since there are quite a few SoCs with the timer that
> behave in this manner.
> 
> I'm not trying to be a thorn in your side. I just want to make sure
> everyone that has an implementation similar to ours is covered, too.

I though about this and I think the only clean way to implement this
would be to add a power domain property to the arch timers. The problem
I am having is that this should not break existing platforms, so
basically we need to understand what the power-domain property would
mean on a system like yours with no power management. We might add
an always-on power domain and attach the arch timer to that, it seems
lots of churn for not much to me.

Adding a property like "state-retained" is not nice either because this
would solve the problem for arch timer but that does not represent a
generic solution.

Thoughts appreciated.

Thanks,
Lorenzo