HVM save restore: vcpu context support

PATCH In review Zhai, Edwin 5 participants 7 messages Jan 11, 2007 – Jan 19, 2007

lore

Share & tools

lore

Zhai, Edwin

author

2007-01-11 14:10 UTC #0 lore raw

PATCH 4/8 HVM save restore: vcpu context support

[PATCH 4/8] HVM save restore: vcpu context support

Signed-off-by Zhai Edwin <[email protected]>

save/restore HVM vcpu context such as vmcs

xen/arch/x86/domain.c +1 -0

diff -r ee20d1905bde xen/arch/x86/domain.c
@@ -573,6 +573,7 @@ int arch_set_info_guest(
     else
     {
         hvm_load_cpu_guest_regs(v, &v->arch.guest_context.user_regs);
+        hvm_load_cpu_context(v, &v->arch.guest_context.hvmcpu_ctxt);
     }
 
     if ( test_bit(_VCPUF_initialised, &v->vcpu_flags) )

xen/arch/x86/domctl.c +3 -1

diff -r ee20d1905bde xen/arch/x86/domctl.c
@@ -322,8 +322,10 @@ void arch_get_info_guest(struct vcpu *v,
 
     if ( is_hvm_vcpu(v) )
     {
-        if ( !IS_COMPAT(v->domain) )
+        if ( !IS_COMPAT(v->domain) ) {
             hvm_store_cpu_guest_regs(v, &c.nat->user_regs, c.nat->ctrlreg);
+            hvm_save_cpu_context(v, &c.nat->hvmcpu_ctxt);
+        }
 #ifdef CONFIG_COMPAT
         else
         {

xen/arch/x86/hvm/vmx/vmx.c +296 -0

308 lines not on this page · raw message

xen/include/asm-x86/hvm/hvm.h +36 -2

diff -r ee20d1905bde xen/include/asm-x86/hvm/hvm.h
@@ -79,6 +79,13 @@ struct hvm_function_table {
         struct vcpu *v, struct cpu_user_regs *r, unsigned long *crs);
     void (*load_cpu_guest_regs)(
         struct vcpu *v, struct cpu_user_regs *r);
+
+    /* save and load hvm guest cpu context for save/restore */
+    void (*save_cpu_ctxt)(
+        struct vcpu *v, struct hvmcpu_context *ctxt);
+    void (*load_cpu_ctxt)(
+        struct vcpu *v, struct hvmcpu_context *ctxt);
+
     /*
      * Examine specifics of the guest state:
      * 1) determine whether paging is enabled,

@@ -157,6 +164,35 @@ hvm_load_cpu_guest_regs(struct vcpu *v, 
     hvm_funcs.load_cpu_guest_regs(v, r);
 }
 
+void hvm_set_guest_time(struct vcpu *v, u64 gtime);
+u64 hvm_get_guest_time(struct vcpu *v);
+
+static inline void
+hvm_save_cpu_context(
+        struct vcpu *v, struct hvmcpu_context *ctxt)
+{
+    hvm_funcs.save_cpu_ctxt(v, ctxt);
+
+    /* save guest time */
+    ctxt->gtime = hvm_get_guest_time(v);
+
+    /* set valid flag to recover whole vmcs when restore */
+    ctxt->valid = 0x55885588;
+}
+
+static inline void
+hvm_load_cpu_context(
+        struct vcpu *v, struct hvmcpu_context *ctxt)
+{
+    if ( ctxt->valid != 0x55885588)
+        return;
+
+    hvm_funcs.load_cpu_ctxt(v, ctxt);
+
+    /* restore guest time*/
+    hvm_set_guest_time(v, ctxt->gtime);
+}
+
 static inline int
 hvm_paging_enabled(struct vcpu *v)
 {

@@ -222,8 +258,6 @@ void hvm_cpuid(unsigned int input, unsig
 void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                    unsigned int *ecx, unsigned int *edx);
 void hvm_stts(struct vcpu *v);
-void hvm_set_guest_time(struct vcpu *v, u64 gtime);
-u64 hvm_get_guest_time(struct vcpu *v);
 void hvm_migrate_timers(struct vcpu *v);
 void hvm_do_resume(struct vcpu *v);

xen/include/public/arch-x86/xen.h +65 -0

77 lines not on this page · raw message

xen/include/xlat.lst +2 -0

diff -r ee20d1905bde xen/include/xlat.lst
@@ -8,6 +8,8 @@
 ?	vcpu_time_info			xen.h
 !	cpu_user_regs			arch-x86/xen-@[email protected]
 !	trap_info			arch-x86/xen.h
+!	hvmcpu_context			arch-x86/xen.h
+!	vmcs_data			arch-x86/xen.h
 !	vcpu_guest_context		arch-x86/xen.h
 ?	acm_getdecision			acm_ops.h
 !	ctl_cpumap			domctl.h

Anthony Liguori

2007-01-11 17:38 UTC #1 lore raw

Quoted 8 lines · Edwin

[PATCH 4/8] HVM save restore: vcpu context support
Signed-off-by Zhai Edwin <[email protected]>
typedef uint64_t tsc_timestamp_t; /* RDTSC timestamp */
xen/include/public/arch-x86/xen.h +3 -0
+
+/*
+ * World vmcs state

xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 122

Quoted 10 lines

xen/include/public/arch-x86/xen.h +10 -0

+ */
+struct vmcs_data {
+    uint64_t  eip;        /* execution pointer */
+    uint64_t  esp;        /* stack pointer */
+    uint64_t  eflags;     /* flags register */
+    uint64_t  cr0;
+    uint64_t  cr3;        /* page table directory */
+    uint64_t  cr4;
+    uint32_t  idtr_limit; /* idt */
+    uint64_t  idtr_base;

If I read the code correctly, vmcs_data ends up becoming part of:

+
+#define HVM_CTXT_SIZE 6144
+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);

Which then gets saved to disk. My first concern would be that struct vmcs_data is not padding safe. How idtr_limit gets padding may change in future versions of GCC which would break the save format.

The second is how HVM_CTXT_SIZE gets defined. Not sure there's a great way to address though (although the first issue is definitely fixable).

Regards,

Anthony Liguori

Zhai, Edwin

author

2007-01-12 01:59 UTC #2 lore raw

Quoted 22 lines · Anthony Liguori

Quoted 8 lines · Edwin
[PATCH 4/8] HVM save restore: vcpu context support
Signed-off-by Zhai Edwin <[email protected]>
typedef uint64_t tsc_timestamp_t; /* RDTSC timestamp */
xen/include/public/arch-x86/xen.h +3 -0
+
+/*
+ * World vmcs state
xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 122
Quoted 10 lines
xen/include/public/arch-x86/xen.h +10 -0
+ */
+struct vmcs_data {
+    uint64_t  eip;        /* execution pointer */
+    uint64_t  esp;        /* stack pointer */
+    uint64_t  eflags;     /* flags register */
+    uint64_t  cr0;
+    uint64_t  cr3;        /* page table directory */
+    uint64_t  cr4;
+    uint32_t  idtr_limit; /* idt */
+    uint64_t  idtr_base;
If I read the code correctly, vmcs_data ends up becoming part of:

Review

Quoted 8 lines

(quoted diff) +8 -0

+
+#define HVM_CTXT_SIZE        6144
+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);

vmcs_data ends up as part of vcpu_guest_context. hvm_domain_context is a long buffer for saving dev state in HV.

Quoted 6 lines

Which then gets saved to disk. My first concern would be that struct vmcs_data is not padding safe. How idtr_limit gets padding may change in future versions of GCC which would break the save format.
The second is how HVM_CTXT_SIZE gets defined. Not sure there's a great

i just define a big enough buffer for hvm context and handle overflow.

the data true length dynamically increase when more vcpus come out, so seems hard to let control panel know it.

Quoted 6 lines

way to address though (although the first issue is definitely fixable).
Regards,
Anthony Liguori

Signature · 2 lines

best rgds,
edwin

Petersson, Mats

2007-01-19 15:30 UTC #3 lore raw

In reply to Zhai, Edwin

I know this is late, because the patch has alreyad gone in, and appologies if I've misunderstood something here...

[snip

Quoted 1 line

+++ b/xen/include/public/arch-x86/xen.h Thu Jan 11

Xen.h is a generic file

xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 114

Quoted 10 lines

16:51:03 2007 +0800

xen/include/public/arch-x86/xen.h +5 -0

@@ -107,6 +107,70 @@ DEFINE_XEN_GUEST_HANDLE(trap_info_t);
 DEFINE_XEN_GUEST_HANDLE(trap_info_t);
 
 typedef uint64_t tsc_timestamp_t; /* RDTSC timestamp */
+
+/*
+ * World vmcs state
+ */
+struct vmcs_data {

The name vmcs data indicate that it's Intel architecture specific to me.

Even if we agree on a format that supports both AMD and Intel, shouldn't this sort of info be in some HVM specific file? [I haven't yet looked at the differences that may exisit between AMD and Intel].

Quoted 4 lines

xen/include/public/arch-x86/xen.h +4 -0

+    uint64_t  eip;        /* execution pointer */
+    uint64_t  esp;        /* stack pointer */
+    uint64_t  eflags;     /* flags register */
+    uint64_t  cr0;

xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 128

Quoted 10 lines

xen/include/public/arch-x86/xen.h +10 -0

+    uint64_t  cr3;        /* page table directory */
+    uint64_t  cr4;
+    uint32_t  idtr_limit; /* idt */
+    uint64_t  idtr_base;
+    uint32_t  gdtr_limit; /* gdt */
+    uint64_t  gdtr_base;
+    uint32_t  cs_sel;     /* cs selector */
+    uint32_t  cs_limit;
+    uint64_t  cs_base;
+    uint32_t  cs_arbytes;

Defining a struct for the segment info that is then used for all segments would make this repeat a whole lot less...

Quoted 29 lines

xen/include/public/arch-x86/xen.h +29 -0

+    uint32_t  ds_sel;     /* ds selector */
+    uint32_t  ds_limit;
+    uint64_t  ds_base;
+    uint32_t  ds_arbytes;
+    uint32_t  es_sel;     /* es selector */
+    uint32_t  es_limit;
+    uint64_t  es_base;
+    uint32_t  es_arbytes;
+    uint32_t  ss_sel;     /* ss selector */
+    uint32_t  ss_limit;
+    uint64_t  ss_base;
+    uint32_t  ss_arbytes;
+    uint32_t  fs_sel;     /* fs selector */
+    uint32_t  fs_limit;
+    uint64_t  fs_base;
+    uint32_t  fs_arbytes;
+    uint32_t  gs_sel;     /* gs selector */
+    uint32_t  gs_limit;
+    uint64_t  gs_base;
+    uint32_t  gs_arbytes;
+    uint32_t  tr_sel;     /* task selector */
+    uint32_t  tr_limit;
+    uint64_t  tr_base;
+    uint32_t  tr_arbytes;
+    uint32_t  ldtr_sel;   /* ldtr selector */
+    uint32_t  ldtr_limit;
+    uint64_t  ldtr_base;
+    uint32_t  ldtr_arbytes;
+    uint32_t  sysenter_cs;

xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 167

Quoted 10 lines

xen/include/public/arch-x86/xen.h +10 -0

+    uint64_t  sysenter_esp;
+    uint64_t  sysenter_eip;
+    /* msr for em64t */
+    uint64_t shadow_gs;
+    uint64_t flags;
+    /* same size as VMX_MSR_COUNT */
+    uint64_t msr_items[6];
+    uint64_t vmxassist_enabled;
+};
+typedef struct vmcs_data vmcs_data_t;

Is vmcs_data_t used anywhere?

xen/include/public/arch-x86/xen.h DEFINE_XEN_GUEST_HANDLE(trap_info_t); · line 173

Quoted 6 lines

xen/include/public/arch-x86/xen.h +6 -0

+
+struct hvmcpu_context {
+    uint32_t valid;
+    struct vmcs_data data;
+    uint64_t gtime;
+};

Surely this struct should also be in a HVM directory, as it's specific to HVM domains?

--
Mats

Quoted 33 lines

(quoted diff) +0 -0
 /*
  * The following is all CPU context. Note that the fpu_ctxt 
block is filled
(quoted diff) +0 -0
@@ -154,6 +218,7 @@ struct vcpu_guest_context {
 #endif
 #endif
     unsigned long vm_assist;                /* VMASST_TYPE_* 
bitmap */
+ struct hvmcpu_context hvmcpu_ctxt; /* whole vmcs region */
 #ifdef __x86_64__
     /* Segment base addresses. */
     uint64_t      fs_base;
diff -r ee20d1905bde xen/include/xlat.lst
xen/include/xlat.lst +2 -0
@@ -8,6 +8,8 @@
 ?	vcpu_time_info			xen.h
 !	cpu_user_regs			arch-x86/xen-@[email protected]
 !	trap_info			arch-x86/xen.h
+!	hvmcpu_context			arch-x86/xen.h
+!	vmcs_data			arch-x86/xen.h
 !	vcpu_guest_context		arch-x86/xen.h
 ?	acm_getdecision			acm_ops.h
 !	ctl_cpumap			domctl.h
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Tim Deegan

2007-01-19 15:39 UTC #4 lore raw

Hi,

Quoted 5 lines · Mats

The name vmcs data indicate that it's Intel architecture specific to me.
Even if we agree on a format that supports both AMD and Intel, shouldn't this sort of info be in some HVM specific file? [I haven't yet looked at the differences that may exisit between AMD and Intel].

I'll be committing some changes to the HVM save-restore in the next few days. This structure will be renamed and pulled out into a public hvm-specific file (and reorganised quite a bit to make fields be aligned nicely). I hope that any AMD/Intel differences can be folded into this structure -- can you let me know what fields you'll need?

Cheers,

Tim.

Petersson, Mats

2007-01-19 15:45 UTC #5 lore raw

Quoted 22 lines

-----Original Message-----
From: Tim Deegan [mailto:[email protected]]
Sent: 19 January 2007 15:39
To: Petersson, Mats
Cc: Zhai, Edwin; Ian Pratt; Keir Fraser; [email protected]
Subject: Re: [Xen-devel] [PATCH 4/8] HVM save restore: vcpu context support
Hi,
Quoted 1 line · Mats
The name vmcs data indicate that it's Intel architecture
specific to me.
Quoted 2 lines
Even if we agree on a format that supports both AMD and
Intel, shouldn't
Quoted 1 line
this sort of info be in some HVM specific file? [I haven't
yet looked at
Quoted 1 line
the differences that may exisit between AMD and Intel].

Quoted 7 lines

I'll be committing some changes to the HVM save-restore in the next few
days. This structure will be renamed and pulled out into a public hvm-specific file (and reorganised quite a bit to make fields be aligned
nicely). I hope that any AMD/Intel differences can be folded into this structure -- can you let me know what fields you'll need?

Yes, of course. So I'll just add my fields to that struct if I need any new ones, and you'll rename it later, is that the plan?

Of course, if you beat me to it with updating it, I'll just move along with that.

I only started looking at this a couple of hours ago...

Is this supposed to be complete, or are there more patches coming?

--
Mats

Quoted 7 lines

Cheers,
Tim.

Keir Fraser

2007-01-19 15:54 UTC #6 lore raw

Quoted 9 lines · Mats

Yes, of course. So I'll just add my fields to that struct if I need any new ones, and you'll rename it later, is that the plan?
Of course, if you beat me to it with updating it, I'll just move along with that.
I only started looking at this a couple of hours ago...
Is this supposed to be complete, or are there more patches coming?

This code is very much in flux. I'm certain there'll be more changes beyond Tim's initial large-scale cleanup. The initial effort is to get the interfaces looking along the right lines so we can all pull in the same direction.

-- Keir