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

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

[Wei Huang]

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

The attached patch fixes the save/restore issue seen with 32bit Windows
guest VMs. The root cause is that current Xen doesn't intercept
SYSENTER-related MSRs for 32bit guest VMs. As a result, the
guest_sysenter_xxx fields contain incorrect values and shouldn't be used
for save/restore. This patch checks the LMA bit of EFER register in the
save/restore code path.

Please apply it to both Xen-4.0 and Xen-unstable trees.

Reported-by: James Harper <james.harper@bendigoit.com.au>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
Acked-by: Christoph Egger <christoph.egger@amd.com>



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

diff -r 5ce41defa1fa xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Wed Jan 26 16:11:41 2011 +0000
+++ b/xen/arch/x86/hvm/svm/svm.c	Sun Jan 30 23:38:00 2011 -0600
@@ -157,6 +157,7 @@
 
 static int svm_vmcb_save(struct vcpu *v, struct hvm_hw_cpu *c)
 {
+    bool_t lma = !!(v->arch.hvm_vcpu.guest_efer & EFER_LMA);
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
 
     c->cr0 = v->arch.hvm_vcpu.guest_cr[0];
@@ -164,9 +165,20 @@
     c->cr3 = v->arch.hvm_vcpu.guest_cr[3];
     c->cr4 = v->arch.hvm_vcpu.guest_cr[4];
 
-    c->sysenter_cs = v->arch.hvm_svm.guest_sysenter_cs;
-    c->sysenter_esp = v->arch.hvm_svm.guest_sysenter_esp;
-    c->sysenter_eip = v->arch.hvm_svm.guest_sysenter_eip;
+    /* SYSENTER MSRs aren't intercepted under 32bit mode. So the data should 
+     * be copied from VMCB save area under 32bit mode. */
+    if ( lma ) 
+    {
+        c->sysenter_cs = v->arch.hvm_svm.guest_sysenter_cs;
+        c->sysenter_esp = v->arch.hvm_svm.guest_sysenter_esp;
+        c->sysenter_eip = v->arch.hvm_svm.guest_sysenter_eip;
+    } 
+    else
+    {
+        c->sysenter_cs = vmcb->sysenter_cs;
+        c->sysenter_esp = vmcb->sysenter_esp;
+        c->sysenter_eip = vmcb->sysenter_eip;
+    }
 
     c->pending_event = 0;
     c->error_code = 0;
@@ -185,8 +197,12 @@
 {
     unsigned long mfn = 0;
     p2m_type_t p2mt;
+    bool_t lma;
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
     struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+    
+    /* Note: Please make sure guest_efer been restored at this point. */
+    lma = !!(v->arch.hvm_vcpu.guest_efer & EFER_LMA);
 
     if ( c->pending_valid &&
          ((c->pending_type == 1) || (c->pending_type > 6) ||
@@ -224,9 +240,19 @@
     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;
+    /* Copy data into VMCB save area under 32bit mode */
+    if ( lma )
+    {
+        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;
+    }
+    else
+    {
+        vmcb->sysenter_cs = c->sysenter_cs;
+        vmcb->sysenter_esp = c->sysenter_esp;
+        vmcb->sysenter_eip = c->sysenter_eip;
+    }
 
     if ( paging_mode_hap(v->domain) )
     {

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

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

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

[Keir Fraser]

On 31/01/2011 18:33, "Wei Huang" <wei.huang2@amd.com> wrote:

> The attached patch fixes the save/restore issue seen with 32bit Windows
> guest VMs. The root cause is that current Xen doesn't intercept
> SYSENTER-related MSRs for 32bit guest VMs. As a result, the
> guest_sysenter_xxx fields contain incorrect values and shouldn't be used
> for save/restore. This patch checks the LMA bit of EFER register in the
> save/restore code path.
>
> Please apply it to both Xen-4.0 and Xen-unstable trees.
> 
> Reported-by: James Harper <james.harper@bendigoit.com.au>
> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> Acked-by: Christoph Egger <christoph.egger@amd.com>

Nacked-by: Keir Fraser <keir@xen.org>

This handling of the SYSENTER MSRs is overly complicated. I suggest
reverting a bunch of the original handling of cross-vendor migration as
follows:
 * Never intercept the SYSENTER MSRs.
 * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
 * Always hvm save/restore from/to the values in the vmcb.
 * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb() and
then read the sysenter msr value from vmcb
 * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
then modify the sysenter msr in the vmcb, and then svm_vmload().

Result is that we get rid of some redundant fields from the vcpu structure
and have one canonical place we always keep the sysenter msr values, in the
vmcb. The extra cost in the msr read/write functions is totally
inconsequential, and only used after guest migration from an Intel CPU
anyway. Hardly something to optimise for.

 -- Keir

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

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

[Keir Fraser]

On 31/01/2011 22:13, "Keir Fraser" <keir@xen.org> wrote:

> This handling of the SYSENTER MSRs is overly complicated.

By the way, apart from the complexity, I suspect the current approach has
further bugs because I don't believe the canonical sysenter msr values are
correctly shuffled between the vmcb and vcpu structures when a guest moves
in and out of long mode.

Just another example why having two different canonical places for one data
item is a bad idea.

 -- Keir

> I suggest
> reverting a bunch of the original handling of cross-vendor migration as
> follows:
>  * Never intercept the SYSENTER MSRs.
>  * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
>  * Always hvm save/restore from/to the values in the vmcb.
>  * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb() and
> then read the sysenter msr value from vmcb
>  * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
> then modify the sysenter msr in the vmcb, and then svm_vmload().
> 
> Result is that we get rid of some redundant fields from the vcpu structure
> and have one canonical place we always keep the sysenter msr values, in the
> vmcb. The extra cost in the msr read/write functions is totally
> inconsequential, and only used after guest migration from an Intel CPU
> anyway. Hardly something to optimise for.
> 
>  -- Keir
> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 

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

[Wei Huang]

Hi Keir,

My comments inline below.

-Wei
On 01/31/2011 03:13 PM, Keir Fraser wrote:
> On 31/01/2011 18:33, "Wei Huang"<wei.huang2@amd.com>  wrote:
>
>> The attached patch fixes the save/restore issue seen with 32bit Windows
>> guest VMs. The root cause is that current Xen doesn't intercept
>> SYSENTER-related MSRs for 32bit guest VMs. As a result, the
>> guest_sysenter_xxx fields contain incorrect values and shouldn't be used
>> for save/restore. This patch checks the LMA bit of EFER register in the
>> save/restore code path.
>>
>> Please apply it to both Xen-4.0 and Xen-unstable trees.
>>
>> Reported-by: James Harper<james.harper@bendigoit.com.au>
>> Signed-off-by: Wei Huang<wei.huang2@amd.com>
>> Acked-by: Christoph Egger<christoph.egger@amd.com>
> Nacked-by: Keir Fraser<keir@xen.org>
>
> This handling of the SYSENTER MSRs is overly complicated. I suggest
> reverting a bunch of the original handling of cross-vendor migration as
> follows:
>   * Never intercept the SYSENTER MSRs.
The reason for Christoph to create this patch is AMD doesn't support 
SYSENTER in long mode. If we don't intercept MSRs under long mode, we 
will get stuck with #UD after migration from Intel platform. Did you 
actually mean "* Always intercept the SYSENTER MSRs" here?
>   * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
>   * Always hvm save/restore from/to the values in the vmcb.
>   * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb() and
> then read the sysenter msr value from vmcb
>   * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
> then modify the sysenter msr in the vmcb, and then svm_vmload().
>
> Result is that we get rid of some redundant fields from the vcpu structure
> and have one canonical place we always keep the sysenter msr values, in the
> vmcb. The extra cost in the msr read/write functions is totally
> inconsequential, and only used after guest migration from an Intel CPU
> anyway. Hardly something to optimise for.
>
>   -- Keir
>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

[Wei Huang]

This is true. I thought about it. Some problematic code path can be 
"enter 32bit mode => execute sysenter => turn on long mode" (or vice 
versa). In this case, SYSENTER MSRs should be copied from VMCB to vcpu 
fields. But current Xen doesn't do so.

-Wei

On 01/31/2011 03:17 PM, Keir Fraser wrote:
>   suspect the current approach has
> further bugs because I don't believe the canonical sysenter msr values are
> correctly shuffled between the vmcb and vcpu structures

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

[Keir Fraser]

On 31/01/2011 22:38, "Wei Huang" <wei.huang2@amd.com> wrote:

>> This handling of the SYSENTER MSRs is overly complicated. I suggest
>> reverting a bunch of the original handling of cross-vendor migration as
>> follows:
>>   * Never intercept the SYSENTER MSRs.
> The reason for Christoph to create this patch is AMD doesn't support
> SYSENTER in long mode.

Yes.

> If we don't intercept MSRs under long mode, we
> will get stuck with #UD after migration from Intel platform.

It's the SYSENTER instruction that causes the UD, right, not the WRMSR
writes to the SYSENTER MSRs? Then my described approach will work -- the
SYSENTER instruction will be handled by Xen's x86_emulate(), calling out to
svm_msr_read_intercept() to grab the SYSENTER MSR values (from the VMCB, as
I described). In fact x86_emulate() handles WRMSR too, so even if WRMSR
caused UD we'd still handle it.

> Did you 
> actually mean "* Always intercept the SYSENTER MSRs" here?

No, I think my approach works as I described it.

 -- Keir

>>   * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
>>   * Always hvm save/restore from/to the values in the vmcb.
>>   * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb() and
>> then read the sysenter msr value from vmcb
>>   * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
>> then modify the sysenter msr in the vmcb, and then svm_vmload().
>> 
>> Result is that we get rid of some redundant fields from the vcpu structure
>> and have one canonical place we always keep the sysenter msr values, in the
>> vmcb. The extra cost in the msr read/write functions is totally
>> inconsequential, and only used after guest migration from an Intel CPU
>> anyway. Hardly something to optimise for.
>> 
>>   -- Keir
>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
> 
> 

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

[Wei Huang]

But there is another issue: upper 32bit of sysenter MSRs in VMCB save
area will be truncated with VMSARE/VMEXIT (see comments in vmcb.h).
Could we use these VMCB fields as a storage for 64bit MSRs? 

Thanks,
-Wei

On Tue, 2011-02-01 at 00:14 -0600, Keir Fraser wrote:
> On 31/01/2011 22:38, "Wei Huang" <wei.huang2@amd.com> wrote:
> 
> >> This handling of the SYSENTER MSRs is overly complicated. I suggest
> >> reverting a bunch of the original handling of cross-vendor migration as
> >> follows:
> >>   * Never intercept the SYSENTER MSRs.
> > The reason for Christoph to create this patch is AMD doesn't support
> > SYSENTER in long mode.
> 
> Yes.
> 
> > If we don't intercept MSRs under long mode, we
> > will get stuck with #UD after migration from Intel platform.
> 
> It's the SYSENTER instruction that causes the UD, right, not the WRMSR
> writes to the SYSENTER MSRs? Then my described approach will work -- the
> SYSENTER instruction will be handled by Xen's x86_emulate(), calling out to
> svm_msr_read_intercept() to grab the SYSENTER MSR values (from the VMCB, as
> I described). In fact x86_emulate() handles WRMSR too, so even if WRMSR
> caused UD we'd still handle it.
> 
> > Did you 
> > actually mean "* Always intercept the SYSENTER MSRs" here?
> 
> No, I think my approach works as I described it.
> 
>  -- Keir
> 
> >>   * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
> >>   * Always hvm save/restore from/to the values in the vmcb.
> >>   * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb() and
> >> then read the sysenter msr value from vmcb
> >>   * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
> >> then modify the sysenter msr in the vmcb, and then svm_vmload().
> >> 
> >> Result is that we get rid of some redundant fields from the vcpu structure
> >> and have one canonical place we always keep the sysenter msr values, in the
> >> vmcb. The extra cost in the msr read/write functions is totally
> >> inconsequential, and only used after guest migration from an Intel CPU
> >> anyway. Hardly something to optimise for.
> >> 
> >>   -- Keir
> >> 
> >>> 
> >>> _______________________________________________
> >>> Xen-devel mailing list
> >>> Xen-devel@lists.xensource.com
> >>> http://lists.xensource.com/xen-devel
> >> 
> >> 
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >> 
> > 
> > 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

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

[Keir Fraser]

On 01/02/2011 07:25, "Wei Huang" <wei.huang2@amd.com> wrote:

> But there is another issue: upper 32bit of sysenter MSRs in VMCB save
> area will be truncated with VMSARE/VMEXIT (see comments in vmcb.h).
> Could we use these VMCB fields as a storage for 64bit MSRs?

Is that a bug? Seems unfortunate and unnecessary.

Well, in that case I would suggest to *always* intercept the MSRs, keep the
vcpu-structure fields, and in svm_msr_write_intercept(MSR_IA32_SYSENTER_*)
update vcpu structure *and* do svm_sync_vmcb(); update vmcb; svm_vmload().

This keeps the canonical version of the msrs always in the vcpu structure,
with a (possibly 32-bit truncated) copy in the vmcb. The truncation is safe
since the MSRs will only directly be used in guest mode by legacy-mode
execution of SYSENTER.

How's that?

 -- Keir

> Thanks,
> -Wei
> 
> On Tue, 2011-02-01 at 00:14 -0600, Keir Fraser wrote:
>> On 31/01/2011 22:38, "Wei Huang" <wei.huang2@amd.com> wrote:
>> 
>>>> This handling of the SYSENTER MSRs is overly complicated. I suggest
>>>> reverting a bunch of the original handling of cross-vendor migration as
>>>> follows:
>>>>   * Never intercept the SYSENTER MSRs.
>>> The reason for Christoph to create this patch is AMD doesn't support
>>> SYSENTER in long mode.
>> 
>> Yes.
>> 
>>> If we don't intercept MSRs under long mode, we
>>> will get stuck with #UD after migration from Intel platform.
>> 
>> It's the SYSENTER instruction that causes the UD, right, not the WRMSR
>> writes to the SYSENTER MSRs? Then my described approach will work -- the
>> SYSENTER instruction will be handled by Xen's x86_emulate(), calling out to
>> svm_msr_read_intercept() to grab the SYSENTER MSR values (from the VMCB, as
>> I described). In fact x86_emulate() handles WRMSR too, so even if WRMSR
>> caused UD we'd still handle it.
>> 
>>> Did you 
>>> actually mean "* Always intercept the SYSENTER MSRs" here?
>> 
>> No, I think my approach works as I described it.
>> 
>>  -- Keir
>> 
>>>>   * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
>>>>   * Always hvm save/restore from/to the values in the vmcb.
>>>>   * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb()
>>>> and
>>>> then read the sysenter msr value from vmcb
>>>>   * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
>>>> then modify the sysenter msr in the vmcb, and then svm_vmload().
>>>> 
>>>> Result is that we get rid of some redundant fields from the vcpu structure
>>>> and have one canonical place we always keep the sysenter msr values, in the
>>>> vmcb. The extra cost in the msr read/write functions is totally
>>>> inconsequential, and only used after guest migration from an Intel CPU
>>>> anyway. Hardly something to optimise for.
>>>> 
>>>>   -- Keir
>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@lists.xensource.com
>>>>> http://lists.xensource.com/xen-devel
>>>> 
>>>> 
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>>> 
>>> 
>>> 
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
> 
> 
> 

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

[Wei Huang]

[-- 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

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

[Keir Fraser]

Yeah, good. I'll double check it next week and apply it then for 4.1.x and
4.0.x.

 Thanks,
 Keir

On 01/02/2011 22:06, "Wei Huang" <wei.huang2@amd.com> wrote:

> 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>
> =================
>