All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frediano Ziglio <freddy77@gmail.com>
To: xen-devel@lists.xenproject.org
Cc: "Edwin Török" <edwin.torok@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Teddy Astie" <teddy.astie@vates.tech>,
	"Anthony PERARD" <anthony.perard@vates.tech>,
	"Juergen Gross" <jgross@suse.com>,
	"Frediano Ziglio" <frediano.ziglio@citrix.com>
Subject: [PATCH v5 11/16] PoC: libs/guest: use foreign copy during migration
Date: Sat, 13 Jun 2026 22:47:44 +0100	[thread overview]
Message-ID: <20260613214749.20620-12-frediano.ziglio@cloud.com> (raw)
In-Reply-To: <20260613214749.20620-1-frediano.ziglio@cloud.com>

From: Edwin Török <edwin.torok@citrix.com>

ministat confirms the improvement:

```
x baseline
+ foreigncopy
    N           Min           Max        Median           Avg        Stddev
x  20     1.1306997     1.1447931     1.1356569     1.1365742   0.003242175
+  20     0.4311504    0.44180303    0.43616705    0.43600089  0.0031094689
Difference at 95.0% confidence
	-0.700573 +/- 0.00203311
	-61.639% +/- 0.133355%
	(Student's t, pooled s = 0.00317652)
```

The tests pass too, which means that it has correctly migrated all guest
memory.

Frediano: This PoC was adapted to be included in a final series.

Signed-off-by: Edwin Török <edwin.torok@citrix.com>
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
---
 tools/libs/guest/xg_sr_common.h  |  1 +
 tools/libs/guest/xg_sr_restore.c | 43 +++--------------
 tools/libs/guest/xg_sr_save.c    | 82 +++++++++-----------------------
 3 files changed, 31 insertions(+), 95 deletions(-)

diff --git a/tools/libs/guest/xg_sr_common.h b/tools/libs/guest/xg_sr_common.h
index e37f805240..d8d8a0f9f7 100644
--- a/tools/libs/guest/xg_sr_common.h
+++ b/tools/libs/guest/xg_sr_common.h
@@ -256,6 +256,7 @@ struct xc_sr_context
             unsigned long nr_deferred_pages;
             xc_hypercall_buffer_t dirty_bitmap_hbuf;
             struct xc_sr_context_save_buffers *buffers;
+            void *dest_buf;
         } save;
 
         struct /* Restore data. */
diff --git a/tools/libs/guest/xg_sr_restore.c b/tools/libs/guest/xg_sr_restore.c
index fb46142d87..ff27560ff7 100644
--- a/tools/libs/guest/xg_sr_restore.c
+++ b/tools/libs/guest/xg_sr_restore.c
@@ -259,8 +259,8 @@ static int process_page_data(struct xc_sr_context *ctx, unsigned int count,
     xen_pfn_t *mfns = malloc(count * sizeof(*mfns));
     int *map_errs = malloc(count * sizeof(*map_errs));
     int rc;
-    void *mapping = NULL, *guest_page = NULL;
     unsigned nr_pages;
+    void *const source = page_data;
 
     if ( !mfns || !map_errs )
     {
@@ -295,27 +295,8 @@ static int process_page_data(struct xc_sr_context *ctx, unsigned int count,
     if ( nr_pages == 0 )
         goto done;
 
-    mapping = guest_page = xenforeignmemory_map(
-        xch->fmem, ctx->domid, PROT_READ | PROT_WRITE,
-        nr_pages, mfns, map_errs);
-    if ( !mapping )
-    {
-        rc = -1;
-        PERROR("Unable to map %u mfns for %u pages of data",
-               nr_pages, count);
-        goto err;
-    }
-
     for ( unsigned i = 0; i < nr_pages; ++i )
     {
-        if ( map_errs[i] )
-        {
-            rc = -1;
-            ERROR("Mapping pfn %#"PRIpfn" (mfn %#"PRIpfn", type %#"PRIx32") failed with %d",
-                  pfns[i], mfns[i], types[i], map_errs[i]);
-            goto err;
-        }
-
         /* Undo page normalisation done by the saver. */
         rc = ctx->restore.ops.localise_page(ctx, types[i], page_data);
         if ( rc )
@@ -325,29 +306,19 @@ static int process_page_data(struct xc_sr_context *ctx, unsigned int count,
             goto err;
         }
 
-        if ( ctx->restore.verify )
-        {
-            /* Verify mode - compare incoming data to what we already have. */
-            if ( memcmp(guest_page, page_data, PAGE_SIZE) )
-                ERROR("verify pfn %#"PRIpfn" failed (type %#"PRIx32")",
-                      pfns[i], types[i] >> XEN_DOMCTL_PFINFO_LTAB_SHIFT);
-        }
-        else
-        {
-            /* Regular mode - copy incoming data into place. */
-            memcpy(guest_page, page_data, PAGE_SIZE);
-        }
-
-        guest_page += PAGE_SIZE;
         page_data += PAGE_SIZE;
     }
+    if ( !ctx->restore.verify )
+    {
+        rc = xg_foreignmemory_copy_to(xch, ctx->domid, nr_pages, mfns, source);
+        if ( rc < 0 )
+            goto err;
+    }
 
  done:
     rc = 0;
 
  err:
-    if ( mapping )
-        xenforeignmemory_unmap(xch->fmem, mapping, nr_pages);
 
     free(map_errs);
     free(mfns);
diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
index 8a22267fdf..7a48f6b0a3 100644
--- a/tools/libs/guest/xg_sr_save.c
+++ b/tools/libs/guest/xg_sr_save.c
@@ -87,12 +87,10 @@ static int write_batch(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
     xen_pfn_t *mfns, *types;
-    void *guest_mapping = NULL;
     void **local_pages;
     int *errors, rc = -1;
-    unsigned int i, p, nr_pages = 0, nr_pages_mapped = 0;
+    unsigned int i, nr_pages = 0;
     unsigned int nr_pfns = ctx->save.nr_batch_pfns;
-    void *page, *orig_page;
     uint64_t *rec_pfns;
     struct iovec *iov; int iovcnt = 0;
     struct {
@@ -167,62 +165,18 @@ static int write_batch(struct xc_sr_context *ctx)
 
     iovcnt = 2;
 
-    if ( nr_pages > 0 )
+    rc = xg_foreignmemory_copy_from(xch, ctx->domid, nr_pages, ctx->save.dest_buf, mfns);
+    if ( rc < 0 )
     {
-        guest_mapping = xenforeignmemory_map(
-            xch->fmem, ctx->domid, PROT_READ, nr_pages, mfns, errors);
-        if ( !guest_mapping )
-        {
-            PERROR("Failed to map guest pages");
-            goto err;
-        }
-        nr_pages_mapped = nr_pages;
-
-        for ( i = 0, p = 0; i < nr_pfns; ++i )
-        {
-            if ( !page_type_has_stream_data(types[i]) )
-                continue;
-
-            if ( errors[p] )
-            {
-                ERROR("Mapping of pfn %#"PRIpfn" (mfn %#"PRIpfn") failed %d",
-                      ctx->save.batch_pfns[i], mfns[p], errors[p]);
-                goto err;
-            }
-
-            orig_page = page = guest_mapping + (p * PAGE_SIZE);
-            rc = ctx->save.ops.normalise_page(ctx, types[i], &page);
-
-            if ( orig_page != page )
-                local_pages[i] = page;
-
-            if ( rc )
-            {
-                if ( rc == -1 && errno == EAGAIN )
-                {
-                    set_bit(ctx->save.batch_pfns[i], ctx->save.deferred_pages);
-                    ++ctx->save.nr_deferred_pages;
-                    types[i] = XEN_DOMCTL_PFINFO_XTAB;
-                    --nr_pages;
-                }
-                else
-                    goto err;
-            }
-            else if ( iov[iovcnt - 1].iov_base + iov[iovcnt - 1].iov_len !=
-                      page )
-            {
-                iov[iovcnt].iov_base = page;
-                iov[iovcnt].iov_len = PAGE_SIZE;
-                iovcnt++;
-            }
-            else
-            {
-                iov[iovcnt - 1].iov_len += PAGE_SIZE;
-            }
+        ERROR("xg_foreignmemory_copy_from failed");
+        goto err;
+    }
 
-            rc = -1;
-            ++p;
-        }
+    if ( nr_pages )
+    {
+        iov[iovcnt].iov_base = ctx->save.dest_buf;
+        iov[iovcnt].iov_len = nr_pages << XC_PAGE_SHIFT;
+        iovcnt++;
     }
 
     hdrs.rec.length += nr_pages * PAGE_SIZE;
@@ -239,8 +193,6 @@ static int write_batch(struct xc_sr_context *ctx)
     rc = ctx->save.nr_batch_pfns = 0;
 
  err:
-    if ( guest_mapping )
-        xenforeignmemory_unmap(xch->fmem, guest_mapping, nr_pages_mapped);
     for ( i = 0; local_pages && i < nr_pfns; ++i )
     {
         free(local_pages[i]);
@@ -765,6 +717,7 @@ static int setup(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
     int rc;
+    const unsigned dest_buf_len = MAX_BATCH_SIZE * XC_PAGE_SIZE;
     DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
                                     &ctx->save.dirty_bitmap_hbuf);
 
@@ -776,6 +729,16 @@ static int setup(struct xc_sr_context *ctx)
         xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
     ctx->save.deferred_pages = bitmap_alloc(ctx->save.p2m_size);
     ctx->save.buffers = calloc(1, sizeof(*ctx->save.buffers));
+    ctx->save.dest_buf = NULL;
+
+    rc = posix_memalign(&ctx->save.dest_buf, XC_PAGE_SIZE, dest_buf_len);
+    if ( rc )
+    {
+        ERROR("Unable to allocate %u bytes of buffer", dest_buf_len);
+        errno = rc;
+        rc = -1;
+        goto err;
+    }
 
     if ( !dirty_bitmap || !ctx->save.deferred_pages || !ctx->save.buffers)
     {
@@ -810,6 +773,7 @@ static void cleanup(struct xc_sr_context *ctx)
                                    NRPAGES(bitmap_size(ctx->save.p2m_size)));
     free(ctx->save.deferred_pages);
     free(ctx->save.buffers);
+    free(ctx->save.dest_buf);
 }
 
 /*
-- 
2.43.0



  parent reply	other threads:[~2026-06-13 21:48 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-13 21:47 [PATCH v5 00/16] xenguest optimisations Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 01/16] libs/guest: Reduce number of parts in write_split_record Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 02/16] libs/guest: Reduce number of I/O vectors in write_batch Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 03/16] " Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 04/16] libs/guest: Use a single write_exact in write_headers Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 05/16] libs/guest: allocate various migration arrays just once Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 06/16] libs/call: cache up to 4 pages in hypercall bounce buffers Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 07/16] libs/guest: avoids using 2 indexes Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 08/16] libs/guest: fill directly iov structure Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 09/16] libs/ctrl: Allows writev_exact to change iov array Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 10/16] libs/guest: add xg_foreignmemory_copy_{from,to} Frediano Ziglio
2026-06-13 21:47 ` Frediano Ziglio [this message]
2026-06-13 21:47 ` [PATCH v5 12/16] xen: implement new foreign copy hypercall Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 13/16] privcmd: Add definition for new Linux privcmd to access new Xen hypercall Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 14/16] libs/guest: use new hypercall if available Frediano Ziglio
2026-06-13 21:47 ` [PATCH v5 15/16] libs/guest: finalize PoC Frediano Ziglio
2026-06-13 21:47 ` [PATCH Linux v5 16/16] xen/privcmd: Add new ABI to allow copying foreign memory Frediano Ziglio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260613214749.20620-12-frediano.ziglio@cloud.com \
    --to=freddy77@gmail.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=edwin.torok@citrix.com \
    --cc=frediano.ziglio@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=teddy.astie@vates.tech \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.