Re: [Qemu-devel] Re: Missing singlestep for already-translated code?

[Jun Koi <junkoi2004@gmail.com>]

On Wed, Apr 14, 2010 at 12:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Alexander Graf wrote:
>> On 13.04.2010, at 15:36, Jan Kiszka wrote:
>>
>>> Jun Koi wrote:
>>>> Hi,
>>>>
>>>> I am looking into the singlestep command in monitor interface, and it
>>>> seems that we only take into account the singlestep flag when we are
>>>> translating code.
>>>> So for the already-translated code, we will miss singlestep?
>>> This feature is broken. For TCG, it should at least flush the
>>> translation buffer, and for KVM it has to enable single-stepping in the
>>> kernel. That's what happens automatically when you call cpu_single_step.
>>> I guess 'singlestep' wants to be somehow orthogonal to this. But this is
>>> the wrong approach.
>>>
>>> Does anyone actually used this feature or still does so? It looks fairly
>>> redundant to me, kind of a poor-man's gdb front-end as part of the
>>> monitor console.
>>
>> Not sure what it does, but I use -singlestep quite a lot to get register dumps for instructions when using -d cpu.
>
> Ah, "singlestep" is not about stopping the VM after each instruction but
> about limiting the TB length to a single instruction. Badly named and
> poorly documented.
>
> In that case, the dynamic switch should already be fine by adding a
> tb_flush() on enable. Still, someone should also patch at least the docs.
>

Do you have any comment on the below patch?

Thanks,
J

diff --git a/monitor.c b/monitor.c
index 5659991..dfa9820 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1190,8 +1190,13 @@ 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 TB 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 {