On 19/04/21 17:16, Sean Christopherson wrote:
On Mon, Apr 19, 2021, Kai Huang wrote:
On Sat, 2021-04-17 at 16:11 +0200, Paolo Bonzini wrote:
On 12/04/21 06:21, Kai Huang wrote:
@@ -4377,6 +4380,15 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
if (!vcpu->kvm->arch.bus_lock_detection_enabled)
exec_control &= ~SECONDARY_EXEC_BUS_LOCK_DETECTION;
+ if (cpu_has_vmx_encls_vmexit() && nested) {
+ if (guest_cpuid_has(vcpu, X86_FEATURE_SGX))
+ vmx->nested.msrs.secondary_ctls_high |=
+ SECONDARY_EXEC_ENCLS_EXITING;
+ else
+ vmx->nested.msrs.secondary_ctls_high &=
+ ~SECONDARY_EXEC_ENCLS_EXITING;
+ }
+
This is incorrect, I've removed it. The MSRs can only be written by
userspace.
vmx_compute_secondary_exec_control() violates that left, right, and center, it's
just buried down in vmx_adjust_secondary_exec_control(). This is an open coded
version of that helper, sans the actual update to exec_control since ENCLS needs
to be conditionally intercepted even when it's exposed to the guest.
Hmm, that's true. I'll place back the version that you had.
Paolo
If SGX is disabled in the guest CPUID, nested_vmx_exit_handled_encls can
just do:
if (!guest_cpuid_has(vcpu, X86_FEATURE_SGX) ||
!nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENCLS_EXITING))
return false;
and the useless ENCLS exiting bitmap in vmcs12 will be ignored.
Paolo
Thanks for queuing this series!
Looks good to me. However if I read code correctly, in this way a side effect would be
vmx->nested.msrs.secondary_ctls_high will always have SECONDARY_EXEC_ENCLS_EXITING bit
set, even SGX is not exposed to guest, which means a guest can set this even SGX is not
present, but I think it is OK since ENCLS exiting bitmap in vmcs12 will be ignored anyway
in nested_vmx_exit_handled_encls() as you mentioned above.
Anyway, I have tested this code and it works at my side (by creating L2 with SGX support
and running SGX workloads inside it).
Sean, please also comment if you have any.
