* Avoid alloc for xsave before xsave_init @ 2011-01-13 18:48 Wei, Gang 2011-01-13 20:21 ` Huang2, Wei 2011-01-13 21:13 ` Keir Fraser 0 siblings, 2 replies; 10+ messages in thread From: Wei, Gang @ 2011-01-13 18:48 UTC (permalink / raw) To: xen-devel@lists.xensource.com; +Cc: Keir Fraser, Wei, Gang 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Avoid alloc for xsave before xsave_init 2011-01-13 18:48 Avoid alloc for xsave before xsave_init Wei, Gang @ 2011-01-13 20:21 ` Huang2, Wei 2011-01-13 21:15 ` Keir Fraser 2011-01-13 21:13 ` Keir Fraser 1 sibling, 1 reply; 10+ messages in thread From: Huang2, Wei @ 2011-01-13 20:21 UTC (permalink / raw) To: Wei, Gang, xen-devel@lists.xensource.com; +Cc: Keir Fraser 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. 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid alloc for xsave before xsave_init 2011-01-13 20:21 ` Huang2, Wei @ 2011-01-13 21:15 ` Keir Fraser 2011-01-13 23:54 ` Wei Huang 2011-01-14 5:20 ` Wei, Gang 0 siblings, 2 replies; 10+ messages in thread From: Keir Fraser @ 2011-01-13 21:15 UTC (permalink / raw) To: Huang2, Wei, Wei, Gang, xen-devel@lists.xensource.com On 13/01/2011 20:21, "Huang2, Wei" <Wei.Huang2@amd.com> 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid alloc for xsave before xsave_init 2011-01-13 21:15 ` Keir Fraser @ 2011-01-13 23:54 ` Wei Huang 2011-01-14 5:20 ` Wei, Gang 1 sibling, 0 replies; 10+ messages in thread From: Wei Huang @ 2011-01-13 23:54 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Wei, Gang 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@amd.com> 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 >> >> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Avoid alloc for xsave before xsave_init 2011-01-13 21:15 ` Keir Fraser 2011-01-13 23:54 ` Wei Huang @ 2011-01-14 5:20 ` Wei, Gang 2011-01-14 7:11 ` Wei, Gang 1 sibling, 1 reply; 10+ messages in thread From: Wei, Gang @ 2011-01-14 5:20 UTC (permalink / raw) To: Keir Fraser, Huang2, Wei, xen-devel@lists.xensource.com; +Cc: Wei, Gang Keir Fraser wrote on 2011-01-14: > On 13/01/2011 20:21, "Huang2, Wei" <Wei.Huang2@amd.com> wrote: >> 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). I agree that do test-and-exit on is_idle_vcpu() in Xsave_{alloc,free}_save_area. Further, We'd better add assert(xsave_cntxt_size>=576) after the test-and-exit clause to ensure no buffer overflow will happen in the future. I reviewed the context switch code and assure context switch code not be doing XSAVE when switching out an idle vcpu. Jimmy >> -----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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Avoid alloc for xsave before xsave_init 2011-01-14 5:20 ` Wei, Gang @ 2011-01-14 7:11 ` Wei, Gang 2011-01-14 10:48 ` Keir Fraser 0 siblings, 1 reply; 10+ messages in thread From: Wei, Gang @ 2011-01-14 7:11 UTC (permalink / raw) To: Keir Fraser, Huang2, Wei, xen-devel@lists.xensource.com; +Cc: Wei, Gang [-- Attachment #1: Type: text/plain, Size: 3796 bytes --] >>>> 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. >>> >>> 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). > > I agree that do test-and-exit on is_idle_vcpu() in Xsave_{alloc,free}_save_area. > Further, We'd better add assert(xsave_cntxt_size>=576) after the > test-and-exit clause to ensure no buffer overflow will happen in the future. > > I reviewed the context switch code and assure context switch code not > be doing XSAVE when switching out an idle vcpu. Here is the patch. diff -r d839631b6048 xen/arch/x86/i387.c --- a/xen/arch/x86/i387.c Thu Jan 13 00:18:35 2011 +0000 +++ b/xen/arch/x86/i387.c Sat Jan 15 20:15:24 2011 +0800 @@ -35,6 +35,9 @@ void save_init_fpu(struct vcpu *v) if ( cpu_has_xsave ) { + if ( is_idle_vcpu(v) ) + goto out; + /* XCR0 normally represents what guest OS set. In case of Xen itself, * we set all accumulated feature mask before doing save/restore. */ @@ -87,6 +90,7 @@ void save_init_fpu(struct vcpu *v) asm volatile ( "fnsave %0 ; fwait" : "=m" (*fpu_ctxt) ); } +out: v->fpu_dirtied = 0; write_cr0(cr0|X86_CR0_TS); } @@ -138,6 +142,7 @@ void restore_fpu(struct vcpu *v) } #define XSTATE_CPUID 0xd +#define XSAVE_AREA_MIN_SIZE (512 + 64) /* * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all @@ -177,7 +182,7 @@ void xsave_init(void) } /* FP/SSE, XSAVE.HEADER, YMM */ - min_size = 512 + 64 + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : 0); + min_size = XSAVE_AREA_MIN_SIZE + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : 0); BUG_ON(ecx < min_size); /* @@ -214,8 +219,10 @@ int xsave_alloc_save_area(struct vcpu *v { void *save_area; - if ( !cpu_has_xsave ) + if ( !cpu_has_xsave || is_idle_vcpu(v) ) return 0; + + BUG_ON(xsave_cntxt_size < XSAVE_AREA_MIN_SIZE); /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */ save_area = _xmalloc(xsave_cntxt_size, 64); @@ -235,6 +242,9 @@ int xsave_alloc_save_area(struct vcpu *v void xsave_free_save_area(struct vcpu *v) { + if ( is_idle_vcpu(v) ) + return; + xfree(v->arch.xsave_area); v->arch.xsave_area = NULL; } diff -r d839631b6048 xen/include/asm-x86/i387.h --- a/xen/include/asm-x86/i387.h Thu Jan 13 00:18:35 2011 +0000 +++ b/xen/include/asm-x86/i387.h Sat Jan 15 20:08:14 2011 +0800 @@ -134,6 +134,9 @@ static inline void setup_fpu(struct vcpu v->fpu_dirtied = 1; if ( cpu_has_xsave ) { + if ( is_idle_vcpu(v) ) + return; + if ( !v->fpu_initialised ) v->fpu_initialised = 1; Jimmy [-- Attachment #2: xsave_init_fix.patch --] [-- Type: application/octet-stream, Size: 2959 bytes --] X86: Avoid buffer overflow caused by calling xsave_alloc_save_area before xsave_init Currently, xsave_alloc_save_area will be called in init_idle_domain->scheduler_init->alloc_vcpu->vcpu_initialise calls with xsave_cntxt_size=0, it is earlier than xsave_init called in identity_cpu(). This may causing buffer overflow on xmem_pool. 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. Meanwhile add !is_idle_vcpu() check to fns calling xsave/xrstor, and a BUG_ON in xsave_alloc_save_area() to avoid any possible buffer overflow. Signed-off-by: Wei Gang <gang.wei@intel.com> diff -r d839631b6048 xen/arch/x86/i387.c --- a/xen/arch/x86/i387.c Thu Jan 13 00:18:35 2011 +0000 +++ b/xen/arch/x86/i387.c Sat Jan 15 20:15:24 2011 +0800 @@ -35,6 +35,9 @@ void save_init_fpu(struct vcpu *v) if ( cpu_has_xsave ) { + if ( is_idle_vcpu(v) ) + goto out; + /* XCR0 normally represents what guest OS set. In case of Xen itself, * we set all accumulated feature mask before doing save/restore. */ @@ -87,6 +90,7 @@ void save_init_fpu(struct vcpu *v) asm volatile ( "fnsave %0 ; fwait" : "=m" (*fpu_ctxt) ); } +out: v->fpu_dirtied = 0; write_cr0(cr0|X86_CR0_TS); } @@ -138,6 +142,7 @@ void restore_fpu(struct vcpu *v) } #define XSTATE_CPUID 0xd +#define XSAVE_AREA_MIN_SIZE (512 + 64) /* * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all @@ -177,7 +182,7 @@ void xsave_init(void) } /* FP/SSE, XSAVE.HEADER, YMM */ - min_size = 512 + 64 + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : 0); + min_size = XSAVE_AREA_MIN_SIZE + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : 0); BUG_ON(ecx < min_size); /* @@ -214,8 +219,10 @@ int xsave_alloc_save_area(struct vcpu *v { void *save_area; - if ( !cpu_has_xsave ) + if ( !cpu_has_xsave || is_idle_vcpu(v) ) return 0; + + BUG_ON(xsave_cntxt_size < XSAVE_AREA_MIN_SIZE); /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */ save_area = _xmalloc(xsave_cntxt_size, 64); @@ -235,6 +242,9 @@ int xsave_alloc_save_area(struct vcpu *v void xsave_free_save_area(struct vcpu *v) { + if ( is_idle_vcpu(v) ) + return; + xfree(v->arch.xsave_area); v->arch.xsave_area = NULL; } diff -r d839631b6048 xen/include/asm-x86/i387.h --- a/xen/include/asm-x86/i387.h Thu Jan 13 00:18:35 2011 +0000 +++ b/xen/include/asm-x86/i387.h Sat Jan 15 20:08:14 2011 +0800 @@ -134,6 +134,9 @@ static inline void setup_fpu(struct vcpu v->fpu_dirtied = 1; if ( cpu_has_xsave ) { + if ( is_idle_vcpu(v) ) + return; + if ( !v->fpu_initialised ) v->fpu_initialised = 1; [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid alloc for xsave before xsave_init 2011-01-14 7:11 ` Wei, Gang @ 2011-01-14 10:48 ` Keir Fraser 2011-01-16 15:23 ` Wei, Gang 0 siblings, 1 reply; 10+ messages in thread From: Keir Fraser @ 2011-01-14 10:48 UTC (permalink / raw) To: Wei, Gang, Huang2, Wei, xen-devel@lists.xensource.com On 14/01/2011 07:11, "Wei, Gang" <gang.wei@intel.com> wrote: >> I agree that do test-and-exit on is_idle_vcpu() in >> Xsave_{alloc,free}_save_area. >> Further, We'd better add assert(xsave_cntxt_size>=576) after the >> test-and-exit clause to ensure no buffer overflow will happen in the future. >> >> I reviewed the context switch code and assure context switch code not >> be doing XSAVE when switching out an idle vcpu. > > Here is the patch. I applied your patch, plus some cleanup, as of c/s 22751. -- Keir > diff -r d839631b6048 xen/arch/x86/i387.c > --- a/xen/arch/x86/i387.c Thu Jan 13 00:18:35 2011 +0000 > +++ b/xen/arch/x86/i387.c Sat Jan 15 20:15:24 2011 +0800 > @@ -35,6 +35,9 @@ void save_init_fpu(struct vcpu *v) > > if ( cpu_has_xsave ) > { > + if ( is_idle_vcpu(v) ) > + goto out; > + > /* XCR0 normally represents what guest OS set. In case of Xen itself, > * we set all accumulated feature mask before doing save/restore. > */ > @@ -87,6 +90,7 @@ void save_init_fpu(struct vcpu *v) > asm volatile ( "fnsave %0 ; fwait" : "=m" (*fpu_ctxt) ); > } > > +out: > v->fpu_dirtied = 0; > write_cr0(cr0|X86_CR0_TS); > } > @@ -138,6 +142,7 @@ void restore_fpu(struct vcpu *v) > } > > #define XSTATE_CPUID 0xd > +#define XSAVE_AREA_MIN_SIZE (512 + 64) > > /* > * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all > @@ -177,7 +182,7 @@ void xsave_init(void) > } > > /* FP/SSE, XSAVE.HEADER, YMM */ > - min_size = 512 + 64 + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : 0); > + min_size = XSAVE_AREA_MIN_SIZE + ((eax & XSTATE_YMM) ? XSTATE_YMM_SIZE : > 0); > BUG_ON(ecx < min_size); > > /* > @@ -214,8 +219,10 @@ int xsave_alloc_save_area(struct vcpu *v > { > void *save_area; > > - if ( !cpu_has_xsave ) > + if ( !cpu_has_xsave || is_idle_vcpu(v) ) > return 0; > + > + BUG_ON(xsave_cntxt_size < XSAVE_AREA_MIN_SIZE); > > /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */ > save_area = _xmalloc(xsave_cntxt_size, 64); > @@ -235,6 +242,9 @@ int xsave_alloc_save_area(struct vcpu *v > > void xsave_free_save_area(struct vcpu *v) > { > + if ( is_idle_vcpu(v) ) > + return; > + > xfree(v->arch.xsave_area); > v->arch.xsave_area = NULL; > } > diff -r d839631b6048 xen/include/asm-x86/i387.h > --- a/xen/include/asm-x86/i387.h Thu Jan 13 00:18:35 2011 +0000 > +++ b/xen/include/asm-x86/i387.h Sat Jan 15 20:08:14 2011 +0800 > @@ -134,6 +134,9 @@ static inline void setup_fpu(struct vcpu > v->fpu_dirtied = 1; > if ( cpu_has_xsave ) > { > + if ( is_idle_vcpu(v) ) > + return; > + > if ( !v->fpu_initialised ) > v->fpu_initialised = 1; > > Jimmy ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Avoid alloc for xsave before xsave_init 2011-01-14 10:48 ` Keir Fraser @ 2011-01-16 15:23 ` Wei, Gang 0 siblings, 0 replies; 10+ messages in thread From: Wei, Gang @ 2011-01-16 15:23 UTC (permalink / raw) To: Keir Fraser, Huang2, Wei, xen-devel@lists.xensource.com; +Cc: Wei, Gang Keir Fraser wrote on 2011-01-14: > On 14/01/2011 07:11, "Wei, Gang" <gang.wei@intel.com> wrote: > >>> I agree that do test-and-exit on is_idle_vcpu() in >>> Xsave_{alloc,free}_save_area. >>> Further, We'd better add assert(xsave_cntxt_size>=576) after the >>> test-and-exit clause to ensure no buffer overflow will happen in the future. >>> >>> I reviewed the context switch code and assure context switch code >>> not be doing XSAVE when switching out an idle vcpu. >> >> Here is the patch. > > I applied your patch, plus some cleanup, as of c/s 22751. Your cleanup looks good. Jimmy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Avoid alloc for xsave before xsave_init 2011-01-13 18:48 Avoid alloc for xsave before xsave_init Wei, Gang 2011-01-13 20:21 ` Huang2, Wei @ 2011-01-13 21:13 ` Keir Fraser 2011-01-14 5:06 ` Wei, Gang 1 sibling, 1 reply; 10+ messages in thread From: Keir Fraser @ 2011-01-13 21:13 UTC (permalink / raw) To: Wei, Gang, xen-devel@lists.xensource.com On 13/01/2011 18:48, "Wei, Gang" <gang.wei@intel.com> wrote: > 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. I doubt idle vcpus need an xsave context. Can we check for is_idle_vcpu() in xsave_{alloc,free}_save_area()? Is this an issue only for xen-unstable/4.1 (not 4.0)? -- Keir > Jimmy ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Avoid alloc for xsave before xsave_init 2011-01-13 21:13 ` Keir Fraser @ 2011-01-14 5:06 ` Wei, Gang 0 siblings, 0 replies; 10+ messages in thread From: Wei, Gang @ 2011-01-14 5:06 UTC (permalink / raw) To: Keir Fraser, xen-devel@lists.xensource.com; +Cc: Wei, Gang Keir Fraser wrote on 2011-01-14: > On 13/01/2011 18:48, "Wei, Gang" <gang.wei@intel.com> wrote: > >> 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. > > I doubt idle vcpus need an xsave context. Can we check for > is_idle_vcpu() in xsave_{alloc,free}_save_area()? > > Is this an issue only for xen-unstable/4.1 (not 4.0)? This issue was induced by c/s 22345 two months ago, which moved the xsave alloc code out from hvm_vcpu_initialise() to support pv guest but forget to exclude idle vcpus. It looks like not back pulled to 4.0. So only 4.1 suffers from it. Jimmy ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-01-16 15:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-13 18:48 Avoid alloc for xsave before xsave_init Wei, Gang 2011-01-13 20:21 ` Huang2, Wei 2011-01-13 21:15 ` Keir Fraser 2011-01-13 23:54 ` Wei Huang 2011-01-14 5:20 ` Wei, Gang 2011-01-14 7:11 ` Wei, Gang 2011-01-14 10:48 ` Keir Fraser 2011-01-16 15:23 ` Wei, Gang 2011-01-13 21:13 ` Keir Fraser 2011-01-14 5:06 ` Wei, Gang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.