>>>> 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
xsave_init_fix.patch
Description: xsave_init_fix.patch
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home