From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [V8 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Date: Thu, 29 Oct 2015 09:57:41 +0000 Message-ID: <5631ED95.7050101@citrix.com> References: <1445593701-5300-1-git-send-email-shuai.ruan@linux.intel.com> <1445593701-5300-3-git-send-email-shuai.ruan@linux.intel.com> <562E40DB02000078000AEA42@prv-mh.provo.novell.com> <20151029075857.GA24529@shuai.ruan@linux.intel.com> <5631EE0A02000078000AFBD2@prv-mh.provo.novell.com> <20151029094702.GA27035@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: <20151029094702.GA27035@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 , Jan Beulich Cc: kevin.tian@intel.com, wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jun.nakajima@intel.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 29/10/15 09:47, Shuai Ruan wrote: > On Thu, Oct 29, 2015 at 02:59:38AM -0600, Jan Beulich wrote: >>>>> On 29.10.15 at 08:58, wrote: >>> On Mon, Oct 26, 2015 at 08:03:55AM -0600, Jan Beulich wrote: >>>>>>> On 23.10.15 at 11:48, 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. >>>>> @@ -343,11 +520,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[X86_FEATURE_XSAVEOPT / 32] = eax; >>>>> >>>>> BUG_ON(eax != boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32]); >>>>> + >>>>> + if ( setup_xstate_features(bsp) ) >>>>> + BUG(); >>>> BUG()? On the BSP maybe, but APs should simply fail being >>>> brought up instead of bringing down the whole system. >>>> >>> For APs, setup_xsate_features will never return error. Just BSP can >>> return error as -ENOMEM. >> This may be the case now, but will whoever ends up editing the >> function remember to audit the code here? >> > Ok. >>>>> --- a/xen/include/public/arch-x86/hvm/save.h >>>>> +++ b/xen/include/public/arch-x86/hvm/save.h >>>>> @@ -140,6 +140,7 @@ struct hvm_hw_cpu { >>>>> uint64_t msr_syscall_mask; >>>>> uint64_t msr_efer; >>>>> uint64_t msr_tsc_aux; >>>>> + uint64_t msr_xss; >>>> You can't extend a public interface structure like this. New MSRs >>>> shouldn't be saved/restored this way anyway - that's what we >>>> have struct hvm_msr for. >>> Yes. I will use the exist function "hvm_save_cpu_msrs" to save this msr. I >>> intend to add save msr logic before "ASSERT(ctxt->count <= msr_count_max);" in >>> hvm_save_cpu_msrs. Is that Ok ? >> No, the code belongs in vmx_save_msr() (and its sibling functions). >> > Ok. > For there is no new area added in vmcs for xss_msr, I will use > " > if ( cpu_has_xsaves) > { > ctxt->msr[ctxt->count].val = v->arch.hvm_vcpu.msr_xss; > if ( ctxt->msr[ctxt->count].val ) > ctxt->msr[ctxt->count++].index = MSR_IA32_XSS; > } > " to save xss_msr. Is it ok to add the save logic between "vmx_vmcs_enter(v);" and "vmx_vmcs_exit(v);" ? Or just add the save logic after "vmx_vmcs_exit(v);" ? That looks ok (after fixing the whitespace issue in the if) As it doesn't rely on the vmcs, it would be better to be outside the enter/exit pair so as to prevent needless holdup of another cpu trying to get at this vmcs. You also need to modify vmx_init_msr() to indicate that the maximum possible count of MSRs has increased. ~Andrew