Hi Paolo/Radim,
I'd like to start a discussion regarding to IA32_SGXLEPUBKEYHASHn
handling here. I also copied SGX driver mailing list (which looks like I
should do when sending out this series, sorry) and Sean, Haim and Haitao
from Intel to have a better discussion.
Basically IA32_SGXLEPUBKEYHASHn (or more generally speaking, SGX Launch
Control) allows us to run different Launch Enclave (LE) signed with
different RSA keys. Only when the value of IA32_SGXLEPUBKEYHASHn matches
the key used to sign the LE, the LE can be initialized, specifically, by
EINIT, successfully. So before calling EINIT for LE, we have to make
sure IA32_SGXLEPUBKEYHASHn contain the matched value. One fact is only
EINIT uses IA32_SGXLEPUBKEYHASHn, and after EINIT, other ENCLS/ENCLU
(ex, EGETKEY) runs correctly even the MSRs are changed to other values.
To support KVM guests to run their own LEs inside guests, KVM traps
IA32_SGXLEPUBKEYHASHn MSR write and keep the value to vcpu internally,
and KVM needs to write the cached value to real MSRs before guest runs
EINIT. The problem at host side, we also run LE, probably multiple LEs
(it seems currently SGX driver plans to run single in-kernel LE but I am
not familiar with details, and IMO we should not assume host will only
run one LE), therefore if KVM changes the physical MSRs for guest,
host may not be able to run LE as it may not re-write the right MSRs
back. There are two approaches to make host and KVM guests work together:
1. Anyone who wants to run LE is responsible for writing the correct
value to IA32_SGXLEPUBKEYHASHn.
My current patch is based on this assumption. For KVM guest, naturally,
we will write the cached value to real MSRs when vcpu is scheduled in.
For host, SGX driver should write its own value to MSRs when it performs
EINIT for LE.
One argument against this approach is KVM guest should never have impact
on host side, meaning host should not be aware of such MSR change, in
which case, if host do some performance optimization thing that won't
update MSRs actively, when host run EINIT, the physical MSRs may contain
incorrect value. Instead, KVM should be responsible for restoring the
original MSRs, which brings us to approach 2 below.
2. KVM should restore MSRs after changing for guest.
To do this, the simplest way for KVM is: 1) to save the original
physical MSRs and update to guest's MSRs before VMENTRY; 2) in VMEXIT
rewrite the original value to physical MSRs.
To me this approach is also arguable, as KVM guest is actually just a
normal process (OK, maybe not that normal), and KVM guest should be
treated as the same as other processes which runs LE, which means
approach 1 is also reasonable.
And approach 2 will have more performance impact than approach 1 for
KVM, as it read/write IA32_SGXLEPUBKEYHASHn during each VMEXIT/VMENTRY,
while approach 1 only write MSRs when vcpu is scheduled in, which is
less frequent.
I'd like to hear all your comments and hopefully we can have some
agreement on this.
Another thing is, not quite related to selecting which approach above,
and either we choose approach 1 or approach 2, KVM still suffers the
performance loss of writing (and/or reading) to IA32_SGXLEPUBKEYHASHn
MSRs, either when vcpu scheduled in or during each VMEXIT/VMENTRY. Given
the fact that the IA32_SGXLEPUBKEYHASHn will only be used by EINIT, We
can actually do some optimization by trapping EINIT from guest and only
update MSRs in EINIT VMEXIT. This works for approach 1, but for approach
2 we have to do some tricky thing during VMEXIT/VMENTRY
to check whether MSRs have been changed by EINIT VMEXIT, and only
restore the original value if EINIT VMEXIT has happened. Guest's LE
continues to run even physical MSRs are changed back to original.
But trapping ENCLS requires either 1) KVM to run ENCLS on hebalf of
guest, in which case we have to reconstruct and remap guest's ENCLS
parameters and skip the ENCLS for guest; 2) using MTF to let guest to
run ENCLS again, while still trapping ENCLS. Either case would introduce
more complicated code and potentially be more buggy, and I don't think
we should do this to save some time of writing MSRs. If we need to turn
on ENCLS VMEXIT anyway we can optimize this.
Thank you in advance.
Thanks,
-Kai
On 5/8/2017 5:24 PM, Kai Huang wrote:
If SGX runtime launch control is enabled on host (IA32_FEATURE_CONTROL[17]
is set), KVM can support running multiple guests with each running LE signed
with different RSA pubkey. KVM traps IA32_SGXLEPUBKEYHASHn MSR write from
and keeps the values to vcpu internally, and when vcpu is scheduled in, KVM
write those values to real IA32_SGXLEPUBKEYHASHn MSR.
Signed-off-by: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
---
arch/x86/include/asm/msr-index.h | 5 ++
arch/x86/kvm/vmx.c | 123 +++++++++++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e3770f570bb9..70482b951b0f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -417,6 +417,11 @@
#define MSR_IA32_TSC_ADJUST 0x0000003b
#define MSR_IA32_BNDCFGS 0x00000d90
+#define MSR_IA32_SGXLEPUBKEYHASH0 0x0000008c
+#define MSR_IA32_SGXLEPUBKEYHASH1 0x0000008d
+#define MSR_IA32_SGXLEPUBKEYHASH2 0x0000008e
+#define MSR_IA32_SGXLEPUBKEYHASH3 0x0000008f
+
#define MSR_IA32_XSS 0x00000da0
#define FEATURE_CONTROL_LOCKED (1<<0)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a16539594a99..c96332b9dd44 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -656,6 +656,9 @@ struct vcpu_vmx {
*/
u64 msr_ia32_feature_control;
u64 msr_ia32_feature_control_valid_bits;
+
+ /* SGX Launch Control public key hash */
+ u64 msr_ia32_sgxlepubkeyhash[4];
};
enum segment_cache_field {
@@ -2244,6 +2247,70 @@ static void decache_tsc_multiplier(struct vcpu_vmx *vmx)
vmcs_write64(TSC_MULTIPLIER, vmx->current_tsc_ratio);
}
+static bool cpu_sgx_lepubkeyhash_writable(void)
+{
+ u64 val, sgx_lc_enabled_mask = (FEATURE_CONTROL_LOCKED |
+ FEATURE_CONTROL_SGX_LAUNCH_CONTROL_ENABLE);
+
+ rdmsrl(MSR_IA32_FEATURE_CONTROL, val);
+
+ return ((val & sgx_lc_enabled_mask) == sgx_lc_enabled_mask);
+}
+
+static bool vmx_sgx_lc_disabled_in_bios(struct kvm_vcpu *vcpu)
+{
+ return (to_vmx(vcpu)->msr_ia32_feature_control & FEATURE_CONTROL_LOCKED)
+ && (!(to_vmx(vcpu)->msr_ia32_feature_control &
+ FEATURE_CONTROL_SGX_LAUNCH_CONTROL_ENABLE));
+}
+
+#define SGX_INTEL_DEFAULT_LEPUBKEYHASH0 0xa6053e051270b7ac
+#define SGX_INTEL_DEFAULT_LEPUBKEYHASH1 0x6cfbe8ba8b3b413d
+#define SGX_INTEL_DEFAULT_LEPUBKEYHASH2 0xc4916d99f2b3735d
+#define SGX_INTEL_DEFAULT_LEPUBKEYHASH3 0xd4f8c05909f9bb3b
+
+static void vmx_sgx_init_lepubkeyhash(struct kvm_vcpu *vcpu)
+{
+ u64 h0, h1, h2, h3;
+
+ /*
+ * If runtime launch control is enabled (IA32_SGXLEPUBKEYHASHn is
+ * writable), we set guest's default value to be Intel's default
+ * hash (which is fixed value and can be hard-coded). Otherwise,
+ * guest can only use machine's IA32_SGXLEPUBKEYHASHn so set guest's
+ * default to that.
+ */
+ if (cpu_sgx_lepubkeyhash_writable()) {
+ h0 = SGX_INTEL_DEFAULT_LEPUBKEYHASH0;
+ h1 = SGX_INTEL_DEFAULT_LEPUBKEYHASH1;
+ h2 = SGX_INTEL_DEFAULT_LEPUBKEYHASH2;
+ h3 = SGX_INTEL_DEFAULT_LEPUBKEYHASH3;
+ }
+ else {
+ rdmsrl(MSR_IA32_SGXLEPUBKEYHASH0, h0);
+ rdmsrl(MSR_IA32_SGXLEPUBKEYHASH1, h1);
+ rdmsrl(MSR_IA32_SGXLEPUBKEYHASH2, h2);
+ rdmsrl(MSR_IA32_SGXLEPUBKEYHASH3, h3);
+ }
+
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[0] = h0;
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[1] = h1;
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[2] = h2;
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[3] = h3;
+}
+
+static void vmx_sgx_lepubkeyhash_load(struct kvm_vcpu *vcpu)
+{
+ wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0,
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[0]);
+ wrmsrl(MSR_IA32_SGXLEPUBKEYHASH1,
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[1]);
+ wrmsrl(MSR_IA32_SGXLEPUBKEYHASH2,
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[2]);
+ wrmsrl(MSR_IA32_SGXLEPUBKEYHASH3,
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[3]);
+}
+
/*
* Switches to specified vcpu, until a matching vcpu_put(), but assumes
* vcpu mutex is already taken.
@@ -2316,6 +2383,14 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
vmx_vcpu_pi_load(vcpu, cpu);
vmx->host_pkru = read_pkru();
+
+ /*
+ * Load guset's SGX LE pubkey hash if runtime launch control is
+ * enabled.
+ */
+ if (guest_cpuid_has_sgx_launch_control(vcpu) &&
+ cpu_sgx_lepubkeyhash_writable())
+ vmx_sgx_lepubkeyhash_load(vcpu);
}
static void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu)
@@ -3225,6 +3300,19 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_IA32_FEATURE_CONTROL:
msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control;
break;
+ case MSR_IA32_SGXLEPUBKEYHASH0 ... MSR_IA32_SGXLEPUBKEYHASH3:
+ /*
+ * SDM 35.1 Model-Specific Registers, table 35-2.
+ * Read permitted if CPUID.0x12.0:EAX[0] = 1. (We have
+ * guaranteed this will be true if guest_cpuid_has_sgx
+ * is true.)
+ */
+ if (!guest_cpuid_has_sgx(vcpu))
+ return 1;
+ msr_info->data =
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[msr_info->index -
+ MSR_IA32_SGXLEPUBKEYHASH0];
+ break;
case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
if (!nested_vmx_allowed(vcpu))
return 1;
@@ -3344,6 +3432,37 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
* SGX has been enabled in BIOS before using SGX.
*/
break;
+ case MSR_IA32_SGXLEPUBKEYHASH0 ... MSR_IA32_SGXLEPUBKEYHASH3:
+ /*
+ * SDM 35.1 Model-Specific Registers, table 35-2.
+ * - If CPUID.0x7.0:ECX[30] = 1, FEATURE_CONTROL[17] is
+ * available.
+ * - Write permitted if CPUID.0x12.0:EAX[0] = 1 &&
+ * FEATURE_CONTROL[17] = 1 && FEATURE_CONTROL[0] = 1.
+ */
+ if (!guest_cpuid_has_sgx(vcpu) ||
+ !guest_cpuid_has_sgx_launch_control(vcpu))
+ return 1;
+ /*
+ * Don't let userspace set guest's IA32_SGXLEPUBKEYHASHn,
+ * if machine's IA32_SGXLEPUBKEYHASHn cannot be changed at
+ * runtime. Note to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash are
+ * set to default in vmx_create_vcpu therefore guest is able
+ * to get the machine's IA32_SGXLEPUBKEYHASHn by rdmsr in
+ * guest.
+ */
+ if (!cpu_sgx_lepubkeyhash_writable())
+ return 1;
+ /*
+ * If guest's FEATURE_CONTROL[17] is not set, guest's
+ * IA32_SGXLEPUBKEYHASHn are not writeable from guest.
+ */
+ if (!vmx_sgx_lc_disabled_in_bios(vcpu) &&
+ !msr_info->host_initiated)
+ return 1;
+ to_vmx(vcpu)->msr_ia32_sgxlepubkeyhash[msr_index -
+ MSR_IA32_SGXLEPUBKEYHASH0] = data;
+ break;
case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
if (!msr_info->host_initiated)
return 1; /* they are read-only */
@@ -9305,6 +9424,10 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
vmx->nested.vpid02 = allocate_vpid();
}
+ /* Set vcpu's default IA32_SGXLEPUBKEYHASHn */
+ if (enable_sgx && boot_cpu_has(X86_FEATURE_SGX_LAUNCH_CONTROL))
+ vmx_sgx_init_lepubkeyhash(&vmx->vcpu);
+
vmx->nested.posted_intr_nv = -1;
vmx->nested.current_vmptr = -1ull;
vmx->nested.current_vmcs12 = NULL;
