From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Han, Huaitong" Subject: Re: [PATCH V8 5/5] x86/hvm: pkeys, add pkeys support for cpuid handling Date: Wed, 3 Feb 2016 10:04:08 +0000 Message-ID: <1454493858.4350.10.camel@intel.com> References: <1454398542-4815-1-git-send-email-huaitong.han@intel.com> <1454398542-4815-6-git-send-email-huaitong.han@intel.com> <56B1DB6F02000078000CDDD8@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56B1DB6F02000078000CDDD8@prv-mh.provo.novell.com> Content-Language: en-US Content-ID: <2B3EA7538C84484BA2CACF3FFEC8B293@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: "JBeulich@suse.com" Cc: "george.dunlap@eu.citrix.com" , "andrew.cooper3@citrix.com" , "keir@xen.org" , "tim@xen.org" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Wed, 2016-02-03 at 02:50 -0700, Jan Beulich wrote: > > > > On 02.02.16 at 08:35, wrote: > > This patch adds pkeys support for cpuid handing. > > > > Pkeys hardware support is CPUID.7.0.ECX[3]:PKU. software support is > > CPUID.7.0.ECX[4]:OSPKE and it reflects the support setting of > > CR4.PKE. > > > > X86_FEATURE_OSXSAVE depends on guest X86_FEATURE_XSAVE, but > > cpu_has_xsave > > function reflects hypervisor X86_FEATURE_XSAVE, it is fixed too. > > > > Signed-off-by: Huaitong Han > > --- > > Changes in v7: > > *Rebase in the latest tree. > > *Add a comment for cpu_has_xsave adjustment. > > *Adjust indentation. > > > > tools/libxc/xc_cpufeature.h | 3 +++ > > tools/libxc/xc_cpuid_x86.c | 6 ++++-- > > xen/arch/x86/hvm/hvm.c | 18 +++++++++++++----- > > 3 files changed, 20 insertions(+), 7 deletions(-) > > This will need a tools maintainer's ack, so upon re-submission you > should Cc them. I will (again) defer this to x86 maintainers. -from wei.liu2@citrix.com > > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -4572,7 +4572,7 @@ void hvm_cpuid(unsigned int input, unsigned > > int *eax, unsigned int *ebx, > > __clear_bit(X86_FEATURE_APIC & 31, edx); > > > > /* Fix up OSXSAVE. */ > > - if ( cpu_has_xsave ) > > + if ( *ecx & cpufeat_mask(X86_FEATURE_XSAVE) ) > > *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & > > X86_CR4_OSXSAVE) ? > > cpufeat_mask(X86_FEATURE_OSXSAVE) : 0; > > First of all I would have wished that since you're already touching > it, > you would have brought it into the same shape as you're doing for > PKU below. And then here as well as ... It will be updated in V9. > > > @@ -4593,16 +4593,24 @@ void hvm_cpuid(unsigned int input, unsigned > > int *eax, unsigned int *ebx, > > if ( !cpu_has_smap ) > > *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP); > > > > - /* Don't expose MPX to hvm when VMX support is not > > available */ > > + /* Don't expose MPX to hvm when VMX support is not > > available. */ > > if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) || > > !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) ) > > *ebx &= ~cpufeat_mask(X86_FEATURE_MPX); > > > > - /* Don't expose INVPCID to non-hap hvm. */ > > if ( !hap_enabled(d) ) > > - *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > > + { > > + /* Don't expose INVPCID to non-hap hvm. */ > > + *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > > + /* X86_FEATURE_PKU is not yet implemented for > > shadow paging. */ > > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); > > + } > > + > > + if ( (*ecx & cpufeat_mask(X86_FEATURE_PKU)) && > > + (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ) > > + *ecx |= cpufeat_mask(X86_FEATURE_OSPKE); > > ... here - shouldn't we also clear the respective flag in the "else" > case? X86_FEATURE_OSPKE is not set by xc_cpuid_hvm_policy(tools), it reflects the setting of CR4.PKE, so it does not need to be cleared like X86_CR4_OSXSAVE. > > Jan >