[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command
From: |
Jun Koi |
Subject: |
Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command |
Date: |
Thu, 22 Apr 2010 16:14:40 +0900 |
On Thu, Apr 22, 2010 at 4:20 AM, Stefan Weil <address@hidden> 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
When dont you want to flush TBs??
Thanks,
J
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, (continued)
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Jan Kiszka, 2010/04/20
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Alexander Graf, 2010/04/20
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Jun Koi, 2010/04/21
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Alexander Graf, 2010/04/21
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Jan Kiszka, 2010/04/21
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Stefan Weil, 2010/04/21
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Jan Kiszka, 2010/04/22
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Stefan Weil, 2010/04/27
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Jun Koi, 2010/04/27
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command, Stefan Weil, 2010/04/28
- Re: [Qemu-devel] Re: [PATCH] flush TB on singlestep command,
Jun Koi <=