Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
From: Paolo Bonzini [mailto:pbonz...@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.dovga...@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
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
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
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
From: Paolo Bonzini [mailto:pbonz...@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
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
On 27/11/2014 10:11, Pavel Dovgaluk wrote:
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.
Does this partial revert work?
Paolo
diff --git a/cpu-exec.c b/cpu-exec.c
index 3913de0..c976095 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -331,6 +331,12 @@ int cpu_exec(CPUArchState *env)
volatile bool have_tb_lock = false;
if (cpu-halted) {
+#ifdef TARGET_I386
+if (cpu-interrupt_request CPU_INTERRUPT_POLL) {
+apic_poll_irq(x86_cpu-apic_state);
+cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
+}
+#endif
if (!cpu_has_work(cpu)) {
return EXCP_HALTED;
}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e9df33e..3f13dfe 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2912,14 +2912,8 @@ static bool x86_cpu_has_work(CPUState *cs)
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = cpu-env;
-#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)
+return ((cs-interrupt_request (CPU_INTERRUPT_HARD |
+ CPU_INTERRUPT_POLL))
(env-eflags IF_MASK)) ||
(cs-interrupt_request (CPU_INTERRUPT_NMI |
CPU_INTERRUPT_INIT |
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
From: Paolo Bonzini [mailto:pbonz...@redhat.com]
On 27/11/2014 10:11, Pavel Dovgaluk wrote:
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.
Does this partial revert work?
This one is better because I can place synchronization point for replay here.
Pavel Dovgalyuk
Paolo
diff --git a/cpu-exec.c b/cpu-exec.c
index 3913de0..c976095 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -331,6 +331,12 @@ int cpu_exec(CPUArchState *env)
volatile bool have_tb_lock = false;
if (cpu-halted) {
+#ifdef TARGET_I386
+if (cpu-interrupt_request CPU_INTERRUPT_POLL) {
+apic_poll_irq(x86_cpu-apic_state);
+cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
+}
+#endif
if (!cpu_has_work(cpu)) {
return EXCP_HALTED;
}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e9df33e..3f13dfe 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2912,14 +2912,8 @@ static bool x86_cpu_has_work(CPUState *cs)
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = cpu-env;
-#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)
+return ((cs-interrupt_request (CPU_INTERRUPT_HARD |
+ CPU_INTERRUPT_POLL))
(env-eflags IF_MASK)) ||
(cs-interrupt_request (CPU_INTERRUPT_NMI |
CPU_INTERRUPT_INIT |
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
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.dovga...@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;
Re: [Qemu-devel] [RFC PATCH v5 22/31] timer: introduce new QEMU_CLOCK_VIRTUAL_RT clock
From: Paolo Bonzini [mailto:pbonz...@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.dovga...@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
