diff mbox

[RFC] icount: warp in the main_loop.

Message ID 1404231220-17339-1-git-send-email-fred.konrad@greensocs.com
State New
Headers show

Commit Message

fred.konrad@greensocs.com July 1, 2014, 4:13 p.m. UTC
From: KONRAD Frederic <fred.konrad@greensocs.com>

This fixes a bug where QEMU stall in icount mode.

It happens when a simple timer callback is created on VIRTUAL CLOCK modding
itself regularly.

The actual warping mechanism is called once and then the time didn't grow
anymore.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 main-loop.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

fred.konrad@greensocs.com July 4, 2014, 7:30 a.m. UTC | #1
On 01/07/2014 18:13, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> This fixes a bug where QEMU stall in icount mode.
>
> It happens when a simple timer callback is created on VIRTUAL CLOCK modding
> itself regularly.
>
> The actual warping mechanism is called once and then the time didn't grow
> anymore.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>   main-loop.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/main-loop.c b/main-loop.c
> index 8a85493..ef889b0 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -489,6 +489,12 @@ int main_loop_wait(int nonblocking)
>   
>       qemu_clock_run_all_timers();
>   
> +    /*
> +     * In icount mode, sometimes the VCPU is blocked and an event is needed to
> +     * continue.
> +     * Just warp to make the time grows and have a chance to run the CPU.
> +     */
> +    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
>       return ret;
>   }
>   
Paolo,
You mentioned some icount patches (I can't find where) can you point me 
to them?
Did you already had this bug?

Thanks,
Fred
Paolo Bonzini July 4, 2014, 7:57 a.m. UTC | #2
Il 04/07/2014 09:30, Frederic Konrad ha scritto:
>>   +    /*
>> +     * In icount mode, sometimes the VCPU is blocked and an event is
>> needed to
>> +     * continue.
>> +     * Just warp to make the time grows and have a chance to run the
>> CPU.
>> +     */
>> +    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
>>       return ret;
>>   }
>>
> Paolo,
> You mentioned some icount patches (I can't find where) can you point me
> to them?
> Did you already had this bug?

Why is this needed?  It's possible that a qemu_clock_warp code is 
missing somewhere, but for timers it should be handled here:

static void timerlist_rearm(QEMUTimerList *timer_list)
{
     /* Interrupt execution to force deadline recalculation.  */
     qemu_clock_warp(timer_list->clock->type);
     timerlist_notify(timer_list);
}

If the VCPU is blocked, it will set vm_clock_warp_start to the realtime 
clock value ("clock") a QEMU_CLOCK_REALTIME timer to the next deadline 
("clock + deadline").  At the next deadline, icount_warp_rt will 
increase QEMU_CLOCK_VIRTUAL by "clock - vm_clock_warp_state" which 
should trigger the clock event.

Paolo
fred.konrad@greensocs.com July 4, 2014, 10:28 a.m. UTC | #3
On 04/07/2014 09:57, Paolo Bonzini wrote:
> Il 04/07/2014 09:30, Frederic Konrad ha scritto:
>>>   +    /*
>>> +     * In icount mode, sometimes the VCPU is blocked and an event is
>>> needed to
>>> +     * continue.
>>> +     * Just warp to make the time grows and have a chance to run the
>>> CPU.
>>> +     */
>>> +    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
>>>       return ret;
>>>   }
>>>
>> Paolo,
>> You mentioned some icount patches (I can't find where) can you point me
>> to them?
>> Did you already had this bug?
>
> Why is this needed?  It's possible that a qemu_clock_warp code is 
> missing somewhere, but for timers it should be handled here:
>
> static void timerlist_rearm(QEMUTimerList *timer_list)
> {
>     /* Interrupt execution to force deadline recalculation.  */
>     qemu_clock_warp(timer_list->clock->type);
>     timerlist_notify(timer_list);
> }
>
> If the VCPU is blocked, it will set vm_clock_warp_start to the 
> realtime clock value ("clock") a QEMU_CLOCK_REALTIME timer to the next 
> deadline ("clock + deadline").  At the next deadline, icount_warp_rt 
> will increase QEMU_CLOCK_VIRTUAL by "clock - vm_clock_warp_state" 
> which should trigger the clock event.

Right, but when I put a timer eg on QEMU_VIRTUAL_CLOCK the guest is stuck.
icount_warp_rt is not called neither qemu_clock_warp(..)..

So yes as you said seems a qemu_clock_warp is missing somewhere.

Shouldn't icount_warp_rt called regularly to advance the time when the 
VCPU is not
executing?

Thanks,
Fred
>
> Paolo
Paolo Bonzini July 4, 2014, 10:36 a.m. UTC | #4
> Right, but when I put a timer eg on QEMU_VIRTUAL_CLOCK the guest is stuck.
> icount_warp_rt is not called neither qemu_clock_warp(..)..

It should be.  timer_mod_ns -> timerlist_rearm -> qemu_clock_warp.

> So yes as you said seems a qemu_clock_warp is missing somewhere.
> 
> Shouldn't icount_warp_rt called regularly to advance the time when the
> VCPU is not executing?

No, everything is done dynamically based on timer deadlines.  Polling is bad. :)

Paolo
diff mbox

Patch

diff --git a/main-loop.c b/main-loop.c
index 8a85493..ef889b0 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -489,6 +489,12 @@  int main_loop_wait(int nonblocking)
 
     qemu_clock_run_all_timers();
 
+    /*
+     * In icount mode, sometimes the VCPU is blocked and an event is needed to
+     * continue.
+     * Just warp to make the time grows and have a chance to run the CPU.
+     */
+    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
     return ret;
 }