From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50421) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7KWi-0001ZR-5P for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:29:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7KWZ-0003ra-LS for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:29:56 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:38419) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7KWZ-0003rN-Bk for qemu-devel@nongnu.org; Tue, 23 Jun 2015 05:29:47 -0400 Received: by wibdq8 with SMTP id dq8so10602926wib.1 for ; Tue, 23 Jun 2015 02:29:46 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <55892707.4040103@redhat.com> Date: Tue, 23 Jun 2015 11:29:43 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1434646046-27150-1-git-send-email-pbonzini@redhat.com> <1434646046-27150-7-git-send-email-pbonzini@redhat.com> <20150623092620.GA5798@ad.nay.redhat.com> In-Reply-To: <20150623092620.GA5798@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/9] 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: Fam Zheng Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org On 23/06/2015 11:26, Fam Zheng wrote: > On Thu, 06/18 18:47, Paolo Bonzini wrote: >> From: Jan Kiszka >> >> This opens the path to get rid of the iothread lock on vmexits in KVM >> mode. On x86, 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. >> >> s390x and ARM should be fine without specific locking as their >> pre/post-run callbacks are empty. MIPS and POWER require locking for >> the pre-run callback. >> >> Signed-off-by: Jan Kiszka >> Signed-off-by: Paolo Bonzini >> --- >> kvm-all.c | 14 ++++++++++++-- >> target-i386/kvm.c | 18 ++++++++++++++++++ >> target-mips/kvm.c | 4 ++++ >> target-ppc/kvm.c | 4 ++++ >> 4 files changed, 38 insertions(+), 2 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index b2b1bc3..2bd8e9b 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -1795,6 +1795,8 @@ int kvm_cpu_exec(CPUState *cpu) >> return EXCP_HLT; >> } >> >> + qemu_mutex_unlock_iothread(); >> + >> do { >> MemTxAttrs attrs; >> >> @@ -1813,11 +1815,9 @@ int kvm_cpu_exec(CPUState *cpu) >> */ >> qemu_cpu_kick_self(); >> } >> - qemu_mutex_unlock_iothread(); >> >> run_ret = kvm_vcpu_ioctl(cpu, KVM_RUN, 0); >> >> - qemu_mutex_lock_iothread(); >> attrs = kvm_arch_post_run(cpu, run); >> >> if (run_ret < 0) { >> @@ -1836,20 +1836,24 @@ int kvm_cpu_exec(CPUState *cpu) >> switch (run->exit_reason) { >> case KVM_EXIT_IO: >> DPRINTF("handle_io\n"); >> + qemu_mutex_lock_iothread(); >> kvm_handle_io(run->io.port, attrs, >> (uint8_t *)run + run->io.data_offset, >> run->io.direction, >> run->io.size, >> run->io.count); >> + qemu_mutex_unlock_iothread(); >> ret = 0; >> break; >> case KVM_EXIT_MMIO: >> DPRINTF("handle_mmio\n"); >> + qemu_mutex_lock_iothread(); >> address_space_rw(&address_space_memory, >> run->mmio.phys_addr, attrs, >> run->mmio.data, >> run->mmio.len, >> run->mmio.is_write); >> + qemu_mutex_unlock_iothread(); >> ret = 0; >> break; >> case KVM_EXIT_IRQ_WINDOW_OPEN: >> @@ -1858,7 +1862,9 @@ int kvm_cpu_exec(CPUState *cpu) >> break; >> case KVM_EXIT_SHUTDOWN: >> DPRINTF("shutdown\n"); >> + qemu_mutex_lock_iothread(); >> qemu_system_reset_request(); >> + qemu_mutex_unlock_iothread(); >> ret = EXCP_INTERRUPT; >> break; >> case KVM_EXIT_UNKNOWN: > > More context: > > fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n", > (uint64_t)run->hw.hardware_exit_reason); > ret = -1; > break; > case KVM_EXIT_INTERNAL_ERROR: > * ret = kvm_handle_internal_error(cpu, run); This one only accesses data internal to the VCPU thread. > break; > case KVM_EXIT_SYSTEM_EVENT: > switch (run->system_event.type) { > case KVM_SYSTEM_EVENT_SHUTDOWN: > * qemu_system_shutdown_request(); > ret = EXCP_INTERRUPT; > break; > case KVM_SYSTEM_EVENT_RESET: > * qemu_system_reset_request(); These two are thread-safe. Paolo > ret = EXCP_INTERRUPT; >> break; >> default: >> DPRINTF("kvm_arch_handle_exit\n"); >> + qemu_mutex_lock_iothread(); >> ret = kvm_arch_handle_exit(cpu, run); >> + qemu_mutex_unlock_iothread(); >> break; >> } >> } while (ret == 0); >> >> + qemu_mutex_lock_iothread(); >> + >> if (ret < 0) { >> cpu_dump_state(cpu, stderr, fprintf, CPU_DUMP_CODE); >> vm_stop(RUN_STATE_INTERNAL_ERROR); > > Could you explain why above three "*" calls are safe? > > Fam > >