[Qemu-devel] [PATCH v5 0/6] Add memfd memory backend

[Qemu-devel] [PATCH v5 0/6] Add memfd memory backend

[Marc-André Lureau]

Add a new Linux-specific memory backend, similar to hostmem-file,
except that it doesn't need file path. It also try to enforce memory
sealing if available. It is thus slightly easier and secure, and is
compatible with transparent huge-pages since Linux 4.8.

Since Linux 4.14, memfd supports explicit hugetlb, however without
sealing.

v5:
- add memfd hugetlb support
- add seal and hugeltb options to memfd memory backend
- update qemu-option to explain sharing works with vhost-user for now
- rebased, misc code changes

v4:
- rebased, now that preliminary patches are merged

v3:
- make vhost-user-test use memfd only if possible
- rebased

v1->v2:
- make it linux-specific
- minor changes and commit message tweaks

Marc-André Lureau (6):
  memfd: split qemu_memfd_alloc()
  memfd: remove needless include
  memfd: add error argument, instead of perror()
  memfd: add hugetlb support
  Add memfd based hostmem
  tests: use memfd in vhost-user-test

 include/qemu/memfd.h     |   4 +-
 backends/hostmem-memfd.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio/vhost.c        |   8 ++-
 tests/vhost-user-test.c  |  70 ++++++++++++++++----------
 util/memfd.c             | 101 ++++++++++++++++++++++----------------
 backends/Makefile.objs   |   2 +
 qemu-options.hx          |  15 ++++++
 7 files changed, 256 insertions(+), 68 deletions(-)
 create mode 100644 backends/hostmem-memfd.c

-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 1/6] memfd: split qemu_memfd_alloc()

[Marc-André Lureau]

Add a function to only create a memfd, without mmap. The function is
used in the following memory backend.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/memfd.h |  1 +
 util/memfd.c         | 61 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
index 745a8c501e..41c24d807c 100644
--- a/include/qemu/memfd.h
+++ b/include/qemu/memfd.h
@@ -16,6 +16,7 @@
 #define F_SEAL_WRITE    0x0008  /* prevent writes */
 #endif
 
+int qemu_memfd_create(const char *name, size_t size, unsigned int seals);
 void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
                        int *fd);
 void qemu_memfd_free(void *ptr, size_t size, int fd);
diff --git a/util/memfd.c b/util/memfd.c
index 4571d1aba8..e9c1a0e700 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -55,6 +55,38 @@ static int memfd_create(const char *name, unsigned int flags)
 #define MFD_ALLOW_SEALING 0x0002U
 #endif
 
+int qemu_memfd_create(const char *name, size_t size, unsigned int seals)
+{
+    int mfd = -1;
+
+#ifdef CONFIG_LINUX
+    unsigned int flags = MFD_CLOEXEC;
+
+    if (seals) {
+        flags |= MFD_ALLOW_SEALING;
+    }
+
+    mfd = memfd_create(name, flags);
+    if (mfd < 0) {
+        return -1;
+    }
+
+    if (ftruncate(mfd, size) == -1) {
+        perror("ftruncate");
+        close(mfd);
+        return -1;
+    }
+
+    if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
+        perror("fcntl");
+        close(mfd);
+        return -1;
+    }
+#endif
+
+    return mfd;
+}
+
 /*
  * This is a best-effort helper for shared memory allocation, with
  * optional sealing. The helper will do his best to allocate using
@@ -65,35 +97,14 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
                        int *fd)
 {
     void *ptr;
-    int mfd = -1;
-
-    *fd = -1;
-
-#ifdef CONFIG_LINUX
-    if (seals) {
-        mfd = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
-    }
+    int mfd = qemu_memfd_create(name, size, seals);
 
+    /* some systems have memfd without sealing */
     if (mfd == -1) {
-        /* some systems have memfd without sealing */
-        mfd = memfd_create(name, MFD_CLOEXEC);
-        seals = 0;
+        mfd = qemu_memfd_create(name, size, 0);
     }
-#endif
-
-    if (mfd != -1) {
-        if (ftruncate(mfd, size) == -1) {
-            perror("ftruncate");
-            close(mfd);
-            return NULL;
-        }
 
-        if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
-            perror("fcntl");
-            close(mfd);
-            return NULL;
-        }
-    } else {
+    if (mfd == -1) {
         const char *tmpdir = g_get_tmp_dir();
         gchar *fname;
 
-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 2/6] memfd: remove needless include

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 util/memfd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/util/memfd.c b/util/memfd.c
index e9c1a0e700..b5b7a41347 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -27,8 +27,6 @@
 
 #include "qemu/osdep.h"
 
-#include <glib/gprintf.h>
-
 #include "qemu/memfd.h"
 
 #ifdef CONFIG_MEMFD
-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 3/6] memfd: add error argument, instead of perror()

[Marc-André Lureau]

This will allow callers to silence error report when the call is
allowed to failed.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/memfd.h |  5 +++--
 hw/virtio/vhost.c    |  8 +++++++-
 util/memfd.c         | 57 +++++++++++++++++++++++++++-------------------------
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
index 41c24d807c..b9d09873b5 100644
--- a/include/qemu/memfd.h
+++ b/include/qemu/memfd.h
@@ -16,9 +16,10 @@
 #define F_SEAL_WRITE    0x0008  /* prevent writes */
 #endif
 
-int qemu_memfd_create(const char *name, size_t size, unsigned int seals);
+int qemu_memfd_create(const char *name, size_t size, unsigned int seals,
+                      Error **errp);
 void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
-                       int *fd);
+                       int *fd, Error **errp);
 void qemu_memfd_free(void *ptr, size_t size, int fd);
 bool qemu_memfd_check(void);
 
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ddc42f0f93..63ec0e8315 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -329,6 +329,7 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev)
 
 static struct vhost_log *vhost_log_alloc(uint64_t size, bool share)
 {
+    Error *err = NULL;
     struct vhost_log *log;
     uint64_t logsize = size * sizeof(*(log->log));
     int fd = -1;
@@ -337,7 +338,12 @@ static struct vhost_log *vhost_log_alloc(uint64_t size, bool share)
     if (share) {
         log->log = qemu_memfd_alloc("vhost-log", logsize,
                                     F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
-                                    &fd);
+                                    &fd, &err);
+        if (err) {
+            error_report_err(err);
+            g_free(log);
+            return NULL;
+        }
         memset(log->log, 0, logsize);
     } else {
         log->log = g_malloc0(logsize);
diff --git a/util/memfd.c b/util/memfd.c
index b5b7a41347..f890976a0e 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -27,6 +27,7 @@
 
 #include "qemu/osdep.h"
 
+#include "qapi/error.h"
 #include "qemu/memfd.h"
 
 #ifdef CONFIG_MEMFD
@@ -53,11 +54,11 @@ static int memfd_create(const char *name, unsigned int flags)
 #define MFD_ALLOW_SEALING 0x0002U
 #endif
 
-int qemu_memfd_create(const char *name, size_t size, unsigned int seals)
+int qemu_memfd_create(const char *name, size_t size,
+                      unsigned int seals, Error **errp)
 {
-    int mfd = -1;
-
 #ifdef CONFIG_LINUX
+    int mfd = -1;
     unsigned int flags = MFD_CLOEXEC;
 
     if (seals) {
@@ -66,23 +67,26 @@ int qemu_memfd_create(const char *name, size_t size, unsigned int seals)
 
     mfd = memfd_create(name, flags);
     if (mfd < 0) {
-        return -1;
+        goto err;
     }
 
     if (ftruncate(mfd, size) == -1) {
-        perror("ftruncate");
-        close(mfd);
-        return -1;
+        goto err;
     }
 
     if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
-        perror("fcntl");
-        close(mfd);
-        return -1;
+        goto err;
     }
-#endif
 
     return mfd;
+
+err:
+    if (mfd >= 0) {
+        close(mfd);
+    }
+#endif
+    error_setg_errno(errp, errno, "failed to create memfd");
+    return -1;
 }
 
 /*
@@ -92,14 +96,14 @@ int qemu_memfd_create(const char *name, size_t size, unsigned int seals)
  * sealing.
  */
 void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
-                       int *fd)
+                       int *fd, Error **errp)
 {
     void *ptr;
-    int mfd = qemu_memfd_create(name, size, seals);
+    int mfd = qemu_memfd_create(name, size, seals, NULL);
 
     /* some systems have memfd without sealing */
     if (mfd == -1) {
-        mfd = qemu_memfd_create(name, size, 0);
+        mfd = qemu_memfd_create(name, size, 0, NULL);
     }
 
     if (mfd == -1) {
@@ -111,27 +115,26 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
         unlink(fname);
         g_free(fname);
 
-        if (mfd == -1) {
-            perror("mkstemp");
-            return NULL;
-        }
-
-        if (ftruncate(mfd, size) == -1) {
-            perror("ftruncate");
-            close(mfd);
-            return NULL;
+        if (mfd == -1 ||
+            ftruncate(mfd, size) == -1) {
+            goto err;
         }
     }
 
     ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
     if (ptr == MAP_FAILED) {
-        perror("mmap");
-        close(mfd);
-        return NULL;
+        goto err;
     }
 
     *fd = mfd;
     return ptr;
+
+err:
+    error_setg_errno(errp, errno, "failed to allocate shared memory");
+    if (mfd >= 0) {
+        close(mfd);
+    }
+    return NULL;
 }
 
 void qemu_memfd_free(void *ptr, size_t size, int fd)
@@ -159,7 +162,7 @@ bool qemu_memfd_check(void)
         int fd;
         void *ptr;
 
-        ptr = qemu_memfd_alloc("test", 4096, 0, &fd);
+        ptr = qemu_memfd_alloc("test", 4096, 0, &fd, NULL);
         memfd_check = ptr ? MEMFD_OK : MEMFD_KO;
         qemu_memfd_free(ptr, 4096, fd);
     }
-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 4/6] memfd: add hugetlb support

[Marc-André Lureau]

Linux commit 749df87bd7bee5a79cef073f5d032ddb2b211de8 (v4.14-rc1)
added a new flag MFD_HUGETLB to memfd_create() that specify the file
to be created resides in the hugetlbfs filesystem.  This is the
generic hugetlbfs filesystem not associated with any specific mount
point.

hugetlbfs does not support sealing operations, therefore specifying
MFD_ALLOW_SEALING with MFD_HUGETLB will result in EINVAL.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/memfd.h |  4 ++--
 util/memfd.c         | 13 ++++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
index b9d09873b5..1d3ecc7458 100644
--- a/include/qemu/memfd.h
+++ b/include/qemu/memfd.h
@@ -16,8 +16,8 @@
 #define F_SEAL_WRITE    0x0008  /* prevent writes */
 #endif
 
-int qemu_memfd_create(const char *name, size_t size, unsigned int seals,
-                      Error **errp);
+int qemu_memfd_create(const char *name, size_t size, bool hugetlb,
+                      unsigned int seals, Error **errp);
 void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
                        int *fd, Error **errp);
 void qemu_memfd_free(void *ptr, size_t size, int fd);
diff --git a/util/memfd.c b/util/memfd.c
index f890976a0e..98294e84de 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -54,7 +54,11 @@ static int memfd_create(const char *name, unsigned int flags)
 #define MFD_ALLOW_SEALING 0x0002U
 #endif
 
-int qemu_memfd_create(const char *name, size_t size,
+#ifndef MFD_HUGETLB
+#define MFD_HUGETLB 0x0004U
+#endif
+
+int qemu_memfd_create(const char *name, size_t size, bool hugetlb,
                       unsigned int seals, Error **errp)
 {
 #ifdef CONFIG_LINUX
@@ -64,6 +68,9 @@ int qemu_memfd_create(const char *name, size_t size,
     if (seals) {
         flags |= MFD_ALLOW_SEALING;
     }
+    if (hugetlb) {
+        flags |= MFD_HUGETLB;
+    }
 
     mfd = memfd_create(name, flags);
     if (mfd < 0) {
@@ -99,11 +106,11 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
                        int *fd, Error **errp)
 {
     void *ptr;
-    int mfd = qemu_memfd_create(name, size, seals, NULL);
+    int mfd = qemu_memfd_create(name, size, false, seals, NULL);
 
     /* some systems have memfd without sealing */
     if (mfd == -1) {
-        mfd = qemu_memfd_create(name, size, 0, NULL);
+        mfd = qemu_memfd_create(name, size, false, 0, NULL);
     }
 
     if (mfd == -1) {
-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 5/6] Add memfd based hostmem

[Marc-André Lureau]

Add a new memory backend, similar to hostmem-file, except that it
doesn't need to create files. It also enforces memory sealing.

This backend is mainly useful for sharing the memory with other
processes.

Note that Linux supports transparent huge-pages of shmem/memfd memory
since 4.8. It is relatively easier to set up THP than a dedicate
hugepage mount point by using "madvise" in
/sys/kernel/mm/transparent_hugepage/shmem_enabled.

Since 4.14, memfd allows to set hugetlb requirement explicitly.

Usage:
-object memory-backend-memfd,id=mem1,size=1G

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/hostmem-memfd.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/vhost-user-test.c  |   2 +-
 backends/Makefile.objs   |   2 +
 qemu-options.hx          |  15 ++++++
 4 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 backends/hostmem-memfd.c

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
new file mode 100644
index 0000000000..448591556a
--- /dev/null
+++ b/backends/hostmem-memfd.c
@@ -0,0 +1,124 @@
+/*
+ * QEMU host memfd memory backend
+ *
+ * Copyright (C) 2016 Red Hat Inc
+ *
+ * Authors:
+ *   Marc-André Lureau <marcandre.lureau@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/hostmem.h"
+#include "sysemu/sysemu.h"
+#include "qom/object_interfaces.h"
+#include "qemu/memfd.h"
+#include "qapi/error.h"
+
+#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
+
+#define MEMORY_BACKEND_MEMFD(obj)                                        \
+    OBJECT_CHECK(HostMemoryBackendMemfd, (obj), TYPE_MEMORY_BACKEND_MEMFD)
+
+typedef struct HostMemoryBackendMemfd HostMemoryBackendMemfd;
+
+struct HostMemoryBackendMemfd {
+    HostMemoryBackend parent_obj;
+
+    bool hugetlb;
+    bool seal;
+};
+
+static void
+memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
+{
+    HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(backend);
+    int fd;
+
+    if (!backend->size) {
+        error_setg(errp, "can't create backend with size 0");
+        return;
+    }
+
+    if (host_memory_backend_mr_inited(backend)) {
+        return;
+    }
+
+    backend->force_prealloc = mem_prealloc;
+    fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
+                           m->hugetlb, m->seal ?
+                           F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL : 0,
+                           errp);
+    if (fd == -1) {
+        return;
+    }
+
+    memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
+                                   object_get_canonical_path(OBJECT(backend)),
+                                   backend->size, true, fd, errp);
+}
+
+static bool memfd_backend_get_hugetlb(Object *o, Error **errp)
+{
+    return MEMORY_BACKEND_MEMFD(o)->hugetlb;
+}
+
+static void memfd_backend_set_hugetlb(Object *o, bool value,
+                                                 Error **errp)
+{
+    MEMORY_BACKEND_MEMFD(o)->hugetlb = value;
+}
+
+static bool memfd_backend_get_seal(Object *o, Error **errp)
+{
+    return MEMORY_BACKEND_MEMFD(o)->seal;
+}
+
+static void memfd_backend_set_seal(Object *o, bool value,
+                                   Error **errp)
+{
+    MEMORY_BACKEND_MEMFD(o)->seal = value;
+}
+
+static void
+memfd_backend_instance_init(Object *obj)
+{
+    HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(obj);
+
+    /* default to sealed file */
+    m->seal = true;
+}
+
+static void
+memfd_backend_class_init(ObjectClass *oc, void *data)
+{
+    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+    bc->alloc = memfd_backend_memory_alloc;
+
+    object_class_property_add_bool(oc, "hugetlb",
+                                   memfd_backend_get_hugetlb,
+                                   memfd_backend_set_hugetlb,
+                                   &error_abort);
+    object_class_property_add_bool(oc, "seal",
+                                   memfd_backend_get_seal,
+                                   memfd_backend_set_seal,
+                                   &error_abort);
+}
+
+static const TypeInfo memfd_backend_info = {
+    .name = TYPE_MEMORY_BACKEND_MEMFD,
+    .parent = TYPE_MEMORY_BACKEND,
+    .instance_init = memfd_backend_instance_init,
+    .class_init = memfd_backend_class_init,
+    .instance_size = sizeof(HostMemoryBackendMemfd),
+};
+
+static void register_types(void)
+{
+    type_register_static(&memfd_backend_info);
+}
+
+type_init(register_types);
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 4b98018478..830f1e4026 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -31,7 +31,7 @@
 #include <linux/virtio_net.h>
 #include <sys/vfs.h>
 
-#define VHOST_USER_NET_TESTS_WORKING 0 /* broken as of 2.10.0 */
+#define VHOST_USER_NET_TESTS_WORKING 1 /* broken as of 2.10.0 */
 
 /* GLIB version compatibility flags */
 #if !GLIB_CHECK_VERSION(2, 26, 0)
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 0400799efd..67eeeba5fc 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -8,3 +8,5 @@ common-obj-$(CONFIG_LINUX) += hostmem-file.o
 
 common-obj-y += cryptodev.o
 common-obj-y += cryptodev-builtin.o
+
+common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
diff --git a/qemu-options.hx b/qemu-options.hx
index 3728e9b4dd..5828caefeb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4208,6 +4208,21 @@ that @option{discard-data} is only an optimization, and QEMU
 might not discard file contents if it aborts unexpectedly or is
 terminated using SIGKILL.
 
+@item -object memory-backend-memfd,id=@var{id},size=@var{size},seal=@var{on|off},hugetlb=@var{on|off}
+
+Creates an anonymous memory file backend object, which allows QEMU to
+share the memory with an external process in some cases (e.g. when
+using vhost-user). The memory is allocated with memfd and optional
+sealing. (Linux only)
+
+The @option{id} parameter is a unique ID that will be used to
+reference this memory region when configuring the @option{-numa}
+argument. The @option{size} option provides the size of the memory
+region, and accepts common suffixes, eg @option{500M}. The
+@option{seal} option creates a sealed-file, that will block further
+resizing the memory ('on' by default). The @option{hugetlb} option
+specify the file to be created resides in the hugetlbfs filesystem.
+
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
 Creates a random number generator backend which obtains entropy from
-- 
2.15.0.rc0.40.gaefcc5f6f

[Qemu-devel] [PATCH v5 6/6] tests: use memfd in vhost-user-test

[Marc-André Lureau]

This will exercise the memfd memory backend and should generally be
better for testing than memory-backend-file (thanks to anonymous files
and sealing).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/vhost-user-test.c | 68 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 830f1e4026..58682f0871 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -17,6 +17,7 @@
 #include "qemu/range.h"
 #include "qemu/sockets.h"
 #include "chardev/char-fe.h"
+#include "qemu/memfd.h"
 #include "sysemu/sysemu.h"
 #include "libqos/libqos.h"
 #include "libqos/pci-pc.h"
@@ -42,15 +43,14 @@
 #define HAVE_MONOTONIC_TIME
 #endif
 
-#define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
+#define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM," \
                         "mem-path=%s,share=on -numa node,memdev=mem"
+#define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
+                        " -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0"
 
-#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
-                        QEMU_CMD_NETDEV QEMU_CMD_NET
-
 #define HUGETLBFS_MAGIC       0x958458f6
 
 /*********** FROM hw/virtio/vhost-user.c *************************************/
@@ -161,6 +161,22 @@ typedef struct TestServer {
 static const char *tmpfs;
 static const char *root;
 
+static char *get_qemu_cmd(TestServer *s, int mem, const char *mem_path,
+                          const char *chr_opts, const char *extra)
+{
+    if (qemu_memfd_check()) {
+        return g_strdup_printf(QEMU_CMD_MEMFD QEMU_CMD_CHR
+                               QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+                               s->chr_name, s->socket_path,
+                               chr_opts, s->chr_name, extra);
+    } else {
+        return g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR
+                               QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+                               mem_path, s->chr_name, s->socket_path,
+                               chr_opts, s->chr_name, extra);
+    }
+}
+
 static void init_virtio_dev(TestServer *s)
 {
     QVirtioPCIDevice *dev;
@@ -474,14 +490,6 @@ static void test_server_listen(TestServer *server)
     test_server_create_chr(server, ",server,nowait");
 }
 
-#define GET_QEMU_CMD(s)                                         \
-    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
-                    (s)->socket_path, "", (s)->chr_name)
-
-#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...)                     \
-    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \
-                    (s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__)
-
 static gboolean _test_server_free(TestServer *server)
 {
     int i;
@@ -625,7 +633,7 @@ static void test_migrate(void)
     char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
     QTestState *global = global_qtest, *from, *to;
     GSource *source;
-    gchar *cmd;
+    gchar *cmd, *tmp;
     QDict *rsp;
     guint8 *log;
     guint64 size;
@@ -633,7 +641,7 @@ static void test_migrate(void)
     test_server_listen(s);
     test_server_listen(dest);
 
-    cmd = GET_QEMU_CMDE(s, 2, "", "");
+    cmd = get_qemu_cmd(s, 2, root, "", "");
     from = qtest_start(cmd);
     g_free(cmd);
 
@@ -642,7 +650,9 @@ static void test_migrate(void)
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
 
-    cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri);
+    tmp = g_strdup_printf(" -incoming %s", uri);
+    cmd = get_qemu_cmd(dest, 2, root, "", tmp);
+    g_free(tmp);
     to = qtest_init(cmd);
     g_free(cmd);
 
@@ -753,7 +763,7 @@ static void test_reconnect_subprocess(void)
     char *cmd;
 
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -789,7 +799,7 @@ static void test_connect_fail_subprocess(void)
 
     s->test_fail = true;
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -817,7 +827,7 @@ static void test_flags_mismatch_subprocess(void)
 
     s->test_flags = TEST_FLAGS_DISCONNECT;
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -885,11 +895,21 @@ static void test_multiqueue(void)
     s->queues = queues;
     test_server_listen(s);
 
-    cmd = g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
-                          "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
-                          512, 512, root, s->chr_name,
-                          s->socket_path, "", s->chr_name,
-                          queues, queues * 2 + 2);
+    if (qemu_memfd_check()) {
+        cmd = g_strdup_printf(
+            QEMU_CMD_MEMFD QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
+            "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
+            512, 512, s->chr_name,
+            s->socket_path, "", s->chr_name,
+            queues, queues * 2 + 2);
+    } else {
+        cmd = g_strdup_printf(
+            QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
+            "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
+            512, 512, root, s->chr_name,
+            s->socket_path, "", s->chr_name,
+            queues, queues * 2 + 2);
+    }
     qtest_start(cmd);
     g_free(cmd);
 
@@ -955,7 +975,7 @@ int main(int argc, char **argv)
     /* run the main loop thread so the chardev may operate */
     thread = g_thread_new(NULL, thread_function, loop);
 
-    qemu_cmd = GET_QEMU_CMD(server);
+    qemu_cmd = get_qemu_cmd(server, 512, root, "", "");
 
     s = qtest_start(qemu_cmd);
     g_free(qemu_cmd);
-- 
2.15.0.rc0.40.gaefcc5f6f

Re: [Qemu-devel] [PATCH v5 4/6] memfd: add hugetlb support

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 10:59:08AM +0100, Marc-André Lureau wrote:
> Linux commit 749df87bd7bee5a79cef073f5d032ddb2b211de8 (v4.14-rc1)
> added a new flag MFD_HUGETLB to memfd_create() that specify the file
> to be created resides in the hugetlbfs filesystem.  This is the
> generic hugetlbfs filesystem not associated with any specific mount
> point.

How do you know and/or control what size huge pages are used,
when the platform supports many sizes ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 6/6] tests: use memfd in vhost-user-test

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 10:59:10AM +0100, Marc-André Lureau wrote:
> This will exercise the memfd memory backend and should generally be
> better for testing than memory-backend-file (thanks to anonymous files
> and sealing).

Since we'll support vhost-user with both backends, it would be preferrable
to test both backends so we don't loose test coverage of the memory-backend-file
which is what's actually used in practice right now.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 2/6] memfd: remove needless include

[Philippe Mathieu-Daudé]

On 10/23/2017 06:59 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  util/memfd.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/util/memfd.c b/util/memfd.c
> index e9c1a0e700..b5b7a41347 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -27,8 +27,6 @@
>  
>  #include "qemu/osdep.h"
>  
> -#include <glib/gprintf.h>
> -
>  #include "qemu/memfd.h"
>  
>  #ifdef CONFIG_MEMFD
> 

Re: [Qemu-devel] [PATCH v5 1/6] memfd: split qemu_memfd_alloc()

[Philippe Mathieu-Daudé]

On 10/23/2017 06:59 AM, Marc-André Lureau wrote:
> Add a function to only create a memfd, without mmap. The function is
> used in the following memory backend.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/memfd.h |  1 +
>  util/memfd.c         | 61 +++++++++++++++++++++++++++++++---------------------
>  2 files changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
> index 745a8c501e..41c24d807c 100644
> --- a/include/qemu/memfd.h
> +++ b/include/qemu/memfd.h
> @@ -16,6 +16,7 @@
>  #define F_SEAL_WRITE    0x0008  /* prevent writes */
>  #endif
>  
> +int qemu_memfd_create(const char *name, size_t size, unsigned int seals);
>  void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
>                         int *fd);
>  void qemu_memfd_free(void *ptr, size_t size, int fd);
> diff --git a/util/memfd.c b/util/memfd.c
> index 4571d1aba8..e9c1a0e700 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -55,6 +55,38 @@ static int memfd_create(const char *name, unsigned int flags)
>  #define MFD_ALLOW_SEALING 0x0002U
>  #endif
>  
> +int qemu_memfd_create(const char *name, size_t size, unsigned int seals)
> +{
> +    int mfd = -1;
> +
> +#ifdef CONFIG_LINUX
> +    unsigned int flags = MFD_CLOEXEC;
> +
> +    if (seals) {
> +        flags |= MFD_ALLOW_SEALING;

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +    }
> +
> +    mfd = memfd_create(name, flags);
> +    if (mfd < 0) {
> +        return -1;
> +    }
> +
> +    if (ftruncate(mfd, size) == -1) {
> +        perror("ftruncate");
> +        close(mfd);
> +        return -1;
> +    }
> +
> +    if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
> +        perror("fcntl");
> +        close(mfd);
> +        return -1;
> +    }
> +#endif
> +
> +    return mfd;
> +}
> +
>  /*
>   * This is a best-effort helper for shared memory allocation, with
>   * optional sealing. The helper will do his best to allocate using
> @@ -65,35 +97,14 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
>                         int *fd)
>  {
>      void *ptr;
> -    int mfd = -1;
> -
> -    *fd = -1;
> -
> -#ifdef CONFIG_LINUX
> -    if (seals) {
> -        mfd = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
> -    }
> +    int mfd = qemu_memfd_create(name, size, seals);
>  
> +    /* some systems have memfd without sealing */
>      if (mfd == -1) {
> -        /* some systems have memfd without sealing */
> -        mfd = memfd_create(name, MFD_CLOEXEC);
> -        seals = 0;
> +        mfd = qemu_memfd_create(name, size, 0);
>      }
> -#endif
> -
> -    if (mfd != -1) {
> -        if (ftruncate(mfd, size) == -1) {
> -            perror("ftruncate");
> -            close(mfd);
> -            return NULL;
> -        }
>  
> -        if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
> -            perror("fcntl");
> -            close(mfd);
> -            return NULL;
> -        }
> -    } else {
> +    if (mfd == -1) {
>          const char *tmpdir = g_get_tmp_dir();
>          gchar *fname;
>  
> 

Re: [Qemu-devel] [PATCH v5 4/6] memfd: add hugetlb support

[Marc-André Lureau]

Hi

----- Original Message -----
> On Mon, Oct 23, 2017 at 10:59:08AM +0100, Marc-André Lureau wrote:
> > Linux commit 749df87bd7bee5a79cef073f5d032ddb2b211de8 (v4.14-rc1)
> > added a new flag MFD_HUGETLB to memfd_create() that specify the file
> > to be created resides in the hugetlbfs filesystem.  This is the
> > generic hugetlbfs filesystem not associated with any specific mount
> > point.
> 
> How do you know and/or control what size huge pages are used,
> when the platform supports many sizes ?

>From linux commit message:
"As with other system calls that request hugetlbfs backed pages, there is
the ability to encode huge page size in the flag arguments."

I didn't add this option to memfd backend, as I don't know how generally useful that is, and if there is already a similar option in qemu (probably not?). It could be easily added later.

> 
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange
> |:|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com
> |:|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange
> |:|
> 

Re: [Qemu-devel] [PATCH v5 4/6] memfd: add hugetlb support

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 07:27:31AM -0400, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > On Mon, Oct 23, 2017 at 10:59:08AM +0100, Marc-André Lureau wrote:
> > > Linux commit 749df87bd7bee5a79cef073f5d032ddb2b211de8 (v4.14-rc1)
> > > added a new flag MFD_HUGETLB to memfd_create() that specify the file
> > > to be created resides in the hugetlbfs filesystem.  This is the
> > > generic hugetlbfs filesystem not associated with any specific mount
> > > point.
> > 
> > How do you know and/or control what size huge pages are used,
> > when the platform supports many sizes ?
> 
> From linux commit message:
> "As with other system calls that request hugetlbfs backed pages, there is
> the ability to encode huge page size in the flag arguments."
> 
> I didn't add this option to memfd backend, as I don't know how generally
> useful that is, and if there is already a similar option in qemu (probably
> not?). It could be easily added later.

For the existing  memory-backend-file, libvirt controls hugepage size
based on what's requested in guest XML. So if we were to support the
new memory-backend-memfd as an alternative, I think we need to be able
to choose huge page size for that too.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 5/6] Add memfd based hostmem

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 10:59:09AM +0100, Marc-André Lureau wrote:
> Add a new memory backend, similar to hostmem-file, except that it
> doesn't need to create files. It also enforces memory sealing.
> 
> This backend is mainly useful for sharing the memory with other
> processes.
> 
> Note that Linux supports transparent huge-pages of shmem/memfd memory
> since 4.8. It is relatively easier to set up THP than a dedicate
> hugepage mount point by using "madvise" in
> /sys/kernel/mm/transparent_hugepage/shmem_enabled.
> 
> Since 4.14, memfd allows to set hugetlb requirement explicitly.
> 
> Usage:
> -object memory-backend-memfd,id=mem1,size=1G

[snip]

> +static void
> +memfd_backend_class_init(ObjectClass *oc, void *data)
> +{
> +    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> +
> +    bc->alloc = memfd_backend_memory_alloc;
> +
> +    object_class_property_add_bool(oc, "hugetlb",
> +                                   memfd_backend_get_hugetlb,
> +                                   memfd_backend_set_hugetlb,
> +                                   &error_abort);

I tend to think that instead of a bool hugetlb, we should take an
integer page size instead eg  hugepagesize=2M instead of hugetlb=true



> diff --git a/qemu-options.hx b/qemu-options.hx
> index 3728e9b4dd..5828caefeb 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4208,6 +4208,21 @@ that @option{discard-data} is only an optimization, and QEMU
>  might not discard file contents if it aborts unexpectedly or is
>  terminated using SIGKILL.
>  
> +@item -object memory-backend-memfd,id=@var{id},size=@var{size},seal=@var{on|off},hugetlb=@var{on|off}
> +
> +Creates an anonymous memory file backend object, which allows QEMU to
> +share the memory with an external process in some cases (e.g. when
> +using vhost-user). The memory is allocated with memfd and optional
> +sealing. (Linux only)
> +
> +The @option{id} parameter is a unique ID that will be used to
> +reference this memory region when configuring the @option{-numa}
> +argument. The @option{size} option provides the size of the memory
> +region, and accepts common suffixes, eg @option{500M}. The
> +@option{seal} option creates a sealed-file, that will block further
> +resizing the memory ('on' by default). The @option{hugetlb} option
> +specify the file to be created resides in the hugetlbfs filesystem.

This should document that you can't combine sealing and huge pages

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 5/6] Add memfd based hostmem

[Marc-André Lureau]

----- Original Message -----
> On Mon, Oct 23, 2017 at 10:59:09AM +0100, Marc-André Lureau wrote:
> > Add a new memory backend, similar to hostmem-file, except that it
> > doesn't need to create files. It also enforces memory sealing.
> > 
> > This backend is mainly useful for sharing the memory with other
> > processes.
> > 
> > Note that Linux supports transparent huge-pages of shmem/memfd memory
> > since 4.8. It is relatively easier to set up THP than a dedicate
> > hugepage mount point by using "madvise" in
> > /sys/kernel/mm/transparent_hugepage/shmem_enabled.
> > 
> > Since 4.14, memfd allows to set hugetlb requirement explicitly.
> > 
> > Usage:
> > -object memory-backend-memfd,id=mem1,size=1G
> 
> [snip]
> 
> > +static void
> > +memfd_backend_class_init(ObjectClass *oc, void *data)
> > +{
> > +    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> > +
> > +    bc->alloc = memfd_backend_memory_alloc;
> > +
> > +    object_class_property_add_bool(oc, "hugetlb",
> > +                                   memfd_backend_get_hugetlb,
> > +                                   memfd_backend_set_hugetlb,
> > +                                   &error_abort);
> 
> I tend to think that instead of a bool hugetlb, we should take an
> integer page size instead eg  hugepagesize=2M instead of hugetlb=true
> 

Well, how would you then create a memfd without explicit hugetlb request or one with default hugetlb size?

I think size should be a different property. I'll add it.

> 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 3728e9b4dd..5828caefeb 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -4208,6 +4208,21 @@ that @option{discard-data} is only an optimization,
> > and QEMU
> >  might not discard file contents if it aborts unexpectedly or is
> >  terminated using SIGKILL.
> >  
> > +@item -object
> > memory-backend-memfd,id=@var{id},size=@var{size},seal=@var{on|off},hugetlb=@var{on|off}
> > +
> > +Creates an anonymous memory file backend object, which allows QEMU to
> > +share the memory with an external process in some cases (e.g. when
> > +using vhost-user). The memory is allocated with memfd and optional
> > +sealing. (Linux only)
> > +
> > +The @option{id} parameter is a unique ID that will be used to
> > +reference this memory region when configuring the @option{-numa}
> > +argument. The @option{size} option provides the size of the memory
> > +region, and accepts common suffixes, eg @option{500M}. The
> > +@option{seal} option creates a sealed-file, that will block further
> > +resizing the memory ('on' by default). The @option{hugetlb} option
> > +specify the file to be created resides in the hugetlbfs filesystem.
> 
> This should document that you can't combine sealing and huge pages
> 

Ok, I am wondering if this is a temporary limitation, I have asked the patch author about it (it looks possible to me, I'll eventually try implementing it)

Re: [Qemu-devel] [PATCH v5 5/6] Add memfd based hostmem

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 07:41:57AM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Original Message -----
> > On Mon, Oct 23, 2017 at 10:59:09AM +0100, Marc-André Lureau wrote:
> > > Add a new memory backend, similar to hostmem-file, except that it
> > > doesn't need to create files. It also enforces memory sealing.
> > > 
> > > This backend is mainly useful for sharing the memory with other
> > > processes.
> > > 
> > > Note that Linux supports transparent huge-pages of shmem/memfd memory
> > > since 4.8. It is relatively easier to set up THP than a dedicate
> > > hugepage mount point by using "madvise" in
> > > /sys/kernel/mm/transparent_hugepage/shmem_enabled.
> > > 
> > > Since 4.14, memfd allows to set hugetlb requirement explicitly.
> > > 
> > > Usage:
> > > -object memory-backend-memfd,id=mem1,size=1G
> > 
> > [snip]
> > 
> > > +static void
> > > +memfd_backend_class_init(ObjectClass *oc, void *data)
> > > +{
> > > +    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> > > +
> > > +    bc->alloc = memfd_backend_memory_alloc;
> > > +
> > > +    object_class_property_add_bool(oc, "hugetlb",
> > > +                                   memfd_backend_get_hugetlb,
> > > +                                   memfd_backend_set_hugetlb,
> > > +                                   &error_abort);
> > 
> > I tend to think that instead of a bool hugetlb, we should take an
> > integer page size instead eg  hugepagesize=2M instead of hugetlb=true
> > 
> 
> Well, how would you then create a memfd without explicit hugetlb
> request or one with default hugetlb size?

Hmm, yes, libvirt would not need that, but that would be useful still for
direct QEMU users.

> I think size should be a different property. I'll add it.

Yep ok

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|