Re: [Qemu-devel] Holding the BQL for emulate_ppc_hypercall

["Alex Bennée" <alex.bennee@linaro.org>]

Alex Bennée <alex.bennee@linaro.org> writes:

> Hi,
>
> In the MTTCG patch set one of the big patches is to remove the
> requirement to hold the BQL while running code:
>
>   tcg: drop global lock during TCG code execution
>
> And this broke the PPC code because emulate_ppc_hypercall can cause
> changes to the global state. This function just calls spapr_hypercall()
> and puts the results into the TCG register file. Normally
> spapr_hypercall() is called under the BQL in KVM as
> kvm_arch_handle_exit() does things with the BQL held.
>
> I blithely wrapped the called in a lock/unlock pair only to find the
> ppc64 check builds failed as the hypercall was made during the
> cc->do_interrupt() code which also holds the BQL.
>
> I'm a little confused by the nature of PPC hypercalls in TCG? Are they
> not all detectable at code generation time? What is the case that causes
> an exception to occur rather than the helper function doing the
> hypercall?
>
> I guess it comes down to can I avoid doing:
>
>   /* If we come via cc->do_interrupt BQL may already be held */
>   if (!qemu_mutex_iothread_locked()) {
>       g_mutex_lock_iothread();
>       env->gpr[3] = spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]);
>       g_muetx_unlock_iothread();
>   } else {
>       env->gpr[3] = spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]);
>   }

Of course I mean:

  /* If we come via cc->do_interrupt BQL may already be held */
  if (!qemu_mutex_iothread_locked()) {
      qemu_mutex_lock_iothread();
      env->gpr[3] = spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]);
      qemu_mutex_unlock_iothread();
  } else {
      env->gpr[3] = spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]);
  }

> Any thoughts?


--
Alex Bennée