Subject: Re: [PATCH] powerpc/time: use get_tb instead of get_vtb in running_clock

From: Benjamin Herrenschmidt <[email-masked]>
Date: Thu, 13 Jul 2017 08:45:04 +1000

On Wed, 2017-07-12 at 23:01 +0800, Jia He wrote:
> Virtual time base(vtb) is a register which increases only in guest.
> Any exit from guest to host will stop the vtb(saved and restored by kvm).
> But if there is an IO causes guest exits to host, the guest's watchdog
> (watchdog_timer_fn -> is_softlockup -> get_timestamp -> running_clock)
> needs to also include the time elapsed in host. get_vtb is not correct in
> this case.
> 
> Also, the TB_OFFSET is well saved and restored by qemu after commit [1].
> So we can use get_tb here.

That completely defeats the purpose here... This was done specifically
to exploit the VTB which doesn't count in hypervisor mode.

> 
> [1] http://git.qemu.org/?p=qemu.git;a=commit;h=42043e4f1
> 
> Signed-off-by: Jia He <[email-masked]>
> ---
>  arch/powerpc/kernel/time.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index fe6f3a2..c542dd3 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -695,16 +695,15 @@ notrace unsigned long long sched_clock(void)
>  unsigned long long running_clock(void)
>  {
>  	/*
> -	 * Don't read the VTB as a host since KVM does not switch in host
> -	 * timebase into the VTB when it takes a guest off the CPU, reading the
> -	 * VTB would result in reading 'last switched out' guest VTB.
> +	 * Use get_tb instead of get_vtb for guest since the TB_OFFSET has been
> +	 * well saved/restored when qemu does suspend/resume.
>  	 *
>  	 * Host kernels are often compiled with CONFIG_PPC_PSERIES checked, it
>  	 * would be unsafe to rely only on the #ifdef above.
>  	 */
>  	if (firmware_has_feature(FW_FEATURE_LPAR) &&
>  	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		return mulhdu(get_vtb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;
> +		return mulhdu(get_tb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;
>  
>  	/*
>  	 * This is a next best approximation without a VTB.