From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Huang Subject: Re: Avoid alloc for xsave before xsave_init Date: Thu, 13 Jan 2011 17:54:11 -0600 Message-ID: <4D2F90A3.5080200@amd.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: "xen-devel@lists.xensource.com" , "Wei, Gang" List-Id: xen-devel@lists.xenproject.org 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" 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@lists.xensource.com >> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Wei, Gang >> Sent: Thursday, January 13, 2011 12:49 PM >> To: xen-devel@lists.xensource.com >> 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@lists.xensource.com >> http://lists.xensource.com/xen-devel >> >> > >