Skip to contentSkip to conversation

replay additions

PATCH Superseded Pavel Dovgalyuk 4 participants 33 messages Jan 24, 2017 – Jan 25, 2017 Block layer

v7 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Pavel Dovgalyuk

author
#0 lore raw

v7 cover replay additions

This set of patches includes several fixes for replay and vmstate.

This patches add rrsnapshot option for icount. rrshapshot option creates start snapshot at record and loads it at replay. It allows preserving the state of disk images used by virtual machine. This vm state can also use used to roll back the execution while replaying.

With these patches operations with audio devices can also be recorded and replayed. All interactions with passthrough audio (including microphone input) are recorded automatically when -soundhw is specified at the command line.

This set of patches includes fixes and additions for icount and record/replay implementation:
- VM start/stop in replay mode
- overlay creation for blkreplay filter
- rrsnapshot option for record/replay
- vmstate fix for integratorcp ARM platform
- vmstate fixes for apic and rtc
- fixes for icount record/replay mode
- record/replay for audio devices

v7 changes:
- Fixed exception replaying when TB cache is full and
when tb_find is called when there are no instructions about to execute
- Added record/replay for audio devices

v6 changes:
- Added overlay creation for blkreplay driver
- Fixed vmstate loading for apic and rtc
- Fixed instruction counting for apic instruction patching

v5 changes:
- Recording is stopped when initial snapshot cannot be created
- Minor changes

v4 changes:
- Overlay option is removed from blkreplay driver (as suggested by Paolo Bonzini)
- Minor changes

v3 changes:
- Added rrsnapshot option for specifying the initial snapshot name (as suggested by Paolo Bonzini)
- Minor changes

---

Pavel Dovgalyuk (14):
icount: update instruction counter on apic patching
replay: improve interrupt handling
replay: exception replay fix
icount: exit cpu loop on expire
apic: save apic_delivered flag
replay: don't use rtc clock on loadvm phase
integratorcp: adding vmstate for save/restore
savevm: add public save_vmstate function
replay: save/load initial state
block: implement bdrv_snapshot_goto for blkreplay
blkreplay: create temporary overlay for underlaying devices
replay: disable default snapshot for record/replay
audio: make audio poll timer deterministic
replay: add record/replay for audio passthrough

 audio/audio.c                   |   15 +++++--
 audio/audio.h                   |    5 ++
 audio/mixeng.c                  |   31 ++++++++++++++
 block/blkreplay.c               |   84 +++++++++++++++++++++++++++++++++++++++
 cpu-exec.c                      |   21 ++++++++--
 docs/replay.txt                 |   23 +++++++++++
 hw/arm/integratorcp.c           |   62 +++++++++++++++++++++++++++++
 hw/i386/kvmvapic.c              |    6 +++
 hw/intc/apic_common.c           |   32 +++++++++++++++
 hw/timer/mc146818rtc.c          |   14 +++++--
 include/hw/i386/apic_internal.h |    2 +
 include/sysemu/replay.h         |   16 +++++++
 include/sysemu/sysemu.h         |    1 
 migration/savevm.c              |   33 ++++++++++-----
 qemu-options.hx                 |    8 +++-
 replay/Makefile.objs            |    1 
 replay/replay-audio.c           |   79 +++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h        |    4 ++
 replay/replay-snapshot.c        |   17 ++++++++
 replay/replay.c                 |    5 ++
 stubs/replay.c                  |    1 
 target/i386/seg_helper.c        |    1 
 vl.c                            |   17 +++++++-
 23 files changed, 450 insertions(+), 28 deletions(-)
 create mode 100644 replay/replay-audio.c
Signature · 1 line
Pavel Dovgalyuk
author
#1 lore raw

PATCH v7 1/14 icount: update instruction counter on apic patching

kvmvapic patches the code when some instructions are executed. E.g. mov 0xff, 0xfffe0080 is interpreted as push 0xff/call ... This patching is also followed by some side effects (changing apic and guest memory state). Therefore deterministic execution should take this operation into account. This patch decreases icount when original mov instruction is trying to execute. Therefore patching becomes deterministic and can be replayed correctly.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/i386/kvmvapic.c |    6 ++++++
 1 file changed, 6 insertions(+)
hw/i386/kvmvapic.c +6 -0
index b30d1b9..146d47c 100644
@@ -412,6 +412,12 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
     if (!kvm_enabled()) {
         cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
                              &current_flags);
+        /* Account this instruction, because we will exit the tb.
+           This is the first instruction in the block. Therefore
+           there is no need in restoring CPU state. */
+        if (use_icount) {
+            --cs->icount_decr.u16.low;
+        }
     }
 
     pause_all_vcpus();
author
#2 lore raw

PATCH v7 2/14 replay: improve interrupt handling

In reply to Pavel Dovgalyuk

This patch improves interrupt handling in record/replay mode.
Now "interrupt" event is saved only when cc->cpu_exec_interrupt returns true. This patch also adds missing return to cpu_exec_interrupt function.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 cpu-exec.c               |    2 +-
 target/i386/seg_helper.c |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
cpu-exec.c +1 -1
index 4188fed..fa08c73 100644
@@ -508,8 +508,8 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
            True when it is, and we should restart on a new TB,
            and via longjmp via cpu_loop_exit.  */
         else {
-            replay_interrupt();
             if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+                replay_interrupt();
                 *last_tb = NULL;
             }
             /* The target hook may have updated the 'cpu->interrupt_request';
target/i386/seg_helper.c +1 -0
index fb79f31..d24574d 100644
@@ -1331,6 +1331,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 #endif
     if (interrupt_request & CPU_INTERRUPT_SIPI) {
         do_cpu_sipi(cpu);
+        ret = true;
     } else if (env->hflags2 & HF2_GIF_MASK) {
         if ((interrupt_request & CPU_INTERRUPT_SMI) &&
             !(env->hflags & HF_SMM_MASK)) {
author
#3 lore raw

PATCH v7 3/14 replay: exception replay fix

In reply to Pavel Dovgalyuk

This patch fixes replaying the exception when TB cache is full. It breaks cpu loop execution through setting exception_index to process such queued work as TB flush.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 cpu-exec.c |    4 ++++
 1 file changed, 4 insertions(+)
cpu-exec.c +4 -0
index fa08c73..79a2167 100644
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;
#4 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

Paolo

#5 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

Pavel Dovgalyuk

#6 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 16 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

Quoted 4 lines

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

Paolo

#7 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 18 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 16 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

Quoted 7 lines
Quoted 4 lines

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

cpu->exit_request does not checked in this loop.
We have to add this checking somewhere then?

Pavel Dovgalyuk

#8 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 23 lines · Pavel Dovgalyuk
Quoted 23 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 20 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

Quoted 5 lines
Quoted 2 lines

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

cpu->exit_request does not checked in this loop.
We have to add this checking somewhere then?

It's checked by cpu_handle_interrupt. Are you not reaching cpu_handle_interrupt then? Why?

Or perhaps cpu_handle_interrupt should not be testing cpu->exit_request, but cpu->exception_index != -1 (and cpu_exit can cmpxchg cpu->exception_index from -1 to EXCP_INTERRUPT)?

Again, it's hard to follow without knowing the invariants. :(

Paolo

#9 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 25 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 23 lines · Pavel Dovgalyuk
Quoted 23 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 20 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

cpu-exec.c static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
cpu-exec.c +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

cpu->exit_request does not checked in this loop.
We have to add this checking somewhere then?

It's checked by cpu_handle_interrupt. Are you not reaching cpu_handle_interrupt then? Why?

Execution doesn't reach cpu_handle_interrupt, because tb_find
calls cpu_loop_exit. And the execution loop enters cpu_handle_exception again and again.

Pavel Dovgalyuk

#10 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 28 lines · Pavel Dovgalyuk
Quoted 28 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 26 lines · Pavel Dovgalyuk
Quoted 25 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 16 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

Review static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
(quoted diff) +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

Quoted 4 lines

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

cpu->exit_request does not checked in this loop.
We have to add this checking somewhere then?

It's checked by cpu_handle_interrupt. Are you not reaching cpu_handle_interrupt then? Why?

Execution doesn't reach cpu_handle_interrupt, because tb_find
calls cpu_loop_exit. And the execution loop enters cpu_handle_exception again and again.

Perhaps tb_gen_code should set cpu->exception_index before calling cpu_loop_exit.

Paolo

Quoted 2 lines

Pavel Dovgalyuk

#11 lore raw

Re: [Qemu-devel] [PATCH v7 03/14] replay: exception replay fix

Quoted 33 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 31 lines · Pavel Dovgalyuk
Quoted 31 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 28 lines · Pavel Dovgalyuk
Quoted 18 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 16 lines · Pavel Dovgalyuk
Quoted 15 lines

From: Paolo Bonzini [mailto:[email protected]]

Review static inline bool cpu_handle_exception(CPUState *cpu, int *ret) · line 460

Quoted 11 lines · Pavel Dovgalyuk
(quoted diff) +4 -0
@@ -451,6 +451,10 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
     } else if (replay_has_exception()
                && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
+        /* Break the execution loop in case of running out of TB cache.
+           This is needed to make flushing of the TB cache, because
+           real flush is queued to be executed outside the cpu loop. */
+        cpu->exception_index = EXCP_INTERRUPT;
         /* try to cause an exception pending in the log */
         cpu_exec_nocache(cpu, 1, tb_find(cpu, NULL, 0), true);
         *ret = -1;

Why is replay_has_exception() related to be running out of TB cache?

Quoted 7 lines
Quoted 4 lines

It doesn't.
Calling tb_find when there is not space in cache causes tb_flush and cpu_loop_exit. But execution loop will continue, because there is no reason to break it (like setting exception_index).

What about setting cpu->exit_request? queue_work_on_cpu calls qemu_cpu_kick.

cpu->exit_request does not checked in this loop.
We have to add this checking somewhere then?

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

It's checked by cpu_handle_interrupt. Are you not reaching cpu_handle_interrupt then? Why?

Execution doesn't reach cpu_handle_interrupt, because tb_find
calls cpu_loop_exit. And the execution loop enters cpu_handle_exception again and again.

Perhaps tb_gen_code should set cpu->exception_index before calling cpu_loop_exit.

Maybe it would be better, because tb_gen_code is called in many other paths. I'll fix the patch.

Pavel Dovgalyuk

author
#12 lore raw

PATCH v7 4/14 icount: exit cpu loop on expire

In reply to Pavel Dovgalyuk

This patch adds check to break cpu loop when icount expires without setting the TB_EXIT_ICOUNT_EXPIRED flag. It happens when there is no available translated blocks and all instructions were executed. In icount replay mode unnecessary tb_find will be called (which may cause an exception) and execution will be non-deterministic.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 cpu-exec.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
cpu-exec.c +12 -3
index 79a2167..e603da4 100644
@@ -582,9 +582,6 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
                 cpu_exec_nocache(cpu, insns_left, *last_tb, false);
                 align_clocks(sc, cpu);
             }
-            cpu->exception_index = EXCP_INTERRUPT;
-            *last_tb = NULL;
-            cpu_loop_exit(cpu);
         }
         break;
 #endif

@@ -638,6 +635,18 @@ int cpu_exec(CPUState *cpu)
 
             for(;;) {
                 cpu_handle_interrupt(cpu, &last_tb);
+                /* icount has expired, we need to break the execution loop.
+                   This check is needed before tb_find to make execution
+                   deterministic - tb_find may cause an exception
+                   while translating the code from non-mapped page. */
+                if (use_icount && ((cpu->icount_extra == 0
+                                    && cpu->icount_decr.u16.low == 0)
+                                || (int32_t)cpu->icount_decr.u32 < 0)) {
+                    if (cpu->exception_index == -1) {
+                        cpu->exception_index = EXCP_INTERRUPT;
+                    }
+                    cpu_loop_exit(cpu);
+                }
                 tb = tb_find(cpu, last_tb, tb_exit);
                 cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
                 /* Try to align the host and virtual clocks
#13 lore raw

Re: [Qemu-devel] [PATCH v7 04/14] icount: exit cpu loop on expire

Quoted 31 lines · Pavel Dovgalyuk

This patch adds check to break cpu loop when icount expires without setting the TB_EXIT_ICOUNT_EXPIRED flag. It happens when there is no available translated blocks and all instructions were executed. In icount replay mode unnecessary tb_find will be called (which may cause an exception) and execution will be non-deterministic.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 cpu-exec.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
cpu-exec.c +2 -3
index 79a2167..e603da4 100644
@@ -582,9 +582,6 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
                 cpu_exec_nocache(cpu, insns_left, *last_tb, false);
                 align_clocks(sc, cpu);
             }
-            cpu->exception_index = EXCP_INTERRUPT;
-            *last_tb = NULL;
-            cpu_loop_exit(cpu);
         }
         break;
 #endif

@@ -638,6 +635,18 @@ int cpu_exec(CPUState *cpu)
 
             for(;;) {
                 cpu_handle_interrupt(cpu, &last_tb);
+                /* icount has expired, we need to break the execution loop.
+                   This check is needed before tb_find to make execution

cpu-exec.c int cpu_exec(CPUState *cpu) · line 649

Quoted 10 lines
cpu-exec.c +10 -0
+                   deterministic - tb_find may cause an exception
+                   while translating the code from non-mapped page. */
+                if (use_icount && ((cpu->icount_extra == 0
+                                    && cpu->icount_decr.u16.low == 0)
+                                || (int32_t)cpu->icount_decr.u32 < 0)) {
+                    if (cpu->exception_index == -1) {
+                        cpu->exception_index = EXCP_INTERRUPT;
+                    }
+                    cpu_loop_exit(cpu);
+                }

Can this can be placed in cpu_handle_interrupt itself? Also, can the cpu->exception_index != -1 case happen?

We really should add assertions on cpu->exception_index, maybe after cpu_handle_exception and cc->cpu_exec_interrupt returns true. Without some idea of the invariants, I'm not competent enough to review this patch.

Paolo

Quoted 4 lines
cpu-exec.c +0 -0
                 tb = tb_find(cpu, last_tb, tb_exit);
                 cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
                 /* Try to align the host and virtual clocks
#14 lore raw

Re: [Qemu-devel] [PATCH v7 04/14] icount: exit cpu loop on expire

Quoted 29 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 27 lines · Pavel Dovgalyuk

This patch adds check to break cpu loop when icount expires without setting the TB_EXIT_ICOUNT_EXPIRED flag. It happens when there is no available translated blocks and all instructions were executed. In icount replay mode unnecessary tb_find will be called (which may cause an exception) and execution will be non-deterministic.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 cpu-exec.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
cpu-exec.c +0 -3
index 79a2167..e603da4 100644
@@ -582,9 +582,6 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
                 cpu_exec_nocache(cpu, insns_left, *last_tb, false);
                 align_clocks(sc, cpu);
             }
-            cpu->exception_index = EXCP_INTERRUPT;
-            *last_tb = NULL;
-            cpu_loop_exit(cpu);
         }
         break;
 #endif

@@ -638,6 +635,18 @@ int cpu_exec(CPUState *cpu)
Quoted 16 lines
Quoted 4 lines
cpu-exec.c +2 -0
             for(;;) {
                 cpu_handle_interrupt(cpu, &last_tb);
+                /* icount has expired, we need to break the execution loop.
+                   This check is needed before tb_find to make execution

cpu-exec.c int cpu_exec(CPUState *cpu) · line 649

Quoted 10 lines
cpu-exec.c +10 -0
+                   deterministic - tb_find may cause an exception
+                   while translating the code from non-mapped page. */
+                if (use_icount && ((cpu->icount_extra == 0
+                                    && cpu->icount_decr.u16.low == 0)
+                                || (int32_t)cpu->icount_decr.u32 < 0)) {
+                    if (cpu->exception_index == -1) {
+                        cpu->exception_index = EXCP_INTERRUPT;
+                    }
+                    cpu_loop_exit(cpu);
+                }

Can this can be placed in cpu_handle_interrupt itself?

I guess it could. I placed it here because it doesn't related to interrupts.

Quoted 1 line

Also, can the cpu->exception_index != -1 case happen?

This branch is executed where there are no instructions to replay. It may happen due to an exception (which is already set) or because execution loop has to break. In the first case exception_index != -1.

Quoted 3 lines

We really should add assertions on cpu->exception_index, maybe after cpu_handle_exception and cc->cpu_exec_interrupt returns true. Without some idea of the invariants, I'm not competent enough to review this patch.

Pavel Dovgalyuk

#15 lore raw

Re: [Qemu-devel] [PATCH v7 04/14] icount: exit cpu loop on expire

Quoted 12 lines · Pavel Dovgalyuk
Quoted 12 lines
Quoted 12 lines
cpu-exec.c +12 -0
+                /* icount has expired, we need to break the execution loop.
+                   This check is needed before tb_find to make execution
+                   deterministic - tb_find may cause an exception
+                   while translating the code from non-mapped page. */
+                if (use_icount && ((cpu->icount_extra == 0
+                                    && cpu->icount_decr.u16.low == 0)
+                                || (int32_t)cpu->icount_decr.u32 < 0)) {
+                    if (cpu->exception_index == -1) {
+                        cpu->exception_index = EXCP_INTERRUPT;
+                    }
+                    cpu_loop_exit(cpu);
+                }
Quoted 2 lines
Quoted 1 line

Can this can be placed in cpu_handle_interrupt itself?

I guess it could. I placed it here because it doesn't related to interrupts.

True, on the other hand neither is

    if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
        atomic_set(&cpu->exit_request, 0);
        cpu->exception_index = EXCP_INTERRUPT;
        cpu_loop_exit(cpu);
    }

Except for the replay_has_interrupt() that you added, but I don't understand that one either...

Paolo

author
#16 lore raw

PATCH v7 5/14 apic: save apic_delivered flag

In reply to Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +32 -0
index d78c885..ac6cc67 100644
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;
+}
+
+static void apic_irq_delivered_pre_save(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    s->apic_irq_delivered = apic_irq_delivered;
+}
+
+static int apic_irq_delivered_post_load(void *opaque, int version_id)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    apic_irq_delivered = s->apic_irq_delivered;
+    return 0;
+}
+
 static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,

@@ -396,6 +414,19 @@ static const VMStateDescription vmstate_apic_common_sipi = {
     }
 };
 
+static const VMStateDescription vmstate_apic_irq_delivered = {
+    .name = "apic_irq_delivered",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = apic_irq_delivered_needed,
+    .pre_save = apic_irq_delivered_pre_save,
+    .post_load = apic_irq_delivered_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,

@@ -430,6 +461,7 @@ static const VMStateDescription vmstate_apic_common = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_apic_common_sipi,
+        &vmstate_apic_irq_delivered,
         NULL
     }
 };
include/hw/i386/apic_internal.h +2 -0
index 1209eb4..20ad28c 100644
@@ -189,6 +189,8 @@ struct APICCommonState {
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
     bool legacy_instance_id;
+
+    int apic_irq_delivered; /* for saving static variable */
 };
 
 typedef struct VAPICState {
#17 lore raw

Re: [Qemu-devel] [PATCH v7 05/14] apic: save apic_delivered flag

Quoted 13 lines · Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +0 -0
index d78c885..ac6cc67 100644

hw/intc/apic_common.c static bool apic_common_sipi_needed(void *opaque) · line 389

Quoted 6 lines
hw/intc/apic_common.c +3 -0
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;

Is it needed for CPUs except the first (or the last?)?

Paolo

Quoted 62 lines
hw/intc/apic_common.c +29 -0
+}
+
+static void apic_irq_delivered_pre_save(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    s->apic_irq_delivered = apic_irq_delivered;
+}
+
+static int apic_irq_delivered_post_load(void *opaque, int version_id)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    apic_irq_delivered = s->apic_irq_delivered;
+    return 0;
+}
+
 static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,

@@ -396,6 +414,19 @@ static const VMStateDescription vmstate_apic_common_sipi = {
     }
 };
 
+static const VMStateDescription vmstate_apic_irq_delivered = {
+    .name = "apic_irq_delivered",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = apic_irq_delivered_needed,
+    .pre_save = apic_irq_delivered_pre_save,
+    .post_load = apic_irq_delivered_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,

@@ -430,6 +461,7 @@ static const VMStateDescription vmstate_apic_common = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_apic_common_sipi,
+        &vmstate_apic_irq_delivered,
         NULL
     }
 };
include/hw/i386/apic_internal.h +2 -0
index 1209eb4..20ad28c 100644
@@ -189,6 +189,8 @@ struct APICCommonState {
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
     bool legacy_instance_id;
+
+    int apic_irq_delivered; /* for saving static variable */
 };
 
 typedef struct VAPICState {
#18 lore raw

Re: [Qemu-devel] [PATCH v7 05/14] apic: save apic_delivered flag

Quoted 18 lines

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini

Quoted 16 lines · Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +0 -0
index d78c885..ac6cc67 100644
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
Quoted 6 lines

hw/intc/apic_common.c static bool apic_common_sipi_needed(void *opaque) · line 389

Quoted 3 lines
hw/intc/apic_common.c +3 -0
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;

Is it needed for CPUs except the first (or the last?)?

As this is global variable, it is needed only for one CPU. Do you mean that APIC state is saved for every CPU?

Quoted 58 lines
Quoted 58 lines
hw/intc/apic_common.c +29 -0
+}
+
+static void apic_irq_delivered_pre_save(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    s->apic_irq_delivered = apic_irq_delivered;
+}
+
+static int apic_irq_delivered_post_load(void *opaque, int version_id)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    apic_irq_delivered = s->apic_irq_delivered;
+    return 0;
+}
+
 static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,

@@ -396,6 +414,19 @@ static const VMStateDescription vmstate_apic_common_sipi = {
     }
 };
 
+static const VMStateDescription vmstate_apic_irq_delivered = {
+    .name = "apic_irq_delivered",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = apic_irq_delivered_needed,
+    .pre_save = apic_irq_delivered_pre_save,
+    .post_load = apic_irq_delivered_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,

@@ -430,6 +461,7 @@ static const VMStateDescription vmstate_apic_common = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_apic_common_sipi,
+        &vmstate_apic_irq_delivered,
         NULL
     }
 };
include/hw/i386/apic_internal.h +2 -0
index 1209eb4..20ad28c 100644
@@ -189,6 +189,8 @@ struct APICCommonState {
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
     bool legacy_instance_id;
+
+    int apic_irq_delivered; /* for saving static variable */
 };
Quoted 4 lines

include/hw/i386/apic_internal.h struct APICCommonState { · line 196

Quoted 4 lines
include/hw/i386/apic_internal.h +0 -0
 typedef struct VAPICState {

Pavel Dovgalyuk

#19 lore raw

Re: [Qemu-devel] [PATCH v7 05/14] apic: save apic_delivered flag

Quoted 25 lines · Pavel Dovgalyuk
Quoted 18 lines

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini

Quoted 16 lines · Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +0 -0
index d78c885..ac6cc67 100644
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
Quoted 6 lines

hw/intc/apic_common.c static bool apic_common_sipi_needed(void *opaque) · line 389

Quoted 3 lines
hw/intc/apic_common.c +3 -0
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;

Is it needed for CPUs except the first (or the last?)?

Quoted 2 lines

As this is global variable, it is needed only for one CPU. Do you mean that APIC state is saved for every CPU?

Yes, each CPU has its own APIC.

Paolo

Quoted 65 lines
Quoted 58 lines
Quoted 58 lines
hw/intc/apic_common.c +29 -0
+}
+
+static void apic_irq_delivered_pre_save(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    s->apic_irq_delivered = apic_irq_delivered;
+}
+
+static int apic_irq_delivered_post_load(void *opaque, int version_id)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    apic_irq_delivered = s->apic_irq_delivered;
+    return 0;
+}
+
 static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,

@@ -396,6 +414,19 @@ static const VMStateDescription vmstate_apic_common_sipi = {
     }
 };
 
+static const VMStateDescription vmstate_apic_irq_delivered = {
+    .name = "apic_irq_delivered",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = apic_irq_delivered_needed,
+    .pre_save = apic_irq_delivered_pre_save,
+    .post_load = apic_irq_delivered_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,

@@ -430,6 +461,7 @@ static const VMStateDescription vmstate_apic_common = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_apic_common_sipi,
+        &vmstate_apic_irq_delivered,
         NULL
     }
 };
include/hw/i386/apic_internal.h +2 -0
index 1209eb4..20ad28c 100644
@@ -189,6 +189,8 @@ struct APICCommonState {
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
     bool legacy_instance_id;
+
+    int apic_irq_delivered; /* for saving static variable */
 };
Quoted 4 lines

include/hw/i386/apic_internal.h struct APICCommonState { · line 196

Quoted 4 lines
include/hw/i386/apic_internal.h +0 -0
 typedef struct VAPICState {

Pavel Dovgalyuk

#20 lore raw

Re: [Qemu-devel] [PATCH v7 05/14] apic: save apic_delivered flag

Quoted 27 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 25 lines · Pavel Dovgalyuk
Quoted 24 lines

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini

Quoted 13 lines · Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +0 -0
index d78c885..ac6cc67 100644

hw/intc/apic_common.c static bool apic_common_sipi_needed(void *opaque) · line 389

Quoted 6 lines
hw/intc/apic_common.c +3 -0
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;

Is it needed for CPUs except the first (or the last?)?

Quoted 4 lines
Quoted 2 lines

As this is global variable, it is needed only for one CPU. Do you mean that APIC state is saved for every CPU?

Yes, each CPU has its own APIC.

How to link this data to a single CPU?

Pavel Dovgalyuk

#21 lore raw

Re: [Qemu-devel] [PATCH v7 05/14] apic: save apic_delivered flag

Quoted 27 lines · Pavel Dovgalyuk
Quoted 27 lines

From: Paolo Bonzini [mailto:[email protected]]

Quoted 25 lines · Pavel Dovgalyuk
Quoted 24 lines

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini

Quoted 13 lines · Pavel Dovgalyuk

This patch implements saving/restoring of static apic_delivered variable.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/intc/apic_common.c           |   32 ++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 34 insertions(+)
hw/intc/apic_common.c +0 -0
index d78c885..ac6cc67 100644

hw/intc/apic_common.c static bool apic_common_sipi_needed(void *opaque) · line 389

Quoted 6 lines
hw/intc/apic_common.c +3 -0
@@ -384,6 +384,24 @@ static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    return true;

Is it needed for CPUs except the first (or the last?)?

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

As this is global variable, it is needed only for one CPU. Do you mean that APIC state is saved for every CPU?

Yes, each CPU has its own APIC.

How to link this data to a single CPU?

Since it's global, you only need to migrate it once. You're migrating the same value many times.

Paolo

author
#22 lore raw

PATCH v7 6/14 replay: don't use rtc clock on loadvm phase

In reply to Pavel Dovgalyuk

This patch disables the update of the periodic timer of mc146818rtc in record/replay mode. State of this timer is saved and therefore does not need to be updated in record/replay mode.
Read of RTC breaks the replay because all rtc reads have to be the same as in record mode.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/timer/mc146818rtc.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
hw/timer/mc146818rtc.c +10 -4
index da209d0..5638777 100644
@@ -27,6 +27,7 @@
 #include "hw/hw.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
 #include "hw/timer/mc146818rtc.h"
 #include "qapi/visitor.h"
 #include "qapi-event.h"

@@ -734,10 +735,15 @@ static int rtc_post_load(void *opaque, int version_id)
         check_update_timer(s);
     }
 
-    uint64_t now = qemu_clock_get_ns(rtc_clock);
-    if (now < s->next_periodic_time ||
-        now > (s->next_periodic_time + get_max_clock_jump())) {
-        periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
+    /* Periodic timer is deterministic in record/replay mode.
+       No need to update it after loading the vmstate.
+       Reading RTC here may break the execution. */
+    if (replay_mode == REPLAY_MODE_NONE) {
+        uint64_t now = qemu_clock_get_ns(rtc_clock);
+        if (now < s->next_periodic_time ||
+            now > (s->next_periodic_time + get_max_clock_jump())) {
+            periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
+        }
     }
 
 #ifdef TARGET_I386
author
#23 lore raw

PATCH v7 7/14 integratorcp: adding vmstate for save/restore

In reply to Pavel Dovgalyuk

From: Pavel Dovgalyuk <[email protected]>

VMState added by this patch preserves correct
loading of the integratorcp device state.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 hw/arm/integratorcp.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
hw/arm/integratorcp.c +62 -0
index 039812a..ca06e1b 100644
@@ -53,6 +53,27 @@ static uint8_t integrator_spd[128] = {
    0xe, 4, 0x1c, 1, 2, 0x20, 0xc0, 0, 0, 0, 0, 0x30, 0x28, 0x30, 0x28, 0x40
 };
 
+static const VMStateDescription vmstate_integratorcm = {
+    .name = "integratorcm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(cm_osc, IntegratorCMState),
+        VMSTATE_UINT32(cm_ctrl, IntegratorCMState),
+        VMSTATE_UINT32(cm_lock, IntegratorCMState),
+        VMSTATE_UINT32(cm_auxosc, IntegratorCMState),
+        VMSTATE_UINT32(cm_sdram, IntegratorCMState),
+        VMSTATE_UINT32(cm_init, IntegratorCMState),
+        VMSTATE_UINT32(cm_flags, IntegratorCMState),
+        VMSTATE_UINT32(cm_nvflags, IntegratorCMState),
+        VMSTATE_UINT32(int_level, IntegratorCMState),
+        VMSTATE_UINT32(irq_enabled, IntegratorCMState),
+        VMSTATE_UINT32(fiq_enabled, IntegratorCMState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static uint64_t integratorcm_read(void *opaque, hwaddr offset,
                                   unsigned size)
 {

@@ -309,6 +330,19 @@ typedef struct icp_pic_state {
     qemu_irq parent_fiq;
 } icp_pic_state;
 
+static const VMStateDescription vmstate_icp_pic = {
+    .name = "icp_pic",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(level, icp_pic_state),
+        VMSTATE_UINT32(irq_enabled, icp_pic_state),
+        VMSTATE_UINT32(fiq_enabled, icp_pic_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void icp_pic_update(icp_pic_state *s)
 {
     uint32_t flags;

@@ -438,6 +472,17 @@ typedef struct ICPCtrlRegsState {
 #define ICP_INTREG_WPROT        (1 << 0)
 #define ICP_INTREG_CARDIN       (1 << 3)
 
+static const VMStateDescription vmstate_icp_control = {
+    .name = "icp_control",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(intreg_state, ICPCtrlRegsState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {

@@ -640,6 +685,21 @@ static void core_class_init(ObjectClass *klass, void *data)
 
     dc->props = core_properties;
     dc->realize = integratorcm_realize;
+    dc->vmsd = &vmstate_integratorcm;
+}
+
+static void icp_pic_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_icp_pic;
+}
+
+static void icp_control_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_icp_control;
 }
 
 static const TypeInfo core_info = {

@@ -655,6 +715,7 @@ static const TypeInfo icp_pic_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(icp_pic_state),
     .instance_init = icp_pic_init,
+    .class_init    = icp_pic_class_init,
 };
 
 static const TypeInfo icp_ctrl_regs_info = {

@@ -662,6 +723,7 @@ static const TypeInfo icp_ctrl_regs_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(ICPCtrlRegsState),
     .instance_init = icp_control_init,
+    .class_init    = icp_control_class_init,
 };
 
 static void integratorcp_register_types(void)
author
#24 lore raw

PATCH v7 8/14 savevm: add public save_vmstate function

In reply to Pavel Dovgalyuk

This patch introduces save_vmstate function to allow saving and loading vmstates from the replay module.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 include/sysemu/sysemu.h |    1 +
 migration/savevm.c      |   33 ++++++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 11 deletions(-)
include/sysemu/sysemu.h +1 -0
index cccde56..4cdba34 100644
@@ -75,6 +75,7 @@ void qemu_add_machine_init_done_notifier(Notifier *notify);
 void qemu_remove_machine_init_done_notifier(Notifier *notify);
 
 void hmp_savevm(Monitor *mon, const QDict *qdict);
+int save_vmstate(Monitor *mon, const char *name);
 int load_vmstate(const char *name);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
migration/savevm.c +22 -11
index f9c06e9..1e69225 100644
@@ -2039,38 +2039,40 @@ int qemu_loadvm_state(QEMUFile *f)
     return ret;
 }
 
-void hmp_savevm(Monitor *mon, const QDict *qdict)
+int save_vmstate(Monitor *mon, const char *name)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
-    int ret;
+    int ret = -1;
     QEMUFile *f;
     int saved_vm_running;
     uint64_t vm_state_size;
     qemu_timeval tv;
     struct tm tm;
-    const char *name = qdict_get_try_str(qdict, "name");
     Error *local_err = NULL;
     AioContext *aio_context;
 
     if (!bdrv_all_can_snapshot(&bs)) {
         monitor_printf(mon, "Device '%s' is writable but does not "
                        "support snapshots.\n", bdrv_get_device_name(bs));
-        return;
+        return ret;
     }
 
     /* Delete old snapshots of the same name */
-    if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
-        error_reportf_err(local_err,
-                          "Error while deleting snapshot on device '%s': ",
-                          bdrv_get_device_name(bs1));
-        return;
+    if (name) {
+        ret = bdrv_all_delete_snapshot(name, &bs1, &local_err);
+        if (ret < 0) {
+            error_reportf_err(local_err,
+                              "Error while deleting snapshot on device '%s': ",
+                              bdrv_get_device_name(bs1));
+            return ret;
+        }
     }
 
     bs = bdrv_all_find_vmstate_bs();
     if (bs == NULL) {
         monitor_printf(mon, "No block device can accept snapshots\n");
-        return;
+        return ret;
     }
     aio_context = bdrv_get_aio_context(bs);
 

@@ -2079,7 +2081,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     ret = global_state_store();
     if (ret) {
         monitor_printf(mon, "Error saving global state\n");
-        return;
+        return ret;
     }
     vm_stop(RUN_STATE_SAVE_VM);
 

@@ -2125,13 +2127,22 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     if (ret < 0) {
         monitor_printf(mon, "Error while creating snapshot on '%s'\n",
                        bdrv_get_device_name(bs));
+        goto the_end;
     }
 
+    ret = 0;
+
  the_end:
     aio_context_release(aio_context);
     if (saved_vm_running) {
         vm_start();
     }
+    return ret;
+}
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+    save_vmstate(mon, qdict_get_try_str(qdict, "name"));
 }
 
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
author
#25 lore raw

PATCH v7 9/14 replay: save/load initial state

In reply to Pavel Dovgalyuk

This patch implements initial vmstate creation or loading at the start of record/replay. It is needed for rewinding the execution in the replay mode.

v4 changes:
- snapshots are not created by default anymore

v3 changes:
- added rrsnapshot option

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 docs/replay.txt          |   16 ++++++++++++++++
 include/sysemu/replay.h  |    9 +++++++++
 qemu-options.hx          |    8 ++++++--
 replay/replay-snapshot.c |   17 +++++++++++++++++
 replay/replay.c          |    5 +++++
 vl.c                     |    7 ++++++-
 6 files changed, 59 insertions(+), 3 deletions(-)
docs/replay.txt +16 -0
index 347b2ff..03e1931 100644
@@ -196,6 +196,22 @@ is recorded to the log. In replay phase the queue is matched with
 events read from the log. Therefore block devices requests are processed
 deterministically.
 
+Snapshotting
+------------
+
+New VM snapshots may be created in replay mode. They can be used later
+to recover the desired VM state. All VM states created in replay mode
+are associated with the moment of time in the replay scenario.
+After recovering the VM state replay will start from that position.
+
+Default starting snapshot name may be specified with icount field
+rrsnapshot as follows:
+ -icount shift=7,rr=record,rrfile=replay.bin,rrsnapshot=snapshot_name
+
+This snapshot is created at start of recording and restored at start
+of replaying. It also can be loaded while replaying to roll back
+the execution.
+
 Network devices
 ---------------
include/sysemu/replay.h +9 -0
index abb35ca..740b425 100644
@@ -43,6 +43,9 @@ typedef struct ReplayNetState ReplayNetState;
 
 extern ReplayMode replay_mode;
 
+/* Name of the initial VM snapshot */
+extern char *replay_snapshot;
+
 /* Replay process control functions */
 
 /*! Enables recording or saving event log with specified parameters */

@@ -149,4 +152,10 @@ void replay_unregister_net(ReplayNetState *rns);
 void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
                              const struct iovec *iov, int iovcnt);
 
+/* VM state operations */
+
+/*! Called at the start of execution.
+    Loads or saves initial vmstate depending on execution mode. */
+void replay_vmstate_init(void);
+
 #endif
qemu-options.hx +6 -2
index c534a2f..32c7d2b 100644
@@ -3390,12 +3390,12 @@ re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [shift=N|auto][,align=on|off][,sleep=on|off,rr=record|replay,rrfile=<filename>]\n" \
+    "-icount [shift=N|auto][,align=on|off][,sleep=on|off,rr=record|replay,rrfile=<filename>,rrsnapshot=<snapshot>]\n" \
     "                enable virtual instruction counter with 2^N clock ticks per\n" \
     "                instruction, enable aligning the host and virtual clocks\n" \
     "                or disable real time cpu sleeping\n", QEMU_ARCH_ALL)
 STEXI
-@item -icount [shift=@var{N}|auto][,rr=record|replay,rrfile=@var{filename}]
+@item -icount [shift=@var{N}|auto][,rr=record|replay,rrfile=@var{filename},rrsnapshot=@var{snapshot}]
 @findex -icount
 Enable virtual instruction counter.  The virtual cpu will execute one
 instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified

@@ -3428,6 +3428,10 @@ when the shift value is high (how high depends on the host machine).
 When @option{rr} option is specified deterministic record/replay is enabled.
 Replay log is written into @var{filename} file in record mode and
 read from this file in replay mode.
+
+Option rrsnapshot is used to create new vm snapshot named @var{snapshot}
+at the start of execution recording. In replay mode this option is used
+to load the initial VM state.
 ETEXI
 
 DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
replay/replay-snapshot.c +17 -0
index 4980597..f2cf748 100644
@@ -59,3 +59,20 @@ void replay_vmstate_register(void)
 {
     vmstate_register(NULL, 0, &vmstate_replay, &replay_state);
 }
+
+void replay_vmstate_init(void)
+{
+    if (replay_snapshot) {
+        if (replay_mode == REPLAY_MODE_RECORD) {
+            if (save_vmstate(cur_mon, replay_snapshot) != 0) {
+                error_report("Could not create snapshot for icount record");
+                exit(1);
+            }
+        } else if (replay_mode == REPLAY_MODE_PLAY) {
+            if (load_vmstate(replay_snapshot) != 0) {
+                error_report("Could not load snapshot for icount replay");
+                exit(1);
+            }
+        }
+    }
+}
replay/replay.c +5 -0
index 7f27cf1..1835b99 100644
@@ -26,6 +26,7 @@
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
 ReplayMode replay_mode = REPLAY_MODE_NONE;
+char *replay_snapshot;
 
 /* Name of replay file  */
 static char *replay_filename;

@@ -292,6 +293,7 @@ void replay_configure(QemuOpts *opts)
         exit(1);
     }
 
+    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
     replay_vmstate_register();
     replay_enable(fname, mode);
 

@@ -346,6 +348,9 @@ void replay_finish(void)
         replay_filename = NULL;
     }
 
+    g_free(replay_snapshot);
+    replay_snapshot = NULL;
+
     replay_finish_events();
     replay_mutex_destroy();
 }
vl.c +6 -1
index c643d3f..52a6ac8 100644
@@ -462,6 +462,9 @@ static QemuOptsList qemu_icount_opts = {
         }, {
             .name = "rrfile",
             .type = QEMU_OPT_STRING,
+        }, {
+            .name = "rrsnapshot",
+            .type = QEMU_OPT_STRING,
         },
         { /* end of list */ }
     },

@@ -4651,7 +4654,9 @@ int main(int argc, char **argv, char **envp)
     replay_checkpoint(CHECKPOINT_RESET);
     qemu_system_reset(VMRESET_SILENT);
     register_global_state();
-    if (loadvm) {
+    if (replay_mode != REPLAY_MODE_NONE) {
+        replay_vmstate_init();
+    } else if (loadvm) {
         if (load_vmstate(loadvm) < 0) {
             autostart = 0;
         }
author
#26 lore raw

PATCH v7 10/14 block: implement bdrv_snapshot_goto for blkreplay

In reply to Pavel Dovgalyuk

This patch enables making snapshots with blkreplay used in block devices.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 block/blkreplay.c |    8 ++++++++
 1 file changed, 8 insertions(+)
block/blkreplay.c +8 -0
index a741654..8a03d62 100644
@@ -130,6 +130,12 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
     return ret;
 }
 
+static int blkreplay_snapshot_goto(BlockDriverState *bs,
+                                   const char *snapshot_id)
+{
+    return bdrv_snapshot_goto(bs->file->bs, snapshot_id);
+}
+
 static BlockDriver bdrv_blkreplay = {
     .format_name            = "blkreplay",
     .protocol_name          = "blkreplay",

@@ -145,6 +151,8 @@ static BlockDriver bdrv_blkreplay = {
     .bdrv_co_pwrite_zeroes  = blkreplay_co_pwrite_zeroes,
     .bdrv_co_pdiscard       = blkreplay_co_pdiscard,
     .bdrv_co_flush          = blkreplay_co_flush,
+
+    .bdrv_snapshot_goto     = blkreplay_snapshot_goto,
 };
 
 static void bdrv_blkreplay_init(void)
author
#27 lore raw

PATCH v7 11/14 blkreplay: create temporary overlay for underlaying devices

In reply to Pavel Dovgalyuk

This patch allows using '-snapshot' behavior in record/replay mode. blkreplay layer creates temporary overlays on top of underlaying disk images. It is needed, because creating an overlay over blkreplay breaks the determinism.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 block/blkreplay.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 stubs/replay.c    |    1 +
 vl.c              |    2 +
 3 files changed, 78 insertions(+), 1 deletion(-)
block/blkreplay.c +76 -0
index 8a03d62..172642f 100644
@@ -14,12 +14,76 @@
 #include "block/block_int.h"
 #include "sysemu/replay.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qstring.h"
 
 typedef struct Request {
     Coroutine *co;
     QEMUBH *bh;
 } Request;
 
+static BlockDriverState *blkreplay_append_snapshot(BlockDriverState *bs,
+                                                   Error **errp)
+{
+    int ret;
+    BlockDriverState *bs_snapshot;
+    int64_t total_size;
+    QemuOpts *opts = NULL;
+    char tmp_filename[PATH_MAX + 1];
+    QDict *snapshot_options = qdict_new();
+
+    /* Prepare options QDict for the overlay file */
+    qdict_put(snapshot_options, "file.driver",
+              qstring_from_str("file"));
+    qdict_put(snapshot_options, "driver",
+              qstring_from_str("qcow2"));
+
+    /* Create temporary file */
+    ret = get_tmp_filename(tmp_filename, PATH_MAX + 1);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not get temporary filename");
+        goto out;
+    }
+    qdict_put(snapshot_options, "file.filename",
+              qstring_from_str(tmp_filename));
+
+    /* Get the required size from the image */
+    total_size = bdrv_getlength(bs);
+    if (total_size < 0) {
+        error_setg_errno(errp, -total_size, "Could not get image size");
+        goto out;
+    }
+
+    opts = qemu_opts_create(bdrv_qcow2.create_opts, NULL, 0, &error_abort);
+    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size, &error_abort);
+    ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, errp);
+    qemu_opts_del(opts);
+    if (ret < 0) {
+        error_prepend(errp, "Could not create temporary overlay '%s': ",
+                      tmp_filename);
+        goto out;
+    }
+
+    bs_snapshot = bdrv_open(NULL, NULL, snapshot_options,
+                            BDRV_O_RDWR | BDRV_O_TEMPORARY, errp);
+    snapshot_options = NULL;
+    if (!bs_snapshot) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* bdrv_append() consumes a strong reference to bs_snapshot (i.e. it will
+     * call bdrv_unref() on it), so in order to be able to return one, we have
+     * to increase bs_snapshot's refcount here */
+    bdrv_ref(bs_snapshot);
+    bdrv_append(bs_snapshot, bs);
+
+    return bs_snapshot;
+
+out:
+    QDECREF(snapshot_options);
+    return NULL;
+}
+
 static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
                           Error **errp)
 {

@@ -35,6 +99,14 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    /* Add temporary snapshot to preserve the image */
+    if (!replay_snapshot
+        && !blkreplay_append_snapshot(bs->file->bs, &local_err)) {
+        ret = -EINVAL;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
     ret = 0;
 fail:
     if (ret < 0) {

@@ -45,6 +117,10 @@ fail:
 
 static void blkreplay_close(BlockDriverState *bs)
 {
+    if (!replay_snapshot) {
+        /* Unref created snapshot file */
+        bdrv_unref(bs->file->bs);
+    }
 }
 
 static int64_t blkreplay_getlength(BlockDriverState *bs)
stubs/replay.c +1 -0
index d9a6da9..e43e467 100644
@@ -3,6 +3,7 @@
 #include "sysemu/sysemu.h"
 
 ReplayMode replay_mode;
+char *replay_snapshot;
 
 int64_t replay_save_clock(unsigned int kind, int64_t clock)
 {
vl.c +1 -1
index 52a6ac8..def0520 100644
@@ -4474,7 +4474,7 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* open the virtual block devices */
-    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+    if (snapshot) {
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
                           NULL, NULL);
     }
author
#28 lore raw

PATCH v7 12/14 replay: disable default snapshot for record/replay

In reply to Pavel Dovgalyuk

This patch disables setting '-snapshot' option on by default in record/replay mode. This is needed for creating vmstates in record and replay modes.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 vl.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
vl.c +7 -1
index def0520..bddb2bc 100644
@@ -3198,7 +3198,13 @@ int main(int argc, char **argv, char **envp)
                 drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
                 break;
             case QEMU_OPTION_snapshot:
-                snapshot = 1;
+                {
+                    Error *blocker = NULL;
+                    snapshot = 1;
+                    error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
+                               "-snapshot");
+                    replay_add_blocker(blocker);
+                }
                 break;
             case QEMU_OPTION_hdachs:
                 {
author
#29 lore raw

PATCH v7 13/14 audio: make audio poll timer deterministic

In reply to Pavel Dovgalyuk

This patch changes resetting strategy of the audio polling timer. It does not change expiration time if the timer is already set.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 audio/audio.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
audio/audio.c +4 -2
index c845a44..1ee95a5 100644
@@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void)
 static void audio_reset_timer (AudioState *s)
 {
     if (audio_is_timer_needed ()) {
-        timer_mod (s->ts,
-            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
+        if (!timer_pending(s->ts)) {
+            timer_mod (s->ts,
+                qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
+        }
     }
     else {
         timer_del (s->ts);
author
#30 lore raw

PATCH v7 14/14 replay: add record/replay for audio passthrough

In reply to Pavel Dovgalyuk

This patch adds recording and replaying audio data. Is saves synchronization information for audio out and inputs from the microphone.

Signed-off-by Pavel Dovgalyuk <[email protected]>
---
 audio/audio.c            |    9 ++++-
 audio/audio.h            |    5 +++
 audio/mixeng.c           |   31 ++++++++++++++++++
 docs/replay.txt          |    7 ++++
 include/sysemu/replay.h  |    7 ++++
 replay/Makefile.objs     |    1 +
 replay/replay-audio.c    |   79 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h |    4 ++
 8 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 replay/replay-audio.c
audio/audio.c +7 -2
index 1ee95a5..415ee1b 100644
@@ -28,6 +28,7 @@
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
+#include "sysemu/replay.h"
 
 #define AUDIO_CAP "audio"
 #include "audio_int.h"

@@ -1389,6 +1390,7 @@ static void audio_run_out (AudioState *s)
 
         prev_rpos = hw->rpos;
         played = hw->pcm_ops->run_out (hw, live);
+        replay_audio_out(&played);
         if (audio_bug (AUDIO_FUNC, hw->rpos >= hw->samples)) {
             dolog ("hw->rpos=%d hw->samples=%d played=%d\n",
                    hw->rpos, hw->samples, played);

@@ -1452,9 +1454,12 @@ static void audio_run_in (AudioState *s)
 
     while ((hw = audio_pcm_hw_find_any_enabled_in (hw))) {
         SWVoiceIn *sw;
-        int captured, min;
+        int captured = 0, min;
 
-        captured = hw->pcm_ops->run_in (hw);
+        if (replay_mode != REPLAY_MODE_PLAY) {
+            captured = hw->pcm_ops->run_in (hw);
+        }
+        replay_audio_in(&captured, hw->conv_buf, &hw->wpos, hw->samples);
 
         min = audio_pcm_hw_find_min_in (hw);
         hw->total_samples_captured += captured - min;
audio/audio.h +5 -0
index c3c5198..f4339a1 100644
@@ -166,4 +166,9 @@ int wav_start_capture (CaptureState *s, const char *path, int freq,
 bool audio_is_cleaning_up(void);
 void audio_cleanup(void);
 
+void audio_sample_to_uint64(void *samples, int pos,
+                            uint64_t *left, uint64_t *right);
+void audio_sample_from_uint64(void *samples, int pos,
+                            uint64_t left, uint64_t right);
+
 #endif /* QEMU_AUDIO_H */
audio/mixeng.c +31 -0
index 66c0328..c23508e 100644
@@ -267,6 +267,37 @@ f_sample *mixeng_clip[2][2][2][3] = {
     }
 };
 
+
+void audio_sample_to_uint64(void *samples, int pos,
+                            uint64_t *left, uint64_t *right)
+{
+    struct st_sample *sample = samples;
+    sample += pos;
+#ifdef FLOAT_MIXENG
+    error_report(
+        "Coreaudio and floating point samples are not supported by replay yet");
+    abort();
+#else
+    *left = sample->l;
+    *right = sample->r;
+#endif
+}
+
+void audio_sample_from_uint64(void *samples, int pos,
+                            uint64_t left, uint64_t right)
+{
+    struct st_sample *sample = samples;
+    sample += pos;
+#ifdef FLOAT_MIXENG
+    error_report(
+        "Coreaudio and floating point samples are not supported by replay yet");
+    abort();
+#else
+    sample->l = left;
+    sample->r = right;
+#endif
+}
+
 /*
  * August 21, 1998
  * Copyright 1998 Fabrice Bellard.
docs/replay.txt +7 -0
index 03e1931..486c1e0 100644
@@ -225,3 +225,10 @@ recording the virtual machine this filter puts all packets coming from
 the outer world into the log. In replay mode packets from the log are
 injected into the network device. All interactions with network backend
 in replay mode are disabled.
+
+Audio devices
+-------------
+
+Audio data is recorded and replay automatically. The command line for recording
+and replaying must contain identical specifications of audio hardware, e.g.:
+ -soundhw ac97
include/sysemu/replay.h +7 -0
index 740b425..073ec59 100644
@@ -152,6 +152,13 @@ void replay_unregister_net(ReplayNetState *rns);
 void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
                              const struct iovec *iov, int iovcnt);
 
+/* Audio */
+
+/*! Saves/restores number of played samples of audio out operation. */
+void replay_audio_out(int *played);
+/*! Saves/restores recorded samples of audio in operation. */
+void replay_audio_in(int *recorded, void *samples, int *wpos, int size);
+
 /* VM state operations */
 
 /*! Called at the start of execution.
replay/Makefile.objs +1 -0
index b2afd40..cee6539 100644
@@ -6,3 +6,4 @@ common-obj-y += replay-input.o
 common-obj-y += replay-char.o
 common-obj-y += replay-snapshot.o
 common-obj-y += replay-net.o
+common-obj-y += replay-audio.o
\ No newline at end of file
replay/replay-audio.c new file +79 -0

79 lines not on this page · raw message

replay/replay-internal.h +4 -0
index c26d079..ed66ed8 100644
@@ -29,6 +29,10 @@ enum ReplayEvents {
     /* for character device read all event */
     EVENT_CHAR_READ_ALL,
     EVENT_CHAR_READ_ALL_ERROR,
+    /* for audio out event */
+    EVENT_AUDIO_OUT,
+    /* for audio in event */
+    EVENT_AUDIO_IN,
     /* for clock read/writes */
     /* some of greater codes are reserved for clocks */
     EVENT_CLOCK,
bot
#31 lore raw
In reply to Pavel Dovgalyuk
Your series seems to have some coding style problems. See output below for more information:

Hi,

Your series seems to have some coding style problems. See output below for more information:

Type: series
Subject: [Qemu-devel] [PATCH v7 00/14] replay additions
Message-id: 20170124071654.4572.41407.stgit@PASHA-ISP

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do

    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))

done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu

 - [tag update]      patchew/[email protected] -> patchew/[email protected]
 - [tag update]      patchew/[email protected] -> patchew/[email protected]
 * [new tag]         patchew/20170124071654.4572.41407.stgit@PASHA-ISP -> patchew/20170124071654.4572.41407.stgit@PASHA-ISP

Switched to a new branch 'test'
0191b3c replay: add record/replay for audio passthrough ca532c5 audio: make audio poll timer deterministic 8690408 replay: disable default snapshot for record/replay 2d11131 blkreplay: create temporary overlay for underlaying devices 7d471b1 block: implement bdrv_snapshot_goto for blkreplay eab8ef5 replay: save/load initial state
870fbf1 savevm: add public save_vmstate function
aff7d02 integratorcp: adding vmstate for save/restore f604803 replay: don't use rtc clock on loadvm phase b01b9f2 apic: save apic_delivered flag
06b4384 icount: exit cpu loop on expire
8995a01 replay: exception replay fix
1de2b63 replay: improve interrupt handling
84b9049 icount: update instruction counter on apic patching

=== OUTPUT BEGIN ===
Checking PATCH 1/14: icount: update instruction counter on apic patching... Checking PATCH 2/14: replay: improve interrupt handling...
Checking PATCH 3/14: replay: exception replay fix...
Checking PATCH 4/14: icount: exit cpu loop on expire...
Checking PATCH 5/14: apic: save apic_delivered flag...
Checking PATCH 6/14: replay: don't use rtc clock on loadvm phase... Checking PATCH 7/14: integratorcp: adding vmstate for save/restore... Checking PATCH 8/14: savevm: add public save_vmstate function... Checking PATCH 9/14: replay: save/load initial state...
Checking PATCH 10/14: block: implement bdrv_snapshot_goto for blkreplay... Checking PATCH 11/14: blkreplay: create temporary overlay for underlaying devices... Checking PATCH 12/14: replay: disable default snapshot for record/replay... Checking PATCH 13/14: audio: make audio poll timer deterministic...
ERROR: space prohibited between function name and open parenthesis '(' #23: FILE: audio/audio.c:1116:
+ timer_mod (s->ts,

total: 1 errors, 0 warnings, 12 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

Checking PATCH 14/14: replay: add record/replay for audio passthrough...
ERROR: space prohibited between function name and open parenthesis '(' #41: FILE: audio/audio.c:1460:
+ captured = hw->pcm_ops->run_in (hw);

total: 1 errors, 0 warnings, 190 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1

---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [email protected]

#32 lore raw
In reply to Pavel Dovgalyuk
Quoted 22 lines · Pavel Dovgalyuk

This set of patches includes several fixes for replay and vmstate.

This patches add rrsnapshot option for icount. rrshapshot option creates start snapshot at record and loads it at replay. It allows preserving the state of disk images used by virtual machine. This vm state can also use used to roll back the execution while replaying.

With these patches operations with audio devices can also be recorded and replayed. All interactions with passthrough audio (including microphone input) are recorded automatically when -soundhw is specified at the command line.

This set of patches includes fixes and additions for icount and record/replay implementation:
- VM start/stop in replay mode
- overlay creation for blkreplay filter
- rrsnapshot option for record/replay
- vmstate fix for integratorcp ARM platform
- vmstate fixes for apic and rtc
- fixes for icount record/replay mode
- record/replay for audio devices

Quoted 4 lines

v7 changes:
- Fixed exception replaying when TB cache is full and
when tb_find is called when there are no instructions about to execute
- Added record/replay for audio devices

Queued patches 1, 2, 6, 8, 9.

Please post patches for ARM, block and audio maintainers separately if you don't get attention soon.

Paolo

Quoted 62 lines

v6 changes:
- Added overlay creation for blkreplay driver
- Fixed vmstate loading for apic and rtc
- Fixed instruction counting for apic instruction patching

v5 changes:
- Recording is stopped when initial snapshot cannot be created
- Minor changes

v4 changes:
- Overlay option is removed from blkreplay driver (as suggested by Paolo Bonzini)
- Minor changes

v3 changes:
- Added rrsnapshot option for specifying the initial snapshot name (as suggested by Paolo Bonzini)
- Minor changes

---

Pavel Dovgalyuk (14):
icount: update instruction counter on apic patching
replay: improve interrupt handling
replay: exception replay fix
icount: exit cpu loop on expire
apic: save apic_delivered flag
replay: don't use rtc clock on loadvm phase
integratorcp: adding vmstate for save/restore
savevm: add public save_vmstate function
replay: save/load initial state
block: implement bdrv_snapshot_goto for blkreplay
blkreplay: create temporary overlay for underlaying devices
replay: disable default snapshot for record/replay
audio: make audio poll timer deterministic
replay: add record/replay for audio passthrough

 audio/audio.c                   |   15 +++++--
 audio/audio.h                   |    5 ++
 audio/mixeng.c                  |   31 ++++++++++++++
 block/blkreplay.c               |   84 +++++++++++++++++++++++++++++++++++++++
 cpu-exec.c                      |   21 ++++++++--
 docs/replay.txt                 |   23 +++++++++++
 hw/arm/integratorcp.c           |   62 +++++++++++++++++++++++++++++
 hw/i386/kvmvapic.c              |    6 +++
 hw/intc/apic_common.c           |   32 +++++++++++++++
 hw/timer/mc146818rtc.c          |   14 +++++--
 include/hw/i386/apic_internal.h |    2 +
 include/sysemu/replay.h         |   16 +++++++
 include/sysemu/sysemu.h         |    1 
 migration/savevm.c              |   33 ++++++++++-----
 qemu-options.hx                 |    8 +++-
 replay/Makefile.objs            |    1 
 replay/replay-audio.c           |   79 +++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h        |    4 ++
 replay/replay-snapshot.c        |   17 ++++++++
 replay/replay.c                 |    5 ++
 stubs/replay.c                  |    1 
 target/i386/seg_helper.c        |    1 
 vl.c                            |   17 +++++++-
 23 files changed, 450 insertions(+), 28 deletions(-)
 create mode 100644 replay/replay-audio.c