[PATCH v3 03/20] arm64: Use the physical counter when available for read_cycles
From: Marc Zyngier <hidden>
Date: 2017-10-18 15:52:26
Also in:
kvm, kvmarm
On Wed, Oct 18 2017 at 1:34:05 pm BST, Christoffer Dall [off-list ref] wrote:
On Mon, Oct 09, 2017 at 05:21:24PM +0100, Marc Zyngier wrote:quoted
On 23/09/17 01:41, Christoffer Dall wrote:quoted
Currently get_cycles() is hardwired to arch_counter_get_cntvct() on arm64, but as we move to using the physical timer for the in-kernel time-keeping, we need to make that more flexible. First, we need to make sure the physical counter can be read on equal terms to the virtual counter, which includes adding physical counter read functions for timers that require errata. Second, we need to make a choice between reading the physical vs virtual counter, depending on which timer is used for time keeping in the kernel otherwise. We can do this using a static key to avoid a performance penalty during runtime when reading the counter. Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <redacted> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Marc Zyngier <redacted> Signed-off-by: Christoffer Dall <redacted>
[...]
quoted hunk
quoted
In my reply to patch #2, I had the following hunk:@@ -310,7 +329,7 @@ static void erratum_set_next_event_tval_generic(const int access, unsigned long struct clock_event_device *clk) { unsigned long ctrl; - u64 cval = evt + arch_counter_get_cntvct(); + u64 cval = evt + arch_timer_read_counter(); ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk); ctrl |= ARCH_TIMER_CTRL_ENABLE;Once we start using a different timer, this could well have an effect...Right, but wouldn't the following be a more correct way to go about it then:diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 9a7b359..07f19db 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c@@ -329,16 +329,19 @@ static void erratum_set_next_event_tval_generic(const int access, unsigned long struct clock_event_device *clk) { unsigned long ctrl; - u64 cval = evt + arch_timer_read_counter(); + u64 cval; ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk); ctrl |= ARCH_TIMER_CTRL_ENABLE; ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; - if (access == ARCH_TIMER_PHYS_ACCESS) + if (access == ARCH_TIMER_PHYS_ACCESS) { + cval = evt + arch_counter_get_cntpct(); write_sysreg(cval, cntp_cval_el0); - else + } else { + cval = evt + arch_counter_get_cntvct(); write_sysreg(cval, cntv_cval_el0); + } arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); }
Yup, that's much better. Thanks, M. -- Jazz is not dead, it just smell funny.