Changeset
exec.c                    | 10 ++++++++++
include/exec/cpu-common.h |  1 +
migration/ram.c           | 42 ++++++++++++++++++++++++++++++++----------
3 files changed, 43 insertions(+), 10 deletions(-)
Git apply log
Switched to a new branch '20180413075200.15217-1-clg@kaod.org'
Applying: migration: discard non-migratable RAMBlocks
To https://github.com/patchew-project/qemu
 * [new tag]         patchew/20180413075200.15217-1-clg@kaod.org -> patchew/20180413075200.15217-1-clg@kaod.org
Test failed: checkpatch

loading

Test passed: docker-mingw@fedora

loading

Test passed: docker-build@min-glib

loading

Test passed: s390x

loading

[Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Cédric Le Goater, 6 days ago
On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the presenter sub-engine.

These MMIO regions are exposed to guests in QEMU with a set of 'ram
device' memory mappings, similarly to VFIO, and the VMAs are populated
dynamically with the appropriate pages using a fault handler.

But, these regions are an issue for migration. We need to discard the
associated RAMBlocks from the RAM state on the source VM and let the
destination VM rebuild the memory mappings on the new host in the
post_load() operation just before resuming the system.

To achieve this goal, the following introduces a new RAMBlock flag
RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
vmstate_unregister_ram() routines. This flag is then used by the
migration to identify RAMBlocks to discard on the source. Some checks
are also performed on the destination to make sure nothing invalid was
sent.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:

 - introduced a new RAMBlock flag RAM_MIGRATABLE
 - fixed RAMBLOCK_FOREACH_MIGRATABLE() macro to handle code that
   omitted {}. 

 exec.c                    | 10 ++++++++++
 include/exec/cpu-common.h |  1 +
 migration/ram.c           | 42 ++++++++++++++++++++++++++++++++----------
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 02b1efebb7c3..e9432c06294e 100644
--- a/exec.c
+++ b/exec.c
@@ -104,6 +104,9 @@ static MemoryRegion io_mem_unassigned;
  * (Set during postcopy)
  */
 #define RAM_UF_ZEROPAGE (1 << 3)
+
+/* RAM can be migrated */
+#define RAM_MIGRATABLE (1 << 4)
 #endif
 
 #ifdef TARGET_PAGE_BITS_VARY
@@ -1807,6 +1810,11 @@ void qemu_ram_set_uf_zeroable(RAMBlock *rb)
     rb->flags |= RAM_UF_ZEROPAGE;
 }
 
+bool qemu_ram_is_migratable(RAMBlock *rb)
+{
+    return rb->flags & RAM_MIGRATABLE;
+}
+
 /* Called with iothread lock held.  */
 void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
 {
@@ -1823,6 +1831,7 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
         }
     }
     pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
+    new_block->flags |= RAM_MIGRATABLE;
 
     rcu_read_lock();
     RAMBLOCK_FOREACH(block) {
@@ -1845,6 +1854,7 @@ void qemu_ram_unset_idstr(RAMBlock *block)
      */
     if (block) {
         memset(block->idstr, 0, sizeof(block->idstr));
+        block->flags &= ~RAM_MIGRATABLE;
     }
 }
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 24d335f95d45..96854519b514 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -75,6 +75,7 @@ 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);
+bool qemu_ram_is_migratable(RAMBlock *rb);
 
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
diff --git a/migration/ram.c b/migration/ram.c
index 0e90efa09236..89c3accc4e26 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -187,6 +187,11 @@ void ramblock_recv_bitmap_set_range(RAMBlock *rb, void *host_addr,
                       nr);
 }
 
+/* Should be holding either ram_list.mutex, or the RCU lock. */
+#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
+    RAMBLOCK_FOREACH(block)                            \
+        if (!qemu_ram_is_migratable(block)) {} else
+
 /*
  * An outstanding page request, on the source, having been received
  * and queued
@@ -780,6 +785,10 @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
     unsigned long *bitmap = rb->bmap;
     unsigned long next;
 
+    if (!qemu_ram_is_migratable(rb)) {
+        return size;
+    }
+
     if (rs->ram_bulk_stage && start > 0) {
         next = start + 1;
     } else {
@@ -825,7 +834,7 @@ uint64_t ram_pagesize_summary(void)
     RAMBlock *block;
     uint64_t summary = 0;
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         summary |= block->page_size;
     }
 
@@ -849,7 +858,7 @@ static void migration_bitmap_sync(RAMState *rs)
 
     qemu_mutex_lock(&rs->bitmap_mutex);
     rcu_read_lock();
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         migration_bitmap_sync_range(rs, block, 0, block->used_length);
     }
     rcu_read_unlock();
@@ -1499,6 +1508,10 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,
     size_t pagesize_bits =
         qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
 
+    if (!qemu_ram_is_migratable(pss->block)) {
+        return 0;
+    }
+
     do {
         tmppages = ram_save_target_page(rs, pss, last_stage);
         if (tmppages < 0) {
@@ -1587,7 +1600,7 @@ uint64_t ram_bytes_total(void)
     uint64_t total = 0;
 
     rcu_read_lock();
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         total += block->used_length;
     }
     rcu_read_unlock();
@@ -1642,7 +1655,7 @@ static void ram_save_cleanup(void *opaque)
      */
     memory_global_dirty_log_stop();
 
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         g_free(block->bmap);
         block->bmap = NULL;
         g_free(block->unsentmap);
@@ -1705,7 +1718,7 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms)
 {
     struct RAMBlock *block;
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         unsigned long *bitmap = block->bmap;
         unsigned long range = block->used_length >> TARGET_PAGE_BITS;
         unsigned long run_start = find_next_zero_bit(bitmap, range, 0);
@@ -1783,7 +1796,7 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
     struct RAMBlock *block;
     int ret;
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         PostcopyDiscardState *pds =
             postcopy_discard_send_init(ms, block->idstr);
 
@@ -1991,7 +2004,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
     rs->last_sent_block = NULL;
     rs->last_page = 0;
 
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         unsigned long pages = block->used_length >> TARGET_PAGE_BITS;
         unsigned long *bitmap = block->bmap;
         unsigned long *unsentmap = block->unsentmap;
@@ -2150,7 +2163,7 @@ static void ram_list_init_bitmaps(void)
 
     /* Skip setting bitmap if there is no RAM */
     if (ram_bytes_total()) {
-        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
+        RAMBLOCK_FOREACH_MIGRATABLE(block) {
             pages = block->max_length >> TARGET_PAGE_BITS;
             block->bmap = bitmap_new(pages);
             bitmap_set(block->bmap, 0, pages);
@@ -2226,7 +2239,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 
     qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         qemu_put_byte(f, strlen(block->idstr));
         qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
         qemu_put_be64(f, block->used_length);
@@ -2471,6 +2484,11 @@ static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
         return NULL;
     }
 
+    if (!qemu_ram_is_migratable(block)) {
+        error_report("block %s should not be migrated !", id);
+        return NULL;
+    }
+
     return block;
 }
 
@@ -2921,7 +2939,11 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 length = qemu_get_be64(f);
 
                 block = qemu_ram_block_by_name(id);
-                if (block) {
+                if (block && !qemu_ram_is_migratable(block)) {
+                    error_report("block %s should not be migrated !", id);
+                    ret = -EINVAL;
+
+                } else if (block) {
                     if (length != block->used_length) {
                         Error *local_err = NULL;
 
-- 
2.13.6


Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by no-reply@patchew.org, 6 days ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180413075200.15217-1-clg@kaod.org
Subject: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/1523372486-13190-1-git-send-email-walling@linux.ibm.com -> patchew/1523372486-13190-1-git-send-email-walling@linux.ibm.com
 * [new tag]               patchew/20180413075200.15217-1-clg@kaod.org -> patchew/20180413075200.15217-1-clg@kaod.org
Switched to a new branch 'test'
59f7dd4351 migration: discard non-migratable RAMBlocks

=== OUTPUT BEGIN ===
Checking PATCH 1/1: migration: discard non-migratable RAMBlocks...
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#96: FILE: migration/ram.c:191:
+#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
+    RAMBLOCK_FOREACH(block)                            \
+        if (!qemu_ram_is_migratable(block)) {} else

ERROR: trailing statements should be on next line
#98: FILE: migration/ram.c:193:
+        if (!qemu_ram_is_migratable(block)) {} else

total: 2 errors, 0 warnings, 167 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Juan Quintela, 6 days ago
no-reply@patchew.org wrote:
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20180413075200.15217-1-clg@kaod.org
> Subject: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
>

> === OUTPUT BEGIN ===
> Checking PATCH 1/1: migration: discard non-migratable RAMBlocks...
> ERROR: Macros with multiple statements should be enclosed in a do - while loop
> #96: FILE: migration/ram.c:191:
> +#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> +    RAMBLOCK_FOREACH(block)                            \
> +        if (!qemu_ram_is_migratable(block)) {} else
>
> ERROR: trailing statements should be on next line
> #98: FILE: migration/ram.c:193:
> +        if (!qemu_ram_is_migratable(block)) {} else
>
> total: 2 errors, 0 warnings, 167 lines checked

To be fair, I don't know how to fix this outside of:

BEGIN_RAMBLOCK_FOREACH(...)

END_RAMBLOCK_FOREACH()

type of construct.  I was wondering about what could happen if the
RAMBLOCK_FOREACH was insied an if, but checkpatch beated me to it.

Later, Juan.

Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Peter Maydell, 6 days ago
On 13 April 2018 at 12:18, Juan Quintela <quintela@redhat.com> wrote:
> no-reply@patchew.org wrote:
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/1: migration: discard non-migratable RAMBlocks...
>> ERROR: Macros with multiple statements should be enclosed in a do - while loop
>> #96: FILE: migration/ram.c:191:
>> +#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
>> +    RAMBLOCK_FOREACH(block)                            \
>> +        if (!qemu_ram_is_migratable(block)) {} else
>>
>> ERROR: trailing statements should be on next line
>> #98: FILE: migration/ram.c:193:
>> +        if (!qemu_ram_is_migratable(block)) {} else
>>
>> total: 2 errors, 0 warnings, 167 lines checked
>
> To be fair, I don't know how to fix this outside of:
>
> BEGIN_RAMBLOCK_FOREACH(...)
>
> END_RAMBLOCK_FOREACH()
>
> type of construct.  I was wondering about what could happen if the
> RAMBLOCK_FOREACH was insied an if, but checkpatch beated me to it.

RAMBLOCK_FOREACH inside if() should work fine here. I don't
think we can expect checkpatch to be able to cope with all
of the complicated macro magic we do, so if we're satisfied
that the macro is doing what we want then we should just
ignore the checkpatch error.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Dr. David Alan Gilbert, 10 hours ago
* Cédric Le Goater (clg@kaod.org) wrote:
> On the POWER9 processor, the XIVE interrupt controller can control
> interrupt sources using MMIO to trigger events, to EOI or to turn off
> the sources. Priority management and interrupt acknowledgment is also
> controlled by MMIO in the presenter sub-engine.
> 
> These MMIO regions are exposed to guests in QEMU with a set of 'ram
> device' memory mappings, similarly to VFIO, and the VMAs are populated
> dynamically with the appropriate pages using a fault handler.
> 
> But, these regions are an issue for migration. We need to discard the
> associated RAMBlocks from the RAM state on the source VM and let the
> destination VM rebuild the memory mappings on the new host in the
> post_load() operation just before resuming the system.
> 
> To achieve this goal, the following introduces a new RAMBlock flag
> RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
> vmstate_unregister_ram() routines. This flag is then used by the
> migration to identify RAMBlocks to discard on the source. Some checks
> are also performed on the destination to make sure nothing invalid was
> sent.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Hi Cédric,
  Yes, this is looking nicer; the macro makes the changes quite small.
A couple of questions:

> ---
> 
>  Changes since v1:
> 
>  - introduced a new RAMBlock flag RAM_MIGRATABLE
>  - fixed RAMBLOCK_FOREACH_MIGRATABLE() macro to handle code that
>    omitted {}. 
> 
>  exec.c                    | 10 ++++++++++
>  include/exec/cpu-common.h |  1 +
>  migration/ram.c           | 42 ++++++++++++++++++++++++++++++++----------
>  3 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 02b1efebb7c3..e9432c06294e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -104,6 +104,9 @@ static MemoryRegion io_mem_unassigned;
>   * (Set during postcopy)
>   */
>  #define RAM_UF_ZEROPAGE (1 << 3)
> +
> +/* RAM can be migrated */
> +#define RAM_MIGRATABLE (1 << 4)
>  #endif
>  
>  #ifdef TARGET_PAGE_BITS_VARY
> @@ -1807,6 +1810,11 @@ void qemu_ram_set_uf_zeroable(RAMBlock *rb)
>      rb->flags |= RAM_UF_ZEROPAGE;
>  }
>  
> +bool qemu_ram_is_migratable(RAMBlock *rb)
> +{
> +    return rb->flags & RAM_MIGRATABLE;
> +}
> +
>  /* Called with iothread lock held.  */
>  void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
>  {
> @@ -1823,6 +1831,7 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev)
>          }
>      }
>      pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
> +    new_block->flags |= RAM_MIGRATABLE;
>  
>      rcu_read_lock();
>      RAMBLOCK_FOREACH(block) {
> @@ -1845,6 +1854,7 @@ void qemu_ram_unset_idstr(RAMBlock *block)
>       */
>      if (block) {
>          memset(block->idstr, 0, sizeof(block->idstr));
> +        block->flags &= ~RAM_MIGRATABLE;
>      }
>  }

Why in qemu_ram_set_idstr and qemu_ram_(un)set_idstr ?  It seems an
odd place to put them.


> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 24d335f95d45..96854519b514 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -75,6 +75,7 @@ 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);
> +bool qemu_ram_is_migratable(RAMBlock *rb);
>  
>  size_t qemu_ram_pagesize(RAMBlock *block);
>  size_t qemu_ram_pagesize_largest(void);
> diff --git a/migration/ram.c b/migration/ram.c
> index 0e90efa09236..89c3accc4e26 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -187,6 +187,11 @@ void ramblock_recv_bitmap_set_range(RAMBlock *rb, void *host_addr,
>                        nr);
>  }
>  
> +/* Should be holding either ram_list.mutex, or the RCU lock. */
> +#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> +    RAMBLOCK_FOREACH(block)                            \
> +        if (!qemu_ram_is_migratable(block)) {} else
> +
>  /*
>   * An outstanding page request, on the source, having been received
>   * and queued
> @@ -780,6 +785,10 @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
>      unsigned long *bitmap = rb->bmap;
>      unsigned long next;
>  
> +    if (!qemu_ram_is_migratable(rb)) {
> +        return size;
> +    }
> +
>      if (rs->ram_bulk_stage && start > 0) {
>          next = start + 1;
>      } else {
> @@ -825,7 +834,7 @@ uint64_t ram_pagesize_summary(void)
>      RAMBlock *block;
>      uint64_t summary = 0;
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          summary |= block->page_size;
>      }
>  
> @@ -849,7 +858,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  
>      qemu_mutex_lock(&rs->bitmap_mutex);
>      rcu_read_lock();
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          migration_bitmap_sync_range(rs, block, 0, block->used_length);
>      }
>      rcu_read_unlock();
> @@ -1499,6 +1508,10 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,
>      size_t pagesize_bits =
>          qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
>  
> +    if (!qemu_ram_is_migratable(pss->block)) {
> +        return 0;
> +    }
> +

We might want to print a diagnostic there - it shouldn't happen right?

>      do {
>          tmppages = ram_save_target_page(rs, pss, last_stage);
>          if (tmppages < 0) {
> @@ -1587,7 +1600,7 @@ uint64_t ram_bytes_total(void)
>      uint64_t total = 0;
>  
>      rcu_read_lock();
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          total += block->used_length;
>      }
>      rcu_read_unlock();
> @@ -1642,7 +1655,7 @@ static void ram_save_cleanup(void *opaque)
>       */
>      memory_global_dirty_log_stop();
>  
> -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          g_free(block->bmap);
>          block->bmap = NULL;
>          g_free(block->unsentmap);
> @@ -1705,7 +1718,7 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms)
>  {
>      struct RAMBlock *block;
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          unsigned long *bitmap = block->bmap;
>          unsigned long range = block->used_length >> TARGET_PAGE_BITS;
>          unsigned long run_start = find_next_zero_bit(bitmap, range, 0);
> @@ -1783,7 +1796,7 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
>      struct RAMBlock *block;
>      int ret;
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          PostcopyDiscardState *pds =
>              postcopy_discard_send_init(ms, block->idstr);
>  
> @@ -1991,7 +2004,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
>      rs->last_sent_block = NULL;
>      rs->last_page = 0;
>  
> -    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          unsigned long pages = block->used_length >> TARGET_PAGE_BITS;
>          unsigned long *bitmap = block->bmap;
>          unsigned long *unsentmap = block->unsentmap;
> @@ -2150,7 +2163,7 @@ static void ram_list_init_bitmaps(void)
>  
>      /* Skip setting bitmap if there is no RAM */
>      if (ram_bytes_total()) {
> -        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> +        RAMBLOCK_FOREACH_MIGRATABLE(block) {
>              pages = block->max_length >> TARGET_PAGE_BITS;
>              block->bmap = bitmap_new(pages);
>              bitmap_set(block->bmap, 0, pages);
> @@ -2226,7 +2239,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  
>      qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          qemu_put_byte(f, strlen(block->idstr));
>          qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
>          qemu_put_be64(f, block->used_length);

Good, that's all quite neat;  we've just got to watch out we don't
accidentally add any RAMBLOCK_FOREACH's back.

> @@ -2471,6 +2484,11 @@ static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
>          return NULL;
>      }
>  
> +    if (!qemu_ram_is_migratable(block)) {
> +        error_report("block %s should not be migrated !", id);
> +        return NULL;
> +    }
> +
>      return block;
>  }
>  
> @@ -2921,7 +2939,11 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>                  length = qemu_get_be64(f);
>  
>                  block = qemu_ram_block_by_name(id);
> -                if (block) {
> +                if (block && !qemu_ram_is_migratable(block)) {
> +                    error_report("block %s should not be migrated !", id);
> +                    ret = -EINVAL;
> +
> +                } else if (block) {
>                      if (length != block->used_length) {
>                          Error *local_err = NULL;
>  
> -- 
> 2.13.6

Dave

> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Peter Maydell, 9 hours ago
On 13 April 2018 at 08:52, Cédric Le Goater <clg@kaod.org> wrote:
> On the POWER9 processor, the XIVE interrupt controller can control
> interrupt sources using MMIO to trigger events, to EOI or to turn off
> the sources. Priority management and interrupt acknowledgment is also
> controlled by MMIO in the presenter sub-engine.
>
> These MMIO regions are exposed to guests in QEMU with a set of 'ram
> device' memory mappings, similarly to VFIO, and the VMAs are populated
> dynamically with the appropriate pages using a fault handler.
>
> But, these regions are an issue for migration. We need to discard the
> associated RAMBlocks from the RAM state on the source VM and let the
> destination VM rebuild the memory mappings on the new host in the
> post_load() operation just before resuming the system.
>
> To achieve this goal, the following introduces a new RAMBlock flag
> RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
> vmstate_unregister_ram() routines. This flag is then used by the
> migration to identify RAMBlocks to discard on the source. Some checks
> are also performed on the destination to make sure nothing invalid was
> sent.

I think in theory this should Just Work for allowing us to
enable migration when the xilinx-spips device is using its
execute-from-mmio feature (we would just need to remove the
code that puts in the migration blocker).

Xilinx folks -- do you have a test image for a board that uses
xilinx-spips and exercises the execute-from-mmio code, that
we can use to test migration/vmsave with?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] migration: discard non-migratable RAMBlocks
Posted by Edgar E. Iglesias, 9 hours ago
On Thu, Apr 19, 2018 at 06:32:07PM +0100, Peter Maydell wrote:
> On 13 April 2018 at 08:52, Cédric Le Goater <clg@kaod.org> wrote:
> > On the POWER9 processor, the XIVE interrupt controller can control
> > interrupt sources using MMIO to trigger events, to EOI or to turn off
> > the sources. Priority management and interrupt acknowledgment is also
> > controlled by MMIO in the presenter sub-engine.
> >
> > These MMIO regions are exposed to guests in QEMU with a set of 'ram
> > device' memory mappings, similarly to VFIO, and the VMAs are populated
> > dynamically with the appropriate pages using a fault handler.
> >
> > But, these regions are an issue for migration. We need to discard the
> > associated RAMBlocks from the RAM state on the source VM and let the
> > destination VM rebuild the memory mappings on the new host in the
> > post_load() operation just before resuming the system.
> >
> > To achieve this goal, the following introduces a new RAMBlock flag
> > RAM_MIGRATABLE which is updated in the vmstate_register_ram() and
> > vmstate_unregister_ram() routines. This flag is then used by the
> > migration to identify RAMBlocks to discard on the source. Some checks
> > are also performed on the destination to make sure nothing invalid was
> > sent.
> 
> I think in theory this should Just Work for allowing us to
> enable migration when the xilinx-spips device is using its
> execute-from-mmio feature (we would just need to remove the
> code that puts in the migration blocker).
> 
> Xilinx folks -- do you have a test image for a board that uses
> xilinx-spips and exercises the execute-from-mmio code, that
> we can use to test migration/vmsave with?

CC:ing Fred

I got an image from Fred a while back ago, I probably still have it
somewhere but I've totally forgotten the name for it.

Fred, do you have the image or roughly remember the filename?

Cheers,
Edgar