Re: [PATCH] KVM: tsc deadline timer works only when hrtimer high resolution configured

[Avi Kivity <avi@redhat.com>]

On 09/07/2012 03:07 PM, Liu, Jinsong wrote:
> Avi Kivity wrote:
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 148ed66..0e64997 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -2210,7 +2210,11 @@ int kvm_dev_ioctl_check_extension(long ext) 
>>>  		r = kvm_has_tsc_control; break;
>>>  	case KVM_CAP_TSC_DEADLINE_TIMER:
>>> +#ifdef CONFIG_HIGH_RES_TIMERS
>>>  		r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER); +#else
>>> +		r = 0;
>>> +#endif
>>>  		break;
>> 
>> I prefer a patch making kvm for x86 depend on hrtimers.  kvm already
>> provides a high resolution timer to the guest in the local apic,
>> backing it with the jiffies event source will likely cause some
>> guests to malfunction.
> 
> Yep, I did a draft test for kvm lapic timer, it also worked fail when CONFIG_HIGH_RES_TIMERS disabled.
> 
> Attached is the udpated patch.
> 
> Thanks,
> Jinsong
> 
> ====================
> From 64d0458ec50a7d6917adf1e9735ba6e6ae6024ad Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <jinsong.liu@intel.com>
> Date: Sat, 8 Sep 2012 03:32:31 +0800
> Subject: [PATCH] KVM: select HIGH_RES_TIMERS when KVM enabled
> 
> This is for 2 reasons:
> 1. it's pointless for kvm lapic timer and tsc deadline timer
> when kernel hrtimer not configured as high resolution, since
> that would be not accurate based on wheel;
> 2. kvm lapic timer and tsc deadline timer based on hrtimer,
> setting a leftmost node to rb tree and then do hrtimer reprogram.
> If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
> do nothing and then make kvm lapic timer and tsc deadline timer fail.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> ---
>  arch/x86/kvm/Kconfig |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index a28f338..5f861ca 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -24,6 +24,8 @@ config KVM
>  	depends on PCI
>  	# for TASKSTATS/TASK_DELAY_ACCT:
>  	depends on NET
> +	# for HIGH_RES_TIMERS
> +	depends on !ARCH_USES_GETTIMEOFFSET
>  	select PREEMPT_NOTIFIERS
>  	select MMU_NOTIFIER
>  	select ANON_INODES
> @@ -37,6 +39,8 @@ config KVM
>  	select TASK_DELAY_ACCT
>  	select PERF_EVENTS
>  	select HAVE_KVM_MSI
> +	select GENERIC_CLOCKEVENTS
> +	select HIGH_RES_TIMERS

hrtimers is an intrusive feature, I don't think we should force-enable
it.  Please change it to a depends on.


-- 
error compiling committee.c: too many arguments to function