postcopy+vhost-user/shared ram

From: "Dr. David Alan Gilbert" <[email protected]>

Add a vu_queue_started method to complement vu_queue_enabled.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 6 ++++++
 contrib/libvhost-user/libvhost-user.h | 9 +++++++++
 2 files changed, 15 insertions(+)

contrib/libvhost-user/libvhost-user.c +0 -0

index 35fa0c5e56..201b9846e9 100644

contrib/libvhost-user/libvhost-user.c +4 -0

+{
+    return vq->started;
+}
+
 static inline uint16_t
 vring_avail_flags(VuVirtq *vq)

{

contrib/libvhost-user/libvhost-user.h +9 -0

index 53ef222c0b..acd019876d 100644
@@ -328,6 +328,15 @@ void vu_queue_set_notification(VuDev *dev, VuVirtq
 *vq, int enable);
 bool vu_queue_enabled(VuDev *dev, VuVirtq *vq);
 
 /**
+ * vu_queue_started:
+ * @dev: a VuDev context
+ * @vq: a VuVirtq queue
+ *
+ * Returns: whether the queue is started.
+ */
+bool vu_queue_started(VuDev *dev, VuVirtq *vq);
+
+/**
  * vu_queue_empty:
  * @dev: a VuDev context

* @vq: a VuVirtq queue
--
2.13.5

--

Hi

On Thu, Aug 24, 2017 at 9:39 PM Dr. David Alan Gilbert (git) < [email protected]> wrote:

Quoted 6 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Add a vu_queue_started method to complement vu_queue_enabled.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

Reviewed-by Marc-André Lureau <[email protected]>

Quoted 15 lines

---
 contrib/libvhost-user/libvhost-user.c | 6 ++++++
 contrib/libvhost-user/libvhost-user.h | 9 +++++++++
 2 files changed, 15 insertions(+)

contrib/libvhost-user/libvhost-user.c +0 -0

index 35fa0c5e56..201b9846e9 100644
@@ -930,6 +930,12 @@ vu_queue_enabled(VuDev *dev, VuVirtq *vq)
     return vq->enable;
 }
 

contrib/libvhost-user/libvhost-user.c vu_queue_enabled(VuDev *dev, VuVirtq *vq)

Quoted 3 lines

contrib/libvhost-user/libvhost-user.c +2 -0

+bool
+vu_queue_started(VuDev *dev, VuVirtq *vq)
 

I guess we could make it const, but this is true for many other functions. Could be done later in one go.

Quoted 33 lines

contrib/libvhost-user/libvhost-user.c +4 -0

+{
+    return vq->started;
+}
+
 static inline uint16_t
 vring_avail_flags(VuVirtq *vq)

{

contrib/libvhost-user/libvhost-user.h +9 -0

index 53ef222c0b..acd019876d 100644
@@ -328,6 +328,15 @@ void vu_queue_set_notification(VuDev *dev, VuVirtq
 *vq, int enable);
 bool vu_queue_enabled(VuDev *dev, VuVirtq *vq);
 
 /**
+ * vu_queue_started:
+ * @dev: a VuDev context
+ * @vq: a VuVirtq queue
+ *
+ * Returns: whether the queue is started.
+ */
+bool vu_queue_started(VuDev *dev, VuVirtq *vq);
+
+/**
  * vu_queue_empty:
  * @dev: a VuDev context

* @vq: a VuVirtq queue
--
2.13.5

Quoted 1 line

--

Marc-André Lureau

Subject: Re: [RFC v2 01/32] vhu: vu_queue_started

Quoted 1 line

Subject: Re: [RFC v2 01/32] vhu: vu_queue_started

I mean, how did we end up with "vhu"? I never meant that to happen :( It's merely in the commit title but maybe we can come up with a better-sounding way to abbreviate vhost user.

Hi

----- Original Message -----

Quoted 5 lines

Quoted 1 line

Subject: Re: [RFC v2 01/32] vhu: vu_queue_started

I mean, how did we end up with "vhu"? I never meant that to happen :( It's merely in the commit title but maybe we can come up with a better-sounding way to abbreviate vhost user.

vhu, no idea :)

I would suggest to use libvhost-user: prefix in commit messages for contrib/libvhost-user changes.

vhost-user: for qemu vhost-user changes.

vhost-user-bridge: (or vub:), vhost-user-scsi: (or vus:) for those backends.

Quoted 9 lines · * Marc-André Lureau ([email protected]) wrote:

Hi

----- Original Message -----

Quoted 5 lines

Quoted 1 line

Subject: Re: [RFC v2 01/32] vhu: vu_queue_started

I mean, how did we end up with "vhu"? I never meant that to happen :( It's merely in the commit title but maybe we can come up with a better-sounding way to abbreviate vhost user.

Quoted 5 lines

vhu, no idea :)

I would suggest to use libvhost-user: prefix in commit messages for contrib/libvhost-user changes.

vhost-user: for qemu vhost-user changes.

Changed to vhost-user.

Dave

Quoted 1 line

vhost-user-bridge: (or vub:), vhost-user-scsi: (or vus:) for those backends.

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

From: "Dr. David Alan Gilbert" <[email protected]>

Only process received packets if the queue has been started.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 tests/vhost-user-bridge.c | 1 +
 1 file changed, 1 insertion(+)

tests/vhost-user-bridge.c +0 -0

index 1e5b5ca3da..324abee53d 100644

From: "Dr. David Alan Gilbert" <[email protected]>

The choice of call to discard a block is getting more complicated
for other cases. We use fallocate PUNCH_HOLE in any file cases;
it works for both hugepage and for tmpfs.
We use the DONTNEED for non-hugepage cases either where they're anonymous or where they're private.

Care should be taken when trying other backing files.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 exec.c       | 35 ++++++++++++++++++++++++-----------
 trace-events |  3 +++
 2 files changed, 27 insertions(+), 11 deletions(-)

exec.c +9 -11

index d20c34ca83..67df2909ce 100644
@@ -3573,6 +3573,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
     }
 
     if ((start + length) <= rb->used_length) {
+        bool need_madvise, need_fallocate;
         uint8_t *host_endaddr = host_startaddr + length;
         if ((uintptr_t)host_endaddr & (rb->page_size - 1)) {
             error_report("ram_block_discard_range: Unaligned end address: %p",

@@ -3582,23 +3583,35 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
 
         errno = ENOTSUP; /* If we are missing MADVISE etc */
 
-        if (rb->page_size == qemu_host_page_size) {
-#if defined(CONFIG_MADVISE)
-            /* Note: We need the madvise MADV_DONTNEED behaviour of definitely
-             * freeing the page.
-             */
-            ret = madvise(host_startaddr, length, MADV_DONTNEED);
-#endif
-        } else {
-            /* Huge page case  - unfortunately it can't do DONTNEED, but
-             * it can do the equivalent by FALLOC_FL_PUNCH_HOLE in the
-             * huge page file.
+        /* The logic here is messy;
+         *    madvise DONTNEED fails for hugepages
+         *    fallocate works on hugepages and shmem
+         */
+        need_madvise = (rb->page_size == qemu_host_page_size);
+        need_fallocate = rb->fd != -1;
+        if (need_fallocate) {
+            /* For a file, this causes the area of the file to be zero'd

exec.c +11 -0

+            /* For normal RAM this causes it to be unmapped,
+             * for shared memory it causes the local mapping to disappear
+             * and to fall back on the file contents (which we just
+             * fallocate'd away).
+             */
+#if defined(CONFIG_MADVISE)
+            ret =  madvise(host_startaddr, length, MADV_DONTNEED);
+#endif
+        }
+        trace_ram_block_discard_range(rb->idstr, host_startaddr,
+                                      need_madvise, need_fallocate, ret);
         if (ret) {
             ret = -errno;
             error_report("ram_block_discard_range: Failed to discard range "

trace-events +3 -0

index 1f50f56d9d..213ee34f89 100644
@@ -55,6 +55,9 @@ dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d cb=%p"
 dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d"
 dma_map_wait(void *dbs) "dbs=%p"
 
+# exec.c
+ram_block_discard_range(const char *rbname, void *hva, bool need_madvise, bool need_fallocate, int ret) "%s@%p: madvise: %d fallocate: %d ret: %d"
+
 # memory.c
 memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

Signature · 1 line

2.13.5

Quoted 51 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

The choice of call to discard a block is getting more complicated
for other cases. We use fallocate PUNCH_HOLE in any file cases;
it works for both hugepage and for tmpfs.
We use the DONTNEED for non-hugepage cases either where they're anonymous or where they're private.

Care should be taken when trying other backing files.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 exec.c       | 35 ++++++++++++++++++++++++-----------
 trace-events |  3 +++
 2 files changed, 27 insertions(+), 11 deletions(-)

exec.c +9 -11

index d20c34ca83..67df2909ce 100644
@@ -3573,6 +3573,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
     }
 
     if ((start + length) <= rb->used_length) {
+        bool need_madvise, need_fallocate;
         uint8_t *host_endaddr = host_startaddr + length;
         if ((uintptr_t)host_endaddr & (rb->page_size - 1)) {
             error_report("ram_block_discard_range: Unaligned end address: %p",

@@ -3582,23 +3583,35 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
 
         errno = ENOTSUP; /* If we are missing MADVISE etc */
 
-        if (rb->page_size == qemu_host_page_size) {
-#if defined(CONFIG_MADVISE)
-            /* Note: We need the madvise MADV_DONTNEED behaviour of definitely
-             * freeing the page.
-             */
-            ret = madvise(host_startaddr, length, MADV_DONTNEED);
-#endif
-        } else {
-            /* Huge page case  - unfortunately it can't do DONTNEED, but
-             * it can do the equivalent by FALLOC_FL_PUNCH_HOLE in the
-             * huge page file.
+        /* The logic here is messy;
+         *    madvise DONTNEED fails for hugepages
+         *    fallocate works on hugepages and shmem
+         */
+        need_madvise = (rb->page_size == qemu_host_page_size);
+        need_fallocate = rb->fd != -1;
+        if (need_fallocate) {
+            /* For a file, this causes the area of the file to be zero'd

exec.c int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) · line 3603

Quoted 10 lines

exec.c +4 -0

+             * if read, and for hugetlbfs also causes it to be unmapped
+             * so a userfault will trigger.
              */
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
             ret = fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
                             start, length);
 #endif
         }
+        /* i.e. need madvise but skip it if the fallocate failed */
+        if (need_madvise && (!need_fallocate || (ret == 0))) {

I'll slightly prefer:

  trace_ram_block_discard_range();

  if (need_fallocate) {
    ret = fallocate();
    if (ret) {
      error_report();
      goto err;
    }
  }

  if (need_madvise) {
    ret = madvise();
    if (ret) {
      error_report();
      goto err;
    }
  }

But it is personal preference. For either way:

Reviewed-by Peter Xu <[email protected]>

Quoted 22 lines

exec.c +11 -0

+            /* For normal RAM this causes it to be unmapped,
+             * for shared memory it causes the local mapping to disappear
+             * and to fall back on the file contents (which we just
+             * fallocate'd away).
+             */
+#if defined(CONFIG_MADVISE)
+            ret =  madvise(host_startaddr, length, MADV_DONTNEED);
+#endif
+        }
+        trace_ram_block_discard_range(rb->idstr, host_startaddr,
+                                      need_madvise, need_fallocate, ret);
         if (ret) {
             ret = -errno;
             error_report("ram_block_discard_range: Failed to discard range "

trace-events +0 -0

index 1f50f56d9d..213ee34f89 100644
@@ -55,6 +55,9 @@ dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d cb=%p"
 dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d"
 dma_map_wait(void *dbs) "dbs=%p"
 

Quoted 9 lines

trace-events +3 -0

+# exec.c
+ram_block_discard_range(const char *rbname, void *hva, bool need_madvise, bool need_fallocate, int ret) "%s@%p: madvise: %d fallocate: %d ret: %d"
+
 # memory.c
 memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

Signature · 1 line

2.13.5

Signature · 1 line

Peter Xu

From: "Dr. David Alan Gilbert" <[email protected]>

Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
  exec.c                    | 10 ++++++++++
  include/exec/cpu-common.h |  1 +
  2 files changed, 11 insertions(+)

exec.c +0 -0

index 67df2909ce..35b4cea2ed 100644

exec.c +5 -0

+    assert(res < rb->max_length);
+
+    return res;
+}
+
  /*
   * Translates a host ptr back to a RAMBlock, a ram_addr and an offset
   * in that RAMBlock.

include/exec/cpu-common.h +1 -0

index 74341b19d2..0d861a6289 100644
@@ -68,6 +68,7 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr);
  RAMBlock *qemu_ram_block_by_name(const char *name);
  RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
                                     ram_addr_t *offset);
+ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host);
  void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
  void qemu_ram_unset_idstr(RAMBlock *block);
  const char *qemu_ram_get_idstr(RAMBlock *rb);

Hi David,

Quoted 11 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
  exec.c                    | 10 ++++++++++
  include/exec/cpu-common.h |  1 +
  2 files changed, 11 insertions(+)

exec.c static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, · line 2236

Quoted 11 lines

exec.c +4 -0

index 67df2909ce..35b4cea2ed 100644
@@ -2231,6 +2231,16 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,
      return ramblock_ptr(block, addr);
  }
+/* Return the offset of a hostpointer within a ramblock */
+ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host)
+{
+    ram_addr_t res = (uint8_t *)host - (uint8_t *)rb->host;

What about using ptrdiff_t here,

exec.c static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, · line 2238

Quoted 1 line

exec.c +1 -0

+    assert((uint8_t *)host >= (uint8_t *)rb->host);

and uintptr_t here?

Quoted 13 lines

exec.c +5 -0

+    assert(res < rb->max_length);
+
+    return res;
+}
+
  /*
   * Translates a host ptr back to a RAMBlock, a ram_addr and an offset
   * in that RAMBlock.

include/exec/cpu-common.h +0 -0

index 74341b19d2..0d861a6289 100644
@@ -68,6 +68,7 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr);

include/exec/cpu-common.h ram_addr_t qemu_ram_addr_from_host(void *ptr); · line 74

Quoted 8 lines

include/exec/cpu-common.h +1 -0

  RAMBlock *qemu_ram_block_by_name(const char *name);
  RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
                                     ram_addr_t *offset);
+ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host);
  void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
  void qemu_ram_unset_idstr(RAMBlock *block);
  const char *qemu_ram_get_idstr(RAMBlock *rb);

From: "Dr. David Alan Gilbert" <[email protected]>

Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)

Signed-off-by Dr. David Alan Gilbert <[email protected]>

From: "Dr. David Alan Gilbert" <[email protected]>

Use a flag on the RAMBlock to state whether it has the UFFDIO_ZEROPAGE capability, use it when it's available.

This allows the use of postcopy on tmpfs as well as hugepage backed files.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 exec.c                    | 15 +++++++++++++++
 include/exec/cpu-common.h |  3 +++
 migration/postcopy-ram.c  | 14 +++++++++++---
 3 files changed, 29 insertions(+), 3 deletions(-)

exec.c +15 -0

index 35b4cea2ed..80c3d1d121 100644
@@ -103,6 +103,11 @@ static MemoryRegion io_mem_unassigned;
  */
 #define RAM_RESIZEABLE (1 << 2)
 
+/* UFFDIO_ZEROPAGE is available on this RAMBlock to atomically
+ * zero the page and wake waiting processes.
+ * (Set during postcopy)
+ */
+#define RAM_UF_ZEROPAGE (1 << 3)
 #endif
 
 #ifdef TARGET_PAGE_BITS_VARY

@@ -1705,6 +1710,16 @@ bool qemu_ram_is_shared(RAMBlock *rb)
     return rb->flags & RAM_SHARED;
 }
 
+bool qemu_ram_is_uf_zeroable(RAMBlock *rb)
+{
+    return rb->flags & RAM_UF_ZEROPAGE;
+}
+
+void qemu_ram_set_uf_zeroable(RAMBlock *rb)
+{
+    rb->flags |= RAM_UF_ZEROPAGE;
+}
+
 /* Called with iothread lock held.  */
 void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
 {

include/exec/cpu-common.h +3 -0

index 0d861a6289..24d335f95d 100644
@@ -73,6 +73,9 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
 void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
 bool qemu_ram_is_shared(RAMBlock *rb);
+bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
+void qemu_ram_set_uf_zeroable(RAMBlock *rb);
+
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
 

migration/postcopy-ram.c +0 -0

index 7a414ebad8..640b72d86d 100644

migration/postcopy-ram.c +5 -3

-        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize(),
-                                rb)) {
+    /* Normal RAMBlocks can zero a page using UFFDIO_ZEROPAGE
+     * but it's not available for everything (e.g. hugetlbpages)
+     */
+    if (qemu_ram_is_uf_zeroable(rb)) {
+        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, pagesize, rb)) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
--

2.13.5

Hi

Quoted 62 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Use a flag on the RAMBlock to state whether it has the UFFDIO_ZEROPAGE capability, use it when it's available.

This allows the use of postcopy on tmpfs as well as hugepage backed files.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 exec.c                    | 15 +++++++++++++++
 include/exec/cpu-common.h |  3 +++
 migration/postcopy-ram.c  | 14 +++++++++++---
 3 files changed, 29 insertions(+), 3 deletions(-)

exec.c +15 -0

index 35b4cea2ed..80c3d1d121 100644
@@ -103,6 +103,11 @@ static MemoryRegion io_mem_unassigned;
  */
 #define RAM_RESIZEABLE (1 << 2)
 
+/* UFFDIO_ZEROPAGE is available on this RAMBlock to atomically
+ * zero the page and wake waiting processes.
+ * (Set during postcopy)
+ */
+#define RAM_UF_ZEROPAGE (1 << 3)
 #endif
 
 #ifdef TARGET_PAGE_BITS_VARY

@@ -1705,6 +1710,16 @@ bool qemu_ram_is_shared(RAMBlock *rb)
     return rb->flags & RAM_SHARED;
 }
 
+bool qemu_ram_is_uf_zeroable(RAMBlock *rb)
+{
+    return rb->flags & RAM_UF_ZEROPAGE;
+}
+
+void qemu_ram_set_uf_zeroable(RAMBlock *rb)
+{
+    rb->flags |= RAM_UF_ZEROPAGE;
+}
+
 /* Called with iothread lock held.  */
 void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
 {

include/exec/cpu-common.h +3 -0

index 0d861a6289..24d335f95d 100644
@@ -73,6 +73,9 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
 void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
 bool qemu_ram_is_shared(RAMBlock *rb);
+bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
+void qemu_ram_set_uf_zeroable(RAMBlock *rb);
+
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
 

Quoted 5 lines

migration/postcopy-ram.c +0 -0

index 7a414ebad8..640b72d86d 100644

migration/postcopy-ram.c static int ram_block_enable_notify(const char *block_name, void *host_addr, · line 416

Quoted 9 lines

migration/postcopy-ram.c +5 -0

@@ -408,6 +408,11 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
         error_report("%s userfault: Region doesn't support COPY", __func__);
         return -1;
     }
+    if (reg_struct.ioctls & ((__u64)1 << _UFFDIO_ZEROPAGE)) {
+        RAMBlock *rb = qemu_ram_block_by_name(block_name);
+        qemu_ram_set_uf_zeroable(rb);
+    }
+
 

extra empty line

Quoted 9 lines

migration/postcopy-ram.c +1 -0

     return 0;
 }

@@ -617,11 +622,14 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
                              RAMBlock *rb)
 {
+    size_t pagesize = qemu_ram_pagesize(rb);
     trace_postcopy_place_page_zero(host);
 

migration/postcopy-ram.c int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, · line 622

Quoted 1 line

migration/postcopy-ram.c +0 -1

-    if (qemu_ram_pagesize(rb) == getpagesize()) {

Is this check drop intentionally?

Quoted 6 lines

migration/postcopy-ram.c +4 -2

-        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize(),
-                                rb)) {
+    /* Normal RAMBlocks can zero a page using UFFDIO_ZEROPAGE
+     * but it's not available for everything (e.g. hugetlbpages)
+     */
+    if (qemu_ram_is_uf_zeroable(rb)) {

Quoted 8 lines

migration/postcopy-ram.c +1 -1

+        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, pagesize, rb)) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
--

2.13.5

Signature · 1 line

Marc-André Lureau

From: "Dr. David Alan Gilbert" <[email protected]>

Add a notifier chain for postcopy with a 'reason' flag and an opportunity for a notifier member to return an error.

Call it when enabling postcopy.

This will initially used to enable devices to declare they're unable to postcopy and later to notify of devices of stages within postcopy.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 migration/postcopy-ram.c | 41 +++++++++++++++++++++++++++++++++++++++++
 migration/postcopy-ram.h | 26 ++++++++++++++++++++++++++
 vl.c                     |  2 ++
 3 files changed, 69 insertions(+)

migration/postcopy-ram.c +2 -0

index 640b72d86d..95007c00ef 100644
@@ -23,6 +23,8 @@
 #include "savevm.h"
 #include "postcopy-ram.h"
 #include "ram.h"
+#include "qapi/error.h"
+#include "qemu/notify.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/balloon.h"
 #include "qemu/error-report.h"

migration/postcopy-ram.c +35 -0

+ */
+static NotifierWithReturnList postcopy_notifier_list;
+
+void postcopy_infrastructure_init(void)
+{
+    notifier_with_return_list_init(&postcopy_notifier_list);
+}
+
+void postcopy_add_notifier(NotifierWithReturn *nn)
+{
+    notifier_with_return_list_add(&postcopy_notifier_list, nn);
+}
+
+void postcopy_remove_notifier(NotifierWithReturn *n)
+{
+    notifier_with_return_remove(n);
+}
+
+int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp)
+{
+    struct PostcopyNotifyData pnd;
+    pnd.reason = reason;
+    pnd.errp = errp;
+
+    return notifier_with_return_list_notify(&postcopy_notifier_list,
+                                            &pnd);
+}
+
 /* Postcopy needs to detect accesses to pages that haven't yet been copied
  * across, and efficiently map new pages in, the techniques for doing this
  * are target OS specific.

@@ -133,6 +167,7 @@ bool postcopy_ram_supported_by_host(void)
     struct uffdio_register reg_struct;
     struct uffdio_range range_struct;
     uint64_t feature_mask;
+    Error *local_err = NULL;
 
     if (qemu_target_page_size() > pagesize) {
         error_report("Target page size bigger than host page size");

@@ -146,6 +181,12 @@ bool postcopy_ram_supported_by_host(void)
         goto out;
     }
 
+    /* Give devices a chance to object */
+    if (postcopy_notify(POSTCOPY_NOTIFY_PROBE, &local_err)) {
+        error_report_err(local_err);
+        goto out;
+    }
+
     /* Version and features check */
     if (!ufd_version_check(ufd)) {
         goto out;

migration/postcopy-ram.h +26 -0

index 78a3591322..d688411674 100644
@@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);
 /* Set the state and return the old state */
 PostcopyState postcopy_state_set(PostcopyState new_state);
 
+/*
+ * To be called once at the start before any device initialisation
+ */
+void postcopy_infrastructure_init(void);
+
+/* Add a notifier to a list to be called when checking whether the devices
+ * can support postcopy.
+ * It's data is a *PostcopyNotifyData
+ * It should return 0 if OK, or a negative value on failure.
+ * On failure it must set the data->errp to an error.
+ *
+ */
+enum PostcopyNotifyReason {
+    POSTCOPY_NOTIFY_PROBE = 0,
+};
+
+struct PostcopyNotifyData {
+    enum PostcopyNotifyReason reason;
+    Error **errp;
+};
+
+void postcopy_add_notifier(NotifierWithReturn *nn);
+void postcopy_remove_notifier(NotifierWithReturn *n);
+/* Call the notifier list set by postcopy_add_start_notifier */
+int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
+
 #endif

vl.c +2 -0

index 8e247cc2a2..65dd9dc324 100644
@@ -95,6 +95,7 @@ int main(int argc, char **argv)
 #include "audio/audio.h"
 #include "sysemu/cpus.h"
 #include "migration/colo.h"
+#include "migration/postcopy-ram.h"
 #include "sysemu/kvm.h"
 #include "sysemu/hax.h"
 #include "qapi/qobject-input-visitor.h"

@@ -3082,6 +3083,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
+    postcopy_infrastructure_init();
 
     if (qcrypto_init(&err) < 0) {
         error_reportf_err(err, "cannot initialize crypto: ");

Signature · 1 line

2.13.5

Quoted 34 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Add a notifier chain for postcopy with a 'reason' flag and an opportunity for a notifier member to return an error.

Call it when enabling postcopy.

This will initially used to enable devices to declare they're unable to postcopy and later to notify of devices of stages within postcopy.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 migration/postcopy-ram.c | 41 +++++++++++++++++++++++++++++++++++++++++
 migration/postcopy-ram.h | 26 ++++++++++++++++++++++++++
 vl.c                     |  2 ++
 3 files changed, 69 insertions(+)

migration/postcopy-ram.c +2 -0

index 640b72d86d..95007c00ef 100644
@@ -23,6 +23,8 @@
 #include "savevm.h"
 #include "postcopy-ram.h"
 #include "ram.h"
+#include "qapi/error.h"
+#include "qemu/notify.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/balloon.h"
 #include "qemu/error-report.h"

@@ -45,6 +47,38 @@ struct PostcopyDiscardState {
     unsigned int nsentcmds;
 };
 

migration/postcopy-ram.c struct PostcopyDiscardState { · line 53

Quoted 4 lines

migration/postcopy-ram.c +4 -0

+/* A notifier chain for postcopy
+ * The notifier should return 0 if it's OK, or a
+ * -errno on error.
+ * The notifier should expect an Error ** as it's data

"PostcopyNotifyData *" but not "Error **"?

Maybe we can just remove this block of comment since there is a similar one in the header below.

Besides:

Reviewed-by Peter Xu <[email protected]>

Quoted 103 lines

migration/postcopy-ram.c +35 -0

+ */
+static NotifierWithReturnList postcopy_notifier_list;
+
+void postcopy_infrastructure_init(void)
+{
+    notifier_with_return_list_init(&postcopy_notifier_list);
+}
+
+void postcopy_add_notifier(NotifierWithReturn *nn)
+{
+    notifier_with_return_list_add(&postcopy_notifier_list, nn);
+}
+
+void postcopy_remove_notifier(NotifierWithReturn *n)
+{
+    notifier_with_return_remove(n);
+}
+
+int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp)
+{
+    struct PostcopyNotifyData pnd;
+    pnd.reason = reason;
+    pnd.errp = errp;
+
+    return notifier_with_return_list_notify(&postcopy_notifier_list,
+                                            &pnd);
+}
+
 /* Postcopy needs to detect accesses to pages that haven't yet been copied
  * across, and efficiently map new pages in, the techniques for doing this
  * are target OS specific.

@@ -133,6 +167,7 @@ bool postcopy_ram_supported_by_host(void)
     struct uffdio_register reg_struct;
     struct uffdio_range range_struct;
     uint64_t feature_mask;
+    Error *local_err = NULL;
 
     if (qemu_target_page_size() > pagesize) {
         error_report("Target page size bigger than host page size");

@@ -146,6 +181,12 @@ bool postcopy_ram_supported_by_host(void)
         goto out;
     }
 
+    /* Give devices a chance to object */
+    if (postcopy_notify(POSTCOPY_NOTIFY_PROBE, &local_err)) {
+        error_report_err(local_err);
+        goto out;
+    }
+
     /* Version and features check */
     if (!ufd_version_check(ufd)) {
         goto out;

migration/postcopy-ram.h +26 -0

index 78a3591322..d688411674 100644
@@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);
 /* Set the state and return the old state */
 PostcopyState postcopy_state_set(PostcopyState new_state);
 
+/*
+ * To be called once at the start before any device initialisation
+ */
+void postcopy_infrastructure_init(void);
+
+/* Add a notifier to a list to be called when checking whether the devices
+ * can support postcopy.
+ * It's data is a *PostcopyNotifyData
+ * It should return 0 if OK, or a negative value on failure.
+ * On failure it must set the data->errp to an error.
+ *
+ */
+enum PostcopyNotifyReason {
+    POSTCOPY_NOTIFY_PROBE = 0,
+};
+
+struct PostcopyNotifyData {
+    enum PostcopyNotifyReason reason;
+    Error **errp;
+};
+
+void postcopy_add_notifier(NotifierWithReturn *nn);
+void postcopy_remove_notifier(NotifierWithReturn *n);
+/* Call the notifier list set by postcopy_add_start_notifier */
+int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
+
 #endif

vl.c +1 -0

index 8e247cc2a2..65dd9dc324 100644
@@ -95,6 +95,7 @@ int main(int argc, char **argv)
 #include "audio/audio.h"
 #include "sysemu/cpus.h"
 #include "migration/colo.h"
+#include "migration/postcopy-ram.h"
 #include "sysemu/kvm.h"
 #include "sysemu/hax.h"
 #include "qapi/qobject-input-visitor.h"

@@ -3082,6 +3083,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_OPTS);
 

Quoted 8 lines

vl.c +1 -0

     runstate_init();
+    postcopy_infrastructure_init();
 
     if (qcrypto_init(&err) < 0) {
         error_reportf_err(err, "cannot initialize crypto: ");

Signature · 1 line

2.13.5

Signature · 1 line

Peter Xu

From: "Dr. David Alan Gilbert" <[email protected]>

Add a vhost feature flag for postcopy support, and use the postcopy notifier to check it before allowing postcopy.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.h |  1 +
 docs/interop/vhost-user.txt           | 10 +++++++++
 hw/virtio/vhost-user.c                | 40 ++++++++++++++++++++++++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

contrib/libvhost-user/libvhost-user.h +1 -0

index acd019876d..95d0d34a28 100644
@@ -34,6 +34,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_MQ = 0,
     VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
     VHOST_USER_PROTOCOL_F_RARP = 2,
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 7,
 
     VHOST_USER_PROTOCOL_F_MAX
 };

docs/interop/vhost-user.txt +10 -0

index 954771d0d8..a279560eb0 100644
@@ -273,6 +273,15 @@ Once the source has finished migration, rings will be stopped by
 the source. No further update must be done before rings are
 restarted.
 
+In postcopy migration the slave is started before all the memory has been
+received from the source host, and care must be taken to avoid accessing pages
+that have yet to be received.  The slave opens a 'userfault'-fd and registers
+the memory with it; this fd is then passed back over to the master.
+The master services requests on the userfaultfd for pages that are accessed
+and when the page is available it performs WAKE ioctl's on the userfaultfd
+to wake the stalled slave.  The client indicates support for this via the
+VHOST_USER_PROTOCOL_F_PAGEFAULT feature.
+
 IOMMU support
 -------------
 

@@ -327,6 +336,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_MTU            4
 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
+#define VHOST_USER_PROTOCOL_F_PAGEFAULT      7
 
 Master message types
 --------------------

hw/virtio/vhost-user.c +36 -1

index 093675ed98..c51bbd1296 100644
@@ -17,6 +17,8 @@
 #include "sysemu/kvm.h"
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
+#include "migration/migration.h"
+#include "migration/postcopy-ram.h"
 
 #include <sys/ioctl.h>
 #include <sys/socket.h>

@@ -34,7 +36,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_NET_MTU = 4,
     VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
     VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 7,
     VHOST_USER_PROTOCOL_F_MAX
 };
 

@@ -123,8 +125,10 @@ static VhostUserMsg m __attribute__ ((unused));
 #define VHOST_USER_VERSION    (0x1)
 
 struct vhost_user {
+    struct vhost_dev *dev;
     CharBackend *chr;
     int slave_fd;
+    NotifierWithReturn postcopy_notifier;
 };
 
 static bool ioeventfd_enabled(void)

@@ -720,6 +724,33 @@ out:
     return ret;
 }
 
+static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
+                                        void *opaque)
+{
+    struct PostcopyNotifyData *pnd = opaque;
+    struct vhost_user *u = container_of(notifier, struct vhost_user,
+                                         postcopy_notifier);
+    struct vhost_dev *dev = u->dev;
+
+    switch (pnd->reason) {
+    case POSTCOPY_NOTIFY_PROBE:
+        if (!virtio_has_feature(dev->protocol_features,
+                                VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
+            /* TODO: Get the device name into this error somehow */
+            error_setg(pnd->errp,
+                       "vhost-user backend not capable of postcopy");
+            return -ENOENT;
+        }
+        break;
+
+    default:
+        /* We ignore notifications we don't know */
+        break;
+    }
+
+    return 0;
+}
+
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
     uint64_t features, protocol_features;

@@ -731,6 +762,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
     u = g_new0(struct vhost_user, 1);
     u->chr = opaque;
     u->slave_fd = -1;
+    u->dev = dev;
     dev->opaque = u;
 
     err = vhost_user_get_features(dev, &features);

@@ -787,6 +819,9 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
         return err;
     }
 
+    u->postcopy_notifier.notify = vhost_user_postcopy_notifier;
+    postcopy_add_notifier(&u->postcopy_notifier);
+
     return 0;
 }
 

hw/virtio/vhost-user.c +2 -0

+        postcopy_remove_notifier(&u->postcopy_notifier);
+    }
     if (u->slave_fd >= 0) {
         qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
         close(u->slave_fd);

Signature · 1 line

2.13.5

Quoted 138 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Add a vhost feature flag for postcopy support, and use the postcopy notifier to check it before allowing postcopy.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.h |  1 +
 docs/interop/vhost-user.txt           | 10 +++++++++
 hw/virtio/vhost-user.c                | 40 ++++++++++++++++++++++++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

contrib/libvhost-user/libvhost-user.h +1 -0

index acd019876d..95d0d34a28 100644
@@ -34,6 +34,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_MQ = 0,
     VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
     VHOST_USER_PROTOCOL_F_RARP = 2,
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 7,
 
     VHOST_USER_PROTOCOL_F_MAX
 };

docs/interop/vhost-user.txt +10 -0

index 954771d0d8..a279560eb0 100644
@@ -273,6 +273,15 @@ Once the source has finished migration, rings will be stopped by
 the source. No further update must be done before rings are
 restarted.
 
+In postcopy migration the slave is started before all the memory has been
+received from the source host, and care must be taken to avoid accessing pages
+that have yet to be received.  The slave opens a 'userfault'-fd and registers
+the memory with it; this fd is then passed back over to the master.
+The master services requests on the userfaultfd for pages that are accessed
+and when the page is available it performs WAKE ioctl's on the userfaultfd
+to wake the stalled slave.  The client indicates support for this via the
+VHOST_USER_PROTOCOL_F_PAGEFAULT feature.
+
 IOMMU support
 -------------
 

@@ -327,6 +336,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_MTU            4
 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
+#define VHOST_USER_PROTOCOL_F_PAGEFAULT      7
 
 Master message types
 --------------------

hw/virtio/vhost-user.c +36 -1

index 093675ed98..c51bbd1296 100644
@@ -17,6 +17,8 @@
 #include "sysemu/kvm.h"
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
+#include "migration/migration.h"
+#include "migration/postcopy-ram.h"
 
 #include <sys/ioctl.h>
 #include <sys/socket.h>

@@ -34,7 +36,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_NET_MTU = 4,
     VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
     VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-
+    VHOST_USER_PROTOCOL_F_PAGEFAULT = 7,
     VHOST_USER_PROTOCOL_F_MAX
 };
 

@@ -123,8 +125,10 @@ static VhostUserMsg m __attribute__ ((unused));
 #define VHOST_USER_VERSION    (0x1)
 
 struct vhost_user {
+    struct vhost_dev *dev;
     CharBackend *chr;
     int slave_fd;
+    NotifierWithReturn postcopy_notifier;
 };
 
 static bool ioeventfd_enabled(void)

@@ -720,6 +724,33 @@ out:
     return ret;
 }
 
+static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
+                                        void *opaque)
+{
+    struct PostcopyNotifyData *pnd = opaque;
+    struct vhost_user *u = container_of(notifier, struct vhost_user,
+                                         postcopy_notifier);
+    struct vhost_dev *dev = u->dev;
+
+    switch (pnd->reason) {
+    case POSTCOPY_NOTIFY_PROBE:
+        if (!virtio_has_feature(dev->protocol_features,
+                                VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
+            /* TODO: Get the device name into this error somehow */
+            error_setg(pnd->errp,
+                       "vhost-user backend not capable of postcopy");
+            return -ENOENT;
+        }
+        break;
+
+    default:
+        /* We ignore notifications we don't know */
+        break;
+    }
+
+    return 0;
+}
+
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
     uint64_t features, protocol_features;

@@ -731,6 +762,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
     u = g_new0(struct vhost_user, 1);
     u->chr = opaque;
     u->slave_fd = -1;
+    u->dev = dev;
     dev->opaque = u;
 
     err = vhost_user_get_features(dev, &features);

@@ -787,6 +819,9 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
         return err;
     }
 
+    u->postcopy_notifier.notify = vhost_user_postcopy_notifier;
+    postcopy_add_notifier(&u->postcopy_notifier);
+
     return 0;
 }
 

hw/virtio/vhost-user.c static int vhost_user_cleanup(struct vhost_dev *dev) · line 835

Quoted 4 lines

hw/virtio/vhost-user.c +1 -0

@@ -797,6 +832,9 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
     u = dev->opaque;
+    if (u->postcopy_notifier.notify) {

Detecting init using the notify hook is slightly strange here for me... If so, not sure whether we also need:

           u->postcopy_notifier.notify = NULL;

Or I'm not sure whether a 2nd call to vhost_user_cleanup() can be dangerous since postcopy_remove_notifier() will be called twice.

Besides that, the patch looks good to me. Thanks,

Quoted 8 lines

hw/virtio/vhost-user.c +2 -0

+        postcopy_remove_notifier(&u->postcopy_notifier);
+    }
     if (u->slave_fd >= 0) {
         qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
         close(u->slave_fd);

Signature · 1 line

2.13.5

Signature · 1 line

Peter Xu

From: "Dr. David Alan Gilbert" <[email protected]>

Wire up a notifier to send a VHOST_USER_POSTCOPY_ADVISE message on an incoming advise.

Later patches will fill in the behaviour/contents of the message.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 21 ++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  6 ++++-
 docs/interop/vhost-user.txt           |  9 +++++++
 hw/virtio/vhost-user.c                | 48 +++++++++++++++++++++++++++++++++++
 migration/postcopy-ram.h              |  1 +
 migration/savevm.c                    |  6 +++++
 6 files changed, 86 insertions(+), 5 deletions(-)

contrib/libvhost-user/libvhost-user.c +0 -0

index 201b9846e9..8bbdf5fb40 100644

contrib/libvhost-user/libvhost-user.c +4 -1

         REQ(VHOST_USER_SET_OWNER),
         REQ(VHOST_USER_RESET_OWNER),
         REQ(VHOST_USER_SET_MEM_TABLE),

@@ -62,7 +59,10 @@ vu_request_to_string(int req)
         REQ(VHOST_USER_GET_QUEUE_NUM),
         REQ(VHOST_USER_SET_VRING_ENABLE),
         REQ(VHOST_USER_SEND_RARP),
-        REQ(VHOST_USER_INPUT_GET_CONFIG),
+        REQ(VHOST_USER_SET_SLAVE_REQ_FD),
+        REQ(VHOST_USER_IOTLB_MSG),
+        REQ(VHOST_USER_SET_VRING_ENDIAN),
+        REQ(VHOST_USER_POSTCOPY_ADVISE),
         REQ(VHOST_USER_MAX),
     };
 #undef REQ

contrib/libvhost-user/libvhost-user.c +9 -0

+     */
+    vmsg->payload.u64 = 0xcafe;
+    vmsg->size = sizeof(vmsg->payload.u64);
+    return true; /* = send a reply */
+}
+
+static bool
 vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
 {
     int do_reply = 0;

@@ -808,6 +819,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
         return vu_set_vring_enable_exec(dev, vmsg);
     case VHOST_USER_NONE:
         break;
+    case VHOST_USER_POSTCOPY_ADVISE:
+        return vu_set_postcopy_advise(dev, vmsg);
     default:
         vmsg_close_fds(vmsg);
         vu_panic(dev, "Unhandled request: %d", vmsg->request);

contrib/libvhost-user/libvhost-user.h +5 -1

index 95d0d34a28..3987ce643d 100644
@@ -62,7 +62,11 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_QUEUE_NUM = 17,
     VHOST_USER_SET_VRING_ENABLE = 18,
     VHOST_USER_SEND_RARP = 19,
-    VHOST_USER_INPUT_GET_CONFIG = 20,
+    VHOST_USER_NET_SET_MTU      = 20,
+    VHOST_USER_SET_SLAVE_REQ_FD = 21,
+    VHOST_USER_IOTLB_MSG        = 22,
+    VHOST_USER_SET_VRING_ENDIAN = 23,
+    VHOST_USER_POSTCOPY_ADVISE  = 24,
     VHOST_USER_MAX
 } VhostUserRequest;
 

docs/interop/vhost-user.txt +9 -0

index a279560eb0..dad2a1b343 100644
@@ -606,6 +606,15 @@ Master message types
       and expect this message once (per VQ) during device configuration
       (ie. before the master starts the VQ).
 
+ * VHOST_USER_POSTCOPY_ADVISE
+      Id: 24
+      Master payload: N/A
+      Slave payload: userfault fd + u64
+
+      Master advises slave that a migration with postcopy enabled is underway,
+      the slave must open a userfaultfd for later use.
+      Note that at this stage the migration is still in precopy mode.
+
 Slave message types
 -------------------
 

hw/virtio/vhost-user.c +48 -0

index c51bbd1296..7063e4df61 100644
@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_SLAVE_REQ_FD = 21,
     VHOST_USER_IOTLB_MSG = 22,
     VHOST_USER_SET_VRING_ENDIAN = 23,
+    VHOST_USER_POSTCOPY_ADVISE  = 24,
     VHOST_USER_MAX
 } VhostUserRequest;
 

@@ -724,6 +725,50 @@ out:
     return ret;
 }
 
+/*
+ * Called at the start of an inbound postcopy on reception of the
+ * 'advise' command.
+ */
+static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
+{
+    struct vhost_user *u = dev->opaque;
+    CharBackend *chr = u->chr;
+    int ufd;
+    VhostUserMsg msg = {
+        .request = VHOST_USER_POSTCOPY_ADVISE,
+        .flags = VHOST_USER_VERSION,
+    };
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        error_setg(errp, "Failed to send postcopy_advise to vhost");
+        return -1;
+    }
+
+    if (vhost_user_read(dev, &msg) < 0) {
+        error_setg(errp, "Failed to get postcopy_advise reply from vhost");
+        return -1;
+    }
+
+    if (msg.request != VHOST_USER_POSTCOPY_ADVISE) {
+        error_setg(errp, "Unexpected msg type. Expected %d received %d",
+                     VHOST_USER_POSTCOPY_ADVISE, msg.request);
+        return -1;
+    }
+
+    if (msg.size != sizeof(msg.payload.u64)) {
+        error_setg(errp, "Received bad msg size.");
+        return -1;
+    }
+    ufd = qemu_chr_fe_get_msgfd(chr);
+    if (ufd < 0) {
+        error_setg(errp, "%s: Failed to get ufd", __func__);
+        return -1;
+    }
+
+    /* TODO: register ufd with userfault thread */
+    return 0;
+}
+
 static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
                                         void *opaque)
 {

@@ -743,6 +788,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
         }
         break;
 
+    case POSTCOPY_NOTIFY_INBOUND_ADVISE:
+        return vhost_user_postcopy_advise(dev, pnd->errp);
+
     default:
         /* We ignore notifications we don't know */
         break;

migration/postcopy-ram.h +1 -0

index d688411674..70d4b09659 100644
@@ -128,6 +128,7 @@ void postcopy_infrastructure_init(void);
  */
 enum PostcopyNotifyReason {
     POSTCOPY_NOTIFY_PROBE = 0,
+    POSTCOPY_NOTIFY_INBOUND_ADVISE,
 };
 
 struct PostcopyNotifyData {

migration/savevm.c +6 -1

index fdd15fa0a7..d35911731d 100644
@@ -1343,6 +1343,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
 {
     PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
     uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps;
+    Error *local_err = NULL;
 
     trace_loadvm_postcopy_handle_advise();
     if (ps != POSTCOPY_INCOMING_NONE) {

@@ -1390,6 +1391,11 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
         return -1;
     }
 
+    if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, &local_err)) {
+        error_report_err(local_err);
+        return -1;
+    }
+
     if (ram_postcopy_incoming_init(mis)) {
         return -1;
     }
--

2.13.5

Hi

Quoted 23 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Wire up a notifier to send a VHOST_USER_POSTCOPY_ADVISE message on an incoming advise.

Later patches will fill in the behaviour/contents of the message.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 21 ++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  6 ++++-
 docs/interop/vhost-user.txt           |  9 +++++++
 hw/virtio/vhost-user.c                | 48 +++++++++++++++++++++++++++++++++++
 migration/postcopy-ram.h              |  1 +
 migration/savevm.c                    |  6 +++++
 6 files changed, 86 insertions(+), 5 deletions(-)

contrib/libvhost-user/libvhost-user.c +0 -0

index 201b9846e9..8bbdf5fb40 100644

contrib/libvhost-user/libvhost-user.c vu_request_to_string(int req) · line 47

Quoted 6 lines

contrib/libvhost-user/libvhost-user.c +0 -3

@@ -42,9 +42,6 @@ vu_request_to_string(int req)
         REQ(VHOST_USER_NONE),
         REQ(VHOST_USER_GET_FEATURES),
         REQ(VHOST_USER_SET_FEATURES),
-        REQ(VHOST_USER_NONE),
-        REQ(VHOST_USER_GET_FEATURES),
-        REQ(VHOST_USER_SET_FEATURES),

nice cleanup ;)

Quoted 18 lines

contrib/libvhost-user/libvhost-user.c +4 -1

         REQ(VHOST_USER_SET_OWNER),
         REQ(VHOST_USER_RESET_OWNER),
         REQ(VHOST_USER_SET_MEM_TABLE),

@@ -62,7 +59,10 @@ vu_request_to_string(int req)
         REQ(VHOST_USER_GET_QUEUE_NUM),
         REQ(VHOST_USER_SET_VRING_ENABLE),
         REQ(VHOST_USER_SEND_RARP),
-        REQ(VHOST_USER_INPUT_GET_CONFIG),
+        REQ(VHOST_USER_SET_SLAVE_REQ_FD),
+        REQ(VHOST_USER_IOTLB_MSG),
+        REQ(VHOST_USER_SET_VRING_ENDIAN),
+        REQ(VHOST_USER_POSTCOPY_ADVISE),
         REQ(VHOST_USER_MAX),
     };
 #undef REQ

@@ -744,6 +744,17 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
 }
 

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 750

Quoted 5 lines

contrib/libvhost-user/libvhost-user.c +4 -0

 static bool
+vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
+{
+    /* TODO: Open ufd, pass it back in the request
+     * TODO: Add addresses

Extra EOL space

Quoted 157 lines

contrib/libvhost-user/libvhost-user.c +9 -0

+     */
+    vmsg->payload.u64 = 0xcafe;
+    vmsg->size = sizeof(vmsg->payload.u64);
+    return true; /* = send a reply */
+}
+
+static bool
 vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
 {
     int do_reply = 0;

@@ -808,6 +819,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
         return vu_set_vring_enable_exec(dev, vmsg);
     case VHOST_USER_NONE:
         break;
+    case VHOST_USER_POSTCOPY_ADVISE:
+        return vu_set_postcopy_advise(dev, vmsg);
     default:
         vmsg_close_fds(vmsg);
         vu_panic(dev, "Unhandled request: %d", vmsg->request);

contrib/libvhost-user/libvhost-user.h +5 -1

index 95d0d34a28..3987ce643d 100644
@@ -62,7 +62,11 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_QUEUE_NUM = 17,
     VHOST_USER_SET_VRING_ENABLE = 18,
     VHOST_USER_SEND_RARP = 19,
-    VHOST_USER_INPUT_GET_CONFIG = 20,
+    VHOST_USER_NET_SET_MTU      = 20,
+    VHOST_USER_SET_SLAVE_REQ_FD = 21,
+    VHOST_USER_IOTLB_MSG        = 22,
+    VHOST_USER_SET_VRING_ENDIAN = 23,
+    VHOST_USER_POSTCOPY_ADVISE  = 24,
     VHOST_USER_MAX
 } VhostUserRequest;
 

docs/interop/vhost-user.txt +9 -0

index a279560eb0..dad2a1b343 100644
@@ -606,6 +606,15 @@ Master message types
       and expect this message once (per VQ) during device configuration
       (ie. before the master starts the VQ).
 
+ * VHOST_USER_POSTCOPY_ADVISE
+      Id: 24
+      Master payload: N/A
+      Slave payload: userfault fd + u64
+
+      Master advises slave that a migration with postcopy enabled is underway,
+      the slave must open a userfaultfd for later use.
+      Note that at this stage the migration is still in precopy mode.
+
 Slave message types
 -------------------
 

hw/virtio/vhost-user.c +48 -0

index c51bbd1296..7063e4df61 100644
@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_SLAVE_REQ_FD = 21,
     VHOST_USER_IOTLB_MSG = 22,
     VHOST_USER_SET_VRING_ENDIAN = 23,
+    VHOST_USER_POSTCOPY_ADVISE  = 24,
     VHOST_USER_MAX
 } VhostUserRequest;
 

@@ -724,6 +725,50 @@ out:
     return ret;
 }
 
+/*
+ * Called at the start of an inbound postcopy on reception of the
+ * 'advise' command.
+ */
+static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
+{
+    struct vhost_user *u = dev->opaque;
+    CharBackend *chr = u->chr;
+    int ufd;
+    VhostUserMsg msg = {
+        .request = VHOST_USER_POSTCOPY_ADVISE,
+        .flags = VHOST_USER_VERSION,
+    };
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        error_setg(errp, "Failed to send postcopy_advise to vhost");
+        return -1;
+    }
+
+    if (vhost_user_read(dev, &msg) < 0) {
+        error_setg(errp, "Failed to get postcopy_advise reply from vhost");
+        return -1;
+    }
+
+    if (msg.request != VHOST_USER_POSTCOPY_ADVISE) {
+        error_setg(errp, "Unexpected msg type. Expected %d received %d",
+                     VHOST_USER_POSTCOPY_ADVISE, msg.request);
+        return -1;
+    }
+
+    if (msg.size != sizeof(msg.payload.u64)) {
+        error_setg(errp, "Received bad msg size.");
+        return -1;
+    }
+    ufd = qemu_chr_fe_get_msgfd(chr);
+    if (ufd < 0) {
+        error_setg(errp, "%s: Failed to get ufd", __func__);
+        return -1;
+    }
+
+    /* TODO: register ufd with userfault thread */
+    return 0;
+}
+
 static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
                                         void *opaque)
 {

@@ -743,6 +788,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
         }
         break;
 
+    case POSTCOPY_NOTIFY_INBOUND_ADVISE:
+        return vhost_user_postcopy_advise(dev, pnd->errp);
+
     default:
         /* We ignore notifications we don't know */
         break;

migration/postcopy-ram.h +1 -0

index d688411674..70d4b09659 100644
@@ -128,6 +128,7 @@ void postcopy_infrastructure_init(void);
  */
 enum PostcopyNotifyReason {
     POSTCOPY_NOTIFY_PROBE = 0,
+    POSTCOPY_NOTIFY_INBOUND_ADVISE,
 };
 
 struct PostcopyNotifyData {

migration/savevm.c +1 -0

index fdd15fa0a7..d35911731d 100644
@@ -1343,6 +1343,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
 {
     PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
     uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps;
+    Error *local_err = NULL;
 
     trace_loadvm_postcopy_handle_advise();
     if (ps != POSTCOPY_INCOMING_NONE) {

@@ -1390,6 +1391,11 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
         return -1;
     }
 

Quoted 9 lines

migration/savevm.c +5 -1

+    if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, &local_err)) {
+        error_report_err(local_err);
+        return -1;
+    }
+
     if (ram_postcopy_incoming_init(mis)) {
         return -1;
     }
--

Quoted 3 lines

2.13.5

Reviewed-by Marc-André Lureau <[email protected]>

Signature · 2 lines

-- 
Marc-André Lureau

From: "Dr. David Alan Gilbert" <[email protected]>

Allow replies with fds (for postcopy)

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

contrib/libvhost-user/libvhost-user.c +4 -0

index 8bbdf5fb40..47884c0a15 100644
@@ -213,6 +213,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
 {
     int rc;
     uint8_t *p = (uint8_t *)vmsg;
+    char control[CMSG_SPACE(VHOST_MEMORY_MAX_NREGIONS * sizeof(int))] = { };
+    struct iovec iov = {
+        .iov_base = (char *)vmsg,
+        .iov_len = VHOST_USER_HDR_SIZE,

contrib/libvhost-user/libvhost-user.c +14 -2

+        size_t fdsize = vmsg->fd_num * sizeof(int);
+        msg.msg_controllen = CMSG_SPACE(fdsize);
+        cmsg = CMSG_FIRSTHDR(&msg);
+        cmsg->cmsg_len = CMSG_LEN(fdsize);
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
+    } else {
+        msg.msg_controllen = 0;
+    }
 
     /* Set the version in the flags when sending the reply */
     vmsg->flags &= ~VHOST_USER_VERSION_MASK;

@@ -220,7 +244,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
     vmsg->flags |= VHOST_USER_REPLY_MASK;
 
     do {
-        rc = write(conn_fd, p, VHOST_USER_HDR_SIZE);
+        rc = sendmsg(conn_fd, &msg, 0);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 
     do {

@@ -313,6 +337,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
     }
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     DPRINT("Sending back to guest u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
 

@@ -454,6 +479,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
     dev->log_size = log_mmap_size;
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }

@@ -698,6 +724,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
 
     vmsg->payload.u64 = features;
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }
--

2.13.5

Hi

Quoted 25 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Allow replies with fds (for postcopy)

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

contrib/libvhost-user/libvhost-user.c +8 -0

index 8bbdf5fb40..47884c0a15 100644
@@ -213,6 +213,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
 {
     int rc;
     uint8_t *p = (uint8_t *)vmsg;
+    char control[CMSG_SPACE(VHOST_MEMORY_MAX_NREGIONS * sizeof(int))] = { };
+    struct iovec iov = {
+        .iov_base = (char *)vmsg,
+        .iov_len = VHOST_USER_HDR_SIZE,
+    };
+    struct msghdr msg = {
+        .msg_iov = &iov,
+        .msg_iovlen = 1,

contrib/libvhost-user/libvhost-user.c vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg) · line 229

Quoted 6 lines

contrib/libvhost-user/libvhost-user.c +6 -0

+        .msg_control = control,
+    };
+    struct cmsghdr *cmsg;
+
+    memset(control, 0, sizeof(control));
+    if (vmsg->fds) {

This is going to be always true, right? Check vmsg->fd_num > 0 instead?

I would also add check or assert(vmsg->fd_num <= VHOST_MEMORY_MAX_NREGIONS)

Quoted 48 lines

contrib/libvhost-user/libvhost-user.c +14 -2

+        size_t fdsize = vmsg->fd_num * sizeof(int);
+        msg.msg_controllen = CMSG_SPACE(fdsize);
+        cmsg = CMSG_FIRSTHDR(&msg);
+        cmsg->cmsg_len = CMSG_LEN(fdsize);
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
+    } else {
+        msg.msg_controllen = 0;
+    }
 
     /* Set the version in the flags when sending the reply */
     vmsg->flags &= ~VHOST_USER_VERSION_MASK;

@@ -220,7 +244,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
     vmsg->flags |= VHOST_USER_REPLY_MASK;
 
     do {
-        rc = write(conn_fd, p, VHOST_USER_HDR_SIZE);
+        rc = sendmsg(conn_fd, &msg, 0);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 
     do {

@@ -313,6 +337,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
     }
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     DPRINT("Sending back to guest u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
 

@@ -454,6 +479,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
     dev->log_size = log_mmap_size;
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }

@@ -698,6 +724,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
 
     vmsg->payload.u64 = features;
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }
--

Quoted 3 lines

2.13.5

other than that

Reviewed-by Marc-André Lureau <[email protected]>

Signature · 2 lines

-- 
Marc-André Lureau

From: "Dr. David Alan Gilbert" <[email protected]>

Open a userfaultfd (on a postcopy_advise) and send it back in the reply to the qemu for it to monitor.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

contrib/libvhost-user/libvhost-user.c +1 -3

index 47884c0a15..f9b5b12b28 100644
@@ -15,6 +15,7 @@
 
 #include <qemu/osdep.h>
 #include <sys/eventfd.h>
+#include <sys/syscall.h>
 #include <linux/vhost.h>
 
 #include "qemu/atomic.h"

@@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-    /* TODO: Open ufd, pass it back in the request
-     * TODO: Add addresses 
-     */

contrib/libvhost-user/libvhost-user.c +6 -0

+    }
+    /* TODO: Stash feature flags somewhere */
+out:
+    /* Return a ufd to the QEMU */
+    vmsg->fd_num = 1;
+    vmsg->fds[0] = dev->postcopy_ufd;
     return true; /* = send a reply */
 }
 

contrib/libvhost-user/libvhost-user.h +3 -0

index 3987ce643d..3e8efdd919 100644
@@ -234,6 +234,9 @@ struct VuDev {
      * re-initialize */
     vu_panic_cb panic;
     const VuDevIface *iface;
+
+    /* Postcopy data */
+    int postcopy_ufd;
 };
 
 typedef struct VuVirtqElement {

Signature · 1 line

2.13.5

Quoted 35 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Open a userfaultfd (on a postcopy_advise) and send it back in the reply to the qemu for it to monitor.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

contrib/libvhost-user/libvhost-user.c +5 -3

index 47884c0a15..f9b5b12b28 100644
@@ -15,6 +15,7 @@
 
 #include <qemu/osdep.h>
 #include <sys/eventfd.h>
+#include <sys/syscall.h>
 #include <linux/vhost.h>
 
 #include "qemu/atomic.h"

@@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-    /* TODO: Open ufd, pass it back in the request
-     * TODO: Add addresses 
-     */
+    struct uffdio_api api_struct;
+
+    dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+    /* TODO: Add addresses */
     vmsg->payload.u64 = 0xcafe;

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 786

Quoted 5 lines

contrib/libvhost-user/libvhost-user.c +4 -0

     vmsg->size = sizeof(vmsg->payload.u64);
+
+    if (dev->postcopy_ufd == -1) {
+        vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
+        goto out;

We got error but still goto out? I feel like we should reply with some kind of error code when any error happens.

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 794

Quoted 8 lines

contrib/libvhost-user/libvhost-user.c +8 -0

+    }
+    api_struct.api = UFFD_API;
+    api_struct.features = 0;
+    if (ioctl(dev->postcopy_ufd, UFFDIO_API, &api_struct)) {
+        vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
+        close(dev->postcopy_ufd);
+        dev->postcopy_ufd = -1;
+        goto out;

Same here.

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 800

Quoted 6 lines

contrib/libvhost-user/libvhost-user.c +6 -0

+    }
+    /* TODO: Stash feature flags somewhere */
+out:
+    /* Return a ufd to the QEMU */
+    vmsg->fd_num = 1;
+    vmsg->fds[0] = dev->postcopy_ufd;

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 802

Quoted 2 lines

contrib/libvhost-user/libvhost-user.c +0 -0

     return true; /* = send a reply */
 }

Quoted 14 lines

contrib/libvhost-user/libvhost-user.h +3 -0

index 3987ce643d..3e8efdd919 100644
@@ -234,6 +234,9 @@ struct VuDev {
      * re-initialize */
     vu_panic_cb panic;
     const VuDevIface *iface;
+
+    /* Postcopy data */
+    int postcopy_ufd;
 };
 

Quoted 4 lines

contrib/libvhost-user/libvhost-user.h +0 -0

 typedef struct VuVirtqElement {

Signature · 1 line

2.13.5

Signature · 1 line

Peter Xu

From: "Dr. David Alan Gilbert" <[email protected]>

Open a userfaultfd (on a postcopy_advise) and send it back in the reply to the qemu for it to monitor.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

contrib/libvhost-user/libvhost-user.c +1 -0

index 47884c0a15..f9b5b12b28 100644
@@ -15,6 +15,7 @@
 
 #include <qemu/osdep.h>
 #include <sys/eventfd.h>
+#include <sys/syscall.h>
 #include <linux/vhost.h>
 
 #include "qemu/atomic.h"

contrib/libvhost-user/libvhost-user.c +19 -0

+    /* TODO: Add addresses */
     vmsg->payload.u64 = 0xcafe;
     vmsg->size = sizeof(vmsg->payload.u64);
+
+    if (dev->postcopy_ufd == -1) {
+        vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
+        goto out;
+    }
+    api_struct.api = UFFD_API;
+    api_struct.features = 0;
+    if (ioctl(dev->postcopy_ufd, UFFDIO_API, &api_struct)) {
+        vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
+        close(dev->postcopy_ufd);
+        dev->postcopy_ufd = -1;
+        goto out;
+    }
+    /* TODO: Stash feature flags somewhere */
+out:
+    /* Return a ufd to the QEMU */
+    vmsg->fd_num = 1;
+    vmsg->fds[0] = dev->postcopy_ufd;
     return true; /* = send a reply */
 }
 

contrib/libvhost-user/libvhost-user.h +3 -1

index 3987ce643d..3e8efdd919 100644
@@ -234,6 +234,9 @@ struct VuDev {
      * re-initialize */
     vu_panic_cb panic;
     const VuDevIface *iface;
+
+    /* Postcopy data */
+    int postcopy_ufd;
 };
 
 typedef struct VuVirtqElement {
--

2.13.5

I would rather use libvhost-user: message prefix (same for similar libvhost-user patches)

Quoted 22 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Open a userfaultfd (on a postcopy_advise) and send it back in the reply to the qemu for it to monitor.

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++++++++++++---
 contrib/libvhost-user/libvhost-user.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

contrib/libvhost-user/libvhost-user.c +1 -0

index 47884c0a15..f9b5b12b28 100644
@@ -15,6 +15,7 @@
 
 #include <qemu/osdep.h>
 #include <sys/eventfd.h>
+#include <sys/syscall.h>
 #include <linux/vhost.h>
 

contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 779

Quoted 11 lines

contrib/libvhost-user/libvhost-user.c +3 -3

 #include "qemu/atomic.h"

@@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-    /* TODO: Open ufd, pass it back in the request
-     * TODO: Add addresses
-     */
+    struct uffdio_api api_struct;
+
+    dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);

This will likely fail to compile on !Linux, could you add some appropriate #ifdef?

Quoted 37 lines

contrib/libvhost-user/libvhost-user.c +19 -0

+    /* TODO: Add addresses */
     vmsg->payload.u64 = 0xcafe;
     vmsg->size = sizeof(vmsg->payload.u64);
+
+    if (dev->postcopy_ufd == -1) {
+        vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
+        goto out;
+    }
+    api_struct.api = UFFD_API;
+    api_struct.features = 0;
+    if (ioctl(dev->postcopy_ufd, UFFDIO_API, &api_struct)) {
+        vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
+        close(dev->postcopy_ufd);
+        dev->postcopy_ufd = -1;
+        goto out;
+    }
+    /* TODO: Stash feature flags somewhere */
+out:
+    /* Return a ufd to the QEMU */
+    vmsg->fd_num = 1;
+    vmsg->fds[0] = dev->postcopy_ufd;
     return true; /* = send a reply */
 }
 

contrib/libvhost-user/libvhost-user.h +3 -0

index 3987ce643d..3e8efdd919 100644
@@ -234,6 +234,9 @@ struct VuDev {
      * re-initialize */
     vu_panic_cb panic;
     const VuDevIface *iface;
+
+    /* Postcopy data */
+    int postcopy_ufd;
 };
 

Quoted 5 lines

contrib/libvhost-user/libvhost-user.h +0 -1

 typedef struct VuVirtqElement {
--

2.13.5

Signature · 1 line

Marc-André Lureau