[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [V9 1/3] x86/xsaves: enable xsaves/xrstors/xsavec in xen
>>> On 03.11.15 at 07:27, <shuai.ruan@xxxxxxxxxxxxxxx> wrote: > This patch uses xsaves/xrstors/xsavec instead of xsaveopt/xrstor > to perform the xsave_area switching so that xen itself > can benefit from them when available. > > For xsaves/xrstors/xsavec only use compact format. Add format conversion > support when perform guest os migration. > > Also, pv guest will not support xsaves/xrstors. > > Signed-off-by: Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx> > --- > xen/arch/x86/domain.c | 7 ++ > xen/arch/x86/domctl.c | 30 +++++- > xen/arch/x86/hvm/hvm.c | 18 +++- > xen/arch/x86/i387.c | 4 + > xen/arch/x86/traps.c | 6 +- > xen/arch/x86/xstate.c | 242 > +++++++++++++++++++++++++++++++++++++------ > xen/include/asm-x86/xstate.h | 2 + > 7 files changed, 265 insertions(+), 44 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index fe3be30..108d4f8 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -883,7 +883,12 @@ int arch_set_info_guest( > { > memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt)); > if ( v->arch.xsave_area ) > + { > v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE; > + if ( cpu_has_xsaves || cpu_has_xsavec ) > + v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_FP_SSE | > + > XSTATE_COMPACTION_ENABLED; > + } So here you nicely extend the existing conditional. > @@ -1568,6 +1573,8 @@ static void __context_switch(void) > if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) ) > BUG(); > } > + if ( cpu_has_xsaves && has_hvm_container_vcpu(n) ) > + set_msr_xss(n->arch.hvm_vcpu.msr_xss); Why not also here (the previous if() uses cpu_has_xsave, which you surely depend on)? Agreed the difference is minor for modern CPUs, but I wanted to ask anyway. I.e. an explanation will do, no need to re-submit just because of this. > @@ -158,6 +334,20 @@ void xsave(struct vcpu *v, uint64_t mask) > ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = word_size; > } > > +#define XSTATE_FIXUP ".section .fixup,\"ax\" \n" \ > + "2: mov %5,%%ecx \n" \ > + " xor %1,%1 \n" \ > + " rep stosb \n" \ > + " lea %2,%0 \n" \ > + " mov %3,%1 \n" \ > + " jmp 1b \n" \ > + ".previous \n" \ > + _ASM_EXTABLE(1b, 2b) \ > + : "+&D" (ptr), "+&a" (lmask) \ > + : "m" (*ptr), "g" (lmask), "d" (hmask), \ > + "m" (xsave_cntxt_size) \ > + : "ecx" > + > void xrstor(struct vcpu *v, uint64_t mask) > { > uint32_t hmask = mask >> 32; > @@ -187,39 +377,22 @@ void xrstor(struct vcpu *v, uint64_t mask) > switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) ) > { > default: > - asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n" > - ".section .fixup,\"ax\" \n" > - "2: mov %5,%%ecx \n" > - " xor %1,%1 \n" > - " rep stosb \n" > - " lea %2,%0 \n" > - " mov %3,%1 \n" > - " jmp 1b \n" > - ".previous \n" > - _ASM_EXTABLE(1b, 2b) > - : "+&D" (ptr), "+&a" (lmask) > - : "m" (*ptr), "g" (lmask), "d" (hmask), > - "m" (xsave_cntxt_size) > - : "ecx" ); > + alternative_input("1: "".byte 0x48,0x0f,0xae,0x2f", > + ".byte 0x48,0x0f,0xc7,0x1f", > + X86_FEATURE_XSAVES, > + "D" (ptr), "m" (*ptr), "a" (lmask), "d" (hmask)); > + asm volatile (XSTATE_FIXUP); > break; > case 4: case 2: > - asm volatile ( "1: .byte 0x0f,0xae,0x2f\n" > - ".section .fixup,\"ax\" \n" > - "2: mov %5,%%ecx \n" > - " xor %1,%1 \n" > - " rep stosb \n" > - " lea %2,%0 \n" > - " mov %3,%1 \n" > - " jmp 1b \n" > - ".previous \n" > - _ASM_EXTABLE(1b, 2b) > - : "+&D" (ptr), "+&a" (lmask) > - : "m" (*ptr), "g" (lmask), "d" (hmask), > - "m" (xsave_cntxt_size) > - : "ecx" ); > + alternative_input("1: "".byte 0x0f,0xae,0x2f", > + ".byte 0x0f,0xc7,0x1f", > + X86_FEATURE_XSAVES, > + "D" (ptr), "m" (*ptr), "a" (lmask), "d" (hmask)); > + asm volatile (XSTATE_FIXUP); > break; > } > } > +#undef XSTATE_FIXUP Repeating my comment on v8: "I wonder whether at least for the restore side alternative asm wouldn't result in better readable code and at the same time in a smaller patch." Did you at least look into that option? > @@ -343,11 +516,18 @@ void xstate_init(struct cpuinfo_x86 *c) > > /* Mask out features not currently understood by Xen. */ > eax &= (cpufeat_mask(X86_FEATURE_XSAVEOPT) | > - cpufeat_mask(X86_FEATURE_XSAVEC)); > + cpufeat_mask(X86_FEATURE_XSAVEC) | > + cpufeat_mask(X86_FEATURE_XGETBV1) | > + cpufeat_mask(X86_FEATURE_XSAVES)); > > c->x86_capability[cpufeat_word(X86_FEATURE_XSAVEOPT)] = eax; > > BUG_ON(eax != > boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_XSAVEOPT)]); > + > + if ( setup_xstate_features(bsp) && bsp ) > + BUG(); Well, not what I had hoped for, but okay. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |