[Xen-devel] Re: Avoid alloc for xsave before xsave_init

[Wei Huang <wei.huang2@xxxxxxx>]

The test showed on my machine showed that xsave areas of idle vcpus are 
inconsistent. With four CPUs, the code did the following:
1. vcpu 0 of idle domain
* xsave_alloc_save_area() is called.
* xsave_cntxt_size is 0; so vcpu->arch.xsave_area is 0 bytes.
* vcpu->arch.xcr0 and vcpu->arch.xcr0_accum is 0x3.
2. Then, xsave_init() is called. xsave_cntxt_size is now initialized 
correctly.
3. After that, vcpu 1, 2, 3 of idle domain have
* xsave_alloc_save_area() is called.
* xsave_cntxt_size is correct; so vcpu->arch.xsave_area points to an 
allocated area.
* vcpu->arch.xcr0 and vcpu->arch.xcr0_accum is 0x3.

In other words, vcpu0 has a different xsave_area from other vcpus. I 
think the following patch should fix the issues above:

diff -r 20b0f709153e xen/arch/x86/i387.c
--- a/xen/arch/x86/i387.c       Wed Jan 12 14:14:13 2011 +0000
+++ b/xen/arch/x86/i387.c       Thu Jan 13 18:08:30 2011 -0600
@@ -33,7 +33,7 @@
     if ( cr0 & X86_CR0_TS )
         clts();

-    if ( cpu_has_xsave )
+    if ( cpu_has_xsave && !is_idle_vcpu(v) )
     {
         /* XCR0 normally represents what guest OS set. In case of Xen 
itself,
          * we set all accumulated feature mask before doing save/restore.
@@ -214,7 +214,7 @@
 {
     void *save_area;

-    if ( !cpu_has_xsave )
+    if ( !cpu_has_xsave || is_idle_vcpu(v) )
         return 0;

     /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */



On 01/13/2011 03:15 PM, Keir Fraser wrote:
> On 13/01/2011 20:21, "Huang2, Wei"<Wei.Huang2@xxxxxxx>  wrote:
>
> > Hi Gang,
> >
> > Was the issue caused by the uninitialized variable xsave_cntxt_size,
> > triggering problem for _xmalloc()? If so, one solution is to set
> > xsave_cntxt_size=576 (the default value after reset) as a default value. When
> > xsave_alloc_save_area() is called for idel VCPU, _xmalloc() will initialize
> > 576 bytes. Idle domain doesn't change xcr0 from my understanding. So its xcr0
> > is XSTATE_FP_SSE all the time.
> Idle domain isn't using FPU,SSE,AVX or any such extended state and doesn't
> need it saved. Xsave_{alloc,free}_save_area() should test-and-exit on
> is_idle_vcpu(), and our context switch code should not be doing XSAVE when
> switching out an idle vcpu (I hope this is the case already, as it would be
> a pointless waste of time).
>
>   -- Keir
>
> > Best,
> > -Wei
> >
> > -----Original Message-----
> > From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> > [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Wei, Gang
> > Sent: Thursday, January 13, 2011 12:49 PM
> > To: xen-devel@xxxxxxxxxxxxxxxxxxx
> > Cc: Keir Fraser; Wei, Gang
> > Subject: [Xen-devel] Avoid alloc for xsave before xsave_init
> >
> > While debugging some weird booting failure bugs, just found currently,
> > xsave_alloc_save_area will be called in
> > init_idle_domain->scheduler_init->alloc_vcpu->vcpu_initialise calls, it is
> > earlier than xsave_init called in identity_cpu(). This may causing buffer
> > overflow on xmem_pool. I am thinking about how to fix it.
> >
> > Jimmy
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel


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