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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Avi Kivity wrote:
> 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@xxxxxxxxx>
>> 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@xxxxxxxxx> ---
>>  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.

Hmm, if it changed as
config KVM
	depends on HIGH_RES_TIMERS
The item 'Kernel-based Virtual Machine (KVM) support (NEW)' even didn't appear to user when make menuconfig (when HIGH_RES_TIMERS disable)

Is it good? I just have a little concern here:)

Thanks,
Jinsong--
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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux