diff mbox

[v7,34/42] Postcopy: Use helpers to map pages during migration

Message ID 1434450415-11339-35-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert June 16, 2015, 10:26 a.m. UTC 
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

In postcopy, the destination guest is running at the same time
as it's receiving pages; as we receive new pages we must put
them into the guests address space atomically to avoid a running
CPU accessing a partially written page.

Use the helpers in postcopy-ram.c to map these pages.

qemu_get_buffer_less_copy is used to avoid a copy out of qemu_file
in the case that postcopy is going to do a copy anyway.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 103 insertions(+), 21 deletions(-)

Comments

Juan Quintela July 14, 2015, 12:34 p.m. UTC | #1 
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> In postcopy, the destination guest is running at the same time
> as it's receiving pages; as we receive new pages we must put
> them into the guests address space atomically to avoid a running
> CPU accessing a partially written page.
>
> Use the helpers in postcopy-ram.c to map these pages.
>
> qemu_get_buffer_less_copy is used to avoid a copy out of qemu_file
> in the case that postcopy is going to do a copy anyway.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> @@ -1742,7 +1752,6 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>              error_report("Ack, bad migration stream!");
>              return NULL;
>          }
> -

Dont' belong here O:-)

>          return memory_region_get_ram_ptr(block->mr) + offset;
>      }
>  
> @@ -1881,6 +1890,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>      int flags = 0, ret = 0;
>      static uint64_t seq_iter;
>      int len = 0;
> +    /*
> +     * System is running in postcopy mode, page inserts to host memory must be
> +     * atomic
> +     */
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    bool postcopy_running = postcopy_state_get(mis) >=
> +                            POSTCOPY_INCOMING_LISTENING;
> +    void *postcopy_host_page = NULL;
> +    bool postcopy_place_needed = false;
> +    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
>  
>      seq_iter++;
>  
> @@ -1896,13 +1915,57 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>      rcu_read_lock();
>      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>          ram_addr_t addr, total_ram_bytes;
> -        void *host;
> +        void *host = 0;
> +        void *page_buffer = 0;
> +        void *postcopy_place_source = 0;

NULL, NULL, NULL?

BTW, do we really need postcopy_place_source?  I think that just doing
s/postcopy_place_source/postcopy_host_page/ would do?

>          uint8_t ch;
> +        bool all_zero = false;
>  
>          addr = qemu_get_be64(f);
>          flags = addr & ~TARGET_PAGE_MASK;
>          addr &= TARGET_PAGE_MASK;
>  
> +        if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
> +                     RAM_SAVE_FLAG_XBZRLE)) {
> +            host = host_from_stream_offset(f, mis, addr, flags);
> +            if (!host) {
> +                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> +                ret = -EINVAL;
> +                break;
> +            }
> +            if (!postcopy_running) {
> +                page_buffer = host;
> +            } else {
> +                /*
> +                 * Postcopy requires that we place whole host pages atomically.
> +                 * To make it atomic, the data is read into a temporary page
> +                 * that's moved into place later.
> +                 * The migration protocol uses,  possibly smaller, target-pages
> +                 * however the source ensures it always sends all the components
> +                 * of a host page in order.
> +                 */
> +                if (!postcopy_host_page) {
> +                    postcopy_host_page = postcopy_get_tmp_page(mis);
> +                }
> +                page_buffer = postcopy_host_page +
> +                              ((uintptr_t)host & ~qemu_host_page_mask);
> +                /* If all TP are zero then we can optimise the place */
> +                if (!((uintptr_t)host & ~qemu_host_page_mask)) {

I don't understand the test, the comment or both :-(

How you arrive from that test that this is a page full of zeros is a
mistery to me :p

Head hurts, would try to convince myself that the rest of changes are ok.
Dr. David Alan Gilbert July 17, 2015, 5:31 p.m. UTC | #2 
* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > In postcopy, the destination guest is running at the same time
> > as it's receiving pages; as we receive new pages we must put
> > them into the guests address space atomically to avoid a running
> > CPU accessing a partially written page.
> >
> > Use the helpers in postcopy-ram.c to map these pages.
> >
> > qemu_get_buffer_less_copy is used to avoid a copy out of qemu_file
> > in the case that postcopy is going to do a copy anyway.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > @@ -1742,7 +1752,6 @@ static inline void *host_from_stream_offset(QEMUFile *f,
> >              error_report("Ack, bad migration stream!");
> >              return NULL;
> >          }
> > -
> 
> Dont' belong here O:-)

Oops, gone.

> >          return memory_region_get_ram_ptr(block->mr) + offset;
> >      }
> >  
> > @@ -1881,6 +1890,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >      int flags = 0, ret = 0;
> >      static uint64_t seq_iter;
> >      int len = 0;
> > +    /*
> > +     * System is running in postcopy mode, page inserts to host memory must be
> > +     * atomic
> > +     */
> > +    MigrationIncomingState *mis = migration_incoming_get_current();
> > +    bool postcopy_running = postcopy_state_get(mis) >=
> > +                            POSTCOPY_INCOMING_LISTENING;
> > +    void *postcopy_host_page = NULL;
> > +    bool postcopy_place_needed = false;
> > +    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
> >  
> >      seq_iter++;
> >  
> > @@ -1896,13 +1915,57 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >      rcu_read_lock();
> >      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> >          ram_addr_t addr, total_ram_bytes;
> > -        void *host;
> > +        void *host = 0;
> > +        void *page_buffer = 0;
> > +        void *postcopy_place_source = 0;
> 
> NULL, NULL, NULL?

Fixed.

> BTW, do we really need postcopy_place_source?  I think that just doing
> s/postcopy_place_source/postcopy_host_page/ would do?

They are not always the same.  In the host-page size = target-page size case
we make use of  qemu_get_buffer_in_place():

+                qemu_get_buffer_in_place(f, (uint8_t **)&postcopy_place_source,
+                                         TARGET_PAGE_SIZE);

depending on the alignment of the buffer in the stream that *may* change
postcopy_place_source to just point into the qemu_file buffer and then we pluck
the data straight out of there without an extra copy.

> >          uint8_t ch;
> > +        bool all_zero = false;
> >  
> >          addr = qemu_get_be64(f);
> >          flags = addr & ~TARGET_PAGE_MASK;
> >          addr &= TARGET_PAGE_MASK;
> >  
> > +        if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
> > +                     RAM_SAVE_FLAG_XBZRLE)) {
> > +            host = host_from_stream_offset(f, mis, addr, flags);
> > +            if (!host) {
> > +                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> > +                ret = -EINVAL;
> > +                break;
> > +            }
> > +            if (!postcopy_running) {
> > +                page_buffer = host;
> > +            } else {
> > +                /*
> > +                 * Postcopy requires that we place whole host pages atomically.
> > +                 * To make it atomic, the data is read into a temporary page
> > +                 * that's moved into place later.
> > +                 * The migration protocol uses,  possibly smaller, target-pages
> > +                 * however the source ensures it always sends all the components
> > +                 * of a host page in order.
> > +                 */
> > +                if (!postcopy_host_page) {
> > +                    postcopy_host_page = postcopy_get_tmp_page(mis);
> > +                }
> > +                page_buffer = postcopy_host_page +
> > +                              ((uintptr_t)host & ~qemu_host_page_mask);
> > +                /* If all TP are zero then we can optimise the place */
> > +                if (!((uintptr_t)host & ~qemu_host_page_mask)) {

Lets include the next line to make it easier to understand:
    +                    all_zero = true;
    +                }

> I don't understand the test, the comment or both :-(
>
> How you arrive from that test that this is a page full of zeros is a
> mistery to me :p

We end up at this code at the start of each target page received
on the stream.  That condition is true if we're at the 1st target page
within a host page - i.e. the bottom bits (qemu_host_page_mask) of
the host address of the page are all zero (the !).  When we are at the 1st target
page in the host page, we initialise a flag 'all_zero' to be true,
which so far must be the case since we've not written anything.  If we
receive any page that's none zero we clear that flag.
If we get to the end of the host-page, find that all_zero is still true, then
we can avoid another copy.

So the important thing here is that we've not yet convinced ourself
that the page is full of zeros; it's just we know we've not read anything
in the host page yet, so we assume it's all zeros until we find out otherwise.

> Head hurts, would try to convince myself that the rest of changes are ok.

Yes, I'll take a week off to recover from the explanation.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Amit Shah July 27, 2015, 7:39 a.m. UTC | #3 
On (Tue) 16 Jun 2015 [11:26:47], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> In postcopy, the destination guest is running at the same time
> as it's receiving pages; as we receive new pages we must put
> them into the guests address space atomically to avoid a running
> CPU accessing a partially written page.
> 
> Use the helpers in postcopy-ram.c to map these pages.
> 
> qemu_get_buffer_less_copy is used to avoid a copy out of qemu_file
> in the case that postcopy is going to do a copy anyway.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


> @@ -1881,6 +1890,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>      int flags = 0, ret = 0;
>      static uint64_t seq_iter;
>      int len = 0;
> +    /*
> +     * System is running in postcopy mode, page inserts to host memory must be
> +     * atomic
> +     */

*If* system is running in postcopy mode ....

> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    bool postcopy_running = postcopy_state_get(mis) >=
> +                            POSTCOPY_INCOMING_LISTENING;
> +    void *postcopy_host_page = NULL;
> +    bool postcopy_place_needed = false;
> +    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
>  
>      seq_iter++;
>  
> @@ -1896,13 +1915,57 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>      rcu_read_lock();
>      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>          ram_addr_t addr, total_ram_bytes;
> -        void *host;
> +        void *host = 0;
> +        void *page_buffer = 0;
> +        void *postcopy_place_source = 0;
>          uint8_t ch;
> +        bool all_zero = false;
>  
>          addr = qemu_get_be64(f);
>          flags = addr & ~TARGET_PAGE_MASK;
>          addr &= TARGET_PAGE_MASK;
>  
> +        if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
> +                     RAM_SAVE_FLAG_XBZRLE)) {
> +            host = host_from_stream_offset(f, mis, addr, flags);
> +            if (!host) {
> +                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> +                ret = -EINVAL;
> +                break;
> +            }

So the host_from_stream_offset was moved here from below.  One
invocation below is still left, which is a bug..

> +            if (!postcopy_running) {
> +                page_buffer = host;
> +            } else {

Instead of this, can we just do:

	   page_buffer = host;
	   if (postcopy_running) {

> +                /*
> +                 * Postcopy requires that we place whole host pages atomically.
> +                 * To make it atomic, the data is read into a temporary page
> +                 * that's moved into place later.
> +                 * The migration protocol uses,  possibly smaller, target-pages
> +                 * however the source ensures it always sends all the components
> +                 * of a host page in order.
> +                 */
> +                if (!postcopy_host_page) {
> +                    postcopy_host_page = postcopy_get_tmp_page(mis);
> +                }
> +                page_buffer = postcopy_host_page +
> +                              ((uintptr_t)host & ~qemu_host_page_mask);
> +                /* If all TP are zero then we can optimise the place */
> +                if (!((uintptr_t)host & ~qemu_host_page_mask)) {
> +                    all_zero = true;
> +                }
> +
> +                /*
> +                 * If it's the last part of a host page then we place the host
> +                 * page
> +                 */
> +                postcopy_place_needed = (((uintptr_t)host + TARGET_PAGE_SIZE) &
> +                                         ~qemu_host_page_mask) == 0;
> +                postcopy_place_source = postcopy_host_page;
> +            }
> +        } else {
> +            postcopy_place_needed = false;
> +        }

... and similar for postcopy_place_needed as well?  It becomes much
easier to read.

>          case RAM_SAVE_FLAG_COMPRESS_PAGE:
> -            host = host_from_stream_offset(f, addr, flags);
> +            all_zero = false;
> +            if (postcopy_running) {
> +                error_report("Compressed RAM in postcopy mode @%zx\n", addr);
> +                return -EINVAL;
> +            }
> +            host = host_from_stream_offset(f, mis, addr, flags);

This line should go (as mentioned above)?


		Amit
Dr. David Alan Gilbert Aug. 6, 2015, 11:22 a.m. UTC | #4 
* Amit Shah (amit.shah@redhat.com) wrote:
> On (Tue) 16 Jun 2015 [11:26:47], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > In postcopy, the destination guest is running at the same time
> > as it's receiving pages; as we receive new pages we must put
> > them into the guests address space atomically to avoid a running
> > CPU accessing a partially written page.
> > 
> > Use the helpers in postcopy-ram.c to map these pages.
> > 
> > qemu_get_buffer_less_copy is used to avoid a copy out of qemu_file
> > in the case that postcopy is going to do a copy anyway.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> 
> > @@ -1881,6 +1890,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >      int flags = 0, ret = 0;
> >      static uint64_t seq_iter;
> >      int len = 0;
> > +    /*
> > +     * System is running in postcopy mode, page inserts to host memory must be
> > +     * atomic
> > +     */
> 
> *If* system is running in postcopy mode ....

Done.

> > +    MigrationIncomingState *mis = migration_incoming_get_current();
> > +    bool postcopy_running = postcopy_state_get(mis) >=
> > +                            POSTCOPY_INCOMING_LISTENING;
> > +    void *postcopy_host_page = NULL;
> > +    bool postcopy_place_needed = false;
> > +    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
> >  
> >      seq_iter++;
> >  
> > @@ -1896,13 +1915,57 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >      rcu_read_lock();
> >      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> >          ram_addr_t addr, total_ram_bytes;
> > -        void *host;
> > +        void *host = 0;
> > +        void *page_buffer = 0;
> > +        void *postcopy_place_source = 0;
> >          uint8_t ch;
> > +        bool all_zero = false;
> >  
> >          addr = qemu_get_be64(f);
> >          flags = addr & ~TARGET_PAGE_MASK;
> >          addr &= TARGET_PAGE_MASK;
> >  
> > +        if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
> > +                     RAM_SAVE_FLAG_XBZRLE)) {
> > +            host = host_from_stream_offset(f, mis, addr, flags);
> > +            if (!host) {
> > +                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
> > +                ret = -EINVAL;
> > +                break;
> > +            }
> 
> So the host_from_stream_offset was moved here from below.  One
> invocation below is still left, which is a bug..

Thanks, fixed.

> > +            if (!postcopy_running) {
> > +                page_buffer = host;
> > +            } else {
> 
> Instead of this, can we just do:
> 
> 	   page_buffer = host;
> 	   if (postcopy_running) {

done.

> > +                /*
> > +                 * Postcopy requires that we place whole host pages atomically.
> > +                 * To make it atomic, the data is read into a temporary page
> > +                 * that's moved into place later.
> > +                 * The migration protocol uses,  possibly smaller, target-pages
> > +                 * however the source ensures it always sends all the components
> > +                 * of a host page in order.
> > +                 */
> > +                if (!postcopy_host_page) {
> > +                    postcopy_host_page = postcopy_get_tmp_page(mis);
> > +                }
> > +                page_buffer = postcopy_host_page +
> > +                              ((uintptr_t)host & ~qemu_host_page_mask);
> > +                /* If all TP are zero then we can optimise the place */
> > +                if (!((uintptr_t)host & ~qemu_host_page_mask)) {
> > +                    all_zero = true;
> > +                }
> > +
> > +                /*
> > +                 * If it's the last part of a host page then we place the host
> > +                 * page
> > +                 */
> > +                postcopy_place_needed = (((uintptr_t)host + TARGET_PAGE_SIZE) &
> > +                                         ~qemu_host_page_mask) == 0;
> > +                postcopy_place_source = postcopy_host_page;
> > +            }
> > +        } else {
> > +            postcopy_place_needed = false;
> > +        }
> 
> ... and similar for postcopy_place_needed as well?  It becomes much
> easier to read.

Done; actually it's just not needed at all - the function entry initialisation
of that flag is sufficient.

> >          case RAM_SAVE_FLAG_COMPRESS_PAGE:
> > -            host = host_from_stream_offset(f, addr, flags);
> > +            all_zero = false;
> > +            if (postcopy_running) {
> > +                error_report("Compressed RAM in postcopy mode @%zx\n", addr);
> > +                return -EINVAL;
> > +            }
> > +            host = host_from_stream_offset(f, mis, addr, flags);
> 
> This line should go (as mentioned above)?

Yes, done.

Dave

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

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 316834b..01a0ab4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1729,7 +1729,17 @@  static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
 /* Must be called from within a rcu critical section.
  * Returns a pointer from within the RCU-protected ram_list.
  */
+/*
+ * Read a RAMBlock ID from the stream f, find the host address of the
+ * start of that block and add on 'offset'
+ *
+ * f: Stream to read from
+ * mis: MigrationIncomingState
+ * offset: Offset within the block
+ * flags: Page flags (mostly to see if it's a continuation of previous block)
+ */
 static inline void *host_from_stream_offset(QEMUFile *f,
+                                            MigrationIncomingState *mis,
                                             ram_addr_t offset,
                                             int flags)
 {
@@ -1742,7 +1752,6 @@  static inline void *host_from_stream_offset(QEMUFile *f,
             error_report("Ack, bad migration stream!");
             return NULL;
         }
-
         return memory_region_get_ram_ptr(block->mr) + offset;
     }
 
@@ -1881,6 +1890,16 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
     int flags = 0, ret = 0;
     static uint64_t seq_iter;
     int len = 0;
+    /*
+     * System is running in postcopy mode, page inserts to host memory must be
+     * atomic
+     */
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    bool postcopy_running = postcopy_state_get(mis) >=
+                            POSTCOPY_INCOMING_LISTENING;
+    void *postcopy_host_page = NULL;
+    bool postcopy_place_needed = false;
+    bool matching_page_sizes = qemu_host_page_size == TARGET_PAGE_SIZE;
 
     seq_iter++;
 
@@ -1896,13 +1915,57 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
     rcu_read_lock();
     while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
         ram_addr_t addr, total_ram_bytes;
-        void *host;
+        void *host = 0;
+        void *page_buffer = 0;
+        void *postcopy_place_source = 0;
         uint8_t ch;
+        bool all_zero = false;
 
         addr = qemu_get_be64(f);
         flags = addr & ~TARGET_PAGE_MASK;
         addr &= TARGET_PAGE_MASK;
 
+        if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
+                     RAM_SAVE_FLAG_XBZRLE)) {
+            host = host_from_stream_offset(f, mis, addr, flags);
+            if (!host) {
+                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
+                ret = -EINVAL;
+                break;
+            }
+            if (!postcopy_running) {
+                page_buffer = host;
+            } else {
+                /*
+                 * Postcopy requires that we place whole host pages atomically.
+                 * To make it atomic, the data is read into a temporary page
+                 * that's moved into place later.
+                 * The migration protocol uses,  possibly smaller, target-pages
+                 * however the source ensures it always sends all the components
+                 * of a host page in order.
+                 */
+                if (!postcopy_host_page) {
+                    postcopy_host_page = postcopy_get_tmp_page(mis);
+                }
+                page_buffer = postcopy_host_page +
+                              ((uintptr_t)host & ~qemu_host_page_mask);
+                /* If all TP are zero then we can optimise the place */
+                if (!((uintptr_t)host & ~qemu_host_page_mask)) {
+                    all_zero = true;
+                }
+
+                /*
+                 * If it's the last part of a host page then we place the host
+                 * page
+                 */
+                postcopy_place_needed = (((uintptr_t)host + TARGET_PAGE_SIZE) &
+                                         ~qemu_host_page_mask) == 0;
+                postcopy_place_source = postcopy_host_page;
+            }
+        } else {
+            postcopy_place_needed = false;
+        }
+
         switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
         case RAM_SAVE_FLAG_MEM_SIZE:
             /* Synchronize RAM block list */
@@ -1941,26 +2004,37 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
             }
             break;
         case RAM_SAVE_FLAG_COMPRESS:
-            host = host_from_stream_offset(f, addr, flags);
-            if (!host) {
-                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
-                ret = -EINVAL;
-                break;
-            }
             ch = qemu_get_byte(f);
-            ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
+            if (!postcopy_running) {
+                ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
+            } else {
+                memset(page_buffer, ch, TARGET_PAGE_SIZE);
+                if (ch) {
+                    all_zero = false;
+                }
+            }
             break;
+
         case RAM_SAVE_FLAG_PAGE:
-            host = host_from_stream_offset(f, addr, flags);
-            if (!host) {
-                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
-                ret = -EINVAL;
-                break;
+            all_zero = false;
+            if (!postcopy_place_needed || !matching_page_sizes) {
+                qemu_get_buffer(f, page_buffer, TARGET_PAGE_SIZE);
+            } else {
+                /* Avoids the qemu_file copy during postcopy, which is
+                 * going to do a copy later; can only do it when we
+                 * do this read in one go (matching page sizes)
+                 */
+                qemu_get_buffer_less_copy(f, (uint8_t **)&postcopy_place_source,
+                                          TARGET_PAGE_SIZE);
             }
-            qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
             break;
         case RAM_SAVE_FLAG_COMPRESS_PAGE:
-            host = host_from_stream_offset(f, addr, flags);
+            all_zero = false;
+            if (postcopy_running) {
+                error_report("Compressed RAM in postcopy mode @%zx\n", addr);
+                return -EINVAL;
+            }
+            host = host_from_stream_offset(f, mis, addr, flags);
             if (!host) {
                 error_report("Invalid RAM offset " RAM_ADDR_FMT, addr);
                 ret = -EINVAL;
@@ -1976,12 +2050,12 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
             qemu_get_buffer(f, compressed_data_buf, len);
             decompress_data_with_multi_threads(compressed_data_buf, host, len);
             break;
+
         case RAM_SAVE_FLAG_XBZRLE:
-            host = host_from_stream_offset(f, addr, flags);
-            if (!host) {
-                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
-                ret = -EINVAL;
-                break;
+            all_zero = false;
+            if (postcopy_running) {
+                error_report("XBZRLE RAM block in postcopy mode @%zx\n", addr);
+                return -EINVAL;
             }
             if (load_xbzrle(f, addr, host) < 0) {
                 error_report("Failed to decompress XBZRLE page at "
@@ -2002,6 +2076,14 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 ret = -EINVAL;
             }
         }
+
+        if (postcopy_place_needed) {
+            /* This gets called at the last target page in the host page */
+            ret = postcopy_place_page(mis, host + TARGET_PAGE_SIZE -
+                                           qemu_host_page_size,
+                                      postcopy_place_source,
+                                      all_zero);
+        }
         if (!ret) {
             ret = qemu_file_get_error(f);
         }