On Wed, 2016-02-03 at 02:50 -0700, Jan Beulich wrote:
> > > > On 02.02.16 at 08:35, <huaitong....@intel.com> 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 <huaitong....@intel.com>
> > ---
> > 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.l...@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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
