Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support

[Keir Fraser <keir@xen.org>]

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

On 14/12/2010 09:59, "Keir Fraser" <keir@xen.org> wrote:

>>> That's Tim's call. I would personally prefer for Xen to accept
>>> truncated chunks, and extend them with sensible 'old save image' defaults,
>>> such as all-zeroes.
>>> That would be a generic solution to this case which will be reusable
>>> in future, and avoid needlessly creating extra chunk types just for
>>> backward compatibility reasons.
>> 
>> Ok. Let's wait for Tim to answer the call.
> 
> Well I propose the attached patch, and then you can use
> hvm_load_entry_zeroextend() in vlapic.c. I would split this patch into two
> pieces if I apply it (code movement first, then the zeroextend logic).
> 
> It needs an Ack from Tim however.

Forgot to attach the patch. Attached this time.

 -- Keir


[-- Attachment #2: 00-hvm-restore-zeroextend --]
[-- Type: application/octet-stream, Size: 8054 bytes --]

diff -r ab785e37499c xen/arch/x86/hvm/hpet.c
--- a/xen/arch/x86/hvm/hpet.c	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/arch/x86/hvm/hpet.c	Tue Dec 14 09:51:09 2010 +0000
@@ -546,7 +546,7 @@
     spin_lock(&hp->lock);
 
     /* Reload the HPET registers */
-    if ( _hvm_check_entry(h, HVM_SAVE_CODE(HPET), HVM_SAVE_LENGTH(HPET)) )
+    if ( _hvm_check_entry(h, HVM_SAVE_CODE(HPET), HVM_SAVE_LENGTH(HPET), 1) )
     {
         spin_unlock(&hp->lock);
         return -EINVAL;
diff -r ab785e37499c xen/common/hvm/save.c
--- a/xen/common/hvm/save.c	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/common/hvm/save.c	Tue Dec 14 09:51:09 2010 +0000
@@ -244,6 +244,69 @@
     /* Not reached */
 }
 
+int _hvm_init_entry(struct hvm_domain_context *h,
+                    uint16_t tc, uint16_t inst, uint32_t len)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( h->size - h->cur < len + sizeof (*d) )
+    {
+        gdprintk(XENLOG_WARNING,
+                 "HVM save: no room for %"PRIu32" + %u bytes "
+                 "for typecode %"PRIu16"\n",
+                 len, (unsigned) sizeof (*d), tc);
+        return -1;
+    }
+    d->typecode = tc;
+    d->instance = inst;
+    d->length = len;
+    h->cur += sizeof (*d);
+    return 0;
+}
+
+void _hvm_write_entry(struct hvm_domain_context *h,
+                      void *src, uint32_t src_len)
+{
+    memcpy(&h->data[h->cur], src, src_len);
+    h->cur += src_len;
+}
+
+int _hvm_check_entry(struct hvm_domain_context *h, 
+                     uint16_t type, uint32_t len, bool_t strict_length)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( len + sizeof (*d) > h->size - h->cur)
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: not enough data left to read %u bytes "
+                 "for type %u\n", len, type);
+        return -1;
+    }    
+    if ( type != d->typecode || (len < d->length) ||
+         (strict_length && (len != d->length)) )
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore mismatch: expected type %u length %u, "
+                 "saw type %u length %u\n", type, len, d->typecode, d->length);
+        return -1;
+    }
+    h->cur += sizeof (*d);
+    return 0;
+}
+
+void _hvm_read_entry(struct hvm_domain_context *h,
+                     void *dest, uint32_t dest_len)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur - sizeof(*d)];
+    BUG_ON(d->length > dest_len);
+    memcpy(dest, &h->data[h->cur], d->length);
+    if ( d->length < dest_len )
+        memset((char *)dest + d->length, 0, dest_len - d->length);
+    h->cur += d->length;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r ab785e37499c xen/include/xen/hvm/save.h
--- a/xen/include/xen/hvm/save.h	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/include/xen/hvm/save.h	Tue Dec 14 09:51:09 2010 +0000
@@ -30,77 +30,50 @@
 } 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)
-{
-    struct hvm_save_descriptor *d 
-        = (struct hvm_save_descriptor *)&h->data[h->cur];
-    if ( h->size - h->cur < len + sizeof (*d) )
-    {
-        gdprintk(XENLOG_WARNING,
-                 "HVM save: no room for %"PRIu32" + %u bytes "
-                 "for typecode %"PRIu16"\n",
-                 len, (unsigned) sizeof (*d), tc);
-        return -1;
-    }
-    d->typecode = tc;
-    d->instance = inst;
-    d->length = len;
-    h->cur += sizeof (*d);
-    return 0;
-}
+int _hvm_init_entry(struct hvm_domain_context *h,
+                    uint16_t tc, uint16_t inst, uint32_t len);
 
 /* Marshalling: copy the contents in a type-safe way */
-#define _hvm_write_entry(_x, _h, _src) do {                     \
-    *(HVM_SAVE_TYPE(_x) *)(&(_h)->data[(_h)->cur]) = *(_src);   \
-    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
-} while (0)
+void _hvm_write_entry(struct hvm_domain_context *h,
+                      void *src, uint32_t src_len);
 
 /* Marshalling: init and copy; evaluates to zero on success */
-#define hvm_save_entry(_x, _inst, _h, _src) ({          \
-    int r;                                              \
-    r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x),        \
-                        (_inst), HVM_SAVE_LENGTH(_x));  \
-    if ( r == 0 )                                       \
-        _hvm_write_entry(_x, (_h), (_src));             \
+#define hvm_save_entry(_x, _inst, _h, _src) ({                  \
+    int r;                                                      \
+    r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x),                \
+                        (_inst), HVM_SAVE_LENGTH(_x));          \
+    if ( r == 0 )                                               \
+        _hvm_write_entry((_h), (_src), HVM_SAVE_LENGTH(_x));    \
     r; })
 
 /* Unmarshalling: test an entry's size and typecode and record the instance */
-static inline int _hvm_check_entry(struct hvm_domain_context *h, 
-                                   uint16_t type, uint32_t len)
-{
-    struct hvm_save_descriptor *d 
-        = (struct hvm_save_descriptor *)&h->data[h->cur];
-    if ( len + sizeof (*d) > h->size - h->cur)
-    {
-        gdprintk(XENLOG_WARNING, 
-                 "HVM restore: not enough data left to read %u bytes "
-                 "for type %u\n", len, type);
-        return -1;
-    }    
-    if ( type != d->typecode || len != d->length )
-    {
-        gdprintk(XENLOG_WARNING, 
-                 "HVM restore mismatch: expected type %u length %u, "
-                 "saw type %u length %u\n", type, len, d->typecode, d->length);
-        return -1;
-    }
-    h->cur += sizeof (*d);
-    return 0;
-}
+int _hvm_check_entry(struct hvm_domain_context *h, 
+                     uint16_t type, uint32_t len, bool_t strict_length);
 
 /* Unmarshalling: copy the contents in a type-safe way */
-#define _hvm_read_entry(_x, _h, _dst) do {                      \
-    *(_dst) = *(HVM_SAVE_TYPE(_x) *) (&(_h)->data[(_h)->cur]);  \
-    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
-} while (0)
+void _hvm_read_entry(struct hvm_domain_context *h,
+                     void *dest, uint32_t dest_len);
 
-/* Unmarshalling: check, then copy. Evaluates to zero on success. */
-#define hvm_load_entry(_x, _h, _dst) ({                                 \
-    int r;                                                              \
-    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x)); \
-    if ( r == 0 )                                                       \
-        _hvm_read_entry(_x, (_h), (_dst));                              \
+/*
+ * Unmarshalling: check, then copy. Evaluates to zero on success. This load
+ * function requires the save entry to be the same size as the dest structure.
+ */
+#define hvm_load_entry(_x, _h, _dst) ({                                    \
+    int r;                                                                 \
+    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x), 1); \
+    if ( r == 0 )                                                          \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));                \
+    r; })
+
+/*
+ * Unmarshalling: check, then copy. Evaluates to zero on success. This load
+ * function will zero-extend a short save entry to fill th edest structure.
+ */
+#define hvm_load_entry_zeroextend(_x, _h, _dst) ({                         \
+    int r;                                                                 \
+    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x), 0); \
+    if ( r == 0 )                                                          \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));                \
     r; })
 
 /* Unmarshalling: what is the instance ID of the next entry? */

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

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