From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: [PATCH] KVM: x86: remove irq disablement around KVM_SET_CLOCK/KVM_GET_CLOCK Date: Wed, 12 Apr 2017 14:23:15 -0300 Message-ID: <20170412172313.GA26589@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , pagupta@redhat.com To: kvm-devel Return-path: Received: from mx1.redhat.com ([209.132.183.28]:35196 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751349AbdDLRXn (ORCPT ); Wed, 12 Apr 2017 13:23:43 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD13D62647 for ; Wed, 12 Apr 2017 17:23:42 +0000 (UTC) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: The disablement of interrupts at KVM_SET_CLOCK/KVM_GET_CLOCK attempts to disable interrupts in that section to protect the values that are calculated in that section from interrupt interference. now_ns is calculated inside the irq protected region, user_ns.clock is passed from userspace (therefore not susceptible to interrupt variation). About the line now_ns = __get_kvmclock_ns(kvm); (1) Interrupts can happen afterwards local_irq_enable(), rendering "now_ns" relative to its execution time PLUS interrupt time. Therefore the local_irq_disable() / local_irq_enable() protection is not necessary (that is: interrupts triggering after local_irq_enable cause the same problem that the protection is trying to avoid). With this reasoning, and the -RT bug that the irq disablement causes (because spin_lock is now a sleeping lock), remove the IRQ protection as it causes: [ 1064.668109] in_atomic(): 0, irqs_disabled(): 1, pid: 15296, name:m [ 1064.668110] INFO: lockdep is turned off. [ 1064.668110] irq event stamp: 0 [ 1064.668112] hardirqs last enabled at (0): [< (null)>] ) [ 1064.668116] hardirqs last disabled at (0): [] c0 [ 1064.668118] softirqs last enabled at (0): [] c0 [ 1064.668118] softirqs last disabled at (0): [< (null)>] ) [ 1064.668121] CPU: 13 PID: 15296 Comm: qemu-kvm Not tainted 3.10.0-1 [ 1064.668121] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 5 [ 1064.668123] ffff8c1796b88000 00000000afe7344c ffff8c179abf3c68 f3 [ 1064.668125] ffff8c179abf3c90 ffffffff930ccb3d ffff8c1b992b3610 f0 [ 1064.668126] 00007ffc1a26fbc0 ffff8c179abf3cb0 ffffffff9375f694 f0 [ 1064.668126] Call Trace: [ 1064.668132] [] dump_stack+0x19/0x1b [ 1064.668135] [] __might_sleep+0x12d/0x1f0 [ 1064.668138] [] rt_spin_lock+0x24/0x60 [ 1064.668155] [] __get_kvmclock_ns+0x36/0x110 [k] [ 1064.668159] [] ? futex_wait_queue_me+0x103/0x10 [ 1064.668171] [] kvm_arch_vm_ioctl+0xa2/0xd70 [k] [ 1064.668173] [] ? futex_wait+0x1ac/0x2a0 On -RT kernels. Signed-off-by: Marcelo Tosatti --- arch/x86/kvm/x86.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7c39b8a..935143b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4062,10 +4062,8 @@ long kvm_arch_vm_ioctl(struct file *filp, goto out; r = 0; - local_irq_disable(); now_ns = __get_kvmclock_ns(kvm); kvm->arch.kvmclock_offset += user_ns.clock - now_ns; - local_irq_enable(); kvm_gen_update_masterclock(kvm); break; } @@ -4073,11 +4071,9 @@ long kvm_arch_vm_ioctl(struct file *filp, struct kvm_clock_data user_ns; u64 now_ns; - local_irq_disable(); now_ns = __get_kvmclock_ns(kvm); user_ns.clock = now_ns; user_ns.flags = kvm->arch.use_master_clock ? KVM_CLOCK_TSC_STABLE : 0; - local_irq_enable(); memset(&user_ns.pad, 0, sizeof(user_ns.pad)); r = -EFAULT;