Re: [PATCH v5 20/24] replay: simple auto-snapshot mode for record
On 18.03.2024 18:46, Nicholas Piggin wrote:
record makes an initial snapshot when the machine is created, to enable
reverse-debugging. Often the issue being debugged appears near the end of
the trace, so it is important for performance to keep snapshots close to
the end.
This implements a periodic snapshot mode that keeps a rolling set of
recent snapshots. This could be done by the debugger or other program
that talks QMP, but for setting up simple scenarios and tests, this is
more convenient.
Signed-off-by: Nicholas Piggin
---
docs/system/replay.rst | 5
include/sysemu/replay.h | 11
replay/replay-snapshot.c | 57
replay/replay.c | 27 +--
system/vl.c | 9 +++
qemu-options.hx | 9 +--
6 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/docs/system/replay.rst b/docs/system/replay.rst
index ca7c17c63d..1ae8614475 100644
--- a/docs/system/replay.rst
+++ b/docs/system/replay.rst
@@ -156,6 +156,11 @@ for storing VM snapshots. Here is the example of the
command line for this:
``empty.qcow2`` drive does not connected to any virtual block device and used
for VM snapshots only.
+``rrsnapmode`` can be used to select just an initial snapshot or periodic
+snapshots, with ``rrsnapcount`` specifying the number of periodic snapshots
+to maintain, and ``rrsnaptime`` the amount of run time in seconds between
+periodic snapshots.
+
.. _network-label:
Network devices
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 8102fa54f0..92fa82842b 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -48,6 +48,17 @@ typedef enum ReplayCheckpoint ReplayCheckpoint;
typedef struct ReplayNetState ReplayNetState;
+enum ReplaySnapshotMode {
+REPLAY_SNAPSHOT_MODE_INITIAL,
+REPLAY_SNAPSHOT_MODE_PERIODIC,
+};
+typedef enum ReplaySnapshotMode ReplaySnapshotMode;
+
+extern ReplaySnapshotMode replay_snapshot_mode;
+
+extern uint64_t replay_snapshot_periodic_delay;
+extern int replay_snapshot_periodic_nr_keep;
Please put the internal variables and enum into the replay-internal.h
+
/* Name of the initial VM snapshot */
extern char *replay_snapshot;
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index ccb4d89dda..762555feaa 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -70,6 +70,53 @@ void replay_vmstate_register(void)
vmstate_register(NULL, 0, _replay, _state);
}
+static QEMUTimer *replay_snapshot_timer;
+static int replay_snapshot_count;
+
+static void replay_snapshot_timer_cb(void *opaque)
+{
+Error *err = NULL;
+char *name;
+
+if (!replay_can_snapshot()) {
+/* Try again soon */
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay / 10);
+return;
+}
+
+name = g_strdup_printf("%s-%d", replay_snapshot, replay_snapshot_count);
+if (!save_snapshot(name,
+ true, NULL, false, NULL, )) {
+error_report_err(err);
+error_report("Could not create periodic snapshot "
+ "for icount record, disabling");
+g_free(name);
+return;
+}
+g_free(name);
+replay_snapshot_count++;
+
+if (replay_snapshot_periodic_nr_keep >= 1 &&
+replay_snapshot_count > replay_snapshot_periodic_nr_keep) {
+int del_nr;
+
+del_nr = replay_snapshot_count - replay_snapshot_periodic_nr_keep - 1;
+name = g_strdup_printf("%s-%d", replay_snapshot, del_nr);
+if (!delete_snapshot(name, false, NULL, )) {
+error_report_err(err);
+error_report("Could not delete periodic snapshot "
+ "for icount record");
+}
+g_free(name);
+}
+
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
+}
+
void replay_vmstate_init(void)
{
Error *err = NULL;
@@ -82,6 +129,16 @@ void replay_vmstate_init(void)
error_report("Could not create snapshot for icount record");
exit(1);
}
+
+if (replay_snapshot_mode == REPLAY_SNAPSHOT_MODE_PERIODIC) {
+replay_snapshot_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+ replay_snapshot_timer_cb,
+ NULL);
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
+}
+
} else if (replay_mode == REPLAY_MODE_PLAY) {
if (!load_snapshot(replay_snapshot, NULL, false, NULL, )) {
error_report_err(err);
diff --git a/replay/replay.c b/replay/replay.c
Re: [PATCH v5 10/24] virtio-net: Use replay_schedule_bh_event for bhs that affect machine state
Reviewed-by: Pavel Dovgalyuk
On 18.03.2024 18:46, Nicholas Piggin wrote:
The regular qemu_bh_schedule() calls result in non-deterministic
execution of the bh in record-replay mode, which causes replay failure.
Signed-off-by: Nicholas Piggin
---
hw/net/virtio-net.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9959f1932b..6ac737f2cf 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -40,6 +40,7 @@
#include "migration/misc.h"
#include "standard-headers/linux/ethtool.h"
#include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
#include "trace.h"
#include "monitor/qdev.h"
#include "monitor/monitor.h"
@@ -416,7 +417,7 @@ static void virtio_net_set_status(struct VirtIODevice
*vdev, uint8_t status)
timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
n->tx_timeout);
} else {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
} else {
if (q->tx_timer) {
@@ -2724,7 +2725,7 @@ static void virtio_net_tx_complete(NetClientState *nc,
ssize_t len)
*/
virtio_queue_set_notification(q->tx_vq, 0);
if (q->tx_bh) {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
} else {
timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
@@ -2879,7 +2880,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev,
VirtQueue *vq)
return;
}
virtio_queue_set_notification(vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
static void virtio_net_tx_timer(void *opaque)
@@ -2962,7 +2963,7 @@ static void virtio_net_tx_bh(void *opaque)
/* If we flush a full burst of packets, assume there are
* more coming and immediately reschedule */
if (ret >= n->tx_burst) {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
return;
}
@@ -2976,7 +2977,7 @@ static void virtio_net_tx_bh(void *opaque)
return;
} else if (ret > 0) {
virtio_queue_set_notification(q->tx_vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
}
}
Re: [PATCH v4 11/24] net: Use virtual time for net announce
On 12.03.2024 14:05, Nicholas Piggin wrote:
On Tue Mar 12, 2024 at 7:09 PM AEST, Pavel Dovgalyuk wrote:
This won't work, as needed. Announce timer can't be enabled, because
it is set in post_load function. Therefore announce callbacks break
the replay, when virtio-net is used with snapshots.
I see. Is that somehow marked as being incompatible with rr?
Here's the prior discussion on it:
https://lore.kernel.org/qemu-devel/8735ovx0zd@linaro.org/t/
Thanks,
Nick
On 11.03.2024 20:40, Nicholas Piggin wrote:
Using virtual time for announce ensures that guest visible effects
are deterministic and don't break replay.
Signed-off-by: Nicholas Piggin
---
net/announce.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/announce.c b/net/announce.c
index 9e99044422..70b5d5e822 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -187,7 +187,7 @@ static void qemu_announce_self_once(void *opaque)
void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
{
-qemu_announce_timer_reset(timer, params, QEMU_CLOCK_REALTIME,
+qemu_announce_timer_reset(timer, params, QEMU_CLOCK_VIRTUAL,
qemu_announce_self_once, timer);
if (params->rounds) {
qemu_announce_self_once(timer);
Re: [PATCH v4 11/24] net: Use virtual time for net announce
This won't work, as needed. Announce timer can't be enabled, because
it is set in post_load function. Therefore announce callbacks break
the replay, when virtio-net is used with snapshots.
On 11.03.2024 20:40, Nicholas Piggin wrote:
Using virtual time for announce ensures that guest visible effects
are deterministic and don't break replay.
Signed-off-by: Nicholas Piggin
---
net/announce.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/announce.c b/net/announce.c
index 9e99044422..70b5d5e822 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -187,7 +187,7 @@ static void qemu_announce_self_once(void *opaque)
void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
{
-qemu_announce_timer_reset(timer, params, QEMU_CLOCK_REALTIME,
+qemu_announce_timer_reset(timer, params, QEMU_CLOCK_VIRTUAL,
qemu_announce_self_once, timer);
if (params->rounds) {
qemu_announce_self_once(timer);
Re: [PATCH v4 20/24] replay: simple auto-snapshot mode for record
On 11.03.2024 20:40, Nicholas Piggin wrote:
record makes an initial snapshot when the machine is created, to enable
reverse-debugging. Often the issue being debugged appears near the end of
the trace, so it is important for performance to keep snapshots close to
the end.
This implements a periodic snapshot mode that keeps a rolling set of
recent snapshots. This could be done by the debugger or other program
that talks QMP, but for setting up simple scenarios and tests, this is
more convenient.
Signed-off-by: Nicholas Piggin
---
docs/system/replay.rst | 5
include/sysemu/replay.h | 11
replay/replay-snapshot.c | 57
replay/replay.c | 27 +--
system/vl.c | 9 +++
qemu-options.hx | 9 +--
6 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/docs/system/replay.rst b/docs/system/replay.rst
index ca7c17c63d..1ae8614475 100644
--- a/docs/system/replay.rst
+++ b/docs/system/replay.rst
@@ -156,6 +156,11 @@ for storing VM snapshots. Here is the example of the
command line for this:
``empty.qcow2`` drive does not connected to any virtual block device and used
for VM snapshots only.
+``rrsnapmode`` can be used to select just an initial snapshot or periodic
+snapshots, with ``rrsnapcount`` specifying the number of periodic snapshots
+to maintain, and ``rrsnaptime`` the amount of run time in seconds between
+periodic snapshots.
+
.. _network-label:
Network devices
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 8102fa54f0..92fa82842b 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -48,6 +48,17 @@ typedef enum ReplayCheckpoint ReplayCheckpoint;
typedef struct ReplayNetState ReplayNetState;
+enum ReplaySnapshotMode {
+REPLAY_SNAPSHOT_MODE_INITIAL,
+REPLAY_SNAPSHOT_MODE_PERIODIC,
+};
This should be defined in replay-internal.h, because it is internal for
replay.
+typedef enum ReplaySnapshotMode ReplaySnapshotMode;
+
+extern ReplaySnapshotMode replay_snapshot_mode;
+
+extern uint64_t replay_snapshot_periodic_delay;
+extern int replay_snapshot_periodic_nr_keep;
These ones are internal too.
+
/* Name of the initial VM snapshot */
extern char *replay_snapshot;
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index ccb4d89dda..762555feaa 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -70,6 +70,53 @@ void replay_vmstate_register(void)
vmstate_register(NULL, 0, _replay, _state);
}
+static QEMUTimer *replay_snapshot_timer;
+static int replay_snapshot_count;
+
+static void replay_snapshot_timer_cb(void *opaque)
+{
+Error *err = NULL;
+char *name;
+
+if (!replay_can_snapshot()) {
+/* Try again soon */
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay / 10);
+return;
+}
+
+name = g_strdup_printf("%s-%d", replay_snapshot, replay_snapshot_count);
+if (!save_snapshot(name,
+ true, NULL, false, NULL, )) {
+error_report_err(err);
+error_report("Could not create periodic snapshot "
+ "for icount record, disabling");
+g_free(name);
+return;
+}
+g_free(name);
+replay_snapshot_count++;
+
+if (replay_snapshot_periodic_nr_keep >= 1 &&
+replay_snapshot_count > replay_snapshot_periodic_nr_keep) {
+int del_nr;
+
+del_nr = replay_snapshot_count - replay_snapshot_periodic_nr_keep - 1;
+name = g_strdup_printf("%s-%d", replay_snapshot, del_nr);
Copy-paste of snapshot name format.
+if (!delete_snapshot(name, false, NULL, )) {
+error_report_err(err);
+error_report("Could not delete periodic snapshot "
+ "for icount record");
+}
+g_free(name);
+}
+
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
+}
+
void replay_vmstate_init(void)
{
Error *err = NULL;
@@ -82,6 +129,16 @@ void replay_vmstate_init(void)
error_report("Could not create snapshot for icount record");
exit(1);
}
+
+if (replay_snapshot_mode == REPLAY_SNAPSHOT_MODE_PERIODIC) {
+replay_snapshot_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+ replay_snapshot_timer_cb,
+ NULL);
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
+}
+
} else if (replay_mode == REPLAY_MODE_PLAY) {
if (!load_snapshot(replay_snapshot, NULL, false, NULL, )) {
Re: [Spam][PATCH] replay: Improve error messages about configuration conflicts
Reviewed-by: Pavel Dovgalyuk
On 01.03.2024 15:06, Markus Armbruster wrote:
Improve
Record/replay feature is not supported for '-rtc base=localtime'
Record/replay feature is not supported for 'smp'
Record/replay feature is not supported for '-snapshot'
to
Record/replay is not supported with -rtc base=localtime
Record/replay is not supported with multiple CPUs
Record/replay is not supported with -snapshot
Signed-off-by: Markus Armbruster
---
replay/replay.c | 2 +-
system/vl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/replay/replay.c b/replay/replay.c
index 3fd241a4fc..a2c576c16e 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -511,7 +511,7 @@ void replay_add_blocker(const char *feature)
{
Error *reason = NULL;
-error_setg(, "Record/replay feature is not supported for '%s'",
+error_setg(, "Record/replay is not supported with %s",
feature);
replay_blockers = g_slist_prepend(replay_blockers, reason);
}
diff --git a/system/vl.c b/system/vl.c
index e480afd7a0..cc03a17c09 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1932,7 +1932,7 @@ static void qemu_apply_machine_options(QDict *qdict)
}
if (current_machine->smp.cpus > 1) {
-replay_add_blocker("smp");
+replay_add_blocker("multiple CPUs");
}
}
Re: [PATCH 4/4] replay: simple auto-snapshot mode for record
On 26.02.2024 10:36, Nicholas Piggin wrote:
On Fri Aug 18, 2023 at 2:36 PM AEST, Pavel Dovgalyuk wrote:
On 14.08.2023 19:31, Nicholas Piggin wrote:
record makes an initial snapshot when the machine is created, to enable
reverse-debugging. Often the issue being debugged appears near the end of
the trace, so it is important for performance to keep snapshots close to
the end.
This implements a periodic snapshot mode that keeps a rolling set of
recent snapshots.
Arguably this should be done by the debugger or a program that talks to
QMP, but for setting up simple scenarios and tests, it is convenient to
have this feature.
I'm looking at resurrecting this to help add a bit of testing...
[snip]
+static void replay_snapshot_timer_cb(void *opaque)
+{
+Error *err = NULL;
+char *name;
+
+if (!replay_can_snapshot()) {
+/* Try again soon */
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay / 10);
+return;
+}
+
+name = g_strdup_printf("%s-%d", replay_snapshot, replay_snapshot_count);
+if (!save_snapshot(name,
+ true, NULL, false, NULL, )) {
+error_report_err(err);
+error_report("Could not create periodic snapshot "
+ "for icount record, disabling");
+g_free(name);
+return;
+}
+g_free(name);
+replay_snapshot_count++;
+
+if (replay_snapshot_periodic_nr_keep >= 1 &&
+replay_snapshot_count > replay_snapshot_periodic_nr_keep) {
+int del_nr;
+
+del_nr = replay_snapshot_count - replay_snapshot_periodic_nr_keep - 1;
+name = g_strdup_printf("%s-%d", replay_snapshot, del_nr);
+if (!delete_snapshot(name, false, NULL, )) {
+error_report_err(err);
+error_report("Could not delete periodic snapshot "
+ "for icount record");
+}
+g_free(name);
+}
+
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
I'm not sure that realtime is not the best choice for such of a timer.
Virtual machine may be stopped or slowed down for some reason.
My thinking was that, say if you snapshot every 10 seconds of real time
executed, then you should have an upper limit on the order of 10 seconds
to perform a reverse-debug operation (so long as you don't exceed your
nr_keep limit).
But in some cases savevm itself could take more than 10 seconds.
We'll have infinite saving in this case. That's why I propose using
virtual clock with the QEMU_TIMER_ATTR_EXTERNAL attribute.
Is it worth worrying about complexity of slowdowns and vm pausing?
Maybe it could stop snapshotting on a host pause.
+}
+
void replay_vmstate_init(void)
{
Error *err = NULL;
@@ -81,6 +128,16 @@ void replay_vmstate_init(void)
error_report("Could not create snapshot for icount record");
exit(1);
}
+
+if (replay_snapshot_mode == REPLAY_SNAPSHOT_MODE_PERIODIC) {
+replay_snapshot_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+ replay_snapshot_timer_cb,
+ NULL);
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+ replay_snapshot_periodic_delay);
+}
+
Please also delete placeholder comment for the snapshotting timer
in replay_enable function.
Wil do.
} else if (replay_mode == REPLAY_MODE_PLAY) {
if (!load_snapshot(replay_snapshot, NULL, false, NULL, )) {
error_report_err(err);
diff --git a/replay/replay.c b/replay/replay.c
index e64f71209a..fa5930700d 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -29,6 +29,10 @@
ReplayMode replay_mode = REPLAY_MODE_NONE;
char *replay_snapshot;
+ReplaySnapshotMode replay_snapshot_mode;
+uint64_t replay_snapshot_periodic_delay;
+int replay_snapshot_periodic_nr_keep;
+
/* Name of replay file */
static char *replay_filename;
ReplayState replay_state;
@@ -313,6 +317,27 @@ void replay_configure(QemuOpts *opts)
}
replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
+if (replay_snapshot && mode == REPLAY_MODE_RECORD) {
Can such a snapshotting may be useful in replay mode?
Does snapshotting do anything in replay mode?
Yes, you can create as many snapshots as you want if 'snapshot=on'
option of the disk image was not used.
I assume if we did
snapshotting based on the machine timer then we'd have to support
it here so the timer events get replayed properly, at least. But
I was trying to get by with minimum complexity :)
Re: [PATCH v1 02/21] tests: correct typos
Reviewed-by: Pavel Dovgalyuk
On 20.02.2024 11:52, Manos Pitsidianakis wrote:
Correct typos automatically found with the `typos` tool
<https://crates.io/crates/typos>
Signed-off-by: Manos Pitsidianakis
---
tests/avocado/acpi-bits/bits-tests/smbios.py2 | 2 +-
tests/avocado/mem-addr-space-check.py | 6 +++---
tests/avocado/reverse_debugging.py| 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/avocado/acpi-bits/bits-tests/smbios.py2
b/tests/avocado/acpi-bits/bits-tests/smbios.py2
index fc623de072..5868a7137a 100644
--- a/tests/avocado/acpi-bits/bits-tests/smbios.py2
+++ b/tests/avocado/acpi-bits/bits-tests/smbios.py2
@@ -1060,7 +1060,7 @@ class EventLogDescriptor(unpack.Struct):
0x16: 'Log Area Reset/Cleared',
0x17: 'System boot',
xrange(0x18, 0x7F): 'Unused, available for assignment',
-xrange(0x80, 0xFE): 'Availalbe for system- and OEM-specific
assignments',
+xrange(0x80, 0xFE): 'Available for system- and OEM-specific
assignments',
0xFF: 'End of log'
}
yield 'log_type', u.unpack_one('B'), unpack.format_table("{}",
_event_log_type_descriptors)
diff --git a/tests/avocado/mem-addr-space-check.py
b/tests/avocado/mem-addr-space-check.py
index 363c3f12a6..af019969c0 100644
--- a/tests/avocado/mem-addr-space-check.py
+++ b/tests/avocado/mem-addr-space-check.py
@@ -165,7 +165,7 @@ def test_phybits_low_tcg_q35_70_amd(self):
For q35-7.0 machines, "above 4G" memory starts are 4G.
pci64_hole size is 32 GiB. Since TCG_PHYS_ADDR_BITS is defined to
be 40, TCG emulated CPUs have maximum of 1 TiB (1024 GiB) of
-directly addressible memory.
+directly addressable memory.
Hence, maxmem value at most can be
1024 GiB - 4 GiB - 1 GiB per slot for alignment - 32 GiB + 0.5 GiB
which is equal to 987.5 GiB. Setting the value to 988 GiB should
@@ -190,7 +190,7 @@ def test_phybits_low_tcg_q35_71_amd(self):
AMD_HT_START is defined to be at 1012 GiB. So for q35 machines
version > 7.0 and AMD cpus, instead of 1024 GiB limit for 40 bit
processor address space, it has to be 1012 GiB , that is 12 GiB
-less than the case above in order to accomodate HT hole.
+less than the case above in order to accommodate HT hole.
Make sure QEMU fails when maxmem size is 976 GiB (12 GiB less
than 988 GiB).
"""
@@ -297,7 +297,7 @@ def test_phybits_ok_tcg_q35_71_amd_41bits(self):
:avocado: tags=arch:x86_64
AMD processor with 41 bits. Max cpu hw address = 2 TiB.
-Same as above but by setting maxram beween 976 GiB and 992 Gib,
+Same as above but by setting maxram between 976 GiB and 992 Gib,
QEMU should start fine.
"""
self.vm.add_args('-S', '-cpu', 'EPYC-v4,phys-bits=41',
diff --git a/tests/avocado/reverse_debugging.py
b/tests/avocado/reverse_debugging.py
index 4cce5a5598..92855a02a5 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -191,7 +191,7 @@ def reverse_debugging(self, shift=7, args=None):
self.check_pc(g, steps[-1])
logger.info('successfully reached %x' % steps[-1])
-logger.info('exitting gdb and qemu')
+logger.info('exiting gdb and qemu')
vm.shutdown()
class ReverseDebugging_X86_64(ReverseDebugging):
Re: [PATCH v2 4/4] tests/avocado: excercise scripts/replay-dump.py in replay tests
Reviewed-by: Pavel Dovgalyuk
On 25.01.2024 19:08, Nicholas Piggin wrote:
This runs replay-dump.py after recording a trace, and fails the test if
the script fails.
replay-dump.py is modified to exit with non-zero if an error is
encountered while parsing, to support this.
Signed-off-by: Nicholas Piggin
---
scripts/replay-dump.py | 6 --
tests/avocado/replay_kernel.py | 16
tests/avocado/replay_linux.py | 16
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py
index a1d7ae0364..bfea9c099b 100755
--- a/scripts/replay-dump.py
+++ b/scripts/replay-dump.py
@@ -21,6 +21,7 @@
import argparse
import struct
import os
+import sys
from collections import namedtuple
from os import path
@@ -104,7 +105,7 @@ def call_decode(table, index, dumpfile):
print("Could not decode index: %d" % (index))
print("Entry is: %s" % (decoder))
print("Decode Table is:\n%s" % (table))
-return False
+raise(Exception("unknown event"))
else:
return decoder.fn(decoder.eid, decoder.name, dumpfile)
@@ -125,7 +126,7 @@ def print_event(eid, name, string=None, event_count=None):
def decode_unimp(eid, name, _unused_dumpfile):
"Unimplemented decoder, will trigger exit"
print("%s not handled - will now stop" % (name))
-return False
+raise(Exception("unhandled event"))
def decode_plain(eid, name, _unused_dumpfile):
"Plain events without additional data"
@@ -439,6 +440,7 @@ def decode_file(filename):
dumpfile)
except Exception as inst:
print(f"error {inst}")
+sys.exit(1)
finally:
print(f"Reached {dumpfile.tell()} of {dumpsize} bytes")
diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 10d99403a4..9b3ee6726b 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -13,6 +13,7 @@
import shutil
import logging
import time
+import subprocess
from avocado import skip
from avocado import skipUnless
@@ -22,6 +23,11 @@
from avocado.utils import process
from boot_linux_console import LinuxKernelTest
+from pathlib import Path
+
+self_dir = Path(__file__).parent
+src_dir = self_dir.parent.parent
+
class ReplayKernelBase(LinuxKernelTest):
"""
Boots a Linux kernel in record mode and checks that the console
@@ -63,6 +69,8 @@ def run_vm(self, kernel_path, kernel_command_line,
console_pattern,
vm.shutdown()
logger.info('finished the recording with log size %s bytes'
% os.path.getsize(replay_path))
+self.run_replay_dump(replay_path)
+logger.info('successfully tested replay-dump.py')
else:
vm.wait()
logger.info('successfully finished the replay')
@@ -70,6 +78,14 @@ def run_vm(self, kernel_path, kernel_command_line,
console_pattern,
logger.info('elapsed time %.2f sec' % elapsed)
return elapsed
+def run_replay_dump(self, replay_path):
+try:
+subprocess.check_call(["./scripts/replay-dump.py",
+ "-f", replay_path],
+ cwd=src_dir, stdout=subprocess.DEVNULL)
+except subprocess.CalledProcessError:
+self.fail('replay-dump.py failed')
+
def run_rr(self, kernel_path, kernel_command_line, console_pattern,
shift=7, args=None):
replay_path = os.path.join(self.workdir, 'replay.bin')
diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index f3a43dc98c..dd148ff639 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -11,6 +11,7 @@
import os
import logging
import time
+import subprocess
from avocado import skipUnless
from avocado_qemu import BUILD_DIR
@@ -21,6 +22,11 @@
from avocado.utils.path import find_command
from avocado_qemu import LinuxTest
+from pathlib import Path
+
+self_dir = Path(__file__).parent
+src_dir = self_dir.parent.parent
+
class ReplayLinux(LinuxTest):
"""
Boots a Linux system, checking for a successful initialization
@@ -94,6 +100,8 @@ def launch_and_wait(self, record, args, shift):
vm.shutdown()
logger.info('finished the recording with log size %s bytes'
% os.path.getsize(replay_path))
+self.run_replay_dump(replay_path)
+logger.info('successfully tested replay-dump.py')
else:
vm.event_wait('SHUTDOWN', self.timeout)
vm.wait()
@@ -108,6 +116,14 @@ def run_rr(self, args=None, shift=7):
logger = logging.getLogger('replay')
logge
Re: [PATCH v2 1/4] replay: allow runstate shutdown->running when replaying trace
Reviewed-by: Pavel Dovgalyuk
On 25.01.2024 19:08, Nicholas Piggin wrote:
When replaying a trace, it is possible to go from shutdown to
running with a reverse-debugging step. This can be useful if the
problem being debugged triggers a reset or shutdown.
Signed-off-by: Nicholas Piggin
---
include/sysemu/runstate.h | 1 +
replay/replay.c | 2 ++
system/runstate.c | 19 +++
3 files changed, 22 insertions(+)
diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
index 0117d243c4..fe25eed3c0 100644
--- a/include/sysemu/runstate.h
+++ b/include/sysemu/runstate.h
@@ -9,6 +9,7 @@ void runstate_set(RunState new_state);
RunState runstate_get(void);
bool runstate_is_running(void);
bool runstate_needs_reset(void);
+void runstate_replay_enable(void);
typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
diff --git a/replay/replay.c b/replay/replay.c
index 3fd241a4fc..2951eed3bd 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -383,6 +383,8 @@ static void replay_enable(const char *fname, int mode)
/* go to the beginning */
fseek(replay_file, HEADER_SIZE, SEEK_SET);
replay_fetch_data_kind();
+
+runstate_replay_enable();
}
replay_init_events();
diff --git a/system/runstate.c b/system/runstate.c
index d6ab860eca..bd0fed8657 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -182,6 +182,12 @@ static const RunStateTransition runstate_transitions_def[]
= {
{ RUN_STATE__MAX, RUN_STATE__MAX },
};
+static const RunStateTransition replay_runstate_transitions_def[] = {
+{ RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING},
+
+{ RUN_STATE__MAX, RUN_STATE__MAX },
+};
+
static bool runstate_valid_transitions[RUN_STATE__MAX][RUN_STATE__MAX];
bool runstate_check(RunState state)
@@ -189,6 +195,19 @@ bool runstate_check(RunState state)
return current_run_state == state;
}
+void runstate_replay_enable(void)
+{
+const RunStateTransition *p;
+
+assert(replay_mode == REPLAY_MODE_PLAY);
+
+for (p = _runstate_transitions_def[0]; p->from != RUN_STATE__MAX;
+ p++) {
+runstate_valid_transitions[p->from][p->to] = true;
+}
+
+}
+
static void runstate_init(void)
{
const RunStateTransition *p;
Re: [PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/
Reviewed-by: Pavel Dovgalyuk
On 24.01.2024 13:16, Philippe Mathieu-Daudé wrote:
Move this x86-specific code out of the generic accel/tcg/.
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/tcg/helper-tcg.h| 1 +
accel/tcg/cpu-exec.c| 9 -
target/i386/tcg/sysemu/seg_helper.c | 10 ++
target/i386/tcg/tcg-cpu.c | 1 +
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index ce34b737bb..253b1f561e 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS >
TARGET_PHYS_ADDR_SPACE_BITS);
*/
void x86_cpu_do_interrupt(CPUState *cpu);
#ifndef CONFIG_USER_ONLY
+bool x86_need_replay_interrupt(int interrupt_request);
bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
#endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 4ab7d6c896..5a978a9e72 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -768,21 +768,12 @@ static inline bool cpu_handle_exception(CPUClass *cc,
CPUState *cpu, int *ret)
}
#ifndef CONFIG_USER_ONLY
-/*
- * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
- * "real" interrupt event later. It does not need to be recorded for
- * replay purposes.
- */
static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
{
-#if defined(TARGET_I386)
-return !(interrupt_request & CPU_INTERRUPT_POLL);
-#else
if (!cc->tcg_ops->need_replay_interrupt) {
return true;
}
return cc->tcg_ops->need_replay_interrupt(interrupt_request);
-#endif
}
#endif /* !CONFIG_USER_ONLY */
diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
index 1cb5a0db45..e6f42282bb 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -127,6 +127,16 @@ void x86_cpu_do_interrupt(CPUState *cs)
}
}
+bool x86_need_replay_interrupt(int interrupt_request)
+{
+/*
+ * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
+ * "real" interrupt event later. It does not need to be recorded for
+ * replay purposes.
+ */
+return !(interrupt_request & CPU_INTERRUPT_POLL);
+}
+
bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
X86CPU *cpu = X86_CPU(cs);
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index e1405b7be9..255d56d4c3 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -123,6 +123,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
.do_unaligned_access = x86_cpu_do_unaligned_access,
.debug_excp_handler = breakpoint_handler,
.debug_check_breakpoint = x86_debug_check_breakpoint,
+.need_replay_interrupt = x86_need_replay_interrupt,
#endif /* !CONFIG_USER_ONLY */
};
Re: [PATCH 6/9] accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler
Reviewed-by: Pavel Dovgalyuk
On 24.01.2024 13:16, Philippe Mathieu-Daudé wrote:
In order to make accel/tcg/ target agnostic,
introduce the need_replay_interrupt() handler.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/core/tcg-cpu-ops.h | 5 +
accel/tcg/cpu-exec.c | 5 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index 479713a36e..2fae3ac70f 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -170,6 +170,11 @@ struct TCGCPUOps {
*/
bool (*io_recompile_replay_branch)(CPUState *cpu,
const TranslationBlock *tb);
+/**
+ * @need_replay_interrupt: Return %true if @interrupt_request
+ * needs to be recorded for replay purposes.
+ */
+bool (*need_replay_interrupt)(int interrupt_request);
#endif /* !CONFIG_USER_ONLY */
#endif /* NEED_CPU_H */
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index b10472cbc7..4ab7d6c896 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -778,7 +778,10 @@ static inline bool need_replay_interrupt(CPUClass *cc, int
interrupt_request)
#if defined(TARGET_I386)
return !(interrupt_request & CPU_INTERRUPT_POLL);
#else
-return true;
+if (!cc->tcg_ops->need_replay_interrupt) {
+return true;
+}
+return cc->tcg_ops->need_replay_interrupt(interrupt_request);
#endif
}
#endif /* !CONFIG_USER_ONLY */
Re: [PATCH 09/11] replay: stop us hanging in rr_wait_io_event
On 05.12.2023 23:41, Alex Bennée wrote:
A lot of the hang I see are when we end up spinning in
rr_wait_io_event for an event that will never come in playback. As a
new check functions which can see if we are in PLAY mode and kick us
us the wait function so the event can be processed.
This fixes most of the failures in replay_kernel.py
Is there an effect for console QEMU only?
I've tested this patch on Windows7 boot scenario and replay speed has
not changed.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2013
Signed-off-by: Alex Bennée
Cc: Pavel Dovgalyuk
---
include/sysemu/replay.h | 5 +
accel/tcg/tcg-accel-ops-rr.c | 2 +-
replay/replay.c | 24
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 08aae5869f..83995ae4bd 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -70,6 +70,11 @@ int replay_get_instructions(void);
/*! Updates instructions counter in replay mode. */
void replay_account_executed_instructions(void);
+/**
+ * replay_can_wait: check if we should pause for wait-io
+ */
+bool replay_can_wait(void);
+
/* Processing clocks and other time sources */
/*! Save the specified clock */
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 611932f3c3..825e35b3dc 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -109,7 +109,7 @@ static void rr_wait_io_event(void)
{
CPUState *cpu;
-while (all_cpu_threads_idle()) {
+while (all_cpu_threads_idle() && replay_can_wait()) {
rr_stop_kick_timer();
qemu_cond_wait_iothread(first_cpu->halt_cond);
}
diff --git a/replay/replay.c b/replay/replay.c
index e83c01285c..042a6a9636 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -347,6 +347,30 @@ void replay_start(void)
replay_enable_events();
}
+/*
+ * For none/record the answer is yes.
+ */
+bool replay_can_wait(void)
+{
+if (replay_mode == REPLAY_MODE_PLAY) {
+/*
+ * For playback we shouldn't ever be at a point we wait. If
+ * the instruction count has reached zero and we have an
+ * unconsumed event we should go around again and consume it.
+ */
+if (replay_state.instruction_count == 0 &&
replay_state.has_unread_data) {
+return false;
+} else {
+fprintf(stderr, "Error: Invalid replay state\n");
+fprintf(stderr,"instruction_count = %d, has = %d, event_kind =
%d\n",
+replay_state.instruction_count,
replay_state.has_unread_data, replay_state.data_kind);
+abort();
+}
+}
+return true;
+}
+
+
void replay_finish(void)
{
if (replay_mode == REPLAY_MODE_NONE) {
Re: [PATCH 11/11] tests/avocado: remove skips from replay_kernel
On 05.12.2023 23:41, Alex Bennée wrote:
With the latest fixes for #2010 and #2013 these tests look pretty
stable now. Of course the only way to be really sure is to run it in
the CI infrastructure and see what breaks.
Signed-off-by: Alex Bennée
---
tests/avocado/replay_kernel.py | 9 -
1 file changed, 9 deletions(-)
diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 1eaa36444c..c54e96c9ff 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -98,8 +98,6 @@ def test_i386_pc(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5)
-# See https://gitlab.com/qemu-project/qemu/-/issues/2010
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test sometimes gets
stuck')
def test_x86_64_pc(self):
"""
:avocado: tags=arch:x86_64
@@ -135,8 +133,6 @@ def test_mips_malta(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5)
-# See https://gitlab.com/qemu-project/qemu/-/issues/2013
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
GitLab')
def test_mips64el_malta(self):
"""
This test requires the ar tool to extract "data.tar.gz" from
@@ -152,7 +148,6 @@ def test_mips64el_malta(self):
:avocado: tags=arch:mips64el
:avocado: tags=machine:malta
-:avocado: tags=flaky
"""
deb_url = ('http://snapshot.debian.org/archive/debian/'
'20130217T032700Z/pool/main/l/linux-2.6/'
@@ -200,8 +195,6 @@ def test_arm_virt(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=1)
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
-
def test_arm_cubieboard_initrd(self):
"""
:avocado: tags=arch:arm
@@ -354,7 +347,6 @@ def test_m68k_mcf5208evb(self):
file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
self.do_test_advcal_2018(file_path, 'sanity-clause.elf')
-@skip("Test currently broken") # Console stuck as of 5.2-rc1
def test_microblaze_s3adsp1800(self):
"""
:avocado: tags=arch:microblaze
@@ -389,7 +381,6 @@ def test_or1k_sim(self):
file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
self.do_test_advcal_2018(file_path, 'vmlinux')
-@skip("nios2 emulation is buggy under record/replay")
def test_nios2_10m50(self):
"""
:avocado: tags=arch:nios2
Acked-by: Pavel Dovgalyuk
Re: [PATCH 08/11] replay: introduce a central report point for sync errors
On 06.12.2023 19:48, Richard Henderson wrote:
On 12/6/23 03:35, Philippe Mathieu-Daudé wrote:
Hi Alex,
On 5/12/23 21:41, Alex Bennée wrote:
Figuring out why replay has failed is tricky at the best of times.
Lets centralise the reporting of a replay sync error and add a little
bit of extra information to help with debugging.
Signed-off-by: Alex Bennée
---
replay/replay-internal.h | 12
replay/replay-char.c | 6 ++
replay/replay-internal.c | 1 +
replay/replay.c | 9 +
4 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 1bc8fd5086..709e2eb4cb 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -74,6 +74,7 @@ enum ReplayEvents {
* @cached_clock: Cached clocks values
* @current_icount: number of processed instructions
* @instruction_count: number of instructions until next event
+ * @current_event: current event index
* @data_kind: current event
* @has_unread_data: true if event not yet processed
* @file_offset: offset into replay log at replay snapshot
@@ -84,6 +85,7 @@ typedef struct ReplayState {
int64_t cached_clock[REPLAY_CLOCK_COUNT];
uint64_t current_icount;
int instruction_count;
+ unsigned int current_event;
unsigned int data_kind;
bool has_unread_data;
uint64_t file_offset;
Shouldn't this field be migrated?
No, it's for diagnostic use only.
It should be migrated, because RR may be started from the snapshot,
which references the middle of replayed scenario.
Pavel Dovgalyuk
Re: [PATCH 10/11] chardev: force write all when recording replay logs
On 05.12.2023 23:41, Alex Bennée wrote:
This is mostly a problem within avocado as serial generally isn't busy
enough to overfill pipes. However the consequences of recording a
failed write will haunt us on replay when causing the log to go out of
sync.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2010
Signed-off-by: Alex Bennée
Cc: Pavel Dovgalyuk
---
chardev/char.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index 996a024c7a..6e5b4d7345 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -171,7 +171,8 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len,
bool write_all)
return res;
}
-res = qemu_chr_write_buffer(s, buf, len, , write_all);
+res = qemu_chr_write_buffer(s, buf, len, ,
+replay_mode == REPLAY_MODE_RECORD ? true :
write_all);
if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
replay_char_write_event_save(res, offset);
Acked-by: Pavel Dovgalyuk
Re: [PATCH 07/11] replay: make has_unread_data a bool
On 05.12.2023 23:41, Alex Bennée wrote: For clarity given it only has two states. Signed-off-by: Alex Bennée --- replay/replay-internal.h | 4 ++-- replay/replay-internal.c | 4 ++-- replay/replay-snapshot.c | 6 +++--- replay/replay.c | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Pavel Dovgalyuk
Re: [PATCH 06/11] replay: add proper kdoc for ReplayState
On 05.12.2023 23:41, Alex Bennée wrote:
Remove the non-standard comment formatting and move the descriptions
into a proper kdoc comment.
Signed-off-by: Alex Bennée
---
replay/replay-internal.h | 27 --
roms/SLOF | 2 +-
tests/tcg/i386/Makefile.softmmu-target | 19 ++
3 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 516147ddbc..98ca3748ed 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -63,24 +63,31 @@ enum ReplayEvents {
EVENT_COUNT
};
+/**
+ * typedef ReplayState - global tracking Replay state
+ *
+ * This structure tracks where we are in the current ReplayState
+ * including the logged events from the recorded replay stream. Some
+ * of the data is also stored/restored from VMStateDescription when VM
+ * save/restore events take place.
+ *
+ * @cached_clock: Cached clocks values
+ * @current_icount: number of processed instructions
+ * @instruction_count: number of instructions until next event
+ * @data_kind: current event
+ * @has_unread_data: 1 if event not yet processed
+ * @file_offset: offset into replay log at replay snapshot
+ * @block_request_id: current serialised block request id
+ * @read_event_id: current async read event id
+ */
typedef struct ReplayState {
-/*! Cached clock values. */
int64_t cached_clock[REPLAY_CLOCK_COUNT];
-/*! Current icount - number of processed instructions. */
uint64_t current_icount;
-/*! Number of instructions to be executed before other events happen. */
int instruction_count;
-/*! Type of the currently executed event. */
unsigned int data_kind;
-/*! Flag which indicates that event is not processed yet. */
unsigned int has_unread_data;
-/*! Temporary variable for saving current log offset. */
uint64_t file_offset;
-/*! Next block operation id.
-This counter is global, because requests from different
-block devices should not get overlapping ids. */
uint64_t block_request_id;
-/*! Asynchronous event id read from the log */
uint64_t read_event_id;
} ReplayState;
extern ReplayState replay_state;
Reviewed-by: Pavel Dovgalyuk
Re: [PATCH 05/11] replay: remove host_clock_last
On 05.12.2023 23:41, Alex Bennée wrote:
Fixes: a02fe2ca70 (replay: Remove host_clock_last)
Signed-off-by: Alex Bennée
---
replay/replay-internal.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index b6836354ac..516147ddbc 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -80,8 +80,6 @@ typedef struct ReplayState {
This counter is global, because requests from different
block devices should not get overlapping ids. */
uint64_t block_request_id;
-/*! Prior value of the host clock */
-uint64_t host_clock_last;
/*! Asynchronous event id read from the log */
uint64_t read_event_id;
} ReplayState;
Reviewed-by: Pavel Dovgalyuk
Re: [PATCH 04/11] scripts/replay_dump: track total number of instructions
On 05.12.2023 23:40, Alex Bennée wrote: This will help in tracking where we are in the stream when debugging. Signed-off-by: Alex Bennée --- scripts/replay-dump.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py index 8b9f914534..2212b09322 100755 --- a/scripts/replay-dump.py +++ b/scripts/replay-dump.py @@ -150,10 +150,13 @@ def decode_async(eid, name, dumpfile): return call_decode(async_decode_table, async_event_kind, dumpfile) +total_insns = 0 def decode_instruction(eid, name, dumpfile): +global total_insns ins_diff = read_dword(dumpfile) -print_event(eid, name, "0x%x" % (ins_diff)) +total_insns += ins_diff +print_event(eid, name, "+ %d -> %d" % (ins_diff, total_insns)) return True def decode_char_write(eid, name, dumpfile): Reviewed-by: Pavel Dovgalyuk
Re: [PATCH 01/11] tests/avocado: add a simple i386 replay kernel test
On 05.12.2023 23:40, Alex Bennée wrote: There are a number of bugs against 32 bit x86 on the tracker. Lets at least establish a baseline pure kernel boot can do record/replay before we start looking at the devices. Signed-off-by: Alex Bennée --- tests/avocado/replay_kernel.py | 16 1 file changed, 16 insertions(+) Acked-by: Pavel Dovgalyuk
Re: [PATCH 02/11] tests/avocado: fix typo in replay_linux
On 05.12.2023 23:40, Alex Bennée wrote:
Signed-off-by: Alex Bennée
---
tests/avocado/replay_linux.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index 270ccc1eae..e95bff3299 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -94,7 +94,7 @@ def launch_and_wait(self, record, args, shift):
else:
vm.event_wait('SHUTDOWN', self.timeout)
vm.wait()
-logger.info('successfully fihished the replay')
+logger.info('successfully finished the replay')
elapsed = time.time() - start_time
logger.info('elapsed time %.2f sec' % elapsed)
return elapsed
Reviewed-by: Pavel Dovgalyuk
Re: [PATCH v6 06/21] net: Remove flag propagation
On 11.11.2023 17:27, Akihiko Odaki wrote: On 2023/11/10 16:35, Pavel Dovgalyuk wrote: You need to bump REPLAY_VERSION in replay/replay.c, because your patch changes the replay log format. Otherwise, for replay part: Acked-by: Pavel Dovgalyuk I'll drop this change then. It's just a cleanup and does not bring an additional benefit worth breaking the log format. Breaking the log format is ok, because replays may be incompatible for different builds, because peripheral or vCPU behavior may change. So bumping the version just helps with not replaying the wrong log.
Re: [PATCH v6 06/21] net: Remove flag propagation
You need to bump REPLAY_VERSION in replay/replay.c, because your patch
changes the replay log format.
Otherwise, for replay part:
Acked-by: Pavel Dovgalyuk
On 30.10.2023 08:12, Akihiko Odaki wrote:
There is no defined flag now.
Signed-off-by: Akihiko Odaki
---
include/net/filter.h| 3 ---
include/net/queue.h | 6 --
include/sysemu/replay.h | 2 +-
net/dump.c | 4 ++--
net/filter-buffer.c | 4 +---
net/filter-mirror.c | 6 ++
net/filter-replay.c | 3 +--
net/filter-rewriter.c | 5 ++---
net/filter.c| 8 +++-
net/net.c | 43 ++---
replay/replay-net.c | 8 ++--
11 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/include/net/filter.h b/include/net/filter.h
index 27ffc630df..e523771e72 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -26,7 +26,6 @@ typedef void (FilterCleanup) (NetFilterState *nf);
*/
typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
NetClientState *sender,
- unsigned flags,
const struct iovec *iov,
int iovcnt,
NetPacketSent *sent_cb);
@@ -65,14 +64,12 @@ struct NetFilterState {
ssize_t qemu_netfilter_receive(NetFilterState *nf,
NetFilterDirection direction,
NetClientState *sender,
- unsigned flags,
const struct iovec *iov,
int iovcnt,
NetPacketSent *sent_cb);
/* pass the packet to the next filter */
ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
-unsigned flags,
const struct iovec *iov,
int iovcnt,
void *opaque);
diff --git a/include/net/queue.h b/include/net/queue.h
index 7a43863be2..571f4e1436 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -30,15 +30,12 @@ typedef struct NetQueue NetQueue;
typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
-#define QEMU_NET_PACKET_FLAG_NONE 0
-
/* Returns:
* >0 - success
*0 - queue packet for future redelivery
* <0 - failure (discard packet)
*/
typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender,
- unsigned flags,
const struct iovec *iov,
int iovcnt,
void *opaque);
@@ -47,7 +44,6 @@ NetQueue *qemu_new_net_queue(NetQueueDeliverFunc *deliver,
void *opaque);
void qemu_net_queue_append_iov(NetQueue *queue,
NetClientState *sender,
- unsigned flags,
const struct iovec *iov,
int iovcnt,
NetPacketSent *sent_cb);
@@ -64,14 +60,12 @@ ssize_t qemu_net_queue_receive_iov(NetQueue *queue,
ssize_t qemu_net_queue_send(NetQueue *queue,
NetClientState *sender,
-unsigned flags,
const uint8_t *data,
size_t size,
NetPacketSent *sent_cb);
ssize_t qemu_net_queue_send_iov(NetQueue *queue,
NetClientState *sender,
-unsigned flags,
const struct iovec *iov,
int iovcnt,
NetPacketSent *sent_cb);
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 08aae5869f..67b2d3ac73 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -158,7 +158,7 @@ ReplayNetState *replay_register_net(NetFilterState *nfs);
/*! Unregisters replay network filter. */
void replay_unregister_net(ReplayNetState *rns);
/*! Called to write network packet to the replay log. */
-void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
+void replay_net_packet_event(ReplayNetState *rns,
const struct iovec *iov, int iovcnt);
/* Audio */
diff --git a/net/dump.c b/net/dump.c
index 956e34a123..dd8ada9b5e 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -150,8 +150,8 @@ struct NetFilterDumpState {
};
static ssize_t filter_dump_receive_iov(NetFilterState *nf, NetClientState *sndr,
- unsigned flags, const struct iovec *iov,
- int iovcnt, NetPacketSent *sent_cb)
+ const struct iovec *io
[PATCH 0/3] Record/replay patches
From: Pavel Dovgalyuk The set of patches include the following modifications: - fix for allowing record/replay with virtio-net - fix for loading non-replay snapshots Pavel Dovgalyuk (3): replay: improve determinism of virtio-net virtio-net: added replay blocker for guest_announce replay: fix for loading non-replay snapshots hw/net/virtio-net.c | 13 + replay/replay-snapshot.c | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) -- 2.34.1
[PATCH 1/3] replay: improve determinism of virtio-net
From: Pavel Dovgalyuk
virtio-net device uses bottom halves for callbacks.
These callbacks should be deterministic, because they affect VM state.
This patch replaces BH invocations with corresponding replay functions,
making them deterministic in record/replay mode.
Signed-off-by: Pavel Dovgalyuk
---
hw/net/virtio-net.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 5a0201c423..50156e8002 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -46,6 +46,7 @@
#include "net_rx_pkt.h"
#include "hw/virtio/vhost.h"
#include "sysemu/qtest.h"
+#include "sysemu/replay.h"
#define VIRTIO_NET_VM_VERSION11
@@ -417,7 +418,7 @@ static void virtio_net_set_status(struct VirtIODevice
*vdev, uint8_t status)
timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
n->tx_timeout);
} else {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
} else {
if (q->tx_timer) {
@@ -2822,7 +2823,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev,
VirtQueue *vq)
return;
}
virtio_queue_set_notification(vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
static void virtio_net_tx_timer(void *opaque)
@@ -2905,7 +2906,7 @@ static void virtio_net_tx_bh(void *opaque)
/* If we flush a full burst of packets, assume there are
* more coming and immediately reschedule */
if (ret >= n->tx_burst) {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
return;
}
@@ -2919,7 +2920,7 @@ static void virtio_net_tx_bh(void *opaque)
return;
} else if (ret > 0) {
virtio_queue_set_notification(q->tx_vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
}
}
--
2.34.1
[PATCH 3/3] replay: fix for loading non-replay snapshots
From: Pavel Dovgalyuk
Snapshots created in regular icount execution mode can't be loaded
in recording mode, because icount value advances only by 32-bit value.
This patch initializes replay icount initial value after loading
the snapshot.
Cc: Pizarro Solar Rafael Ulises Luzius
Signed-off-by: Pavel Dovgalyuk
---
replay/replay-snapshot.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 10a7cf7992..1e32ada1f6 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -75,6 +75,7 @@ void replay_vmstate_init(void)
if (replay_snapshot) {
if (replay_mode == REPLAY_MODE_RECORD) {
+replay_state.current_icount = replay_get_current_icount();
if (!save_snapshot(replay_snapshot,
true, NULL, false, NULL, )) {
error_report_err(err);
--
2.34.1
[PATCH 2/3] virtio-net: added replay blocker for guest_announce
From: Pavel Dovgalyuk
This patch blocks record/replay when guest_announce is enabled,
because this flag breaks loading the snapshots in deterministic
execution mode.
Signed-off-by: Pavel Dovgalyuk
---
hw/net/virtio-net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 50156e8002..eb50b7e030 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3603,6 +3603,10 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
}
+if (n->host_features & (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE)) {
+replay_add_blocker("-device virtio-net-device,guest_announce=true");
+}
+
if (n->net_conf.duplex_str) {
if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
n->net_conf.duplex = DUPLEX_HALF;
--
2.34.1
Re: [RFC v2 PATCH] record-replay: support SMP target machine
On 11.08.2023 04:47, Nicholas Piggin wrote:
RR CPU switching is driven by timers and events so it is deterministic
like everything else. Record a CPU switch event and use that to drive
the CPU switch on replay.
Signed-off-by: Nicholas Piggin
---
This is still in RFC phase because so far I've only really testd ppc
pseries, and only with patches that are not yet upstream (but posted
to list).
It works with smp 2, can step, reverse-step, reverse-continue, etc.
throughout a Linux boot.
I still didn't have time to test it, but here are some comments.
One issue is reverse-step on one gdb thread (vCPU) only steps back one
icount, so if another thread is currently running then it is that one
which goes back one instruction and the selected thread doesn't move. I
would call this a separate issue from the record-replay mechanism, which
is in the replay-debugging policy. I think we could record in each vCPU
an icount of the last instruction it executed before switching, then
reverse step for that vCPU could replay to there. I think that's not so
important yet until this mechanism is solid. But if you test and rsi is
not going backwards, then check your other threads.
Thanks,
Nick
accel/tcg/tcg-accel-ops-icount.c | 9 +++-
accel/tcg/tcg-accel-ops-rr.c | 73 +---
include/exec/replay-core.h | 3 ++
replay/replay-internal.h | 1 +
replay/replay.c | 34 ++-
scripts/replay-dump.py | 5 +++
softmmu/vl.c | 4 --
7 files changed, 115 insertions(+), 14 deletions(-)
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c
index 3d2cfbbc97..c26782a56a 100644
--- a/accel/tcg/tcg-accel-ops-icount.c
+++ b/accel/tcg/tcg-accel-ops-icount.c
@@ -93,10 +93,15 @@ void icount_handle_deadline(void)
int64_t icount_percpu_budget(int cpu_count)
{
int64_t limit = icount_get_limit();
-int64_t timeslice = limit / cpu_count;
+int64_t timeslice;
-if (timeslice == 0) {
+if (replay_mode == REPLAY_MODE_PLAY) {
timeslice = limit;
+} else {
+timeslice = limit / cpu_count;
+if (timeslice == 0) {
+timeslice = limit;
+}
}
return timeslice;
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 212d6f8df4..ce040a687e 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -27,6 +27,7 @@
#include "qemu/lockable.h"
#include "sysemu/tcg.h"
#include "sysemu/replay.h"
+#include "sysemu/reset.h"
#include "sysemu/cpu-timers.h"
#include "qemu/main-loop.h"
#include "qemu/notify.h"
@@ -61,6 +62,22 @@ void rr_kick_vcpu_thread(CPUState *unused)
static QEMUTimer *rr_kick_vcpu_timer;
static CPUState *rr_current_cpu;
+static CPUState *rr_next_cpu;
+static CPUState *rr_last_cpu;
+
+/*
+ * Reset the vCPU scheduler to the initial state.
+ */
+static void record_replay_reset(void *param)
+{
+if (rr_kick_vcpu_timer) {
+timer_del(rr_kick_vcpu_timer);
+}
+g_assert(!rr_current_cpu);
+rr_next_cpu = NULL;
+rr_last_cpu = first_cpu;
+current_cpu = NULL;
+}
static inline int64_t rr_next_kick_time(void)
{
@@ -184,6 +201,8 @@ static void *rr_cpu_thread_fn(void *arg)
Notifier force_rcu;
CPUState *cpu = arg;
+qemu_register_reset(record_replay_reset, NULL);
+
assert(tcg_enabled());
rcu_register_thread();
force_rcu.notify = rr_force_rcu;
@@ -238,14 +257,20 @@ static void *rr_cpu_thread_fn(void *arg)
cpu_budget = icount_percpu_budget(cpu_count);
}
+if (!rr_next_cpu) {
+qatomic_set_mb(_next_cpu, first_cpu);
+}
+cpu = rr_next_cpu;
+
+if (cpu != rr_last_cpu) {
+replay_switch_cpu();
+qatomic_set_mb(_last_cpu, cpu);
+}
+
rr_start_kick_timer();
replay_mutex_unlock();
-if (!cpu) {
-cpu = first_cpu;
-}
-
while (cpu && cpu_work_list_empty(cpu) && !cpu->exit_request) {
/* Store rr_current_cpu before evaluating cpu_can_run(). */
qatomic_set_mb(_current_cpu, cpu);
@@ -284,7 +309,34 @@ static void *rr_cpu_thread_fn(void *arg)
break;
}
-cpu = CPU_NEXT(cpu);
+if (replay_mode == REPLAY_MODE_NONE) {
+cpu = CPU_NEXT(cpu);
+} else if (replay_mode == REPLAY_MODE_RECORD) {
+/*
+ * Exit the loop immediately so CPU switch events can be
+ * recorded. This may be able to be improved to record
+ * switch events here.
+ */
+cpu = CPU_NEXT(cpu);
+break;
+} else if (replay_mode == REPLAY_MODE_PLAY) {
+/*
+ * Play can exit from tcg_cpus_exec at different times than
+ * record,
Re: [PATCH 4/4] replay: simple auto-snapshot mode for record
On 14.08.2023 19:31, Nicholas Piggin wrote:
record makes an initial snapshot when the machine is created, to enable
reverse-debugging. Often the issue being debugged appears near the end of
the trace, so it is important for performance to keep snapshots close to
the end.
This implements a periodic snapshot mode that keeps a rolling set of
recent snapshots.
Arguably this should be done by the debugger or a program that talks to
QMP, but for setting up simple scenarios and tests, it is convenient to
have this feature.
Signed-off-by: Nicholas Piggin
---
docs/system/replay.rst | 5
include/sysemu/replay.h | 11
qemu-options.hx | 9 +--
replay/replay-snapshot.c | 57
replay/replay.c | 25 ++
softmmu/vl.c | 9 +++
6 files changed, 114 insertions(+), 2 deletions(-)
diff --git a/docs/system/replay.rst b/docs/system/replay.rst
index 3105327423..bef9ea4171 100644
--- a/docs/system/replay.rst
+++ b/docs/system/replay.rst
@@ -156,6 +156,11 @@ for storing VM snapshots. Here is the example of the
command line for this:
``empty.qcow2`` drive does not connected to any virtual block device and used
for VM snapshots only.
+``rrsnapmode`` can be used to select just an initial snapshot or periodic
+snapshots, with ``rrsnapcount`` specifying the number of periodic snapshots
+to maintain, and ``rrsnaptime`` the amount of run time in seconds between
+periodic snapshots.
+
.. _network-label:
Network devices
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 08aae5869f..a83e54afc6 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -45,6 +45,17 @@ typedef enum ReplayCheckpoint ReplayCheckpoint;
typedef struct ReplayNetState ReplayNetState;
+enum ReplaySnapshotMode {
+REPLAY_SNAPSHOT_MODE_INITIAL,
+REPLAY_SNAPSHOT_MODE_PERIODIC,
+};
+typedef enum ReplaySnapshotMode ReplaySnapshotMode;
+
+extern ReplaySnapshotMode replay_snapshot_mode;
+
+extern uint64_t replay_snapshot_periodic_delay;
+extern int replay_snapshot_periodic_nr_keep;
+
It seems that all of these doesn't have to be exported,
you can add it into the internal replay header.
/* Name of the initial VM snapshot */
extern char *replay_snapshot
diff --git a/qemu-options.hx b/qemu-options.hx
index 29b98c3d4c..0dce93eeab 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4530,13 +4530,13 @@ SRST
ERST
DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-"-icount
[shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=[,rrsnapshot=]]\n"
\
+"-icount
[shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=[,rrsnapshot=][,rrsnapmode=initial|periodic][,rrsnaptime=secs][,rrsnapcount=N]\n"
\
"enable virtual instruction counter with 2^N clock ticks
per\n" \
"instruction, enable aligning the host and virtual
clocks\n" \
"or disable real time cpu sleeping, and optionally
enable\n" \
"record-and-replay mode\n", QEMU_ARCH_ALL)
SRST
-``-icount
[shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=filename[,rrsnapshot=snapshot]]``
+``-icount
[shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=filename[,rrsnapshot=snapshot][,rrsnapmode=initial|periodic][,rrsnaptime=secs][,rrsnapcount=N]]``
Enable virtual instruction counter. The virtual cpu will execute one
instruction every 2^N ns of virtual time. If ``auto`` is specified
then the virtual cpu speed will be automatically adjusted to keep
@@ -4578,6 +4578,11 @@ SRST
name. In record mode, a new VM snapshot with the given name is created
at the start of execution recording. In replay mode this option
specifies the snapshot name used to load the initial VM state.
+``rrsnapmode=periodic`` will additionally cause a periodic snapshot to
+be created after ``rrsnaptime=secs`` seconds of real runtime. The last
+``rrsnapcount=N`` periodic snapshots (not including the initial) will
+be kept (0 for infinite). Periodic snapshots are useful to speed
+reverse debugging operations near the end of the recorded trace.
ERST
DEF("watchdog-action", HAS_ARG, QEMU_OPTION_watchdog_action, \
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 10a7cf7992..38eac61c43 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -69,6 +69,53 @@ void replay_vmstate_register(void)
vmstate_register(NULL, 0, _replay, _state);
}
+static QEMUTimer *replay_snapshot_timer;
+static int replay_snapshot_count;
+
+static void replay_snapshot_timer_cb(void *opaque)
+{
+Error *err = NULL;
+char *name;
+
+if (!replay_can_snapshot()) {
+/* Try again soon */
+timer_mod(replay_snapshot_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+
Re: [PATCH 3/4] replay: allow runstate shutdown->running when replaying trace
Acked-by: Pavel Dovgalyuk
On 14.08.2023 19:31, Nicholas Piggin wrote:
When replaying a trace, it is possible to go from shutdown to
running with a reverse-debugging step. This can be useful if the
problem being debugged triggers a reset or shutdown.
Signed-off-by: Nicholas Piggin
---
include/sysemu/runstate.h | 1 +
replay/replay.c | 2 ++
softmmu/runstate.c| 19 +++
3 files changed, 22 insertions(+)
diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
index 7beb29c2e2..85a1167ccb 100644
--- a/include/sysemu/runstate.h
+++ b/include/sysemu/runstate.h
@@ -9,6 +9,7 @@ void runstate_set(RunState new_state);
RunState runstate_get(void);
bool runstate_is_running(void);
bool runstate_needs_reset(void);
+void runstate_replay_enable(void);
typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
diff --git a/replay/replay.c b/replay/replay.c
index 0f7d766efe..e64f71209a 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -272,6 +272,8 @@ static void replay_enable(const char *fname, int mode)
/* go to the beginning */
fseek(replay_file, HEADER_SIZE, SEEK_SET);
replay_fetch_data_kind();
+
+runstate_replay_enable();
}
replay_init_events();
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index f3bd862818..9fd3e57485 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -174,6 +174,12 @@ static const RunStateTransition runstate_transitions_def[]
= {
{ RUN_STATE__MAX, RUN_STATE__MAX },
};
+static const RunStateTransition replay_runstate_transitions_def[] = {
+{ RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING},
+
+{ RUN_STATE__MAX, RUN_STATE__MAX },
+};
+
static bool runstate_valid_transitions[RUN_STATE__MAX][RUN_STATE__MAX];
bool runstate_check(RunState state)
@@ -181,6 +187,19 @@ bool runstate_check(RunState state)
return current_run_state == state;
}
+void runstate_replay_enable(void)
+{
+const RunStateTransition *p;
+
+assert(replay_mode == REPLAY_MODE_PLAY);
+
+for (p = _runstate_transitions_def[0]; p->from != RUN_STATE__MAX;
+ p++) {
+runstate_valid_transitions[p->from][p->to] = true;
+}
+
+}
+
static void runstate_init(void)
{
const RunStateTransition *p;
Re: [PATCH 2/4] tests/avocado: replay_linux.py add replay-dump.py test
On 14.08.2023 19:31, Nicholas Piggin wrote:
This runs replay-dump.py after recording a trace, and fails the test if
the script fails.
replay-dump.py is modified to exit with non-zero if an error is
encountered while parsing.
I would like to have separate test for replay-dump, because
replay-linux tests are very heavy to replay and knowing the exact
reason of the failure in advance would be more convenient.
What do you think of splitting the test?
Signed-off-by: Nicholas Piggin
---
It's possible this could introduce failures to existing test if an
unimplemented event gets recorded. I would make a new test for this but
it takes quite a while to record such a long trace that includes some
block and net events to excercise the script.
Thanks,
Nick
scripts/replay-dump.py| 6 --
tests/avocado/replay_linux.py | 16 +++-
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py
index 937ae19ff1..8f4715632a 100755
--- a/scripts/replay-dump.py
+++ b/scripts/replay-dump.py
@@ -21,6 +21,7 @@
import argparse
import struct
import os
+import sys
from collections import namedtuple
# This mirrors some of the global replay state which some of the
@@ -97,7 +98,7 @@ def call_decode(table, index, dumpfile):
print("Could not decode index: %d" % (index))
print("Entry is: %s" % (decoder))
print("Decode Table is:\n%s" % (table))
-return False
+sys.exit(1)
else:
return decoder.fn(decoder.eid, decoder.name, dumpfile)
@@ -118,7 +119,7 @@ def print_event(eid, name, string=None, event_count=None):
def decode_unimp(eid, name, _unused_dumpfile):
"Unimplimented decoder, will trigger exit"
print("%s not handled - will now stop" % (name))
-return False
+sys.exit(1)
# Checkpoint decoder
def swallow_async_qword(eid, name, dumpfile):
@@ -401,3 +402,4 @@ def decode_file(filename):
if __name__ == "__main__":
args = parse_arguments()
decode_file(args.file)
+sys.exit(0)
diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index a76dd507fc..12937ce0ec 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -11,6 +11,7 @@
import os
import logging
import time
+import subprocess
from avocado import skipUnless
from avocado_qemu import BUILD_DIR
@@ -21,6 +22,11 @@
from avocado.utils.path import find_command
from avocado_qemu import LinuxTest
+from pathlib import Path
+
+self_dir = Path(__file__).parent
+src_dir = self_dir.parent.parent
+
class ReplayLinux(LinuxTest):
"""
Boots a Linux system, checking for a successful initialization
@@ -94,7 +100,7 @@ def launch_and_wait(self, record, args, shift):
else:
vm.event_wait('SHUTDOWN', self.timeout)
vm.shutdown(True)
-logger.info('successfully fihished the replay')
+logger.info('successfully finished the replay')
elapsed = time.time() - start_time
logger.info('elapsed time %.2f sec' % elapsed)
return elapsed
@@ -105,6 +111,14 @@ def run_rr(self, args=None, shift=7):
logger = logging.getLogger('replay')
logger.info('replay overhead {:.2%}'.format(t2 / t1 - 1))
+try:
+replay_path = os.path.join(self.workdir, 'replay.bin')
+subprocess.check_call(["./scripts/replay-dump.py",
+ "-f", replay_path],
+ cwd=src_dir, stdout=subprocess.DEVNULL)
+except subprocess.CalledProcessError:
+self.fail('replay-dump.py failed')
+
@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
class ReplayLinuxX8664(ReplayLinux):
"""
Re: [PATCH 1/4] scripts/replay_dump.sh: Update to current rr record format
On 14.08.2023 19:31, Nicholas Piggin wrote:
This thing seems to have fallen by the wayside. This gets it working with
the current format, although does not quite implement all events.
Signed-off-by: Nicholas Piggin
The code looks ok, therefore
Rewieved-by: Pavel Dovgalyuk
However, there is one thing about idea or replay-dump script.
I think it never will be used for parsing the older versions of the log.
Record/replay can only replay the log generated by the same
QEMU version. Any of virtual hw behavior change or some main loop
refactoring may break the replay.
That is why I think that support of the past replay log
formats is useless.
---
My python skills are not good. Any help on this or patch 2 is
appreciated.
Thanks,
Nick
scripts/replay-dump.py | 107 ++---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py
index 3ba97a6d30..937ae19ff1 100755
--- a/scripts/replay-dump.py
+++ b/scripts/replay-dump.py
@@ -20,6 +20,7 @@
import argparse
import struct
+import os
from collections import namedtuple
# This mirrors some of the global replay state which some of the
@@ -62,6 +63,10 @@ def read_byte(fin):
"Read a single byte"
return struct.unpack('>B', fin.read(1))[0]
+def read_bytes(fin, nr):
+"Read a nr bytes"
+return fin.read(nr)
+
def read_event(fin):
"Read a single byte event, but save some state"
if replay_state.already_read:
@@ -122,12 +127,18 @@ def swallow_async_qword(eid, name, dumpfile):
print(" %s(%d) @ %d" % (name, eid, step_id))
return True
+def swallow_bytes(eid, name, dumpfile, nr):
+"Swallow nr bytes of data without looking at it"
+dumpfile.seek(nr, os.SEEK_CUR)
+return True
+
async_decode_table = [ Decoder(0, "REPLAY_ASYNC_EVENT_BH",
swallow_async_qword),
- Decoder(1, "REPLAY_ASYNC_INPUT", decode_unimp),
- Decoder(2, "REPLAY_ASYNC_INPUT_SYNC", decode_unimp),
- Decoder(3, "REPLAY_ASYNC_CHAR_READ", decode_unimp),
- Decoder(4, "REPLAY_ASYNC_EVENT_BLOCK", decode_unimp),
- Decoder(5, "REPLAY_ASYNC_EVENT_NET", decode_unimp),
+ Decoder(1, "REPLAY_ASYNC_BH_ONESHOT", decode_unimp),
+ Decoder(2, "REPLAY_ASYNC_INPUT", decode_unimp),
+ Decoder(3, "REPLAY_ASYNC_INPUT_SYNC", decode_unimp),
+ Decoder(4, "REPLAY_ASYNC_CHAR_READ", decode_unimp),
+ Decoder(5, "REPLAY_ASYNC_EVENT_BLOCK", decode_unimp),
+ Decoder(6, "REPLAY_ASYNC_EVENT_NET", decode_unimp),
]
# See replay_read_events/replay_read_event
def decode_async(eid, name, dumpfile):
@@ -156,6 +167,13 @@ def decode_audio_out(eid, name, dumpfile):
print_event(eid, name, "%d" % (audio_data))
return True
+def decode_random(eid, name, dumpfile):
+ret = read_dword(dumpfile)
+size = read_dword(dumpfile)
+swallow_bytes(eid, name, dumpfile, size)
+print_event(eid, name, "%d %d" % (ret, size))
+return True
+
def decode_checkpoint(eid, name, dumpfile):
"""Decode a checkpoint.
@@ -184,6 +202,38 @@ def decode_interrupt(eid, name, dumpfile):
print_event(eid, name)
return True
+def decode_exception(eid, name, dumpfile):
+print_event(eid, name)
+return True
+
+def decode_shutdown(eid, name, dumpfile):
+print_event(eid, name)
+return True
+
+def decode_end(eid, name, dumpfile):
+print_event(eid, name)
+return False
+
+def decode_char_write(eid, name, dumpfile):
+res = read_dword(dumpfile)
+offset = read_dword(dumpfile)
+print_event(eid, name)
+return True
+
+def decode_async_char_read(eid, name, dumpfile):
+char_id = read_byte(dumpfile)
+size = read_dword(dumpfile)
+print_event(eid, name, "device:%x chars:%s" % (char_id,
read_bytes(dumpfile, size)))
+return True
+
+def decode_async_net(eid, name, dumpfile):
+net_id = read_byte(dumpfile)
+flags = read_dword(dumpfile)
+size = read_dword(dumpfile)
+swallow_bytes(eid, name, dumpfile, size)
+print_event(eid, name, "net:%x flags:%x bytes:%d" % (net_id, flags, size))
+return True
+
def decode_clock(eid, name, dumpfile):
clock_data = read_qword(dumpfile)
print_event(eid, name, "0x%x" % (clock_data))
@@ -268,6 +318,48 @@ def decode_clock(eid, name, dumpfile):
Decoder(28, "EVENT_CP_RESET", decode_checkpoint),
]
+v12_event_table = [Decoder(0, "EVENT_INSTRUCTION", decode_instruction),
+ Decoder(1, "EVENT_INTERRUPT", decode_interrup
[PATCH 1/3] replay: improve determinism of virtio-net
From: Pavel Dovgalyuk
virtio-net device uses bottom halves for callbacks.
These callbacks should be deterministic, because they affect VM state.
This patch replaces BH invocations with corresponding replay functions,
making them deterministic in record/replay mode.
Signed-off-by: Pavel Dovgalyuk
---
hw/net/virtio-net.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7102ec4817..43fd34b0c8 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -46,6 +46,7 @@
#include "net_rx_pkt.h"
#include "hw/virtio/vhost.h"
#include "sysemu/qtest.h"
+#include "sysemu/replay.h"
#define VIRTIO_NET_VM_VERSION11
@@ -417,7 +418,7 @@ static void virtio_net_set_status(struct VirtIODevice
*vdev, uint8_t status)
timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
n->tx_timeout);
} else {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
} else {
if (q->tx_timer) {
@@ -2799,7 +2800,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev,
VirtQueue *vq)
return;
}
virtio_queue_set_notification(vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
}
static void virtio_net_tx_timer(void *opaque)
@@ -2882,7 +2883,7 @@ static void virtio_net_tx_bh(void *opaque)
/* If we flush a full burst of packets, assume there are
* more coming and immediately reschedule */
if (ret >= n->tx_burst) {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
return;
}
@@ -2896,7 +2897,7 @@ static void virtio_net_tx_bh(void *opaque)
return;
} else if (ret > 0) {
virtio_queue_set_notification(q->tx_vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
q->tx_waiting = 1;
}
}
--
2.34.1
[PATCH 3/3] tests/avocado: fix waiting for vm shutdown in replay_linux
From: Pavel Dovgalyuk
This patch fixes the race condition in waiting for shutdown
of the replay linux test.
Signed-off-by: Pavel Dovgalyuk
Suggested-by: John Snow
---
tests/avocado/replay_linux.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index a76dd507fc..270ccc1eae 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -93,7 +93,7 @@ def launch_and_wait(self, record, args, shift):
% os.path.getsize(replay_path))
else:
vm.event_wait('SHUTDOWN', self.timeout)
-vm.shutdown(True)
+vm.wait()
logger.info('successfully fihished the replay')
elapsed = time.time() - start_time
logger.info('elapsed time %.2f sec' % elapsed)
--
2.34.1
[PATCH 0/3] Record/replay patches
From: Pavel Dovgalyuk The set of patches include the following modifications: - fix for allowing record/replay with virtio-net - fix of the record/replay test Pavel Dovgalyuk (3): replay: improve determinism of virtio-net virtio-net: added replay blocker for guest_announce tests/avocado: fix waiting for vm shutdown in replay_linux hw/net/virtio-net.c | 13 + tests/avocado/replay_linux.py | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) -- 2.34.1
[PATCH 2/3] virtio-net: added replay blocker for guest_announce
From: Pavel Dovgalyuk
This patch blocks record/replay when guest_announce is enabled,
because this flag breaks loading the snapshots in deterministic
execution mode.
Signed-off-by: Pavel Dovgalyuk
---
hw/net/virtio-net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 43fd34b0c8..2b6c246efb 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3580,6 +3580,10 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
}
+if (n->host_features & (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE)) {
+replay_add_blocker("-device virtio-net-device,guest_announce=true");
+}
+
if (n->net_conf.duplex_str) {
if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
n->net_conf.duplex = DUPLEX_HALF;
--
2.34.1
Re: [PATCH 4/7] spapr: Fix record-replay machine reset consuming too many events
On 08.08.2023 06:09, Nicholas Piggin wrote: On Sun Aug 6, 2023 at 9:46 PM AEST, Nicholas Piggin wrote: On Fri Aug 4, 2023 at 6:50 PM AEST, Pavel Dovgalyuk wrote: BTW, there is a function qemu_register_reset_nosnapshotload that can be used in similar cases. Can you just use it without changing the code of the reset handler? I didn't know that, thanks for pointing it out. I'll take a closer look at it before reposting. Seems a bit tricky because the device tree has to be rebuilt at reset time (including snapshot load), but it uses the random number. So It seems strange to me, that loading the existing configuration has to randomize the device tree. having a second nosnapshotload reset function might not be called in the correct order, I think? For now I will keep it as is. Ok, let's wait for other reviewers. Pavel Dovgalyuk
Re: [PATCH 4/7] spapr: Fix record-replay machine reset consuming too many events
BTW, there is a function qemu_register_reset_nosnapshotload that can be
used in similar cases.
Can you just use it without changing the code of the reset handler?
On 26.07.2023 21:35, Nicholas Piggin wrote:
spapr_machine_reset gets a random number to populate the device-tree
rng seed with. When loading a snapshot for record-replay, the machine
is reset again, and that tries to consume the random event record
again, crashing due to inconsistent record
Fix this by saving the seed to populate the device tree with, and
skipping the rng on snapshot load.
Cc: Pavel Dovgalyuk
Signed-off-by: Nicholas Piggin
---
hw/ppc/spapr.c | 12 +---
include/hw/ppc/spapr.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7d84244f03..ecfbdb0030 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1022,7 +1022,6 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
{
MachineState *machine = MACHINE(spapr);
SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
-uint8_t rng_seed[32];
int chosen;
_FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
@@ -1100,8 +1099,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
spapr_dt_ov5_platform_support(spapr, fdt, chosen);
}
-qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-_FDT(fdt_setprop(fdt, chosen, "rng-seed", rng_seed, sizeof(rng_seed)));
+_FDT(fdt_setprop(fdt, chosen, "rng-seed", spapr->fdt_rng_seed, 32));
_FDT(spapr_dt_ovec(fdt, chosen, spapr->ov5_cas, "ibm,architecture-vec-5"));
}
@@ -1654,6 +1652,14 @@ static void spapr_machine_reset(MachineState *machine,
ShutdownCause reason)
void *fdt;
int rc;
+if (reason != SHUTDOWN_CAUSE_SNAPSHOT_LOAD) {
+/*
+ * Record-replay snapshot load must not consume random, this was
+ * already replayed from initial machine reset.
+ */
+qemu_guest_getrandom_nofail(spapr->fdt_rng_seed, 32);
+}
+
pef_kvm_reset(machine->cgs, _fatal);
spapr_caps_apply(spapr);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f47e8419a5..f4bd204d86 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -204,6 +204,7 @@ struct SpaprMachineState {
uint32_t fdt_size;
uint32_t fdt_initial_size;
void *fdt_blob;
+uint8_t fdt_rng_seed[32];
long kernel_size;
bool kernel_le;
uint64_t kernel_addr;
Re: [PATCH 7/7] tests/avocado: ppc64 reverse debugging tests for pseries and powernv
Reviewed-by: Pavel Dovgalyuk
On 26.07.2023 21:35, Nicholas Piggin wrote:
These machines run reverse-debugging well enough to pass basic tests.
Wire them up.
Cc: Pavel Dovgalyuk
Signed-off-by: Nicholas Piggin
---
tests/avocado/reverse_debugging.py | 29 +
1 file changed, 29 insertions(+)
diff --git a/tests/avocado/reverse_debugging.py
b/tests/avocado/reverse_debugging.py
index 7d1a478df1..fc47874eda 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -233,3 +233,32 @@ def test_aarch64_virt(self):
self.reverse_debugging(
args=('-kernel', kernel_path))
+
+class ReverseDebugging_ppc64(ReverseDebugging):
+"""
+:avocado: tags=accel:tcg
+"""
+
+REG_PC = 0x40
+
+# unidentified gitlab timeout problem
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
+def test_ppc64_pseries(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+"""
+# SLOF branches back to its entry point, which causes this test
+# to take the 'hit a breakpoint again' path. That's not a problem,
+# just slightly different than the other machines.
+self.endian_is_le = False
+self.reverse_debugging()
+
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
+def test_ppc64_powernv(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:powernv
+"""
+self.endian_is_le = False
+self.reverse_debugging()
Re: [PATCH 6/7] tests/avocado: reverse-debugging cope with re-executing breakpoints
Reviewed-by: Pavel Dovgalyuk
On 26.07.2023 21:35, Nicholas Piggin wrote:
The reverse-debugging test creates a trace, then replays it and:
1. Steps the first 10 instructions and records their addresses.
2. Steps backward and verifies their addresses match.
3. Runs to (near) the end of the trace.
4. Sets breakpoints on the first 10 instructions.
5. Continues backward and verifies execution stops at the last
breakpoint.
Step 5 breaks if any of the other 9 breakpoints are re-executed in the
trace after the 10th instruction is run, because those will be
unexpectedly hit when reverse continuing. This situation does arise
with the ppc pseries machine, the SLOF bios branches to its own entry
point.
Permit this breakpoint re-execution by switching steps 4 and 5, so that
the trace will be run to the end *or* the next breakpoint hit.
Reversing from there to the 10th intsruction will not hit another
breakpoint, by definition.
Another step is added between steps 2 and 3, which steps forward over
the first 10 instructions and verifies their addresses, to support this.
Cc: Pavel Dovgalyuk
Signed-off-by: Nicholas Piggin
---
tests/avocado/reverse_debugging.py | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/tests/avocado/reverse_debugging.py
b/tests/avocado/reverse_debugging.py
index 680c314cfc..7d1a478df1 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -150,16 +150,33 @@ def reverse_debugging(self, shift=7, args=None):
self.check_pc(g, addr)
logger.info('found position %x' % addr)
-logger.info('seeking to the end (icount %s)' % (last_icount - 1))
-vm.qmp('replay-break', icount=last_icount - 1)
-# continue - will return after pausing
-g.cmd(b'c', b'T02thread:01;')
+# visit the recorded instruction in forward order
+logger.info('stepping forward')
+for addr in steps:
+self.check_pc(g, addr)
+self.gdb_step(g)
+logger.info('found position %x' % addr)
+# set breakpoints for the instructions just stepped over
logger.info('setting breakpoints')
for addr in steps:
# hardware breakpoint at addr with len=1
g.cmd(b'Z1,%x,1' % addr, b'OK')
+# this may hit a breakpoint if first instructions are executed
+# again
+logger.info('continuing execution')
+vm.qmp('replay-break', icount=last_icount - 1)
+# continue - will return after pausing
+# This could stop at the end and get a T02 return, or by
+# re-executing one of the breakpoints and get a T05 return.
+g.cmd(b'c')
+if self.vm_get_icount(vm) == last_icount - 1:
+logger.info('reached the end (icount %s)' % (last_icount - 1))
+else:
+logger.info('hit a breakpoint again at %x (icount %s)' %
+(self.get_pc(g), self.vm_get_icount(vm)))
+
logger.info('running reverse continue to reach %x' % steps[-1])
# reverse continue - will return after stopping at the breakpoint
g.cmd(b'bc', b'T05thread:01;')
Re: [PATCH 5/7] tests/avocado: boot ppc64 pseries replay-record test to Linux VFS mount
Acked-by: Pavel Dovgalyuk On 26.07.2023 21:35, Nicholas Piggin wrote: This the ppc64 record-replay test is able to replay the full kernel boot so try enabling it. Cc: Pavel Dovgalyuk Signed-off-by: Nicholas Piggin --- tests/avocado/replay_kernel.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py index 79c607b0e7..a18610542e 100644 --- a/tests/avocado/replay_kernel.py +++ b/tests/avocado/replay_kernel.py @@ -255,8 +255,7 @@ def test_ppc64_pseries(self): kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash) kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0' -# icount is not good enough for PPC64 for complete boot yet -console_pattern = 'Kernel command line: %s' % kernel_command_line +console_pattern = 'VFS: Cannot open root device' self.run_rr(kernel_path, kernel_command_line, console_pattern) def test_ppc64_powernv(self):
Re: [PATCH 4/7] spapr: Fix record-replay machine reset consuming too many events
Acked-by: Pavel Dovgalyuk
On 26.07.2023 21:35, Nicholas Piggin wrote:
spapr_machine_reset gets a random number to populate the device-tree
rng seed with. When loading a snapshot for record-replay, the machine
is reset again, and that tries to consume the random event record
again, crashing due to inconsistent record
Fix this by saving the seed to populate the device tree with, and
skipping the rng on snapshot load.
Cc: Pavel Dovgalyuk
Signed-off-by: Nicholas Piggin
---
hw/ppc/spapr.c | 12 +---
include/hw/ppc/spapr.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7d84244f03..ecfbdb0030 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1022,7 +1022,6 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
{
MachineState *machine = MACHINE(spapr);
SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
-uint8_t rng_seed[32];
int chosen;
_FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
@@ -1100,8 +1099,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
spapr_dt_ov5_platform_support(spapr, fdt, chosen);
}
-qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-_FDT(fdt_setprop(fdt, chosen, "rng-seed", rng_seed, sizeof(rng_seed)));
+_FDT(fdt_setprop(fdt, chosen, "rng-seed", spapr->fdt_rng_seed, 32));
_FDT(spapr_dt_ovec(fdt, chosen, spapr->ov5_cas, "ibm,architecture-vec-5"));
}
@@ -1654,6 +1652,14 @@ static void spapr_machine_reset(MachineState *machine,
ShutdownCause reason)
void *fdt;
int rc;
+if (reason != SHUTDOWN_CAUSE_SNAPSHOT_LOAD) {
+/*
+ * Record-replay snapshot load must not consume random, this was
+ * already replayed from initial machine reset.
+ */
+qemu_guest_getrandom_nofail(spapr->fdt_rng_seed, 32);
+}
+
pef_kvm_reset(machine->cgs, _fatal);
spapr_caps_apply(spapr);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f47e8419a5..f4bd204d86 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -204,6 +204,7 @@ struct SpaprMachineState {
uint32_t fdt_size;
uint32_t fdt_initial_size;
void *fdt_blob;
+uint8_t fdt_rng_seed[32];
long kernel_size;
bool kernel_le;
uint64_t kernel_addr;
[PATCH] tests/avocado: fix waiting for vm shutdown in replay_linux This patch fixes the race condition in waiting for shutdown of the replay linux test.
From: Pavel Dovgalyuk
Signed-off-by: Pavel Dovgalyuk
Suggested-by: John Snow
---
tests/avocado/replay_linux.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index a76dd507fc..270ccc1eae 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -93,7 +93,7 @@ def launch_and_wait(self, record, args, shift):
% os.path.getsize(replay_path))
else:
vm.event_wait('SHUTDOWN', self.timeout)
-vm.shutdown(True)
+vm.wait()
logger.info('successfully fihished the replay')
elapsed = time.time() - start_time
logger.info('elapsed time %.2f sec' % elapsed)
--
2.34.1
Re: tests/avocado/replay_kernel: ReplayKernelNormal.test_aarch64_virt failing
On 10.07.2023 20:14, Philippe Mathieu-Daudé wrote: Hi, Per tests/avocado/replay_kernel.py: def test_aarch64_virt(self): ... console_pattern = 'VFS: Cannot open root device' the test is succeeding, but Avocado reports an error. Does the emulator exit when error "Cannot open root device" happens? Therefore 'quit' command can't be processed in this case. I could get a verbose debug.log, so sharing it here, enjoy! Regards, Phil.
Re: [PATCH 4/7] spapr: Fix record-replay machine reset consuming too many events
e500 has the same problem, I think, according to this issue:
https://gitlab.com/qemu-project/qemu/-/issues/1634
Btw, ARM virt platform rebuilds fdt only at initialization phase, not
when reset.
Isn't this behavior correct? Shouldn't PPC platforms do the similar thing?
On 23.06.2023 15:57, Nicholas Piggin wrote:
spapr_machine_reset gets a random number to populate the device-tree
rng seed with. When loading a snapshot for record-replay, the machine
is reset again, and that tries to consume the random event record
again, crashing due to inconsistent record
Fix this by saving the seed to populate the device tree with, and
skipping the rng on snapshot load.
Signed-off-by: Nicholas Piggin
---
hw/ppc/spapr.c | 12 +---
include/hw/ppc/spapr.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d290acfa95..55948f233f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1017,7 +1017,6 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
{
MachineState *machine = MACHINE(spapr);
SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
-uint8_t rng_seed[32];
int chosen;
_FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
@@ -1095,8 +1094,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
void *fdt, bool reset)
spapr_dt_ov5_platform_support(spapr, fdt, chosen);
}
-qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-_FDT(fdt_setprop(fdt, chosen, "rng-seed", rng_seed, sizeof(rng_seed)));
+_FDT(fdt_setprop(fdt, chosen, "rng-seed", spapr->fdt_rng_seed, 32));
_FDT(spapr_dt_ovec(fdt, chosen, spapr->ov5_cas, "ibm,architecture-vec-5"));
}
@@ -1649,6 +1647,14 @@ static void spapr_machine_reset(MachineState *machine,
ShutdownCause reason)
void *fdt;
int rc;
+if (reason != SHUTDOWN_CAUSE_SNAPSHOT_LOAD) {
+/*
+ * Record-replay snapshot load must not consume random, this was
+ * already replayed from initial machine reset.
+ */
+qemu_guest_getrandom_nofail(spapr->fdt_rng_seed, 32);
+}
+
pef_kvm_reset(machine->cgs, _fatal);
spapr_caps_apply(spapr);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f47e8419a5..f4bd204d86 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -204,6 +204,7 @@ struct SpaprMachineState {
uint32_t fdt_size;
uint32_t fdt_initial_size;
void *fdt_blob;
+uint8_t fdt_rng_seed[32];
long kernel_size;
bool kernel_le;
uint64_t kernel_addr;
Re: [PATCH 5/7] target/ppc: Fix timebase reset with record-replay
Reviewed-by: Pavel Dovgalyuk
On 23.06.2023 15:57, Nicholas Piggin wrote:
Timebase save uses a random number for a legacy vmstate field, which
makes rr snapshot loading unbalanced. The easiest way to deal with this
is just to skip the rng if record-replay is active.
Signed-off-by: Nicholas Piggin
---
hw/ppc/ppc.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 82e4408c5c..7b7db30f95 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -32,6 +32,7 @@
#include "qemu/main-loop.h"
#include "qemu/error-report.h"
#include "sysemu/kvm.h"
+#include "sysemu/replay.h"
#include "sysemu/runstate.h"
#include "kvm_ppc.h"
#include "migration/vmstate.h"
@@ -933,8 +934,14 @@ static void timebase_save(PPCTimebase *tb)
return;
}
-/* not used anymore, we keep it for compatibility */
-tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
+if (replay_mode == REPLAY_MODE_NONE) {
+/* not used anymore, we keep it for compatibility */
+tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
+} else {
+/* simpler for record-replay to avoid this event, compat not needed */
+tb->time_of_the_day_ns = 0;
+}
+
/*
* tb_offset is only expected to be changed by QEMU so
* there is no need to update it from KVM here
Re: [PATCH 7/7] tests/avocado: ppc64 pseries reverse debugging test
On 23.06.2023 15:57, Nicholas Piggin wrote:
pseries can run reverse-debugging well enough to pass basic tests.
There is strangeness with reverse-continue possibly relating to a bp
being set on the first instruction or on a snapshot, that causes
the PC to be reported on the first instruction rather than last
breakpoint, so a workaround is added for that for now.
It means that the test reveals some kind of a bug in PPC debugging
server implementation.
In this case it is better to fix that instead of adding workaround.
Signed-off-by: Nicholas Piggin
---
tests/avocado/reverse_debugging.py | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/tests/avocado/reverse_debugging.py
b/tests/avocado/reverse_debugging.py
index 680c314cfc..553c931994 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -94,7 +94,7 @@ def gdb_bstep(g):
def vm_get_icount(vm):
return vm.qmp('query-replay')['return']['icount']
-def reverse_debugging(self, shift=7, args=None):
+def reverse_debugging(self, shift=7, args=None, initial_skip=False):
logger = logging.getLogger('replay')
# create qcow2 for snapshots
@@ -135,6 +135,10 @@ def reverse_debugging(self, shift=7, args=None):
self.fail('Reverse continue is not supported by QEMU')
logger.info('stepping forward')
+
+if initial_skip:
+self.gdb_step(g)
+
steps = []
# record first instruction addresses
for _ in range(self.STEPS):
@@ -216,3 +220,25 @@ def test_aarch64_virt(self):
self.reverse_debugging(
args=('-kernel', kernel_path))
+
+class ReverseDebugging_ppc64(ReverseDebugging):
+"""
+:avocado: tags=accel:tcg
+"""
+
+REG_PC = 0x40
+
+# unidentified gitlab timeout problem
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
+def test_ppc64_pseries(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+"""
+# start with BIOS only
+self.endian_is_le = False
+# reverse-continue fails (seems to end up at the start) if a break
+# is put on the first instruction. initial_skip skips one before the
+# first bp, and it works. Could be due to break at the same icount
+# as the snapshot?
+self.reverse_debugging(initial_skip=True)
Re: [PATCH 1/7] target/ppc: Fix CPU reservation migration for record-replay
Acked-by: Pavel Dovgalyuk
On 23.06.2023 15:57, Nicholas Piggin wrote:
ppc only migrates reserve_addr, so the destination machine can get a
valid reservation with an incorrect reservation value of 0. Prior to
commit 392d328abe753 ("target/ppc: Ensure stcx size matches larx"),
this could permit a stcx. to incorrectly succeed. That commit
inadvertently fixed that bug because the target machine starts with an
impossible reservation size of 0, so any stcx. will fail.
This behaviour is permitted by the ISA because reservation loss may
have implementation-dependent cause. What's more, with KVM machines it
is impossible save or reasonably restore reservation state. However if
the vmstate is being used for record-replay, the reservation must be
saved and restored exactly in order for execution from snapshot to
match the record.
This patch deprecates the existing incomplete reserve_addr vmstate,
and adds a new vmstate subsection with complete reservation state.
The new vmstate is needed only when record-replay mode is active.
Signed-off-by: Nicholas Piggin
---
target/ppc/cpu.h | 2 ++
target/ppc/machine.c | 26 --
target/ppc/translate.c | 2 ++
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 4138a25801..0087ce66e2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1119,7 +1119,9 @@ struct CPUArchState {
target_ulong reserve_addr; /* Reservation address */
target_ulong reserve_length; /* Reservation larx op size (bytes) */
target_ulong reserve_val;/* Reservation value */
+#if defined(TARGET_PPC64)
target_ulong reserve_val2;
+#endif
/* These are used in supervisor mode only */
target_ulong msr; /* machine state register */
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 134b16c625..a817532e5b 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -10,6 +10,7 @@
#include "qemu/main-loop.h"
#include "kvm_ppc.h"
#include "power8-pmu.h"
+#include "sysemu/replay.h"
static void post_load_update_msr(CPUPPCState *env)
{
@@ -671,6 +672,27 @@ static const VMStateDescription vmstate_compat = {
}
};
+static bool reservation_needed(void *opaque)
+{
+return (replay_mode != REPLAY_MODE_NONE);
+}
+
+static const VMStateDescription vmstate_reservation = {
+.name = "cpu/reservation",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = reservation_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINTTL(env.reserve_addr, PowerPCCPU),
+VMSTATE_UINTTL(env.reserve_length, PowerPCCPU),
+VMSTATE_UINTTL(env.reserve_val, PowerPCCPU),
+#if defined(TARGET_PPC64)
+VMSTATE_UINTTL(env.reserve_val2, PowerPCCPU),
+#endif
+VMSTATE_END_OF_LIST()
+}
+};
+
const VMStateDescription vmstate_ppc_cpu = {
.name = "cpu",
.version_id = 5,
@@ -692,8 +714,7 @@ const VMStateDescription vmstate_ppc_cpu = {
VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024),
VMSTATE_UINT64(env.spe_acc, PowerPCCPU),
-/* Reservation */
-VMSTATE_UINTTL(env.reserve_addr, PowerPCCPU),
+VMSTATE_UNUSED(sizeof(target_ulong)), /* was env.reserve_addr */
/* Supervisor mode architected state */
VMSTATE_UINTTL(env.msr, PowerPCCPU),
@@ -722,6 +743,7 @@ const VMStateDescription vmstate_ppc_cpu = {
_tlbemb,
_tlbmas,
_compat,
+_reservation,
NULL
}
};
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c9fb7b40a5..eb278c2683 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -77,7 +77,9 @@ static TCGv cpu_xer, cpu_so, cpu_ov, cpu_ca, cpu_ov32,
cpu_ca32;
static TCGv cpu_reserve;
static TCGv cpu_reserve_length;
static TCGv cpu_reserve_val;
+#if defined(TARGET_PPC64)
static TCGv cpu_reserve_val2;
+#endif
static TCGv cpu_fpscr;
static TCGv_i32 cpu_access_type;
Re: [PATCH 2/4] virtio-input: add a virtio-mulitouch device
replay/replay-input.c part:
Reviewed-by: Pavel Dovgalyuk
On 18.02.2023 19:22, Sergio Lopez wrote:
Add a virtio-multitouch device to the family of devices emulated by
virtio-input implementing the Multi-touch protocol as descripted here:
https://www.kernel.org/doc/html/latest/input/multi-touch-protocol.html?highlight=multi+touch
This patch just add the device itself, without connecting it to any
backends. The following patches will add helpers in "ui" and will enable
the GTK3 backend to transpose multi-touch events from the host to the
guest.
Signed-off-by: Sergio Lopez
---
hw/input/virtio-input-hid.c | 123 ++-
hw/virtio/virtio-input-pci.c | 25 ++-
include/hw/virtio/virtio-input.h | 9 ++-
include/ui/input.h | 3 +
qapi/ui.json | 45 ++-
replay/replay-input.c| 18 +
ui/input.c | 6 ++
ui/trace-events | 1 +
8 files changed, 216 insertions(+), 14 deletions(-)
diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index d28dab69ba..34109873ac 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -16,9 +16,11 @@
#include "standard-headers/linux/input.h"
-#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
-#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
-#define VIRTIO_ID_NAME_TABLET "QEMU Virtio Tablet"
+#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
+#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
+#define VIRTIO_ID_NAME_TABLET "QEMU Virtio Tablet"
+#define VIRTIO_ID_NAME_MULTITOUCH "QEMU Virtio Multitouch"
+#define VIRTIO_ID_SERIAL_MULTITOUCH "virtio-touchscreen-0"
/* - */
@@ -30,6 +32,7 @@ static const unsigned short keymap_button[INPUT_BUTTON__MAX] = {
[INPUT_BUTTON_WHEEL_DOWN]= BTN_GEAR_DOWN,
[INPUT_BUTTON_SIDE] = BTN_SIDE,
[INPUT_BUTTON_EXTRA] = BTN_EXTRA,
+[INPUT_BUTTON_TOUCH] = BTN_TOUCH,
};
static const unsigned short axismap_rel[INPUT_AXIS__MAX] = {
@@ -42,6 +45,11 @@ static const unsigned short axismap_abs[INPUT_AXIS__MAX] = {
[INPUT_AXIS_Y] = ABS_Y,
};
+static const unsigned short axismap_tch[INPUT_AXIS__MAX] = {
+[INPUT_AXIS_X] = ABS_MT_POSITION_X,
+[INPUT_AXIS_Y] = ABS_MT_POSITION_Y,
+};
+
/* - */
static void virtio_input_extend_config(VirtIOInput *vinput,
@@ -81,6 +89,7 @@ static void virtio_input_handle_event(DeviceState *dev,
QemuConsole *src,
InputKeyEvent *key;
InputMoveEvent *move;
InputBtnEvent *btn;
+InputMultitouchEvent *mtt;
switch (evt->type) {
case INPUT_EVENT_KIND_KEY:
@@ -137,6 +146,24 @@ static void virtio_input_handle_event(DeviceState *dev,
QemuConsole *src,
event.value = cpu_to_le32(move->value);
virtio_input_send(vinput, );
break;
+case INPUT_EVENT_KIND_MTT:
+mtt = evt->u.mtt.data;
+if (mtt->type == INPUT_MULTITOUCH_TYPE_DATA) {
+event.type = cpu_to_le16(EV_ABS);
+event.code = cpu_to_le16(axismap_tch[mtt->axis]);
+event.value = cpu_to_le32(mtt->value);
+virtio_input_send(vinput, );
+} else {
+event.type = cpu_to_le16(EV_ABS);
+event.code = cpu_to_le16(ABS_MT_SLOT);
+event.value = cpu_to_le32(mtt->slot);
+virtio_input_send(vinput, );
+event.type = cpu_to_le16(EV_ABS);
+event.code = cpu_to_le16(ABS_MT_TRACKING_ID);
+event.value = cpu_to_le32(mtt->tracking_id);
+virtio_input_send(vinput, );
+}
+break;
default:
/* keep gcc happy */
break;
@@ -515,12 +542,102 @@ static const TypeInfo virtio_tablet_info = {
/* - */
+static QemuInputHandler virtio_multitouch_handler = {
+.name = VIRTIO_ID_NAME_MULTITOUCH,
+.mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_MTT,
+.event = virtio_input_handle_event,
+.sync = virtio_input_handle_sync,
+};
+
+static struct virtio_input_config virtio_multitouch_config[] = {
+{
+.select= VIRTIO_INPUT_CFG_ID_NAME,
+.size = sizeof(VIRTIO_ID_NAME_MULTITOUCH),
+.u.string = VIRTIO_ID_NAME_MULTITOUCH,
+},{
+.select= VIRTIO_INPUT_CFG_ID_SERIAL,
+.size = sizeof(VIRTIO_ID_SERIAL_MULTITOUCH),
+.u.string = VIRTIO_ID_SERIAL_MULTITOUCH,
+},{
+.select= VIRTIO_INPUT_CFG_ID_DEVIDS,
+.size = sizeof(struct virtio_input_devids),
+
[PATCH v2 2/5] target/avr: implement small RAM/large RAM feature
translate.c functions use RAMPZ for RAM access. This register
is also used for ROM reads. However, in MCUs with 64k RAM support
RAMPZ is used for ROM only. Therefore when RAMPZ is set,
addressing the RAM becomes incorrect in the emulator.
This patch adds LARGE RAM feature which can be used in xmega controllers,
that could be added later. For the currently supported MCUs this
feature is disabled and RAMPZ is not used for RAM access.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
---
target/avr/cpu.h |2 ++
target/avr/translate.c | 63 ++--
2 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index f19dd72926..7c3895b65e 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -106,6 +106,8 @@ typedef enum AVRFeature {
AVR_FEATURE_RAMPX,
AVR_FEATURE_RAMPY,
AVR_FEATURE_RAMPZ,
+
+AVR_FEATURE_LARGE_RAM,
} AVRFeature;
typedef struct CPUArchState {
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 552f739b3d..40b15d116e 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1542,13 +1542,17 @@ static bool trans_BRBS(DisasContext *ctx, arg_BRBS *a)
* M assumed to be in 0x00ff format
* L assumed to be in 0x00ff format
*/
-static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+static void gen_set_addr_short(TCGv addr, TCGv M, TCGv L)
{
-
tcg_gen_andi_tl(L, addr, 0x00ff);
tcg_gen_andi_tl(M, addr, 0xff00);
tcg_gen_shri_tl(M, M, 8);
+}
+
+static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+{
+gen_set_addr_short(addr, M, L);
tcg_gen_andi_tl(H, addr, 0x00ff);
}
@@ -1563,9 +1567,13 @@ static void gen_set_yaddr(TCGv addr)
gen_set_addr(addr, cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static void gen_set_zaddr(TCGv addr)
+static void gen_set_zaddr(DisasContext *ctx, TCGv addr, bool ram)
{
-gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+gen_set_addr_short(addr, cpu_r[31], cpu_r[30]);
+}
}
static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
@@ -1588,9 +1596,16 @@ static TCGv gen_get_yaddr(void)
return gen_get_addr(cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static TCGv gen_get_zaddr(void)
+static TCGv gen_get_zaddr(DisasContext *ctx, bool ram)
{
-return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+TCGv zero = tcg_const_i32(0);
+TCGv res = gen_get_addr(zero, cpu_r[31], cpu_r[30]);
+tcg_temp_free_i32(zero);
+return res;
+}
}
/*
@@ -1868,12 +1883,12 @@ static bool trans_LDDY(DisasContext *ctx, arg_LDDY *a)
static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_load(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1883,12 +1898,12 @@ static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_load(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1898,7 +1913,7 @@ static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
static bool trans_LDDZ(DisasContext *ctx, arg_LDDZ *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_addi_tl(addr, addr, a->imm); /* addr = addr + q */
gen_data_load(ctx, Rd, addr);
@@ -2088,12 +2103,12 @@ static bool trans_STDY(DisasContext *ctx, arg_STDY *a)
static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_store(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2103,12 +2118,12 @@ static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
static bool trans_STZ3(DisasContext *ctx, arg_STZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_store(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2118,7 +2133,7 @
[PATCH v2 1/5] target/avr: fix long address calculation
AVR ELPMX instruction (and some others) use three registers to
form long 24-bit address from RAMPZ and two 8-bit registers.
RAMPZ stores shifted 8 bits like ff to simplify address calculation.
This patch fixes full address calculation in function gen_get_addr
by changing the mess in offsets of deposit tcg instructions.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
Reviewed-by: Michael Rolnik
---
target/avr/translate.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 2bed56f135..552f739b3d 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1572,8 +1572,8 @@ static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
{
TCGv addr = tcg_temp_new_i32();
-tcg_gen_deposit_tl(addr, M, H, 8, 8);
-tcg_gen_deposit_tl(addr, L, addr, 8, 16);
+tcg_gen_deposit_tl(addr, H, M, 8, 8);
+tcg_gen_deposit_tl(addr, addr, L, 0, 8);
return addr;
}
[PATCH v2 4/5] target/avr: fix interrupt processing
Interrupt bit vector has 64 bits, but interrupt vector is found with ctz32
function. This patch replaces it with ctz64.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/avr/helper.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 156dde4e92..61ab6feb25 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -51,7 +51,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
if (interrupt_request & CPU_INTERRUPT_HARD) {
if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
-int index = ctz32(env->intsrc);
+int index = ctz64(env->intsrc);
cs->exception_index = EXCP_INT(index);
avr_cpu_do_interrupt(cs);
@@ -78,7 +78,7 @@ void avr_cpu_do_interrupt(CPUState *cs)
if (cs->exception_index == EXCP_RESET) {
vector = 0;
} else if (env->intsrc != 0) {
-vector = ctz32(env->intsrc) + 1;
+vector = ctz64(env->intsrc) + 1;
}
if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
[PATCH v2 0/5] AVR target fixes
This set of patches includes multiple changes for AVR target. v2 changes: - fixed instruction translation in icount mode --- Pavel Dovgalyuk (5): target/avr: fix long address calculation target/avr: implement small RAM/large RAM feature target/avr: fix avr features processing target/avr: fix interrupt processing target/avr: enable icount mode target/avr/cpu.h | 6 ++- target/avr/helper.c| 4 +- target/avr/translate.c | 93 +++--- 3 files changed, 75 insertions(+), 28 deletions(-) -- Pavel Dovgalyuk
[PATCH v2 5/5] target/avr: enable icount mode
Icount mode requires correct can_do_io flag management for checking
that IO operations are performed only in the last TB instruction.
This patch sets this flag before every helper which can lead to
virtual hardware access. It enables deterministic execution
in icount mode for AVR.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
---
target/avr/translate.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 40b15d116e..ee137dfe54 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1406,6 +1406,10 @@ static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
{
TCGv temp = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(temp, cpu_env, temp);
tcg_gen_andi_tl(temp, temp, 1 << a->bit);
ctx->skip_cond = TCG_COND_EQ;
@@ -1424,6 +1428,10 @@ static bool trans_SBIS(DisasContext *ctx, arg_SBIS *a)
{
TCGv temp = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(temp, cpu_env, temp);
tcg_gen_andi_tl(temp, temp, 1 << a->bit);
ctx->skip_cond = TCG_COND_NE;
@@ -1621,6 +1629,9 @@ static TCGv gen_get_zaddr(DisasContext *ctx, bool ram)
static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
{
if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
gen_helper_fullwr(cpu_env, data, addr);
} else {
tcg_gen_qemu_st8(data, addr, MMU_DATA_IDX); /* mem[addr] = data */
@@ -1630,6 +1641,9 @@ static void gen_data_store(DisasContext *ctx, TCGv data,
TCGv addr)
static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
{
if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
gen_helper_fullrd(data, cpu_env, addr);
} else {
tcg_gen_qemu_ld8u(data, addr, MMU_DATA_IDX); /* data = mem[addr] */
@@ -2335,6 +2349,10 @@ static bool trans_IN(DisasContext *ctx, arg_IN *a)
TCGv Rd = cpu_r[a->rd];
TCGv port = tcg_const_i32(a->imm);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(Rd, cpu_env, port);
tcg_temp_free_i32(port);
@@ -2351,6 +2369,10 @@ static bool trans_OUT(DisasContext *ctx, arg_OUT *a)
TCGv Rd = cpu_r[a->rd];
TCGv port = tcg_const_i32(a->imm);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_outb(cpu_env, port, Rd);
tcg_temp_free_i32(port);
@@ -2651,6 +2673,10 @@ static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
TCGv data = tcg_temp_new_i32();
TCGv port = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(data, cpu_env, port);
tcg_gen_ori_tl(data, data, 1 << a->bit);
gen_helper_outb(cpu_env, port, data);
@@ -2670,6 +2696,10 @@ static bool trans_CBI(DisasContext *ctx, arg_CBI *a)
TCGv data = tcg_temp_new_i32();
TCGv port = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(data, cpu_env, port);
tcg_gen_andi_tl(data, data, ~(1 << a->bit));
gen_helper_outb(cpu_env, port, data);
[PATCH v2 3/5] target/avr: fix avr features processing
Bit vector for features has 64 bits. This patch fixes bit shifts in
avr_feature and set_avr_feature functions to be 64-bit too.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Michael Rolnik
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/avr/cpu.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 7c3895b65e..280edc495b 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -166,12 +166,12 @@ vaddr avr_cpu_gdb_adjust_breakpoint(CPUState *cpu, vaddr
addr);
static inline int avr_feature(CPUAVRState *env, AVRFeature feature)
{
-return (env->features & (1U << feature)) != 0;
+return (env->features & (1ULL << feature)) != 0;
}
static inline void set_avr_feature(CPUAVRState *env, int feature)
{
-env->features |= (1U << feature);
+env->features |= (1ULL << feature);
}
#define cpu_list avr_cpu_list
[PATCH v2 1/5] target/avr: fix long address calculation
AVR ELPMX instruction (and some others) use three registers to
form long 24-bit address from RAMPZ and two 8-bit registers.
RAMPZ stores shifted 8 bits like ff to simplify address calculation.
This patch fixes full address calculation in function gen_get_addr
by changing the mess in offsets of deposit tcg instructions.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
Reviewed-by: Michael Rolnik
---
target/avr/translate.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 2bed56f135..552f739b3d 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1572,8 +1572,8 @@ static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
{
TCGv addr = tcg_temp_new_i32();
-tcg_gen_deposit_tl(addr, M, H, 8, 8);
-tcg_gen_deposit_tl(addr, L, addr, 8, 16);
+tcg_gen_deposit_tl(addr, H, M, 8, 8);
+tcg_gen_deposit_tl(addr, addr, L, 0, 8);
return addr;
}
[PATCH v2 2/5] target/avr: implement small RAM/large RAM feature
translate.c functions use RAMPZ for RAM access. This register
is also used for ROM reads. However, in MCUs with 64k RAM support
RAMPZ is used for ROM only. Therefore when RAMPZ is set,
addressing the RAM becomes incorrect in the emulator.
This patch adds LARGE RAM feature which can be used in xmega controllers,
that could be added later. For the currently supported MCUs this
feature is disabled and RAMPZ is not used for RAM access.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/cpu.h |2 ++
target/avr/translate.c | 63 ++--
2 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index f19dd72926..7c3895b65e 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -106,6 +106,8 @@ typedef enum AVRFeature {
AVR_FEATURE_RAMPX,
AVR_FEATURE_RAMPY,
AVR_FEATURE_RAMPZ,
+
+AVR_FEATURE_LARGE_RAM,
} AVRFeature;
typedef struct CPUArchState {
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 552f739b3d..40b15d116e 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1542,13 +1542,17 @@ static bool trans_BRBS(DisasContext *ctx, arg_BRBS *a)
* M assumed to be in 0x00ff format
* L assumed to be in 0x00ff format
*/
-static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+static void gen_set_addr_short(TCGv addr, TCGv M, TCGv L)
{
-
tcg_gen_andi_tl(L, addr, 0x00ff);
tcg_gen_andi_tl(M, addr, 0xff00);
tcg_gen_shri_tl(M, M, 8);
+}
+
+static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+{
+gen_set_addr_short(addr, M, L);
tcg_gen_andi_tl(H, addr, 0x00ff);
}
@@ -1563,9 +1567,13 @@ static void gen_set_yaddr(TCGv addr)
gen_set_addr(addr, cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static void gen_set_zaddr(TCGv addr)
+static void gen_set_zaddr(DisasContext *ctx, TCGv addr, bool ram)
{
-gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+gen_set_addr_short(addr, cpu_r[31], cpu_r[30]);
+}
}
static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
@@ -1588,9 +1596,16 @@ static TCGv gen_get_yaddr(void)
return gen_get_addr(cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static TCGv gen_get_zaddr(void)
+static TCGv gen_get_zaddr(DisasContext *ctx, bool ram)
{
-return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+TCGv zero = tcg_const_i32(0);
+TCGv res = gen_get_addr(zero, cpu_r[31], cpu_r[30]);
+tcg_temp_free_i32(zero);
+return res;
+}
}
/*
@@ -1868,12 +1883,12 @@ static bool trans_LDDY(DisasContext *ctx, arg_LDDY *a)
static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_load(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1883,12 +1898,12 @@ static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_load(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1898,7 +1913,7 @@ static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
static bool trans_LDDZ(DisasContext *ctx, arg_LDDZ *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_addi_tl(addr, addr, a->imm); /* addr = addr + q */
gen_data_load(ctx, Rd, addr);
@@ -2088,12 +2103,12 @@ static bool trans_STDY(DisasContext *ctx, arg_STDY *a)
static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_store(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2103,12 +2118,12 @@ static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
static bool trans_STZ3(DisasContext *ctx, arg_STZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_store(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2118,7 +2133,7 @@ static bool trans_STZ3(Dis
[PATCH v2 3/5] target/avr: fix avr features processing
Bit vector for features has 64 bits. This patch fixes bit shifts in
avr_feature and set_avr_feature functions to be 64-bit too.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Michael Rolnik
---
target/avr/cpu.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 7c3895b65e..280edc495b 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -166,12 +166,12 @@ vaddr avr_cpu_gdb_adjust_breakpoint(CPUState *cpu, vaddr
addr);
static inline int avr_feature(CPUAVRState *env, AVRFeature feature)
{
-return (env->features & (1U << feature)) != 0;
+return (env->features & (1ULL << feature)) != 0;
}
static inline void set_avr_feature(CPUAVRState *env, int feature)
{
-env->features |= (1U << feature);
+env->features |= (1ULL << feature);
}
#define cpu_list avr_cpu_list
[PATCH v2 0/5] AVR target fixes
This set of patches includes multiple changes for AVR target. v2 changes: - fixed instruction translation in icount mode --- Pavel Dovgalyuk (5): target/avr: fix long address calculation target/avr: implement small RAM/large RAM feature target/avr: fix avr features processing target/avr: fix interrupt processing target/avr: enable icount mode target/avr/cpu.h | 6 ++- target/avr/helper.c| 4 +- target/avr/translate.c | 93 +++--- 3 files changed, 75 insertions(+), 28 deletions(-) -- Pavel Dovgalyuk
Re: reverse-{debugging,continue} not working on v7.2.0, i386 guest
On 19.01.2023 07:40, Hyeonggon Yoo wrote: On Wed, Jan 18, 2023 at 12:39:16PM +0300, Pavel Dovgalyuk wrote: Sometimes replay (or reverse debugging) have problems due to incomplete or incorrect virtual device save/load implementation. Can you try removing -cpu from your command line? Or you can provide the files you load and I'll debug this case. Ah, sorry to bother. I installed breakpoint _after_ kernel panic, and installing breakpoint before boot worked fine. Every seems great! Glad to hear that. Just a side question, is there a reason QEMU record/replay does not support -smp N (> 1)? is this feature planed, or should I use other tools to debug SMP bugs? Parallel SMP deterministic emulation is very hard. However, I think multiple-cores-on-single-core deterministic emulation will be supported someday. On 18.01.2023 11:47, Hyeonggon Yoo wrote: On Wed, Jan 18, 2023 at 10:12:48AM +0300, Pavel Dovgalyuk wrote: As replay works well, the reverse debugging should be ok too. But for "going back" it needs a VM snapshot that can be used for reload. Snapshots are saved on qcow2 images connected to QEMU. Therefore you need to add an empty qcow2 to your command line with the following option: -drive file=empty.qcow2,if=none,id=rr Oh, I guessed it's possible to reverse-debug without snapshot, and your comments definitely helped! adding empty disk and snapshotting solved it. But I faced another problem: (gdb) b __list_del_entry_valid (gdb) reverse-continue (it stuck forever) ^C (gdb) info registers eax0xefe19f74 -270426252 ecx0x0 0 edx0xefe19f74 -270426252 ebx0xf6ff4620 -151042528 esp0xc02e9a34 0xc02e9a34 ebp0xc02e9a6c 0xc02e9a6c esi0xc4fffb20 -989856992 edi0xefe19f70 -270426256 eip0xc1f38400 0xc1f38400 <__list_del_entry_valid> eflags 0x6 [ IOPL=0 PF ] cs 0x6096 ss 0x68104 ds 0x7b123 es 0x7b123 fs 0xd8216 gs 0x0 0 fs_base0x31cb4000 835403776 gs_base0x0 0 k_gs_base 0x0 0 cr00x80050033 [ PG AM WP NE ET MP PE ] cr20xffcb1000 -3469312 cr30x534e000 [ PDBR=0 PCID=0 ] cr40x406d0 [ PSE MCE PGE OSFXSR OSXMMEXCPT OSXSAVE ] cr80x1 1 efer 0x0 [ ] it stuck here and it's not 'last breakpoint hit' from the panic (it's early in boot), and stepi, nexti, continue commands do not work and there's no forward progress. (eip doesn't change) Did I miss something or did something wrong? thank you so much with your help. -- Best regards, Hyeonggon And you also need to add rrsnapshot to icount for creating the snapshot at the start of VM execution: -icount shift=auto,rr=record,rrfile=$REPLAY_FILE,rrsnapshot=start On 18.01.2023 09:14, Hyeonggon Yoo wrote: Hello QEMU folks. I was struggling to fix a recent heisenbug in the Linux kernel, and fortunately the bug was reproducible with TCG and -smp 1. I'm using qemu version 7.2.0, and guest architecture is i386. I tried to inspect the bug using record/replay and reverse-debugging feature in the QEMU. recorded with: qemu-system-i386 \ -icount shift=auto,rr=record,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -append "page_owner=on console=ttyS0" and replayed with: qemu-system-i386 \ -icount shift=auto,rr=replay,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -s \ -append "page_owner=on console=ttyS0" (I'm using a initrd image instead of a disk file.) The record and replay works well. The bug is reliably reproduced when relaying. but when I try to reverse-continue or reverse-stepi after kernel panic, the gdb only says: "remote failure reply 'E14'" Is there something I'm missing, or record/replay do not work with QEMU v7.2.0 or i386? -- Best regards, Hyeonggon
[PATCH v2 4/5] target/avr: fix interrupt processing
Interrupt bit vector has 64 bits, but interrupt vector is found with ctz32
function. This patch replaces it with ctz64.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/helper.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 156dde4e92..61ab6feb25 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -51,7 +51,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
if (interrupt_request & CPU_INTERRUPT_HARD) {
if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
-int index = ctz32(env->intsrc);
+int index = ctz64(env->intsrc);
cs->exception_index = EXCP_INT(index);
avr_cpu_do_interrupt(cs);
@@ -78,7 +78,7 @@ void avr_cpu_do_interrupt(CPUState *cs)
if (cs->exception_index == EXCP_RESET) {
vector = 0;
} else if (env->intsrc != 0) {
-vector = ctz32(env->intsrc) + 1;
+vector = ctz64(env->intsrc) + 1;
}
if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
[PATCH v2 5/5] target/avr: enable icount mode
Icount mode requires correct can_do_io flag management for checking
that IO operations are performed only in the last TB instruction.
This patch sets this flag before every helper which can lead to
virtual hardware access. It enables deterministic execution
in icount mode for AVR.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/translate.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 40b15d116e..ee137dfe54 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1406,6 +1406,10 @@ static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
{
TCGv temp = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(temp, cpu_env, temp);
tcg_gen_andi_tl(temp, temp, 1 << a->bit);
ctx->skip_cond = TCG_COND_EQ;
@@ -1424,6 +1428,10 @@ static bool trans_SBIS(DisasContext *ctx, arg_SBIS *a)
{
TCGv temp = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(temp, cpu_env, temp);
tcg_gen_andi_tl(temp, temp, 1 << a->bit);
ctx->skip_cond = TCG_COND_NE;
@@ -1621,6 +1629,9 @@ static TCGv gen_get_zaddr(DisasContext *ctx, bool ram)
static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
{
if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
gen_helper_fullwr(cpu_env, data, addr);
} else {
tcg_gen_qemu_st8(data, addr, MMU_DATA_IDX); /* mem[addr] = data */
@@ -1630,6 +1641,9 @@ static void gen_data_store(DisasContext *ctx, TCGv data,
TCGv addr)
static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
{
if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
gen_helper_fullrd(data, cpu_env, addr);
} else {
tcg_gen_qemu_ld8u(data, addr, MMU_DATA_IDX); /* data = mem[addr] */
@@ -2335,6 +2349,10 @@ static bool trans_IN(DisasContext *ctx, arg_IN *a)
TCGv Rd = cpu_r[a->rd];
TCGv port = tcg_const_i32(a->imm);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(Rd, cpu_env, port);
tcg_temp_free_i32(port);
@@ -2351,6 +2369,10 @@ static bool trans_OUT(DisasContext *ctx, arg_OUT *a)
TCGv Rd = cpu_r[a->rd];
TCGv port = tcg_const_i32(a->imm);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_outb(cpu_env, port, Rd);
tcg_temp_free_i32(port);
@@ -2651,6 +2673,10 @@ static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
TCGv data = tcg_temp_new_i32();
TCGv port = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(data, cpu_env, port);
tcg_gen_ori_tl(data, data, 1 << a->bit);
gen_helper_outb(cpu_env, port, data);
@@ -2670,6 +2696,10 @@ static bool trans_CBI(DisasContext *ctx, arg_CBI *a)
TCGv data = tcg_temp_new_i32();
TCGv port = tcg_const_i32(a->reg);
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+
gen_helper_inb(data, cpu_env, port);
tcg_gen_andi_tl(data, data, ~(1 << a->bit));
gen_helper_outb(cpu_env, port, data);
Re: reverse-{debugging,continue} not working on v7.2.0, i386 guest
Sometimes replay (or reverse debugging) have problems due to incomplete or incorrect virtual device save/load implementation. Can you try removing -cpu from your command line? Or you can provide the files you load and I'll debug this case. On 18.01.2023 11:47, Hyeonggon Yoo wrote: On Wed, Jan 18, 2023 at 10:12:48AM +0300, Pavel Dovgalyuk wrote: As replay works well, the reverse debugging should be ok too. But for "going back" it needs a VM snapshot that can be used for reload. Snapshots are saved on qcow2 images connected to QEMU. Therefore you need to add an empty qcow2 to your command line with the following option: -drive file=empty.qcow2,if=none,id=rr Oh, I guessed it's possible to reverse-debug without snapshot, and your comments definitely helped! adding empty disk and snapshotting solved it. But I faced another problem: (gdb) b __list_del_entry_valid (gdb) reverse-continue (it stuck forever) ^C (gdb) info registers eax0xefe19f74 -270426252 ecx0x0 0 edx0xefe19f74 -270426252 ebx0xf6ff4620 -151042528 esp0xc02e9a34 0xc02e9a34 ebp0xc02e9a6c 0xc02e9a6c esi0xc4fffb20 -989856992 edi0xefe19f70 -270426256 eip0xc1f38400 0xc1f38400 <__list_del_entry_valid> eflags 0x6 [ IOPL=0 PF ] cs 0x6096 ss 0x68104 ds 0x7b123 es 0x7b123 fs 0xd8216 gs 0x0 0 fs_base0x31cb4000 835403776 gs_base0x0 0 k_gs_base 0x0 0 cr00x80050033 [ PG AM WP NE ET MP PE ] cr20xffcb1000 -3469312 cr30x534e000 [ PDBR=0 PCID=0 ] cr40x406d0 [ PSE MCE PGE OSFXSR OSXMMEXCPT OSXSAVE ] cr80x1 1 efer 0x0 [ ] it stuck here and it's not 'last breakpoint hit' from the panic (it's early in boot), and stepi, nexti, continue commands do not work and there's no forward progress. (eip doesn't change) Did I miss something or did something wrong? thank you so much with your help. -- Best regards, Hyeonggon And you also need to add rrsnapshot to icount for creating the snapshot at the start of VM execution: -icount shift=auto,rr=record,rrfile=$REPLAY_FILE,rrsnapshot=start On 18.01.2023 09:14, Hyeonggon Yoo wrote: Hello QEMU folks. I was struggling to fix a recent heisenbug in the Linux kernel, and fortunately the bug was reproducible with TCG and -smp 1. I'm using qemu version 7.2.0, and guest architecture is i386. I tried to inspect the bug using record/replay and reverse-debugging feature in the QEMU. recorded with: qemu-system-i386 \ -icount shift=auto,rr=record,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -append "page_owner=on console=ttyS0" and replayed with: qemu-system-i386 \ -icount shift=auto,rr=replay,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -s \ -append "page_owner=on console=ttyS0" (I'm using a initrd image instead of a disk file.) The record and replay works well. The bug is reliably reproduced when relaying. but when I try to reverse-continue or reverse-stepi after kernel panic, the gdb only says: "remote failure reply 'E14'" Is there something I'm missing, or record/replay do not work with QEMU v7.2.0 or i386? -- Best regards, Hyeonggon
Re: reverse-{debugging,continue} not working on v7.2.0, i386 guest
As replay works well, the reverse debugging should be ok too. But for "going back" it needs a VM snapshot that can be used for reload. Snapshots are saved on qcow2 images connected to QEMU. Therefore you need to add an empty qcow2 to your command line with the following option: -drive file=empty.qcow2,if=none,id=rr And you also need to add rrsnapshot to icount for creating the snapshot at the start of VM execution: -icount shift=auto,rr=record,rrfile=$REPLAY_FILE,rrsnapshot=start On 18.01.2023 09:14, Hyeonggon Yoo wrote: Hello QEMU folks. I was struggling to fix a recent heisenbug in the Linux kernel, and fortunately the bug was reproducible with TCG and -smp 1. I'm using qemu version 7.2.0, and guest architecture is i386. I tried to inspect the bug using record/replay and reverse-debugging feature in the QEMU. recorded with: qemu-system-i386 \ -icount shift=auto,rr=record,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -append "page_owner=on console=ttyS0" and replayed with: qemu-system-i386 \ -icount shift=auto,rr=replay,rrfile=$REPLAY_FILE \ -kernel arch/x86/boot/bzImage \ -cpu SandyBridge \ -initrd debian-i386.cgz \ -smp 1 \ -m 1024 \ -nographic \ -net none \ -s \ -append "page_owner=on console=ttyS0" (I'm using a initrd image instead of a disk file.) The record and replay works well. The bug is reliably reproduced when relaying. but when I try to reverse-continue or reverse-stepi after kernel panic, the gdb only says: "remote failure reply 'E14'" Is there something I'm missing, or record/replay do not work with QEMU v7.2.0 or i386? -- Best regards, Hyeonggon
Re: [PATCH 4/5] replay: Extract core API to 'exec/replay-core.h'
Reviewed-by: Pavel Dovgalyuk
On 19.12.2022 20:08, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daude
replay API is used deeply within TCG common code (common to user
and system emulation). Unfortunately "sysemu/replay.h" requires
some QAPI headers for few system-specific declarations, example:
void replay_input_event(QemuConsole *src, InputEvent *evt);
Since commit c2651c0eaa ("qapi/meson: Restrict UI module to system
emulation and tools") the QAPI header defining the InputEvent is
not generated anymore.
To keep it simple, extract the 'core' replay prototypes to a new
"exec/replay-core.h" header which we include in the TCG code that
doesn't need the rest of the replay API.
Signed-off-by: Philippe Mathieu-Daudé
---
MAINTAINERS| 1 +
accel/tcg/cpu-exec.c | 2 +-
accel/tcg/tcg-all.c| 2 +-
accel/tcg/translator.c | 2 +-
accel/tcg/user-exec-stub.c | 2 +-
cpu.c | 2 +-
gdbstub/gdbstub.c | 2 +-
hw/core/ptimer.c | 2 +-
include/exec/replay-core.h | 76 ++
include/sysemu/replay.h| 67 ++
stubs/replay.c | 2 +-
tests/unit/ptimer-test-stubs.c | 2 +-
util/guest-random.c| 2 +-
13 files changed, 91 insertions(+), 73 deletions(-)
create mode 100644 include/exec/replay-core.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 3bd433b65a..04aa77fd37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3206,6 +3206,7 @@ S: Supported
F: replay/*
F: block/blkreplay.c
F: net/filter-replay.c
+F: include/exec/replay-core.h
F: include/sysemu/replay.h
F: docs/devel/replay.rst
F: docs/system/replay.rst
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5a7825dce1..040de10440 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -37,7 +37,7 @@
#include "sysemu/cpus.h"
#include "exec/cpu-all.h"
#include "sysemu/cpu-timers.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
#include "sysemu/tcg.h"
#include "exec/helper-proto.h"
#include "tb-jmp-cache.h"
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 30b503fb22..5dab1ae9dd 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -25,7 +25,7 @@
#include "qemu/osdep.h"
#include "sysemu/tcg.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
#include "sysemu/cpu-timers.h"
#include "tcg/tcg.h"
#include "qapi/error.h"
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 061519691f..3a40f9eaca 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -16,7 +16,7 @@
#include "exec/log.h"
#include "exec/translator.h"
#include "exec/plugin-gen.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
/* Pairs with tcg_clear_temp_count.
To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 968cd3ca60..874e1f1a20 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -1,6 +1,6 @@
#include "qemu/osdep.h"
#include "hw/core/cpu.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
bool enable_cpu_pm = false;
diff --git a/cpu.c b/cpu.c
index 4a7d865427..5503e2ff12 100644
--- a/cpu.c
+++ b/cpu.c
@@ -33,7 +33,7 @@
#endif
#include "sysemu/tcg.h"
#include "sysemu/kvm.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
#include "exec/cpu-common.h"
#include "exec/exec-all.h"
#include "exec/translate-all.h"
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index c3fbc31123..fb9c49e0fd 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -48,7 +48,7 @@
#include "sysemu/runstate.h"
#include "semihosting/semihost.h"
#include "exec/exec-all.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
#include "internals.h"
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index eb5ba1aff7..e03165febf 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -10,7 +10,7 @@
#include "hw/ptimer.h"
#include "migration/vmstate.h"
#include "qemu/host-utils.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
#include "sysemu/cpu-timers.h"
#include "sysemu/qtest.h"
#include "block/aio.h"
diff --git a/include/exec/replay-core.h b/include/exec/replay-core.h
new file mode 100644
index 00..9ec20cb124
--- /dev/null
+++ b/include/exec/replay-core.h
@@ -0,0 +1,76 @@
+/*
+
Re: [PATCH for-7.2] replay: Fix declaration of replay_read_next_clock
Reviewed-by: Pavel Dovgalyuk
On 29.11.2022 04:05, Richard Henderson wrote:
Fixes the build with gcc 13:
replay/replay-time.c:34:6: error: conflicting types for \
'replay_read_next_clock' due to enum/integer mismatch; \
have 'void(ReplayClockKind)' [-Werror=enum-int-mismatch]
34 | void replay_read_next_clock(ReplayClockKind kind)
| ^~
In file included from ../qemu/replay/replay-time.c:14:
replay/replay-internal.h:139:6: note: previous declaration of \
'replay_read_next_clock' with type 'void(unsigned int)'
139 | void replay_read_next_clock(unsigned int kind);
| ^~
Fixes: 8eda206e090 ("replay: recording and replaying clock ticks")
Signed-off-by: Richard Henderson
---
replay/replay-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 89e377be90..b6836354ac 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -136,7 +136,7 @@ bool replay_next_event_is(int event);
/*! Reads next clock value from the file.
If clock kind read from the file is different from the parameter,
the value is not used. */
-void replay_read_next_clock(unsigned int kind);
+void replay_read_next_clock(ReplayClockKind kind);
/* Asynchronous events queue */
[PATCH 2/4] target/avr: implement small RAM/large RAM feature
translate.c functions use RAMPZ for RAM access. This register
is also used for ROM reads. However, in MCUs with 64k RAM support
RAMPZ is used for ROM only. Therefore when RAMPZ is set,
addressing the RAM becomes incorrect in the emulator.
This patch adds LARGE RAM feature which can be used in xmega controllers,
that could be added later. For the currently supported MCUs this
feature is disabled and RAMPZ is not used for RAM access.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/cpu.h |2 ++
target/avr/translate.c | 63 ++--
2 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 96419c0c2b..cfdc0ecb70 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -106,6 +106,8 @@ typedef enum AVRFeature {
AVR_FEATURE_RAMPX,
AVR_FEATURE_RAMPY,
AVR_FEATURE_RAMPZ,
+
+AVR_FEATURE_LARGE_RAM,
} AVRFeature;
typedef struct CPUArchState {
diff --git a/target/avr/translate.c b/target/avr/translate.c
index c9a0a39c2d..e4900d630f 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1542,13 +1542,17 @@ static bool trans_BRBS(DisasContext *ctx, arg_BRBS *a)
* M assumed to be in 0x00ff format
* L assumed to be in 0x00ff format
*/
-static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+static void gen_set_addr_short(TCGv addr, TCGv M, TCGv L)
{
-
tcg_gen_andi_tl(L, addr, 0x00ff);
tcg_gen_andi_tl(M, addr, 0xff00);
tcg_gen_shri_tl(M, M, 8);
+}
+
+static void gen_set_addr(TCGv addr, TCGv H, TCGv M, TCGv L)
+{
+gen_set_addr_short(addr, M, L);
tcg_gen_andi_tl(H, addr, 0x00ff);
}
@@ -1563,9 +1567,13 @@ static void gen_set_yaddr(TCGv addr)
gen_set_addr(addr, cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static void gen_set_zaddr(TCGv addr)
+static void gen_set_zaddr(DisasContext *ctx, TCGv addr, bool ram)
{
-gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+gen_set_addr(addr, cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+gen_set_addr_short(addr, cpu_r[31], cpu_r[30]);
+}
}
static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
@@ -1588,9 +1596,16 @@ static TCGv gen_get_yaddr(void)
return gen_get_addr(cpu_rampY, cpu_r[29], cpu_r[28]);
}
-static TCGv gen_get_zaddr(void)
+static TCGv gen_get_zaddr(DisasContext *ctx, bool ram)
{
-return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+if (!ram || avr_feature(ctx->env, AVR_FEATURE_LARGE_RAM)) {
+return gen_get_addr(cpu_rampZ, cpu_r[31], cpu_r[30]);
+} else {
+TCGv zero = tcg_const_i32(0);
+TCGv res = gen_get_addr(zero, cpu_r[31], cpu_r[30]);
+tcg_temp_free_i32(zero);
+return res;
+}
}
/*
@@ -1868,12 +1883,12 @@ static bool trans_LDDY(DisasContext *ctx, arg_LDDY *a)
static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_load(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1883,12 +1898,12 @@ static bool trans_LDZ2(DisasContext *ctx, arg_LDZ2 *a)
static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_load(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -1898,7 +1913,7 @@ static bool trans_LDZ3(DisasContext *ctx, arg_LDZ3 *a)
static bool trans_LDDZ(DisasContext *ctx, arg_LDDZ *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_addi_tl(addr, addr, a->imm); /* addr = addr + q */
gen_data_load(ctx, Rd, addr);
@@ -2088,12 +2103,12 @@ static bool trans_STDY(DisasContext *ctx, arg_STDY *a)
static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
gen_data_store(ctx, Rd, addr);
tcg_gen_addi_tl(addr, addr, 1); /* addr = addr + 1 */
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2103,12 +2118,12 @@ static bool trans_STZ2(DisasContext *ctx, arg_STZ2 *a)
static bool trans_STZ3(DisasContext *ctx, arg_STZ3 *a)
{
TCGv Rd = cpu_r[a->rd];
-TCGv addr = gen_get_zaddr();
+TCGv addr = gen_get_zaddr(ctx, true);
tcg_gen_subi_tl(addr, addr, 1); /* addr = addr - 1 */
gen_data_store(ctx, Rd, addr);
-gen_set_zaddr(addr);
+gen_set_zaddr(ctx, addr, true);
tcg_temp_free_i32(addr);
@@ -2118,7 +2133,7 @@ static bool trans_STZ3(Dis
[PATCH 1/4] target/avr: fix long address calculation
AVR ELPMX instruction (and some others) use three registers to
form long 24-bit address from RAMPZ and two 8-bit registers.
RAMPZ stores shifted 8 bits like ff to simplify address calculation.
This patch fixes full address calculation in function gen_get_addr
by changing the mess in offsets of deposit tcg instructions.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
Reviewed-by: Michael Rolnik
---
target/avr/translate.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index e65b6008c0..c9a0a39c2d 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1572,8 +1572,8 @@ static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
{
TCGv addr = tcg_temp_new_i32();
-tcg_gen_deposit_tl(addr, M, H, 8, 8);
-tcg_gen_deposit_tl(addr, L, addr, 8, 16);
+tcg_gen_deposit_tl(addr, H, M, 8, 8);
+tcg_gen_deposit_tl(addr, addr, L, 0, 8);
return addr;
}
[PATCH 4/4] target/avr: fix interrupt processing
Interrupt bit vector has 64 bits, but interrupt vector is found with ctz32
function. This patch replaces it with ctz64.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/helper.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 156dde4e92..61ab6feb25 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -51,7 +51,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
if (interrupt_request & CPU_INTERRUPT_HARD) {
if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
-int index = ctz32(env->intsrc);
+int index = ctz64(env->intsrc);
cs->exception_index = EXCP_INT(index);
avr_cpu_do_interrupt(cs);
@@ -78,7 +78,7 @@ void avr_cpu_do_interrupt(CPUState *cs)
if (cs->exception_index == EXCP_RESET) {
vector = 0;
} else if (env->intsrc != 0) {
-vector = ctz32(env->intsrc) + 1;
+vector = ctz64(env->intsrc) + 1;
}
if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
[PATCH 3/4] target/avr: fix avr features processing
Bit vector for features has 64 bits. This patch fixes bit shifts in
avr_feature and set_avr_feature functions to be 64-bit too.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/cpu.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index cfdc0ecb70..8295e50fa0 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -166,12 +166,12 @@ vaddr avr_cpu_gdb_adjust_breakpoint(CPUState *cpu, vaddr
addr);
static inline int avr_feature(CPUAVRState *env, AVRFeature feature)
{
-return (env->features & (1U << feature)) != 0;
+return (env->features & (1ULL << feature)) != 0;
}
static inline void set_avr_feature(CPUAVRState *env, int feature)
{
-env->features |= (1U << feature);
+env->features |= (1ULL << feature);
}
#define cpu_list avr_cpu_list
[PATCH 0/4] AVR target fixes
--- Pavel Dovgalyuk (4): target/avr: fix long address calculation target/avr: implement small RAM/large RAM feature target/avr: fix avr features processing target/avr: fix interrupt processing target/avr/cpu.h | 6 ++-- target/avr/helper.c| 4 +-- target/avr/translate.c | 63 ++ 3 files changed, 45 insertions(+), 28 deletions(-) -- Pavel Dovgalyuk
[PATCH] target/avr: fix long address calculation
AVR ELPMX instruction (and some others) use three registers to
form long 24-bit address from RAMPZ and two 8-bit registers.
RAMPZ stores shifted 8 bits like ff to simplify address calculation.
This patch fixes full address calculation in function gen_get_addr
by changing the mess in offsets of deposit tcg instructions.
Signed-off-by: Pavel Dovgalyuk
---
target/avr/translate.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index e65b6008c0..c9a0a39c2d 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1572,8 +1572,8 @@ static TCGv gen_get_addr(TCGv H, TCGv M, TCGv L)
{
TCGv addr = tcg_temp_new_i32();
-tcg_gen_deposit_tl(addr, M, H, 8, 8);
-tcg_gen_deposit_tl(addr, L, addr, 8, 16);
+tcg_gen_deposit_tl(addr, H, M, 8, 8);
+tcg_gen_deposit_tl(addr, addr, L, 0, 8);
return addr;
}
[PATCH] tests/avocado: using several workers while testing
From: bakulinm
make check-avocado takes a lot of time, and avocado since version 91 has
multithreaded mode for running several tests simultaneously.
This patch allows to run "make check-avocado -j" to use all cores or,
for example, "make check-avocado -j4" to select number of workers to use.
By default ("make check-avocado") only one worker is used.
Changes:
1) Version of avocado in requirements.txt upgraded from 88.1 to <93
(LTS version is used, as mentioned here
https://avocado-framework.readthedocs.io/en/latest/releases/lts/92_0.html )
2) Makefile 4.1 (used in e.g. Ubuntu 18.04) doesn't provide number of jobs
in $MAKEFLAGS, so python script from here
https://stackoverflow.com/a/67247743/5936122 is used. Python script parses
Makeflags, communicates with jobserver to get the number of available jobs
and outputs it as a number, that is used as nrunner-max-parallel-tasks.
If -j is used, it outputs total number of cores. If -j is not used at all,
it returns 1. Limitations: script can probably report lower number of jobs
than provided, if some slots are already taken by other makefiles, so make
check-avocado should be run separately from other make tasks.
Signed-off-by: Maksim Bakulin
Signed-off-by: Pavel Dovgalyuk
---
tests/Makefile.include | 20 ++--
tests/jobs.py | 42 ++
tests/requirements.txt |2 +-
3 files changed, 61 insertions(+), 3 deletions(-)
create mode 100644 tests/jobs.py
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..6da5b6b45e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -93,6 +93,9 @@ TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python3
+# 1 parallel task is used by default
+NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks
+
ifndef AVOCADO_TESTS
AVOCADO_TESTS=tests/avocado
endif
@@ -111,6 +114,19 @@ quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
$(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
"VENVPIP","$1")
+
+get_avocado_max_tasks:
+# Make doesn't have easy ways to get number of jobs, using python for that
+# Python script communicates with jobserver to detect number of jobs
+ +$(eval MAKE_JOBS:=$$(shell python3 $(SRC_PATH)/tests/jobs.py
$(MAKEFLAGS)))
+ @echo Using $(MAKE_JOBS) jobs for avocado testing
+# If 2 jobs or more are used set nrunner-max-parallel-tasks accordingly
+ @if [ ${MAKE_JOBS} -gt 1 ] ; then \
+ $(eval NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks
$$(MAKE_JOBS)) true; \
+ fi
+
+$(TESTS_VENV_REQ): get_avocado_max_tasks
+
$(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
$(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
@@ -145,8 +161,8 @@ check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
$(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
--filter-by-tags-include-empty-key) \
$(AVOCADO_CMDLINE_TAGS) \
-$(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
-"AVOCADO", "tests/avocado")
+$(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS)
$(NRUNNER_MAX_TASKS), \
+"AVOCADO", "tests/avocado" \)
check-acceptance-deprecated-warning:
@echo
diff --git a/tests/jobs.py b/tests/jobs.py
new file mode 100644
index 00..a339192d97
--- /dev/null
+++ b/tests/jobs.py
@@ -0,0 +1,42 @@
+import argparse, os
+import sys
+
+def safe_int(s):
+try:
+return int(s)
+except:
+return -1
+
+class JobserverArgs:
+known_names = ["jobserver-fds","jobserver-auth"]
+def __init__(self):
+self.fds = "-1,-1"
+
+@staticmethod
+def from_argv():
+ja = JobserverArgs()
+parser = argparse.ArgumentParser()
+for name in JobserverArgs.known_names:
+parser.add_argument('--'+name, dest="fds")
+parser.parse_known_args(namespace=ja)
+return ja
+
+def get_fds(self):
+return tuple([safe_int(fd) for fd in (self.fds+",").split(",")][:2])
+
+fd_in, fd_out = JobserverArgs.from_argv().get_fds()
+
+if fd_in == -1 or fd_out == -1:
+# if no jobserver is used, but -j is present, use total number of cpu cores
+if '-j' in sys.argv:
+print(os.cpu_count())
+# use single thread
+else:
+print(1)
+else:
+os.set_blocking(fd_in, False)
+
+tokens = os.read(fd_in, 1024)
+os.write(fd_out, tokens)
+
+print(len(tokens)+1)
\ No newline at end of file
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..3b8c4d4706 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.t
Re: Intermittent hang on x86 replay avocado test?
On 04.11.2022 21:53, Peter Maydell wrote: On my machine this avocado test: ./build/all/tests/venv/bin/avocado run ./build/all/tests/avocado/replay_kernel.py:ReplayKernelNormal.test_x86_64_pc seems to hang intermittently (maybe 1 time in 3?). Does anybody else see this? Looking at the avocado logs suggests the record part runs fine but the replay part hangs very early in the kernel bootup. (Or possibly Avocado has got confused and isn't logging all the output. > I couldn't trigger it outside avocado. I sometimes have the same problem with one of the replay tests (I don't remember which one). It hangs with avocado, but does not hang when I run it with the same command line without avocado. It could be some replay issue (like infinite waiting for input in main_loop_wait), but I couldn't trigger this behavior with logging/debugging enabled. Pavel Dovgalyuk
Re: [PATCH v2 3/3] target/mips: Disable DSP ASE for Octeon68XX
On 31.10.2022 16:25, Jiaxun Yang wrote: I don't have access to Octeon68XX hardware but accroading to my investigation Octeon never had DSP ASE support. As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference Manual" CP0C3_DSPP is reserved bit and read as 0. Also I do have access to a Ubiquiti Edgerouter 4 which has Octeon CN7130 processor and I can confirm CP0C3_DSPP is read as 0 on that processor. Further more, in linux kernel: arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h cpu_has_dsp is overridden as 0. So I believe we shouldn't emulate DSP in QEMU as well. That's true. But there is one exception: LBX/LWX/LDX instruction. These are grouped into DSP extension in QEMU, but Octeon supports them. I've sent a patch for enabling these instructions. Reviewed-by: Pavel Dovgalyuk Signed-off-by: Jiaxun Yang Acked-by: Richard Henderson --- target/mips/cpu-defs.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc index 7f53c94ec8..480e60aeec 100644 --- a/target/mips/cpu-defs.c.inc +++ b/target/mips/cpu-defs.c.inc @@ -934,7 +934,7 @@ const mips_def_t mips_defs[] = (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) | (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP), .CP0_Config2 = MIPS_CONFIG2, -.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) , +.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA), .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) | (3U << CP0C4_MMUSizeExt), @@ -946,7 +946,7 @@ const mips_def_t mips_defs[] = .CP0_Status_rw_bitmask = 0x12F8, .SEGBITS = 42, .PABITS = 49, -.insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP, +.insn_flags = CPU_MIPS64R2 | INSN_OCTEON, .mmu_type = MMU_TYPE_R4000, },
[PATCH] target/mips: enable LBX/LWX/* instructions for Octeon
This patch changes condition and function name for enabling
indexed load instructions for Octeon vCPUs. Octeons do not
have DSP extension, but implement LBX-and-others.
Signed-off-by: Pavel Dovgalyuk
---
target/mips/tcg/translate.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index c3f92ea652..6248143c62 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -12173,12 +12173,16 @@ enum {
#include "nanomips_translate.c.inc"
/* MIPSDSP functions. */
-static void gen_mipsdsp_ld(DisasContext *ctx, uint32_t opc,
+
+/* Indexed load is not for DSP only */
+static void gen_mips_lx(DisasContext *ctx, uint32_t opc,
int rd, int base, int offset)
{
TCGv t0;
-check_dsp(ctx);
+if (!(ctx->insn_flags & INSN_OCTEON)) {
+check_dsp(ctx);
+}
t0 = tcg_temp_new();
if (base == 0) {
@@ -14523,7 +14527,7 @@ static void decode_opc_special3_legacy(CPUMIPSState
*env, DisasContext *ctx)
case OPC_LBUX:
case OPC_LHX:
case OPC_LWX:
-gen_mipsdsp_ld(ctx, op2, rd, rs, rt);
+gen_mips_lx(ctx, op2, rd, rs, rt);
break;
default:/* Invalid */
MIPS_INVAL("MASK LX");
Re: [PATCH v2 2/3] target/mips: Cast offset field of Octeon BBIT to int16_t
Acked-by: Pavel Dovgalyuk On 31.10.2022 16:25, Jiaxun Yang wrote: As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference Manual" offset field is signed 16 bit value. However arg_BBIT.offset is unsigned. We need to cast it as signed to do address calculation. Signed-off-by: Jiaxun Yang --- v2: Do casting in decodetree. (philmd) --- target/mips/tcg/octeon.decode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode index 8929ad088e..0c787cb498 100644 --- a/target/mips/tcg/octeon.decode +++ b/target/mips/tcg/octeon.decode @@ -12,7 +12,7 @@ # BBIT13210 . . %bbit_p 28:1 16:5 -BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p +BBIT 11 set:1 . 10 rs:5 . offset:s16 p=%bbit_p # Arithmetic # BADDU rd, rs, rt
Re: [PATCH 1/6] device-tree: add re-randomization helper function
On 10.10.2022 18:32, Peter Maydell wrote:
On Mon, 10 Oct 2022 at 16:21, Jason A. Donenfeld wrote:
On Mon, Oct 10, 2022 at 11:54:50AM +0100, Peter Maydell wrote:
The error is essentially the record-and-replay subsystem saying "the
replay just asked for a random number at point when the recording
did not ask for one, and so there's no 'this is what the number was'
info in the record".
I have had a quick look, and I think the reason for this is that
load_snapshot() ("reset the VM state to the snapshot state stored in the
disk image or migration stream") does a system reset. The replay
process involves a lot of "load state from a snapshot and play
forwards from there" operations. It doesn't expect that load_snapshot()
would result in something reading random data, but now that we are
calling qemu_guest_getrandom() in a reset hook, that happens.
Hmm... so this seems like a bug in the replay code then? Shouldn't that
reset handler get hit during both passes, so the entry should be in
each?
No, because record is just
"reset the system, record all the way to the end stop",
but replay is
"set the system to the point we want to start at by using
load_snapshot, play from there", and depending on the actions
you do in the debugger like reverse-continue we might repeatedly
do "reload that snapshot (implying a system reset) and play from there"
multiple times.
The idea of the patches is fdt randomization during reset, right?
But reset is used not only for real reboot, but also for restoring the
snapshots.
In the latter case it is like "just clear the hw registers to simplify
the initialization".
Therefore no other virtual hardware tried to read external data yet. And
random numbers are external to the machine, they come from the outer world.
It means that this is completely new reset case and new solution should
be found for it.
Pavel Dovgalyuk
[PATCH] tests/avocado: fix replay-linux test
Last line of the test is missing by accident. This patch fixes the script. Signed-off-by: Pavel Dovgalyuk --- tests/avocado/replay_linux.py |1 + 1 file changed, 1 insertion(+) diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py index 40e4f6908e..e1f9981a34 100644 --- a/tests/avocado/replay_linux.py +++ b/tests/avocado/replay_linux.py @@ -189,3 +189,4 @@ def test_virt_gicv3(self): self.run_rr(shift=3, args=(*self.get_common_args(), + "-machine", "virt,gic-version=3"))
Re: [PATCH for-7.1] icount: Take iothread lock when running QEMU timers
Tested-by: Pavel Dovgalyuk
On 01.08.2022 19:45, Peter Maydell wrote:
The function icount_prepare_for_run() is called with the iothread
unlocked, but it can call icount_notify_aio_contexts() which will
run qemu timer handlers. Those are supposed to be run only with
the iothread lock held, so take the lock while we do that.
Since icount mode runs everything on a single thread anyway,
not holding the lock is likely mostly not going to introduce
races, but it can cause us to trip over assertions that we
do hold the lock, such as the one reported in issue 1130.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1130
Signed-off-by: Peter Maydell
---
accel/tcg/tcg-accel-ops-icount.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c
index 8f1dda4344c..84cc7421be8 100644
--- a/accel/tcg/tcg-accel-ops-icount.c
+++ b/accel/tcg/tcg-accel-ops-icount.c
@@ -109,7 +109,13 @@ void icount_prepare_for_run(CPUState *cpu)
replay_mutex_lock();
if (cpu->icount_budget == 0) {
+/*
+ * We're called without the iothread lock, so must take it while
+ * we're calling timer handlers.
+ */
+qemu_mutex_lock_iothread();
icount_notify_aio_contexts();
+qemu_mutex_unlock_iothread();
}
}
Re: [PATCH v3 4/4] target/mips: introduce Cavium Octeon CPU model
On 06.07.2022 23:53, Philippe Mathieu-Daudé wrote: On 4/7/22 12:59, Pavel Dovgalyuk wrote: ping This is the only non-reviewed patch in the series. I've been looking for doc/datasheets but no luck (except the Linux kernel comments). What kind of testing are you doing? BTW, we found SDK here: https://github.com/amitmisra16/OCTEON-SDK On 20.06.2022 15:05, Pavel Dovgalyuk wrote: This patch adds Cavium Octeon 68XX vCPU which provides Octeon-specific instructions. Signed-off-by: Pavel Dovgalyuk -- v3 changes: - split the patch to instruction set introduction and new vCPU (suggested by Philippe Mathieu-Daudé) v2 changes: - vCPU name changed to Octeon68XX (suggested by Richard Henderson) --- target/mips/cpu-defs.c.inc | 28 1 file changed, 28 insertions(+)
Re: [PATCH v3 4/4] target/mips: introduce Cavium Octeon CPU model
On 06.07.2022 23:53, Philippe Mathieu-Daudé wrote: On 4/7/22 12:59, Pavel Dovgalyuk wrote: ping This is the only non-reviewed patch in the series. I've been looking for doc/datasheets but no luck (except the Linux kernel comments). What kind of testing are you doing? We compared the instruction emulation with the behavior on dev board. On 20.06.2022 15:05, Pavel Dovgalyuk wrote: This patch adds Cavium Octeon 68XX vCPU which provides Octeon-specific instructions. Signed-off-by: Pavel Dovgalyuk -- v3 changes: - split the patch to instruction set introduction and new vCPU (suggested by Philippe Mathieu-Daudé) v2 changes: - vCPU name changed to Octeon68XX (suggested by Richard Henderson) --- target/mips/cpu-defs.c.inc | 28 1 file changed, 28 insertions(+)
Re: [PATCH v3 4/4] target/mips: introduce Cavium Octeon CPU model
ping
This is the only non-reviewed patch in the series.
On 20.06.2022 15:05, Pavel Dovgalyuk wrote:
This patch adds Cavium Octeon 68XX vCPU which provides
Octeon-specific instructions.
Signed-off-by: Pavel Dovgalyuk
--
v3 changes:
- split the patch to instruction set introduction and new vCPU
(suggested by Philippe Mathieu-Daudé)
v2 changes:
- vCPU name changed to Octeon68XX (suggested by Richard Henderson)
---
target/mips/cpu-defs.c.inc | 28
1 file changed, 28 insertions(+)
diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index 582f940070..7f53c94ec8 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -921,6 +921,34 @@ const mips_def_t mips_defs[] =
.insn_flags = CPU_MIPS64R2 | ASE_DSP | ASE_DSP_R2,
.mmu_type = MMU_TYPE_R4000,
},
+{
+/*
+ * Octeon 68xx with MIPS64 Cavium Octeon features.
+ */
+.name = "Octeon68XX",
+.CP0_PRid = 0x000D9100,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+ (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (0x3F << CP0C1_MMU) |
+ (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+ (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+ (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) ,
+.CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) |
+ (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) |
+ (3U << CP0C4_MMUSizeExt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.CP0_PageGrain = (1 << CP0PG_ELPA),
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x12F8,
+.SEGBITS = 42,
+.PABITS = 49,
+.insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP,
+.mmu_type = MMU_TYPE_R4000,
+},
#endif
};
[PATCH v3 3/4] target/mips: implement Octeon-specific arithmetic instructions
This patch implements several Octeon-specific instructions:
- BADDU
- DMUL
- EXTS/EXTS32
- CINS/CINS32
- POP/DPOP
- SEQ/SEQI
- SNE/SNEI
Signed-off-by: Pavel Dovgalyuk
--
v3 changes:
- Fixed length field for EXTS/CINS
(bug found by Richard Henderson)
v2 changes:
- Using existing tcg instructions for exts, cins, pop
(suggested by Richard Henderson)
---
target/mips/tcg/octeon.decode | 26 ++
target/mips/tcg/octeon_translate.c | 155
2 files changed, 181 insertions(+)
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
index 8062715578..8929ad088e 100644
--- a/target/mips/tcg/octeon.decode
+++ b/target/mips/tcg/octeon.decode
@@ -13,3 +13,29 @@
%bbit_p 28:1 16:5
BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p
+
+# Arithmetic
+# BADDU rd, rs, rt
+# DMUL rd, rs, rt
+# EXTS rt, rs, p, lenm1
+# EXTS32 rt, rs, p, lenm1
+# CINS rt, rs, p, lenm1
+# CINS32 rt, rs, p, lenm1
+# DPOP rd, rs
+# POP rd, rs
+# SEQ rd, rs, rt
+# SEQI rt, rs, immediate
+# SNE rd, rs, rt
+# SNEI rt, rs, immediate
+
+@r3 .. rs:5 rt:5 rd:5 . ..
+%bitfield_p 0:1 6:5
+@bitfield.. rs:5 rt:5 lenm1:5 . . . p=%bitfield_p
+
+BADDU011100 . . . 0 101000 @r3
+DMUL 011100 . . . 0 11 @r3
+EXTS 011100 . . . . 11101 . @bitfield
+CINS 011100 . . . . 11001 . @bitfield
+POP 011100 rs:5 0 rd:5 0 10110 dw:1
+SEQNE011100 rs:5 rt:5 rd:5 0 10101 ne:1
+SEQNEI 011100 rs:5 rt:5 imm:s10 10111 ne:1
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
index 1558f74a8e..6a207d2e7e 100644
--- a/target/mips/tcg/octeon_translate.c
+++ b/target/mips/tcg/octeon_translate.c
@@ -44,3 +44,158 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
tcg_temp_free(t0);
return true;
}
+
+static bool trans_BADDU(DisasContext *ctx, arg_BADDU *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_add_tl(t0, t0, t1);
+tcg_gen_andi_i64(cpu_gpr[a->rd], t0, 0xff);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_DMUL(DisasContext *ctx, arg_DMUL *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_mul_i64(cpu_gpr[a->rd], t0, t1);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_EXTS(DisasContext *ctx, arg_EXTS *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+tcg_gen_sextract_tl(t0, t0, a->p, a->lenm1 + 1);
+gen_store_gpr(t0, a->rt);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_CINS(DisasContext *ctx, arg_CINS *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+tcg_gen_deposit_z_tl(t0, t0, a->p, a->lenm1 + 1);
+gen_store_gpr(t0, a->rt);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_POP(DisasContext *ctx, arg_POP *a)
+{
+TCGv t0;
+
+if (a->rd == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+if (!a->dw) {
+tcg_gen_andi_i64(t0, t0, 0x);
+}
+tcg_gen_ctpop_tl(t0, t0);
+gen_store_gpr(t0, a->rd);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_SEQNE(DisasContext *ctx, arg_SEQNE *a)
+{
+TCGv t0, t1;
+
+if (a->rd == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+if (a->ne) {
+tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr[a->rd], t1, t0);
+} else {
+tcg_gen_setcond_tl(TCG_COND_EQ, cpu_gpr[a->rd], t1, t0);
+}
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_SEQNEI(DisasContext *ctx, arg_SEQNEI *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+
+gen_load_gpr(t0, a->rs);
+
+/* Sign-extend to 64 bit value */
+target_ulong imm = a->imm;
+if (a->ne) {
+tcg_gen_setcondi_tl(TCG_COND_NE, cpu_gpr[a->rt], t0, imm);
+} else {
+tcg_gen_setcondi_tl(TCG_COND_EQ, cpu_gpr[a->rt], t0, imm);
+}
+
+tcg_temp_free(t0);
+
+return true;
+}
[PATCH v3 0/4] Cavium Octeon MIPS extensions
The following series includes emulation of the platform-specific MIPS extension for Cavium Octeon CPUS: - basic Octeon vCPU model - custom instruction decoder for Octeon - implementation of arithmetic and logic instructions v3 changes: - separated vCPU model definition and decodetree for Octeon (suggested by Philippe Mathieu-Daudé) - fixed length field for EXTS/CINS (bug found by Richard Henderson) v2 changes: - simplified instruction decoding and translation (suggested by Richard Henderson) --- Pavel Dovgalyuk (4): target/mips: introduce decodetree structure for Cavium Octeon extension target/mips: implement Octeon-specific BBIT instructions target/mips: implement Octeon-specific arithmetic instructions target/mips: introduce Cavium Octeon CPU model target/mips/cpu-defs.c.inc | 28 + target/mips/tcg/octeon.decode | 35 ++ target/mips/tcg/octeon_translate.c | 185 + 3 files changed, 248 insertions(+) -- Pavel Dovgalyuk
[PATCH v3 2/4] target/mips: implement Octeon-specific BBIT instructions
This patch introduces Octeon-specific decoder and implements
check-bit-and-jump instructions.
Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Richard Henderson
--
v3 changes:
- Split new decodetree and BBIT decoding into two patches
(suggested by Philippe Mathieu-Daudé)
v2 changes:
- Changed insn field description and simplified the jumps
(suggested by Richard Henderson)
---
target/mips/tcg/octeon.decode |9 +
target/mips/tcg/octeon_translate.c | 30 ++
2 files changed, 39 insertions(+)
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
index b21c735a6c..8062715578 100644
--- a/target/mips/tcg/octeon.decode
+++ b/target/mips/tcg/octeon.decode
@@ -4,3 +4,12 @@
#
# SPDX-License-Identifier: LGPL-2.1-or-later
#
+
+# Branch on bit set or clear
+# BBIT0 110010 . .
+# BBIT032110110 . .
+# BBIT1 111010 . .
+# BBIT13210 . .
+
+%bbit_p 28:1 16:5
+BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
index 8b5eb1a823..1558f74a8e 100644
--- a/target/mips/tcg/octeon_translate.c
+++ b/target/mips/tcg/octeon_translate.c
@@ -14,3 +14,33 @@
/* Include the auto-generated decoder. */
#include "decode-octeon.c.inc"
+
+static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
+{
+TCGv p;
+
+if (ctx->hflags & MIPS_HFLAG_BMASK) {
+LOG_DISAS("Branch in delay / forbidden slot at PC 0x"
+ TARGET_FMT_lx "\n", ctx->base.pc_next);
+generate_exception_end(ctx, EXCP_RI);
+return true;
+}
+
+/* Load needed operands */
+TCGv t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+
+p = tcg_constant_tl(1ULL << a->p);
+if (a->set) {
+tcg_gen_and_tl(bcond, p, t0);
+} else {
+tcg_gen_andc_tl(bcond, p, t0);
+}
+
+ctx->hflags |= MIPS_HFLAG_BC;
+ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
+ctx->hflags |= MIPS_HFLAG_BDS32;
+
+tcg_temp_free(t0);
+return true;
+}
[PATCH v3 0/2] Cavium Octeon MIPS extensions
The following series includes emulation of the platform-specific MIPS extension for Cavium Octeon CPUS: - basic Octeon vCPU model - custom instruction decoder for Octeon - implementation of arithmetic and logic instructions v3 changes: - separated vCPU model definition and decodetree for Octeon (suggested by Philippe Mathieu-Daudé) - fixed length field for EXTS/CINS (bug found by Richard Henderson) v2 changes: - simplified instruction decoding and translation (suggested by Richard Henderson) --- Pavel Dovgalyuk (2): target/mips: introduce Cavium Octeon CPU model target/mips: implement Octeon-specific arithmetic instructions target/mips/cpu-defs.c.inc | 28 1 file changed, 28 deletions(-) -- Pavel Dovgalyuk
[PATCH v3 4/4] target/mips: introduce Cavium Octeon CPU model
This patch adds Cavium Octeon 68XX vCPU which provides
Octeon-specific instructions.
Signed-off-by: Pavel Dovgalyuk
--
v3 changes:
- split the patch to instruction set introduction and new vCPU
(suggested by Philippe Mathieu-Daudé)
v2 changes:
- vCPU name changed to Octeon68XX (suggested by Richard Henderson)
---
target/mips/cpu-defs.c.inc | 28
1 file changed, 28 insertions(+)
diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index 582f940070..7f53c94ec8 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -921,6 +921,34 @@ const mips_def_t mips_defs[] =
.insn_flags = CPU_MIPS64R2 | ASE_DSP | ASE_DSP_R2,
.mmu_type = MMU_TYPE_R4000,
},
+{
+/*
+ * Octeon 68xx with MIPS64 Cavium Octeon features.
+ */
+.name = "Octeon68XX",
+.CP0_PRid = 0x000D9100,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+ (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (0x3F << CP0C1_MMU) |
+ (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+ (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+ (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) ,
+.CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) |
+ (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) |
+ (3U << CP0C4_MMUSizeExt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.CP0_PageGrain = (1 << CP0PG_ELPA),
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x12F8,
+.SEGBITS = 42,
+.PABITS = 49,
+.insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP,
+.mmu_type = MMU_TYPE_R4000,
+},
#endif
};
[PATCH v3 1/4] target/mips: introduce decodetree structure for Cavium Octeon extension
This patch adds decodetree for Cavium Octeon extension and
an instruction set extension flag for using it in CPU models.
Signed-off-by: Pavel Dovgalyuk
---
target/mips/mips-defs.h|1 +
target/mips/tcg/meson.build|2 ++
target/mips/tcg/octeon.decode |6 ++
target/mips/tcg/octeon_translate.c | 16
target/mips/tcg/translate.c|5 +
target/mips/tcg/translate.h|1 +
6 files changed, 31 insertions(+)
create mode 100644 target/mips/tcg/octeon.decode
create mode 100644 target/mips/tcg/octeon_translate.c
diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index 0a12d982a7..a6cebe0265 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -42,6 +42,7 @@
#define INSN_LOONGSON2E 0x0400ULL
#define INSN_LOONGSON2F 0x0800ULL
#define INSN_LOONGSON3A 0x1000ULL
+#define INSN_OCTEON 0x2000ULL
/*
* bits 52-63: vendor-specific ASEs
*/
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index 98003779ae..7ee969ec8f 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -3,6 +3,7 @@ gen = [
decodetree.process('msa.decode', extra_args: '--decode=decode_ase_msa'),
decodetree.process('tx79.decode', extra_args: '--static-decode=decode_tx79'),
decodetree.process('vr54xx.decode', extra_args:
'--decode=decode_ext_vr54xx'),
+ decodetree.process('octeon.decode', extra_args:
'--decode=decode_ext_octeon'),
]
mips_ss.add(gen)
@@ -24,6 +25,7 @@ mips_ss.add(files(
))
mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
'tx79_translate.c',
+ 'octeon_translate.c',
), if_false: files(
'mxu_translate.c',
))
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
new file mode 100644
index 00..b21c735a6c
--- /dev/null
+++ b/target/mips/tcg/octeon.decode
@@ -0,0 +1,6 @@
+# Octeon Architecture Module instruction set
+#
+# Copyright (C) 2022 Pavel Dovgalyuk
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
new file mode 100644
index 00..8b5eb1a823
--- /dev/null
+++ b/target/mips/tcg/octeon_translate.c
@@ -0,0 +1,16 @@
+/*
+ * Octeon-specific instructions translation routines
+ *
+ * Copyright (c) 2022 Pavel Dovgalyuk
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "tcg/tcg-op-gvec.h"
+#include "exec/helper-gen.h"
+#include "translate.h"
+
+/* Include the auto-generated decoder. */
+#include "decode-octeon.c.inc"
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 5f460fb687..f4345ce0c7 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15963,6 +15963,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx,
ctx->opcode)) {
return;
}
+#if defined(TARGET_MIPS64)
+if (cpu_supports_isa(env, INSN_OCTEON) && decode_ext_octeon(ctx,
ctx->opcode)) {
+return;
+}
+#endif
/* ISA extensions */
if (ase_msa_available(env) && decode_ase_msa(ctx, ctx->opcode)) {
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 9997fe2f3c..55053226ae 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -215,6 +215,7 @@ bool decode_ase_msa(DisasContext *ctx, uint32_t insn);
bool decode_ext_txx9(DisasContext *ctx, uint32_t insn);
#if defined(TARGET_MIPS64)
bool decode_ext_tx79(DisasContext *ctx, uint32_t insn);
+bool decode_ext_octeon(DisasContext *ctx, uint32_t insn);
#endif
bool decode_ext_vr54xx(DisasContext *ctx, uint32_t insn);
Re: [PATCH v2 3/3] target/mips: implement Octeon-specific arithmetic instructions
On 09.06.2022 18:53, Richard Henderson wrote:
On 6/9/22 01:23, Pavel Dovgalyuk wrote:
+static bool trans_BADDU(DisasContext *ctx, arg_BADDU *a)
+{
+ TCGv t0, t1;
+
+ if (a->rt == 0) {
+ /* nop */
+ return true;
+ }
I believe that we're standardizing on using gen_store_gpr, and not
checking for r0 everywhere.
I didn't remove this condition for making translation a bit faster.
Now there are no jumps or helpers, and I believe that optimizer
can remove everything in case of r0.
But if you insist, I'll remove this check.
+static bool trans_EXTS(DisasContext *ctx, arg_EXTS *a)
+{
+ TCGv t0;
+
+ if (a->rt == 0) {
+ /* nop */
+ return true;
+ }
+
+ t0 = tcg_temp_new();
+ gen_load_gpr(t0, a->rs);
+ tcg_gen_sextract_tl(t0, t0, a->p, a->lenm1);
a->lenm1 + 1.
+ tcg_gen_deposit_z_tl(t0, t0, a->p, a->lenm1);
Likewise.
r~
[PATCH v2 2/3] target/mips: implement Octeon-specific BBIT instructions
This patch introduces Octeon-specific decoder and implements
check-bit-and-jump instructions.
Signed-off-by: Pavel Dovgalyuk
--
v2 changes:
- Changed insn field description and simplified the jumps
(suggested by Richard Henderson)
---
target/mips/tcg/meson.build|2 ++
target/mips/tcg/octeon.decode | 15
target/mips/tcg/octeon_translate.c | 46
target/mips/tcg/translate.c|5
target/mips/tcg/translate.h|1 +
5 files changed, 69 insertions(+)
create mode 100644 target/mips/tcg/octeon.decode
create mode 100644 target/mips/tcg/octeon_translate.c
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index 98003779ae..7ee969ec8f 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -3,6 +3,7 @@ gen = [
decodetree.process('msa.decode', extra_args: '--decode=decode_ase_msa'),
decodetree.process('tx79.decode', extra_args: '--static-decode=decode_tx79'),
decodetree.process('vr54xx.decode', extra_args:
'--decode=decode_ext_vr54xx'),
+ decodetree.process('octeon.decode', extra_args:
'--decode=decode_ext_octeon'),
]
mips_ss.add(gen)
@@ -24,6 +25,7 @@ mips_ss.add(files(
))
mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
'tx79_translate.c',
+ 'octeon_translate.c',
), if_false: files(
'mxu_translate.c',
))
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
new file mode 100644
index 00..8062715578
--- /dev/null
+++ b/target/mips/tcg/octeon.decode
@@ -0,0 +1,15 @@
+# Octeon Architecture Module instruction set
+#
+# Copyright (C) 2022 Pavel Dovgalyuk
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+
+# Branch on bit set or clear
+# BBIT0 110010 . .
+# BBIT032110110 . .
+# BBIT1 111010 . .
+# BBIT13210 . .
+
+%bbit_p 28:1 16:5
+BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
new file mode 100644
index 00..1558f74a8e
--- /dev/null
+++ b/target/mips/tcg/octeon_translate.c
@@ -0,0 +1,46 @@
+/*
+ * Octeon-specific instructions translation routines
+ *
+ * Copyright (c) 2022 Pavel Dovgalyuk
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "tcg/tcg-op-gvec.h"
+#include "exec/helper-gen.h"
+#include "translate.h"
+
+/* Include the auto-generated decoder. */
+#include "decode-octeon.c.inc"
+
+static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
+{
+TCGv p;
+
+if (ctx->hflags & MIPS_HFLAG_BMASK) {
+LOG_DISAS("Branch in delay / forbidden slot at PC 0x"
+ TARGET_FMT_lx "\n", ctx->base.pc_next);
+generate_exception_end(ctx, EXCP_RI);
+return true;
+}
+
+/* Load needed operands */
+TCGv t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+
+p = tcg_constant_tl(1ULL << a->p);
+if (a->set) {
+tcg_gen_and_tl(bcond, p, t0);
+} else {
+tcg_gen_andc_tl(bcond, p, t0);
+}
+
+ctx->hflags |= MIPS_HFLAG_BC;
+ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
+ctx->hflags |= MIPS_HFLAG_BDS32;
+
+tcg_temp_free(t0);
+return true;
+}
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 6de5b66650..4f41a9b00a 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15963,6 +15963,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx,
ctx->opcode)) {
return;
}
+#if defined(TARGET_MIPS64)
+if (cpu_supports_isa(env, INSN_OCTEON) && decode_ext_octeon(ctx,
ctx->opcode)) {
+return;
+}
+#endif
/* ISA extensions */
if (ase_msa_available(env) && decode_ase_msa(ctx, ctx->opcode)) {
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 9997fe2f3c..55053226ae 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -215,6 +215,7 @@ bool decode_ase_msa(DisasContext *ctx, uint32_t insn);
bool decode_ext_txx9(DisasContext *ctx, uint32_t insn);
#if defined(TARGET_MIPS64)
bool decode_ext_tx79(DisasContext *ctx, uint32_t insn);
+bool decode_ext_octeon(DisasContext *ctx, uint32_t insn);
#endif
bool decode_ext_vr54xx(DisasContext *ctx, uint32_t insn);
[PATCH v2 1/3] target/mips: introduce Cavium Octeon CPU model
This patch adds Cavium Octeon vCPU for providing
Octeon-specific instructions.
Signed-off-by: Pavel Dovgalyuk
--
v2 changes:
- vCPU name changed to Octeon68XX (suggested by Richard Henderson)
---
target/mips/cpu-defs.c.inc | 28
target/mips/mips-defs.h|1 +
2 files changed, 29 insertions(+)
diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index 582f940070..7f53c94ec8 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -921,6 +921,34 @@ const mips_def_t mips_defs[] =
.insn_flags = CPU_MIPS64R2 | ASE_DSP | ASE_DSP_R2,
.mmu_type = MMU_TYPE_R4000,
},
+{
+/*
+ * Octeon 68xx with MIPS64 Cavium Octeon features.
+ */
+.name = "Octeon68XX",
+.CP0_PRid = 0x000D9100,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+ (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (0x3F << CP0C1_MMU) |
+ (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+ (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+ (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) ,
+.CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) |
+ (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) |
+ (3U << CP0C4_MMUSizeExt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.CP0_PageGrain = (1 << CP0PG_ELPA),
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x12F8,
+.SEGBITS = 42,
+.PABITS = 49,
+.insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP,
+.mmu_type = MMU_TYPE_R4000,
+},
#endif
};
diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index 0a12d982a7..a6cebe0265 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -42,6 +42,7 @@
#define INSN_LOONGSON2E 0x0400ULL
#define INSN_LOONGSON2F 0x0800ULL
#define INSN_LOONGSON3A 0x1000ULL
+#define INSN_OCTEON 0x2000ULL
/*
* bits 52-63: vendor-specific ASEs
*/
[PATCH v2 0/3] Cavium Octeon MIPS extensions
The following series includes emulation of the platform-specific MIPS extension for Cavium Octeon CPUS: - basic Octeon vCPU model - custom instruction decoder for Octeon - implementation of arithmetic and logic instructions v2 changes: - simplified instruction decoding and translation (suggested by Richard Henderson) --- Pavel Dovgalyuk (3): target/mips: introduce Cavium Octeon CPU model target/mips: implement Octeon-specific BBIT instructions target/mips: implement Octeon-specific arithmetic instructions target/mips/tcg/meson.build| 2 + target/mips/tcg/octeon.decode | 41 ++ target/mips/tcg/octeon_translate.c | 201 + target/mips/tcg/translate.c| 5 + target/mips/tcg/translate.h| 1 + 5 files changed, 250 insertions(+) create mode 100644 target/mips/tcg/octeon.decode create mode 100644 target/mips/tcg/octeon_translate.c -- Pavel Dovgalyuk
[PATCH v2 3/3] target/mips: implement Octeon-specific arithmetic instructions
This patch implements several Octeon-specific instructions:
- BADDU
- DMUL
- EXTS/EXTS32
- CINS/CINS32
- POP/DPOP
- SEQ/SEQI
- SNE/SNEI
Signed-off-by: Pavel Dovgalyuk
--
v2 changes:
- Using existing tcg instructions for exts, cins, pop
(suggested by Richard Henderson)
---
target/mips/tcg/octeon.decode | 26 ++
target/mips/tcg/octeon_translate.c | 155
2 files changed, 181 insertions(+)
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
index 8062715578..8929ad088e 100644
--- a/target/mips/tcg/octeon.decode
+++ b/target/mips/tcg/octeon.decode
@@ -13,3 +13,29 @@
%bbit_p 28:1 16:5
BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p
+
+# Arithmetic
+# BADDU rd, rs, rt
+# DMUL rd, rs, rt
+# EXTS rt, rs, p, lenm1
+# EXTS32 rt, rs, p, lenm1
+# CINS rt, rs, p, lenm1
+# CINS32 rt, rs, p, lenm1
+# DPOP rd, rs
+# POP rd, rs
+# SEQ rd, rs, rt
+# SEQI rt, rs, immediate
+# SNE rd, rs, rt
+# SNEI rt, rs, immediate
+
+@r3 .. rs:5 rt:5 rd:5 . ..
+%bitfield_p 0:1 6:5
+@bitfield.. rs:5 rt:5 lenm1:5 . . . p=%bitfield_p
+
+BADDU011100 . . . 0 101000 @r3
+DMUL 011100 . . . 0 11 @r3
+EXTS 011100 . . . . 11101 . @bitfield
+CINS 011100 . . . . 11001 . @bitfield
+POP 011100 rs:5 0 rd:5 0 10110 dw:1
+SEQNE011100 rs:5 rt:5 rd:5 0 10101 ne:1
+SEQNEI 011100 rs:5 rt:5 imm:s10 10111 ne:1
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
index 1558f74a8e..0470605e1e 100644
--- a/target/mips/tcg/octeon_translate.c
+++ b/target/mips/tcg/octeon_translate.c
@@ -44,3 +44,158 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
tcg_temp_free(t0);
return true;
}
+
+static bool trans_BADDU(DisasContext *ctx, arg_BADDU *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_add_tl(t0, t0, t1);
+tcg_gen_andi_i64(cpu_gpr[a->rd], t0, 0xff);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_DMUL(DisasContext *ctx, arg_DMUL *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_mul_i64(cpu_gpr[a->rd], t0, t1);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_EXTS(DisasContext *ctx, arg_EXTS *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+tcg_gen_sextract_tl(t0, t0, a->p, a->lenm1);
+gen_store_gpr(t0, a->rt);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_CINS(DisasContext *ctx, arg_CINS *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+tcg_gen_deposit_z_tl(t0, t0, a->p, a->lenm1);
+gen_store_gpr(t0, a->rt);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_POP(DisasContext *ctx, arg_POP *a)
+{
+TCGv t0;
+
+if (a->rd == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+if (!a->dw) {
+tcg_gen_andi_i64(t0, t0, 0x);
+}
+tcg_gen_ctpop_tl(t0, t0);
+gen_store_gpr(t0, a->rd);
+tcg_temp_free(t0);
+
+return true;
+}
+
+static bool trans_SEQNE(DisasContext *ctx, arg_SEQNE *a)
+{
+TCGv t0, t1;
+
+if (a->rd == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+if (a->ne) {
+tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr[a->rd], t1, t0);
+} else {
+tcg_gen_setcond_tl(TCG_COND_EQ, cpu_gpr[a->rd], t1, t0);
+}
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_SEQNEI(DisasContext *ctx, arg_SEQNEI *a)
+{
+TCGv t0;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+
+gen_load_gpr(t0, a->rs);
+
+/* Sign-extend to 64 bit value */
+target_ulong imm = a->imm;
+if (a->ne) {
+tcg_gen_setcondi_tl(TCG_COND_NE, cpu_gpr[a->rt], t0, imm);
+} else {
+tcg_gen_setcondi_tl(TCG_COND_EQ, cpu_gpr[a->rt], t0, imm);
+}
+
+tcg_temp_free(t0);
+
+return true;
+}
Re: [PATCH 2/3] target/mips: implement Octeon-specific BBIT instructions
On 07.06.2022 20:06, Richard Henderson wrote:
On 6/7/22 01:59, Pavel Dovgalyuk wrote:
+# Branch on bit set or clear
+# BBIT0 110010 . .
+# BBIT032 110110 . .
+# BBIT1 111010 . .
+# BBIT132 10 . .
+
+BBIT 11 set:1 shift:1 10 rs:5 p:5 offset:16
shift + p are logically one field -- all you need to do is concatenate
them.
%bbit_p 28:1 16:5
BBIT 11 set:1 . 10 rs:5 . offset:16 p=%bbit_p
Thank you.
I will make a new version soon.
+ if (ctx->hflags & MIPS_HFLAG_BMASK) {
+#ifdef MIPS_DEBUG_DISAS
+ LOG_DISAS("Branch in delay / forbidden slot at PC 0x"
+ TARGET_FMT_lx "\n", ctx->base.pc_next);
+#endif
Ifdef isn't needed -- it's always defined, even to 0.
+ tcg_gen_andi_tl(t0, t0, 1ULL << p);
+
+ /* Jump conditions */
+ if (a->set) {
+ tcg_gen_setcondi_tl(TCG_COND_NE, bcond, t0, 0);
+ } else {
+ tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, t0, 0);
+ }
You don't need to produce a boolean, MIPS_HFLAG_BC tests for non-zero.
Thus you can simplify this to
p = tcg_constant_tl(1ull << a->p);
if (a->set) {
tcg_gen_and_tl(bcond, rs, p);
} else {
tcg_gen_andc_tl(bcond, p, rs);
}
r~
[PATCH 3/3] target/mips: implement Octeon-specific arithmetic instructions
This patch implements several Octeon-specific instructions:
- BADDU
- DMUL
- EXTS/EXTS32
- CINS/CINS32
- POP/DPOP
- SEQ/SEQI
- SNE/SNEI
Signed-off-by: Pavel Dovgalyuk
---
target/mips/helper.h|1
target/mips/tcg/meson.build |1
target/mips/tcg/octeon.decode | 25 +
target/mips/tcg/octeon_helper.c | 22
target/mips/tcg/octeon_helper.h.inc | 10 ++
target/mips/tcg/octeon_translate.c | 182 +++
6 files changed, 241 insertions(+)
create mode 100644 target/mips/tcg/octeon_helper.c
create mode 100644 target/mips/tcg/octeon_helper.h.inc
diff --git a/target/mips/helper.h b/target/mips/helper.h
index de32d82e98..d68abdeac1 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -597,3 +597,4 @@ DEF_HELPER_FLAGS_2(rddsp, 0, tl, tl, env)
/* Vendor extensions */
#include "tcg/vr54xx_helper.h.inc"
+#include "tcg/octeon_helper.h.inc"
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index 7ee969ec8f..1852366846 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -26,6 +26,7 @@ mips_ss.add(files(
mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
'tx79_translate.c',
'octeon_translate.c',
+ 'octeon_helper.c',
), if_false: files(
'mxu_translate.c',
))
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
index c06d576292..ababf59e42 100644
--- a/target/mips/tcg/octeon.decode
+++ b/target/mips/tcg/octeon.decode
@@ -12,3 +12,28 @@
# BBIT13210 . .
BBIT 11 set:1 shift:1 10 rs:5 p:5 offset:16
+
+# Arithmetic
+# BADDU rd, rs, rt
+# DMUL rd, rs, rt
+# EXTS rt, rs, p, lenm1
+# EXTS32 rt, rs, p, lenm1
+# CINS rt, rs, p, lenm1
+# CINS32 rt, rs, p, lenm1
+# DPOP rd, rs
+# POP rd, rs
+# SEQ rd, rs, rt
+# SEQI rt, rs, immediate
+# SNE rd, rs, rt
+# SNEI rt, rs, immediate
+
+@r3 .. rs:5 rt:5 rd:5 . ..
+@bitfield.. rs:5 rt:5 lenm1:5 p:5 . shift:1
+
+BADDU011100 . . . 0 101000 @r3
+DMUL 011100 . . . 0 11 @r3
+EXTS 011100 . . . . 11101 . @bitfield
+CINS 011100 . . . . 11001 . @bitfield
+POP 011100 rs:5 0 rd:5 0 10110 dw:1
+SEQNE011100 rs:5 rt:5 rd:5 0 10101 ne:1
+SEQNEI 011100 rs:5 rt:5 imm:s10 10111 ne:1
diff --git a/target/mips/tcg/octeon_helper.c b/target/mips/tcg/octeon_helper.c
new file mode 100644
index 00..e9650c58bd
--- /dev/null
+++ b/target/mips/tcg/octeon_helper.c
@@ -0,0 +1,22 @@
+/*
+ * MIPS Octeon emulation helpers
+ *
+ * Copyright (c) 2022 Pavel Dovgalyuk
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/helper-proto.h"
+
+target_ulong helper_pop(target_ulong arg)
+{
+int i;
+int res = 0;
+for (i = 0 ; i < 64 ; ++i) {
+res += arg & 1;
+arg >>= 1;
+}
+
+return res;
+}
diff --git a/target/mips/tcg/octeon_helper.h.inc
b/target/mips/tcg/octeon_helper.h.inc
new file mode 100644
index 00..cfc051ef47
--- /dev/null
+++ b/target/mips/tcg/octeon_helper.h.inc
@@ -0,0 +1,10 @@
+/*
+ * MIPS Octeon emulation helpers
+ *
+ * Copyright (c) 2022 Pavel Dovgalyuk
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#if defined(TARGET_MIPS64)
+DEF_HELPER_1(pop, tl, tl)
+#endif
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
index bd87066b01..c4ef3e5bcb 100644
--- a/target/mips/tcg/octeon_translate.c
+++ b/target/mips/tcg/octeon_translate.c
@@ -51,3 +51,185 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
tcg_temp_free(t0);
return true;
}
+
+static bool trans_BADDU(DisasContext *ctx, arg_BADDU *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_add_tl(t0, t0, t1);
+tcg_gen_andi_i64(cpu_gpr[a->rd], t0, 0xff);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_DMUL(DisasContext *ctx, arg_DMUL *a)
+{
+TCGv t0, t1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+gen_load_gpr(t1, a->rt);
+
+tcg_gen_mul_i64(cpu_gpr[a->rd], t0, t1);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+
+return true;
+}
+
+static bool trans_EXTS(DisasContext *ctx, arg_EXTS *a)
+{
+TCGv t0, t1;
+int p;
+TCGLabel *l1;
+
+if (a->rt == 0) {
+/* nop */
+return true;
+}
+
+p = a->p;
+if (a->shift) {
+p += 32;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t1, a->r
[PATCH 2/3] target/mips: implement Octeon-specific BBIT instructions
This patch introduces Octeon-specific decoder and implements
check-bit-and-jump instructions.
Signed-off-by: Pavel Dovgalyuk
---
target/mips/tcg/meson.build|2 +
target/mips/tcg/octeon.decode | 14 ++
target/mips/tcg/octeon_translate.c | 53
target/mips/tcg/translate.c|5 +++
target/mips/tcg/translate.h|1 +
5 files changed, 75 insertions(+)
create mode 100644 target/mips/tcg/octeon.decode
create mode 100644 target/mips/tcg/octeon_translate.c
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index 98003779ae..7ee969ec8f 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -3,6 +3,7 @@ gen = [
decodetree.process('msa.decode', extra_args: '--decode=decode_ase_msa'),
decodetree.process('tx79.decode', extra_args: '--static-decode=decode_tx79'),
decodetree.process('vr54xx.decode', extra_args:
'--decode=decode_ext_vr54xx'),
+ decodetree.process('octeon.decode', extra_args:
'--decode=decode_ext_octeon'),
]
mips_ss.add(gen)
@@ -24,6 +25,7 @@ mips_ss.add(files(
))
mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
'tx79_translate.c',
+ 'octeon_translate.c',
), if_false: files(
'mxu_translate.c',
))
diff --git a/target/mips/tcg/octeon.decode b/target/mips/tcg/octeon.decode
new file mode 100644
index 00..c06d576292
--- /dev/null
+++ b/target/mips/tcg/octeon.decode
@@ -0,0 +1,14 @@
+# Octeon Architecture Module instruction set
+#
+# Copyright (C) 2022 Pavel Dovgalyuk
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+
+# Branch on bit set or clear
+# BBIT0 110010 . .
+# BBIT032110110 . .
+# BBIT1 111010 . .
+# BBIT13210 . .
+
+BBIT 11 set:1 shift:1 10 rs:5 p:5 offset:16
diff --git a/target/mips/tcg/octeon_translate.c
b/target/mips/tcg/octeon_translate.c
new file mode 100644
index 00..bd87066b01
--- /dev/null
+++ b/target/mips/tcg/octeon_translate.c
@@ -0,0 +1,53 @@
+/*
+ * Octeon-specific instructions translation routines
+ *
+ * Copyright (c) 2022 Pavel Dovgalyuk
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "tcg/tcg-op-gvec.h"
+#include "exec/helper-gen.h"
+#include "translate.h"
+
+/* Include the auto-generated decoder. */
+#include "decode-octeon.c.inc"
+
+static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
+{
+int p = a->p;
+
+if (ctx->hflags & MIPS_HFLAG_BMASK) {
+#ifdef MIPS_DEBUG_DISAS
+LOG_DISAS("Branch in delay / forbidden slot at PC 0x"
+ TARGET_FMT_lx "\n", ctx->base.pc_next);
+#endif
+generate_exception_end(ctx, EXCP_RI);
+return true;
+}
+
+/* Load needed operands */
+TCGv t0 = tcg_temp_new();
+gen_load_gpr(t0, a->rs);
+
+if (a->shift) {
+p += 32;
+}
+tcg_gen_andi_tl(t0, t0, 1ULL << p);
+
+/* Jump conditions */
+if (a->set) {
+tcg_gen_setcondi_tl(TCG_COND_NE, bcond, t0, 0);
+} else {
+tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, t0, 0);
+}
+
+ctx->hflags |= MIPS_HFLAG_BC;
+ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
+ctx->hflags |= MIPS_HFLAG_BDS32;
+
+tcg_temp_free(t0);
+return true;
+}
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 6de5b66650..4f41a9b00a 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15963,6 +15963,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx,
ctx->opcode)) {
return;
}
+#if defined(TARGET_MIPS64)
+if (cpu_supports_isa(env, INSN_OCTEON) && decode_ext_octeon(ctx,
ctx->opcode)) {
+return;
+}
+#endif
/* ISA extensions */
if (ase_msa_available(env) && decode_ase_msa(ctx, ctx->opcode)) {
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 9997fe2f3c..55053226ae 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -215,6 +215,7 @@ bool decode_ase_msa(DisasContext *ctx, uint32_t insn);
bool decode_ext_txx9(DisasContext *ctx, uint32_t insn);
#if defined(TARGET_MIPS64)
bool decode_ext_tx79(DisasContext *ctx, uint32_t insn);
+bool decode_ext_octeon(DisasContext *ctx, uint32_t insn);
#endif
bool decode_ext_vr54xx(DisasContext *ctx, uint32_t insn);
[PATCH 1/3] target/mips: introduce generic Cavium Octeon CPU model
This patch adds generic Octeon vCPU for providing
Octeon-specific instructions.
Signed-off-by: Pavel Dovgalyuk
---
target/mips/cpu-defs.c.inc | 30 ++
target/mips/mips-defs.h|1 +
2 files changed, 31 insertions(+)
diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index 582f940070..a60e48433c 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -921,6 +921,36 @@ const mips_def_t mips_defs[] =
.insn_flags = CPU_MIPS64R2 | ASE_DSP | ASE_DSP_R2,
.mmu_type = MMU_TYPE_R4000,
},
+{
+/*
+ * A generic CPU providing MIPS64 Cavium Octeon features.
+ * PRid is taken from Octeon 68xx CPUs
+ * FIXME: Eventually this should be replaced by a real CPU model.
+ */
+.name = "Octeon",
+.CP0_PRid = 0x000D9100,
+.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
+ (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (0x3F << CP0C1_MMU) |
+ (1 << CP0C1_IS) | (4 << CP0C1_IL) | (1 << CP0C1_IA) |
+ (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
+ (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) ,
+.CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) |
+ (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) |
+ (3U << CP0C4_MMUSizeExt),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 4,
+.CP0_PageGrain = (1 << CP0PG_ELPA),
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x12F8,
+.SEGBITS = 42,
+.PABITS = 49,
+.insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP,
+.mmu_type = MMU_TYPE_R4000,
+},
#endif
};
diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index 0a12d982a7..a6cebe0265 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -42,6 +42,7 @@
#define INSN_LOONGSON2E 0x0400ULL
#define INSN_LOONGSON2F 0x0800ULL
#define INSN_LOONGSON3A 0x1000ULL
+#define INSN_OCTEON 0x2000ULL
/*
* bits 52-63: vendor-specific ASEs
*/
[PATCH 0/3] Cavium Octeon MIPS extensions
The following series includes emulation of the platform-specific MIPS extension for Cavium Octeon CPUS: - basic Octeon vCPU model - custom instruction decoder for Octeon - implementation of arithmetic and logic instructions --- Pavel Dovgalyuk (3): target/mips: introduce generic Cavium Octeon CPU model target/mips: implement Octeon-specific BBIT instructions target/mips: implement Octeon-specific arithmetic instructions target/mips/helper.h| 1 + target/mips/tcg/meson.build | 3 + target/mips/tcg/octeon.decode | 39 + target/mips/tcg/octeon_helper.c | 22 +++ target/mips/tcg/octeon_helper.h.inc | 10 ++ target/mips/tcg/octeon_translate.c | 235 target/mips/tcg/translate.c | 5 + target/mips/tcg/translate.h | 1 + 8 files changed, 316 insertions(+) create mode 100644 target/mips/tcg/octeon.decode create mode 100644 target/mips/tcg/octeon_helper.c create mode 100644 target/mips/tcg/octeon_helper.h.inc create mode 100644 target/mips/tcg/octeon_translate.c -- Pavel Dovgalyuk
