Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Emanuele Giuseppe Esposito <eesposit@redhat.com>
To:     kvm@vger.kernel.org
Cc:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>, Bandan Das <bsd@redhat.com>,
        linux-kernel@vger.kernel.org,
        Emanuele Giuseppe Esposito <eesposit@redhat.com>
Subject: [PATCH] KVM: vmx/nested: avoid blindly setting SECONDARY_EXEC_ENCLS_EXITING when sgx is enabled
Date:   Mon, 24 Oct 2022 08:48:45 -0400
Message-Id: <20221024124845.1927035-1-eesposit@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Currently vmx enables SECONDARY_EXEC_ENCLS_EXITING even when sgx
is not set in the host MSR.
This was probably introduced when sgx was not yet fully supported, and
we wanted to trap guests trying to use the feature.

When booting a guest, KVM checks that the cpuid bit is actually set
in vmx.c, and if not, it does not enable the feature.

However, in nesting this control bit is blindly copied, and will be
propagated to VMCS12 and VMCS02. Therefore, when L1 tries to boot
the guest, the host will try to execute VMLOAD with VMCS02 containing
a feature that the hardware does not support, making it fail with
hardware error 0x7.

According with section A.3.3 of Intel System Programming Guide,
we should *always* check the value in the actual
MSR_IA32_VMX_PROCBASED_CTLS2 before enabling this bit.

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2127128

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 arch/x86/kvm/vmx/nested.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 8f67a9c4a287..f651084010cc 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6678,6 +6678,25 @@ static u64 nested_vmx_calc_vmcs_enum_msr(void)
 	return (u64)max_idx << VMCS_FIELD_INDEX_SHIFT;
 }
 
+/*
+ * According with section A.3.3 of Intel System Programming Guide,
+ * we *can* set the guest MSR control X (in our case
+ * SECONDARY_EXEC_ENCLS_EXITING) *iff* bit 32+X of
+ * MSR_IA32_VMX_PROCBASED_CTLS2 is set to 1.
+ * Otherwise it must remain zero.
+ */
+static void nested_vmx_setup_encls_exiting(struct nested_vmx_msrs *msrs)
+{
+	u32 vmx_msr_procb_low, vmx_msr_procb_high;
+
+	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2, vmx_msr_procb_low, vmx_msr_procb_high);
+
+	WARN_ON(vmx_msr_procb_low & SECONDARY_EXEC_ENCLS_EXITING);
+
+	if (enable_sgx && (vmx_msr_procb_high & SECONDARY_EXEC_ENCLS_EXITING))
+		msrs->secondary_ctls_high |= SECONDARY_EXEC_ENCLS_EXITING;
+}
+
 /*
  * nested_vmx_setup_ctls_msrs() sets up variables containing the values to be
  * returned for the various VMX controls MSRs when nested VMX is enabled.
@@ -6874,8 +6893,7 @@ void nested_vmx_setup_ctls_msrs(struct vmcs_config *vmcs_conf, u32 ept_caps)
 		msrs->secondary_ctls_high |=
 			SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
 
-	if (enable_sgx)
-		msrs->secondary_ctls_high |= SECONDARY_EXEC_ENCLS_EXITING;
+	nested_vmx_setup_encls_exiting(msrs);
 
 	/* miscellaneous data */
 	msrs->misc_low = (u32)vmcs_conf->misc & VMX_MISC_SAVE_EFER_LMA;
-- 
2.31.1