[HVM] Save/restore: clean up marshalling code

- All entries are now defined as structs and saved/restored
  in self-contained operations.
- Save/restore operations are type-safe, to tie each entry's
  typecode to a particular struct and its length.
- Save/restore handlers are registered once per host instead of
  per domain.
- Detect buffer overrun before it happens and abort.

Signed-off-by: Tim Deegan <Tim.Deegan@xensource.com>

1 files changed, 106 insertions, 219 deletions

diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c
index c41a937b03..d5a9393c1d 100644
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -157,287 +157,174 @@ static inline void hvm_mmio_access(struct vcpu *v,
     }
 }
 
-
-int hvm_register_savevm(struct domain *d,
-                    const char *idstr,
-                    int instance_id,
-                    int version_id,
-                    SaveStateHandler *save_state,
-                    LoadStateHandler *load_state,
-                    void *opaque)
+/* List of handlers for various HVM save and restore types */
+static struct { 
+    hvm_save_handler save;
+    hvm_load_handler load; 
+} hvm_sr_handlers [HVM_SAVE_CODE_MAX + 1] = {{NULL, NULL},};
+
+/* Init-time function to add entries to that list */
+void hvm_register_savevm(uint16_t typecode, 
+                         hvm_save_handler save_state,
+                         hvm_load_handler load_state)
 {
-    HVMStateEntry *se, **pse;
-
-    if ( (se = xmalloc(struct HVMStateEntry)) == NULL ){
-        printk("allocat hvmstate entry fail.\n");
-        return -1;
-    }
-
-    safe_strcpy(se->idstr, idstr);
-
-    se->instance_id = instance_id;
-    se->version_id = version_id;
-    se->save_state = save_state;
-    se->load_state = load_state;
-    se->opaque = opaque;
-    se->next = NULL;
-
-    /* add at the end of list */
-    pse = &d->arch.hvm_domain.first_se;
-    while (*pse != NULL)
-        pse = &(*pse)->next;
-    *pse = se;
-    return 0;
+    ASSERT(typecode <= HVM_SAVE_CODE_MAX);
+    ASSERT(hvm_sr_handlers[typecode].save == NULL);
+    ASSERT(hvm_sr_handlers[typecode].load == NULL);
+    hvm_sr_handlers[typecode].save = save_state;
+    hvm_sr_handlers[typecode].load = load_state;
 }
 
+
 int hvm_save(struct domain *d, hvm_domain_context_t *h)
 {
-    uint32_t len, len_pos, cur_pos;
     uint32_t eax, ebx, ecx, edx;
-    HVMStateEntry *se;
-    char *chgset;
+    char *c;
     struct hvm_save_header hdr;
+    struct hvm_save_end end;
+    hvm_save_handler handler;
+    uint16_t i;
 
     hdr.magic = HVM_FILE_MAGIC;
     hdr.version = HVM_FILE_VERSION;
+
+    /* Save some CPUID bits */
     cpuid(1, &eax, &ebx, &ecx, &edx);
     hdr.cpuid = eax;
-    hvm_put_struct(h, &hdr);
 
-    /* save xen changeset */
-    chgset = strrchr(XEN_CHANGESET, ' ');
-    if ( chgset )
-        chgset++;
-    else
-        chgset = XEN_CHANGESET;
-
-    len = strlen(chgset);
-    hvm_put_8u(h, len);
-    hvm_put_buffer(h, chgset, len);
-
-    for(se = d->arch.hvm_domain.first_se; se != NULL; se = se->next) {
-        /* ID string */
-        len = strnlen(se->idstr, sizeof(se->idstr));
-        hvm_put_8u(h, len);
-        hvm_put_buffer(h, se->idstr, len);
-
-        hvm_put_32u(h, se->instance_id);
-        hvm_put_32u(h, se->version_id);
-
-        /* record size */
-        len_pos = hvm_ctxt_tell(h);
-        hvm_put_32u(h, 0);
+    /* Save xen changeset */
+    c = strrchr(XEN_CHANGESET, ':');
+    if ( c )
+        hdr.changeset = simple_strtoll(c, NULL, 16);
+    else 
+        hdr.changeset = -1ULL; /* Unknown */
 
-        se->save_state(h, se->opaque);
-
-        cur_pos = hvm_ctxt_tell(h);
-        len = cur_pos - len_pos - 4;
-        hvm_ctxt_seek(h, len_pos);
-        hvm_put_32u(h, len);
-        hvm_ctxt_seek(h, cur_pos);
+    if ( hvm_save_entry(HEADER, 0, h, &hdr) != 0 )
+    {
+        gdprintk(XENLOG_ERR, "HVM save: failed to write header\n");
+        return -1;
+    } 
 
+    /* Save all available kinds of state */
+    for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ ) 
+    {
+        handler = hvm_sr_handlers[i].save;
+        if ( handler != NULL ) 
+        {
+            if ( handler(d, h) != 0 ) 
+            {
+                gdprintk(XENLOG_ERR, 
+                         "HVM save: failed to save type %"PRIu16"\n", i);
+                return -1;
+            } 
+        }
     }
 
-    h->size = hvm_ctxt_tell(h);
-    hvm_ctxt_seek(h, 0);
-
-    if (h->size >= HVM_CTXT_SIZE) {
-        printk("hvm_domain_context overflow when hvm_save! need %"PRId32" bytes for use.\n", h->size);
+    /* Save an end-of-file marker */
+    if ( hvm_save_entry(END, 0, h, &end) != 0 )
+    {
+        /* Run out of data */
+        gdprintk(XENLOG_ERR, "HVM save: no room for end marker.\n");
         return -1;
     }
 
+    /* Save macros should not have let us overrun */
+    ASSERT(h->cur <= h->size);
     return 0;
-
-}
-
-static HVMStateEntry *find_se(struct domain *d, const char *idstr, int instance_id)
-{
-    HVMStateEntry *se;
-
-    for(se = d->arch.hvm_domain.first_se; se != NULL; se = se->next) {
-        if (!strncmp(se->idstr, idstr, sizeof(se->idstr)) &&
-            instance_id == se->instance_id){
-            return se;
-        }
-    }
-    return NULL;
 }
 
 int hvm_load(struct domain *d, hvm_domain_context_t *h)
 {
-    uint32_t len, rec_len, rec_pos, instance_id, version_id;
     uint32_t eax, ebx, ecx, edx;
-    HVMStateEntry *se;
-    char idstr[HVM_SE_IDSTR_LEN];
-    xen_changeset_info_t chgset;
-    char *cur_chgset;
-    int ret;
+    char *c;
+    uint64_t cset;
     struct hvm_save_header hdr;
+    struct hvm_save_descriptor *desc;
+    hvm_load_handler handler;
     struct vcpu *v;
-
-    if (h->size >= HVM_CTXT_SIZE) {
-        printk("hvm_load fail! seems hvm_domain_context overflow when hvm_save! need %"PRId32" bytes.\n", h->size);
+    
+    /* Read the save header, which must be first */
+    if ( hvm_load_entry(HEADER, h, &hdr) != 0 ) 
         return -1;
-    }
-
-    hvm_ctxt_seek(h, 0);
-
-    hvm_get_struct(h, &hdr);
 
     if (hdr.magic != HVM_FILE_MAGIC) {
-        printk("HVM restore magic dismatch!\n");
+        gdprintk(XENLOG_ERR, 
+                 "HVM restore: bad magic number %#"PRIx32"\n", hdr.magic);
         return -1;
     }
 
     if (hdr.version != HVM_FILE_VERSION) {
-        printk("HVM restore version dismatch!\n");
+        gdprintk(XENLOG_ERR, 
+                 "HVM restore: unsupported version %u\n", hdr.version);
         return -1;
     }
 
-    /* check cpuid */
     cpuid(1, &eax, &ebx, &ecx, &edx);
-    /*TODO: need difine how big difference is acceptable */
+    /*TODO: need to define how big a difference is acceptable */
     if (hdr.cpuid != eax)
-        printk("warnings: try to restore hvm guest(0x%"PRIx32") "
-               "on a different type processor(0x%"PRIx32").\n",
-                hdr.cpuid,
-                eax);
+        gdprintk(XENLOG_WARNING, "HVM restore: saved CPUID (%#"PRIx32") "
+               "does not match host (%#"PRIx32").\n", hdr.cpuid, eax);
 
 
-    /* check xen change set */
-    cur_chgset = strrchr(XEN_CHANGESET, ' ');
-    if ( cur_chgset )
-        cur_chgset++;
+    c = strrchr(XEN_CHANGESET, ':');
+    if ( hdr.changeset == -1ULL )
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: Xen changeset was not saved.\n");
+    else if ( c == NULL )
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: Xen changeset is not available.\n");
     else
-        cur_chgset = XEN_CHANGESET;
-
-    len = hvm_get_8u(h);
-    if (len > 20) { /*typical length is 18 -- "revision number:changeset id" */
-        printk("wrong change set length %d when hvm restore!\n", len);
-        return -1;
+    {
+        cset = simple_strtoll(c, NULL, 16);
+        if ( hdr.changeset != cset )
+        gdprintk(XENLOG_WARNING, "HVM restore: saved Xen changeset (%#"PRIx64
+                 ") does not match host (%#"PRIx64").\n", hdr.changeset, cset);
     }
 
-    hvm_get_buffer(h, chgset, len);
-    chgset[len] = '\0';
-    if (strncmp(cur_chgset, chgset, len + 1))
-        printk("warnings: try to restore hvm guest(%s) on a different changeset %s.\n",
-                chgset, cur_chgset);
-
-
-    if ( !strcmp(cur_chgset, "unavailable") )
-        printk("warnings: try to restore hvm guest when changeset is unavailable.\n");
-
-
     /* Down all the vcpus: we only re-enable the ones that had state saved. */
     for_each_vcpu(d, v) 
         if ( test_and_set_bit(_VCPUF_down, &v->vcpu_flags) )
             vcpu_sleep_nosync(v);
 
     while(1) {
-        if (hvm_ctxt_end(h)) {
-            break;
-        }
 
-        /* ID string */
-        len = hvm_get_8u(h);
-        if (len > HVM_SE_IDSTR_LEN) {
-            printk("wrong HVM save entry idstr len %d!", len);
+        if ( h->size - h->cur < sizeof(struct hvm_save_descriptor) )
+        {
+            /* Run out of data */
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: save did not end with a null entry\n");
             return -1;
         }
-
-        hvm_get_buffer(h, idstr, len);
-        idstr[len] = '\0';
-
-        instance_id = hvm_get_32u(h);
-        version_id = hvm_get_32u(h);
-
-        printk("HVM S/R Loading \"%s\" instance %#x\n", idstr, instance_id);
-
-        rec_len = hvm_get_32u(h);
-        rec_pos = hvm_ctxt_tell(h);
-
-        se = find_se(d, idstr, instance_id);
-        if (se == NULL) {
-            printk("warnings: hvm load can't find device %s's instance %d!\n",
-                    idstr, instance_id);
-        } else {
-            ret = se->load_state(h, se->opaque, version_id);
-            if (ret < 0)
-                printk("warnings: loading state fail for device %s instance %d!\n",
-                        idstr, instance_id);
+        
+        /* Read the typecode of the next entry  and check for the end-marker */
+        desc = (struct hvm_save_descriptor *)(&h->data[h->cur]);
+        if ( desc->typecode == 0 )
+            return 0; 
+        
+        /* Find the handler for this entry */
+        if ( desc->typecode > HVM_SAVE_CODE_MAX 
+             || (handler = hvm_sr_handlers[desc->typecode].load) == NULL ) 
+        {
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: unknown entry typecode %u\n", 
+                     desc->typecode);
+            return -1;
         }
-                    
 
-        /* make sure to jump end of record */
-        if ( hvm_ctxt_tell(h) - rec_pos != rec_len) {
-            printk("wrong hvm record size, maybe some dismatch between save&restore handler!\n");
+        /* Load the entry */
+        if ( handler(d, h) != 0 ) 
+        {
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: failed to load entry %u/%u\n", 
+                     desc->typecode, desc->instance);
+            return -1;
         }
-        hvm_ctxt_seek(h, rec_pos + rec_len);
     }
 
-    return 0;
+    /* Not reached */
 }
 
 
-#ifdef HVM_DEBUG_SUSPEND
-static void shpage_info(shared_iopage_t *sh)
-{
-
-    vcpu_iodata_t *p = &sh->vcpu_iodata[0];
-    ioreq_t *req = &p->vp_ioreq;
-    printk("*****sharepage_info******!\n");
-    printk("vp_eport=%d\n", p->vp_eport);
-    printk("io packet: "
-                     "state:%x, pvalid: %x, dir:%x, port: %"PRIx64", "
-                     "data: %"PRIx64", count: %"PRIx64", size: %"PRIx64"\n",
-                     req->state, req->data_is_ptr, req->dir, req->addr,
-                     req->data, req->count, req->size);
-}
-#else
-static void shpage_info(shared_iopage_t *sh)
-{
-}
-#endif
-
-static void shpage_save(hvm_domain_context_t *h, void *opaque)
-{
-    /* XXX:no action required for shpage save/restore, since it's in guest memory
-     * keep it for debug purpose only */
-
-#if 0
-    struct shared_iopage *s = opaque;
-    /* XXX:smp */
-    struct ioreq *req = &s->vcpu_iodata[0].vp_ioreq;
-    
-    shpage_info(s);
-
-    hvm_put_buffer(h, (char*)req, sizeof(struct ioreq));
-#endif
-}
-
-static int shpage_load(hvm_domain_context_t *h, void *opaque, int version_id)
-{
-    struct shared_iopage *s = opaque;
-#if 0
-    /* XXX:smp */
-    struct ioreq *req = &s->vcpu_iodata[0].vp_ioreq;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    hvm_get_buffer(h, (char*)req, sizeof(struct ioreq));
-
-
-#endif
-    shpage_info(s);
-    return 0;
-}
-
-void shpage_init(struct domain *d, shared_iopage_t *sp)
-{
-    hvm_register_savevm(d, "xen_hvm_shpage", 0x10, 1, shpage_save, shpage_load, sp);
-}
-
 int hvm_buffered_io_intercept(ioreq_t *p)
 {
     struct vcpu *v = current;