From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Date: Mon, 12 Oct 2015 11:05:33 +0100 Message-ID: <561B85ED.2080507@citrix.com> References: <1444630048-19411-1-git-send-email-shuai.ruan@linux.intel.com> <1444630048-19411-3-git-send-email-shuai.ruan@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1444630048-19411-3-git-send-email-shuai.ruan@linux.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Shuai Ruan , xen-devel@lists.xen.org Cc: kevin.tian@intel.com, wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, eddie.dong@intel.com, ian.jackson@eu.citrix.com, jbeulich@suse.com, jun.nakajima@intel.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 12/10/15 07:07, Shuai Ruan 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 > --- > xen/arch/x86/domain.c | 2 + > xen/arch/x86/domctl.c | 35 ++++++- > xen/arch/x86/hvm/hvm.c | 19 +++- > xen/arch/x86/i387.c | 4 + > xen/arch/x86/traps.c | 6 +- > xen/arch/x86/xstate.c | 221 ++++++++++++++++++++++++++++++++++++------- > xen/include/asm-x86/xstate.h | 2 + > 7 files changed, 244 insertions(+), 45 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index 7ca9b93..7b3d9f4 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -1569,6 +1569,8 @@ static void __context_switch(void) > if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) ) > BUG(); > } > + if ( cpu_has_xsaves && is_hvm_vcpu(n) ) Because of the current implementation of PVH mode, the correct predicate here is has_hvm_container(n) > + set_msr_xss(n->arch.hvm_vcpu.msr_xss); > vcpu_restore_fpu_eager(n); > n->arch.ctxt_switch_to(n); > } > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > index f8a559c..9517741 100644 > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -897,9 +897,30 @@ long arch_do_domctl( > ret = -EFAULT; > > offset += sizeof(v->arch.xcr0_accum); > - if ( !ret && copy_to_guest_offset(evc->buffer, offset, > - (void *)v->arch.xsave_area, > - size - 2 * sizeof(uint64_t)) ) > + > + if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) ) > + { > + void *xsave_area; > + > + xsave_area = xmalloc_bytes(size); > + if ( !xsave_area ) > + { > + ret = -ENOMEM; > + vcpu_unpause(v); > + goto vcpuextstate_out; > + } > + > + expand_xsave_states(v, xsave_area, > + evc->size - 2 * sizeof(uint64_t)); > + > + if ( copy_to_guest_offset(evc->buffer, offset, xsave_area, > + size - 2 * sizeof(uint64_t)) ) > + ret = -EFAULT; > + xfree(xsave_area); > + } > + else if ( !ret && copy_to_guest_offset(evc->buffer, offset, > + (void *)v->arch.xsave_area, > + size - 2 * sizeof(uint64_t)) ) > ret = -EFAULT; > > vcpu_unpause(v); > @@ -955,8 +976,12 @@ long arch_do_domctl( > v->arch.xcr0_accum = _xcr0_accum; > if ( _xcr0_accum & XSTATE_NONLAZY ) > v->arch.nonlazy_xstate_used = 1; > - memcpy(v->arch.xsave_area, _xsave_area, > - evc->size - 2 * sizeof(uint64_t)); > + if ( (cpu_has_xsaves || cpu_has_xsavec) ) You can drop the inner set of brackets. > + compress_xsave_states(v, _xsave_area, > + evc->size - 2 * sizeof(uint64_t)); > + else > + memcpy(v->arch.xsave_area, (void *)_xsave_area, > + evc->size - 2 * sizeof(uint64_t)); > vcpu_unpause(v); > } > else > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 3fa2280..177cf5f 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2157,8 +2157,12 @@ static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h) > ctxt->xfeature_mask = xfeature_mask; > ctxt->xcr0 = v->arch.xcr0; > ctxt->xcr0_accum = v->arch.xcr0_accum; > - memcpy(&ctxt->save_area, v->arch.xsave_area, > - size - offsetof(struct hvm_hw_cpu_xsave, save_area)); > + if ( (cpu_has_xsaves || cpu_has_xsavec) ) And here > + expand_xsave_states(v, &ctxt->save_area, > + size - offsetof(typeof(*ctxt), save_area)); > + else > + memcpy(&ctxt->save_area, v->arch.xsave_area, > + size - offsetof(struct hvm_hw_cpu_xsave, save_area)); > } > > return 0; > @@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h) > v->arch.xcr0_accum = ctxt->xcr0_accum; > if ( ctxt->xcr0_accum & XSTATE_NONLAZY ) > v->arch.nonlazy_xstate_used = 1; > - memcpy(v->arch.xsave_area, &ctxt->save_area, > - min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave, > - save_area)); > + if ( (cpu_has_xsaves || cpu_has_xsavec) ) And here. With these 4 things fixed, Reviewed-by: Andrew Cooper