qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command


From: Stefan Weil
Subject: Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command
Date: Tue, 27 Apr 2010 21:55:59 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100411 Iceowl/1.0b1 Icedove/3.0.4

Am 22.04.2010 09:02, schrieb Jan Kiszka:
Stefan Weil wrote:
Jan Kiszka schrieb:
Alexander Graf wrote:

On 21.04.2010, at 12:04, Jun Koi wrote:


On Tue, Apr 20, 2010 at 8:44 PM, Alexander Graf <address@hidden> wrote:

On 20.04.2010, at 13:38, Jan Kiszka wrote:


Alexander Graf wrote:

On 20.04.2010, at 09:18, Jan Kiszka wrote:


Jun Koi wrote:

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 <address@hidden>


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);
+ }
+}
+

The smaller your patch are, the more people pick on it. :)

I was about to suggest moving this close to tb_flush, but then I
realized that the env argument of that service is misleading. In fact,
it already flushes the one and only translation buffer pool.


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);
}


Let's just pass mon->mon_cpu to tb_flush and skip the redundant loop.

That doesn't help, no? singlestep is a global variable. Flushing only the current vcpu would still not affect the others, while the singlestep switch would.

tb_flush uses env only to dump some state when a problem occurred.


According to your above comment the cache is global, but I don't think we should rely on that.

It might make sense to define some tb_flush_all() as tb_flush(first_cpu) for now to establish the infrastructure. Then we are prepared for the
day the tb_flush implementation may change.

Right. But then the call to tb_flush_all here is still correct.

So what is the final solution do you want?

I still think that having flush_all_tb() like in the last patch is good enough.

I agree. And I like the patch as is.

Acked-by: Alexander Graf <address@hidden>


Sorry, nack for keeping this service in /monitor.c/. But a bonus ack if
you avoid the needless loop when moving it to exec.c, adding a comment
that current tb_flush has global, env-invariant scope.

Thanks,
Jan

flush_all_tb() is now called for singlestep on and off, that's fine.
But it's called always - no way to disable this call. That's not good.
Sometimes I don't want to flush all TBs when I switch singlestep mode
(that's the reason why I suggested a separate monitor command which
flushes all TBs - I still think that would be the best solution).

Mind to tell us the use case?

Typical use case: execution trace of some code which is
run after OS boot with an explicit trigger.

This can be loading of a linux kernel module, a user space
application or kernel code which handles a rare event.

I can enable logging and single stepping before that code
starts. There is no need to re-translate existing TBs:
they are faster than TBs with only single steps, so only
the execution of the new code is slow, and only new TBs
will appear in qemu.log which is exactly what I want.

Typically, I use single stepping like this to examine a
problem with QEMU's emulation or code generation. Two examples:
some years ago aptitude crashed in mips emulation (fpu emulation
problem), and now I use it to examine differences between
native TCG and TCI (tiny code interpreter).

Regards,
Stefan





reply via email to

[Prev in Thread] Current Thread [Next in Thread]