| 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
| ||
|
Message-ID: <CANRm+Cx3NE1=BC2MhZo_XajjMODe8ju+wjNZySDaZVwHk-+_Eg@mail.gmail.com>
Date: Thu, 29 Jun 2017 20:05:48 +0800
From: Wanpeng Li <kernellwp@...il.com>
To: Paolo Bonzini <pbonzini@...hat.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
2017-06-29 15:55 GMT+08:00 Paolo Bonzini <pbonzini@...hat.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
Powered by blists - more mailing lists