On Tue, 06/23 11:29, Paolo Bonzini wrote:
>
>
> On 23/06/2015 11:26, Fam Zheng wrote:
> > On Thu, 06/18 18:47, Paolo Bonzini wrote:
> >> From: Jan Kiszka <jan.kis...@siemens.com>
> >>
> >> 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 <jan.kis...@siemens.com>
> >> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> >> ---
> >> 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.
But above you add lock/unlock around the qemu_system_reset_request() under
KVM_EXIT_SHUTDOWN. What's different?
Fam
>
> 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
> >
> >
