Re: [PATCH][SVM] Fix 32bit Windows guest VMs save/restore

[Wei Huang <wei.huang2@amd.com>]

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

How does this one look? It should address your concern about duplicating 
data in both vcpu and vmcb. I have tested it with both 32bit and 64bit 
Windows successfully.

-Wei

=================
Fix 32bit guest VM save/restore issues associated with SYSENTER MSRs on 
AMD platforms.

This patch turn-on SYSENTER MSRs interception for 32bit guest VMs on AMD 
CPUs. With it, hvm_svm.guest_sysenter_xx fields always contain the 
canonical version of SYSENTER MSRs and are used in guest save/restore. 
The data fields in VMCB save area are updated as necessary.

Reported-by: James Harper <james.harper@bendigoit.com.au>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
=================


[-- Attachment #2: amd_fix_32bit_save_restore.txt --]
[-- Type: text/plain, Size: 2551 bytes --]

diff -r d972e89797f1 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Mon Jan 31 23:57:19 2011 -0600
+++ b/xen/arch/x86/hvm/svm/svm.c	Tue Feb 01 15:03:15 2011 -0600
@@ -224,10 +224,11 @@
     hvm_update_guest_cr(v, 2);
     hvm_update_guest_cr(v, 4);
 
-    v->arch.hvm_svm.guest_sysenter_cs = c->sysenter_cs;
-    v->arch.hvm_svm.guest_sysenter_esp = c->sysenter_esp;
-    v->arch.hvm_svm.guest_sysenter_eip = c->sysenter_eip;
-
+    /* load sysenter MSRs into VMCB save area and VCPU fields */
+    vmcb->sysenter_cs = v->arch.hvm_svm.guest_sysenter_cs = c->sysenter_cs;
+    vmcb->sysenter_esp = v->arch.hvm_svm.guest_sysenter_esp = c->sysenter_esp;
+    vmcb->sysenter_eip = v->arch.hvm_svm.guest_sysenter_eip = c->sysenter_eip;
+    
     if ( paging_mode_hap(v->domain) )
     {
         vmcb_set_np_enable(vmcb, 1);
@@ -433,14 +434,6 @@
     if ( lma )
         new_efer |= EFER_LME;
     vmcb_set_efer(vmcb, new_efer);
-
-    /*
-     * In legacy mode (EFER.LMA=0) we natively support SYSENTER/SYSEXIT with
-     * no need for MSR intercepts. When EFER.LMA=1 we must trap and emulate.
-     */
-    svm_intercept_msr(v, MSR_IA32_SYSENTER_CS, lma);
-    svm_intercept_msr(v, MSR_IA32_SYSENTER_ESP, lma);
-    svm_intercept_msr(v, MSR_IA32_SYSENTER_EIP, lma);
 }
 
 static void svm_sync_vmcb(struct vcpu *v)
@@ -1142,6 +1135,21 @@
 {
     struct vcpu *v = current;
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+    int sync = 0;
+
+    switch ( msr )
+    {
+    case MSR_IA32_SYSENTER_CS:
+    case MSR_IA32_SYSENTER_ESP:
+    case MSR_IA32_SYSENTER_EIP:
+        sync = 1;
+        break;
+    default:
+        break;
+    }
+
+    if ( sync )
+        svm_sync_vmcb(v);    
 
     switch ( msr )
     {
@@ -1149,13 +1157,13 @@
         goto gpf;
 
     case MSR_IA32_SYSENTER_CS:
-        v->arch.hvm_svm.guest_sysenter_cs = msr_content;
+        vmcb->sysenter_cs = v->arch.hvm_svm.guest_sysenter_cs = msr_content;
         break;
     case MSR_IA32_SYSENTER_ESP:
-        v->arch.hvm_svm.guest_sysenter_esp = msr_content;
+        vmcb->sysenter_esp = v->arch.hvm_svm.guest_sysenter_esp = msr_content;
         break;
     case MSR_IA32_SYSENTER_EIP:
-        v->arch.hvm_svm.guest_sysenter_eip = msr_content;
+        vmcb->sysenter_eip = v->arch.hvm_svm.guest_sysenter_eip = msr_content;
         break;
 
     case MSR_IA32_DEBUGCTLMSR:
@@ -1213,6 +1221,10 @@
         wrmsr_hypervisor_regs(msr, msr_content);
         break;
     }
+
+    if ( sync )
+        svm_vmload(vmcb);
+
     return X86EMUL_OKAY;
 
  gpf:

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

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