* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
@ 2014-09-18 14:59 Nathan Lynch
2014-09-18 14:59 ` [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable Nathan Lynch
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Nathan Lynch @ 2014-09-18 14:59 UTC (permalink / raw)
To: linux-arm-kernel
This series contains the necessary changes to allow architected timer
access from user-space on 32-bit ARM. This allows the VDSO to support
high resolution timestamps for clock_gettime and gettimeofday. This
also merges substantially similar code from arm and arm64 into the
core arm_arch_timer driver.
The functional changes are:
- When available, CNTVCT is made readable by user space on arm, as it
is on arm64.
- The clocksource name becomes "arch_mem_counter" if CP15 access to
the counter is not available.
These changes have been carried as part of the ARM VDSO patch set over
the last several months, but I am splitting them out here as I assume
they should go through the clocksource maintainers.
Changes since v1:
- Fix minor checkpatch complaints.
- Rework patches 2-4. v1 copied arm64's arch_timer_evtstrm_enable
and arch_counter_set_user_access to the driver with slightly
different names in one patch, then removed the unused functions in
subsequent patches for each of arm and arm64. This seemed kind of
awkward to me, so I've reorganized those changes into two patches
that should be easier to review. Patch #1 is unchanged.
Nathan Lynch (3):
clocksource: arm_arch_timer: change clocksource name if CP15
unavailable
clocksource: arm_arch_timer: enable counter access for 32-bit ARM
clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable
arch/arm/include/asm/arch_timer.h | 25 --------------------
arch/arm64/include/asm/arch_timer.h | 31 -------------------------
drivers/clocksource/arm_arch_timer.c | 44 ++++++++++++++++++++++++++++++++++--
3 files changed, 42 insertions(+), 58 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable 2014-09-18 14:59 [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Nathan Lynch @ 2014-09-18 14:59 ` Nathan Lynch 2014-09-26 7:04 ` Daniel Lezcano 2014-09-18 14:59 ` [PATCH v2 2/3] clocksource: arm_arch_timer: enable counter access for 32-bit ARM Nathan Lynch ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: Nathan Lynch @ 2014-09-18 14:59 UTC (permalink / raw) To: linux-arm-kernel The arm and arm64 VDSOs need CP15 access to the architected counter. If this is unavailable (which is allowed by ARM v7), indicate this by changing the clocksource name to "arch_mem_counter" before registering the clocksource. Suggested by Stephen Boyd. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/clocksource/arm_arch_timer.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 5163ec13429d..c99afdf12e98 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -429,11 +429,19 @@ static void __init arch_counter_register(unsigned type) u64 start_count; /* Register the CP15 based counter if we have one */ - if (type & ARCH_CP15_TIMER) + if (type & ARCH_CP15_TIMER) { arch_timer_read_counter = arch_counter_get_cntvct; - else + } else { arch_timer_read_counter = arch_counter_get_cntvct_mem; + /* If the clocksource name is "arch_sys_counter" the + * VDSO will attempt to read the CP15-based counter. + * Ensure this does not happen when CP15-based + * counter is not available. + */ + clocksource_counter.name = "arch_mem_counter"; + } + start_count = arch_timer_read_counter(); clocksource_register_hz(&clocksource_counter, arch_timer_rate); cyclecounter.mult = clocksource_counter.mult; -- 1.9.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable 2014-09-18 14:59 ` [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable Nathan Lynch @ 2014-09-26 7:04 ` Daniel Lezcano 2014-09-26 9:26 ` Will Deacon 2014-09-26 14:55 ` Nathan Lynch 0 siblings, 2 replies; 22+ messages in thread From: Daniel Lezcano @ 2014-09-26 7:04 UTC (permalink / raw) To: linux-arm-kernel On 09/18/2014 04:59 PM, Nathan Lynch wrote: > The arm and arm64 VDSOs need CP15 access to the architected counter. > If this is unavailable (which is allowed by ARM v7), indicate this by > changing the clocksource name to "arch_mem_counter" before registering > the clocksource. > > Suggested by Stephen Boyd. > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> Hi Nathan, is it possible to have the acked-by of the different maintainers for the other patches ? Are these patches targeted to be merged for 3.18 ? Thanks -- Daniel > --- > drivers/clocksource/arm_arch_timer.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index 5163ec13429d..c99afdf12e98 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -429,11 +429,19 @@ static void __init arch_counter_register(unsigned type) > u64 start_count; > > /* Register the CP15 based counter if we have one */ > - if (type & ARCH_CP15_TIMER) > + if (type & ARCH_CP15_TIMER) { > arch_timer_read_counter = arch_counter_get_cntvct; > - else > + } else { > arch_timer_read_counter = arch_counter_get_cntvct_mem; > > + /* If the clocksource name is "arch_sys_counter" the > + * VDSO will attempt to read the CP15-based counter. > + * Ensure this does not happen when CP15-based > + * counter is not available. > + */ > + clocksource_counter.name = "arch_mem_counter"; > + } > + > start_count = arch_timer_read_counter(); > clocksource_register_hz(&clocksource_counter, arch_timer_rate); > cyclecounter.mult = clocksource_counter.mult; > -- <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable 2014-09-26 7:04 ` Daniel Lezcano @ 2014-09-26 9:26 ` Will Deacon 2014-09-26 11:34 ` Daniel Lezcano 2014-09-26 14:55 ` Nathan Lynch 1 sibling, 1 reply; 22+ messages in thread From: Will Deacon @ 2014-09-26 9:26 UTC (permalink / raw) To: linux-arm-kernel On Fri, Sep 26, 2014 at 08:04:30AM +0100, Daniel Lezcano wrote: > On 09/18/2014 04:59 PM, Nathan Lynch wrote: > > The arm and arm64 VDSOs need CP15 access to the architected counter. > > If this is unavailable (which is allowed by ARM v7), indicate this by > > changing the clocksource name to "arch_mem_counter" before registering > > the clocksource. > > > > Suggested by Stephen Boyd. > > > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > > Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> > > Hi Nathan, > > is it possible to have the acked-by of the different maintainers for the > other patches ? FWIW: I acked the series: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/289143.html There's ongoing debate about whether or not we should have a vdso, but this series still makes sense in isolation. Will ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable 2014-09-26 9:26 ` Will Deacon @ 2014-09-26 11:34 ` Daniel Lezcano 0 siblings, 0 replies; 22+ messages in thread From: Daniel Lezcano @ 2014-09-26 11:34 UTC (permalink / raw) To: linux-arm-kernel On 09/26/2014 11:26 AM, Will Deacon wrote: > On Fri, Sep 26, 2014 at 08:04:30AM +0100, Daniel Lezcano wrote: >> On 09/18/2014 04:59 PM, Nathan Lynch wrote: >>> The arm and arm64 VDSOs need CP15 access to the architected counter. >>> If this is unavailable (which is allowed by ARM v7), indicate this by >>> changing the clocksource name to "arch_mem_counter" before registering >>> the clocksource. >>> >>> Suggested by Stephen Boyd. >>> >>> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> >>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> >> >> Hi Nathan, >> >> is it possible to have the acked-by of the different maintainers for the >> other patches ? > > FWIW: I acked the series: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/289143.html > > There's ongoing debate about whether or not we should have a vdso, but this > series still makes sense in isolation. Ok. Thanks for the clarification. > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable 2014-09-26 7:04 ` Daniel Lezcano 2014-09-26 9:26 ` Will Deacon @ 2014-09-26 14:55 ` Nathan Lynch 1 sibling, 0 replies; 22+ messages in thread From: Nathan Lynch @ 2014-09-26 14:55 UTC (permalink / raw) To: linux-arm-kernel Hi Daniel, On 09/26/2014 02:04 AM, Daniel Lezcano wrote: > On 09/18/2014 04:59 PM, Nathan Lynch wrote: >> The arm and arm64 VDSOs need CP15 access to the architected counter. >> If this is unavailable (which is allowed by ARM v7), indicate this by >> changing the clocksource name to "arch_mem_counter" before registering >> the clocksource. >> >> Suggested by Stephen Boyd. >> >> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> >> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> > > Hi Nathan, > > is it possible to have the acked-by of the different maintainers for the > other patches ? As Will said, he acked the series. > Are these patches targeted to be merged for 3.18 ? 3.18 is preferable, if it's not too late. But nothing will break if it needs to wait. Thanks, Nathan ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] clocksource: arm_arch_timer: enable counter access for 32-bit ARM 2014-09-18 14:59 [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable Nathan Lynch @ 2014-09-18 14:59 ` Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 3/3] clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable Nathan Lynch 2014-09-22 15:39 ` [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Will Deacon 3 siblings, 0 replies; 22+ messages in thread From: Nathan Lynch @ 2014-09-18 14:59 UTC (permalink / raw) To: linux-arm-kernel The only difference between arm and arm64's implementations of arch_counter_set_user_access is that 32-bit ARM does not enable user access to the virtual counter. We want to enable this access for the 32-bit ARM VDSO, so copy the arm64 version to the driver itself, and remove the arch-specific implementations. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- arch/arm/include/asm/arch_timer.h | 14 -------------- arch/arm64/include/asm/arch_timer.h | 17 ----------------- drivers/clocksource/arm_arch_timer.c | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index 0704e0cf5571..b7ae44464231 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -99,20 +99,6 @@ 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); -} - static inline void arch_timer_evtstrm_enable(int divider) { u32 cntkctl = arch_timer_get_cntkctl(); diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h index 9400596a0f39..49e94c677e7a 100644 --- a/arch/arm64/include/asm/arch_timer.h +++ b/arch/arm64/include/asm/arch_timer.h @@ -104,23 +104,6 @@ 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 */ - /* 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_PCT_ACCESS_EN); - - /* Enable user access to the virtual counter */ - cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN; - - arch_timer_set_cntkctl(cntkctl); -} - static inline void arch_timer_evtstrm_enable(int divider) { u32 cntkctl = arch_timer_get_cntkctl(); diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index c99afdf12e98..31781fdd7c19 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -312,6 +312,23 @@ static void arch_timer_configure_evtstream(void) arch_timer_evtstrm_enable(min(pos, 15)); } +static void arch_counter_set_user_access(void) +{ + u32 cntkctl = arch_timer_get_cntkctl(); + + /* Disable user access to the timers and the physical counter */ + /* 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_PCT_ACCESS_EN); + + /* Enable user access to the virtual counter */ + cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN; + + arch_timer_set_cntkctl(cntkctl); +} + static int arch_timer_setup(struct clock_event_device *clk) { __arch_timer_setup(ARCH_CP15_TIMER, clk); -- 1.9.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable 2014-09-18 14:59 [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 2/3] clocksource: arm_arch_timer: enable counter access for 32-bit ARM Nathan Lynch @ 2014-09-18 14:59 ` Nathan Lynch 2014-09-22 15:39 ` [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Will Deacon 3 siblings, 0 replies; 22+ messages in thread From: Nathan Lynch @ 2014-09-18 14:59 UTC (permalink / raw) To: linux-arm-kernel The arch_timer_evtstrm_enable hooks in arm and arm64 are substantially similar, the only difference being a CONFIG_COMPAT-conditional section which is relevant only for arm64. Copy the arm64 version to the driver, removing the arch-specific hooks. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- arch/arm/include/asm/arch_timer.h | 11 ----------- arch/arm64/include/asm/arch_timer.h | 14 -------------- drivers/clocksource/arm_arch_timer.c | 15 +++++++++++++++ 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index b7ae44464231..92793ba69c40 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -99,17 +99,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl) asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl)); } -static inline void arch_timer_evtstrm_enable(int divider) -{ - u32 cntkctl = arch_timer_get_cntkctl(); - 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; - arch_timer_set_cntkctl(cntkctl); - elf_hwcap |= HWCAP_EVTSTRM; -} - #endif #endif diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h index 49e94c677e7a..f19097134b02 100644 --- a/arch/arm64/include/asm/arch_timer.h +++ b/arch/arm64/include/asm/arch_timer.h @@ -104,20 +104,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl) asm volatile("msr cntkctl_el1, %0" : : "r" (cntkctl)); } -static inline void arch_timer_evtstrm_enable(int divider) -{ - u32 cntkctl = arch_timer_get_cntkctl(); - 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; - arch_timer_set_cntkctl(cntkctl); - elf_hwcap |= HWCAP_EVTSTRM; -#ifdef CONFIG_COMPAT - compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM; -#endif -} - static inline u64 arch_counter_get_cntvct(void) { u64 cval; diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 31781fdd7c19..ce5f99e957b9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -299,6 +299,21 @@ static void __arch_timer_setup(unsigned type, clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff); } +static void arch_timer_evtstrm_enable(int divider) +{ + u32 cntkctl = arch_timer_get_cntkctl(); + + 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; + arch_timer_set_cntkctl(cntkctl); + elf_hwcap |= HWCAP_EVTSTRM; +#ifdef CONFIG_COMPAT + compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM; +#endif +} + static void arch_timer_configure_evtstream(void) { int evt_stream_div, pos; -- 1.9.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-18 14:59 [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Nathan Lynch ` (2 preceding siblings ...) 2014-09-18 14:59 ` [PATCH v2 3/3] clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable Nathan Lynch @ 2014-09-22 15:39 ` Will Deacon 2014-09-22 16:15 ` Nathan Lynch 2014-09-22 22:30 ` Russell King - ARM Linux 3 siblings, 2 replies; 22+ messages in thread From: Will Deacon @ 2014-09-22 15:39 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: > This series contains the necessary changes to allow architected timer > access from user-space on 32-bit ARM. This allows the VDSO to support > high resolution timestamps for clock_gettime and gettimeofday. This > also merges substantially similar code from arm and arm64 into the > core arm_arch_timer driver. > > The functional changes are: > - When available, CNTVCT is made readable by user space on arm, as it > is on arm64. > - The clocksource name becomes "arch_mem_counter" if CP15 access to > the counter is not available. > > These changes have been carried as part of the ARM VDSO patch set over > the last several months, but I am splitting them out here as I assume > they should go through the clocksource maintainers. For the series: Acked-by: Will Deacon <will.deacon@arm.com> I'm not sure which tree the arch-timer stuff usually goes through, but the arm/arm64 bits look fine so I'm happy for them to merged together. Will ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-22 15:39 ` [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Will Deacon @ 2014-09-22 16:15 ` Nathan Lynch 2014-09-22 18:56 ` Daniel Lezcano 2014-09-22 22:30 ` Russell King - ARM Linux 1 sibling, 1 reply; 22+ messages in thread From: Nathan Lynch @ 2014-09-22 16:15 UTC (permalink / raw) To: linux-arm-kernel On 09/22/2014 10:39 AM, Will Deacon wrote: > On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: >> This series contains the necessary changes to allow architected timer >> access from user-space on 32-bit ARM. This allows the VDSO to support >> high resolution timestamps for clock_gettime and gettimeofday. This >> also merges substantially similar code from arm and arm64 into the >> core arm_arch_timer driver. >> >> The functional changes are: >> - When available, CNTVCT is made readable by user space on arm, as it >> is on arm64. >> - The clocksource name becomes "arch_mem_counter" if CP15 access to >> the counter is not available. >> >> These changes have been carried as part of the ARM VDSO patch set over >> the last several months, but I am splitting them out here as I assume >> they should go through the clocksource maintainers. > > For the series: > > Acked-by: Will Deacon <will.deacon@arm.com> > > I'm not sure which tree the arch-timer stuff usually goes through, but > the arm/arm64 bits look fine so I'm happy for them to merged together. Thanks Will. MAINTAINERS does not have a specific entry for drivers/clocksource/arm_arch_timer.c, so I am working under the assumption that the series would go through Daniel or Thomas. Daniel and/or Thomas, can you take this series please? ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-22 16:15 ` Nathan Lynch @ 2014-09-22 18:56 ` Daniel Lezcano 0 siblings, 0 replies; 22+ messages in thread From: Daniel Lezcano @ 2014-09-22 18:56 UTC (permalink / raw) To: linux-arm-kernel On 09/22/2014 06:15 PM, Nathan Lynch wrote: [ ... ] > MAINTAINERS does not have a specific entry for > drivers/clocksource/arm_arch_timer.c, so I am working under the > assumption that the series would go through Daniel or Thomas. > > Daniel and/or Thomas, can you take this series please? > Yes, I will take care of it. -- Daniel -- <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-22 15:39 ` [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Will Deacon 2014-09-22 16:15 ` Nathan Lynch @ 2014-09-22 22:30 ` Russell King - ARM Linux 2014-09-23 0:28 ` Nathan Lynch 2014-09-24 14:45 ` Catalin Marinas 1 sibling, 2 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2014-09-22 22:30 UTC (permalink / raw) To: linux-arm-kernel On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: > On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: > > This series contains the necessary changes to allow architected timer > > access from user-space on 32-bit ARM. This allows the VDSO to support > > high resolution timestamps for clock_gettime and gettimeofday. This > > also merges substantially similar code from arm and arm64 into the > > core arm_arch_timer driver. > > > > The functional changes are: > > - When available, CNTVCT is made readable by user space on arm, as it > > is on arm64. > > - The clocksource name becomes "arch_mem_counter" if CP15 access to > > the counter is not available. > > > > These changes have been carried as part of the ARM VDSO patch set over > > the last several months, but I am splitting them out here as I assume > > they should go through the clocksource maintainers. > > For the series: > > Acked-by: Will Deacon <will.deacon@arm.com> > > I'm not sure which tree the arch-timer stuff usually goes through, but > the arm/arm64 bits look fine so I'm happy for them to merged together. I raised a while back with Will whether there's much point to having this on ARM. While it's useful for virtualisation, the majority of 32-bit ARM doesn't run virtualised. So there's little point in having the VDSO on the majority of platforms - it will just add additional unnecessary cycles slowing down the system calls that the VDSO is designed to try to speed up. So, my view is that this VDSO will only be of very limited use for 32-bit ARM, and should not be exposed to userspace unless there is a reason for it to be exposed (iow, the hardware necessary to support it is present.) -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-22 22:30 ` Russell King - ARM Linux @ 2014-09-23 0:28 ` Nathan Lynch 2014-09-24 14:12 ` Christopher Covington 2014-09-24 14:45 ` Catalin Marinas 1 sibling, 1 reply; 22+ messages in thread From: Nathan Lynch @ 2014-09-23 0:28 UTC (permalink / raw) To: linux-arm-kernel On 09/22/2014 05:30 PM, Russell King - ARM Linux wrote: > On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: >> On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: >>> This series contains the necessary changes to allow architected timer >>> access from user-space on 32-bit ARM. This allows the VDSO to support >>> high resolution timestamps for clock_gettime and gettimeofday. This >>> also merges substantially similar code from arm and arm64 into the >>> core arm_arch_timer driver. >>> >>> The functional changes are: >>> - When available, CNTVCT is made readable by user space on arm, as it >>> is on arm64. >>> - The clocksource name becomes "arch_mem_counter" if CP15 access to >>> the counter is not available. >>> >>> These changes have been carried as part of the ARM VDSO patch set over >>> the last several months, but I am splitting them out here as I assume >>> they should go through the clocksource maintainers. >> >> For the series: >> >> Acked-by: Will Deacon <will.deacon@arm.com> >> >> I'm not sure which tree the arch-timer stuff usually goes through, but >> the arm/arm64 bits look fine so I'm happy for them to merged together. > > I raised a while back with Will whether there's much point to having > this on ARM. While it's useful for virtualisation, the majority of > 32-bit ARM doesn't run virtualised. So there's little point in having > the VDSO on the majority of platforms - it will just add additional > unnecessary cycles slowing down the system calls that the VDSO is > designed to try to speed up. Hmm, this patch set is merely exposing the hardware counter when it is present for the VDSO's use; I take it you have no objection to that? While the 32-bit ARM VDSO I've posted (in a different thread) exploits a facility that is required by the virtualization option in the architecture, its utility is not limited to guest operating systems. > So, my view is that this VDSO will only be of very limited use for > 32-bit ARM, and should not be exposed to userspace unless there is > a reason for it to be exposed (iow, the hardware necessary to support > it is present.) My thinking is that it should prove useful in a growing subset of v7 CPUs. It is useful today on Cortex-A15 and -A7, and I believe -A12 and -A17 implement the generic timer facility as well. Now if you're saying that we shouldn't slow down gettimeofday on systems which lack a hardware counter that can be safely exposed to userspace, I can work with that. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-23 0:28 ` Nathan Lynch @ 2014-09-24 14:12 ` Christopher Covington 2014-09-24 14:32 ` Nathan Lynch 0 siblings, 1 reply; 22+ messages in thread From: Christopher Covington @ 2014-09-24 14:12 UTC (permalink / raw) To: linux-arm-kernel Hi Nathan, On 09/22/2014 08:28 PM, Nathan Lynch wrote: > On 09/22/2014 05:30 PM, Russell King - ARM Linux wrote: >> On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: >>> On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: >>>> This series contains the necessary changes to allow architected timer >>>> access from user-space on 32-bit ARM. This allows the VDSO to support >>>> high resolution timestamps for clock_gettime and gettimeofday. This >>>> also merges substantially similar code from arm and arm64 into the >>>> core arm_arch_timer driver. >>>> >>>> The functional changes are: >>>> - When available, CNTVCT is made readable by user space on arm, as it >>>> is on arm64. >>>> - The clocksource name becomes "arch_mem_counter" if CP15 access to >>>> the counter is not available. >>>> >>>> These changes have been carried as part of the ARM VDSO patch set over >>>> the last several months, but I am splitting them out here as I assume >>>> they should go through the clocksource maintainers. >>> >>> For the series: >>> >>> Acked-by: Will Deacon <will.deacon@arm.com> >>> >>> I'm not sure which tree the arch-timer stuff usually goes through, but >>> the arm/arm64 bits look fine so I'm happy for them to merged together. >> >> I raised a while back with Will whether there's much point to having >> this on ARM. While it's useful for virtualisation, the majority of >> 32-bit ARM doesn't run virtualised. So there's little point in having >> the VDSO on the majority of platforms - it will just add additional >> unnecessary cycles slowing down the system calls that the VDSO is >> designed to try to speed up. > > Hmm, this patch set is merely exposing the hardware counter when it is > present for the VDSO's use; I take it you have no objection to that? > > While the 32-bit ARM VDSO I've posted (in a different thread) exploits a > facility that is required by the virtualization option in the > architecture, its utility is not limited to guest operating systems. Just to clarify, were the performance improvements you measured from a virtualized guest or native? >> So, my view is that this VDSO will only be of very limited use for >> 32-bit ARM, and should not be exposed to userspace unless there is >> a reason for it to be exposed (iow, the hardware necessary to support >> it is present.) > > My thinking is that it should prove useful in a growing subset of v7 > CPUs. It is useful today on Cortex-A15 and -A7, and I believe -A12 and > -A17 implement the generic timer facility as well. I count 18 dts* files that have "arm,armv7-timer", including platforms with Krait, Exynos, and Tegra processors. Christopher -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 14:12 ` Christopher Covington @ 2014-09-24 14:32 ` Nathan Lynch 2014-09-24 14:50 ` Russell King - ARM Linux 0 siblings, 1 reply; 22+ messages in thread From: Nathan Lynch @ 2014-09-24 14:32 UTC (permalink / raw) To: linux-arm-kernel On 09/24/2014 09:12 AM, Christopher Covington wrote: > Hi Nathan, > > On 09/22/2014 08:28 PM, Nathan Lynch wrote: >> On 09/22/2014 05:30 PM, Russell King - ARM Linux wrote: >>> On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: >>>> On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: >>>>> This series contains the necessary changes to allow architected timer >>>>> access from user-space on 32-bit ARM. This allows the VDSO to support >>>>> high resolution timestamps for clock_gettime and gettimeofday. This >>>>> also merges substantially similar code from arm and arm64 into the >>>>> core arm_arch_timer driver. >>>>> >>>>> The functional changes are: >>>>> - When available, CNTVCT is made readable by user space on arm, as it >>>>> is on arm64. >>>>> - The clocksource name becomes "arch_mem_counter" if CP15 access to >>>>> the counter is not available. >>>>> >>>>> These changes have been carried as part of the ARM VDSO patch set over >>>>> the last several months, but I am splitting them out here as I assume >>>>> they should go through the clocksource maintainers. >>>> >>>> For the series: >>>> >>>> Acked-by: Will Deacon <will.deacon@arm.com> >>>> >>>> I'm not sure which tree the arch-timer stuff usually goes through, but >>>> the arm/arm64 bits look fine so I'm happy for them to merged together. >>> >>> I raised a while back with Will whether there's much point to having >>> this on ARM. While it's useful for virtualisation, the majority of >>> 32-bit ARM doesn't run virtualised. So there's little point in having >>> the VDSO on the majority of platforms - it will just add additional >>> unnecessary cycles slowing down the system calls that the VDSO is >>> designed to try to speed up. >> >> Hmm, this patch set is merely exposing the hardware counter when it is >> present for the VDSO's use; I take it you have no objection to that? >> >> While the 32-bit ARM VDSO I've posted (in a different thread) exploits a >> facility that is required by the virtualization option in the >> architecture, its utility is not limited to guest operating systems. > > Just to clarify, were the performance improvements you measured from a > virtualized guest or native? Yeah I should have been explicit about this. My tests and measurements (and all test results I've received from others, I believe) have been on native/host kernels, not guests. >>> So, my view is that this VDSO will only be of very limited use for >>> 32-bit ARM, and should not be exposed to userspace unless there is >>> a reason for it to be exposed (iow, the hardware necessary to support >>> it is present.) >> >> My thinking is that it should prove useful in a growing subset of v7 >> CPUs. It is useful today on Cortex-A15 and -A7, and I believe -A12 and >> -A17 implement the generic timer facility as well. > > I count 18 dts* files that have "arm,armv7-timer", including platforms with > Krait, Exynos, and Tegra processors. Yup. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 14:32 ` Nathan Lynch @ 2014-09-24 14:50 ` Russell King - ARM Linux 2014-09-24 16:58 ` Nathan Lynch 0 siblings, 1 reply; 22+ messages in thread From: Russell King - ARM Linux @ 2014-09-24 14:50 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 09:32:54AM -0500, Nathan Lynch wrote: > On 09/24/2014 09:12 AM, Christopher Covington wrote: > > Hi Nathan, > > > > On 09/22/2014 08:28 PM, Nathan Lynch wrote: > >> Hmm, this patch set is merely exposing the hardware counter when it is > >> present for the VDSO's use; I take it you have no objection to that? > >> > >> While the 32-bit ARM VDSO I've posted (in a different thread) exploits a > >> facility that is required by the virtualization option in the > >> architecture, its utility is not limited to guest operating systems. > > > > Just to clarify, were the performance improvements you measured from a > > virtualized guest or native? > > Yeah I should have been explicit about this. My tests and measurements > (and all test results I've received from others, I believe) have been on > native/host kernels, not guests. Have there been any measurements on systems without the architected timers? > > I count 18 dts* files that have "arm,armv7-timer", including platforms with > > Krait, Exynos, and Tegra processors. > > Yup. That's not the full story. Almost every ARM to date has not had an architected timer. Architected timers are a recent addition - as pointed out, a Cortex A7/A12/A15 invention. Most of the platforms I see are Cortex A9 which doesn't have any architected timers. Yes, it may be fun to work on new hardware and make that perform much better than previous, but we should not loose sight that there is older hardware out there, and we shouldn't unnecessarily penalise it when adding new features. What we /need/ to know is what the effect providing a VDSO in an environment without an architected timer (so using the VDSO fallback functions calling the syscalls) and having glibc use it is compared to the current situation where there is no VDSO for glibc to use. If you can show that there's no difference, then I'm happy to go with always providing the VDSO. If there's a detrimental effect (which I suspect there may be, since we now have to have glibc test to see if the VDSO is there, jump to the VDSO, the VDSO then tests whether we have an architected timer, and then we finally get to issue the syscall), then we must avoid providing the VDSO on systems which have no architected timer. Things like the time keeping syscalls tend to be hot paths, and having extra unnecessary cycles in them can hurt userspace performance. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 14:50 ` Russell King - ARM Linux @ 2014-09-24 16:58 ` Nathan Lynch 2014-09-24 18:58 ` Russell King - ARM Linux 0 siblings, 1 reply; 22+ messages in thread From: Nathan Lynch @ 2014-09-24 16:58 UTC (permalink / raw) To: linux-arm-kernel On 09/24/2014 09:50 AM, Russell King - ARM Linux wrote: > On Wed, Sep 24, 2014 at 09:32:54AM -0500, Nathan Lynch wrote: >> On 09/24/2014 09:12 AM, Christopher Covington wrote: >>> Hi Nathan, >>> >>> On 09/22/2014 08:28 PM, Nathan Lynch wrote: >>>> Hmm, this patch set is merely exposing the hardware counter when it is >>>> present for the VDSO's use; I take it you have no objection to that? >>>> >>>> While the 32-bit ARM VDSO I've posted (in a different thread) exploits a >>>> facility that is required by the virtualization option in the >>>> architecture, its utility is not limited to guest operating systems. >>> >>> Just to clarify, were the performance improvements you measured from a >>> virtualized guest or native? >> >> Yeah I should have been explicit about this. My tests and measurements >> (and all test results I've received from others, I believe) have been on >> native/host kernels, not guests. > > Have there been any measurements on systems without the architected > timers? I do test on iMX6 regularly. Afraid I don't have any pre-v7 hardware to check though. Here's a report from you from an earlier submission that shows little/no impact: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267552.html But admittedly vdsotest is just doing rudimentary microbenchmarking. Running a lttng-ust workload that emits tracepoints as fast as possible (lttng-ust calls clock_gettime and getcpu on every tracepoint), I see about 1% degradation on iMX6. >>> I count 18 dts* files that have "arm,armv7-timer", including platforms with >>> Krait, Exynos, and Tegra processors. >> >> Yup. > > That's not the full story. Almost every ARM to date has not had an > architected timer. Architected timers are a recent addition - as > pointed out, a Cortex A7/A12/A15 invention. Most of the platforms I > see are Cortex A9 which doesn't have any architected timers. > > Yes, it may be fun to work on new hardware and make that perform > much better than previous, but we should not loose sight that there > is older hardware out there, and we shouldn't unnecessarily penalise > it when adding new features. Agreed, of course, and I'll include more detailed results from systems without the architected timer in future submissions. > What we /need/ to know is what the effect providing a VDSO in an > environment without an architected timer (so using the VDSO fallback > functions calling the syscalls) and having glibc use it is compared > to the current situation where there is no VDSO for glibc to use. > > If you can show that there's no difference, then I'm happy to go with > always providing the VDSO. If there's a detrimental effect (which I > suspect there may be, since we now have to have glibc test to see if > the VDSO is there, jump to the VDSO, the VDSO then tests whether we > have an architected timer, and then we finally get to issue the > syscall), then we must avoid providing the VDSO on systems which have > no architected timer. One point I would like to raise is that the VDSO provides (or could be made to provide) acceleration for APIs that are unrelated to the architected timer: - clock_gettime with CLOCK_REALTIME_COARSE and CLOCK_MONOTONIC_COARSE. This is currently included. - getcpu, which I had planned on submitting later. I don't know whether the coarse clock support is compelling; they don't seem to be commonly used. But there is a nice 4-5x speedup for those on iMX6. getcpu, on the other hand, is one of the two system calls lttng-ust uses in every tracepoint emitted, and I would like to have it available in the VDSO on all systems capable of supporting the implementation, which may take the form of co-opting TPIDRURW or some other register. So the question of whether to provide the VDSO may not hinge on whether the architected timer is available. None of which is to argue that unnecessarily degrading gettimeofday performance on some systems for the benefit of others is acceptable. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 16:58 ` Nathan Lynch @ 2014-09-24 18:58 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2014-09-24 18:58 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 11:58:19AM -0500, Nathan Lynch wrote: > On 09/24/2014 09:50 AM, Russell King - ARM Linux wrote: > > On Wed, Sep 24, 2014 at 09:32:54AM -0500, Nathan Lynch wrote: > >> On 09/24/2014 09:12 AM, Christopher Covington wrote: > >>> Hi Nathan, > >>> > >>> On 09/22/2014 08:28 PM, Nathan Lynch wrote: > >>>> Hmm, this patch set is merely exposing the hardware counter when it is > >>>> present for the VDSO's use; I take it you have no objection to that? > >>>> > >>>> While the 32-bit ARM VDSO I've posted (in a different thread) exploits a > >>>> facility that is required by the virtualization option in the > >>>> architecture, its utility is not limited to guest operating systems. > >>> > >>> Just to clarify, were the performance improvements you measured from a > >>> virtualized guest or native? > >> > >> Yeah I should have been explicit about this. My tests and measurements > >> (and all test results I've received from others, I believe) have been on > >> native/host kernels, not guests. > > > > Have there been any measurements on systems without the architected > > timers? > > I do test on iMX6 regularly. Afraid I don't have any pre-v7 hardware to > check though. Right, and iMX6 is Cortex-A9, which doesn't have the architected timer. > Here's a report from you from an earlier submission that shows little/no > impact: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267552.html Yes, that's my email. > But admittedly vdsotest is just doing rudimentary microbenchmarking. vdsotest is comparing its own implementation (a function directly calling the syscall) with a direct call into the vdso. This isn't what I'm interested in, because applications won't be calling the vdso directly. They will be calling the vdso _via_ glibc. What I'm really interested in is what the difference is to an application between the present case of not having any vdso (iow. not having the vdso support code in glibc) and: - having the vdso support code in glibc, but without the vdso being provided. - having the vdso support code in glibc, having the vdso provided, but without the architected timer. Only then can we actually see what the /overall/ impact of VDSO support is on CPUs without the architected timer. > > Running a lttng-ust workload that emits tracepoints as fast as possible > (lttng-ust calls clock_gettime and getcpu on every tracepoint), I see > about 1% degradation on iMX6. > > > >>> I count 18 dts* files that have "arm,armv7-timer", including platforms with > >>> Krait, Exynos, and Tegra processors. > >> > >> Yup. > > > > That's not the full story. Almost every ARM to date has not had an > > architected timer. Architected timers are a recent addition - as > > pointed out, a Cortex A7/A12/A15 invention. Most of the platforms I > > see are Cortex A9 which doesn't have any architected timers. > > > > Yes, it may be fun to work on new hardware and make that perform > > much better than previous, but we should not loose sight that there > > is older hardware out there, and we shouldn't unnecessarily penalise > > it when adding new features. > > Agreed, of course, and I'll include more detailed results from systems > without the architected timer in future submissions. > > > > What we /need/ to know is what the effect providing a VDSO in an > > environment without an architected timer (so using the VDSO fallback > > functions calling the syscalls) and having glibc use it is compared > > to the current situation where there is no VDSO for glibc to use. > > > > If you can show that there's no difference, then I'm happy to go with > > always providing the VDSO. If there's a detrimental effect (which I > > suspect there may be, since we now have to have glibc test to see if > > the VDSO is there, jump to the VDSO, the VDSO then tests whether we > > have an architected timer, and then we finally get to issue the > > syscall), then we must avoid providing the VDSO on systems which have > > no architected timer. > > One point I would like to raise is that the VDSO provides (or could be > made to provide) acceleration for APIs that are unrelated to the > architected timer: > > - clock_gettime with CLOCK_REALTIME_COARSE and CLOCK_MONOTONIC_COARSE. > This is currently included. > > - getcpu, which I had planned on submitting later. > > I don't know whether the coarse clock support is compelling; they don't > seem to be commonly used. But there is a nice 4-5x speedup for those on > iMX6. > > getcpu, on the other hand, is one of the two system calls lttng-ust uses > in every tracepoint emitted, and I would like to have it available in > the VDSO on all systems capable of supporting the implementation, which > may take the form of co-opting TPIDRURW or some other register. Right, so the decision is whether a vdso implementation of the uncomon coarse clocks and eventually getcpu benefit from this. I don't think normal applications make many getcpu calls either - most applications don't care which CPU they run on. > None of which is to argue that unnecessarily degrading gettimeofday > performance on some systems for the benefit of others is acceptable. Right - the fine grained time functions are far more important as applications tend to call those quite a lot (some applications call them a truely excessive number of times). -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-22 22:30 ` Russell King - ARM Linux 2014-09-23 0:28 ` Nathan Lynch @ 2014-09-24 14:45 ` Catalin Marinas 2014-09-24 14:52 ` Russell King - ARM Linux 1 sibling, 1 reply; 22+ messages in thread From: Catalin Marinas @ 2014-09-24 14:45 UTC (permalink / raw) To: linux-arm-kernel On Mon, Sep 22, 2014 at 11:30:23PM +0100, Russell King - ARM Linux wrote: > On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: > > On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: > > > This series contains the necessary changes to allow architected timer > > > access from user-space on 32-bit ARM. This allows the VDSO to support > > > high resolution timestamps for clock_gettime and gettimeofday. This > > > also merges substantially similar code from arm and arm64 into the > > > core arm_arch_timer driver. > > > > > > The functional changes are: > > > - When available, CNTVCT is made readable by user space on arm, as it > > > is on arm64. > > > - The clocksource name becomes "arch_mem_counter" if CP15 access to > > > the counter is not available. > > > > > > These changes have been carried as part of the ARM VDSO patch set over > > > the last several months, but I am splitting them out here as I assume > > > they should go through the clocksource maintainers. > > > > For the series: > > > > Acked-by: Will Deacon <will.deacon@arm.com> > > > > I'm not sure which tree the arch-timer stuff usually goes through, but > > the arm/arm64 bits look fine so I'm happy for them to merged together. > > I raised a while back with Will whether there's much point to having > this on ARM. While it's useful for virtualisation, the majority of > 32-bit ARM doesn't run virtualised. This has nothing to do with virtualisation. The main reason we use CNTVCT is to not require kernel binary differences when running the OS as host or guest. But it does _not_ mean that it is only used when running as a guest. > So there's little point in having the VDSO on the majority of > platforms - it will just add additional unnecessary cycles slowing > down the system calls that the VDSO is designed to try to speed up. A good reason for VDSO is to avoid a system call for gettimeofday when you can read the clocks source from user space. That's a significant improvement on CPUs like A7, A15. Now, I understand there will be a few cycles lost with the syscall indirection via VDSO when the architected timers are not available. If you are concerned about this, maybe the VDSO (or the corresponding gettimeofday) function should be disabled at boot time. IIRC, the VDSO gettimeofday function would override the glibc one at dynamic loading time but if the VDSO isn't present, glibc would still work as usual. -- Catalin ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 14:45 ` Catalin Marinas @ 2014-09-24 14:52 ` Russell King - ARM Linux 2014-09-24 15:04 ` Catalin Marinas 0 siblings, 1 reply; 22+ messages in thread From: Russell King - ARM Linux @ 2014-09-24 14:52 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 03:45:41PM +0100, Catalin Marinas wrote: > On Mon, Sep 22, 2014 at 11:30:23PM +0100, Russell King - ARM Linux wrote: > > I raised a while back with Will whether there's much point to having > > this on ARM. While it's useful for virtualisation, the majority of > > 32-bit ARM doesn't run virtualised. > > This has nothing to do with virtualisation. The main reason we use > CNTVCT is to not require kernel binary differences when running the OS > as host or guest. But it does _not_ mean that it is only used when > running as a guest. > > > So there's little point in having the VDSO on the majority of > > platforms - it will just add additional unnecessary cycles slowing > > down the system calls that the VDSO is designed to try to speed up. > > A good reason for VDSO is to avoid a system call for gettimeofday when > you can read the clocks source from user space. That's a significant > improvement on CPUs like A7, A15. I'm *not* arguing against having a VDSO to speed up that crap. What I'm trying to get to the bottom of - something which has been totally lost sight of - is what the friggin effect of this stuff is on CPUs *without* the architected timer. Until I get an answer to what the measured effect is, I'm saying no to VDSO on ARM, because - as seems to be the norm - the evaluation job is only half done. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 14:52 ` Russell King - ARM Linux @ 2014-09-24 15:04 ` Catalin Marinas 2014-09-24 15:08 ` Russell King - ARM Linux 0 siblings, 1 reply; 22+ messages in thread From: Catalin Marinas @ 2014-09-24 15:04 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 03:52:57PM +0100, Russell King - ARM Linux wrote: > On Wed, Sep 24, 2014 at 03:45:41PM +0100, Catalin Marinas wrote: > > On Mon, Sep 22, 2014 at 11:30:23PM +0100, Russell King - ARM Linux wrote: > > > I raised a while back with Will whether there's much point to having > > > this on ARM. While it's useful for virtualisation, the majority of > > > 32-bit ARM doesn't run virtualised. > > > > This has nothing to do with virtualisation. The main reason we use > > CNTVCT is to not require kernel binary differences when running the OS > > as host or guest. But it does _not_ mean that it is only used when > > running as a guest. > > > > > So there's little point in having the VDSO on the majority of > > > platforms - it will just add additional unnecessary cycles slowing > > > down the system calls that the VDSO is designed to try to speed up. > > > > A good reason for VDSO is to avoid a system call for gettimeofday when > > you can read the clocks source from user space. That's a significant > > improvement on CPUs like A7, A15. > > I'm *not* arguing against having a VDSO to speed up that crap. What > I'm trying to get to the bottom of - something which has been totally > lost sight of - is what the friggin effect of this stuff is on CPUs > *without* the architected timer. > > Until I get an answer to what the measured effect is, I'm saying no to > VDSO on ARM, because - as seems to be the norm - the evaluation job is > only half done. I agree. If there is an overhead (possibly), I think it can be solved in software maybe by having two VDSO images, one with gettimeofday and one without. If it's only gettimeofday in VDSO (and signal return still via the vectors page), we could just avoid inserting it into the user address space when arch timers aren't present. On arm64 we expect the presence of the arch timer and didn't bother with such scenarios. -- Catalin ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation 2014-09-24 15:04 ` Catalin Marinas @ 2014-09-24 15:08 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2014-09-24 15:08 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 04:04:09PM +0100, Catalin Marinas wrote: > On Wed, Sep 24, 2014 at 03:52:57PM +0100, Russell King - ARM Linux wrote: > > I'm *not* arguing against having a VDSO to speed up that crap. What > > I'm trying to get to the bottom of - something which has been totally > > lost sight of - is what the friggin effect of this stuff is on CPUs > > *without* the architected timer. > > > > Until I get an answer to what the measured effect is, I'm saying no to > > VDSO on ARM, because - as seems to be the norm - the evaluation job is > > only half done. > > I agree. > > If there is an overhead (possibly), I think it can be solved in software > maybe by having two VDSO images, one with gettimeofday and one without. > If it's only gettimeofday in VDSO (and signal return still via the > vectors page), we could just avoid inserting it into the user address > space when arch timers aren't present. The signal handling is no longer in the vectors page (it hasn't been for over a year now), it is in a separate page which is mapped randomly. These VDSO patches change it to place it along side the VDSO pages. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-09-26 14:55 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-18 14:59 [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable Nathan Lynch 2014-09-26 7:04 ` Daniel Lezcano 2014-09-26 9:26 ` Will Deacon 2014-09-26 11:34 ` Daniel Lezcano 2014-09-26 14:55 ` Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 2/3] clocksource: arm_arch_timer: enable counter access for 32-bit ARM Nathan Lynch 2014-09-18 14:59 ` [PATCH v2 3/3] clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable Nathan Lynch 2014-09-22 15:39 ` [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation Will Deacon 2014-09-22 16:15 ` Nathan Lynch 2014-09-22 18:56 ` Daniel Lezcano 2014-09-22 22:30 ` Russell King - ARM Linux 2014-09-23 0:28 ` Nathan Lynch 2014-09-24 14:12 ` Christopher Covington 2014-09-24 14:32 ` Nathan Lynch 2014-09-24 14:50 ` Russell King - ARM Linux 2014-09-24 16:58 ` Nathan Lynch 2014-09-24 18:58 ` Russell King - ARM Linux 2014-09-24 14:45 ` Catalin Marinas 2014-09-24 14:52 ` Russell King - ARM Linux 2014-09-24 15:04 ` Catalin Marinas 2014-09-24 15:08 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).