From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:32991)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1QrWJd-0007E1-6q
	for qemu-devel@nongnu.org; Thu, 11 Aug 2011 10:33:01 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1QrWJY-0007vw-LO
	for qemu-devel@nongnu.org; Thu, 11 Aug 2011 10:32:57 -0400
Received: from mx1.redhat.com ([209.132.183.28]:41063)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1QrWJY-0007vo-8e
	for qemu-devel@nongnu.org; Thu, 11 Aug 2011 10:32:52 -0400
Message-ID: <4E43E808.4090100@redhat.com>
Date: Thu, 11 Aug 2011 16:32:40 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <4E0CA885.2050308@siemens.com>
	<BANLkTi=Unfpkty5TYVUvJEBp1hzbN4du=g@mail.gmail.com>
	<BANLkTimW27g3rE4a30vr1T10XccQEAi+sA@mail.gmail.com>
	<4E0ECDDB.9030001@web.de>
	<CAFEAcA81p4F836+cX_y6sdoERC3ewFQzrPDtao=xTRHuV7--UA@mail.gmail.com>
	<4E43C80B.1050300@redhat.com>
	<CAFEAcA8m1CvqZLqK5Jk1KE9rkpsK0_D8rO9GCdFigWS9RupU7g@mail.gmail.com>
	<4E43D585.90907@redhat.com>
	<CAFEAcA-_xqNuXaOKV_tN-wac8RHZtKgeah=P=YJgRosjUZUcsg@mail.gmail.com>
	<4E43E2BB.1020403@redhat.com>
	<CAFEAcA_Mq_fF7rKA89LByKG5_Qk3MFAJa_XeNk_SPpKwSS=ySw@mail.gmail.com>
In-Reply-To: <CAFEAcA_Mq_fF7rKA89LByKG5_Qk3MFAJa_XeNk_SPpKwSS=ySw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] tcg: Reload local variables after return
	from longjmp
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Blue Swirl <blauwirbel@gmail.com>, TeLeMan <geleman@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, qemu-devel <qemu-devel@nongnu.org>, David Gilbert <david.gilbert@linaro.org>

On 08/11/2011 04:24 PM, Peter Maydell wrote:
>> I cannot think off-hand of a reason why thread-local cpu_single_env should
>> not work for iothread under Unix, BTW.  Since cpu_single_env is only
>> set/used by a thread at a time (under the global lock), its users cannot
>> distinguish between a thread-local variable and a global.
>
> Thanks for the clarification. As you say, as long as we don't ever
> try to access it from another thread we're fine...

Yes, and the current usage of the lock should be enough of a guarantee.

>> The only problem would be Windows, which runs cpu_signal in a thread
>> different than the CPU thread.  But that can be fixed easily in
>> qemu_cpu_kick_thread.
>
> ...and we just need to fix this.

Untested (uncompiled) patch follows:

diff --git a/cpus.c b/cpus.c
index 6bf4e3f..04e52fe 100644
--- a/cpus.c
+++ b/cpus.c
@@ -179,10 +179,10 @@ static void cpu_handle_guest_debug(CPUState *env)
  }

  #ifdef CONFIG_IOTHREAD
-static void cpu_signal(int sig)
+static inline void do_cpu_kick(CPUState *env)
  {
-    if (cpu_single_env) {
-        cpu_exit(cpu_single_env);
+    if (env) {
+        cpu_exit(env);
      }
      exit_request = 1;
  }
@@ -476,6 +476,13 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
      }
  }

+#ifdef CONFIG_IOTHREAD
+static void cpu_signal(int sig)
+{
+    do_cpu_kick(cpu_single_env);
+}
+#endif
+
  static void qemu_tcg_init_cpu_signals(void)
  {
  #ifdef CONFIG_IOTHREAD
@@ -858,7 +865,7 @@ static void qemu_cpu_kick_thread(CPUState *env)
  #else /* _WIN32 */
      if (!qemu_cpu_is_self(env)) {
          SuspendThread(env->thread->thread);
-        cpu_signal(0);
+        do_cpu_kick(env);
          ResumeThread(env->thread->thread);
      }
  #endif