[PATCH v6 3/6] net/colo-compare.c: Fix deadlock in compare_chr_send
The chr_out chardev is connected to a filter-redirector
running in the main loop. qemu_chr_fe_write_all might block
here in compare_chr_send if the (socket-)buffer is full.
If another filter-redirector in the main loop want's to
send data to chr_pri_in it might also block if the buffer
is full. This leads to a deadlock because both event loops
get blocked.
Fix this by converting compare_chr_send to a coroutine and
putting the packets in a send queue.
Signed-off-by: Lukas Straub
Reviewed-by: Zhang Chen
Tested-by: Zhang Chen
---
net/colo-compare.c | 193 ++---
net/colo.c | 7 ++
net/colo.h | 1 +
3 files changed, 156 insertions(+), 45 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 1de4220fe2..e47c8c6049 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -32,6 +32,9 @@
#include "migration/migration.h"
#include "util.h"
+#include "block/aio-wait.h"
+#include "qemu/coroutine.h"
+
#define TYPE_COLO_COMPARE "colo-compare"
#define COLO_COMPARE(obj) \
OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
@@ -77,6 +80,23 @@ static int event_unhandled_count;
*|packet | |packet +|packet | |packet +
*++ ++++ ++
*/
+
+typedef struct SendCo {
+Coroutine *co;
+struct CompareState *s;
+CharBackend *chr;
+GQueue send_list;
+bool notify_remote_frame;
+bool done;
+int ret;
+} SendCo;
+
+typedef struct SendEntry {
+uint32_t size;
+uint32_t vnet_hdr_len;
+uint8_t *buf;
+} SendEntry;
+
typedef struct CompareState {
Object parent;
@@ -91,6 +111,8 @@ typedef struct CompareState {
SocketReadState pri_rs;
SocketReadState sec_rs;
SocketReadState notify_rs;
+SendCo out_sendco;
+SendCo notify_sendco;
bool vnet_hdr;
uint32_t compare_timeout;
uint32_t expired_scan_cycle;
@@ -124,10 +146,11 @@ enum {
static int compare_chr_send(CompareState *s,
-const uint8_t *buf,
+uint8_t *buf,
uint32_t size,
uint32_t vnet_hdr_len,
-bool notify_remote_frame);
+bool notify_remote_frame,
+bool zero_copy);
static bool packet_matches_str(const char *str,
const uint8_t *buf,
@@ -145,7 +168,7 @@ static void notify_remote_frame(CompareState *s)
char msg[] = "DO_CHECKPOINT";
int ret = 0;
-ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
+ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true, false);
if (ret < 0) {
error_report("Notify Xen COLO-frame failed");
}
@@ -272,12 +295,13 @@ static void colo_release_primary_pkt(CompareState *s,
Packet *pkt)
pkt->data,
pkt->size,
pkt->vnet_hdr_len,
- false);
+ false,
+ true);
if (ret < 0) {
error_report("colo send primary packet failed");
}
trace_colo_compare_main("packet same and release packet");
-packet_destroy(pkt, NULL);
+packet_destroy_partial(pkt, NULL);
}
/*
@@ -699,65 +723,115 @@ static void colo_compare_connection(void *opaque, void
*user_data)
}
}
-static int compare_chr_send(CompareState *s,
-const uint8_t *buf,
-uint32_t size,
-uint32_t vnet_hdr_len,
-bool notify_remote_frame)
+static void coroutine_fn _compare_chr_send(void *opaque)
{
+SendCo *sendco = opaque;
+CompareState *s = sendco->s;
int ret = 0;
-uint32_t len = htonl(size);
-if (!size) {
-return 0;
-}
+while (!g_queue_is_empty(&sendco->send_list)) {
+SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
+uint32_t len = htonl(entry->size);
-if (notify_remote_frame) {
-ret = qemu_chr_fe_write_all(&s->chr_notify_dev,
-(uint8_t *)&len,
-sizeof(len));
-} else {
-ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
-}
+ret = qemu_chr_fe_write_all(sendco->chr, (uint8_t *)&len, sizeof(len));
-if (ret != sizeof(len)) {
-goto err;
-}
+if (ret != sizeof(len)) {
+g_free(entry->buf);
+g_slice_free(SendEntry, entry);
+goto err;
+}
-if (s->vnet_hdr) {
-/*
- * We send vnet header len make other module(like filter-redirector)
- * know how to parse net packet correctly.
- */
-len = htonl(vnet_hdr_len);
+if (!sendco->notify_remote_frame && s->vnet_hdr) {
+/*
+
[PATCH v6 6/6] net/colo-compare.c: Correct ordering in complete and finalize
In colo_compare_complete, insert CompareState into net_compares
only after everything has been initialized.
In colo_compare_finalize, remove CompareState from net_compares
before anything is deinitialized.
Signed-off-by: Lukas Straub
Reviewed-by: Zhang Chen
---
net/colo-compare.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 64d2453450..160776d39e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1283,15 +1283,6 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
s->vnet_hdr);
}
-qemu_mutex_lock(&colo_compare_mutex);
-if (!colo_compare_active) {
-qemu_mutex_init(&event_mtx);
-qemu_cond_init(&event_complete_cond);
-colo_compare_active = true;
-}
-QTAILQ_INSERT_TAIL(&net_compares, s, next);
-qemu_mutex_unlock(&colo_compare_mutex);
-
s->out_sendco.s = s;
s->out_sendco.chr = &s->chr_out;
s->out_sendco.notify_remote_frame = false;
@@ -1314,6 +1305,16 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
connection_destroy);
colo_compare_iothread(s);
+
+qemu_mutex_lock(&colo_compare_mutex);
+if (!colo_compare_active) {
+qemu_mutex_init(&event_mtx);
+qemu_cond_init(&event_complete_cond);
+colo_compare_active = true;
+}
+QTAILQ_INSERT_TAIL(&net_compares, s, next);
+qemu_mutex_unlock(&colo_compare_mutex);
+
return;
}
@@ -1386,19 +1387,6 @@ static void colo_compare_finalize(Object *obj)
CompareState *s = COLO_COMPARE(obj);
CompareState *tmp = NULL;
-qemu_chr_fe_deinit(&s->chr_pri_in, false);
-qemu_chr_fe_deinit(&s->chr_sec_in, false);
-qemu_chr_fe_deinit(&s->chr_out, false);
-if (s->notify_dev) {
-qemu_chr_fe_deinit(&s->chr_notify_dev, false);
-}
-
-if (s->iothread) {
-colo_compare_timer_del(s);
-}
-
-qemu_bh_delete(s->event_bh);
-
qemu_mutex_lock(&colo_compare_mutex);
QTAILQ_FOREACH(tmp, &net_compares, next) {
if (tmp == s) {
@@ -1413,6 +1401,19 @@ static void colo_compare_finalize(Object *obj)
}
qemu_mutex_unlock(&colo_compare_mutex);
+qemu_chr_fe_deinit(&s->chr_pri_in, false);
+qemu_chr_fe_deinit(&s->chr_sec_in, false);
+qemu_chr_fe_deinit(&s->chr_out, false);
+if (s->notify_dev) {
+qemu_chr_fe_deinit(&s->chr_notify_dev, false);
+}
+
+if (s->iothread) {
+colo_compare_timer_del(s);
+}
+
+qemu_bh_delete(s->event_bh);
+
AioContext *ctx = iothread_get_aio_context(s->iothread);
aio_context_acquire(ctx);
AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
--
2.20.1
pgpcGEqiPOxU9.pgp
Description: OpenPGP digital signature
[PATCH] replay: notify the main loop when there are no instructions
When QEMU is executed in console mode without any external event sources,
main loop may sleep for a very long time. But in case of replay
there is another event source - event log.
This patch adds main loop notification when the vCPU loop has nothing
to do and main loop should process the inputs from the event log.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/cpus.c b/cpus.c
index 7ce0d569b3..b4d0d9f21b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1362,6 +1362,13 @@ static int64_t tcg_get_icount_limit(void)
}
}
+static void notify_aio_contexts(void)
+{
+/* Wake up other AioContexts. */
+qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
+qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
+}
+
static void handle_icount_deadline(void)
{
assert(qemu_in_vcpu_thread());
@@ -1370,9 +1377,7 @@ static void handle_icount_deadline(void)
QEMU_TIMER_ATTR_ALL);
if (deadline == 0) {
-/* Wake up other AioContexts. */
-qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
-qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
+notify_aio_contexts();
}
}
}
@@ -1395,6 +1400,10 @@ static void prepare_icount_for_run(CPUState *cpu)
cpu->icount_extra = cpu->icount_budget - insns_left;
replay_mutex_lock();
+
+if (cpu->icount_budget == 0 && replay_has_checkpoint()) {
+notify_aio_contexts();
+}
}
}
[PATCH v6 2/6] chardev/char.c: Use qemu_co_sleep_ns if in coroutine
This will be needed in the next patch so compare_chr_send can be
converted to a coroutine.
Signed-off-by: Lukas Straub
Reviewed-by: Marc-André Lureau
Reviewed-by: Zhang Chen
Reviewed-by: Philippe Mathieu-Daudé
---
chardev/char.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index e77564060d..5c8014199f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -38,6 +38,7 @@
#include "qemu/module.h"
#include "qemu/option.h"
#include "qemu/id.h"
+#include "qemu/coroutine.h"
#include "chardev/char-mux.h"
@@ -119,7 +120,11 @@ static int qemu_chr_write_buffer(Chardev *s,
retry:
res = cc->chr_write(s, buf + *offset, len - *offset);
if (res < 0 && errno == EAGAIN && write_all) {
-g_usleep(100);
+if (qemu_in_coroutine()) {
+qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 10);
+} else {
+g_usleep(100);
+}
goto retry;
}
--
2.20.1
pgpBP00jSUtTY.pgp
Description: OpenPGP digital signature
[PATCH v6 1/6] net/colo-compare.c: Create event_bh with the right AioContext
qemu_bh_new will set the bh to be executed in the main
loop. This causes crashes as colo_compare_handle_event assumes
that it has exclusive access the queues, which are also
concurrently accessed in the iothread.
Create the bh with the AioContext of the iothread to fulfill
these assumptions and fix the crashes. This is safe, because
the bh already takes the appropriate locks.
Signed-off-by: Lukas Straub
Reviewed-by: Zhang Chen
Reviewed-by: Derek Su
Tested-by: Derek Su
---
net/colo-compare.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 10c0239f9d..1de4220fe2 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -890,6 +890,7 @@ static void colo_compare_handle_event(void *opaque)
static void colo_compare_iothread(CompareState *s)
{
+AioContext *ctx = iothread_get_aio_context(s->iothread);
object_ref(OBJECT(s->iothread));
s->worker_context = iothread_get_g_main_context(s->iothread);
@@ -906,7 +907,7 @@ static void colo_compare_iothread(CompareState *s)
}
colo_compare_timer_init(s);
-s->event_bh = qemu_bh_new(colo_compare_handle_event, s);
+s->event_bh = aio_bh_new(ctx, colo_compare_handle_event, s);
}
static char *compare_get_pri_indev(Object *obj, Error **errp)
--
2.20.1
pgptfnvDYpkF5.pgp
Description: OpenPGP digital signature
[PATCH v6 0/6] colo-compare bugfixes
Hello Everyone, Here are fixes for bugs that I found in my tests. I have tested this with my test suite and everything works fine. Regards, Lukas Straub Version changes: v6: -don't touch softmmu/vl.c, use __contructor__ attribute instead -better wording v5: -initialize and use notify_sendco only if notify_dev is set v4: -fix potential deadlock with notify_remote_frame -avoid malloc and memcpy in many cases v3: -fix checkpatch.pl error v2: -better wording -fix performance-regression in patch 3 "net/colo-compare.c: Fix deadlock in compare_chr_send" -add more bugfixes Lukas Straub (6): net/colo-compare.c: Create event_bh with the right AioContext chardev/char.c: Use qemu_co_sleep_ns if in coroutine net/colo-compare.c: Fix deadlock in compare_chr_send net/colo-compare.c: Only hexdump packets if tracing is enabled net/colo-compare.c: Check that colo-compare is active net/colo-compare.c: Correct ordering in complete and finalize chardev/char.c | 7 +- net/colo-compare.c | 254 ++--- net/colo.c | 7 ++ net/colo.h | 1 + 4 files changed, 206 insertions(+), 63 deletions(-) -- 2.20.1 pgpxRwzURvRoG.pgp Description: OpenPGP digital signature
[PATCH v6 4/6] net/colo-compare.c: Only hexdump packets if tracing is enabled
Else the log will be flooded if there is a lot of network
traffic.
Signed-off-by: Lukas Straub
Reviewed-by: Zhang Chen
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
---
net/colo-compare.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index e47c8c6049..7886444cdf 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -483,10 +483,12 @@ sec:
g_queue_push_head(&conn->primary_list, ppkt);
g_queue_push_head(&conn->secondary_list, spkt);
-qemu_hexdump((char *)ppkt->data, stderr,
- "colo-compare ppkt", ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr,
- "colo-compare spkt", spkt->size);
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+qemu_hexdump((char *)ppkt->data, stderr,
+"colo-compare ppkt", ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr,
+"colo-compare spkt", spkt->size);
+}
colo_compare_inconsistency_notify(s);
}
--
2.20.1
pgpJjlTB5L_Wi.pgp
Description: OpenPGP digital signature
[PATCH] replay: fix replay shutdown for console mode
When QEMU is used without any graphical window,
QEMU execution is terminated with the signal (e.g., Ctrl-C).
Signal processing in QEMU does not include
qemu_system_shutdown_request call. That is why shutdown
event is not recorded by record/replay in this case.
This patch adds shutdown event to the end of the record log.
Now every replay will shutdown the machine at the end.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/replay/replay.c b/replay/replay.c
index 53edad1377..83ed9e0e24 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -366,6 +366,11 @@ void replay_finish(void)
/* finalize the file */
if (replay_file) {
if (replay_mode == REPLAY_MODE_RECORD) {
+/*
+ * Can't do it in the signal handler, therefore
+ * add shutdown event here for the case of Ctrl-C.
+ */
+replay_shutdown_request(SHUTDOWN_CAUSE_HOST_SIGNAL);
/* write end event */
replay_put_event(EVENT_END);
Re: [PATCH] replay: synchronize on every virtual timer callback
On 21.05.2020 16:22, Paolo Bonzini wrote: On 06/05/20 10:17, Pavel Dovgalyuk wrote: Sometimes virtual timer callbacks depend on order of virtual timer processing and warping of virtual clock. Therefore every callback should be logged to make replay deterministic. This patch creates a checkpoint before every virtual timer callback. With these checkpoints virtual timers processing and clock warping events order is completely deterministic. Signed-off-by: Pavel Dovgalyuk --- util/qemu-timer.c |5 + 1 file changed, 5 insertions(+) diff --git a/util/qemu-timer.c b/util/qemu-timer.c index d548d3c1ad..47833f338f 100644 --- a/util/qemu-timer.c +++ b/util/qemu-timer.c @@ -588,6 +588,11 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) qemu_mutex_lock(&timer_list->active_timers_lock); progress = true; +/* + * Callback may insert new checkpoints, therefore add new checkpoint + * for the virtual timers. + */ +need_replay_checkpoint = timer_list->clock->type == QEMU_CLOCK_VIRTUAL; You need to check replay_mode != REPLAY_MODE_NONE, either here or in the "if (need_replay_checkpoint)" above. If you choose the latter, you can remove the other "if (replay_mode != REPLAY_MODE_NONE)". I forgot about the changes that prohibited event processing for the virtual clock checkpoint. This allowed to make this part simpler, please check the new version. However, event processing still waits for refactoring. I'll do it after upstreaming the tests for the record/replay to prevent regression. Also, there is a comment that says that checkpointing "must only be done once since the clock value stays the same". Is that actually a "can" rather than a "must"? Should the central replay logic have something like a checkpoint count, that prevents adding back-to-back equal checkpoints? I don't really think that this happens very often, but it worth implementing. I'll try it later. Pavel Dovgalyuk
[PATCH v2] replay: synchronize on every virtual timer callback
Sometimes virtual timer callbacks depend on order
of virtual timer processing and warping of virtual clock.
Therefore every callback should be logged to make replay deterministic.
This patch creates a checkpoint before every virtual timer callback.
With these checkpoints virtual timers processing and clock warping
events order is completely deterministic.
Signed-off-by: Pavel Dovgalyuk
Acked-by: Alex Bennée
--
v2:
- remove mutex lock/unlock for virtual clock checkpoint since it is
not process any asynchronous events (commit
ca9759c2a92f528f256fef0e3922416f7bb47bf9)
- bump record/replay log file version
---
0 files changed
diff --git a/replay/replay.c b/replay/replay.c
index 706c7b4f4b..53edad1377 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -22,7 +22,7 @@
/* Current version of the replay mechanism.
Increase it when file format changes. */
-#define REPLAY_VERSION 0xe02009
+#define REPLAY_VERSION 0xe0200a
/* Size of replay log header */
#define HEADER_SIZE (sizeof(uint32_t) + sizeof(uint64_t))
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index b6575a2cd5..f62b4feecd 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -501,7 +501,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
bool progress = false;
QEMUTimerCB *cb;
void *opaque;
-bool need_replay_checkpoint = false;
if (!atomic_read(&timer_list->active_timers)) {
return false;
@@ -517,16 +516,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
break;
default:
case QEMU_CLOCK_VIRTUAL:
-if (replay_mode != REPLAY_MODE_NONE) {
-/* Checkpoint for virtual clock is redundant in cases where
- * it's being triggered with only non-EXTERNAL timers, because
- * these timers don't change guest state directly.
- * Since it has conditional dependence on specific timers, it is
- * subject to race conditions and requires special handling.
- * See below.
- */
-need_replay_checkpoint = true;
-}
break;
case QEMU_CLOCK_HOST:
if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
@@ -559,19 +548,16 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
*/
break;
}
-if (need_replay_checkpoint
-&& !(ts->attributes & QEMU_TIMER_ATTR_EXTERNAL)) {
-/* once we got here, checkpoint clock only once */
-need_replay_checkpoint = false;
+/* Checkpoint for virtual clock is redundant in cases where
+ * it's being triggered with only non-EXTERNAL timers, because
+ * these timers don't change guest state directly.
+ */
+if (replay_mode != REPLAY_MODE_NONE
+&& timer_list->clock->type == QEMU_CLOCK_VIRTUAL
+&& !(ts->attributes & QEMU_TIMER_ATTR_EXTERNAL)
+&& !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
qemu_mutex_unlock(&timer_list->active_timers_lock);
-if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
-goto out;
-}
-qemu_mutex_lock(&timer_list->active_timers_lock);
-/* The lock was released; start over again in case the list was
- * modified.
- */
-continue;
+goto out;
}
/* remove timer from the list before calling the callback */
Re: [PATCH v3 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()
22.05.2020 01:29, Eric Blake wrote:
On 4/30/20 6:10 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, prepare bdrv_co_do_copy_on_readv() now.
Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 6 +++---
block/trace-events | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 8bb4ea6285..6990d8cabe 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1088,7 +1088,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs,
int64_t offset,
}
static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
- int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset, int flags)
Widens from 32-bit to 63-bit. One caller:
bdrv_aligned_preadv() passes unsigned int (for now) - safe
{
BlockDriverState *bs = child->bs;
@@ -1103,11 +1103,11 @@ static int coroutine_fn
bdrv_co_do_copy_on_readv(BdrvChild *child,
BlockDriver *drv = bs->drv;
int64_t cluster_offset;
int64_t cluster_bytes;
- size_t skip_bytes;
+ int64_t skip_bytes;
int ret;
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
BDRV_REQUEST_MAX_BYTES);
- unsigned int progress = 0;
+ int64_t progress = 0;
bool skip_write;
Use of 'bytes', 'sskip_bytes', and 'progress' within the function:
bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset, &cluster_bytes);
- safe, takes int64_t. Pre-patch, cluster_bytes could be 33 bits (a misaligned
request just under UINT_MAX can expand to > UINT_MAX when aligned to clusters),
but only if bytes could be larger than our <2G cap that we use elsewhere. But
even if we relax that 2G cap in later patches, we should be okay even if 'bytes'
starts at larger than 32 bits, because we don't allow images that would overflow
INT64_MAX when rounded up to cluster boundaries
skip_bytes = offset - cluster_offset;
- actually oversized - the difference is never going to be larger than a
cluster (which is capped at 2M for qcow2, for example), but doesn't hurt that
it is now a 64-bit value
trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
- safe, tweaked in this patch
assert(progress >= bytes);
- safe: progress never exceeds pnum, and both variables are same type pre-
and post-patch
assert(skip_bytes < pnum);
- safe
qemu_iovec_from_buf(qiov, qiov_offset + progress,
bounce_buffer + skip_bytes,
MIN(pnum - skip_bytes, bytes - progress));
- tricky - pre-patch, pnum was int64_t, post-patch, we have three more
int64_t entities. Either way, we're passing int64_t to a size_t parameter,
which narrows on 64-bit. However, we're safe: this call is in a loop where
pnum is clamped at MAX_BOUNCE_BUFFER which is less than 32 bits, and the MIN()
here means we never overflow
ret = bdrv_driver_preadv(bs, offset + progress,
MIN(pnum - skip_bytes, bytes - progress),
qiov, qiov_offset + progress, 0);
- safe - takes int64_t (earlier in this series), and same analysis about the
MIN() picking something clamped at MAX_BOUNCE_BUFFER
progress += pnum - skip_bytes;
skip_bytes = 0;
- safe
Reviewed-by: Eric Blake
Thanks! Hmm. I'm afraid that I'll rebase this back to master, postponing "[PATCH v2
0/9] block/io: safer inc/dec in_flight sections", as I doubt that it is needed..
--
Best regards,
Vladimir
[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU
Got it!
BTW, you can confirm this is bug for qemu-kvm, right?
thank you!
Cliff
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879425
Title:
The thread of "CPU 0 /KVM" keeping 99.9%CPU
Status in QEMU:
Incomplete
Bug description:
Hi Expert:
The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by
qemu-kvm.
Notes:
for VM:
OS: RHEL8.1
CPU: 1
MEM:4G
For qemu-kvm(host kernel RHEL7):
1) version:
/usr/libexec/qemu-kvm -version
QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by
com "top -p VM_pro_ID"
PID UDER PR NI RES S % CPU %MEM TIME+COMMAND
872067 qemu 20 0 1.6g R 99.9 0.6 37:08.87 CPU 0/KVM
3) use "pstack 493307" and below is function trace
Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
#0 0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
#1 0x55ff34bdf4a9 in qemu_poll_ns ()
#2 0x55ff34be02a8 in main_loop_wait ()
#3 0x55ff348bfb1a in main ()
4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
Therefore, I think the thread "CPU 0/KVM" is in tight loop.
5) use reset can recover this issue. however, it will reoccurred again.
Current work around is increase one CPU for this VM, then issue is gone.
thanks
Cliff
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879425/+subscriptions
[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU
I think you should definitely try a newer version if available -
otherwise they'll likely refuse to help you, too (nobody wants to debug
old versions when bugs are already fixed in newer ones)
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879425
Title:
The thread of "CPU 0 /KVM" keeping 99.9%CPU
Status in QEMU:
Incomplete
Bug description:
Hi Expert:
The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by
qemu-kvm.
Notes:
for VM:
OS: RHEL8.1
CPU: 1
MEM:4G
For qemu-kvm(host kernel RHEL7):
1) version:
/usr/libexec/qemu-kvm -version
QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by
com "top -p VM_pro_ID"
PID UDER PR NI RES S % CPU %MEM TIME+COMMAND
872067 qemu 20 0 1.6g R 99.9 0.6 37:08.87 CPU 0/KVM
3) use "pstack 493307" and below is function trace
Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
#0 0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
#1 0x55ff34bdf4a9 in qemu_poll_ns ()
#2 0x55ff34be02a8 in main_loop_wait ()
#3 0x55ff348bfb1a in main ()
4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
Therefore, I think the thread "CPU 0/KVM" is in tight loop.
5) use reset can recover this issue. however, it will reoccurred again.
Current work around is increase one CPU for this VM, then issue is gone.
thanks
Cliff
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879425/+subscriptions
Re: [RFC 1/3] cpu-throttle: new module, extracted from cpus.c
> From: "Claudio Fontana" > Sent: Thursday, May 21, 2020 8:54:05 PM > > this is a first step in the refactoring of cpus.c. Could you maybe extend the commit message in the next version a little bit? ... say something about *what* you are moving to a separate file (and maybe why it is ok to move it), etc.? Thanks, Thomas
[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU
Hi Thomas,
Do you have any quick suggestion before report it on CentOS?
thanks
Cliff
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879425
Title:
The thread of "CPU 0 /KVM" keeping 99.9%CPU
Status in QEMU:
Incomplete
Bug description:
Hi Expert:
The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by
qemu-kvm.
Notes:
for VM:
OS: RHEL8.1
CPU: 1
MEM:4G
For qemu-kvm(host kernel RHEL7):
1) version:
/usr/libexec/qemu-kvm -version
QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by
com "top -p VM_pro_ID"
PID UDER PR NI RES S % CPU %MEM TIME+COMMAND
872067 qemu 20 0 1.6g R 99.9 0.6 37:08.87 CPU 0/KVM
3) use "pstack 493307" and below is function trace
Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
#0 0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
#1 0x55ff34bdf4a9 in qemu_poll_ns ()
#2 0x55ff34be02a8 in main_loop_wait ()
#3 0x55ff348bfb1a in main ()
4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
Therefore, I think the thread "CPU 0/KVM" is in tight loop.
5) use reset can recover this issue. however, it will reoccurred again.
Current work around is increase one CPU for this VM, then issue is gone.
thanks
Cliff
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879425/+subscriptions
[Bug 1693649] Re: x86 pause misbehaves with -cpu haswell
Can you still reproduce this issue with the latest version of QEMU (currently 5.0)? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1693649 Title: x86 pause misbehaves with -cpu haswell Status in QEMU: Incomplete Bug description: Using qemu-2.9.0 When booting NetBSD using '-cpu haswell -smp 4', the system fails to initialize the additional CPUs. It appears as though the "application processor" enters routine x86_pause() but never returns. x86_pause() is simply two assembler instructions: 'pause; ret;' Replacing the routine with 'nop; nop; ret;' allows the system to proceed, of course without the benefit of the pause instruction on spin-loops! Additionally, booting with '-cpu phenom -smp 4' also works, although the system does seem confused about the type of CPU being used. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1693649/+subscriptions
[Bug 1693667] Re: -cpu haswell / broadwell have no MONITOR in features1
Can you still reproduce this issue with the latest version of QEMU? Looking at https://git.qemu.org/?p=qemu.git;a=commitdiff;h=0723cc8a5558c94388 for example, it might have been fixed since QEMU v4.2... ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1693667 Title: -cpu haswell / broadwell have no MONITOR in features1 Status in QEMU: Incomplete Bug description: In qemu 2.9.0 if you run qemu-system-x86_64 -cpu Broadwell (or Haswell) then the CPU features1 flag include the SSE3 bit, but do NOT include the MONITOR/MWAIT bit. This is so even when the host includes the features. Additionally, running qemu in this manner results in several error messages: warning: TCG doesn't support requested feature: CPUID.01H:ECX.fma [bit 12] warning: TCG doesn't support requested feature: CPUID.01H:ECX.pcid [bit 17] warning: TCG doesn't support requested feature: CPUID.01H:ECX.x2apic [bit 21] warning: TCG doesn't support requested feature: CPUID.01H:ECX.tsc-deadline [bit 24] warning: TCG doesn't support requested feature: CPUID.01H:ECX.avx [bit 28] warning: TCG doesn't support requested feature: CPUID.01H:ECX.f16c [bit 29] warning: TCG doesn't support requested feature: CPUID.01H:ECX.rdrand [bit 30] warning: TCG doesn't support requested feature: CPUID.07H:EBX.hle [bit 4] warning: TCG doesn't support requested feature: CPUID.07H:EBX.avx2 [bit 5] warning: TCG doesn't support requested feature: CPUID.07H:EBX.invpcid [bit 10] warning: TCG doesn't support requested feature: CPUID.07H:EBX.rtm [bit 11] warning: TCG doesn't support requested feature: CPUID.07H:EBX.rdseed [bit 18] warning: TCG doesn't support requested feature: CPUID.8001H:ECX.3dnowprefetch (Among possible other uses, the lack of the MONITOR feature bit causes NetBSD to fall-back on a check-and-pause loop while an application CPU is waiting to be told to proceed by the boot CPU.) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1693667/+subscriptions
[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU
Can you try with a newer version of CentOS? I think there should be newer
versions of qemu-kvm-ev available, so maybe the problem is gone there.
Otherwise, please either try to reproduce this problem with upstream QEMU, or
report it to the CentOS bug tracker (https://bugs.centos.org/), since we do not
provide support for distribution builds in the upstream QEMU project here.
** Changed in: qemu
Status: New => Incomplete
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879425
Title:
The thread of "CPU 0 /KVM" keeping 99.9%CPU
Status in QEMU:
Incomplete
Bug description:
Hi Expert:
The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by
qemu-kvm.
Notes:
for VM:
OS: RHEL8.1
CPU: 1
MEM:4G
For qemu-kvm(host kernel RHEL7):
1) version:
/usr/libexec/qemu-kvm -version
QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by
com "top -p VM_pro_ID"
PID UDER PR NI RES S % CPU %MEM TIME+COMMAND
872067 qemu 20 0 1.6g R 99.9 0.6 37:08.87 CPU 0/KVM
3) use "pstack 493307" and below is function trace
Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
#0 0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
#1 0x55ff34bdf4a9 in qemu_poll_ns ()
#2 0x55ff34be02a8 in main_loop_wait ()
#3 0x55ff348bfb1a in main ()
4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
Therefore, I think the thread "CPU 0/KVM" is in tight loop.
5) use reset can recover this issue. however, it will reoccurred again.
Current work around is increase one CPU for this VM, then issue is gone.
thanks
Cliff
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879425/+subscriptions
[Bug 1879998] Re: Bad check for return value of mmap()
And concerning the mmap in roms/u-boot/, please report that issue to the U-Boot project instead: https://www.denx.de/wiki/U-Boot/ -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1879998 Title: Bad check for return value of mmap() Status in QEMU: Won't Fix Bug description: In ./roms/skiboot/extract-gcov.c there is this code: addr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); assert(addr != NULL); This check is wrong, mmap never returns NULL, on errors it returns MAP_FAILED (or -1). (Also sidenote: asserts usually shouldn't be used for error checking.) In roms/skiboot/libstb/print-container.c there's a similar issue: payload = mmap(NULL, payload_st.st_size - SECURE_BOOT_HEADERS_SIZE, PROT_READ, MAP_PRIVATE, fdin, SECURE_BOOT_HEADERS_SIZE); if (!payload) This if should be (payload == MAP_FAILED). Another one is in ./roms/skiboot/libstb/create-container.c And in ./roms/u-boot/tools/aisimage.c there's an mmap call that does not check the return value at all. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1879998/+subscriptions
[Bug 1879998] Re: Bad check for return value of mmap()
skiboot is a separate project, we do not manage its code in the QEMU project, but just include the source code in our release tarballs since we ship the skiboot binary with QEMU. Please report these problems to the skiboot project instead: https://github.com/open-power/skiboot ** Changed in: qemu Status: New => Won't Fix -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1879998 Title: Bad check for return value of mmap() Status in QEMU: Won't Fix Bug description: In ./roms/skiboot/extract-gcov.c there is this code: addr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); assert(addr != NULL); This check is wrong, mmap never returns NULL, on errors it returns MAP_FAILED (or -1). (Also sidenote: asserts usually shouldn't be used for error checking.) In roms/skiboot/libstb/print-container.c there's a similar issue: payload = mmap(NULL, payload_st.st_size - SECURE_BOOT_HEADERS_SIZE, PROT_READ, MAP_PRIVATE, fdin, SECURE_BOOT_HEADERS_SIZE); if (!payload) This if should be (payload == MAP_FAILED). Another one is in ./roms/skiboot/libstb/create-container.c And in ./roms/u-boot/tools/aisimage.c there's an mmap call that does not check the return value at all. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1879998/+subscriptions
[Bug 1878915] Re: util/fdmon-io_uring.c:95: get_sqe: Assertion `ret > 1' failed.
** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878915 Title: util/fdmon-io_uring.c:95: get_sqe: Assertion `ret > 1' failed. Status in QEMU: Fix Committed Bug description: qemu 5.0.0, liburing1 0.6-3, Linux 5.6.0-1-686-pae (Debian) Stack trace: Stack trace of thread 31002: #0 0xb7faf1cd __kernel_vsyscall (linux-gate.so.1 + 0x11cd) #1 0xb6c618e2 __libc_signal_restore_set (libc.so.6 + 0x348e2) #2 0xb6c4a309 __GI_abort (libc.so.6 + 0x1d309) #3 0xb6c4a1d1 __assert_fail_base (libc.so.6 + 0x1d1d1) #4 0xb6c59929 __GI___assert_fail (libc.so.6 + 0x2c929) #5 0x00ba80be get_sqe (qemu-system-i386 + 0x6d00be) #6 0x00ba80cb add_poll_add_sqe (qemu-system-i386 + 0x6d00cb) #7 0x00ba820c fill_sq_ring (qemu-system-i386 + 0x6d020c) #8 0x00ba7145 aio_poll (qemu-system-i386 + 0x6cf145) #9 0x00aede63 blk_prw (qemu-system-i386 + 0x615e63) #10 0x00aeef95 blk_pread (qemu-system-i386 + 0x616f95) #11 0x008abbfa fdctrl_transfer_handler (qemu-system-i386 + 0x3d3bfa) #12 0x00906c3d i8257_channel_run (qemu-system-i386 + 0x42ec3d) #13 0x008ac119 fdctrl_start_transfer (qemu-system-i386 + 0x3d4119) #14 0x008ab233 fdctrl_write_data (qemu-system-i386 + 0x3d3233) #15 0x00708ae7 memory_region_write_accessor (qemu-system-i386 + 0x230ae7) #16 0x007059e1 access_with_adjusted_size (qemu-system-i386 + 0x22d9e1) #17 0x0070b931 memory_region_dispatch_write (qemu-system-i386 + 0x233931) #18 0x006a87a2 address_space_stb (qemu-system-i386 + 0x1d07a2) #19 0x00829216 helper_outb (qemu-system-i386 + 0x351216) #20 0xb06d9fdc n/a (n/a + 0x0) Steps: 0. qemu-img create -f raw fda.img 3840K 1. mformat -i fda.img -n 48 -t 80 -h 2 2. qemu-system-i386 -fda fda.img -hda freedos.qcow2 3. Attempt to run 'dosfsck a:' in the guest According to hw/block/fdc.c, a 3840K image should result in a virtual floppy with a geometry of 48 sectors/track x 80 tracks x 2 sides. The assert seems bogus either way. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1878915/+subscriptions
Re: [PATCH 0/6] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines
Patchew URL: https://patchew.org/QEMU/20200521192133.127559-1-hskinnem...@google.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === Broken pipe /tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) TESTiotest-qcow2: 089 ERROR - too few tests run (expected 68, got 21) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTiotest-qcow2: 090 TESTiotest-qcow2: 097 --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=935cc7f201a94a2e865cf081636a2cf4', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-evkby52n/src/docker-src.2020-05-22-00.39.52.17361:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=935cc7f201a94a2e865cf081636a2cf4 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-evkby52n/src' make: *** [docker-run-test-quick@centos7] Error 2 real17m34.256s user0m9.006s The full log is available at http://patchew.org/logs/20200521192133.127559-1-hskinnem...@google.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Bug 1877716] Re: Win10 guest unusable after a few minutes
** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1877716 Title: Win10 guest unusable after a few minutes Status in QEMU: Fix Committed Bug description: On Arch Linux, the recent qemu package update seems to misbehave on some systems. In my case, my Windows 10 guest runs fine for around 5 minutes and then start to get really sluggish, even unresponsive. It needs to be forced off. I could reproduce this on a minimal VM with no passthrough, although my current testing setup involves an nvme pcie passthrough. I bisected it to the following commit which rapidly starts to run sluggishly on my setup: https://github.com/qemu/qemu/commit/73fd282e7b6dd4e4ea1c3bbb3d302c8db51e4ccf I've ran the previous commit ( https://github.com/qemu/qemu/commit/b321051cf48ccc2d3d832af111d688f2282f089b ) for the entire night without an issue so far. I believe this might be a duplicate of https://bugs.launchpad.net/qemu/+bug/1873032 , although I'm not sure. Linux cc 5.6.10-arch1-1 #1 SMP PREEMPT Sat, 02 May 2020 19:11:54 + x86_64 GNU/Linux AMD Ryzen 7 2700X Eight-Core Processor To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1877716/+subscriptions
[PATCH v1 0/1] target/microblaze: Add GDB XML files
Hi all, This adds GDB XML files for Microblaze CPUs. For Microblaze, it's split up into core and stack protect XML files. Thanks! Joe Joe Komlodi (1): target/microblaze: Add GDB XML files for Microblaze configure| 1 + gdb-xml/microblaze-core.xml | 62 gdb-xml/microblaze-stack-protect.xml | 12 +++ target/microblaze/cpu.c | 1 + 4 files changed, 76 insertions(+) create mode 100644 gdb-xml/microblaze-core.xml create mode 100644 gdb-xml/microblaze-stack-protect.xml -- 2.7.4
Re: [PATCH v3 11/11] hw/semihosting: Make the feature depend of TCG, and allow to disable it
On 5/21/20 12:59 PM, Philippe Mathieu-Daudé wrote: > +++ b/hw/semihosting/Kconfig > @@ -1,3 +1,5 @@ > > +# default is 'n' > config SEMIHOSTING > - bool > +bool > +depends on TCG > diff --git a/target/arm/Kconfig b/target/arm/Kconfig > new file mode 100644 > index 00..035592dd86 > --- /dev/null > +++ b/target/arm/Kconfig > @@ -0,0 +1,2 @@ > +config SEMIHOSTING > +default y if TCG Did you answer my question about replicating the TCG test? If you did, I missed it... r~
Re: [PATCH v3 09/11] rules.mak: Add base-arch() rule
On 5/21/20 12:59 PM, Philippe Mathieu-Daudé wrote:
> + $(if $(findstring risc,$1),risc,\
Eh? riscv{32,64} vs openrisc.
> + $(if $(findstring x86,$1),i386,\
Do we really not need an exact match for x86_64?
> + $(if $(findstring aarch64,$1),arm,$1)))
Exact match?
r~
[PATCH v1 1/1] target/microblaze: Add GDB XML files for Microblaze
This adds the Microblaze core and stack protect XML files, and also modifies the configuration to build those files. Signed-off-by: Joe Komlodi --- configure| 1 + gdb-xml/microblaze-core.xml | 62 gdb-xml/microblaze-stack-protect.xml | 12 +++ target/microblaze/cpu.c | 1 + 4 files changed, 76 insertions(+) create mode 100644 gdb-xml/microblaze-core.xml create mode 100644 gdb-xml/microblaze-stack-protect.xml diff --git a/configure b/configure index 2fc05c4..8aed2fd 100755 --- a/configure +++ b/configure @@ -7832,6 +7832,7 @@ case "$target_name" in TARGET_ARCH=microblaze TARGET_SYSTBL_ABI=common bflt="yes" +gdb_xml_files="microblaze-core.xml microblaze-stack-protect.xml" echo "TARGET_ABI32=y" >> $config_target_mak ;; mips|mipsel) diff --git a/gdb-xml/microblaze-core.xml b/gdb-xml/microblaze-core.xml new file mode 100644 index 000..d30e596 --- /dev/null +++ b/gdb-xml/microblaze-core.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/gdb-xml/microblaze-stack-protect.xml b/gdb-xml/microblaze-stack-protect.xml new file mode 100644 index 000..1b16223 --- /dev/null +++ b/gdb-xml/microblaze-stack-protect.xml @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c index 51e5c85..faa88e5 100644 --- a/target/microblaze/cpu.c +++ b/target/microblaze/cpu.c @@ -330,6 +330,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data) dc->vmsd = &vmstate_mb_cpu; device_class_set_props(dc, mb_properties); cc->gdb_num_core_regs = 32 + 27; +cc->gdb_core_xml_file = "microblaze-core.xml"; cc->disas_set_info = mb_disas_set_info; cc->tcg_initialize = mb_tcg_init; -- 2.7.4
Re: [PATCH v3 25/25] ppc64: Clean up reginfo handling
On 5/21/20 7:34 PM, Richard Henderson wrote: > +return memcmp(m, a, sizeof(*m)); Ho hum. memcmp(...) == 0. Fixed locally. r~
[Bug 1880066] [NEW] Microphone input dies in guest when switching evdev input
Public bug reported: justin@justin-3900x:~$ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description:Ubuntu 20.04 LTS Release:20.04 Codename: focal justin@justin-3900x:~$ apt list --installed | egrep '*qemu*|*kvm*' WARNING: apt does not have a stable CLI interface. Use with caution in scripts. ipxe-qemu-256k-compat-efi-roms/focal,focal,now 1.0.0+git-20150424.a25a16d-0ubuntu4 all [installed,automatic] ipxe-qemu/focal,focal,now 1.0.0+git-20190109.133f4c4-0ubuntu3 all [installed,automatic] libvirt-daemon-driver-qemu/focal,now 6.0.0-0ubuntu8 amd64 [installed,automatic] qemu-block-extra/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-kvm/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] qemu-system-common/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-system-data/focal-updates,focal-updates,focal-security,focal-security,now 1:4.2-3ubuntu6.1 all [installed,automatic] qemu-system-gui/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-system-x86/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] qemu-utils/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] justin@justin-3900x:~$ This did not happen in Eoan (qemu 4.0.0). I was able to switch in/out of a VM with my audio coming through fine. I enabled Eoan in my sources.list, downgraded all my qemu packages, and the issue was resolved. This happens on the latest Windows 10 guest when a sound device is listening for the microphone. /var/log/libvirt/qemu/.log spews this error out when I switch with evdev (which is just the keyboard and mouse, the audio is passed through I assume spice): audio: live=228193 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=228675 sw->hw->conv_buf->size=1920 A bug was just triggered in audio_pcm_hw_get_live_in Context: audio: live=228675 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=229156 sw->hw->conv_buf->size=1920 A bug was just triggered in audio_pcm_hw_get_live_in Context: audio: live=229156 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=229638 sw->hw->conv_buf->size=1920 A bug was just triggered in audio_pcm_hw_get_live_in Context: audio: live=229638 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=230119 sw->hw->conv_buf->size=1920 A bug was just triggered in audio_pcm_hw_get_live_in Context: audio: live=230119 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=230600 sw->hw->conv_buf->size=1920 A bug was just triggered in audio_pcm_hw_get_live_in Context: audio: live=230600 hw->conv_buf->size=1920 A bug was just triggered in audio_get_avail Context: audio: live=231081 sw->hw->conv_buf->size=1920 ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1880066 Title: Microphone input dies in guest when switching evdev input Status in QEMU: New Bug description: justin@justin-3900x:~$ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04 LTS Release: 20.04 Codename: focal justin@justin-3900x:~$ apt list --installed | egrep '*qemu*|*kvm*' WARNING: apt does not have a stable CLI interface. Use with caution in scripts. ipxe-qemu-256k-compat-efi-roms/focal,focal,now 1.0.0+git-20150424.a25a16d-0ubuntu4 all [installed,automatic] ipxe-qemu/focal,focal,now 1.0.0+git-20190109.133f4c4-0ubuntu3 all [installed,automatic] libvirt-daemon-driver-qemu/focal,now 6.0.0-0ubuntu8 amd64 [installed,automatic] qemu-block-extra/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-kvm/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] qemu-system-common/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-system-data/focal-updates,focal-updates,focal-security,focal-security,now 1:4.2-3ubuntu6.1 all [installed,automatic] qemu-system-gui/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu-system-x86/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] qemu-utils/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed,automatic] qemu/focal-updates,focal-security,now 1:4.2-3ubuntu6.1 amd64 [installed] justin@justin-3900x:~$ This did not happen in Eoan (qemu 4.0.0). I was able to switch in/out of a VM with my audio coming through fine. I enabled Eoan in my sources.list, downgraded all my qemu packages, and the issue was resolved. This happens on the latest Win
Re: [PATCH v1 12/15] cpus-common: ensure auto-assigned cpu_indexes don't clash
On 5/20/20 7:05 AM, Alex Bennée wrote: > Basing the cpu_index on the number of currently allocated vCPUs fails > when vCPUs aren't removed in a LIFO manner. This is especially true > when we are allocating a cpu_index for each guest thread in > linux-user where there is no ordering constraint on their allocation > and de-allocation. > > [I've dropped the assert which is there to guard against out-of-order > removal as this should probably be caught higher up the stack. Maybe > we could just ifdef CONFIG_SOFTTMU it?] > > Signed-off-by: Alex Bennée > Cc: Nikolay Igotti > Cc: Paolo Bonzini > Cc: Igor Mammedov > Cc: Eduardo Habkost > > --- > v2 > - slightly tweak the index algorithm to preserve cpu_index = 0 > --- > cpus-common.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] minikconf: explicitly set encoding to UTF-8
On 5/21/20 8:36 AM, Stefan Hajnoczi wrote: > QEMU currently only has ASCII Kconfig files but Linux actually uses > UTF-8. Explicitly specify the encoding and that we're doing text file > I/O. > > It's unclear whether or not QEMU will ever need Unicode in its Kconfig > files. If we start using the help text then it will become an issue > sooner or later. Make this change now for consistency with Linux > Kconfig. > > Reported-by: Philippe Mathieu-Daudé > Signed-off-by: Stefan Hajnoczi > --- > scripts/minikconf.py | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
On 5/21/20 6:57 AM, Alex Bennée wrote:
> Newer clangs rightly spot that you can never exceed the full address
> space of 64 bit hosts with:
>
> linux-user/elfload.c:2076:41: error: result of comparison 'unsigned
> long' > 18446744073709551615 is always false
> [-Werror,-Wtautological-type-limit-compare]
> 4685 if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
> 4686 ~~~ ^ ~
> 4687 1 error generated.
>
> So lets limit the check to 32 bit hosts only.
>
> Fixes: ee94743034bf
> Reported-by: Thomas Huth
> Signed-off-by: Alex Bennée
>
> ---
I say again that I'd prefer that we disable this warning.
r~
Re: [PATCH 0/2] Update use_goto_tb() in hppa and rx targets
On 5/21/20 4:32 AM, Ahmed Karaman wrote: > Does this mean that there is a bug > in this function for the other targets? Yes, I think so. > That we have to do the page crossings check in both modes to avoid the > user-mode failures that you have mentioned above? Well, that or we need to fix linux-user/mmap.c to do all the invalidations required. r~
Re: [PATCH v1 11/15] tests/tcg/aarch64: Add bti smoke test
On 5/20/20 7:05 AM, Alex Bennée wrote: > From: Richard Henderson > > Requires gcc 10 for -mbranch-protection=standard. > > Signed-off-by: Richard Henderson > Message-Id: <20200520023139.9812-3-richard.hender...@linaro.org> > [AJB: add CROSS_CC_HAS_ARMV8_BTI check] > Signed-off-by: Alex Bennée > --- > tests/tcg/aarch64/bti-1.c | 62 +++ > tests/tcg/aarch64/bti-crt.inc.c | 51 + > tests/tcg/aarch64/Makefile.target | 9 + > tests/tcg/configure.sh| 4 ++ > 4 files changed, 126 insertions(+) > create mode 100644 tests/tcg/aarch64/bti-1.c > create mode 100644 tests/tcg/aarch64/bti-crt.inc.c This should be dropped from this patch set. I have adjusted it and it and posted as https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05891.html r~
Re: [PATCH v1 10/15] tests/docker: use a gcc-10 based image for arm64 tests
On 5/20/20 7:05 AM, Alex Bennée wrote: > As we enable newer features that we want to test on arm64 targets we > need newer compilers. Split off a new debian-arm64-test-cross image > which we can use to build these new tests. > > Signed-off-by: Alex Bennée > --- > tests/docker/Makefile.include | 2 ++ > .../dockerfiles/debian-arm64-test-cross.docker | 13 + > tests/tcg/configure.sh | 4 ++-- > 3 files changed, 17 insertions(+), 2 deletions(-) > create mode 100644 tests/docker/dockerfiles/debian-arm64-test-cross.docker Reviewed-by: Richard Henderson r~
Re: [PATCH v1 09/15] tests/docker: add debian11 base image
On 5/20/20 7:05 AM, Alex Bennée wrote: > We won't use this for building QEMU but we do need newer GCC's and > binutils for building some of our test cases. > > Signed-off-by: Alex Bennée > --- > tests/docker/Makefile.include| 2 +- > tests/docker/dockerfiles/debian11.docker | 18 ++ > 2 files changed, 19 insertions(+), 1 deletion(-) > create mode 100644 tests/docker/dockerfiles/debian11.docker Reviewed-by: Richard Henderson r~
Re: [PATCH v1 08/15] tests/docker: bump fedora to 32
On 5/20/20 7:05 AM, Alex Bennée wrote: > We should be keeping this up to date as Fedora goes out of support > quite quickly. > > Signed-off-by: Alex Bennée > --- > tests/docker/dockerfiles/fedora.docker | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 07/15] tests/tcg: better detect confused gdb which can't connect
On 5/20/20 7:05 AM, Alex Bennée wrote: > While we may gamely give the right information it can still confuse > the wide range of GDBs out there. For example ppc64abi32-linux-user > reports: > > warning: Selected architecture powerpc:common is not compatible with > reported target architecture powerpc:common64 > warning: Architecture rejected target-supplied description > > but still connects. Add a test for a 0 pc and exit early if that is > the case. This may actually be a bug we need to fix? > > Signed-off-by: Alex Bennée > Cc: Richard Henderson > --- > tests/tcg/multiarch/gdbstub/sha1.py | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 06/15] tests/fp: split and audit the conversion tests
On 5/20/20 7:05 AM, Alex Bennée wrote: > Split the float conversion tests into separate groups and audit the > tests to check what is still broken. I was able to enable a bunch of > tests that had been missed before: > > all the float to float conversions > ui32_to_extF80 > ui64_to_extF80 > extF80_to_ui32 > extF80_to_ui32_r_minMag > extF80_to_ui64 > extF80_to_ui64_r_minMag > > Signed-off-by: Alex Bennée > --- > tests/Makefile.include | 37 - > 1 file changed, 32 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 05/15] tests/fp: enable extf80_le_quite tests
On 5/20/20 7:05 AM, Alex Bennée wrote: > These have been fixed now so we no longer need a special version of > the le_quiet rule to skip the test. > > Signed-off-by: Alex Bennée > --- > tests/Makefile.include | 7 --- > 1 file changed, 7 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PULL 0/1] qemu-openbios queue 20200521
Patchew URL: https://patchew.org/QEMU/20200521200707.13826-1-mark.cave-ayl...@ilande.co.uk/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === qemu-system-aarch64: cannot set up guest memory 'ram': Cannot allocate memory Broken pipe /tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) ERROR - too few tests run (expected 66, got 30) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTcheck-qtest-x86_64: tests/qtest/machine-none-test TESTcheck-qtest-x86_64: tests/qtest/qmp-test --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=34e9eaa0e74c4dec8af41a7a50588e70', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-jzg0dkoj/src/docker-src.2020-05-21-22.41.37.24340:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=34e9eaa0e74c4dec8af41a7a50588e70 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-jzg0dkoj/src' make: *** [docker-run-test-quick@centos7] Error 2 real15m56.891s user0m8.829s The full log is available at http://patchew.org/logs/20200521200707.13826-1-mark.cave-ayl...@ilande.co.uk/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH v5] char-socket: initialize reconnect timer only when the timer doesn't start
When the disconnect event is triggered in the connecting stage,
the tcp_chr_disconnect_locked may be called twice.
The first call:
#0 qemu_chr_socket_restart_timer (chr=0x5582ee90) at
chardev/char-socket.c:120
#1 0x5558e38c in tcp_chr_disconnect_locked (chr=)
at chardev/char-socket.c:490
#2 0x5558e3cd in tcp_chr_disconnect (chr=0x5582ee90) at
chardev/char-socket.c:497
#3 0x5558ea32 in tcp_chr_new_client (chr=chr@entry=0x5582ee90,
sioc=sioc@entry=0x5582f0b0) at chardev/char-socket.c:892
#4 0x5558eeb8 in qemu_chr_socket_connected (task=0x5582f300,
opaque=) at chardev/char-socket.c:1090
#5 0x55574352 in qio_task_complete
(task=task@entry=0x5582f300) at io/task.c:196
#6 0x555745f4 in qio_task_thread_result (opaque=0x5582f300) at
io/task.c:111
#7 qio_task_wait_thread (task=0x5582f300) at io/task.c:190
#8 0x5558f17e in tcp_chr_wait_connected (chr=0x5582ee90,
errp=0x55802a08 ) at chardev/char-socket.c:1013
#9 0x55567cbd in char_socket_client_reconnect_test
(opaque=0x557fe020 ) at tests/test-char.c:1152
The second call:
#0 0x75ac3277 in raise () from /lib64/libc.so.6
#1 0x75ac4968 in abort () from /lib64/libc.so.6
#2 0x75abc096 in __assert_fail_base () from /lib64/libc.so.6
#3 0x75abc142 in __assert_fail () from /lib64/libc.so.6
#4 0x5558d10a in qemu_chr_socket_restart_timer
(chr=0x5582ee90) at chardev/char-socket.c:125
#5 0x5558df0c in tcp_chr_disconnect_locked (chr=)
at chardev/char-socket.c:490
#6 0x5558df4d in tcp_chr_disconnect (chr=0x5582ee90) at
chardev/char-socket.c:497
#7 0x5558e5b2 in tcp_chr_new_client (chr=chr@entry=0x5582ee90,
sioc=sioc@entry=0x5582f0b0) at chardev/char-socket.c:892
#8 0x5558e93a in tcp_chr_connect_client_sync
(chr=chr@entry=0x5582ee90, errp=errp@entry=0x7fffd178) at
chardev/char-socket.c:944
#9 0x5558ec78 in tcp_chr_wait_connected (chr=0x5582ee90,
errp=0x55802a08 ) at chardev/char-socket.c:1035
#10 0x5556804b in char_socket_client_test (opaque=0x557fe020
) at tests/test-char.c:1023
Run test/test-char to reproduce this issue.
test-char: chardev/char-socket.c:125: qemu_chr_socket_restart_timer: Assertion
`!s->reconnect_timer' failed.
Signed-off-by: Li Feng
---
v5:
- rebase to master
v4:
- remove the wrong patch
- fix the char_socket_ping_pong to support the reconnect exception test
v3:
- add a patch to fix a crash when recvmsg return 0
- make the tests reproduce the two crash
v2:
- add unit test
chardev/char-socket.c | 2 +-
tests/test-char.c | 73 +++
2 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index e77699db48..8af7fdce88 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -486,7 +486,7 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
if (emit_close) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-if (s->reconnect_time) {
+if (s->reconnect_time && !s->reconnect_timer) {
qemu_chr_socket_restart_timer(chr);
}
}
diff --git a/tests/test-char.c b/tests/test-char.c
index 3afc9b1b8d..73ba1cf601 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -625,12 +625,14 @@ static void char_udp_test(void)
typedef struct {
int event;
bool got_pong;
+CharBackend *be;
} CharSocketTestData;
#define SOCKET_PING "Hello"
#define SOCKET_PONG "World"
+typedef void (*char_socket_cb)(void *opaque, QEMUChrEvent event);
static void
char_socket_event(void *opaque, QEMUChrEvent event)
@@ -639,6 +641,27 @@ char_socket_event(void *opaque, QEMUChrEvent event)
data->event = event;
}
+static void
+char_socket_event_with_error(void *opaque, QEMUChrEvent event)
+{
+static bool first_error;
+CharSocketTestData *data = opaque;
+CharBackend *be = data->be;
+data->event = event;
+switch (event) {
+case CHR_EVENT_OPENED:
+if (!first_error) {
+first_error = true;
+qemu_chr_fe_disconnect(be);
+}
+return;
+case CHR_EVENT_CLOSED:
+return;
+default:
+return;
+}
+}
+
static void
char_socket_read(void *opaque, const uint8_t *buf, int size)
@@ -699,19 +722,24 @@ char_socket_addr_to_opt_str(SocketAddress *addr, bool
fd_pass,
}
-static void
-char_socket_ping_pong(QIOChannel *ioc)
+static int
+char_socket_ping_pong(QIOChannel *ioc, Error **errp)
{
char greeting[sizeof(SOCKET_PING)];
const char *response = SOCKET_PONG;
-qio_channel_read_all(ioc, greeting, sizeof(greeting), &error_abort);
+int ret;
+ret = qio_channel_read_all(ioc, greeting, sizeof(greeting), errp);
+if (ret != 0) {
+object_unref(OBJECT(ioc));
+r
Re: [PATCH v4 0/6] hyperv: VMBus implementation
On 21/05/2020, Paolo Bonzini wrote: On 24/04/20 14:34, Jon Doron wrote: This is a rebase of the old patchset from Roman for HyperV VMBus implementation. How to use: -device vmbus-bridge Later on new paravirtualized devices can be implemented on top of it (Network/SCSI/etc.) VMBus is a collection of technologies. At its lowest layer, it's a message passing and signaling mechanism, allowing efficient passing of messages to and from guest VMs. A layer higher, it's a mechanism for defining channels of communication, where each channel is tagged with a type (which implies a protocol) and a instance ID. A layer higher than that, it's a bus driver, serving as the basis of device enumeration within a VM, where a channel can optionally be exposed as a paravirtual device. When a server-side (paravirtual back-end) component wishes to offer a channel to a guest VM, it does so by specifying a channel type, a mode, and an instance ID. VMBus then exposes this in the guest. More information about VMBus can be found in the file vmbuskernelmodeclientlibapi.h in Microsoft's WDK. v4: Decided to ditch the patch that envolves handling of EOM as there is still a discussion going on with it in the KVM mailing list. v3: Fixed an error asan v2: Rebased on top of latest patchset from Roman and Maciej Jon Doron (6): hyperv: expose API to determine if synic is enabled vmbus: add vmbus protocol definitions vmbus: vmbus implementation i386:pc: whitelist dynamic vmbus-bridge i386: Hyper-V VMBus ACPI DSDT entry vmbus: add infrastructure to save/load vmbus requests Makefile.objs|1 + hw/hyperv/Kconfig|5 + hw/hyperv/Makefile.objs |1 + hw/hyperv/hyperv.c |8 + hw/hyperv/trace-events | 18 + hw/hyperv/vmbus.c| 2778 ++ hw/i386/acpi-build.c | 43 + hw/i386/pc_piix.c|2 + hw/i386/pc_q35.c |2 + include/hw/hyperv/hyperv.h |1 + include/hw/hyperv/vmbus-bridge.h | 35 + include/hw/hyperv/vmbus-proto.h | 222 +++ include/hw/hyperv/vmbus.h| 230 +++ 13 files changed, 3346 insertions(+) create mode 100644 hw/hyperv/trace-events create mode 100644 hw/hyperv/vmbus.c create mode 100644 include/hw/hyperv/vmbus-bridge.h create mode 100644 include/hw/hyperv/vmbus-proto.h create mode 100644 include/hw/hyperv/vmbus.h Queued, thanks. I'll take a look at the EOM handling threads next. Paolo Hi Paolo, there is no need to look at the EOM, we have scraped it as you can see in v4 :)
[PATCH v3 25/25] ppc64: Clean up reginfo handling
Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
which is related to system calls. Omit those from the original
reginfo_init(), so that any differences are automatically hidden.
Do not only compare bit 4 of CCR -- this register is 32 bits wide
with 8 cr subfields. We should compare all of them.
Tidy reginfo_dump() output. Especially, do not dump the non-
relevant fields.
Signed-off-by: Richard Henderson
---
risu_reginfo_ppc64.c | 114 +--
1 file changed, 44 insertions(+), 70 deletions(-)
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 134a152..62d7ff2 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -21,19 +21,30 @@
#include "risu.h"
#include "risu_reginfo_ppc64.h"
-#define XER 37
-#define CCR 38
+/* Names for indexes within gregset_t, ignoring those irrelevant here */
+enum {
+NIP = 32,
+MSR = 33,
+CTR = 35,
+LNK = 36,
+XER = 37,
+CCR = 38,
+};
const struct option * const arch_long_opts;
const char * const arch_extra_help;
static const char * const greg_names[NGREG] = {
-"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
-"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
- "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
- "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
- [XER] = "xer",
- [CCR] = "ccr",
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+[NIP] = "nip",
+[MSR] = "msr",
+[CTR] = "ctr",
+[LNK] = "lnk",
+[XER] = "xer",
+[CCR] = "ccr",
};
void process_arch_opt(int opt, const char *arg)
@@ -61,8 +72,13 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->nip = uc->uc_mcontext.regs->nip - image_start_address;
for (i = 0; i < NGREG; i++) {
-ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+/* Do not copy gp_reg entries not relevant to the context. */
+if (greg_names[i]) {
+ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+}
}
+ri->gregs[1] = 0xdeadbeef; /* sp */
+ri->gregs[13] = 0xdeadbeef; /* gp */
memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
ri->fpscr = uc->uc_mcontext.fp_regs[32];
@@ -76,79 +92,37 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
{
-int i;
-for (i = 0; i < 32; i++) {
-if (i == 1 || i == 13) {
-continue;
-}
-
-if (m->gregs[i] != a->gregs[i]) {
-return 0;
-}
-}
-
-if (m->gregs[XER] != a->gregs[XER]) {
-return 0;
-}
-
-if ((m->gregs[CCR] & 0x10) != (a->gregs[CCR] & 0x10)) {
-return 0;
-}
-
-for (i = 0; i < 32; i++) {
-if (m->fpregs[i] != a->fpregs[i]) {
-return 0;
-}
-}
-
-for (i = 0; i < 32; i++) {
-if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
-m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
-m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
-m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
-return 0;
-}
-}
-return 1;
+return memcmp(m, a, sizeof(*m));
}
/* reginfo_dump: print state to a stream */
void reginfo_dump(struct reginfo *ri, FILE * f)
{
-int i;
+const char *sep;
+int i, j;
-fprintf(f, " faulting insn 0x%x\n", ri->faulting_insn);
-fprintf(f, " prev insn 0x%x\n", ri->prev_insn);
-fprintf(f, " prev addr0x%" PRIx64 "\n\n", ri->nip);
+fprintf(f, "%6s: %08x\n", "insn", ri->faulting_insn);
+fprintf(f, "%6s: %016lx\n", "pc", ri->nip);
-for (i = 0; i < 16; i++) {
-fprintf(f, "\tr%2d: %16lx\tr%2d: %16lx\n", i, ri->gregs[i],
-i + 16, ri->gregs[i + 16]);
+sep = "";
+for (i = j = 0; i < NGREG; i++) {
+if (greg_names[i] != NULL) {
+fprintf(f, "%s%6s: %016lx", sep, greg_names[i], ri->gregs[i]);
+sep = (++j & 1 ? " " : "\n");
+}
}
-fprintf(f, "\n");
-fprintf(f, "\tnip: %16lx\n", ri->gregs[32]);
-fprintf(f, "\tmsr: %16lx\n", ri->gregs[33]);
-fprintf(f, "\torig r3: %16lx\n", ri->gregs[34]);
-fprintf(f, "\tctr: %16lx\n", ri->gregs[35]);
-fprintf(f, "\tlnk: %16lx\n", ri->gregs[36]);
-fprintf(f, "\txer: %16lx\n", ri->gregs[37]);
-fprintf(f, "\tccr: %16lx\n", ri->gregs[38]);
-fprintf(f, "\tmq : %16lx\n", ri->gregs[39]);
-fprintf(f, "\ttrap : %16lx\n", ri->gregs[40]);
-fprintf(f, "\tdar: %16lx\n", ri->gregs[41]);
-fprintf(f, "\tdsisr : %16lx\n", ri->gregs[42]);
-fprintf(f, "\tresult : %16lx\n", ri->gregs[43]);
-fpr
[PATCH v3 21/25] ppc64: Use uint64_t to represent double
We want to do exact bitwise comparisons of the data,
not be held hostage to IEEE comparisons and NaNs.
Signed-off-by: Richard Henderson
---
risu_reginfo_ppc64.h | 3 ++-
risu_reginfo_ppc64.c | 29 +
2 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/risu_reginfo_ppc64.h b/risu_reginfo_ppc64.h
index 7f2c962..4b1d8bd 100644
--- a/risu_reginfo_ppc64.h
+++ b/risu_reginfo_ppc64.h
@@ -20,7 +20,8 @@ struct reginfo {
uint64_t nip;
uint64_t prev_addr;
gregset_t gregs;
-fpregset_t fpregs;
+uint64_t fpregs[32];
+uint64_t fpscr;
vrregset_t vrregs;
};
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index c80e387..9899b36 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -45,6 +45,7 @@ int reginfo_size(struct reginfo *ri)
void reginfo_init(struct reginfo *ri, ucontext_t *uc)
{
int i;
+
memset(ri, 0, sizeof(*ri));
ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.regs->nip);
@@ -54,16 +55,11 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
}
-for (i = 0; i < NFPREG; i++) {
-ri->fpregs[i] = uc->uc_mcontext.fp_regs[i];
-}
+memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
+ri->fpscr = uc->uc_mcontext.fp_regs[32];
-for (i = 0; i < 32; i++) {
-ri->vrregs.vrregs[i][0] = uc->uc_mcontext.v_regs->vrregs[i][0];
-ri->vrregs.vrregs[i][1] = uc->uc_mcontext.v_regs->vrregs[i][1];
-ri->vrregs.vrregs[i][2] = uc->uc_mcontext.v_regs->vrregs[i][2];
-ri->vrregs.vrregs[i][3] = uc->uc_mcontext.v_regs->vrregs[i][3];
-}
+memcpy(ri->vrregs.vrregs, uc->uc_mcontext.v_regs->vrregs,
+ sizeof(ri->vrregs.vrregs[0]) * 32);
ri->vrregs.vscr = uc->uc_mcontext.v_regs->vscr;
ri->vrregs.vrsave = uc->uc_mcontext.v_regs->vrsave;
}
@@ -91,10 +87,6 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
}
for (i = 0; i < 32; i++) {
-if (isnan(m->fpregs[i]) && isnan(a->fpregs[i])) {
-continue;
-}
-
if (m->fpregs[i] != a->fpregs[i]) {
return 0;
}
@@ -141,10 +133,10 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, "\tdscr : %16lx\n\n", ri->gregs[44]);
for (i = 0; i < 16; i++) {
-fprintf(f, "\tf%2d: %.4f\tf%2d: %.4f\n", i, ri->fpregs[i],
+fprintf(f, "\tf%2d: %016lx\tf%2d: %016lx\n", i, ri->fpregs[i],
i + 16, ri->fpregs[i + 16]);
}
-fprintf(f, "\tfpscr: %f\n\n", ri->fpregs[32]);
+fprintf(f, "\tfpscr: %016lx\n\n", ri->fpscr);
for (i = 0; i < 32; i++) {
fprintf(f, "vr%02d: %8x, %8x, %8x, %8x\n", i,
@@ -181,13 +173,10 @@ int reginfo_dump_mismatch(struct reginfo *m, struct
reginfo *a, FILE *f)
}
for (i = 0; i < 32; i++) {
-if (isnan(m->fpregs[i]) && isnan(a->fpregs[i])) {
-continue;
-}
-
if (m->fpregs[i] != a->fpregs[i]) {
fprintf(f, "Mismatch: Register f%d\n", i);
-fprintf(f, "m: [%f] != a: [%f]\n", m->fpregs[i], a->fpregs[i]);
+fprintf(f, "m: [%016lx] != a: [%016lx]\n",
+m->fpregs[i], a->fpregs[i]);
}
}
--
2.20.1
[PATCH v3 24/25] Remove return value from reginfo_dump
No uses actually checked the error indication. Even if we wanted
to check ferror on the stream, we should do that generically rather
than per arch.
Signed-off-by: Richard Henderson
---
risu.h | 4 ++--
risu_reginfo_aarch64.c | 8 +++-
risu_reginfo_arm.c | 6 ++
risu_reginfo_i386.c| 6 ++
risu_reginfo_m68k.c| 6 ++
risu_reginfo_ppc64.c | 6 ++
6 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/risu.h b/risu.h
index 99f0d8e..6eceb9f 100644
--- a/risu.h
+++ b/risu.h
@@ -120,8 +120,8 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc);
/* return 1 if structs are equal, 0 otherwise. */
int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2);
-/* print reginfo state to a stream, returns 1 on success, 0 on failure */
-int reginfo_dump(struct reginfo *ri, FILE * f);
+/* print reginfo state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f);
/* reginfo_dump_mismatch: print mismatch details to a stream */
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index b443745..64e0e8b 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -258,8 +258,8 @@ static void sve_dump_zreg_diff(FILE *f, int vq, const
uint64_t *za,
}
#endif
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE * f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE * f)
{
int i;
fprintf(f, " faulting insn %08x\n", ri->faulting_insn);
@@ -303,7 +303,7 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
sve_dump_preg(f, vq, p);
fprintf(f, "\n");
}
-return !ferror(f);
+return;
}
#endif
@@ -312,8 +312,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
(uint64_t) (ri->simd.vregs[i] >> 64),
(uint64_t) (ri->simd.vregs[i]));
}
-
-return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index ba1035e..09813c4 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -161,8 +161,8 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
return memcmp(r1, r2, sizeof(*r1)) == 0;/* ok since we memset 0 */
}
-/* reginfo_dump: print the state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print the state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
int i;
if (ri->faulting_insn_size == 2) {
@@ -179,8 +179,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
i, (unsigned long long) ri->fpregs[i]);
}
fprintf(f, " fpscr: %08x\n", ri->fpscr);
-
-return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 57e4c00..37506fa 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -310,8 +310,8 @@ static char get_vecletter(uint64_t features)
}
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
uint64_t features;
int i, j, n, w;
@@ -345,8 +345,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
fprintf(f, " k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
}
}
-
-return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 29edce9..38d7dd3 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -92,8 +92,8 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
return 1;
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
int i;
fprintf(f, " pc\e[1;101;37m0x%08x\e[0m\n", ri->pc);
@@ -114,8 +114,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
}
fprintf(f, "\n");
-
-return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index e96dc48..134a152 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -112,8 +112,8 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
return 1;
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE * f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE * f)
{
int i;
@@ -152,8 +152,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
ri->vrreg
[PATCH v3 18/25] Compute reginfo_size based on the reginfo
This will allow dumping of SVE frames without having
to know the SVE vector length beforehand.
Signed-off-by: Richard Henderson
---
risu.h | 2 +-
risu.c | 9 +++--
risu_reginfo_aarch64.c | 4 ++--
risu_reginfo_arm.c | 4 ++--
risu_reginfo_i386.c| 4 ++--
risu_reginfo_m68k.c| 4 ++--
risu_reginfo_ppc64.c | 4 ++--
7 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/risu.h b/risu.h
index bfcf0af..3cad3d5 100644
--- a/risu.h
+++ b/risu.h
@@ -126,6 +126,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f);
int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
/* return size of reginfo */
-const int reginfo_size(void);
+int reginfo_size(struct reginfo *ri);
#endif /* RISU_H */
diff --git a/risu.c b/risu.c
index a248db1..a70b778 100644
--- a/risu.c
+++ b/risu.c
@@ -125,7 +125,7 @@ static RisuResult send_register_info(void *uc)
case OP_TESTEND:
case OP_COMPARE:
case OP_SIGILL:
-header.size = reginfo_size();
+header.size = reginfo_size(&ri[MASTER]);
extra = &ri[MASTER];
break;
case OP_COMPAREMEM:
@@ -209,7 +209,12 @@ static RisuResult recv_register_info(struct reginfo *ri)
return RES_BAD_SIZE;
}
respond(RES_OK);
-return read_buffer(ri, header.size);
+res = read_buffer(ri, header.size);
+if (res == RES_OK && header.size != reginfo_size(ri)) {
+/* The payload size is not self-consistent with the data. */
+return RES_BAD_SIZE;
+}
+return res;
case OP_COMPAREMEM:
if (header.size != MEMBLOCKLEN) {
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 028c690..7044648 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -69,7 +69,7 @@ void process_arch_opt(int opt, const char *arg)
#endif
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
int size = offsetof(struct reginfo, simd.end);
#ifdef SVE_MAGIC
@@ -194,7 +194,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
{
-return memcmp(r1, r2, reginfo_size()) == 0;
+return memcmp(r1, r2, reginfo_size(r1)) == 0;
}
#ifdef SVE_MAGIC
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 3662f12..47c52e8 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -36,9 +36,9 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
-return sizeof(struct reginfo);
+return sizeof(*ri);
}
static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 60fc239..50505ab 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -74,9 +74,9 @@ void process_arch_opt(int opt, const char *arg)
}
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
-return sizeof(struct reginfo);
+return sizeof(*ri);
}
static void *xsave_feature_buf(struct _xstate *xs, int feature)
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 32b28c8..4eb30cd 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -23,9 +23,9 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
-return sizeof(struct reginfo);
+return sizeof(*ri);
}
/* reginfo_init: initialize with a ucontext */
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 071c951..39e8f1c 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -32,9 +32,9 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
-return sizeof(struct reginfo);
+return sizeof(*ri);
}
/* reginfo_init: initialize with a ucontext */
--
2.20.1
[PATCH v3 20/25] aarch64: Use arch_init to configure sve
Adjust some of the aarch64 code to look at the reginfo struct
instead of looking at test_sve, so that we do not need to pass
the --test-sve option in order to dump sve trace files.
Signed-off-by: Richard Henderson
---
risu.h | 1 +
risu.c | 3 +++
risu_reginfo_aarch64.c | 55 +++---
risu_reginfo_arm.c | 4 +++
risu_reginfo_i386.c| 4 +++
risu_reginfo_m68k.c| 4 +++
risu_reginfo_ppc64.c | 4 +++
7 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/risu.h b/risu.h
index 3cad3d5..bdb70c1 100644
--- a/risu.h
+++ b/risu.h
@@ -23,6 +23,7 @@
extern const struct option * const arch_long_opts;
extern const char * const arch_extra_help;
void process_arch_opt(int opt, const char *arg);
+void arch_init(void);
#define FIRST_ARCH_OPT 0x100
/* GCC computed include to pull in the correct risu_reginfo_*.h for
diff --git a/risu.c b/risu.c
index a70b778..1c096a8 100644
--- a/risu.c
+++ b/risu.c
@@ -617,6 +617,9 @@ int main(int argc, char **argv)
load_image(imgfile);
+/* E.g. select requested SVE vector length. */
+arch_init();
+
if (ismaster) {
return master();
} else {
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index a1020ac..fb8e11a 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -44,8 +44,6 @@ const char * const arch_extra_help
void process_arch_opt(int opt, const char *arg)
{
#ifdef SVE_MAGIC
-long want, got;
-
assert(opt == FIRST_ARCH_OPT);
test_sve = strtol(arg, 0, 10);
@@ -53,22 +51,37 @@ void process_arch_opt(int opt, const char *arg)
fprintf(stderr, "Invalid value for VQ (1-%d)\n", SVE_VQ_MAX);
exit(EXIT_FAILURE);
}
-want = sve_vl_from_vq(test_sve);
-got = prctl(PR_SVE_SET_VL, want);
-if (want != got) {
-if (got < 0) {
-perror("prctl PR_SVE_SET_VL");
-} else {
-fprintf(stderr, "Unsupported value for VQ (%d != %d)\n",
-test_sve, (int)sve_vq_from_vl(got));
-}
-exit(EXIT_FAILURE);
-}
#else
abort();
#endif
}
+void arch_init(void)
+{
+#ifdef SVE_MAGIC
+long want, got1, got2;
+
+if (test_sve == 0) {
+return;
+}
+
+want = sve_vl_from_vq(test_sve);
+asm(".arch_extension sve\n\trdvl %0, #1" : "=r"(got1));
+if (want != got1) {
+got2 = prctl(PR_SVE_SET_VL, want);
+if (want != got2) {
+if (got2 < 0) {
+perror("prctl PR_SVE_SET_VL");
+got2 = got1;
+}
+fprintf(stderr, "Unsupported value for VQ (%d != %d)\n",
+test_sve, (int)sve_vq_from_vl(got1));
+exit(EXIT_FAILURE);
+}
+}
+#endif
+}
+
int reginfo_size(struct reginfo *ri)
{
#ifdef SVE_MAGIC
@@ -170,6 +183,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
if (sve->head.size == sizeof(*sve)) {
/* SVE state is empty -- not an error. */
+goto do_simd;
} else {
fprintf(stderr, "risu_reginfo_aarch64: "
"failed to get complete SVE state\n");
@@ -182,6 +196,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
return;
}
+ do_simd:
#endif /* SVE_MAGIC */
for (i = 0; i < 32; i++) {
@@ -260,8 +275,9 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " fpcr : %08x\n", ri->fpcr);
#ifdef SVE_MAGIC
-if (test_sve) {
-int q, vq = test_sve;
+if (ri->sve_vl) {
+int vq = sve_vq_from_vl(ri->sve_vl);
+int q;
fprintf(f, " vl : %d\n", ri->sve_vl);
@@ -339,13 +355,12 @@ int reginfo_dump_mismatch(struct reginfo *m, struct
reginfo *a, FILE * f)
}
#ifdef SVE_MAGIC
-if (test_sve) {
+if (m->sve_vl != a->sve_vl) {
+fprintf(f, " vl: %d vs %d\n", m->sve_vl, a->sve_vl);
+}
+if (m->sve_vl) {
int vq = sve_vq_from_vl(m->sve_vl);
-if (m->sve_vl != a->sve_vl) {
-fprintf(f, " vl: %d vs %d\n", m->sve_vl, a->sve_vl);
-}
-
for (i = 0; i < SVE_NUM_ZREGS; i++) {
uint64_t *zm = reginfo_zreg(m, vq, i);
uint64_t *za = reginfo_zreg(a, vq, i);
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 47c52e8..202120b 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -36,6 +36,10 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
+void arch_init(void)
+{
+}
+
int reginfo_size(struct reginfo *ri)
{
return sizeof(*ri);
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 50505ab..e9730be 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -74,6 +74,10 @@ void process_arch_opt(int opt, const char *arg)
}
}
+void arch_init(void)
+{
+}
+
int reginf
[PATCH v3 17/25] Add magic and size to the trace header
Sanity check that we're not getting out of sync with
the trace stream. This will be especially bad with
the change in size of the sve save data.
Signed-off-by: Richard Henderson
---
risu.h | 10 +++-
risu.c | 162 -
2 files changed, 136 insertions(+), 36 deletions(-)
diff --git a/risu.h b/risu.h
index dd9fda5..bfcf0af 100644
--- a/risu.h
+++ b/risu.h
@@ -55,7 +55,11 @@ typedef enum {
RES_END,
RES_MISMATCH_REG,
RES_MISMATCH_MEM,
+RES_MISMATCH_OP,
RES_BAD_IO,
+RES_BAD_MAGIC,
+RES_BAD_SIZE,
+RES_BAD_OP,
} RisuResult;
/* The memory block should be this long */
@@ -69,10 +73,14 @@ typedef enum {
struct reginfo;
typedef struct {
- uintptr_t pc;
+ uint32_t magic;
+ uint32_t size;
uint32_t risu_op;
+ uintptr_t pc;
} trace_header_t;
+#define RISU_MAGIC (('R' << 24) | ('I' << 16) | ('S' << 8) | 'U')
+
/* Socket related routines */
int master_connect(int port);
int apprentice_connect(const char *hostname, int port);
diff --git a/risu.c b/risu.c
index 80bc3b1..a248db1 100644
--- a/risu.c
+++ b/risu.c
@@ -111,32 +111,54 @@ static RisuResult send_register_info(void *uc)
uint64_t paramreg;
RisuResult res;
RisuOp op;
+void *extra;
reginfo_init(&ri[MASTER], uc);
op = get_risuop(&ri[MASTER]);
/* Write a header with PC/op to keep in sync */
+header.magic = RISU_MAGIC;
header.pc = get_pc(&ri[MASTER]);
header.risu_op = op;
+
+switch (op) {
+case OP_TESTEND:
+case OP_COMPARE:
+case OP_SIGILL:
+header.size = reginfo_size();
+extra = &ri[MASTER];
+break;
+case OP_COMPAREMEM:
+header.size = MEMBLOCKLEN;
+extra = memblock;
+break;
+case OP_SETMEMBLOCK:
+case OP_GETMEMBLOCK:
+header.size = 0;
+extra = NULL;
+break;
+default:
+abort();
+}
+
res = write_buffer(&header, sizeof(header));
if (res != RES_OK) {
return res;
}
+if (extra) {
+res = write_buffer(extra, header.size);
+if (res != RES_OK) {
+return res;
+}
+}
switch (op) {
case OP_COMPARE:
-case OP_TESTEND:
case OP_SIGILL:
-/*
- * Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
-res = write_buffer(&ri[MASTER], reginfo_size());
-/* For OP_TEST_END, force exit. */
-if (res == RES_OK && op == OP_TESTEND) {
-res = RES_END;
-}
+case OP_COMPAREMEM:
break;
+case OP_TESTEND:
+return RES_END;
case OP_SETMEMBLOCK:
paramreg = get_reginfo_paramreg(&ri[MASTER]);
memblock = (void *)(uintptr_t)paramreg;
@@ -145,12 +167,10 @@ static RisuResult send_register_info(void *uc)
paramreg = get_reginfo_paramreg(&ri[MASTER]);
set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
-case OP_COMPAREMEM:
-return write_buffer(memblock, MEMBLOCKLEN);
default:
abort();
}
-return res;
+return RES_OK;
}
static void master_sigill(int sig, siginfo_t *si, void *uc)
@@ -175,22 +195,35 @@ static RisuResult recv_register_info(struct reginfo *ri)
return res;
}
-/* send OK for the header */
-respond(RES_OK);
+if (header.magic != RISU_MAGIC) {
+/* If the magic number is wrong, we can't trust the rest. */
+return RES_BAD_MAGIC;
+}
switch (header.risu_op) {
case OP_COMPARE:
case OP_TESTEND:
case OP_SIGILL:
-return read_buffer(ri, reginfo_size());
+/* If we can't store the data, report invalid size. */
+if (header.size > sizeof(*ri)) {
+return RES_BAD_SIZE;
+}
+respond(RES_OK);
+return read_buffer(ri, header.size);
+
case OP_COMPAREMEM:
+if (header.size != MEMBLOCKLEN) {
+return RES_BAD_SIZE;
+}
+respond(RES_OK);
return read_buffer(other_memblock, MEMBLOCKLEN);
+
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
-return RES_OK;
+return header.size == 0 ? RES_OK : RES_BAD_SIZE;
+
default:
-/* TODO: Create a better error message. */
-return RES_BAD_IO;
+return RES_BAD_OP;
}
}
@@ -204,48 +237,71 @@ static RisuResult recv_and_compare_register_info(void *uc)
res = recv_register_info(&ri[MASTER]);
if (res != RES_OK) {
-/* I/O error. Tell master to exit. */
-respond(RES_END);
-return res;
+goto done;
}
op = get_risuop(&ri[APPRENTICE]);
-if (header.risu_op != op) {
-/* We are out of sync. Tell master to exit. */
-respond(RES_END);
-return RES_BAD_IO;
-}
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
case OP_SIGILL:
-if (!reginfo_is_eq(&ri[MASTER], &ri[APPR
[PATCH v3 15/25] Rearrange reginfo and memblock buffers
For send_register_info from master_sigill, do not keep a
reginfo buffer on the stack. At the moment, this struct
is quite large for aarch64.
Put the two reginfo buffers into an array, for the benefit
of future dumping. For recv_and_compare_register_info,
index this array with constants, so it's a simple rename.
Signed-off-by: Richard Henderson
---
risu.c | 58 --
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index a0e20d5..b91ad38 100644
--- a/risu.c
+++ b/risu.c
@@ -28,10 +28,16 @@
#include "config.h"
#include "risu.h"
-static void *memblock;
-static struct reginfo master_ri, apprentice_ri;
-static uint8_t master_memblock[MEMBLOCKLEN];
+enum {
+MASTER = 0, APPRENTICE = 1
+};
+static struct reginfo ri[2];
+static uint8_t other_memblock[MEMBLOCKLEN];
+static trace_header_t header;
+
+/* Memblock pointer into the execution image. */
+static void *memblock;
static int comm_fd;
static bool trace;
@@ -102,16 +108,15 @@ static void respond(RisuResult r)
static RisuResult send_register_info(void *uc)
{
-struct reginfo ri;
-trace_header_t header;
+uint64_t paramreg;
RisuResult res;
RisuOp op;
-reginfo_init(&ri, uc);
-op = get_risuop(&ri);
+reginfo_init(&ri[MASTER], uc);
+op = get_risuop(&ri[MASTER]);
/* Write a header with PC/op to keep in sync */
-header.pc = get_pc(&ri);
+header.pc = get_pc(&ri[MASTER]);
header.risu_op = op;
res = write_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -126,18 +131,19 @@ static RisuResult send_register_info(void *uc)
* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
-res = write_buffer(&ri, reginfo_size());
+res = write_buffer(&ri[MASTER], reginfo_size());
/* For OP_TEST_END, force exit. */
if (res == RES_OK && op == OP_TESTEND) {
res = RES_END;
}
break;
case OP_SETMEMBLOCK:
-memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
+paramreg = get_reginfo_paramreg(&ri[MASTER]);
+memblock = (void *)(uintptr_t)paramreg;
break;
case OP_GETMEMBLOCK:
-set_ucontext_paramreg(uc,
- get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+paramreg = get_reginfo_paramreg(&ri[MASTER]);
+set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
return write_buffer(memblock, MEMBLOCKLEN);
@@ -162,12 +168,12 @@ static void master_sigill(int sig, siginfo_t *si, void
*uc)
static RisuResult recv_and_compare_register_info(void *uc)
{
+uint64_t paramreg;
RisuResult res;
-trace_header_t header;
RisuOp op;
-reginfo_init(&apprentice_ri, uc);
-op = get_risuop(&apprentice_ri);
+reginfo_init(&ri[APPRENTICE], uc);
+op = get_risuop(&ri[APPRENTICE]);
res = read_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -190,10 +196,10 @@ static RisuResult recv_and_compare_register_info(void *uc)
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
-res = read_buffer(&master_ri, reginfo_size());
+res = read_buffer(&ri[MASTER], reginfo_size());
if (res != RES_OK) {
/* fail */
-} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+} else if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
/* register mismatch */
res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
@@ -202,17 +208,18 @@ static RisuResult recv_and_compare_register_info(void *uc)
respond(res == RES_OK ? RES_OK : RES_END);
break;
case OP_SETMEMBLOCK:
-memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
+paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
+memblock = (void *)(uintptr_t)paramreg;
break;
case OP_GETMEMBLOCK:
-set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
- (uintptr_t)memblock);
+paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
+set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
-res = read_buffer(master_memblock, MEMBLOCKLEN);
+res = read_buffer(other_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
/* fail */
-} else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
+} else if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
@@ -221,7 +228,6 @@ static RisuResult recv_and_compare_register_info(void *uc)
default:
abort();
}
-
return res;
}
@@ -342,10 +348,10 @@ static int apprentice(v
[PATCH v3 13/25] Split RES_MISMATCH for registers and memory
By remembering the specific comparison that failed, we do not
have to try again when it comes time to report. This makes
the mem_used flag redundant. Also, packet_mismatch is now
redundant with RES_BAD_IO.
This means that the only thing that report_match_status does
is to report on register status, so rename to report_mismatch_reg.
Also, we know there is a failure, so don't return a status from
the report.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.h| 13 ++---
reginfo.c | 45 -
risu.c| 10 +++---
3 files changed, 21 insertions(+), 47 deletions(-)
diff --git a/risu.h b/risu.h
index f383b64..77d6128 100644
--- a/risu.h
+++ b/risu.h
@@ -54,7 +54,8 @@ typedef enum {
typedef enum {
RES_OK = 0,
RES_END,
-RES_MISMATCH,
+RES_MISMATCH_REG,
+RES_MISMATCH_MEM,
RES_BAD_IO,
} RisuResult;
@@ -100,13 +101,11 @@ RisuResult send_register_info(void *uc);
*/
RisuResult recv_and_compare_register_info(void *uc);
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
+/*
+ * Print a useful report on the status of the last reg comparison
+ * done in recv_and_compare_register_info().
*/
-int report_match_status(void);
+void report_mismatch_reg(void);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index 31bc699..a007f16 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -17,9 +17,6 @@
static struct reginfo master_ri, apprentice_ri;
static uint8_t master_memblock[MEMBLOCKLEN];
-static int mem_used;
-static int packet_mismatch;
-
RisuResult send_register_info(void *uc)
{
struct reginfo ri;
@@ -107,10 +104,10 @@ RisuResult recv_and_compare_register_info(void *uc)
*/
res = read_buffer(&master_ri, reginfo_size());
if (res != RES_OK) {
-packet_mismatch = 1;
+/* fail */
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
/* register mismatch */
-res = RES_MISMATCH;
+res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
res = RES_END;
}
@@ -124,13 +121,12 @@ RisuResult recv_and_compare_register_info(void *uc)
(uintptr_t)memblock);
break;
case OP_COMPAREMEM:
-mem_used = 1;
res = read_buffer(master_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
-packet_mismatch = 1;
+/* fail */
} else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
-res = RES_MISMATCH;
+res = RES_MISMATCH_MEM;
}
respond(res == RES_OK ? RES_OK : RES_END);
break;
@@ -141,40 +137,15 @@ RisuResult recv_and_compare_register_info(void *uc)
return res;
}
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
+/*
+ * Print a useful report on the status of the last reg comparison
+ * done in recv_and_compare_register_info().
*/
-int report_match_status(void)
+void report_mismatch_reg(void)
{
-int resp = 0;
-fprintf(stderr, "match status...\n");
-if (packet_mismatch) {
-fprintf(stderr, "packet mismatch (probably disagreement "
-"about UNDEF on load/store)\n");
-return 1;
-}
-if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
-fprintf(stderr, "mismatch on regs!\n");
-resp = 1;
-}
-if (mem_used
-&& memcmp(memblock, &master_memblock, MEMBLOCKLEN) != 0) {
-fprintf(stderr, "mismatch on memory!\n");
-resp = 1;
-}
-if (!resp) {
-fprintf(stderr, "match!\n");
-return 0;
-}
-
fprintf(stderr, "master reginfo:\n");
reginfo_dump(&master_ri, stderr);
fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&apprentice_ri, stderr);
-
reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
-return resp;
}
diff --git a/risu.c b/risu.c
index 199f697..d6c2deb 100644
--- a/risu.c
+++ b/risu.c
@@ -224,9 +224,13 @@ static int apprentice(void)
case RES_END:
return EXIT_SUCCESS;
-case RES_MISMATCH:
-fprintf(stderr, "mismatch after %zd checkpoints\n", signal_count);
-report_match_status();
+case RES_MISMATCH_REG:
+fprintf(stderr, "mismatch reg after %zd checkpoints\n", signal_count);
+report_mismatch_reg();
+return EXIT_FAILURE;
+
+case RES_MISMATCH_MEM:
+fprintf(stderr, "mismatch mem after %zd checkpoints\n", signal_count);
[QUESTION] How to improve virtio-gpu performance
Hi Gerd, I'm using glmark2 to test virtio-gpu on AMDGPU, but the result is unsatisfactory. GPU: AMD Radeon Pro WX 5100 Host OS: CentOS Linux release 7.7.1908 (AltArch) --QEMU: 4.0.0 --linux kernel 4.18 --mesa 18.3 --virglrenderer --spice server 0.14.2 --spice-gtk3-0.35 --libepoxy 1.5.2 Guest OS: CentOS Linux release 7.7.1908 (AltArch) --linux kernel 4.18 --mesa 18.3 --xorg server 1.20 TestSuit: glmark2 Host result: glmark2 Score: 10043 Single Guest result: glmark2 Score: 2209 Multi Guest result: Guest1: glmark2 Score: 1136 Guest2: glmark2 Score: 1231 Guest3: glmark2 Score: 1282 Guest4: glmark2 Score: 1175 Guest5: glmark2 Score: 1398 It seems the whole performance of multi-guests is better than single guest, but each one has lower performance. I change 'ctrl_vq' from 256 to 1024, but has little effect. I wonder the result is normal or abnormal? And which is the bottleneck? I'm appreciated for you help. thanks, Jun
[PATCH v3 12/25] Simplify syncing with master
Do not pass status like RES_BAD_IO from apprentice to master.
This means that when master reports i/o error that we know it
came from master; the apprentice will report its own i/o error.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
reginfo.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/reginfo.c b/reginfo.c
index c37c5df..31bc699 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -90,10 +90,9 @@ RisuResult recv_and_compare_register_info(void *uc)
}
if (header.risu_op != op) {
-/* We are out of sync */
-res = RES_BAD_IO;
-respond(res);
-return res;
+/* We are out of sync. Tell master to exit. */
+respond(RES_END);
+return RES_BAD_IO;
}
/* send OK for the header */
@@ -115,7 +114,7 @@ RisuResult recv_and_compare_register_info(void *uc)
} else if (op == OP_TESTEND) {
res = RES_END;
}
-respond(res);
+respond(res == RES_OK ? RES_OK : RES_END);
break;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
@@ -133,7 +132,7 @@ RisuResult recv_and_compare_register_info(void *uc)
/* memory mismatch */
res = RES_MISMATCH;
}
-respond(res);
+respond(res == RES_OK ? RES_OK : RES_END);
break;
default:
abort();
--
2.20.1
[PATCH v3 23/25] Add --fulldump and --diffdup options
These allow the inspection of the trace files.
Signed-off-by: Richard Henderson
---
risu.c | 117 +
1 file changed, 102 insertions(+), 15 deletions(-)
diff --git a/risu.c b/risu.c
index f613fa9..8d907d9 100644
--- a/risu.c
+++ b/risu.c
@@ -484,23 +484,101 @@ static int apprentice(void)
}
}
-static int ismaster;
+static int dump_trace(bool isfull)
+{
+RisuResult res;
+int tick = 0;
+
+while (1) {
+struct reginfo *this_ri;
+
+this_ri = &ri[tick & 1];
+res = recv_register_info(this_ri);
+
+switch (res) {
+case RES_OK:
+switch (header.risu_op) {
+case OP_COMPARE:
+case OP_TESTEND:
+case OP_SIGILL:
+printf("%s: (pc %#lx)\n", op_name(header.risu_op),
+ (unsigned long)header.pc);
+
+if (isfull || tick == 0) {
+reginfo_dump(this_ri, stdout);
+} else {
+struct reginfo *prev_ri = &ri[(tick - 1) & 1];
+
+if (reginfo_is_eq(prev_ri, this_ri)) {
+/*
+ * ??? There should never be no change -- at minimum
+ * PC should have advanced. But for completeness...
+ */
+printf("change detail: none\n");
+} else {
+printf("change detail (prev : next):\n");
+reginfo_dump_mismatch(prev_ri, this_ri, stdout);
+}
+}
+putchar('\n');
+if (header.risu_op == OP_TESTEND) {
+return EXIT_SUCCESS;
+}
+tick++;
+break;
+
+case OP_COMPAREMEM:
+/* TODO: Dump 8k of data? */
+/* fall through */
+default:
+printf("%s\n", op_name(header.risu_op));
+break;
+}
+break;
+
+case RES_BAD_IO:
+fprintf(stderr, "I/O error\n");
+return EXIT_FAILURE;
+case RES_BAD_MAGIC:
+fprintf(stderr, "Unexpected magic number: %#08x\n", header.magic);
+return EXIT_FAILURE;
+case RES_BAD_SIZE:
+fprintf(stderr, "Unexpected payload size: %u\n", header.size);
+return EXIT_FAILURE;
+case RES_BAD_OP:
+fprintf(stderr, "Unexpected opcode: %d\n", header.risu_op);
+return EXIT_FAILURE;
+default:
+fprintf(stderr, "Unexpected recv result %d\n", res);
+return EXIT_FAILURE;
+}
+}
+}
+
+enum {
+DO_APPRENTICE,
+DO_MASTER,
+DO_FULLDUMP,
+DO_DIFFDUMP,
+};
+
+static int operation = DO_APPRENTICE;
static void usage(void)
{
fprintf(stderr,
-"Usage: risu [--master] [--host ] [--port ] "
-"\n\n");
-fprintf(stderr,
-"Run through the pattern file verifying each instruction\n");
-fprintf(stderr, "between master and apprentice risu processes.\n\n");
-fprintf(stderr, "Options:\n");
-fprintf(stderr, " --master Be the master (server)\n");
-fprintf(stderr, " -t, --trace=FILE Record/playback " TRACE_TYPE " trace
file\n");
-fprintf(stderr,
-" -h, --host=HOST Specify master host machine (apprentice only)"
-"\n");
-fprintf(stderr,
+"Usage: risu [--master|--fulldump|--diffdump]\n"
+"[--host ] [--port ] \n"
+"\n"
+"Run through the pattern file verifying each instruction\n"
+"between master and apprentice risu processes.\n"
+"\n"
+"Options:\n"
+" --master Be the master (server)\n"
+" --fulldumpDump each record\n"
+" --diffdumpDump difference between each record\n"
+" -t, --trace=FILE Record/playback " TRACE_TYPE " trace file\n"
+" -h, --host=HOST Specify master host machine\n"
" -p, --port=PORT Specify the port to connect to/listen on "
"(default 9191)\n");
if (arch_extra_help) {
@@ -512,7 +590,9 @@ static struct option * setup_options(char **short_opts)
{
static struct option default_longopts[] = {
{"help", no_argument, 0, '?'},
-{"master", no_argument, &ismaster, 1},
+{"master", no_argument, &operation, DO_MASTER},
+{"fulldump", no_argument, &operation, DO_FULLDUMP},
+{"diffdump", no_argument, &operation, DO_DIFFDUMP},
{"host", required_argument, 0, 'h'},
{"port", required_argument, 0, 'p'},
{"trace", required_argument, 0, 't'},
@@ -520,7 +600,7 @@ static struct option * setup_options(char **short_opts)
};
struct option *lopts = &default_longopts[0];
-*short_opts = "h:p:t:";
+*short_op
[PATCH v3 09/25] Unify i/o functions and use RisuResult
Push the trace check down from the function calling the reginfo
function down into the i/o function. This means we don't have
to pass a function pointer.
Return a RisuResult from the i/o functions. This fixes a minor bug
in send_register_info (even before the conversion to RisuResult),
which returned the write_fn result directly.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.h| 32 +-
comms.c | 8 +++---
reginfo.c | 60
risu.c| 82 ++-
4 files changed, 81 insertions(+), 101 deletions(-)
diff --git a/risu.h b/risu.h
index e6d07eb..c83b803 100644
--- a/risu.h
+++ b/risu.h
@@ -34,13 +34,6 @@ void process_arch_opt(int opt, const char *arg);
#include REGINFO_HEADER(ARCH)
-/* Socket related routines */
-int master_connect(int port);
-int apprentice_connect(const char *hostname, int port);
-int send_data_pkt(int sock, void *pkt, int pktlen);
-int recv_data_pkt(int sock, void *pkt, int pktlen);
-void send_response_byte(int sock, int resp);
-
extern uintptr_t image_start_address;
extern void *memblock;
@@ -80,31 +73,32 @@ typedef struct {
uint32_t risu_op;
} trace_header_t;
+/* Socket related routines */
+int master_connect(int port);
+int apprentice_connect(const char *hostname, int port);
+RisuResult send_data_pkt(int sock, void *pkt, int pktlen);
+RisuResult recv_data_pkt(int sock, void *pkt, int pktlen);
+void send_response_byte(int sock, int resp);
+
/* Functions operating on reginfo */
-/* Function prototypes for read/write helper functions.
- *
- * We pass the helper function to send_register_info and
- * recv_and_compare_register_info which can either be backed by the
- * traditional network socket or a trace file.
- */
-typedef int (*write_fn) (void *ptr, size_t bytes);
-typedef int (*read_fn) (void *ptr, size_t bytes);
-typedef void (*respond_fn) (RisuResult response);
+/* Function prototypes for read/write helper functions. */
+RisuResult write_buffer(void *ptr, size_t bytes);
+RisuResult read_buffer(void *ptr, size_t bytes);
+void respond(RisuResult response);
/*
* Send the register information from the struct ucontext down the socket.
* NB: called from a signal handler.
*/
-RisuResult send_register_info(write_fn write_fn, void *uc);
+RisuResult send_register_info(void *uc);
/*
* Read register info from the socket and compare it with that from the
* ucontext.
* NB: called from a signal handler.
*/
-RisuResult recv_and_compare_register_info(read_fn read_fn,
- respond_fn respond, void *uc);
+RisuResult recv_and_compare_register_info(void *uc);
/* Print a useful report on the status of the last comparison
* done in recv_and_compare_register_info(). This is called on
diff --git a/comms.c b/comms.c
index 861e845..21968da 100644
--- a/comms.c
+++ b/comms.c
@@ -168,7 +168,7 @@ ssize_t safe_writev(int fd, struct iovec *iov_in, int
iovcnt)
* Note that both ends must agree on the length of the
* block of data.
*/
-int send_data_pkt(int sock, void *pkt, int pktlen)
+RisuResult send_data_pkt(int sock, void *pkt, int pktlen)
{
unsigned char resp;
/* First we send the packet length as a network-order 32 bit value.
@@ -196,7 +196,7 @@ int send_data_pkt(int sock, void *pkt, int pktlen)
return resp;
}
-int recv_data_pkt(int sock, void *pkt, int pktlen)
+RisuResult recv_data_pkt(int sock, void *pkt, int pktlen)
{
uint32_t net_pktlen;
recv_bytes(sock, &net_pktlen, sizeof(net_pktlen));
@@ -206,10 +206,10 @@ int recv_data_pkt(int sock, void *pkt, int pktlen)
* a response back.
*/
recv_and_discard_bytes(sock, net_pktlen);
-return 1;
+return RES_BAD_IO;
}
recv_bytes(sock, pkt, pktlen);
-return 0;
+return RES_OK;
}
void send_response_byte(int sock, int resp)
diff --git a/reginfo.c b/reginfo.c
index b909a1f..fee025e 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,10 +21,11 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
-RisuResult send_register_info(write_fn write_fn, void *uc)
+RisuResult send_register_info(void *uc)
{
struct reginfo ri;
trace_header_t header;
+RisuResult res;
RisuOp op;
reginfo_init(&ri, uc);
@@ -33,8 +34,9 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
/* Write a header with PC/op to keep in sync */
header.pc = get_pc(&ri);
header.risu_op = op;
-if (write_fn(&header, sizeof(header)) != 0) {
-return RES_BAD_IO;
+res = write_buffer(&header, sizeof(header));
+if (res != RES_OK) {
+return res;
}
switch (op) {
@@ -45,11 +47,12 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
-if (write_f
[PATCH v3 11/25] Always write for --master
For trace, master of course must write to the file we create.
For sockets, we can report mismatches from either end. At present,
we are reporting mismatches from master. Reverse that so that we
report mismatches from the apprentice, just as we do for trace.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.h| 2 +-
reginfo.c | 38 --
risu.c| 96 ++-
3 files changed, 61 insertions(+), 75 deletions(-)
diff --git a/risu.h b/risu.h
index c83b803..f383b64 100644
--- a/risu.h
+++ b/risu.h
@@ -106,7 +106,7 @@ RisuResult recv_and_compare_register_info(void *uc);
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(bool trace);
+int report_match_status(void);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index fee025e..c37c5df 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -14,9 +14,8 @@
#include
#include "risu.h"
-struct reginfo master_ri, apprentice_ri;
-
-uint8_t apprentice_memblock[MEMBLOCKLEN];
+static struct reginfo master_ri, apprentice_ri;
+static uint8_t master_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
@@ -82,8 +81,8 @@ RisuResult recv_and_compare_register_info(void *uc)
trace_header_t header;
RisuOp op;
-reginfo_init(&master_ri, uc);
-op = get_risuop(&master_ri);
+reginfo_init(&apprentice_ri, uc);
+op = get_risuop(&apprentice_ri);
res = read_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -107,7 +106,7 @@ RisuResult recv_and_compare_register_info(void *uc)
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
-res = read_buffer(&apprentice_ri, reginfo_size());
+res = read_buffer(&master_ri, reginfo_size());
if (res != RES_OK) {
packet_mismatch = 1;
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
@@ -119,18 +118,18 @@ RisuResult recv_and_compare_register_info(void *uc)
respond(res);
break;
case OP_SETMEMBLOCK:
-memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
break;
case OP_GETMEMBLOCK:
-set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
(uintptr_t)memblock);
break;
case OP_COMPAREMEM:
mem_used = 1;
-res = read_buffer(apprentice_memblock, MEMBLOCKLEN);
+res = read_buffer(master_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
packet_mismatch = 1;
-} else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+} else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH;
}
@@ -149,18 +148,13 @@ RisuResult recv_and_compare_register_info(void *uc)
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(bool trace)
+int report_match_status(void)
{
int resp = 0;
fprintf(stderr, "match status...\n");
if (packet_mismatch) {
fprintf(stderr, "packet mismatch (probably disagreement "
"about UNDEF on load/store)\n");
-/* We don't have valid reginfo from the apprentice side
- * so stop now rather than printing anything about it.
- */
-fprintf(stderr, "%s reginfo:\n", trace ? "this" : "master");
-reginfo_dump(&master_ri, stderr);
return 1;
}
if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
@@ -168,7 +162,7 @@ int report_match_status(bool trace)
resp = 1;
}
if (mem_used
-&& memcmp(memblock, &apprentice_memblock, MEMBLOCKLEN) != 0) {
+&& memcmp(memblock, &master_memblock, MEMBLOCKLEN) != 0) {
fprintf(stderr, "mismatch on memory!\n");
resp = 1;
}
@@ -177,15 +171,11 @@ int report_match_status(bool trace)
return 0;
}
-fprintf(stderr, "%s reginfo:\n", trace ? "this" : "master");
+fprintf(stderr, "master reginfo:\n");
reginfo_dump(&master_ri, stderr);
-fprintf(stderr, "%s reginfo:\n", trace ? "trace" : "apprentice");
+fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&apprentice_ri, stderr);
-if (trace) {
-reginfo_dump_mismatch(&apprentice_ri, &master_ri, stderr);
-} else {
-reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
-}
+reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
return resp;
}
diff --git a/risu.c b/risu.c
index f238117..199f697 100644
--- a/risu.c
+++ b/risu.c
@@ -102,11 +102,7 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
RisuResult r;
[PATCH v3 22/25] Standardize reginfo_dump_mismatch printing
Hoist the "master vs apprentice" label to apprentice(), since
we will want different labels for dumping. Remove all of the
"mismatch" text from reginfo_dump_mismatch -- just print "vs".
Signed-off-by: Richard Henderson
---
risu.h | 4 ++--
risu.c | 1 +
risu_reginfo_aarch64.c | 12 +---
risu_reginfo_arm.c | 18 -
risu_reginfo_i386.c| 6 +-
risu_reginfo_m68k.c| 23 +++---
risu_reginfo_ppc64.c | 44 ++
7 files changed, 44 insertions(+), 64 deletions(-)
diff --git a/risu.h b/risu.h
index bdb70c1..99f0d8e 100644
--- a/risu.h
+++ b/risu.h
@@ -123,8 +123,8 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2);
/* print reginfo state to a stream, returns 1 on success, 0 on failure */
int reginfo_dump(struct reginfo *ri, FILE * f);
-/* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
+/* reginfo_dump_mismatch: print mismatch details to a stream */
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
/* return size of reginfo */
int reginfo_size(struct reginfo *ri);
diff --git a/risu.c b/risu.c
index 1c096a8..f613fa9 100644
--- a/risu.c
+++ b/risu.c
@@ -449,6 +449,7 @@ static int apprentice(void)
reginfo_dump(&ri[MASTER], stderr);
fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&ri[APPRENTICE], stderr);
+fprintf(stderr, "mismatch detail (master : apprentice):\n");
reginfo_dump_mismatch(&ri[MASTER], &ri[APPRENTICE], stderr);
return EXIT_FAILURE;
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index fb8e11a..b443745 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -316,15 +316,15 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
return !ferror(f);
}
-/* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
{
int i;
-fprintf(f, "mismatch detail (master : apprentice):\n");
+
if (m->faulting_insn != a->faulting_insn) {
-fprintf(f, " faulting insn mismatch %08x vs %08x\n",
+fprintf(f, " faulting insn: %08x vs %08x\n",
m->faulting_insn, a->faulting_insn);
}
+
for (i = 0; i < 31; i++) {
if (m->regs[i] != a->regs[i]) {
fprintf(f, " X%-2d: %016" PRIx64 " vs %016" PRIx64 "\n",
@@ -383,7 +383,7 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo
*a, FILE * f)
sve_dump_preg_diff(f, vq, pm, pa);
}
}
-return !ferror(f);
+return;
}
#endif
@@ -398,6 +398,4 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo
*a, FILE * f)
(uint64_t) a->simd.vregs[i]);
}
}
-
-return !ferror(f);
}
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 202120b..ba1035e 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -183,32 +183,33 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
return !ferror(f);
}
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
{
int i;
-fprintf(f, "mismatch detail (master : apprentice):\n");
if (m->faulting_insn_size != a->faulting_insn_size) {
-fprintf(f, " faulting insn size mismatch %d vs %d\n",
+fprintf(f, " faulting insn size: %d vs %d\n",
m->faulting_insn_size, a->faulting_insn_size);
} else if (m->faulting_insn != a->faulting_insn) {
if (m->faulting_insn_size == 2) {
-fprintf(f, " faulting insn mismatch %04x vs %04x\n",
+fprintf(f, " faulting insn: %04x vs %04x\n",
m->faulting_insn, a->faulting_insn);
} else {
-fprintf(f, " faulting insn mismatch %08x vs %08x\n",
+fprintf(f, " faulting insn: %08x vs %08x\n",
m->faulting_insn, a->faulting_insn);
}
}
+
for (i = 0; i < 16; i++) {
if (m->gpreg[i] != a->gpreg[i]) {
-fprintf(f, " r%d: %08x vs %08x\n", i, m->gpreg[i],
-a->gpreg[i]);
+fprintf(f, " r%d: %08x vs %08x\n", i, m->gpreg[i], a->gpreg[i]);
}
}
+
if (m->cpsr != a->cpsr) {
fprintf(f, " cpsr: %08x vs %08x\n", m->cpsr, a->cpsr);
}
+
for (i = 0; i < 32; i++) {
if (m->fpregs[i] != a->fpregs[i]) {
fprintf(f, " d%d: %016llx vs %016llx\n", i,
@@ -216,9 +217,8 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo
*a, FILE *f)
(unsigned long long) a->fpregs[i]);
}
}
+
if (m->fpscr != a->fpscr) {
fprin
[PATCH v3 10/25] Pass non-OK result back through siglongjmp
Rather than doing some work in the signal handler and
some work outside, move all of the non-resume work outside.
This works because we arranged for RES_OK to be 0, which
is the normal return from sigsetjmp.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.c | 50 --
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index 1917311..f238117 100644
--- a/risu.c
+++ b/risu.c
@@ -107,15 +107,10 @@ static void master_sigill(int sig, siginfo_t *si, void
*uc)
} else {
r = recv_and_compare_register_info(uc);
}
-
-switch (r) {
-case RES_OK:
-/* match OK */
+if (r == RES_OK) {
advance_pc(uc);
-return;
-default:
-/* mismatch, or end of test */
-siglongjmp(jmpbuf, 1);
+} else {
+siglongjmp(jmpbuf, r);
}
}
@@ -129,21 +124,10 @@ static void apprentice_sigill(int sig, siginfo_t *si,
void *uc)
} else {
r = send_register_info(uc);
}
-
-switch (r) {
-case RES_OK:
-/* match OK */
+if (r == RES_OK) {
advance_pc(uc);
-return;
-case RES_END:
-/* end of test */
-exit(EXIT_SUCCESS);
-default:
-/* mismatch */
-if (trace) {
-siglongjmp(jmpbuf, 1);
-}
-exit(EXIT_FAILURE);
+} else {
+siglongjmp(jmpbuf, r);
}
}
@@ -200,7 +184,9 @@ static void load_image(const char *imgfile)
static int master(void)
{
-if (sigsetjmp(jmpbuf, 1)) {
+RisuResult res = sigsetjmp(jmpbuf, 1);
+
+if (res != RES_OK) {
#ifdef HAVE_ZLIB
if (trace && comm_fd != STDOUT_FILENO) {
gzclose(gz_trace_file);
@@ -226,15 +212,27 @@ static int master(void)
static int apprentice(void)
{
-if (sigsetjmp(jmpbuf, 1)) {
+RisuResult res = sigsetjmp(jmpbuf, 1);
+
+if (res != RES_OK) {
#ifdef HAVE_ZLIB
if (trace && comm_fd != STDIN_FILENO) {
gzclose(gz_trace_file);
}
#endif
close(comm_fd);
-fprintf(stderr, "finished early after %zd checkpoints\n",
signal_count);
-return report_match_status(true);
+
+switch (res) {
+case RES_END:
+return EXIT_SUCCESS;
+default:
+if (!trace) {
+return EXIT_FAILURE;
+}
+fprintf(stderr, "finished early after %zd checkpoints\n",
+signal_count);
+return report_match_status(true);
+}
}
set_sigill_handler(&apprentice_sigill);
fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
--
2.20.1
[PATCH v3 08/25] Add enum RisuResult
Formalize the random set of numbers into an enum. Doing this
makes it easy to see that one of the responses in
recv_and_compare_register_info was inconsistent.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.h| 25 +
reginfo.c | 32
risu.c| 18 +-
3 files changed, 42 insertions(+), 33 deletions(-)
diff --git a/risu.h b/risu.h
index a7aa929..e6d07eb 100644
--- a/risu.h
+++ b/risu.h
@@ -57,6 +57,14 @@ typedef enum {
OP_COMPAREMEM = 4,
} RisuOp;
+/* Result of operation */
+typedef enum {
+RES_OK = 0,
+RES_END,
+RES_MISMATCH,
+RES_BAD_IO,
+} RisuResult;
+
/* The memory block should be this long */
#define MEMBLOCKLEN 8192
@@ -82,20 +90,21 @@ typedef struct {
*/
typedef int (*write_fn) (void *ptr, size_t bytes);
typedef int (*read_fn) (void *ptr, size_t bytes);
-typedef void (*respond_fn) (int response);
+typedef void (*respond_fn) (RisuResult response);
-/* Send the register information from the struct ucontext down the socket.
- * Return the response code from the master.
+/*
+ * Send the register information from the struct ucontext down the socket.
* NB: called from a signal handler.
*/
-int send_register_info(write_fn write_fn, void *uc);
+RisuResult send_register_info(write_fn write_fn, void *uc);
-/* Read register info from the socket and compare it with that from the
- * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
+/*
+ * Read register info from the socket and compare it with that from the
+ * ucontext.
* NB: called from a signal handler.
*/
-int recv_and_compare_register_info(read_fn read_fn,
- respond_fn respond, void *uc);
+RisuResult recv_and_compare_register_info(read_fn read_fn,
+ respond_fn respond, void *uc);
/* Print a useful report on the status of the last comparison
* done in recv_and_compare_register_info(). This is called on
diff --git a/reginfo.c b/reginfo.c
index 2d67c93..b909a1f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
-int send_register_info(write_fn write_fn, void *uc)
+RisuResult send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
trace_header_t header;
@@ -34,7 +34,7 @@ int send_register_info(write_fn write_fn, void *uc)
header.pc = get_pc(&ri);
header.risu_op = op;
if (write_fn(&header, sizeof(header)) != 0) {
-return -1;
+return RES_BAD_IO;
}
switch (op) {
@@ -46,10 +46,10 @@ int send_register_info(write_fn write_fn, void *uc)
* (b) end of test (c) a non-risuop UNDEF
*/
if (write_fn(&ri, reginfo_size()) != 0) {
-return -1;
+return RES_BAD_IO;
}
/* For OP_TEST_END, force return 1 to exit. */
-return op == OP_TESTEND;
+return op == OP_TESTEND ? RES_END : RES_OK;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -63,7 +63,7 @@ int send_register_info(write_fn write_fn, void *uc)
default:
abort();
}
-return 0;
+return RES_OK;
}
/* Read register info from the socket and compare it with that from the
@@ -74,10 +74,10 @@ int send_register_info(write_fn write_fn, void *uc)
* that says whether it is register or memory data, so if the two
* sides get out of sync then we will fail obscurely.
*/
-int recv_and_compare_register_info(read_fn read_fn,
- respond_fn resp_fn, void *uc)
+RisuResult recv_and_compare_register_info(read_fn read_fn,
+ respond_fn resp_fn, void *uc)
{
-int resp = 0;
+RisuResult resp = RES_OK;
trace_header_t header;
RisuOp op;
@@ -85,18 +85,18 @@ int recv_and_compare_register_info(read_fn read_fn,
op = get_risuop(&master_ri);
if (read_fn(&header, sizeof(header)) != 0) {
-return -1;
+return RES_BAD_IO;
}
if (header.risu_op != op) {
/* We are out of sync */
-resp = 2;
+resp = RES_BAD_IO;
resp_fn(resp);
return resp;
}
/* send OK for the header */
-resp_fn(0);
+resp_fn(RES_OK);
switch (op) {
case OP_COMPARE:
@@ -107,12 +107,12 @@ int recv_and_compare_register_info(read_fn read_fn,
*/
if (read_fn(&apprentice_ri, reginfo_size())) {
packet_mismatch = 1;
-resp = 2;
+resp = RES_BAD_IO;
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
/* register mismatch */
-resp = 2;
+resp = RES_MISMATCH;
} else if (op == OP_TESTEND) {
-resp = 1;
+resp = RES_END;
}
resp_fn(resp);
break;
@@ -127,10 +127,10 @@ int recv_an
[PATCH v3 19/25] aarch64: Reorg sve reginfo to save space
Mirror the signal frame by storing all of the registers
as a lump. Use the signal macros to pull out the values.
Signed-off-by: Richard Henderson
---
risu_reginfo_aarch64.h | 16 +
risu_reginfo_aarch64.c | 135 +
2 files changed, 73 insertions(+), 78 deletions(-)
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index c33b86f..01076b4 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -17,20 +17,8 @@
struct simd_reginfo {
__uint128_t vregs[32];
-char end[0];
};
-#ifdef SVE_MAGIC
-struct sve_reginfo {
-/* SVE */
-uint16_tvl; /* current VL */
-__uint128_t zregs[SVE_NUM_ZREGS][SVE_VQ_MAX];
-uint16_tpregs[SVE_NUM_PREGS][SVE_VQ_MAX];
-uint16_tffr[SVE_VQ_MAX];
-char end[0];
-};
-#endif
-
/* The kernel headers set this based on future arch extensions.
The current arch maximum is 16. Save space below. */
#undef SVE_VQ_MAX
@@ -47,11 +35,13 @@ struct reginfo {
/* FP/SIMD */
uint32_t fpsr;
uint32_t fpcr;
+uint32_t sve_vl;
union {
struct simd_reginfo simd;
#ifdef SVE_MAGIC
-struct sve_reginfo sve;
+char sve[SVE_SIG_CONTEXT_SIZE(16) - SVE_SIG_REGS_OFFSET]
+__attribute__((aligned(16)));
#endif
};
};
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 7044648..a1020ac 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -71,15 +71,30 @@ void process_arch_opt(int opt, const char *arg)
int reginfo_size(struct reginfo *ri)
{
-int size = offsetof(struct reginfo, simd.end);
#ifdef SVE_MAGIC
-if (test_sve) {
-size = offsetof(struct reginfo, sve.end);
+if (ri->sve_vl) {
+int vq = sve_vq_from_vl(ri->sve_vl);
+return (offsetof(struct reginfo, sve) +
+SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
}
#endif
-return size;
+return offsetof(struct reginfo, simd) + sizeof(ri->simd);
}
+#ifdef SVE_MAGIC
+static uint64_t *reginfo_zreg(struct reginfo *ri, int vq, int i)
+{
+return (uint64_t *)(ri->sve + SVE_SIG_ZREG_OFFSET(vq, i) -
+SVE_SIG_REGS_OFFSET);
+}
+
+static uint16_t *reginfo_preg(struct reginfo *ri, int vq, int i)
+{
+return (uint16_t *)(ri->sve + SVE_SIG_PREG_OFFSET(vq, i) -
+SVE_SIG_REGS_OFFSET);
+}
+#endif
+
/* reginfo_init: initialize with a ucontext */
void reginfo_init(struct reginfo *ri, ucontext_t *uc)
{
@@ -152,8 +167,6 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
return;
}
-ri->sve.vl = sve->vl;
-
if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
if (sve->head.size == sizeof(*sve)) {
/* SVE state is empty -- not an error. */
@@ -164,24 +177,9 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
return;
}
-/* Copy ZREG's one at a time */
-for (i = 0; i < SVE_NUM_ZREGS; i++) {
-memcpy(&ri->sve.zregs[i],
- (void *)sve + SVE_SIG_ZREG_OFFSET(vq, i),
- SVE_SIG_ZREG_SIZE(vq));
-}
-
-/* Copy PREG's one at a time */
-for (i = 0; i < SVE_NUM_PREGS; i++) {
-memcpy(&ri->sve.pregs[i],
- (void *)sve + SVE_SIG_PREG_OFFSET(vq, i),
- SVE_SIG_PREG_SIZE(vq));
-}
-
-/* Finally the FFR */
-memcpy(&ri->sve.ffr, (void *)sve + SVE_SIG_FFR_OFFSET(vq),
- SVE_SIG_FFR_SIZE(vq));
-
+ri->sve_vl = sve->vl;
+memcpy(ri->sve, (char *)sve + SVE_SIG_REGS_OFFSET,
+ SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
return;
}
#endif /* SVE_MAGIC */
@@ -225,18 +223,20 @@ static void sve_dump_preg_diff(FILE *f, int vq, const
uint16_t *p1,
fprintf(f, "\n");
}
-static void sve_dump_zreg_diff(FILE *f, int vq, const __uint128_t *z1,
- const __uint128_t *z2)
+static void sve_dump_zreg_diff(FILE *f, int vq, const uint64_t *za,
+ const uint64_t *zb)
{
const char *pad = "";
int q;
for (q = 0; q < vq; ++q) {
-if (z1[q] != z2[q]) {
+uint64_t za0 = za[2 * q], za1 = za[2 * q + 1];
+uint64_t zb0 = zb[2 * q], zb1 = zb[2 * q + 1];
+
+if (za0 != zb0 || za1 != zb1) {
fprintf(f, "%sq%-2d: %016" PRIx64 "%016" PRIx64
-" vs %016" PRIx64 "%016" PRIx64"\n", pad, q,
-(uint64_t)(z1[q] >> 64), (uint64_t)z1[q],
-(uint64_t)(z2[q] >> 64), (uint64_t)z2[q]);
+" vs %016" PRIx64 "%016" PRIx64"\n",
+pad, q, za1, za0, zb1, zb0);
pad = " ";
}
}
@@ -263,28 +263,30 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
if (test_sve) {
int q, vq = test_sve;
-fprintf(f, " vl : %d\n", ri->sve.vl);
+fprintf(f, " vl :
[PATCH v3 07/25] Add enum RisuOp
Formalize the set of defines, plus -1, into an enum.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.h | 23 +++
reginfo.c | 32 +++-
risu_aarch64.c | 6 +++---
risu_arm.c | 6 +++---
risu_i386.c| 4 ++--
risu_m68k.c| 4 ++--
risu_ppc64.c | 4 ++--
7 files changed, 46 insertions(+), 33 deletions(-)
diff --git a/risu.h b/risu.h
index e2b4508..a7aa929 100644
--- a/risu.h
+++ b/risu.h
@@ -45,11 +45,17 @@ extern uintptr_t image_start_address;
extern void *memblock;
/* Ops code under test can request from risu: */
-#define OP_COMPARE 0
-#define OP_TESTEND 1
-#define OP_SETMEMBLOCK 2
-#define OP_GETMEMBLOCK 3
-#define OP_COMPAREMEM 4
+typedef enum {
+/* Any other sigill besides the destignated undefined insn. */
+OP_SIGILL = -1,
+
+/* These are generated by the designated undefined insn. */
+OP_COMPARE = 0,
+OP_TESTEND = 1,
+OP_SETMEMBLOCK = 2,
+OP_GETMEMBLOCK = 3,
+OP_COMPAREMEM = 4,
+} RisuOp;
/* The memory block should be this long */
#define MEMBLOCKLEN 8192
@@ -114,10 +120,11 @@ void set_ucontext_paramreg(void *vuc, uint64_t value);
/* Return the value of the parameter register from a reginfo. */
uint64_t get_reginfo_paramreg(struct reginfo *ri);
-/* Return the risu operation number we have been asked to do,
- * or -1 if this was a SIGILL for a non-risuop insn.
+/*
+ * Return the risu operation number we have been asked to do,
+ * or OP_SIGILL if this was a SIGILL for a non-risuop insn.
*/
-int get_risuop(struct reginfo *ri);
+RisuOp get_risuop(struct reginfo *ri);
/* Return the PC from a reginfo */
uintptr_t get_pc(struct reginfo *ri);
diff --git a/reginfo.c b/reginfo.c
index 1b2a821..2d67c93 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -11,7 +11,7 @@
#include
#include
-
+#include
#include "risu.h"
struct reginfo master_ri, apprentice_ri;
@@ -25,7 +25,7 @@ int send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
trace_header_t header;
-int op;
+RisuOp op;
reginfo_init(&ri, uc);
op = get_risuop(&ri);
@@ -38,11 +38,18 @@ int send_register_info(write_fn write_fn, void *uc)
}
switch (op) {
+case OP_COMPARE:
case OP_TESTEND:
-write_fn(&ri, reginfo_size());
-/* if we are tracing write_fn will return 0 unlike a remote
- end, hence we force return of 1 here */
-return 1;
+case OP_SIGILL:
+/*
+ * Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+if (write_fn(&ri, reginfo_size()) != 0) {
+return -1;
+}
+/* For OP_TEST_END, force return 1 to exit. */
+return op == OP_TESTEND;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -53,12 +60,8 @@ int send_register_info(write_fn write_fn, void *uc)
case OP_COMPAREMEM:
return write_fn(memblock, MEMBLOCKLEN);
break;
-case OP_COMPARE:
default:
-/* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
-return write_fn(&ri, reginfo_size());
+abort();
}
return 0;
}
@@ -74,8 +77,9 @@ int send_register_info(write_fn write_fn, void *uc)
int recv_and_compare_register_info(read_fn read_fn,
respond_fn resp_fn, void *uc)
{
-int resp = 0, op;
+int resp = 0;
trace_header_t header;
+RisuOp op;
reginfo_init(&master_ri, uc);
op = get_risuop(&master_ri);
@@ -97,7 +101,7 @@ int recv_and_compare_register_info(read_fn read_fn,
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
-default:
+case OP_SIGILL:
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
@@ -130,6 +134,8 @@ int recv_and_compare_register_info(read_fn read_fn,
}
resp_fn(resp);
break;
+default:
+abort();
}
return resp;
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 492d141..f8a8412 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -29,16 +29,16 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->regs[0];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
/* Return the risuop we have been asked to do
- * (or -1 if this was a SIGILL for a non-risuop insn)
+ * (or OP_SIGILL if this was a SIGILL for a non-risuop insn)
*/
uint32_t insn = ri->faulting_insn;
uint32_t op = insn & 0xf;
uint32_t key = insn & ~0xf;
uint32_t risukey = 0x5af0;
-return (key != risukey) ? -1 : op;
+return (key != risukey) ? OP_SIGILL : op;
}
uintptr_t get_pc(struct reginfo *ri)
diff --git a/risu_arm.c b/risu_arm.c
index 5fcb2a5..a20bf73 100644
--- a/risu_arm.c
+++ b/r
[PATCH v3 06/25] Make some risu.c symbols static
These are unused in other translation units.
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
risu.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/risu.c b/risu.c
index 26dc116..ab17c71 100644
--- a/risu.c
+++ b/risu.c
@@ -31,18 +31,18 @@
void *memblock;
static int comm_fd;
-bool trace;
-size_t signal_count;
+static bool trace;
+static size_t signal_count;
#ifdef HAVE_ZLIB
#include
-gzFile gz_trace_file;
+static gzFile gz_trace_file;
#define TRACE_TYPE "compressed"
#else
#define TRACE_TYPE "uncompressed"
#endif
-sigjmp_buf jmpbuf;
+static sigjmp_buf jmpbuf;
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
@@ -113,7 +113,7 @@ void respond_trace(int r)
}
}
-void master_sigill(int sig, siginfo_t *si, void *uc)
+static void master_sigill(int sig, siginfo_t *si, void *uc)
{
int r;
signal_count++;
@@ -135,7 +135,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
}
}
-void apprentice_sigill(int sig, siginfo_t *si, void *uc)
+static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
int r;
signal_count++;
@@ -180,9 +180,9 @@ static void set_sigill_handler(void (*fn) (int, siginfo_t
*, void *))
typedef void entrypoint_fn(void);
uintptr_t image_start_address;
-entrypoint_fn *image_start;
+static entrypoint_fn *image_start;
-void load_image(const char *imgfile)
+static void load_image(const char *imgfile)
{
/* Load image file into memory as executable */
struct stat st;
@@ -214,7 +214,7 @@ void load_image(const char *imgfile)
image_start_address = (uintptr_t) addr;
}
-int master(void)
+static int master(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
@@ -240,7 +240,7 @@ int master(void)
return EXIT_FAILURE;
}
-int apprentice(void)
+static int apprentice(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
@@ -261,9 +261,9 @@ int apprentice(void)
return EXIT_FAILURE;
}
-int ismaster;
+static int ismaster;
-void usage(void)
+static void usage(void)
{
fprintf(stderr,
"Usage: risu [--master] [--host ] [--port ] "
--
2.20.1
[PATCH v3 05/25] Use EXIT_FAILURE, EXIT_SUCCESS
Some of the time we exit via the return value from main.
This can make it easier to tell what it is we're returning.
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
comms.c| 26 +-
risu.c | 22 +++---
risu_reginfo_aarch64.c | 4 ++--
risu_reginfo_i386.c| 2 +-
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/comms.c b/comms.c
index 6946fd9..861e845 100644
--- a/comms.c
+++ b/comms.c
@@ -31,7 +31,7 @@ int apprentice_connect(const char *hostname, int port)
sock = socket(PF_INET, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket");
-exit(1);
+exit(EXIT_FAILURE);
}
struct hostent *hostinfo;
sa.sin_family = AF_INET;
@@ -39,12 +39,12 @@ int apprentice_connect(const char *hostname, int port)
hostinfo = gethostbyname(hostname);
if (!hostinfo) {
fprintf(stderr, "Unknown host %s\n", hostname);
-exit(1);
+exit(EXIT_FAILURE);
}
sa.sin_addr = *(struct in_addr *) hostinfo->h_addr;
if (connect(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
perror("connect");
-exit(1);
+exit(EXIT_FAILURE);
}
return sock;
}
@@ -56,13 +56,13 @@ int master_connect(int port)
sock = socket(PF_INET, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket");
-exit(1);
+exit(EXIT_FAILURE);
}
int sora = 1;
if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &sora, sizeof(sora)) !=
0) {
perror("setsockopt(SO_REUSEADDR)");
-exit(1);
+exit(EXIT_FAILURE);
}
sa.sin_family = AF_INET;
@@ -70,11 +70,11 @@ int master_connect(int port)
sa.sin_addr.s_addr = htonl(INADDR_ANY);
if (bind(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
perror("bind");
-exit(1);
+exit(EXIT_FAILURE);
}
if (listen(sock, 1) < 0) {
perror("listen");
-exit(1);
+exit(EXIT_FAILURE);
}
/* Just block until we get a connection */
fprintf(stderr, "master: waiting for connection on port %d...\n",
@@ -84,7 +84,7 @@ int master_connect(int port)
int nsock = accept(sock, (struct sockaddr *) &csa, &csasz);
if (nsock < 0) {
perror("accept");
-exit(1);
+exit(EXIT_FAILURE);
}
/* We're done with the server socket now */
close(sock);
@@ -104,7 +104,7 @@ static void recv_bytes(int sock, void *pkt, int pktlen)
continue;
}
perror("read failed");
-exit(1);
+exit(EXIT_FAILURE);
}
pktlen -= i;
p += i;
@@ -127,7 +127,7 @@ static void recv_and_discard_bytes(int sock, int pktlen)
continue;
}
perror("read failed");
-exit(1);
+exit(EXIT_FAILURE);
}
pktlen -= i;
}
@@ -186,12 +186,12 @@ int send_data_pkt(int sock, void *pkt, int pktlen)
if (safe_writev(sock, iov, 2) == -1) {
perror("writev failed");
-exit(1);
+exit(EXIT_FAILURE);
}
if (read(sock, &resp, 1) != 1) {
perror("read failed");
-exit(1);
+exit(EXIT_FAILURE);
}
return resp;
}
@@ -217,6 +217,6 @@ void send_response_byte(int sock, int resp)
unsigned char r = resp;
if (write(sock, &r, 1) != 1) {
perror("write failed");
-exit(1);
+exit(EXIT_FAILURE);
}
}
diff --git a/risu.c b/risu.c
index 819b786..26dc116 100644
--- a/risu.c
+++ b/risu.c
@@ -153,13 +153,13 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
return;
case 1:
/* end of test */
-exit(0);
+exit(EXIT_SUCCESS);
default:
/* mismatch */
if (trace) {
siglongjmp(jmpbuf, 1);
}
-exit(1);
+exit(EXIT_FAILURE);
}
}
@@ -173,7 +173,7 @@ static void set_sigill_handler(void (*fn) (int, siginfo_t
*, void *))
sigemptyset(&sa.sa_mask);
if (sigaction(SIGILL, &sa, 0) != 0) {
perror("sigaction");
-exit(1);
+exit(EXIT_FAILURE);
}
}
@@ -190,11 +190,11 @@ void load_image(const char *imgfile)
int fd = open(imgfile, O_RDONLY);
if (fd < 0) {
fprintf(stderr, "failed to open image file %s\n", imgfile);
-exit(1);
+exit(EXIT_FAILURE);
}
if (fstat(fd, &st) != 0) {
perror("fstat");
-exit(1);
+exit(EXIT_FAILURE);
}
size_t len = st.st_size;
void *addr;
@@ -207,7 +207,7 @@ void load_image(const char *imgfile)
0);
if (!addr) {
perror("mmap");
-exit(1);
+exit(EXIT_FAILURE);
}
close(fd);
image_start = addr;
@@ -226,7 +226,7 @@ int master(void)
if (trace) {
fprintf(stderr, "trace complete after %zd checkpoints\n",
[PATCH v3 16/25] Split out recv_register_info
We will want to share this code when dumping.
Signed-off-by: Richard Henderson
---
risu.c | 50 ++
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/risu.c b/risu.c
index b91ad38..80bc3b1 100644
--- a/risu.c
+++ b/risu.c
@@ -166,6 +166,34 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
}
}
+static RisuResult recv_register_info(struct reginfo *ri)
+{
+RisuResult res;
+
+res = read_buffer(&header, sizeof(header));
+if (res != RES_OK) {
+return res;
+}
+
+/* send OK for the header */
+respond(RES_OK);
+
+switch (header.risu_op) {
+case OP_COMPARE:
+case OP_TESTEND:
+case OP_SIGILL:
+return read_buffer(ri, reginfo_size());
+case OP_COMPAREMEM:
+return read_buffer(other_memblock, MEMBLOCKLEN);
+case OP_SETMEMBLOCK:
+case OP_GETMEMBLOCK:
+return RES_OK;
+default:
+/* TODO: Create a better error message. */
+return RES_BAD_IO;
+}
+}
+
static RisuResult recv_and_compare_register_info(void *uc)
{
uint64_t paramreg;
@@ -173,33 +201,26 @@ static RisuResult recv_and_compare_register_info(void *uc)
RisuOp op;
reginfo_init(&ri[APPRENTICE], uc);
-op = get_risuop(&ri[APPRENTICE]);
-res = read_buffer(&header, sizeof(header));
+res = recv_register_info(&ri[MASTER]);
if (res != RES_OK) {
+/* I/O error. Tell master to exit. */
+respond(RES_END);
return res;
}
+op = get_risuop(&ri[APPRENTICE]);
if (header.risu_op != op) {
/* We are out of sync. Tell master to exit. */
respond(RES_END);
return RES_BAD_IO;
}
-/* send OK for the header */
-respond(RES_OK);
-
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
case OP_SIGILL:
-/* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
-res = read_buffer(&ri[MASTER], reginfo_size());
-if (res != RES_OK) {
-/* fail */
-} else if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
+if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
/* register mismatch */
res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
@@ -216,10 +237,7 @@ static RisuResult recv_and_compare_register_info(void *uc)
set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
-res = read_buffer(other_memblock, MEMBLOCKLEN);
-if (res != RES_OK) {
-/* fail */
-} else if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
+if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
--
2.20.1
[PATCH v3 03/25] Hoist trace file and socket opening
We will want to share this code with --dump.
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
v3: Hoist socket connecting as well as trace file opening.
---
risu.c | 49 +++--
1 file changed, 23 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index 059348f..2f6a677 100644
--- a/risu.c
+++ b/risu.c
@@ -363,6 +363,29 @@ int main(int argc, char **argv)
}
}
+if (trace) {
+if (strcmp(trace_fn, "-") == 0) {
+comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
+} else {
+if (ismaster) {
+comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+} else {
+comm_fd = open(trace_fn, O_RDONLY);
+}
+#ifdef HAVE_ZLIB
+gz_trace_file = gzdopen(comm_fd, ismaster ? "wb9" : "rb");
+#endif
+}
+} else {
+if (ismaster) {
+fprintf(stderr, "master port %d\n", port);
+comm_fd = master_connect(port);
+} else {
+fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
+comm_fd = apprentice_connect(hostname, port);
+}
+}
+
imgfile = argv[optind];
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
@@ -373,34 +396,8 @@ int main(int argc, char **argv)
load_image(imgfile);
if (ismaster) {
-if (trace) {
-if (strcmp(trace_fn, "-") == 0) {
-comm_fd = STDOUT_FILENO;
-} else {
-comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
-#ifdef HAVE_ZLIB
-gz_trace_file = gzdopen(comm_fd, "wb9");
-#endif
-}
-} else {
-fprintf(stderr, "master port %d\n", port);
-comm_fd = master_connect(port);
-}
return master();
} else {
-if (trace) {
-if (strcmp(trace_fn, "-") == 0) {
-comm_fd = STDIN_FILENO;
-} else {
-comm_fd = open(trace_fn, O_RDONLY);
-#ifdef HAVE_ZLIB
-gz_trace_file = gzdopen(comm_fd, "rb");
-#endif
-}
-} else {
-fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
-comm_fd = apprentice_connect(hostname, port);
-}
return apprentice();
}
}
--
2.20.1
[PATCH v3 02/25] Unify master_fd and apprentice_fd to comm_fd
Any one invocation cannot be both master and apprentice.
Let's use only one variable for the file descriptor.
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
risu.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/risu.c b/risu.c
index 79b1092..059348f 100644
--- a/risu.c
+++ b/risu.c
@@ -30,7 +30,7 @@
void *memblock;
-int apprentice_fd, master_fd;
+static int comm_fd;
bool trace;
size_t signal_count;
@@ -50,7 +50,7 @@ sigjmp_buf jmpbuf;
int read_sock(void *ptr, size_t bytes)
{
-return recv_data_pkt(master_fd, ptr, bytes);
+return recv_data_pkt(comm_fd, ptr, bytes);
}
int write_trace(void *ptr, size_t bytes)
@@ -58,9 +58,9 @@ int write_trace(void *ptr, size_t bytes)
size_t res;
#ifdef HAVE_ZLIB
-if (master_fd == STDOUT_FILENO) {
+if (comm_fd == STDOUT_FILENO) {
#endif
-res = write(master_fd, ptr, bytes);
+res = write(comm_fd, ptr, bytes);
#ifdef HAVE_ZLIB
} else {
res = gzwrite(gz_trace_file, ptr, bytes);
@@ -71,14 +71,14 @@ int write_trace(void *ptr, size_t bytes)
void respond_sock(int r)
{
-send_response_byte(master_fd, r);
+send_response_byte(comm_fd, r);
}
/* Apprentice function */
int write_sock(void *ptr, size_t bytes)
{
-return send_data_pkt(apprentice_fd, ptr, bytes);
+return send_data_pkt(comm_fd, ptr, bytes);
}
int read_trace(void *ptr, size_t bytes)
@@ -86,9 +86,9 @@ int read_trace(void *ptr, size_t bytes)
size_t res;
#ifdef HAVE_ZLIB
-if (apprentice_fd == STDIN_FILENO) {
+if (comm_fd == STDIN_FILENO) {
#endif
-res = read(apprentice_fd, ptr, bytes);
+res = read(comm_fd, ptr, bytes);
#ifdef HAVE_ZLIB
} else {
res = gzread(gz_trace_file, ptr, bytes);
@@ -218,11 +218,11 @@ int master(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
-if (trace && master_fd != STDOUT_FILENO) {
+if (trace && comm_fd != STDOUT_FILENO) {
gzclose(gz_trace_file);
}
#endif
-close(master_fd);
+close(comm_fd);
if (trace) {
fprintf(stderr, "trace complete after %zd checkpoints\n",
signal_count);
@@ -244,11 +244,11 @@ int apprentice(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
-if (trace && apprentice_fd != STDIN_FILENO) {
+if (trace && comm_fd != STDIN_FILENO) {
gzclose(gz_trace_file);
}
#endif
-close(apprentice_fd);
+close(comm_fd);
fprintf(stderr, "finished early after %zd checkpoints\n",
signal_count);
return report_match_status(true);
}
@@ -375,31 +375,31 @@ int main(int argc, char **argv)
if (ismaster) {
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
-master_fd = STDOUT_FILENO;
+comm_fd = STDOUT_FILENO;
} else {
-master_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
#ifdef HAVE_ZLIB
-gz_trace_file = gzdopen(master_fd, "wb9");
+gz_trace_file = gzdopen(comm_fd, "wb9");
#endif
}
} else {
fprintf(stderr, "master port %d\n", port);
-master_fd = master_connect(port);
+comm_fd = master_connect(port);
}
return master();
} else {
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
-apprentice_fd = STDIN_FILENO;
+comm_fd = STDIN_FILENO;
} else {
-apprentice_fd = open(trace_fn, O_RDONLY);
+comm_fd = open(trace_fn, O_RDONLY);
#ifdef HAVE_ZLIB
-gz_trace_file = gzdopen(apprentice_fd, "rb");
+gz_trace_file = gzdopen(comm_fd, "rb");
#endif
}
} else {
fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
-apprentice_fd = apprentice_connect(hostname, port);
+comm_fd = apprentice_connect(hostname, port);
}
return apprentice();
}
--
2.20.1
[PATCH v3 00/25] risu cleanups and improvements
Version 3 changes the --dump option to --fulldump and --diffdump, after an off-hand suggestion by Alex. These are now mode options, similar to --master. Which means that dumping is an orthogonal apprentice type, which means that we can dump from a socket. I'm not sure that will be useful as such, but I think it makes main be a bit cleaner. If using old trace files with the new risu, you get Unexpected magic number: 0x78 If for somehow you use different risu for master and apprentice on sockets, the apprentice will hang waiting for data that the master will never write. This is less than helpful, but should be trivial to avoid. While cleaning up the interface for reginfo_dump_mismatch(), I noticed some bugs on the ppc64 side. The patches without reviews are: 0014-Merge-reginfo.c-into-risu.c.patch 0015-Rearrange-reginfo-and-memblock-buffers.patch 0016-Split-out-recv_register_info.patch 0017-Add-magic-and-size-to-the-trace-header.patch 0018-Compute-reginfo_size-based-on-the-reginfo.patch 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch 0020-aarch64-Use-arch_init-to-configure-sve.patch 0021-ppc64-Use-uint64_t-to-represent-double.patch 0022-Standardize-reginfo_dump_mismatch-printing.patch 0023-Add-fulldump-and-diffdup-options.patch 0024-Remove-return-value-from-reginfo_dump.patch 0025-ppc64-Clean-up-reginfo-handling.patch most of which are new, and those that aren't new have had significant modifications. r~ Richard Henderson (25): Use bool for tracing variables Unify master_fd and apprentice_fd to comm_fd Hoist trace file and socket opening Adjust tracefile open for write Use EXIT_FAILURE, EXIT_SUCCESS Make some risu.c symbols static Add enum RisuOp Add enum RisuResult Unify i/o functions and use RisuResult Pass non-OK result back through siglongjmp Always write for --master Simplify syncing with master Split RES_MISMATCH for registers and memory Merge reginfo.c into risu.c Rearrange reginfo and memblock buffers Split out recv_register_info Add magic and size to the trace header Compute reginfo_size based on the reginfo aarch64: Reorg sve reginfo to save space aarch64: Use arch_init to configure sve ppc64: Use uint64_t to represent double Standardize reginfo_dump_mismatch printing Add --fulldump and --diffdup options Remove return value from reginfo_dump ppc64: Clean up reginfo handling Makefile | 2 +- risu.h | 103 +++ risu_reginfo_aarch64.h | 16 +- risu_reginfo_ppc64.h | 3 +- comms.c| 34 +-- reginfo.c | 183 --- risu.c | 676 ++--- risu_aarch64.c | 6 +- risu_arm.c | 6 +- risu_i386.c| 4 +- risu_m68k.c| 4 +- risu_ppc64.c | 4 +- risu_reginfo_aarch64.c | 212 +++-- risu_reginfo_arm.c | 32 +- risu_reginfo_i386.c| 22 +- risu_reginfo_m68k.c| 37 +-- risu_reginfo_ppc64.c | 183 +-- 17 files changed, 803 insertions(+), 724 deletions(-) delete mode 100644 reginfo.c -- 2.20.1
[PATCH v3 04/25] Adjust tracefile open for write
Truncate the new output file. Rely on umask to remove
group+other file permissions, if desired.
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
risu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/risu.c b/risu.c
index 2f6a677..819b786 100644
--- a/risu.c
+++ b/risu.c
@@ -368,7 +368,7 @@ int main(int argc, char **argv)
comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
} else {
if (ismaster) {
-comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+comm_fd = open(trace_fn, O_WRONLY | O_CREAT | O_TRUNC, 0666);
} else {
comm_fd = open(trace_fn, O_RDONLY);
}
--
2.20.1
[PATCH v3 14/25] Merge reginfo.c into risu.c
The distinction between the two is artificial. Following
patches will rearrange the functions involved to make it
easier for dumping of the trace file.
Signed-off-by: Richard Henderson
---
Makefile | 2 +-
risu.h| 28 +-
reginfo.c | 151 --
risu.c| 129 --
4 files changed, 126 insertions(+), 184 deletions(-)
delete mode 100644 reginfo.c
diff --git a/Makefile b/Makefile
index 6ab014a..ad7f879 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@ CFLAGS ?= -g
ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) -U$(ARCH) $(BUILD_INC)
$(CFLAGS) $(EXTRA_CFLAGS)
PROG=risu
-SRCS=risu.c comms.c reginfo.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
+SRCS=risu.c comms.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
HDRS=risu.h risu_reginfo_$(ARCH).h
BINS=test_$(ARCH).bin
diff --git a/risu.h b/risu.h
index 77d6128..dd9fda5 100644
--- a/risu.h
+++ b/risu.h
@@ -35,7 +35,6 @@ void process_arch_opt(int opt, const char *arg);
#include REGINFO_HEADER(ARCH)
extern uintptr_t image_start_address;
-extern void *memblock;
/* Ops code under test can request from risu: */
typedef enum {
@@ -83,34 +82,9 @@ void send_response_byte(int sock, int resp);
/* Functions operating on reginfo */
-/* Function prototypes for read/write helper functions. */
-RisuResult write_buffer(void *ptr, size_t bytes);
-RisuResult read_buffer(void *ptr, size_t bytes);
-void respond(RisuResult response);
-
-/*
- * Send the register information from the struct ucontext down the socket.
- * NB: called from a signal handler.
- */
-RisuResult send_register_info(void *uc);
-
-/*
- * Read register info from the socket and compare it with that from the
- * ucontext.
- * NB: called from a signal handler.
- */
-RisuResult recv_and_compare_register_info(void *uc);
-
-/*
- * Print a useful report on the status of the last reg comparison
- * done in recv_and_compare_register_info().
- */
-void report_mismatch_reg(void);
-
/* Interface provided by CPU-specific code: */
-/* Move the PC past this faulting insn by adjusting ucontext
- */
+/* Move the PC past this faulting insn by adjusting ucontext. */
void advance_pc(void *uc);
/* Set the parameter register in a ucontext_t to the specified value.
diff --git a/reginfo.c b/reginfo.c
deleted file mode 100644
index a007f16..000
--- a/reginfo.c
+++ /dev/null
@@ -1,151 +0,0 @@
-/**
- * Copyright (c) 2017 Linaro Limited
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- * Peter Maydell (Linaro) - initial implementation
- */
-
-#include
-#include
-#include
-#include "risu.h"
-
-static struct reginfo master_ri, apprentice_ri;
-static uint8_t master_memblock[MEMBLOCKLEN];
-
-RisuResult send_register_info(void *uc)
-{
-struct reginfo ri;
-trace_header_t header;
-RisuResult res;
-RisuOp op;
-
-reginfo_init(&ri, uc);
-op = get_risuop(&ri);
-
-/* Write a header with PC/op to keep in sync */
-header.pc = get_pc(&ri);
-header.risu_op = op;
-res = write_buffer(&header, sizeof(header));
-if (res != RES_OK) {
-return res;
-}
-
-switch (op) {
-case OP_COMPARE:
-case OP_TESTEND:
-case OP_SIGILL:
-/*
- * Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
-res = write_buffer(&ri, reginfo_size());
-/* For OP_TEST_END, force exit. */
-if (res == RES_OK && op == OP_TESTEND) {
-res = RES_END;
-}
-break;
-case OP_SETMEMBLOCK:
-memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
-break;
-case OP_GETMEMBLOCK:
-set_ucontext_paramreg(uc,
- get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
-break;
-case OP_COMPAREMEM:
-return write_buffer(memblock, MEMBLOCKLEN);
-default:
-abort();
-}
-return res;
-}
-
-/* Read register info from the socket and compare it with that from the
- * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
- * NB: called from a signal handler.
- *
- * We don't have any kind of identifying info in the incoming data
- * that says whether it is register or memory data, so if the two
- * sides get out of sync then we will fail obscurely.
- */
-RisuResult recv_and_compare_register_info(void *uc)
-{
-RisuResult res;
-trace_header_t header;
-RisuOp op;
-
-reginfo_init(&apprentice_ri, uc);
-op = get_risuop(&apprentice_ri);
-
-res = read_buffer(&header, sizeof(header));
-if (res !=
[PATCH v3 01/25] Use bool for tracing variables
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
risu.h| 3 ++-
reginfo.c | 2 +-
risu.c| 8
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/risu.h b/risu.h
index 8d2d646..e2b4508 100644
--- a/risu.h
+++ b/risu.h
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
/* Extra option processing for architectures */
extern const struct option * const arch_long_opts;
@@ -96,7 +97,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(int trace);
+int report_match_status(bool trace);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index dd42ae2..1b2a821 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -141,7 +141,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(int trace)
+int report_match_status(bool trace)
{
int resp = 0;
fprintf(stderr, "match status...\n");
diff --git a/risu.c b/risu.c
index 01525d2..79b1092 100644
--- a/risu.c
+++ b/risu.c
@@ -31,7 +31,7 @@
void *memblock;
int apprentice_fd, master_fd;
-int trace;
+bool trace;
size_t signal_count;
#ifdef HAVE_ZLIB
@@ -228,7 +228,7 @@ int master(void)
signal_count);
return 0;
} else {
-return report_match_status(0);
+return report_match_status(false);
}
}
set_sigill_handler(&master_sigill);
@@ -250,7 +250,7 @@ int apprentice(void)
#endif
close(apprentice_fd);
fprintf(stderr, "finished early after %zd checkpoints\n",
signal_count);
-return report_match_status(1);
+return report_match_status(true);
}
set_sigill_handler(&apprentice_sigill);
fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
@@ -344,7 +344,7 @@ int main(int argc, char **argv)
break;
case 't':
trace_fn = optarg;
-trace = 1;
+trace = true;
break;
case 'h':
hostname = optarg;
--
2.20.1
Re: [PATCH v3 00/11] accel: Allow targets to use Kconfig, disable semihosting by default
Patchew URL: https://patchew.org/QEMU/20200521195911.19685-1-phi...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTiotest-qcow2: 040 socket_accept failed: Resource temporarily unavailable ** ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) /tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) make: *** [check-qtest-x86_64] Error 1 make: *** Waiting for unfinished jobs TESTiotest-qcow2: 041 TESTiotest-qcow2: 042 --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=42446f1cb3d84b08973bc47e5fc241a2', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-m5v271eo/src/docker-src.2020-05-21-22.14.17.8285:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=42446f1cb3d84b08973bc47e5fc241a2 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-m5v271eo/src' make: *** [docker-run-test-quick@centos7] Error 2 real15m44.566s user0m8.933s The full log is available at http://patchew.org/logs/20200521195911.19685-1-phi...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH] target/i386: correct fix for pcmpxstrx substring search
This corrects a bug introduced in my previous fix for SSE4.2 pcmpestri
/ pcmpestrm / pcmpistri / pcmpistrm substring search, commit
ae35eea7e4a9f21dd147406dfbcd0c4c6aaf2a60.
That commit fixed a bug that showed up in four GCC tests with one libc
implementation. The tests in question generate random inputs to the
intrinsics and compare results to a C implementation, but they only
test 1024 possible random inputs, and when the tests use the cases of
those instructions that work with word rather than byte inputs, it's
easy to have problematic cases that show up much less frequently than
that. Thus, testing with a different libc implementation, and so a
different random number generator, showed up a problem with the
previous patch.
When investigating the previous test failures, I found the description
of these instructions in the Intel manuals (starting from computing a
16x16 or 8x8 set of comparison results) confusing and hard to match up
with the more optimized implementation in QEMU, and referred to AMD
manuals which described the instructions in a different way. Those
AMD descriptions are very explicit that the whole of the string being
searched for must be found in the other operand, not running off the
end of that operand; they say "If the prototype and the SUT are equal
in length, the two strings must be identical for the comparison to be
TRUE.". However, that statement is incorrect.
In my previous commit message, I noted:
The operation in this case is a search for a string (argument d to
the helper) in another string (argument s to the helper); if a copy
of d at a particular position would run off the end of s, the
resulting output bit should be 0 whether or not the strings match in
the region where they overlap, but the QEMU implementation was
wrongly comparing only up to the point where s ends and counting it
as a match if an initial segment of d matched a terminal segment of
s. Here, "run off the end of s" means that some byte of d would
overlap some byte outside of s; thus, if d has zero length, it is
considered to match everywhere, including after the end of s.
The description "some byte of d would overlap some byte outside of s"
is accurate only when understood to refer to overlapping some byte
*within the 16-byte operand* but at or after the zero terminator; it
is valid to run over the end of s if the end of s is the end of the
16-byte operand. So the fix in the previous patch for the case of d
being empty was correct, but the other part of that patch was not
correct (as it never allowed partial matches even at the end of the
16-byte operand). Nor was the code before the previous patch correct
for the case of d nonempty, as it would always have allowed partial
matches at the end of s.
Fix with a partial revert of my previous change, combined with
inserting a check for the special case of s having maximum length to
determine where it is necessary to check for matches.
In the added test, test 1 is for the case of empty strings, which
failed before my 2017 patch, test 2 is for the bug introduced by my
2017 patch and test 3 deals with the case where a match of an initial
segment at the end of the string is not valid when the string ends
before the end of the 16-byte operand (that is, the case that would be
broken by a simple revert of the non-empty-string part of my 2017
patch).
Signed-off-by: Joseph Myers
---
target/i386/ops_sse.h| 4 ++--
tests/tcg/i386/Makefile.target | 3 +++
tests/tcg/i386/test-i386-pcmpistri.c | 33
3 files changed, 38 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/i386/test-i386-pcmpistri.c
diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index ec1ec745d0..f5ede2ca27 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -2076,10 +2076,10 @@ static inline unsigned pcmpxstrx(CPUX86State *env, Reg
*d, Reg *s,
res = (2 << upper) - 1;
break;
}
-for (j = valids - validd; j >= 0; j--) {
+for (j = valids == upper ? valids : valids - validd; j >= 0; j--) {
res <<= 1;
v = 1;
-for (i = validd; i >= 0; i--) {
+for (i = MIN(valids - j, validd); i >= 0; i--) {
v &= (pcmp_val(s, ctrl, i + j) == pcmp_val(d, ctrl, i));
}
res |= v;
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index 43ee2e181e..de5a3a275f 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -10,6 +10,9 @@ ALL_X86_TESTS=$(I386_SRCS:.c=)
SKIP_I386_TESTS=test-i386-ssse3
X86_64_TESTS:=$(filter test-i386-ssse3, $(ALL_X86_TESTS))
+test-i386-pcmpistri: CFLAGS += -msse4.2
+test-i386-pcmpistri: QEMU_OPTS += -cpu max
+
#
# hello-i386 is a barebones app
#
diff --git a/tests/tcg/i386/test-i386-pcmpistri.c
b/tests/tcg/i386/test-i386-pcmpistri.c
new file mode 100644
index 00..37cb56d669
--- /dev/nul
Re: cluster_size got from backup_calculate_cluster_size()
On 2020/5/22 上午4:53, Vladimir Sementsov-Ogievskiy wrote:
21.05.2020 21:19, John Snow wrote:
On 5/21/20 5:56 AM, Derek Su wrote:
Hi,
The cluster_size got from backup_calculate_cluster_size(),
MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size), is 64K regardless
of the target image's cluster size.
Not regardless -- but it is using 64K as a minimum.
Right, 64K is a minimum. Thanks for the correctness.
For example:
If the cluster size of source and target qcow2 images are both 16K, the
64K from backup_calculate_cluster_size() results in unwanted copies of
clusters.
The behavior slows down the backup (block-copy) process when the
source image receives lots of rand writes.
Are we talking about incremental, full, or top?
Our use case is MIRROR_SYNC_MODE_NONE (only copy new writes from source
image to the target).
Is the following calculation reasonable for the above issue?
```
static int64_t backup_calculate_cluster_size(BlockDriverState *target,
Error **errp)
{
...
ret = bdrv_get_info(target, &bdi);
...
return (bdi.cluster_size == 0 ?
BACKUP_CLUSTER_SIZE_DEFAULT : cluster_size);
}
```
Thanks.
Regards,
Derek
Might be reasonable. When I made this the "default", it actually used to
just be "hardcoded." We could continue in this direction and go all the
way and turn it into a tune-able.
I didn't think to make it lower than 64K because I was thinking about
the linear, full backup case where cluster sizes that were too small
might slow down the loop with too much metadata.
Yes, currently backup-loop is copying cluster-by-cluster, so if we allow
clusters less than 64k, it may become slower (at least for full-backup).
Still, my work about backup-performance is close to its end, and I hope,
in 5.1 will be merged. One of effects is that backup copying loop may
copy more than a cluster at once, so this problem will gone.
Keeping this in mind, I think we can allow smaller clusters now, as
anyway, small clusters is a rare special case.
(And the default qcow2 is 64K, so it seemed like a good choice at the
time.)
We could create a default-cluster-size tunable, perhaps. It's at 64K
now, but perhaps you can override it down to 16 or lower. We could
possibly treat a value of 0 as "no minimum; use the smallest common
size."
This is a separate issue entirely, but we may also wish to begin
offering a cluster-size override directly. In the case that we know this
value is unsafe, we reject the command. In the case that it's ambiguous
due to lack of info, we can defer to the user's judgment. This would
allow us to force the backup to run in cases where we presently abort
out of fear.
CCing Vladimir who has been working on the backup loop extensively.
--js
Yes. I agree with that the minimum 64K or larger is a good choice for
the linear and full backup choice.
In the COLO application (https://wiki.qemu.org/Features/COLO), the
primary vm (PVM)'s write requests are replicated to the secondary
vm(SVM)'s secondary disk. The writes also trigger block copies copying
the old data on the secondary disk (source) to the hidden disk (target).
However, the checkpoint mechanism empties the hidden disk periodically,
so it is a endless backup (block-copy) process.
If the PVM write requests are random 4K blocks, the block copy on SVM
becomes inefficient in the above use case.
I conducted some performance tests.
If the qcow2 cluster size 16K, compared with the 64K backup granularity
size, the 16K backup granularity size shows that the seq. write
performance decreases by 10% and the rand. write performance increases
by 40%.
I think a tunable default-cluster-size seems good for such applications.
Thanks.
Regards,
Derek
[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU
Appreciate any comments or clues
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879425
Title:
The thread of "CPU 0 /KVM" keeping 99.9%CPU
Status in QEMU:
New
Bug description:
Hi Expert:
The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by
qemu-kvm.
Notes:
for VM:
OS: RHEL8.1
CPU: 1
MEM:4G
For qemu-kvm(host kernel RHEL7):
1) version:
/usr/libexec/qemu-kvm -version
QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by
com "top -p VM_pro_ID"
PID UDER PR NI RES S % CPU %MEM TIME+COMMAND
872067 qemu 20 0 1.6g R 99.9 0.6 37:08.87 CPU 0/KVM
3) use "pstack 493307" and below is function trace
Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
#0 0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
#1 0x55ff34bdf4a9 in qemu_poll_ns ()
#2 0x55ff34be02a8 in main_loop_wait ()
#3 0x55ff348bfb1a in main ()
4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8,
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82,
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121,
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
Therefore, I think the thread "CPU 0/KVM" is in tight loop.
5) use reset can recover this issue. however, it will reoccurred again.
Current work around is increase one CPU for this VM, then issue is gone.
thanks
Cliff
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879425/+subscriptions
Re: [PATCH v2 0/7] Misc display/sm501 clean ups and fixes
Patchew URL: https://patchew.org/QEMU/cover.1590089984.git.bala...@eik.bme.hu/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTiotest-qcow2: 039 socket_accept failed: Resource temporarily unavailable ** ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) /tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) make: *** [check-qtest-x86_64] Error 1 make: *** Waiting for unfinished jobs TESTiotest-qcow2: 040 qemu-system-aarch64: -accel kvm: invalid accelerator kvm --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=41602df6cffc4816b3eba0e376c10099', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-oarug6fo/src/docker-src.2020-05-21-21.34.23.8549:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=41602df6cffc4816b3eba0e376c10099 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-oarug6fo/src' make: *** [docker-run-test-quick@centos7] Error 2 real16m0.876s user0m10.783s The full log is available at http://patchew.org/logs/cover.1590089984.git.bala...@eik.bme.hu/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 1/3] hw/riscv: spike: Remove deprecated ISA specific machines
On Fri, May 8, 2020 at 3:21 AM Alistair Francis
wrote:
>
> The ISA specific Spike machines have been deprecated in QEMU since 4.1,
> let's finally remove them.
>
> Signed-off-by: Alistair Francis
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/riscv/spike.c | 217 ---
> include/hw/riscv/spike.h | 6 +-
> 2 files changed, 2 insertions(+), 221 deletions(-)
>
[snip]
> diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
> index dc770421bc..b98cfea0e4 100644
> --- a/include/hw/riscv/spike.h
> +++ b/include/hw/riscv/spike.h
> @@ -39,11 +39,9 @@ enum {
> };
>
> #if defined(TARGET_RISCV32)
> -#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV32GCSU_V1_09_1
> -#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV32GCSU_V1_10_0
> +#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U34
> #elif defined(TARGET_RISCV64)
> -#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV64GCSU_V1_09_1
> -#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV64GCSU_V1_10_0
> +#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U54
> #endif
On a second thought, I think we should not use the SIFIVE CPU type
here for Spike.
It should use the one that is used by 'virt', eg: TYPE_RISCV_CPU_BASE{32,64}
Regards,
Bin
Re: [PATCH v2 1/2] spapr: Add associativity reference point count to machine info
On Thu, May 21, 2020 at 01:34:37AM +0200, Greg Kurz wrote: On Mon, 18 May 2020 16:44:17 -0500 Reza Arbab wrote: @@ -944,8 +946,9 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt) qemu_hypertas->str, qemu_hypertas->len)); g_string_free(qemu_hypertas, TRUE); +nr_refpoints = MIN(smc->nr_assoc_refpoints, ARRAY_SIZE(refpoints)); Having the machine requesting more reference points than available would clearly be a bug. I'd rather add an assert() than silently clipping to the size of refpoints[]. I'll rework nr_assoc_refpoints into a bool as David suggested. Struggling for a name at the moment, but I'll think on it. _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points", - refpoints, sizeof(refpoints))); + refpoints, nr_refpoints * sizeof(uint32_t))); Size can be expressed without yet another explicit reference to the uint32_t type: nr_refpoints * sizeof(refpoints[0]) Will do. @@ -4541,6 +4544,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) smc->linux_pci_probe = true; smc->smp_threads_vsmt = true; smc->nr_xirqs = SPAPR_NR_XIRQS; +smc->nr_assoc_refpoints = 2; When adding a new setting for the default machine type, we usually take care of older machine types at the same time, ie. folding this patch into the next one. Both patches are simple enough that it should be okay and this would avoid this line to be touched again. No problem. I'll squash the patches together and work in that PHB compat property as well. Thanks for your feedback! -- Reza Arbab
Re: [PATCH v3 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()
On 4/30/20 6:10 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, prepare bdrv_co_do_copy_on_readv() now.
Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 6 +++---
block/trace-events | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 8bb4ea6285..6990d8cabe 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1088,7 +1088,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs,
int64_t offset,
}
static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
-int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+int64_t offset, int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset, int flags)
Widens from 32-bit to 63-bit. One caller:
bdrv_aligned_preadv() passes unsigned int (for now) - safe
{
BlockDriverState *bs = child->bs;
@@ -1103,11 +1103,11 @@ static int coroutine_fn
bdrv_co_do_copy_on_readv(BdrvChild *child,
BlockDriver *drv = bs->drv;
int64_t cluster_offset;
int64_t cluster_bytes;
-size_t skip_bytes;
+int64_t skip_bytes;
int ret;
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
BDRV_REQUEST_MAX_BYTES);
-unsigned int progress = 0;
+int64_t progress = 0;
bool skip_write;
Use of 'bytes', 'sskip_bytes', and 'progress' within the function:
bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset,
&cluster_bytes);
- safe, takes int64_t. Pre-patch, cluster_bytes could be 33 bits (a
misaligned request just under UINT_MAX can expand to > UINT_MAX when
aligned to clusters), but only if bytes could be larger than our <2G cap
that we use elsewhere. But even if we relax that 2G cap in later
patches, we should be okay even if 'bytes' starts at larger than 32
bits, because we don't allow images that would overflow INT64_MAX when
rounded up to cluster boundaries
skip_bytes = offset - cluster_offset;
- actually oversized - the difference is never going to be larger than
a cluster (which is capped at 2M for qcow2, for example), but doesn't
hurt that it is now a 64-bit value
trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
- safe, tweaked in this patch
assert(progress >= bytes);
- safe: progress never exceeds pnum, and both variables are same type
pre- and post-patch
assert(skip_bytes < pnum);
- safe
qemu_iovec_from_buf(qiov, qiov_offset + progress,
bounce_buffer + skip_bytes,
MIN(pnum - skip_bytes, bytes -
progress));
- tricky - pre-patch, pnum was int64_t, post-patch, we have three more
int64_t entities. Either way, we're passing int64_t to a size_t
parameter, which narrows on 64-bit. However, we're safe: this call is
in a loop where pnum is clamped at MAX_BOUNCE_BUFFER which is less than
32 bits, and the MIN() here means we never overflow
ret = bdrv_driver_preadv(bs, offset + progress,
MIN(pnum - skip_bytes, bytes -
progress),
qiov, qiov_offset + progress, 0);
- safe - takes int64_t (earlier in this series), and same analysis
about the MIN() picking something clamped at MAX_BOUNCE_BUFFER
progress += pnum - skip_bytes;
skip_bytes = 0;
- safe
Reviewed-by: Eric Blake
if (!drv) {
diff --git a/block/trace-events b/block/trace-events
index 29dff8881c..179b47bf63 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -14,7 +14,7 @@ blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p
bs %p"
bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset
%"PRId64" nbytes %"PRId64" flags 0x%x"
bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset
%"PRId64" nbytes %"PRId64" flags 0x%x"
bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset
%"PRId64" count %d flags 0x%x"
-bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, int64_t
cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64"
cluster_bytes %"PRId64
+bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes)
"bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes
%" PRId64
bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int
read_flags, int write_flags) "src %p offset
Re: [PATCH 1/2] hw/riscv: sifive_u: Remove the riscv_ prefix of the soc* functions
On Thu, May 21, 2020 at 7:48 AM Philippe Mathieu-Daudé wrote:
>
> On 5/21/20 4:42 PM, Bin Meng wrote:
> > From: Bin Meng
> >
> > To keep consistency with the machine* functions, remove the riscv_
> > prefix of the soc* functions.
> >
> > Signed-off-by: Bin Meng
Reviewed-by: Alistair Francis
Alistair
> > ---
> >
> > hw/riscv/sifive_u.c | 24
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 4299bdf..f9fef2b 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -481,7 +481,7 @@ static void sifive_u_machine_init_register_types(void)
> >
> > type_init(sifive_u_machine_init_register_types)
> >
> > -static void riscv_sifive_u_soc_init(Object *obj)
> > +static void sifive_u_soc_instance_init(Object *obj)
> > {
> > MachineState *ms = MACHINE(qdev_get_machine());
> > SiFiveUSoCState *s = RISCV_U_SOC(obj);
> > @@ -520,7 +520,7 @@ static void riscv_sifive_u_soc_init(Object *obj)
> > TYPE_CADENCE_GEM);
> > }
> >
> > -static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
> > +static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
> > {
> > MachineState *ms = MACHINE(qdev_get_machine());
> > SiFiveUSoCState *s = RISCV_U_SOC(dev);
> > @@ -635,32 +635,32 @@ static void riscv_sifive_u_soc_realize(DeviceState
> > *dev, Error **errp)
> > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size);
> > }
> >
> > -static Property riscv_sifive_u_soc_props[] = {
> > +static Property sifive_u_soc_props[] = {
> > DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
> > DEFINE_PROP_END_OF_LIST()
> > };
> >
> > -static void riscv_sifive_u_soc_class_init(ObjectClass *oc, void *data)
> > +static void sifive_u_soc_class_init(ObjectClass *oc, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(oc);
> >
> > -device_class_set_props(dc, riscv_sifive_u_soc_props);
> > -dc->realize = riscv_sifive_u_soc_realize;
> > +device_class_set_props(dc, sifive_u_soc_props);
> > +dc->realize = sifive_u_soc_realize;
> > /* Reason: Uses serial_hds in realize function, thus can't be used
> > twice */
> > dc->user_creatable = false;
> > }
> >
> > -static const TypeInfo riscv_sifive_u_soc_type_info = {
> > +static const TypeInfo sifive_u_soc_type_info = {
> > .name = TYPE_RISCV_U_SOC,
> > .parent = TYPE_DEVICE,
> > .instance_size = sizeof(SiFiveUSoCState),
> > -.instance_init = riscv_sifive_u_soc_init,
> > -.class_init = riscv_sifive_u_soc_class_init,
> > +.instance_init = sifive_u_soc_instance_init,
> > +.class_init = sifive_u_soc_class_init,
> > };
> >
> > -static void riscv_sifive_u_soc_register_types(void)
> > +static void sifive_u_soc_register_types(void)
> > {
> > -type_register_static(&riscv_sifive_u_soc_type_info);
> > +type_register_static(&sifive_u_soc_type_info);
> > }
> >
> > -type_init(riscv_sifive_u_soc_register_types)
> > +type_init(sifive_u_soc_register_types)
> >
>
> Reviewed-by: Philippe Mathieu-Daudé
>
>
Re: [PATCH v3 2/8] hw/watchdog: Implement full i.MX watchdog support
On 5/21/20 2:04 PM, Peter Maydell wrote:
> On Sun, 17 May 2020 at 17:21, Guenter Roeck wrote:
>>
>> Implement full support for the watchdog in i.MX systems.
>> Pretimeout support is optional because the watchdog hardware
>> on i.MX31 does not support pretimeouts.
>>
>> Signed-off-by: Guenter Roeck
>> ---
>
>> +static Property imx2_wdt_properties[] = {
>> +DEFINE_PROP_BOOL("pretimeout-support", IMX2WdtState, pretimeout_support,
>> + false),
>> +};
>
> This Property array is missing the DEFINE_PROP_END_OF_LIST()
> terminator entry, which makes QEMU crash on startup on
> various host architectures but not x86-64, presumably by
> random luck meaning there's some zeros after it there.
>
> I'm going to fix up the version of this commit in my tree.
>
Oops. Sorry, and thanks for the fix!
Guenter
[PATCH v4 6/6] migration/block-dirty-bitmap: forbid migration by generated node-name
It actually never worked with libvirt, as auto-generated names are
different on source and destination.
It's unsafe and useless to migrate by auto-generated node-names, so
let's forbid it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Andrey Shinkevich
Reviewed-by: Eric Blake
---
migration/block-dirty-bitmap.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 65f2948f07..71528581e4 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -285,6 +285,13 @@ static int add_bitmaps_to_list(BlockDriverState *bs, const
char *bs_name)
return -1;
}
+if (bs_name[0] == '#') {
+error_report("Bitmap '%s' in a node with auto-generated "
+ "name '%s' can't be migrated",
+ bdrv_dirty_bitmap_name(bitmap), bs_name);
+return -1;
+}
+
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
if (!bdrv_dirty_bitmap_name(bitmap)) {
continue;
--
2.21.0
[PATCH v4 4/6] iotests: 194: test also migration of dirty bitmap
Test that dirty bitmap migration works when we deal with mirror.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
tests/qemu-iotests/194 | 14 ++
tests/qemu-iotests/194.out | 6 ++
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index 8b1f720af4..3fad7c6c1a 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -42,6 +42,8 @@ with iotests.FilePath('source.img') as source_img_path, \
.add_incoming('unix:{0}'.format(migration_sock_path))
.launch())
+source_vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0')
+
iotests.log('Launching NBD server on destination...')
iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data':
{'path': nbd_sock_path}}))
iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
@@ -61,12 +63,14 @@ with iotests.FilePath('source.img') as source_img_path, \
filters=[iotests.filter_qmp_event])
iotests.log('Starting migration...')
-source_vm.qmp('migrate-set-capabilities',
- capabilities=[{'capability': 'events', 'state': True}])
-dest_vm.qmp('migrate-set-capabilities',
-capabilities=[{'capability': 'events', 'state': True}])
+capabilities = [{'capability': 'events', 'state': True},
+{'capability': 'dirty-bitmaps', 'state': True}]
+source_vm.qmp('migrate-set-capabilities', capabilities=capabilities)
+dest_vm.qmp('migrate-set-capabilities', capabilities=capabilities)
iotests.log(source_vm.qmp('migrate',
uri='unix:{0}'.format(migration_sock_path)))
+source_vm.qmp_log('migrate-start-postcopy')
+
while True:
event1 = source_vm.event_wait('MIGRATION')
iotests.log(event1, filters=[iotests.filter_qmp_event])
@@ -82,3 +86,5 @@ with iotests.FilePath('source.img') as source_img_path, \
iotests.log('Stopping the NBD server on destination...')
iotests.log(dest_vm.qmp('nbd-server-stop'))
break
+
+iotests.log(source_vm.qmp('query-block')['return'][0]['dirty-bitmaps'])
diff --git a/tests/qemu-iotests/194.out b/tests/qemu-iotests/194.out
index 71857853fb..dd60dcc14f 100644
--- a/tests/qemu-iotests/194.out
+++ b/tests/qemu-iotests/194.out
@@ -1,4 +1,6 @@
Launching VMs...
+{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node":
"drive0"}}
+{"return": {}}
Launching NBD server on destination...
{"return": {}}
{"return": {}}
@@ -8,11 +10,15 @@ Waiting for `drive-mirror` to complete...
{"data": {"device": "mirror-job0", "len": 1073741824, "offset": 1073741824,
"speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_READY", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
Starting migration...
{"return": {}}
+{"execute": "migrate-start-postcopy", "arguments": {}}
+{"return": {}}
{"data": {"status": "setup"}, "event": "MIGRATION", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
{"data": {"status": "active"}, "event": "MIGRATION", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"status": "postcopy-active"}, "event": "MIGRATION", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
{"data": {"status": "completed"}, "event": "MIGRATION", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
Gracefully ending the `drive-mirror` job on source...
{"return": {}}
{"data": {"device": "mirror-job0", "len": 1073741824, "offset": 1073741824,
"speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_COMPLETED", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
Stopping the NBD server on destination...
{"return": {}}
+[{"busy": false, "count": 0, "granularity": 65536, "name": "bitmap0",
"persistent": false, "recording": true, "status": "active"}]
--
2.21.0
Re: [PATCH 3/4] hw/riscv: Allow creating multiple instances of PLIC
On Fri, May 15, 2020 at 11:39 PM Anup Patel wrote:
>
> We extend PLIC emulation to allow multiple instances of PLIC in
> a QEMU RISC-V machine. To achieve this, we remove first HART id
> zero assumption from PLIC emulation.
>
> Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/riscv/sifive_e.c| 2 +-
> hw/riscv/sifive_plic.c | 24 +---
> hw/riscv/sifive_u.c| 2 +-
> hw/riscv/virt.c| 2 +-
> include/hw/riscv/sifive_plic.h | 12 +++-
> 5 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 1c3b37d0ba..bd122e71ae 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -152,7 +152,7 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev,
> Error **errp)
>
> /* MMIO */
> s->plic = sifive_plic_create(memmap[SIFIVE_E_PLIC].base,
> -(char *)SIFIVE_E_PLIC_HART_CONFIG,
> +(char *)SIFIVE_E_PLIC_HART_CONFIG, 0,
> SIFIVE_E_PLIC_NUM_SOURCES,
> SIFIVE_E_PLIC_NUM_PRIORITIES,
> SIFIVE_E_PLIC_PRIORITY_BASE,
> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
> index c1e04cbb98..f88bb48053 100644
> --- a/hw/riscv/sifive_plic.c
> +++ b/hw/riscv/sifive_plic.c
> @@ -352,6 +352,7 @@ static const MemoryRegionOps sifive_plic_ops = {
>
> static Property sifive_plic_properties[] = {
> DEFINE_PROP_STRING("hart-config", SiFivePLICState, hart_config),
> +DEFINE_PROP_UINT32("hartid-base", SiFivePLICState, hartid_base, 0),
> DEFINE_PROP_UINT32("num-sources", SiFivePLICState, num_sources, 0),
> DEFINE_PROP_UINT32("num-priorities", SiFivePLICState, num_priorities, 0),
> DEFINE_PROP_UINT32("priority-base", SiFivePLICState, priority_base, 0),
> @@ -400,10 +401,12 @@ static void parse_hart_config(SiFivePLICState *plic)
> }
> hartid++;
>
> -/* store hart/mode combinations */
> plic->num_addrs = addrid;
> +plic->num_harts = hartid;
> +
> +/* store hart/mode combinations */
> plic->addr_config = g_new(PLICAddr, plic->num_addrs);
> -addrid = 0, hartid = 0;
> +addrid = 0, hartid = plic->hartid_base;
> p = plic->hart_config;
> while ((c = *p++)) {
> if (c == ',') {
> @@ -429,8 +432,6 @@ static void sifive_plic_irq_request(void *opaque, int
> irq, int level)
>
> static void sifive_plic_realize(DeviceState *dev, Error **errp)
> {
> -MachineState *ms = MACHINE(qdev_get_machine());
> -unsigned int smp_cpus = ms->smp.cpus;
> SiFivePLICState *plic = SIFIVE_PLIC(dev);
> int i;
>
> @@ -451,8 +452,8 @@ static void sifive_plic_realize(DeviceState *dev, Error
> **errp)
> * lost a interrupt in the case a PLIC is attached. The SEIP bit must be
> * hardware controlled when a PLIC is attached.
> */
> -for (i = 0; i < smp_cpus; i++) {
> -RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(i));
> +for (i = 0; i < plic->num_harts; i++) {
> +RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(plic->hartid_base + i));
> if (riscv_cpu_claim_interrupts(cpu, MIP_SEIP) < 0) {
> error_report("SEIP already claimed");
> exit(1);
> @@ -488,16 +489,17 @@ type_init(sifive_plic_register_types)
> * Create PLIC device.
> */
> DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> -uint32_t num_sources, uint32_t num_priorities,
> -uint32_t priority_base, uint32_t pending_base,
> -uint32_t enable_base, uint32_t enable_stride,
> -uint32_t context_base, uint32_t context_stride,
> -uint32_t aperture_size)
> +uint32_t hartid_base, uint32_t num_sources,
> +uint32_t num_priorities, uint32_t priority_base,
> +uint32_t pending_base, uint32_t enable_base,
> +uint32_t enable_stride, uint32_t context_base,
> +uint32_t context_stride, uint32_t aperture_size)
> {
> DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_PLIC);
> assert(enable_stride == (enable_stride & -enable_stride));
> assert(context_stride == (context_stride & -context_stride));
> qdev_prop_set_string(dev, "hart-config", hart_config);
> +qdev_prop_set_uint32(dev, "hartid-base", hartid_base);
> qdev_prop_set_uint32(dev, "num-sources", num_sources);
> qdev_prop_set_uint32(dev, "num-priorities", num_priorities);
> qdev_prop_set_uint32(dev, "priority-base", priority_base);
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 22997fbf13..69dbd7980b 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -585,7 +585,7 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev,
> Error **errp)
>
> /* MMIO */
> s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
> -plic_hart_config,
> +plic_hart_config, 0,
> SIFIVE_U_PLIC_NUM_SOURCES,
> SIFIVE_U_PLIC_NUM_PRIORITIES,
> SIFIVE_U_PLIC_PRIORITY_BASE,
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index dcb8a8
[PATCH v4 3/6] migration/block-dirty-bitmap: fix bitmaps pre-blockdev migration during mirror job
Important thing for bitmap migration is to select destination block
node to obtain the migrated bitmap.
Prepatch, on source we use bdrv_get_device_or_node_name() to identify
the node, and on target we do bdrv_lookup_bs.
bdrv_get_device_or_node_name() returns blk name only for direct
children of blk. So, bitmaps of direct children of blks are migrated by
blk name and others - by node name.
Old libvirt is unprepared to bitmap migration by node-name,
node-names are mostly auto-generated. So actually only migration by blk
name works for it.
Newer libvirt will use new interface (which will be added soon) to
specify node-mapping for bitmaps migration explicitly. Still, let's
improve the current behavior a bit.
Now, consider classic libvirt migrations assisted by mirror block job:
mirror block job inserts filter, so our source is not a direct child of
blk, and bitmaps are migrated by node-names. And this just doesn't work
with auto-generated node names.
Let's fix it by using blk-name even if some implicit filters are
inserted.
Note2: we, of course, can't skip filters and use blk name to migrate
bitmaps in filtered node by blk name for this blk if these filters have
named bitmaps which should be migrated.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652424
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
migration/block-dirty-bitmap.c | 45 +-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 7e93718086..04a5525fd1 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -319,14 +319,54 @@ static int init_dirty_bitmap_migration(void)
{
BlockDriverState *bs;
DirtyBitmapMigBitmapState *dbms;
+GHashTable *handled_by_blk = g_hash_table_new(NULL, NULL);
+BlockBackend *blk;
dirty_bitmap_mig_state.bulk_completed = false;
dirty_bitmap_mig_state.prev_bs = NULL;
dirty_bitmap_mig_state.prev_bitmap = NULL;
dirty_bitmap_mig_state.no_bitmaps = false;
+/*
+ * Use blockdevice name for direct (or filtered) children of named block
+ * backends.
+ */
+for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+const char *name = blk_name(blk);
+
+if (!name || strcmp(name, "") == 0) {
+continue;
+}
+
+bs = blk_bs(blk);
+
+/* Skip filters without bitmaos */
+while (bs && bs->drv && bs->drv->is_filter &&
+ !bdrv_has_named_bitmaps(bs))
+{
+if (bs->backing) {
+bs = bs->backing->bs;
+} else if (bs->file) {
+bs = bs->file->bs;
+} else {
+bs = NULL;
+}
+}
+
+if (bs && bs->drv && !bs->drv->is_filter) {
+if (add_bitmaps_to_list(bs, name)) {
+goto fail;
+}
+g_hash_table_add(handled_by_blk, bs);
+}
+}
+
for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
-if (add_bitmaps_to_list(bs, bdrv_get_device_or_node_name(bs))) {
+if (g_hash_table_contains(handled_by_blk, bs)) {
+continue;
+}
+
+if (add_bitmaps_to_list(bs, bdrv_get_node_name(bs))) {
goto fail;
}
}
@@ -340,9 +380,12 @@ static int init_dirty_bitmap_migration(void)
dirty_bitmap_mig_state.no_bitmaps = true;
}
+g_hash_table_destroy(handled_by_blk);
+
return 0;
fail:
+g_hash_table_destroy(handled_by_blk);
dirty_bitmap_mig_cleanup();
return -1;
--
2.21.0
[PATCH v4 5/6] migration/block-dirty-bitmap: add_bitmaps_to_list: check disk name once
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Andrey Shinkevich
Reviewed-by: Eric Blake
---
migration/block-dirty-bitmap.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 04a5525fd1..65f2948f07 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -274,17 +274,22 @@ static int add_bitmaps_to_list(BlockDriverState *bs,
const char *bs_name)
DirtyBitmapMigBitmapState *dbms;
Error *local_err = NULL;
+bitmap = bdrv_dirty_bitmap_first(bs);
+if (!bitmap) {
+return 0;
+}
+
+if (!bs_name || strcmp(bs_name, "") == 0) {
+error_report("Bitmap '%s' in unnamed node can't be migrated",
+ bdrv_dirty_bitmap_name(bitmap));
+return -1;
+}
+
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
if (!bdrv_dirty_bitmap_name(bitmap)) {
continue;
}
-if (!bs_name || strcmp(bs_name, "") == 0) {
-error_report("Found bitmap '%s' in unnamed node %p. It can't "
- "be migrated", bdrv_dirty_bitmap_name(bitmap), bs);
-return -1;
-}
-
if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
error_report_err(local_err);
return -1;
--
2.21.0
[PATCH v4 0/6] fix migration with bitmaps and mirror
v4: (Max's patch marking filters as filters already in master) 03: careful select child of filter, avoid crash 04: add Eric's r-b 05-06: tweak error message, keep Andrey's r-b, add Eric's r-b Vladimir Sementsov-Ogievskiy (6): migration/block-dirty-bitmap: refactor init_dirty_bitmap_migration block/dirty-bitmap: add bdrv_has_named_bitmaps helper migration/block-dirty-bitmap: fix bitmaps pre-blockdev migration during mirror job iotests: 194: test also migration of dirty bitmap migration/block-dirty-bitmap: add_bitmaps_to_list: check disk name once migration/block-dirty-bitmap: forbid migration by generated node-name include/block/dirty-bitmap.h | 1 + block/dirty-bitmap.c | 13 migration/block-dirty-bitmap.c | 130 - tests/qemu-iotests/194 | 14 +++- tests/qemu-iotests/194.out | 6 ++ 5 files changed, 127 insertions(+), 37 deletions(-) -- 2.21.0
[PATCH v4 2/6] block/dirty-bitmap: add bdrv_has_named_bitmaps helper
To be used for bitmap migration in further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Andrey Shinkevich
Reviewed-by: Eric Blake
---
include/block/dirty-bitmap.h | 1 +
block/dirty-bitmap.c | 13 +
2 files changed, 14 insertions(+)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 5a8d52e4de..36e8da4fc2 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -95,6 +95,7 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
+bool bdrv_has_named_bitmaps(BlockDriverState *bs);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index f9bfc77985..c01319b188 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -818,6 +818,19 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs)
return false;
}
+bool bdrv_has_named_bitmaps(BlockDriverState *bs)
+{
+BdrvDirtyBitmap *bm;
+
+QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+if (bdrv_dirty_bitmap_name(bm)) {
+return true;
+}
+}
+
+return false;
+}
+
/* Called with BQL taken. */
void bdrv_dirty_bitmap_set_persistence(BdrvDirtyBitmap *bitmap, bool
persistent)
{
--
2.21.0
[PATCH v4 1/6] migration/block-dirty-bitmap: refactor init_dirty_bitmap_migration
Split out handling one bs, it is needed for the following commit, which
will handle BlockBackends separately.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
migration/block-dirty-bitmap.c | 89 +++---
1 file changed, 49 insertions(+), 40 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 7eafface61..7e93718086 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -268,57 +268,66 @@ static void dirty_bitmap_mig_cleanup(void)
}
/* Called with iothread lock taken. */
-static int init_dirty_bitmap_migration(void)
+static int add_bitmaps_to_list(BlockDriverState *bs, const char *bs_name)
{
-BlockDriverState *bs;
BdrvDirtyBitmap *bitmap;
DirtyBitmapMigBitmapState *dbms;
Error *local_err = NULL;
-dirty_bitmap_mig_state.bulk_completed = false;
-dirty_bitmap_mig_state.prev_bs = NULL;
-dirty_bitmap_mig_state.prev_bitmap = NULL;
-dirty_bitmap_mig_state.no_bitmaps = false;
+FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
+if (!bdrv_dirty_bitmap_name(bitmap)) {
+continue;
+}
-for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
-const char *name = bdrv_get_device_or_node_name(bs);
+if (!bs_name || strcmp(bs_name, "") == 0) {
+error_report("Found bitmap '%s' in unnamed node %p. It can't "
+ "be migrated", bdrv_dirty_bitmap_name(bitmap), bs);
+return -1;
+}
-FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
-if (!bdrv_dirty_bitmap_name(bitmap)) {
-continue;
-}
+if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
+error_report_err(local_err);
+return -1;
+}
-if (!name || strcmp(name, "") == 0) {
-error_report("Found bitmap '%s' in unnamed node %p. It can't "
- "be migrated", bdrv_dirty_bitmap_name(bitmap),
bs);
-goto fail;
-}
+bdrv_ref(bs);
+bdrv_dirty_bitmap_set_busy(bitmap, true);
+
+dbms = g_new0(DirtyBitmapMigBitmapState, 1);
+dbms->bs = bs;
+dbms->node_name = bs_name;
+dbms->bitmap = bitmap;
+dbms->total_sectors = bdrv_nb_sectors(bs);
+dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
+bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+if (bdrv_dirty_bitmap_enabled(bitmap)) {
+dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
+}
+if (bdrv_dirty_bitmap_get_persistence(bitmap)) {
+dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+}
-if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT,
-&local_err)) {
-error_report_err(local_err);
-goto fail;
-}
+QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
+ dbms, entry);
+}
-bdrv_ref(bs);
-bdrv_dirty_bitmap_set_busy(bitmap, true);
-
-dbms = g_new0(DirtyBitmapMigBitmapState, 1);
-dbms->bs = bs;
-dbms->node_name = name;
-dbms->bitmap = bitmap;
-dbms->total_sectors = bdrv_nb_sectors(bs);
-dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
-bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
-if (bdrv_dirty_bitmap_enabled(bitmap)) {
-dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
-}
-if (bdrv_dirty_bitmap_get_persistence(bitmap)) {
-dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
-}
+return 0;
+}
+
+/* Called with iothread lock taken. */
+static int init_dirty_bitmap_migration(void)
+{
+BlockDriverState *bs;
+DirtyBitmapMigBitmapState *dbms;
+
+dirty_bitmap_mig_state.bulk_completed = false;
+dirty_bitmap_mig_state.prev_bs = NULL;
+dirty_bitmap_mig_state.prev_bitmap = NULL;
+dirty_bitmap_mig_state.no_bitmaps = false;
-QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
- dbms, entry);
+for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
+if (add_bitmaps_to_list(bs, bdrv_get_device_or_node_name(bs))) {
+goto fail;
}
}
--
2.21.0
Re: [PATCH v3 2/2] net: Drop the NetLegacy structure, always use Netdev instead
On 5/18/20 1:01 PM, Thomas Huth wrote: Now that the "name" parameter is gone, there is hardly any difference between NetLegacy and Netdev anymore, so we can drop NetLegacy and always use Netdev to simplify the code quite a bit. The only two differences that were really left between Netdev and NetLegacy: 1) NetLegacy does not allow a "hubport" type. We can continue to block this with a simple check in net_client_init1() for this type. 2) The "id" parameter was optional in NetLegacy (and an internal id was chosen via assign_name() during initialization), but it is mandatory for Netdev. To avoid that the visitor code bails out here, we have to add an internal id to the QemuOpts already earlier now. Signed-off-by: Thomas Huth --- Note: I did not rename the "is_netdev" parameter of the function (as Eric suggested) - you really have to think of "-netdev" vs. "-net" here and not about "Netdev" vs. "NetLegacy". But if this "is_netdev" thing still confuses us in the future, we can still rename it with an additional follow-up patch later instead. Works for me. It still might be nice mentioning "-netdev" vs. "-net" in the commit message (and the fact that "-net" was what was previously using the legacy type). But with the explanations you've given, the code looks correct, and a commit message tweak does not change: Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 5/7] iotests: 194: test also migration of dirty bitmap
On 5/15/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote: Test that dirty bitmap migration works when we deal with mirror. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/194 | 14 ++ tests/qemu-iotests/194.out | 6 ++ 2 files changed, 16 insertions(+), 4 deletions(-) Definitely needed ;) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 6/7] migration/block-dirty-bitmap: add_bitmaps_to_list: check disk name once
On 5/15/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Andrey Shinkevich
---
migration/block-dirty-bitmap.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 5d3a7d2b07..e0e081ce60 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -274,17 +274,22 @@ static int add_bitmaps_to_list(BlockDriverState *bs,
const char *bs_name)
DirtyBitmapMigBitmapState *dbms;
Error *local_err = NULL;
+bitmap = bdrv_dirty_bitmap_first(bs);
+if (!bitmap) {
+return 0;
+}
+
+if (!bs_name || strcmp(bs_name, "") == 0) {
+error_report("Found bitmap '%s' in unnamed node %p. It can't "
+ "be migrated", bdrv_dirty_bitmap_name(bitmap), bs);
The %p is unusual; it does not help the end user, but only a developer
with gdb access.
Maybe we could compress to:
"Bitmap '%s' in unnamed node can't be migrated"
+return -1;
+}
+
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
if (!bdrv_dirty_bitmap_name(bitmap)) {
continue;
}
-if (!bs_name || strcmp(bs_name, "") == 0) {
-error_report("Found bitmap '%s' in unnamed node %p. It can't "
- "be migrated", bdrv_dirty_bitmap_name(bitmap), bs);
-return -1;
-}
-
But since this is just code motion (hoisting an check outside of a loop,
for fewer executions of something that does not change within the loop),
it doesn't matter whether this patch goes in as-is or if you also tweak
the error message.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PULL v2 00/29] target-arm queue
v2: added Property array terminator (which caused crashes on various non-x86 host architectures). The following changes since commit ae3aa5da96f4ccf0c2a28851449d92db9fcfad71: Merge remote-tracking branch 'remotes/berrange/tags/socket-next-pull-request' into staging (2020-05-21 16:47:28 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200521-1 for you to fetch changes up to fafe7229272f39500c14845bc7ea60a8504a5a20: linux-user/arm/signal.c: Drop TARGET_CONFIG_CPU_32 (2020-05-21 22:05:27 +0100) target-arm queue: * tests/acceptance: Add a test for the canon-a1100 machine * docs/system: Document some of the Arm development boards * linux-user: make BKPT insn cause SIGTRAP, not be a syscall * target/arm: Remove unused GEN_NEON_INTEGER_OP macro * fsl-imx25, fsl-imx31, fsl-imx6, fsl-imx6ul, fsl-imx7: implement watchdog * hw/arm: Use qemu_log_mask() instead of hw_error() in various places * ARM: PL061: Introduce N_GPIOS * target/arm: Improve clear_vec_high() usage * target/arm: Allow user-mode code to write CPSR.E via MSR * linux-user/arm: Reset CPSR_E when entering a signal handler * linux-user/arm/signal.c: Drop TARGET_CONFIG_CPU_32 Amanieu d'Antras (1): linux-user/arm: Reset CPSR_E when entering a signal handler Geert Uytterhoeven (1): ARM: PL061: Introduce N_GPIOS Guenter Roeck (8): hw: Move i.MX watchdog driver to hw/watchdog hw/watchdog: Implement full i.MX watchdog support hw/arm/fsl-imx25: Wire up watchdog hw/arm/fsl-imx31: Wire up watchdog hw/arm/fsl-imx6: Connect watchdog interrupts hw/arm/fsl-imx6ul: Connect watchdog interrupts hw/arm/fsl-imx7: Instantiate various unimplemented devices hw/arm/fsl-imx7: Connect watchdog interrupts Peter Maydell (12): docs/system: Add 'Arm' to the Integrator/CP document title docs/system: Sort Arm board index into alphabetical order docs/system: Document Arm Versatile Express boards docs/system: Document the various MPS2 models docs/system: Document Musca boards linux-user/arm: BKPT should cause SIGTRAP, not be a syscall linux-user/arm: Remove bogus SVC 0xf0002 handling linux-user/arm: Handle invalid arm-specific syscalls correctly linux-user/arm: Fix identification of syscall numbers target/arm: Remove unused GEN_NEON_INTEGER_OP macro target/arm: Allow user-mode code to write CPSR.E via MSR linux-user/arm/signal.c: Drop TARGET_CONFIG_CPU_32 Philippe Mathieu-Daudé (4): hw/arm/integratorcp: Replace hw_error() by qemu_log_mask() hw/arm/pxa2xx: Replace hw_error() by qemu_log_mask() hw/char/xilinx_uartlite: Replace hw_error() by qemu_log_mask() hw/timer/exynos4210_mct: Replace hw_error() by qemu_log_mask() Richard Henderson (2): target/arm: Use tcg_gen_gvec_mov for clear_vec_high target/arm: Use clear_vec_high more effectively Thomas Huth (1): tests/acceptance: Add a test for the canon-a1100 machine docs/system/arm/integratorcp.rst | 4 +- docs/system/arm/mps2.rst | 29 +++ docs/system/arm/musca.rst | 31 +++ docs/system/arm/vexpress.rst | 60 ++ docs/system/target-arm.rst | 20 +- include/hw/arm/fsl-imx25.h | 5 + include/hw/arm/fsl-imx31.h | 4 + include/hw/arm/fsl-imx6.h | 2 +- include/hw/arm/fsl-imx6ul.h| 2 +- include/hw/arm/fsl-imx7.h | 23 ++- include/hw/misc/imx2_wdt.h | 33 include/hw/watchdog/wdt_imx2.h | 90 + target/arm/cpu.h | 2 +- hw/arm/fsl-imx25.c | 10 + hw/arm/fsl-imx31.c | 6 + hw/arm/fsl-imx6.c | 9 + hw/arm/fsl-imx6ul.c| 10 + hw/arm/fsl-imx7.c | 35 hw/arm/integratorcp.c | 23 ++- hw/arm/pxa2xx_gpio.c | 7 +- hw/char/xilinx_uartlite.c | 5 +- hw/display/pxa2xx_lcd.c| 8 +- hw/dma/pxa2xx_dma.c| 14 +- hw/gpio/pl061.c| 12 +- hw/misc/imx2_wdt.c | 90 - hw/timer/exynos4210_mct.c | 12 +- hw/watchdog/wdt_imx2.c | 304 + linux-user/arm/cpu_loop.c | 145 -- linux-user/arm/signal.c| 15 +- target/arm/translate-a64.c | 63 +++--- target/arm/translate.c | 23 --- MAINTAINERS| 6 + hw/arm/Kconfig
Re: [PATCH v3 7/7] migration/block-dirty-bitmap: forbid migration by generated node-name
On 5/15/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote:
It actually never worked with libvirt, as auto-generated names are
different on source and destination.
It's unsafe and useless to migrate by auto-generated node-names, so
let's forbid it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Andrey Shinkevich
---
migration/block-dirty-bitmap.c | 7 +++
1 file changed, 7 insertions(+)
Reviewed-by: Eric Blake
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index e0e081ce60..f5744c35e6 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -285,6 +285,13 @@ static int add_bitmaps_to_list(BlockDriverState *bs, const
char *bs_name)
return -1;
}
+if (bs_name[0] == '#') {
+error_report("Found bitmap '%s' in a node with auto-generated "
+ "name: %s. It can't be migrated",
Maybe compress to:
"Bitmap '%s' in auto-generated node '%s' can't be migrated"
+ bdrv_dirty_bitmap_name(bitmap), bs_name);
+return -1;
+}
+
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
if (!bdrv_dirty_bitmap_name(bitmap)) {
continue;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 2/8] hw/watchdog: Implement full i.MX watchdog support
On Sun, 17 May 2020 at 17:21, Guenter Roeck wrote:
>
> Implement full support for the watchdog in i.MX systems.
> Pretimeout support is optional because the watchdog hardware
> on i.MX31 does not support pretimeouts.
>
> Signed-off-by: Guenter Roeck
> ---
> +static Property imx2_wdt_properties[] = {
> +DEFINE_PROP_BOOL("pretimeout-support", IMX2WdtState, pretimeout_support,
> + false),
> +};
This Property array is missing the DEFINE_PROP_END_OF_LIST()
terminator entry, which makes QEMU crash on startup on
various host architectures but not x86-64, presumably by
random luck meaning there's some zeros after it there.
I'm going to fix up the version of this commit in my tree.
thanks
-- PMM
Re: [PATCH v3 4/7] migration/block-dirty-bitmap: fix bitmaps pre-blockdev migration during mirror job
On 5/19/20 5:51 AM, Vladimir Sementsov-Ogievskiy wrote:
18.05.2020 23:36, Eric Blake wrote:
On 5/15/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote:
Important thing for bitmap migration is to select destination block
node to obtain the migrated bitmap.
+ /* Skip filters without bitmaos */
+ while (bs && bs->drv && bs->drv->is_filter &&
+ !bdrv_has_named_bitmaps(bs))
+ {
+ bs = bs->backing->bs ?: bs->file->bs;
Is this correct, or should it be:
bs = bs->backing ? bs->backing->bs : bs->file->bs;
Hmm, yes, otherwise it should crash on file-based filter :)
Otherwise looks reasonable, but I'm hesitant to include it in today's
bitmap pull request in order to give it more review/testing time. It
should be ready for a pull request next week, though.
Can you post a v4, to make it easier for me to build?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: cluster_size got from backup_calculate_cluster_size()
21.05.2020 21:19, John Snow wrote:
On 5/21/20 5:56 AM, Derek Su wrote:
Hi,
The cluster_size got from backup_calculate_cluster_size(),
MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size), is 64K regardless
of the target image's cluster size.
Not regardless -- but it is using 64K as a minimum.
For example:
If the cluster size of source and target qcow2 images are both 16K, the
64K from backup_calculate_cluster_size() results in unwanted copies of
clusters.
The behavior slows down the backup (block-copy) process when the
source image receives lots of rand writes.
Are we talking about incremental, full, or top?
Is the following calculation reasonable for the above issue?
```
static int64_t backup_calculate_cluster_size(BlockDriverState *target,
Error **errp)
{
...
ret = bdrv_get_info(target, &bdi);
...
return (bdi.cluster_size == 0 ?
BACKUP_CLUSTER_SIZE_DEFAULT : cluster_size);
}
```
Thanks.
Regards,
Derek
Might be reasonable. When I made this the "default", it actually used to
just be "hardcoded." We could continue in this direction and go all the
way and turn it into a tune-able.
I didn't think to make it lower than 64K because I was thinking about
the linear, full backup case where cluster sizes that were too small
might slow down the loop with too much metadata.
Yes, currently backup-loop is copying cluster-by-cluster, so if we allow
clusters less than 64k, it may become slower (at least for full-backup). Still,
my work about backup-performance is close to its end, and I hope, in 5.1 will
be merged. One of effects is that backup copying loop may copy more than a
cluster at once, so this problem will gone.
Keeping this in mind, I think we can allow smaller clusters now, as anyway,
small clusters is a rare special case.
(And the default qcow2 is 64K, so it seemed like a good choice at the time.)
We could create a default-cluster-size tunable, perhaps. It's at 64K
now, but perhaps you can override it down to 16 or lower. We could
possibly treat a value of 0 as "no minimum; use the smallest common size."
This is a separate issue entirely, but we may also wish to begin
offering a cluster-size override directly. In the case that we know this
value is unsafe, we reject the command. In the case that it's ambiguous
due to lack of info, we can defer to the user's judgment. This would
allow us to force the backup to run in cases where we presently abort
out of fear.
CCing Vladimir who has been working on the backup loop extensively.
--js
--
Best regards,
Vladimir
Re: [PATCH QEMU v23 09/18] vfio: Add save state functions to SaveVMHandlers
On Wed, May 20, 2020 at 11:54:39PM +0530, Kirti Wankhede wrote:
> In _SAVING|_RUNNING device state or pre-copy phase:
> - read pending_bytes. If pending_bytes > 0, go through below steps.
> - read data_offset - indicates kernel driver to write data to staging
> buffer.
> - read data_size - amount of data in bytes written by vendor driver in
> migration region.
> - read data_size bytes of data from data_offset in the migration region.
> - Write data packet to file stream as below:
> {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data,
> VFIO_MIG_FLAG_END_OF_STATE }
Hi, Kirti,
(Sorry if this question is silly, since I didn't follow up with all these
threads...)
Can I understand this commit as it only tracks dirty device BAR memory but not
guest system memory? How did you track device writes to system memory when
without the vIOMMU?
Thanks,
--
Peter Xu
[PATCH 2/6] hw/misc: Add NPCM7xx System Global Control Registers device model
Implement a device model for the System Global Control Registers in the
NPCM730 and NPCM750 BMC SoCs.
This is primarily used to enable SMP boot (the boot ROM spins reading
the SCRPAD register); other registers are best effort for now.
The reset values of the MDLR and PWRON registers are determined by the
SoC variant (730 vs 750) and board straps respectively.
Reviewed-by: Tyrone Ting
Signed-off-by: Havard Skinnemoen
---
MAINTAINERS | 8 ++
hw/misc/Makefile.objs | 1 +
hw/misc/npcm7xx_gcr.c | 160 ++
hw/misc/trace-events | 4 +
include/hw/misc/npcm7xx_gcr.h | 74
5 files changed, 247 insertions(+)
create mode 100644 hw/misc/npcm7xx_gcr.c
create mode 100644 include/hw/misc/npcm7xx_gcr.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 87a412c229..5b150184ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -715,6 +715,14 @@ S: Odd Fixes
F: hw/arm/musicpal.c
F: docs/system/arm/musicpal.rst
+Nuvoton NPCM7xx
+M: Havard Skinnemoen
+M: Tyrone Ting
+L: qemu-...@nongnu.org
+S: Supported
+F: hw/misc/npcm7xx*
+F: include/hw/misc/npcm7xx*
+
nSeries
M: Andrzej Zaborowski
M: Peter Maydell
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 68aae2eabb..4d83fa337b 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -51,6 +51,7 @@ common-obj-$(CONFIG_IMX) += imx_rngc.o
common-obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
common-obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
common-obj-$(CONFIG_MAINSTONE) += mst_fpga.o
+common-obj-$(CONFIG_NPCM7XX) += npcm7xx_gcr.o
common-obj-$(CONFIG_OMAP) += omap_clk.o
common-obj-$(CONFIG_OMAP) += omap_gpmc.o
common-obj-$(CONFIG_OMAP) += omap_l4.o
diff --git a/hw/misc/npcm7xx_gcr.c b/hw/misc/npcm7xx_gcr.c
new file mode 100644
index 00..cd8690a968
--- /dev/null
+++ b/hw/misc/npcm7xx_gcr.c
@@ -0,0 +1,160 @@
+/*
+ * Nuvoton NPCM7xx System Global Control Registers.
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/misc/npcm7xx_gcr.h"
+#include "hw/qdev-properties.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+
+#include "trace.h"
+
+static const uint32_t cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
+[NPCM7XX_GCR_PDID] = 0x04A92750, /* Poleg A1 */
+[NPCM7XX_GCR_MISCPE]= 0x,
+[NPCM7XX_GCR_SPSWC] = 0x0003,
+[NPCM7XX_GCR_INTCR] = 0x035E,
+[NPCM7XX_GCR_HIFCR] = 0x004E,
+[NPCM7XX_GCR_RESSR] = 0x8000,
+[NPCM7XX_GCR_DSCNT] = 0x00c0,
+[NPCM7XX_GCR_DAVCLVLR] = 0x5A00F3CF,
+[NPCM7XX_GCR_INTCR3]= 0x1002,
+[NPCM7XX_GCR_SCRPAD]= 0x0008,
+[NPCM7XX_GCR_USB1PHYCTL]= 0x034730E4,
+[NPCM7XX_GCR_USB2PHYCTL]= 0x034730E4,
+};
+
+static uint64_t npcm7xx_gcr_read(void *opaque, hwaddr offset, unsigned size)
+{
+uint32_t reg = offset / sizeof(uint32_t);
+NPCM7xxGCRState *s = opaque;
+
+if (reg >= NPCM7XX_GCR_NR_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: offset 0x%04x out of range\n",
+ __func__, (unsigned int)offset);
+return 0;
+}
+
+trace_npcm7xx_gcr_read(offset, s->regs[reg]);
+
+return s->regs[reg];
+}
+
+static void npcm7xx_gcr_write(void *opaque, hwaddr offset,
+ uint64_t v, unsigned size)
+{
+uint32_t reg = offset / sizeof(uint32_t);
+NPCM7xxGCRState *s = opaque;
+uint32_t value = v;
+
+trace_npcm7xx_gcr_write(offset, value);
+
+if (reg >= NPCM7XX_GCR_NR_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: offset 0x%04x out of range\n",
+ __func__, (unsigned int)offset);
+return;
+}
+
+switch (reg) {
+case NPCM7XX_GCR_PDID:
+case NPCM7XX_GCR_PWRON:
+case NPCM7XX_GCR_INTSR:
+qemu_log_mask(LOG_GUEST_ERROR, "%s: register @ 0x%04x is read-only\n",
+ __func__, (unsigned int)offset);
+return;
+
+case NPCM7XX_GCR_RESSR:
+case NPCM7XX_GCR_CP2BST:
+/* Write 1 to clear */
+value = s->regs[reg] & ~value;
+break;
+
+case NPCM7XX_GCR_RLOCKR1:
+case NPCM7XX_GCR_MDLR:
+/* Write 1 to set */
+value |= s->regs[reg];
+break;
+};
+
+s->regs[reg] = value;
+}
+
+static const struct MemoryRegionOps npcm7xx_gcr_ops = {
+.read = npcm7xx_gcr_read,
+.write = npcm7xx_gcr_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+
[PATCH 6/6] hw/arm: Add two NPCM7xx-based machines
This adds two new machines:
- npcm750-evb: Nuvoton NPCM750 Evaluation Board.
- quanta-gsj: A board with a NPCM730 chip.
They rely on the NPCM7xx SoC device to do the heavy lifting. They are
almost completely identical at the moment, apart from the SoC type,
which currently only changes the reset contents of one register
(GCR.MDLR), but they might grow apart a bit more as more functionality
is added.
Both machines can boot the Linux kernel into /bin/sh.
Reviewed-by: Tyrone Ting
Signed-off-by: Havard Skinnemoen
---
hw/arm/Makefile.objs | 2 +-
hw/arm/npcm7xx_boards.c | 108 +++
include/hw/arm/npcm7xx.h | 19 +++
3 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644 hw/arm/npcm7xx_boards.c
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 13d163a599..c333548ce1 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -41,7 +41,7 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_STM32F405_SOC) += stm32f405_soc.o
obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
-obj-$(CONFIG_NPCM7XX) += npcm7xx.o
+obj-$(CONFIG_NPCM7XX) += npcm7xx.o npcm7xx_boards.o
obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
new file mode 100644
index 00..b89819f6e2
--- /dev/null
+++ b/hw/arm/npcm7xx_boards.c
@@ -0,0 +1,108 @@
+/*
+ * Machine definitions for boards featuring an NPCM7xx SoC.
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/arm/boot.h"
+#include "hw/arm/npcm7xx.h"
+#include "hw/core/cpu.h"
+#include "qapi/error.h"
+#include "qemu/units.h"
+
+static struct arm_boot_info npcm7xx_binfo = {
+.loader_start = NPCM7XX_LOADER_START,
+.smp_loader_start = NPCM7XX_SMP_LOADER_START,
+.smp_bootreg_addr = NPCM7XX_SMP_BOOTREG_ADDR,
+.gic_cpu_if_addr= NPCM7XX_GIC_CPU_IF_ADDR,
+.write_secondary_boot = npcm7xx_write_secondary_boot,
+.board_id = -1,
+};
+
+static void npcm7xx_machine_init(MachineState *machine)
+{
+NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_GET_CLASS(machine);
+NPCM7xxState *soc;
+
+soc = NPCM7XX(object_new_with_props(nmc->soc_type, OBJECT(machine), "soc",
+&error_abort, NULL));
+object_property_set_int(OBJECT(soc), machine->smp.cpus, "num-cpus",
+&error_abort);
+object_property_set_link(OBJECT(soc), OBJECT(machine->ram), "dram",
+ &error_abort);
+object_property_set_bool(OBJECT(soc), true, "realized", &error_abort);
+
+npcm7xx_binfo.ram_size = machine->ram_size;
+npcm7xx_binfo.nb_cpus = machine->smp.cpus;
+
+arm_load_kernel(soc->cpu[0], machine, &npcm7xx_binfo);
+}
+
+static void npcm7xx_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->init= npcm7xx_machine_init;
+mc->max_cpus= NPCM7XX_MAX_NUM_CPUS;
+mc->default_cpus= NPCM7XX_MAX_NUM_CPUS;
+mc->no_floppy = 1;
+mc->no_cdrom= 1;
+mc->no_parallel = 1;
+mc->default_ram_id = "ram";
+}
+
+/*
+ * Schematics:
+ *
https://github.com/Nuvoton-Israel/nuvoton-info/blob/master/npcm7xx-poleg/evaluation-board/board_deliverables/NPCM750x_EB_ver.A1.1_COMPLETE.pdf
+ */
+static void npcm750_evb_machine_class_init(ObjectClass *oc, void *data)
+{
+NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_CLASS(oc);
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc= "Nuvoton NPCM750 Evaluation Board (Cortex A9)";
+nmc->soc_type = TYPE_NPCM750;
+mc->default_ram_size = 512 * MiB;
+};
+
+static void gsj_machine_class_init(ObjectClass *oc, void *data)
+{
+NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_CLASS(oc);
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc= "Quanta GSJ (Cortex A9)";
+nmc->soc_type = TYPE_NPCM730;
+mc->default_ram_size = 512 * MiB;
+};
+
+static const TypeInfo npcm7xx_machine_types[] = {
+{
+.name = TYPE_NPCM7XX_MACHINE,
+.parent = TYPE_MACHINE,
+.instance_size = sizeof(NPCM7xxMachine),
+.class_size = sizeof(NPCM7xxMachineClass),
+.class_init = npcm7xx_machine_class_init,
+.abstract = true,
+}, {
+.name = MACHINE
[PATCH 1/6] npcm7xx: Add config symbol
Add a config symbol for the NPCM7xx BMC SoC family that subsequent patches can use in Makefiles. Reviewed-by: Tyrone Ting Signed-off-by: Havard Skinnemoen --- default-configs/arm-softmmu.mak | 1 + hw/arm/Kconfig | 8 2 files changed, 9 insertions(+) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 8fc09a4a51..9a94ebd0be 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -27,6 +27,7 @@ CONFIG_GUMSTIX=y CONFIG_SPITZ=y CONFIG_TOSA=y CONFIG_Z2=y +CONFIG_NPCM7XX=y CONFIG_COLLIE=y CONFIG_ASPEED_SOC=y CONFIG_NETDUINO2=y diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 5364172537..47d0a3ca43 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -354,6 +354,14 @@ config XLNX_VERSAL select VIRTIO_MMIO select UNIMP +config NPCM7XX +bool +select A9MPCORE +select ARM_GIC +select PL310 # cache controller +select SERIAL +select UNIMP + config FSL_IMX25 bool select IMX -- 2.27.0.rc0.183.gde8f92d652-goog
