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.
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
>
>
