All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/15] Reverse debugging
@ 2020-09-02  8:15 Pavel Dovgalyuk
  2020-09-02  8:15 ` [PATCH v3 01/15] replay: don't record interrupt poll Pavel Dovgalyuk
                   ` (14 more replies)
  0 siblings, 15 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

GDB remote protocol supports reverse debugging of the targets.
It includes 'reverse step' and 'reverse continue' operations.
The first one finds the previous step of the execution,
and the second one is intended to stop at the last breakpoint that
would happen when the program is executed normally.

Reverse debugging is possible in the replay mode, when at least
one snapshot was created at the record or replay phase.
QEMU can use these snapshots for travelling back in time with GDB.

Running the execution in replay mode allows using GDB reverse debugging
commands:
 - reverse-stepi (or rsi): Steps one instruction to the past.
   QEMU loads on of the prior snapshots and proceeds to the desired
   instruction forward. When that step is reaches, execution stops.
 - reverse-continue (or rc): Runs execution "backwards".
   QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
   and replaying the execution. Then QEMU loads snapshots again and
   replays to the latest breakpoint. When there are no breakpoints in
   the examined section of the execution, QEMU finds one more snapshot
   and tries again. After the first snapshot is processed, execution
   stops at this snapshot.

The set of patches include the following modifications:
 - gdbstub update for reverse debugging support
 - functions that automatically perform reverse step and reverse
   continue operations
 - hmp/qmp commands for manipulating the replay process
 - improvement of the snapshotting for saving the execution step
   in the snapshot parameters
 - avocado-based acceptance tests for reverse debugging

The patches are available in the repository:
https://github.com/ispras/qemu/tree/rr-200901

v3 changes:
 - rebased to support the new build system
 - bumped avocado framework version for using fixed remote gdb client
v2 changes:
 - rebased to the latest upstream version
 - fixed replaying of the POLL interrupts after the latest debug changes

---

Pavel Dovgaluk (14):
      replay: provide an accessor for rr filename
      qcow2: introduce icount field for snapshots
      migration: introduce icount field for snapshots
      iotests: update snapshot test for new output format
      qapi: introduce replay.json for record/replay-related stuff
      replay: introduce info hmp/qmp command
      replay: introduce breakpoint at the specified step
      replay: implement replay-seek command
      replay: flush rr queue before loading the vmstate
      gdbstub: add reverse step support in replay mode
      gdbstub: add reverse continue support in replay mode
      replay: describe reverse debugging in docs/replay.txt
      tests: bump avocado version
      tests/acceptance: add reverse debugging test

Pavel Dovgalyuk (1):
      replay: don't record interrupt poll


 MAINTAINERS                           |    2 
 accel/tcg/cpu-exec.c                  |   11 +
 accel/tcg/translator.c                |    1 
 block/qapi.c                          |   18 +-
 block/qcow2-snapshot.c                |    9 +
 block/qcow2.h                         |    3 
 blockdev.c                            |   10 +
 docs/interop/qcow2.txt                |    5 +
 docs/replay.txt                       |   33 +++
 exec.c                                |    8 +
 gdbstub.c                             |   63 ++++++
 hmp-commands-info.hx                  |   11 +
 hmp-commands.hx                       |   50 +++++
 include/block/snapshot.h              |    1 
 include/monitor/hmp.h                 |    4 
 include/sysemu/replay.h               |   24 ++
 migration/savevm.c                    |   17 +-
 qapi/block-core.json                  |   11 +
 qapi/meson.build                      |    1 
 qapi/misc.json                        |   18 --
 qapi/qapi-schema.json                 |    1 
 qapi/replay.json                      |  121 ++++++++++++
 replay/meson.build                    |    1 
 replay/replay-debugging.c             |  325 +++++++++++++++++++++++++++++++++
 replay/replay-events.c                |    4 
 replay/replay-internal.h              |    6 -
 replay/replay.c                       |   22 ++
 softmmu/cpus.c                        |   19 ++
 stubs/replay.c                        |   15 ++
 tests/Makefile.include                |    2 
 tests/acceptance/reverse_debugging.py |  203 +++++++++++++++++++++
 tests/qemu-iotests/267.out            |   48 ++---
 tests/requirements.txt                |    2 
 33 files changed, 1003 insertions(+), 66 deletions(-)
 create mode 100644 qapi/replay.json
 create mode 100644 replay/replay-debugging.c
 create mode 100644 tests/acceptance/reverse_debugging.py

--
Pavel Dovgalyuk


^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v3 01/15] replay: don't record interrupt poll
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
@ 2020-09-02  8:15 ` Pavel Dovgalyuk
  2020-09-07 10:17   ` Alex Bennée
  2020-09-02  8:15 ` [PATCH v3 02/15] replay: provide an accessor for rr filename Pavel Dovgalyuk
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

Interrupt poll is not a real interrupt event. It is needed only for
thread safety. This interrupt is used for i386 and converted
to hardware interrupt by cpu_handle_interrupt function.
Therefore it is not needed to be recorded, because hardware
interrupt will be recorded after converting.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 accel/tcg/cpu-exec.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 66d38f9d85..28aff1bb1e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -429,8 +429,7 @@ static inline bool cpu_handle_halt(CPUState *cpu)
 {
     if (cpu->halted) {
 #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
-        if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
-            && replay_interrupt()) {
+        if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
             X86CPU *x86_cpu = X86_CPU(cpu);
             qemu_mutex_lock_iothread();
             apic_poll_irq(x86_cpu->apic_state);
@@ -587,7 +586,13 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
            and via longjmp via cpu_loop_exit.  */
         else {
             if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
-                replay_interrupt();
+                if (true
+#if defined(TARGET_I386)
+                    && !(interrupt_request & CPU_INTERRUPT_POLL)
+#endif
+                ) {
+                    replay_interrupt();
+                }
                 /*
                  * After processing the interrupt, ensure an EXCP_DEBUG is
                  * raised when single-stepping so that GDB doesn't miss the



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 02/15] replay: provide an accessor for rr filename
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
  2020-09-02  8:15 ` [PATCH v3 01/15] replay: don't record interrupt poll Pavel Dovgalyuk
@ 2020-09-02  8:15 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 03/15] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch adds an accessor function for the name of the record/replay
log file. Adding an accessor instead of making variable global,
prevents accidental modification of this variable by other modules.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/sysemu/replay.h |    2 ++
 replay/replay.c         |    5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 5471bb514d..c9c896ae8d 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -72,6 +72,8 @@ void replay_start(void);
 void replay_finish(void);
 /*! Adds replay blocker with the specified error description */
 void replay_add_blocker(Error *reason);
+/* Returns name of the replay log file */
+const char *replay_get_filename(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay.c b/replay/replay.c
index 83ed9e0e24..42e82f7bc7 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -399,3 +399,8 @@ void replay_add_blocker(Error *reason)
 {
     replay_blockers = g_slist_prepend(replay_blockers, reason);
 }
+
+const char *replay_get_filename(void)
+{
+    return replay_filename;
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 03/15] qcow2: introduce icount field for snapshots
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
  2020-09-02  8:15 ` [PATCH v3 01/15] replay: don't record interrupt poll Pavel Dovgalyuk
  2020-09-02  8:15 ` [PATCH v3 02/15] replay: provide an accessor for rr filename Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 04/15] migration: " Pavel Dovgalyuk
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch introduces the icount field for saving within the snapshot.
It is required for navigation between the snapshots in record/replay mode.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-snapshot.c |    7 +++++++
 block/qcow2.h          |    3 +++
 docs/interop/qcow2.txt |    5 +++++
 3 files changed, 15 insertions(+)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 2756b37d24..d14e7be1aa 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -164,6 +164,12 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair,
             sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
         }
 
+        if (sn->extra_data_size >= endof(QCowSnapshotExtraData, icount)) {
+            sn->icount = be64_to_cpu(extra.icount);
+        } else {
+            sn->icount = -1ULL;
+        }
+
         if (sn->extra_data_size > sizeof(extra)) {
             uint64_t extra_data_end;
             size_t unknown_extra_data_size;
@@ -333,6 +339,7 @@ int qcow2_write_snapshots(BlockDriverState *bs)
         memset(&extra, 0, sizeof(extra));
         extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size);
         extra.disk_size = cpu_to_be64(sn->disk_size);
+        extra.icount = cpu_to_be64(sn->icount);
 
         id_str_size = strlen(sn->id_str);
         name_size = strlen(sn->name);
diff --git a/block/qcow2.h b/block/qcow2.h
index 065ec3df0b..c39a1e1265 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -200,6 +200,7 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
 typedef struct QEMU_PACKED QCowSnapshotExtraData {
     uint64_t vm_state_size_large;
     uint64_t disk_size;
+    uint64_t icount;
 } QCowSnapshotExtraData;
 
 
@@ -213,6 +214,8 @@ typedef struct QCowSnapshot {
     uint32_t date_sec;
     uint32_t date_nsec;
     uint64_t vm_clock_nsec;
+    /* icount value for the moment when snapshot was taken */
+    uint64_t icount;
     /* Size of all extra data, including QCowSnapshotExtraData if available */
     uint32_t extra_data_size;
     /* Data beyond QCowSnapshotExtraData, if any */
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 7da0d81df8..0463f761ef 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -707,6 +707,11 @@ Snapshot table entry:
 
                     Byte 48 - 55:   Virtual disk size of the snapshot in bytes
 
+                    Byte 56 - 63:   icount value which corresponds to
+                                    the record/replay instruction count
+                                    when the snapshot was taken. Set to -1
+                                    if icount was disabled
+
                     Version 3 images must include extra data at least up to
                     byte 55.
 



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 04/15] migration: introduce icount field for snapshots
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (2 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 03/15] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 05/15] iotests: update snapshot test for new output format Pavel Dovgalyuk
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

Saving icount as a parameters of the snapshot allows navigation between
them in the execution replay scenario.
This information can be used for finding a specific snapshot for proceeding
the recorded execution to the specific moment of the time.
E.g., 'reverse step' action (introduced in one of the following patches)
needs to load the nearest snapshot which is prior to the current moment
of time.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c             |   18 ++++++++++++++----
 block/qcow2-snapshot.c   |    2 ++
 blockdev.c               |   10 ++++++++++
 include/block/snapshot.h |    1 +
 migration/savevm.c       |    5 +++++
 qapi/block-core.json     |   10 ++++++++--
 stubs/replay.c           |    5 +++++
 7 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index afd9f3b4a7..3c73db54c4 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -226,6 +226,8 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
         info->date_nsec     = sn_tab[i].date_nsec;
         info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
         info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+        info->icount        = sn_tab[i].icount;
+        info->has_icount    = sn_tab[i].icount != -1ULL;
 
         info_list = g_new0(SnapshotInfoList, 1);
         info_list->value = info;
@@ -658,14 +660,15 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
 void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 {
     char date_buf[128], clock_buf[128];
+    char icount_buf[128] = {0};
     struct tm tm;
     time_t ti;
     int64_t secs;
     char *sizing = NULL;
 
     if (!sn) {
-        qemu_printf("%-10s%-20s%11s%20s%15s",
-                    "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
+                    "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
     } else {
         ti = sn->date_sec;
         localtime_r(&ti, &tm);
@@ -679,11 +682,16 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
                  (int)(secs % 60),
                  (int)((sn->vm_clock_nsec / 1000000) % 1000));
         sizing = size_to_str(sn->vm_state_size);
-        qemu_printf("%-10s%-20s%11s%20s%15s",
+        if (sn->icount != -1ULL) {
+            snprintf(icount_buf, sizeof(icount_buf),
+                "%"PRId64, sn->icount);
+        }
+        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
                     sn->id_str, sn->name,
                     sizing,
                     date_buf,
-                    clock_buf);
+                    clock_buf,
+                    icount_buf);
     }
     g_free(sizing);
 }
@@ -845,6 +853,8 @@ void bdrv_image_info_dump(ImageInfo *info)
                 .date_nsec = elem->value->date_nsec,
                 .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
                                  elem->value->vm_clock_nsec,
+                .icount = elem->value->has_icount ?
+                          elem->value->icount : -1ULL,
             };
 
             pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index d14e7be1aa..4b127ea6af 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -663,6 +663,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     sn->date_sec = sn_info->date_sec;
     sn->date_nsec = sn_info->date_nsec;
     sn->vm_clock_nsec = sn_info->vm_clock_nsec;
+    sn->icount = sn_info->icount;
     sn->extra_data_size = sizeof(QCowSnapshotExtraData);
 
     /* Allocate the L1 table of the snapshot and copy the current one there. */
@@ -1007,6 +1008,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         sn_info->date_sec = sn->date_sec;
         sn_info->date_nsec = sn->date_nsec;
         sn_info->vm_clock_nsec = sn->vm_clock_nsec;
+        sn_info->icount = sn->icount;
     }
     *psn_tab = sn_tab;
     return s->nb_snapshots;
diff --git a/blockdev.c b/blockdev.c
index 3848a9c8ab..622a436c94 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -59,6 +59,7 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/qtest.h"
 #include "sysemu/runstate.h"
+#include "sysemu/replay.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/main-loop.h"
@@ -1190,6 +1191,10 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
     info->vm_state_size = sn.vm_state_size;
     info->vm_clock_nsec = sn.vm_clock_nsec % 1000000000;
     info->vm_clock_sec = sn.vm_clock_nsec / 1000000000;
+    if (sn.icount != -1ULL) {
+        info->icount = sn.icount;
+        info->has_icount = true;
+    }
 
     return info;
 
@@ -1350,6 +1355,11 @@ static void internal_snapshot_prepare(BlkActionState *common,
     sn->date_sec = tv.tv_sec;
     sn->date_nsec = tv.tv_usec * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    if (replay_mode != REPLAY_MODE_NONE) {
+        sn->icount = replay_get_current_icount();
+    } else {
+        sn->icount = -1ULL;
+    }
 
     ret1 = bdrv_snapshot_create(bs, sn);
     if (ret1 < 0) {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 2bfcd57578..b0fe42993d 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -42,6 +42,7 @@ typedef struct QEMUSnapshotInfo {
     uint32_t date_sec; /* UTC date of the snapshot */
     uint32_t date_nsec;
     uint64_t vm_clock_nsec; /* VM clock relative to boot */
+    uint64_t icount; /* record/replay step */
 } QEMUSnapshotInfo;
 
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
diff --git a/migration/savevm.c b/migration/savevm.c
index 304d98ff78..74378f8ebd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2723,6 +2723,11 @@ int save_snapshot(const char *name, Error **errp)
     sn->date_sec = tv.tv_sec;
     sn->date_nsec = tv.tv_usec * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    if (replay_mode != REPLAY_MODE_NONE) {
+        sn->icount = replay_get_current_icount();
+    } else {
+        sn->icount = -1ULL;
+    }
 
     if (name) {
         ret = bdrv_snapshot_find(bs, old_sn, name);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index db08c58d78..0d8265a78e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -27,13 +27,18 @@
 #
 # @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
 #
+# @icount: Current instruction count. Appears when execution record/replay
+#          is enabled. Used for "time-traveling" to match the moment
+#          in the recorded execution with the snapshots. (since 5.2)
+#
 # Since: 1.3
 #
 ##
 { 'struct': 'SnapshotInfo',
   'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
             'date-sec': 'int', 'date-nsec': 'int',
-            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int',
+            '*icount': 'int' } }
 
 ##
 # @ImageInfoSpecificQCow2EncryptionBase:
@@ -5519,7 +5524,8 @@
 #                    "date-sec": 1000012,
 #                    "date-nsec": 10,
 #                    "vm-clock-sec": 100,
-#                    "vm-clock-nsec": 20
+#                    "vm-clock-nsec": 20,
+#                    "icount": 220414
 #      }
 #    }
 #
diff --git a/stubs/replay.c b/stubs/replay.c
index 5974ec1f50..eacb366aa8 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -88,3 +88,8 @@ int replay_read_random(void *buf, size_t len)
 {
     return 0;
 }
+
+uint64_t replay_get_current_icount(void)
+{
+    return 0;
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (3 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 04/15] migration: " Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-07 15:26   ` Alex Bennée
  2020-09-08 13:10   ` Eric Blake
  2020-09-02  8:16 ` [PATCH v3 06/15] qapi: introduce replay.json for record/replay-related stuff Pavel Dovgalyuk
                   ` (9 subsequent siblings)
  14 siblings, 2 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>

This patch updates iotests that verify qemu monitor output.
New output format for snapshot listing include ICOUNT column.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
index 215902b3ad..27471ffae8 100644
--- a/tests/qemu-iotests/267.out
+++ b/tests/qemu-iotests/267.out
@@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
@@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
 Internal snapshots on overlay:
 Snapshot list:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
-1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 Internal snapshots on backing file:
 
 === -blockdev with NBD server on the backing file ===
@@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 (qemu) info snapshots
 List of snapshots present on all disks:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 (qemu) loadvm snap0
 (qemu) quit
 
 Internal snapshots on overlay:
 Snapshot list:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
-1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 Internal snapshots on backing file:
 Snapshot list:
-ID        TAG                 VM SIZE                DATE       VM CLOCK
-1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
+1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
 *** done



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 06/15] qapi: introduce replay.json for record/replay-related stuff
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (4 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 05/15] iotests: update snapshot test for new output format Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 07/15] replay: introduce info hmp/qmp command Pavel Dovgalyuk
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch adds replay.json file. It will be
used for adding record/replay-related data structures and commands.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 MAINTAINERS             |    1 +
 include/sysemu/replay.h |    1 +
 qapi/meson.build        |    1 +
 qapi/misc.json          |   18 ------------------
 qapi/qapi-schema.json   |    1 +
 qapi/replay.json        |   26 ++++++++++++++++++++++++++
 6 files changed, 30 insertions(+), 18 deletions(-)
 create mode 100644 qapi/replay.json

diff --git a/MAINTAINERS b/MAINTAINERS
index 5a22c8be42..e49af700c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2644,6 +2644,7 @@ F: include/sysemu/replay.h
 F: docs/replay.txt
 F: stubs/replay.c
 F: tests/acceptance/replay_kernel.py
+F: qapi/replay.json
 
 IOVA Tree
 M: Peter Xu <peterx@redhat.com>
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index c9c896ae8d..e00ed2f4a5 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -14,6 +14,7 @@
 
 #include "qapi/qapi-types-misc.h"
 #include "qapi/qapi-types-run-state.h"
+#include "qapi/qapi-types-replay.h"
 #include "qapi/qapi-types-ui.h"
 #include "block/aio.h"
 
diff --git a/qapi/meson.build b/qapi/meson.build
index 2b2872a41d..f4fd514379 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -36,6 +36,7 @@ qapi_all_modules = [
   'qdev',
   'qom',
   'rdma',
+  'replay',
   'rocker',
   'run-state',
   'sockets',
diff --git a/qapi/misc.json b/qapi/misc.json
index 9d32820dc1..87fcb90135 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1556,24 +1556,6 @@
 { 'event': 'ACPI_DEVICE_OST',
      'data': { 'info': 'ACPIOSTInfo' } }
 
-##
-# @ReplayMode:
-#
-# Mode of the replay subsystem.
-#
-# @none: normal execution mode. Replay or record are not enabled.
-#
-# @record: record mode. All non-deterministic data is written into the
-#          replay log.
-#
-# @play: replay mode. Non-deterministic data required for system execution
-#        is read from the log.
-#
-# Since: 2.5
-##
-{ 'enum': 'ReplayMode',
-  'data': [ 'none', 'record', 'play' ] }
-
 ##
 # @xen-load-devices-state:
 #
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index f03ff91ceb..2604fcf6ec 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -82,6 +82,7 @@
 { 'include': 'qdev.json' }
 { 'include': 'machine.json' }
 { 'include': 'machine-target.json' }
+{ 'include': 'replay.json' }
 { 'include': 'misc.json' }
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
diff --git a/qapi/replay.json b/qapi/replay.json
new file mode 100644
index 0000000000..9e13551d20
--- /dev/null
+++ b/qapi/replay.json
@@ -0,0 +1,26 @@
+# -*- Mode: Python -*-
+#
+
+##
+# = Record/replay
+##
+
+{ 'include': 'common.json' }
+
+##
+# @ReplayMode:
+#
+# Mode of the replay subsystem.
+#
+# @none: normal execution mode. Replay or record are not enabled.
+#
+# @record: record mode. All non-deterministic data is written into the
+#          replay log.
+#
+# @play: replay mode. Non-deterministic data required for system execution
+#        is read from the log.
+#
+# Since: 2.5
+##
+{ 'enum': 'ReplayMode',
+  'data': [ 'none', 'record', 'play' ] }



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 07/15] replay: introduce info hmp/qmp command
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (5 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 06/15] qapi: introduce replay.json for record/replay-related stuff Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 08/15] replay: introduce breakpoint at the specified step Pavel Dovgalyuk
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch introduces 'info replay' monitor command and
corresponding qmp request.
These commands request the current record/replay mode, replay log file
name, and the instruction count (number of recorded/replayed
instructions).  The instruction count can be used with the
replay_seek/replay_break commands added in the next two patches.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 hmp-commands-info.hx      |   11 +++++++++++
 include/monitor/hmp.h     |    1 +
 qapi/block-core.json      |    3 ++-
 qapi/replay.json          |   39 +++++++++++++++++++++++++++++++++++++++
 replay/meson.build        |    1 +
 replay/replay-debugging.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 replay/replay-debugging.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 30209e3903..117ba25f91 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -881,4 +881,15 @@ SRST
     Show SEV information.
 ERST
 
+    {
+        .name       = "replay",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show record/replay information",
+        .cmd        = hmp_info_replay,
+    },
 
+SRST
+  ``info replay``
+    Display the record/replay information: mode and the current icount.
+ERST
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index c986cfd28b..a790589b9e 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -130,5 +130,6 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
+void hmp_info_replay(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d8265a78e..1fe45625d6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -29,7 +29,8 @@
 #
 # @icount: Current instruction count. Appears when execution record/replay
 #          is enabled. Used for "time-traveling" to match the moment
-#          in the recorded execution with the snapshots. (since 5.2)
+#          in the recorded execution with the snapshots. This counter may
+#          be obtained through @query-replay command (since 5.2)
 #
 # Since: 1.3
 #
diff --git a/qapi/replay.json b/qapi/replay.json
index 9e13551d20..e6b3f6001d 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -24,3 +24,42 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @ReplayInfo:
+#
+# Record/replay information.
+#
+# @mode: current mode.
+#
+# @filename: name of the record/replay log file.
+#            It is present only in record or replay modes, when the log
+#            is recorded or replayed.
+#
+# @icount: current number of executed instructions.
+#
+# Since: 5.2
+#
+##
+{ 'struct': 'ReplayInfo',
+  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'icount': 'int' } }
+
+##
+# @query-replay:
+#
+# Retrieve the record/replay information.
+# It includes current instruction count which may be used for
+# @replay-break and @replay-seek commands.
+#
+# Returns: record/replay information.
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "icount": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+  'returns': 'ReplayInfo' }
diff --git a/replay/meson.build b/replay/meson.build
index 8783aea7c8..f91163fb1e 100644
--- a/replay/meson.build
+++ b/replay/meson.build
@@ -9,4 +9,5 @@ softmmu_ss.add(files(
   'replay-net.c',
   'replay-audio.c',
   'replay-random.c',
+  'replay-debugging.c',
 ))
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
new file mode 100644
index 0000000000..51a6de4e81
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,43 @@
+/*
+ * replay-debugging.c
+ *
+ * Copyright (c) 2010-2020 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-replay.h"
+
+void hmp_info_replay(Monitor *mon, const QDict *qdict)
+{
+    if (replay_mode == REPLAY_MODE_NONE) {
+        monitor_printf(mon, "Record/replay is not active\n");
+    } else {
+        monitor_printf(mon,
+            "%s execution '%s': instruction count = %"PRId64"\n",
+            replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
+            replay_get_filename(), replay_get_current_icount());
+    }
+}
+
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+    ReplayInfo *retval = g_new0(ReplayInfo, 1);
+
+    retval->mode = replay_mode;
+    if (replay_get_filename()) {
+        retval->filename = g_strdup(replay_get_filename());
+        retval->has_filename = true;
+    }
+    retval->icount = replay_get_current_icount();
+    return retval;
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 08/15] replay: introduce breakpoint at the specified step
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (6 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 07/15] replay: introduce info hmp/qmp command Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:16 ` [PATCH v3 09/15] replay: implement replay-seek command Pavel Dovgalyuk
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch introduces replay_break, replay_delete_break
qmp and hmp commands.
These commands allow stopping at the specified instruction.
It may be useful for debugging when there are some known
events that should be investigated.
replay_break command has one argument - number of instructions
executed since the start of the replay.
replay_delete_break removes previously set breakpoint.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 hmp-commands.hx           |   32 +++++++++++++++++
 include/monitor/hmp.h     |    2 +
 qapi/replay.json          |   36 +++++++++++++++++++
 replay/replay-debugging.c |   86 +++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h  |    4 ++
 replay/replay.c           |   17 +++++++++
 6 files changed, 177 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 60f395c276..e8ce385879 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1819,6 +1819,38 @@ SRST
   Set QOM property *property* of object at location *path* to value *value*
 ERST
 
+    {
+        .name       = "replay_break",
+        .args_type  = "icount:i",
+        .params     = "icount",
+        .help       = "set breakpoint at the specified instruction count",
+        .cmd        = hmp_replay_break,
+    },
+
+SRST
+``replay_break`` *icount*
+  Set replay breakpoint at instruction count *icount*.
+  Execution stops when the specified instruction is reached.
+  There can be at most one breakpoint. When breakpoint is set, any prior
+  one is removed.  The breakpoint may be set only in replay mode and only
+  "in the future", i.e. at instruction counts greater than the current one.
+  The current instruction count can be observed with ``info replay``.
+ERST
+
+    {
+        .name       = "replay_delete_break",
+        .args_type  = "",
+        .params     = "",
+        .help       = "remove replay breakpoint",
+        .cmd        = hmp_replay_delete_break,
+    },
+
+SRST
+``replay_delete_break``
+  Remove replay breakpoint which was previously set with ``replay_break``.
+  The command is ignored when there are no replay breakpoints.
+ERST
+
     {
         .name       = "info",
         .args_type  = "item:s?",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index a790589b9e..21849bdda5 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -131,5 +131,7 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
+void hmp_replay_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index e6b3f6001d..173ba76107 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -63,3 +63,39 @@
 ##
 { 'command': 'query-replay',
   'returns': 'ReplayInfo' }
+
+##
+# @replay-break:
+#
+# Set replay breakpoint at instruction count @icount.
+# Execution stops when the specified instruction is reached.
+# There can be at most one breakpoint. When breakpoint is set, any prior
+# one is removed.  The breakpoint may be set only in replay mode and only
+# "in the future", i.e. at instruction counts greater than the current one.
+# The current instruction count can be observed with @query-replay.
+#
+# @icount: instruction count to stop at
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-break", "data": { "icount": 220414 } }
+#
+##
+{ 'command': 'replay-break', 'data': { 'icount': 'int' } }
+
+##
+# @replay-delete-break:
+#
+# Remove replay breakpoint which was set with @replay-break.
+# The command is ignored when there are no replay breakpoints.
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-delete-break" }
+#
+##
+{ 'command': 'replay-delete-break' }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 51a6de4e81..86e19bb217 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -12,10 +12,13 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "sysemu/replay.h"
+#include "sysemu/runstate.h"
 #include "replay-internal.h"
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "qapi/qapi-commands-replay.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/timer.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -41,3 +44,86 @@ ReplayInfo *qmp_query_replay(Error **errp)
     retval->icount = replay_get_current_icount();
     return retval;
 }
+
+static void replay_break(uint64_t icount, QEMUTimerCB callback, void *opaque)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    assert(replay_mutex_locked());
+    assert(replay_break_icount >= replay_get_current_icount());
+    assert(callback);
+
+    replay_break_icount = icount;
+
+    if (replay_break_timer) {
+        timer_del(replay_break_timer);
+    }
+    replay_break_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
+                                      callback, opaque);
+}
+
+static void replay_delete_break(void)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    assert(replay_mutex_locked());
+
+    if (replay_break_timer) {
+        timer_del(replay_break_timer);
+        timer_free(replay_break_timer);
+        replay_break_timer = NULL;
+    }
+    replay_break_icount = -1ULL;
+}
+
+static void replay_stop_vm(void *opaque)
+{
+    vm_stop(RUN_STATE_PAUSED);
+    replay_delete_break();
+}
+
+void qmp_replay_break(int64_t icount, Error **errp)
+{
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        if (icount >= replay_get_current_icount()) {
+            replay_break(icount, replay_stop_vm, NULL);
+        } else {
+            error_setg(errp,
+                "cannot set breakpoint at the instruction in the past");
+        }
+    } else {
+        error_setg(errp, "setting the breakpoint is allowed only in play mode");
+    }
+}
+
+void hmp_replay_break(Monitor *mon, const QDict *qdict)
+{
+    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_break(icount, &err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}
+
+void qmp_replay_delete_break(Error **errp)
+{
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        replay_delete_break();
+    } else {
+        error_setg(errp, "replay breakpoints are allowed only in play mode");
+    }
+}
+
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qmp_replay_delete_break(&err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 33ac551e78..2f6145ec7c 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -94,6 +94,10 @@ extern ReplayState replay_state;
 
 /* File for replay writing */
 extern FILE *replay_file;
+/* Instruction count of the replay breakpoint */
+extern uint64_t replay_break_icount;
+/* Timer for the replay breakpoint callback */
+extern QEMUTimer *replay_break_timer;
 
 void replay_put_byte(uint8_t byte);
 void replay_put_event(uint8_t event);
diff --git a/replay/replay.c b/replay/replay.c
index 42e82f7bc7..220886e32e 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -34,6 +34,10 @@ static char *replay_filename;
 ReplayState replay_state;
 static GSList *replay_blockers;
 
+/* Replay breakpoints */
+uint64_t replay_break_icount = -1ULL;
+QEMUTimer *replay_break_timer;
+
 bool replay_next_event_is(int event)
 {
     bool res = false;
@@ -73,6 +77,13 @@ int replay_get_instructions(void)
     replay_mutex_lock();
     if (replay_next_event_is(EVENT_INSTRUCTION)) {
         res = replay_state.instruction_count;
+        if (replay_break_icount != -1LL) {
+            uint64_t current = replay_get_current_icount();
+            assert(replay_break_icount >= current);
+            if (current + res > replay_break_icount) {
+                res = replay_break_icount - current;
+            }
+        }
     }
     replay_mutex_unlock();
     return res;
@@ -99,6 +110,12 @@ void replay_account_executed_instructions(void)
                    will be read from the log. */
                 qemu_notify_event();
             }
+            /* Execution reached the break step */
+            if (replay_break_icount == replay_state.current_icount) {
+                /* Cannot make callback directly from the vCPU thread */
+                timer_mod_ns(replay_break_timer,
+                    qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
+            }
         }
     }
 }



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (7 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 08/15] replay: introduce breakpoint at the specified step Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-07 12:45   ` Alex Bennée
  2020-09-07 12:58   ` Alex Bennée
  2020-09-02  8:16 ` [PATCH v3 10/15] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
                   ` (5 subsequent siblings)
  14 siblings, 2 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
the execution to the specified instruction count.
The command automatically loads nearest snapshot and replays the execution
to find the desired instruction count.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 hmp-commands.hx           |   18 +++++++++
 include/monitor/hmp.h     |    1 
 qapi/replay.json          |   20 ++++++++++
 replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 131 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e8ce385879..4288274c4e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1851,6 +1851,24 @@ SRST
   The command is ignored when there are no replay breakpoints.
 ERST
 
+    {
+        .name       = "replay_seek",
+        .args_type  = "icount:i",
+        .params     = "icount",
+        .help       = "replay execution to the specified instruction count",
+        .cmd        = hmp_replay_seek,
+    },
+
+SRST
+``replay_seek`` *icount*
+Automatically proceed to the instruction count *icount*, when
+replaying the execution. The command automatically loads nearest
+snapshot and replays the execution to find the desired instruction.
+When there is no preceding snapshot or the execution is not replayed,
+then the command fails.
+*icount* for the reference may be observed with ``info replay`` command.
+ERST
+
     {
         .name       = "info",
         .args_type  = "item:s?",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 21849bdda5..655eb81a4c 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_seek(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index 173ba76107..bfd83d7591 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -99,3 +99,23 @@
 #
 ##
 { 'command': 'replay-delete-break' }
+
+##
+# @replay-seek:
+#
+# Automatically proceed to the instruction count @icount, when
+# replaying the execution. The command automatically loads nearest
+# snapshot and replays the execution to find the desired instruction.
+# When there is no preceding snapshot or the execution is not replayed,
+# then the command fails.
+# icount for the reference may be obtained with @query-replay command.
+#
+# @icount: target instruction count
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
+##
+{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 86e19bb217..cfd0221692 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -19,6 +19,8 @@
 #include "qapi/qapi-commands-replay.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/timer.h"
+#include "block/snapshot.h"
+#include "migration/snapshot.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
         return;
     }
 }
+
+static char *replay_find_nearest_snapshot(int64_t icount,
+                                          int64_t *snapshot_icount)
+{
+    BlockDriverState *bs;
+    QEMUSnapshotInfo *sn_tab;
+    QEMUSnapshotInfo *nearest = NULL;
+    char *ret = NULL;
+    int nb_sns, i;
+    AioContext *aio_context;
+
+    *snapshot_icount = -1;
+
+    bs = bdrv_all_find_vmstate_bs();
+    if (!bs) {
+        goto fail;
+    }
+    aio_context = bdrv_get_aio_context(bs);
+
+    aio_context_acquire(aio_context);
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    aio_context_release(aio_context);
+
+    for (i = 0; i < nb_sns; i++) {
+        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
+            if (sn_tab[i].icount != -1ULL
+                && sn_tab[i].icount <= icount
+                && (!nearest || nearest->icount < sn_tab[i].icount)) {
+                nearest = &sn_tab[i];
+            }
+        }
+    }
+    if (nearest) {
+        ret = g_strdup(nearest->name);
+        *snapshot_icount = nearest->icount;
+    }
+    g_free(sn_tab);
+
+fail:
+    return ret;
+}
+
+static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
+{
+    char *snapshot = NULL;
+    int64_t snapshot_icount;
+
+    if (replay_mode != REPLAY_MODE_PLAY) {
+        error_setg(errp, "replay must be enabled to seek");
+        return;
+    }
+    if (!replay_snapshot) {
+        error_setg(errp, "snapshotting is disabled");
+        return;
+    }
+
+    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
+    if (snapshot) {
+        if (icount < replay_get_current_icount()
+            || replay_get_current_icount() < snapshot_icount) {
+            vm_stop(RUN_STATE_RESTORE_VM);
+            load_snapshot(snapshot, errp);
+        }
+        g_free(snapshot);
+    }
+    if (replay_get_current_icount() <= icount) {
+        replay_break(icount, callback, NULL);
+        vm_start();
+    } else {
+        error_setg(errp, "cannot seek to the specified instruction count");
+    }
+}
+
+void qmp_replay_seek(int64_t icount, Error **errp)
+{
+    replay_seek(icount, replay_stop_vm, errp);
+}
+
+void hmp_replay_seek(Monitor *mon, const QDict *qdict)
+{
+    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_seek(icount, &err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 10/15] replay: flush rr queue before loading the vmstate
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (8 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 09/15] replay: implement replay-seek command Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-07 13:37   ` Alex Bennée
  2020-09-02  8:16 ` [PATCH v3 11/15] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

Non-empty record/replay queue prevents saving and loading the VM state,
because it includes pending bottom halves and block coroutines.
But when the new VM state is loaded, we don't have to preserve the consistency
of the current state anymore. Therefore this patch just flushes the queue
allowing the coroutines to finish and removes checking for empty rr queue
for load_snapshot function.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 include/sysemu/replay.h  |    2 ++
 migration/savevm.c       |   12 ++++++------
 replay/replay-events.c   |    4 ++++
 replay/replay-internal.h |    2 --
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index e00ed2f4a5..239c01e7df 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -149,6 +149,8 @@ void replay_disable_events(void);
 void replay_enable_events(void);
 /*! Returns true when saving events is enabled */
 bool replay_events_enabled(void);
+/* Flushes events queue */
+void replay_flush_events(void);
 /*! Adds bottom half event to the queue */
 void replay_bh_schedule_event(QEMUBH *bh);
 /* Adds oneshot bottom half event to the queue */
diff --git a/migration/savevm.c b/migration/savevm.c
index 74378f8ebd..7905aab053 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2880,12 +2880,6 @@ int load_snapshot(const char *name, Error **errp)
     AioContext *aio_context;
     MigrationIncomingState *mis = migration_incoming_get_current();
 
-    if (!replay_can_snapshot()) {
-        error_setg(errp, "Record/replay does not allow loading snapshot "
-                   "right now. Try once more later.");
-        return -EINVAL;
-    }
-
     if (!bdrv_all_can_snapshot(&bs)) {
         error_setg(errp,
                    "Device '%s' is writable but does not support snapshots",
@@ -2919,6 +2913,12 @@ int load_snapshot(const char *name, Error **errp)
         return -EINVAL;
     }
 
+    /*
+     * Flush the record/replay queue. Now the VM state is going
+     * to change. Therefore we don't need to preserve its consistency
+     */
+    replay_flush_events();
+
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all_begin();
 
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 302b84043a..a1c6bb934e 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -77,6 +77,10 @@ bool replay_has_events(void)
 
 void replay_flush_events(void)
 {
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return;
+    }
+
     g_assert(replay_mutex_locked());
 
     while (!QTAILQ_EMPTY(&events_list)) {
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 2f6145ec7c..97649ed8d7 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -149,8 +149,6 @@ void replay_read_next_clock(unsigned int kind);
 void replay_init_events(void);
 /*! Clears internal data structures for events handling */
 void replay_finish_events(void);
-/*! Flushes events queue */
-void replay_flush_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 11/15] gdbstub: add reverse step support in replay mode
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (9 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 10/15] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-07 16:30   ` Alex Bennée
  2020-09-08 11:16   ` Alex Bennée
  2020-09-02  8:16 ` [PATCH v3 12/15] gdbstub: add reverse continue " Pavel Dovgalyuk
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

GDB remote protocol supports two reverse debugging commands:
reverse step and reverse continue.
This patch adds support of the first one to the gdbstub.
Reverse step is intended to step one instruction in the backwards
direction. This is not possible in regular execution.
But replayed execution is deterministic, therefore we can load one of
the prior snapshots and proceed to the desired step. It is equivalent
to stepping one instruction back.
There should be at least one snapshot preceding the debugged part of
the replay log.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 accel/tcg/translator.c    |    1 +
 exec.c                    |    7 ++++++
 gdbstub.c                 |   55 +++++++++++++++++++++++++++++++++++++++++++--
 include/sysemu/replay.h   |   11 +++++++++
 replay/replay-debugging.c |   33 +++++++++++++++++++++++++++
 softmmu/cpus.c            |   14 +++++++++--
 stubs/replay.c            |    5 ++++
 7 files changed, 121 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 603d17ff83..fb1e19c585 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -17,6 +17,7 @@
 #include "exec/log.h"
 #include "exec/translator.h"
 #include "exec/plugin-gen.h"
+#include "sysemu/replay.h"
 
 /* Pairs with tcg_clear_temp_count.
    To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/exec.c b/exec.c
index 7683afb6a8..47512e950c 100644
--- a/exec.c
+++ b/exec.c
@@ -2750,6 +2750,13 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
     QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
         if (watchpoint_address_matches(wp, addr, len)
             && (wp->flags & flags)) {
+            if (replay_running_debug()) {
+                /*
+                 * Don't process the watchpoints when we are
+                 * in a reverse debugging operation.
+                 */
+                return;
+            }
             if (flags == BP_MEM_READ) {
                 wp->flags |= BP_WATCHPOINT_HIT_READ;
             } else {
diff --git a/gdbstub.c b/gdbstub.c
index 9dfb6e4142..79e8ccc050 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -51,6 +51,7 @@
 #include "sysemu/runstate.h"
 #include "hw/semihosting/semihost.h"
 #include "exec/exec-all.h"
+#include "sysemu/replay.h"
 
 #ifdef CONFIG_USER_ONLY
 #define GDB_ATTACHED "0"
@@ -375,6 +376,20 @@ typedef struct GDBState {
  */
 static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
 
+/* Retrieves flags for single step mode. */
+static int get_sstep_flags(void)
+{
+    /*
+     * In replay mode all events written into the log should be replayed.
+     * That is why NOIRQ flag is removed in this mode.
+     */
+    if (replay_mode != REPLAY_MODE_NONE) {
+        return SSTEP_ENABLE;
+    } else {
+        return sstep_flags;
+    }
+}
+
 static GDBState gdbserver_state;
 
 static void init_gdbserver_state(void)
@@ -501,7 +516,7 @@ static int gdb_continue_partial(char *newstates)
                 break; /* nothing to do here */
             case 's':
                 trace_gdbstub_op_stepping(cpu->cpu_index);
-                cpu_single_step(cpu, sstep_flags);
+                cpu_single_step(cpu, get_sstep_flags());
                 cpu_resume(cpu);
                 flag = 1;
                 break;
@@ -1874,10 +1889,31 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
         gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
     }
 
-    cpu_single_step(gdbserver_state.c_cpu, sstep_flags);
+    cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags());
     gdb_continue();
 }
 
+static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (replay_mode != REPLAY_MODE_PLAY) {
+        put_packet("E22");
+    }
+    if (gdb_ctx->num_params == 1) {
+        switch (gdb_ctx->params[0].opcode) {
+        case 's':
+            if (replay_reverse_step()) {
+                gdb_continue();
+            } else {
+                put_packet("E14");
+            }
+            return;
+        }
+    }
+
+    /* Default invalid command */
+    put_packet("");
+}
+
 static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     put_packet("vCont;c;C;s;S");
@@ -2124,6 +2160,10 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
         g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
     }
 
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
+    }
+
     if (gdb_ctx->num_params &&
         strstr(gdb_ctx->params[0].data, "multiprocess+")) {
         gdbserver_state.multiprocess = true;
@@ -2460,6 +2500,17 @@ static int gdb_handle_packet(const char *line_buf)
             cmd_parser = &step_cmd_desc;
         }
         break;
+    case 'b':
+        {
+            static const GdbCmdParseEntry backward_cmd_desc = {
+                .handler = handle_backward,
+                .cmd = "b",
+                .cmd_startswith = 1,
+                .schema = "o0"
+            };
+            cmd_parser = &backward_cmd_desc;
+        }
+        break;
     case 'F':
         {
             static const GdbCmdParseEntry file_io_cmd_desc = {
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 239c01e7df..13a8123b09 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -75,6 +75,17 @@ void replay_finish(void);
 void replay_add_blocker(Error *reason);
 /* Returns name of the replay log file */
 const char *replay_get_filename(void);
+/*
+ * Start making one step in backward direction.
+ * Used by gdbstub for backwards debugging.
+ * Returns true on success.
+ */
+bool replay_reverse_step(void);
+/*
+ * Returns true if replay module is processing
+ * reverse_continue or reverse_step request
+ */
+bool replay_running_debug(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index cfd0221692..aa3ca040e2 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -22,6 +22,13 @@
 #include "block/snapshot.h"
 #include "migration/snapshot.h"
 
+static bool replay_is_debugging;
+
+bool replay_running_debug(void)
+{
+    return replay_is_debugging;
+}
+
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
     if (replay_mode == REPLAY_MODE_NONE) {
@@ -219,3 +226,29 @@ void hmp_replay_seek(Monitor *mon, const QDict *qdict)
         return;
     }
 }
+
+static void replay_stop_vm_debug(void *opaque)
+{
+    replay_is_debugging = false;
+    vm_stop(RUN_STATE_DEBUG);
+    replay_delete_break();
+}
+
+bool replay_reverse_step(void)
+{
+    Error *err = NULL;
+
+    assert(replay_mode == REPLAY_MODE_PLAY);
+
+    if (replay_get_current_icount() != 0) {
+        replay_seek(replay_get_current_icount() - 1, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            return false;
+        }
+        replay_is_debugging = true;
+        return true;
+    }
+
+    return false;
+}
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a802e899ab..377fe3298c 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -1004,9 +1004,17 @@ static bool cpu_can_run(CPUState *cpu)
 
 static void cpu_handle_guest_debug(CPUState *cpu)
 {
-    gdb_set_stop_cpu(cpu);
-    qemu_system_debug_request();
-    cpu->stopped = true;
+    if (!replay_running_debug()) {
+        gdb_set_stop_cpu(cpu);
+        qemu_system_debug_request();
+        cpu->stopped = true;
+    } else {
+        if (!cpu->singlestep_enabled) {
+            cpu_single_step(cpu, SSTEP_ENABLE);
+        } else {
+            cpu_single_step(cpu, 0);
+        }
+    }
 }
 
 #ifdef CONFIG_LINUX
diff --git a/stubs/replay.c b/stubs/replay.c
index eacb366aa8..d5b52302e9 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -93,3 +93,8 @@ uint64_t replay_get_current_icount(void)
 {
     return 0;
 }
+
+bool replay_reverse_step(void)
+{
+    return false;
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 12/15] gdbstub: add reverse continue support in replay mode
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (10 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 11/15] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
@ 2020-09-02  8:16 ` Pavel Dovgalyuk
  2020-09-02  8:17 ` [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch adds support of the reverse continue operation for gdbstub.
Reverse continue finds the last breakpoint that would happen in normal
execution from the beginning to the current moment.
Implementation of the reverse continue replays the execution twice:
to find the breakpoints that were hit and to seek to the last breakpoint.
Reverse continue loads the previous snapshot and tries to find the breakpoint
since that moment. If there are no such breakpoints, it proceeds to
the earlier snapshot, and so on. When no breakpoints or watchpoints were
hit at all, execution stops at the beginning of the replay log.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 exec.c                    |    1 +
 gdbstub.c                 |   10 ++++++
 include/sysemu/replay.h   |    8 +++++
 replay/replay-debugging.c |   71 +++++++++++++++++++++++++++++++++++++++++++++
 softmmu/cpus.c            |    5 +++
 stubs/replay.c            |    5 +++
 6 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 47512e950c..8a97908a38 100644
--- a/exec.c
+++ b/exec.c
@@ -2755,6 +2755,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
                  * Don't process the watchpoints when we are
                  * in a reverse debugging operation.
                  */
+                replay_breakpoint();
                 return;
             }
             if (flags == BP_MEM_READ) {
diff --git a/gdbstub.c b/gdbstub.c
index 79e8ccc050..ac92273018 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1907,6 +1907,13 @@ static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
                 put_packet("E14");
             }
             return;
+        case 'c':
+            if (replay_reverse_continue()) {
+                gdb_continue();
+            } else {
+                put_packet("E14");
+            }
+            return;
         }
     }
 
@@ -2161,7 +2168,8 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     if (replay_mode == REPLAY_MODE_PLAY) {
-        g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
+        g_string_append(gdbserver_state.str_buf,
+            ";ReverseStep+;ReverseContinue+");
     }
 
     if (gdb_ctx->num_params &&
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 13a8123b09..b6cac175c4 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -81,11 +81,19 @@ const char *replay_get_filename(void);
  * Returns true on success.
  */
 bool replay_reverse_step(void);
+/*
+ * Start searching the last breakpoint/watchpoint.
+ * Used by gdbstub for backwards debugging.
+ * Returns true if the process successfully started.
+ */
+bool replay_reverse_continue(void);
 /*
  * Returns true if replay module is processing
  * reverse_continue or reverse_step request
  */
 bool replay_running_debug(void);
+/* Called in reverse debugging mode to collect breakpoint information */
+void replay_breakpoint(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index aa3ca040e2..27af103118 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -23,6 +23,8 @@
 #include "migration/snapshot.h"
 
 static bool replay_is_debugging;
+static int64_t replay_last_breakpoint;
+static int64_t replay_last_snapshot;
 
 bool replay_running_debug(void)
 {
@@ -252,3 +254,72 @@ bool replay_reverse_step(void)
 
     return false;
 }
+
+static void replay_continue_end(void)
+{
+    replay_is_debugging = false;
+    vm_stop(RUN_STATE_DEBUG);
+    replay_delete_break();
+}
+
+static void replay_continue_stop(void *opaque)
+{
+    Error *err = NULL;
+    if (replay_last_breakpoint != -1LL) {
+        replay_seek(replay_last_breakpoint, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        return;
+    }
+    /*
+     * No breakpoints since the last snapshot.
+     * Find previous snapshot and try again.
+     */
+    if (replay_last_snapshot != 0) {
+        replay_seek(replay_last_snapshot - 1, replay_continue_stop, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        replay_last_snapshot = replay_get_current_icount();
+        return;
+    } else {
+        /* Seek to the very first step */
+        replay_seek(0, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        return;
+    }
+    replay_continue_end();
+}
+
+bool replay_reverse_continue(void)
+{
+    Error *err = NULL;
+
+    assert(replay_mode == REPLAY_MODE_PLAY);
+
+    if (replay_get_current_icount() != 0) {
+        replay_seek(replay_get_current_icount() - 1, replay_continue_stop, &err);
+        if (err) {
+            error_free(err);
+            return false;
+        }
+        replay_last_breakpoint = -1LL;
+        replay_is_debugging = true;
+        replay_last_snapshot = replay_get_current_icount();
+        return true;
+    }
+
+    return false;
+}
+
+void replay_breakpoint(void)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    replay_last_breakpoint = replay_get_current_icount();
+}
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 377fe3298c..7714cb663c 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -1010,6 +1010,11 @@ static void cpu_handle_guest_debug(CPUState *cpu)
         cpu->stopped = true;
     } else {
         if (!cpu->singlestep_enabled) {
+            /*
+             * Report about the breakpoint and
+             * make a single step to skip it
+             */
+            replay_breakpoint();
             cpu_single_step(cpu, SSTEP_ENABLE);
         } else {
             cpu_single_step(cpu, 0);
diff --git a/stubs/replay.c b/stubs/replay.c
index d5b52302e9..45ebe77fb9 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -98,3 +98,8 @@ bool replay_reverse_step(void)
 {
     return false;
 }
+
+bool replay_reverse_continue(void)
+{
+    return false;
+}



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (11 preceding siblings ...)
  2020-09-02  8:16 ` [PATCH v3 12/15] gdbstub: add reverse continue " Pavel Dovgalyuk
@ 2020-09-02  8:17 ` Pavel Dovgalyuk
  2020-09-08 11:27   ` Alex Bennée
  2020-09-02  8:17 ` [PATCH v3 14/15] tests: bump avocado version Pavel Dovgalyuk
  2020-09-02  8:17 ` [PATCH v3 15/15] tests/acceptance: add reverse debugging test Pavel Dovgalyuk
  14 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

This patch updates the documentation and describes usage of the reverse
debugging in QEMU+GDB.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 docs/replay.txt |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/docs/replay.txt b/docs/replay.txt
index 70c27edb36..18d6169f3b 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -294,6 +294,39 @@ for recording and replaying must contain identical number of ports in record
 and replay modes, but their backends may differ.
 E.g., '-serial stdio' in record mode, and '-serial null' in replay mode.
 
+Reverse debugging
+-----------------
+
+Reverse debugging allows "executing" the program in reverse direction.
+GDB remote protocol supports "reverse step" and "reverse continue"
+commands. The first one steps single instruction backwards in time,
+and the second one finds the last breakpoint in the past.
+
+Recorded executions may be used to enable reverse debugging. QEMU can't
+execute the code in backwards direction, but can load a snapshot and
+replay forward to find the desired position or breakpoint.
+
+The following GDB commands are supported:
+ - reverse-stepi (or rsi) - step one instruction backwards
+ - reverse-continue (or rc) - find last breakpoint in the past
+
+Reverse step loads the nearest snapshot and replays the execution until
+the required instruction is met.
+
+Reverse continue may include several passes of examining the execution
+between the snapshots. Each of the passes include the following steps:
+ 1. loading the snapshot
+ 2. replaying to examine the breakpoints
+ 3. if breakpoint or watchpoint was met
+    - loading the snaphot again
+    - replaying to the required breakpoint
+ 4. else
+    - proceeding to the p.1 with the earlier snapshot
+
+Therefore usage of the reverse debugging requires at least one snapshot
+created in advance. See the "Snapshotting" section to learn about running
+record/replay and creating the snapshot in these modes.
+
 Replay log format
 -----------------
 



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 14/15] tests: bump avocado version
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (12 preceding siblings ...)
  2020-09-02  8:17 ` [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
@ 2020-09-02  8:17 ` Pavel Dovgalyuk
  2020-09-02 17:02   ` Willian Rampazzo
  2020-09-04 21:39   ` Cleber Rosa
  2020-09-02  8:17 ` [PATCH v3 15/15] tests/acceptance: add reverse debugging test Pavel Dovgalyuk
  14 siblings, 2 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>

Reverse debugging test uses gdb remote client of avocado framework.
This client was fixed since the currently used version 76.
Therefore this patch bumps the version to 81 and fixes command
line version compatibility issue.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 tests/Makefile.include |    2 +-
 tests/requirements.txt |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9ac8f5b86a..0687c8bcda 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -517,7 +517,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
             --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
             $(AVOCADO_TAGS) \
-            $(if $(GITLAB_CI),,--failfast=on) tests/acceptance, \
+            $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
             "AVOCADO", "tests/acceptance")
 
 # Consolidated targets
diff --git a/tests/requirements.txt b/tests/requirements.txt
index f9c84b4ba1..036691c922 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,5 +1,5 @@
 # Add Python module requirements, one per line, to be installed
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
-avocado-framework==76.0
+avocado-framework==81.0
 pycdlib==1.9.0



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 15/15] tests/acceptance: add reverse debugging test
  2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
                   ` (13 preceding siblings ...)
  2020-09-02  8:17 ` [PATCH v3 14/15] tests: bump avocado version Pavel Dovgalyuk
@ 2020-09-02  8:17 ` Pavel Dovgalyuk
  2020-09-08 13:01   ` Alex Bennée
  14 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-02  8:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, wrampazz, pavel.dovgalyuk, ehabkost, alex.bennee,
	mtosatti, armbru, mreitz, stefanha, crosa, pbonzini, philmd,
	zhiwei_liu, rth

From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>

This is a test for GDB reverse debugging commands: reverse step and reverse continue.
Every test in this suite consists of two phases: record and replay.
Recording saves the execution of some instructions and makes an initial
VM snapshot to allow reverse execution.
Replay saves the order of the first instructions and then checks that they
are executed backwards in the correct order.
After that the execution is replayed to the end, and reverse continue
command is checked by setting several breakpoints, and asserting
that the execution is stopped at the last of them.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
 MAINTAINERS                           |    1 
 tests/acceptance/reverse_debugging.py |  203 +++++++++++++++++++++++++++++++++
 2 files changed, 204 insertions(+)
 create mode 100644 tests/acceptance/reverse_debugging.py

diff --git a/MAINTAINERS b/MAINTAINERS
index e49af700c9..76450f1bdf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2644,6 +2644,7 @@ F: include/sysemu/replay.h
 F: docs/replay.txt
 F: stubs/replay.c
 F: tests/acceptance/replay_kernel.py
+F: tests/acceptance/reverse_debugging.py
 F: qapi/replay.json
 
 IOVA Tree
diff --git a/tests/acceptance/reverse_debugging.py b/tests/acceptance/reverse_debugging.py
new file mode 100644
index 0000000000..dda42e1c1a
--- /dev/null
+++ b/tests/acceptance/reverse_debugging.py
@@ -0,0 +1,203 @@
+# Reverse debugging test
+#
+# Copyright (c) 2020 ISP RAS
+#
+# Author:
+#  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+import os
+import logging
+
+from avocado_qemu import BUILD_DIR
+from avocado.utils import gdb
+from avocado.utils import process
+from avocado.utils.path import find_command
+from boot_linux_console import LinuxKernelTest
+
+class ReverseDebugging(LinuxKernelTest):
+    """
+    Test GDB reverse debugging commands: reverse step and reverse continue.
+    Recording saves the execution of some instructions and makes an initial
+    VM snapshot to allow reverse execution.
+    Replay saves the order of the first instructions and then checks that they
+    are executed backwards in the correct order.
+    After that the execution is replayed to the end, and reverse continue
+    command is checked by setting several breakpoints, and asserting
+    that the execution is stopped at the last of them.
+    """
+
+    timeout = 10
+    STEPS = 10
+    endian_is_le = True
+
+    def run_vm(self, record, shift, args, replay_path, image_path):
+        logger = logging.getLogger('replay')
+        vm = self.get_vm()
+        vm.set_console()
+        if record:
+            logger.info('recording the execution...')
+            mode = 'record'
+        else:
+            logger.info('replaying the execution...')
+            mode = 'replay'
+            vm.add_args('-s', '-S')
+        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
+                    (shift, mode, replay_path),
+                    '-net', 'none')
+        vm.add_args('-drive', 'file=%s,if=none' % image_path)
+        if args:
+            vm.add_args(*args)
+        vm.launch()
+        return vm
+
+    @staticmethod
+    def get_reg_le(g, reg):
+        res = g.cmd(b'p%x' % reg)
+        num = 0
+        for i in range(len(res))[-2::-2]:
+            num = 0x100 * num + int(res[i:i + 2], 16)
+        return num
+
+    @staticmethod
+    def get_reg_be(g, reg):
+        res = g.cmd(b'p%x' % reg)
+        return int(res, 16)
+
+    def get_reg(self, g, reg):
+        # value may be encoded in BE or LE order
+        if self.endian_is_le:
+            return self.get_reg_le(g, reg)
+        else:
+            return self.get_reg_be(g, reg)
+
+    def get_pc(self, g):
+        return self.get_reg(g, self.REG_PC)
+
+    def check_pc(self, g, addr):
+        pc = self.get_pc(g)
+        if pc != addr:
+            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
+
+    @staticmethod
+    def gdb_step(g):
+        g.cmd(b's', b'T05thread:01;')
+
+    @staticmethod
+    def gdb_bstep(g):
+        g.cmd(b'bs', b'T05thread:01;')
+
+    @staticmethod
+    def vm_get_icount(vm):
+        return vm.qmp('query-replay')['return']['icount']
+
+    def reverse_debugging(self, shift=7, args=None):
+        logger = logging.getLogger('replay')
+
+        # create qcow2 for snapshots
+        logger.info('creating qcow2 image for VM snapshots')
+        image_path = os.path.join(self.workdir, 'disk.qcow2')
+        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
+        if not os.path.exists(qemu_img):
+            qemu_img = find_command('qemu-img', False)
+        if qemu_img is False:
+            self.cancel('Could not find "qemu-img", which is required to '
+                        'create the temporary qcow2 image')
+        cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
+        process.run(cmd)
+
+        replay_path = os.path.join(self.workdir, 'replay.bin')
+
+        # record the log
+        vm = self.run_vm(True, shift, args, replay_path, image_path)
+        while self.vm_get_icount(vm) <= self.STEPS:
+            pass
+        last_icount = self.vm_get_icount(vm)
+        vm.shutdown()
+
+        logger.info("recorded log with %s+ steps" % last_icount)
+
+        # replay and run debug commands
+        vm = self.run_vm(False, shift, args, replay_path, image_path)
+        logger.info('connecting to gdbstub')
+        g = gdb.GDBRemote('127.0.0.1', 1234, False, False)
+        g.connect()
+        r = g.cmd(b'qSupported')
+        if b'qXfer:features:read+' in r:
+            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
+        if b'ReverseStep+' not in r:
+            self.fail('Reverse step is not supported by QEMU')
+        if b'ReverseContinue+' not in r:
+            self.fail('Reverse continue is not supported by QEMU')
+
+        logger.info('stepping forward')
+        steps = []
+        # record first instruction addresses
+        for _ in range(self.STEPS):
+            pc = self.get_pc(g)
+            logger.info('saving position %x' % pc)
+            steps.append(pc)
+            self.gdb_step(g)
+
+        # visit the recorded instruction in reverse order
+        logger.info('stepping backward')
+        for addr in steps[::-1]:
+            self.gdb_bstep(g)
+            self.check_pc(g, addr)
+            logger.info('found position %x' % addr)
+
+        logger.info('seeking to the end (icount %s)' % (last_icount - 1))
+        vm.qmp('replay-break', icount=last_icount - 1)
+        # continue - will return after pausing
+        g.cmd(b'c', b'T02thread:01;')
+
+        logger.info('setting breakpoints')
+        for addr in steps:
+            # hardware breakpoint at addr with len=1
+            g.cmd(b'Z1,%x,1' % addr, b'OK')
+
+        logger.info('running reverse continue to reach %x' % steps[-1])
+        # reverse continue - will return after stopping at the breakpoint
+        g.cmd(b'bc', b'T05thread:01;')
+
+        # assume that none of the first instructions is executed again
+        # breaking the order of the breakpoints
+        self.check_pc(g, steps[-1])
+        logger.info('successfully reached %x' % steps[-1])
+
+        logger.info('exitting gdb and qemu')
+        vm.shutdown()
+
+class ReverseDebugging_X86_64(ReverseDebugging):
+    REG_PC = 0x10
+    REG_CS = 0x12
+    def get_pc(self, g):
+        return self.get_reg_le(g, self.REG_PC) \
+            + self.get_reg_le(g, self.REG_CS) * 0x10
+
+    def test_x86_64_pc(self):
+        """
+        :avocado: tags=arch:x86_64
+        :avocado: tags=machine:pc
+        """
+        # start with BIOS only
+        self.reverse_debugging()
+
+class ReverseDebugging_AArch64(ReverseDebugging):
+    REG_PC = 32
+
+    def test_aarch64_virt(self):
+        """
+        :avocado: tags=arch:aarch64
+        :avocado: tags=machine:virt
+        :avocado: tags=cpu:cortex-a53
+        """
+        kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+                      '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
+                      '/vmlinuz')
+        kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
+        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+        self.reverse_debugging(
+            args=('-kernel', kernel_path, '-cpu', 'cortex-a53'))



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 14/15] tests: bump avocado version
  2020-09-02  8:17 ` [PATCH v3 14/15] tests: bump avocado version Pavel Dovgalyuk
@ 2020-09-02 17:02   ` Willian Rampazzo
  2020-09-04 21:39   ` Cleber Rosa
  1 sibling, 0 replies; 43+ messages in thread
From: Willian Rampazzo @ 2020-09-02 17:02 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, Eduardo Habkost, Alex Bennée, Marcelo Tosatti,
	qemu-devel, armbru, Stefan Hajnoczi, Cleber Rosa Junior,
	Paolo Bonzini, mreitz, Philippe Mathieu Daude, zhiwei_liu,
	Richard Henderson

On Wed, Sep 2, 2020 at 5:17 AM Pavel Dovgalyuk
<pavel.dovgalyuk@ispras.ru> wrote:
>
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
>
> Reverse debugging test uses gdb remote client of avocado framework.
> This client was fixed since the currently used version 76.
> Therefore this patch bumps the version to 81 and fixes command
> line version compatibility issue.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  tests/Makefile.include |    2 +-
>  tests/requirements.txt |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 9ac8f5b86a..0687c8bcda 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -517,7 +517,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
>              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
>              --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
>              $(AVOCADO_TAGS) \
> -            $(if $(GITLAB_CI),,--failfast=on) tests/acceptance, \
> +            $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
>              "AVOCADO", "tests/acceptance")
>
>  # Consolidated targets
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index f9c84b4ba1..036691c922 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -1,5 +1,5 @@
>  # Add Python module requirements, one per line, to be installed
>  # in the tests/venv Python virtual environment. For more info,
>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> -avocado-framework==76.0
> +avocado-framework==81.0
>  pycdlib==1.9.0
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 14/15] tests: bump avocado version
  2020-09-02  8:17 ` [PATCH v3 14/15] tests: bump avocado version Pavel Dovgalyuk
  2020-09-02 17:02   ` Willian Rampazzo
@ 2020-09-04 21:39   ` Cleber Rosa
  1 sibling, 0 replies; 43+ messages in thread
From: Cleber Rosa @ 2020-09-04 21:39 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, alex.bennee, mtosatti, qemu-devel,
	armbru, stefanha, pbonzini, mreitz, philmd, zhiwei_liu, rth

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

On Wed, Sep 02, 2020 at 11:17:08AM +0300, Pavel Dovgalyuk wrote:
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
> 
> Reverse debugging test uses gdb remote client of avocado framework.
> This client was fixed since the currently used version 76.
> Therefore this patch bumps the version to 81 and fixes command
> line version compatibility issue.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  tests/Makefile.include |    2 +-
>  tests/requirements.txt |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

I'm queuing this patch on my queue.  Thanks!

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 01/15] replay: don't record interrupt poll
  2020-09-02  8:15 ` [PATCH v3 01/15] replay: don't record interrupt poll Pavel Dovgalyuk
@ 2020-09-07 10:17   ` Alex Bennée
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 10:17 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> Interrupt poll is not a real interrupt event. It is needed only for
> thread safety. This interrupt is used for i386 and converted
> to hardware interrupt by cpu_handle_interrupt function.
> Therefore it is not needed to be recorded, because hardware
> interrupt will be recorded after converting.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  accel/tcg/cpu-exec.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 66d38f9d85..28aff1bb1e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -429,8 +429,7 @@ static inline bool cpu_handle_halt(CPUState *cpu)
>  {
>      if (cpu->halted) {
>  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
> -        if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
> -            && replay_interrupt()) {
> +        if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
>              X86CPU *x86_cpu = X86_CPU(cpu);
>              qemu_mutex_lock_iothread();
>              apic_poll_irq(x86_cpu->apic_state);
> @@ -587,7 +586,13 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>             and via longjmp via cpu_loop_exit.  */
>          else {
>              if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> -                replay_interrupt();
> +                if (true
> +#if defined(TARGET_I386)
> +                    && !(interrupt_request & CPU_INTERRUPT_POLL)
> +#endif
> +                ) {
> +                    replay_interrupt();
> +                }

The cpu-exec code has enough special casing in it without adding more
ugly ifdefs. You could hoist the check into:

  /*
   * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
   * "real" interrupt event later. It does not need to be recorded for
   * replay purposes.
   */
  static inline bool replay_valid_interrupt(int interrupt) {
  #ifdef TARGET_I386
      return !(interrupt_request & CPU_INTERRUPT_POLL);
  #else
      return true;
  #endif
  }

and then the logic is a cleaner in the function:

  if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
      if (replay_valid_interrupt(interrupt_request)) {
          replay_interrupt();
      }

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-02  8:16 ` [PATCH v3 09/15] replay: implement replay-seek command Pavel Dovgalyuk
@ 2020-09-07 12:45   ` Alex Bennée
  2020-09-07 13:32     ` Pavel Dovgalyuk
  2020-09-07 12:58   ` Alex Bennée
  1 sibling, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 12:45 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> the execution to the specified instruction count.
> The command automatically loads nearest snapshot and replays the execution
> to find the desired instruction count.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hmp-commands.hx           |   18 +++++++++
>  include/monitor/hmp.h     |    1 
>  qapi/replay.json          |   20 ++++++++++
>  replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e8ce385879..4288274c4e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1851,6 +1851,24 @@ SRST
>    The command is ignored when there are no replay breakpoints.
>  ERST
>  
> +    {
> +        .name       = "replay_seek",
> +        .args_type  = "icount:i",
> +        .params     = "icount",
> +        .help       = "replay execution to the specified instruction count",
> +        .cmd        = hmp_replay_seek,
> +    },
> +
> +SRST
> +``replay_seek`` *icount*
> +Automatically proceed to the instruction count *icount*, when
> +replaying the execution. The command automatically loads nearest
> +snapshot and replays the execution to find the desired instruction.
> +When there is no preceding snapshot or the execution is not replayed,
> +then the command fails.
> +*icount* for the reference may be observed with ``info replay`` command.
> +ERST
> +
>      {
>          .name       = "info",
>          .args_type  = "item:s?",


This seems to break the build:

  Warning, treated as error:
  /home/alex/lsrc/qemu.git/docs/../hmp-commands.hx:1863:Definition list ends without a blank line; unexpected unindent.



-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-02  8:16 ` [PATCH v3 09/15] replay: implement replay-seek command Pavel Dovgalyuk
  2020-09-07 12:45   ` Alex Bennée
@ 2020-09-07 12:58   ` Alex Bennée
  2020-09-07 13:27     ` Pavel Dovgalyuk
  1 sibling, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 12:58 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> the execution to the specified instruction count.
> The command automatically loads nearest snapshot and replays the execution
> to find the desired instruction count.

Should there be an initial snapshot created at instruction 0? Using a
separate monitor channel:

  (qemu) replay_break 190505
  replay_break 190505
  (qemu) c
  (qemu) info replay
  info replay
  Replaying execution 'record.out': instruction count = 190505
  (qemu) replay_seek 190000
  replay_seek 190000
  snapshotting is disabled

And then the guest dies with a sigabort:

  ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -monitor telnet:127.0.0.1:4444 -S
  *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
  Hello World! qemu_cortex_a53
  double free or corruption (out)
  fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)

>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hmp-commands.hx           |   18 +++++++++
>  include/monitor/hmp.h     |    1 
>  qapi/replay.json          |   20 ++++++++++
>  replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e8ce385879..4288274c4e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1851,6 +1851,24 @@ SRST
>    The command is ignored when there are no replay breakpoints.
>  ERST
>  
> +    {
> +        .name       = "replay_seek",
> +        .args_type  = "icount:i",
> +        .params     = "icount",
> +        .help       = "replay execution to the specified instruction count",
> +        .cmd        = hmp_replay_seek,
> +    },
> +
> +SRST
> +``replay_seek`` *icount*
> +Automatically proceed to the instruction count *icount*, when
> +replaying the execution. The command automatically loads nearest
> +snapshot and replays the execution to find the desired instruction.
> +When there is no preceding snapshot or the execution is not replayed,
> +then the command fails.
> +*icount* for the reference may be observed with ``info replay`` command.
> +ERST
> +
>      {
>          .name       = "info",
>          .args_type  = "item:s?",
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 21849bdda5..655eb81a4c 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>  void hmp_info_replay(Monitor *mon, const QDict *qdict);
>  void hmp_replay_break(Monitor *mon, const QDict *qdict);
>  void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi/replay.json b/qapi/replay.json
> index 173ba76107..bfd83d7591 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -99,3 +99,23 @@
>  #
>  ##
>  { 'command': 'replay-delete-break' }
> +
> +##
> +# @replay-seek:
> +#
> +# Automatically proceed to the instruction count @icount, when
> +# replaying the execution. The command automatically loads nearest
> +# snapshot and replays the execution to find the desired instruction.
> +# When there is no preceding snapshot or the execution is not replayed,
> +# then the command fails.
> +# icount for the reference may be obtained with @query-replay command.
> +#
> +# @icount: target instruction count
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
> +##
> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> index 86e19bb217..cfd0221692 100644
> --- a/replay/replay-debugging.c
> +++ b/replay/replay-debugging.c
> @@ -19,6 +19,8 @@
>  #include "qapi/qapi-commands-replay.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qemu/timer.h"
> +#include "block/snapshot.h"
> +#include "migration/snapshot.h"
>  
>  void hmp_info_replay(Monitor *mon, const QDict *qdict)
>  {
> @@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
>          return;
>      }
>  }
> +
> +static char *replay_find_nearest_snapshot(int64_t icount,
> +                                          int64_t *snapshot_icount)
> +{
> +    BlockDriverState *bs;
> +    QEMUSnapshotInfo *sn_tab;
> +    QEMUSnapshotInfo *nearest = NULL;
> +    char *ret = NULL;
> +    int nb_sns, i;
> +    AioContext *aio_context;
> +
> +    *snapshot_icount = -1;
> +
> +    bs = bdrv_all_find_vmstate_bs();
> +    if (!bs) {
> +        goto fail;
> +    }
> +    aio_context = bdrv_get_aio_context(bs);
> +
> +    aio_context_acquire(aio_context);
> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> +    aio_context_release(aio_context);
> +
> +    for (i = 0; i < nb_sns; i++) {
> +        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
> +            if (sn_tab[i].icount != -1ULL
> +                && sn_tab[i].icount <= icount
> +                && (!nearest || nearest->icount < sn_tab[i].icount)) {
> +                nearest = &sn_tab[i];
> +            }
> +        }
> +    }
> +    if (nearest) {
> +        ret = g_strdup(nearest->name);
> +        *snapshot_icount = nearest->icount;
> +    }
> +    g_free(sn_tab);
> +
> +fail:
> +    return ret;
> +}
> +
> +static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
> +{
> +    char *snapshot = NULL;
> +    int64_t snapshot_icount;
> +
> +    if (replay_mode != REPLAY_MODE_PLAY) {
> +        error_setg(errp, "replay must be enabled to seek");
> +        return;
> +    }
> +    if (!replay_snapshot) {
> +        error_setg(errp, "snapshotting is disabled");
> +        return;
> +    }
> +
> +    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
> +    if (snapshot) {
> +        if (icount < replay_get_current_icount()
> +            || replay_get_current_icount() < snapshot_icount) {
> +            vm_stop(RUN_STATE_RESTORE_VM);
> +            load_snapshot(snapshot, errp);
> +        }
> +        g_free(snapshot);
> +    }
> +    if (replay_get_current_icount() <= icount) {
> +        replay_break(icount, callback, NULL);
> +        vm_start();
> +    } else {
> +        error_setg(errp, "cannot seek to the specified instruction count");
> +    }
> +}
> +
> +void qmp_replay_seek(int64_t icount, Error **errp)
> +{
> +    replay_seek(icount, replay_stop_vm, errp);
> +}
> +
> +void hmp_replay_seek(Monitor *mon, const QDict *qdict)
> +{
> +    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
> +    Error *err = NULL;
> +
> +    qmp_replay_seek(icount, &err);
> +    if (err) {
> +        error_report_err(err);
> +        error_free(err);
> +        return;
> +    }
> +}


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 12:58   ` Alex Bennée
@ 2020-09-07 13:27     ` Pavel Dovgalyuk
  2020-09-07 14:59       ` Alex Bennée
  0 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-07 13:27 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 15:58, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>
>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>> the execution to the specified instruction count.
>> The command automatically loads nearest snapshot and replays the execution
>> to find the desired instruction count.
> 
> Should there be an initial snapshot created at instruction 0? Using a
> separate monitor channel:

Right, you can't go to the prior state, when there is no preceding 
snapshot available.

> 
>    (qemu) replay_break 190505
>    replay_break 190505
>    (qemu) c
>    (qemu) info replay
>    info replay
>    Replaying execution 'record.out': instruction count = 190505
>    (qemu) replay_seek 190000
>    replay_seek 190000
>    snapshotting is disabled
> 
> And then the guest dies with a sigabort:

This could be a bug, thanks.

> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -monitor telnet:127.0.0.1:4444 -S
>    *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>    Hello World! qemu_cortex_a53
>    double free or corruption (out)
>    fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)
> 
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   hmp-commands.hx           |   18 +++++++++
>>   include/monitor/hmp.h     |    1
>>   qapi/replay.json          |   20 ++++++++++
>>   replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 131 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index e8ce385879..4288274c4e 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1851,6 +1851,24 @@ SRST
>>     The command is ignored when there are no replay breakpoints.
>>   ERST
>>   
>> +    {
>> +        .name       = "replay_seek",
>> +        .args_type  = "icount:i",
>> +        .params     = "icount",
>> +        .help       = "replay execution to the specified instruction count",
>> +        .cmd        = hmp_replay_seek,
>> +    },
>> +
>> +SRST
>> +``replay_seek`` *icount*
>> +Automatically proceed to the instruction count *icount*, when
>> +replaying the execution. The command automatically loads nearest
>> +snapshot and replays the execution to find the desired instruction.
>> +When there is no preceding snapshot or the execution is not replayed,
>> +then the command fails.
>> +*icount* for the reference may be observed with ``info replay`` command.
>> +ERST
>> +
>>       {
>>           .name       = "info",
>>           .args_type  = "item:s?",
>> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
>> index 21849bdda5..655eb81a4c 100644
>> --- a/include/monitor/hmp.h
>> +++ b/include/monitor/hmp.h
>> @@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>>   void hmp_info_replay(Monitor *mon, const QDict *qdict);
>>   void hmp_replay_break(Monitor *mon, const QDict *qdict);
>>   void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>>   
>>   #endif
>> diff --git a/qapi/replay.json b/qapi/replay.json
>> index 173ba76107..bfd83d7591 100644
>> --- a/qapi/replay.json
>> +++ b/qapi/replay.json
>> @@ -99,3 +99,23 @@
>>   #
>>   ##
>>   { 'command': 'replay-delete-break' }
>> +
>> +##
>> +# @replay-seek:
>> +#
>> +# Automatically proceed to the instruction count @icount, when
>> +# replaying the execution. The command automatically loads nearest
>> +# snapshot and replays the execution to find the desired instruction.
>> +# When there is no preceding snapshot or the execution is not replayed,
>> +# then the command fails.
>> +# icount for the reference may be obtained with @query-replay command.
>> +#
>> +# @icount: target instruction count
>> +#
>> +# Since: 5.2
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
>> +##
>> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
>> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
>> index 86e19bb217..cfd0221692 100644
>> --- a/replay/replay-debugging.c
>> +++ b/replay/replay-debugging.c
>> @@ -19,6 +19,8 @@
>>   #include "qapi/qapi-commands-replay.h"
>>   #include "qapi/qmp/qdict.h"
>>   #include "qemu/timer.h"
>> +#include "block/snapshot.h"
>> +#include "migration/snapshot.h"
>>   
>>   void hmp_info_replay(Monitor *mon, const QDict *qdict)
>>   {
>> @@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
>>           return;
>>       }
>>   }
>> +
>> +static char *replay_find_nearest_snapshot(int64_t icount,
>> +                                          int64_t *snapshot_icount)
>> +{
>> +    BlockDriverState *bs;
>> +    QEMUSnapshotInfo *sn_tab;
>> +    QEMUSnapshotInfo *nearest = NULL;
>> +    char *ret = NULL;
>> +    int nb_sns, i;
>> +    AioContext *aio_context;
>> +
>> +    *snapshot_icount = -1;
>> +
>> +    bs = bdrv_all_find_vmstate_bs();
>> +    if (!bs) {
>> +        goto fail;
>> +    }
>> +    aio_context = bdrv_get_aio_context(bs);
>> +
>> +    aio_context_acquire(aio_context);
>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> +    aio_context_release(aio_context);
>> +
>> +    for (i = 0; i < nb_sns; i++) {
>> +        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
>> +            if (sn_tab[i].icount != -1ULL
>> +                && sn_tab[i].icount <= icount
>> +                && (!nearest || nearest->icount < sn_tab[i].icount)) {
>> +                nearest = &sn_tab[i];
>> +            }
>> +        }
>> +    }
>> +    if (nearest) {
>> +        ret = g_strdup(nearest->name);
>> +        *snapshot_icount = nearest->icount;
>> +    }
>> +    g_free(sn_tab);
>> +
>> +fail:
>> +    return ret;
>> +}
>> +
>> +static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
>> +{
>> +    char *snapshot = NULL;
>> +    int64_t snapshot_icount;
>> +
>> +    if (replay_mode != REPLAY_MODE_PLAY) {
>> +        error_setg(errp, "replay must be enabled to seek");
>> +        return;
>> +    }
>> +    if (!replay_snapshot) {
>> +        error_setg(errp, "snapshotting is disabled");
>> +        return;
>> +    }
>> +
>> +    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
>> +    if (snapshot) {
>> +        if (icount < replay_get_current_icount()
>> +            || replay_get_current_icount() < snapshot_icount) {
>> +            vm_stop(RUN_STATE_RESTORE_VM);
>> +            load_snapshot(snapshot, errp);
>> +        }
>> +        g_free(snapshot);
>> +    }
>> +    if (replay_get_current_icount() <= icount) {
>> +        replay_break(icount, callback, NULL);
>> +        vm_start();
>> +    } else {
>> +        error_setg(errp, "cannot seek to the specified instruction count");
>> +    }
>> +}
>> +
>> +void qmp_replay_seek(int64_t icount, Error **errp)
>> +{
>> +    replay_seek(icount, replay_stop_vm, errp);
>> +}
>> +
>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>> +{
>> +    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
>> +    Error *err = NULL;
>> +
>> +    qmp_replay_seek(icount, &err);
>> +    if (err) {
>> +        error_report_err(err);
>> +        error_free(err);
>> +        return;
>> +    }
>> +}
> 
> 



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 12:45   ` Alex Bennée
@ 2020-09-07 13:32     ` Pavel Dovgalyuk
  0 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-07 13:32 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 15:45, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>
>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>> the execution to the specified instruction count.
>> The command automatically loads nearest snapshot and replays the execution
>> to find the desired instruction count.
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   hmp-commands.hx           |   18 +++++++++
>>   include/monitor/hmp.h     |    1
>>   qapi/replay.json          |   20 ++++++++++
>>   replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 131 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index e8ce385879..4288274c4e 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1851,6 +1851,24 @@ SRST
>>     The command is ignored when there are no replay breakpoints.
>>   ERST
>>   
>> +    {
>> +        .name       = "replay_seek",
>> +        .args_type  = "icount:i",
>> +        .params     = "icount",
>> +        .help       = "replay execution to the specified instruction count",
>> +        .cmd        = hmp_replay_seek,
>> +    },
>> +
>> +SRST
>> +``replay_seek`` *icount*
>> +Automatically proceed to the instruction count *icount*, when
>> +replaying the execution. The command automatically loads nearest
>> +snapshot and replays the execution to find the desired instruction.
>> +When there is no preceding snapshot or the execution is not replayed,
>> +then the command fails.
>> +*icount* for the reference may be observed with ``info replay`` command.
>> +ERST
>> +
>>       {
>>           .name       = "info",
>>           .args_type  = "item:s?",
> 
> 
> This seems to break the build:
> 
>    Warning, treated as error:
>    /home/alex/lsrc/qemu.git/docs/../hmp-commands.hx:1863:Definition list ends without a blank line; unexpected unindent.

Thanks, I've added an indent.


Pavel Dovgalyuk


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 10/15] replay: flush rr queue before loading the vmstate
  2020-09-02  8:16 ` [PATCH v3 10/15] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
@ 2020-09-07 13:37   ` Alex Bennée
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 13:37 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> Non-empty record/replay queue prevents saving and loading the VM state,
> because it includes pending bottom halves and block coroutines.
> But when the new VM state is loaded, we don't have to preserve the consistency
> of the current state anymore. Therefore this patch just flushes the queue
> allowing the coroutines to finish and removes checking for empty rr queue
> for load_snapshot function.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 13:27     ` Pavel Dovgalyuk
@ 2020-09-07 14:59       ` Alex Bennée
  2020-09-07 15:46         ` Pavel Dovgalyuk
  0 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 14:59 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> On 07.09.2020 15:58, Alex Bennée wrote:
>> 
>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>> 
>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>
>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>> the execution to the specified instruction count.
>>> The command automatically loads nearest snapshot and replays the execution
>>> to find the desired instruction count.
>> 
>> Should there be an initial snapshot created at instruction 0? Using a
>> separate monitor channel:
>
> Right, you can't go to the prior state, when there is no preceding 
> snapshot available.

It seems creating an initial snapshot automatically would be more user
friendly? What can you do to trigger a snapshot, say for example on a
gdb connect?

>
>> 
>>    (qemu) replay_break 190505
>>    replay_break 190505
>>    (qemu) c
>>    (qemu) info replay
>>    info replay
>>    Replaying execution 'record.out': instruction count = 190505
>>    (qemu) replay_seek 190000
>>    replay_seek 190000
>>    snapshotting is disabled
>> 
>> And then the guest dies with a sigabort:
>
> This could be a bug, thanks.
>
>> 
>>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -monitor telnet:127.0.0.1:4444 -S
>>    *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>>    Hello World! qemu_cortex_a53
>>    double free or corruption (out)
>>    fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)
>> 
>>>
>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>   hmp-commands.hx           |   18 +++++++++
>>>   include/monitor/hmp.h     |    1
>>>   qapi/replay.json          |   20 ++++++++++
>>>   replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 131 insertions(+)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index e8ce385879..4288274c4e 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -1851,6 +1851,24 @@ SRST
>>>     The command is ignored when there are no replay breakpoints.
>>>   ERST
>>>   
>>> +    {
>>> +        .name       = "replay_seek",
>>> +        .args_type  = "icount:i",
>>> +        .params     = "icount",
>>> +        .help       = "replay execution to the specified instruction count",
>>> +        .cmd        = hmp_replay_seek,
>>> +    },
>>> +
>>> +SRST
>>> +``replay_seek`` *icount*
>>> +Automatically proceed to the instruction count *icount*, when
>>> +replaying the execution. The command automatically loads nearest
>>> +snapshot and replays the execution to find the desired instruction.
>>> +When there is no preceding snapshot or the execution is not replayed,
>>> +then the command fails.
>>> +*icount* for the reference may be observed with ``info replay`` command.
>>> +ERST
>>> +
>>>       {
>>>           .name       = "info",
>>>           .args_type  = "item:s?",
>>> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
>>> index 21849bdda5..655eb81a4c 100644
>>> --- a/include/monitor/hmp.h
>>> +++ b/include/monitor/hmp.h
>>> @@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>>>   void hmp_info_replay(Monitor *mon, const QDict *qdict);
>>>   void hmp_replay_break(Monitor *mon, const QDict *qdict);
>>>   void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>>>   
>>>   #endif
>>> diff --git a/qapi/replay.json b/qapi/replay.json
>>> index 173ba76107..bfd83d7591 100644
>>> --- a/qapi/replay.json
>>> +++ b/qapi/replay.json
>>> @@ -99,3 +99,23 @@
>>>   #
>>>   ##
>>>   { 'command': 'replay-delete-break' }
>>> +
>>> +##
>>> +# @replay-seek:
>>> +#
>>> +# Automatically proceed to the instruction count @icount, when
>>> +# replaying the execution. The command automatically loads nearest
>>> +# snapshot and replays the execution to find the desired instruction.
>>> +# When there is no preceding snapshot or the execution is not replayed,
>>> +# then the command fails.
>>> +# icount for the reference may be obtained with @query-replay command.
>>> +#
>>> +# @icount: target instruction count
>>> +#
>>> +# Since: 5.2
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
>>> +##
>>> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
>>> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
>>> index 86e19bb217..cfd0221692 100644
>>> --- a/replay/replay-debugging.c
>>> +++ b/replay/replay-debugging.c
>>> @@ -19,6 +19,8 @@
>>>   #include "qapi/qapi-commands-replay.h"
>>>   #include "qapi/qmp/qdict.h"
>>>   #include "qemu/timer.h"
>>> +#include "block/snapshot.h"
>>> +#include "migration/snapshot.h"
>>>   
>>>   void hmp_info_replay(Monitor *mon, const QDict *qdict)
>>>   {
>>> @@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
>>>           return;
>>>       }
>>>   }
>>> +
>>> +static char *replay_find_nearest_snapshot(int64_t icount,
>>> +                                          int64_t *snapshot_icount)
>>> +{
>>> +    BlockDriverState *bs;
>>> +    QEMUSnapshotInfo *sn_tab;
>>> +    QEMUSnapshotInfo *nearest = NULL;
>>> +    char *ret = NULL;
>>> +    int nb_sns, i;
>>> +    AioContext *aio_context;
>>> +
>>> +    *snapshot_icount = -1;
>>> +
>>> +    bs = bdrv_all_find_vmstate_bs();
>>> +    if (!bs) {
>>> +        goto fail;
>>> +    }
>>> +    aio_context = bdrv_get_aio_context(bs);
>>> +
>>> +    aio_context_acquire(aio_context);
>>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>>> +    aio_context_release(aio_context);
>>> +
>>> +    for (i = 0; i < nb_sns; i++) {
>>> +        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
>>> +            if (sn_tab[i].icount != -1ULL
>>> +                && sn_tab[i].icount <= icount
>>> +                && (!nearest || nearest->icount < sn_tab[i].icount)) {
>>> +                nearest = &sn_tab[i];
>>> +            }
>>> +        }
>>> +    }
>>> +    if (nearest) {
>>> +        ret = g_strdup(nearest->name);
>>> +        *snapshot_icount = nearest->icount;
>>> +    }
>>> +    g_free(sn_tab);
>>> +
>>> +fail:
>>> +    return ret;
>>> +}
>>> +
>>> +static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
>>> +{
>>> +    char *snapshot = NULL;
>>> +    int64_t snapshot_icount;
>>> +
>>> +    if (replay_mode != REPLAY_MODE_PLAY) {
>>> +        error_setg(errp, "replay must be enabled to seek");
>>> +        return;
>>> +    }
>>> +    if (!replay_snapshot) {
>>> +        error_setg(errp, "snapshotting is disabled");
>>> +        return;
>>> +    }
>>> +
>>> +    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
>>> +    if (snapshot) {
>>> +        if (icount < replay_get_current_icount()
>>> +            || replay_get_current_icount() < snapshot_icount) {
>>> +            vm_stop(RUN_STATE_RESTORE_VM);
>>> +            load_snapshot(snapshot, errp);
>>> +        }
>>> +        g_free(snapshot);
>>> +    }
>>> +    if (replay_get_current_icount() <= icount) {
>>> +        replay_break(icount, callback, NULL);
>>> +        vm_start();
>>> +    } else {
>>> +        error_setg(errp, "cannot seek to the specified instruction count");
>>> +    }
>>> +}
>>> +
>>> +void qmp_replay_seek(int64_t icount, Error **errp)
>>> +{
>>> +    replay_seek(icount, replay_stop_vm, errp);
>>> +}
>>> +
>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>>> +{
>>> +    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
>>> +    Error *err = NULL;
>>> +
>>> +    qmp_replay_seek(icount, &err);
>>> +    if (err) {
>>> +        error_report_err(err);
>>> +        error_free(err);
>>> +        return;
>>> +    }
>>> +}
>> 
>> 


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-02  8:16 ` [PATCH v3 05/15] iotests: update snapshot test for new output format Pavel Dovgalyuk
@ 2020-09-07 15:26   ` Alex Bennée
  2020-09-07 15:41     ` Pavel Dovgalyuk
  2020-09-08 13:10   ` Eric Blake
  1 sibling, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 15:26 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>
>
> This patch updates iotests that verify qemu monitor output.
> New output format for snapshot listing include ICOUNT column.

I was curious if the monitor should still function during replay. In my
setup:

  ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial mon:stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -s -S
  *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
  Hello World! qemu_cortex_a53
  qemu-system-aarch64: Missing character write event in the replay log

although technically the C-a shouldn't be a character that ever makes it
to the guest. 

>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
> index 215902b3ad..27471ffae8 100644
> --- a/tests/qemu-iotests/267.out
> +++ b/tests/qemu-iotests/267.out
> @@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
> @@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
>  Internal snapshots on overlay:
>  Snapshot list:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  Internal snapshots on backing file:
>  
>  === -blockdev with NBD server on the backing file ===
> @@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) savevm snap0
>  (qemu) info snapshots
>  List of snapshots present on all disks:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  (qemu) loadvm snap0
>  (qemu) quit
>  
>  Internal snapshots on overlay:
>  Snapshot list:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  Internal snapshots on backing file:
>  Snapshot list:
> -ID        TAG                 VM SIZE                DATE       VM CLOCK
> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000           
>  *** done


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-07 15:26   ` Alex Bennée
@ 2020-09-07 15:41     ` Pavel Dovgalyuk
  2020-09-07 16:00       ` Alex Bennée
  0 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-07 15:41 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 18:26, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>
>>
>> This patch updates iotests that verify qemu monitor output.
>> New output format for snapshot listing include ICOUNT column.
> 
> I was curious if the monitor should still function during replay. In my
> setup:
> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial mon:stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -s -S
>    *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>    Hello World! qemu_cortex_a53
>    qemu-system-aarch64: Missing character write event in the replay log

And what about -monitor stdio instead of -serial mon:stdio?

> 
> although technically the C-a shouldn't be a character that ever makes it
> to the guest.
> 
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>> ---
>>   tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
>>   1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
>> index 215902b3ad..27471ffae8 100644
>> --- a/tests/qemu-iotests/267.out
>> +++ b/tests/qemu-iotests/267.out
>> @@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>> @@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>>   Internal snapshots on overlay:
>>   Snapshot list:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   Internal snapshots on backing file:
>>   
>>   === -blockdev with NBD server on the backing file ===
>> @@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>   (qemu) savevm snap0
>>   (qemu) info snapshots
>>   List of snapshots present on all disks:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   (qemu) loadvm snap0
>>   (qemu) quit
>>   
>>   Internal snapshots on overlay:
>>   Snapshot list:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   Internal snapshots on backing file:
>>   Snapshot list:
>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>   *** done
> 
> 



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 14:59       ` Alex Bennée
@ 2020-09-07 15:46         ` Pavel Dovgalyuk
  2020-09-07 16:25           ` Alex Bennée
  0 siblings, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-07 15:46 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 17:59, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>
>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>
>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>> the execution to the specified instruction count.
>>>> The command automatically loads nearest snapshot and replays the execution
>>>> to find the desired instruction count.
>>>
>>> Should there be an initial snapshot created at instruction 0? Using a
>>> separate monitor channel:
>>
>> Right, you can't go to the prior state, when there is no preceding
>> snapshot available.
> 
> It seems creating an initial snapshot automatically would be more user

Please take a look at 'Snapshotting' section of docs/replay.txt.
Reverse debugging is considered to be run with disk image (overlay)
and rrsnapshot option of icount, which allows creating an initial
VM snapshot.

> friendly? What can you do to trigger a snapshot, say for example on a
> gdb connect?

This makes sense when executing with temporary overlay, thanks.

> 
>>
>>>
>>>     (qemu) replay_break 190505
>>>     replay_break 190505
>>>     (qemu) c
>>>     (qemu) info replay
>>>     info replay
>>>     Replaying execution 'record.out': instruction count = 190505
>>>     (qemu) replay_seek 190000
>>>     replay_seek 190000
>>>     snapshotting is disabled
>>>
>>> And then the guest dies with a sigabort:
>>
>> This could be a bug, thanks.
>>
>>>
>>>     ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -monitor telnet:127.0.0.1:4444 -S
>>>     *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>>>     Hello World! qemu_cortex_a53
>>>     double free or corruption (out)
>>>     fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)
>>>
>>>>
>>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>>    hmp-commands.hx           |   18 +++++++++
>>>>    include/monitor/hmp.h     |    1
>>>>    qapi/replay.json          |   20 ++++++++++
>>>>    replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 131 insertions(+)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index e8ce385879..4288274c4e 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -1851,6 +1851,24 @@ SRST
>>>>      The command is ignored when there are no replay breakpoints.
>>>>    ERST
>>>>    
>>>> +    {
>>>> +        .name       = "replay_seek",
>>>> +        .args_type  = "icount:i",
>>>> +        .params     = "icount",
>>>> +        .help       = "replay execution to the specified instruction count",
>>>> +        .cmd        = hmp_replay_seek,
>>>> +    },
>>>> +
>>>> +SRST
>>>> +``replay_seek`` *icount*
>>>> +Automatically proceed to the instruction count *icount*, when
>>>> +replaying the execution. The command automatically loads nearest
>>>> +snapshot and replays the execution to find the desired instruction.
>>>> +When there is no preceding snapshot or the execution is not replayed,
>>>> +then the command fails.
>>>> +*icount* for the reference may be observed with ``info replay`` command.
>>>> +ERST
>>>> +
>>>>        {
>>>>            .name       = "info",
>>>>            .args_type  = "item:s?",
>>>> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
>>>> index 21849bdda5..655eb81a4c 100644
>>>> --- a/include/monitor/hmp.h
>>>> +++ b/include/monitor/hmp.h
>>>> @@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>>>>    void hmp_info_replay(Monitor *mon, const QDict *qdict);
>>>>    void hmp_replay_break(Monitor *mon, const QDict *qdict);
>>>>    void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
>>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>>>>    
>>>>    #endif
>>>> diff --git a/qapi/replay.json b/qapi/replay.json
>>>> index 173ba76107..bfd83d7591 100644
>>>> --- a/qapi/replay.json
>>>> +++ b/qapi/replay.json
>>>> @@ -99,3 +99,23 @@
>>>>    #
>>>>    ##
>>>>    { 'command': 'replay-delete-break' }
>>>> +
>>>> +##
>>>> +# @replay-seek:
>>>> +#
>>>> +# Automatically proceed to the instruction count @icount, when
>>>> +# replaying the execution. The command automatically loads nearest
>>>> +# snapshot and replays the execution to find the desired instruction.
>>>> +# When there is no preceding snapshot or the execution is not replayed,
>>>> +# then the command fails.
>>>> +# icount for the reference may be obtained with @query-replay command.
>>>> +#
>>>> +# @icount: target instruction count
>>>> +#
>>>> +# Since: 5.2
>>>> +#
>>>> +# Example:
>>>> +#
>>>> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
>>>> +##
>>>> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
>>>> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
>>>> index 86e19bb217..cfd0221692 100644
>>>> --- a/replay/replay-debugging.c
>>>> +++ b/replay/replay-debugging.c
>>>> @@ -19,6 +19,8 @@
>>>>    #include "qapi/qapi-commands-replay.h"
>>>>    #include "qapi/qmp/qdict.h"
>>>>    #include "qemu/timer.h"
>>>> +#include "block/snapshot.h"
>>>> +#include "migration/snapshot.h"
>>>>    
>>>>    void hmp_info_replay(Monitor *mon, const QDict *qdict)
>>>>    {
>>>> @@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
>>>>            return;
>>>>        }
>>>>    }
>>>> +
>>>> +static char *replay_find_nearest_snapshot(int64_t icount,
>>>> +                                          int64_t *snapshot_icount)
>>>> +{
>>>> +    BlockDriverState *bs;
>>>> +    QEMUSnapshotInfo *sn_tab;
>>>> +    QEMUSnapshotInfo *nearest = NULL;
>>>> +    char *ret = NULL;
>>>> +    int nb_sns, i;
>>>> +    AioContext *aio_context;
>>>> +
>>>> +    *snapshot_icount = -1;
>>>> +
>>>> +    bs = bdrv_all_find_vmstate_bs();
>>>> +    if (!bs) {
>>>> +        goto fail;
>>>> +    }
>>>> +    aio_context = bdrv_get_aio_context(bs);
>>>> +
>>>> +    aio_context_acquire(aio_context);
>>>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>>>> +    aio_context_release(aio_context);
>>>> +
>>>> +    for (i = 0; i < nb_sns; i++) {
>>>> +        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
>>>> +            if (sn_tab[i].icount != -1ULL
>>>> +                && sn_tab[i].icount <= icount
>>>> +                && (!nearest || nearest->icount < sn_tab[i].icount)) {
>>>> +                nearest = &sn_tab[i];
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +    if (nearest) {
>>>> +        ret = g_strdup(nearest->name);
>>>> +        *snapshot_icount = nearest->icount;
>>>> +    }
>>>> +    g_free(sn_tab);
>>>> +
>>>> +fail:
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
>>>> +{
>>>> +    char *snapshot = NULL;
>>>> +    int64_t snapshot_icount;
>>>> +
>>>> +    if (replay_mode != REPLAY_MODE_PLAY) {
>>>> +        error_setg(errp, "replay must be enabled to seek");
>>>> +        return;
>>>> +    }
>>>> +    if (!replay_snapshot) {
>>>> +        error_setg(errp, "snapshotting is disabled");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
>>>> +    if (snapshot) {
>>>> +        if (icount < replay_get_current_icount()
>>>> +            || replay_get_current_icount() < snapshot_icount) {
>>>> +            vm_stop(RUN_STATE_RESTORE_VM);
>>>> +            load_snapshot(snapshot, errp);
>>>> +        }
>>>> +        g_free(snapshot);
>>>> +    }
>>>> +    if (replay_get_current_icount() <= icount) {
>>>> +        replay_break(icount, callback, NULL);
>>>> +        vm_start();
>>>> +    } else {
>>>> +        error_setg(errp, "cannot seek to the specified instruction count");
>>>> +    }
>>>> +}
>>>> +
>>>> +void qmp_replay_seek(int64_t icount, Error **errp)
>>>> +{
>>>> +    replay_seek(icount, replay_stop_vm, errp);
>>>> +}
>>>> +
>>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>>>> +{
>>>> +    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
>>>> +    Error *err = NULL;
>>>> +
>>>> +    qmp_replay_seek(icount, &err);
>>>> +    if (err) {
>>>> +        error_report_err(err);
>>>> +        error_free(err);
>>>> +        return;
>>>> +    }
>>>> +}
>>>
>>>
> 
> 



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-07 15:41     ` Pavel Dovgalyuk
@ 2020-09-07 16:00       ` Alex Bennée
  2020-09-07 16:05         ` Pavel Dovgalyuk
  0 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 16:00 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> On 07.09.2020 18:26, Alex Bennée wrote:
>> 
>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>> 
>>> From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>
>>>
>>> This patch updates iotests that verify qemu monitor output.
>>> New output format for snapshot listing include ICOUNT column.
>> 
>> I was curious if the monitor should still function during replay. In my
>> setup:
>> 
>>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial mon:stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -s -S
>>    *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>>    Hello World! qemu_cortex_a53
>>    qemu-system-aarch64: Missing character write event in the replay log
>
> And what about -monitor stdio instead of -serial mon:stdio?

Well I switched to:

  -monitor telnet:127.0.0.1:4444

and controlled that way. I appreciate having a multiplexed
serial/monitor is a tricky edge case but I'm curious as to why it broke.


>
>> 
>> although technically the C-a shouldn't be a character that ever makes it
>> to the guest.
>> 
>>>
>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>> ---
>>>   tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
>>>   1 file changed, 24 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
>>> index 215902b3ad..27471ffae8 100644
>>> --- a/tests/qemu-iotests/267.out
>>> +++ b/tests/qemu-iotests/267.out
>>> @@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>> @@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>>   Internal snapshots on overlay:
>>>   Snapshot list:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   Internal snapshots on backing file:
>>>   
>>>   === -blockdev with NBD server on the backing file ===
>>> @@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>   (qemu) savevm snap0
>>>   (qemu) info snapshots
>>>   List of snapshots present on all disks:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   (qemu) loadvm snap0
>>>   (qemu) quit
>>>   
>>>   Internal snapshots on overlay:
>>>   Snapshot list:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   Internal snapshots on backing file:
>>>   Snapshot list:
>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>   *** done
>> 
>> 


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-07 16:00       ` Alex Bennée
@ 2020-09-07 16:05         ` Pavel Dovgalyuk
  0 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-07 16:05 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 19:00, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> On 07.09.2020 18:26, Alex Bennée wrote:
>>>
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>
>>>> From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>
>>>>
>>>> This patch updates iotests that verify qemu monitor output.
>>>> New output format for snapshot listing include ICOUNT column.
>>>
>>> I was curious if the monitor should still function during replay. In my
>>> setup:
>>>
>>>     ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial mon:stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -s -S
>>>     *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>>>     Hello World! qemu_cortex_a53
>>>     qemu-system-aarch64: Missing character write event in the replay log
>>
>> And what about -monitor stdio instead of -serial mon:stdio?
> 
> Well I switched to:
> 
>    -monitor telnet:127.0.0.1:4444
> 
> and controlled that way. I appreciate having a multiplexed
> serial/monitor is a tricky edge case but I'm curious as to why it broke.

Serial port interaction is recorded and replayed.
Therefore configuration of the serial ports should match in 
record/replay command lines.

> 
> 
>>
>>>
>>> although technically the C-a shouldn't be a character that ever makes it
>>> to the guest.
>>>
>>>>
>>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>>> ---
>>>>    tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
>>>>    1 file changed, 24 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
>>>> index 215902b3ad..27471ffae8 100644
>>>> --- a/tests/qemu-iotests/267.out
>>>> +++ b/tests/qemu-iotests/267.out
>>>> @@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>> @@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>>    Internal snapshots on overlay:
>>>>    Snapshot list:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    Internal snapshots on backing file:
>>>>    
>>>>    === -blockdev with NBD server on the backing file ===
>>>> @@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>>>    (qemu) savevm snap0
>>>>    (qemu) info snapshots
>>>>    List of snapshots present on all disks:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> ---        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +--        snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    (qemu) loadvm snap0
>>>>    (qemu) quit
>>>>    
>>>>    Internal snapshots on overlay:
>>>>    Snapshot list:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    Internal snapshots on backing file:
>>>>    Snapshot list:
>>>> -ID        TAG                 VM SIZE                DATE       VM CLOCK
>>>> -1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
>>>> +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>> +1         snap0                SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
>>>>    *** done
>>>
>>>
> 
> 



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 15:46         ` Pavel Dovgalyuk
@ 2020-09-07 16:25           ` Alex Bennée
  2020-09-08  7:44             ` Pavel Dovgalyuk
  2020-09-08 10:54             ` Pavel Dovgalyuk
  0 siblings, 2 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 16:25 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> On 07.09.2020 17:59, Alex Bennée wrote:
>> 
>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>> 
>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>
>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>
>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>
>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>> the execution to the specified instruction count.
>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>> to find the desired instruction count.
>>>>
>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>> separate monitor channel:
>>>
>>> Right, you can't go to the prior state, when there is no preceding
>>> snapshot available.
>> 
>> It seems creating an initial snapshot automatically would be more user
>
> Please take a look at 'Snapshotting' section of docs/replay.txt.
> Reverse debugging is considered to be run with disk image (overlay)
> and rrsnapshot option of icount, which allows creating an initial
> VM snapshot.

Given that I'm using the block device purely for VM snapshots I think it
would be useful to document the minimal "no disk" approach - i.e. where
the disk is only used for record/replay.

However I'm still having trouble. I can record the trace with:

  ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
    -machine virt -kernel zephyr.elf -net none \
    -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
    -drive file=record.qcow2,if=none,id=rr \
    -monitor telnet:127.0.0.1:4444 -S

which shows:

  (qemu) info snapshots
  info snapshots
  List of snapshots present on all disks:
  ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
  --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0

but do I need a whole separate overlay in the replay case? I thought
supplying snapshot to the drive would prevent the replay case
overwriting what has been recorded but with:

    -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
    -drive file=record.qcow2,if=none,id=rr,snapshot

but I get:

  (qemu) info snapshots
  info snapshots
  There is no snapshot available.

so if I drop the ,snapshot from the line I can at least see the snapshot
but continue doesn't seem to work:

  (qemu) info snapshots
  info snapshots
  List of snapshots present on all disks:
  ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
  --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
  (qemu) replay_break 190505
  replay_break 190505
  (qemu) c
  c
  (qemu) info replay
  info replay
  Replaying execution 'record.out': instruction count = 0
  (qemu)

If I manually loadvm then we get somewhere but replay_seek breaks:

  (qemu) loadvm rrstart
  loadvm rrstart
  (qemu) info replay
  info replay
  Replaying execution 'record.out': instruction count = 190505
  (qemu) replay_seek 190000
  replay_seek 190000
  snapshotting is disabled

with a crash:

  ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out
 -drive file=record.qcow2,if=none,id=rr -monitor telnet:127.0.0.1:4444 -S
*** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
Hello World! qemu_cortex_a53
free(): invalid pointer
fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)


>
>> friendly? What can you do to trigger a snapshot, say for example on a
>> gdb connect?
>
> This makes sense when executing with temporary overlay, thanks.
>
>> 
>>>
>>>>
>>>>     (qemu) replay_break 190505
>>>>     replay_break 190505
>>>>     (qemu) c
>>>>     (qemu) info replay
>>>>     info replay
>>>>     Replaying execution 'record.out': instruction count = 190505
>>>>     (qemu) replay_seek 190000
>>>>     replay_seek 190000
>>>>     snapshotting is disabled
>>>>
>>>> And then the guest dies with a sigabort:
>>>
>>> This could be a bug, thanks.
>>>
>>>>
>>>>     ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out -drive file=record.qcow2,if=none,snapshot,id=rr -monitor telnet:127.0.0.1:4444 -S
>>>>     *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
>>>>     Hello World! qemu_cortex_a53
>>>>     double free or corruption (out)
>>>>     fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)
>>>>
>>>>>
>>>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>>>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>>>> ---
>>>>>    hmp-commands.hx           |   18 +++++++++
>>>>>    include/monitor/hmp.h     |    1
>>>>>    qapi/replay.json          |   20 ++++++++++
>>>>>    replay/replay-debugging.c |   92 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>    4 files changed, 131 insertions(+)
>>>>>
>>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>>> index e8ce385879..4288274c4e 100644
>>>>> --- a/hmp-commands.hx
>>>>> +++ b/hmp-commands.hx
>>>>> @@ -1851,6 +1851,24 @@ SRST
>>>>>      The command is ignored when there are no replay breakpoints.
>>>>>    ERST
>>>>>    
>>>>> +    {
>>>>> +        .name       = "replay_seek",
>>>>> +        .args_type  = "icount:i",
>>>>> +        .params     = "icount",
>>>>> +        .help       = "replay execution to the specified instruction count",
>>>>> +        .cmd        = hmp_replay_seek,
>>>>> +    },
>>>>> +
>>>>> +SRST
>>>>> +``replay_seek`` *icount*
>>>>> +Automatically proceed to the instruction count *icount*, when
>>>>> +replaying the execution. The command automatically loads nearest
>>>>> +snapshot and replays the execution to find the desired instruction.
>>>>> +When there is no preceding snapshot or the execution is not replayed,
>>>>> +then the command fails.
>>>>> +*icount* for the reference may be observed with ``info replay`` command.
>>>>> +ERST
>>>>> +
>>>>>        {
>>>>>            .name       = "info",
>>>>>            .args_type  = "item:s?",
>>>>> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
>>>>> index 21849bdda5..655eb81a4c 100644
>>>>> --- a/include/monitor/hmp.h
>>>>> +++ b/include/monitor/hmp.h
>>>>> @@ -133,5 +133,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>>>>>    void hmp_info_replay(Monitor *mon, const QDict *qdict);
>>>>>    void hmp_replay_break(Monitor *mon, const QDict *qdict);
>>>>>    void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
>>>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>>>>>    
>>>>>    #endif
>>>>> diff --git a/qapi/replay.json b/qapi/replay.json
>>>>> index 173ba76107..bfd83d7591 100644
>>>>> --- a/qapi/replay.json
>>>>> +++ b/qapi/replay.json
>>>>> @@ -99,3 +99,23 @@
>>>>>    #
>>>>>    ##
>>>>>    { 'command': 'replay-delete-break' }
>>>>> +
>>>>> +##
>>>>> +# @replay-seek:
>>>>> +#
>>>>> +# Automatically proceed to the instruction count @icount, when
>>>>> +# replaying the execution. The command automatically loads nearest
>>>>> +# snapshot and replays the execution to find the desired instruction.
>>>>> +# When there is no preceding snapshot or the execution is not replayed,
>>>>> +# then the command fails.
>>>>> +# icount for the reference may be obtained with @query-replay command.
>>>>> +#
>>>>> +# @icount: target instruction count
>>>>> +#
>>>>> +# Since: 5.2
>>>>> +#
>>>>> +# Example:
>>>>> +#
>>>>> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
>>>>> +##
>>>>> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
>>>>> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
>>>>> index 86e19bb217..cfd0221692 100644
>>>>> --- a/replay/replay-debugging.c
>>>>> +++ b/replay/replay-debugging.c
>>>>> @@ -19,6 +19,8 @@
>>>>>    #include "qapi/qapi-commands-replay.h"
>>>>>    #include "qapi/qmp/qdict.h"
>>>>>    #include "qemu/timer.h"
>>>>> +#include "block/snapshot.h"
>>>>> +#include "migration/snapshot.h"
>>>>>    
>>>>>    void hmp_info_replay(Monitor *mon, const QDict *qdict)
>>>>>    {
>>>>> @@ -127,3 +129,93 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
>>>>>            return;
>>>>>        }
>>>>>    }
>>>>> +
>>>>> +static char *replay_find_nearest_snapshot(int64_t icount,
>>>>> +                                          int64_t *snapshot_icount)
>>>>> +{
>>>>> +    BlockDriverState *bs;
>>>>> +    QEMUSnapshotInfo *sn_tab;
>>>>> +    QEMUSnapshotInfo *nearest = NULL;
>>>>> +    char *ret = NULL;
>>>>> +    int nb_sns, i;
>>>>> +    AioContext *aio_context;
>>>>> +
>>>>> +    *snapshot_icount = -1;
>>>>> +
>>>>> +    bs = bdrv_all_find_vmstate_bs();
>>>>> +    if (!bs) {
>>>>> +        goto fail;
>>>>> +    }
>>>>> +    aio_context = bdrv_get_aio_context(bs);
>>>>> +
>>>>> +    aio_context_acquire(aio_context);
>>>>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>>>>> +    aio_context_release(aio_context);
>>>>> +
>>>>> +    for (i = 0; i < nb_sns; i++) {
>>>>> +        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
>>>>> +            if (sn_tab[i].icount != -1ULL
>>>>> +                && sn_tab[i].icount <= icount
>>>>> +                && (!nearest || nearest->icount < sn_tab[i].icount)) {
>>>>> +                nearest = &sn_tab[i];
>>>>> +            }
>>>>> +        }
>>>>> +    }
>>>>> +    if (nearest) {
>>>>> +        ret = g_strdup(nearest->name);
>>>>> +        *snapshot_icount = nearest->icount;
>>>>> +    }
>>>>> +    g_free(sn_tab);
>>>>> +
>>>>> +fail:
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
>>>>> +{
>>>>> +    char *snapshot = NULL;
>>>>> +    int64_t snapshot_icount;
>>>>> +
>>>>> +    if (replay_mode != REPLAY_MODE_PLAY) {
>>>>> +        error_setg(errp, "replay must be enabled to seek");
>>>>> +        return;
>>>>> +    }
>>>>> +    if (!replay_snapshot) {
>>>>> +        error_setg(errp, "snapshotting is disabled");
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
>>>>> +    if (snapshot) {
>>>>> +        if (icount < replay_get_current_icount()
>>>>> +            || replay_get_current_icount() < snapshot_icount) {
>>>>> +            vm_stop(RUN_STATE_RESTORE_VM);
>>>>> +            load_snapshot(snapshot, errp);
>>>>> +        }
>>>>> +        g_free(snapshot);
>>>>> +    }
>>>>> +    if (replay_get_current_icount() <= icount) {
>>>>> +        replay_break(icount, callback, NULL);
>>>>> +        vm_start();
>>>>> +    } else {
>>>>> +        error_setg(errp, "cannot seek to the specified instruction count");
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +void qmp_replay_seek(int64_t icount, Error **errp)
>>>>> +{
>>>>> +    replay_seek(icount, replay_stop_vm, errp);
>>>>> +}
>>>>> +
>>>>> +void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>>>>> +{
>>>>> +    int64_t icount = qdict_get_try_int(qdict, "icount", -1LL);
>>>>> +    Error *err = NULL;
>>>>> +
>>>>> +    qmp_replay_seek(icount, &err);
>>>>> +    if (err) {
>>>>> +        error_report_err(err);
>>>>> +        error_free(err);
>>>>> +        return;
>>>>> +    }
>>>>> +}
>>>>
>>>>
>> 
>> 


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 11/15] gdbstub: add reverse step support in replay mode
  2020-09-02  8:16 ` [PATCH v3 11/15] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
@ 2020-09-07 16:30   ` Alex Bennée
  2020-09-08 11:16   ` Alex Bennée
  1 sibling, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-07 16:30 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> GDB remote protocol supports two reverse debugging commands:
> reverse step and reverse continue.
> This patch adds support of the first one to the gdbstub.
> Reverse step is intended to step one instruction in the backwards
> direction. This is not possible in regular execution.
> But replayed execution is deterministic, therefore we can load one of
> the prior snapshots and proceed to the desired step. It is equivalent
> to stepping one instruction back.
> There should be at least one snapshot preceding the debugged part of
> the replay log.

Apropos of the 10/15 thread I currently get:

  (gdb) reverse-stepi
  warning: Remote failure reply: E14

  Program stopped.
  _isr_wrapper () at /home/galak/git/zephyr/arch/arm/core/aarch64/isr_wrapper.S:36
  36      in /home/galak/git/zephyr/arch/arm/core/aarch64/isr_wrapper.S

After having manually triggered a loadvm rrstart in the monitor. The
step never happened:

  (qemu) loadvm rrstart
  loadvm rrstart
  (qemu) info replay
  info replay
  Replaying execution 'record.out': instruction count = 190506

  * reverse-stepi called in gdb window *

  (qemu) info replay
  info replay
  Replaying execution 'record.out': instruction count = 190506
  (qemu) info snapshots
  info snapshots
  List of snapshots present on all disks:
  ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
  --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0


>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  accel/tcg/translator.c    |    1 +
>  exec.c                    |    7 ++++++
>  gdbstub.c                 |   55 +++++++++++++++++++++++++++++++++++++++++++--
>  include/sysemu/replay.h   |   11 +++++++++
>  replay/replay-debugging.c |   33 +++++++++++++++++++++++++++
>  softmmu/cpus.c            |   14 +++++++++--
>  stubs/replay.c            |    5 ++++
>  7 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 603d17ff83..fb1e19c585 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -17,6 +17,7 @@
>  #include "exec/log.h"
>  #include "exec/translator.h"
>  #include "exec/plugin-gen.h"
> +#include "sysemu/replay.h"
>  
>  /* Pairs with tcg_clear_temp_count.
>     To be called by #TranslatorOps.{translate_insn,tb_stop} if
> diff --git a/exec.c b/exec.c
> index 7683afb6a8..47512e950c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2750,6 +2750,13 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
>      QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
>          if (watchpoint_address_matches(wp, addr, len)
>              && (wp->flags & flags)) {
> +            if (replay_running_debug()) {
> +                /*
> +                 * Don't process the watchpoints when we are
> +                 * in a reverse debugging operation.
> +                 */
> +                return;
> +            }
>              if (flags == BP_MEM_READ) {
>                  wp->flags |= BP_WATCHPOINT_HIT_READ;
>              } else {
> diff --git a/gdbstub.c b/gdbstub.c
> index 9dfb6e4142..79e8ccc050 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -51,6 +51,7 @@
>  #include "sysemu/runstate.h"
>  #include "hw/semihosting/semihost.h"
>  #include "exec/exec-all.h"
> +#include "sysemu/replay.h"
>  
>  #ifdef CONFIG_USER_ONLY
>  #define GDB_ATTACHED "0"
> @@ -375,6 +376,20 @@ typedef struct GDBState {
>   */
>  static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
>  
> +/* Retrieves flags for single step mode. */
> +static int get_sstep_flags(void)
> +{
> +    /*
> +     * In replay mode all events written into the log should be replayed.
> +     * That is why NOIRQ flag is removed in this mode.
> +     */
> +    if (replay_mode != REPLAY_MODE_NONE) {
> +        return SSTEP_ENABLE;
> +    } else {
> +        return sstep_flags;
> +    }
> +}
> +
>  static GDBState gdbserver_state;
>  
>  static void init_gdbserver_state(void)
> @@ -501,7 +516,7 @@ static int gdb_continue_partial(char *newstates)
>                  break; /* nothing to do here */
>              case 's':
>                  trace_gdbstub_op_stepping(cpu->cpu_index);
> -                cpu_single_step(cpu, sstep_flags);
> +                cpu_single_step(cpu, get_sstep_flags());
>                  cpu_resume(cpu);
>                  flag = 1;
>                  break;
> @@ -1874,10 +1889,31 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
>          gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
>      }
>  
> -    cpu_single_step(gdbserver_state.c_cpu, sstep_flags);
> +    cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags());
>      gdb_continue();
>  }
>  
> +static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (replay_mode != REPLAY_MODE_PLAY) {
> +        put_packet("E22");
> +    }
> +    if (gdb_ctx->num_params == 1) {
> +        switch (gdb_ctx->params[0].opcode) {
> +        case 's':
> +            if (replay_reverse_step()) {
> +                gdb_continue();
> +            } else {
> +                put_packet("E14");
> +            }
> +            return;
> +        }
> +    }
> +
> +    /* Default invalid command */
> +    put_packet("");
> +}
> +
>  static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
>  {
>      put_packet("vCont;c;C;s;S");
> @@ -2124,6 +2160,10 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
>          g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
>      }
>  
> +    if (replay_mode == REPLAY_MODE_PLAY) {
> +        g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
> +    }
> +
>      if (gdb_ctx->num_params &&
>          strstr(gdb_ctx->params[0].data, "multiprocess+")) {
>          gdbserver_state.multiprocess = true;
> @@ -2460,6 +2500,17 @@ static int gdb_handle_packet(const char *line_buf)
>              cmd_parser = &step_cmd_desc;
>          }
>          break;
> +    case 'b':
> +        {
> +            static const GdbCmdParseEntry backward_cmd_desc = {
> +                .handler = handle_backward,
> +                .cmd = "b",
> +                .cmd_startswith = 1,
> +                .schema = "o0"
> +            };
> +            cmd_parser = &backward_cmd_desc;
> +        }
> +        break;
>      case 'F':
>          {
>              static const GdbCmdParseEntry file_io_cmd_desc = {
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 239c01e7df..13a8123b09 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -75,6 +75,17 @@ void replay_finish(void);
>  void replay_add_blocker(Error *reason);
>  /* Returns name of the replay log file */
>  const char *replay_get_filename(void);
> +/*
> + * Start making one step in backward direction.
> + * Used by gdbstub for backwards debugging.
> + * Returns true on success.
> + */
> +bool replay_reverse_step(void);
> +/*
> + * Returns true if replay module is processing
> + * reverse_continue or reverse_step request
> + */
> +bool replay_running_debug(void);
>  
>  /* Processing the instructions */
>  
> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> index cfd0221692..aa3ca040e2 100644
> --- a/replay/replay-debugging.c
> +++ b/replay/replay-debugging.c
> @@ -22,6 +22,13 @@
>  #include "block/snapshot.h"
>  #include "migration/snapshot.h"
>  
> +static bool replay_is_debugging;
> +
> +bool replay_running_debug(void)
> +{
> +    return replay_is_debugging;
> +}
> +
>  void hmp_info_replay(Monitor *mon, const QDict *qdict)
>  {
>      if (replay_mode == REPLAY_MODE_NONE) {
> @@ -219,3 +226,29 @@ void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>          return;
>      }
>  }
> +
> +static void replay_stop_vm_debug(void *opaque)
> +{
> +    replay_is_debugging = false;
> +    vm_stop(RUN_STATE_DEBUG);
> +    replay_delete_break();
> +}
> +
> +bool replay_reverse_step(void)
> +{
> +    Error *err = NULL;
> +
> +    assert(replay_mode == REPLAY_MODE_PLAY);
> +
> +    if (replay_get_current_icount() != 0) {
> +        replay_seek(replay_get_current_icount() - 1, replay_stop_vm_debug, &err);
> +        if (err) {
> +            error_free(err);
> +            return false;
> +        }
> +        replay_is_debugging = true;
> +        return true;
> +    }
> +
> +    return false;
> +}
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index a802e899ab..377fe3298c 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -1004,9 +1004,17 @@ static bool cpu_can_run(CPUState *cpu)
>  
>  static void cpu_handle_guest_debug(CPUState *cpu)
>  {
> -    gdb_set_stop_cpu(cpu);
> -    qemu_system_debug_request();
> -    cpu->stopped = true;
> +    if (!replay_running_debug()) {
> +        gdb_set_stop_cpu(cpu);
> +        qemu_system_debug_request();
> +        cpu->stopped = true;
> +    } else {
> +        if (!cpu->singlestep_enabled) {
> +            cpu_single_step(cpu, SSTEP_ENABLE);
> +        } else {
> +            cpu_single_step(cpu, 0);
> +        }
> +    }
>  }
>  
>  #ifdef CONFIG_LINUX
> diff --git a/stubs/replay.c b/stubs/replay.c
> index eacb366aa8..d5b52302e9 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -93,3 +93,8 @@ uint64_t replay_get_current_icount(void)
>  {
>      return 0;
>  }
> +
> +bool replay_reverse_step(void)
> +{
> +    return false;
> +}


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 16:25           ` Alex Bennée
@ 2020-09-08  7:44             ` Pavel Dovgalyuk
  2020-09-08  9:13               ` Alex Bennée
  2020-09-08 10:54             ` Pavel Dovgalyuk
  1 sibling, 1 reply; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-08  7:44 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 19:25, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>
>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>
>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>
>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>
>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>> the execution to the specified instruction count.
>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>> to find the desired instruction count.
>>>>>
>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>> separate monitor channel:
>>>>
>>>> Right, you can't go to the prior state, when there is no preceding
>>>> snapshot available.
>>>
>>> It seems creating an initial snapshot automatically would be more user
>>
>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>> Reverse debugging is considered to be run with disk image (overlay)
>> and rrsnapshot option of icount, which allows creating an initial
>> VM snapshot.
> 
> Given that I'm using the block device purely for VM snapshots I think it
> would be useful to document the minimal "no disk" approach - i.e. where
> the disk is only used for record/replay.
> 
> However I'm still having trouble. I can record the trace with:
> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>      -machine virt -kernel zephyr.elf -net none \
>      -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>      -drive file=record.qcow2,if=none,id=rr \
>      -monitor telnet:127.0.0.1:4444 -S

Can you provide your zephyr.elf image?

> 
> which shows:
> 
>    (qemu) info snapshots
>    info snapshots
>    List of snapshots present on all disks:
>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
> 
> but do I need a whole separate overlay in the replay case? I thought
> supplying snapshot to the drive would prevent the replay case
> overwriting what has been recorded but with:
> 
>      -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>      -drive file=record.qcow2,if=none,id=rr,snapshot

When you provide qcow2 (overlay or not) for snapshotting, you don't need 
any 'snapshot' option on drive.

> 
> but I get:
> 
>    (qemu) info snapshots
>    info snapshots
>    There is no snapshot available.
> 
> so if I drop the ,snapshot from the line I can at least see the snapshot
> but continue doesn't seem to work:
> 
>    (qemu) info snapshots
>    info snapshots
>    List of snapshots present on all disks:
>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>    (qemu) replay_break 190505
>    replay_break 190505
>    (qemu) c
>    c
>    (qemu) info replay
>    info replay
>    Replaying execution 'record.out': instruction count = 0

It seems, that replay hangs. Can you try removing '-S' in record command 
line?

>    (qemu)
> 
> If I manually loadvm then we get somewhere but replay_seek breaks:
> 
>    (qemu) loadvm rrstart
>    loadvm rrstart
>    (qemu) info replay
>    info replay
>    Replaying execution 'record.out': instruction count = 190505
>    (qemu) replay_seek 190000
>    replay_seek 190000
>    snapshotting is disabled
> 
> with a crash:
> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out
>   -drive file=record.qcow2,if=none,id=rr -monitor telnet:127.0.0.1:4444 -S
> *** Booting Zephyr OS build zephyr-v2.3.0-1183-ge5628ad0faf3  ***
> Hello World! qemu_cortex_a53
> free(): invalid pointer
> fish: “./qemu-system-aarch64 -cpu cort…” terminated by signal SIGABRT (Abort)
> 
> 
>>
>>> friendly? What can you do to trigger a snapshot, say for example on a
>>> gdb connect?
>>
>> This makes sense when executing with temporary overlay, thanks.
>>
>>>
>>>>
>>>>>

Pavel Dovgalyuk


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-08  7:44             ` Pavel Dovgalyuk
@ 2020-09-08  9:13               ` Alex Bennée
  2020-09-08 10:57                 ` Pavel Dovgalyuk
  2020-09-08 11:10                 ` Alex Bennée
  0 siblings, 2 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-08  9:13 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> On 07.09.2020 19:25, Alex Bennée wrote:
>> 
>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>> 
>>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>>
>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>
>>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>>
>>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>>
>>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>>
>>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>>> the execution to the specified instruction count.
>>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>>> to find the desired instruction count.
>>>>>>
>>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>>> separate monitor channel:
>>>>>
>>>>> Right, you can't go to the prior state, when there is no preceding
>>>>> snapshot available.
>>>>
>>>> It seems creating an initial snapshot automatically would be more user
>>>
>>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>>> Reverse debugging is considered to be run with disk image (overlay)
>>> and rrsnapshot option of icount, which allows creating an initial
>>> VM snapshot.
>> 
>> Given that I'm using the block device purely for VM snapshots I think it
>> would be useful to document the minimal "no disk" approach - i.e. where
>> the disk is only used for record/replay.
>> 
>> However I'm still having trouble. I can record the trace with:
>> 
>>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>>      -machine virt -kernel zephyr.elf -net none \
>>      -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>>      -drive file=record.qcow2,if=none,id=rr \
>>      -monitor telnet:127.0.0.1:4444 -S
>
> Can you provide your zephyr.elf image?
>
>> 
>> which shows:
>> 
>>    (qemu) info snapshots
>>    info snapshots
>>    List of snapshots present on all disks:
>>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>> 
>> but do I need a whole separate overlay in the replay case? I thought
>> supplying snapshot to the drive would prevent the replay case
>> overwriting what has been recorded but with:
>> 
>>      -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>>      -drive file=record.qcow2,if=none,id=rr,snapshot
>
> When you provide qcow2 (overlay or not) for snapshotting, you don't need 
> any 'snapshot' option on drive.
>
>> 
>> but I get:
>> 
>>    (qemu) info snapshots
>>    info snapshots
>>    There is no snapshot available.
>> 
>> so if I drop the ,snapshot from the line I can at least see the snapshot
>> but continue doesn't seem to work:
>> 
>>    (qemu) info snapshots
>>    info snapshots
>>    List of snapshots present on all disks:
>>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>    (qemu) replay_break 190505
>>    replay_break 190505
>>    (qemu) c
>>    c
>>    (qemu) info replay
>>    info replay
>>    Replaying execution 'record.out': instruction count = 0
>
> It seems, that replay hangs. Can you try removing '-S' in record command 
> line?

That doesn't make any difference removing from both the record and
replay cases. It seems to need a loadvm to start things off.

I've sent you an image off list. Please let me know if you can replicate.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-07 16:25           ` Alex Bennée
  2020-09-08  7:44             ` Pavel Dovgalyuk
@ 2020-09-08 10:54             ` Pavel Dovgalyuk
  1 sibling, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-08 10:54 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 07.09.2020 19:25, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>
>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>
>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>
>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>
>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>> the execution to the specified instruction count.
>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>> to find the desired instruction count.
>>>>>
>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>> separate monitor channel:
>>>>
>>>> Right, you can't go to the prior state, when there is no preceding
>>>> snapshot available.
>>>
>>> It seems creating an initial snapshot automatically would be more user
>>
>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>> Reverse debugging is considered to be run with disk image (overlay)
>> and rrsnapshot option of icount, which allows creating an initial
>> VM snapshot.
> 
> Given that I'm using the block device purely for VM snapshots I think it
> would be useful to document the minimal "no disk" approach - i.e. where
> the disk is only used for record/replay.
> 
> However I'm still having trouble. I can record the trace with:
> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>      -machine virt -kernel zephyr.elf -net none \
>      -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>      -drive file=record.qcow2,if=none,id=rr \
>      -monitor telnet:127.0.0.1:4444 -S
> 
> which shows:
> 
>    (qemu) info snapshots
>    info snapshots
>    List of snapshots present on all disks:
>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
> 
> but do I need a whole separate overlay in the replay case? I thought
> supplying snapshot to the drive would prevent the replay case
> overwriting what has been recorded but with:
> 
>      -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>      -drive file=record.qcow2,if=none,id=rr,snapshot
> 
> but I get:
> 
>    (qemu) info snapshots
>    info snapshots
>    There is no snapshot available.
> 
> so if I drop the ,snapshot from the line I can at least see the snapshot
> but continue doesn't seem to work:
> 
>    (qemu) info snapshots
>    info snapshots
>    List of snapshots present on all disks:
>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>    (qemu) replay_break 190505
>    replay_break 190505
>    (qemu) c
>    c
>    (qemu) info replay
>    info replay
>    Replaying execution 'record.out': instruction count = 0
>    (qemu)
> 
> If I manually loadvm then we get somewhere but replay_seek breaks:
> 
>    (qemu) loadvm rrstart
>    loadvm rrstart
>    (qemu) info replay
>    info replay
>    Replaying execution 'record.out': instruction count = 190505
>    (qemu) replay_seek 190000
>    replay_seek 190000
>    snapshotting is disabled
> 
> with a crash:
> 
>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio -machine virt -kernel zephyr.elf -net none -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out
>   -drive file=record.qcow2,if=none,id=rr -monitor telnet:127.0.0.1:4444 -S

I missed that you forgot rrsnapshot in replay command line.
The execution was recorded with initial snapshot. Therefore it should be 
replayed with it too.


Pavel Dovgalyuk


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-08  9:13               ` Alex Bennée
@ 2020-09-08 10:57                 ` Pavel Dovgalyuk
  2020-09-08 11:10                 ` Alex Bennée
  1 sibling, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-08 10:57 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 08.09.2020 12:13, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> On 07.09.2020 19:25, Alex Bennée wrote:
>>>
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>
>>>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>>>
>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>
>>>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>>>
>>>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>>>
>>>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>>>
>>>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>>>> the execution to the specified instruction count.
>>>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>>>> to find the desired instruction count.
>>>>>>>
>>>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>>>> separate monitor channel:
>>>>>>
>>>>>> Right, you can't go to the prior state, when there is no preceding
>>>>>> snapshot available.
>>>>>
>>>>> It seems creating an initial snapshot automatically would be more user
>>>>
>>>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>>>> Reverse debugging is considered to be run with disk image (overlay)
>>>> and rrsnapshot option of icount, which allows creating an initial
>>>> VM snapshot.
>>>
>>> Given that I'm using the block device purely for VM snapshots I think it
>>> would be useful to document the minimal "no disk" approach - i.e. where
>>> the disk is only used for record/replay.
>>>
>>> However I'm still having trouble. I can record the trace with:
>>>
>>>     ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>>>       -machine virt -kernel zephyr.elf -net none \
>>>       -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>>>       -drive file=record.qcow2,if=none,id=rr \
>>>       -monitor telnet:127.0.0.1:4444 -S
>>
>> Can you provide your zephyr.elf image?
>>
>>>
>>> which shows:
>>>
>>>     (qemu) info snapshots
>>>     info snapshots
>>>     List of snapshots present on all disks:
>>>     ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>     --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>>
>>> but do I need a whole separate overlay in the replay case? I thought
>>> supplying snapshot to the drive would prevent the replay case
>>> overwriting what has been recorded but with:
>>>
>>>       -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>>>       -drive file=record.qcow2,if=none,id=rr,snapshot
>>
>> When you provide qcow2 (overlay or not) for snapshotting, you don't need
>> any 'snapshot' option on drive.
>>
>>>
>>> but I get:
>>>
>>>     (qemu) info snapshots
>>>     info snapshots
>>>     There is no snapshot available.
>>>
>>> so if I drop the ,snapshot from the line I can at least see the snapshot
>>> but continue doesn't seem to work:
>>>
>>>     (qemu) info snapshots
>>>     info snapshots
>>>     List of snapshots present on all disks:
>>>     ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>     --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>>     (qemu) replay_break 190505
>>>     replay_break 190505
>>>     (qemu) c
>>>     c
>>>     (qemu) info replay
>>>     info replay
>>>     Replaying execution 'record.out': instruction count = 0
>>
>> It seems, that replay hangs. Can you try removing '-S' in record command
>> line?
> 
> That doesn't make any difference removing from both the record and
> replay cases. It seems to need a loadvm to start things off.
> 
> I've sent you an image off list. Please let me know if you can replicate.
> 

With rrsnapshot in replay reverse debugging of your image seem to be ok:

(gdb) set arch aarch64
The target architecture is assumed to be aarch64
(gdb) tar rem :1234
Remote debugging using :1234
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0x00000000400003f8 in ?? ()
(gdb) monitor info replay
Replaying execution 'record.out': instruction count = 0
(gdb) monitor replay_break 100000
(gdb) c
Continuing.

Program received signal SIGINT, Interrupt.
0x0000000040001690 in ?? ()
(gdb) monitor info replay
Replaying execution 'record.out': instruction count = 100000
(gdb) rsi
0x0000000040001670 in ?? ()
(gdb) monitor info replay
Replaying execution 'record.out': instruction count = 99999
(gdb)


Pavel Dovgalyuk




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-08  9:13               ` Alex Bennée
  2020-09-08 10:57                 ` Pavel Dovgalyuk
@ 2020-09-08 11:10                 ` Alex Bennée
  2020-09-08 12:15                   ` Pavel Dovgalyuk
  1 sibling, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-08 11:10 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Alex Bennée <alex.bennee@linaro.org> writes:

> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>
>> On 07.09.2020 19:25, Alex Bennée wrote:
>>> 
>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>> 
>>>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>>>
>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>
>>>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>>>
>>>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>>>
>>>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>>>
>>>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>>>> the execution to the specified instruction count.
>>>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>>>> to find the desired instruction count.
>>>>>>>
>>>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>>>> separate monitor channel:
>>>>>>
>>>>>> Right, you can't go to the prior state, when there is no preceding
>>>>>> snapshot available.
>>>>>
>>>>> It seems creating an initial snapshot automatically would be more user
>>>>
>>>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>>>> Reverse debugging is considered to be run with disk image (overlay)
>>>> and rrsnapshot option of icount, which allows creating an initial
>>>> VM snapshot.
>>> 
>>> Given that I'm using the block device purely for VM snapshots I think it
>>> would be useful to document the minimal "no disk" approach - i.e. where
>>> the disk is only used for record/replay.
>>> 
>>> However I'm still having trouble. I can record the trace with:
>>> 
>>>    ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>>>      -machine virt -kernel zephyr.elf -net none \
>>>      -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>>>      -drive file=record.qcow2,if=none,id=rr \
>>>      -monitor telnet:127.0.0.1:4444 -S
>>
>> Can you provide your zephyr.elf image?
>>
>>> 
>>> which shows:
>>> 
>>>    (qemu) info snapshots
>>>    info snapshots
>>>    List of snapshots present on all disks:
>>>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>> 
>>> but do I need a whole separate overlay in the replay case? I thought
>>> supplying snapshot to the drive would prevent the replay case
>>> overwriting what has been recorded but with:
>>> 
>>>      -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>>>      -drive file=record.qcow2,if=none,id=rr,snapshot
>>
>> When you provide qcow2 (overlay or not) for snapshotting, you don't need 
>> any 'snapshot' option on drive.
>>
>>> 
>>> but I get:
>>> 
>>>    (qemu) info snapshots
>>>    info snapshots
>>>    There is no snapshot available.
>>> 
>>> so if I drop the ,snapshot from the line I can at least see the snapshot
>>> but continue doesn't seem to work:
>>> 
>>>    (qemu) info snapshots
>>>    info snapshots
>>>    List of snapshots present on all disks:
>>>    ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>    --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>>    (qemu) replay_break 190505
>>>    replay_break 190505
>>>    (qemu) c
>>>    c
>>>    (qemu) info replay
>>>    info replay
>>>    Replaying execution 'record.out': instruction count = 0
>>
>> It seems, that replay hangs. Can you try removing '-S' in record command 
>> line?
>
> That doesn't make any difference removing from both the record and
> replay cases. It seems to need a loadvm to start things off.
>
> I've sent you an image off list. Please let me know if you can
> replicate.

OK I can successfully use gdb to reverse debug the acceptance test (\o/)
so I suspect there are differences in the calling setup.

The first one is ensuring that rrsnapshot is set for both record and
replay. For this reason I think a more user friendly automatic snapshot
would be worth setting up when record/replay is being used.

-icount sleep=off definitely breaks things. Do we keep track of the
 icount bias as save and restore state?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 11/15] gdbstub: add reverse step support in replay mode
  2020-09-02  8:16 ` [PATCH v3 11/15] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
  2020-09-07 16:30   ` Alex Bennée
@ 2020-09-08 11:16   ` Alex Bennée
  1 sibling, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-08 11:16 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> GDB remote protocol supports two reverse debugging commands:
> reverse step and reverse continue.
> This patch adds support of the first one to the gdbstub.
> Reverse step is intended to step one instruction in the backwards
> direction. This is not possible in regular execution.
> But replayed execution is deterministic, therefore we can load one of
> the prior snapshots and proceed to the desired step. It is equivalent
> to stepping one instruction back.
> There should be at least one snapshot preceding the debugged part of
> the replay log.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  accel/tcg/translator.c    |    1 +
>  exec.c                    |    7 ++++++
>  gdbstub.c                 |   55 +++++++++++++++++++++++++++++++++++++++++++--
>  include/sysemu/replay.h   |   11 +++++++++
>  replay/replay-debugging.c |   33 +++++++++++++++++++++++++++
>  softmmu/cpus.c            |   14 +++++++++--
>  stubs/replay.c            |    5 ++++
>  7 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 603d17ff83..fb1e19c585 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -17,6 +17,7 @@
>  #include "exec/log.h"
>  #include "exec/translator.h"
>  #include "exec/plugin-gen.h"
> +#include "sysemu/replay.h"
>  
>  /* Pairs with tcg_clear_temp_count.
>     To be called by #TranslatorOps.{translate_insn,tb_stop} if
> diff --git a/exec.c b/exec.c
> index 7683afb6a8..47512e950c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2750,6 +2750,13 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
>      QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
>          if (watchpoint_address_matches(wp, addr, len)
>              && (wp->flags & flags)) {
> +            if (replay_running_debug()) {
> +                /*
> +                 * Don't process the watchpoints when we are
> +                 * in a reverse debugging operation.
> +                 */
> +                return;
> +            }
>              if (flags == BP_MEM_READ) {
>                  wp->flags |= BP_WATCHPOINT_HIT_READ;
>              } else {
> diff --git a/gdbstub.c b/gdbstub.c
> index 9dfb6e4142..79e8ccc050 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -51,6 +51,7 @@
>  #include "sysemu/runstate.h"
>  #include "hw/semihosting/semihost.h"
>  #include "exec/exec-all.h"
> +#include "sysemu/replay.h"
>  
>  #ifdef CONFIG_USER_ONLY
>  #define GDB_ATTACHED "0"
> @@ -375,6 +376,20 @@ typedef struct GDBState {
>   */
>  static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
>  
> +/* Retrieves flags for single step mode. */
> +static int get_sstep_flags(void)
> +{
> +    /*
> +     * In replay mode all events written into the log should be replayed.
> +     * That is why NOIRQ flag is removed in this mode.
> +     */
> +    if (replay_mode != REPLAY_MODE_NONE) {
> +        return SSTEP_ENABLE;
> +    } else {
> +        return sstep_flags;
> +    }
> +}
> +
>  static GDBState gdbserver_state;
>  
>  static void init_gdbserver_state(void)
> @@ -501,7 +516,7 @@ static int gdb_continue_partial(char *newstates)
>                  break; /* nothing to do here */
>              case 's':
>                  trace_gdbstub_op_stepping(cpu->cpu_index);
> -                cpu_single_step(cpu, sstep_flags);
> +                cpu_single_step(cpu, get_sstep_flags());
>                  cpu_resume(cpu);
>                  flag = 1;
>                  break;
> @@ -1874,10 +1889,31 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
>          gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
>      }
>  
> -    cpu_single_step(gdbserver_state.c_cpu, sstep_flags);
> +    cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags());
>      gdb_continue();
>  }
>  
> +static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (replay_mode != REPLAY_MODE_PLAY) {
> +        put_packet("E22");
> +    }
> +    if (gdb_ctx->num_params == 1) {
> +        switch (gdb_ctx->params[0].opcode) {
> +        case 's':
> +            if (replay_reverse_step()) {
> +                gdb_continue();
> +            } else {
> +                put_packet("E14");
> +            }
> +            return;
> +        }
> +    }
> +
> +    /* Default invalid command */
> +    put_packet("");
> +}
> +
>  static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
>  {
>      put_packet("vCont;c;C;s;S");
> @@ -2124,6 +2160,10 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
>          g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
>      }
>  
> +    if (replay_mode == REPLAY_MODE_PLAY) {
> +        g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
> +    }
> +
>      if (gdb_ctx->num_params &&
>          strstr(gdb_ctx->params[0].data, "multiprocess+")) {
>          gdbserver_state.multiprocess = true;
> @@ -2460,6 +2500,17 @@ static int gdb_handle_packet(const char *line_buf)
>              cmd_parser = &step_cmd_desc;
>          }
>          break;
> +    case 'b':
> +        {
> +            static const GdbCmdParseEntry backward_cmd_desc = {
> +                .handler = handle_backward,
> +                .cmd = "b",
> +                .cmd_startswith = 1,
> +                .schema = "o0"
> +            };
> +            cmd_parser = &backward_cmd_desc;
> +        }
> +        break;
>      case 'F':
>          {
>              static const GdbCmdParseEntry file_io_cmd_desc = {
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 239c01e7df..13a8123b09 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -75,6 +75,17 @@ void replay_finish(void);
>  void replay_add_blocker(Error *reason);
>  /* Returns name of the replay log file */
>  const char *replay_get_filename(void);
> +/*
> + * Start making one step in backward direction.
> + * Used by gdbstub for backwards debugging.
> + * Returns true on success.
> + */
> +bool replay_reverse_step(void);
> +/*
> + * Returns true if replay module is processing
> + * reverse_continue or reverse_step request
> + */
> +bool replay_running_debug(void);
>  
>  /* Processing the instructions */
>  
> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> index cfd0221692..aa3ca040e2 100644
> --- a/replay/replay-debugging.c
> +++ b/replay/replay-debugging.c
> @@ -22,6 +22,13 @@
>  #include "block/snapshot.h"
>  #include "migration/snapshot.h"
>  
> +static bool replay_is_debugging;
> +
> +bool replay_running_debug(void)
> +{
> +    return replay_is_debugging;
> +}
> +
>  void hmp_info_replay(Monitor *mon, const QDict *qdict)
>  {
>      if (replay_mode == REPLAY_MODE_NONE) {
> @@ -219,3 +226,29 @@ void hmp_replay_seek(Monitor *mon, const QDict *qdict)
>          return;
>      }
>  }
> +
> +static void replay_stop_vm_debug(void *opaque)
> +{
> +    replay_is_debugging = false;
> +    vm_stop(RUN_STATE_DEBUG);
> +    replay_delete_break();
> +}
> +
> +bool replay_reverse_step(void)
> +{
> +    Error *err = NULL;
> +
> +    assert(replay_mode == REPLAY_MODE_PLAY);
> +
> +    if (replay_get_current_icount() != 0) {
> +        replay_seek(replay_get_current_icount() - 1, replay_stop_vm_debug, &err);
> +        if (err) {
> +            error_free(err);
> +            return false;
> +        }
> +        replay_is_debugging = true;
> +        return true;
> +    }
> +
> +    return false;
> +}
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index a802e899ab..377fe3298c 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -1004,9 +1004,17 @@ static bool cpu_can_run(CPUState *cpu)
>  
>  static void cpu_handle_guest_debug(CPUState *cpu)
>  {
> -    gdb_set_stop_cpu(cpu);
> -    qemu_system_debug_request();
> -    cpu->stopped = true;
> +    if (!replay_running_debug()) {
> +        gdb_set_stop_cpu(cpu);
> +        qemu_system_debug_request();
> +        cpu->stopped = true;
> +    } else {
> +        if (!cpu->singlestep_enabled) {
> +            cpu_single_step(cpu, SSTEP_ENABLE);
> +        } else {
> +            cpu_single_step(cpu, 0);
> +        }
> +    }

I'd prefer to avoid this negative if cases as it scans poorly. Just do:

  if (replay_running_debug()) {
     /* replay stuff */
  } else {
     /* normal stuff */
  }

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt
  2020-09-02  8:17 ` [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
@ 2020-09-08 11:27   ` Alex Bennée
  2020-09-08 12:57     ` Pavel Dovgalyuk
  0 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2020-09-08 11:27 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> This patch updates the documentation and describes usage of the reverse
> debugging in QEMU+GDB.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  docs/replay.txt |   33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)

Minor aside - it is probably worth having a separate patch to convert
this file into an .rst and put it in the docs/system folder so we can
properly incorporate it into the user documentation. 

>
> diff --git a/docs/replay.txt b/docs/replay.txt
> index 70c27edb36..18d6169f3b 100644
> --- a/docs/replay.txt
> +++ b/docs/replay.txt
> @@ -294,6 +294,39 @@ for recording and replaying must contain identical number of ports in record
>  and replay modes, but their backends may differ.
>  E.g., '-serial stdio' in record mode, and '-serial null' in replay mode.
>  
> +Reverse debugging
> +-----------------
> +
> +Reverse debugging allows "executing" the program in reverse direction.
> +GDB remote protocol supports "reverse step" and "reverse continue"
> +commands. The first one steps single instruction backwards in time,
> +and the second one finds the last breakpoint in the past.
> +
> +Recorded executions may be used to enable reverse debugging. QEMU can't
> +execute the code in backwards direction, but can load a snapshot and
> +replay forward to find the desired position or breakpoint.
> +
> +The following GDB commands are supported:
> + - reverse-stepi (or rsi) - step one instruction backwards
> + - reverse-continue (or rc) - find last breakpoint in the past
> +
> +Reverse step loads the nearest snapshot and replays the execution until
> +the required instruction is met.
> +
> +Reverse continue may include several passes of examining the execution
> +between the snapshots. Each of the passes include the following steps:
> + 1. loading the snapshot
> + 2. replaying to examine the breakpoints
> + 3. if breakpoint or watchpoint was met
> +    - loading the snaphot again
> +    - replaying to the required breakpoint
> + 4. else
> +    - proceeding to the p.1 with the earlier snapshot
> +
> +Therefore usage of the reverse debugging requires at least one snapshot
> +created in advance. See the "Snapshotting" section to learn about running
> +record/replay and creating the snapshot in these modes.

I would explicitly state you need:

  - a block device for storing VM snapshots (independent of storage
    devices you may have)
  - to specify the starting rrsnapshot in both the record and replay runs

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 09/15] replay: implement replay-seek command
  2020-09-08 11:10                 ` Alex Bennée
@ 2020-09-08 12:15                   ` Pavel Dovgalyuk
  0 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-08 12:15 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 08.09.2020 14:10, Alex Bennée wrote:
> 
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>
>>> On 07.09.2020 19:25, Alex Bennée wrote:
>>>>
>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>
>>>>> On 07.09.2020 17:59, Alex Bennée wrote:
>>>>>>
>>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>>
>>>>>>> On 07.09.2020 15:58, Alex Bennée wrote:
>>>>>>>>
>>>>>>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>>>>>>>>
>>>>>>>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>>>>
>>>>>>>>> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
>>>>>>>>> the execution to the specified instruction count.
>>>>>>>>> The command automatically loads nearest snapshot and replays the execution
>>>>>>>>> to find the desired instruction count.
>>>>>>>>
>>>>>>>> Should there be an initial snapshot created at instruction 0? Using a
>>>>>>>> separate monitor channel:
>>>>>>>
>>>>>>> Right, you can't go to the prior state, when there is no preceding
>>>>>>> snapshot available.
>>>>>>
>>>>>> It seems creating an initial snapshot automatically would be more user
>>>>>
>>>>> Please take a look at 'Snapshotting' section of docs/replay.txt.
>>>>> Reverse debugging is considered to be run with disk image (overlay)
>>>>> and rrsnapshot option of icount, which allows creating an initial
>>>>> VM snapshot.
>>>>
>>>> Given that I'm using the block device purely for VM snapshots I think it
>>>> would be useful to document the minimal "no disk" approach - i.e. where
>>>> the disk is only used for record/replay.
>>>>
>>>> However I'm still having trouble. I can record the trace with:
>>>>
>>>>     ./qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
>>>>       -machine virt -kernel zephyr.elf -net none \
>>>>       -icount shift=6,align=off,sleep=off,rr=record,rrfile=record.out,rrsnapshot=rrstart  \
>>>>       -drive file=record.qcow2,if=none,id=rr \
>>>>       -monitor telnet:127.0.0.1:4444 -S
>>>
>>> Can you provide your zephyr.elf image?
>>>
>>>>
>>>> which shows:
>>>>
>>>>     (qemu) info snapshots
>>>>     info snapshots
>>>>     List of snapshots present on all disks:
>>>>     ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>>     --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>>>
>>>> but do I need a whole separate overlay in the replay case? I thought
>>>> supplying snapshot to the drive would prevent the replay case
>>>> overwriting what has been recorded but with:
>>>>
>>>>       -icount shift=6,align=off,sleep=off,rr=replay,rrfile=record.out \
>>>>       -drive file=record.qcow2,if=none,id=rr,snapshot
>>>
>>> When you provide qcow2 (overlay or not) for snapshotting, you don't need
>>> any 'snapshot' option on drive.
>>>
>>>>
>>>> but I get:
>>>>
>>>>     (qemu) info snapshots
>>>>     info snapshots
>>>>     There is no snapshot available.
>>>>
>>>> so if I drop the ,snapshot from the line I can at least see the snapshot
>>>> but continue doesn't seem to work:
>>>>
>>>>     (qemu) info snapshots
>>>>     info snapshots
>>>>     List of snapshots present on all disks:
>>>>     ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
>>>>     --        rrstart           653 KiB 2020-09-07 17:12:42 00:00:00.000          0
>>>>     (qemu) replay_break 190505
>>>>     replay_break 190505
>>>>     (qemu) c
>>>>     c
>>>>     (qemu) info replay
>>>>     info replay
>>>>     Replaying execution 'record.out': instruction count = 0
>>>
>>> It seems, that replay hangs. Can you try removing '-S' in record command
>>> line?
>>
>> That doesn't make any difference removing from both the record and
>> replay cases. It seems to need a loadvm to start things off.
>>
>> I've sent you an image off list. Please let me know if you can
>> replicate.
> 
> OK I can successfully use gdb to reverse debug the acceptance test (\o/)
> so I suspect there are differences in the calling setup.
> 
> The first one is ensuring that rrsnapshot is set for both record and
> replay. For this reason I think a more user friendly automatic snapshot
> would be worth setting up when record/replay is being used.
> 
> -icount sleep=off definitely breaks things. Do we keep track of the

It was ok for me:
qemu-system-aarch64 -cpu cortex-a53 -display none -serial stdio \
  -machine virt -kernel zephyr-64.elf -net none \
  -icount 
shift=6,align=off,sleep=off,rr=replay,rrfile=record.out,rrsnapshot=rrstart 
  \
  -drive file=record.qcow2,if=none,id=rr -s -S

>   icount bias as save and restore state?
> 

I don't know anything about sleep, but qemu_icount_bias is saved in 
vmstate when icount is enabled.

However, I noticed a strange condition at cpus.c:855
Shouldn't we check !sleep here instead of !icount_sleep?

     } else if (!icount_sleep) {
         error_setg(errp, "shift=auto and sleep=off are incompatible");
         return;
     }

     icount_sleep = sleep;




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt
  2020-09-08 11:27   ` Alex Bennée
@ 2020-09-08 12:57     ` Pavel Dovgalyuk
  0 siblings, 0 replies; 43+ messages in thread
From: Pavel Dovgalyuk @ 2020-09-08 12:57 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth

On 08.09.2020 14:27, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>
>> This patch updates the documentation and describes usage of the reverse
>> debugging in QEMU+GDB.
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>> ---
>>   docs/replay.txt |   33 +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
> 
> Minor aside - it is probably worth having a separate patch to convert
> this file into an .rst and put it in the docs/system folder so we can
> properly incorporate it into the user documentation.

Ok, I renamed and moved replay.txt, but havent't found how to build the 
docs.
Can you give any hints?


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 15/15] tests/acceptance: add reverse debugging test
  2020-09-02  8:17 ` [PATCH v3 15/15] tests/acceptance: add reverse debugging test Pavel Dovgalyuk
@ 2020-09-08 13:01   ` Alex Bennée
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2020-09-08 13:01 UTC (permalink / raw)
  To: Pavel Dovgalyuk
  Cc: kwolf, wrampazz, ehabkost, mtosatti, qemu-devel, armbru,
	stefanha, crosa, pbonzini, mreitz, philmd, zhiwei_liu, rth


Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
>
> This is a test for GDB reverse debugging commands: reverse step and reverse continue.
> Every test in this suite consists of two phases: record and replay.
> Recording saves the execution of some instructions and makes an initial
> VM snapshot to allow reverse execution.
> Replay saves the order of the first instructions and then checks that they
> are executed backwards in the correct order.
> After that the execution is replayed to the end, and reverse continue
> command is checked by setting several breakpoints, and asserting
> that the execution is stopped at the last of them.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  MAINTAINERS                           |    1 
>  tests/acceptance/reverse_debugging.py |  203 +++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+)
>  create mode 100644 tests/acceptance/reverse_debugging.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e49af700c9..76450f1bdf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2644,6 +2644,7 @@ F: include/sysemu/replay.h
>  F: docs/replay.txt
>  F: stubs/replay.c
>  F: tests/acceptance/replay_kernel.py
> +F: tests/acceptance/reverse_debugging.py
>  F: qapi/replay.json
>  
>  IOVA Tree
> diff --git a/tests/acceptance/reverse_debugging.py b/tests/acceptance/reverse_debugging.py
> new file mode 100644
> index 0000000000..dda42e1c1a
> --- /dev/null
> +++ b/tests/acceptance/reverse_debugging.py
> @@ -0,0 +1,203 @@
> +# Reverse debugging test
> +#
> +# Copyright (c) 2020 ISP RAS
> +#
> +# Author:
> +#  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +import os
> +import logging
> +
> +from avocado_qemu import BUILD_DIR
> +from avocado.utils import gdb
> +from avocado.utils import process
> +from avocado.utils.path import find_command
> +from boot_linux_console import LinuxKernelTest
> +
> +class ReverseDebugging(LinuxKernelTest):
> +    """
> +    Test GDB reverse debugging commands: reverse step and reverse continue.
> +    Recording saves the execution of some instructions and makes an initial
> +    VM snapshot to allow reverse execution.
> +    Replay saves the order of the first instructions and then checks that they
> +    are executed backwards in the correct order.
> +    After that the execution is replayed to the end, and reverse continue
> +    command is checked by setting several breakpoints, and asserting
> +    that the execution is stopped at the last of them.
> +    """
> +
> +    timeout = 10
> +    STEPS = 10
> +    endian_is_le = True
> +
> +    def run_vm(self, record, shift, args, replay_path, image_path):
> +        logger = logging.getLogger('replay')
> +        vm = self.get_vm()
> +        vm.set_console()
> +        if record:
> +            logger.info('recording the execution...')
> +            mode = 'record'
> +        else:
> +            logger.info('replaying the execution...')
> +            mode = 'replay'
> +            vm.add_args('-s', '-S')
> +        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
> +                    (shift, mode, replay_path),
> +                    '-net', 'none')
> +        vm.add_args('-drive', 'file=%s,if=none' % image_path)
> +        if args:
> +            vm.add_args(*args)
> +        vm.launch()
> +        return vm
> +
> +    @staticmethod
> +    def get_reg_le(g, reg):
> +        res = g.cmd(b'p%x' % reg)
> +        num = 0
> +        for i in range(len(res))[-2::-2]:
> +            num = 0x100 * num + int(res[i:i + 2], 16)
> +        return num
> +
> +    @staticmethod
> +    def get_reg_be(g, reg):
> +        res = g.cmd(b'p%x' % reg)
> +        return int(res, 16)
> +
> +    def get_reg(self, g, reg):
> +        # value may be encoded in BE or LE order
> +        if self.endian_is_le:
> +            return self.get_reg_le(g, reg)
> +        else:
> +            return self.get_reg_be(g, reg)

These seem a little hacky. Can't we issue normal gdb commands. In the
SVE tests I use:

    frame = gdb.selected_frame()
    for i in range(0, 32):
        rname = "z%d" % (i)
        zreg = frame.read_register(rname)

which works with the symbolic name and doesn't need endian tricks to
sort it out.

> +
> +    def get_pc(self, g):
> +        return self.get_reg(g, self.REG_PC)
> +
> +    def check_pc(self, g, addr):
> +        pc = self.get_pc(g)
> +        if pc != addr:
> +            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
> +
> +    @staticmethod
> +    def gdb_step(g):
> +        g.cmd(b's', b'T05thread:01;')
> +
> +    @staticmethod
> +    def gdb_bstep(g):
> +        g.cmd(b'bs', b'T05thread:01;')

Hmm so these are packet commands? Can't we access via the python API? Clebber?

> +
> +    @staticmethod
> +    def vm_get_icount(vm):
> +        return vm.qmp('query-replay')['return']['icount']
> +
> +    def reverse_debugging(self, shift=7, args=None):
> +        logger = logging.getLogger('replay')
> +
> +        # create qcow2 for snapshots
> +        logger.info('creating qcow2 image for VM snapshots')
> +        image_path = os.path.join(self.workdir, 'disk.qcow2')
> +        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
> +        if not os.path.exists(qemu_img):
> +            qemu_img = find_command('qemu-img', False)
> +        if qemu_img is False:
> +            self.cancel('Could not find "qemu-img", which is required to '
> +                        'create the temporary qcow2 image')
> +        cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
> +        process.run(cmd)
> +
> +        replay_path = os.path.join(self.workdir, 'replay.bin')
> +
> +        # record the log
> +        vm = self.run_vm(True, shift, args, replay_path, image_path)
> +        while self.vm_get_icount(vm) <= self.STEPS:
> +            pass
> +        last_icount = self.vm_get_icount(vm)
> +        vm.shutdown()
> +
> +        logger.info("recorded log with %s+ steps" % last_icount)
> +
> +        # replay and run debug commands
> +        vm = self.run_vm(False, shift, args, replay_path, image_path)
> +        logger.info('connecting to gdbstub')
> +        g = gdb.GDBRemote('127.0.0.1', 1234, False, False)
> +        g.connect()
> +        r = g.cmd(b'qSupported')
> +        if b'qXfer:features:read+' in r:
> +            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
> +        if b'ReverseStep+' not in r:
> +            self.fail('Reverse step is not supported by QEMU')
> +        if b'ReverseContinue+' not in r:
> +            self.fail('Reverse continue is not supported by QEMU')
> +
> +        logger.info('stepping forward')
> +        steps = []
> +        # record first instruction addresses
> +        for _ in range(self.STEPS):
> +            pc = self.get_pc(g)
> +            logger.info('saving position %x' % pc)
> +            steps.append(pc)
> +            self.gdb_step(g)
> +
> +        # visit the recorded instruction in reverse order
> +        logger.info('stepping backward')
> +        for addr in steps[::-1]:
> +            self.gdb_bstep(g)
> +            self.check_pc(g, addr)
> +            logger.info('found position %x' % addr)
> +
> +        logger.info('seeking to the end (icount %s)' % (last_icount - 1))
> +        vm.qmp('replay-break', icount=last_icount - 1)
> +        # continue - will return after pausing
> +        g.cmd(b'c', b'T02thread:01;')
> +
> +        logger.info('setting breakpoints')
> +        for addr in steps:
> +            # hardware breakpoint at addr with len=1
> +            g.cmd(b'Z1,%x,1' % addr, b'OK')
> +
> +        logger.info('running reverse continue to reach %x' % steps[-1])
> +        # reverse continue - will return after stopping at the breakpoint
> +        g.cmd(b'bc', b'T05thread:01;')
> +
> +        # assume that none of the first instructions is executed again
> +        # breaking the order of the breakpoints
> +        self.check_pc(g, steps[-1])
> +        logger.info('successfully reached %x' % steps[-1])
> +
> +        logger.info('exitting gdb and qemu')
> +        vm.shutdown()
> +
> +class ReverseDebugging_X86_64(ReverseDebugging):
> +    REG_PC = 0x10
> +    REG_CS = 0x12
> +    def get_pc(self, g):
> +        return self.get_reg_le(g, self.REG_PC) \
> +            + self.get_reg_le(g, self.REG_CS) * 0x10
> +
> +    def test_x86_64_pc(self):
> +        """
> +        :avocado: tags=arch:x86_64
> +        :avocado: tags=machine:pc
> +        """
> +        # start with BIOS only
> +        self.reverse_debugging()
> +
> +class ReverseDebugging_AArch64(ReverseDebugging):
> +    REG_PC = 32
> +
> +    def test_aarch64_virt(self):
> +        """
> +        :avocado: tags=arch:aarch64
> +        :avocado: tags=machine:virt
> +        :avocado: tags=cpu:cortex-a53
> +        """
> +        kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
> +                      '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> +                      '/vmlinuz')
> +        kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> +
> +        self.reverse_debugging(
> +            args=('-kernel', kernel_path, '-cpu', 'cortex-a53'))

Quibbles aside it's excellent to have a test that exercises the
functionality.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 05/15] iotests: update snapshot test for new output format
  2020-09-02  8:16 ` [PATCH v3 05/15] iotests: update snapshot test for new output format Pavel Dovgalyuk
  2020-09-07 15:26   ` Alex Bennée
@ 2020-09-08 13:10   ` Eric Blake
  1 sibling, 0 replies; 43+ messages in thread
From: Eric Blake @ 2020-09-08 13:10 UTC (permalink / raw)
  To: Pavel Dovgalyuk, qemu-devel
  Cc: kwolf, ehabkost, philmd, mtosatti, stefanha, armbru, mreitz,
	wrampazz, crosa, pbonzini, alex.bennee, zhiwei_liu, rth

On 9/2/20 3:16 AM, Pavel Dovgalyuk wrote:
> From: Pavel Dovgalyuk <pavel.dovgaluk@gmail.com>
> 
> This patch updates iotests that verify qemu monitor output.
> New output format for snapshot listing include ICOUNT column.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>   tests/qemu-iotests/267.out |   48 ++++++++++++++++++++++----------------------
>   1 file changed, 24 insertions(+), 24 deletions(-)

This should be squashed with the patch that altered the format, to 
reduce the chance of git bisect landing on a patch where iotests have a 
known failure.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2020-09-08 13:12 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02  8:15 [PATCH v3 00/15] Reverse debugging Pavel Dovgalyuk
2020-09-02  8:15 ` [PATCH v3 01/15] replay: don't record interrupt poll Pavel Dovgalyuk
2020-09-07 10:17   ` Alex Bennée
2020-09-02  8:15 ` [PATCH v3 02/15] replay: provide an accessor for rr filename Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 03/15] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 04/15] migration: " Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 05/15] iotests: update snapshot test for new output format Pavel Dovgalyuk
2020-09-07 15:26   ` Alex Bennée
2020-09-07 15:41     ` Pavel Dovgalyuk
2020-09-07 16:00       ` Alex Bennée
2020-09-07 16:05         ` Pavel Dovgalyuk
2020-09-08 13:10   ` Eric Blake
2020-09-02  8:16 ` [PATCH v3 06/15] qapi: introduce replay.json for record/replay-related stuff Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 07/15] replay: introduce info hmp/qmp command Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 08/15] replay: introduce breakpoint at the specified step Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 09/15] replay: implement replay-seek command Pavel Dovgalyuk
2020-09-07 12:45   ` Alex Bennée
2020-09-07 13:32     ` Pavel Dovgalyuk
2020-09-07 12:58   ` Alex Bennée
2020-09-07 13:27     ` Pavel Dovgalyuk
2020-09-07 14:59       ` Alex Bennée
2020-09-07 15:46         ` Pavel Dovgalyuk
2020-09-07 16:25           ` Alex Bennée
2020-09-08  7:44             ` Pavel Dovgalyuk
2020-09-08  9:13               ` Alex Bennée
2020-09-08 10:57                 ` Pavel Dovgalyuk
2020-09-08 11:10                 ` Alex Bennée
2020-09-08 12:15                   ` Pavel Dovgalyuk
2020-09-08 10:54             ` Pavel Dovgalyuk
2020-09-02  8:16 ` [PATCH v3 10/15] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
2020-09-07 13:37   ` Alex Bennée
2020-09-02  8:16 ` [PATCH v3 11/15] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
2020-09-07 16:30   ` Alex Bennée
2020-09-08 11:16   ` Alex Bennée
2020-09-02  8:16 ` [PATCH v3 12/15] gdbstub: add reverse continue " Pavel Dovgalyuk
2020-09-02  8:17 ` [PATCH v3 13/15] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
2020-09-08 11:27   ` Alex Bennée
2020-09-08 12:57     ` Pavel Dovgalyuk
2020-09-02  8:17 ` [PATCH v3 14/15] tests: bump avocado version Pavel Dovgalyuk
2020-09-02 17:02   ` Willian Rampazzo
2020-09-04 21:39   ` Cleber Rosa
2020-09-02  8:17 ` [PATCH v3 15/15] tests/acceptance: add reverse debugging test Pavel Dovgalyuk
2020-09-08 13:01   ` Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.