Patchwork flush TB on singlestep command

login
register
mail settings
Submitter Jun Koi
Date April 16, 2010, 1:03 a.m.
Message ID <o2hfdaac4d51004151803j9d1069d2nd9dfe3297bb64439@mail.gmail.com>
Download mbox | patch
Permalink /patch/50299/
State New
Headers show

Comments

Jun Koi - April 16, 2010, 1:03 a.m.
(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 <junkoi2004@gmail.com>
Stefan Weil - April 16, 2010, 9:13 p.m.
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 <junkoi2004@gmail.com>
>
>
>
> 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

Patch

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 {