diff mbox series

[v4,06/11] KVM: VMX: Frame in ENCLS handler for SGX virtualization

Message ID 4be4b49f63a6c66911683d0f093ca5ef0d3996d5.1617825858.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM SGX virtualization support (KVM part) | expand

Commit Message

Huang, Kai April 7, 2021, 8:49 p.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

Introduce sgx.c and sgx.h, along with the framework for handling ENCLS
VM-Exits.  Add a bool, enable_sgx, that will eventually be wired up to a
module param to control whether or not SGX virtualization is enabled at
runtime.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/kvm/Makefile  |  2 ++
 arch/x86/kvm/vmx/sgx.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx/sgx.h | 15 +++++++++++++
 arch/x86/kvm/vmx/vmx.c |  9 +++++---
 4 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/kvm/vmx/sgx.c
 create mode 100644 arch/x86/kvm/vmx/sgx.h

Comments

Sean Christopherson April 7, 2021, 10:16 p.m. UTC | #1 
On Thu, Apr 08, 2021, Kai Huang wrote:
> +int handle_encls(struct kvm_vcpu *vcpu)
> +{
> +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];

Please use kvm_rax_read(), I've been trying to discourage direct access to the
array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)

> +
> +	if (!encls_leaf_enabled_in_guest(vcpu, leaf)) {
> +		kvm_queue_exception(vcpu, UD_VECTOR);
> +	} else if (!sgx_enabled_in_guest_bios(vcpu)) {
> +		kvm_inject_gp(vcpu, 0);
> +	} else {
> +		WARN(1, "KVM: unexpected exit on ENCLS[%u]", leaf);
> +		vcpu->run->exit_reason = KVM_EXIT_UNKNOWN;
> +		vcpu->run->hw.hardware_exit_reason = EXIT_REASON_ENCLS;
> +		return 0;
> +	}
> +	return 1;
> +}
Huang, Kai April 7, 2021, 10:33 p.m. UTC | #2 
On Wed, 7 Apr 2021 22:16:59 +0000 Sean Christopherson wrote:
> On Thu, Apr 08, 2021, Kai Huang wrote:
> > +int handle_encls(struct kvm_vcpu *vcpu)
> > +{
> > +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
> 
> Please use kvm_rax_read(), I've been trying to discourage direct access to the
> array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)

Sure. But I think still, we should convert it to (u32) explicitly, so:

	u32 leaf = (u32)kvm_rax_read(vcpu); 

?

> 
> > +
> > +	if (!encls_leaf_enabled_in_guest(vcpu, leaf)) {
> > +		kvm_queue_exception(vcpu, UD_VECTOR);
> > +	} else if (!sgx_enabled_in_guest_bios(vcpu)) {
> > +		kvm_inject_gp(vcpu, 0);
> > +	} else {
> > +		WARN(1, "KVM: unexpected exit on ENCLS[%u]", leaf);
> > +		vcpu->run->exit_reason = KVM_EXIT_UNKNOWN;
> > +		vcpu->run->hw.hardware_exit_reason = EXIT_REASON_ENCLS;
> > +		return 0;
> > +	}
> > +	return 1;
> > +}
Sean Christopherson April 7, 2021, 10:35 p.m. UTC | #3 
On Thu, Apr 08, 2021, Kai Huang wrote:
> On Wed, 7 Apr 2021 22:16:59 +0000 Sean Christopherson wrote:
> > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > +int handle_encls(struct kvm_vcpu *vcpu)
> > > +{
> > > +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
> > 
> > Please use kvm_rax_read(), I've been trying to discourage direct access to the
> > array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)
> 
> Sure. But I think still, we should convert it to (u32) explicitly, so:
> 
> 	u32 leaf = (u32)kvm_rax_read(vcpu); 
> 
> ?

Ya, agreed, it helps document that it's deliberate.
Huang, Kai April 7, 2021, 10:44 p.m. UTC | #4 
On Wed, 7 Apr 2021 22:35:32 +0000 Sean Christopherson wrote:
> On Thu, Apr 08, 2021, Kai Huang wrote:
> > On Wed, 7 Apr 2021 22:16:59 +0000 Sean Christopherson wrote:
> > > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > > +int handle_encls(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
> > > 
> > > Please use kvm_rax_read(), I've been trying to discourage direct access to the
> > > array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)
> > 
> > Sure. But I think still, we should convert it to (u32) explicitly, so:
> > 
> > 	u32 leaf = (u32)kvm_rax_read(vcpu); 
> > 
> > ?
> 
> Ya, agreed, it helps document that it's deliberate.

Do you have any other comments regarding to other patches? If no I can send
another version rather quickly :)
Sean Christopherson April 7, 2021, 10:47 p.m. UTC | #5 
On Thu, Apr 08, 2021, Kai Huang wrote:
> On Wed, 7 Apr 2021 22:35:32 +0000 Sean Christopherson wrote:
> > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > On Wed, 7 Apr 2021 22:16:59 +0000 Sean Christopherson wrote:
> > > > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > > > +int handle_encls(struct kvm_vcpu *vcpu)
> > > > > +{
> > > > > +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
> > > > 
> > > > Please use kvm_rax_read(), I've been trying to discourage direct access to the
> > > > array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)
> > > 
> > > Sure. But I think still, we should convert it to (u32) explicitly, so:
> > > 
> > > 	u32 leaf = (u32)kvm_rax_read(vcpu); 
> > > 
> > > ?
> > 
> > Ya, agreed, it helps document that it's deliberate.
> 
> Do you have any other comments regarding to other patches? If no I can send
> another version rather quickly :)

Nope, nothing at this time.  Though I'd give folks a few days to review before
sending the next version, I don't think any of my feedback will affect other
reviews.
Huang, Kai April 7, 2021, 10:59 p.m. UTC | #6 
On Wed, 7 Apr 2021 22:47:07 +0000 Sean Christopherson wrote:
> On Thu, Apr 08, 2021, Kai Huang wrote:
> > On Wed, 7 Apr 2021 22:35:32 +0000 Sean Christopherson wrote:
> > > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > > On Wed, 7 Apr 2021 22:16:59 +0000 Sean Christopherson wrote:
> > > > > On Thu, Apr 08, 2021, Kai Huang wrote:
> > > > > > +int handle_encls(struct kvm_vcpu *vcpu)
> > > > > > +{
> > > > > > +	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
> > > > > 
> > > > > Please use kvm_rax_read(), I've been trying to discourage direct access to the
> > > > > array.  Which is ironic because I'm 100% certain I'm to blame for this. :-)
> > > > 
> > > > Sure. But I think still, we should convert it to (u32) explicitly, so:
> > > > 
> > > > 	u32 leaf = (u32)kvm_rax_read(vcpu); 
> > > > 
> > > > ?
> > > 
> > > Ya, agreed, it helps document that it's deliberate.
> > 
> > Do you have any other comments regarding to other patches? If no I can send
> > another version rather quickly :)
> 
> Nope, nothing at this time.  Though I'd give folks a few days to review before
> sending the next version, I don't think any of my feedback will affect other
> reviews.

My thinking too, but OK I'll wait for other people's review, plus I'd like to
hear about on how to proceed given the current series has some merge conflicts
with latest kvm/queue, although they are quite straightforward to resolve.
diff mbox series

Patch

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 1b4766fe1de2..87f514c36eae 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -23,6 +23,8 @@  kvm-$(CONFIG_KVM_XEN)	+= xen.o
 
 kvm-intel-y		+= vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
 			   vmx/evmcs.o vmx/nested.o vmx/posted_intr.o
+kvm-intel-$(CONFIG_X86_SGX_KVM)	+= vmx/sgx.o
+
 kvm-amd-y		+= svm/svm.o svm/vmenter.o svm/pmu.o svm/nested.o svm/avic.o svm/sev.o
 
 obj-$(CONFIG_KVM)	+= kvm.o
diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c
new file mode 100644
index 000000000000..f68adbe38750
--- /dev/null
+++ b/arch/x86/kvm/vmx/sgx.c
@@ -0,0 +1,50 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*  Copyright(c) 2021 Intel Corporation. */
+
+#include <asm/sgx.h>
+
+#include "cpuid.h"
+#include "kvm_cache_regs.h"
+#include "sgx.h"
+#include "vmx.h"
+#include "x86.h"
+
+bool __read_mostly enable_sgx;
+
+static inline bool encls_leaf_enabled_in_guest(struct kvm_vcpu *vcpu, u32 leaf)
+{
+	if (!enable_sgx || !guest_cpuid_has(vcpu, X86_FEATURE_SGX))
+		return false;
+
+	if (leaf >= ECREATE && leaf <= ETRACK)
+		return guest_cpuid_has(vcpu, X86_FEATURE_SGX1);
+
+	if (leaf >= EAUG && leaf <= EMODT)
+		return guest_cpuid_has(vcpu, X86_FEATURE_SGX2);
+
+	return false;
+}
+
+static inline bool sgx_enabled_in_guest_bios(struct kvm_vcpu *vcpu)
+{
+	const u64 bits = FEAT_CTL_SGX_ENABLED | FEAT_CTL_LOCKED;
+
+	return (to_vmx(vcpu)->msr_ia32_feature_control & bits) == bits;
+}
+
+int handle_encls(struct kvm_vcpu *vcpu)
+{
+	u32 leaf = (u32)vcpu->arch.regs[VCPU_REGS_RAX];
+
+	if (!encls_leaf_enabled_in_guest(vcpu, leaf)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+	} else if (!sgx_enabled_in_guest_bios(vcpu)) {
+		kvm_inject_gp(vcpu, 0);
+	} else {
+		WARN(1, "KVM: unexpected exit on ENCLS[%u]", leaf);
+		vcpu->run->exit_reason = KVM_EXIT_UNKNOWN;
+		vcpu->run->hw.hardware_exit_reason = EXIT_REASON_ENCLS;
+		return 0;
+	}
+	return 1;
+}
diff --git a/arch/x86/kvm/vmx/sgx.h b/arch/x86/kvm/vmx/sgx.h
new file mode 100644
index 000000000000..6e17ecd4aca3
--- /dev/null
+++ b/arch/x86/kvm/vmx/sgx.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __KVM_X86_SGX_H
+#define __KVM_X86_SGX_H
+
+#include <linux/kvm_host.h>
+
+#ifdef CONFIG_X86_SGX_KVM
+extern bool __read_mostly enable_sgx;
+
+int handle_encls(struct kvm_vcpu *vcpu);
+#else
+#define enable_sgx 0
+#endif
+
+#endif /* __KVM_X86_SGX_H */
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 9dd185a53a3e..ef668047a8f9 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -57,6 +57,7 @@ 
 #include "mmu.h"
 #include "nested.h"
 #include "pmu.h"
+#include "sgx.h"
 #include "trace.h"
 #include "vmcs.h"
 #include "vmcs12.h"
@@ -5673,16 +5674,18 @@  static int handle_vmx_instruction(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+#ifndef CONFIG_X86_SGX_KVM
 static int handle_encls(struct kvm_vcpu *vcpu)
 {
 	/*
-	 * SGX virtualization is not yet supported.  There is no software
-	 * enable bit for SGX, so we have to trap ENCLS and inject a #UD
-	 * to prevent the guest from executing ENCLS.
+	 * SGX virtualization is disabled.  There is no software enable bit for
+	 * SGX, so KVM intercepts all ENCLS leafs and injects a #UD to prevent
+	 * the guest from executing ENCLS (when SGX is supported by hardware).
 	 */
 	kvm_queue_exception(vcpu, UD_VECTOR);
 	return 1;
 }
+#endif /* CONFIG_X86_SGX_KVM */
 
 static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu)
 {