| Message ID | 20141126104049.7772.3594.stgit@PASHA-ISP |
|---|---|
| State | New |
| Headers | show |
On 26/11/2014 11:40, Pavel Dovgalyuk wrote: > This patch introduces new QEMU_CLOCK_VIRTUAL_RT clock, which > should be used for icount warping. Separate timer is needed > for replaying the execution, because warping callbacks should > be deterministic. We cannot make realtime clock deterministic > because it is used for screen updates and other simulator-specific > actions. That is why we added new clock which is recorded and > replayed when needed. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > include/qemu/timer.h | 7 +++++++ > qemu-timer.c | 2 ++ > replay/replay.h | 4 +++- > 3 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/include/qemu/timer.h b/include/qemu/timer.h > index 7b43331..df27157 100644 > --- a/include/qemu/timer.h > +++ b/include/qemu/timer.h > @@ -37,12 +37,19 @@ > * is suspended, and it will reflect system time changes the host may > * undergo (e.g. due to NTP). The host clock has the same precision as > * the virtual clock. > + * > + * @QEMU_CLOCK_VIRTUAL_RT: realtime clock used for icount warp > + * > + * This clock runs as a realtime clock, but is used for icount warp > + * and thus should be traced with record/replay to make warp function > + * behave deterministically. > */ I think it should also stop/restart across "stop" and "cont" commands, similar to QEMU_CLOCK_VIRTUAL. This is as simple as changing get_clock() to cpu_get_clock(). This way, QEMU_CLOCK_VIRTUAL_RT is "what QEMU_CLOCK_VIRTUAL does without -icount". This makes a lot of sense and can be merged in 2.3 independent of the rest of the series. Paolo > typedef enum { > QEMU_CLOCK_REALTIME = 0, > QEMU_CLOCK_VIRTUAL = 1, > QEMU_CLOCK_HOST = 2, > + QEMU_CLOCK_VIRTUAL_RT = 3, > QEMU_CLOCK_MAX > } QEMUClockType; > > diff --git a/qemu-timer.c b/qemu-timer.c > index 8307913..3f99af5 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -567,6 +567,8 @@ int64_t qemu_clock_get_ns(QEMUClockType type) > notifier_list_notify(&clock->reset_notifiers, &now); > } > return now; > + case QEMU_CLOCK_VIRTUAL_RT: > + return REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT, get_clock()); > } > } > > diff --git a/replay/replay.h b/replay/replay.h > index 143fe85..0c02e03 100755 > --- a/replay/replay.h > +++ b/replay/replay.h > @@ -22,8 +22,10 @@ > #define REPLAY_CLOCK_REAL_TICKS 0 > /* host_clock */ > #define REPLAY_CLOCK_HOST 1 > +/* virtual_rt_clock */ > +#define REPLAY_CLOCK_VIRTUAL_RT 2 > > -#define REPLAY_CLOCK_COUNT 2 > +#define REPLAY_CLOCK_COUNT 3 > > extern ReplayMode replay_mode; > extern char *replay_image_suffix; >
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 26/11/2014 11:40, Pavel Dovgalyuk wrote: > > This patch introduces new QEMU_CLOCK_VIRTUAL_RT clock, which > > should be used for icount warping. Separate timer is needed > > for replaying the execution, because warping callbacks should > > be deterministic. We cannot make realtime clock deterministic > > because it is used for screen updates and other simulator-specific > > actions. That is why we added new clock which is recorded and > > replayed when needed. > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > --- > > include/qemu/timer.h | 7 +++++++ > > qemu-timer.c | 2 ++ > > replay/replay.h | 4 +++- > > 3 files changed, 12 insertions(+), 1 deletions(-) > > > > diff --git a/include/qemu/timer.h b/include/qemu/timer.h > > index 7b43331..df27157 100644 > > --- a/include/qemu/timer.h > > +++ b/include/qemu/timer.h > > @@ -37,12 +37,19 @@ > > * is suspended, and it will reflect system time changes the host may > > * undergo (e.g. due to NTP). The host clock has the same precision as > > * the virtual clock. > > + * > > + * @QEMU_CLOCK_VIRTUAL_RT: realtime clock used for icount warp > > + * > > + * This clock runs as a realtime clock, but is used for icount warp > > + * and thus should be traced with record/replay to make warp function > > + * behave deterministically. > > */ > > I think it should also stop/restart across "stop" and "cont" commands, > similar to QEMU_CLOCK_VIRTUAL. This is as simple as changing > get_clock() to cpu_get_clock(). Ok, then I'll have to remove !use_icount check from here and retest the series. void cpu_enable_ticks(void) { /* Here, the really thing protected by seqlock is cpu_clock_offset. */ seqlock_write_lock(&timers_state.vm_clock_seqlock); if (!timers_state.cpu_ticks_enabled) { if (!use_icount) { timers_state.cpu_ticks_offset -= cpu_get_real_ticks(); timers_state.cpu_clock_offset -= get_clock(); } timers_state.cpu_ticks_enabled = 1; } seqlock_write_unlock(&timers_state.vm_clock_seqlock); } > This way, QEMU_CLOCK_VIRTUAL_RT is "what QEMU_CLOCK_VIRTUAL does without > -icount". This makes a lot of sense and can be merged in 2.3 > independent of the rest of the series. Pavel Dovgalyuk
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 26/11/2014 11:40, Pavel Dovgalyuk wrote: > > + * @QEMU_CLOCK_VIRTUAL_RT: realtime clock used for icount warp > > + * > > + * This clock runs as a realtime clock, but is used for icount warp > > + * and thus should be traced with record/replay to make warp function > > + * behave deterministically. > > */ > > I think it should also stop/restart across "stop" and "cont" commands, > similar to QEMU_CLOCK_VIRTUAL. This is as simple as changing > get_clock() to cpu_get_clock(). > > This way, QEMU_CLOCK_VIRTUAL_RT is "what QEMU_CLOCK_VIRTUAL does without > -icount". This makes a lot of sense and can be merged in 2.3 > independent of the rest of the series. I've updated QEMU to master and started testing changed that you proposed. And one more problem came out here. The problem is related to patch 60e68042cf70f271308dc6b4b22b609d054af929 It changes x86_cpu_has_work function. And this function instead of just checking the CPU state changes it: - return ((cs->interrupt_request & (CPU_INTERRUPT_HARD | - CPU_INTERRUPT_POLL)) && +#if !defined(CONFIG_USER_ONLY) + if (cs->interrupt_request & CPU_INTERRUPT_POLL) { + apic_poll_irq(cpu->apic_state); + cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL); + } +#endif + + return ((cs->interrupt_request & CPU_INTERRUPT_HARD) && These changes break the deterministic execution, because cpu_has_work() may be called at any moment of the execution. When POLL interrupt request is processed by x86_cpu_exec_interrupt function, as it were before, everything is ok, because I ensure that these calls occur at the same moments in record/replay. Pavel Dovgalyuk
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 26/11/2014 11:40, Pavel Dovgalyuk wrote: > > This patch introduces new QEMU_CLOCK_VIRTUAL_RT clock, which > > should be used for icount warping. Separate timer is needed > > for replaying the execution, because warping callbacks should > > be deterministic. We cannot make realtime clock deterministic > > because it is used for screen updates and other simulator-specific > > actions. That is why we added new clock which is recorded and > > replayed when needed. > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > --- > > include/qemu/timer.h | 7 +++++++ > > qemu-timer.c | 2 ++ > > replay/replay.h | 4 +++- > > 3 files changed, 12 insertions(+), 1 deletions(-) > > > > diff --git a/include/qemu/timer.h b/include/qemu/timer.h > > index 7b43331..df27157 100644 > > --- a/include/qemu/timer.h > > +++ b/include/qemu/timer.h > > @@ -37,12 +37,19 @@ > > * is suspended, and it will reflect system time changes the host may > > * undergo (e.g. due to NTP). The host clock has the same precision as > > * the virtual clock. > > + * > > + * @QEMU_CLOCK_VIRTUAL_RT: realtime clock used for icount warp > > + * > > + * This clock runs as a realtime clock, but is used for icount warp > > + * and thus should be traced with record/replay to make warp function > > + * behave deterministically. > > */ > > I think it should also stop/restart across "stop" and "cont" commands, > similar to QEMU_CLOCK_VIRTUAL. This is as simple as changing > get_clock() to cpu_get_clock(). Not so easy :) cpu_get_clock() checks vm_clock_seqlock which is locked in icount_warp_rt(). And after locking it requests the value of QEMU_CLOCK_VIRTUAL_RT: seqlock_write_lock(&timers_state.vm_clock_seqlock); if (runstate_is_running()) { int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT); Pavel Dovgalyuk
On 28/11/2014 12:28, Pavel Dovgaluk wrote: > Not so easy :) > cpu_get_clock() checks vm_clock_seqlock which is locked in icount_warp_rt(). > And after locking it requests the value of QEMU_CLOCK_VIRTUAL_RT: > > seqlock_write_lock(&timers_state.vm_clock_seqlock); > if (runstate_is_running()) { > int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT); Not so hard :D You can use cpu_get_clock_locked() there. In fact, cpu_get_clock_locked() is already used below in the "if", so we can reuse "clock" instead of the other variable "cur_time". Paolo
diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 7b43331..df27157 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -37,12 +37,19 @@ * is suspended, and it will reflect system time changes the host may * undergo (e.g. due to NTP). The host clock has the same precision as * the virtual clock. + * + * @QEMU_CLOCK_VIRTUAL_RT: realtime clock used for icount warp + * + * This clock runs as a realtime clock, but is used for icount warp + * and thus should be traced with record/replay to make warp function + * behave deterministically. */ typedef enum { QEMU_CLOCK_REALTIME = 0, QEMU_CLOCK_VIRTUAL = 1, QEMU_CLOCK_HOST = 2, + QEMU_CLOCK_VIRTUAL_RT = 3, QEMU_CLOCK_MAX } QEMUClockType; diff --git a/qemu-timer.c b/qemu-timer.c index 8307913..3f99af5 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -567,6 +567,8 @@ int64_t qemu_clock_get_ns(QEMUClockType type) notifier_list_notify(&clock->reset_notifiers, &now); } return now; + case QEMU_CLOCK_VIRTUAL_RT: + return REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT, get_clock()); } } diff --git a/replay/replay.h b/replay/replay.h index 143fe85..0c02e03 100755 --- a/replay/replay.h +++ b/replay/replay.h @@ -22,8 +22,10 @@ #define REPLAY_CLOCK_REAL_TICKS 0 /* host_clock */ #define REPLAY_CLOCK_HOST 1 +/* virtual_rt_clock */ +#define REPLAY_CLOCK_VIRTUAL_RT 2 -#define REPLAY_CLOCK_COUNT 2 +#define REPLAY_CLOCK_COUNT 3 extern ReplayMode replay_mode; extern char *replay_image_suffix;
This patch introduces new QEMU_CLOCK_VIRTUAL_RT clock, which should be used for icount warping. Separate timer is needed for replaying the execution, because warping callbacks should be deterministic. We cannot make realtime clock deterministic because it is used for screen updates and other simulator-specific actions. That is why we added new clock which is recorded and replayed when needed. Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> --- include/qemu/timer.h | 7 +++++++ qemu-timer.c | 2 ++ replay/replay.h | 4 +++- 3 files changed, 12 insertions(+), 1 deletions(-)