[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] RE: 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. >>> >>> 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:
xsave_init_fix.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |