public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
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	[thread overview]
Message-ID: <20221024124845.1927035-1-eesposit@redhat.com> (raw)

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


             reply	other threads:[~2022-10-24 14:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-24 12:48 Emanuele Giuseppe Esposito [this message]
2022-10-24 16:52 ` [PATCH] KVM: vmx/nested: avoid blindly setting SECONDARY_EXEC_ENCLS_EXITING when sgx is enabled Sean Christopherson
2022-10-25 12:36   ` Emanuele Giuseppe Esposito
  -- strict thread matches above, loose matches on Subject: below --
2022-10-25 12:37 Emanuele Giuseppe Esposito
2022-10-25 17:21 ` Sean Christopherson
2022-10-27 10:29   ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221024124845.1927035-1-eesposit@redhat.com \
    --to=eesposit@redhat.com \
    --cc=bp@alien8.de \
    --cc=bsd@redhat.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox