[PATCH v4 0/5] ARM/ARM64 architected timer updates

[PATCH v4 0/5] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch series adds support to configure the rate and enable the
event stream for architected timer. The event streams can be used to
impose a timeout on a wfe, to safeguard against any programming error
in case an expected event is not generated or even to implement
wfe-based timeouts for userspace locking implementations. This feature
can be disabled(enabled by default).

Since the timer control register is reset to zero on warm boot, CPU
PM notifier is added to re-initialize it.

Changes v3->v4:
1. Added a config option to en/disable the timer event stream feature.
   Due to this the ordering of the patches as well as some cosmetic
   code changes are done.( So dropped previous reviewed-by tags)
2. Removed exporting compat_dyn_elf_hwcap as there are no modules
   requiring it for now.

Changes v2->v3:
1. Moved ARM and ARM64 changes into separate patches
2. Added native hwcaps definations(ARM/ARM64) and compat-specific
   definitions(ARM64) to the users for the event stream feature. 

Changes v1->v2:
1. Saved event stream divider value on cold reset path and re-used it
   in CPU PM notifier instead of calculating every time.
2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
   arch_counter_set_user_access"(already done as part of __cpuinit*
   deletion)

Regards,
Sudeep

Sudeep KarkadaNagesha (5):
  ARM/ARM64: arch_timer: add macros for bits in control register
  ARM: arch_timer: add support to configure and enable event stream
  ARM64: arch_timer: add support to configure and enable event stream
  drivers: clocksource: add support for ARM architected timer event
    stream
  drivers: clocksource: add CPU PM notifier for ARM architected timer

 arch/arm/include/asm/arch_timer.h    | 41 ++++++++++++++++++++---
 arch/arm/include/uapi/asm/hwcap.h    |  1 +
 arch/arm/kernel/setup.c              |  1 +
 arch/arm64/include/asm/arch_timer.h  | 48 +++++++++++++++++++++++----
 arch/arm64/include/asm/hwcap.h       |  7 +++-
 arch/arm64/include/uapi/asm/hwcap.h  |  1 +
 arch/arm64/kernel/setup.c            |  5 +++
 drivers/clocksource/Kconfig          | 14 ++++++++
 drivers/clocksource/arm_arch_timer.c | 64 ++++++++++++++++++++++++++++++++++++
 include/clocksource/arm_arch_timer.h | 10 ++++++
 10 files changed, 180 insertions(+), 12 deletions(-)

-- 
1.8.1.2

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Add macros to describe the bitfields in the ARM architected timer
control register to make code easy to understand.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h    |  9 +++++++--
 arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
 include/clocksource/arm_arch_timer.h |  8 ++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index e406d57..9ef74da 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
 
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
-	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (7 << 0));
+	/* Disable user access to both physical/virtual counters/timers. */
+	/* Also disable virtual event stream */
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 98abd47..00b09d0 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
 {
 	u32 cntkctl;
 
-	/* Disable user access to the timers and the physical counter. */
 	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
-	cntkctl &= ~((3 << 8) | (1 << 0));
 
-	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (1 << 1);
+	/* Disable user access to the timers and the physical counter. */
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+
+	/* Enable user access to the virtual counter. */
+	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index c463ce9..551f7e9 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,14 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
+#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
+#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
+#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
+#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
+#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Catalin Marinas]

On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> Add macros to describe the bitfields in the ARM architected timer
> control register to make code easy to understand.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index e406d57..9ef74da 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
>  
>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>  
> -	/* disable user access to everything */
> -	cntkctl &= ~((3 << 8) | (7 << 0));
> +	/* Disable user access to both physical/virtual counters/timers. */
> +	/* Also disable virtual event stream */
> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> +			| ARCH_TIMER_VIRT_EVT_EN
> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>  
>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>  }
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 98abd47..00b09d0 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
>  {
>  	u32 cntkctl;
>  
> -	/* Disable user access to the timers and the physical counter. */
>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
> -	cntkctl &= ~((3 << 8) | (1 << 0));
>  
> -	/* Enable user access to the virtual counter and frequency. */
> -	cntkctl |= (1 << 1);
> +	/* Disable user access to the timers and the physical counter. */
> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> +
> +	/* Enable user access to the virtual counter. */
> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> +
>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));

For consistency with arm, I think we should also disable the event
stream explicitly here.

-- 
Catalin

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

On 27/08/13 12:21, Catalin Marinas wrote:
> On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> Add macros to describe the bitfields in the ARM architected timer
>> control register to make code easy to understand.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>> ---
>>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
>> index e406d57..9ef74da 100644
>> --- a/arch/arm/include/asm/arch_timer.h
>> +++ b/arch/arm/include/asm/arch_timer.h
>> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
>>  
>>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>>  
>> -	/* disable user access to everything */
>> -	cntkctl &= ~((3 << 8) | (7 << 0));
>> +	/* Disable user access to both physical/virtual counters/timers. */
>> +	/* Also disable virtual event stream */
>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +			| ARCH_TIMER_VIRT_EVT_EN
>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>>  }
>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>> index 98abd47..00b09d0 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl;
>>  
>> -	/* Disable user access to the timers and the physical counter. */
>>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
>> -	cntkctl &= ~((3 << 8) | (1 << 0));
>>  
>> -	/* Enable user access to the virtual counter and frequency. */
>> -	cntkctl |= (1 << 1);
>> +	/* Disable user access to the timers and the physical counter. */
>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>> +
>> +	/* Enable user access to the virtual counter. */
>> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +
>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> 
> For consistency with arm, I think we should also disable the event
> stream explicitly here.
> 
Yes it's done. In PATCH 3/5, a new function arch_timer_evtstrm_config is
added which is always called(PATCH 4/5). It's either enabled or disabled
explicitly based on config option for event stream.

Let me know if you think that's not sufficient.

Regards,
Sudeep

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Catalin Marinas]

On Tue, Aug 27, 2013 at 12:37:38PM +0100, Sudeep KarkadaNagesha wrote:
> On 27/08/13 12:21, Catalin Marinas wrote:
> > On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
> >> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> >>
> >> Add macros to describe the bitfields in the ARM architected timer
> >> control register to make code easy to understand.
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> Reviewed-by: Will Deacon <will.deacon@arm.com>
> >> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> >> ---
> >>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
> >>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
> >>  include/clocksource/arm_arch_timer.h |  8 ++++++++
> >>  3 files changed, 23 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> >> index e406d57..9ef74da 100644
> >> --- a/arch/arm/include/asm/arch_timer.h
> >> +++ b/arch/arm/include/asm/arch_timer.h
> >> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
> >>  
> >>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
> >>  
> >> -	/* disable user access to everything */
> >> -	cntkctl &= ~((3 << 8) | (7 << 0));
> >> +	/* Disable user access to both physical/virtual counters/timers. */
> >> +	/* Also disable virtual event stream */
> >> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> >> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> >> +			| ARCH_TIMER_VIRT_EVT_EN
> >> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
> >> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> >>  
> >>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
> >>  }
> >> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> >> index 98abd47..00b09d0 100644
> >> --- a/arch/arm64/include/asm/arch_timer.h
> >> +++ b/arch/arm64/include/asm/arch_timer.h
> >> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
> >>  {
> >>  	u32 cntkctl;
> >>  
> >> -	/* Disable user access to the timers and the physical counter. */
> >>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
> >> -	cntkctl &= ~((3 << 8) | (1 << 0));
> >>  
> >> -	/* Enable user access to the virtual counter and frequency. */
> >> -	cntkctl |= (1 << 1);
> >> +	/* Disable user access to the timers and the physical counter. */
> >> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> >> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> >> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> >> +
> >> +	/* Enable user access to the virtual counter. */
> >> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> >> +
> >>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> > 
> > For consistency with arm, I think we should also disable the event
> > stream explicitly here.
> > 
> Yes it's done. In PATCH 3/5, a new function arch_timer_evtstrm_config is
> added which is always called(PATCH 4/5). It's either enabled or disabled
> explicitly based on config option for event stream.

OK, just that in this patch arm and arm64 had different settings with
regards to the event stream.

BTW, can we not avoid clearing the event stream via the
arch_timer_evtstrm_config(false) and always assume the default as
disabled? Do you ever go back into low power mode with event stream
disabled and come back with it enabled?

-- 
Catalin

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

On 27/08/13 15:52, Catalin Marinas wrote:
> On Tue, Aug 27, 2013 at 12:37:38PM +0100, Sudeep KarkadaNagesha wrote:
>> On 27/08/13 12:21, Catalin Marinas wrote:
>>> On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>
>>>> Add macros to describe the bitfields in the ARM architected timer
>>>> control register to make code easy to understand.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>> ---
>>>>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>>>>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>>>>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>>>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
>>>> index e406d57..9ef74da 100644
>>>> --- a/arch/arm/include/asm/arch_timer.h
>>>> +++ b/arch/arm/include/asm/arch_timer.h
>>>> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
>>>>  
>>>>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>>>>  
>>>> -	/* disable user access to everything */
>>>> -	cntkctl &= ~((3 << 8) | (7 << 0));
>>>> +	/* Disable user access to both physical/virtual counters/timers. */
>>>> +	/* Also disable virtual event stream */
>>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>>>> +			| ARCH_TIMER_VIRT_EVT_EN
>>>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
>>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>>>  
>>>>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>>>>  }
>>>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>>>> index 98abd47..00b09d0 100644
>>>> --- a/arch/arm64/include/asm/arch_timer.h
>>>> +++ b/arch/arm64/include/asm/arch_timer.h
>>>> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
>>>>  {
>>>>  	u32 cntkctl;
>>>>  
>>>> -	/* Disable user access to the timers and the physical counter. */
>>>>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
>>>> -	cntkctl &= ~((3 << 8) | (1 << 0));
>>>>  
>>>> -	/* Enable user access to the virtual counter and frequency. */
>>>> -	cntkctl |= (1 << 1);
>>>> +	/* Disable user access to the timers and the physical counter. */
>>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>>> +
>>>> +	/* Enable user access to the virtual counter. */
>>>> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>>>> +
>>>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>>>
>>> For consistency with arm, I think we should also disable the event
>>> stream explicitly here.
>>>
>> Yes it's done. In PATCH 3/5, a new function arch_timer_evtstrm_config is
>> added which is always called(PATCH 4/5). It's either enabled or disabled
>> explicitly based on config option for event stream.
> 
> OK, just that in this patch arm and arm64 had different settings with
> regards to the event stream.
> 
Yes correct but that's how it is currently. This patch is not modifying
anything functional, just adding macros.

> BTW, can we not avoid clearing the event stream via the
> arch_timer_evtstrm_config(false) and always assume the default as
> disabled? Do you ever go back into low power mode with event stream
> disabled and come back with it enabled?
> 
Yes that can be done. But since the cold/warm reset involves other
firmware which can modify that bit, IMO it would be better to do it
explicitly as CONFIG_ARM_ARCH_TIMER_EVTSTREAM=n has to ensure its
disabled. Though I don't have a strong opinion on this, the reason for
my inclination towards explicit disable is because of platform firmware.
e.g. on V2P_CA15_A7/TC2 bootmon/secure firmware enables this bit based
on some settings in board.txt
I can remove as it's disabled on reset(ARM ARM says its reset value is
0) but with the assumption that other firmware don't modify that bit.

Regards,
Sudeep

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Catalin Marinas]

On Tue, Aug 27, 2013 at 04:19:04PM +0100, Sudeep KarkadaNagesha wrote:
> On 27/08/13 15:52, Catalin Marinas wrote:
> > On Tue, Aug 27, 2013 at 12:37:38PM +0100, Sudeep KarkadaNagesha wrote:
> >> On 27/08/13 12:21, Catalin Marinas wrote:
> >>> On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
> >>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> >>>>
> >>>> Add macros to describe the bitfields in the ARM architected timer
> >>>> control register to make code easy to understand.
> >>>>
> >>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>> Reviewed-by: Will Deacon <will.deacon@arm.com>
> >>>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> >>>> ---
> >>>>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
> >>>>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
> >>>>  include/clocksource/arm_arch_timer.h |  8 ++++++++
> >>>>  3 files changed, 23 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> >>>> index e406d57..9ef74da 100644
> >>>> --- a/arch/arm/include/asm/arch_timer.h
> >>>> +++ b/arch/arm/include/asm/arch_timer.h
> >>>> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
> >>>>  
> >>>>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
> >>>>  
> >>>> -	/* disable user access to everything */
> >>>> -	cntkctl &= ~((3 << 8) | (7 << 0));
> >>>> +	/* Disable user access to both physical/virtual counters/timers. */
> >>>> +	/* Also disable virtual event stream */
> >>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> >>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> >>>> +			| ARCH_TIMER_VIRT_EVT_EN
> >>>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
> >>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> >>>>  
> >>>>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
> >>>>  }
> >>>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> >>>> index 98abd47..00b09d0 100644
> >>>> --- a/arch/arm64/include/asm/arch_timer.h
> >>>> +++ b/arch/arm64/include/asm/arch_timer.h
> >>>> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
> >>>>  {
> >>>>  	u32 cntkctl;
> >>>>  
> >>>> -	/* Disable user access to the timers and the physical counter. */
> >>>>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
> >>>> -	cntkctl &= ~((3 << 8) | (1 << 0));
> >>>>  
> >>>> -	/* Enable user access to the virtual counter and frequency. */
> >>>> -	cntkctl |= (1 << 1);
> >>>> +	/* Disable user access to the timers and the physical counter. */
> >>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> >>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> >>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> >>>> +
> >>>> +	/* Enable user access to the virtual counter. */
> >>>> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> >>>> +
> >>>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> >>>
> >>> For consistency with arm, I think we should also disable the event
> >>> stream explicitly here.
> >>>
> >> Yes it's done. In PATCH 3/5, a new function arch_timer_evtstrm_config is
> >> added which is always called(PATCH 4/5). It's either enabled or disabled
> >> explicitly based on config option for event stream.
> > 
> > OK, just that in this patch arm and arm64 had different settings with
> > regards to the event stream.
> 
> Yes correct but that's how it is currently. This patch is not modifying
> anything functional, just adding macros.
> 
> > BTW, can we not avoid clearing the event stream via the
> > arch_timer_evtstrm_config(false) and always assume the default as
> > disabled? Do you ever go back into low power mode with event stream
> > disabled and come back with it enabled?
> 
> Yes that can be done. But since the cold/warm reset involves other
> firmware which can modify that bit, IMO it would be better to do it
> explicitly as CONFIG_ARM_ARCH_TIMER_EVTSTREAM=n has to ensure its
> disabled. Though I don't have a strong opinion on this, the reason for
> my inclination towards explicit disable is because of platform firmware.
> e.g. on V2P_CA15_A7/TC2 bootmon/secure firmware enables this bit based
> on some settings in board.txt
> I can remove as it's disabled on reset(ARM ARM says its reset value is
> 0) but with the assumption that other firmware don't modify that bit.

For initial boot we can initialise it and clear the EVNTEN bit. For
subsequent CPU low power events I assume we save/restore the cntkctl
register? If we do, we only need to check
CONFIG_ARM_ARCH_TIMER_EVTSTREAM once during initialisation.

-- 
Catalin

[PATCH v4 1/5] ARM/ARM64: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

On 27/08/13 17:53, Catalin Marinas wrote:
> On Tue, Aug 27, 2013 at 04:19:04PM +0100, Sudeep KarkadaNagesha wrote:
>> On 27/08/13 15:52, Catalin Marinas wrote:
>>> On Tue, Aug 27, 2013 at 12:37:38PM +0100, Sudeep KarkadaNagesha wrote:
>>>> On 27/08/13 12:21, Catalin Marinas wrote:
>>>>> On Fri, Aug 23, 2013 at 05:19:05PM +0100, Sudeep KarkadaNagesha wrote:
>>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>>
>>>>>> Add macros to describe the bitfields in the ARM architected timer
>>>>>> control register to make code easy to understand.
>>>>>>
>>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>>>>>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>> ---
>>>>>>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>>>>>>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>>>>>>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>>>>>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
>>>>>> index e406d57..9ef74da 100644
>>>>>> --- a/arch/arm/include/asm/arch_timer.h
>>>>>> +++ b/arch/arm/include/asm/arch_timer.h
>>>>>> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
>>>>>>  
>>>>>>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>>>>>>  
>>>>>> -	/* disable user access to everything */
>>>>>> -	cntkctl &= ~((3 << 8) | (7 << 0));
>>>>>> +	/* Disable user access to both physical/virtual counters/timers. */
>>>>>> +	/* Also disable virtual event stream */
>>>>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>>>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>>>>>> +			| ARCH_TIMER_VIRT_EVT_EN
>>>>>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
>>>>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>>>>>  
>>>>>>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>>>>>>  }
>>>>>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>>>>>> index 98abd47..00b09d0 100644
>>>>>> --- a/arch/arm64/include/asm/arch_timer.h
>>>>>> +++ b/arch/arm64/include/asm/arch_timer.h
>>>>>> @@ -101,12 +101,16 @@ static inline void arch_counter_set_user_access(void)
>>>>>>  {
>>>>>>  	u32 cntkctl;
>>>>>>  
>>>>>> -	/* Disable user access to the timers and the physical counter. */
>>>>>>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
>>>>>> -	cntkctl &= ~((3 << 8) | (1 << 0));
>>>>>>  
>>>>>> -	/* Enable user access to the virtual counter and frequency. */
>>>>>> -	cntkctl |= (1 << 1);
>>>>>> +	/* Disable user access to the timers and the physical counter. */
>>>>>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>>>>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>>>>>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>>>>> +
>>>>>> +	/* Enable user access to the virtual counter. */
>>>>>> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>>>>>> +
>>>>>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>>>>>
>>>>> For consistency with arm, I think we should also disable the event
>>>>> stream explicitly here.
>>>>>
>>>> Yes it's done. In PATCH 3/5, a new function arch_timer_evtstrm_config is
>>>> added which is always called(PATCH 4/5). It's either enabled or disabled
>>>> explicitly based on config option for event stream.
>>>
>>> OK, just that in this patch arm and arm64 had different settings with
>>> regards to the event stream.
>>
>> Yes correct but that's how it is currently. This patch is not modifying
>> anything functional, just adding macros.
>>
>>> BTW, can we not avoid clearing the event stream via the
>>> arch_timer_evtstrm_config(false) and always assume the default as
>>> disabled? Do you ever go back into low power mode with event stream
>>> disabled and come back with it enabled?
>>
>> Yes that can be done. But since the cold/warm reset involves other
>> firmware which can modify that bit, IMO it would be better to do it
>> explicitly as CONFIG_ARM_ARCH_TIMER_EVTSTREAM=n has to ensure its
>> disabled. Though I don't have a strong opinion on this, the reason for
>> my inclination towards explicit disable is because of platform firmware.
>> e.g. on V2P_CA15_A7/TC2 bootmon/secure firmware enables this bit based
>> on some settings in board.txt
>> I can remove as it's disabled on reset(ARM ARM says its reset value is
>> 0) but with the assumption that other firmware don't modify that bit.
> 
> For initial boot we can initialise it and clear the EVNTEN bit. For
> subsequent CPU low power events I assume we save/restore the cntkctl
> register? If we do, we only need to check
> CONFIG_ARM_ARCH_TIMER_EVTSTREAM once during initialisation.
> 
Yes that makes sense, I will post v5 with these changes.

Regards,
Sudeep

[PATCH v4 2/5] ARM: arch_timer: add support to configure and enable event stream

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch adds support for configure the event stream frequency
and enabling it.

It also adds the hwcaps definitions to the user to detect this event
stream feature.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h | 36 +++++++++++++++++++++++++++++++-----
 arch/arm/include/uapi/asm/hwcap.h |  1 +
 arch/arm/kernel/setup.c           |  1 +
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 9ef74da..9617327 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,21 +89,47 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline u32 arch_timer_get_cntkctl(void)
 {
 	u32 cntkctl;
-
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
+	return cntkctl;
+}
+
+static inline void arch_timer_set_cntkctl(u32 cntkctl)
+{
+	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
+}
+
+static inline void arch_counter_set_user_access(void)
+{
+	u32 cntkctl = arch_timer_get_cntkctl();
 
 	/* Disable user access to both physical/virtual counters/timers. */
-	/* Also disable virtual event stream */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
-			| ARCH_TIMER_VIRT_EVT_EN
 			| ARCH_TIMER_USR_VCT_ACCESS_EN
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+	arch_timer_set_cntkctl(cntkctl);
+}
 
-	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
+static inline void arch_timer_evtstrm_config(bool enable, int divider)
+{
+	u32 cntkctl = arch_timer_get_cntkctl();
+	if (enable) {
+		cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
+		/* Set the divider and enable virtual event stream */
+		cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+				| ARCH_TIMER_VIRT_EVT_EN;
+	} else {
+		cntkctl &= ~ARCH_TIMER_VIRT_EVT_EN; /* disable event stream */
+	}
+	arch_timer_set_cntkctl(cntkctl);
+}
+
+static inline void arch_timer_set_hwcap_evtstrm(void)
+{
+	elf_hwcap |= HWCAP_EVTSTRM;
 }
 #endif
 
diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index 6d34d08..7dcc10d 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -26,5 +26,6 @@
 #define HWCAP_VFPD32	(1 << 19)	/* set if VFP has 32 regs (not 16) */
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 #define HWCAP_LPAE	(1 << 20)
+#define HWCAP_EVTSTRM	(1 << 21)
 
 #endif /* _UAPI__ASMARM_HWCAP_H */
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index afc2489..305ffc9 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -975,6 +975,7 @@ static const char *hwcap_str[] = {
 	"idivt",
 	"vfpd32",
 	"lpae",
+	"evtstrm",
 	NULL
 };
 
-- 
1.8.1.2

[PATCH v4 3/5] ARM64: arch_timer: add support to configure and enable event stream

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch adds support for configure the event stream frequency
and enabling it.

It also adds the hwcaps as well as compat-specific definitions to
the user to detect this event stream feature.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 40 +++++++++++++++++++++++++++++++++----
 arch/arm64/include/asm/hwcap.h      |  7 ++++++-
 arch/arm64/include/uapi/asm/hwcap.h |  1 +
 arch/arm64/kernel/setup.c           |  5 +++++
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 00b09d0..0f57158 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,21 +97,53 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline u32 arch_timer_get_cntkctl(void)
 {
 	u32 cntkctl;
-
 	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
+	return cntkctl;
+}
+
+static inline void arch_timer_set_cntkctl(u32 cntkctl)
+{
+	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
+}
+
+static inline void arch_counter_set_user_access(void)
+{
+	u32 cntkctl = arch_timer_get_cntkctl();
 
 	/* Disable user access to the timers and the physical counter. */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
-	/* Enable user access to the virtual counter. */
+	/* Enable user access to the virtual counter */
 	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
 
-	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
+	arch_timer_set_cntkctl(cntkctl);
+}
+
+static inline void arch_timer_evtstrm_config(bool enable, int divider)
+{
+	u32 cntkctl = arch_timer_get_cntkctl();
+	if (enable) {
+		cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
+		/* Set the divider and enable virtual event stream */
+		cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+				| ARCH_TIMER_VIRT_EVT_EN;
+	} else {
+		cntkctl &= ~ARCH_TIMER_VIRT_EVT_EN; /* disable event stream */
+	}
+	arch_timer_set_cntkctl(cntkctl);
+}
+
+static inline void arch_timer_set_hwcap_evtstrm(void)
+{
+	elf_hwcap |= HWCAP_EVTSTRM;
+#ifdef CONFIG_COMPAT
+	compat_dyn_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
+#endif
 }
 
 static inline u64 arch_counter_get_cntvct(void)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 6d4482f..022d771 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -30,6 +30,7 @@
 #define COMPAT_HWCAP_IDIVA	(1 << 17)
 #define COMPAT_HWCAP_IDIVT	(1 << 18)
 #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
+#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
 
 #ifndef __ASSEMBLY__
 /*
@@ -37,11 +38,15 @@
  * instruction set this cpu supports.
  */
 #define ELF_HWCAP		(elf_hwcap)
+#ifdef CONFIG_COMPAT
 #define COMPAT_ELF_HWCAP	(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
 				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
 				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
 				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
-				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
+				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
+				 compat_dyn_elf_hwcap)
+extern unsigned int compat_dyn_elf_hwcap;
+#endif
 
 extern unsigned int elf_hwcap;
 #endif
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index eea4975..9b12476 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -21,6 +21,7 @@
  */
 #define HWCAP_FP		(1 << 0)
 #define HWCAP_ASIMD		(1 << 1)
+#define HWCAP_EVTSTRM		(1 << 2)
 
 
 #endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index add6ea6..50aaca814 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -60,6 +60,10 @@ EXPORT_SYMBOL(processor_id);
 unsigned int elf_hwcap __read_mostly;
 EXPORT_SYMBOL_GPL(elf_hwcap);
 
+#ifdef CONFIG_COMPAT
+unsigned int compat_dyn_elf_hwcap __read_mostly;
+#endif
+
 static const char *cpu_name;
 static const char *machine_name;
 phys_addr_t __fdt_pointer __initdata;
@@ -309,6 +313,7 @@ subsys_initcall(topology_init);
 static const char *hwcap_str[] = {
 	"fp",
 	"asimd",
+	"evtstrm",
 	NULL
 };
 
-- 
1.8.1.2

[PATCH v4 3/5] ARM64: arch_timer: add support to configure and enable event stream

[Catalin Marinas]

On Fri, Aug 23, 2013 at 05:19:07PM +0100, Sudeep KarkadaNagesha wrote:
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 00b09d0..0f57158 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -97,21 +97,53 @@ static inline u32 arch_timer_get_cntfrq(void)
>  	return val;
>  }
>  
> -static inline void arch_counter_set_user_access(void)
> +static inline u32 arch_timer_get_cntkctl(void)
>  {
>  	u32 cntkctl;
> -
>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
> +	return cntkctl;
> +}
> +
> +static inline void arch_timer_set_cntkctl(u32 cntkctl)
> +{
> +	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> +}
> +
> +static inline void arch_counter_set_user_access(void)
> +{
> +	u32 cntkctl = arch_timer_get_cntkctl();
>  
>  	/* Disable user access to the timers and the physical counter. */
>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>  
> -	/* Enable user access to the virtual counter. */
> +	/* Enable user access to the virtual counter */

This change isn't needed here. Just move it to the first patch which
adds the comment.

>  	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>  
> -	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> +	arch_timer_set_cntkctl(cntkctl);
> +}
> +
> +static inline void arch_timer_evtstrm_config(bool enable, int divider)
> +{
> +	u32 cntkctl = arch_timer_get_cntkctl();
> +	if (enable) {
> +		cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
> +		/* Set the divider and enable virtual event stream */
> +		cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +				| ARCH_TIMER_VIRT_EVT_EN;
> +	} else {
> +		cntkctl &= ~ARCH_TIMER_VIRT_EVT_EN; /* disable event stream */
> +	}
> +	arch_timer_set_cntkctl(cntkctl);
> +}
> +
> +static inline void arch_timer_set_hwcap_evtstrm(void)
> +{
> +	elf_hwcap |= HWCAP_EVTSTRM;
> +#ifdef CONFIG_COMPAT
> +	compat_dyn_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
> +#endif
>  }
>  
>  static inline u64 arch_counter_get_cntvct(void)
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 6d4482f..022d771 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
> @@ -30,6 +30,7 @@
>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
>  
>  #ifndef __ASSEMBLY__
>  /*
> @@ -37,11 +38,15 @@
>   * instruction set this cpu supports.
>   */
>  #define ELF_HWCAP		(elf_hwcap)
> +#ifdef CONFIG_COMPAT
>  #define COMPAT_ELF_HWCAP	(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
> +				 compat_dyn_elf_hwcap)
> +extern unsigned int compat_dyn_elf_hwcap;
> +#endif

Can we just have a compat_elf_hwcap which is initialised to all the
above and avoid compat_dyn_elf_hwcap?

-- 
Catalin

[PATCH v4 3/5] ARM64: arch_timer: add support to configure and enable event stream

[Sudeep KarkadaNagesha]

Hi Catalin,

On 27/08/13 12:19, Catalin Marinas wrote:
> On Fri, Aug 23, 2013 at 05:19:07PM +0100, Sudeep KarkadaNagesha wrote:
>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>> index 00b09d0..0f57158 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -97,21 +97,53 @@ static inline u32 arch_timer_get_cntfrq(void)
>>  	return val;
>>  }
>>  
>> -static inline void arch_counter_set_user_access(void)
>> +static inline u32 arch_timer_get_cntkctl(void)
>>  {
>>  	u32 cntkctl;
>> -
>>  	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
>> +	return cntkctl;
>> +}
>> +
>> +static inline void arch_timer_set_cntkctl(u32 cntkctl)
>> +{
>> +	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>> +}
>> +
>> +static inline void arch_counter_set_user_access(void)
>> +{
>> +	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>>  	/* Disable user access to the timers and the physical counter. */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter. */
>> +	/* Enable user access to the virtual counter */
> 
> This change isn't needed here. Just move it to the first patch which
> adds the comment.
> 
It's a mistake, accidentally removed leading '.'
I will remove this.

>>  	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>>  
>> -	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>> +	arch_timer_set_cntkctl(cntkctl);
>> +}
>> +
>> +static inline void arch_timer_evtstrm_config(bool enable, int divider)
>> +{
>> +	u32 cntkctl = arch_timer_get_cntkctl();
>> +	if (enable) {
>> +		cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>> +		/* Set the divider and enable virtual event stream */
>> +		cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +				| ARCH_TIMER_VIRT_EVT_EN;
>> +	} else {
>> +		cntkctl &= ~ARCH_TIMER_VIRT_EVT_EN; /* disable event stream */
>> +	}
>> +	arch_timer_set_cntkctl(cntkctl);
>> +}
>> +
>> +static inline void arch_timer_set_hwcap_evtstrm(void)
>> +{
>> +	elf_hwcap |= HWCAP_EVTSTRM;
>> +#ifdef CONFIG_COMPAT
>> +	compat_dyn_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
>> +#endif
>>  }
>>  
>>  static inline u64 arch_counter_get_cntvct(void)
>> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
>> index 6d4482f..022d771 100644
>> --- a/arch/arm64/include/asm/hwcap.h
>> +++ b/arch/arm64/include/asm/hwcap.h
>> @@ -30,6 +30,7 @@
>>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
>> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
>>  
>>  #ifndef __ASSEMBLY__
>>  /*
>> @@ -37,11 +38,15 @@
>>   * instruction set this cpu supports.
>>   */
>>  #define ELF_HWCAP		(elf_hwcap)
>> +#ifdef CONFIG_COMPAT
>>  #define COMPAT_ELF_HWCAP	(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
>>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
>>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
>>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
>> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
>> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
>> +				 compat_dyn_elf_hwcap)
>> +extern unsigned int compat_dyn_elf_hwcap;
>> +#endif
> 
> Can we just have a compat_elf_hwcap which is initialised to all the
> above and avoid compat_dyn_elf_hwcap?
> 
It was Will's suggestion(https://lkml.org/lkml/2013/8/20/425)
Moreover with config option now it makes more sense to retain it.
It avoids having conditional definition. Let me know your opinion.

Regards,
Sudeep

[PATCH v4 4/5] drivers: clocksource: add support for ARM architected timer event stream

[Sudeep KarkadaNagesha]

From: Will Deacon <will.deacon@arm.com>

The ARM architected timer can generate events (used for waking up
CPUs executing the wfe instruction) at a frequency represented as a
power-of-2 divisor of the clock rate.

An event stream might be used:
- To impose a time-out on a wfe polling loop.
- To safeguard against any programming error in case an expected event
  is not generated.
- To implement wfe-based timeouts for userspace locking implementations.

This patch computes the event stream frequency aiming for a period
of 100us between events. It uses ARM/ARM64 specific backends to configure
and enable the event stream.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
[sudeep: moving ARM/ARM64 changes into separate patches
         and adding Kconfig option]
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/clocksource/Kconfig          | 14 ++++++++++++++
 drivers/clocksource/arm_arch_timer.c | 23 +++++++++++++++++++++++
 include/clocksource/arm_arch_timer.h |  2 ++
 3 files changed, 39 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index b7b9b04..8c10d9a 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -74,6 +74,20 @@ config ARM_ARCH_TIMER
 	bool
 	select CLKSRC_OF if OF
 
+config ARM_ARCH_TIMER_EVTSTREAM
+	bool "Support for ARM architected timer event stream generation"
+	default y if ARM_ARCH_TIMER
+	help
+	  This option enables support for event stream generation based on
+	  the ARM architected timer. It is used for waking up CPUs executing
+	  the wfe instruction at a frequency represented as a power-of-2
+	  divisor of the clock rate. An event stream might be useful for
+	  imposing timeout on a wfe, safeguarding against any programming
+	  errors in case an expected event is not generated or even to
+	  implement wfe-based timeouts for userspace locking implementations.
+	  This can be disabled for hardware validation purposes to detect any
+	  hardware anamolies of missing events.
+
 config ARM_GLOBAL_TIMER
 	bool
 	select CLKSRC_OF if OF
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ffadd83..e331818 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -123,6 +123,28 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 	return 0;
 }
 
+#ifdef CONFIG_ARM_ARCH_TIMER_EVTSTREAM
+static void arch_timer_setup_evtstream(void)
+{
+	int evt_stream_div, pos;
+
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	/* enable event stream */
+	arch_timer_evtstrm_config(true, min(pos, 15));
+	/* enable hwcap definition to the users for event stream feature */
+	arch_timer_set_hwcap_evtstrm();
+}
+#else
+static void arch_timer_setup_evtstream(void)
+{
+	arch_timer_evtstrm_config(false, 0);
+}
+#endif
+
 static int arch_timer_setup(struct clock_event_device *clk)
 {
 	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
@@ -154,6 +176,7 @@ static int arch_timer_setup(struct clock_event_device *clk)
 	}
 
 	arch_counter_set_user_access();
+	arch_timer_setup_evtstream();
 
 	return 0;
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 551f7e9..c4d0fc4 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -37,6 +37,8 @@
 #define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
 #define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
 
+#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

[PATCH v4 5/5] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Few control settings done in architected timer as part of initialisation
can be lost when CPU enters deeper power states. They need to be
re-initialised when the CPU is (warm)reset again.

This patch adds CPU PM notifiers to do the timer initialisation on warm
resets. It also save the event stream divider value calculated during
cold reset and uses the same in warm reset path.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 47 +++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index e331818..1d5b75d 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
@@ -124,6 +125,12 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 }
 
 #ifdef CONFIG_ARM_ARCH_TIMER_EVTSTREAM
+static int arch_timer_evtstream_div;
+static void arch_timer_evtstream_reset(void)
+{
+	/* enable event stream */
+	arch_timer_evtstrm_config(true, arch_timer_evtstream_div);
+}
 static void arch_timer_setup_evtstream(void)
 {
 	int evt_stream_div, pos;
@@ -133,16 +140,19 @@ static void arch_timer_setup_evtstream(void)
 	pos = fls(evt_stream_div);
 	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
 		pos--;
-	/* enable event stream */
-	arch_timer_evtstrm_config(true, min(pos, 15));
+	/* save divider value for use in CPU PM notifier */
+	arch_timer_evtstream_div = min(pos, 15);
+	arch_timer_evtstream_reset();
 	/* enable hwcap definition to the users for event stream feature */
 	arch_timer_set_hwcap_evtstrm();
 }
 #else
-static void arch_timer_setup_evtstream(void)
+static void arch_timer_evtstream_reset(void)
 {
+	/* disable event stream */
 	arch_timer_evtstrm_config(false, 0);
 }
+#define arch_timer_setup_evtstream	arch_timer_evtstream_reset
 #endif
 
 static int arch_timer_setup(struct clock_event_device *clk)
@@ -283,6 +293,31 @@ static struct notifier_block arch_timer_cpu_nb = {
 	.notifier_call = arch_timer_cpu_notify,
 };
 
+#ifdef CONFIG_CPU_PM
+static int arch_timer_cpu_pm_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	if (action == CPU_PM_EXIT)
+		arch_timer_evtstream_reset();
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block arch_timer_cpu_pm_notifier = {
+	.notifier_call = arch_timer_cpu_pm_notify,
+};
+
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return cpu_pm_register_notifier(&arch_timer_cpu_pm_notifier);
+}
+#else
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return 0;
+}
+#endif
+
 static int __init arch_timer_register(void)
 {
 	int err;
@@ -332,11 +367,17 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out_free_irq;
 
+	err = arch_timer_cpu_pm_init();
+	if (err)
+		goto out_unreg_notify;
+
 	/* Immediately configure the timer on the boot CPU */
 	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
 
 	return 0;
 
+out_unreg_notify:
+	unregister_cpu_notifier(&arch_timer_cpu_nb);
 out_free_irq:
 	if (arch_timer_use_virtual)
 		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
-- 
1.8.1.2

[PATCH v4 0/5] ARM/ARM64 architected timer updates

[Catalin Marinas]

On Fri, Aug 23, 2013 at 05:19:04PM +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch series adds support to configure the rate and enable the
> event stream for architected timer. The event streams can be used to
> impose a timeout on a wfe, to safeguard against any programming error
> in case an expected event is not generated or even to implement
> wfe-based timeouts for userspace locking implementations. This feature
> can be disabled(enabled by default).

I had two minor comments. Once they are addressed:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>