lkml.org 
[lkml]   [2017]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay
    2017-06-29 15:55 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
    >
    >
    > On 29/06/2017 05:42, Wanpeng Li wrote:
    >> So how about something like this?
    >>
    >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
    >> index d24c874..900ce86 100644
    >> --- a/arch/x86/kvm/lapic.c
    >> +++ b/arch/x86/kvm/lapic.c
    >> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
    >> static bool start_hv_timer(struct kvm_lapic *apic)
    >> {
    >> u64 tscdeadline = apic->lapic_timer.tscdeadline;
    >> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
    >> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
    >> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
    >> + if (r >= 0) {
    >> + need_cancel = false;
    >> + apic->lapic_timer.hv_timer_in_use = true;
    >> + hrtimer_cancel(&apic->lapic_timer.timer);
    >> +
    >> + /* In case the sw timer triggered in the window */
    >> + if (atomic_read(&apic->lapic_timer.pending) &&
    >> + !apic_lvtt_period(apic))
    >> + need_cancel = true;
    >> + else if (r && (apic_lvtt_oneshot(apic) ||
    >> apic_lvtt_tscdeadline(apic)))
    >> + apic_timer_expired(apic);
    >> + }
    >> + }
    >>
    >> - if ((atomic_read(&apic->lapic_timer.pending) &&
    >> - !apic_lvtt_period(apic)) ||
    >> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
    >> - if (apic->lapic_timer.hv_timer_in_use)
    >> - cancel_hv_timer(apic);
    >> - } else {
    >> - apic->lapic_timer.hv_timer_in_use = true;
    >> - hrtimer_cancel(&apic->lapic_timer.timer);
    >> + if (need_cancel)
    >> + cancel_hv_timer(apic);
    >>
    >> - /* In case the sw timer triggered in the window */
    >> - if (atomic_read(&apic->lapic_timer.pending) &&
    >> - !apic_lvtt_period(apic))
    >> - cancel_hv_timer(apic);
    >> - }
    >> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
    >> apic->lapic_timer.hv_timer_in_use);
    >> return apic->lapic_timer.hv_timer_in_use;
    >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
    >> index 4f616db..0a0e696 100644
    >> --- a/arch/x86/kvm/vmx.c
    >> +++ b/arch/x86/kvm/vmx.c
    >> @@ -11125,6 +11125,9 @@ static int vmx_set_hv_timer(struct kvm_vcpu
    >> *vcpu, u64 guest_deadline_tsc)
    >> u64 guest_tscl = kvm_read_l1_tsc(vcpu, tscl);
    >> u64 delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl;
    >>
    >> + if (delta_tsc == 0)
    >> + return 1;
    >
    > You still need to enable the preemption timer even if you return 1, so
    > in lapic.c it becomes
    >
    > if (!apic_lvtt_period(apic)) {
    > if (r)
    > apic_timer_expired(apic);
    > if (atomic_read(&apic->lapic_timer.pending))
    > need_cancel = true;
    > }

    As you point out, we should cancel the preemption timer if it returns
    1 and the APIC timer's mode is oneshot/tscdeadline. How about
    something like this:

    diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
    index d24c874..b801385 100644
    --- a/arch/x86/kvm/lapic.c
    +++ b/arch/x86/kvm/lapic.c
    @@ -1504,21 +1504,28 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
    static bool start_hv_timer(struct kvm_lapic *apic)
    {
    u64 tscdeadline = apic->lapic_timer.tscdeadline;
    + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
    + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
    + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
    + if (r >= 0) {
    + need_cancel = false;
    + apic->lapic_timer.hv_timer_in_use = true;
    + hrtimer_cancel(&apic->lapic_timer.timer);
    +
    + /* In case the sw timer triggered in the window */
    + if (!apic_lvtt_period(apic)) {
    + if (r || atomic_read(&apic->lapic_timer.pending)) {
    + need_cancel = true;
    + if (r)
    + apic_timer_expired(apic);
    + }
    + }
    + }
    + }

    - if ((atomic_read(&apic->lapic_timer.pending) &&
    - !apic_lvtt_period(apic)) ||
    - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
    - if (apic->lapic_timer.hv_timer_in_use)
    - cancel_hv_timer(apic);
    - } else {
    - apic->lapic_timer.hv_timer_in_use = true;
    - hrtimer_cancel(&apic->lapic_timer.timer);
    + if (need_cancel)
    + cancel_hv_timer(apic);

    - /* In case the sw timer triggered in the window */
    - if (atomic_read(&apic->lapic_timer.pending) &&
    - !apic_lvtt_period(apic))
    - cancel_hv_timer(apic);
    - }
    trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
    apic->lapic_timer.hv_timer_in_use);
    return apic->lapic_timer.hv_timer_in_use;
    diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
    index ca5d2b9..b8bde13 100644
    --- a/arch/x86/kvm/vmx.c
    +++ b/arch/x86/kvm/vmx.c
    @@ -11134,7 +11134,11 @@ static int vmx_set_hv_timer(struct kvm_vcpu
    *vcpu, u64 guest_deadline_tsc)
    vmx->hv_deadline_tsc = tscl + delta_tsc;
    vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL,
    PIN_BASED_VMX_PREEMPTION_TIMER);
    - return 0;
    +
    + if (delta_tsc == 0)
    + return 1;
    + else
    + return 0;
    }

    static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu)
    Regards,
    Wanpeng Li

    \
     
     \ /
      Last update: 2017-06-29 14:07    [W:2.088 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site