| Message ID | 1374492082-13686-2-git-send-email-Sudeep.KarkadaNagesha@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote: > 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. > > This patch configures the event stream, aiming for a period of 100us > between events. This can be used to implement wfe-based timeouts for > userspace locking implementations. ... > --- a/include/clocksource/arm_arch_timer.h > +++ b/include/clocksource/arm_arch_timer.h > @@ -29,6 +29,8 @@ > #define ARCH_TIMER_PHYS_ACCESS 0 > #define ARCH_TIMER_VIRT_ACCESS 1 > > +#define ARCH_TIMER_EVT_STREAM_FREQ 10000 /* 100us */ BTW, if user-space starts using this, it will become an ABI. Is this the right frequency? In addition, do we want to expose this via hwcap? Something like HWCAP_EVSTR100US?
On Tue, Jul 23, 2013 at 11:23:34AM +0100, Catalin Marinas wrote: > On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote: > > 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. > > > > This patch configures the event stream, aiming for a period of 100us > > between events. This can be used to implement wfe-based timeouts for > > userspace locking implementations. > ... > > --- a/include/clocksource/arm_arch_timer.h > > +++ b/include/clocksource/arm_arch_timer.h > > @@ -29,6 +29,8 @@ > > #define ARCH_TIMER_PHYS_ACCESS 0 > > #define ARCH_TIMER_VIRT_ACCESS 1 > > > > +#define ARCH_TIMER_EVT_STREAM_FREQ 10000 /* 100us */ > > BTW, if user-space starts using this, it will become an ABI. Is this the > right frequency? It doesn't quite become ABI; not all platforms will use the architected timers and not all timers can support an arbitrary frequency. The best we can do is calculate something as close to the target value as possible. I spoke to both tools developers and some HSA driver guys about the frequency, and this is what ended up being suggested. > In addition, do we want to expose this via hwcap? Something like > HWCAP_EVSTR100US? Hmm, maybe, but we don't want people to try and use this for any accurate time measurements, so I wouldn't include the period. Will
On Tue, Jul 23, 2013 at 11:33:33AM +0100, Will Deacon wrote: > On Tue, Jul 23, 2013 at 11:23:34AM +0100, Catalin Marinas wrote: > > On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote: > > > 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. > > > > > > This patch configures the event stream, aiming for a period of 100us > > > between events. This can be used to implement wfe-based timeouts for > > > userspace locking implementations. > > ... > > > --- a/include/clocksource/arm_arch_timer.h > > > +++ b/include/clocksource/arm_arch_timer.h > > > @@ -29,6 +29,8 @@ > > > #define ARCH_TIMER_PHYS_ACCESS 0 > > > #define ARCH_TIMER_VIRT_ACCESS 1 > > > > > > +#define ARCH_TIMER_EVT_STREAM_FREQ 10000 /* 100us */ > > > > BTW, if user-space starts using this, it will become an ABI. Is this the > > right frequency? > > It doesn't quite become ABI; not all platforms will use the architected > timers and not all timers can support an arbitrary frequency. The best we > can do is calculate something as close to the target value as possible. ABI in the sense that if it is available and advertised by the kernel as such, people may use it. > I spoke to both tools developers and some HSA driver guys about the frequency, > and this is what ended up being suggested. > > > In addition, do we want to expose this via hwcap? Something like > > HWCAP_EVSTR100US? > > Hmm, maybe, but we don't want people to try and use this for any accurate > time measurements, so I wouldn't include the period. Definitely not for accurate time but some user-space may find the delay too small or too large. I'm fine without specifying the period, maybe add a comment in the kernel like /* currently 100us */.
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index e406d57..c3d9ef7 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -89,14 +89,15 @@ static inline u64 arch_counter_get_cntvct(void) return cval; } -static inline void arch_counter_set_user_access(void) +static inline void arch_counter_set_user_access(int divider) { u32 cntkctl; asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl)); /* disable user access to everything */ - cntkctl &= ~((3 << 8) | (7 << 0)); + cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0)); + cntkctl |= (divider << 4) | (1 << 2); 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..8c1e42f 100644 --- a/arch/arm64/include/asm/arch_timer.h +++ b/arch/arm64/include/asm/arch_timer.h @@ -97,16 +97,16 @@ static inline u32 arch_timer_get_cntfrq(void) return val; } -static inline void arch_counter_set_user_access(void) +static inline void arch_counter_set_user_access(int divider) { 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)); + cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0)); /* Enable user access to the virtual counter and frequency. */ - cntkctl |= (1 << 1); + cntkctl |= (divider << 4) | (1 << 2) | (1 << 1); asm volatile("msr cntkctl_el1, %0" : : "r" (cntkctl)); } diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index ffadd83..6301ee5 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt, static int arch_timer_setup(struct clock_event_device *clk) { + int evt_stream_div, pos; + clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP; clk->name = "arch_sys_timer"; clk->rating = 450; @@ -153,7 +155,12 @@ static int arch_timer_setup(struct clock_event_device *clk) enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0); } - arch_counter_set_user_access(); + /* 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--; + arch_counter_set_user_access(min(pos, 15)); return 0; } diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h index c463ce9..24dc140 100644 --- a/include/clocksource/arm_arch_timer.h +++ b/include/clocksource/arm_arch_timer.h @@ -29,6 +29,8 @@ #define ARCH_TIMER_PHYS_ACCESS 0 #define ARCH_TIMER_VIRT_ACCESS 1 +#define ARCH_TIMER_EVT_STREAM_FREQ 10000 /* 100us */ + #ifdef CONFIG_ARM_ARCH_TIMER extern u32 arch_timer_get_rate(void);