linux-kernel - Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0f94e3f2-d869-c0a0-4032-84f3a4dd6436@redhat.com>
Date:   Wed, 28 Jun 2017 16:00:11 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Wanpeng Li <kernellwp@...il.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Wanpeng Li <wanpeng.li@...mail.com>
Subject: Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay



On 28/06/2017 15:55, Wanpeng Li wrote:
>>>       if ((atomic_read(&apic->lapic_timer.pending) &&
>>>               !apic_lvtt_period(apic)) ||
>>> -             kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
>>> +             (ret = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline))) {
>>>               if (apic->lapic_timer.hv_timer_in_use)
>>>                       cancel_hv_timer(apic);
>>> +             if (ret == 1) {
>>> +                     apic_timer_expired(apic);
>>> +                     return true;
>>> +             }
>> The preemption timer can also be used for modes other than TSC deadline.
>>
>> In periodic mode, your patch would miss a call to
>> advance_periodic_target_expiration, which is only called by
>> kvm_lapic_expired_hv_timer.
> Actually I considered this before, however, I referred to apic timer
> periodic mode which is emulated by hrtimer

Periodic mode can also be emulated by preemption timer... it was added
by some Wanpeng Li in commit 8003c9ae204e ("KVM: LAPIC: add APIC Timer
periodic/oneshot mode VMX preemption timer support", 2016-11-02), do you
know him? ;)

> , there is no hrtimer start
> for the next period in start_sw_period(). If it is also buggy?

start_sw_period always goes through the hrtimer for periodic timer:

        if (apic_lvtt_oneshot(apic) &&
            ktime_after(ktime_get(),
                        apic->lapic_timer.target_expiration)) {
                apic_timer_expired(apic);
                return;
        }

        hrtimer_start(&apic->lapic_timer.timer,
                apic->lapic_timer.target_expiration,
                HRTIMER_MODE_ABS_PINNED);

(the direct call to apic_timer_expired is conditonal to
apic_lvtt_oneshot).  This way, apic_timer_fn takes care of advancing the
hrtimer deadline and returning HRTIMER_RESTART.

Paolo