From patchwork Thu Mar 10 08:22:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shuai Ruan X-Patchwork-Id: 8554401 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 01F99C0553 for ; Thu, 10 Mar 2016 08:29:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B75AB202F2 for ; Thu, 10 Mar 2016 08:29:02 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6DA96202F0 for ; Thu, 10 Mar 2016 08:29:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1advup-00037d-Kq; Thu, 10 Mar 2016 08:25:51 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1advun-00037T-QX for xen-devel@lists.xen.org; Thu, 10 Mar 2016 08:25:49 +0000 Received: from [85.158.139.211] by server-4.bemta-5.messagelabs.com id 1E/CC-22079-D8F21E65; Thu, 10 Mar 2016 08:25:49 +0000 X-Env-Sender: shuai.ruan@linux.intel.com X-Msg-Ref: server-4.tower-206.messagelabs.com!1457598347!28053185!1 X-Originating-IP: [134.134.136.65] X-SpamReason: No, hits=0.5 required=7.0 tests=BODY_RANDOM_LONG X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 53979 invoked from network); 10 Mar 2016 08:25:47 -0000 Received: from mga03.intel.com (HELO mga03.intel.com) (134.134.136.65) by server-4.tower-206.messagelabs.com with SMTP; 10 Mar 2016 08:25:47 -0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 10 Mar 2016 00:25:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,314,1455004800"; d="scan'208";a="906790015" Received: from rs-vmm.bj.intel.com ([10.238.135.71]) by orsmga001.jf.intel.com with ESMTP; 10 Mar 2016 00:25:40 -0800 From: Shuai Ruan To: xen-devel@lists.xen.org Date: Thu, 10 Mar 2016 16:22:45 +0800 Message-Id: <1457598165-10393-1-git-send-email-shuai.ruan@linux.intel.com> X-Mailer: git-send-email 1.9.1 Cc: andrew.cooper3@citrix.com, keir@xen.org, jbeulich@suse.com Subject: [Xen-devel] [V4] x86/xsaves: fix overwriting between non-lazy/lazy xsaves X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The offset at which components xsaved by xsave[sc] are not fixed. So when when a save with v->fpu_dirtied set is followed by one with v->fpu_dirtied clear, non-lazy xsave[sc] may overwriting data written by the lazy one. The solution is when using_xsave_compact is enabled and taking xcr0_accum into consideration, if guest has ever used XSTATE_LAZY & ~XSTATE_FP_SSE (XSTATE_FP_SSE will be excluded beacause xsave will write XSTATE_FP_SSE part in legacy region of xsave area which is fixed, saving XSTATE_FS_SSE will not cause overwriting problem), vcpu_xsave_mask will return XSTATE_ALL. Otherwise vcpu_xsave_mask will return XSTATE_NONLAZY. This may cause overhead save on lazy states which will cause performance impact. After doing some performance tests on xsavec and xsaveopt (suggested by jan), the results show xsaveopt performs better than xsavec. So hypervisor will not use xsavec anymore. As xsaves will be used until suprevised state is instroduced in hypervisor. So in this patch add a new synthetic CPU feature X86_FEATURE_XSAVE_COMPACT which indicates whether hypervisor uses compact xsave area or not. Code related with hypervisor xsave will depend on this feature. Signed-off-by: Shuai Ruan Reported-by: Jan Beulich --- v4: Address comments from Jan 1. Add synthetic CPU feature X86_FEATURE_XSAVE_COMPACT and use this feature indicate whether hypervisor use compact xsave area or not. 2. Fix type/grammer errors of the comment in vcpu_xsave_mask. v3: Address comments from Jan 1. Add xsavc clean up code and disable xsaves. 2. Add comment on why certain mask should be return in vcpu_xsave_mask. v2: Address comments from Jan add performance impact and next step to do in the description. xen/arch/x86/domain.c | 2 +- xen/arch/x86/domctl.c | 2 +- xen/arch/x86/hvm/hvm.c | 4 ++-- xen/arch/x86/i387.c | 18 ++++++++++++++++-- xen/arch/x86/xstate.c | 22 ++++++++++------------ xen/include/asm-x86/cpufeature.h | 2 ++ 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index a6d721b..83db3a5 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -948,7 +948,7 @@ int arch_set_info_guest( fpu_sse->fcw = FCW_DEFAULT; fpu_sse->mxcsr = MXCSR_DEFAULT; } - if ( cpu_has_xsaves ) + if ( using_xsave_compact ) { ASSERT(v->arch.xsave_area); v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED | diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 55aecdc..1b56732 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -922,7 +922,7 @@ long arch_do_domctl( ret = -EFAULT; offset += sizeof(v->arch.xcr0_accum); - if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) ) + if ( !ret && using_xsave_compact ) { void *xsave_area; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 5bc2812..215df9a 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2202,7 +2202,7 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) xsave_area->xsave_hdr.xstate_bv = 0; xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT; } - if ( cpu_has_xsaves && xsave_area ) + if ( using_xsave_compact && xsave_area ) xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED | xsave_area->xsave_hdr.xstate_bv; @@ -5591,7 +5591,7 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip) if ( v->arch.xsave_area ) { v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP; - v->arch.xsave_area->xsave_hdr.xcomp_bv = cpu_has_xsaves + v->arch.xsave_area->xsave_hdr.xcomp_bv = using_xsave_compact ? XSTATE_COMPACTION_ENABLED | XSTATE_FP : 0; } diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c index c29d0fa..51a8614 100644 --- a/xen/arch/x86/i387.c +++ b/xen/arch/x86/i387.c @@ -118,7 +118,21 @@ static inline uint64_t vcpu_xsave_mask(const struct vcpu *v) if ( v->fpu_dirtied ) return v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY; - return v->arch.nonlazy_xstate_used ? XSTATE_NONLAZY : 0; + /* + * The offsets of components which live in the extended region of + * compact xsave area are not fixed. Xsave area may be overwritten + * when v->fpu_dirtied set is followed by one with v->fpu_dirtied + * clear. + * In such case, if hypervisor uses compact xsave area and guest + * has ever used lazy states (checking xcr0_accum also excluding + * XSTATE_FP_SSE), vcpu_xsave_mask will return XSTATE_ALL. Otherwise + * return XSTATE_NONLAZY. + * XSTATE_FP_SSE may be excluded, because the offsets of XSTATE_FP_SSE + * (in the legacy region of xsave area) are fixed, so saving + * XSTATE_FS_SSE will not cause overwriting problem. + */ + return using_xsave_compact && (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE) ? + XSTATE_ALL : XSTATE_NONLAZY; } /* Save x87 extended state */ @@ -277,7 +291,7 @@ int vcpu_init_fpu(struct vcpu *v) if ( v->arch.xsave_area ) { v->arch.fpu_ctxt = &v->arch.xsave_area->fpu_sse; - if ( cpu_has_xsaves ) + if ( using_xsave_compact ) v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED; } else diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index 8316bd9..c08e171 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -165,7 +165,7 @@ void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size) u64 xstate_bv = xsave->xsave_hdr.xstate_bv; u64 valid; - if ( !cpu_has_xsaves && !cpu_has_xsavec ) + if ( !(xsave->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) ) { memcpy(dest, xsave, size); return; @@ -206,7 +206,7 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size) u64 xstate_bv = ((const struct xsave_struct *)src)->xsave_hdr.xstate_bv; u64 valid; - if ( !cpu_has_xsaves && !cpu_has_xsavec ) + if ( !(xsave->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) ) { memcpy(xsave, src, size); return; @@ -251,13 +251,11 @@ void xsave(struct vcpu *v, uint64_t mask) uint32_t lmask = mask; unsigned int fip_width = v->domain->arch.x87_fip_width; #define XSAVE(pfx) \ - alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \ + alternative_io_2(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \ ".byte " pfx "0x0f,0xae,0x37\n", /* xsaveopt */ \ X86_FEATURE_XSAVEOPT, \ - ".byte " pfx "0x0f,0xc7,0x27\n", /* xsavec */ \ - X86_FEATURE_XSAVEC, \ ".byte " pfx "0x0f,0xc7,0x2f\n", /* xsaves */ \ - X86_FEATURE_XSAVES, \ + X86_FEATURE_XSAVE_COMPACT, \ "=m" (*ptr), \ "a" (lmask), "d" (hmask), "D" (ptr)) @@ -274,7 +272,7 @@ void xsave(struct vcpu *v, uint64_t mask) typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel; typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel; - if ( cpu_has_xsaveopt || cpu_has_xsaves ) + if ( cpu_has_xsaveopt || using_xsave_compact ) { /* * XSAVEOPT/XSAVES may not write the FPU portion even when the @@ -300,7 +298,7 @@ void xsave(struct vcpu *v, uint64_t mask) (!(ptr->fpu_sse.fsw & 0x0080) && boot_cpu_data.x86_vendor == X86_VENDOR_AMD) ) { - if ( cpu_has_xsaveopt || cpu_has_xsaves ) + if ( cpu_has_xsaveopt || using_xsave_compact ) { ptr->fpu_sse.fip.sel = fcs; ptr->fpu_sse.fdp.sel = fds; @@ -370,7 +368,7 @@ void xrstor(struct vcpu *v, uint64_t mask) " .previous\n" \ _ASM_EXTABLE(1b, 2b), \ ".byte " pfx "0x0f,0xc7,0x1f\n", \ - X86_FEATURE_XSAVES, \ + X86_FEATURE_XSAVE_COMPACT, \ ASM_OUTPUT2([mem] "+m" (*ptr), [faults] "+g" (faults)), \ [lmask] "a" (lmask), [hmask] "d" (hmask), \ [ptr] "D" (ptr)) @@ -409,7 +407,7 @@ void xrstor(struct vcpu *v, uint64_t mask) ((mask & XSTATE_YMM) && !(ptr->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED))) ) ptr->fpu_sse.mxcsr &= mxcsr_mask; - if ( cpu_has_xsaves || cpu_has_xsavec ) + if ( using_xsave_compact ) { ptr->xsave_hdr.xcomp_bv &= this_cpu(xcr0) | this_cpu(xss); ptr->xsave_hdr.xstate_bv &= ptr->xsave_hdr.xcomp_bv; @@ -426,7 +424,7 @@ void xrstor(struct vcpu *v, uint64_t mask) case 2: /* Stage 2: Reset all state. */ ptr->fpu_sse.mxcsr = MXCSR_DEFAULT; ptr->xsave_hdr.xstate_bv = 0; - ptr->xsave_hdr.xcomp_bv = cpu_has_xsaves + ptr->xsave_hdr.xcomp_bv = using_xsave_compact ? XSTATE_COMPACTION_ENABLED : 0; continue; } @@ -575,7 +573,7 @@ void xstate_init(struct cpuinfo_x86 *c) if ( setup_xstate_features(bsp) && bsp ) BUG(); - if ( bsp && (cpu_has_xsaves || cpu_has_xsavec) ) + if ( bsp && using_xsave_compact ) setup_xstate_comp(); } diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index 07ba368..cf247f0 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -76,6 +76,7 @@ #define X86_FEATURE_CPUID_FAULTING (3*32+14) /* cpuid faulting */ #define X86_FEATURE_CLFLUSH_MONITOR (3*32+15) /* clflush reqd with monitor */ #define X86_FEATURE_APERFMPERF (3*32+16) /* APERFMPERF */ +#define X86_FEATURE_XSAVE_COMPACT (3*32 +17) /* use compact xsave area */ /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */ #define X86_FEATURE_XMM3 (4*32+ 0) /* Streaming SIMD Extensions-3 */ @@ -224,6 +225,7 @@ #define cpu_has_xsavec boot_cpu_has(X86_FEATURE_XSAVEC) #define cpu_has_xgetbv1 boot_cpu_has(X86_FEATURE_XGETBV1) #define cpu_has_xsaves boot_cpu_has(X86_FEATURE_XSAVES) +#define using_xsave_compact boot_cpu_has(X86_FEATURE_XSAVE_COMPACT) enum _cache_type { CACHE_TYPE_NULL = 0,