[HVM] Save/restore: clean up the new hypercall interface

Pass a buffer and size through instead of fixed-size structure.

Signed-off-by: Zhai Edwin <edwin.zhai@intel.com>
Signed-off-by: Tim Deegan <Tim.Deegan@xensource.com>

tools/libxc/xc_domain.c

@@ -241,45 +241,49 @@ int xc_domain_getinfolist(int xc_handle,
 /* get info from hvm guest for save */
 int xc_domain_hvm_getcontext(int xc_handle,
                              uint32_t domid,
-                             hvm_domain_context_t *hvm_ctxt)
+                             uint8_t *ctxt_buf,
+                             uint32_t size)
 {
-    int rc;
+    int ret;
     DECLARE_DOMCTL;
 
     domctl.cmd = XEN_DOMCTL_gethvmcontext;
     domctl.domain = (domid_t)domid;
-    set_xen_guest_handle(domctl.u.hvmcontext.ctxt, hvm_ctxt);
+    domctl.u.hvmcontext.size = size;
+    set_xen_guest_handle(domctl.u.hvmcontext.buffer, ctxt_buf);
 
-    if ( (rc = mlock(hvm_ctxt, sizeof(*hvm_ctxt))) != 0 )
-        return rc;
+    if ( (ret = lock_pages(ctxt_buf, size)) != 0 )
+        return ret;
 
-    rc = do_domctl(xc_handle, &domctl);
+    ret = do_domctl(xc_handle, &domctl);
 
-    safe_munlock(hvm_ctxt, sizeof(*hvm_ctxt));
+    unlock_pages(ctxt_buf, size);
 
-    return rc;
+    return (ret < 0 ? -1 : domctl.u.hvmcontext.size);
 }
 
 /* set info to hvm guest for restore */
 int xc_domain_hvm_setcontext(int xc_handle,
                              uint32_t domid,
-                             hvm_domain_context_t *hvm_ctxt)
+                             uint8_t *ctxt_buf,
+                             uint32_t size)
 {
-    int rc;
+    int ret;
     DECLARE_DOMCTL;
 
     domctl.cmd = XEN_DOMCTL_sethvmcontext;
     domctl.domain = domid;
-    set_xen_guest_handle(domctl.u.hvmcontext.ctxt, hvm_ctxt);
+    domctl.u.hvmcontext.size = size;
+    set_xen_guest_handle(domctl.u.hvmcontext.buffer, ctxt_buf);
 
-    if ( (rc = mlock(hvm_ctxt, sizeof(*hvm_ctxt))) != 0 )
-        return rc;
+    if ( (ret = lock_pages(ctxt_buf, size)) != 0 )
+        return ret;
 
-    rc = do_domctl(xc_handle, &domctl);
+    ret = do_domctl(xc_handle, &domctl);
 
-    safe_munlock(hvm_ctxt, sizeof(*hvm_ctxt));
+    unlock_pages(ctxt_buf, size);
 
-    return rc;
+    return ret;
 }
 
 int xc_vcpu_getcontext(int xc_handle,

tools/libxc/xc_hvm_restore.c

@@ -87,7 +87,7 @@ int xc_hvm_restore(int xc_handle, int io_fd,
     xc_dominfo_t info;
     unsigned int rc = 1, n, i;
     uint32_t rec_len, nr_vcpus;
-    hvm_domain_context_t hvm_ctxt;
+    uint8_t *hvm_buf = NULL;
     unsigned long long v_end, memsize;
     unsigned long shared_page_nr;
 
@@ -128,8 +128,7 @@ int xc_hvm_restore(int xc_handle, int io_fd,
     }
 
 
-    p2m        = malloc(max_pfn * sizeof(xen_pfn_t));
-
+    p2m = malloc(max_pfn * sizeof(xen_pfn_t));
     if (p2m == NULL) {
         ERROR("memory alloc failed");
         errno = ENOMEM;
@@ -297,18 +296,21 @@ int xc_hvm_restore(int xc_handle, int io_fd,
         ERROR("error read hvm context size!\n");
         goto out;
     }
-    if (rec_len != sizeof(hvm_ctxt)) {
-        ERROR("hvm context size dismatch!\n");
+
+    hvm_buf = malloc(rec_len);
+    if (hvm_buf == NULL) {
+        ERROR("memory alloc for hvm context buffer failed");
+        errno = ENOMEM;
         goto out;
     }
 
-    if (!read_exact(io_fd, &hvm_ctxt, sizeof(hvm_ctxt))) {
-        ERROR("error read hvm context!\n");
+    if (!read_exact(io_fd, hvm_buf, rec_len)) {
+        ERROR("error read hvm buffer!\n");
         goto out;
     }
 
-    if (( rc = xc_domain_hvm_setcontext(xc_handle, dom, &hvm_ctxt))) {
-        ERROR("error set hvm context!\n");
+    if (( rc = xc_domain_hvm_setcontext(xc_handle, dom, hvm_buf, rec_len))) {
+        ERROR("error set hvm buffer!\n");
         goto out;
     }
 
@@ -361,6 +363,7 @@ int xc_hvm_restore(int xc_handle, int io_fd,
     if ( (rc != 0) && (dom != 0) )
         xc_domain_destroy(xc_handle, dom);
     free(p2m);
+    free(hvm_buf);
 
     DPRINTF("Restore exit with rc=%d\n", rc);
 

tools/libxc/xc_hvm_save.c

@@ -32,6 +32,12 @@
 #include "xg_private.h"
 #include "xg_save_restore.h"
 
+/*
+ * Size of a buffer big enough to take the HVM state of a domain.
+ * Ought to calculate this a bit more carefully, or maybe ask Xen.
+ */
+#define HVM_CTXT_SIZE 8192
+
 /*
 ** Default values for important tuning parameters. Can override by passing
 ** non-zero replacement values to xc_hvm_save().
@@ -279,8 +285,8 @@ int xc_hvm_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
     unsigned long *pfn_type = NULL;
     unsigned long *pfn_batch = NULL;
 
-    /* A copy of hvm domain context */
-    hvm_domain_context_t hvm_ctxt;
+    /* A copy of hvm domain context buffer*/
+    uint8_t *hvm_buf = NULL;
 
     /* Live mapping of shared info structure */
     shared_info_t *live_shinfo = NULL;
@@ -423,8 +429,12 @@ int xc_hvm_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
     to_send = malloc(BITMAP_SIZE);
     to_skip = malloc(BITMAP_SIZE);
 
-    if (!to_send ||!to_skip) {
-        ERROR("Couldn't allocate to_send array");
+    page_array = (unsigned long *) malloc( sizeof(unsigned long) * max_pfn);
+
+    hvm_buf = malloc(HVM_CTXT_SIZE);
+
+    if (!to_send ||!to_skip ||!page_array ||!hvm_buf ) {
+        ERROR("Couldn't allocate memory");
         goto out;
     }
 
@@ -444,11 +454,6 @@ int xc_hvm_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
     analysis_phase(xc_handle, dom, max_pfn, to_skip, 0);
 
     /* get all the HVM domain pfns */
-    if ( (page_array = (unsigned long *) malloc (sizeof(unsigned long) * max_pfn)) == NULL) {
-        ERROR("HVM:malloc fail!\n");
-        goto out;
-    }
-
     for ( i = 0; i < max_pfn; i++)
         page_array[i] = i;
 
@@ -655,24 +660,18 @@ int xc_hvm_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
         goto out;
     }
 
-    /* save hvm hypervisor state including pic/pit/shpage */
-    if (mlock(&hvm_ctxt, sizeof(hvm_ctxt))) {
-        ERROR("Unable to mlock ctxt");
-        return 1;
-    }
-
-    if (xc_domain_hvm_getcontext(xc_handle, dom, &hvm_ctxt)){
-        ERROR("HVM:Could not get hvm context");
+    if ( (rec_size = xc_domain_hvm_getcontext(xc_handle, dom, hvm_buf, 
+                                              HVM_CTXT_SIZE)) == -1) {
+        ERROR("HVM:Could not get hvm buffer");
         goto out;
     }
 
-    rec_size = sizeof(hvm_ctxt);
     if (!write_exact(io_fd, &rec_size, sizeof(uint32_t))) {
-        ERROR("error write hvm ctxt size");
+        ERROR("error write hvm buffer size");
         goto out;
     }
 
-    if ( !write_exact(io_fd, &hvm_ctxt, sizeof(hvm_ctxt)) ) {
+    if ( !write_exact(io_fd, hvm_buf, rec_size) ) {
         ERROR("write HVM info failed!\n");
     }
 
@@ -722,6 +721,7 @@ int xc_hvm_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
         }
     }
 
+    free(hvm_buf);
     free(page_array);
 
     free(pfn_type);

tools/libxc/xenctrl.h

@@ -328,13 +328,15 @@ int xc_domain_getinfolist(int xc_handle,
  * This function returns information about the context of a hvm domain
  * @parm xc_handle a handle to an open hypervisor interface
  * @parm domid the domain to get information from
- * @parm hvm_ctxt a pointer to a structure to store the execution context of the
- *            hvm domain
+ * @parm ctxt_buf a pointer to a structure to store the execution context of
+ *            the hvm domain
+ * @parm size the size of ctxt_buf in bytes
  * @return 0 on success, -1 on failure
  */
 int xc_domain_hvm_getcontext(int xc_handle,
                              uint32_t domid,
-                             hvm_domain_context_t *hvm_ctxt);
+                             uint8_t *ctxt_buf,
+                             uint32_t size);
 
 /**
  * This function will set the context for hvm domain
@@ -342,11 +344,13 @@ int xc_domain_hvm_getcontext(int xc_handle,
  * @parm xc_handle a handle to an open hypervisor interface
  * @parm domid the domain to set the hvm domain context for
  * @parm hvm_ctxt pointer to the the hvm context with the values to set
+ * @parm size the size of hvm_ctxt in bytes
  * @return 0 on success, -1 on failure
  */
 int xc_domain_hvm_setcontext(int xc_handle,
                              uint32_t domid,
-                             hvm_domain_context_t *hvm_ctxt);
+                             uint8_t *hvm_ctxt,
+                             uint32_t size);
 
 /**
  * This function returns information about the execution context of a

xen/arch/x86/domctl.c

@@ -288,71 +288,73 @@ long arch_do_domctl(
 
     case XEN_DOMCTL_sethvmcontext:
     { 
-        struct hvm_domain_context *c;
+        struct hvm_domain_context c;
         struct domain             *d;
 
+        c.cur = 0;
+        c.size = domctl->u.hvmcontext.size;
+        c.data = NULL;
+        
         ret = -ESRCH;
         if ( (d = get_domain_by_id(domctl->domain)) == NULL )
             break;
 
-        ret = -ENOMEM;
-        if ( (c = xmalloc(struct hvm_domain_context)) == NULL )
-            goto sethvmcontext_out;
-        
-        ret = -EFAULT;
-        if ( copy_from_guest(c, domctl->u.hvmcontext.ctxt, 1) != 0 )
-            goto sethvmcontext_out;
-        c->size = sizeof (c->data);
-        c->cur = 0;
-
         ret = -EINVAL;
         if ( !is_hvm_domain(d) ) 
             goto sethvmcontext_out;
 
-        ret = hvm_load(d, c);
+        ret = -ENOMEM;
+        if ( (c.data = xmalloc_bytes(c.size)) == NULL )
+            goto sethvmcontext_out;
 
-        xfree(c);
+        ret = -EFAULT;
+        if ( copy_from_guest(c.data, domctl->u.hvmcontext.buffer, c.size) != 0)
+            goto sethvmcontext_out;
+
+        ret = hvm_load(d, &c);
 
     sethvmcontext_out:
-        put_domain(d);
+        if ( c.data != NULL )
+            xfree(c.data);
 
+        put_domain(d);
     }
     break;
 
     case XEN_DOMCTL_gethvmcontext:
     { 
-        struct hvm_domain_context *c;
+        struct hvm_domain_context c;
         struct domain             *d;
 
+        c.cur = 0;
+        c.size = domctl->u.hvmcontext.size;
+        c.data = NULL;
+
         ret = -ESRCH;
         if ( (d = get_domain_by_id(domctl->domain)) == NULL )
             break;
 
-        ret = -ENOMEM;
-        if ( (c = xmalloc(struct hvm_domain_context)) == NULL )
-            goto gethvmcontext_out;
-        memset(c, 0, sizeof(*c));
-        c->size = sizeof (c->data);
-        
-        ret = -ENODATA;
+        ret = -EINVAL;
         if ( !is_hvm_domain(d) ) 
             goto gethvmcontext_out;
-        
-        ret = 0;
-        if (hvm_save(d, c) != 0)
-            ret = -EFAULT;
 
-        if ( copy_to_guest(domctl->u.hvmcontext.ctxt, c, 1) )
-            ret = -EFAULT;
+        ret = -ENOMEM;
+        if ( (c.data = xmalloc_bytes(c.size)) == NULL )
+            goto gethvmcontext_out;
 
-        xfree(c);
+        ret = hvm_save(d, &c);
+
+        if ( copy_to_guest(domctl->u.hvmcontext.buffer, c.data, c.size) != 0 )
+            ret = -EFAULT;
 
         if ( copy_to_guest(u_domctl, domctl, 1) )
             ret = -EFAULT;
 
     gethvmcontext_out:
-        put_domain(d);
+        if ( c.data != NULL )
+            xfree(c.data);
 
+        put_domain(d);
     }
     break;
 

xen/arch/x86/hvm/intercept.c

@@ -205,7 +205,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
     if ( hvm_save_entry(HEADER, 0, h, &hdr) != 0 )
     {
         gdprintk(XENLOG_ERR, "HVM save: failed to write header\n");
-        return -1;
+        return -EFAULT;
     } 
 
     /* Save all available kinds of state */
@@ -219,7 +219,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
             {
                 gdprintk(XENLOG_ERR, 
                          "HVM save: failed to save type %"PRIu16"\n", i);
-                return -1;
+                return -EFAULT;
             } 
         }
     }
@@ -229,7 +229,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
     {
         /* Run out of data */
         gdprintk(XENLOG_ERR, "HVM save: no room for end marker.\n");
-        return -1;
+        return -EFAULT;
     }
 
     /* Save macros should not have let us overrun */

xen/include/asm-x86/hvm/domain.h

@@ -49,8 +49,6 @@ struct hvm_domain {
     spinlock_t             pbuf_lock;
 
     uint64_t               params[HVM_NR_PARAMS];
-
-    struct hvm_domain_context *hvm_ctxt;
 };
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */

xen/include/asm-x86/hvm/support.h

@@ -123,6 +123,13 @@ extern unsigned int opt_hvm_debug_level;
  * Save/restore support 
  */
 
+/* Marshalling and unmarshalling uses a buffer with size and cursor. */
+typedef struct hvm_domain_context {
+    uint32_t cur;
+    uint32_t size;
+    uint8_t *data;
+} hvm_domain_context_t;
+
 /* Marshalling an entry: check space and fill in the header */
 static inline int _hvm_init_entry(struct hvm_domain_context *h,
                                   uint16_t tc, uint16_t inst, uint32_t len)

xen/include/public/domctl.h

@@ -386,18 +386,11 @@ struct xen_domctl_settimeoffset {
 typedef struct xen_domctl_settimeoffset xen_domctl_settimeoffset_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_settimeoffset_t);
  
-#define HVM_CTXT_SIZE        8192
-typedef struct hvm_domain_context {
-    uint32_t cur;
-    uint32_t size;
-    uint8_t data[HVM_CTXT_SIZE];
-} hvm_domain_context_t;
-DEFINE_XEN_GUEST_HANDLE(hvm_domain_context_t);
-
 #define XEN_DOMCTL_gethvmcontext   33
 #define XEN_DOMCTL_sethvmcontext   34
 typedef struct xen_domctl_hvmcontext {
-    XEN_GUEST_HANDLE_64(hvm_domain_context_t) ctxt;  /* IN/OUT */
+    uint32_t size; /* IN/OUT: size of buffer / bytes filled */
+    XEN_GUEST_HANDLE(uint8_t) buffer; /* IN/OUT */
 } xen_domctl_hvmcontext_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_hvmcontext_t);