All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/02] Handles broken page occurred before migration
@ 2012-10-31 11:19 Liu, Jinsong
  2012-10-31 12:45 ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Liu, Jinsong @ 2012-10-31 11:19 UTC (permalink / raw)
  To: George Dunlap, Ian Jackson, Jan Beulich; +Cc: xen-devel@lists.xensource.com

[-- Attachment #1: Type: text/plain, Size: 9003 bytes --]

Handles broken page occurred before migration

This patch handles guest broken page which occurred before migration.

At sender, the broken page would be mapped but not copied to target
(otherwise it may trigger more serious error, say, SRAR error).
While its pfn_type and pfn number would be transferred to target
so that target take appropriate action.

At target, it would set p2m as p2m_ram_broken for broken page, so that
if guest access the broken page again, it would kill itself as expected.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r 199e829bf89c tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain.c	Thu Nov 01 00:36:38 2012 +0800
@@ -283,6 +283,22 @@
     return ret;
 }
 
+/* set broken page p2m */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn)
+{
+    int ret;
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_broken_page_p2m.pfn = pfn;
+    ret = do_domctl(xch, &domctl);
+
+    return ret ? -1 : 0;
+}
+
 /* get info from hvm guest for save */
 int xc_domain_hvm_getcontext(xc_interface *xch,
                              uint32_t domid,
diff -r 199e829bf89c tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 00:36:38 2012 +0800
@@ -962,9 +962,15 @@
 
     countpages = count;
     for (i = oldcount; i < buf->nr_pages; ++i)
-        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XTAB
-            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XALLOC)
+    {
+        unsigned long pagetype;
+
+        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
+        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
+             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
+             pagetype == XEN_DOMCTL_PFINFO_XALLOC )
             --countpages;
+    }
 
     if (!countpages)
         return count;
@@ -1200,6 +1206,17 @@
             /* a bogus/unmapped/allocate-only page: skip it */
             continue;
 
+        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN )
+        {
+            if ( xc_set_broken_page_p2m(xch, dom, pfn) )
+            {
+                ERROR("Set p2m for broken page failed, "
+                      "dom=%d, pfn=%lx\n", dom, pfn);
+                goto err_mapped;
+            }
+            continue;
+        }
+
         if (pfn_err[i])
         {
             ERROR("unexpected PFN mapping failure pfn %lx map_mfn %lx p2m_mfn %lx",
diff -r 199e829bf89c tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 00:36:38 2012 +0800
@@ -1277,6 +1277,13 @@
                 if ( !hvm )
                     gmfn = pfn_to_mfn(gmfn);
 
+                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
+                {
+                    pfn_type[j] |= pfn_batch[j];
+                    ++run;
+                    continue;
+                }
+
                 if ( pfn_err[j] )
                 {
                     if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
@@ -1371,8 +1378,12 @@
                     }
                 }
 
-                /* skip pages that aren't present or are alloc-only */
+                /*
+                 * skip pages that aren't present,
+                 * or are broken, or are alloc-only
+                 */
                 if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
+                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
                     || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
                     continue;
 
diff -r 199e829bf89c tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xenctrl.h	Thu Nov 01 00:36:38 2012 +0800
@@ -575,6 +575,17 @@
                           xc_domaininfo_t *info);
 
 /**
+ * This function set p2m for broken page
+ * &parm xch a handle to an open hypervisor interface
+ * @parm domid the domain id which broken page belong to
+ * @parm pfn the pfn number of the broken page
+ * @return 0 on success, -1 on failure
+ */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn);
+
+/**
  * This function returns information about the context of a hvm domain
  * @parm xch a handle to an open hypervisor interface
  * @parm domid the domain to get information from
diff -r 199e829bf89c xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/xen/arch/x86/domctl.c	Thu Nov 01 00:36:38 2012 +0800
@@ -209,12 +209,18 @@
                 for ( j = 0; j < k; j++ )
                 {
                     unsigned long type = 0;
+                    p2m_type_t t;
 
-                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
+                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
 
                     if ( unlikely(!page) ||
                          unlikely(is_xen_heap_page(page)) )
-                        type = XEN_DOMCTL_PFINFO_XTAB;
+                    {
+                        if ( p2m_is_broken(t) )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
+                        else
+                            type = XEN_DOMCTL_PFINFO_XTAB;
+                    }
                     else
                     {
                         switch( page->u.inuse.type_info & PGT_type_mask )
@@ -235,6 +241,9 @@
 
                         if ( page->u.inuse.type_info & PGT_pinned )
                             type |= XEN_DOMCTL_PFINFO_LPINTAB;
+
+                        if ( page->count_info & PGC_broken )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
                     }
 
                     if ( page )
@@ -1568,6 +1577,38 @@
     }
     break;
 
+    case XEN_DOMCTL_set_broken_page_p2m:
+    {
+        struct domain *d;
+        p2m_type_t pt;
+        unsigned long pfn;
+        mfn_t mfn;
+
+        d = rcu_lock_domain_by_id(domctl->domain);
+        if ( d != NULL )
+        {
+            pfn = domctl->u.set_broken_page_p2m.pfn;
+
+            mfn = get_gfn_query(d, pfn, &pt);
+            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) )
+            {
+                put_gfn(d, pfn);
+                rcu_unlock_domain(d);
+                ret = -EINVAL;
+                break;
+            }
+
+            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
+                ret = -EINVAL;
+
+            put_gfn(d, pfn);
+            rcu_unlock_domain(d);
+        }
+        else
+            ret = -ESRCH;
+    }
+    break;
+
     default:
         ret = iommu_do_domctl(domctl, u_domctl);
         break;
diff -r 199e829bf89c xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Tue Oct 30 21:26:56 2012 +0800
+++ b/xen/include/public/domctl.h	Thu Nov 01 00:36:38 2012 +0800
@@ -136,6 +136,7 @@
 #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
 #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
 #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
+#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
 #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
 #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
 
@@ -835,6 +836,12 @@
 typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
 
+struct xen_domctl_set_broken_page_p2m {
+    uint64_aligned_t pfn;
+};
+typedef struct xen_domctl_set_broken_page_p2m xen_domctl_set_broken_page_p2m_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -900,6 +907,7 @@
 #define XEN_DOMCTL_set_access_required           64
 #define XEN_DOMCTL_audit_p2m                     65
 #define XEN_DOMCTL_set_virq_handler              66
+#define XEN_DOMCTL_set_broken_page_p2m           67
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -955,6 +963,7 @@
         struct xen_domctl_audit_p2m         audit_p2m;
         struct xen_domctl_set_virq_handler  set_virq_handler;
         struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
+        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
         struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         uint8_t                             pad[128];

[-- Attachment #2: broken_page_occur_before_migration.patch --]
[-- Type: application/octet-stream, Size: 8761 bytes --]

Handles broken page occurred before migration

This patch handles guest broken page which occurred before migration.

At sender, the broken page would be mapped but not copied to target
(otherwise it may trigger more serious error, say, SRAR error).
While its pfn_type and pfn number would be transferred to target
so that target take appropriate action.

At target, it would set p2m as p2m_ram_broken for broken page, so that
if guest access the broken page again, it would kill itself as expected.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r 199e829bf89c tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain.c	Thu Nov 01 00:36:38 2012 +0800
@@ -283,6 +283,22 @@
     return ret;
 }
 
+/* set broken page p2m */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn)
+{
+    int ret;
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_broken_page_p2m.pfn = pfn;
+    ret = do_domctl(xch, &domctl);
+
+    return ret ? -1 : 0;
+}
+
 /* get info from hvm guest for save */
 int xc_domain_hvm_getcontext(xc_interface *xch,
                              uint32_t domid,
diff -r 199e829bf89c tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 00:36:38 2012 +0800
@@ -962,9 +962,15 @@
 
     countpages = count;
     for (i = oldcount; i < buf->nr_pages; ++i)
-        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XTAB
-            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XALLOC)
+    {
+        unsigned long pagetype;
+
+        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
+        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
+             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
+             pagetype == XEN_DOMCTL_PFINFO_XALLOC )
             --countpages;
+    }
 
     if (!countpages)
         return count;
@@ -1200,6 +1206,17 @@
             /* a bogus/unmapped/allocate-only page: skip it */
             continue;
 
+        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN )
+        {
+            if ( xc_set_broken_page_p2m(xch, dom, pfn) )
+            {
+                ERROR("Set p2m for broken page failed, "
+                      "dom=%d, pfn=%lx\n", dom, pfn);
+                goto err_mapped;
+            }
+            continue;
+        }
+
         if (pfn_err[i])
         {
             ERROR("unexpected PFN mapping failure pfn %lx map_mfn %lx p2m_mfn %lx",
diff -r 199e829bf89c tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 00:36:38 2012 +0800
@@ -1277,6 +1277,13 @@
                 if ( !hvm )
                     gmfn = pfn_to_mfn(gmfn);
 
+                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
+                {
+                    pfn_type[j] |= pfn_batch[j];
+                    ++run;
+                    continue;
+                }
+
                 if ( pfn_err[j] )
                 {
                     if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
@@ -1371,8 +1378,12 @@
                     }
                 }
 
-                /* skip pages that aren't present or are alloc-only */
+                /*
+                 * skip pages that aren't present,
+                 * or are broken, or are alloc-only
+                 */
                 if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
+                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
                     || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
                     continue;
 
diff -r 199e829bf89c tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Tue Oct 30 21:26:56 2012 +0800
+++ b/tools/libxc/xenctrl.h	Thu Nov 01 00:36:38 2012 +0800
@@ -575,6 +575,17 @@
                           xc_domaininfo_t *info);
 
 /**
+ * This function set p2m for broken page
+ * &parm xch a handle to an open hypervisor interface
+ * @parm domid the domain id which broken page belong to
+ * @parm pfn the pfn number of the broken page
+ * @return 0 on success, -1 on failure
+ */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn);
+
+/**
  * This function returns information about the context of a hvm domain
  * @parm xch a handle to an open hypervisor interface
  * @parm domid the domain to get information from
diff -r 199e829bf89c xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c	Tue Oct 30 21:26:56 2012 +0800
+++ b/xen/arch/x86/domctl.c	Thu Nov 01 00:36:38 2012 +0800
@@ -209,12 +209,18 @@
                 for ( j = 0; j < k; j++ )
                 {
                     unsigned long type = 0;
+                    p2m_type_t t;
 
-                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
+                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
 
                     if ( unlikely(!page) ||
                          unlikely(is_xen_heap_page(page)) )
-                        type = XEN_DOMCTL_PFINFO_XTAB;
+                    {
+                        if ( p2m_is_broken(t) )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
+                        else
+                            type = XEN_DOMCTL_PFINFO_XTAB;
+                    }
                     else
                     {
                         switch( page->u.inuse.type_info & PGT_type_mask )
@@ -235,6 +241,9 @@
 
                         if ( page->u.inuse.type_info & PGT_pinned )
                             type |= XEN_DOMCTL_PFINFO_LPINTAB;
+
+                        if ( page->count_info & PGC_broken )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
                     }
 
                     if ( page )
@@ -1568,6 +1577,38 @@
     }
     break;
 
+    case XEN_DOMCTL_set_broken_page_p2m:
+    {
+        struct domain *d;
+        p2m_type_t pt;
+        unsigned long pfn;
+        mfn_t mfn;
+
+        d = rcu_lock_domain_by_id(domctl->domain);
+        if ( d != NULL )
+        {
+            pfn = domctl->u.set_broken_page_p2m.pfn;
+
+            mfn = get_gfn_query(d, pfn, &pt);
+            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) )
+            {
+                put_gfn(d, pfn);
+                rcu_unlock_domain(d);
+                ret = -EINVAL;
+                break;
+            }
+
+            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
+                ret = -EINVAL;
+
+            put_gfn(d, pfn);
+            rcu_unlock_domain(d);
+        }
+        else
+            ret = -ESRCH;
+    }
+    break;
+
     default:
         ret = iommu_do_domctl(domctl, u_domctl);
         break;
diff -r 199e829bf89c xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Tue Oct 30 21:26:56 2012 +0800
+++ b/xen/include/public/domctl.h	Thu Nov 01 00:36:38 2012 +0800
@@ -136,6 +136,7 @@
 #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
 #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
 #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
+#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
 #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
 #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
 
@@ -835,6 +836,12 @@
 typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
 
+struct xen_domctl_set_broken_page_p2m {
+    uint64_aligned_t pfn;
+};
+typedef struct xen_domctl_set_broken_page_p2m xen_domctl_set_broken_page_p2m_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -900,6 +907,7 @@
 #define XEN_DOMCTL_set_access_required           64
 #define XEN_DOMCTL_audit_p2m                     65
 #define XEN_DOMCTL_set_virq_handler              66
+#define XEN_DOMCTL_set_broken_page_p2m           67
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -955,6 +963,7 @@
         struct xen_domctl_audit_p2m         audit_p2m;
         struct xen_domctl_set_virq_handler  set_virq_handler;
         struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
+        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
         struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         uint8_t                             pad[128];

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH 01/02] Handles broken page occurred before migration
  2012-10-31 11:19 [PATCH 01/02] Handles broken page occurred before migration Liu, Jinsong
@ 2012-10-31 12:45 ` Jan Beulich
  2012-10-31 16:14   ` Liu, Jinsong
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2012-10-31 12:45 UTC (permalink / raw)
  To: Jinsong Liu; +Cc: George Dunlap, Ian Jackson, xen-devel

>>> On 31.10.12 at 12:19, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:
> @@ -1568,6 +1577,38 @@
>      }
>      break;
>  
> +    case XEN_DOMCTL_set_broken_page_p2m:
> +    {
> +        struct domain *d;
> +        p2m_type_t pt;
> +        unsigned long pfn;
> +        mfn_t mfn;

If you use scope restricted local variables (which I appreciate),
please declare them in the innermost possible scope, ...

> +
> +        d = rcu_lock_domain_by_id(domctl->domain);
> +        if ( d != NULL )
> +        {
> +            pfn = domctl->u.set_broken_page_p2m.pfn;
> +
> +            mfn = get_gfn_query(d, pfn, &pt);

... i.e. here (and then both assignments can become initializers
at once).

> +            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) )
> +            {
> +                put_gfn(d, pfn);
> +                rcu_unlock_domain(d);
> +                ret = -EINVAL;
> +                break;
> +            }
> +
> +            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
> +                ret = -EINVAL;

The two if() conditions can be easily joined (the more that they
both produce -EINVAL).

> +
> +            put_gfn(d, pfn);
> +            rcu_unlock_domain(d);
> +        }
> +        else
> +            ret = -ESRCH;
> +    }
> +    break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, u_domctl);
>          break;

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

* Re: [PATCH 01/02] Handles broken page occurred before migration
  2012-10-31 12:45 ` Jan Beulich
@ 2012-10-31 16:14   ` Liu, Jinsong
  2012-11-01 11:14     ` George Dunlap
  0 siblings, 1 reply; 5+ messages in thread
From: Liu, Jinsong @ 2012-10-31 16:14 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, Ian Jackson, xen-devel

[-- Attachment #1: Type: text/plain, Size: 9814 bytes --]

[snip]
> 
> If you use scope restricted local variables (which I appreciate),
> please declare them in the innermost possible scope, ...
> 
>> +
>> +        d = rcu_lock_domain_by_id(domctl->domain); +        if ( d
>> != NULL ) +        {
>> +            pfn = domctl->u.set_broken_page_p2m.pfn; +
>> +            mfn = get_gfn_query(d, pfn, &pt);
> 
> ... i.e. here (and then both assignments can become initializers
> at once).
> 
>> +            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ) +     
>> { +                put_gfn(d, pfn);
>> +                rcu_unlock_domain(d);
>> +                ret = -EINVAL;
>> +                break;
>> +            }
>> +
>> +            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
>> +                ret = -EINVAL;
> 
> The two if() conditions can be easily joined (the more that they
> both produce -EINVAL).
> 

Thanks, updated as attached.

Jinsong

======================

Handles broken page occurred before migration

This patch handles guest broken page which occurred before migration.

At sender, the broken page would be mapped but not copied to target
(otherwise it may trigger more serious error, say, SRAR error).
While its pfn_type and pfn number would be transferred to target
so that target take appropriate action.

At target, it would set p2m as p2m_ram_broken for broken page, so that
if guest access the broken page again, it would kill itself as expected.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r e84a79d11d7a tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain.c	Thu Nov 01 07:52:41 2012 +0800
@@ -283,6 +283,22 @@
     return ret;
 }
 
+/* set broken page p2m */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn)
+{
+    int ret;
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_broken_page_p2m.pfn = pfn;
+    ret = do_domctl(xch, &domctl);
+
+    return ret ? -1 : 0;
+}
+
 /* get info from hvm guest for save */
 int xc_domain_hvm_getcontext(xc_interface *xch,
                              uint32_t domid,
diff -r e84a79d11d7a tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 07:52:41 2012 +0800
@@ -962,9 +962,15 @@
 
     countpages = count;
     for (i = oldcount; i < buf->nr_pages; ++i)
-        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XTAB
-            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XALLOC)
+    {
+        unsigned long pagetype;
+
+        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
+        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
+             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
+             pagetype == XEN_DOMCTL_PFINFO_XALLOC )
             --countpages;
+    }
 
     if (!countpages)
         return count;
@@ -1200,6 +1206,17 @@
             /* a bogus/unmapped/allocate-only page: skip it */
             continue;
 
+        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN )
+        {
+            if ( xc_set_broken_page_p2m(xch, dom, pfn) )
+            {
+                ERROR("Set p2m for broken page failed, "
+                      "dom=%d, pfn=%lx\n", dom, pfn);
+                goto err_mapped;
+            }
+            continue;
+        }
+
         if (pfn_err[i])
         {
             ERROR("unexpected PFN mapping failure pfn %lx map_mfn %lx p2m_mfn %lx",
diff -r e84a79d11d7a tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 07:52:41 2012 +0800
@@ -1277,6 +1277,13 @@
                 if ( !hvm )
                     gmfn = pfn_to_mfn(gmfn);
 
+                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
+                {
+                    pfn_type[j] |= pfn_batch[j];
+                    ++run;
+                    continue;
+                }
+
                 if ( pfn_err[j] )
                 {
                     if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
@@ -1371,8 +1378,12 @@
                     }
                 }
 
-                /* skip pages that aren't present or are alloc-only */
+                /*
+                 * skip pages that aren't present,
+                 * or are broken, or are alloc-only
+                 */
                 if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
+                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
                     || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
                     continue;
 
diff -r e84a79d11d7a tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xenctrl.h	Thu Nov 01 07:52:41 2012 +0800
@@ -575,6 +575,17 @@
                           xc_domaininfo_t *info);
 
 /**
+ * This function set p2m for broken page
+ * &parm xch a handle to an open hypervisor interface
+ * @parm domid the domain id which broken page belong to
+ * @parm pfn the pfn number of the broken page
+ * @return 0 on success, -1 on failure
+ */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn);
+
+/**
  * This function returns information about the context of a hvm domain
  * @parm xch a handle to an open hypervisor interface
  * @parm domid the domain to get information from
diff -r e84a79d11d7a xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/xen/arch/x86/domctl.c	Thu Nov 01 07:52:41 2012 +0800
@@ -209,12 +209,18 @@
                 for ( j = 0; j < k; j++ )
                 {
                     unsigned long type = 0;
+                    p2m_type_t t;
 
-                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
+                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
 
                     if ( unlikely(!page) ||
                          unlikely(is_xen_heap_page(page)) )
-                        type = XEN_DOMCTL_PFINFO_XTAB;
+                    {
+                        if ( p2m_is_broken(t) )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
+                        else
+                            type = XEN_DOMCTL_PFINFO_XTAB;
+                    }
                     else
                     {
                         switch( page->u.inuse.type_info & PGT_type_mask )
@@ -235,6 +241,9 @@
 
                         if ( page->u.inuse.type_info & PGT_pinned )
                             type |= XEN_DOMCTL_PFINFO_LPINTAB;
+
+                        if ( page->count_info & PGC_broken )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
                     }
 
                     if ( page )
@@ -1568,6 +1577,29 @@
     }
     break;
 
+    case XEN_DOMCTL_set_broken_page_p2m:
+    {
+        struct domain *d;
+        
+        d = rcu_lock_domain_by_id(domctl->domain);
+        if ( d != NULL )
+        {
+            p2m_type_t pt;
+            unsigned long pfn = domctl->u.set_broken_page_p2m.pfn;
+            mfn_t mfn = get_gfn_query(d, pfn, &pt);
+
+            if ( unlikely(!mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ||
+                         (p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt)) )
+                ret = -EINVAL;
+
+            put_gfn(d, pfn);
+            rcu_unlock_domain(d);
+        }
+        else
+            ret = -ESRCH;
+    }
+    break;
+
     default:
         ret = iommu_do_domctl(domctl, u_domctl);
         break;
diff -r e84a79d11d7a xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Thu Nov 01 01:41:03 2012 +0800
+++ b/xen/include/public/domctl.h	Thu Nov 01 07:52:41 2012 +0800
@@ -136,6 +136,7 @@
 #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
 #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
 #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
+#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
 #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
 #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
 
@@ -835,6 +836,12 @@
 typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
 
+struct xen_domctl_set_broken_page_p2m {
+    uint64_aligned_t pfn;
+};
+typedef struct xen_domctl_set_broken_page_p2m xen_domctl_set_broken_page_p2m_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -900,6 +907,7 @@
 #define XEN_DOMCTL_set_access_required           64
 #define XEN_DOMCTL_audit_p2m                     65
 #define XEN_DOMCTL_set_virq_handler              66
+#define XEN_DOMCTL_set_broken_page_p2m           67
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -955,6 +963,7 @@
         struct xen_domctl_audit_p2m         audit_p2m;
         struct xen_domctl_set_virq_handler  set_virq_handler;
         struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
+        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
         struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         uint8_t                             pad[128];

[-- Attachment #2: broken_page_occur_before_migration.patch --]
[-- Type: application/octet-stream, Size: 8605 bytes --]

Handles broken page occurred before migration

This patch handles guest broken page which occurred before migration.

At sender, the broken page would be mapped but not copied to target
(otherwise it may trigger more serious error, say, SRAR error).
While its pfn_type and pfn number would be transferred to target
so that target take appropriate action.

At target, it would set p2m as p2m_ram_broken for broken page, so that
if guest access the broken page again, it would kill itself as expected.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>

diff -r e84a79d11d7a tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain.c	Thu Nov 01 07:52:41 2012 +0800
@@ -283,6 +283,22 @@
     return ret;
 }
 
+/* set broken page p2m */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn)
+{
+    int ret;
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_broken_page_p2m.pfn = pfn;
+    ret = do_domctl(xch, &domctl);
+
+    return ret ? -1 : 0;
+}
+
 /* get info from hvm guest for save */
 int xc_domain_hvm_getcontext(xc_interface *xch,
                              uint32_t domid,
diff -r e84a79d11d7a tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 07:52:41 2012 +0800
@@ -962,9 +962,15 @@
 
     countpages = count;
     for (i = oldcount; i < buf->nr_pages; ++i)
-        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XTAB
-            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XALLOC)
+    {
+        unsigned long pagetype;
+
+        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
+        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
+             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
+             pagetype == XEN_DOMCTL_PFINFO_XALLOC )
             --countpages;
+    }
 
     if (!countpages)
         return count;
@@ -1200,6 +1206,17 @@
             /* a bogus/unmapped/allocate-only page: skip it */
             continue;
 
+        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN )
+        {
+            if ( xc_set_broken_page_p2m(xch, dom, pfn) )
+            {
+                ERROR("Set p2m for broken page failed, "
+                      "dom=%d, pfn=%lx\n", dom, pfn);
+                goto err_mapped;
+            }
+            continue;
+        }
+
         if (pfn_err[i])
         {
             ERROR("unexpected PFN mapping failure pfn %lx map_mfn %lx p2m_mfn %lx",
diff -r e84a79d11d7a tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 07:52:41 2012 +0800
@@ -1277,6 +1277,13 @@
                 if ( !hvm )
                     gmfn = pfn_to_mfn(gmfn);
 
+                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
+                {
+                    pfn_type[j] |= pfn_batch[j];
+                    ++run;
+                    continue;
+                }
+
                 if ( pfn_err[j] )
                 {
                     if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
@@ -1371,8 +1378,12 @@
                     }
                 }
 
-                /* skip pages that aren't present or are alloc-only */
+                /*
+                 * skip pages that aren't present,
+                 * or are broken, or are alloc-only
+                 */
                 if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
+                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
                     || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
                     continue;
 
diff -r e84a79d11d7a tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Thu Nov 01 01:41:03 2012 +0800
+++ b/tools/libxc/xenctrl.h	Thu Nov 01 07:52:41 2012 +0800
@@ -575,6 +575,17 @@
                           xc_domaininfo_t *info);
 
 /**
+ * This function set p2m for broken page
+ * &parm xch a handle to an open hypervisor interface
+ * @parm domid the domain id which broken page belong to
+ * @parm pfn the pfn number of the broken page
+ * @return 0 on success, -1 on failure
+ */
+int xc_set_broken_page_p2m(xc_interface *xch,
+                           uint32_t domid,
+                           unsigned long pfn);
+
+/**
  * This function returns information about the context of a hvm domain
  * @parm xch a handle to an open hypervisor interface
  * @parm domid the domain to get information from
diff -r e84a79d11d7a xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c	Thu Nov 01 01:41:03 2012 +0800
+++ b/xen/arch/x86/domctl.c	Thu Nov 01 07:52:41 2012 +0800
@@ -209,12 +209,18 @@
                 for ( j = 0; j < k; j++ )
                 {
                     unsigned long type = 0;
+                    p2m_type_t t;
 
-                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
+                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
 
                     if ( unlikely(!page) ||
                          unlikely(is_xen_heap_page(page)) )
-                        type = XEN_DOMCTL_PFINFO_XTAB;
+                    {
+                        if ( p2m_is_broken(t) )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
+                        else
+                            type = XEN_DOMCTL_PFINFO_XTAB;
+                    }
                     else
                     {
                         switch( page->u.inuse.type_info & PGT_type_mask )
@@ -235,6 +241,9 @@
 
                         if ( page->u.inuse.type_info & PGT_pinned )
                             type |= XEN_DOMCTL_PFINFO_LPINTAB;
+
+                        if ( page->count_info & PGC_broken )
+                            type = XEN_DOMCTL_PFINFO_BROKEN;
                     }
 
                     if ( page )
@@ -1568,6 +1577,29 @@
     }
     break;
 
+    case XEN_DOMCTL_set_broken_page_p2m:
+    {
+        struct domain *d;
+        
+        d = rcu_lock_domain_by_id(domctl->domain);
+        if ( d != NULL )
+        {
+            p2m_type_t pt;
+            unsigned long pfn = domctl->u.set_broken_page_p2m.pfn;
+            mfn_t mfn = get_gfn_query(d, pfn, &pt);
+
+            if ( unlikely(!mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ||
+                         (p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt)) )
+                ret = -EINVAL;
+
+            put_gfn(d, pfn);
+            rcu_unlock_domain(d);
+        }
+        else
+            ret = -ESRCH;
+    }
+    break;
+
     default:
         ret = iommu_do_domctl(domctl, u_domctl);
         break;
diff -r e84a79d11d7a xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Thu Nov 01 01:41:03 2012 +0800
+++ b/xen/include/public/domctl.h	Thu Nov 01 07:52:41 2012 +0800
@@ -136,6 +136,7 @@
 #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
 #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
 #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
+#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
 #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
 #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
 
@@ -835,6 +836,12 @@
 typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
 
+struct xen_domctl_set_broken_page_p2m {
+    uint64_aligned_t pfn;
+};
+typedef struct xen_domctl_set_broken_page_p2m xen_domctl_set_broken_page_p2m_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -900,6 +907,7 @@
 #define XEN_DOMCTL_set_access_required           64
 #define XEN_DOMCTL_audit_p2m                     65
 #define XEN_DOMCTL_set_virq_handler              66
+#define XEN_DOMCTL_set_broken_page_p2m           67
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -955,6 +963,7 @@
         struct xen_domctl_audit_p2m         audit_p2m;
         struct xen_domctl_set_virq_handler  set_virq_handler;
         struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
+        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
         struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         uint8_t                             pad[128];

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH 01/02] Handles broken page occurred before migration
  2012-10-31 16:14   ` Liu, Jinsong
@ 2012-11-01 11:14     ` George Dunlap
  2012-11-01 16:06       ` Liu, Jinsong
  0 siblings, 1 reply; 5+ messages in thread
From: George Dunlap @ 2012-11-01 11:14 UTC (permalink / raw)
  To: Liu, Jinsong; +Cc: Ian Jackson, Jan Beulich, xen-devel

Are you actually using "hg email" (or patchbomb) to send these?

Having changes to patches included in-line to replies makes it hard to 
keep track of what the newest version is.  Tradition is to send the 
whole series again, with "v2" or "v3" added to the end. If you're using 
patchbomb, the relevant command would be:

$ hg email -o --flag "v3"

Also, the second patch doesn't seem to apply to xen-unstable tip anymore 
-- can you rebase?

  -George

On 31/10/12 17:14, Liu, Jinsong wrote:
> [snip]
>> If you use scope restricted local variables (which I appreciate),
>> please declare them in the innermost possible scope, ...
>>
>>> +
>>> +        d = rcu_lock_domain_by_id(domctl->domain); +        if ( d
>>> != NULL ) +        {
>>> +            pfn = domctl->u.set_broken_page_p2m.pfn; +
>>> +            mfn = get_gfn_query(d, pfn, &pt);
>> ... i.e. here (and then both assignments can become initializers
>> at once).
>>
>>> +            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ) +
>>> { +                put_gfn(d, pfn);
>>> +                rcu_unlock_domain(d);
>>> +                ret = -EINVAL;
>>> +                break;
>>> +            }
>>> +
>>> +            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
>>> +                ret = -EINVAL;
>> The two if() conditions can be easily joined (the more that they
>> both produce -EINVAL).
>>
> Thanks, updated as attached.
>
> Jinsong
>
> ======================
>
> Handles broken page occurred before migration
>
> This patch handles guest broken page which occurred before migration.
>
> At sender, the broken page would be mapped but not copied to target
> (otherwise it may trigger more serious error, say, SRAR error).
> While its pfn_type and pfn number would be transferred to target
> so that target take appropriate action.
>
> At target, it would set p2m as p2m_ram_broken for broken page, so that
> if guest access the broken page again, it would kill itself as expected.
>
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>
> diff -r e84a79d11d7a tools/libxc/xc_domain.c
> --- a/tools/libxc/xc_domain.c	Thu Nov 01 01:41:03 2012 +0800
> +++ b/tools/libxc/xc_domain.c	Thu Nov 01 07:52:41 2012 +0800
> @@ -283,6 +283,22 @@
>       return ret;
>   }
>   
> +/* set broken page p2m */
> +int xc_set_broken_page_p2m(xc_interface *xch,
> +                           uint32_t domid,
> +                           unsigned long pfn)
> +{
> +    int ret;
> +    DECLARE_DOMCTL;
> +
> +    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.set_broken_page_p2m.pfn = pfn;
> +    ret = do_domctl(xch, &domctl);
> +
> +    return ret ? -1 : 0;
> +}
> +
>   /* get info from hvm guest for save */
>   int xc_domain_hvm_getcontext(xc_interface *xch,
>                                uint32_t domid,
> diff -r e84a79d11d7a tools/libxc/xc_domain_restore.c
> --- a/tools/libxc/xc_domain_restore.c	Thu Nov 01 01:41:03 2012 +0800
> +++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 07:52:41 2012 +0800
> @@ -962,9 +962,15 @@
>   
>       countpages = count;
>       for (i = oldcount; i < buf->nr_pages; ++i)
> -        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XTAB
> -            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) == XEN_DOMCTL_PFINFO_XALLOC)
> +    {
> +        unsigned long pagetype;
> +
> +        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
> +        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
> +             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
> +             pagetype == XEN_DOMCTL_PFINFO_XALLOC )
>               --countpages;
> +    }
>   
>       if (!countpages)
>           return count;
> @@ -1200,6 +1206,17 @@
>               /* a bogus/unmapped/allocate-only page: skip it */
>               continue;
>   
> +        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN )
> +        {
> +            if ( xc_set_broken_page_p2m(xch, dom, pfn) )
> +            {
> +                ERROR("Set p2m for broken page failed, "
> +                      "dom=%d, pfn=%lx\n", dom, pfn);
> +                goto err_mapped;
> +            }
> +            continue;
> +        }
> +
>           if (pfn_err[i])
>           {
>               ERROR("unexpected PFN mapping failure pfn %lx map_mfn %lx p2m_mfn %lx",
> diff -r e84a79d11d7a tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c	Thu Nov 01 01:41:03 2012 +0800
> +++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 07:52:41 2012 +0800
> @@ -1277,6 +1277,13 @@
>                   if ( !hvm )
>                       gmfn = pfn_to_mfn(gmfn);
>   
> +                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
> +                {
> +                    pfn_type[j] |= pfn_batch[j];
> +                    ++run;
> +                    continue;
> +                }
> +
>                   if ( pfn_err[j] )
>                   {
>                       if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
> @@ -1371,8 +1378,12 @@
>                       }
>                   }
>   
> -                /* skip pages that aren't present or are alloc-only */
> +                /*
> +                 * skip pages that aren't present,
> +                 * or are broken, or are alloc-only
> +                 */
>                   if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
> +                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
>                       || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
>                       continue;
>   
> diff -r e84a79d11d7a tools/libxc/xenctrl.h
> --- a/tools/libxc/xenctrl.h	Thu Nov 01 01:41:03 2012 +0800
> +++ b/tools/libxc/xenctrl.h	Thu Nov 01 07:52:41 2012 +0800
> @@ -575,6 +575,17 @@
>                             xc_domaininfo_t *info);
>   
>   /**
> + * This function set p2m for broken page
> + * &parm xch a handle to an open hypervisor interface
> + * @parm domid the domain id which broken page belong to
> + * @parm pfn the pfn number of the broken page
> + * @return 0 on success, -1 on failure
> + */
> +int xc_set_broken_page_p2m(xc_interface *xch,
> +                           uint32_t domid,
> +                           unsigned long pfn);
> +
> +/**
>    * This function returns information about the context of a hvm domain
>    * @parm xch a handle to an open hypervisor interface
>    * @parm domid the domain to get information from
> diff -r e84a79d11d7a xen/arch/x86/domctl.c
> --- a/xen/arch/x86/domctl.c	Thu Nov 01 01:41:03 2012 +0800
> +++ b/xen/arch/x86/domctl.c	Thu Nov 01 07:52:41 2012 +0800
> @@ -209,12 +209,18 @@
>                   for ( j = 0; j < k; j++ )
>                   {
>                       unsigned long type = 0;
> +                    p2m_type_t t;
>   
> -                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
> +                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
>   
>                       if ( unlikely(!page) ||
>                            unlikely(is_xen_heap_page(page)) )
> -                        type = XEN_DOMCTL_PFINFO_XTAB;
> +                    {
> +                        if ( p2m_is_broken(t) )
> +                            type = XEN_DOMCTL_PFINFO_BROKEN;
> +                        else
> +                            type = XEN_DOMCTL_PFINFO_XTAB;
> +                    }
>                       else
>                       {
>                           switch( page->u.inuse.type_info & PGT_type_mask )
> @@ -235,6 +241,9 @@
>   
>                           if ( page->u.inuse.type_info & PGT_pinned )
>                               type |= XEN_DOMCTL_PFINFO_LPINTAB;
> +
> +                        if ( page->count_info & PGC_broken )
> +                            type = XEN_DOMCTL_PFINFO_BROKEN;
>                       }
>   
>                       if ( page )
> @@ -1568,6 +1577,29 @@
>       }
>       break;
>   
> +    case XEN_DOMCTL_set_broken_page_p2m:
> +    {
> +        struct domain *d;
> +
> +        d = rcu_lock_domain_by_id(domctl->domain);
> +        if ( d != NULL )
> +        {
> +            p2m_type_t pt;
> +            unsigned long pfn = domctl->u.set_broken_page_p2m.pfn;
> +            mfn_t mfn = get_gfn_query(d, pfn, &pt);
> +
> +            if ( unlikely(!mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ||
> +                         (p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt)) )
> +                ret = -EINVAL;
> +
> +            put_gfn(d, pfn);
> +            rcu_unlock_domain(d);
> +        }
> +        else
> +            ret = -ESRCH;
> +    }
> +    break;
> +
>       default:
>           ret = iommu_do_domctl(domctl, u_domctl);
>           break;
> diff -r e84a79d11d7a xen/include/public/domctl.h
> --- a/xen/include/public/domctl.h	Thu Nov 01 01:41:03 2012 +0800
> +++ b/xen/include/public/domctl.h	Thu Nov 01 07:52:41 2012 +0800
> @@ -136,6 +136,7 @@
>   #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
>   #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
>   #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
> +#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
>   #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
>   #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
>   
> @@ -835,6 +836,12 @@
>   typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
>   DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
>   
> +struct xen_domctl_set_broken_page_p2m {
> +    uint64_aligned_t pfn;
> +};
> +typedef struct xen_domctl_set_broken_page_p2m xen_domctl_set_broken_page_p2m_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t);
> +
>   struct xen_domctl {
>       uint32_t cmd;
>   #define XEN_DOMCTL_createdomain                   1
> @@ -900,6 +907,7 @@
>   #define XEN_DOMCTL_set_access_required           64
>   #define XEN_DOMCTL_audit_p2m                     65
>   #define XEN_DOMCTL_set_virq_handler              66
> +#define XEN_DOMCTL_set_broken_page_p2m           67
>   #define XEN_DOMCTL_gdbsx_guestmemio            1000
>   #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>   #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -955,6 +963,7 @@
>           struct xen_domctl_audit_p2m         audit_p2m;
>           struct xen_domctl_set_virq_handler  set_virq_handler;
>           struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
> +        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
>           struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
>           struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
>           uint8_t                             pad[128];

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

* Re: [PATCH 01/02] Handles broken page occurred before migration
  2012-11-01 11:14     ` George Dunlap
@ 2012-11-01 16:06       ` Liu, Jinsong
  0 siblings, 0 replies; 5+ messages in thread
From: Liu, Jinsong @ 2012-11-01 16:06 UTC (permalink / raw)
  To: George Dunlap; +Cc: Ian Jackson, Jan Beulich, xen-devel

George Dunlap wrote:
> Are you actually using "hg email" (or patchbomb) to send these?
> 
> Having changes to patches included in-line to replies makes it hard to
> keep track of what the newest version is.  Tradition is to send the
> whole series again, with "v2" or "v3" added to the end. If you're
> using patchbomb, the relevant command would be:
> 
> $ hg email -o --flag "v3"

OK, I will use 'hg email' next time updating vmce patch.
(I'm just not habit of using it, say, defaultly the patch is included as text in the email body for easy reviewing, but when I use 'a' hoping to attach patch file, the patch context disappear in the email body -- only patch description left in the email body)

> 
> Also, the second patch doesn't seem to apply to xen-unstable tip
> anymore -- can you rebase?
> 
>   -George

Sure.

Thanks,
Jinsong

> 
> On 31/10/12 17:14, Liu, Jinsong wrote:
>> [snip]
>>> If you use scope restricted local variables (which I appreciate),
>>> please declare them in the innermost possible scope, ...
>>> 
>>>> +
>>>> +        d = rcu_lock_domain_by_id(domctl->domain); +        if (
>>>> d != NULL ) +        { +            pfn =
>>>> domctl->u.set_broken_page_p2m.pfn; + +            mfn =
>>>> get_gfn_query(d, pfn, &pt); 
>>> ... i.e. here (and then both assignments can become initializers at
>>> once). 
>>> 
>>>> +            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) ) +
>>>> { +                put_gfn(d, pfn);
>>>> +                rcu_unlock_domain(d);
>>>> +                ret = -EINVAL;
>>>> +                break;
>>>> +            }
>>>> +
>>>> +            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) !=
>>>> pt ) +                ret = -EINVAL;
>>> The two if() conditions can be easily joined (the more that they
>>> both produce -EINVAL). 
>>> 
>> Thanks, updated as attached.
>> 
>> Jinsong
>> 
>> ======================
>> 
>> Handles broken page occurred before migration
>> 
>> This patch handles guest broken page which occurred before migration.
>> 
>> At sender, the broken page would be mapped but not copied to target
>> (otherwise it may trigger more serious error, say, SRAR error).
>> While its pfn_type and pfn number would be transferred to target
>> so that target take appropriate action.
>> 
>> At target, it would set p2m as p2m_ram_broken for broken page, so
>> that 
>> if guest access the broken page again, it would kill itself as
>> expected. 
>> 
>> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>> 
>> diff -r e84a79d11d7a tools/libxc/xc_domain.c
>> --- a/tools/libxc/xc_domain.c	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/tools/libxc/xc_domain.c	Thu Nov 01 07:52:41 2012 +0800 @@
>>       -283,6 +283,22 @@ return ret;
>>   }
>> 
>> +/* set broken page p2m */
>> +int xc_set_broken_page_p2m(xc_interface *xch,
>> +                           uint32_t domid,
>> +                           unsigned long pfn)
>> +{
>> +    int ret;
>> +    DECLARE_DOMCTL;
>> +
>> +    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
>> +    domctl.domain = (domid_t)domid;
>> +    domctl.u.set_broken_page_p2m.pfn = pfn;
>> +    ret = do_domctl(xch, &domctl);
>> +
>> +    return ret ? -1 : 0;
>> +}
>> +
>>   /* get info from hvm guest for save */
>>   int xc_domain_hvm_getcontext(xc_interface *xch,
>>                                uint32_t domid,
>> diff -r e84a79d11d7a tools/libxc/xc_domain_restore.c
>> --- a/tools/libxc/xc_domain_restore.c	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/tools/libxc/xc_domain_restore.c	Thu Nov 01 07:52:41 2012 +0800
>> @@ -962,9 +962,15 @@ 
>> 
>>       countpages = count;
>>       for (i = oldcount; i < buf->nr_pages; ++i)
>> -        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) ==
>> XEN_DOMCTL_PFINFO_XTAB 
>> -            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) ==
>> XEN_DOMCTL_PFINFO_XALLOC) +    { +        unsigned long pagetype;
>> +
>> +        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
>> +        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
>> +             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
>> +             pagetype == XEN_DOMCTL_PFINFO_XALLOC )              
>> --countpages; +    }
>> 
>>       if (!countpages)
>>           return count;
>> @@ -1200,6 +1206,17 @@
>>               /* a bogus/unmapped/allocate-only page: skip it */    
>> continue; 
>> 
>> +        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN ) +        {
>> +            if ( xc_set_broken_page_p2m(xch, dom, pfn) ) +         
>> { +                ERROR("Set p2m for broken page failed, "
>> +                      "dom=%d, pfn=%lx\n", dom, pfn);
>> +                goto err_mapped;
>> +            }
>> +            continue;
>> +        }
>> +
>>           if (pfn_err[i])
>>           {
>>               ERROR("unexpected PFN mapping failure pfn %lx map_mfn
>> %lx p2m_mfn %lx", 
>> diff -r e84a79d11d7a tools/libxc/xc_domain_save.c
>> --- a/tools/libxc/xc_domain_save.c	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/tools/libxc/xc_domain_save.c	Thu Nov 01 07:52:41 2012 +0800 @@
>>                   -1277,6 +1277,13 @@ if ( !hvm )
>>                       gmfn = pfn_to_mfn(gmfn);
>> 
>> +                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN ) +   
>> { +                    pfn_type[j] |= pfn_batch[j];
>> +                    ++run;
>> +                    continue;
>> +                }
>> +
>>                   if ( pfn_err[j] )
>>                   {
>>                       if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
>>                       @@ -1371,8 +1378,12 @@ }
>>                   }
>> 
>> -                /* skip pages that aren't present or are alloc-only
>> */ +                /* +                 * skip pages that aren't
>> present, +                 * or are broken, or are alloc-only +     
>>                   */ if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
>> +                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
>>                       || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
>>                       continue;
>> 
>> diff -r e84a79d11d7a tools/libxc/xenctrl.h
>> --- a/tools/libxc/xenctrl.h	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/tools/libxc/xenctrl.h	Thu Nov 01 07:52:41 2012 +0800 @@ -575,6
>>                             +575,17 @@ xc_domaininfo_t *info);
>> 
>>   /**
>> + * This function set p2m for broken page
>> + * &parm xch a handle to an open hypervisor interface
>> + * @parm domid the domain id which broken page belong to
>> + * @parm pfn the pfn number of the broken page
>> + * @return 0 on success, -1 on failure
>> + */
>> +int xc_set_broken_page_p2m(xc_interface *xch,
>> +                           uint32_t domid,
>> +                           unsigned long pfn);
>> +
>> +/**
>>    * This function returns information about the context of a hvm
>> domain 
>>    * @parm xch a handle to an open hypervisor interface
>>    * @parm domid the domain to get information from
>> diff -r e84a79d11d7a xen/arch/x86/domctl.c
>> --- a/xen/arch/x86/domctl.c	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/xen/arch/x86/domctl.c	Thu Nov 01 07:52:41 2012 +0800 @@
>>                   -209,12 +209,18 @@ for ( j = 0; j < k; j++ )
>>                   {
>>                       unsigned long type = 0;
>> +                    p2m_type_t t;
>> 
>> -                    page = get_page_from_gfn(d, arr[j], NULL,
>> P2M_ALLOC); +                    page = get_page_from_gfn(d, arr[j],
>> &t, P2M_ALLOC); 
>> 
>>                       if ( unlikely(!page) ||
>>                            unlikely(is_xen_heap_page(page)) )
>> -                        type = XEN_DOMCTL_PFINFO_XTAB; +           
>> { +                        if ( p2m_is_broken(t) )
>> +                            type = XEN_DOMCTL_PFINFO_BROKEN; +     
>> else +                            type = XEN_DOMCTL_PFINFO_XTAB; +  
>>                       } else
>>                       {
>>                           switch( page->u.inuse.type_info &
>> PGT_type_mask ) @@ -235,6 +241,9 @@ 
>> 
>>                           if ( page->u.inuse.type_info & PGT_pinned )
>>                               type |= XEN_DOMCTL_PFINFO_LPINTAB; +
>> +                        if ( page->count_info & PGC_broken )
>> +                            type = XEN_DOMCTL_PFINFO_BROKEN;       
>> } 
>> 
>>                       if ( page )
>> @@ -1568,6 +1577,29 @@
>>       }
>>       break;
>> 
>> +    case XEN_DOMCTL_set_broken_page_p2m:
>> +    {
>> +        struct domain *d;
>> +
>> +        d = rcu_lock_domain_by_id(domctl->domain); +        if ( d
>> != NULL ) +        {
>> +            p2m_type_t pt;
>> +            unsigned long pfn = domctl->u.set_broken_page_p2m.pfn;
>> +            mfn_t mfn = get_gfn_query(d, pfn, &pt); +
>> +            if ( unlikely(!mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt)
>> || +                         (p2m_change_type(d, pfn, pt,
>> p2m_ram_broken) != pt)) ) +                ret = -EINVAL;
>> +
>> +            put_gfn(d, pfn);
>> +            rcu_unlock_domain(d);
>> +        }
>> +        else
>> +            ret = -ESRCH;
>> +    }
>> +    break;
>> +
>>       default:
>>           ret = iommu_do_domctl(domctl, u_domctl);
>>           break;
>> diff -r e84a79d11d7a xen/include/public/domctl.h
>> --- a/xen/include/public/domctl.h	Thu Nov 01 01:41:03 2012 +0800
>> +++ b/xen/include/public/domctl.h	Thu Nov 01 07:52:41 2012 +0800 @@
>>   -136,6 +136,7 @@ #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
>>   #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
>>   #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page
>> */ +#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
>>   #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
>>   #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
>> 
>> @@ -835,6 +836,12 @@
>>   typedef struct xen_domctl_set_access_required
>>   xen_domctl_set_access_required_t;
>> DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t); 
>> 
>> +struct xen_domctl_set_broken_page_p2m {
>> +    uint64_aligned_t pfn;
>> +};
>> +typedef struct xen_domctl_set_broken_page_p2m
>> xen_domctl_set_broken_page_p2m_t;
>>   +DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t); +
>>       struct xen_domctl { uint32_t cmd;
>>   #define XEN_DOMCTL_createdomain                   1 @@ -900,6
>>   +907,7 @@ #define XEN_DOMCTL_set_access_required           64
>>   #define XEN_DOMCTL_audit_p2m                     65
>>   #define XEN_DOMCTL_set_virq_handler              66
>> +#define XEN_DOMCTL_set_broken_page_p2m           67
>>   #define XEN_DOMCTL_gdbsx_guestmemio            1000
>>   #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>>   #define XEN_DOMCTL_gdbsx_unpausevcpu           1002 @@ -955,6
>>           +963,7 @@ struct xen_domctl_audit_p2m         audit_p2m;
>>           struct xen_domctl_set_virq_handler  set_virq_handler;
>>           struct xen_domctl_gdbsx_memio       gdbsx_guest_memio;
>> +        struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
>>           struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
>>           struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
>>           uint8_t                             pad[128];

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

end of thread, other threads:[~2012-11-01 16:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-31 11:19 [PATCH 01/02] Handles broken page occurred before migration Liu, Jinsong
2012-10-31 12:45 ` Jan Beulich
2012-10-31 16:14   ` Liu, Jinsong
2012-11-01 11:14     ` George Dunlap
2012-11-01 16:06       ` Liu, Jinsong

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.