From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SiMNl-0006Yj-6e for qemu-devel@nongnu.org; Sat, 23 Jun 2012 05:11:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SiMNj-00073g-3x for qemu-devel@nongnu.org; Sat, 23 Jun 2012 05:11:52 -0400 Received: from mout.web.de ([212.227.15.3]:51597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SiMNi-00073Y-Pv for qemu-devel@nongnu.org; Sat, 23 Jun 2012 05:11:51 -0400 Message-ID: <4FE58854.2060604@web.de> Date: Sat, 23 Jun 2012 11:11:48 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4FE4F56D.1020201@web.de> <4FE4F8D3.5030203@codemonkey.ws> In-Reply-To: <4FE4F8D3.5030203@codemonkey.ws> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6B94179E532A26CB26C24645" 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: Anthony Liguori Cc: Liu Ping Fan , qemu-devel , Alexander Graf This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig6B94179E532A26CB26C24645 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2012-06-23 00:59, Anthony Liguori wrote: > On 06/22/2012 05:45 PM, Jan Kiszka wrote: >> This sketches a possible path to get rid of the iothread lock on vmexi= ts >> 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 =3D kvm_vcpu_ioctl(env, KVM_RUN, 0); >> >> - qemu_mutex_lock_iothread(); >> kvm_arch_post_run(env, run); >> >> + /* TODO: push coalesced mmio flushing to the point where we >> access >> + * devices that are using it (currently VGA and E1000). */ >> + qemu_mutex_lock_iothread(); >> kvm_flush_coalesced_mmio_buffer(); >> + qemu_mutex_unlock_iothread(); >> >> if (run_ret< 0) { >> if (run_ret =3D=3D -EINTR || run_ret =3D=3D -EAGAIN) { >> @@ -1499,19 +1503,23 @@ int kvm_cpu_exec(CPUArchState *env) >> switch (run->exit_reason) { >> case KVM_EXIT_IO: >> DPRINTF("handle_io\n"); >> + qemu_mutex_lock_iothread(); >> kvm_handle_io(run->io.port, >> (uint8_t *)run + run->io.data_offset, >> run->io.direction, >> run->io.size, >> run->io.count); >> + qemu_mutex_unlock_iothread(); >> ret =3D 0; >> break; >> case KVM_EXIT_MMIO: >> DPRINTF("handle_mmio\n"); >> + qemu_mutex_lock_iothread(); >> cpu_physical_memory_rw(run->mmio.phys_addr, >> run->mmio.data, >> run->mmio.len, >> run->mmio.is_write); >> + qemu_mutex_unlock_iothread(); >> ret =3D 0; >> break; >> case KVM_EXIT_IRQ_WINDOW_OPEN: >> @@ -1520,7 +1528,9 @@ int kvm_cpu_exec(CPUArchState *env) >> break; >> case KVM_EXIT_SHUTDOWN: >> DPRINTF("shutdown\n"); >> + qemu_mutex_lock_iothread(); >> qemu_system_reset_request(); >> + qemu_mutex_unlock_iothread(); >> ret =3D EXCP_INTERRUPT; >> break; >> case KVM_EXIT_UNKNOWN: >> @@ -1533,11 +1543,15 @@ int kvm_cpu_exec(CPUArchState *env) >> break; >> default: >> DPRINTF("kvm_arch_handle_exit\n"); >> + qemu_mutex_lock_iothread(); >> ret =3D kvm_arch_handle_exit(env, run); >> + qemu_mutex_unlock_iothread(); >> break; >> } >> } while (ret =3D=3D 0); >> >> + qemu_mutex_lock_iothread(); >> + >> if (ret< 0) { >> cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE); >> vm_stop(RUN_STATE_INTERNAL_ERROR); >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >> index 0d0d8f6..0ad64d1 100644 >> --- a/target-i386/kvm.c >> +++ b/target-i386/kvm.c >> @@ -1631,7 +1631,10 @@ void kvm_arch_pre_run(CPUX86State *env, struct >> kvm_run *run) >> >> /* Inject NMI */ >> if (env->interrupt_request& CPU_INTERRUPT_NMI) { >=20 > Strictly speaking, wouldn't we need to use testbit() and setbit()? I > would expect in the very least a barrier would be needed. I need to think about this as well. We ignored it so far, just saw it when hacking up this patch. >=20 > Looks pretty nice overall. I'll need to apply and spend some time > carefully walking through the code. Without getting the coalesced mmio flushing out of the way, it does not buy us that much yet. But I have some idea... Jan --------------enig6B94179E532A26CB26C24645 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/liFQACgkQitSsb3rl5xSr2gCeJUxsCkISdBClHaCY/Y2cSImL bmIAoMlMhhBaG230w4J0h8l4exVT/qre =kB8p -----END PGP SIGNATURE----- --------------enig6B94179E532A26CB26C24645--