[PATCH v4 0/3] arch/x86: Enable MPK feature on AMD

[PATCH v4 0/3] arch/x86: Enable MPK feature on AMD

[Babu Moger]

AMD's next generation of EPYC processors support the MPK (Memory
Protection Keys) feature.

This series enables the feature on AMD and updates config parameters
and documentation to reflect the MPK support on x86 platforms.

AMD documentation for MPK feature is available at "AMD64 Architecture
Programmer’s Manual Volume 2: System Programming, Pub. 24593 Rev. 3.34,
Section 5.6.6 Memory Protection Keys (MPK) Bit".

The documentation can be obtained at the link below:
https://bugzilla.kernel.org/show_bug.cgi?id=206537

---
v4:
  - Removed all the source changes related to config parameter.
    Just kept the doc changes and add new config parameter
    X86_MEMORY_PROTECTION_KEYS which shadows X86_INTEL_MEMORY_PROTECTION_KEYS.
 - Minor change in feature detection in kvm/cpuid.c 

v3:
  https://lore.kernel.org/lkml/158923982830.20128.14580309786525588408.stgit@naples-babu.amd.com/#r
  - Fixed the problem Jim Mattson pointed out which can cause pkru
    resources to get corrupted during host and guest switches. 
  - Moved the PKU feature detection code from VMX.c to common code.
  
v2:
  https://lore.kernel.org/lkml/158897190718.22378.3974700869904223395.stgit@naples-babu.amd.com/
  - Introduced intermediate config option X86_MEMORY_PROTECTION_KEYS to
    avoid user propmpts. Kept X86_INTEL_MEMORY_PROTECTION_KEYS as is.
    Eventually, we will be moving to X86_MEMORY_PROTECTION_KEYS after
    couple of kernel revisions. 
  - Moved pkru data structures to kvm_vcpu_arch. Moved save/restore pkru
    to kvm_load_host_xsave_state/kvm_load_guest_xsave_state.

v1:
  https://lore.kernel.org/lkml/158880240546.11615.2219410169137148044.stgit@naples-babu.amd.com/

Babu Moger (3):
      arch/x86: Update config and kernel doc for MPK feature on AMD
      KVM: x86: Move pkru save/restore to x86.c
      KVM: x86: Move MPK feature detection to common code


 Documentation/core-api/protection-keys.rst |    3 ++-
 arch/x86/Kconfig                           |   14 ++++++++++++--
 arch/x86/include/asm/kvm_host.h            |    1 +
 arch/x86/kvm/cpuid.c                       |    9 ++++++++-
 arch/x86/kvm/vmx/vmx.c                     |   22 ----------------------
 arch/x86/kvm/x86.c                         |   17 +++++++++++++++++
 6 files changed, 40 insertions(+), 26 deletions(-)

--
Signature

[PATCH v4 1/3] arch/x86: Update config and kernel doc for MPK feature on AMD

[Babu Moger]

AMD's next generation of EPYC processors support the MPK (Memory
Protection Keys) feature.

Add a generic X86_MEMORY_PROTECTION_KEYS config shadowing
X86_INTEL_MEMORY_PROTECTION_KEYS and update the kernel
documentation.

No functional changes.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/core-api/protection-keys.rst |    3 ++-
 arch/x86/Kconfig                           |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/core-api/protection-keys.rst b/Documentation/core-api/protection-keys.rst
index 49d9833af871..d25e89e53c59 100644
--- a/Documentation/core-api/protection-keys.rst
+++ b/Documentation/core-api/protection-keys.rst
@@ -6,7 +6,8 @@ Memory Protection Keys
 
 Memory Protection Keys for Userspace (PKU aka PKEYs) is a feature
 which is found on Intel's Skylake "Scalable Processor" Server CPUs.
-It will be avalable in future non-server parts.
+It will be available in future non-server parts. Also, AMD64
+Architecture Programmer’s Manual defines PKU feature in AMD processors.
 
 For anyone wishing to test or use this feature, it is available in
 Amazon's EC2 C5 instances and is known to work there using an Ubuntu
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1197b5596d5a..15bda3a60c1d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1887,10 +1887,10 @@ config X86_UMIP
 	  results are dummy.
 
 config X86_INTEL_MEMORY_PROTECTION_KEYS
-	prompt "Intel Memory Protection Keys"
+	prompt "Memory Protection Keys"
 	def_bool y
 	# Note: only available in 64-bit mode
-	depends on CPU_SUP_INTEL && X86_64
+	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_HAS_PKEYS
 	---help---
@@ -1902,6 +1902,16 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
 
 	  If unsure, say y.
 
+config X86_MEMORY_PROTECTION_KEYS
+	# Both Intel and AMD platforms support "Memory Protection Keys"
+	# feature. So add a generic option X86_MEMORY_PROTECTION_KEYS
+	# and set the option whenever X86_INTEL_MEMORY_PROTECTION_KEYS
+	# is set. This is to avoid the confusion about the feature
+	# availability on AMD platforms. Also renaming the old option
+	# would cause the user an extra prompt during the kernel
+	# configuration. So avoided changing the old config name.
+	def_bool X86_INTEL_MEMORY_PROTECTION_KEYS
+
 choice
 	prompt "TSX enable mode"
 	depends on CPU_SUP_INTEL

[PATCH v4 2/3] KVM: x86: Move pkru save/restore to x86.c

[Babu Moger]

MPK feature is supported by both VMX and SVM. So we can
safely move pkru state save/restore to common code. Also
move all the pkru data structure to kvm_vcpu_arch.

Also fixes the problem Jim Mattson pointed and suggested below.

"Though rdpkru and wrpkru are contingent upon CR4.PKE, the PKRU
resource isn't. It can be read with XSAVE and written with XRSTOR.
So, if we don't set the guest PKRU value here(kvm_load_guest_xsave_state),
the guest can read the host value.

In case of kvm_load_host_xsave_state, guest with CR4.PKE clear could
potentially use XRSTOR to change the host PKRU value"

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/vmx/vmx.c          |   18 ------------------
 arch/x86/kvm/x86.c              |   17 +++++++++++++++++
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 42a2d0d3984a..afd8f3780ae0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -578,6 +578,7 @@ struct kvm_vcpu_arch {
 	unsigned long cr4;
 	unsigned long cr4_guest_owned_bits;
 	unsigned long cr8;
+	u32 host_pkru;
 	u32 pkru;
 	u32 hflags;
 	u64 efer;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c2c6335a998c..46898a476ba7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1372,7 +1372,6 @@ void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 	vmx_vcpu_pi_load(vcpu, cpu);
 
-	vmx->host_pkru = read_pkru();
 	vmx->host_debugctlmsr = get_debugctlmsr();
 }
 
@@ -6577,11 +6576,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	kvm_load_guest_xsave_state(vcpu);
 
-	if (static_cpu_has(X86_FEATURE_PKU) &&
-	    kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
-	    vcpu->arch.pkru != vmx->host_pkru)
-		__write_pkru(vcpu->arch.pkru);
-
 	pt_guest_enter(vmx);
 
 	if (vcpu_to_pmu(vcpu)->version)
@@ -6671,18 +6665,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	pt_guest_exit(vmx);
 
-	/*
-	 * eager fpu is enabled if PKEY is supported and CR4 is switched
-	 * back on host, so it is safe to read guest PKRU from current
-	 * XSAVE.
-	 */
-	if (static_cpu_has(X86_FEATURE_PKU) &&
-	    kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
-		vcpu->arch.pkru = rdpkru();
-		if (vcpu->arch.pkru != vmx->host_pkru)
-			__write_pkru(vmx->host_pkru);
-	}
-
 	kvm_load_host_xsave_state(vcpu);
 
 	vmx->nested.nested_run_pending = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c5835f9cb9ad..98baeb74452c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -836,11 +836,25 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu)
 		    vcpu->arch.ia32_xss != host_xss)
 			wrmsrl(MSR_IA32_XSS, vcpu->arch.ia32_xss);
 	}
+
+	if (static_cpu_has(X86_FEATURE_PKU) &&
+	    (kvm_read_cr4_bits(vcpu, X86_CR4_PKE) ||
+	     (vcpu->arch.xcr0 & XFEATURE_MASK_PKRU)) &&
+	    vcpu->arch.pkru != vcpu->arch.host_pkru)
+		__write_pkru(vcpu->arch.pkru);
 }
 EXPORT_SYMBOL_GPL(kvm_load_guest_xsave_state);
 
 void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu)
 {
+	if (static_cpu_has(X86_FEATURE_PKU) &&
+	    (kvm_read_cr4_bits(vcpu, X86_CR4_PKE) ||
+	     (vcpu->arch.xcr0 & XFEATURE_MASK_PKRU))) {
+		vcpu->arch.pkru = rdpkru();
+		if (vcpu->arch.pkru != vcpu->arch.host_pkru)
+			__write_pkru(vcpu->arch.host_pkru);
+	}
+
 	if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE)) {
 
 		if (vcpu->arch.xcr0 != host_xcr0)
@@ -3570,6 +3584,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 	kvm_x86_ops.vcpu_load(vcpu, cpu);
 
+	/* Save host pkru register if supported */
+	vcpu->arch.host_pkru = read_pkru();
+
 	/* Apply any externally detected TSC adjustments (due to suspend) */
 	if (unlikely(vcpu->arch.tsc_offset_adjustment)) {
 		adjust_tsc_offset_host(vcpu, vcpu->arch.tsc_offset_adjustment);

[PATCH v4 3/3] KVM: x86: Move MPK feature detection to common code

[Babu Moger]

Both Intel and AMD support (MPK) Memory Protection Key feature.
Move the feature detection from VMX to the common code. It should
work for both the platforms now.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/cpuid.c   |    9 ++++++++-
 arch/x86/kvm/vmx/vmx.c |    4 ----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 901cd1fdecd9..d2c36768667d 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -326,7 +326,7 @@ void kvm_set_cpu_caps(void)
 	);
 
 	kvm_cpu_cap_mask(CPUID_7_ECX,
-		F(AVX512VBMI) | F(LA57) | 0 /*PKU*/ | 0 /*OSPKE*/ | F(RDPID) |
+		F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ | F(RDPID) |
 		F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
 		F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
 		F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B) | 0 /*WAITPKG*/
@@ -335,6 +335,13 @@ void kvm_set_cpu_caps(void)
 	if (cpuid_ecx(7) & F(LA57))
 		kvm_cpu_cap_set(X86_FEATURE_LA57);
 
+	/*
+	 * PKU not yet implemented for shadow paging and requires OSPKE
+	 * to be set on the host. Clear it if that is not the case
+	 */
+	if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
+		kvm_cpu_cap_clear(X86_FEATURE_PKU);
+
 	kvm_cpu_cap_mask(CPUID_7_EDX,
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
 		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 46898a476ba7..d153732ed88f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7136,10 +7136,6 @@ static __init void vmx_set_cpu_caps(void)
 	if (vmx_pt_mode_is_host_guest())
 		kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT);
 
-	/* PKU is not yet implemented for shadow paging. */
-	if (enable_ept && boot_cpu_has(X86_FEATURE_OSPKE))
-		kvm_cpu_cap_check_and_set(X86_FEATURE_PKU);
-
 	if (vmx_umip_emulated())
 		kvm_cpu_cap_set(X86_FEATURE_UMIP);
 

Re: [PATCH v4 1/3] arch/x86: Update config and kernel doc for MPK feature on AMD

[Dave Hansen]

On 5/12/20 4:58 PM, Babu Moger wrote:
> +config X86_MEMORY_PROTECTION_KEYS
> +	# Both Intel and AMD platforms support "Memory Protection Keys"
> +	# feature. So add a generic option X86_MEMORY_PROTECTION_KEYS
> +	# and set the option whenever X86_INTEL_MEMORY_PROTECTION_KEYS
> +	# is set. This is to avoid the confusion about the feature
> +	# availability on AMD platforms. Also renaming the old option
> +	# would cause the user an extra prompt during the kernel
> +	# configuration. So avoided changing the old config name.
> +	def_bool X86_INTEL_MEMORY_PROTECTION_KEYS

Hi Babu,

I made a request earlier for an end date (or version) to be included
here.  I believe that appeared in one of your earlier versions, but it
was removed in later ones.

Was there a reason for that?

I'd really prefer to put some kind of expiration date on the config
option.  It will outlive us all otherwise.

>  Memory Protection Keys for Userspace (PKU aka PKEYs) is a feature
>  which is found on Intel's Skylake "Scalable Processor" Server CPUs.
> -It will be avalable in future non-server parts.
> +It will be available in future non-server parts. Also, AMD64
> +Architecture Programmer’s Manual defines PKU feature in AMD processors.

I actually worked pretty hard to make that sentence useful to Linux
users.  Instead of forcing them to imply that it will be available on
future AMD CPUs, can we just come out and say it?  Can we give any more
information to our users?

Naming the AMD manual in which the feature is defined doesn't really
help our users.  Let's not waste the bytes on it.

How about:

	Memory Protection Keys for Userspace (PKU aka PKEYs) is a
	feature which is found on Intel's Skylake (and later) "Scalable
	Processor" Server CPUs.  It will be avalable in future non-
	server Intel parts and future AMD parts.

Any clarity you can add, such as to say what AMD is doing for server vs.
client  would be nice.

BTW, when I first submitted pkeys, I didn't have any statement like this
in the changelog or documentation.  Ingo, I think, asked for it and I
worked with folks inside Intel to figure out how much we could say
publicly about our plans.  A similar effort from AMD would be much
appreciated here.

Re: [PATCH v4 0/3] arch/x86: Enable MPK feature on AMD

[Paolo Bonzini]

On 13/05/20 01:58, Babu Moger wrote:
> AMD's next generation of EPYC processors support the MPK (Memory
> Protection Keys) feature.
> 
> This series enables the feature on AMD and updates config parameters
> and documentation to reflect the MPK support on x86 platforms.
> 
> AMD documentation for MPK feature is available at "AMD64 Architecture
> Programmer’s Manual Volume 2: System Programming, Pub. 24593 Rev. 3.34,
> Section 5.6.6 Memory Protection Keys (MPK) Bit".
> 
> The documentation can be obtained at the link below:
> https://bugzilla.kernel.org/show_bug.cgi?id=206537

I'm queuing patches 2 and 3, since they are do not need any support in
common code.

Paolo

Re: [PATCH v4 1/3] arch/x86: Update config and kernel doc for MPK feature on AMD

[Babu Moger]

On 5/13/20 10:09 AM, Dave Hansen wrote:
> On 5/12/20 4:58 PM, Babu Moger wrote:
>> +config X86_MEMORY_PROTECTION_KEYS
>> +	# Both Intel and AMD platforms support "Memory Protection Keys"
>> +	# feature. So add a generic option X86_MEMORY_PROTECTION_KEYS
>> +	# and set the option whenever X86_INTEL_MEMORY_PROTECTION_KEYS
>> +	# is set. This is to avoid the confusion about the feature
>> +	# availability on AMD platforms. Also renaming the old option
>> +	# would cause the user an extra prompt during the kernel
>> +	# configuration. So avoided changing the old config name.
>> +	def_bool X86_INTEL_MEMORY_PROTECTION_KEYS
> 
> Hi Babu,
> 
> I made a request earlier for an end date (or version) to be included
> here.  I believe that appeared in one of your earlier versions, but it
> was removed in later ones.
> 
> Was there a reason for that?

Dave, Sorry, I misunderstood that. I thought we probably are not going to
change the sources/makefile(ifdefs mostly) as it was technically not
required. Now I am reading that we are going to change that in the future
and just keep X86_MEMORY_PROTECTION_KEYS going forward.
Sure. I will add the text you proposed. Please feel free to correct again.

> 
> I'd really prefer to put some kind of expiration date on the config
> option.  It will outlive us all otherwise.
> 
>>  Memory Protection Keys for Userspace (PKU aka PKEYs) is a feature
>>  which is found on Intel's Skylake "Scalable Processor" Server CPUs.
>> -It will be avalable in future non-server parts.
>> +It will be available in future non-server parts. Also, AMD64
>> +Architecture Programmer’s Manual defines PKU feature in AMD processors.
> 
> I actually worked pretty hard to make that sentence useful to Linux
> users.  Instead of forcing them to imply that it will be available on
> future AMD CPUs, can we just come out and say it?  Can we give any more
> information to our users?
> 
> Naming the AMD manual in which the feature is defined doesn't really
> help our users.  Let's not waste the bytes on it.
> 
> How about:
> 
> 	Memory Protection Keys for Userspace (PKU aka PKEYs) is a
> 	feature which is found on Intel's Skylake (and later) "Scalable
> 	Processor" Server CPUs.  It will be avalable in future non-
> 	server Intel parts and future AMD parts.

This should be good enough. Thanks.
> 
> Any clarity you can add, such as to say what AMD is doing for server vs.
> client  would be nice.
> 
> BTW, when I first submitted pkeys, I didn't have any statement like this
> in the changelog or documentation.  Ingo, I think, asked for it and I
> worked with folks inside Intel to figure out how much we could say
> publicly about our plans.  A similar effort from AMD would be much
> appreciated here.
> 

Re: [PATCH v4 0/3] arch/x86: Enable MPK feature on AMD

[Babu Moger]

On 5/13/20 10:35 AM, Paolo Bonzini wrote:
> On 13/05/20 01:58, Babu Moger wrote:
>> AMD's next generation of EPYC processors support the MPK (Memory
>> Protection Keys) feature.
>>
>> This series enables the feature on AMD and updates config parameters
>> and documentation to reflect the MPK support on x86 platforms.
>>
>> AMD documentation for MPK feature is available at "AMD64 Architecture
>> Programmer’s Manual Volume 2: System Programming, Pub. 24593 Rev. 3.34,
>> Section 5.6.6 Memory Protection Keys (MPK) Bit".
>>
>> The documentation can be obtained at the link below:
> 
> I'm queuing patches 2 and 3, since they are do not need any support in
> common code.

Thanks Paolo. I will just resubmit the patch #1 addressing Dave's feedback.