Skip to contentSkip to conversation

RFC: hvm_save backwards compatibility

PATCH In review George Dunlap 5 participants 7 messages Mar 31, 2011 – Apr 6, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RFC: hvm_save backwards compatibility

This patch series introduces the ability to be backwards-compatible with previous versions of hvm_save structures. The first patch introduces the compatibility code; the second takes advantage of it to allow Xen to handle hvm_hw_cpu entries from before Xen 4.0.

#1 lore raw

PATCH 1/1 xen: hvm_save: infrastructure for backwards compatibility

The hvm_save code is used to save and restore hypervisor-related hvm state, either for classic save/restore, or for migration (including remus). This is meant to be backwards-compatible across some hypervisor versions; but if it does change, there is no way to handle the old format as well as the new.

This patch introduces the infrastructure to allow a single older version ("compat") of any given "save type" to be defined, along with a function to turn the "old" version into the "new" version. If the size check fails for the "normal" version, it will check the "compat" version, and if it matches, will read the old entry and call the conversion function.

This patch involves some preprocessor hackery, but I'm only extending the hackery that's already there.

xen/include/public/hvm/save.h +24 -2
diff -r 45eeeb6d0481 -r ba5c95ba74d3 xen/include/public/hvm/save.h
@@ -61,13 +61,35 @@
  * ugliness.
  */
 
-#define DECLARE_HVM_SAVE_TYPE(_x, _code, _type)                   \
-  struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; }
+#ifdef __XEN__
+# define DECLARE_HVM_SAVE_TYPE_COMPAT(_x, _code, _type, _ctype, _fix)     \
+    static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h) { return _fix(h); } \
+    struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[2];}; \
+    struct __HVM_SAVE_TYPE_COMPAT_##_x { _ctype t; }                   
+
+# define DECLARE_HVM_SAVE_TYPE(_x, _code, _type)                         \
+    static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h) { BUG(); return -1; } \
+    struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[1];}; \
+    struct __HVM_SAVE_TYPE_COMPAT_##_x { _type t; }                   
+#else
+# define DECLARE_HVM_SAVE_TYPE_COMPAT(_x, _code, _type, _ctype, _fix)     \
+    struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[2];} 
+
+# define DECLARE_HVM_SAVE_TYPE(_x, _code, _type)                         \
+    struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[1];} 
+#endif
 
 #define HVM_SAVE_TYPE(_x) typeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->t)
 #define HVM_SAVE_LENGTH(_x) (sizeof (HVM_SAVE_TYPE(_x)))
 #define HVM_SAVE_CODE(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->c))
 
+#ifdef __XEN__
+# define HVM_SAVE_TYPE_COMPAT(_x) typeof (((struct __HVM_SAVE_TYPE_COMPAT_##_x *)(0))->t)
+# define HVM_SAVE_LENGTH_COMPAT(_x) (sizeof (HVM_SAVE_TYPE_COMPAT(_x)))
+
+# define HVM_SAVE_HAS_COMPAT(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->cpt)-1)
+# define HVM_SAVE_FIX_COMPAT(_x, _dst) __HVM_SAVE_FIX_COMPAT_##_x(_dst)
+#endif
 
 /* 
  * The series of save records is teminated by a zero-type, zero-length 
xen/include/xen/hvm/save.h +12 -6
diff -r 45eeeb6d0481 -r ba5c95ba74d3 xen/include/xen/hvm/save.h
@@ -58,13 +58,19 @@
  * 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, _strict) ({               \
-    int r;                                                      \
-    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x),               \
-                         HVM_SAVE_LENGTH(_x), (_strict));       \
-    if ( r == 0 )                                               \
-        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));     \
+#define _hvm_load_entry(_x, _h, _dst, _strict) ({                       \
+    int r;                                                              \
+    if ( (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x),                 \
+               HVM_SAVE_LENGTH(_x), (_strict))) == 0 )                  \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));             \
+    else if (HVM_SAVE_HAS_COMPAT(_x)                                    \
+             && (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x),          \
+                       HVM_SAVE_LENGTH_COMPAT(_x), (_strict))) == 0 ) { \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH_COMPAT(_x));      \
+        r=HVM_SAVE_FIX_COMPAT(_x, (_dst));                              \
+    }                                                                   \
     r; })
+
 #define hvm_load_entry(_x, _h, _dst)            \
     _hvm_load_entry(_x, _h, _dst, 1)
 #define hvm_load_entry_zeroextend(_x, _h, _dst) \
#2 lore raw

PATCH 1/1 xen: hvm_save: Add pre-3.4 compatibility for hvm_hw_cpu

In reply to George Dunlap

Xen 4.0 added "msr_tsc_aux" in the middle of the hvm_hw_cpu structure, making it incompatible with pre-3.4 savefiles. This patch uses the recently introduced backwards-compatibility infrastructure to convert the old to the new.

xen/include/public/arch-x86/hvm/save.h +126 -1
diff -r ba5c95ba74d3 -r e2f2377fedef xen/include/public/arch-x86/hvm/save.h
@@ -46,6 +46,8 @@
 
 /*
  * Processor
+ *
+ * Compat: Pre-3.4 didn't have msr_tsc_aux
  */
 
 struct hvm_hw_cpu {
@@ -157,8 +159,131 @@
     uint32_t error_code;
 };
 
-DECLARE_HVM_SAVE_TYPE(CPU, 2, struct hvm_hw_cpu);
+struct hvm_hw_cpu_compat {
+    uint8_t  fpu_regs[512];
 
+    uint64_t rax;
+    uint64_t rbx;
+    uint64_t rcx;
+    uint64_t rdx;
+    uint64_t rbp;
+    uint64_t rsi;
+    uint64_t rdi;
+    uint64_t rsp;
+    uint64_t r8;
+    uint64_t r9;
+    uint64_t r10;
+    uint64_t r11;
+    uint64_t r12;
+    uint64_t r13;
+    uint64_t r14;
+    uint64_t r15;
+
+    uint64_t rip;
+    uint64_t rflags;
+
+    uint64_t cr0;
+    uint64_t cr2;
+    uint64_t cr3;
+    uint64_t cr4;
+
+    uint64_t dr0;
+    uint64_t dr1;
+    uint64_t dr2;
+    uint64_t dr3;
+    uint64_t dr6;
+    uint64_t dr7;    
+
+    uint32_t cs_sel;
+    uint32_t ds_sel;
+    uint32_t es_sel;
+    uint32_t fs_sel;
+    uint32_t gs_sel;
+    uint32_t ss_sel;
+    uint32_t tr_sel;
+    uint32_t ldtr_sel;
+
+    uint32_t cs_limit;
+    uint32_t ds_limit;
+    uint32_t es_limit;
+    uint32_t fs_limit;
+    uint32_t gs_limit;
+    uint32_t ss_limit;
+    uint32_t tr_limit;
+    uint32_t ldtr_limit;
+    uint32_t idtr_limit;
+    uint32_t gdtr_limit;
+
+    uint64_t cs_base;
+    uint64_t ds_base;
+    uint64_t es_base;
+    uint64_t fs_base;
+    uint64_t gs_base;
+    uint64_t ss_base;
+    uint64_t tr_base;
+    uint64_t ldtr_base;
+    uint64_t idtr_base;
+    uint64_t gdtr_base;
+
+    uint32_t cs_arbytes;
+    uint32_t ds_arbytes;
+    uint32_t es_arbytes;
+    uint32_t fs_arbytes;
+    uint32_t gs_arbytes;
+    uint32_t ss_arbytes;
+    uint32_t tr_arbytes;
+    uint32_t ldtr_arbytes;
+
+    uint64_t sysenter_cs;
+    uint64_t sysenter_esp;
+    uint64_t sysenter_eip;
+
+    /* msr for em64t */
+    uint64_t shadow_gs;
+
+    /* msr content saved/restored. */
+    uint64_t msr_flags;
+    uint64_t msr_lstar;
+    uint64_t msr_star;
+    uint64_t msr_cstar;
+    uint64_t msr_syscall_mask;
+    uint64_t msr_efer;
+    /*uint64_t msr_tsc_aux; COMPAT */
+
+    /* guest's idea of what rdtsc() would return */
+    uint64_t tsc;
+
+    /* pending event, if any */
+    union {
+        uint32_t pending_event;
+        struct {
+            uint8_t  pending_vector:8;
+            uint8_t  pending_type:3;
+            uint8_t  pending_error_valid:1;
+            uint32_t pending_reserved:19;
+            uint8_t  pending_valid:1;
+        };
+    };
+    /* error code for pending event */
+    uint32_t error_code;
+};
+
+static inline int _hvm_hw_fix_cpu(void *h) {
+    struct hvm_hw_cpu *new=h;
+    struct hvm_hw_cpu_compat *old=h;
+
+    /* If we copy from the end backwards, we should
+     * be able to do the modification in-place */
+    new->error_code=old->error_code;
+    new->pending_event=old->pending_event;
+    new->tsc=old->tsc;
+    new->msr_tsc_aux=0;
+
+    return 0;
+}
+
+DECLARE_HVM_SAVE_TYPE_COMPAT(CPU, 2, struct hvm_hw_cpu, \
+                             struct hvm_hw_cpu_compat, _hvm_hw_fix_cpu);
 
 /*
  * PIC
#3 lore raw

Re: [PATCH 2 of 2] xen: hvm_save: Add pre-3.4 compatibility for hvm_hw_cpu

xen/include/public/arch-x86/hvm/save.h line 50

Quoted 6 lines · George Dunlap
xen/include/public/arch-x86/hvm/save.h +2 -0
@@ -46,6 +46,8 @@
 
 /*
  * Processor
+ *
+ * Compat: Pre-3.4 didn't have msr_tsc_aux

Do you mean Pre-4.0? i.e. 3.4 and earlier?

xen/include/public/arch-x86/hvm/save.h line 162

Quoted 9 lines
xen/include/public/arch-x86/hvm/save.h +1 -1
  */
 
 struct hvm_hw_cpu {
@@ -157,8 +159,131 @@
     uint32_t error_code;
 };
 
-DECLARE_HVM_SAVE_TYPE(CPU, 2, struct hvm_hw_cpu);
+struct hvm_hw_cpu_compat { 

"struct hvm_hw_cpu_3_4_compat" to make it clear which version compat is with?

Ian.

#4 lore raw

Re: [PATCH 2 of 2] xen: hvm_save: Add pre-3.4 compatibility for hvm_hw_cpu

Quoted 9 lines · Ian Campbell

xen/include/public/arch-x86/hvm/save.h line 50

Quoted 6 lines · George Dunlap
xen/include/public/arch-x86/hvm/save.h +2 -0
@@ -46,6 +46,8 @@
 
 /*
  * Processor
+ *
+ * Compat: Pre-3.4 didn't have msr_tsc_aux

Do you mean Pre-4.0? i.e. 3.4 and earlier?

Yes, I noticed that a bit too late. :-)

Quoted 8 lines
Quoted 8 lines
xen/include/public/arch-x86/hvm/save.h +0 -0
  */
 
 struct hvm_hw_cpu {
@@ -157,8 +159,131 @@
     uint32_t error_code;
 };
 
Quoted 5 lines

xen/include/public/arch-x86/hvm/save.h line 162

Quoted 2 lines
xen/include/public/arch-x86/hvm/save.h +1 -1
-DECLARE_HVM_SAVE_TYPE(CPU, 2, struct hvm_hw_cpu);
+struct hvm_hw_cpu_compat { 

"struct hvm_hw_cpu_3_4_compat" to make it clear which version compat is with?

Sounds like a good idea. If Tim thinks it's worth including, I'll spin up another series.

-George

#5 lore raw

Re: [PATCH 2 of 2] xen: hvm_save: Add pre-3.4 compatibility for hvm_hw_cpu

In reply to George Dunlap

Hi,

Quoted 2 lines · George Dunlap

Xen 4.0 added "msr_tsc_aux" in the middle of the hvm_hw_cpu structure, making it incompatible with pre-3.4 savefiles.

How embarrassing. :( Thanks for the fix; I've applied both patches.

Cheers,

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
#6 lore raw
In reply to George Dunlap
Quoted 4 lines · George Dunlap

This patch series introduces the ability to be backwards-compatible with previous versions of hvm_save structures. The first patch introduces the compatibility code; the second takes advantage of it to allow Xen to handle hvm_hw_cpu entries from before Xen 4.0.

Ends up with quite a bit of struct duplication, but maybe that's still the clearest and best way. Anyhow, I'll let Tim Ack and apply these. If they look okay we can then backport to the 4.1 and 4.0 branches.

Thanks,
Keir

Quoted 5 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel