On 01/03/2016 12:07, Pavel Dovgalyuk wrote: > qemu_clock_warp function is called to update virtual clock when CPU > is sleeping. This function includes replay checkpoint to make execution > deterministic in icount mode. > Record/replay module flushes async event queue at checkpoints. > Some of the events (e.g., block devices operations) include interaction > with hardware. E.g., APIC polled by block devices sets one of IRQ flags. > Flag to be set depends on currently executed thread (CPU or iothread). > Therefore in replay mode we have to process the checkpoints in the same thread > as they were recorded. > qemu_clock_warp function (and its checkpoint) may be called from different > thread. This patch introduces new checkpoint which distinguished warp > checkpoint calls from different threads. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
I think we need two different kinds of "warp" behavior, one to start the warp timer (from the main loop and when a timer is set) and one to end it (from the CPUs). Then the need for two checkpoints is much clearer, though I suggestnaming them without a reference to TCG; something like CHECKPOINT_CLOCK_WARP_START and CHECKPOINT_CLOCK_WARP_ACCOUNT for example. The start would be where you call qemu_clock_warp(QEMU_CLOCK_VIRTUAL, false): if (!use_icount) { return; } if (!runstate_is_running()) { return; } if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_START)) { return; } /* I think calling icount_warp_rt here is unnecessary. */ if (!all_cpu_threads_idle()) { return; } if (qtest_enabled()) { /* When testing, qtest commands advance icount. */ return; } /* We want to use the earliest deadline from ALL vm_clocks */ clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT); deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL); ... The end or account function, instead, would be called from tcg_exec_all() and also from icount_dummy_timer() (this is what makes the call to icount_warp_rt unnecessary above): if (!use_icount || !icount_isleep) { return; } if (!runstate_is_running()) { return; } if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_END)) { return; } timer_del(icount_warp_timer); /* * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer now. * This ensures that the deadline for the timer is computed correctly * below. * This also makes sure that the insn counter is synchronized before * the CPU starts running, in case the CPU is woken by an event other * than the earliest QEMU_CLOCK_VIRTUAL timer. */ // ...include icount_warp_rt function here... qemu_clock_warp would only be called from qemu-timer.c, and it would be simply be if (type == QEMU_CLOCK_VIRTUAL) { qemu_start_warp_timer(); } Separating the two boundaries of the warp this way would make the code much easier to understand, and would also make the need for a new checkpoint more obvious. Paolo