| Message ID | 1450682504-32286-7-git-send-email-huaitong.han@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 21.12.15 at 08:21, <huaitong.han@intel.com> wrote: > @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx, > /* Don't expose INVPCID to non-hap hvm. */ > if ( (count == 0) && !hap_enabled(d) ) > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > + > + /* X86_FEATURE_PKU is not yet implemented for shadow paging. */ > + if ( (count == 0) && !hap_enabled(d) ) > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); I'm still missing the xsave dependency here. > + if ( (count == 0) && (*ecx & cpufeat_mask(X86_FEATURE_PKU)) ) > + *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ? > + cpufeat_mask(X86_FEATURE_OSPKE) : 0; Also all (would now be 6!) count == 0 dependencies should be combined instead of repeating that condition so many times. Jan
On Mon, 2015-12-21 at 08:07 -0700, Jan Beulich wrote: > > > > On 21.12.15 at 08:21, <huaitong.han@intel.com> wrote: > > @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, unsigned > > int *eax, unsigned int *ebx, > > /* Don't expose INVPCID to non-hap hvm. */ > > if ( (count == 0) && !hap_enabled(d) ) > > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > > + > > + /* X86_FEATURE_PKU is not yet implemented for shadow > > paging. */ > > + if ( (count == 0) && !hap_enabled(d) ) > > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); > > I'm still missing the xsave dependency here. Xsave dependency deletion becasue we use RDPKRU to get PKRU register value instead of XSAVE now. > > > + if ( (count == 0) && (*ecx & > > cpufeat_mask(X86_FEATURE_PKU)) ) > > + *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ? > > + cpufeat_mask(X86_FEATURE_OSPKE) : 0; > > Also all (would now be 6!) count == 0 dependencies should be > combined instead of repeating that condition so many times. > > Jan >
>>> On 22.12.15 at 03:54, <huaitong.han@intel.com> wrote: > On Mon, 2015-12-21 at 08:07 -0700, Jan Beulich wrote: >> > > > On 21.12.15 at 08:21, <huaitong.han@intel.com> wrote: >> > @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, unsigned >> > int *eax, unsigned int *ebx, >> > /* Don't expose INVPCID to non-hap hvm. */ >> > if ( (count == 0) && !hap_enabled(d) ) >> > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); >> > + >> > + /* X86_FEATURE_PKU is not yet implemented for shadow >> > paging. */ >> > + if ( (count == 0) && !hap_enabled(d) ) >> > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); >> >> I'm still missing the xsave dependency here. > Xsave dependency deletion becasue we use RDPKRU to get PKRU register > value instead of XSAVE now. What the hypervisor does doesn't matter here. The question is whether from an architectural standpoint XSAVE is a prerequsite. If it is, then you need to clear PKU when _guest_ XSAVE is clear. Jan
On Tue, 2015-12-22 at 01:30 -0700, Jan Beulich wrote: > > > > On 22.12.15 at 03:54, <huaitong.han@intel.com> wrote: > > On Mon, 2015-12-21 at 08:07 -0700, Jan Beulich wrote: > > > > > > On 21.12.15 at 08:21, <huaitong.han@intel.com> wrote: > > > > @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, > > > > unsigned > > > > int *eax, unsigned int *ebx, > > > > /* Don't expose INVPCID to non-hap hvm. */ > > > > if ( (count == 0) && !hap_enabled(d) ) > > > > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > > > > + > > > > + /* X86_FEATURE_PKU is not yet implemented for shadow > > > > paging. */ > > > > + if ( (count == 0) && !hap_enabled(d) ) > > > > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); > > > > > > I'm still missing the xsave dependency here. > > Xsave dependency deletion becasue we use RDPKRU to get PKRU > > register > > value instead of XSAVE now. > > What the hypervisor does doesn't matter here. The question is > whether from an architectural standpoint XSAVE is a prerequsite. > If it is, then you need to clear PKU when _guest_ XSAVE is clear. No, XSAVE is not a prerequsite. > > Jan >
>>> On 22.12.15 at 10:25, <huaitong.han@intel.com> wrote: > On Tue, 2015-12-22 at 01:30 -0700, Jan Beulich wrote: >> > > > On 22.12.15 at 03:54, <huaitong.han@intel.com> wrote: >> > On Mon, 2015-12-21 at 08:07 -0700, Jan Beulich wrote: >> > > > > > On 21.12.15 at 08:21, <huaitong.han@intel.com> wrote: >> > > > @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, >> > > > unsigned >> > > > int *eax, unsigned int *ebx, >> > > > /* Don't expose INVPCID to non-hap hvm. */ >> > > > if ( (count == 0) && !hap_enabled(d) ) >> > > > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); >> > > > + >> > > > + /* X86_FEATURE_PKU is not yet implemented for shadow >> > > > paging. */ >> > > > + if ( (count == 0) && !hap_enabled(d) ) >> > > > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); >> > > >> > > I'm still missing the xsave dependency here. >> > Xsave dependency deletion becasue we use RDPKRU to get PKRU >> > register >> > value instead of XSAVE now. >> >> What the hypervisor does doesn't matter here. The question is >> whether from an architectural standpoint XSAVE is a prerequsite. >> If it is, then you need to clear PKU when _guest_ XSAVE is clear. > No, XSAVE is not a prerequsite. I.e. OSes are expected to deal with both cases (PKU w/ XSAVE and PKU w/o XSAVE)? Sounds like a recipe for trouble, but well - okay. Jan
diff --git a/tools/libxc/xc_cpufeature.h b/tools/libxc/xc_cpufeature.h index c3ddc80..f6a9778 100644 --- a/tools/libxc/xc_cpufeature.h +++ b/tools/libxc/xc_cpufeature.h @@ -141,5 +141,7 @@ #define X86_FEATURE_ADX 19 /* ADCX, ADOX instructions */ #define X86_FEATURE_SMAP 20 /* Supervisor Mode Access Protection */ +/* 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 8882c01..1ce979b 100644 --- a/tools/libxc/xc_cpuid_x86.c +++ b/tools/libxc/xc_cpuid_x86.c @@ -427,9 +427,11 @@ static void xc_cpuid_hvm_policy(xc_interface *xch, bitmaskof(X86_FEATURE_ADX) | bitmaskof(X86_FEATURE_SMAP) | bitmaskof(X86_FEATURE_FSGSBASE)); + 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 59916ed..05821ed 100644 --- 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; @@ -4600,6 +4600,14 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx, /* Don't expose INVPCID to non-hap hvm. */ if ( (count == 0) && !hap_enabled(d) ) *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); + + /* X86_FEATURE_PKU is not yet implemented for shadow paging. */ + if ( (count == 0) && !hap_enabled(d) ) + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU); + + if ( (count == 0) && (*ecx & cpufeat_mask(X86_FEATURE_PKU)) ) + *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) ? + cpufeat_mask(X86_FEATURE_OSPKE) : 0; break; case 0xb: /* Fix the x2APIC identifier. */
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. Signed-off-by: Huaitong Han <huaitong.han@intel.com> --- tools/libxc/xc_cpufeature.h | 2 ++ tools/libxc/xc_cpuid_x86.c | 6 ++++-- xen/arch/x86/hvm/hvm.c | 10 +++++++++- 3 files changed, 15 insertions(+), 3 deletions(-)