| Message ID | DE8DF0795D48FD4CA783C40EC8292335322311@SHSMSX101.ccr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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@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. 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/09/2012 05:54 PM, Liu, Jinsong wrote: >> >> 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:) It's not good, but that's what we have. It's okay to force-enable low-impact features (like preempt notifies). hrimers, on the other hand, change kernel behaviour quite deeply. Maybe over time someone will fix the config tools to unhide features that can be enabled by turning on a dependency.
==================== 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 ---help--- Support hosting fully virtualized guest machines using hardware virtualization extensions. You will need a fairly recent