* [for-5.0 PATCH 01/11] replay: provide an accessor for rr filename
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
@ 2019-12-23 9:46 ` Pavel Dovgalyuk
2019-12-23 9:46 ` [for-5.0 PATCH 02/11] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
` (11 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
---
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 706c7b4f4b..9ec6ce9996 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -394,3 +394,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] 34+ messages in thread* [for-5.0 PATCH 02/11] qcow2: introduce icount field for snapshots
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
2019-12-23 9:46 ` [for-5.0 PATCH 01/11] replay: provide an accessor for rr filename Pavel Dovgalyuk
@ 2019-12-23 9:46 ` Pavel Dovgalyuk
2020-01-09 11:48 ` Kevin Wolf
2019-12-23 9:47 ` [for-5.0 PATCH 03/11] migration: " Pavel Dovgalyuk
` (10 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
Acked-by: Kevin Wolf <kwolf@redhat.com>
--
v2:
- documented format changes in docs/interop/qcow2.txt
(suggested by Eric Blake)
v10:
- updated the documentation
---
block/qcow2-snapshot.c | 7 +++++++
block/qcow2.h | 3 +++
docs/interop/qcow2.txt | 4 ++++
3 files changed, 14 insertions(+)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 5ab64da1ec..b04b3e1634 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -163,6 +163,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;
@@ -332,6 +338,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 0942126232..f3e0d9aa56 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -171,6 +171,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;
@@ -184,6 +185,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 af5711e533..aa9d447cda 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -584,6 +584,10 @@ 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
+
Version 3 images must include extra data at least up to
byte 55.
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [for-5.0 PATCH 02/11] qcow2: introduce icount field for snapshots
2019-12-23 9:46 ` [for-5.0 PATCH 02/11] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
@ 2020-01-09 11:48 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2020-01-09 11:48 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: peter.maydell, pavel.dovgaluk, pbonzini, quintela, ciro.santilli,
jasowang, crosthwaite.peter, qemu-devel, armbru, alex.bennee,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
dovgaluk, mreitz, artem.k.pisarenko, dgilbert, rth
Am 23.12.2019 um 10:46 hat Pavel Dovgalyuk geschrieben:
> 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.Dovgaluk@ispras.ru>
> Acked-by: Kevin Wolf <kwolf@redhat.com>
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index af5711e533..aa9d447cda 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -584,6 +584,10 @@ 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
> +
In the context of the next patch, I noticed that we should probably also
mention that -1ULL means "invalid".
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
2019-12-23 9:46 ` [for-5.0 PATCH 01/11] replay: provide an accessor for rr filename Pavel Dovgalyuk
2019-12-23 9:46 ` [for-5.0 PATCH 02/11] qcow2: introduce icount field for snapshots Pavel Dovgalyuk
@ 2019-12-23 9:47 ` Pavel Dovgalyuk
2020-01-09 11:59 ` Kevin Wolf
2019-12-23 9:47 ` [for-5.0 PATCH 04/11] qapi: introduce replay.json for record/replay-related stuff Pavel Dovgalyuk
` (9 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
--
v2:
- made icount in SnapshotInfo optional (suggested by Eric Blake)
v7:
- added more comments for icount member (suggested by Markus Armbruster)
v9:
- updated icount comment
v10:
- updated icount comment again
---
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 | 7 ++++++-
qapi/block.json | 3 ++-
7 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 9a5d0c9b27..110ac253ab 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -219,6 +219,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;
@@ -651,14 +653,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%7s%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);
@@ -672,11 +675,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%7s%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);
}
@@ -838,6 +846,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 b04b3e1634..2c003514ef 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -662,6 +662,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. */
@@ -995,6 +996,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 8e029e9c01..6383a64ddd 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"
@@ -1242,6 +1243,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;
@@ -1449,6 +1454,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 a71b930b91..ae84bf6ab0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2681,6 +2681,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 0cf68fea14..db3e435c74 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -26,13 +26,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.0)
+#
# 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:
diff --git a/qapi/block.json b/qapi/block.json
index 145c268bb6..f389bb6f1a 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -176,7 +176,8 @@
# "date-sec": 1000012,
# "date-nsec": 10,
# "vm-clock-sec": 100,
-# "vm-clock-nsec": 20
+# "vm-clock-nsec": 20,
+# "icount": 220414
# }
# }
#
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
2019-12-23 9:47 ` [for-5.0 PATCH 03/11] migration: " Pavel Dovgalyuk
@ 2020-01-09 11:59 ` Kevin Wolf
2020-01-13 17:11 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2020-01-09 11:59 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: peter.maydell, pavel.dovgaluk, pbonzini, quintela, ciro.santilli,
jasowang, crosthwaite.peter, qemu-devel, armbru, alex.bennee,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
dovgaluk, mreitz, artem.k.pisarenko, dgilbert, rth
Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
> 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.Dovgaluk@ispras.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
2020-01-09 11:59 ` Kevin Wolf
@ 2020-01-13 17:11 ` Alex Bennée
2020-01-14 9:32 ` Pavel Dovgalyuk
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2020-01-13 17:11 UTC (permalink / raw)
To: Kevin Wolf
Cc: peter.maydell, pavel.dovgaluk, pbonzini, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
Pavel Dovgalyuk, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, mreitz, artem.k.pisarenko, dgilbert,
rth
Kevin Wolf <kwolf@redhat.com> writes:
> Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
>> 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.Dovgaluk@ispras.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> Acked-by: Kevin Wolf <kwolf@redhat.com>
Apologies my mailer ignored my replay-all:
This commit breaks when of the iotests for me:
git bisect run /bin/sh -c "cd builds/all && make -j4 \
&& cd tests/qemu-iotests && ./check -qcow2 267"
Gives:
make[1]: Entering directory '/home/alex.bennee/lsrc/qemu.git/slirp'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/slirp'
QEMU -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64" -nodefaults -display none -accel qtest
QEMU_IMG -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-img"
QEMU_IO -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-io" --cache writeback -f qcow2
QEMU_NBD -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-nbd"
IMGFMT -- qcow2 (compat=1.1)
IMGPROTO -- file
PLATFORM -- Linux/x86_64 hackbox2 4.15.0-66-generic
TEST_DIR -- /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/scratch
SOCK_DIR -- /tmp/tmp.NV0n5HqCUs
SOCKET_SCM_HELPER -- /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/socket_scm_helper
267 fail [12:17:36] [12:17:38] (last: 1s) output mismatch (see 267.out.bad)
--- /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/267.out 2019-10-31 10:46:30.559805129 +0000
+++ /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/267.out.bad 2020-01-13 12:17:38.096181947 +0000
@@ -33,7 +33,7 @@
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK
+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,7 +44,7 @@
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK
+ID TAG VM SIZE DATE VM CLOCK ICOUNT
But I've also seen:
ERROR:/home/.../qemu.git/replay/replay-events.c:80:replay_flush_events:
assertion failed: (replay_mutex_locked())
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
2020-01-13 17:11 ` Alex Bennée
@ 2020-01-14 9:32 ` Pavel Dovgalyuk
0 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-14 9:32 UTC (permalink / raw)
To: 'Alex Bennée', 'Kevin Wolf'
Cc: peter.maydell, pavel.dovgaluk, crosthwaite.peter, ciro.santilli,
jasowang, quintela, qemu-devel, armbru, 'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
> >> 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.Dovgaluk@ispras.ru>
> >> Acked-by: Markus Armbruster <armbru@redhat.com>
> >
> > Acked-by: Kevin Wolf <kwolf@redhat.com>
>
> Apologies my mailer ignored my replay-all:
>
> This commit breaks when of the iotests for me:
>
> git bisect run /bin/sh -c "cd builds/all && make -j4 \
> && cd tests/qemu-iotests && ./check -qcow2 267"
>
>
> List of snapshots present on all disks:
> -ID TAG VM SIZE DATE VM CLOCK
> +ID TAG VM SIZE DATE VM CLOCK ICOUNT
>
> But I've also seen:
>
> ERROR:/home/.../qemu.git/replay/replay-events.c:80:replay_flush_events:
> assertion failed: (replay_mutex_locked())
Thank you, I've updated the code.
I also added a patch for fixing the test output.
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread
* [for-5.0 PATCH 04/11] qapi: introduce replay.json for record/replay-related stuff
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (2 preceding siblings ...)
2019-12-23 9:47 ` [for-5.0 PATCH 03/11] migration: " Pavel Dovgalyuk
@ 2019-12-23 9:47 ` Pavel Dovgalyuk
2019-12-23 9:47 ` [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command Pavel Dovgalyuk
` (8 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.dovgaluk@ispras.ru>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
--
v10:
- minor changes
v13:
- rebased to the new QAPI files
---
MAINTAINERS | 1 +
include/sysemu/replay.h | 1 +
qapi/Makefile.objs | 2 +-
qapi/misc.json | 18 ------------------
qapi/qapi-schema.json | 1 +
qapi/replay.json | 26 ++++++++++++++++++++++++++
6 files changed, 30 insertions(+), 19 deletions(-)
create mode 100644 qapi/replay.json
diff --git a/MAINTAINERS b/MAINTAINERS
index 387879aebc..7ad3001b0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2311,6 +2311,7 @@ F: net/filter-replay.c
F: include/sysemu/replay.h
F: docs/replay.txt
F: stubs/replay.c
+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/Makefile.objs b/qapi/Makefile.objs
index dd3f5e6f94..4e84247d0c 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -7,7 +7,7 @@ util-obj-y += qapi-util.o
QAPI_COMMON_MODULES = audio authz block-core block char common crypto
QAPI_COMMON_MODULES += dump error introspect job machine migration misc net
-QAPI_COMMON_MODULES += qdev qom rdma rocker run-state sockets tpm
+QAPI_COMMON_MODULES += qdev qom rdma replay rocker run-state sockets tpm
QAPI_COMMON_MODULES += trace transaction ui
QAPI_TARGET_MODULES = machine-target misc-target
QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)
diff --git a/qapi/misc.json b/qapi/misc.json
index 33b94e3589..76a5f86e7f 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1694,24 +1694,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 9751b11f8f..62f425410c 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -103,6 +103,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] 34+ messages in thread* [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (3 preceding siblings ...)
2019-12-23 9:47 ` [for-5.0 PATCH 04/11] qapi: introduce replay.json for record/replay-related stuff Pavel Dovgalyuk
@ 2019-12-23 9:47 ` Pavel Dovgalyuk
2020-01-09 12:23 ` Alex Bennée
2019-12-23 9:47 ` [for-5.0 PATCH 06/11] replay: introduce breakpoint at the specified step Pavel Dovgalyuk
` (7 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
--
v2:
- renamed info_replay qmp into query-replay (suggested by Eric Blake)
v7:
- added empty line (suggested by Markus Armbruster)
v9:
- changed 'step' parameter name to 'icount'
- moved json stuff to replay.json and updated the descriptions
(suggested by Markus Armbruster)
v10:
- updated descriptions and messages for rr stuff
---
hmp-commands-info.hx | 14 ++++++++++++++
include/monitor/hmp.h | 1 +
qapi/block-core.json | 3 ++-
qapi/replay.json | 39 +++++++++++++++++++++++++++++++++++++++
replay/Makefile.objs | 1 +
replay/replay-debugging.c | 43 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 replay/replay-debugging.c
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 257ee7d7a3..0288860db0 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -930,6 +930,20 @@ STEXI
@item info sev
@findex info sev
Show SEV information.
+ETEXI
+
+ {
+ .name = "replay",
+ .args_type = "",
+ .params = "",
+ .help = "show record/replay information",
+ .cmd = hmp_info_replay,
+ },
+
+STEXI
+@item info replay
+@findex info replay
+Display the record/replay information: mode and the current icount.
ETEXI
STEXI
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 3d329853b2..783784cf10 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -153,5 +153,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 db3e435c74..1b665b1ad4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -28,7 +28,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.0)
+# in the recorded execution with the snapshots. This counter may
+# be obtained through @query-replay command (since 5.0)
#
# Since: 1.3
#
diff --git a/qapi/replay.json b/qapi/replay.json
index 9e13551d20..67f2d1f859 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.0
+#
+##
+{ '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.0
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "icount": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+ 'returns': 'ReplayInfo' }
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index 939be964a9..f847c5c023 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -8,3 +8,4 @@ common-obj-y += replay-snapshot.o
common-obj-y += replay-net.o
common-obj-y += replay-audio.o
common-obj-y += replay-random.o
+common-obj-y += replay-debugging.o
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
new file mode 100644
index 0000000000..8cf15ebc11
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,43 @@
+/*
+ * replay-debugging.c
+ *
+ * Copyright (c) 2010-2018 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] 34+ messages in thread* Re: [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command
2019-12-23 9:47 ` [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command Pavel Dovgalyuk
@ 2020-01-09 12:23 ` Alex Bennée
2020-01-09 12:27 ` Pavel Dovgalyuk
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2020-01-09 12:23 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini,
crosthwaite.peter, ciro.santilli, jasowang, quintela, qemu-devel,
armbru, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, mreitz, artem.k.pisarenko, dgilbert,
rth
Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
> 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.Dovgaluk@ispras.ru>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Acked-by: Markus Armbruster <armbru@redhat.com>
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index 939be964a9..f847c5c023 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -8,3 +8,4 @@ common-obj-y += replay-snapshot.o
common-obj-y += replay-net.o
common-obj-y += replay-audio.o
common-obj-y += replay-random.o
+common-obj-y += replay-debugging.o
this has a minor merge conflict. I seem to be missing replay-random.
--
Alex Bennée
^ permalink raw reply related [flat|nested] 34+ messages in thread
* RE: [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command
2020-01-09 12:23 ` Alex Bennée
@ 2020-01-09 12:27 ` Pavel Dovgalyuk
0 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-09 12:27 UTC (permalink / raw)
To: 'Alex Bennée'
Cc: kwolf, peter.maydell, pavel.dovgaluk, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
>
> > 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.Dovgaluk@ispras.ru>
> > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Acked-by: Markus Armbruster <armbru@redhat.com>
>
> diff --git a/replay/Makefile.objs b/replay/Makefile.objs
> index 939be964a9..f847c5c023 100644
> --- a/replay/Makefile.objs
> +++ b/replay/Makefile.objs
> @@ -8,3 +8,4 @@ common-obj-y += replay-snapshot.o
> common-obj-y += replay-net.o
> common-obj-y += replay-audio.o
> common-obj-y += replay-random.o
> +common-obj-y += replay-debugging.o
>
> this has a minor merge conflict. I seem to be missing replay-random.
Sorry, forgot about this. random-related patch was pulled yesterday.
It is not meaningful for trying reverse debugging.
All patches are committed into this branch, if you want to try: https://github.com/ispras/qemu/tree/rr-191223
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread
* [for-5.0 PATCH 06/11] replay: introduce breakpoint at the specified step
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (4 preceding siblings ...)
2019-12-23 9:47 ` [for-5.0 PATCH 05/11] replay: introduce info hmp/qmp command Pavel Dovgalyuk
@ 2019-12-23 9:47 ` Pavel Dovgalyuk
2019-12-23 9:47 ` [for-5.0 PATCH 07/11] replay: implement replay-seek command Pavel Dovgalyuk
` (6 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
--
v2:
- renamed replay_break qmp command into replay-break
(suggested by Eric Blake)
v7:
- introduces replay_delete_break command
v9:
- changed 'step' parameter name to 'icount'
- moved json stuff to replay.json and updated the description
(suggested by Markus Armbruster)
v10:
- updated descriptions (suggested by Markus Armbruster)
v11:
- fixed replay_break rearm bug
---
hmp-commands.hx | 34 ++++++++++++++++++
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, 179 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index cfcc044ce4..3704294da8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1930,6 +1930,40 @@ ETEXI
STEXI
@item qom-set @var{path} @var{property} @var{value}
Set QOM property @var{property} of object at location @var{path} to value @var{value}
+ETEXI
+
+ {
+ .name = "replay_break",
+ .args_type = "icount:i",
+ .params = "icount",
+ .help = "set breakpoint at the specified instruction count",
+ .cmd = hmp_replay_break,
+ },
+
+STEXI
+@item replay_break @var{icount}
+@findex replay_break
+Set replay breakpoint at instruction count @var{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'.
+ETEXI
+
+ {
+ .name = "replay_delete_break",
+ .args_type = "",
+ .params = "",
+ .help = "removes replay breakpoint",
+ .cmd = hmp_replay_delete_break,
+ },
+
+STEXI
+@item replay_delete_break
+@findex replay_delete_break
+Remove replay breakpoint which was previously set with replay_break.
+The command is ignored when there are no replay breakpoints.
ETEXI
{
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 783784cf10..0fd477aa13 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -154,5 +154,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 67f2d1f859..e3266ef3a9 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.0
+#
+# 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.0
+#
+# Example:
+#
+# -> { "execute": "replay-delete-break" }
+#
+##
+{ 'command': 'replay-delete-break' }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 8cf15ebc11..166ba10d2c 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 9ec6ce9996..86b3e943c2 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] 34+ messages in thread* [for-5.0 PATCH 07/11] replay: implement replay-seek command
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (5 preceding siblings ...)
2019-12-23 9:47 ` [for-5.0 PATCH 06/11] replay: introduce breakpoint at the specified step Pavel Dovgalyuk
@ 2019-12-23 9:47 ` Pavel Dovgalyuk
2019-12-23 9:48 ` [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
` (5 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
--
v2:
- renamed replay_seek qmp command into replay-seek
(suggested by Eric Blake)
v7:
- small fixes related to Markus Armbruster's review
v9:
- changed 'step' parameter name to 'icount'
- moved json stuff to replay.json and updated the description
(suggested by Markus Armbruster)
v10:
- updated the descriptions
---
hmp-commands.hx | 19 +++++++++
include/monitor/hmp.h | 1
qapi/replay.json | 20 ++++++++++
replay/replay-debugging.c | 92 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3704294da8..420565c7f8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1964,6 +1964,25 @@ STEXI
@findex replay_delete_break
Remove replay breakpoint which was previously set with replay_break.
The command is ignored when there are no replay breakpoints.
+ETEXI
+
+ {
+ .name = "replay_seek",
+ .args_type = "icount:i",
+ .params = "icount",
+ .help = "replay execution to the specified instruction count",
+ .cmd = hmp_replay_seek,
+ },
+
+STEXI
+@item replay_seek @var{icount}
+@findex replay_seek
+Automatically proceed to the instruction count @var{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.
ETEXI
{
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 0fd477aa13..bd97d4a8c6 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -156,5 +156,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 e3266ef3a9..5d24cdc680 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.0
+#
+# 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 166ba10d2c..f5a02a5aa1 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] 34+ messages in thread* [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (6 preceding siblings ...)
2019-12-23 9:47 ` [for-5.0 PATCH 07/11] replay: implement replay-seek command Pavel Dovgalyuk
@ 2019-12-23 9:48 ` Pavel Dovgalyuk
2020-01-13 17:48 ` Alex Bennée
2019-12-23 9:48 ` [for-5.0 PATCH 09/11] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
` (4 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:48 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
---
include/sysemu/replay.h | 2 ++
migration/savevm.c | 12 ++++++------
replay/replay-internal.h | 2 --
3 files changed, 8 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 ae84bf6ab0..0c5cac372a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2834,12 +2834,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",
@@ -2873,6 +2867,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-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] 34+ messages in thread* Re: [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate
2019-12-23 9:48 ` [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
@ 2020-01-13 17:48 ` Alex Bennée
2020-01-14 13:57 ` Pavel Dovgalyuk
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2020-01-13 17:48 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini,
crosthwaite.peter, ciro.santilli, jasowang, quintela, qemu-devel,
armbru, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, mreitz, artem.k.pisarenko, dgilbert,
rth
Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> 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.Dovgaluk@ispras.ru>
> ---
> include/sysemu/replay.h | 2 ++
> migration/savevm.c | 12 ++++++------
> replay/replay-internal.h | 2 --
> 3 files changed, 8 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 ae84bf6ab0..0c5cac372a 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2834,12 +2834,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",
> @@ -2873,6 +2867,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();
> +
<snip>
This is the commit that introduces:
ERROR:/home/alex.bennee/lsrc/qemu.git/replay/replay-events.c:80:replay_flush_events:
assertion failed: (replay_mutex_locked())
To the already failing:
/bin/sh -c "cd builds/all && make -j4 && cd tests/qemu-iotests && ./check -qcow2 267"
test case.
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate
2020-01-13 17:48 ` Alex Bennée
@ 2020-01-14 13:57 ` Pavel Dovgalyuk
0 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-14 13:57 UTC (permalink / raw)
To: 'Alex Bennée'
Cc: kwolf, peter.maydell, pavel.dovgaluk, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> 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.Dovgaluk@ispras.ru>
> > ---
> > include/sysemu/replay.h | 2 ++
> > migration/savevm.c | 12 ++++++------
> > replay/replay-internal.h | 2 --
> > 3 files changed, 8 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 ae84bf6ab0..0c5cac372a 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2834,12 +2834,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",
> > @@ -2873,6 +2867,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();
> > +
> <snip>
>
> This is the commit that introduces:
>
> ERROR:/home/alex.bennee/lsrc/qemu.git/replay/replay-events.c:80:replay_flush_events:
> assertion failed: (replay_mutex_locked())
>
> To the already failing:
>
> /bin/sh -c "cd builds/all && make -j4 && cd tests/qemu-iotests && ./check -qcow2 267"
>
> test case.
Please apply the following update to continue the testing:
--- 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;
+ }
+
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread
* [for-5.0 PATCH 09/11] gdbstub: add reverse step support in replay mode
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (7 preceding siblings ...)
2019-12-23 9:48 ` [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate Pavel Dovgalyuk
@ 2019-12-23 9:48 ` Pavel Dovgalyuk
2019-12-23 9:48 ` [for-5.0 PATCH 10/11] gdbstub: add reverse continue " Pavel Dovgalyuk
` (3 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:48 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
---
accel/tcg/translator.c | 1 +
cpus.c | 14 +++++++++--
exec.c | 7 ++++++
gdbstub.c | 56 +++++++++++++++++++++++++++++++++++++++++++--
include/sysemu/replay.h | 11 +++++++++
replay/replay-debugging.c | 33 +++++++++++++++++++++++++++
stubs/replay.c | 5 ++++
7 files changed, 121 insertions(+), 6 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/cpus.c b/cpus.c
index be2d655f37..4951a68796 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1062,9 +1062,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/exec.c b/exec.c
index d4b769d0d4..861fcc7ea3 100644
--- a/exec.c
+++ b/exec.c
@@ -2743,6 +2743,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 4cf8af365e..6539c8017e 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"
@@ -372,6 +373,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;
bool gdb_has_xml;
@@ -462,7 +477,7 @@ static int gdb_continue_partial(GDBState *s, char *newstates)
CPU_FOREACH(cpu) {
if (newstates[cpu->cpu_index] == 's') {
trace_gdbstub_op_stepping(cpu->cpu_index);
- cpu_single_step(cpu, sstep_flags);
+ cpu_single_step(cpu, get_sstep_flags());
}
}
s->running_state = 1;
@@ -481,7 +496,7 @@ static int gdb_continue_partial(GDBState *s, 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;
@@ -1847,10 +1862,31 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
gdb_set_cpu_pc(gdb_ctx->s, (target_ulong)gdb_ctx->params[0].val_ull);
}
- cpu_single_step(gdb_ctx->s->c_cpu, sstep_flags);
+ cpu_single_step(gdb_ctx->s->c_cpu, get_sstep_flags());
gdb_continue(gdb_ctx->s);
}
+static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+ if (replay_mode != REPLAY_MODE_PLAY) {
+ put_packet(gdb_ctx->s, "E22");
+ }
+ if (gdb_ctx->num_params == 1) {
+ switch (gdb_ctx->params[0].opcode) {
+ case 's':
+ if (replay_reverse_step()) {
+ gdb_continue(gdb_ctx->s);
+ } else {
+ put_packet(gdb_ctx->s, "E14");
+ }
+ return;
+ }
+ }
+
+ /* Default invalid command */
+ put_packet(gdb_ctx->s, "");
+}
+
static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
{
put_packet(gdb_ctx->s, "vCont;c;C;s;S");
@@ -2105,6 +2141,9 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
";qXfer:features:read+");
}
+ if (replay_mode == REPLAY_MODE_PLAY) {
+ pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";ReverseStep+");
+ }
if (gdb_ctx->num_params &&
strstr(gdb_ctx->params[0].data, "multiprocess+")) {
@@ -2444,6 +2483,17 @@ static int gdb_handle_packet(GDBState *s, 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 f5a02a5aa1..cdc01af4a2 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/stubs/replay.c b/stubs/replay.c
index 5974ec1f50..3d9f99ebb6 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -88,3 +88,8 @@ int replay_read_random(void *buf, size_t len)
{
return 0;
}
+
+bool replay_reverse_step(void)
+{
+ return false;
+}
^ permalink raw reply related [flat|nested] 34+ messages in thread* [for-5.0 PATCH 10/11] gdbstub: add reverse continue support in replay mode
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (8 preceding siblings ...)
2019-12-23 9:48 ` [for-5.0 PATCH 09/11] gdbstub: add reverse step support in replay mode Pavel Dovgalyuk
@ 2019-12-23 9:48 ` Pavel Dovgalyuk
2019-12-23 9:48 ` [for-5.0 PATCH 11/11] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
` (2 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:48 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
---
cpus.c | 5 +++
exec.c | 1 +
gdbstub.c | 10 ++++++
include/sysemu/replay.h | 8 +++++
replay/replay-debugging.c | 71 +++++++++++++++++++++++++++++++++++++++++++++
stubs/replay.c | 5 +++
6 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 4951a68796..5a5e7e5f4e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1068,6 +1068,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/exec.c b/exec.c
index 861fcc7ea3..0d5ea6b6fe 100644
--- a/exec.c
+++ b/exec.c
@@ -2748,6 +2748,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 6539c8017e..2298e4cb98 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1880,6 +1880,13 @@ static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
put_packet(gdb_ctx->s, "E14");
}
return;
+ case 'c':
+ if (replay_reverse_continue()) {
+ gdb_continue(gdb_ctx->s);
+ } else {
+ put_packet(gdb_ctx->s, "E14");
+ }
+ return;
}
}
@@ -2142,7 +2149,8 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
";qXfer:features:read+");
}
if (replay_mode == REPLAY_MODE_PLAY) {
- pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";ReverseStep+");
+ pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->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 cdc01af4a2..e4a083949e 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/stubs/replay.c b/stubs/replay.c
index 3d9f99ebb6..a7ca49a717 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -93,3 +93,8 @@ bool replay_reverse_step(void)
{
return false;
}
+
+bool replay_reverse_continue(void)
+{
+ return false;
+}
^ permalink raw reply related [flat|nested] 34+ messages in thread* [for-5.0 PATCH 11/11] replay: describe reverse debugging in docs/replay.txt
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (9 preceding siblings ...)
2019-12-23 9:48 ` [for-5.0 PATCH 10/11] gdbstub: add reverse continue " Pavel Dovgalyuk
@ 2019-12-23 9:48 ` Pavel Dovgalyuk
2020-01-09 6:13 ` [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
2020-01-16 11:11 ` Alex Bennée
12 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2019-12-23 9:48 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini, quintela,
ciro.santilli, jasowang, crosthwaite.peter, armbru, mreitz,
alex.bennee, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, artem.k.pisarenko, dgilbert, 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.Dovgaluk@ispras.ru>
---
docs/replay.txt | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/docs/replay.txt b/docs/replay.txt
index f4619a62a3..07104492f2 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] 34+ messages in thread* RE: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (10 preceding siblings ...)
2019-12-23 9:48 ` [for-5.0 PATCH 11/11] replay: describe reverse debugging in docs/replay.txt Pavel Dovgalyuk
@ 2020-01-09 6:13 ` Pavel Dovgalyuk
2020-01-09 12:00 ` Kevin Wolf
2020-01-16 11:11 ` Alex Bennée
12 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-09 6:13 UTC (permalink / raw)
To: 'Pavel Dovgalyuk', qemu-devel
Cc: kwolf, peter.maydell, pavel.dovgaluk, quintela, ciro.santilli,
jasowang, crosthwaite.peter, armbru, mreitz, alex.bennee,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, artem.k.pisarenko, dgilbert, rth
Ping.
Pavel Dovgalyuk
> -----Original Message-----
> From: Pavel Dovgalyuk [mailto:pavel.dovgaluk@gmail.com]
> Sent: Monday, December 23, 2019 12:46 PM
> To: qemu-devel@nongnu.org
> Cc: kwolf@redhat.com; peter.maydell@linaro.org; crosthwaite.peter@gmail.com;
> boost.lists@gmail.com; artem.k.pisarenko@gmail.com; quintela@redhat.com;
> ciro.santilli@gmail.com; jasowang@redhat.com; mst@redhat.com; armbru@redhat.com;
> mreitz@redhat.com; maria.klimushenkova@ispras.ru; dovgaluk@ispras.ru; kraxel@redhat.com;
> pavel.dovgaluk@ispras.ru; thomas.dullien@googlemail.com; pbonzini@redhat.com;
> alex.bennee@linaro.org; dgilbert@redhat.com; rth@twiddle.net
> Subject: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
>
> 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
>
> The patches are available in the repository:
> https://github.com/ispras/qemu/tree/rr-191223
>
> ---
>
> Pavel Dovgaluk (11):
> replay: provide an accessor for rr filename
> qcow2: introduce icount field for snapshots
> migration: introduce icount field for snapshots
> 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
>
>
> MAINTAINERS | 1
> accel/tcg/translator.c | 1
> block/qapi.c | 18 ++
> block/qcow2-snapshot.c | 9 +
> block/qcow2.h | 3
> blockdev.c | 10 +
> cpus.c | 19 ++-
> docs/interop/qcow2.txt | 4 +
> docs/replay.txt | 33 +++++
> exec.c | 8 +
> gdbstub.c | 64 ++++++++-
> hmp-commands-info.hx | 14 ++
> hmp-commands.hx | 53 +++++++
> include/block/snapshot.h | 1
> include/monitor/hmp.h | 4 +
> include/sysemu/replay.h | 24 +++
> migration/savevm.c | 17 ++
> qapi/Makefile.objs | 2
> qapi/block-core.json | 8 +
> qapi/block.json | 3
> qapi/misc.json | 18 --
> qapi/qapi-schema.json | 1
> qapi/replay.json | 121 +++++++++++++++++
> replay/Makefile.objs | 1
> replay/replay-debugging.c | 325 +++++++++++++++++++++++++++++++++++++++++++++
> replay/replay-internal.h | 6 +
> replay/replay.c | 22 +++
> stubs/replay.c | 10 +
> 28 files changed, 761 insertions(+), 39 deletions(-)
> create mode 100644 qapi/replay.json
> create mode 100644 replay/replay-debugging.c
>
> --
> Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-09 6:13 ` [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
@ 2020-01-09 12:00 ` Kevin Wolf
2020-01-09 12:12 ` Pavel Dovgalyuk
2020-01-13 9:29 ` Markus Armbruster
0 siblings, 2 replies; 34+ messages in thread
From: Kevin Wolf @ 2020-01-09 12:00 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: peter.maydell, pavel.dovgaluk, crosthwaite.peter, ciro.santilli,
jasowang, quintela, qemu-devel, armbru, alex.bennee,
'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
> Ping.
I think you have my Acked-by for the block-related patches in this
series now. If I missed something, please let me know.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-09 12:00 ` Kevin Wolf
@ 2020-01-09 12:12 ` Pavel Dovgalyuk
2020-01-13 9:29 ` Markus Armbruster
1 sibling, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-09 12:12 UTC (permalink / raw)
To: 'Kevin Wolf'
Cc: peter.maydell, pavel.dovgaluk, crosthwaite.peter, ciro.santilli,
jasowang, quintela, qemu-devel, armbru, alex.bennee,
'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
> > Ping.
>
> I think you have my Acked-by for the block-related patches in this
> series now. If I missed something, please let me know.
Thank you.
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-09 12:00 ` Kevin Wolf
2020-01-09 12:12 ` Pavel Dovgalyuk
@ 2020-01-13 9:29 ` Markus Armbruster
2020-01-13 9:35 ` Pavel Dovgalyuk
1 sibling, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2020-01-13 9:29 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: Kevin Wolf, peter.maydell, boost.lists, artem.k.pisarenko,
crosthwaite.peter, ciro.santilli, jasowang, quintela, qemu-devel,
armbru, 'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, pavel.dovgaluk, thomas.dullien,
pbonzini, mreitz, alex.bennee, dgilbert, rth
Kevin Wolf <kwolf@redhat.com> writes:
> Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
>> Ping.
>
> I think you have my Acked-by for the block-related patches in this
> series now. If I missed something, please let me know.
Pavel, whom are you nudging to do what?
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 9:29 ` Markus Armbruster
@ 2020-01-13 9:35 ` Pavel Dovgalyuk
2020-01-13 10:06 ` Kevin Wolf
2020-01-13 17:55 ` Alex Bennée
0 siblings, 2 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-13 9:35 UTC (permalink / raw)
To: 'Markus Armbruster'
Cc: 'Kevin Wolf',
peter.maydell, boost.lists, artem.k.pisarenko, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, mreitz,
'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, pavel.dovgaluk, thomas.dullien,
pbonzini, alex.bennee, dgilbert, rth
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
> >> Ping.
> >
> > I think you have my Acked-by for the block-related patches in this
> > series now. If I missed something, please let me know.
>
> Pavel, whom are you nudging to do what?
I'm not sure.
My prior patches for record/replay were pulled by Paolo.
But reverse debugging is more like a modification of things,
and not a completely new subsystem.
Everything but gdbstub was already acked by the maintainers.
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 9:35 ` Pavel Dovgalyuk
@ 2020-01-13 10:06 ` Kevin Wolf
2020-01-13 10:14 ` Peter Maydell
2020-01-13 17:55 ` Alex Bennée
1 sibling, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2020-01-13 10:06 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: peter.maydell, boost.lists, artem.k.pisarenko, crosthwaite.peter,
ciro.santilli, jasowang, quintela, 'Markus Armbruster',
qemu-devel, 'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, pavel.dovgaluk, thomas.dullien,
pbonzini, mreitz, alex.bennee, dgilbert, rth
Am 13.01.2020 um 10:35 hat Pavel Dovgalyuk geschrieben:
> > From: Markus Armbruster [mailto:armbru@redhat.com]
> > Kevin Wolf <kwolf@redhat.com> writes:
> >
> > > Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
> > >> Ping.
> > >
> > > I think you have my Acked-by for the block-related patches in this
> > > series now. If I missed something, please let me know.
> >
> > Pavel, whom are you nudging to do what?
>
> I'm not sure.
> My prior patches for record/replay were pulled by Paolo.
> But reverse debugging is more like a modification of things,
> and not a completely new subsystem.
In MAINTAINERS, you are listed yourself as the maintainer for
record/replay. I wonder whether you shouldn't just be sending pull
requests after getting Acked-by or Reviewed-by from the maintainers of
other subsystems you touch.
> Everything but gdbstub was already acked by the maintainers.
The GDB stub seems to be maintained by Alex Bennée, so he is probably
the one you need to nudge to give at least an Acked-by.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 10:06 ` Kevin Wolf
@ 2020-01-13 10:14 ` Peter Maydell
2020-01-13 10:27 ` Kevin Wolf
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2020-01-13 10:14 UTC (permalink / raw)
To: Kevin Wolf
Cc: Michael S. Tsirkin, Artem Pisarenko, Peter Crosthwaite,
Ciro Santilli, Jason Wang, Juan Quintela, Thomas Dullien,
Markus Armbruster, QEMU Developers, Pavel Dovgalyuk,
maria.klimushenkova, Pavel Dovgalyuk, Gerd Hoffmann,
Pavel Dovgaluk, Igor R, Paolo Bonzini, Max Reitz,
Alex Bennée, Dr. David Alan Gilbert, Richard Henderson
On Mon, 13 Jan 2020 at 10:07, Kevin Wolf <kwolf@redhat.com> wrote:
> In MAINTAINERS, you are listed yourself as the maintainer for
> record/replay. I wonder whether you shouldn't just be sending pull
> requests after getting Acked-by or Reviewed-by from the maintainers of
> other subsystems you touch.
Ideally somebody else should be interested enough in record/replay
to review patches. "I'm a subsystem maintainer and send pull
requests" ideally shouldn't be something we give out just because
patches aren't getting code review, though I know that it
does sometimes degenerate into that...
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 10:14 ` Peter Maydell
@ 2020-01-13 10:27 ` Kevin Wolf
2020-01-13 10:47 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2020-01-13 10:27 UTC (permalink / raw)
To: Peter Maydell
Cc: Michael S. Tsirkin, Artem Pisarenko, Peter Crosthwaite,
Ciro Santilli, Jason Wang, Juan Quintela, Thomas Dullien,
Markus Armbruster, QEMU Developers, Pavel Dovgalyuk,
maria.klimushenkova, Pavel Dovgalyuk, Gerd Hoffmann,
Pavel Dovgaluk, Igor R, Paolo Bonzini, Max Reitz,
Alex Bennée, Dr. David Alan Gilbert, Richard Henderson
Am 13.01.2020 um 11:14 hat Peter Maydell geschrieben:
> On Mon, 13 Jan 2020 at 10:07, Kevin Wolf <kwolf@redhat.com> wrote:
> > In MAINTAINERS, you are listed yourself as the maintainer for
> > record/replay. I wonder whether you shouldn't just be sending pull
> > requests after getting Acked-by or Reviewed-by from the maintainers of
> > other subsystems you touch.
>
> Ideally somebody else should be interested enough in record/replay
> to review patches. "I'm a subsystem maintainer and send pull
> requests" ideally shouldn't be something we give out just because
> patches aren't getting code review, though I know that it
> does sometimes degenerate into that...
I had the impression that he said he had collected (almost) all of the
necessary reviews, but nobody really seems to be interested to take the
series through their tree because no matter who you ask, the majority of
changes will always be for other subsystems.
And as record/replay is already listed as a separate subsystem in
MAINTAINERS, it seems to make sense to me that it also gets its own pull
requests rather than trying to get patches merged though the trees of
various subsystem maintainers who all aren't really responsible for it.
Kevin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 10:27 ` Kevin Wolf
@ 2020-01-13 10:47 ` Peter Maydell
0 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2020-01-13 10:47 UTC (permalink / raw)
To: Kevin Wolf
Cc: Michael S. Tsirkin, Artem Pisarenko, Peter Crosthwaite,
Ciro Santilli, Jason Wang, Juan Quintela, Thomas Dullien,
Markus Armbruster, QEMU Developers, Pavel Dovgalyuk,
maria.klimushenkova, Pavel Dovgalyuk, Gerd Hoffmann,
Pavel Dovgaluk, Igor R, Paolo Bonzini, Max Reitz,
Alex Bennée, Dr. David Alan Gilbert, Richard Henderson
On Mon, 13 Jan 2020 at 10:27, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 13.01.2020 um 11:14 hat Peter Maydell geschrieben:
> > On Mon, 13 Jan 2020 at 10:07, Kevin Wolf <kwolf@redhat.com> wrote:
> > > In MAINTAINERS, you are listed yourself as the maintainer for
> > > record/replay. I wonder whether you shouldn't just be sending pull
> > > requests after getting Acked-by or Reviewed-by from the maintainers of
> > > other subsystems you touch.
> >
> > Ideally somebody else should be interested enough in record/replay
> > to review patches. "I'm a subsystem maintainer and send pull
> > requests" ideally shouldn't be something we give out just because
> > patches aren't getting code review, though I know that it
> > does sometimes degenerate into that...
>
> I had the impression that he said he had collected (almost) all of the
> necessary reviews, but nobody really seems to be interested to take the
> series through their tree because no matter who you ask, the majority of
> changes will always be for other subsystems.
No, the series has only got acked-bys so far, except for the one patch
that touches qapi, which Markus reviewed. (The bulk of the changes
here are in replay/, and so far nobody's looked at those AFAIK.)
> And as record/replay is already listed as a separate subsystem in
> MAINTAINERS, it seems to make sense to me that it also gets its own pull
> requests rather than trying to get patches merged though the trees of
> various subsystem maintainers who all aren't really responsible for it.
Yeah, pull requests would be fine. Pull requests of whole
patchsets that are basically unreviewed are something I think
we should try to avoid if we can.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-13 9:35 ` Pavel Dovgalyuk
2020-01-13 10:06 ` Kevin Wolf
@ 2020-01-13 17:55 ` Alex Bennée
1 sibling, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2020-01-13 17:55 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: 'Kevin Wolf',
peter.maydell, boost.lists, crosthwaite.peter, ciro.santilli,
jasowang, quintela, 'Markus Armbruster',
qemu-devel, 'Pavel Dovgalyuk',
maria.klimushenkova, mst, kraxel, pavel.dovgaluk, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
(looping the list back in)
Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>> From: Alex Bennée [mailto:alex.bennee@linaro.org]
>> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>>
>> >> From: Markus Armbruster [mailto:armbru@redhat.com]
>> >> Kevin Wolf <kwolf@redhat.com> writes:
>> >>
>> >> > Am 09.01.2020 um 07:13 hat Pavel Dovgalyuk geschrieben:
>> >> >> Ping.
>> >> >
>> >> > I think you have my Acked-by for the block-related patches in this
>> >> > series now. If I missed something, please let me know.
>> >>
>> >> Pavel, whom are you nudging to do what?
>> >
>> > I'm not sure.
>> > My prior patches for record/replay were pulled by Paolo.
>> > But reverse debugging is more like a modification of things,
>> > and not a completely new subsystem.
>> > Everything but gdbstub was already acked by the maintainers.
>>
>> I'm having a look at the series now. What would help is if we can have
>> some sort of test case to ensure expected behaviour is protected. We
>> have a basic smoke test for record/replay and my gdbstub series:
>>
>> https://github.com/stsquad/qemu/tree/gdbstub/sve-registers-v5
>>
>> introduces some infrastructure for testing thing via gdbstub. The main
>> thing needed now is some sort of gdb capability test to ensure the gdb
>> we are using supports whatever extensions it needs to work.
>
> The smoke test may be rather easy:
> 1. Execute qemu record with qcow2 image and rrsnapshot
> 2. Start qemu replay with -S
> 3. Connect gdb to qemu
> 4. stepi
> 5. break $pc
> 6. Save pc to X
> 7. stepi 10
> 8. Save pc to Y
> 9. stepi
> 10. reverse-stepi
> 11. pc should be Y
> 12. reverse-continue
> 13. pc should be X
>
> Is it possible with your infrastructure?
I think so - even if the reverse step instructions are not in the python
API the script can always do:
gdb.parse_and_eval("command")
The main thing is ensuring probing the gdb for a version/feature first
so we can safely exit and skip the test if the host gdb isn't capable.
For the SVE testing I've had to use my own compiled version of GDB which
is a little ugly:
~/lsrc/qemu.git/tests/guest-debug/run-test.py \
--qemu ./aarch64-linux-user/qemu-aarch64 \
--qargs "-cpu max" \
--bin ./tests/tcg/aarch64-linux-user/sve-ioctls \
--test ~/lsrc/qemu.git/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py \
--gdb ~/src/tools/binutils-gdb.git/builds/all/install/bin/gdb
We should probably wrap that up into the tests/tcg configure.sh code so
the GDB used can persist with the build. Maybe via:
./configure --with-gdb=~/src/tools/binutils-gdb.git/builds/all/install/bin/gdb
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2019-12-23 9:45 [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
` (11 preceding siblings ...)
2020-01-09 6:13 ` [for-5.0 PATCH 00/11] Support for reverse debugging with GDB Pavel Dovgalyuk
@ 2020-01-16 11:11 ` Alex Bennée
2020-01-16 11:26 ` Pavel Dovgalyuk
12 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2020-01-16 11:11 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: kwolf, peter.maydell, pavel.dovgaluk, pbonzini,
crosthwaite.peter, ciro.santilli, jasowang, quintela, qemu-devel,
armbru, maria.klimushenkova, mst, kraxel, boost.lists,
thomas.dullien, dovgaluk, mreitz, artem.k.pisarenko, dgilbert,
rth
Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
> 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
>
> The patches are available in the repository:
> https://github.com/ispras/qemu/tree/rr-191223
So I tried with your additional patch. Launching QEMU as:
./aarch64-softmmu//qemu-system-aarch64 -monitor none \
-display none -M virt -cpu max -display none \
-semihosting-config enable=on \
-kernel ./tests/tcg/aarch64-softmmu/memory \
-icount shift=5,rr=replay,rrfile=record.bin \
-s -S -d trace:gdbstub\*
And gdb:
gdb tests/tcg/aarch64-softmmu/memory \
-ex "target remote localhost:1234"
I get the following log:
(gdb) x/3i $pc
=> 0x400037b0 <__start>: adr x0, 0x40003000 <vector_table>
0x400037b4 <__start+4>: msr vbar_el1, x0
0x400037b8 <__start+8>: adrp x0, 0x40200000
(gdb) p/x $x0
$1 = 0x0
(gdb) si
92 msr vbar_el1, x0
(gdb) p/x $x0
$2 = 0x40003000
(gdb) rsi
warning: Remote failure reply: E14
Program stopped.
__start () at /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/system/boot.S:92
92 msr vbar_el1, x0
(gdb) p/x $x0
$3 = 0x40003000
So it doesn't seem to be working.
>
> ---
>
> Pavel Dovgaluk (11):
> replay: provide an accessor for rr filename
> qcow2: introduce icount field for snapshots
> migration: introduce icount field for snapshots
> 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
>
>
> MAINTAINERS | 1
> accel/tcg/translator.c | 1
> block/qapi.c | 18 ++
> block/qcow2-snapshot.c | 9 +
> block/qcow2.h | 3
> blockdev.c | 10 +
> cpus.c | 19 ++-
> docs/interop/qcow2.txt | 4 +
> docs/replay.txt | 33 +++++
> exec.c | 8 +
> gdbstub.c | 64 ++++++++-
> hmp-commands-info.hx | 14 ++
> hmp-commands.hx | 53 +++++++
> include/block/snapshot.h | 1
> include/monitor/hmp.h | 4 +
> include/sysemu/replay.h | 24 +++
> migration/savevm.c | 17 ++
> qapi/Makefile.objs | 2
> qapi/block-core.json | 8 +
> qapi/block.json | 3
> qapi/misc.json | 18 --
> qapi/qapi-schema.json | 1
> qapi/replay.json | 121 +++++++++++++++++
> replay/Makefile.objs | 1
> replay/replay-debugging.c | 325 +++++++++++++++++++++++++++++++++++++++++++++
> replay/replay-internal.h | 6 +
> replay/replay.c | 22 +++
> stubs/replay.c | 10 +
> 28 files changed, 761 insertions(+), 39 deletions(-)
> create mode 100644 qapi/replay.json
> create mode 100644 replay/replay-debugging.c
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-16 11:11 ` Alex Bennée
@ 2020-01-16 11:26 ` Pavel Dovgalyuk
2020-01-17 17:40 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-16 11:26 UTC (permalink / raw)
To: 'Alex Bennée'
Cc: kwolf, peter.maydell, pavel.dovgaluk, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
>
> > 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
> >
> > The patches are available in the repository:
> > https://github.com/ispras/qemu/tree/rr-191223
>
> So I tried with your additional patch. Launching QEMU as:
>
> ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
> -display none -M virt -cpu max -display none \
> -semihosting-config enable=on \
> -kernel ./tests/tcg/aarch64-softmmu/memory \
> -icount shift=5,rr=replay,rrfile=record.bin \
> -s -S -d trace:gdbstub\*
>
> And gdb:
>
> gdb tests/tcg/aarch64-softmmu/memory \
> -ex "target remote localhost:1234"
>
> I get the following log:
>
> (gdb) x/3i $pc
> => 0x400037b0 <__start>: adr x0, 0x40003000 <vector_table>
> 0x400037b4 <__start+4>: msr vbar_el1, x0
> 0x400037b8 <__start+8>: adrp x0, 0x40200000
> (gdb) p/x $x0
> $1 = 0x0
> (gdb) si
> 92 msr vbar_el1, x0
> (gdb) p/x $x0
> $2 = 0x40003000
> (gdb) rsi
> warning: Remote failure reply: E14
>
> Program stopped.
> __start () at /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/system/boot.S:92
> 92 msr vbar_el1, x0
> (gdb) p/x $x0
> $3 = 0x40003000
>
> So it doesn't seem to be working.
That's ok, you'll need at least one VM snapshot available to recover the initial VM state.
Try changing the command lines in the following way:
First, create empty.qcow2 which will be used for saving the snapshots.
Then record with initial snapshot and attached empty.qcow2:
./aarch64-softmmu//qemu-system-aarch64 -monitor none \
-display none -M virt -cpu max \
-kernel ./tests/tcg/aarch64-softmmu/memory \
-icount shift=5,rr=record,rrfile=record.bin,rrsnapshot=init \
-drive file=empty.qcow2
After that you can replay the execution with the support of reverse debugging:
./aarch64-softmmu//qemu-system-aarch64 -monitor none \
-display none -M virt -cpu max \
-kernel ./tests/tcg/aarch64-softmmu/memory \
-icount shift=5,rr=replay,rrfile=record.bin,rrsnapshot=init \
-drive file=empty.qcow2
I removed "-semihosting-config enable=on", because I'm not sure what is it for.
It may break the replay, because there is no implementation of
semihosting input record/replay.
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-16 11:26 ` Pavel Dovgalyuk
@ 2020-01-17 17:40 ` Alex Bennée
2020-01-20 11:58 ` Pavel Dovgalyuk
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2020-01-17 17:40 UTC (permalink / raw)
To: Pavel Dovgalyuk
Cc: kwolf, peter.maydell, pavel.dovgaluk, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>> From: Alex Bennée [mailto:alex.bennee@linaro.org]
>> Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
>>
>> > 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
>> >
>> > The patches are available in the repository:
>> > https://github.com/ispras/qemu/tree/rr-191223
>>
>> So I tried with your additional patch. Launching QEMU as:
>>
>> ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
>> -display none -M virt -cpu max -display none \
>> -semihosting-config enable=on \
>> -kernel ./tests/tcg/aarch64-softmmu/memory \
>> -icount shift=5,rr=replay,rrfile=record.bin \
>> -s -S -d trace:gdbstub\*
>>
>> And gdb:
>>
>> gdb tests/tcg/aarch64-softmmu/memory \
>> -ex "target remote localhost:1234"
>>
>> I get the following log:
>>
>> (gdb) x/3i $pc
>> => 0x400037b0 <__start>: adr x0, 0x40003000 <vector_table>
>> 0x400037b4 <__start+4>: msr vbar_el1, x0
>> 0x400037b8 <__start+8>: adrp x0, 0x40200000
>> (gdb) p/x $x0
>> $1 = 0x0
>> (gdb) si
>> 92 msr vbar_el1, x0
>> (gdb) p/x $x0
>> $2 = 0x40003000
>> (gdb) rsi
>> warning: Remote failure reply: E14
>>
>> Program stopped.
>> __start () at /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/system/boot.S:92
>> 92 msr vbar_el1, x0
>> (gdb) p/x $x0
>> $3 = 0x40003000
>>
>> So it doesn't seem to be working.
>
> That's ok, you'll need at least one VM snapshot available to recover the initial VM state.
> Try changing the command lines in the following way:
>
> First, create empty.qcow2 which will be used for saving the snapshots.
> Then record with initial snapshot and attached empty.qcow2:
>
> ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
> -display none -M virt -cpu max \
> -kernel ./tests/tcg/aarch64-softmmu/memory \
> -icount shift=5,rr=record,rrfile=record.bin,rrsnapshot=init \
> -drive file=empty.qcow2
./aarch64-softmmu//qemu-system-aarch64 -monitor none -display none -M virt -cpu max -display none -semihosting-config enable=on -kernel ./tests/tcg/aarch64-softmmu/memory -icount shift=5,rr=record,rrfile=record.bin,rrsnapshot=init -drive file=empty.qcow2
qemu-system-aarch64: invalid accelerator kvm
qemu-system-aarch64: falling back to tcg
qemu-system-aarch64: The qcow format used by node '#block163' does not support live migration
qemu-system-aarch64: Could not create snapshot for icount record
I don't know if this is down to the current state of the tree but I
haven't been able to get it working.
>
> After that you can replay the execution with the support of reverse debugging:
>
> ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
> -display none -M virt -cpu max \
> -kernel ./tests/tcg/aarch64-softmmu/memory \
> -icount shift=5,rr=replay,rrfile=record.bin,rrsnapshot=init \
> -drive file=empty.qcow2
>
> I removed "-semihosting-config enable=on", because I'm not sure what is it for.
> It may break the replay, because there is no implementation of
> semihosting input record/replay.
For this testcase semihosting in just a convenient output device (in
lieu of a serial device). We probably need to come up with a strategy on
how we handle all these devices otherwise we will end up with a random
selection of hardware combinations which work.
>
> Pavel Dovgalyuk
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [for-5.0 PATCH 00/11] Support for reverse debugging with GDB
2020-01-17 17:40 ` Alex Bennée
@ 2020-01-20 11:58 ` Pavel Dovgalyuk
0 siblings, 0 replies; 34+ messages in thread
From: Pavel Dovgalyuk @ 2020-01-20 11:58 UTC (permalink / raw)
To: 'Alex Bennée'
Cc: kwolf, peter.maydell, pavel.dovgaluk, crosthwaite.peter,
ciro.santilli, jasowang, quintela, qemu-devel, armbru,
maria.klimushenkova, mst, kraxel, boost.lists, thomas.dullien,
pbonzini, mreitz, artem.k.pisarenko, dgilbert, rth
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>
> >> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> >> Pavel Dovgalyuk <pavel.dovgaluk@gmail.com> writes:
> >>
> >> > 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
> >> >
> >> > The patches are available in the repository:
> >> > https://github.com/ispras/qemu/tree/rr-191223
> >>
> >> So I tried with your additional patch. Launching QEMU as:
> >>
> >> ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
> >> -display none -M virt -cpu max -display none \
> >> -semihosting-config enable=on \
> >> -kernel ./tests/tcg/aarch64-softmmu/memory \
> >> -icount shift=5,rr=replay,rrfile=record.bin \
> >> -s -S -d trace:gdbstub\*
> >>
> >> And gdb:
> >>
> >> gdb tests/tcg/aarch64-softmmu/memory \
> >> -ex "target remote localhost:1234"
> >>
> >> I get the following log:
> >>
> >> (gdb) x/3i $pc
> >> => 0x400037b0 <__start>: adr x0, 0x40003000 <vector_table>
> >> 0x400037b4 <__start+4>: msr vbar_el1, x0
> >> 0x400037b8 <__start+8>: adrp x0, 0x40200000
> >> (gdb) p/x $x0
> >> $1 = 0x0
> >> (gdb) si
> >> 92 msr vbar_el1, x0
> >> (gdb) p/x $x0
> >> $2 = 0x40003000
> >> (gdb) rsi
> >> warning: Remote failure reply: E14
> >>
> >> Program stopped.
> >> __start () at /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/system/boot.S:92
> >> 92 msr vbar_el1, x0
> >> (gdb) p/x $x0
> >> $3 = 0x40003000
> >>
> >> So it doesn't seem to be working.
> >
> > That's ok, you'll need at least one VM snapshot available to recover the initial VM state.
> > Try changing the command lines in the following way:
> >
> > First, create empty.qcow2 which will be used for saving the snapshots.
> > Then record with initial snapshot and attached empty.qcow2:
> >
> > ./aarch64-softmmu//qemu-system-aarch64 -monitor none \
> > -display none -M virt -cpu max \
> > -kernel ./tests/tcg/aarch64-softmmu/memory \
> > -icount shift=5,rr=record,rrfile=record.bin,rrsnapshot=init \
> > -drive file=empty.qcow2
>
> ./aarch64-softmmu//qemu-system-aarch64 -monitor none -display none -M virt -cpu max -display
> none -semihosting-config enable=on -kernel ./tests/tcg/aarch64-softmmu/memory -icount
> shift=5,rr=record,rrfile=record.bin,rrsnapshot=init -drive file=empty.qcow2
> qemu-system-aarch64: invalid accelerator kvm
> qemu-system-aarch64: falling back to tcg
> qemu-system-aarch64: The qcow format used by node '#block163' does not support live migration
> qemu-system-aarch64: Could not create snapshot for icount record
It seems that you have some problems with your disk image. Is it qcow2 or just qcow?
> For this testcase semihosting in just a convenient output device (in
> lieu of a serial device).
I tried this test kernel with your options and everything was ok.
> We probably need to come up with a strategy on
> how we handle all these devices otherwise we will end up with a random
> selection of hardware combinations which work.
All correctly implemented virtual hardware should support record/replay.
But real semihosting (like file IO) should not, because it provides
untracked virtual machine inputs.
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 34+ messages in thread