From patchwork Tue Apr 20 01:17:55 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jun Koi X-Patchwork-Id: 50504 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4726EB7D0B for ; Tue, 20 Apr 2010 11:18:57 +1000 (EST) Received: from localhost ([127.0.0.1]:51841 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O4274-0003JE-Cu for incoming@patchwork.ozlabs.org; Mon, 19 Apr 2010 21:18:54 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O426W-0003Iu-6F for qemu-devel@nongnu.org; Mon, 19 Apr 2010 21:18:20 -0400 Received: from [140.186.70.92] (port=47943 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O426U-0003IZ-QA for qemu-devel@nongnu.org; Mon, 19 Apr 2010 21:18:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O426T-0007Jx-FW for qemu-devel@nongnu.org; Mon, 19 Apr 2010 21:18:18 -0400 Received: from mail-iw0-f194.google.com ([209.85.223.194]:65442) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O426T-0007Ji-BL for qemu-devel@nongnu.org; Mon, 19 Apr 2010 21:18:17 -0400 Received: by iwn32 with SMTP id 32so3555657iwn.18 for ; Mon, 19 Apr 2010 18:18:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :from:date:received:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=n/DNlNIa/dtzxuuVIxFGjE05BsOCRqFTYgz8EvtFKUM=; b=rCPgXdVxudFsGssU6PwAsPSLSfs/NCwD//xDwcFeD3MxkfHMxjCbjoFkf4SI1+mRlj +IMWIKIWUT0v2eURvJD3zOjIP6IpEAFFebRiUR1r7j6oPKIepEjjRkYV69rj0potaquA ViDZ680qc4oZOuT9M2UisAOGIvzent2jB2+z8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=mtQwSmpiCqc2lrDdXXbmESsnF53zqY+91WelxPZapN0whwbdzBcW33IgBW+pxKCuo8 O88Uj6L/rciqOOvIqk+5xaLo4J5I2YTdR1fNJZtHRJmUGSMzhivahSQqO+Rb7lc9wspA VArqady2DfqcyOOZOzJUz9ZjiygW9iTCdlD+8= MIME-Version: 1.0 Received: by 10.231.147.209 with HTTP; Mon, 19 Apr 2010 18:17:55 -0700 (PDT) In-Reply-To: <4BC8D2E8.3030309@mail.berlios.de> References: <4BC8D2E8.3030309@mail.berlios.de> From: Jun Koi Date: Tue, 20 Apr 2010 10:17:55 +0900 Received: by 10.231.166.16 with SMTP id k16mr1793077iby.14.1271726295956; Mon, 19 Apr 2010 18:18:15 -0700 (PDT) Message-ID: Subject: Re: [Qemu-devel] [PATCH] flush TB on singlestep command To: Stefan Weil X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: Jan Kiszka , qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Thank you for the explanation of this code. Qemu has a command named singlestep, which reduces the translated code block to be only one instruction. This new patch flushes TBs both when singlestep is on and off. Signed-off-by: Jun Koi On Sat, Apr 17, 2010 at 6:13 AM, Stefan Weil wrote: > Jun Koi schrieb: >> (Thanks to Jan for comments on the last patch) >> >> Qemu has a command named singlestep, which reduces the translated code >> block to be only one instruction. >> However, there is one flaw when this command is triggered via monitor >> interface: we do not flush all the current TBs, so we will miss >> single-step on already translated code. >> This patch fixes the problem by flushing all the TB to force new code >> generation. >> >> Signed-off-by: Jun Koi >> >> >> >> diff --git a/monitor.c b/monitor.c >> index 5659991..948b861 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1190,8 +1190,14 @@ static void do_log(Monitor *mon, const QDict >> *qdict) >> static void do_singlestep(Monitor *mon, const QDict *qdict) >> { >> const char *option = qdict_get_try_str(qdict, "option"); >> + CPUState *env; >> + >> if (!option || !strcmp(option, "on")) { >> singlestep = 1; >> + /* flush all the TBs to force new code generation */ >> + for (env = first_cpu; env != NULL; env = env->next_cpu) { >> + tb_flush(env); >> + } >> } else if (!strcmp(option, "off")) { >> singlestep = 0; >> } else { > > Hi, > > sorry that my feedback comes rather late. I read the discussion, > but had no time to answer. > > I wrote the code for the singlestep command line and monitor option > (which already existed before as a compile time option) and still use > it frequently. Up to now, I did not miss the tb flushing, but I see that > it might be useful in certain cases. > > My typical use cases for "singlestep" are > > * Compare execution of same code (usually a user mode program or a kernel) >  running in different environments (32 bit or 64 bit host, big or little >  endian host, different host architectures). >  In combination with -D in_asm,cpu it is possible to find wrong tcg code >  by comparing the resulting qemu.log files. > > * Use singlestep to slow down the execution (yes, this is sometimes useful). > > In most cases, I use the command line option, but sometimes I use the > monitor command, too. > > There is no logical relation between switching singlestep on or off and > tb flushing. If there is the need for tb flush, I'd suggest to add a new > monitor command. > > If the translated tbs should match the singlestep setting, > you would also have to flush them when singlestep is disabled: > in that case, the translated tbs only contain a single target instruction, > so they are not very efficient - and they remain so even after > singlestep was disabled. > > So either flush for singlestep on and off, or better add a new monitor > command > which flushes tbs. > > Regards, > Stefan > > Acked-by: Alexander Graf diff --git a/monitor.c b/monitor.c index 5659991..2b2005b 100644 --- a/monitor.c +++ b/monitor.c @@ -1187,13 +1187,26 @@ static void do_log(Monitor *mon, const QDict *qdict) cpu_set_log(mask); } +/* flush all the TBs to force new code generation */ +static void flush_all_tb(void) +{ + CPUState *env; + + for (env = first_cpu; env != NULL; env = env->next_cpu) { + tb_flush(env); + } +} + static void do_singlestep(Monitor *mon, const QDict *qdict) { const char *option = qdict_get_try_str(qdict, "option"); + if (!option || !strcmp(option, "on")) { singlestep = 1; + flush_all_tb(); } else if (!strcmp(option, "off")) { singlestep = 0; + flush_all_tb(); } else { monitor_printf(mon, "unexpected option %s\n", option); }