* [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time @ 2019-05-23 4:18 Wanpeng Li 2019-05-23 4:18 ` [PATCH 2/2] KVM: LAPIC: remove the trailing newline used in the fmt parameter of TP_printk Wanpeng Li 2019-05-30 19:36 ` [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Sean Christopherson 0 siblings, 2 replies; 6+ messages in thread From: Wanpeng Li @ 2019-05-23 4:18 UTC (permalink / raw) To: linux-kernel, kvm Cc: Paolo Bonzini, Radim Krčmář, Sean Christopherson From: Wanpeng Li <wanpengli@tencent.com> Advance lapic timer tries to hidden the hypervisor overhead between the host emulated timer fires and the guest awares the timer is fired. However, even though after more sustaining optimizations, kvm-unit-tests/tscdeadline_latency still awares ~1000 cycles latency since we lost the time between the end of wait_lapic_expire and the guest awares the timer is fired. There are codes between the end of wait_lapic_expire and the world switch, futhermore, the world switch itself also has overhead. Actually the guest_tsc is equal to the target deadline time in wait_lapic_expire is too late, guest will aware the latency between the end of wait_lapic_expire() and after vmentry to the guest. This patch takes this time into consideration. The vmentry_lapic_timer_advance_ns module parameter should be well tuned by host admin, it can reduce average cyclictest latency from 3us to 2us on Skylake server. (guest w/ nohz=off, idle=poll, host w/ preemption_timer=N, the cyclictest latency is not too sensitive when preemption_timer=Y for this optimization in my testing), kvm-unit-tests/tscdeadline_latency can reach 0. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Radim Krčmář <rkrcmar@redhat.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> --- arch/x86/kvm/lapic.c | 17 +++++++++++++++-- arch/x86/kvm/lapic.h | 1 + arch/x86/kvm/vmx/vmx.c | 2 +- arch/x86/kvm/x86.c | 3 +++ arch/x86/kvm/x86.h | 2 ++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index fcf42a3..6f85221 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1531,6 +1531,19 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu, apic->lapic_timer.timer_advance_ns = timer_advance_ns; } +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu) +{ + u64 vmentry_lapic_timer_advance_cycles = 0; + + if (vmentry_lapic_timer_advance_ns) { + vmentry_lapic_timer_advance_cycles = vmentry_lapic_timer_advance_ns * + vcpu->arch.virtual_tsc_khz; + do_div(vmentry_lapic_timer_advance_cycles, 1000000); + } + return vmentry_lapic_timer_advance_cycles; +} +EXPORT_SYMBOL_GPL(get_vmentry_advance_delta); + void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) { struct kvm_lapic *apic = vcpu->arch.apic; @@ -1544,7 +1557,7 @@ void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) tsc_deadline = apic->lapic_timer.expired_tscdeadline; apic->lapic_timer.expired_tscdeadline = 0; - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); apic->lapic_timer.advance_expire_delta = guest_tsc - tsc_deadline; if (guest_tsc < tsc_deadline) @@ -1572,7 +1585,7 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic) local_irq_save(flags); now = ktime_get(); - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); ns = (tscdeadline - guest_tsc) * 1000000ULL; do_div(ns, this_tsc_khz); diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index f974a3d..df2fe17 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -221,6 +221,7 @@ static inline int kvm_lapic_latched_init(struct kvm_vcpu *vcpu) bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector); void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu); +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu); bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm, struct kvm_lapic_irq *irq, struct kvm_vcpu **dest_vcpu); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index da24f18..0199ac3 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7047,7 +7047,7 @@ static int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc, vmx = to_vmx(vcpu); tscl = rdtsc(); - guest_tscl = kvm_read_l1_tsc(vcpu, tscl); + guest_tscl = kvm_read_l1_tsc(vcpu, tscl) + get_vmentry_advance_delta(vcpu); delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl; lapic_timer_advance_cycles = nsec_to_cycles(vcpu, ktimer->timer_advance_ns); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index a4eb711..a02e2c3 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -145,6 +145,9 @@ module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); static int __read_mostly lapic_timer_advance_ns = -1; module_param(lapic_timer_advance_ns, int, S_IRUGO | S_IWUSR); +u32 __read_mostly vmentry_lapic_timer_advance_ns = 0; +module_param(vmentry_lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); + static bool __read_mostly vector_hashing = true; module_param(vector_hashing, bool, S_IRUGO); diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 275b3b6..b0a3b84 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -294,6 +294,8 @@ extern u64 kvm_supported_xcr0(void); extern unsigned int min_timer_period_us; +extern unsigned int vmentry_lapic_timer_advance_ns; + extern bool enable_vmware_backdoor; extern struct static_key kvm_no_apic_vcpu; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: LAPIC: remove the trailing newline used in the fmt parameter of TP_printk 2019-05-23 4:18 [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Wanpeng Li @ 2019-05-23 4:18 ` Wanpeng Li 2019-05-30 19:36 ` [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Sean Christopherson 1 sibling, 0 replies; 6+ messages in thread From: Wanpeng Li @ 2019-05-23 4:18 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Radim Krčmář From: Wanpeng Li <wanpengli@tencent.com> The trailing newlines will lead to extra newlines in the trace file which looks like the following output, so remove it. qemu-system-x86-15695 [002] ...1 15774.839240: kvm_hv_timer_state: vcpu_id 0 hv_timer 1 qemu-system-x86-15695 [002] ...1 15774.839309: kvm_hv_timer_state: vcpu_id 0 hv_timer 1 Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Radim Krčmář <rkrcmar@redhat.com> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> --- arch/x86/kvm/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h index 4d47a26..b5c831e 100644 --- a/arch/x86/kvm/trace.h +++ b/arch/x86/kvm/trace.h @@ -1365,7 +1365,7 @@ TRACE_EVENT(kvm_hv_timer_state, __entry->vcpu_id = vcpu_id; __entry->hv_timer_in_use = hv_timer_in_use; ), - TP_printk("vcpu_id %x hv_timer %x\n", + TP_printk("vcpu_id %x hv_timer %x", __entry->vcpu_id, __entry->hv_timer_in_use) ); -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time 2019-05-23 4:18 [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Wanpeng Li 2019-05-23 4:18 ` [PATCH 2/2] KVM: LAPIC: remove the trailing newline used in the fmt parameter of TP_printk Wanpeng Li @ 2019-05-30 19:36 ` Sean Christopherson 2019-05-31 6:41 ` Wanpeng Li 2019-05-31 9:01 ` Paolo Bonzini 1 sibling, 2 replies; 6+ messages in thread From: Sean Christopherson @ 2019-05-30 19:36 UTC (permalink / raw) To: Wanpeng Li; +Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář On Thu, May 23, 2019 at 12:18:50PM +0800, Wanpeng Li wrote: > From: Wanpeng Li <wanpengli@tencent.com> > > Advance lapic timer tries to hidden the hypervisor overhead between the > host emulated timer fires and the guest awares the timer is fired. However, > even though after more sustaining optimizations, kvm-unit-tests/tscdeadline_latency > still awares ~1000 cycles latency since we lost the time between the end of > wait_lapic_expire and the guest awares the timer is fired. There are > codes between the end of wait_lapic_expire and the world switch, futhermore, > the world switch itself also has overhead. Actually the guest_tsc is equal > to the target deadline time in wait_lapic_expire is too late, guest will > aware the latency between the end of wait_lapic_expire() and after vmentry > to the guest. This patch takes this time into consideration. > > The vmentry_lapic_timer_advance_ns module parameter should be well tuned by > host admin, it can reduce average cyclictest latency from 3us to 2us on > Skylake server. (guest w/ nohz=off, idle=poll, host w/ preemption_timer=N, > the cyclictest latency is not too sensitive when preemption_timer=Y for this > optimization in my testing), kvm-unit-tests/tscdeadline_latency can reach 0. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > arch/x86/kvm/lapic.c | 17 +++++++++++++++-- > arch/x86/kvm/lapic.h | 1 + > arch/x86/kvm/vmx/vmx.c | 2 +- > arch/x86/kvm/x86.c | 3 +++ > arch/x86/kvm/x86.h | 2 ++ > 5 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index fcf42a3..6f85221 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1531,6 +1531,19 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu, > apic->lapic_timer.timer_advance_ns = timer_advance_ns; > } > > +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu) Hmm, this isn't a delta, I think get_vmentry_advance_cycles would be more appropriate. > +{ > + u64 vmentry_lapic_timer_advance_cycles = 0; > + > + if (vmentry_lapic_timer_advance_ns) { > + vmentry_lapic_timer_advance_cycles = vmentry_lapic_timer_advance_ns * > + vcpu->arch.virtual_tsc_khz; > + do_div(vmentry_lapic_timer_advance_cycles, 1000000); > + } > + return vmentry_lapic_timer_advance_cycles; > +} > +EXPORT_SYMBOL_GPL(get_vmentry_advance_delta); > + > void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) > { > struct kvm_lapic *apic = vcpu->arch.apic; > @@ -1544,7 +1557,7 @@ void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) > > tsc_deadline = apic->lapic_timer.expired_tscdeadline; > apic->lapic_timer.expired_tscdeadline = 0; > - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); > apic->lapic_timer.advance_expire_delta = guest_tsc - tsc_deadline; > > if (guest_tsc < tsc_deadline) > @@ -1572,7 +1585,7 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic) > local_irq_save(flags); > > now = ktime_get(); > - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); > > ns = (tscdeadline - guest_tsc) * 1000000ULL; > do_div(ns, this_tsc_khz); > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index f974a3d..df2fe17 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -221,6 +221,7 @@ static inline int kvm_lapic_latched_init(struct kvm_vcpu *vcpu) > bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector); > > void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu); > +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu); > > bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm, struct kvm_lapic_irq *irq, > struct kvm_vcpu **dest_vcpu); > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index da24f18..0199ac3 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -7047,7 +7047,7 @@ static int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc, > > vmx = to_vmx(vcpu); > tscl = rdtsc(); > - guest_tscl = kvm_read_l1_tsc(vcpu, tscl); > + guest_tscl = kvm_read_l1_tsc(vcpu, tscl) + get_vmentry_advance_delta(vcpu); > delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl; > lapic_timer_advance_cycles = nsec_to_cycles(vcpu, > ktimer->timer_advance_ns); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index a4eb711..a02e2c3 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -145,6 +145,9 @@ module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); > static int __read_mostly lapic_timer_advance_ns = -1; > module_param(lapic_timer_advance_ns, int, S_IRUGO | S_IWUSR); > > +u32 __read_mostly vmentry_lapic_timer_advance_ns = 0; > +module_param(vmentry_lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); Hmm, an interesting idea would be to have some way to "lock" this param, e.g. setting bit 0 locks the param. That would allow KVM to calculate the cycles value to avoid the function call and the MUL+DIV. If I'm not mistaken, vcpu->arch.virtual_tsc_khz is set only in kvm_set_tsc_khz(). For example, if get_vmentry_advance_cycles() sees the value is locked, it caches the value in struct kvm_lapic. The cached value would also need to be updated in kvm_set_tsc_khz() if it has been set. static inline u64 get_vmentry_advance_cycles(struct kvm_lapic *lapic) { if (lapic->vmentry_advance_cycles) return lapic->vmentry_advance_cycles; return compute_vmentry_advance_cycles(lapic); } u64 compute_vmentry_advance_cycles(struct kvm_lapic *lapic) { u64 val = vmentry_lapic_timer_advance_ns; u64 cycles = (val & ~1ULL) * lapic->vcpu->arch.virtual_tsc_khz; do_div(cycles, 1000000); if (val & 1) lapic->vmentry_advance_cycles = cycles; return cycles; } > + > static bool __read_mostly vector_hashing = true; > module_param(vector_hashing, bool, S_IRUGO); > > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index 275b3b6..b0a3b84 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -294,6 +294,8 @@ extern u64 kvm_supported_xcr0(void); > > extern unsigned int min_timer_period_us; > > +extern unsigned int vmentry_lapic_timer_advance_ns; > + > extern bool enable_vmware_backdoor; > > extern struct static_key kvm_no_apic_vcpu; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time 2019-05-30 19:36 ` [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Sean Christopherson @ 2019-05-31 6:41 ` Wanpeng Li 2019-05-31 9:01 ` Paolo Bonzini 1 sibling, 0 replies; 6+ messages in thread From: Wanpeng Li @ 2019-05-31 6:41 UTC (permalink / raw) To: Sean Christopherson; +Cc: LKML, kvm, Paolo Bonzini, Radim Krčmář On Fri, 31 May 2019 at 03:36, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Thu, May 23, 2019 at 12:18:50PM +0800, Wanpeng Li wrote: > > From: Wanpeng Li <wanpengli@tencent.com> > > > > Advance lapic timer tries to hidden the hypervisor overhead between the > > host emulated timer fires and the guest awares the timer is fired. However, > > even though after more sustaining optimizations, kvm-unit-tests/tscdeadline_latency > > still awares ~1000 cycles latency since we lost the time between the end of > > wait_lapic_expire and the guest awares the timer is fired. There are > > codes between the end of wait_lapic_expire and the world switch, futhermore, > > the world switch itself also has overhead. Actually the guest_tsc is equal > > to the target deadline time in wait_lapic_expire is too late, guest will > > aware the latency between the end of wait_lapic_expire() and after vmentry > > to the guest. This patch takes this time into consideration. > > > > The vmentry_lapic_timer_advance_ns module parameter should be well tuned by > > host admin, it can reduce average cyclictest latency from 3us to 2us on > > Skylake server. (guest w/ nohz=off, idle=poll, host w/ preemption_timer=N, > > the cyclictest latency is not too sensitive when preemption_timer=Y for this > > optimization in my testing), kvm-unit-tests/tscdeadline_latency can reach 0. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Radim Krčmář <rkrcmar@redhat.com> > > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > > --- > > arch/x86/kvm/lapic.c | 17 +++++++++++++++-- > > arch/x86/kvm/lapic.h | 1 + > > arch/x86/kvm/vmx/vmx.c | 2 +- > > arch/x86/kvm/x86.c | 3 +++ > > arch/x86/kvm/x86.h | 2 ++ > > 5 files changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index fcf42a3..6f85221 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -1531,6 +1531,19 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu, > > apic->lapic_timer.timer_advance_ns = timer_advance_ns; > > } > > > > +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu) > > Hmm, this isn't a delta, I think get_vmentry_advance_cycles would be more > appropriate. > > > +{ > > + u64 vmentry_lapic_timer_advance_cycles = 0; > > + > > + if (vmentry_lapic_timer_advance_ns) { > > + vmentry_lapic_timer_advance_cycles = vmentry_lapic_timer_advance_ns * > > + vcpu->arch.virtual_tsc_khz; > > + do_div(vmentry_lapic_timer_advance_cycles, 1000000); > > + } > > + return vmentry_lapic_timer_advance_cycles; > > +} > > +EXPORT_SYMBOL_GPL(get_vmentry_advance_delta); > > + > > void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) > > { > > struct kvm_lapic *apic = vcpu->arch.apic; > > @@ -1544,7 +1557,7 @@ void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) > > > > tsc_deadline = apic->lapic_timer.expired_tscdeadline; > > apic->lapic_timer.expired_tscdeadline = 0; > > - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > > + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); > > apic->lapic_timer.advance_expire_delta = guest_tsc - tsc_deadline; > > > > if (guest_tsc < tsc_deadline) > > @@ -1572,7 +1585,7 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic) > > local_irq_save(flags); > > > > now = ktime_get(); > > - guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > > + guest_tsc = kvm_read_l1_tsc(vcpu, rdtsc()) + get_vmentry_advance_delta(vcpu); > > > > ns = (tscdeadline - guest_tsc) * 1000000ULL; > > do_div(ns, this_tsc_khz); > > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > > index f974a3d..df2fe17 100644 > > --- a/arch/x86/kvm/lapic.h > > +++ b/arch/x86/kvm/lapic.h > > @@ -221,6 +221,7 @@ static inline int kvm_lapic_latched_init(struct kvm_vcpu *vcpu) > > bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector); > > > > void kvm_wait_lapic_expire(struct kvm_vcpu *vcpu); > > +u64 get_vmentry_advance_delta(struct kvm_vcpu *vcpu); > > > > bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm, struct kvm_lapic_irq *irq, > > struct kvm_vcpu **dest_vcpu); > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > index da24f18..0199ac3 100644 > > --- a/arch/x86/kvm/vmx/vmx.c > > +++ b/arch/x86/kvm/vmx/vmx.c > > @@ -7047,7 +7047,7 @@ static int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc, > > > > vmx = to_vmx(vcpu); > > tscl = rdtsc(); > > - guest_tscl = kvm_read_l1_tsc(vcpu, tscl); > > + guest_tscl = kvm_read_l1_tsc(vcpu, tscl) + get_vmentry_advance_delta(vcpu); > > delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl; > > lapic_timer_advance_cycles = nsec_to_cycles(vcpu, > > ktimer->timer_advance_ns); > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index a4eb711..a02e2c3 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -145,6 +145,9 @@ module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); > > static int __read_mostly lapic_timer_advance_ns = -1; > > module_param(lapic_timer_advance_ns, int, S_IRUGO | S_IWUSR); > > > > +u32 __read_mostly vmentry_lapic_timer_advance_ns = 0; > > +module_param(vmentry_lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); > > Hmm, an interesting idea would be to have some way to "lock" this param, > e.g. setting bit 0 locks the param. That would allow KVM to calculate the > cycles value to avoid the function call and the MUL+DIV. If I'm not > mistaken, vcpu->arch.virtual_tsc_khz is set only in kvm_set_tsc_khz(). > > For example, if get_vmentry_advance_cycles() sees the value is locked, it > caches the value in struct kvm_lapic. The cached value would also need to > be updated in kvm_set_tsc_khz() if it has been set. Good point, handle it in v2. Regards, Wanpeng Li > > static inline u64 get_vmentry_advance_cycles(struct kvm_lapic *lapic) > { > if (lapic->vmentry_advance_cycles) > return lapic->vmentry_advance_cycles; > > return compute_vmentry_advance_cycles(lapic); > } > > u64 compute_vmentry_advance_cycles(struct kvm_lapic *lapic) > { > u64 val = vmentry_lapic_timer_advance_ns; > u64 cycles = (val & ~1ULL) * lapic->vcpu->arch.virtual_tsc_khz; > > do_div(cycles, 1000000); > > if (val & 1) > lapic->vmentry_advance_cycles = cycles; > return cycles; > } > > > + > > static bool __read_mostly vector_hashing = true; > > module_param(vector_hashing, bool, S_IRUGO); > > > > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > > index 275b3b6..b0a3b84 100644 > > --- a/arch/x86/kvm/x86.h > > +++ b/arch/x86/kvm/x86.h > > @@ -294,6 +294,8 @@ extern u64 kvm_supported_xcr0(void); > > > > extern unsigned int min_timer_period_us; > > > > +extern unsigned int vmentry_lapic_timer_advance_ns; > > + > > extern bool enable_vmware_backdoor; > > > > extern struct static_key kvm_no_apic_vcpu; > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time 2019-05-30 19:36 ` [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Sean Christopherson 2019-05-31 6:41 ` Wanpeng Li @ 2019-05-31 9:01 ` Paolo Bonzini 2019-06-12 9:38 ` Wanpeng Li 1 sibling, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2019-05-31 9:01 UTC (permalink / raw) To: Sean Christopherson, Wanpeng Li Cc: linux-kernel, kvm, Radim Krčmář On 30/05/19 21:36, Sean Christopherson wrote: >> +u32 __read_mostly vmentry_lapic_timer_advance_ns = 0; >> +module_param(vmentry_lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); > Hmm, an interesting idea would be to have some way to "lock" this param, > e.g. setting bit 0 locks the param. That would allow KVM to calculate the > cycles value to avoid the function call and the MUL+DIV. If I'm not > mistaken, vcpu->arch.virtual_tsc_khz is set only in kvm_set_tsc_khz(). I would just make it read-only. But I'm afraid we're entering somewhat dangerous territory. There is a risk that the guest ends up entering the interrupt handler before the TSC deadline has actually expired, and there would be no way to know what would happen; even guest hangs are possible. Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time 2019-05-31 9:01 ` Paolo Bonzini @ 2019-06-12 9:38 ` Wanpeng Li 0 siblings, 0 replies; 6+ messages in thread From: Wanpeng Li @ 2019-06-12 9:38 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Sean Christopherson, LKML, kvm, Radim Krčmář On Fri, 31 May 2019 at 17:01, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 30/05/19 21:36, Sean Christopherson wrote: > >> +u32 __read_mostly vmentry_lapic_timer_advance_ns = 0; > >> +module_param(vmentry_lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); > > Hmm, an interesting idea would be to have some way to "lock" this param, > > e.g. setting bit 0 locks the param. That would allow KVM to calculate the > > cycles value to avoid the function call and the MUL+DIV. If I'm not > > mistaken, vcpu->arch.virtual_tsc_khz is set only in kvm_set_tsc_khz(). > > I would just make it read-only. But I'm afraid we're entering somewhat > dangerous territory. There is a risk that the guest ends up entering > the interrupt handler before the TSC deadline has actually expired, and > there would be no way to know what would happen; even guest hangs are > possible. Agreed, do it in v3. Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-12 9:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-23 4:18 [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Wanpeng Li 2019-05-23 4:18 ` [PATCH 2/2] KVM: LAPIC: remove the trailing newline used in the fmt parameter of TP_printk Wanpeng Li 2019-05-30 19:36 ` [PATCH 1/2] KVM: LAPIC: Optimize timer latency consider world switch time Sean Christopherson 2019-05-31 6:41 ` Wanpeng Li 2019-05-31 9:01 ` Paolo Bonzini 2019-06-12 9:38 ` Wanpeng Li
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.