[PULL,08/24] tcg: drop global lock during TCG code execution

From: Jan Kiszka <jan.kiszka@siemens.com>


This finally allows TCG to benefit from the iothread introduction: Drop
the global mutex while running pure TCG CPU code. Reacquire the lock
when entering MMIO or PIO emulation, or when leaving the TCG loop.

We have to revert a few optimization for the current TCG threading
model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
kicking it in qemu_cpu_kick. We also need to disable RAM block
reordering until we have a more efficient locking mechanism at hand.

Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
These numbers demonstrate where we gain something:

20338 jan       20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm
20337 jan       20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

The guest CPU was fully loaded, but the iothread could still run mostly
independent on a second core. Without the patch we don't get beyond

32206 jan       20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm
32204 jan       20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

We don't benefit significantly, though, when the guest is not fully
loading a host CPU.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>
[FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

[EGC: fixed iothread lock for cpu-exec IRQ handling]
Signed-off-by: Emilio G. Cota <cota@braap.org>

[AJB: -smp single-threaded fix, clean commit msg, BQL fixes]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

[PM: target-arm changes]
Acked-by: Peter Maydell <peter.maydell@linaro.org>

---
 cpu-exec.c                 | 23 +++++++++++++++++++++--
 cpus.c                     | 28 +++++-----------------------
 cputlb.c                   | 21 ++++++++++++++++++++-
 exec.c                     | 12 +++++++++---
 hw/core/irq.c              |  1 +
 hw/i386/kvmvapic.c         |  4 ++--
 hw/intc/arm_gicv3_cpuif.c  |  3 +++
 hw/ppc/ppc.c               | 16 +++++++++++++++-
 hw/ppc/spapr.c             |  3 +++
 include/qom/cpu.h          |  1 +
 memory.c                   |  2 ++
 qom/cpu.c                  | 10 ++++++++++
 target/arm/helper.c        |  6 ++++++
 target/arm/op_helper.c     | 43 +++++++++++++++++++++++++++++++++++++++----
 target/i386/smm_helper.c   |  7 +++++++
 target/s390x/misc_helper.c |  5 ++++-
 translate-all.c            |  9 +++++++--
 translate-common.c         | 21 +++++++++++----------
 18 files changed, 166 insertions(+), 49 deletions(-)

-- 
2.11.0

Hello,

On Fri, Feb 24, 2017 at 12:20 PM, Alex Bennée <alex.bennee@linaro.org> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>

>

> This finally allows TCG to benefit from the iothread introduction: Drop

> the global mutex while running pure TCG CPU code. Reacquire the lock

> when entering MMIO or PIO emulation, or when leaving the TCG loop.

>

> We have to revert a few optimization for the current TCG threading

> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not

> kicking it in qemu_cpu_kick. We also need to disable RAM block

> reordering until we have a more efficient locking mechanism at hand.

>

> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.

> These numbers demonstrate where we gain something:

>

> 20338 jan       20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm

> 20337 jan       20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

>

> The guest CPU was fully loaded, but the iothread could still run mostly

> independent on a second core. Without the patch we don't get beyond

>

> 32206 jan       20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm

> 32204 jan       20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

>

> We don't benefit significantly, though, when the guest is not fully

> loading a host CPU.


I tried this patch (8d04fb55 in the repository) with the following image:

   http://wiki.qemu.org/download/arm-test-0.2.tar.gz

Running the image with no option works fine.  But specifying '-icount
1' results in a (guest) deadlock. Enabling some heavy logging (-d
in_asm,exec) sometimes results in a 'Bad ram offset'.

Is it expected that this patch breaks -icount?

Thanks,

Laurent

PS - To clarify 791158d9 works.

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>

> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]

> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

> [EGC: fixed iothread lock for cpu-exec IRQ handling]

> Signed-off-by: Emilio G. Cota <cota@braap.org>

> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> Reviewed-by: Richard Henderson <rth@twiddle.net>

> Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

> [PM: target-arm changes]

> Acked-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  cpu-exec.c                 | 23 +++++++++++++++++++++--

>  cpus.c                     | 28 +++++-----------------------

>  cputlb.c                   | 21 ++++++++++++++++++++-

>  exec.c                     | 12 +++++++++---

>  hw/core/irq.c              |  1 +

>  hw/i386/kvmvapic.c         |  4 ++--

>  hw/intc/arm_gicv3_cpuif.c  |  3 +++

>  hw/ppc/ppc.c               | 16 +++++++++++++++-

>  hw/ppc/spapr.c             |  3 +++

>  include/qom/cpu.h          |  1 +

>  memory.c                   |  2 ++

>  qom/cpu.c                  | 10 ++++++++++

>  target/arm/helper.c        |  6 ++++++

>  target/arm/op_helper.c     | 43 +++++++++++++++++++++++++++++++++++++++----

>  target/i386/smm_helper.c   |  7 +++++++

>  target/s390x/misc_helper.c |  5 ++++-

>  translate-all.c            |  9 +++++++--

>  translate-common.c         | 21 +++++++++++----------

>  18 files changed, 166 insertions(+), 49 deletions(-)

>

> diff --git a/cpu-exec.c b/cpu-exec.c

> index 06a6b25564..1bd3d72002 100644

> --- a/cpu-exec.c

> +++ b/cpu-exec.c

> @@ -29,6 +29,7 @@

>  #include "qemu/rcu.h"

>  #include "exec/tb-hash.h"

>  #include "exec/log.h"

> +#include "qemu/main-loop.h"

>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)

>  #include "hw/i386/apic.h"

>  #endif

> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)

>          if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)

>              && replay_interrupt()) {

>              X86CPU *x86_cpu = X86_CPU(cpu);

> +            qemu_mutex_lock_iothread();

>              apic_poll_irq(x86_cpu->apic_state);

>              cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);

> +            qemu_mutex_unlock_iothread();

>          }

>  #endif

>          if (!cpu_has_work(cpu)) {

> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)

>  #else

>              if (replay_exception()) {

>                  CPUClass *cc = CPU_GET_CLASS(cpu);

> +                qemu_mutex_lock_iothread();

>                  cc->do_interrupt(cpu);

> +                qemu_mutex_unlock_iothread();

>                  cpu->exception_index = -1;

>              } else if (!replay_has_interrupt()) {

>                  /* give a chance to iothread in replay mode */

> @@ -469,9 +474,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>                                          TranslationBlock **last_tb)

>  {

>      CPUClass *cc = CPU_GET_CLASS(cpu);

> -    int interrupt_request = cpu->interrupt_request;

>

> -    if (unlikely(interrupt_request)) {

> +    if (unlikely(atomic_read(&cpu->interrupt_request))) {

> +        int interrupt_request;

> +        qemu_mutex_lock_iothread();

> +        interrupt_request = cpu->interrupt_request;

>          if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {

>              /* Mask out external interrupts for this step. */

>              interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;

> @@ -479,6 +486,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>          if (interrupt_request & CPU_INTERRUPT_DEBUG) {

>              cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;

>              cpu->exception_index = EXCP_DEBUG;

> +            qemu_mutex_unlock_iothread();

>              return true;

>          }

>          if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {

> @@ -488,6 +496,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>              cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;

>              cpu->halted = 1;

>              cpu->exception_index = EXCP_HLT;

> +            qemu_mutex_unlock_iothread();

>              return true;

>          }

>  #if defined(TARGET_I386)

> @@ -498,12 +507,14 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>              cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);

>              do_cpu_init(x86_cpu);

>              cpu->exception_index = EXCP_HALTED;

> +            qemu_mutex_unlock_iothread();

>              return true;

>          }

>  #else

>          else if (interrupt_request & CPU_INTERRUPT_RESET) {

>              replay_interrupt();

>              cpu_reset(cpu);

> +            qemu_mutex_unlock_iothread();

>              return true;

>          }

>  #endif

> @@ -526,7 +537,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>                 the program flow was changed */

>              *last_tb = NULL;

>          }

> +

> +        /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */

> +        qemu_mutex_unlock_iothread();

>      }

> +

> +

>      if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {

>          atomic_set(&cpu->exit_request, 0);

>          cpu->exception_index = EXCP_INTERRUPT;

> @@ -643,6 +659,9 @@ int cpu_exec(CPUState *cpu)

>  #endif /* buggy compiler */

>          cpu->can_do_io = 1;

>          tb_lock_reset();

> +        if (qemu_mutex_iothread_locked()) {

> +            qemu_mutex_unlock_iothread();

> +        }

>      }

>

>      /* if an exception is pending, we execute it here */

> diff --git a/cpus.c b/cpus.c

> index 860034a794..0ae8f69be5 100644

> --- a/cpus.c

> +++ b/cpus.c

> @@ -1027,8 +1027,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)

>  #endif /* _WIN32 */

>

>  static QemuMutex qemu_global_mutex;

> -static QemuCond qemu_io_proceeded_cond;

> -static unsigned iothread_requesting_mutex;

>

>  static QemuThread io_thread;

>

> @@ -1042,7 +1040,6 @@ void qemu_init_cpu_loop(void)

>      qemu_init_sigbus();

>      qemu_cond_init(&qemu_cpu_cond);

>      qemu_cond_init(&qemu_pause_cond);

> -    qemu_cond_init(&qemu_io_proceeded_cond);

>      qemu_mutex_init(&qemu_global_mutex);

>

>      qemu_thread_get_self(&io_thread);

> @@ -1085,10 +1082,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)

>

>      start_tcg_kick_timer();

>

> -    while (iothread_requesting_mutex) {

> -        qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);

> -    }

> -

>      CPU_FOREACH(cpu) {

>          qemu_wait_io_event_common(cpu);

>      }

> @@ -1249,9 +1242,11 @@ static int tcg_cpu_exec(CPUState *cpu)

>          cpu->icount_decr.u16.low = decr;

>          cpu->icount_extra = count;

>      }

> +    qemu_mutex_unlock_iothread();

>      cpu_exec_start(cpu);

>      ret = cpu_exec(cpu);

>      cpu_exec_end(cpu);

> +    qemu_mutex_lock_iothread();

>  #ifdef CONFIG_PROFILER

>      tcg_time += profile_getclock() - ti;

>  #endif

> @@ -1479,27 +1474,14 @@ bool qemu_mutex_iothread_locked(void)

>

>  void qemu_mutex_lock_iothread(void)

>  {

> -    atomic_inc(&iothread_requesting_mutex);

> -    /* In the simple case there is no need to bump the VCPU thread out of

> -     * TCG code execution.

> -     */

> -    if (!tcg_enabled() || qemu_in_vcpu_thread() ||

> -        !first_cpu || !first_cpu->created) {

> -        qemu_mutex_lock(&qemu_global_mutex);

> -        atomic_dec(&iothread_requesting_mutex);

> -    } else {

> -        if (qemu_mutex_trylock(&qemu_global_mutex)) {

> -            qemu_cpu_kick_rr_cpu();

> -            qemu_mutex_lock(&qemu_global_mutex);

> -        }

> -        atomic_dec(&iothread_requesting_mutex);

> -        qemu_cond_broadcast(&qemu_io_proceeded_cond);

> -    }

> +    g_assert(!qemu_mutex_iothread_locked());

> +    qemu_mutex_lock(&qemu_global_mutex);

>      iothread_locked = true;

>  }

>

>  void qemu_mutex_unlock_iothread(void)

>  {

> +    g_assert(qemu_mutex_iothread_locked());

>      iothread_locked = false;

>      qemu_mutex_unlock(&qemu_global_mutex);

>  }

> diff --git a/cputlb.c b/cputlb.c

> index 6c39927455..1cc9d9da51 100644

> --- a/cputlb.c

> +++ b/cputlb.c

> @@ -18,6 +18,7 @@

>   */

>

>  #include "qemu/osdep.h"

> +#include "qemu/main-loop.h"

>  #include "cpu.h"

>  #include "exec/exec-all.h"

>  #include "exec/memory.h"

> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      hwaddr physaddr = iotlbentry->addr;

>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

>      uint64_t val;

> +    bool locked = false;

>

>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>      cpu->mem_io_pc = retaddr;

> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      }

>

>      cpu->mem_io_vaddr = addr;

> +

> +    if (mr->global_locking) {

> +        qemu_mutex_lock_iothread();

> +        locked = true;

> +    }

>      memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);

> +    if (locked) {

> +        qemu_mutex_unlock_iothread();

> +    }

> +

>      return val;

>  }

>

> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>      CPUState *cpu = ENV_GET_CPU(env);

>      hwaddr physaddr = iotlbentry->addr;

>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

> +    bool locked = false;

>

>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {

>          cpu_io_recompile(cpu, retaddr);

>      }

> -

>      cpu->mem_io_vaddr = addr;

>      cpu->mem_io_pc = retaddr;

> +

> +    if (mr->global_locking) {

> +        qemu_mutex_lock_iothread();

> +        locked = true;

> +    }

>      memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);

> +    if (locked) {

> +        qemu_mutex_unlock_iothread();

> +    }

>  }

>

>  /* Return true if ADDR is present in the victim tlb, and has been copied

> diff --git a/exec.c b/exec.c

> index 865a1e8295..3adf2b1861 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -2134,9 +2134,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)

>                  }

>                  cpu->watchpoint_hit = wp;

>

> -                /* The tb_lock will be reset when cpu_loop_exit or

> -                 * cpu_loop_exit_noexc longjmp back into the cpu_exec

> -                 * main loop.

> +                /* Both tb_lock and iothread_mutex will be reset when

> +                 * cpu_loop_exit or cpu_loop_exit_noexc longjmp

> +                 * back into the cpu_exec main loop.

>                   */

>                  tb_lock();

>                  tb_check_watchpoint(cpu);

> @@ -2371,8 +2371,14 @@ static void io_mem_init(void)

>      memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);

>      memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,

>                            NULL, UINT64_MAX);

> +

> +    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,

> +     * which can be called without the iothread mutex.

> +     */

>      memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,

>                            NULL, UINT64_MAX);

> +    memory_region_clear_global_locking(&io_mem_notdirty);

> +

>      memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,

>                            NULL, UINT64_MAX);

>  }

> diff --git a/hw/core/irq.c b/hw/core/irq.c

> index 49ff2e64fe..b98d1d69f5 100644

> --- a/hw/core/irq.c

> +++ b/hw/core/irq.c

> @@ -22,6 +22,7 @@

>   * THE SOFTWARE.

>   */

>  #include "qemu/osdep.h"

> +#include "qemu/main-loop.h"

>  #include "qemu-common.h"

>  #include "hw/irq.h"

>  #include "qom/object.h"

> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c

> index 7135633863..82a49556af 100644

> --- a/hw/i386/kvmvapic.c

> +++ b/hw/i386/kvmvapic.c

> @@ -457,8 +457,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)

>      resume_all_vcpus();

>

>      if (!kvm_enabled()) {

> -        /* tb_lock will be reset when cpu_loop_exit_noexc longjmps

> -         * back into the cpu_exec loop. */

> +        /* Both tb_lock and iothread_mutex will be reset when

> +         *  longjmps back into the cpu_exec loop. */

>          tb_lock();

>          tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);

>          cpu_loop_exit_noexc(cs);

> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c

> index c25ee03556..f775aba507 100644

> --- a/hw/intc/arm_gicv3_cpuif.c

> +++ b/hw/intc/arm_gicv3_cpuif.c

> @@ -14,6 +14,7 @@

>

>  #include "qemu/osdep.h"

>  #include "qemu/bitops.h"

> +#include "qemu/main-loop.h"

>  #include "trace.h"

>  #include "gicv3_internal.h"

>  #include "cpu.h"

> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)

>      ARMCPU *cpu = ARM_CPU(cs->cpu);

>      CPUARMState *env = &cpu->env;

>

> +    g_assert(qemu_mutex_iothread_locked());

> +

>      trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,

>                               cs->hppi.grp, cs->hppi.prio);

>

> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c

> index d171e60b5c..5f93083d4a 100644

> --- a/hw/ppc/ppc.c

> +++ b/hw/ppc/ppc.c

> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>  {

>      CPUState *cs = CPU(cpu);

>      CPUPPCState *env = &cpu->env;

> -    unsigned int old_pending = env->pending_interrupts;

> +    unsigned int old_pending;

> +    bool locked = false;

> +

> +    /* We may already have the BQL if coming from the reset path */

> +    if (!qemu_mutex_iothread_locked()) {

> +        locked = true;

> +        qemu_mutex_lock_iothread();

> +    }

> +

> +    old_pending = env->pending_interrupts;

>

>      if (level) {

>          env->pending_interrupts |= 1 << n_IRQ;

> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>  #endif

>      }

>

> +

>      LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32

>                  "req %08x\n", __func__, env, n_IRQ, level,

>                  env->pending_interrupts, CPU(cpu)->interrupt_request);

> +

> +    if (locked) {

> +        qemu_mutex_unlock_iothread();

> +    }

>  }

>

>  /* PowerPC 6xx / 7xx internal IRQ controller */

> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

> index e465d7ac98..b1e374f3f9 100644

> --- a/hw/ppc/spapr.c

> +++ b/hw/ppc/spapr.c

> @@ -1010,6 +1010,9 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,

>  {

>      CPUPPCState *env = &cpu->env;

>

> +    /* The TCG path should also be holding the BQL at this point */

> +    g_assert(qemu_mutex_iothread_locked());

> +

>      if (msr_pr) {

>          hcall_dprintf("Hypercall made with MSR[PR]=1\n");

>          env->gpr[3] = H_PRIVILEGE;

> diff --git a/include/qom/cpu.h b/include/qom/cpu.h

> index 2cf4ecf144..10db89b16a 100644

> --- a/include/qom/cpu.h

> +++ b/include/qom/cpu.h

> @@ -329,6 +329,7 @@ struct CPUState {

>      bool unplug;

>      bool crash_occurred;

>      bool exit_request;

> +    /* updates protected by BQL */

>      uint32_t interrupt_request;

>      int singlestep_enabled;

>      int64_t icount_extra;

> diff --git a/memory.c b/memory.c

> index ed8b5aa83e..d61caee867 100644

> --- a/memory.c

> +++ b/memory.c

> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)

>      AddressSpace *as;

>

>      assert(memory_region_transaction_depth);

> +    assert(qemu_mutex_iothread_locked());

> +

>      --memory_region_transaction_depth;

>      if (!memory_region_transaction_depth) {

>          if (memory_region_update_pending) {

> diff --git a/qom/cpu.c b/qom/cpu.c

> index ed87c50cea..58784bcbea 100644

> --- a/qom/cpu.c

> +++ b/qom/cpu.c

> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,

>      error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");

>  }

>

> +/* Resetting the IRQ comes from across the code base so we take the

> + * BQL here if we need to.  cpu_interrupt assumes it is held.*/

>  void cpu_reset_interrupt(CPUState *cpu, int mask)

>  {

> +    bool need_lock = !qemu_mutex_iothread_locked();

> +

> +    if (need_lock) {

> +        qemu_mutex_lock_iothread();

> +    }

>      cpu->interrupt_request &= ~mask;

> +    if (need_lock) {

> +        qemu_mutex_unlock_iothread();

> +    }

>  }

>

>  void cpu_exit(CPUState *cpu)

> diff --git a/target/arm/helper.c b/target/arm/helper.c

> index 47250bcf16..753a69d40d 100644

> --- a/target/arm/helper.c

> +++ b/target/arm/helper.c

> @@ -6769,6 +6769,12 @@ void arm_cpu_do_interrupt(CPUState *cs)

>          arm_cpu_do_interrupt_aarch32(cs);

>      }

>

> +    /* Hooks may change global state so BQL should be held, also the

> +     * BQL needs to be held for any modification of

> +     * cs->interrupt_request.

> +     */

> +    g_assert(qemu_mutex_iothread_locked());

> +

>      arm_call_el_change_hook(cpu);

>

>      if (!kvm_enabled()) {

> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c

> index fb366fdc35..5f3e3bdae2 100644

> --- a/target/arm/op_helper.c

> +++ b/target/arm/op_helper.c

> @@ -18,6 +18,7 @@

>   */

>  #include "qemu/osdep.h"

>  #include "qemu/log.h"

> +#include "qemu/main-loop.h"

>  #include "cpu.h"

>  #include "exec/helper-proto.h"

>  #include "internals.h"

> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)

>       */

>      env->regs[15] &= (env->thumb ? ~1 : ~3);

>

> +    qemu_mutex_lock_iothread();

>      arm_call_el_change_hook(arm_env_get_cpu(env));

> +    qemu_mutex_unlock_iothread();

>  }

>

>  /* Access to user mode registers from privileged modes.  */

> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)

>  {

>      const ARMCPRegInfo *ri = rip;

>

> -    ri->writefn(env, ri, value);

> +    if (ri->type & ARM_CP_IO) {

> +        qemu_mutex_lock_iothread();

> +        ri->writefn(env, ri, value);

> +        qemu_mutex_unlock_iothread();

> +    } else {

> +        ri->writefn(env, ri, value);

> +    }

>  }

>

>  uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)

>  {

>      const ARMCPRegInfo *ri = rip;

> +    uint32_t res;

>

> -    return ri->readfn(env, ri);

> +    if (ri->type & ARM_CP_IO) {

> +        qemu_mutex_lock_iothread();

> +        res = ri->readfn(env, ri);

> +        qemu_mutex_unlock_iothread();

> +    } else {

> +        res = ri->readfn(env, ri);

> +    }

> +

> +    return res;

>  }

>

>  void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)

>  {

>      const ARMCPRegInfo *ri = rip;

>

> -    ri->writefn(env, ri, value);

> +    if (ri->type & ARM_CP_IO) {

> +        qemu_mutex_lock_iothread();

> +        ri->writefn(env, ri, value);

> +        qemu_mutex_unlock_iothread();

> +    } else {

> +        ri->writefn(env, ri, value);

> +    }

>  }

>

>  uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)

>  {

>      const ARMCPRegInfo *ri = rip;

> +    uint64_t res;

> +

> +    if (ri->type & ARM_CP_IO) {

> +        qemu_mutex_lock_iothread();

> +        res = ri->readfn(env, ri);

> +        qemu_mutex_unlock_iothread();

> +    } else {

> +        res = ri->readfn(env, ri);

> +    }

>

> -    return ri->readfn(env, ri);

> +    return res;

>  }

>

>  void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)

> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)

>                        cur_el, new_el, env->pc);

>      }

>

> +    qemu_mutex_lock_iothread();

>      arm_call_el_change_hook(arm_env_get_cpu(env));

> +    qemu_mutex_unlock_iothread();

>

>      return;

>

> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c

> index 4dd6a2c544..f051a77c4a 100644

> --- a/target/i386/smm_helper.c

> +++ b/target/i386/smm_helper.c

> @@ -18,6 +18,7 @@

>   */

>

>  #include "qemu/osdep.h"

> +#include "qemu/main-loop.h"

>  #include "cpu.h"

>  #include "exec/helper-proto.h"

>  #include "exec/log.h"

> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)

>  #define SMM_REVISION_ID 0x00020000

>  #endif

>

> +/* Called with iothread lock taken */

>  void cpu_smm_update(X86CPU *cpu)

>  {

>      CPUX86State *env = &cpu->env;

>      bool smm_enabled = (env->hflags & HF_SMM_MASK);

>

> +    g_assert(qemu_mutex_iothread_locked());

> +

>      if (cpu->smram) {

>          memory_region_set_enabled(cpu->smram, smm_enabled);

>      }

> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)

>      }

>      env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;

>      env->hflags &= ~HF_SMM_MASK;

> +

> +    qemu_mutex_lock_iothread();

>      cpu_smm_update(cpu);

> +    qemu_mutex_unlock_iothread();

>

>      qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");

>      log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);

> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c

> index c9604ea9c7..3cb942e8bb 100644

> --- a/target/s390x/misc_helper.c

> +++ b/target/s390x/misc_helper.c

> @@ -25,6 +25,7 @@

>  #include "exec/helper-proto.h"

>  #include "sysemu/kvm.h"

>  #include "qemu/timer.h"

> +#include "qemu/main-loop.h"

>  #include "exec/address-spaces.h"

>  #ifdef CONFIG_KVM

>  #include <linux/kvm.h>

> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)

>  /* SCLP service call */

>  uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)

>  {

> +    qemu_mutex_lock_iothread();

>      int r = sclp_service_call(env, r1, r2);

>      if (r < 0) {

>          program_interrupt(env, -r, 4);

> -        return 0;

> +        r = 0;

>      }

> +    qemu_mutex_unlock_iothread();

>      return r;

>  }

>

> diff --git a/translate-all.c b/translate-all.c

> index 8a861cb583..f810259c41 100644

> --- a/translate-all.c

> +++ b/translate-all.c

> @@ -55,6 +55,7 @@

>  #include "translate-all.h"

>  #include "qemu/bitmap.h"

>  #include "qemu/timer.h"

> +#include "qemu/main-loop.h"

>  #include "exec/log.h"

>

>  /* #define DEBUG_TB_INVALIDATE */

> @@ -1523,7 +1524,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,

>  #ifdef CONFIG_SOFTMMU

>  /* len must be <= 8 and start must be a multiple of len.

>   * Called via softmmu_template.h when code areas are written to with

> - * tb_lock held.

> + * iothread mutex not held.

>   */

>  void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)

>  {

> @@ -1725,7 +1726,10 @@ void tb_check_watchpoint(CPUState *cpu)

>

>  #ifndef CONFIG_USER_ONLY

>  /* in deterministic execution mode, instructions doing device I/Os

> -   must be at the end of the TB */

> + * must be at the end of the TB.

> + *

> + * Called by softmmu_template.h, with iothread mutex not held.

> + */

>  void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)

>  {

>  #if defined(TARGET_MIPS) || defined(TARGET_SH4)

> @@ -1937,6 +1941,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)

>

>  void cpu_interrupt(CPUState *cpu, int mask)

>  {

> +    g_assert(qemu_mutex_iothread_locked());

>      cpu->interrupt_request |= mask;

>      cpu->tcg_exit_req = 1;

>  }

> diff --git a/translate-common.c b/translate-common.c

> index 5e989cdf70..d504dd0d33 100644

> --- a/translate-common.c

> +++ b/translate-common.c

> @@ -21,6 +21,7 @@

>  #include "qemu-common.h"

>  #include "qom/cpu.h"

>  #include "sysemu/cpus.h"

> +#include "qemu/main-loop.h"

>

>  uintptr_t qemu_real_host_page_size;

>  intptr_t qemu_real_host_page_mask;

> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;

>  static void tcg_handle_interrupt(CPUState *cpu, int mask)

>  {

>      int old_mask;

> +    g_assert(qemu_mutex_iothread_locked());

>

>      old_mask = cpu->interrupt_request;

>      cpu->interrupt_request |= mask;

> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)

>       */

>      if (!qemu_cpu_is_self(cpu)) {

>          qemu_cpu_kick(cpu);

> -        return;

> -    }

> -

> -    if (use_icount) {

> -        cpu->icount_decr.u16.high = 0xffff;

> -        if (!cpu->can_do_io

> -            && (mask & ~old_mask) != 0) {

> -            cpu_abort(cpu, "Raised interrupt while not in I/O function");

> -        }

>      } else {

> -        cpu->tcg_exit_req = 1;

> +        if (use_icount) {

> +            cpu->icount_decr.u16.high = 0xffff;

> +            if (!cpu->can_do_io

> +                && (mask & ~old_mask) != 0) {

> +                cpu_abort(cpu, "Raised interrupt while not in I/O function");

> +            }

> +        } else {

> +            cpu->tcg_exit_req = 1;

> +        }

>      }

>  }

>

> --

> 2.11.0

>

>

Laurent Desnogues <laurent.desnogues@gmail.com> writes:

> Hello,

>

> On Fri, Feb 24, 2017 at 12:20 PM, Alex Bennée <alex.bennee@linaro.org> wrote:

>> From: Jan Kiszka <jan.kiszka@siemens.com>

>>

>> This finally allows TCG to benefit from the iothread introduction: Drop

>> the global mutex while running pure TCG CPU code. Reacquire the lock

>> when entering MMIO or PIO emulation, or when leaving the TCG loop.

>>

>> We have to revert a few optimization for the current TCG threading

>> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not

>> kicking it in qemu_cpu_kick. We also need to disable RAM block

>> reordering until we have a more efficient locking mechanism at hand.

>>

>> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.

>> These numbers demonstrate where we gain something:

>>

>> 20338 jan       20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm

>> 20337 jan       20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

>>

>> The guest CPU was fully loaded, but the iothread could still run mostly

>> independent on a second core. Without the patch we don't get beyond

>>

>> 32206 jan       20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm

>> 32204 jan       20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

>>

>> We don't benefit significantly, though, when the guest is not fully

>> loading a host CPU.

>

> I tried this patch (8d04fb55 in the repository) with the following image:

>

>    http://wiki.qemu.org/download/arm-test-0.2.tar.gz

>

> Running the image with no option works fine.  But specifying '-icount

> 1' results in a (guest) deadlock. Enabling some heavy logging (-d

> in_asm,exec) sometimes results in a 'Bad ram offset'.

>

> Is it expected that this patch breaks -icount?


Not really. Using icount will disable MTTCG and run single threaded as
before. Paolo reported another icount failure so they may be related. I
shall have a look at it.

Thanks for the report.

>

> Thanks,

>

> Laurent

>

> PS - To clarify 791158d9 works.

>

>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

>> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>

>> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]

>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

>> [EGC: fixed iothread lock for cpu-exec IRQ handling]

>> Signed-off-by: Emilio G. Cota <cota@braap.org>

>> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> Reviewed-by: Richard Henderson <rth@twiddle.net>

>> Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

>> [PM: target-arm changes]

>> Acked-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>>  cpu-exec.c                 | 23 +++++++++++++++++++++--

>>  cpus.c                     | 28 +++++-----------------------

>>  cputlb.c                   | 21 ++++++++++++++++++++-

>>  exec.c                     | 12 +++++++++---

>>  hw/core/irq.c              |  1 +

>>  hw/i386/kvmvapic.c         |  4 ++--

>>  hw/intc/arm_gicv3_cpuif.c  |  3 +++

>>  hw/ppc/ppc.c               | 16 +++++++++++++++-

>>  hw/ppc/spapr.c             |  3 +++

>>  include/qom/cpu.h          |  1 +

>>  memory.c                   |  2 ++

>>  qom/cpu.c                  | 10 ++++++++++

>>  target/arm/helper.c        |  6 ++++++

>>  target/arm/op_helper.c     | 43 +++++++++++++++++++++++++++++++++++++++----

>>  target/i386/smm_helper.c   |  7 +++++++

>>  target/s390x/misc_helper.c |  5 ++++-

>>  translate-all.c            |  9 +++++++--

>>  translate-common.c         | 21 +++++++++++----------

>>  18 files changed, 166 insertions(+), 49 deletions(-)

>>

>> diff --git a/cpu-exec.c b/cpu-exec.c

>> index 06a6b25564..1bd3d72002 100644

>> --- a/cpu-exec.c

>> +++ b/cpu-exec.c

>> @@ -29,6 +29,7 @@

>>  #include "qemu/rcu.h"

>>  #include "exec/tb-hash.h"

>>  #include "exec/log.h"

>> +#include "qemu/main-loop.h"

>>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)

>>  #include "hw/i386/apic.h"

>>  #endif

>> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)

>>          if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)

>>              && replay_interrupt()) {

>>              X86CPU *x86_cpu = X86_CPU(cpu);

>> +            qemu_mutex_lock_iothread();

>>              apic_poll_irq(x86_cpu->apic_state);

>>              cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);

>> +            qemu_mutex_unlock_iothread();

>>          }

>>  #endif

>>          if (!cpu_has_work(cpu)) {

>> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)

>>  #else

>>              if (replay_exception()) {

>>                  CPUClass *cc = CPU_GET_CLASS(cpu);

>> +                qemu_mutex_lock_iothread();

>>                  cc->do_interrupt(cpu);

>> +                qemu_mutex_unlock_iothread();

>>                  cpu->exception_index = -1;

>>              } else if (!replay_has_interrupt()) {

>>                  /* give a chance to iothread in replay mode */

>> @@ -469,9 +474,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>>                                          TranslationBlock **last_tb)

>>  {

>>      CPUClass *cc = CPU_GET_CLASS(cpu);

>> -    int interrupt_request = cpu->interrupt_request;

>>

>> -    if (unlikely(interrupt_request)) {

>> +    if (unlikely(atomic_read(&cpu->interrupt_request))) {

>> +        int interrupt_request;

>> +        qemu_mutex_lock_iothread();

>> +        interrupt_request = cpu->interrupt_request;

>>          if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {

>>              /* Mask out external interrupts for this step. */

>>              interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;

>> @@ -479,6 +486,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>>          if (interrupt_request & CPU_INTERRUPT_DEBUG) {

>>              cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;

>>              cpu->exception_index = EXCP_DEBUG;

>> +            qemu_mutex_unlock_iothread();

>>              return true;

>>          }

>>          if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {

>> @@ -488,6 +496,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>>              cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;

>>              cpu->halted = 1;

>>              cpu->exception_index = EXCP_HLT;

>> +            qemu_mutex_unlock_iothread();

>>              return true;

>>          }

>>  #if defined(TARGET_I386)

>> @@ -498,12 +507,14 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>>              cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);

>>              do_cpu_init(x86_cpu);

>>              cpu->exception_index = EXCP_HALTED;

>> +            qemu_mutex_unlock_iothread();

>>              return true;

>>          }

>>  #else

>>          else if (interrupt_request & CPU_INTERRUPT_RESET) {

>>              replay_interrupt();

>>              cpu_reset(cpu);

>> +            qemu_mutex_unlock_iothread();

>>              return true;

>>          }

>>  #endif

>> @@ -526,7 +537,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>>                 the program flow was changed */

>>              *last_tb = NULL;

>>          }

>> +

>> +        /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */

>> +        qemu_mutex_unlock_iothread();

>>      }

>> +

>> +

>>      if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {

>>          atomic_set(&cpu->exit_request, 0);

>>          cpu->exception_index = EXCP_INTERRUPT;

>> @@ -643,6 +659,9 @@ int cpu_exec(CPUState *cpu)

>>  #endif /* buggy compiler */

>>          cpu->can_do_io = 1;

>>          tb_lock_reset();

>> +        if (qemu_mutex_iothread_locked()) {

>> +            qemu_mutex_unlock_iothread();

>> +        }

>>      }

>>

>>      /* if an exception is pending, we execute it here */

>> diff --git a/cpus.c b/cpus.c

>> index 860034a794..0ae8f69be5 100644

>> --- a/cpus.c

>> +++ b/cpus.c

>> @@ -1027,8 +1027,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)

>>  #endif /* _WIN32 */

>>

>>  static QemuMutex qemu_global_mutex;

>> -static QemuCond qemu_io_proceeded_cond;

>> -static unsigned iothread_requesting_mutex;

>>

>>  static QemuThread io_thread;

>>

>> @@ -1042,7 +1040,6 @@ void qemu_init_cpu_loop(void)

>>      qemu_init_sigbus();

>>      qemu_cond_init(&qemu_cpu_cond);

>>      qemu_cond_init(&qemu_pause_cond);

>> -    qemu_cond_init(&qemu_io_proceeded_cond);

>>      qemu_mutex_init(&qemu_global_mutex);

>>

>>      qemu_thread_get_self(&io_thread);

>> @@ -1085,10 +1082,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)

>>

>>      start_tcg_kick_timer();

>>

>> -    while (iothread_requesting_mutex) {

>> -        qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);

>> -    }

>> -

>>      CPU_FOREACH(cpu) {

>>          qemu_wait_io_event_common(cpu);

>>      }

>> @@ -1249,9 +1242,11 @@ static int tcg_cpu_exec(CPUState *cpu)

>>          cpu->icount_decr.u16.low = decr;

>>          cpu->icount_extra = count;

>>      }

>> +    qemu_mutex_unlock_iothread();

>>      cpu_exec_start(cpu);

>>      ret = cpu_exec(cpu);

>>      cpu_exec_end(cpu);

>> +    qemu_mutex_lock_iothread();

>>  #ifdef CONFIG_PROFILER

>>      tcg_time += profile_getclock() - ti;

>>  #endif

>> @@ -1479,27 +1474,14 @@ bool qemu_mutex_iothread_locked(void)

>>

>>  void qemu_mutex_lock_iothread(void)

>>  {

>> -    atomic_inc(&iothread_requesting_mutex);

>> -    /* In the simple case there is no need to bump the VCPU thread out of

>> -     * TCG code execution.

>> -     */

>> -    if (!tcg_enabled() || qemu_in_vcpu_thread() ||

>> -        !first_cpu || !first_cpu->created) {

>> -        qemu_mutex_lock(&qemu_global_mutex);

>> -        atomic_dec(&iothread_requesting_mutex);

>> -    } else {

>> -        if (qemu_mutex_trylock(&qemu_global_mutex)) {

>> -            qemu_cpu_kick_rr_cpu();

>> -            qemu_mutex_lock(&qemu_global_mutex);

>> -        }

>> -        atomic_dec(&iothread_requesting_mutex);

>> -        qemu_cond_broadcast(&qemu_io_proceeded_cond);

>> -    }

>> +    g_assert(!qemu_mutex_iothread_locked());

>> +    qemu_mutex_lock(&qemu_global_mutex);

>>      iothread_locked = true;

>>  }

>>

>>  void qemu_mutex_unlock_iothread(void)

>>  {

>> +    g_assert(qemu_mutex_iothread_locked());

>>      iothread_locked = false;

>>      qemu_mutex_unlock(&qemu_global_mutex);

>>  }

>> diff --git a/cputlb.c b/cputlb.c

>> index 6c39927455..1cc9d9da51 100644

>> --- a/cputlb.c

>> +++ b/cputlb.c

>> @@ -18,6 +18,7 @@

>>   */

>>

>>  #include "qemu/osdep.h"

>> +#include "qemu/main-loop.h"

>>  #include "cpu.h"

>>  #include "exec/exec-all.h"

>>  #include "exec/memory.h"

>> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>>      hwaddr physaddr = iotlbentry->addr;

>>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

>>      uint64_t val;

>> +    bool locked = false;

>>

>>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>>      cpu->mem_io_pc = retaddr;

>> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>>      }

>>

>>      cpu->mem_io_vaddr = addr;

>> +

>> +    if (mr->global_locking) {

>> +        qemu_mutex_lock_iothread();

>> +        locked = true;

>> +    }

>>      memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);

>> +    if (locked) {

>> +        qemu_mutex_unlock_iothread();

>> +    }

>> +

>>      return val;

>>  }

>>

>> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>>      CPUState *cpu = ENV_GET_CPU(env);

>>      hwaddr physaddr = iotlbentry->addr;

>>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

>> +    bool locked = false;

>>

>>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {

>>          cpu_io_recompile(cpu, retaddr);

>>      }

>> -

>>      cpu->mem_io_vaddr = addr;

>>      cpu->mem_io_pc = retaddr;

>> +

>> +    if (mr->global_locking) {

>> +        qemu_mutex_lock_iothread();

>> +        locked = true;

>> +    }

>>      memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);

>> +    if (locked) {

>> +        qemu_mutex_unlock_iothread();

>> +    }

>>  }

>>

>>  /* Return true if ADDR is present in the victim tlb, and has been copied

>> diff --git a/exec.c b/exec.c

>> index 865a1e8295..3adf2b1861 100644

>> --- a/exec.c

>> +++ b/exec.c

>> @@ -2134,9 +2134,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)

>>                  }

>>                  cpu->watchpoint_hit = wp;

>>

>> -                /* The tb_lock will be reset when cpu_loop_exit or

>> -                 * cpu_loop_exit_noexc longjmp back into the cpu_exec

>> -                 * main loop.

>> +                /* Both tb_lock and iothread_mutex will be reset when

>> +                 * cpu_loop_exit or cpu_loop_exit_noexc longjmp

>> +                 * back into the cpu_exec main loop.

>>                   */

>>                  tb_lock();

>>                  tb_check_watchpoint(cpu);

>> @@ -2371,8 +2371,14 @@ static void io_mem_init(void)

>>      memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);

>>      memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,

>>                            NULL, UINT64_MAX);

>> +

>> +    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,

>> +     * which can be called without the iothread mutex.

>> +     */

>>      memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,

>>                            NULL, UINT64_MAX);

>> +    memory_region_clear_global_locking(&io_mem_notdirty);

>> +

>>      memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,

>>                            NULL, UINT64_MAX);

>>  }

>> diff --git a/hw/core/irq.c b/hw/core/irq.c

>> index 49ff2e64fe..b98d1d69f5 100644

>> --- a/hw/core/irq.c

>> +++ b/hw/core/irq.c

>> @@ -22,6 +22,7 @@

>>   * THE SOFTWARE.

>>   */

>>  #include "qemu/osdep.h"

>> +#include "qemu/main-loop.h"

>>  #include "qemu-common.h"

>>  #include "hw/irq.h"

>>  #include "qom/object.h"

>> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c

>> index 7135633863..82a49556af 100644

>> --- a/hw/i386/kvmvapic.c

>> +++ b/hw/i386/kvmvapic.c

>> @@ -457,8 +457,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)

>>      resume_all_vcpus();

>>

>>      if (!kvm_enabled()) {

>> -        /* tb_lock will be reset when cpu_loop_exit_noexc longjmps

>> -         * back into the cpu_exec loop. */

>> +        /* Both tb_lock and iothread_mutex will be reset when

>> +         *  longjmps back into the cpu_exec loop. */

>>          tb_lock();

>>          tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);

>>          cpu_loop_exit_noexc(cs);

>> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c

>> index c25ee03556..f775aba507 100644

>> --- a/hw/intc/arm_gicv3_cpuif.c

>> +++ b/hw/intc/arm_gicv3_cpuif.c

>> @@ -14,6 +14,7 @@

>>

>>  #include "qemu/osdep.h"

>>  #include "qemu/bitops.h"

>> +#include "qemu/main-loop.h"

>>  #include "trace.h"

>>  #include "gicv3_internal.h"

>>  #include "cpu.h"

>> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)

>>      ARMCPU *cpu = ARM_CPU(cs->cpu);

>>      CPUARMState *env = &cpu->env;

>>

>> +    g_assert(qemu_mutex_iothread_locked());

>> +

>>      trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,

>>                               cs->hppi.grp, cs->hppi.prio);

>>

>> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c

>> index d171e60b5c..5f93083d4a 100644

>> --- a/hw/ppc/ppc.c

>> +++ b/hw/ppc/ppc.c

>> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>>  {

>>      CPUState *cs = CPU(cpu);

>>      CPUPPCState *env = &cpu->env;

>> -    unsigned int old_pending = env->pending_interrupts;

>> +    unsigned int old_pending;

>> +    bool locked = false;

>> +

>> +    /* We may already have the BQL if coming from the reset path */

>> +    if (!qemu_mutex_iothread_locked()) {

>> +        locked = true;

>> +        qemu_mutex_lock_iothread();

>> +    }

>> +

>> +    old_pending = env->pending_interrupts;

>>

>>      if (level) {

>>          env->pending_interrupts |= 1 << n_IRQ;

>> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>>  #endif

>>      }

>>

>> +

>>      LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32

>>                  "req %08x\n", __func__, env, n_IRQ, level,

>>                  env->pending_interrupts, CPU(cpu)->interrupt_request);

>> +

>> +    if (locked) {

>> +        qemu_mutex_unlock_iothread();

>> +    }

>>  }

>>

>>  /* PowerPC 6xx / 7xx internal IRQ controller */

>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

>> index e465d7ac98..b1e374f3f9 100644

>> --- a/hw/ppc/spapr.c

>> +++ b/hw/ppc/spapr.c

>> @@ -1010,6 +1010,9 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,

>>  {

>>      CPUPPCState *env = &cpu->env;

>>

>> +    /* The TCG path should also be holding the BQL at this point */

>> +    g_assert(qemu_mutex_iothread_locked());

>> +

>>      if (msr_pr) {

>>          hcall_dprintf("Hypercall made with MSR[PR]=1\n");

>>          env->gpr[3] = H_PRIVILEGE;

>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h

>> index 2cf4ecf144..10db89b16a 100644

>> --- a/include/qom/cpu.h

>> +++ b/include/qom/cpu.h

>> @@ -329,6 +329,7 @@ struct CPUState {

>>      bool unplug;

>>      bool crash_occurred;

>>      bool exit_request;

>> +    /* updates protected by BQL */

>>      uint32_t interrupt_request;

>>      int singlestep_enabled;

>>      int64_t icount_extra;

>> diff --git a/memory.c b/memory.c

>> index ed8b5aa83e..d61caee867 100644

>> --- a/memory.c

>> +++ b/memory.c

>> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)

>>      AddressSpace *as;

>>

>>      assert(memory_region_transaction_depth);

>> +    assert(qemu_mutex_iothread_locked());

>> +

>>      --memory_region_transaction_depth;

>>      if (!memory_region_transaction_depth) {

>>          if (memory_region_update_pending) {

>> diff --git a/qom/cpu.c b/qom/cpu.c

>> index ed87c50cea..58784bcbea 100644

>> --- a/qom/cpu.c

>> +++ b/qom/cpu.c

>> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,

>>      error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");

>>  }

>>

>> +/* Resetting the IRQ comes from across the code base so we take the

>> + * BQL here if we need to.  cpu_interrupt assumes it is held.*/

>>  void cpu_reset_interrupt(CPUState *cpu, int mask)

>>  {

>> +    bool need_lock = !qemu_mutex_iothread_locked();

>> +

>> +    if (need_lock) {

>> +        qemu_mutex_lock_iothread();

>> +    }

>>      cpu->interrupt_request &= ~mask;

>> +    if (need_lock) {

>> +        qemu_mutex_unlock_iothread();

>> +    }

>>  }

>>

>>  void cpu_exit(CPUState *cpu)

>> diff --git a/target/arm/helper.c b/target/arm/helper.c

>> index 47250bcf16..753a69d40d 100644

>> --- a/target/arm/helper.c

>> +++ b/target/arm/helper.c

>> @@ -6769,6 +6769,12 @@ void arm_cpu_do_interrupt(CPUState *cs)

>>          arm_cpu_do_interrupt_aarch32(cs);

>>      }

>>

>> +    /* Hooks may change global state so BQL should be held, also the

>> +     * BQL needs to be held for any modification of

>> +     * cs->interrupt_request.

>> +     */

>> +    g_assert(qemu_mutex_iothread_locked());

>> +

>>      arm_call_el_change_hook(cpu);

>>

>>      if (!kvm_enabled()) {

>> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c

>> index fb366fdc35..5f3e3bdae2 100644

>> --- a/target/arm/op_helper.c

>> +++ b/target/arm/op_helper.c

>> @@ -18,6 +18,7 @@

>>   */

>>  #include "qemu/osdep.h"

>>  #include "qemu/log.h"

>> +#include "qemu/main-loop.h"

>>  #include "cpu.h"

>>  #include "exec/helper-proto.h"

>>  #include "internals.h"

>> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)

>>       */

>>      env->regs[15] &= (env->thumb ? ~1 : ~3);

>>

>> +    qemu_mutex_lock_iothread();

>>      arm_call_el_change_hook(arm_env_get_cpu(env));

>> +    qemu_mutex_unlock_iothread();

>>  }

>>

>>  /* Access to user mode registers from privileged modes.  */

>> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)

>>  {

>>      const ARMCPRegInfo *ri = rip;

>>

>> -    ri->writefn(env, ri, value);

>> +    if (ri->type & ARM_CP_IO) {

>> +        qemu_mutex_lock_iothread();

>> +        ri->writefn(env, ri, value);

>> +        qemu_mutex_unlock_iothread();

>> +    } else {

>> +        ri->writefn(env, ri, value);

>> +    }

>>  }

>>

>>  uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)

>>  {

>>      const ARMCPRegInfo *ri = rip;

>> +    uint32_t res;

>>

>> -    return ri->readfn(env, ri);

>> +    if (ri->type & ARM_CP_IO) {

>> +        qemu_mutex_lock_iothread();

>> +        res = ri->readfn(env, ri);

>> +        qemu_mutex_unlock_iothread();

>> +    } else {

>> +        res = ri->readfn(env, ri);

>> +    }

>> +

>> +    return res;

>>  }

>>

>>  void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)

>>  {

>>      const ARMCPRegInfo *ri = rip;

>>

>> -    ri->writefn(env, ri, value);

>> +    if (ri->type & ARM_CP_IO) {

>> +        qemu_mutex_lock_iothread();

>> +        ri->writefn(env, ri, value);

>> +        qemu_mutex_unlock_iothread();

>> +    } else {

>> +        ri->writefn(env, ri, value);

>> +    }

>>  }

>>

>>  uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)

>>  {

>>      const ARMCPRegInfo *ri = rip;

>> +    uint64_t res;

>> +

>> +    if (ri->type & ARM_CP_IO) {

>> +        qemu_mutex_lock_iothread();

>> +        res = ri->readfn(env, ri);

>> +        qemu_mutex_unlock_iothread();

>> +    } else {

>> +        res = ri->readfn(env, ri);

>> +    }

>>

>> -    return ri->readfn(env, ri);

>> +    return res;

>>  }

>>

>>  void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)

>> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)

>>                        cur_el, new_el, env->pc);

>>      }

>>

>> +    qemu_mutex_lock_iothread();

>>      arm_call_el_change_hook(arm_env_get_cpu(env));

>> +    qemu_mutex_unlock_iothread();

>>

>>      return;

>>

>> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c

>> index 4dd6a2c544..f051a77c4a 100644

>> --- a/target/i386/smm_helper.c

>> +++ b/target/i386/smm_helper.c

>> @@ -18,6 +18,7 @@

>>   */

>>

>>  #include "qemu/osdep.h"

>> +#include "qemu/main-loop.h"

>>  #include "cpu.h"

>>  #include "exec/helper-proto.h"

>>  #include "exec/log.h"

>> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)

>>  #define SMM_REVISION_ID 0x00020000

>>  #endif

>>

>> +/* Called with iothread lock taken */

>>  void cpu_smm_update(X86CPU *cpu)

>>  {

>>      CPUX86State *env = &cpu->env;

>>      bool smm_enabled = (env->hflags & HF_SMM_MASK);

>>

>> +    g_assert(qemu_mutex_iothread_locked());

>> +

>>      if (cpu->smram) {

>>          memory_region_set_enabled(cpu->smram, smm_enabled);

>>      }

>> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)

>>      }

>>      env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;

>>      env->hflags &= ~HF_SMM_MASK;

>> +

>> +    qemu_mutex_lock_iothread();

>>      cpu_smm_update(cpu);

>> +    qemu_mutex_unlock_iothread();

>>

>>      qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");

>>      log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);

>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c

>> index c9604ea9c7..3cb942e8bb 100644

>> --- a/target/s390x/misc_helper.c

>> +++ b/target/s390x/misc_helper.c

>> @@ -25,6 +25,7 @@

>>  #include "exec/helper-proto.h"

>>  #include "sysemu/kvm.h"

>>  #include "qemu/timer.h"

>> +#include "qemu/main-loop.h"

>>  #include "exec/address-spaces.h"

>>  #ifdef CONFIG_KVM

>>  #include <linux/kvm.h>

>> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)

>>  /* SCLP service call */

>>  uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)

>>  {

>> +    qemu_mutex_lock_iothread();

>>      int r = sclp_service_call(env, r1, r2);

>>      if (r < 0) {

>>          program_interrupt(env, -r, 4);

>> -        return 0;

>> +        r = 0;

>>      }

>> +    qemu_mutex_unlock_iothread();

>>      return r;

>>  }

>>

>> diff --git a/translate-all.c b/translate-all.c

>> index 8a861cb583..f810259c41 100644

>> --- a/translate-all.c

>> +++ b/translate-all.c

>> @@ -55,6 +55,7 @@

>>  #include "translate-all.h"

>>  #include "qemu/bitmap.h"

>>  #include "qemu/timer.h"

>> +#include "qemu/main-loop.h"

>>  #include "exec/log.h"

>>

>>  /* #define DEBUG_TB_INVALIDATE */

>> @@ -1523,7 +1524,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,

>>  #ifdef CONFIG_SOFTMMU

>>  /* len must be <= 8 and start must be a multiple of len.

>>   * Called via softmmu_template.h when code areas are written to with

>> - * tb_lock held.

>> + * iothread mutex not held.

>>   */

>>  void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)

>>  {

>> @@ -1725,7 +1726,10 @@ void tb_check_watchpoint(CPUState *cpu)

>>

>>  #ifndef CONFIG_USER_ONLY

>>  /* in deterministic execution mode, instructions doing device I/Os

>> -   must be at the end of the TB */

>> + * must be at the end of the TB.

>> + *

>> + * Called by softmmu_template.h, with iothread mutex not held.

>> + */

>>  void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)

>>  {

>>  #if defined(TARGET_MIPS) || defined(TARGET_SH4)

>> @@ -1937,6 +1941,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)

>>

>>  void cpu_interrupt(CPUState *cpu, int mask)

>>  {

>> +    g_assert(qemu_mutex_iothread_locked());

>>      cpu->interrupt_request |= mask;

>>      cpu->tcg_exit_req = 1;

>>  }

>> diff --git a/translate-common.c b/translate-common.c

>> index 5e989cdf70..d504dd0d33 100644

>> --- a/translate-common.c

>> +++ b/translate-common.c

>> @@ -21,6 +21,7 @@

>>  #include "qemu-common.h"

>>  #include "qom/cpu.h"

>>  #include "sysemu/cpus.h"

>> +#include "qemu/main-loop.h"

>>

>>  uintptr_t qemu_real_host_page_size;

>>  intptr_t qemu_real_host_page_mask;

>> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;

>>  static void tcg_handle_interrupt(CPUState *cpu, int mask)

>>  {

>>      int old_mask;

>> +    g_assert(qemu_mutex_iothread_locked());

>>

>>      old_mask = cpu->interrupt_request;

>>      cpu->interrupt_request |= mask;

>> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)

>>       */

>>      if (!qemu_cpu_is_self(cpu)) {

>>          qemu_cpu_kick(cpu);

>> -        return;

>> -    }

>> -

>> -    if (use_icount) {

>> -        cpu->icount_decr.u16.high = 0xffff;

>> -        if (!cpu->can_do_io

>> -            && (mask & ~old_mask) != 0) {

>> -            cpu_abort(cpu, "Raised interrupt while not in I/O function");

>> -        }

>>      } else {

>> -        cpu->tcg_exit_req = 1;

>> +        if (use_icount) {

>> +            cpu->icount_decr.u16.high = 0xffff;

>> +            if (!cpu->can_do_io

>> +                && (mask & ~old_mask) != 0) {

>> +                cpu_abort(cpu, "Raised interrupt while not in I/O function");

>> +            }

>> +        } else {

>> +            cpu->tcg_exit_req = 1;

>> +        }

>>      }

>>  }

>>

>> --

>> 2.11.0

>>

>>



--
Alex Bennée

On Feb 27 14:39, Alex Bennée wrote:
> 

> Laurent Desnogues <laurent.desnogues@gmail.com> writes:

> 

> > Hello,

> >

> > On Fri, Feb 24, 2017 at 12:20 PM, Alex Bennée <alex.bennee@linaro.org> wrote:

> >> From: Jan Kiszka <jan.kiszka@siemens.com>

> >>

> >> This finally allows TCG to benefit from the iothread introduction: Drop

> >> the global mutex while running pure TCG CPU code. Reacquire the lock

> >> when entering MMIO or PIO emulation, or when leaving the TCG loop.

> >>

> >> We have to revert a few optimization for the current TCG threading

> >> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not

> >> kicking it in qemu_cpu_kick. We also need to disable RAM block

> >> reordering until we have a more efficient locking mechanism at hand.

> >>

> >> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.

> >> These numbers demonstrate where we gain something:

> >>

> >> 20338 jan       20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm

> >> 20337 jan       20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

> >>

> >> The guest CPU was fully loaded, but the iothread could still run mostly

> >> independent on a second core. Without the patch we don't get beyond

> >>

> >> 32206 jan       20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm

> >> 32204 jan       20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

> >>

> >> We don't benefit significantly, though, when the guest is not fully

> >> loading a host CPU.

> >

> > I tried this patch (8d04fb55 in the repository) with the following image:

> >

> >    http://wiki.qemu.org/download/arm-test-0.2.tar.gz

> >

> > Running the image with no option works fine.  But specifying '-icount

> > 1' results in a (guest) deadlock. Enabling some heavy logging (-d

> > in_asm,exec) sometimes results in a 'Bad ram offset'.

> >

> > Is it expected that this patch breaks -icount?

> 

> Not really. Using icount will disable MTTCG and run single threaded as

> before. Paolo reported another icount failure so they may be related. I

> shall have a look at it.

> 

> Thanks for the report.


I have not experienced a guest deadlock, but for me this patch makes
booting a simple Linux distribution take about an order of magnitude
longer when using '-icount 0' (from about 1.6 seconds to 17.9). It was
slow enough to get to the printk the first time after recompiling that I
killed it thinking it *had* deadlocked.

`perf report` from before this patch (snipped to >1%):
 23.81%  qemu-system-aar  perf-9267.map        [.] 0x0000000041a5cc9e
  7.15%  qemu-system-aar  [kernel.kallsyms]    [k] 0xffffffff8172bc82
  6.29%  qemu-system-aar  qemu-system-aarch64  [.] cpu_exec
  4.99%  qemu-system-aar  qemu-system-aarch64  [.] tcg_gen_code
  4.71%  qemu-system-aar  qemu-system-aarch64  [.] cpu_get_tb_cpu_state
  4.39%  qemu-system-aar  qemu-system-aarch64  [.] tcg_optimize
  3.28%  qemu-system-aar  qemu-system-aarch64  [.] helper_dc_zva
  2.66%  qemu-system-aar  qemu-system-aarch64  [.] liveness_pass_1
  1.98%  qemu-system-aar  qemu-system-aarch64  [.] qht_lookup
  1.93%  qemu-system-aar  qemu-system-aarch64  [.] tcg_out_opc
  1.81%  qemu-system-aar  qemu-system-aarch64  [.] get_phys_addr_lpae
  1.71%  qemu-system-aar  qemu-system-aarch64  [.] object_class_dynamic_cast_assert
  1.38%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi1
  1.10%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi0

and after this patch:
 20.10%  qemu-system-aar  perf-3285.map        [.] 0x0000000040a3b690
*18.08%  qemu-system-aar  [kernel.kallsyms]    [k] 0xffffffff81371865
  7.87%  qemu-system-aar  qemu-system-aarch64  [.] cpu_exec
  4.70%  qemu-system-aar  qemu-system-aarch64  [.] cpu_get_tb_cpu_state
* 2.64%  qemu-system-aar  qemu-system-aarch64  [.] g_mutex_get_impl
  2.39%  qemu-system-aar  qemu-system-aarch64  [.] gic_update
* 1.89%  qemu-system-aar  qemu-system-aarch64  [.] pthread_mutex_unlock
  1.61%  qemu-system-aar  qemu-system-aarch64  [.] object_class_dynamic_cast_assert
* 1.55%  qemu-system-aar  qemu-system-aarch64  [.] pthread_mutex_lock
  1.31%  qemu-system-aar  qemu-system-aarch64  [.] get_phys_addr_lpae
  1.21%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi0
  1.13%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi1

I've put asterisks by a few suspicious mutex-related functions, though I wonder
if the slowdowns are also partially inlined into some of the other functions.
The kernel also jumps up, presumably from handling more mutexes?

I confess I'm not familiar enough with this code to suggest optimizations, but
I'll be glad to test any.

-Aaron

> 

> >

> > Thanks,

> >

> > Laurent

> >

> > PS - To clarify 791158d9 works.

> >

> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

> >> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>

> >> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]

> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

> >> [EGC: fixed iothread lock for cpu-exec IRQ handling]

> >> Signed-off-by: Emilio G. Cota <cota@braap.org>

> >> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]

> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> >> Reviewed-by: Richard Henderson <rth@twiddle.net>

> >> Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

> >> [PM: target-arm changes]

> >> Acked-by: Peter Maydell <peter.maydell@linaro.org>

> >> ---

> >>  cpu-exec.c                 | 23 +++++++++++++++++++++--

> >>  cpus.c                     | 28 +++++-----------------------

> >>  cputlb.c                   | 21 ++++++++++++++++++++-

> >>  exec.c                     | 12 +++++++++---

> >>  hw/core/irq.c              |  1 +

> >>  hw/i386/kvmvapic.c         |  4 ++--

> >>  hw/intc/arm_gicv3_cpuif.c  |  3 +++

> >>  hw/ppc/ppc.c               | 16 +++++++++++++++-

> >>  hw/ppc/spapr.c             |  3 +++

> >>  include/qom/cpu.h          |  1 +

> >>  memory.c                   |  2 ++

> >>  qom/cpu.c                  | 10 ++++++++++

> >>  target/arm/helper.c        |  6 ++++++

> >>  target/arm/op_helper.c     | 43 +++++++++++++++++++++++++++++++++++++++----

> >>  target/i386/smm_helper.c   |  7 +++++++

> >>  target/s390x/misc_helper.c |  5 ++++-

> >>  translate-all.c            |  9 +++++++--

> >>  translate-common.c         | 21 +++++++++++----------

> >>  18 files changed, 166 insertions(+), 49 deletions(-)

> >>

> >> diff --git a/cpu-exec.c b/cpu-exec.c

> >> index 06a6b25564..1bd3d72002 100644

> >> --- a/cpu-exec.c

> >> +++ b/cpu-exec.c

> >> @@ -29,6 +29,7 @@

> >>  #include "qemu/rcu.h"

> >>  #include "exec/tb-hash.h"

> >>  #include "exec/log.h"

> >> +#include "qemu/main-loop.h"

> >>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)

> >>  #include "hw/i386/apic.h"

> >>  #endif

> >> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)

> >>          if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)

> >>              && replay_interrupt()) {

> >>              X86CPU *x86_cpu = X86_CPU(cpu);

> >> +            qemu_mutex_lock_iothread();

> >>              apic_poll_irq(x86_cpu->apic_state);

> >>              cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);

> >> +            qemu_mutex_unlock_iothread();

> >>          }

> >>  #endif

> >>          if (!cpu_has_work(cpu)) {

> >> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)

> >>  #else

> >>              if (replay_exception()) {

> >>                  CPUClass *cc = CPU_GET_CLASS(cpu);

> >> +                qemu_mutex_lock_iothread();

> >>                  cc->do_interrupt(cpu);

> >> +                qemu_mutex_unlock_iothread();

> >>                  cpu->exception_index = -1;

> >>              } else if (!replay_has_interrupt()) {

> >>                  /* give a chance to iothread in replay mode */

> >> @@ -469,9 +474,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

> >>                                          TranslationBlock **last_tb)

> >>  {

> >>      CPUClass *cc = CPU_GET_CLASS(cpu);

> >> -    int interrupt_request = cpu->interrupt_request;

> >>

> >> -    if (unlikely(interrupt_request)) {

> >> +    if (unlikely(atomic_read(&cpu->interrupt_request))) {

> >> +        int interrupt_request;

> >> +        qemu_mutex_lock_iothread();

> >> +        interrupt_request = cpu->interrupt_request;

> >>          if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {

> >>              /* Mask out external interrupts for this step. */

> >>              interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;

> >> @@ -479,6 +486,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

> >>          if (interrupt_request & CPU_INTERRUPT_DEBUG) {

> >>              cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;

> >>              cpu->exception_index = EXCP_DEBUG;

> >> +            qemu_mutex_unlock_iothread();

> >>              return true;

> >>          }

> >>          if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {

> >> @@ -488,6 +496,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

> >>              cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;

> >>              cpu->halted = 1;

> >>              cpu->exception_index = EXCP_HLT;

> >> +            qemu_mutex_unlock_iothread();

> >>              return true;

> >>          }

> >>  #if defined(TARGET_I386)

> >> @@ -498,12 +507,14 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

> >>              cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);

> >>              do_cpu_init(x86_cpu);

> >>              cpu->exception_index = EXCP_HALTED;

> >> +            qemu_mutex_unlock_iothread();

> >>              return true;

> >>          }

> >>  #else

> >>          else if (interrupt_request & CPU_INTERRUPT_RESET) {

> >>              replay_interrupt();

> >>              cpu_reset(cpu);

> >> +            qemu_mutex_unlock_iothread();

> >>              return true;

> >>          }

> >>  #endif

> >> @@ -526,7 +537,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

> >>                 the program flow was changed */

> >>              *last_tb = NULL;

> >>          }

> >> +

> >> +        /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */

> >> +        qemu_mutex_unlock_iothread();

> >>      }

> >> +

> >> +

> >>      if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {

> >>          atomic_set(&cpu->exit_request, 0);

> >>          cpu->exception_index = EXCP_INTERRUPT;

> >> @@ -643,6 +659,9 @@ int cpu_exec(CPUState *cpu)

> >>  #endif /* buggy compiler */

> >>          cpu->can_do_io = 1;

> >>          tb_lock_reset();

> >> +        if (qemu_mutex_iothread_locked()) {

> >> +            qemu_mutex_unlock_iothread();

> >> +        }

> >>      }

> >>

> >>      /* if an exception is pending, we execute it here */

> >> diff --git a/cpus.c b/cpus.c

> >> index 860034a794..0ae8f69be5 100644

> >> --- a/cpus.c

> >> +++ b/cpus.c

> >> @@ -1027,8 +1027,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)

> >>  #endif /* _WIN32 */

> >>

> >>  static QemuMutex qemu_global_mutex;

> >> -static QemuCond qemu_io_proceeded_cond;

> >> -static unsigned iothread_requesting_mutex;

> >>

> >>  static QemuThread io_thread;

> >>

> >> @@ -1042,7 +1040,6 @@ void qemu_init_cpu_loop(void)

> >>      qemu_init_sigbus();

> >>      qemu_cond_init(&qemu_cpu_cond);

> >>      qemu_cond_init(&qemu_pause_cond);

> >> -    qemu_cond_init(&qemu_io_proceeded_cond);

> >>      qemu_mutex_init(&qemu_global_mutex);

> >>

> >>      qemu_thread_get_self(&io_thread);

> >> @@ -1085,10 +1082,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)

> >>

> >>      start_tcg_kick_timer();

> >>

> >> -    while (iothread_requesting_mutex) {

> >> -        qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);

> >> -    }

> >> -

> >>      CPU_FOREACH(cpu) {

> >>          qemu_wait_io_event_common(cpu);

> >>      }

> >> @@ -1249,9 +1242,11 @@ static int tcg_cpu_exec(CPUState *cpu)

> >>          cpu->icount_decr.u16.low = decr;

> >>          cpu->icount_extra = count;

> >>      }

> >> +    qemu_mutex_unlock_iothread();

> >>      cpu_exec_start(cpu);

> >>      ret = cpu_exec(cpu);

> >>      cpu_exec_end(cpu);

> >> +    qemu_mutex_lock_iothread();

> >>  #ifdef CONFIG_PROFILER

> >>      tcg_time += profile_getclock() - ti;

> >>  #endif

> >> @@ -1479,27 +1474,14 @@ bool qemu_mutex_iothread_locked(void)

> >>

> >>  void qemu_mutex_lock_iothread(void)

> >>  {

> >> -    atomic_inc(&iothread_requesting_mutex);

> >> -    /* In the simple case there is no need to bump the VCPU thread out of

> >> -     * TCG code execution.

> >> -     */

> >> -    if (!tcg_enabled() || qemu_in_vcpu_thread() ||

> >> -        !first_cpu || !first_cpu->created) {

> >> -        qemu_mutex_lock(&qemu_global_mutex);

> >> -        atomic_dec(&iothread_requesting_mutex);

> >> -    } else {

> >> -        if (qemu_mutex_trylock(&qemu_global_mutex)) {

> >> -            qemu_cpu_kick_rr_cpu();

> >> -            qemu_mutex_lock(&qemu_global_mutex);

> >> -        }

> >> -        atomic_dec(&iothread_requesting_mutex);

> >> -        qemu_cond_broadcast(&qemu_io_proceeded_cond);

> >> -    }

> >> +    g_assert(!qemu_mutex_iothread_locked());

> >> +    qemu_mutex_lock(&qemu_global_mutex);

> >>      iothread_locked = true;

> >>  }

> >>

> >>  void qemu_mutex_unlock_iothread(void)

> >>  {

> >> +    g_assert(qemu_mutex_iothread_locked());

> >>      iothread_locked = false;

> >>      qemu_mutex_unlock(&qemu_global_mutex);

> >>  }

> >> diff --git a/cputlb.c b/cputlb.c

> >> index 6c39927455..1cc9d9da51 100644

> >> --- a/cputlb.c

> >> +++ b/cputlb.c

> >> @@ -18,6 +18,7 @@

> >>   */

> >>

> >>  #include "qemu/osdep.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "cpu.h"

> >>  #include "exec/exec-all.h"

> >>  #include "exec/memory.h"

> >> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

> >>      hwaddr physaddr = iotlbentry->addr;

> >>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

> >>      uint64_t val;

> >> +    bool locked = false;

> >>

> >>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

> >>      cpu->mem_io_pc = retaddr;

> >> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

> >>      }

> >>

> >>      cpu->mem_io_vaddr = addr;

> >> +

> >> +    if (mr->global_locking) {

> >> +        qemu_mutex_lock_iothread();

> >> +        locked = true;

> >> +    }

> >>      memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);

> >> +    if (locked) {

> >> +        qemu_mutex_unlock_iothread();

> >> +    }

> >> +

> >>      return val;

> >>  }

> >>

> >> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

> >>      CPUState *cpu = ENV_GET_CPU(env);

> >>      hwaddr physaddr = iotlbentry->addr;

> >>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

> >> +    bool locked = false;

> >>

> >>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

> >>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {

> >>          cpu_io_recompile(cpu, retaddr);

> >>      }

> >> -

> >>      cpu->mem_io_vaddr = addr;

> >>      cpu->mem_io_pc = retaddr;

> >> +

> >> +    if (mr->global_locking) {

> >> +        qemu_mutex_lock_iothread();

> >> +        locked = true;

> >> +    }

> >>      memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);

> >> +    if (locked) {

> >> +        qemu_mutex_unlock_iothread();

> >> +    }

> >>  }

> >>

> >>  /* Return true if ADDR is present in the victim tlb, and has been copied

> >> diff --git a/exec.c b/exec.c

> >> index 865a1e8295..3adf2b1861 100644

> >> --- a/exec.c

> >> +++ b/exec.c

> >> @@ -2134,9 +2134,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)

> >>                  }

> >>                  cpu->watchpoint_hit = wp;

> >>

> >> -                /* The tb_lock will be reset when cpu_loop_exit or

> >> -                 * cpu_loop_exit_noexc longjmp back into the cpu_exec

> >> -                 * main loop.

> >> +                /* Both tb_lock and iothread_mutex will be reset when

> >> +                 * cpu_loop_exit or cpu_loop_exit_noexc longjmp

> >> +                 * back into the cpu_exec main loop.

> >>                   */

> >>                  tb_lock();

> >>                  tb_check_watchpoint(cpu);

> >> @@ -2371,8 +2371,14 @@ static void io_mem_init(void)

> >>      memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);

> >>      memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,

> >>                            NULL, UINT64_MAX);

> >> +

> >> +    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,

> >> +     * which can be called without the iothread mutex.

> >> +     */

> >>      memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,

> >>                            NULL, UINT64_MAX);

> >> +    memory_region_clear_global_locking(&io_mem_notdirty);

> >> +

> >>      memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,

> >>                            NULL, UINT64_MAX);

> >>  }

> >> diff --git a/hw/core/irq.c b/hw/core/irq.c

> >> index 49ff2e64fe..b98d1d69f5 100644

> >> --- a/hw/core/irq.c

> >> +++ b/hw/core/irq.c

> >> @@ -22,6 +22,7 @@

> >>   * THE SOFTWARE.

> >>   */

> >>  #include "qemu/osdep.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "qemu-common.h"

> >>  #include "hw/irq.h"

> >>  #include "qom/object.h"

> >> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c

> >> index 7135633863..82a49556af 100644

> >> --- a/hw/i386/kvmvapic.c

> >> +++ b/hw/i386/kvmvapic.c

> >> @@ -457,8 +457,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)

> >>      resume_all_vcpus();

> >>

> >>      if (!kvm_enabled()) {

> >> -        /* tb_lock will be reset when cpu_loop_exit_noexc longjmps

> >> -         * back into the cpu_exec loop. */

> >> +        /* Both tb_lock and iothread_mutex will be reset when

> >> +         *  longjmps back into the cpu_exec loop. */

> >>          tb_lock();

> >>          tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);

> >>          cpu_loop_exit_noexc(cs);

> >> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c

> >> index c25ee03556..f775aba507 100644

> >> --- a/hw/intc/arm_gicv3_cpuif.c

> >> +++ b/hw/intc/arm_gicv3_cpuif.c

> >> @@ -14,6 +14,7 @@

> >>

> >>  #include "qemu/osdep.h"

> >>  #include "qemu/bitops.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "trace.h"

> >>  #include "gicv3_internal.h"

> >>  #include "cpu.h"

> >> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)

> >>      ARMCPU *cpu = ARM_CPU(cs->cpu);

> >>      CPUARMState *env = &cpu->env;

> >>

> >> +    g_assert(qemu_mutex_iothread_locked());

> >> +

> >>      trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,

> >>                               cs->hppi.grp, cs->hppi.prio);

> >>

> >> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c

> >> index d171e60b5c..5f93083d4a 100644

> >> --- a/hw/ppc/ppc.c

> >> +++ b/hw/ppc/ppc.c

> >> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

> >>  {

> >>      CPUState *cs = CPU(cpu);

> >>      CPUPPCState *env = &cpu->env;

> >> -    unsigned int old_pending = env->pending_interrupts;

> >> +    unsigned int old_pending;

> >> +    bool locked = false;

> >> +

> >> +    /* We may already have the BQL if coming from the reset path */

> >> +    if (!qemu_mutex_iothread_locked()) {

> >> +        locked = true;

> >> +        qemu_mutex_lock_iothread();

> >> +    }

> >> +

> >> +    old_pending = env->pending_interrupts;

> >>

> >>      if (level) {

> >>          env->pending_interrupts |= 1 << n_IRQ;

> >> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

> >>  #endif

> >>      }

> >>

> >> +

> >>      LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32

> >>                  "req %08x\n", __func__, env, n_IRQ, level,

> >>                  env->pending_interrupts, CPU(cpu)->interrupt_request);

> >> +

> >> +    if (locked) {

> >> +        qemu_mutex_unlock_iothread();

> >> +    }

> >>  }

> >>

> >>  /* PowerPC 6xx / 7xx internal IRQ controller */

> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

> >> index e465d7ac98..b1e374f3f9 100644

> >> --- a/hw/ppc/spapr.c

> >> +++ b/hw/ppc/spapr.c

> >> @@ -1010,6 +1010,9 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,

> >>  {

> >>      CPUPPCState *env = &cpu->env;

> >>

> >> +    /* The TCG path should also be holding the BQL at this point */

> >> +    g_assert(qemu_mutex_iothread_locked());

> >> +

> >>      if (msr_pr) {

> >>          hcall_dprintf("Hypercall made with MSR[PR]=1\n");

> >>          env->gpr[3] = H_PRIVILEGE;

> >> diff --git a/include/qom/cpu.h b/include/qom/cpu.h

> >> index 2cf4ecf144..10db89b16a 100644

> >> --- a/include/qom/cpu.h

> >> +++ b/include/qom/cpu.h

> >> @@ -329,6 +329,7 @@ struct CPUState {

> >>      bool unplug;

> >>      bool crash_occurred;

> >>      bool exit_request;

> >> +    /* updates protected by BQL */

> >>      uint32_t interrupt_request;

> >>      int singlestep_enabled;

> >>      int64_t icount_extra;

> >> diff --git a/memory.c b/memory.c

> >> index ed8b5aa83e..d61caee867 100644

> >> --- a/memory.c

> >> +++ b/memory.c

> >> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)

> >>      AddressSpace *as;

> >>

> >>      assert(memory_region_transaction_depth);

> >> +    assert(qemu_mutex_iothread_locked());

> >> +

> >>      --memory_region_transaction_depth;

> >>      if (!memory_region_transaction_depth) {

> >>          if (memory_region_update_pending) {

> >> diff --git a/qom/cpu.c b/qom/cpu.c

> >> index ed87c50cea..58784bcbea 100644

> >> --- a/qom/cpu.c

> >> +++ b/qom/cpu.c

> >> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,

> >>      error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");

> >>  }

> >>

> >> +/* Resetting the IRQ comes from across the code base so we take the

> >> + * BQL here if we need to.  cpu_interrupt assumes it is held.*/

> >>  void cpu_reset_interrupt(CPUState *cpu, int mask)

> >>  {

> >> +    bool need_lock = !qemu_mutex_iothread_locked();

> >> +

> >> +    if (need_lock) {

> >> +        qemu_mutex_lock_iothread();

> >> +    }

> >>      cpu->interrupt_request &= ~mask;

> >> +    if (need_lock) {

> >> +        qemu_mutex_unlock_iothread();

> >> +    }

> >>  }

> >>

> >>  void cpu_exit(CPUState *cpu)

> >> diff --git a/target/arm/helper.c b/target/arm/helper.c

> >> index 47250bcf16..753a69d40d 100644

> >> --- a/target/arm/helper.c

> >> +++ b/target/arm/helper.c

> >> @@ -6769,6 +6769,12 @@ void arm_cpu_do_interrupt(CPUState *cs)

> >>          arm_cpu_do_interrupt_aarch32(cs);

> >>      }

> >>

> >> +    /* Hooks may change global state so BQL should be held, also the

> >> +     * BQL needs to be held for any modification of

> >> +     * cs->interrupt_request.

> >> +     */

> >> +    g_assert(qemu_mutex_iothread_locked());

> >> +

> >>      arm_call_el_change_hook(cpu);

> >>

> >>      if (!kvm_enabled()) {

> >> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c

> >> index fb366fdc35..5f3e3bdae2 100644

> >> --- a/target/arm/op_helper.c

> >> +++ b/target/arm/op_helper.c

> >> @@ -18,6 +18,7 @@

> >>   */

> >>  #include "qemu/osdep.h"

> >>  #include "qemu/log.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "cpu.h"

> >>  #include "exec/helper-proto.h"

> >>  #include "internals.h"

> >> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)

> >>       */

> >>      env->regs[15] &= (env->thumb ? ~1 : ~3);

> >>

> >> +    qemu_mutex_lock_iothread();

> >>      arm_call_el_change_hook(arm_env_get_cpu(env));

> >> +    qemu_mutex_unlock_iothread();

> >>  }

> >>

> >>  /* Access to user mode registers from privileged modes.  */

> >> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)

> >>  {

> >>      const ARMCPRegInfo *ri = rip;

> >>

> >> -    ri->writefn(env, ri, value);

> >> +    if (ri->type & ARM_CP_IO) {

> >> +        qemu_mutex_lock_iothread();

> >> +        ri->writefn(env, ri, value);

> >> +        qemu_mutex_unlock_iothread();

> >> +    } else {

> >> +        ri->writefn(env, ri, value);

> >> +    }

> >>  }

> >>

> >>  uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)

> >>  {

> >>      const ARMCPRegInfo *ri = rip;

> >> +    uint32_t res;

> >>

> >> -    return ri->readfn(env, ri);

> >> +    if (ri->type & ARM_CP_IO) {

> >> +        qemu_mutex_lock_iothread();

> >> +        res = ri->readfn(env, ri);

> >> +        qemu_mutex_unlock_iothread();

> >> +    } else {

> >> +        res = ri->readfn(env, ri);

> >> +    }

> >> +

> >> +    return res;

> >>  }

> >>

> >>  void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)

> >>  {

> >>      const ARMCPRegInfo *ri = rip;

> >>

> >> -    ri->writefn(env, ri, value);

> >> +    if (ri->type & ARM_CP_IO) {

> >> +        qemu_mutex_lock_iothread();

> >> +        ri->writefn(env, ri, value);

> >> +        qemu_mutex_unlock_iothread();

> >> +    } else {

> >> +        ri->writefn(env, ri, value);

> >> +    }

> >>  }

> >>

> >>  uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)

> >>  {

> >>      const ARMCPRegInfo *ri = rip;

> >> +    uint64_t res;

> >> +

> >> +    if (ri->type & ARM_CP_IO) {

> >> +        qemu_mutex_lock_iothread();

> >> +        res = ri->readfn(env, ri);

> >> +        qemu_mutex_unlock_iothread();

> >> +    } else {

> >> +        res = ri->readfn(env, ri);

> >> +    }

> >>

> >> -    return ri->readfn(env, ri);

> >> +    return res;

> >>  }

> >>

> >>  void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)

> >> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)

> >>                        cur_el, new_el, env->pc);

> >>      }

> >>

> >> +    qemu_mutex_lock_iothread();

> >>      arm_call_el_change_hook(arm_env_get_cpu(env));

> >> +    qemu_mutex_unlock_iothread();

> >>

> >>      return;

> >>

> >> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c

> >> index 4dd6a2c544..f051a77c4a 100644

> >> --- a/target/i386/smm_helper.c

> >> +++ b/target/i386/smm_helper.c

> >> @@ -18,6 +18,7 @@

> >>   */

> >>

> >>  #include "qemu/osdep.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "cpu.h"

> >>  #include "exec/helper-proto.h"

> >>  #include "exec/log.h"

> >> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)

> >>  #define SMM_REVISION_ID 0x00020000

> >>  #endif

> >>

> >> +/* Called with iothread lock taken */

> >>  void cpu_smm_update(X86CPU *cpu)

> >>  {

> >>      CPUX86State *env = &cpu->env;

> >>      bool smm_enabled = (env->hflags & HF_SMM_MASK);

> >>

> >> +    g_assert(qemu_mutex_iothread_locked());

> >> +

> >>      if (cpu->smram) {

> >>          memory_region_set_enabled(cpu->smram, smm_enabled);

> >>      }

> >> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)

> >>      }

> >>      env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;

> >>      env->hflags &= ~HF_SMM_MASK;

> >> +

> >> +    qemu_mutex_lock_iothread();

> >>      cpu_smm_update(cpu);

> >> +    qemu_mutex_unlock_iothread();

> >>

> >>      qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");

> >>      log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);

> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c

> >> index c9604ea9c7..3cb942e8bb 100644

> >> --- a/target/s390x/misc_helper.c

> >> +++ b/target/s390x/misc_helper.c

> >> @@ -25,6 +25,7 @@

> >>  #include "exec/helper-proto.h"

> >>  #include "sysemu/kvm.h"

> >>  #include "qemu/timer.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "exec/address-spaces.h"

> >>  #ifdef CONFIG_KVM

> >>  #include <linux/kvm.h>

> >> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)

> >>  /* SCLP service call */

> >>  uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)

> >>  {

> >> +    qemu_mutex_lock_iothread();

> >>      int r = sclp_service_call(env, r1, r2);

> >>      if (r < 0) {

> >>          program_interrupt(env, -r, 4);

> >> -        return 0;

> >> +        r = 0;

> >>      }

> >> +    qemu_mutex_unlock_iothread();

> >>      return r;

> >>  }

> >>

> >> diff --git a/translate-all.c b/translate-all.c

> >> index 8a861cb583..f810259c41 100644

> >> --- a/translate-all.c

> >> +++ b/translate-all.c

> >> @@ -55,6 +55,7 @@

> >>  #include "translate-all.h"

> >>  #include "qemu/bitmap.h"

> >>  #include "qemu/timer.h"

> >> +#include "qemu/main-loop.h"

> >>  #include "exec/log.h"

> >>

> >>  /* #define DEBUG_TB_INVALIDATE */

> >> @@ -1523,7 +1524,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,

> >>  #ifdef CONFIG_SOFTMMU

> >>  /* len must be <= 8 and start must be a multiple of len.

> >>   * Called via softmmu_template.h when code areas are written to with

> >> - * tb_lock held.

> >> + * iothread mutex not held.

> >>   */

> >>  void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)

> >>  {

> >> @@ -1725,7 +1726,10 @@ void tb_check_watchpoint(CPUState *cpu)

> >>

> >>  #ifndef CONFIG_USER_ONLY

> >>  /* in deterministic execution mode, instructions doing device I/Os

> >> -   must be at the end of the TB */

> >> + * must be at the end of the TB.

> >> + *

> >> + * Called by softmmu_template.h, with iothread mutex not held.

> >> + */

> >>  void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)

> >>  {

> >>  #if defined(TARGET_MIPS) || defined(TARGET_SH4)

> >> @@ -1937,6 +1941,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)

> >>

> >>  void cpu_interrupt(CPUState *cpu, int mask)

> >>  {

> >> +    g_assert(qemu_mutex_iothread_locked());

> >>      cpu->interrupt_request |= mask;

> >>      cpu->tcg_exit_req = 1;

> >>  }

> >> diff --git a/translate-common.c b/translate-common.c

> >> index 5e989cdf70..d504dd0d33 100644

> >> --- a/translate-common.c

> >> +++ b/translate-common.c

> >> @@ -21,6 +21,7 @@

> >>  #include "qemu-common.h"

> >>  #include "qom/cpu.h"

> >>  #include "sysemu/cpus.h"

> >> +#include "qemu/main-loop.h"

> >>

> >>  uintptr_t qemu_real_host_page_size;

> >>  intptr_t qemu_real_host_page_mask;

> >> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;

> >>  static void tcg_handle_interrupt(CPUState *cpu, int mask)

> >>  {

> >>      int old_mask;

> >> +    g_assert(qemu_mutex_iothread_locked());

> >>

> >>      old_mask = cpu->interrupt_request;

> >>      cpu->interrupt_request |= mask;

> >> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)

> >>       */

> >>      if (!qemu_cpu_is_self(cpu)) {

> >>          qemu_cpu_kick(cpu);

> >> -        return;

> >> -    }

> >> -

> >> -    if (use_icount) {

> >> -        cpu->icount_decr.u16.high = 0xffff;

> >> -        if (!cpu->can_do_io

> >> -            && (mask & ~old_mask) != 0) {

> >> -            cpu_abort(cpu, "Raised interrupt while not in I/O function");

> >> -        }

> >>      } else {

> >> -        cpu->tcg_exit_req = 1;

> >> +        if (use_icount) {

> >> +            cpu->icount_decr.u16.high = 0xffff;

> >> +            if (!cpu->can_do_io

> >> +                && (mask & ~old_mask) != 0) {

> >> +                cpu_abort(cpu, "Raised interrupt while not in I/O function");

> >> +            }

> >> +        } else {

> >> +            cpu->tcg_exit_req = 1;

> >> +        }

> >>      }

> >>  }

> >>

> >> --

> >> 2.11.0

> >>

> >>

> 

> 

> --

> Alex Bennée

> 


-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Aaron Lindsay <alindsay@codeaurora.org> writes:

> On Feb 27 14:39, Alex Bennée wrote:

>>

>> Laurent Desnogues <laurent.desnogues@gmail.com> writes:

>>

>> > Hello,

>> >

>> > On Fri, Feb 24, 2017 at 12:20 PM, Alex Bennée <alex.bennee@linaro.org> wrote:

>> >> From: Jan Kiszka <jan.kiszka@siemens.com>

>> >>

>> >> This finally allows TCG to benefit from the iothread introduction: Drop

>> >> the global mutex while running pure TCG CPU code. Reacquire the lock

>> >> when entering MMIO or PIO emulation, or when leaving the TCG loop.

>> >>

>> >> We have to revert a few optimization for the current TCG threading

>> >> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not

>> >> kicking it in qemu_cpu_kick. We also need to disable RAM block

>> >> reordering until we have a more efficient locking mechanism at hand.

>> >>

>> >> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.

>> >> These numbers demonstrate where we gain something:

>> >>

>> >> 20338 jan       20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm

>> >> 20337 jan       20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

>> >>

>> >> The guest CPU was fully loaded, but the iothread could still run mostly

>> >> independent on a second core. Without the patch we don't get beyond

>> >>

>> >> 32206 jan       20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm

>> >> 32204 jan       20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

>> >>

>> >> We don't benefit significantly, though, when the guest is not fully

>> >> loading a host CPU.

>> >

>> > I tried this patch (8d04fb55 in the repository) with the following image:

>> >

>> >    http://wiki.qemu.org/download/arm-test-0.2.tar.gz

>> >

>> > Running the image with no option works fine.  But specifying '-icount

>> > 1' results in a (guest) deadlock. Enabling some heavy logging (-d

>> > in_asm,exec) sometimes results in a 'Bad ram offset'.

>> >

>> > Is it expected that this patch breaks -icount?

>>

>> Not really. Using icount will disable MTTCG and run single threaded as

>> before. Paolo reported another icount failure so they may be related. I

>> shall have a look at it.

>>

>> Thanks for the report.

>

> I have not experienced a guest deadlock, but for me this patch makes

> booting a simple Linux distribution take about an order of magnitude

> longer when using '-icount 0' (from about 1.6 seconds to 17.9). It was

> slow enough to get to the printk the first time after recompiling that I

> killed it thinking it *had* deadlocked.

>

> `perf report` from before this patch (snipped to >1%):

>  23.81%  qemu-system-aar  perf-9267.map        [.] 0x0000000041a5cc9e

>   7.15%  qemu-system-aar  [kernel.kallsyms]    [k] 0xffffffff8172bc82

>   6.29%  qemu-system-aar  qemu-system-aarch64  [.] cpu_exec

>   4.99%  qemu-system-aar  qemu-system-aarch64  [.] tcg_gen_code

>   4.71%  qemu-system-aar  qemu-system-aarch64  [.] cpu_get_tb_cpu_state

>   4.39%  qemu-system-aar  qemu-system-aarch64  [.] tcg_optimize

>   3.28%  qemu-system-aar  qemu-system-aarch64  [.] helper_dc_zva

>   2.66%  qemu-system-aar  qemu-system-aarch64  [.] liveness_pass_1

>   1.98%  qemu-system-aar  qemu-system-aarch64  [.] qht_lookup

>   1.93%  qemu-system-aar  qemu-system-aarch64  [.] tcg_out_opc

>   1.81%  qemu-system-aar  qemu-system-aarch64  [.] get_phys_addr_lpae

>   1.71%  qemu-system-aar  qemu-system-aarch64  [.] object_class_dynamic_cast_assert

>   1.38%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi1

>   1.10%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi0

>

> and after this patch:

>  20.10%  qemu-system-aar  perf-3285.map        [.] 0x0000000040a3b690

> *18.08%  qemu-system-aar  [kernel.kallsyms]    [k] 0xffffffff81371865

>   7.87%  qemu-system-aar  qemu-system-aarch64  [.] cpu_exec

>   4.70%  qemu-system-aar  qemu-system-aarch64  [.] cpu_get_tb_cpu_state

> * 2.64%  qemu-system-aar  qemu-system-aarch64  [.] g_mutex_get_impl

>   2.39%  qemu-system-aar  qemu-system-aarch64  [.] gic_update

> * 1.89%  qemu-system-aar  qemu-system-aarch64  [.] pthread_mutex_unlock

>   1.61%  qemu-system-aar  qemu-system-aarch64  [.] object_class_dynamic_cast_assert

> * 1.55%  qemu-system-aar  qemu-system-aarch64  [.] pthread_mutex_lock

>   1.31%  qemu-system-aar  qemu-system-aarch64  [.] get_phys_addr_lpae

>   1.21%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi0

>   1.13%  qemu-system-aar  qemu-system-aarch64  [.] arm_regime_tbi1

>

> I've put asterisks by a few suspicious mutex-related functions, though I wonder

> if the slowdowns are also partially inlined into some of the other functions.

> The kernel also jumps up, presumably from handling more mutexes?

>

> I confess I'm not familiar enough with this code to suggest optimizations, but

> I'll be glad to test any.


Please see the series Paolo posted:

  Subject: [PATCH 0/6] tcg: fix icount super slowdown
  Date: Fri,  3 Mar 2017 14:11:08 +0100
  Message-Id: <20170303131113.25898-1-pbonzini@redhat.com>

>

> -Aaron

>

>>

>> >

>> > Thanks,

>> >

>> > Laurent

>> >

>> > PS - To clarify 791158d9 works.

>> >

>> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

>> >> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>

>> >> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]

>> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

>> >> [EGC: fixed iothread lock for cpu-exec IRQ handling]

>> >> Signed-off-by: Emilio G. Cota <cota@braap.org>

>> >> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]

>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> >> Reviewed-by: Richard Henderson <rth@twiddle.net>

>> >> Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

>> >> [PM: target-arm changes]

>> >> Acked-by: Peter Maydell <peter.maydell@linaro.org>

>> >> ---

>> >>  cpu-exec.c                 | 23 +++++++++++++++++++++--

>> >>  cpus.c                     | 28 +++++-----------------------

>> >>  cputlb.c                   | 21 ++++++++++++++++++++-

>> >>  exec.c                     | 12 +++++++++---

>> >>  hw/core/irq.c              |  1 +

>> >>  hw/i386/kvmvapic.c         |  4 ++--

>> >>  hw/intc/arm_gicv3_cpuif.c  |  3 +++

>> >>  hw/ppc/ppc.c               | 16 +++++++++++++++-

>> >>  hw/ppc/spapr.c             |  3 +++

>> >>  include/qom/cpu.h          |  1 +

>> >>  memory.c                   |  2 ++

>> >>  qom/cpu.c                  | 10 ++++++++++

>> >>  target/arm/helper.c        |  6 ++++++

>> >>  target/arm/op_helper.c     | 43 +++++++++++++++++++++++++++++++++++++++----

>> >>  target/i386/smm_helper.c   |  7 +++++++

>> >>  target/s390x/misc_helper.c |  5 ++++-

>> >>  translate-all.c            |  9 +++++++--

>> >>  translate-common.c         | 21 +++++++++++----------

>> >>  18 files changed, 166 insertions(+), 49 deletions(-)

>> >>

>> >> diff --git a/cpu-exec.c b/cpu-exec.c

>> >> index 06a6b25564..1bd3d72002 100644

>> >> --- a/cpu-exec.c

>> >> +++ b/cpu-exec.c

>> >> @@ -29,6 +29,7 @@

>> >>  #include "qemu/rcu.h"

>> >>  #include "exec/tb-hash.h"

>> >>  #include "exec/log.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)

>> >>  #include "hw/i386/apic.h"

>> >>  #endif

>> >> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)

>> >>          if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)

>> >>              && replay_interrupt()) {

>> >>              X86CPU *x86_cpu = X86_CPU(cpu);

>> >> +            qemu_mutex_lock_iothread();

>> >>              apic_poll_irq(x86_cpu->apic_state);

>> >>              cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);

>> >> +            qemu_mutex_unlock_iothread();

>> >>          }

>> >>  #endif

>> >>          if (!cpu_has_work(cpu)) {

>> >> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)

>> >>  #else

>> >>              if (replay_exception()) {

>> >>                  CPUClass *cc = CPU_GET_CLASS(cpu);

>> >> +                qemu_mutex_lock_iothread();

>> >>                  cc->do_interrupt(cpu);

>> >> +                qemu_mutex_unlock_iothread();

>> >>                  cpu->exception_index = -1;

>> >>              } else if (!replay_has_interrupt()) {

>> >>                  /* give a chance to iothread in replay mode */

>> >> @@ -469,9 +474,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>> >>                                          TranslationBlock **last_tb)

>> >>  {

>> >>      CPUClass *cc = CPU_GET_CLASS(cpu);

>> >> -    int interrupt_request = cpu->interrupt_request;

>> >>

>> >> -    if (unlikely(interrupt_request)) {

>> >> +    if (unlikely(atomic_read(&cpu->interrupt_request))) {

>> >> +        int interrupt_request;

>> >> +        qemu_mutex_lock_iothread();

>> >> +        interrupt_request = cpu->interrupt_request;

>> >>          if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {

>> >>              /* Mask out external interrupts for this step. */

>> >>              interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;

>> >> @@ -479,6 +486,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>> >>          if (interrupt_request & CPU_INTERRUPT_DEBUG) {

>> >>              cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;

>> >>              cpu->exception_index = EXCP_DEBUG;

>> >> +            qemu_mutex_unlock_iothread();

>> >>              return true;

>> >>          }

>> >>          if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {

>> >> @@ -488,6 +496,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>> >>              cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;

>> >>              cpu->halted = 1;

>> >>              cpu->exception_index = EXCP_HLT;

>> >> +            qemu_mutex_unlock_iothread();

>> >>              return true;

>> >>          }

>> >>  #if defined(TARGET_I386)

>> >> @@ -498,12 +507,14 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>> >>              cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);

>> >>              do_cpu_init(x86_cpu);

>> >>              cpu->exception_index = EXCP_HALTED;

>> >> +            qemu_mutex_unlock_iothread();

>> >>              return true;

>> >>          }

>> >>  #else

>> >>          else if (interrupt_request & CPU_INTERRUPT_RESET) {

>> >>              replay_interrupt();

>> >>              cpu_reset(cpu);

>> >> +            qemu_mutex_unlock_iothread();

>> >>              return true;

>> >>          }

>> >>  #endif

>> >> @@ -526,7 +537,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,

>> >>                 the program flow was changed */

>> >>              *last_tb = NULL;

>> >>          }

>> >> +

>> >> +        /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */

>> >> +        qemu_mutex_unlock_iothread();

>> >>      }

>> >> +

>> >> +

>> >>      if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {

>> >>          atomic_set(&cpu->exit_request, 0);

>> >>          cpu->exception_index = EXCP_INTERRUPT;

>> >> @@ -643,6 +659,9 @@ int cpu_exec(CPUState *cpu)

>> >>  #endif /* buggy compiler */

>> >>          cpu->can_do_io = 1;

>> >>          tb_lock_reset();

>> >> +        if (qemu_mutex_iothread_locked()) {

>> >> +            qemu_mutex_unlock_iothread();

>> >> +        }

>> >>      }

>> >>

>> >>      /* if an exception is pending, we execute it here */

>> >> diff --git a/cpus.c b/cpus.c

>> >> index 860034a794..0ae8f69be5 100644

>> >> --- a/cpus.c

>> >> +++ b/cpus.c

>> >> @@ -1027,8 +1027,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)

>> >>  #endif /* _WIN32 */

>> >>

>> >>  static QemuMutex qemu_global_mutex;

>> >> -static QemuCond qemu_io_proceeded_cond;

>> >> -static unsigned iothread_requesting_mutex;

>> >>

>> >>  static QemuThread io_thread;

>> >>

>> >> @@ -1042,7 +1040,6 @@ void qemu_init_cpu_loop(void)

>> >>      qemu_init_sigbus();

>> >>      qemu_cond_init(&qemu_cpu_cond);

>> >>      qemu_cond_init(&qemu_pause_cond);

>> >> -    qemu_cond_init(&qemu_io_proceeded_cond);

>> >>      qemu_mutex_init(&qemu_global_mutex);

>> >>

>> >>      qemu_thread_get_self(&io_thread);

>> >> @@ -1085,10 +1082,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)

>> >>

>> >>      start_tcg_kick_timer();

>> >>

>> >> -    while (iothread_requesting_mutex) {

>> >> -        qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);

>> >> -    }

>> >> -

>> >>      CPU_FOREACH(cpu) {

>> >>          qemu_wait_io_event_common(cpu);

>> >>      }

>> >> @@ -1249,9 +1242,11 @@ static int tcg_cpu_exec(CPUState *cpu)

>> >>          cpu->icount_decr.u16.low = decr;

>> >>          cpu->icount_extra = count;

>> >>      }

>> >> +    qemu_mutex_unlock_iothread();

>> >>      cpu_exec_start(cpu);

>> >>      ret = cpu_exec(cpu);

>> >>      cpu_exec_end(cpu);

>> >> +    qemu_mutex_lock_iothread();

>> >>  #ifdef CONFIG_PROFILER

>> >>      tcg_time += profile_getclock() - ti;

>> >>  #endif

>> >> @@ -1479,27 +1474,14 @@ bool qemu_mutex_iothread_locked(void)

>> >>

>> >>  void qemu_mutex_lock_iothread(void)

>> >>  {

>> >> -    atomic_inc(&iothread_requesting_mutex);

>> >> -    /* In the simple case there is no need to bump the VCPU thread out of

>> >> -     * TCG code execution.

>> >> -     */

>> >> -    if (!tcg_enabled() || qemu_in_vcpu_thread() ||

>> >> -        !first_cpu || !first_cpu->created) {

>> >> -        qemu_mutex_lock(&qemu_global_mutex);

>> >> -        atomic_dec(&iothread_requesting_mutex);

>> >> -    } else {

>> >> -        if (qemu_mutex_trylock(&qemu_global_mutex)) {

>> >> -            qemu_cpu_kick_rr_cpu();

>> >> -            qemu_mutex_lock(&qemu_global_mutex);

>> >> -        }

>> >> -        atomic_dec(&iothread_requesting_mutex);

>> >> -        qemu_cond_broadcast(&qemu_io_proceeded_cond);

>> >> -    }

>> >> +    g_assert(!qemu_mutex_iothread_locked());

>> >> +    qemu_mutex_lock(&qemu_global_mutex);

>> >>      iothread_locked = true;

>> >>  }

>> >>

>> >>  void qemu_mutex_unlock_iothread(void)

>> >>  {

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >>      iothread_locked = false;

>> >>      qemu_mutex_unlock(&qemu_global_mutex);

>> >>  }

>> >> diff --git a/cputlb.c b/cputlb.c

>> >> index 6c39927455..1cc9d9da51 100644

>> >> --- a/cputlb.c

>> >> +++ b/cputlb.c

>> >> @@ -18,6 +18,7 @@

>> >>   */

>> >>

>> >>  #include "qemu/osdep.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "cpu.h"

>> >>  #include "exec/exec-all.h"

>> >>  #include "exec/memory.h"

>> >> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>> >>      hwaddr physaddr = iotlbentry->addr;

>> >>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

>> >>      uint64_t val;

>> >> +    bool locked = false;

>> >>

>> >>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>> >>      cpu->mem_io_pc = retaddr;

>> >> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>> >>      }

>> >>

>> >>      cpu->mem_io_vaddr = addr;

>> >> +

>> >> +    if (mr->global_locking) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        locked = true;

>> >> +    }

>> >>      memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);

>> >> +    if (locked) {

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    }

>> >> +

>> >>      return val;

>> >>  }

>> >>

>> >> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,

>> >>      CPUState *cpu = ENV_GET_CPU(env);

>> >>      hwaddr physaddr = iotlbentry->addr;

>> >>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);

>> >> +    bool locked = false;

>> >>

>> >>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;

>> >>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {

>> >>          cpu_io_recompile(cpu, retaddr);

>> >>      }

>> >> -

>> >>      cpu->mem_io_vaddr = addr;

>> >>      cpu->mem_io_pc = retaddr;

>> >> +

>> >> +    if (mr->global_locking) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        locked = true;

>> >> +    }

>> >>      memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);

>> >> +    if (locked) {

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    }

>> >>  }

>> >>

>> >>  /* Return true if ADDR is present in the victim tlb, and has been copied

>> >> diff --git a/exec.c b/exec.c

>> >> index 865a1e8295..3adf2b1861 100644

>> >> --- a/exec.c

>> >> +++ b/exec.c

>> >> @@ -2134,9 +2134,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)

>> >>                  }

>> >>                  cpu->watchpoint_hit = wp;

>> >>

>> >> -                /* The tb_lock will be reset when cpu_loop_exit or

>> >> -                 * cpu_loop_exit_noexc longjmp back into the cpu_exec

>> >> -                 * main loop.

>> >> +                /* Both tb_lock and iothread_mutex will be reset when

>> >> +                 * cpu_loop_exit or cpu_loop_exit_noexc longjmp

>> >> +                 * back into the cpu_exec main loop.

>> >>                   */

>> >>                  tb_lock();

>> >>                  tb_check_watchpoint(cpu);

>> >> @@ -2371,8 +2371,14 @@ static void io_mem_init(void)

>> >>      memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);

>> >>      memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,

>> >>                            NULL, UINT64_MAX);

>> >> +

>> >> +    /* io_mem_notdirty calls tb_invalidate_phys_page_fast,

>> >> +     * which can be called without the iothread mutex.

>> >> +     */

>> >>      memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,

>> >>                            NULL, UINT64_MAX);

>> >> +    memory_region_clear_global_locking(&io_mem_notdirty);

>> >> +

>> >>      memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,

>> >>                            NULL, UINT64_MAX);

>> >>  }

>> >> diff --git a/hw/core/irq.c b/hw/core/irq.c

>> >> index 49ff2e64fe..b98d1d69f5 100644

>> >> --- a/hw/core/irq.c

>> >> +++ b/hw/core/irq.c

>> >> @@ -22,6 +22,7 @@

>> >>   * THE SOFTWARE.

>> >>   */

>> >>  #include "qemu/osdep.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "qemu-common.h"

>> >>  #include "hw/irq.h"

>> >>  #include "qom/object.h"

>> >> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c

>> >> index 7135633863..82a49556af 100644

>> >> --- a/hw/i386/kvmvapic.c

>> >> +++ b/hw/i386/kvmvapic.c

>> >> @@ -457,8 +457,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)

>> >>      resume_all_vcpus();

>> >>

>> >>      if (!kvm_enabled()) {

>> >> -        /* tb_lock will be reset when cpu_loop_exit_noexc longjmps

>> >> -         * back into the cpu_exec loop. */

>> >> +        /* Both tb_lock and iothread_mutex will be reset when

>> >> +         *  longjmps back into the cpu_exec loop. */

>> >>          tb_lock();

>> >>          tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);

>> >>          cpu_loop_exit_noexc(cs);

>> >> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c

>> >> index c25ee03556..f775aba507 100644

>> >> --- a/hw/intc/arm_gicv3_cpuif.c

>> >> +++ b/hw/intc/arm_gicv3_cpuif.c

>> >> @@ -14,6 +14,7 @@

>> >>

>> >>  #include "qemu/osdep.h"

>> >>  #include "qemu/bitops.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "trace.h"

>> >>  #include "gicv3_internal.h"

>> >>  #include "cpu.h"

>> >> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)

>> >>      ARMCPU *cpu = ARM_CPU(cs->cpu);

>> >>      CPUARMState *env = &cpu->env;

>> >>

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >> +

>> >>      trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,

>> >>                               cs->hppi.grp, cs->hppi.prio);

>> >>

>> >> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c

>> >> index d171e60b5c..5f93083d4a 100644

>> >> --- a/hw/ppc/ppc.c

>> >> +++ b/hw/ppc/ppc.c

>> >> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>> >>  {

>> >>      CPUState *cs = CPU(cpu);

>> >>      CPUPPCState *env = &cpu->env;

>> >> -    unsigned int old_pending = env->pending_interrupts;

>> >> +    unsigned int old_pending;

>> >> +    bool locked = false;

>> >> +

>> >> +    /* We may already have the BQL if coming from the reset path */

>> >> +    if (!qemu_mutex_iothread_locked()) {

>> >> +        locked = true;

>> >> +        qemu_mutex_lock_iothread();

>> >> +    }

>> >> +

>> >> +    old_pending = env->pending_interrupts;

>> >>

>> >>      if (level) {

>> >>          env->pending_interrupts |= 1 << n_IRQ;

>> >> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)

>> >>  #endif

>> >>      }

>> >>

>> >> +

>> >>      LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32

>> >>                  "req %08x\n", __func__, env, n_IRQ, level,

>> >>                  env->pending_interrupts, CPU(cpu)->interrupt_request);

>> >> +

>> >> +    if (locked) {

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    }

>> >>  }

>> >>

>> >>  /* PowerPC 6xx / 7xx internal IRQ controller */

>> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

>> >> index e465d7ac98..b1e374f3f9 100644

>> >> --- a/hw/ppc/spapr.c

>> >> +++ b/hw/ppc/spapr.c

>> >> @@ -1010,6 +1010,9 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,

>> >>  {

>> >>      CPUPPCState *env = &cpu->env;

>> >>

>> >> +    /* The TCG path should also be holding the BQL at this point */

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >> +

>> >>      if (msr_pr) {

>> >>          hcall_dprintf("Hypercall made with MSR[PR]=1\n");

>> >>          env->gpr[3] = H_PRIVILEGE;

>> >> diff --git a/include/qom/cpu.h b/include/qom/cpu.h

>> >> index 2cf4ecf144..10db89b16a 100644

>> >> --- a/include/qom/cpu.h

>> >> +++ b/include/qom/cpu.h

>> >> @@ -329,6 +329,7 @@ struct CPUState {

>> >>      bool unplug;

>> >>      bool crash_occurred;

>> >>      bool exit_request;

>> >> +    /* updates protected by BQL */

>> >>      uint32_t interrupt_request;

>> >>      int singlestep_enabled;

>> >>      int64_t icount_extra;

>> >> diff --git a/memory.c b/memory.c

>> >> index ed8b5aa83e..d61caee867 100644

>> >> --- a/memory.c

>> >> +++ b/memory.c

>> >> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)

>> >>      AddressSpace *as;

>> >>

>> >>      assert(memory_region_transaction_depth);

>> >> +    assert(qemu_mutex_iothread_locked());

>> >> +

>> >>      --memory_region_transaction_depth;

>> >>      if (!memory_region_transaction_depth) {

>> >>          if (memory_region_update_pending) {

>> >> diff --git a/qom/cpu.c b/qom/cpu.c

>> >> index ed87c50cea..58784bcbea 100644

>> >> --- a/qom/cpu.c

>> >> +++ b/qom/cpu.c

>> >> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,

>> >>      error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");

>> >>  }

>> >>

>> >> +/* Resetting the IRQ comes from across the code base so we take the

>> >> + * BQL here if we need to.  cpu_interrupt assumes it is held.*/

>> >>  void cpu_reset_interrupt(CPUState *cpu, int mask)

>> >>  {

>> >> +    bool need_lock = !qemu_mutex_iothread_locked();

>> >> +

>> >> +    if (need_lock) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +    }

>> >>      cpu->interrupt_request &= ~mask;

>> >> +    if (need_lock) {

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    }

>> >>  }

>> >>

>> >>  void cpu_exit(CPUState *cpu)

>> >> diff --git a/target/arm/helper.c b/target/arm/helper.c

>> >> index 47250bcf16..753a69d40d 100644

>> >> --- a/target/arm/helper.c

>> >> +++ b/target/arm/helper.c

>> >> @@ -6769,6 +6769,12 @@ void arm_cpu_do_interrupt(CPUState *cs)

>> >>          arm_cpu_do_interrupt_aarch32(cs);

>> >>      }

>> >>

>> >> +    /* Hooks may change global state so BQL should be held, also the

>> >> +     * BQL needs to be held for any modification of

>> >> +     * cs->interrupt_request.

>> >> +     */

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >> +

>> >>      arm_call_el_change_hook(cpu);

>> >>

>> >>      if (!kvm_enabled()) {

>> >> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c

>> >> index fb366fdc35..5f3e3bdae2 100644

>> >> --- a/target/arm/op_helper.c

>> >> +++ b/target/arm/op_helper.c

>> >> @@ -18,6 +18,7 @@

>> >>   */

>> >>  #include "qemu/osdep.h"

>> >>  #include "qemu/log.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "cpu.h"

>> >>  #include "exec/helper-proto.h"

>> >>  #include "internals.h"

>> >> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)

>> >>       */

>> >>      env->regs[15] &= (env->thumb ? ~1 : ~3);

>> >>

>> >> +    qemu_mutex_lock_iothread();

>> >>      arm_call_el_change_hook(arm_env_get_cpu(env));

>> >> +    qemu_mutex_unlock_iothread();

>> >>  }

>> >>

>> >>  /* Access to user mode registers from privileged modes.  */

>> >> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)

>> >>  {

>> >>      const ARMCPRegInfo *ri = rip;

>> >>

>> >> -    ri->writefn(env, ri, value);

>> >> +    if (ri->type & ARM_CP_IO) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        ri->writefn(env, ri, value);

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    } else {

>> >> +        ri->writefn(env, ri, value);

>> >> +    }

>> >>  }

>> >>

>> >>  uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)

>> >>  {

>> >>      const ARMCPRegInfo *ri = rip;

>> >> +    uint32_t res;

>> >>

>> >> -    return ri->readfn(env, ri);

>> >> +    if (ri->type & ARM_CP_IO) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        res = ri->readfn(env, ri);

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    } else {

>> >> +        res = ri->readfn(env, ri);

>> >> +    }

>> >> +

>> >> +    return res;

>> >>  }

>> >>

>> >>  void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)

>> >>  {

>> >>      const ARMCPRegInfo *ri = rip;

>> >>

>> >> -    ri->writefn(env, ri, value);

>> >> +    if (ri->type & ARM_CP_IO) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        ri->writefn(env, ri, value);

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    } else {

>> >> +        ri->writefn(env, ri, value);

>> >> +    }

>> >>  }

>> >>

>> >>  uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)

>> >>  {

>> >>      const ARMCPRegInfo *ri = rip;

>> >> +    uint64_t res;

>> >> +

>> >> +    if (ri->type & ARM_CP_IO) {

>> >> +        qemu_mutex_lock_iothread();

>> >> +        res = ri->readfn(env, ri);

>> >> +        qemu_mutex_unlock_iothread();

>> >> +    } else {

>> >> +        res = ri->readfn(env, ri);

>> >> +    }

>> >>

>> >> -    return ri->readfn(env, ri);

>> >> +    return res;

>> >>  }

>> >>

>> >>  void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)

>> >> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)

>> >>                        cur_el, new_el, env->pc);

>> >>      }

>> >>

>> >> +    qemu_mutex_lock_iothread();

>> >>      arm_call_el_change_hook(arm_env_get_cpu(env));

>> >> +    qemu_mutex_unlock_iothread();

>> >>

>> >>      return;

>> >>

>> >> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c

>> >> index 4dd6a2c544..f051a77c4a 100644

>> >> --- a/target/i386/smm_helper.c

>> >> +++ b/target/i386/smm_helper.c

>> >> @@ -18,6 +18,7 @@

>> >>   */

>> >>

>> >>  #include "qemu/osdep.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "cpu.h"

>> >>  #include "exec/helper-proto.h"

>> >>  #include "exec/log.h"

>> >> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)

>> >>  #define SMM_REVISION_ID 0x00020000

>> >>  #endif

>> >>

>> >> +/* Called with iothread lock taken */

>> >>  void cpu_smm_update(X86CPU *cpu)

>> >>  {

>> >>      CPUX86State *env = &cpu->env;

>> >>      bool smm_enabled = (env->hflags & HF_SMM_MASK);

>> >>

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >> +

>> >>      if (cpu->smram) {

>> >>          memory_region_set_enabled(cpu->smram, smm_enabled);

>> >>      }

>> >> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)

>> >>      }

>> >>      env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;

>> >>      env->hflags &= ~HF_SMM_MASK;

>> >> +

>> >> +    qemu_mutex_lock_iothread();

>> >>      cpu_smm_update(cpu);

>> >> +    qemu_mutex_unlock_iothread();

>> >>

>> >>      qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");

>> >>      log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);

>> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c

>> >> index c9604ea9c7..3cb942e8bb 100644

>> >> --- a/target/s390x/misc_helper.c

>> >> +++ b/target/s390x/misc_helper.c

>> >> @@ -25,6 +25,7 @@

>> >>  #include "exec/helper-proto.h"

>> >>  #include "sysemu/kvm.h"

>> >>  #include "qemu/timer.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "exec/address-spaces.h"

>> >>  #ifdef CONFIG_KVM

>> >>  #include <linux/kvm.h>

>> >> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)

>> >>  /* SCLP service call */

>> >>  uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)

>> >>  {

>> >> +    qemu_mutex_lock_iothread();

>> >>      int r = sclp_service_call(env, r1, r2);

>> >>      if (r < 0) {

>> >>          program_interrupt(env, -r, 4);

>> >> -        return 0;

>> >> +        r = 0;

>> >>      }

>> >> +    qemu_mutex_unlock_iothread();

>> >>      return r;

>> >>  }

>> >>

>> >> diff --git a/translate-all.c b/translate-all.c

>> >> index 8a861cb583..f810259c41 100644

>> >> --- a/translate-all.c

>> >> +++ b/translate-all.c

>> >> @@ -55,6 +55,7 @@

>> >>  #include "translate-all.h"

>> >>  #include "qemu/bitmap.h"

>> >>  #include "qemu/timer.h"

>> >> +#include "qemu/main-loop.h"

>> >>  #include "exec/log.h"

>> >>

>> >>  /* #define DEBUG_TB_INVALIDATE */

>> >> @@ -1523,7 +1524,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,

>> >>  #ifdef CONFIG_SOFTMMU

>> >>  /* len must be <= 8 and start must be a multiple of len.

>> >>   * Called via softmmu_template.h when code areas are written to with

>> >> - * tb_lock held.

>> >> + * iothread mutex not held.

>> >>   */

>> >>  void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)

>> >>  {

>> >> @@ -1725,7 +1726,10 @@ void tb_check_watchpoint(CPUState *cpu)

>> >>

>> >>  #ifndef CONFIG_USER_ONLY

>> >>  /* in deterministic execution mode, instructions doing device I/Os

>> >> -   must be at the end of the TB */

>> >> + * must be at the end of the TB.

>> >> + *

>> >> + * Called by softmmu_template.h, with iothread mutex not held.

>> >> + */

>> >>  void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)

>> >>  {

>> >>  #if defined(TARGET_MIPS) || defined(TARGET_SH4)

>> >> @@ -1937,6 +1941,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)

>> >>

>> >>  void cpu_interrupt(CPUState *cpu, int mask)

>> >>  {

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >>      cpu->interrupt_request |= mask;

>> >>      cpu->tcg_exit_req = 1;

>> >>  }

>> >> diff --git a/translate-common.c b/translate-common.c

>> >> index 5e989cdf70..d504dd0d33 100644

>> >> --- a/translate-common.c

>> >> +++ b/translate-common.c

>> >> @@ -21,6 +21,7 @@

>> >>  #include "qemu-common.h"

>> >>  #include "qom/cpu.h"

>> >>  #include "sysemu/cpus.h"

>> >> +#include "qemu/main-loop.h"

>> >>

>> >>  uintptr_t qemu_real_host_page_size;

>> >>  intptr_t qemu_real_host_page_mask;

>> >> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;

>> >>  static void tcg_handle_interrupt(CPUState *cpu, int mask)

>> >>  {

>> >>      int old_mask;

>> >> +    g_assert(qemu_mutex_iothread_locked());

>> >>

>> >>      old_mask = cpu->interrupt_request;

>> >>      cpu->interrupt_request |= mask;

>> >> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)

>> >>       */

>> >>      if (!qemu_cpu_is_self(cpu)) {

>> >>          qemu_cpu_kick(cpu);

>> >> -        return;

>> >> -    }

>> >> -

>> >> -    if (use_icount) {

>> >> -        cpu->icount_decr.u16.high = 0xffff;

>> >> -        if (!cpu->can_do_io

>> >> -            && (mask & ~old_mask) != 0) {

>> >> -            cpu_abort(cpu, "Raised interrupt while not in I/O function");

>> >> -        }

>> >>      } else {

>> >> -        cpu->tcg_exit_req = 1;

>> >> +        if (use_icount) {

>> >> +            cpu->icount_decr.u16.high = 0xffff;

>> >> +            if (!cpu->can_do_io

>> >> +                && (mask & ~old_mask) != 0) {

>> >> +                cpu_abort(cpu, "Raised interrupt while not in I/O function");

>> >> +            }

>> >> +        } else {

>> >> +            cpu->tcg_exit_req = 1;

>> >> +        }

>> >>      }

>> >>  }

>> >>

>> >> --

>> >> 2.11.0

>> >>

>> >>

>>

>>

>> --

>> Alex Bennée

>>



--
Alex Bennée