[Qemu-devel] [PATCH 0/1] qmp: extend QMP to provide read/write access to physical memory

[Qemu-devel] [PATCH 0/1] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

Summary:
This patch improves Qemu support for virtual machine introspection.

Background:
Virtual machine introspection (VMI) is a technique where one accesses the
memory of a (usually) paused guest. This access is typically used to perform
security checks, debugging, or malware analysis. The LibVMI project provides
and open source library that simplifies VMI programming. LibVMI supports 
both Xen and KVM environments.

Under KVM, LibVMI can work on systems today (albeit slowly) using the human
monitor command functionality to extract memory with the xp command. This
access is too slow for performance sensitive applications, so the LibVMI
project has created and maintained a QEMU patch that enables faster access.
We have used this patch for about 3 years now and it appears to be working
nicely for our community.

The patch in this email is an updated version of the LibVMI patch that aims
to conform to the Qemu coding guidelines. It is my hope that we can include
this in Qemu so that LibVMI users can leverage this faster access without
needing to do custom Qemu builds on their KVM systems.


Bryan D. Payne (1):
  qmp: extend QMP to provide read/write access to physical memory

 Makefile.target |   2 +-
 memory-access.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory-access.h |  11 ++++
 monitor.c       |  10 +++
 qmp-commands.hx |  27 ++++++++
 5 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 memory-access.c
 create mode 100644 memory-access.h

-- 
1.9.1

[Qemu-devel] [PATCH 1/1] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

This patch adds a new QMP command that sets up a domain socket. This
socket can then be used for fast read/write access to the guest's
physical memory. The key benefit to this system over existing solutions
is speed. Using this patch, guest memory can be copied out at a rate of
~200MB/sec, depending on the hardware. Existing solutions only achieve
a small fraction of this speed.

Signed-off-by: Bryan D. Payne <bdpayne@acm.org>
---
 Makefile.target |   2 +-
 memory-access.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory-access.h |  11 ++++
 monitor.c       |  10 +++
 qmp-commands.hx |  27 ++++++++
 5 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 memory-access.c
 create mode 100644 memory-access.h

diff --git a/Makefile.target b/Makefile.target
index e9ff1ee..4b3cd99 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -127,7 +127,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
-obj-y += qtest.o bootdevice.o
+obj-y += qtest.o bootdevice.o memory-access.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
diff --git a/memory-access.c b/memory-access.c
new file mode 100644
index 0000000..f696d7b
--- /dev/null
+++ b/memory-access.c
@@ -0,0 +1,200 @@
+/*
+ * Access guest physical memory via a domain socket.
+ *
+ * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
+ */
+
+#include "memory-access.h"
+#include "qemu-common.h"
+#include "exec/cpu-common.h"
+#include "config.h"
+
+#include <glib.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <pthread.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdint.h>
+
+struct request {
+    uint8_t type;      /* 0 quit, 1 read, 2 write, ... rest reserved */
+    uint64_t address;  /* address to read from OR write to */
+    uint64_t length;   /* number of bytes to read OR write */
+};
+
+static uint64_t
+connection_read_memory(uint64_t user_paddr, void *buf, uint64_t user_len)
+{
+    hwaddr paddr = (hwaddr) user_paddr;
+    hwaddr len = (hwaddr) user_len;
+    void *guestmem = cpu_physical_memory_map(paddr, &len, 0);
+    if (!guestmem) {
+        return 0;
+    }
+    memcpy(buf, guestmem, len);
+    cpu_physical_memory_unmap(guestmem, len, 0, len);
+
+    return len;
+}
+
+static uint64_t
+connection_write_memory(uint64_t user_paddr,
+                        const void *buf,
+                        uint64_t user_len)
+{
+    hwaddr paddr = (hwaddr) user_paddr;
+    hwaddr len = (hwaddr) user_len;
+    void *guestmem = cpu_physical_memory_map(paddr, &len, 1);
+    if (!guestmem) {
+        return 0;
+    }
+    memcpy(guestmem, buf, len);
+    cpu_physical_memory_unmap(guestmem, len, 0, len);
+
+    return len;
+}
+
+static void
+send_success_ack(int connection_fd)
+{
+    uint8_t success = 1;
+    int nbytes = write(connection_fd, &success, 1);
+    if (nbytes != 1) {
+        printf("QemuMemoryAccess: failed to send success ack\n");
+    }
+}
+
+static void
+send_fail_ack(int connection_fd)
+{
+    uint8_t fail = 0;
+    int nbytes = write(connection_fd, &fail, 1);
+    if (nbytes != 1) {
+        printf("QemuMemoryAccess: failed to send fail ack\n");
+    }
+}
+
+static void
+connection_handler(int connection_fd)
+{
+    int nbytes;
+    struct request req;
+
+    while (1) {
+        /* client request should match the struct request format */
+        nbytes = read(connection_fd, &req, sizeof(struct request));
+        if (nbytes != sizeof(struct request)) {
+            /* error */
+            send_fail_ack(connection_fd);
+            continue;
+        } else if (req.type == 0) {
+            /* request to quit, goodbye */
+            break;
+        } else if (req.type == 1) {
+            /* request to read */
+            char *buf = g_malloc(req.length + 1);
+            nbytes = connection_read_memory(req.address, buf, req.length);
+            if (nbytes != req.length) {
+                /* read failure, return failure message */
+                buf[req.length] = 0; /* set last byte to 0 for failure */
+                nbytes = write(connection_fd, buf, 1);
+            } else {
+                /* read success, return bytes */
+                buf[req.length] = 1; /* set last byte to 1 for success */
+                nbytes = write(connection_fd, buf, nbytes + 1);
+            }
+            g_free(buf);
+        } else if (req.type == 2) {
+            /* request to write */
+            void *write_buf = g_malloc(req.length);
+            nbytes = read(connection_fd, &write_buf, req.length);
+            if (nbytes != req.length) {
+                /* failed reading the message to write */
+                send_fail_ack(connection_fd);
+            } else{
+                /* do the write */
+                nbytes = connection_write_memory(req.address,
+                                                 write_buf,
+                                                 req.length);
+                if (nbytes == req.length) {
+                    send_success_ack(connection_fd);
+                } else {
+                    send_fail_ack(connection_fd);
+                }
+            }
+            g_free(write_buf);
+        } else {
+            /* unknown command */
+            printf("QemuMemoryAccess: ignoring unknown command (%d)\n",
+                   req.type);
+            send_fail_ack(connection_fd);
+        }
+    }
+
+    close(connection_fd);
+}
+
+static void *
+memory_access_thread(void *path)
+{
+    struct sockaddr_un address;
+    int socket_fd, connection_fd, path_len;
+    socklen_t address_length;
+
+    socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (socket_fd < 0) {
+        printf("QemuMemoryAccess: socket failed\n");
+        goto error_exit;
+    }
+
+    unlink(path); /* handle unlikely case that this temp file exists */
+    memset(&address, 0, sizeof(struct sockaddr_un));
+    address.sun_family = AF_UNIX;
+    path_len = sprintf(address.sun_path, "%s", (char *) path);
+    address_length = sizeof(address.sun_family) + path_len;
+
+    if (bind(socket_fd, (struct sockaddr *) &address, address_length) != 0) {
+        printf("QemuMemoryAccess: bind failed\n");
+        goto error_exit;
+    }
+    if (listen(socket_fd, 0) != 0) {
+        printf("QemuMemoryAccess: listen failed\n");
+        goto error_exit;
+    }
+
+    connection_fd = accept(socket_fd,
+                           (struct sockaddr *) &address,
+                           &address_length);
+    connection_handler(connection_fd);
+
+    close(socket_fd);
+    unlink(path);
+error_exit:
+    g_free(path);
+    return NULL;
+}
+
+int
+memory_access_start(const char *path)
+{
+    pthread_t thread;
+    sigset_t set, oldset;
+    int ret;
+
+    /* create a copy of path that we can safely use */
+    char *pathcopy = g_malloc(strlen(path) + 1);
+    memcpy(pathcopy, path, strlen(path) + 1);
+
+    /* start the thread */
+    sigfillset(&set);
+    pthread_sigmask(SIG_SETMASK, &set, &oldset);
+    ret = pthread_create(&thread, NULL, memory_access_thread, pathcopy);
+    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+
+    return ret;
+}
diff --git a/memory-access.h b/memory-access.h
new file mode 100644
index 0000000..cb5fe33
--- /dev/null
+++ b/memory-access.h
@@ -0,0 +1,11 @@
+/*
+ * Access guest physical memory via a domain socket.
+ *
+ * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
+ */
+#ifndef MEMORY_ACCESS_H
+#define MEMORY_ACCESS_H
+
+int memory_access_start(const char *path);
+
+#endif /* MEMORY_ACCESS_H */
diff --git a/monitor.c b/monitor.c
index fa00594..35e276a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -72,6 +72,7 @@
 #include "block/qapi.h"
 #include "qapi/qmp-event.h"
 #include "qapi-event.h"
+#include "memory-access.h"
 
 /* for pic/irq_info */
 #if defined(TARGET_SPARC)
@@ -1371,6 +1372,15 @@ static void do_print(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "\n");
 }
 
+static int do_physical_memory_access(Monitor *mon,
+                                     const QDict *qdict,
+                                     QObject **ret_data)
+{
+    const char *path = qdict_get_str(qdict, "path");
+    memory_access_start(path);
+    return 0;
+}
+
 static void do_sum(Monitor *mon, const QDict *qdict)
 {
     uint32_t addr;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 718dd92..ebe0e2a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -609,6 +609,33 @@ Example:
 EQMP
 
     {
+        .name       = "pmemaccess",
+        .args_type  = "path:s",
+        .params     = "path",
+        .help       = "mount guest physical memory image at 'path'",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_physical_memory_access,
+    },
+
+SQMP
+pmemaccess
+----------
+
+Mount guest physical memory image at 'path'.
+
+Arguments:
+
+- "path": mount point path (json-string)
+
+Example:
+
+-> { "execute": "pmemaccess",
+             "arguments": { "path": "/tmp/guestname" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/1] qmp: extend QMP to provide read/write access to physical memory

[Eric Blake]

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

On 11/26/2014 01:41 AM, Bryan D. Payne wrote:
> This patch adds a new QMP command that sets up a domain socket. This
> socket can then be used for fast read/write access to the guest's
> physical memory. The key benefit to this system over existing solutions
> is speed. Using this patch, guest memory can be copied out at a rate of
> ~200MB/sec, depending on the hardware. Existing solutions only achieve
> a small fraction of this speed.
> 
> Signed-off-by: Bryan D. Payne <bdpayne@acm.org>
> ---
>  Makefile.target |   2 +-
>  memory-access.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory-access.h |  11 ++++
>  monitor.c       |  10 +++
>  qmp-commands.hx |  27 ++++++++

Where is the *.json file that adds the QMP command contract?

> +++ b/qmp-commands.hx
> @@ -609,6 +609,33 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "pmemaccess",
> +        .args_type  = "path:s",
> +        .params     = "path",
> +        .help       = "mount guest physical memory image at 'path'",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_physical_memory_access,
> +    },
> +
> +SQMP
> +pmemaccess
> +----------
> +
> +Mount guest physical memory image at 'path'.
> +
> +Arguments:
> +
> +- "path": mount point path (json-string)
> +
> +Example:
> +
> +-> { "execute": "pmemaccess",
> +             "arguments": { "path": "/tmp/guestname" } }

Sounds like you are missing this entry (probably best to put it in the
top-level qapi-schema.json):

{ 'command': 'pmemaccess', 'data': { 'path': 'str' } }

as well as documentation that mentions it is targetted for addition in 2.3.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

[Qemu-devel] [PATCH v2] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

Thanks for the feedback Eric, I've updated the patch.

v2 changes:
- added QMP command contract to qapi-schema.json
- corrected some comments
- rewired QMP command to use schema code

[Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

This patch adds a new QMP command that sets up a domain socket. This
socket can then be used for fast read/write access to the guest's
physical memory. The key benefit to this system over existing solutions
is speed. Using this patch, guest memory can be copied out at a rate of
~200MB/sec, depending on the hardware. Existing solutions only achieve
a small fraction of this speed.

Signed-off-by: Bryan D. Payne <bdpayne@acm.org>
---
 Makefile.target  |   2 +-
 memory-access.c  | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory-access.h  |  11 +++
 monitor.c        |   6 ++
 qapi-schema.json |  12 ++++
 qmp-commands.hx  |  28 ++++++++
 6 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 memory-access.c
 create mode 100644 memory-access.h

diff --git a/Makefile.target b/Makefile.target
index e9ff1ee..4b3cd99 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -127,7 +127,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
-obj-y += qtest.o bootdevice.o
+obj-y += qtest.o bootdevice.o memory-access.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
diff --git a/memory-access.c b/memory-access.c
new file mode 100644
index 0000000..f696d7b
--- /dev/null
+++ b/memory-access.c
@@ -0,0 +1,200 @@
+/*
+ * Access guest physical memory via a domain socket.
+ *
+ * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
+ */
+
+#include "memory-access.h"
+#include "qemu-common.h"
+#include "exec/cpu-common.h"
+#include "config.h"
+
+#include <glib.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <pthread.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdint.h>
+
+struct request {
+    uint8_t type;      /* 0 quit, 1 read, 2 write, ... rest reserved */
+    uint64_t address;  /* address to read from OR write to */
+    uint64_t length;   /* number of bytes to read OR write */
+};
+
+static uint64_t
+connection_read_memory(uint64_t user_paddr, void *buf, uint64_t user_len)
+{
+    hwaddr paddr = (hwaddr) user_paddr;
+    hwaddr len = (hwaddr) user_len;
+    void *guestmem = cpu_physical_memory_map(paddr, &len, 0);
+    if (!guestmem) {
+        return 0;
+    }
+    memcpy(buf, guestmem, len);
+    cpu_physical_memory_unmap(guestmem, len, 0, len);
+
+    return len;
+}
+
+static uint64_t
+connection_write_memory(uint64_t user_paddr,
+                        const void *buf,
+                        uint64_t user_len)
+{
+    hwaddr paddr = (hwaddr) user_paddr;
+    hwaddr len = (hwaddr) user_len;
+    void *guestmem = cpu_physical_memory_map(paddr, &len, 1);
+    if (!guestmem) {
+        return 0;
+    }
+    memcpy(guestmem, buf, len);
+    cpu_physical_memory_unmap(guestmem, len, 0, len);
+
+    return len;
+}
+
+static void
+send_success_ack(int connection_fd)
+{
+    uint8_t success = 1;
+    int nbytes = write(connection_fd, &success, 1);
+    if (nbytes != 1) {
+        printf("QemuMemoryAccess: failed to send success ack\n");
+    }
+}
+
+static void
+send_fail_ack(int connection_fd)
+{
+    uint8_t fail = 0;
+    int nbytes = write(connection_fd, &fail, 1);
+    if (nbytes != 1) {
+        printf("QemuMemoryAccess: failed to send fail ack\n");
+    }
+}
+
+static void
+connection_handler(int connection_fd)
+{
+    int nbytes;
+    struct request req;
+
+    while (1) {
+        /* client request should match the struct request format */
+        nbytes = read(connection_fd, &req, sizeof(struct request));
+        if (nbytes != sizeof(struct request)) {
+            /* error */
+            send_fail_ack(connection_fd);
+            continue;
+        } else if (req.type == 0) {
+            /* request to quit, goodbye */
+            break;
+        } else if (req.type == 1) {
+            /* request to read */
+            char *buf = g_malloc(req.length + 1);
+            nbytes = connection_read_memory(req.address, buf, req.length);
+            if (nbytes != req.length) {
+                /* read failure, return failure message */
+                buf[req.length] = 0; /* set last byte to 0 for failure */
+                nbytes = write(connection_fd, buf, 1);
+            } else {
+                /* read success, return bytes */
+                buf[req.length] = 1; /* set last byte to 1 for success */
+                nbytes = write(connection_fd, buf, nbytes + 1);
+            }
+            g_free(buf);
+        } else if (req.type == 2) {
+            /* request to write */
+            void *write_buf = g_malloc(req.length);
+            nbytes = read(connection_fd, &write_buf, req.length);
+            if (nbytes != req.length) {
+                /* failed reading the message to write */
+                send_fail_ack(connection_fd);
+            } else{
+                /* do the write */
+                nbytes = connection_write_memory(req.address,
+                                                 write_buf,
+                                                 req.length);
+                if (nbytes == req.length) {
+                    send_success_ack(connection_fd);
+                } else {
+                    send_fail_ack(connection_fd);
+                }
+            }
+            g_free(write_buf);
+        } else {
+            /* unknown command */
+            printf("QemuMemoryAccess: ignoring unknown command (%d)\n",
+                   req.type);
+            send_fail_ack(connection_fd);
+        }
+    }
+
+    close(connection_fd);
+}
+
+static void *
+memory_access_thread(void *path)
+{
+    struct sockaddr_un address;
+    int socket_fd, connection_fd, path_len;
+    socklen_t address_length;
+
+    socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (socket_fd < 0) {
+        printf("QemuMemoryAccess: socket failed\n");
+        goto error_exit;
+    }
+
+    unlink(path); /* handle unlikely case that this temp file exists */
+    memset(&address, 0, sizeof(struct sockaddr_un));
+    address.sun_family = AF_UNIX;
+    path_len = sprintf(address.sun_path, "%s", (char *) path);
+    address_length = sizeof(address.sun_family) + path_len;
+
+    if (bind(socket_fd, (struct sockaddr *) &address, address_length) != 0) {
+        printf("QemuMemoryAccess: bind failed\n");
+        goto error_exit;
+    }
+    if (listen(socket_fd, 0) != 0) {
+        printf("QemuMemoryAccess: listen failed\n");
+        goto error_exit;
+    }
+
+    connection_fd = accept(socket_fd,
+                           (struct sockaddr *) &address,
+                           &address_length);
+    connection_handler(connection_fd);
+
+    close(socket_fd);
+    unlink(path);
+error_exit:
+    g_free(path);
+    return NULL;
+}
+
+int
+memory_access_start(const char *path)
+{
+    pthread_t thread;
+    sigset_t set, oldset;
+    int ret;
+
+    /* create a copy of path that we can safely use */
+    char *pathcopy = g_malloc(strlen(path) + 1);
+    memcpy(pathcopy, path, strlen(path) + 1);
+
+    /* start the thread */
+    sigfillset(&set);
+    pthread_sigmask(SIG_SETMASK, &set, &oldset);
+    ret = pthread_create(&thread, NULL, memory_access_thread, pathcopy);
+    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+
+    return ret;
+}
diff --git a/memory-access.h b/memory-access.h
new file mode 100644
index 0000000..cb5fe33
--- /dev/null
+++ b/memory-access.h
@@ -0,0 +1,11 @@
+/*
+ * Access guest physical memory via a domain socket.
+ *
+ * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
+ */
+#ifndef MEMORY_ACCESS_H
+#define MEMORY_ACCESS_H
+
+int memory_access_start(const char *path);
+
+#endif /* MEMORY_ACCESS_H */
diff --git a/monitor.c b/monitor.c
index fa00594..19724b8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -72,6 +72,7 @@
 #include "block/qapi.h"
 #include "qapi/qmp-event.h"
 #include "qapi-event.h"
+#include "memory-access.h"
 
 /* for pic/irq_info */
 #if defined(TARGET_SPARC)
@@ -1371,6 +1372,11 @@ static void do_print(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "\n");
 }
 
+void qmp_pmemaccess(const char *path, Error **err)
+{
+    memory_access_start(path);
+}
+
 static void do_sum(Monitor *mon, const QDict *qdict)
 {
     uint32_t addr;
diff --git a/qapi-schema.json b/qapi-schema.json
index 9ffdcf8..37a6657 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3515,3 +3515,15 @@
 # Since: 2.1
 ##
 { 'command': 'rtc-reset-reinjection' }
+
+##
+# @pmemaccess
+#
+# This command enables access to guest physical memory using
+# a simple protocol over a UNIX domain socket.
+#
+# @path Location to use for the UNIX domain socket
+#
+# Since: 2.3
+##
+{ 'command': 'pmemaccess', 'data': { 'path': 'str' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 718dd92..fab1322 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -609,6 +609,34 @@ Example:
 EQMP
 
     {
+        .name       = "pmemaccess",
+        .args_type  = "path:s",
+        .params     = "path",
+        .help       = "access to guest memory via domain socket at 'path'",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = qmp_marshal_input_pmemaccess,
+    },
+
+SQMP
+pmemaccess
+----------
+
+This command enables access to guest physical memory using a simple protocol
+over a UNIX domain socket.
+
+Arguments:
+
+- "path": path for domain socket (json-string)
+
+Example:
+
+-> { "execute": "pmemaccess",
+             "arguments": { "path": "/tmp/guestname" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
-- 
1.9.1

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Wed, 11/26 12:27, Bryan D. Payne wrote:
> This patch adds a new QMP command that sets up a domain socket. This
> socket can then be used for fast read/write access to the guest's
> physical memory. The key benefit to this system over existing solutions
> is speed. Using this patch, guest memory can be copied out at a rate of
> ~200MB/sec, depending on the hardware. Existing solutions only achieve
> a small fraction of this speed.

Out of curiosity, what are existing solutions?

> 
> Signed-off-by: Bryan D. Payne <bdpayne@acm.org>
> ---
>  Makefile.target  |   2 +-
>  memory-access.c  | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory-access.h  |  11 +++
>  monitor.c        |   6 ++
>  qapi-schema.json |  12 ++++
>  qmp-commands.hx  |  28 ++++++++
>  6 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 memory-access.c
>  create mode 100644 memory-access.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index e9ff1ee..4b3cd99 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -127,7 +127,7 @@ endif #CONFIG_BSD_USER
>  # System emulator target
>  ifdef CONFIG_SOFTMMU
>  obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
> -obj-y += qtest.o bootdevice.o
> +obj-y += qtest.o bootdevice.o memory-access.o
>  obj-y += hw/
>  obj-$(CONFIG_FDT) += device_tree.o
>  obj-$(CONFIG_KVM) += kvm-all.o
> diff --git a/memory-access.c b/memory-access.c
> new file mode 100644
> index 0000000..f696d7b
> --- /dev/null
> +++ b/memory-access.c
> @@ -0,0 +1,200 @@
> +/*
> + * Access guest physical memory via a domain socket.
> + *
> + * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
> + */
> +
> +#include "memory-access.h"
> +#include "qemu-common.h"
> +#include "exec/cpu-common.h"
> +#include "config.h"
> +
> +#include <glib.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <pthread.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <stdint.h>
> +
> +struct request {
> +    uint8_t type;      /* 0 quit, 1 read, 2 write, ... rest reserved */
> +    uint64_t address;  /* address to read from OR write to */
> +    uint64_t length;   /* number of bytes to read OR write */
> +};

Please add QEMU_PACKED to this structure, and probably name it QEMUMARequest,
for name collision avoidance and CamelCase convension.

> +
> +static uint64_t
> +connection_read_memory(uint64_t user_paddr, void *buf, uint64_t user_len)
> +{
> +    hwaddr paddr = (hwaddr) user_paddr;
> +    hwaddr len = (hwaddr) user_len;
> +    void *guestmem = cpu_physical_memory_map(paddr, &len, 0);

Accessing guest memory from another thread is not safe without holding the
lock: qemu_mutex_lock_iothread is needed before processing a request, and
unlock it after responding, to let guest continue.

> +    if (!guestmem) {
> +        return 0;
> +    }
> +    memcpy(buf, guestmem, len);
> +    cpu_physical_memory_unmap(guestmem, len, 0, len);
> +
> +    return len;
> +}
> +
> +static uint64_t
> +connection_write_memory(uint64_t user_paddr,
> +                        const void *buf,
> +                        uint64_t user_len)
> +{
> +    hwaddr paddr = (hwaddr) user_paddr;
> +    hwaddr len = (hwaddr) user_len;
> +    void *guestmem = cpu_physical_memory_map(paddr, &len, 1);
> +    if (!guestmem) {
> +        return 0;
> +    }
> +    memcpy(guestmem, buf, len);
> +    cpu_physical_memory_unmap(guestmem, len, 0, len);
> +
> +    return len;
> +}
> +
> +static void
> +send_success_ack(int connection_fd)
> +{
> +    uint8_t success = 1;
> +    int nbytes = write(connection_fd, &success, 1);
> +    if (nbytes != 1) {
> +        printf("QemuMemoryAccess: failed to send success ack\n");
> +    }
> +}
> +
> +static void
> +send_fail_ack(int connection_fd)
> +{
> +    uint8_t fail = 0;
> +    int nbytes = write(connection_fd, &fail, 1);
> +    if (nbytes != 1) {
> +        printf("QemuMemoryAccess: failed to send fail ack\n");

fprintf(stderr ?

> +    }
> +}
> +
> +static void
> +connection_handler(int connection_fd)
> +{
> +    int nbytes;
> +    struct request req;
> +
> +    while (1) {
> +        /* client request should match the struct request format */
> +        nbytes = read(connection_fd, &req, sizeof(struct request));
> +        if (nbytes != sizeof(struct request)) {
> +            /* error */
> +            send_fail_ack(connection_fd);
> +            continue;
> +        } else if (req.type == 0) {

I suggest using macro names for enum instead of magic values 0, 1, 2.

> +            /* request to quit, goodbye */
> +            break;
> +        } else if (req.type == 1) {
> +            /* request to read */
> +            char *buf = g_malloc(req.length + 1);

Is there a limit for r/w length? We could abort if req.length is too big.

> +            nbytes = connection_read_memory(req.address, buf, req.length);
> +            if (nbytes != req.length) {
> +                /* read failure, return failure message */
> +                buf[req.length] = 0; /* set last byte to 0 for failure */
> +                nbytes = write(connection_fd, buf, 1);
> +            } else {
> +                /* read success, return bytes */
> +                buf[req.length] = 1; /* set last byte to 1 for success */
> +                nbytes = write(connection_fd, buf, nbytes + 1);
> +            }
> +            g_free(buf);
> +        } else if (req.type == 2) {
> +            /* request to write */
> +            void *write_buf = g_malloc(req.length);
> +            nbytes = read(connection_fd, &write_buf, req.length);
> +            if (nbytes != req.length) {

Is short read possible here?

> +                /* failed reading the message to write */
> +                send_fail_ack(connection_fd);
> +            } else{

s/else{/else {

> +                /* do the write */
> +                nbytes = connection_write_memory(req.address,
> +                                                 write_buf,
> +                                                 req.length);
> +                if (nbytes == req.length) {
> +                    send_success_ack(connection_fd);
> +                } else {
> +                    send_fail_ack(connection_fd);
> +                }
> +            }
> +            g_free(write_buf);
> +        } else {
> +            /* unknown command */
> +            printf("QemuMemoryAccess: ignoring unknown command (%d)\n",
> +                   req.type);
> +            send_fail_ack(connection_fd);
> +        }
> +    }
> +
> +    close(connection_fd);
> +}
> +
> +static void *
> +memory_access_thread(void *path)
> +{
> +    struct sockaddr_un address;
> +    int socket_fd, connection_fd, path_len;
> +    socklen_t address_length;
> +
> +    socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);
> +    if (socket_fd < 0) {
> +        printf("QemuMemoryAccess: socket failed\n");
> +        goto error_exit;
> +    }
> +
> +    unlink(path); /* handle unlikely case that this temp file exists */
> +    memset(&address, 0, sizeof(struct sockaddr_un));
> +    address.sun_family = AF_UNIX;
> +    path_len = sprintf(address.sun_path, "%s", (char *) path);
> +    address_length = sizeof(address.sun_family) + path_len;
> +
> +    if (bind(socket_fd, (struct sockaddr *) &address, address_length) != 0) {
> +        printf("QemuMemoryAccess: bind failed\n");
> +        goto error_exit;

socket_fd is left open on error.

> +    }
> +    if (listen(socket_fd, 0) != 0) {
> +        printf("QemuMemoryAccess: listen failed\n");
> +        goto error_exit;
> +    }
> +
> +    connection_fd = accept(socket_fd,
> +                           (struct sockaddr *) &address,
> +                           &address_length);
> +    connection_handler(connection_fd);
> +
> +    close(socket_fd);
> +    unlink(path);
> +error_exit:
> +    g_free(path);
> +    return NULL;
> +}
> +
> +int
> +memory_access_start(const char *path)
> +{
> +    pthread_t thread;
> +    sigset_t set, oldset;
> +    int ret;
> +
> +    /* create a copy of path that we can safely use */
> +    char *pathcopy = g_malloc(strlen(path) + 1);
> +    memcpy(pathcopy, path, strlen(path) + 1);

char *pathcopy = g_strdup(path);

> +
> +    /* start the thread */
> +    sigfillset(&set);
> +    pthread_sigmask(SIG_SETMASK, &set, &oldset);
> +    ret = pthread_create(&thread, NULL, memory_access_thread, pathcopy);
> +    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> +
> +    return ret;
> +}
> diff --git a/memory-access.h b/memory-access.h
> new file mode 100644
> index 0000000..cb5fe33
> --- /dev/null
> +++ b/memory-access.h
> @@ -0,0 +1,11 @@
> +/*
> + * Access guest physical memory via a domain socket.
> + *
> + * Copyright (c) 2014 Bryan D. Payne (bdpayne@acm.org)
> + */
> +#ifndef MEMORY_ACCESS_H
> +#define MEMORY_ACCESS_H
> +
> +int memory_access_start(const char *path);
> +
> +#endif /* MEMORY_ACCESS_H */
> diff --git a/monitor.c b/monitor.c
> index fa00594..19724b8 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -72,6 +72,7 @@
>  #include "block/qapi.h"
>  #include "qapi/qmp-event.h"
>  #include "qapi-event.h"
> +#include "memory-access.h"
>  
>  /* for pic/irq_info */
>  #if defined(TARGET_SPARC)
> @@ -1371,6 +1372,11 @@ static void do_print(Monitor *mon, const QDict *qdict)
>      monitor_printf(mon, "\n");
>  }
>  
> +void qmp_pmemaccess(const char *path, Error **err)
> +{
> +    memory_access_start(path);
> +}
> +
>  static void do_sum(Monitor *mon, const QDict *qdict)
>  {
>      uint32_t addr;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9ffdcf8..37a6657 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3515,3 +3515,15 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @pmemaccess
> +#
> +# This command enables access to guest physical memory using
> +# a simple protocol over a UNIX domain socket.
> +#
> +# @path Location to use for the UNIX domain socket
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'pmemaccess', 'data': { 'path': 'str' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 718dd92..fab1322 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -609,6 +609,34 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "pmemaccess",
> +        .args_type  = "path:s",
> +        .params     = "path",
> +        .help       = "access to guest memory via domain socket at 'path'",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = qmp_marshal_input_pmemaccess,
> +    },
> +
> +SQMP
> +pmemaccess
> +----------
> +
> +This command enables access to guest physical memory using a simple protocol
> +over a UNIX domain socket.

Please document the protocol, as it's the most important part of this command.

Fam

> +
> +Arguments:
> +
> +- "path": path for domain socket (json-string)
> +
> +Example:
> +
> +-> { "execute": "pmemaccess",
> +             "arguments": { "path": "/tmp/guestname" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "migrate",
>          .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>          .mhandler.cmd_new = qmp_marshal_input_migrate,
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Out of curiosity, what are existing solutions?
>

Basically just attaching gdb and pulling memory out manually (or writing a
program to do the same).


> +struct request {
> > +    uint8_t type;      /* 0 quit, 1 read, 2 write, ... rest reserved */
> > +    uint64_t address;  /* address to read from OR write to */
> > +    uint64_t length;   /* number of bytes to read OR write */
> > +};
>
> Please add QEMU_PACKED to this structure, and probably name it
> QEMUMARequest,
> for name collision avoidance and CamelCase convension.
>

How critical is QEMU_PACKED?  This changes the layout of the struct which
means that existing clients (LibVMI) would need to change their code as
well.

-bryan

[-- Attachment #2: Type: text/html, Size: 1193 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Wed, 12/03 19:37, Bryan D. Payne wrote:
> >
> > Out of curiosity, what are existing solutions?
> >
> 
> Basically just attaching gdb and pulling memory out manually (or writing a
> program to do the same).
> 
> 
> > +struct request {
> > > +    uint8_t type;      /* 0 quit, 1 read, 2 write, ... rest reserved */
> > > +    uint64_t address;  /* address to read from OR write to */
> > > +    uint64_t length;   /* number of bytes to read OR write */
> > > +};
> >
> > Please add QEMU_PACKED to this structure, and probably name it
> > QEMUMARequest,
> > for name collision avoidance and CamelCase convension.
> >
> 
> How critical is QEMU_PACKED?  This changes the layout of the struct which
> means that existing clients (LibVMI) would need to change their code as
> well.

It is critical as a transport data structure. You have to define a byte-by-byte
layout (concerning endianness and padding) and use padding fields together with
QEMU_PACKED, so the representation is not dependent on alignment:

http://en.wikipedia.org/wiki/Data_structure_alignment

For example:

typedef struct {
    union {
        uint8_t type;
        uint64_t padding;
    };
    uint64_t address;
    uint64_t length;
} QEMU_PACKED QEMUMARequest;

It also means that client need to use the same endianness with us, but there is
no way to query it.

An alternative might be carry endianness information in the protocol
negotiation or in request.

Fam

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> It is critical as a transport data structure. You have to define a
> byte-by-byte
> layout (concerning endianness and padding) and use padding fields together
> with
> QEMU_PACKED, so the representation is not dependent on alignment


This makes sense for network protocols.  But, in this case, the protocol is
always taking placing over a unix socket on the localhost.  This is why I
wasn't sure that it was necessary.

Just to be clear... the QMP connection could go over the network, but its
only purpose is to set up the unix socket on the local machine (meaning
that doing it over the network is kind of pointless, even if it is
possible).  Once the unix socket is setup, the memory access protocol
happens over that socket.

Thoughts?

-bryan

[-- Attachment #2: Type: text/html, Size: 1141 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Wed, 12/03 22:28, Bryan D. Payne wrote:
> >
> > It is critical as a transport data structure. You have to define a
> > byte-by-byte
> > layout (concerning endianness and padding) and use padding fields together
> > with
> > QEMU_PACKED, so the representation is not dependent on alignment
> 
> 
> This makes sense for network protocols.  But, in this case, the protocol is
> always taking placing over a unix socket on the localhost.  This is why I
> wasn't sure that it was necessary.
> 
> Just to be clear... the QMP connection could go over the network, but its
> only purpose is to set up the unix socket on the local machine (meaning
> that doing it over the network is kind of pointless, even if it is
> possible).  Once the unix socket is setup, the memory access protocol
> happens over that socket.

This doesn't stop the client from using a different alignment than we expect.
It's necessary to be explicit as a binary protocol.

Fam

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> This doesn't stop the client from using a different alignment than we
> expect.
> It's necessary to be explicit as a binary protocol.
>

Ok, I'll move ahead with packing the data and sort out the backwards compat
issues on the client side.
-bryan

[-- Attachment #2: Type: text/html, Size: 584 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Thu, 12/04 08:43, Bryan D. Payne wrote:
> >
> > This doesn't stop the client from using a different alignment than we
> > expect.
> > It's necessary to be explicit as a binary protocol.
> >
> 
> Ok, I'll move ahead with packing the data and sort out the backwards compat
> issues on the client side.

Or use a backwards compatible packed structure. :)

Fam

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Markus Armbruster]

"Bryan D. Payne" <bdpayne@acm.org> writes:

>>
>> Out of curiosity, what are existing solutions?
>>
>
> Basically just attaching gdb and pulling memory out manually (or writing a
> program to do the same).

Can you explain again why the existing commands to read guest memory
(from the top of my head: dump-guest-memory, memsave, pmemsave) are
insufficient?  How does your solution improve on them?  What exactly can
it do what these commands can't?  What exactly can't it do what these
commands can?

I feel we need to understand the answers to these questions to sensibly
evolve the API in this area.

[...]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Can you explain again why the existing commands to read guest memory
> (from the top of my head: dump-guest-memory, memsave, pmemsave) are
> insufficient?  How does your solution improve on them?  What exactly can
> it do what these commands can't?  What exactly can't it do what these
> commands can?
>
> I feel we need to understand the answers to these questions to sensibly
> evolve the API in this area.


Certainly.  The main issue with this series of commands is that they dump
the memory to a file on disk.  What I'm trying to facilitate here is an
application that monitors the guest memory in real time while the guest is
running (likely with brief pauses during memory analysis periods).  Writing
all of this data to disk, and then reading it back again for the analysis
application is several orders of magnitude too slow for these types of
applications.

This new method that I'm proposing here keeps everything in memory, which
makes it much faster.

Tldr; existing solutions are suitable for forensic analysis, whereas I'm
looking to solve the runtime analysis problem.

-bryan

[-- Attachment #2: Type: text/html, Size: 1448 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Markus Armbruster]

"Bryan D. Payne" <bdpayne@acm.org> writes:

>>
>> Can you explain again why the existing commands to read guest memory
>> (from the top of my head: dump-guest-memory, memsave, pmemsave) are
>> insufficient?  How does your solution improve on them?  What exactly can
>> it do what these commands can't?  What exactly can't it do what these
>> commands can?
>>
>> I feel we need to understand the answers to these questions to sensibly
>> evolve the API in this area.
>
>
> Certainly.  The main issue with this series of commands is that they dump
> the memory to a file on disk.  What I'm trying to facilitate here is an
> application that monitors the guest memory in real time while the guest is
> running (likely with brief pauses during memory analysis periods).  Writing
> all of this data to disk, and then reading it back again for the analysis
> application is several orders of magnitude too slow for these types of
> applications.
>
> This new method that I'm proposing here keeps everything in memory, which
> makes it much faster.

Actually, it sends it through a UNIX domain socket.  Still I/O, just
different I/O.  "Faster" is plausible, but by how much I can't say
without measurements.  "Several orders of magnitude" needs evidence.

I think you could make another supporting argument: the socket is easier
to use than futzing around with files.

> Tldr; existing solutions are suitable for forensic analysis, whereas I'm
> looking to solve the runtime analysis problem.

Understood.

Based on your qapi-schema.json change only, I figure your new command
sets up a special-purpose monitor-like thing on a newly created a UNIX
domain socket.  This monitor-like thing talks its own language, which is
so far undocmented.

Why restrict this to a UNIX domain socket?  In other places involving
byte streams we use a QEMU character device.

Can you explain why you need a separate stream instead of embedding the
conversation in QMP?  We already have such embedded I/O: ringbuf-read,
ringbuf-write.  Yes, it got multiple documented bugs, but that's because
it was badly done, not because such a thing couldn't be done nicely.

If you add a separate stream talking its own language, you get to
document the language.  A file in docs/ should do.  The QMP command
documentation should point to it.

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Actually, it sends it through a UNIX domain socket.  Still I/O, just
> different I/O.  "Faster" is plausible, but by how much I can't say
> without measurements.  "Several orders of magnitude" needs evidence.
>

I've been testing introspection performance on both Xen and KVM for over 5
years now, and I'm pretty comfortable with this statement.  Bottom line is
that sending lots of data through a disk is much slower than sending lots
of data through memory (for example, see Figure 3 in
http://queue.acm.org/detail.cfm?id=1563874, which shows about 5 orders of
magnitude difference for random access data@).  A UNIX domain socket is
just going through memory.

I have other Qemu modifications that can reduce the use of the memory bus
by using shared memory instead.  This provides another big bump in
performance.  However, this work is still in earlier stages.


Based on your qapi-schema.json change only, I figure your new command
> sets up a special-purpose monitor-like thing on a newly created a UNIX
> domain socket.  This monitor-like thing talks its own language, which is
> so far undocmented.
>

Correct.  Per request from one of the reviewers in this thread, I have an
updated patch that has full documentation on this protocol.  I've been
waiting to push that as I try to sort out some of the remaining discussion
in this thread, though.



> Why restrict this to a UNIX domain socket?  In other places involving
> byte streams we use a QEMU character device.
>
> Can you explain why you need a separate stream instead of embedding the
> conversation in QMP?  We already have such embedded I/O: ringbuf-read,
> ringbuf-write.  Yes, it got multiple documented bugs, but that's because
> it was badly done, not because such a thing couldn't be done nicely.
>

I think that it could be done in QMP.  I would want to validate the
performance of that approach though as there will be some extra overhead.
I would also be concerned about access to this data over a network.

For security reasons, it feels better to me to restrict this feature to
someone that is on the local machine.  Using a UNIX domain socket forces
this constraint.  I don't believe that is the case with QMP, but please
correct me if I'm wrong about that.


If you add a separate stream talking its own language, you get to
> document the language.  A file in docs/ should do.  The QMP command
> documentation should point to it.
>

My updated patch documents it as a comment directly in the C source, as
that is what I felt a previous reviewer was asking for.  Is docs more
appropriate or ??

Cheers,
-bryan

[-- Attachment #2: Type: text/html, Size: 3880 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Markus Armbruster]

"Bryan D. Payne" <bdpayne@acm.org> writes:

>>
>> Actually, it sends it through a UNIX domain socket.  Still I/O, just
>> different I/O.  "Faster" is plausible, but by how much I can't say
>> without measurements.  "Several orders of magnitude" needs evidence.
>>
>
> I've been testing introspection performance on both Xen and KVM for over 5
> years now, and I'm pretty comfortable with this statement.  Bottom line is
> that sending lots of data through a disk is much slower than sending lots
> of data through memory (for example, see Figure 3 in
> http://queue.acm.org/detail.cfm?id=1563874, which shows about 5 orders of
> magnitude difference for random access data@).  A UNIX domain socket is
> just going through memory.

By "evidence", I mean actual numbers for actual QEMU code.  Nothing
sophisticated, just use your new interface in a way you consider
relevant for your own use case, then approximate this use with existing
interfaces.  The approximation can be very rough.  For instance, showing
that doing the whole job with your approach is a much faster than doing
a necessary part of the job with existing commands would do.

> I have other Qemu modifications that can reduce the use of the memory bus
> by using shared memory instead.  This provides another big bump in
> performance.  However, this work is still in earlier stages.
>
>
> Based on your qapi-schema.json change only, I figure your new command
>> sets up a special-purpose monitor-like thing on a newly created a UNIX
>> domain socket.  This monitor-like thing talks its own language, which is
>> so far undocmented.
>>
>
> Correct.  Per request from one of the reviewers in this thread, I have an
> updated patch that has full documentation on this protocol.  I've been
> waiting to push that as I try to sort out some of the remaining discussion
> in this thread, though.

Makes sense.

>> Why restrict this to a UNIX domain socket?  In other places involving
>> byte streams we use a QEMU character device.
>>
>> Can you explain why you need a separate stream instead of embedding the
>> conversation in QMP?  We already have such embedded I/O: ringbuf-read,
>> ringbuf-write.  Yes, it got multiple documented bugs, but that's because
>> it was badly done, not because such a thing couldn't be done nicely.
>>
>
> I think that it could be done in QMP.  I would want to validate the
> performance of that approach though as there will be some extra overhead.

The QMP overhead could be too high.  QMP is control plane, not data
plane.  How much data do you envisage flowing here?

In theory, even the character device overhead could be too high.
Measuring it shouldn't be too hard.

> I would also be concerned about access to this data over a network.
>
> For security reasons, it feels better to me to restrict this feature to
> someone that is on the local machine.  Using a UNIX domain socket forces
> this constraint.  I don't believe that is the case with QMP, but please
> correct me if I'm wrong about that.

If you control a QMP monitor, you own the guest.  If you set up your QMP
monitor in a way that permits untrusted users to control it, you're
giving away the guest.  Adding more commands to access guest memory
doesn't really make this worse.  It could perhaps make p0wning more
efficient.

> If you add a separate stream talking its own language, you get to
>> document the language.  A file in docs/ should do.  The QMP command
>> documentation should point to it.
>>
>
> My updated patch documents it as a comment directly in the C source, as
> that is what I felt a previous reviewer was asking for.  Is docs more
> appropriate or ??

I feel code comment is find for internal interfaces, but for external
interfaces, a separate interface document is more appropriate.

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> By "evidence", I mean actual numbers for actual QEMU code.  Nothing
> sophisticated, just use your new interface in a way you consider
> relevant for your own use case, then approximate this use with existing
> interfaces.  The approximation can be very rough.  For instance, showing
> that doing the whole job with your approach is a much faster than doing
> a necessary part of the job with existing commands would do.
>

Sure, I can put together some numbers to help with this discussion.



> The QMP overhead could be too high.  QMP is control plane, not data
> plane.  How much data do you envisage flowing here?
>
> In theory, even the character device overhead could be too high.
> Measuring it shouldn't be too hard.
>

This discussion has me thinking about whether QMP would be viable.  I think
I'll take a little time to explore that approach in a little more depth
before proceeding here.  I can report back with what I find.


If you control a QMP monitor, you own the guest.


Good point.  So I'll explore the performance aspects and let that drive the
decision.



> I feel code comment is find for internal interfaces, but for external
> interfaces, a separate interface document is more appropriate.
>

Sounds good.

Cheers,
-bryan

[-- Attachment #2: Type: text/html, Size: 2131 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

> By "evidence", I mean actual numbers for actual QEMU code.  Nothing
>> sophisticated, just use your new interface in a way you consider
>> relevant for your own use case, then approximate this use with existing
>> interfaces.  The approximation can be very rough.  For instance, showing
>> that doing the whole job with your approach is a much faster than doing
>> a necessary part of the job with existing commands would do.
>>
>
> Sure, I can put together some numbers to help with this discussion.
>

I've done some performance testing on my laptop.  I have setup LibVMI to
access KVM guest memory using one of three different options:

QMP/pmemsave (dump bytes to a file and then read that back into memory)
HMC/xp command (dump bytes via human monitor command and repack the output
to binary form)
QMP/pmemaccess (access bytes through a Unix socket, this is the patch we
are discussing in this thread)

For each of these I measured the time required to translate a guest kernel
virtual address into a guest physical address.  This is done with a 64-bit
Ubuntu Trusty guest and the addresses are translated by walking the page
tables in the guest using the selected memory access technique.

You can see the graph here:
http://cl.ly/image/322a3s0h1V05

For those that prefer text, here's the numbers (in microseconds):
QMP/pmemsave: 77706
HMC/xp command: 92552
QMP/pmemaccess: 95

As you can see, the technique proposed in this patch is about 3 orders of
magnitude faster than the options available in Qemu today.  I will also
note that this is for a _single_ virtual address translation.  A typical
introspection application will be doing much more work than this, which
will only compound the problem.

Hopefully this helps to explain why I believe that this is an improvement
to the existing mechanisms.  Please let me know if you have any questions
about these results.

I'll continue some exploration of doing the pmemaccess bulk data transfer
over QMP, instead of the UNIX socket.  And I'll report back here once I've
come up with some thoughts on the best way to proceed.

Cheers,
-bryan

[-- Attachment #2: Type: text/html, Size: 3049 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Wed, 12/10 21:33, Bryan D. Payne wrote:
> > By "evidence", I mean actual numbers for actual QEMU code.  Nothing
> >> sophisticated, just use your new interface in a way you consider
> >> relevant for your own use case, then approximate this use with existing
> >> interfaces.  The approximation can be very rough.  For instance, showing
> >> that doing the whole job with your approach is a much faster than doing
> >> a necessary part of the job with existing commands would do.
> >>
> >
> > Sure, I can put together some numbers to help with this discussion.
> >
> 
> I've done some performance testing on my laptop.  I have setup LibVMI to
> access KVM guest memory using one of three different options:
> 
> QMP/pmemsave (dump bytes to a file and then read that back into memory)
> HMC/xp command (dump bytes via human monitor command and repack the output
> to binary form)
> QMP/pmemaccess (access bytes through a Unix socket, this is the patch we
> are discussing in this thread)
> 
> For each of these I measured the time required to translate a guest kernel
> virtual address into a guest physical address.  This is done with a 64-bit
> Ubuntu Trusty guest and the addresses are translated by walking the page
> tables in the guest using the selected memory access technique.
> 
> You can see the graph here:
> http://cl.ly/image/322a3s0h1V05
> 
> For those that prefer text, here's the numbers (in microseconds):
> QMP/pmemsave: 77706
> HMC/xp command: 92552
> QMP/pmemaccess: 95
> 
> As you can see, the technique proposed in this patch is about 3 orders of
> magnitude faster than the options available in Qemu today.  I will also
> note that this is for a _single_ virtual address translation.  A typical
> introspection application will be doing much more work than this, which
> will only compound the problem.
> 
> Hopefully this helps to explain why I believe that this is an improvement
> to the existing mechanisms.  Please let me know if you have any questions
> about these results.
> 
> I'll continue some exploration of doing the pmemaccess bulk data transfer
> over QMP, instead of the UNIX socket.  And I'll report back here once I've
> come up with some thoughts on the best way to proceed.
> 

Look good. I believe QMP will be in between, and if it doesn't work as well,
could you also try to use QEMU's char dev instead of limit this to unix socket?

Fam

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Look good. I believe QMP will be in between, and if it doesn't work as
> well,
> could you also try to use QEMU's char dev instead of limit this to unix
> socket?
>

Sure, I'll look into that as well.
-bryan

[-- Attachment #2: Type: text/html, Size: 543 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> > For those that prefer text, here's the numbers (in microseconds):
> > QMP/pmemsave: 77706
> > HMC/xp command: 92552
> > QMP/pmemaccess: 95
>

I completed a proof of concept implementation for doing this memory access
completely over QMP.  Basically, it works much like pmemsave except instead
of sending the data to a file it does a base64 encode and sends it back
over the QMP connection.  Interestingly, my testing shows that this is
slightly slower than the pmemsave option.  Running the same test as before
(virtual address translation), I get 88063 microsecs on average.  So I
don't believe this option is viable.


Look good. I believe QMP will be in between, and if it doesn't work as well,
> could you also try to use QEMU's char dev instead of limit this to unix
> socket?


I'm moving forward to try the Qemu chardev approach now.  I haven't working
much with this construct before, so any pointers are appreciated.  From
what I'm seeing, it looks like the user would create a chardev using one of
the QMP @chardev* commands?  The schema doesn't indicate that these
commands return the chardev id, which seems odd as I was then assuming that
one could obtain the id and pass this into the pmemaccess QMP command.
Thoughts?

-bryan

[-- Attachment #2: Type: text/html, Size: 1857 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Thu, 12/11 20:28, Bryan D. Payne wrote:
> >
> > > For those that prefer text, here's the numbers (in microseconds):
> > > QMP/pmemsave: 77706
> > > HMC/xp command: 92552
> > > QMP/pmemaccess: 95
> >
> 
> I completed a proof of concept implementation for doing this memory access
> completely over QMP.  Basically, it works much like pmemsave except instead
> of sending the data to a file it does a base64 encode and sends it back
> over the QMP connection.  Interestingly, my testing shows that this is
> slightly slower than the pmemsave option.  Running the same test as before
> (virtual address translation), I get 88063 microsecs on average.  So I
> don't believe this option is viable.

How did you setup your QMP, and how does the QMP transactions look like in your
test? Is it Unix domain socket? If the # of requests are in a high rate, I
think the overhead may come from QMP framework, mostly event loop
synchronization: QMP messages are not processed if the device emulation code is
running.

Also did you try to compress the memory data with zlib, to compensate for the
base64 encoding inflation?

> 
> 
> Look good. I believe QMP will be in between, and if it doesn't work as well,
> > could you also try to use QEMU's char dev instead of limit this to unix
> > socket?
> 
> 
> I'm moving forward to try the Qemu chardev approach now.  I haven't working
> much with this construct before, so any pointers are appreciated.  From
> what I'm seeing, it looks like the user would create a chardev using one of
> the QMP @chardev* commands?  The schema doesn't indicate that these
> commands return the chardev id, which seems odd as I was then assuming that
> one could obtain the id and pass this into the pmemaccess QMP command.
> Thoughts?

The chardev will be assigned an "id" by user, as the paremeter of chardev-add,
and later the user will pass id to pmemaccess.

In pmemaccess implementation, we will find the chardev by qemu_chr_find, and
read/write with qemu_chr_* functions. You can take examples from hw/char/* for
devices' usage.

Fam

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Eric Blake]

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

On 11/26/2014 01:27 PM, Bryan D. Payne wrote:
> This patch adds a new QMP command that sets up a domain socket. This
> socket can then be used for fast read/write access to the guest's
> physical memory. The key benefit to this system over existing solutions
> is speed. Using this patch, guest memory can be copied out at a rate of
> ~200MB/sec, depending on the hardware. Existing solutions only achieve
> a small fraction of this speed.
> 
> Signed-off-by: Bryan D. Payne <bdpayne@acm.org>

> +
> +##
> +# @pmemaccess
> +#
> +# This command enables access to guest physical memory using
> +# a simple protocol over a UNIX domain socket.
> +#
> +# @path Location to use for the UNIX domain socket
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'pmemaccess', 'data': { 'path': 'str' } }

In addition to Fam's review, I have a question - does this code properly
use qemu_open() so that I can use 'add-fd' to pass in a pre-opened
socket fd into fdset 1, then call pmemaccess with '/dev/fdset/1'?  If
not, can you please fix it to allow this usage?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> In addition to Fam's review, I have a question - does this code properly
> use qemu_open() so that I can use 'add-fd' to pass in a pre-opened
> socket fd into fdset 1, then call pmemaccess with '/dev/fdset/1'?  If
> not, can you please fix it to allow this usage?
>

I'm not familiar with this setup.  Could you point me to an example of
where this is done properly somewhere else in the Qemu code base?  Once I
figure this out, I'll be able to post the v3 patch.

Thanks,
-bryan

[-- Attachment #2: Type: text/html, Size: 779 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Eric Blake]

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

On 12/03/2014 03:07 PM, Bryan D. Payne wrote:
>>
>> In addition to Fam's review, I have a question - does this code properly
>> use qemu_open() so that I can use 'add-fd' to pass in a pre-opened
>> socket fd into fdset 1, then call pmemaccess with '/dev/fdset/1'?  If
>> not, can you please fix it to allow this usage?
>>
> 
> I'm not familiar with this setup.  Could you point me to an example of
> where this is done properly somewhere else in the Qemu code base?  Once I
> figure this out, I'll be able to post the v3 patch.

Off the top of my head, I know the -tpm command line options (related to
the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c
for that implementation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Off the top of my head, I know the -tpm command line options (related to
> the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c
> for that implementation.
>

Thanks, I'll check that out.

Cheers,
-bryan

[-- Attachment #2: Type: text/html, Size: 587 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

>
> Off the top of my head, I know the -tpm command line options (related to
> the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c
> for that implementation.


So now I do see what you are talking about.  But I don't think it applies
to this patch.  I'm not using qemu_open in this patch.

-bryan

[-- Attachment #2: Type: text/html, Size: 673 bytes --]

Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

[Eric Blake]

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

On 12/04/2014 11:40 AM, Bryan D. Payne wrote:
>>
>> Off the top of my head, I know the -tpm command line options (related to
>> the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c
>> for that implementation.
> 
> 
> So now I do see what you are talking about.  But I don't think it applies
> to this patch.  I'm not using qemu_open in this patch.

But my argument is that you SHOULD be using qemu_open, so that someone
can have the option of passing in a pre-opened fd rather than having
qemu open it.  It matters for cases like libvirt where qemu is running
under restricted privileges, and it is easier to have libvirt pass in a
pre-opened fd than to figure out how to give qemu the privileges it
needs to open the file itself.  Even if you don't think your feature
will be exposed by libvirt any time soon, it's still worth designing
things to be flexible from the beginning.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2] qmp: extend QMP to provide read/write access to physical memory

[Eric Blake]

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

On 11/26/2014 01:27 PM, Bryan D. Payne wrote:
> Thanks for the feedback Eric, I've updated the patch.
> 
> v2 changes:
> - added QMP command contract to qapi-schema.json
> - corrected some comments
> - rewired QMP command to use schema code

When sending a v2, it's best to send it as a new top-level thread,
instead of buried in-reply-to an existing thread.  Also, for a single
patch, a cover letter is not strictly necessary (the information you
gave here can instead be given after the --- separator of the
one-and-only patch email).  Cover letters are mandatory only for
multi-patch series.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2] qmp: extend QMP to provide read/write access to physical memory

[Bryan D. Payne]

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

Ok thanks for the advice, I'll adjust for v3.  This is (clearly!) my first
contribution to Qemu so I'm still learning how you guys operate.

Cheers,
-bryan

On Mon, Dec 1, 2014 at 2:12 PM, Eric Blake <eblake@redhat.com> wrote:

> On 11/26/2014 01:27 PM, Bryan D. Payne wrote:
> > Thanks for the feedback Eric, I've updated the patch.
> >
> > v2 changes:
> > - added QMP command contract to qapi-schema.json
> > - corrected some comments
> > - rewired QMP command to use schema code
>
> When sending a v2, it's best to send it as a new top-level thread,
> instead of buried in-reply-to an existing thread.  Also, for a single
> patch, a cover letter is not strictly necessary (the information you
> gave here can instead be given after the --- separator of the
> one-and-only patch email).  Cover letters are mandatory only for
> multi-patch series.
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>

[-- Attachment #2: Type: text/html, Size: 1598 bytes --]

Re: [Qemu-devel] [PATCH v2] qmp: extend QMP to provide read/write access to physical memory

[Fam Zheng]

On Mon, 12/01 20:36, Bryan D. Payne wrote:
> Ok thanks for the advice, I'll adjust for v3.  This is (clearly!) my first
> contribution to Qemu so I'm still learning how you guys operate.

Great. Looking forward to the next version. BTW please try to use inline
replying.

Fam

> 
> Cheers,
> -bryan
> 
> On Mon, Dec 1, 2014 at 2:12 PM, Eric Blake <eblake@redhat.com> wrote:
> 
> > On 11/26/2014 01:27 PM, Bryan D. Payne wrote:
> > > Thanks for the feedback Eric, I've updated the patch.
> > >
> > > v2 changes:
> > > - added QMP command contract to qapi-schema.json
> > > - corrected some comments
> > > - rewired QMP command to use schema code
> >
> > When sending a v2, it's best to send it as a new top-level thread,
> > instead of buried in-reply-to an existing thread.  Also, for a single
> > patch, a cover letter is not strictly necessary (the information you
> > gave here can instead be given after the --- separator of the
> > one-and-only patch email).  Cover letters are mandatory only for
> > multi-patch series.
> >
> > --
> > Eric Blake   eblake redhat com    +1-919-301-3266
> > Libvirt virtualization library http://libvirt.org
> >
> >