From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=39451 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pht5b-0001kJ-Bu for qemu-devel@nongnu.org; Tue, 25 Jan 2011 19:18:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pht5Z-0005tM-HX for qemu-devel@nongnu.org; Tue, 25 Jan 2011 19:18:22 -0500 Received: from mail-qy0-f173.google.com ([209.85.216.173]:47913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pht5Z-0005tE-F8 for qemu-devel@nongnu.org; Tue, 25 Jan 2011 19:18:21 -0500 Received: by qyl38 with SMTP id 38so5408659qyl.4 for ; Tue, 25 Jan 2011 16:18:20 -0800 (PST) Message-ID: <4D3F6849.2060701@codemonkey.ws> Date: Tue, 25 Jan 2011 18:18:17 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v5 2/4] virtio-pci: Use ioeventfd for virtqueue notify References: <1292166128-10874-1-git-send-email-stefanha@linux.vnet.ibm.com> <1292166128-10874-3-git-send-email-stefanha@linux.vnet.ibm.com> <4D3DCADC.6010308@redhat.com> <20110124193653.GC29941@redhat.com> <4D3DD775.3060003@redhat.com> <20110124194729.GE29941@redhat.com> <4D3DDB99.8090001@redhat.com> <4D3F2222.8010405@linux.vnet.ibm.com> <4D3F29A8.7040205@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Avi Kivity , Stefan Hajnoczi , "Michael S. Tsirkin" On 01/25/2011 01:59 PM, Stefan Hajnoczi wrote: > int kvm_cpu_exec(CPUState *env) > { > struct kvm_run *run = env->kvm_run; > int ret; > > DPRINTF("kvm_cpu_exec()\n"); > > do { > > This is broken because a signal handler could change env->exit_request > after this check: > > #ifndef CONFIG_IOTHREAD > if (env->exit_request) { > DPRINTF("interrupt exit requested\n"); > ret = 0; > break; > } > #endif > Yeah, this is classic signal/select race with ioctl(KVM_RUN) subbing in for select. But this is supposed to be mitigated by the fact that we block SIG_IPI except for when we execute KVM_RUN which means that we can reliably send SIG_IPI. Of course, that doesn't help for SIGALRM unless we send a SIG_IPI from the SIGALRM handler which we do with the I/O thread but not w/o it. At any rate, post stable-0.14, I want to enable I/O thread by default so I don't know that we really need to fix this... > if (kvm_arch_process_irqchip_events(env)) { > ret = 0; > break; > } > > if (env->kvm_vcpu_dirty) { > kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE); > env->kvm_vcpu_dirty = 0; > } > > kvm_arch_pre_run(env, run); > cpu_single_env = NULL; > qemu_mutex_unlock_iothread(); > > env->exit_request might be set but we still reenter, possibly without > rearming the timer: > ret = kvm_vcpu_ioctl(env, KVM_RUN, 0); > > >>> I can think of two solutions: >>> 1. Block SIGALRM during critical regions, not sure if the necessary >>> atomic signal mask capabilities are there in KVM. Haven't looked at >>> TCG yet either. >>> 2. Make a portion of the timer code signal-safe and rearm the timer >>> from within the SIGLARM handler. >>> >>> >> Or, switch to timerfd and stop using a signal based alarm timer. >> > Doesn't work for !CONFIG_IOTHREAD. > Yeah, we need to get rid of !CONFIG_IOTHREAD. We need to run select() in parallel with TCG/KVM and interrupt the VCPUs appropriately when select() returns. Regards, Anthony Liguori > Stefan > >