All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] support linear p2m list in migrate stream v2
@ 2015-12-16  9:24 Juergen Gross
  2015-12-16  9:24 ` [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function Juergen Gross
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Juergen Gross @ 2015-12-16  9:24 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3
  Cc: Juergen Gross

Add support for the virtual mapped linear p2m list of pv-domains in the
v2 migrate stream. This will allow to migrate domains larger than 512
GB.

Tested with 32- and 64-bit pv-domains both with and without linear p2m
list and with a hvm domain.

Changes in V3:
- Moved defines to header file in patch 2 as requested by Andrew Cooper

Changes in V2:
- Added some sanity tests in patch 2 as suggested by Andrew Cooper
- Modified patch 3 according to Andrew Cooper's requests: rename of
  check_iteration to check_vm_state, call check_vm_state after each
  checkpoint, don't change check_vm_state hook but do the check decision
  internally
- Modified docs/features/migration.pandoc according to changes done in the
  series in patch 4 (requested by Andrew Cooper)

Juergen Gross (4):
  libxc: split mapping p2m leaves into a separate function
  libxc: support of linear p2m list for migration of pv-domains
  libxc: stop migration in case of p2m list structural changes
  libxc: set flag for support of linear p2m list in domain builder

 docs/features/migration.pandoc    |   7 +-
 tools/libxc/xc_dom_compat_linux.c |   2 +-
 tools/libxc/xc_dom_core.c         |   2 +
 tools/libxc/xc_sr_common.h        |  12 ++
 tools/libxc/xc_sr_common_x86_pv.h |   7 +
 tools/libxc/xc_sr_save.c          |   7 +-
 tools/libxc/xc_sr_save_x86_hvm.c  |   7 +
 tools/libxc/xc_sr_save_x86_pv.c   | 304 +++++++++++++++++++++++++++++++++-----
 8 files changed, 304 insertions(+), 44 deletions(-)

-- 
2.6.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function
  2015-12-16  9:24 [PATCH v3 0/4] support linear p2m list in migrate stream v2 Juergen Gross
@ 2015-12-16  9:24 ` Juergen Gross
  2016-01-06 15:39   ` Wei Liu
  2015-12-16  9:24 ` [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains Juergen Gross
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-12-16  9:24 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3
  Cc: Juergen Gross

In order to prepare using the virtual mapped linear p2m list for
migration split mapping of the p2m leaf pages into a separate function.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_save_x86_pv.c | 77 ++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index c8d6f0b..d7acd37 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/tools/libxc/xc_sr_save_x86_pv.c
@@ -68,6 +68,50 @@ static int copy_mfns_from_guest(const struct xc_sr_context *ctx,
 }
 
 /*
+ * Map the p2m leave pages and build an array of their pfns.
+ */
+static int map_p2m_leaves(struct xc_sr_context *ctx, xen_pfn_t *mfns,
+                          size_t n_mfns)
+{
+    xc_interface *xch = ctx->xch;
+    unsigned x;
+
+    ctx->x86_pv.p2m = xc_map_foreign_pages(xch, ctx->domid, PROT_READ,
+                                           mfns, n_mfns);
+    if ( !ctx->x86_pv.p2m )
+    {
+        PERROR("Failed to map p2m frames");
+        return -1;
+    }
+
+    ctx->save.p2m_size = ctx->x86_pv.max_pfn + 1;
+    ctx->x86_pv.p2m_frames = n_mfns;
+    ctx->x86_pv.p2m_pfns = malloc(n_mfns * sizeof(*mfns));
+    if ( !ctx->x86_pv.p2m_pfns )
+    {
+        ERROR("Cannot allocate %zu bytes for p2m pfns list",
+              n_mfns * sizeof(*mfns));
+        return -1;
+    }
+
+    /* Convert leaf frames from mfns to pfns. */
+    for ( x = 0; x < n_mfns; ++x )
+    {
+        if ( !mfn_in_pseudophysmap(ctx, mfns[x]) )
+        {
+            ERROR("Bad mfn in p2m_frame_list[%u]", x);
+            dump_bad_pseudophysmap_entry(ctx, mfns[x]);
+            errno = ERANGE;
+            return -1;
+        }
+
+        ctx->x86_pv.p2m_pfns[x] = mfn_to_pfn(ctx, mfns[x]);
+    }
+
+    return 0;
+}
+
+/*
  * Walk the guests frame list list and frame list to identify and map the
  * frames making up the guests p2m table.  Construct a list of pfns making up
  * the table.
@@ -173,7 +217,6 @@ static int map_p2m(struct xc_sr_context *ctx)
     ctx->x86_pv.p2m_frames = (ctx->x86_pv.max_pfn + fpp) / fpp;
     DPRINTF("max_pfn %#lx, p2m_frames %d", ctx->x86_pv.max_pfn,
             ctx->x86_pv.p2m_frames);
-    ctx->save.p2m_size = ctx->x86_pv.max_pfn + 1;
     fl_entries  = (ctx->x86_pv.max_pfn / fpp) + 1;
 
     /* Map the guest mid p2m frames. */
@@ -211,38 +254,8 @@ static int map_p2m(struct xc_sr_context *ctx)
     }
 
     /* Map the p2m leaves themselves. */
-    ctx->x86_pv.p2m = xc_map_foreign_pages(xch, ctx->domid, PROT_READ,
-                                           local_fl, fl_entries);
-    if ( !ctx->x86_pv.p2m )
-    {
-        PERROR("Failed to map p2m frames");
-        goto err;
-    }
+    rc = map_p2m_leaves(ctx, local_fl, fl_entries);
 
-    ctx->x86_pv.p2m_frames = fl_entries;
-    ctx->x86_pv.p2m_pfns = malloc(local_fl_size);
-    if ( !ctx->x86_pv.p2m_pfns )
-    {
-        ERROR("Cannot allocate %zu bytes for p2m pfns list",
-              local_fl_size);
-        goto err;
-    }
-
-    /* Convert leaf frames from mfns to pfns. */
-    for ( x = 0; x < fl_entries; ++x )
-    {
-        if ( !mfn_in_pseudophysmap(ctx, local_fl[x]) )
-        {
-            ERROR("Bad mfn in p2m_frame_list[%u]", x);
-            dump_bad_pseudophysmap_entry(ctx, local_fl[x]);
-            errno = ERANGE;
-            goto err;
-        }
-
-        ctx->x86_pv.p2m_pfns[x] = mfn_to_pfn(ctx, local_fl[x]);
-    }
-
-    rc = 0;
 err:
 
     free(local_fl);
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains
  2015-12-16  9:24 [PATCH v3 0/4] support linear p2m list in migrate stream v2 Juergen Gross
  2015-12-16  9:24 ` [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function Juergen Gross
@ 2015-12-16  9:24 ` Juergen Gross
  2016-01-06 15:40   ` Wei Liu
  2015-12-16  9:24 ` [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes Juergen Gross
  2015-12-16  9:24 ` [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder Juergen Gross
  3 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-12-16  9:24 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3
  Cc: Juergen Gross

In order to be able to migrate pv-domains with more than 512 GB of RAM
the p2m information can be specified by the guest kernel via a virtual
mapped linear p2m list instead of a 3 level tree.

Add support for this new p2m format in libxc.

As the sanity checking of the virtual p2m address needs defines for the
xen regions use those defines when doing page table checks as well.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_common_x86_pv.h |   7 ++
 tools/libxc/xc_sr_save_x86_pv.c   | 194 +++++++++++++++++++++++++++++++++++---
 2 files changed, 188 insertions(+), 13 deletions(-)

diff --git a/tools/libxc/xc_sr_common_x86_pv.h b/tools/libxc/xc_sr_common_x86_pv.h
index 3234944..f80c753 100644
--- a/tools/libxc/xc_sr_common_x86_pv.h
+++ b/tools/libxc/xc_sr_common_x86_pv.h
@@ -3,6 +3,13 @@
 
 #include "xc_sr_common_x86.h"
 
+/* Virtual address ranges reserved for hypervisor. */
+#define HYPERVISOR_VIRT_START_X86_64 0xFFFF800000000000ULL
+#define HYPERVISOR_VIRT_END_X86_64   0xFFFF87FFFFFFFFFFULL
+
+#define HYPERVISOR_VIRT_START_X86_32 0x00000000F5800000ULL
+#define HYPERVISOR_VIRT_END_X86_32   0x00000000FFFFFFFFULL
+
 /*
  * Convert an mfn to a pfn, given Xen's m2p table.
  *
diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index d7acd37..40aa655 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/tools/libxc/xc_sr_save_x86_pv.c
@@ -3,6 +3,12 @@
 
 #include "xc_sr_common_x86_pv.h"
 
+/* Check a 64 bit virtual address for being canonical. */
+static inline bool is_canonical_address(xen_vaddr_t vaddr)
+{
+    return ((int64_t)vaddr >> 47) == ((int64_t)vaddr >> 63);
+}
+
 /*
  * Maps the guests shared info page.
  */
@@ -116,7 +122,7 @@ static int map_p2m_leaves(struct xc_sr_context *ctx, xen_pfn_t *mfns,
  * frames making up the guests p2m table.  Construct a list of pfns making up
  * the table.
  */
-static int map_p2m(struct xc_sr_context *ctx)
+static int map_p2m_tree(struct xc_sr_context *ctx)
 {
     /* Terminology:
      *
@@ -138,8 +144,6 @@ static int map_p2m(struct xc_sr_context *ctx)
     void *guest_fl = NULL;
     size_t local_fl_size;
 
-    ctx->x86_pv.max_pfn = GET_FIELD(ctx->x86_pv.shinfo, arch.max_pfn,
-                                    ctx->x86_pv.width) - 1;
     fpp = PAGE_SIZE / ctx->x86_pv.width;
     fll_entries = (ctx->x86_pv.max_pfn / (fpp * fpp)) + 1;
     if ( fll_entries > fpp )
@@ -270,6 +274,170 @@ err:
 }
 
 /*
+ * Map the guest p2m frames specified via a cr3 value, a virtual address, and
+ * the maximum pfn. PTE entries are 64 bits vor both, 32 and 64 bit guests as
+ * in 32 bit case we support PAE guests only.
+ */
+static int map_p2m_list(struct xc_sr_context *ctx, uint64_t p2m_cr3)
+{
+    xc_interface *xch = ctx->xch;
+    xen_vaddr_t p2m_vaddr, p2m_end, mask, off;
+    xen_pfn_t p2m_mfn, mfn, saved_mfn, max_pfn;
+    uint64_t *ptes;
+    xen_pfn_t *mfns;
+    unsigned fpp, n_pages, level, shift, idx_start, idx_end, idx, saved_idx;
+    int rc = -1;
+
+    p2m_mfn = cr3_to_mfn(ctx, p2m_cr3);
+    assert(p2m_mfn != 0);
+    if ( p2m_mfn > ctx->x86_pv.max_mfn )
+    {
+        ERROR("Bad p2m_cr3 value %#lx", p2m_cr3);
+        errno = ERANGE;
+        return -1;
+    }
+
+    p2m_vaddr = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_vaddr,
+                          ctx->x86_pv.width);
+    fpp = PAGE_SIZE / ctx->x86_pv.width;
+    ctx->x86_pv.p2m_frames = ctx->x86_pv.max_pfn / fpp + 1;
+    p2m_end = p2m_vaddr + ctx->x86_pv.p2m_frames * PAGE_SIZE - 1;
+
+    if ( ctx->x86_pv.width == 8 )
+    {
+        mask = 0x0000ffffffffffffULL;
+        if ( !is_canonical_address(p2m_vaddr) ||
+             !is_canonical_address(p2m_end) ||
+             p2m_end < p2m_vaddr ||
+             (p2m_vaddr <= HYPERVISOR_VIRT_END_X86_64 &&
+              p2m_end > HYPERVISOR_VIRT_START_X86_64) )
+        {
+            ERROR("Bad virtual p2m address range %#lx-%#lx",
+                  p2m_vaddr, p2m_end);
+            errno = ERANGE;
+            return -1;
+        }
+    }
+    else
+    {
+        mask = 0x00000000ffffffffULL;
+        if ( p2m_vaddr > mask || p2m_end > mask || p2m_end < p2m_vaddr ||
+             (p2m_vaddr <= HYPERVISOR_VIRT_END_X86_32 &&
+              p2m_end > HYPERVISOR_VIRT_START_X86_32) )
+        {
+            ERROR("Bad virtual p2m address range %#lx-%#lx",
+                  p2m_vaddr, p2m_end);
+            errno = ERANGE;
+            return -1;
+        }
+    }
+
+    DPRINTF("p2m list from %#lx to %#lx, root at %#lx", p2m_vaddr, p2m_end,
+            p2m_mfn);
+    DPRINTF("max_pfn %#lx, p2m_frames %d", ctx->x86_pv.max_pfn,
+            ctx->x86_pv.p2m_frames);
+
+    mfns = malloc(sizeof(*mfns));
+    if ( !mfns )
+    {
+        ERROR("Cannot allocate memory for array of %u mfns", 1);
+        goto err;
+    }
+    mfns[0] = p2m_mfn;
+    off = 0;
+    saved_mfn = 0;
+    idx_start = idx_end = saved_idx = 0;
+
+    for ( level = ctx->x86_pv.levels; level > 0; level-- )
+    {
+        n_pages = idx_end - idx_start + 1;
+        ptes = xc_map_foreign_pages(xch, ctx->domid, PROT_READ, mfns, n_pages);
+        if ( !ptes )
+        {
+            PERROR("Failed to map %u page table pages for p2m list", n_pages);
+            goto err;
+        }
+        free(mfns);
+
+        shift = level * 9 + 3;
+        idx_start = ((p2m_vaddr - off) & mask) >> shift;
+        idx_end = ((p2m_end - off) & mask) >> shift;
+        idx = idx_end - idx_start + 1;
+        mfns = malloc(sizeof(*mfns) * idx);
+        if ( !mfns )
+        {
+            ERROR("Cannot allocate memory for array of %u mfns", idx);
+            goto err;
+        }
+
+        for ( idx = idx_start; idx <= idx_end; idx++ )
+        {
+            mfn = pte_to_frame(ptes[idx]);
+            if ( mfn == 0 || mfn > ctx->x86_pv.max_mfn )
+            {
+                ERROR("Bad mfn %#lx during page table walk for vaddr %#lx at level %d of p2m list",
+                      mfn, off + ((xen_vaddr_t)idx << shift), level);
+                errno = ERANGE;
+                goto err;
+            }
+            mfns[idx - idx_start] = mfn;
+
+            /* Maximum pfn check at level 2. Same reasoning as for p2m tree. */
+            if ( level == 2 )
+            {
+                if ( mfn != saved_mfn )
+                {
+                    saved_mfn = mfn;
+                    saved_idx = idx - idx_start;
+                }
+            }
+        }
+
+        if ( level == 2 )
+        {
+            max_pfn = ((xen_pfn_t)saved_idx << 9) * fpp - 1;
+            if ( max_pfn < ctx->x86_pv.max_pfn )
+            {
+                ctx->x86_pv.max_pfn = max_pfn;
+                ctx->x86_pv.p2m_frames = (ctx->x86_pv.max_pfn + fpp) / fpp;
+                p2m_end = p2m_vaddr + ctx->x86_pv.p2m_frames * PAGE_SIZE - 1;
+                idx_end = idx_start + saved_idx;
+            }
+        }
+
+        munmap(ptes, n_pages * PAGE_SIZE);
+        ptes = NULL;
+        off = p2m_vaddr & ((mask >> shift) << shift);
+    }
+
+    /* Map the p2m leaves themselves. */
+    rc = map_p2m_leaves(ctx, mfns, idx_end - idx_start + 1);
+
+err:
+    free(mfns);
+    if ( ptes )
+        munmap(ptes, n_pages * PAGE_SIZE);
+
+    return rc;
+}
+
+/*
+ * Map the guest p2m frames.
+ * Depending on guest support this might either be a virtual mapped linear
+ * list (preferred format) or a 3 level tree linked via mfns.
+ */
+static int map_p2m(struct xc_sr_context *ctx)
+{
+    uint64_t p2m_cr3;
+
+    ctx->x86_pv.max_pfn = GET_FIELD(ctx->x86_pv.shinfo, arch.max_pfn,
+                                    ctx->x86_pv.width) - 1;
+    p2m_cr3 = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_cr3, ctx->x86_pv.width);
+
+    return p2m_cr3 ? map_p2m_list(ctx, p2m_cr3) : map_p2m_tree(ctx);
+}
+
+/*
  * Obtain a specific vcpus basic state and write an X86_PV_VCPU_BASIC record
  * into the stream.  Performs mfn->pfn conversion on architectural state.
  */
@@ -681,8 +849,10 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
         /* 64bit guests only have Xen mappings in their L4 tables. */
         if ( type == XEN_DOMCTL_PFINFO_L4TAB )
         {
-            xen_first = 256;
-            xen_last = 271;
+            xen_first = (HYPERVISOR_VIRT_START_X86_64 >>
+                         L4_PAGETABLE_SHIFT_X86_64) & 511;
+            xen_last = (HYPERVISOR_VIRT_END_X86_64 >>
+                        L4_PAGETABLE_SHIFT_X86_64) & 511;
         }
     }
     else
@@ -698,21 +868,19 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
             /* 32bit guests can only use the first 4 entries of their L3 tables.
              * All other are potentially used by Xen. */
             xen_first = 4;
-            xen_last = 512;
+            xen_last = 511;
             break;
 
         case XEN_DOMCTL_PFINFO_L2TAB:
             /* It is hard to spot Xen mappings in a 32bit guest's L2.  Most
              * are normal but only a few will have Xen mappings.
-             *
-             * 428 = (HYPERVISOR_VIRT_START_PAE >> L2_PAGETABLE_SHIFT_PAE)&0x1ff
-             *
-             * ...which is conveniently unavailable to us in a 64bit build.
              */
-            if ( pte_to_frame(src[428]) == ctx->x86_pv.compat_m2p_mfn0 )
+            i = (HYPERVISOR_VIRT_START_X86_32 >> L2_PAGETABLE_SHIFT_PAE) & 511;
+            if ( pte_to_frame(src[i]) == ctx->x86_pv.compat_m2p_mfn0 )
             {
-                xen_first = 428;
-                xen_last = 512;
+                xen_first = i;
+                xen_last = (HYPERVISOR_VIRT_END_X86_32 >>
+                            L2_PAGETABLE_SHIFT_PAE) & 511;
             }
             break;
         }
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes
  2015-12-16  9:24 [PATCH v3 0/4] support linear p2m list in migrate stream v2 Juergen Gross
  2015-12-16  9:24 ` [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function Juergen Gross
  2015-12-16  9:24 ` [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains Juergen Gross
@ 2015-12-16  9:24 ` Juergen Gross
  2016-01-06 15:40   ` Wei Liu
  2015-12-16  9:24 ` [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder Juergen Gross
  3 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-12-16  9:24 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3
  Cc: Juergen Gross

With support of the virtual mapped linear p2m list for migration it is
now possible to detect structural changes of the p2m list which before
would either lead to a crashing or otherwise wrong behaving domU.

A guest supporting the linear p2m list will increment the
p2m_generation counter located in the shared info page before and after
each modification of a mapping related to the p2m list. A change of
that counter can be detected by the tools and reacted upon.

As such a change should occur only very rarely once the domU is up the
most simple reaction is to cancel migration in such an event.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_common.h       | 12 +++++++++++
 tools/libxc/xc_sr_save.c         |  7 ++++++-
 tools/libxc/xc_sr_save_x86_hvm.c |  7 +++++++
 tools/libxc/xc_sr_save_x86_pv.c  | 45 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 9aecde2..60b43e8 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -83,6 +83,15 @@ struct xc_sr_save_ops
     int (*end_of_checkpoint)(struct xc_sr_context *ctx);
 
     /**
+     * Check state of guest to decide whether it makes sense to continue
+     * migration.  This is called in each iteration or checkpoint to check
+     * whether all criteria for the migration are still met.  If that's not
+     * the case either migration is cancelled via a bad rc or the situation
+     * is handled, e.g. by sending appropriate records.
+     */
+    int (*check_vm_state)(struct xc_sr_context *ctx);
+
+    /**
      * Clean up the local environment.  Will be called exactly once, either
      * after a successful save, or upon encountering an error.
      */
@@ -280,6 +289,9 @@ struct xc_sr_context
             /* Read-only mapping of guests shared info page */
             shared_info_any_t *shinfo;
 
+            /* p2m generation count for verifying validity of local p2m. */
+            uint64_t p2m_generation;
+
             union
             {
                 struct
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index cefcef5..88d85ef 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -394,7 +394,8 @@ static int send_dirty_pages(struct xc_sr_context *ctx,
         DPRINTF("Bitmap contained more entries than expected...");
 
     xc_report_progress_step(xch, entries, entries);
-    return 0;
+
+    return ctx->save.ops.check_vm_state(ctx);
 }
 
 /*
@@ -751,6 +752,10 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
         if ( rc )
             goto err;
 
+        rc = ctx->save.ops.check_vm_state(ctx);
+        if ( rc )
+            goto err;
+
         if ( ctx->save.live )
             rc = send_domain_memory_live(ctx);
         else if ( ctx->save.checkpointed )
diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
index f3d6cee..e347b3b 100644
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/tools/libxc/xc_sr_save_x86_hvm.c
@@ -175,6 +175,12 @@ static int x86_hvm_start_of_checkpoint(struct xc_sr_context *ctx)
     return 0;
 }
 
+static int x86_hvm_check_vm_state(struct xc_sr_context *ctx)
+{
+    /* no-op */
+    return 0;
+}
+
 static int x86_hvm_end_of_checkpoint(struct xc_sr_context *ctx)
 {
     int rc;
@@ -221,6 +227,7 @@ struct xc_sr_save_ops save_ops_x86_hvm =
     .start_of_stream     = x86_hvm_start_of_stream,
     .start_of_checkpoint = x86_hvm_start_of_checkpoint,
     .end_of_checkpoint   = x86_hvm_end_of_checkpoint,
+    .check_vm_state      = x86_hvm_check_vm_state,
     .cleanup             = x86_hvm_cleanup,
 };
 
diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index 40aa655..a0ec5c9 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/tools/libxc/xc_sr_save_x86_pv.c
@@ -274,6 +274,39 @@ err:
 }
 
 /*
+ * Get p2m_generation count.
+ * Returns an error if the generation count has changed since the last call.
+ */
+static int get_p2m_generation(struct xc_sr_context *ctx)
+{
+    uint64_t p2m_generation;
+    int rc;
+
+    p2m_generation = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_generation,
+                               ctx->x86_pv.width);
+
+    rc = (p2m_generation == ctx->x86_pv.p2m_generation) ? 0 : -1;
+    ctx->x86_pv.p2m_generation = p2m_generation;
+
+    return rc;
+}
+
+static int x86_pv_check_vm_state_p2m_list(struct xc_sr_context *ctx)
+{
+    xc_interface *xch = ctx->xch;
+    int rc;
+
+    if ( !ctx->save.live )
+        return 0;
+
+    rc = get_p2m_generation(ctx);
+    if ( rc )
+        ERROR("p2m generation count changed. Migration aborted.");
+
+    return rc;
+}
+
+/*
  * Map the guest p2m frames specified via a cr3 value, a virtual address, and
  * the maximum pfn. PTE entries are 64 bits vor both, 32 and 64 bit guests as
  * in 32 bit case we support PAE guests only.
@@ -297,6 +330,8 @@ static int map_p2m_list(struct xc_sr_context *ctx, uint64_t p2m_cr3)
         return -1;
     }
 
+    get_p2m_generation(ctx);
+
     p2m_vaddr = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_vaddr,
                           ctx->x86_pv.width);
     fpp = PAGE_SIZE / ctx->x86_pv.width;
@@ -430,6 +465,7 @@ static int map_p2m(struct xc_sr_context *ctx)
 {
     uint64_t p2m_cr3;
 
+    ctx->x86_pv.p2m_generation = ~0ULL;
     ctx->x86_pv.max_pfn = GET_FIELD(ctx->x86_pv.shinfo, arch.max_pfn,
                                     ctx->x86_pv.width) - 1;
     p2m_cr3 = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_cr3, ctx->x86_pv.width);
@@ -1069,6 +1105,14 @@ static int x86_pv_end_of_checkpoint(struct xc_sr_context *ctx)
     return 0;
 }
 
+static int x86_pv_check_vm_state(struct xc_sr_context *ctx)
+{
+    if ( ctx->x86_pv.p2m_generation == ~0ULL )
+        return 0;
+
+    return x86_pv_check_vm_state_p2m_list(ctx);
+}
+
 /*
  * save_ops function.  Cleanup.
  */
@@ -1096,6 +1140,7 @@ struct xc_sr_save_ops save_ops_x86_pv =
     .start_of_stream     = x86_pv_start_of_stream,
     .start_of_checkpoint = x86_pv_start_of_checkpoint,
     .end_of_checkpoint   = x86_pv_end_of_checkpoint,
+    .check_vm_state      = x86_pv_check_vm_state,
     .cleanup             = x86_pv_cleanup,
 };
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder
  2015-12-16  9:24 [PATCH v3 0/4] support linear p2m list in migrate stream v2 Juergen Gross
                   ` (2 preceding siblings ...)
  2015-12-16  9:24 ` [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes Juergen Gross
@ 2015-12-16  9:24 ` Juergen Gross
  2016-01-06 15:40   ` Wei Liu
  2016-01-06 16:05   ` Ian Campbell
  3 siblings, 2 replies; 14+ messages in thread
From: Juergen Gross @ 2015-12-16  9:24 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3
  Cc: Juergen Gross

Set the SIF_VIRT_P2M_4TOOLS flag for pv-domUs in the domain builder
to indicate the Xen tools have full support for the virtual mapped
linear p2m list.

This will enable pv-domUs to drop support of the 3 level p2m tree
and use the linear list only. Without setting this flag some kernels
might limit themselves to 512 GB memory size in order not to break
migration.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 docs/features/migration.pandoc    | 7 ++++---
 tools/libxc/xc_dom_compat_linux.c | 2 +-
 tools/libxc/xc_dom_core.c         | 2 ++
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/features/migration.pandoc b/docs/features/migration.pandoc
index 9852a19..151c50d 100644
--- a/docs/features/migration.pandoc
+++ b/docs/features/migration.pandoc
@@ -1,5 +1,5 @@
 % Migration
-% Revision 1
+% Revision 2
 
 \clearpage
 
@@ -95,7 +95,6 @@ scenarios, which will involve starting with VMs from Xen 4.5
 # Areas for improvement
 
 * Arm support
-* Linear P2M support for x86 PV
 * Live looping parameters
 
 # Known issues
@@ -105,7 +104,8 @@ scenarios, which will involve starting with VMs from Xen 4.5
 * x86 HVM with nested-virt (no relevant information included in the
   stream)
 * x86 PV ballooning (P2M marked dirty, target frame not marked)
-* x86 PV P2M structure changes (not noticed, stale mappings used)
+* x86 PV P2M structure changes (not noticed, stale mappings used) for
+  guests not using the linear p2m layout
 
 # References
 
@@ -120,4 +120,5 @@ for Migration v2
 Date       Revision Version  Notes
 ---------- -------- -------- -------------------------------------------
 2015-10-24 1        Xen 4.6  Document written
+2015-12-11 2        Xen 4.7  Support of linear p2m list
 ---------- -------- -------- -------------------------------------------
diff --git a/tools/libxc/xc_dom_compat_linux.c b/tools/libxc/xc_dom_compat_linux.c
index abbc09f..c922c61 100644
--- a/tools/libxc/xc_dom_compat_linux.c
+++ b/tools/libxc/xc_dom_compat_linux.c
@@ -59,7 +59,7 @@ int xc_linux_build(xc_interface *xch, uint32_t domid,
          ((rc = xc_dom_ramdisk_file(dom, initrd_name)) != 0) )
         goto out;
 
-    dom->flags = flags;
+    dom->flags |= flags;
     dom->console_evtchn = console_evtchn;
     dom->xenstore_evtchn = store_evtchn;
 
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 2061ba6..55c779d 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -777,6 +777,8 @@ struct xc_dom_image *xc_dom_allocate(xc_interface *xch,
     dom->parms.elf_paddr_offset = UNSET_ADDR;
     dom->parms.p2m_base = UNSET_ADDR;
 
+    dom->flags = SIF_VIRT_P2M_4TOOLS;
+
     dom->alloc_malloc += sizeof(*dom);
     return dom;
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function
  2015-12-16  9:24 ` [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function Juergen Gross
@ 2016-01-06 15:39   ` Wei Liu
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2016-01-06 15:39 UTC (permalink / raw)
  To: Juergen Gross
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel

On Wed, Dec 16, 2015 at 10:24:17AM +0100, Juergen Gross wrote:
> In order to prepare using the virtual mapped linear p2m list for
> migration split mapping of the p2m leaf pages into a separate function.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains
  2015-12-16  9:24 ` [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains Juergen Gross
@ 2016-01-06 15:40   ` Wei Liu
  2016-01-07 10:21     ` Juergen Gross
  0 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2016-01-06 15:40 UTC (permalink / raw)
  To: Juergen Gross
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel

On Wed, Dec 16, 2015 at 10:24:18AM +0100, Juergen Gross wrote:
[...]
> @@ -698,21 +868,19 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
>              /* 32bit guests can only use the first 4 entries of their L3 tables.
>               * All other are potentially used by Xen. */
>              xen_first = 4;
> -            xen_last = 512;
> +            xen_last = 511;

Is this a bug fix in its own right?

Wei.

>              break;
>  
>          case XEN_DOMCTL_PFINFO_L2TAB:
>              /* It is hard to spot Xen mappings in a 32bit guest's L2.  Most
>               * are normal but only a few will have Xen mappings.
> -             *
> -             * 428 = (HYPERVISOR_VIRT_START_PAE >> L2_PAGETABLE_SHIFT_PAE)&0x1ff
> -             *
> -             * ...which is conveniently unavailable to us in a 64bit build.
>               */
> -            if ( pte_to_frame(src[428]) == ctx->x86_pv.compat_m2p_mfn0 )
> +            i = (HYPERVISOR_VIRT_START_X86_32 >> L2_PAGETABLE_SHIFT_PAE) & 511;
> +            if ( pte_to_frame(src[i]) == ctx->x86_pv.compat_m2p_mfn0 )
>              {
> -                xen_first = 428;
> -                xen_last = 512;
> +                xen_first = i;
> +                xen_last = (HYPERVISOR_VIRT_END_X86_32 >>
> +                            L2_PAGETABLE_SHIFT_PAE) & 511;
>              }
>              break;
>          }
> -- 
> 2.6.2
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes
  2015-12-16  9:24 ` [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes Juergen Gross
@ 2016-01-06 15:40   ` Wei Liu
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2016-01-06 15:40 UTC (permalink / raw)
  To: Juergen Gross
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel

On Wed, Dec 16, 2015 at 10:24:19AM +0100, Juergen Gross wrote:
> With support of the virtual mapped linear p2m list for migration it is
> now possible to detect structural changes of the p2m list which before
> would either lead to a crashing or otherwise wrong behaving domU.
> 
> A guest supporting the linear p2m list will increment the
> p2m_generation counter located in the shared info page before and after
> each modification of a mapping related to the p2m list. A change of
> that counter can be detected by the tools and reacted upon.
> 
> As such a change should occur only very rarely once the domU is up the
> most simple reaction is to cancel migration in such an event.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder
  2015-12-16  9:24 ` [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder Juergen Gross
@ 2016-01-06 15:40   ` Wei Liu
  2016-01-06 16:05   ` Ian Campbell
  1 sibling, 0 replies; 14+ messages in thread
From: Wei Liu @ 2016-01-06 15:40 UTC (permalink / raw)
  To: Juergen Gross
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel

On Wed, Dec 16, 2015 at 10:24:20AM +0100, Juergen Gross wrote:
> Set the SIF_VIRT_P2M_4TOOLS flag for pv-domUs in the domain builder
> to indicate the Xen tools have full support for the virtual mapped
> linear p2m list.
> 
> This will enable pv-domUs to drop support of the 3 level p2m tree
> and use the linear list only. Without setting this flag some kernels
> might limit themselves to 512 GB memory size in order not to break
> migration.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder
  2015-12-16  9:24 ` [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder Juergen Gross
  2016-01-06 15:40   ` Wei Liu
@ 2016-01-06 16:05   ` Ian Campbell
  2016-01-07  7:02     ` Juergen Gross
  1 sibling, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2016-01-06 16:05 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3

On Wed, 2015-12-16 at 10:24 +0100, Juergen Gross wrote:
> Set the SIF_VIRT_P2M_4TOOLS flag for pv-domUs in the domain builder
> to indicate the Xen tools have full support for the virtual mapped
> linear p2m list.
> 
> This will enable pv-domUs to drop support of the 3 level p2m tree
> and use the linear list only. Without setting this flag some kernels
> might limit themselves to 512 GB memory size in order not to break
> migration.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I would have applied all 4 on the basis of Andy's other Reviewed-by to the
patches purely touching migration v2 files, except for this on 32-bit:

xc_sr_save_x86_pv.c:328:9: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
         ERROR("Bad p2m_cr3 value %#lx", p2m_cr3);
         ^
xc_sr_save_x86_pv.c:350:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘xen_vaddr_t’ [-Werror=format=]
             ERROR("Bad virtual p2m address range %#lx-%#lx",
             ^
xc_sr_save_x86_pv.c:350:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘xen_vaddr_t’ [-Werror=format=]
xc_sr_save_x86_pv.c:363:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘xen_vaddr_t’ [-Werror=format=]
             ERROR("Bad virtual p2m address range %#lx-%#lx",
             ^
xc_sr_save_x86_pv.c:363:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘xen_vaddr_t’ [-Werror=format=]
xc_sr_save_x86_pv.c:370:5: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘xen_vaddr_t’ [-Werror=format=]
     DPRINTF("p2m list from %#lx to %#lx, root at %#lx", p2m_vaddr, p2m_end,
     ^
xc_sr_save_x86_pv.c:370:5: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘xen_vaddr_t’ [-Werror=format=]
xc_sr_save_x86_pv.c:413:17: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘long long unsigned int’ [-Werror=format=]
                 ERROR("Bad mfn %#lx during page table walk for vaddr %#lx at level %d of p2m list",
                 ^
cc1: all warnings being treated as errors
/local/scratch/ianc/devel/committer-i686.git/tools/libxc/../../tools/Rules.mk:163: recipe for target 'xc_sr_save_x86_pv.o' failed

> @@ -1,5 +1,5 @@
>  % Migration
> -% Revision 1
> +% Revision 2

Well done on this, I predict it will get forgotten more often than not ;-)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder
  2016-01-06 16:05   ` Ian Campbell
@ 2016-01-07  7:02     ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2016-01-07  7:02 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini,
	wei.liu2, andrew.cooper3

On 06/01/16 17:05, Ian Campbell wrote:
> On Wed, 2015-12-16 at 10:24 +0100, Juergen Gross wrote:
>> Set the SIF_VIRT_P2M_4TOOLS flag for pv-domUs in the domain builder
>> to indicate the Xen tools have full support for the virtual mapped
>> linear p2m list.
>>
>> This will enable pv-domUs to drop support of the 3 level p2m tree
>> and use the linear list only. Without setting this flag some kernels
>> might limit themselves to 512 GB memory size in order not to break
>> migration.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> I would have applied all 4 on the basis of Andy's other Reviewed-by to the
> patches purely touching migration v2 files, except for this on 32-bit:

Ouch. Will correct.


Juergen

> 
> xc_sr_save_x86_pv.c:328:9: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
>          ERROR("Bad p2m_cr3 value %#lx", p2m_cr3);
>          ^
> xc_sr_save_x86_pv.c:350:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘xen_vaddr_t’ [-Werror=format=]
>              ERROR("Bad virtual p2m address range %#lx-%#lx",
>              ^
> xc_sr_save_x86_pv.c:350:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘xen_vaddr_t’ [-Werror=format=]
> xc_sr_save_x86_pv.c:363:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘xen_vaddr_t’ [-Werror=format=]
>              ERROR("Bad virtual p2m address range %#lx-%#lx",
>              ^
> xc_sr_save_x86_pv.c:363:13: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘xen_vaddr_t’ [-Werror=format=]
> xc_sr_save_x86_pv.c:370:5: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘xen_vaddr_t’ [-Werror=format=]
>      DPRINTF("p2m list from %#lx to %#lx, root at %#lx", p2m_vaddr, p2m_end,
>      ^
> xc_sr_save_x86_pv.c:370:5: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘xen_vaddr_t’ [-Werror=format=]
> xc_sr_save_x86_pv.c:413:17: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘long long unsigned int’ [-Werror=format=]
>                  ERROR("Bad mfn %#lx during page table walk for vaddr %#lx at level %d of p2m list",
>                  ^
> cc1: all warnings being treated as errors
> /local/scratch/ianc/devel/committer-i686.git/tools/libxc/../../tools/Rules.mk:163: recipe for target 'xc_sr_save_x86_pv.o' failed
> 
>> @@ -1,5 +1,5 @@
>>  % Migration
>> -% Revision 1
>> +% Revision 2
> 
> Well done on this, I predict it will get forgotten more often than not ;-)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains
  2016-01-06 15:40   ` Wei Liu
@ 2016-01-07 10:21     ` Juergen Gross
  2016-01-07 10:33       ` Wei Liu
  0 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2016-01-07 10:21 UTC (permalink / raw)
  To: Wei Liu
  Cc: andrew.cooper3, xen-devel, ian.jackson, Ian.Campbell, stefano.stabellini

On 06/01/16 16:40, Wei Liu wrote:
> On Wed, Dec 16, 2015 at 10:24:18AM +0100, Juergen Gross wrote:
> [...]
>> @@ -698,21 +868,19 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
>>              /* 32bit guests can only use the first 4 entries of their L3 tables.
>>               * All other are potentially used by Xen. */
>>              xen_first = 4;
>> -            xen_last = 512;
>> +            xen_last = 511;
> 
> Is this a bug fix in its own right?

Hmm, bug fix is too much. It is a harmonization with the change below
using macros to set xen_last to 511 at maximum. In fact it doesn't
matter, because xen_last is used in:

   if ( i >= xen_first && i <= xen_last )

with i being in the range from 0 to 511.


Juergen

> 
> Wei.
> 
>>              break;
>>  
>>          case XEN_DOMCTL_PFINFO_L2TAB:
>>              /* It is hard to spot Xen mappings in a 32bit guest's L2.  Most
>>               * are normal but only a few will have Xen mappings.
>> -             *
>> -             * 428 = (HYPERVISOR_VIRT_START_PAE >> L2_PAGETABLE_SHIFT_PAE)&0x1ff
>> -             *
>> -             * ...which is conveniently unavailable to us in a 64bit build.
>>               */
>> -            if ( pte_to_frame(src[428]) == ctx->x86_pv.compat_m2p_mfn0 )
>> +            i = (HYPERVISOR_VIRT_START_X86_32 >> L2_PAGETABLE_SHIFT_PAE) & 511;
>> +            if ( pte_to_frame(src[i]) == ctx->x86_pv.compat_m2p_mfn0 )
>>              {
>> -                xen_first = 428;
>> -                xen_last = 512;
>> +                xen_first = i;
>> +                xen_last = (HYPERVISOR_VIRT_END_X86_32 >>
>> +                            L2_PAGETABLE_SHIFT_PAE) & 511;
>>              }
>>              break;
>>          }
>> -- 
>> 2.6.2
>>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains
  2016-01-07 10:21     ` Juergen Gross
@ 2016-01-07 10:33       ` Wei Liu
  2016-01-07 10:37         ` Juergen Gross
  0 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2016-01-07 10:33 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Wei Liu, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel

On Thu, Jan 07, 2016 at 11:21:04AM +0100, Juergen Gross wrote:
> On 06/01/16 16:40, Wei Liu wrote:
> > On Wed, Dec 16, 2015 at 10:24:18AM +0100, Juergen Gross wrote:
> > [...]
> >> @@ -698,21 +868,19 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
> >>              /* 32bit guests can only use the first 4 entries of their L3 tables.
> >>               * All other are potentially used by Xen. */
> >>              xen_first = 4;
> >> -            xen_last = 512;
> >> +            xen_last = 511;
> > 
> > Is this a bug fix in its own right?
> 
> Hmm, bug fix is too much. It is a harmonization with the change below
> using macros to set xen_last to 511 at maximum. In fact it doesn't
> matter, because xen_last is used in:
> 
>    if ( i >= xen_first && i <= xen_last )
> 
> with i being in the range from 0 to 511.
> 

Yes, that's what I was thinking. There seemed to be an off-by-one error
in the code. But as you said, i is within [0,511] so the code is fine.

Could you add a words or two about this hunk in commit message please.

With that:

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Wei.

> 
> Juergen
> 
> > 
> > Wei.
> > 
> >>              break;
> >>  
> >>          case XEN_DOMCTL_PFINFO_L2TAB:
> >>              /* It is hard to spot Xen mappings in a 32bit guest's L2.  Most
> >>               * are normal but only a few will have Xen mappings.
> >> -             *
> >> -             * 428 = (HYPERVISOR_VIRT_START_PAE >> L2_PAGETABLE_SHIFT_PAE)&0x1ff
> >> -             *
> >> -             * ...which is conveniently unavailable to us in a 64bit build.
> >>               */
> >> -            if ( pte_to_frame(src[428]) == ctx->x86_pv.compat_m2p_mfn0 )
> >> +            i = (HYPERVISOR_VIRT_START_X86_32 >> L2_PAGETABLE_SHIFT_PAE) & 511;
> >> +            if ( pte_to_frame(src[i]) == ctx->x86_pv.compat_m2p_mfn0 )
> >>              {
> >> -                xen_first = 428;
> >> -                xen_last = 512;
> >> +                xen_first = i;
> >> +                xen_last = (HYPERVISOR_VIRT_END_X86_32 >>
> >> +                            L2_PAGETABLE_SHIFT_PAE) & 511;
> >>              }
> >>              break;
> >>          }
> >> -- 
> >> 2.6.2
> >>
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains
  2016-01-07 10:33       ` Wei Liu
@ 2016-01-07 10:37         ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2016-01-07 10:37 UTC (permalink / raw)
  To: Wei Liu
  Cc: andrew.cooper3, xen-devel, ian.jackson, Ian.Campbell, stefano.stabellini

On 07/01/16 11:33, Wei Liu wrote:
> On Thu, Jan 07, 2016 at 11:21:04AM +0100, Juergen Gross wrote:
>> On 06/01/16 16:40, Wei Liu wrote:
>>> On Wed, Dec 16, 2015 at 10:24:18AM +0100, Juergen Gross wrote:
>>> [...]
>>>> @@ -698,21 +868,19 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
>>>>              /* 32bit guests can only use the first 4 entries of their L3 tables.
>>>>               * All other are potentially used by Xen. */
>>>>              xen_first = 4;
>>>> -            xen_last = 512;
>>>> +            xen_last = 511;
>>>
>>> Is this a bug fix in its own right?
>>
>> Hmm, bug fix is too much. It is a harmonization with the change below
>> using macros to set xen_last to 511 at maximum. In fact it doesn't
>> matter, because xen_last is used in:
>>
>>    if ( i >= xen_first && i <= xen_last )
>>
>> with i being in the range from 0 to 511.
>>
> 
> Yes, that's what I was thinking. There seemed to be an off-by-one error
> in the code. But as you said, i is within [0,511] so the code is fine.
> 
> Could you add a words or two about this hunk in commit message please.

Sure.

> 
> With that:
> 
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Thanks,

Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-01-07 10:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-16  9:24 [PATCH v3 0/4] support linear p2m list in migrate stream v2 Juergen Gross
2015-12-16  9:24 ` [PATCH v3 1/4] libxc: split mapping p2m leaves into a separate function Juergen Gross
2016-01-06 15:39   ` Wei Liu
2015-12-16  9:24 ` [PATCH v3 2/4] libxc: support of linear p2m list for migration of pv-domains Juergen Gross
2016-01-06 15:40   ` Wei Liu
2016-01-07 10:21     ` Juergen Gross
2016-01-07 10:33       ` Wei Liu
2016-01-07 10:37         ` Juergen Gross
2015-12-16  9:24 ` [PATCH v3 3/4] libxc: stop migration in case of p2m list structural changes Juergen Gross
2016-01-06 15:40   ` Wei Liu
2015-12-16  9:24 ` [PATCH v3 4/4] libxc: set flag for support of linear p2m list in domain builder Juergen Gross
2016-01-06 15:40   ` Wei Liu
2016-01-06 16:05   ` Ian Campbell
2016-01-07  7:02     ` Juergen Gross

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.