[Qemu-devel] [RFC] save/restore with icount enabled.

[Qemu-devel] [RFC] save/restore with icount enabled.

[KONRAD Frédéric]

Hi,

We are trying to do a simple save/restore on the VM with icount enabled.

We saw that qemu_icount_bias and qemu_icount in cpus.c are not 
saved/restored,
and icount_extra, icount_decr in CPUState neither, so the vm_clock is just
growing normally after restoring the VM is that normal?

We think that this is making the "replay" undeterminastic, which is bad for
reverse execution.

Is there a good reason for them not being saved?

Thanks,
Fred

Re: [Qemu-devel] [RFC] save/restore with icount enabled.

[Paolo Bonzini]

Il 15/05/2013 11:31, KONRAD Frédéric ha scritto:
> Hi,
> 
> We are trying to do a simple save/restore on the VM with icount enabled.
> 
> We saw that qemu_icount_bias and qemu_icount in cpus.c are not
> saved/restored,
> and icount_extra, icount_decr in CPUState neither, so the vm_clock is just
> growing normally after restoring the VM is that normal?
> 
> We think that this is making the "replay" undeterminastic, which is bad for
> reverse execution.
> 
> Is there a good reason for them not being saved?

No.  You can probably move icount to timers_state.  Something like
zeroing qemu_icount in cpu_disable_ticks() is required in order to save
qemu_icount_bias like the other members of timers_state.  There is even
a dummy field that you can reuse to avoid the pain of adding a
subsection to vmstate_timers. :)

Paolo

> 
> Thanks,
> Fred
> 
> 

Re: [Qemu-devel] [RFC] save/restore with icount enabled.

[KONRAD Frédéric]

On 15/05/2013 14:50, Paolo Bonzini wrote:
> Il 15/05/2013 11:31, KONRAD Frédéric ha scritto:
>> Hi,
>>
>> We are trying to do a simple save/restore on the VM with icount enabled.
>>
>> We saw that qemu_icount_bias and qemu_icount in cpus.c are not
>> saved/restored,
>> and icount_extra, icount_decr in CPUState neither, so the vm_clock is just
>> growing normally after restoring the VM is that normal?
>>
>> We think that this is making the "replay" undeterminastic, which is bad for
>> reverse execution.
>>
>> Is there a good reason for them not being saved?
> No.  You can probably move icount to timers_state.  Something like
> zeroing qemu_icount in cpu_disable_ticks() is required in order to save
> qemu_icount_bias like the other members of timers_state.  There is even
> a dummy field that you can reuse to avoid the pain of adding a
> subsection to vmstate_timers. :)
>
> Paolo

Yes nice point :).

What about icount_extra and icount_decr defined in CPU_COMMON? in 
cpu-defs.h?

     int64_t icount_extra; /* Instructions until next timer event. */   \
     /* Number of cycles left, with interrupt flag in high bit.          \
        This allows a single read-compare-cbranch-write sequence to test \
        for both decrementer underflow and exceptions. */               \
     union {                                                             \
         uint32_t u32;                                                   \
         icount_decr_u16 u16;                                            \
     } icount_decr;                                                      \

Thanks,
Fred
>> Thanks,
>> Fred
>>
>>

Re: [Qemu-devel] [RFC] save/restore with icount enabled.

[Paolo Bonzini]

Il 15/05/2013 15:35, KONRAD Frédéric ha scritto:
> On 15/05/2013 14:50, Paolo Bonzini wrote:
>> Il 15/05/2013 11:31, KONRAD Frédéric ha scritto:
>>> Hi,
>>>
>>> We are trying to do a simple save/restore on the VM with icount enabled.
>>>
>>> We saw that qemu_icount_bias and qemu_icount in cpus.c are not
>>> saved/restored,
>>> and icount_extra, icount_decr in CPUState neither, so the vm_clock is
>>> just
>>> growing normally after restoring the VM is that normal?
>>>
>>> We think that this is making the "replay" undeterminastic, which is
>>> bad for
>>> reverse execution.
>>>
>>> Is there a good reason for them not being saved?
>> No.  You can probably move icount to timers_state.  Something like
>> zeroing qemu_icount in cpu_disable_ticks() is required in order to save
>> qemu_icount_bias like the other members of timers_state.  There is even
>> a dummy field that you can reuse to avoid the pain of adding a
>> subsection to vmstate_timers. :)
>>
>> Paolo
> 
> Yes nice point :).
> 
> What about icount_extra and icount_decr defined in CPU_COMMON? in
> cpu-defs.h?
> 
>     int64_t icount_extra; /* Instructions until next timer event. */   \
>     /* Number of cycles left, with interrupt flag in high bit.          \
>        This allows a single read-compare-cbranch-write sequence to test \
>        for both decrementer underflow and exceptions. */               \
>     union {                                                             \
>         uint32_t u32;                                                   \
>         icount_decr_u16 u16;                                            \
>     } icount_decr;                                                      \

I'm not sure, I don't know that part well.  But IIRC they are recomputed
every time the VM restarts, so you don't need to save them?

Paolo