From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SiOf8-0007Dv-DT for qemu-devel@nongnu.org; Sat, 23 Jun 2012 07:37:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SiOf6-0000W3-Gu for qemu-devel@nongnu.org; Sat, 23 Jun 2012 07:37:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31719) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SiOf6-0000Vq-96 for qemu-devel@nongnu.org; Sat, 23 Jun 2012 07:37:56 -0400 Date: Sat, 23 Jun 2012 06:06:46 -0300 From: Marcelo Tosatti Message-ID: <20120623090646.GA21908@amt.cnet> References: <4FE4F56D.1020201@web.de> <4FE4F7F5.7030400@web.de> <20120623002259.GA13440@amt.cnet> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120623002259.GA13440@amt.cnet> Subject: Re: [Qemu-devel] [PATCH] kvm: First step to push iothread lock out of inner run loop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Liu Ping Fan , kvm , qemu-devel , Alexander Graf , Avi Kivity , Anthony Liguori On Fri, Jun 22, 2012 at 09:22:59PM -0300, Marcelo Tosatti wrote: > On Sat, Jun 23, 2012 at 12:55:49AM +0200, Jan Kiszka wrote: > > Should have declared this [RFC] in the subject and CC'ed kvm... > > > > On 2012-06-23 00:45, Jan Kiszka wrote: > > > This sketches a possible path to get rid of the iothread lock on vmexits > > > in KVM mode. On x86, the the in-kernel irqchips has to be used because > > > we otherwise need to synchronize APIC and other per-cpu state accesses > > > that could be changed concurrently. Not yet fully analyzed is the NMI > > > injection path in the absence of an APIC. > > > > > > s390x should be fine without specific locking as their pre/post-run > > > callbacks are empty. Power requires locking for the pre-run callback. > > > > > > This patch is untested, but a similar version was successfully used in > > > a x86 setup with a network I/O path that needed no central iothread > > > locking anymore (required special MMIO exit handling). > > > --- > > > kvm-all.c | 18 ++++++++++++++++-- > > > target-i386/kvm.c | 7 +++++++ > > > target-ppc/kvm.c | 4 ++++ > > > 3 files changed, 27 insertions(+), 2 deletions(-) > > > > > > diff --git a/kvm-all.c b/kvm-all.c > > > index f8e4328..9c3e26f 100644 > > > --- a/kvm-all.c > > > +++ b/kvm-all.c > > > @@ -1460,6 +1460,8 @@ int kvm_cpu_exec(CPUArchState *env) > > > return EXCP_HLT; > > > } > > > > > > + qemu_mutex_unlock_iothread(); > > > + > > > do { > > > if (env->kvm_vcpu_dirty) { > > > kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE); > > > @@ -1476,14 +1478,16 @@ int kvm_cpu_exec(CPUArchState *env) > > > */ > > > qemu_cpu_kick_self(); > > > } > > > - qemu_mutex_unlock_iothread(); > > > > > > run_ret = kvm_vcpu_ioctl(env, KVM_RUN, 0); > > > > > > - qemu_mutex_lock_iothread(); > > > kvm_arch_post_run(env, run); > > target-i386/kvm.c > > void kvm_arch_post_run(CPUX86State *env, struct kvm_run *run) > { > if (run->if_flag) { > env->eflags |= IF_MASK; > } else { > env->eflags &= ~IF_MASK; > } > cpu_set_apic_tpr(env->apic_state, run->cr8); > cpu_set_apic_base(env->apic_state, run->apic_base); > } > > Clearly there is no structure to any of the writes around the writes > in x86's kvm_arch_post_run, so it is unsafe. No access protocol to the CPUState and apic devices (who can write when, who can read when).