On 3/3/2020 2:56 PM, Dr. David Alan Gilbert wrote:
* Jagannathan Raman (jag.ra...@oracle.com) wrote:
In some cases, for example MMIO read, QEMU has to wait for the remote to
complete a command before proceeding. An eventfd based mechanism is
added to synchronize QEMU & remote process.
Signed-off-by: John G Johnson <john.g.john...@oracle.com>
Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com>
Signed-off-by: Elena Ufimtseva <elena.ufimts...@oracle.com>
---
include/io/mpqemu-link.h | 7 +++++++
io/mpqemu-link.c | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
index 2f2dd83..ae04fca 100644
--- a/include/io/mpqemu-link.h
+++ b/include/io/mpqemu-link.h
@@ -135,4 +135,11 @@ void mpqemu_link_set_callback(MPQemuLinkState *s,
mpqemu_link_callback callback);
void mpqemu_start_coms(MPQemuLinkState *s);
+#define GET_REMOTE_WAIT eventfd(0, EFD_CLOEXEC)
+#define PUT_REMOTE_WAIT(wait) close(wait)
+#define PROXY_LINK_WAIT_DONE 1
+
+uint64_t wait_for_remote(int efd);
+void notify_proxy(int fd, uint64_t val);
+
#endif
diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
index bac120b..73b7032 100644
--- a/io/mpqemu-link.c
+++ b/io/mpqemu-link.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include <poll.h>
#include "qemu/module.h"
#include "io/mpqemu-link.h"
@@ -216,6 +217,46 @@ int mpqemu_msg_recv(MPQemuMsg *msg, MPQemuChannel *chan)
return rc;
}
+uint64_t wait_for_remote(int efd)
+{
+ struct pollfd pfd = { .fd = efd, .events = POLLIN };
+ uint64_t val;
+ int ret;
+
+ ret = poll(&pfd, 1, 1000);
+
+ switch (ret) {
+ case 0:
+ qemu_log_mask(LOG_REMOTE_DEBUG, "Error wait_for_remote: Timed out\n");
+ /* TODO: Kick-off error recovery */
+ return ULLONG_MAX;
Shouldn't these be UINT64_MAX?
Sure, we'll change these to UINT64_MAX.
+ case -1:
+ qemu_log_mask(LOG_REMOTE_DEBUG, "Poll error wait_for_remote: %s\n",
+ strerror(errno));
+ return ULLONG_MAX;
+ default:
+ if (read(efd, &val, sizeof(val)) == -1) {
+ qemu_log_mask(LOG_REMOTE_DEBUG, "Error wait_for_remote: %s\n",
+ strerror(errno));
+ return ULLONG_MAX;
+ }
+ }
+
+ val = (val == ULLONG_MAX) ? val : (val - 1);
Can you explain what's going on there??
This wait_for_remote() function serves two purposes. This first one is
to synchronize QEMU and the remote process. Secondly, it allows the
remote process to return a 64-bit value to QEMU. When the remote process
has completed the task, it uses the notify_proxy() function to return to
QEMU along with a return value.
We have implemented this synchronization mechanism using eventfd, as
it's easy to setup. So the remote process could write a non-zero value
to the eventfd to wake QEMU up. However, the drawback of using eventfd
for this purpose is that a return value of zero wouldn't wake QEMU up.
Therefore, we offset the return value by one at the remote process and
correct it in the QEMU end.
--
Jag
+ return val;
+}
+
+void notify_proxy(int efd, uint64_t val)
+{
+ val = (val == ULLONG_MAX) ? val : (val + 1);
+
+ if (write(efd, &val, sizeof(val)) == -1) {
I'd actually check the write/read's are returning sizeof(val) - they
can on a bad day return 0 or send only a few bytes; in theory?
Dave
+ qemu_log_mask(LOG_REMOTE_DEBUG, "Error notify_proxy: %s\n",
+ strerror(errno));
+ }
+}
+
static gboolean mpqemu_link_handler_prepare(GSource *gsrc, gint *timeout)
{
g_assert(timeout);
--
1.8.3.1
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK