On 06/01/17 10:00, Christoffer Dall wrote: > Hi Marc, > > On Thu, Jan 05, 2017 at 06:11:14PM +0000, Marc Zyngier wrote: >> [adding the arm64 maintainers, plus Mark as arch timer maintainer] > > Right, sorry, I should have done that already. > >> >> On 10/12/16 20:47, Christoffer Dall wrote: >>> Using the physical counter allows KVM to retain the offset between the >>> virtual and physical counter as long as it is actively running a VCPU. >>> >>> As soon as a VCPU is released, another thread is scheduled or we start >>> running userspace applications, we reset the offset to 0, so that VDSO >>> operations can still read the virtual counter and get the same view of >>> time as the kernel. >>> >>> This opens up potential improvements for KVM performance. >>> >>> VHE kernels or kernels using the virtual timer are unaffected by this. >>> >>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> >>> --- >>> arch/arm64/include/asm/arch_timer.h | 6 ++++-- >>> drivers/clocksource/arm_arch_timer.c | 2 +- >>> 2 files changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h >>> index eaa5bbe..cec2549 100644 >>> --- a/arch/arm64/include/asm/arch_timer.h >>> +++ b/arch/arm64/include/asm/arch_timer.h >>> @@ -139,11 +139,13 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl) >>> >>> static inline u64 arch_counter_get_cntpct(void) >>> { >>> + u64 cval; >>> /* >>> * AArch64 kernel and user space mandate the use of CNTVCT. >>> */ >>> - BUG(); >>> - return 0; >>> + isb(); >>> + asm volatile("mrs %0, cntpct_el0" : "=r" (cval)); >>> + return cval; >>> } >>> >>> static inline u64 arch_counter_get_cntvct(void) >>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c >>> index 73c487d..a5b0789 100644 >>> --- a/drivers/clocksource/arm_arch_timer.c >>> +++ b/drivers/clocksource/arm_arch_timer.c >>> @@ -597,7 +597,7 @@ static void __init arch_counter_register(unsigned type) >>> >>> /* Register the CP15 based counter if we have one */ >>> if (type & ARCH_CP15_TIMER) { >>> - if (IS_ENABLED(CONFIG_ARM64) || arch_timer_uses_ppi == VIRT_PPI) >>> + if (arch_timer_uses_ppi == VIRT_PPI || is_kernel_in_hyp_mode()) >> >> Why do we have this is_kernel_in_hyp_mode clause? I can't think of any >> reason for a VHE kernel to use the virtual counter at all... >> > > Good question. I think I just didn't want to change behavior from the > existing functionality mre than necessary. > > Note that on a VHE kernel this will be the EL2 virtual counter, not the > EL1 virtual counter, due to the register redirection. Are the virtual > and physical EL2 counters always equivalent on a VHE system? Yes, they are. CNTVOFF_EL2 is ignored in that case, and you get an extra interrupt for the new EL2 virtual timer as well. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html