diff mbox

[V9,5/5] x86/hvm: pkeys, add pkeys support for cpuid handling

Message ID 1454508725-22786-6-git-send-email-huaitong.han@intel.com
State New, archived
Headers show

Commit Message

Huaitong Han Feb. 3, 2016, 2:12 p.m. UTC
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.han@intel.com>
---
Changes in v9:
*Clear X86_FEATURE_OSPKE and X86_FEATURE_OSXSAVE when the condition is not satisfied.

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      | 26 +++++++++++++++++++-------
 3 files changed, 26 insertions(+), 9 deletions(-)

Comments

Wei Liu Feb. 3, 2016, 3:02 p.m. UTC | #1
On Wed, Feb 03, 2016 at 10:12:05PM +0800, Huaitong Han 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.han@intel.com>

As far as I can tell changes to libxc are in line with overall coding
style and convention in that file.  I'm happy to give my ack on this
patch if hypervisor maintainers are happy about the changes.

There is one nit below.
[...]
> -            /* Don't expose PCOMMIT to hvm when VMX support is not available */
> +            /* Don't expose PCOMMIT to hvm when VMX support is not available. */

White space only change. This should not be included here.

>              if ( !cpu_has_vmx_pcommit )
>                  *ebx &= ~cpufeat_mask(X86_FEATURE_PCOMMIT);
>          }
> -- 
> 2.4.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Wei Liu Feb. 3, 2016, 3:06 p.m. UTC | #2
On Wed, Feb 03, 2016 at 03:02:07PM +0000, Wei Liu wrote:
> On Wed, Feb 03, 2016 at 10:12:05PM +0800, Huaitong Han 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.han@intel.com>
> 
> As far as I can tell changes to libxc are in line with overall coding
> style and convention in that file.  I'm happy to give my ack on this
> patch if hypervisor maintainers are happy about the changes.
> 
> There is one nit below.
> [...]
> > -            /* Don't expose PCOMMIT to hvm when VMX support is not available */
> > +            /* Don't expose PCOMMIT to hvm when VMX support is not available. */
> 
> White space only change. This should not be included here.
> 

Actually it's not white space change. Ignore my comment please.

> >              if ( !cpu_has_vmx_pcommit )
> >                  *ebx &= ~cpufeat_mask(X86_FEATURE_PCOMMIT);
> >          }
> > -- 
> > 2.4.3
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
Jan Beulich Feb. 3, 2016, 3:18 p.m. UTC | #3
>>> On 03.02.16 at 15:12, <huaitong.han@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.han@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox

Patch

diff --git a/tools/libxc/xc_cpufeature.h b/tools/libxc/xc_cpufeature.h
index ee53679..866cf0b 100644
--- a/tools/libxc/xc_cpufeature.h
+++ b/tools/libxc/xc_cpufeature.h
@@ -144,4 +144,7 @@ 
 #define X86_FEATURE_CLFLUSHOPT  23 /* CLFLUSHOPT instruction */
 #define X86_FEATURE_CLWB        24 /* CLWB instruction */
 
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
+#define X86_FEATURE_PKU     3
+
 #endif /* __LIBXC_CPUFEATURE_H */
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index c142595..5408dd0 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -430,9 +430,11 @@  static void xc_cpuid_hvm_policy(xc_interface *xch,
                         bitmaskof(X86_FEATURE_PCOMMIT) |
                         bitmaskof(X86_FEATURE_CLWB) |
                         bitmaskof(X86_FEATURE_CLFLUSHOPT));
+            regs[2] &= bitmaskof(X86_FEATURE_PKU);
         } else
-            regs[1] = 0;
-        regs[0] = regs[2] = regs[3] = 0;
+            regs[1] = regs[2] = 0;
+
+        regs[0] = regs[3] = 0;
         break;
 
     case 0x0000000d:
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5ec2ae1..73fb54c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4572,9 +4572,11 @@  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 )
-            *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ?
-                     cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;
+        if ( *ecx & cpufeat_mask(X86_FEATURE_XSAVE) &&
+             (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) )
+            *ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);
+        else
+            *ecx &= ~cpufeat_mask(X86_FEATURE_OSXSAVE);
 
         /* Don't expose PCID to non-hap hvm. */
         if ( !hap_enabled(d) )
@@ -4593,16 +4595,26 @@  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);
+            else
+                *ecx &= ~cpufeat_mask(X86_FEATURE_OSPKE);
 
-            /* Don't expose PCOMMIT to hvm when VMX support is not available */
+            /* Don't expose PCOMMIT to hvm when VMX support is not available. */
             if ( !cpu_has_vmx_pcommit )
                 *ebx &= ~cpufeat_mask(X86_FEATURE_PCOMMIT);
         }