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: Jan Kiszka
Subject: Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command
Date: Tue, 20 Apr 2010 13:38:53 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

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.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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