[RFC 1/4] KVM: nVMX: Don't copy L2's CET state to L1 if VM-entry didn't load it

[Zhao Liu <zhao1.liu@intel.com>]

On a nested VM-exit that disables VM_EXIT_LOAD_CET_STATE, only copy L2's
CET state from vmcs12 to vmcs01 if VM-entry really loaded that state,
i.e. don't copy when VM-entry fails before loading guest state.

The state, that L1 should see after a L2 VM-exit, depends on three
things: the VM-exit load (host state) control, whether VM-entry loaded
L2's state, and whether L2 ran.

For CET, there are 4 cases:

 1) VM_EXIT_LOAD_CET_STATE is set. Load L1's CET state from vmcs12's
    host fields, no matter what happened before. KVM already does this.

 2) VM_EXIT_LOAD_CET_STATE is clear, and VM-entry loaded L2's CET
    state. Whether it's the normal VM-exit or VM-entry failure exit,
    the guest's (L2's) state should be retained, so copy vmcs12's guest
    fields into vmcs01 to give L1 the same result.

 3) VM_EXIT_LOAD_CET_STATE is clear, VM-entry didn't load L2's CET
    state, and L2 never ran. This is the typical case that VM-entry
    fails before loading guest state, the CPU keeps L1's own state, so
    do nothing.

 4) VM_EXIT_LOAD_CET_STATE is clear, VM-entry didn't load L2's CET
    state, but L2 ran and exited normally. The CPU keeps L1's state
    again, but L2 could have changed it while running, so still copy
    vmcs12's guest fields into vmcs01, because they hold what L2 left
    behind.

Case 3) is broken today. When VM-entry fails, KVM copies vmcs12's guest
CET fields into vmcs01 as long as VM_EXIT_LOAD_CET_STATE is clear, so L1
gets the state it wrote for L2 instead of its own state. KVM never syncs
vmcs02 back to vmcs12 on this path, so those guest fields still hold
what L1 wrote with VMWRITE.

To fix case 3), it's necessary to distinguish case 2), case 3) and case
4). But one "VM-entry failed" flag is not enough, since it only tells
whether L2 ran, and lacks the information about whether VM-entry loaded
L2's state - and this is important, EXIT_REASON_MSR_LOAD_FAIL is
triggered after guest state loading, but EXIT_REASON_INVALID_STATE is
not.

Note, SDM vol.3, chapter 29, "VM ENTRIES", does not guarantee the order
of the guest state check and the guest state load, however KVM can more
directly assume that the guest state load occurs after the check,
thereby simplifying the emulation of state handling when
EXIT_REASON_INVALID_STATE occurs (corresponding to Case 3). But MSR list
loading is after guest state loading, so at EXIT_REASON_MSR_LOAD_FAIL,
guest state has been loaded.

Therefore, to determine whether the VM-entry loaded L2's state and
whether L2 ran, introduce the nested_l2_state enumeration to mark the L2
guest state phase, thereby helping to distinguish between Case 2), Case
3), and Case 4) in a helper nested_l2_state_is_live().

This pattern can be reused to support additional features that have load
controls, such as BNDCFGS, PAT, and FRED.

Fixes: 625884996bff ("KVM: nVMX: Prepare for enabling CET support for nested guest")
Reported-by: Xin Li <xin@zytor.com>
Suggested-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 arch/x86/kvm/vmx/nested.c | 59 +++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407abd..35f0bf84b373 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3613,8 +3613,45 @@ static int nested_vmx_check_permission(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+/*
+ * Describe the loading state of L2 guest state, i.e. whether VM-Entry loaded
+ * L2's state from vmcs12 into vmcs02, and whether L2's state is synced back to
+ * vmcs12.
+ */
+enum nested_l2_state {
+	/* VM-entry failed before finishing loading L2's state. */
+	L2_STATE_NOT_LOADED,
+	/* VM-entry loaded L2's state from vmcs12 into vmcs02 before L2 runs. */
+	L2_STATE_LOADED_FROM_VMCS12,
+	/* L2 ran, and KVM saved L2's live state to vmcs12 from vmcs02 on VM-exit. */
+	L2_STATE_SAVED_TO_VMCS12,
+};
+
+/*
+ * Return true if L2's guest state in vmcs12 needs to be loaded into vmcs01,
+ * i.e. if L1 should observe L2's state retained on hardware when L1 runs.
+ * @vm_entry_load_control is the VM-Entry control that loads the state on
+ * VM-Entry.
+ *
+ * Note: this helper is used when the VM-exit load (host state) control is off.
+ * Otherwise, host state (L1 state) should be loaded into vmcs01.
+ */
+static bool nested_l2_state_is_live(struct vmcs12 *vmcs12,
+				    u32 vm_entry_load_control,
+				    enum nested_l2_state l2_state)
+{
+	/* normal VM-exit. */
+	if (l2_state == L2_STATE_SAVED_TO_VMCS12)
+		return true;
+
+	/* true iff VM-entry failed after loading L2's state. */
+	return l2_state == L2_STATE_LOADED_FROM_VMCS12 &&
+	       (vmcs12->vm_entry_controls & vm_entry_load_control);
+}
+
 static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
-				   struct vmcs12 *vmcs12);
+				   struct vmcs12 *vmcs12,
+				   enum nested_l2_state l2_state);
 
 /*
  * If from_vmentry is false, this is being called from state restore (either RSM
@@ -3636,6 +3673,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 		.basic = EXIT_REASON_INVALID_STATE,
 		.failed_vmentry = 1,
 	};
+	enum nested_l2_state l2_state = L2_STATE_NOT_LOADED;
 	u32 failed_index;
 
 	trace_kvm_nested_vmenter(kvm_rip_read(vcpu),
@@ -3700,6 +3738,13 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 		goto vmentry_fail_vmexit_guest_mode;
 	}
 
+	/*
+	 * VM-entry has completed the architectural guest-state loading phase;
+	 * MSRs are loaded after guest state, so failures below should retain
+	 * L2's state (see nested_l2_state_is_live()).
+	 */
+	l2_state = L2_STATE_LOADED_FROM_VMCS12;
+
 	if (from_vmentry) {
 		failed_index = nested_vmx_load_msr(vcpu,
 						   vmcs12->vm_entry_msr_load_addr,
@@ -3778,7 +3823,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 
 	nested_put_vmcs12_pages(vcpu);
 
-	load_vmcs12_host_state(vcpu, vmcs12);
+	load_vmcs12_host_state(vcpu, vmcs12, l2_state);
 	vmcs12->vm_exit_reason = exit_reason.full;
 	if (enable_shadow_vmcs || nested_vmx_is_evmptr12_valid(vmx))
 		vmx->nested.need_vmcs12_to_shadow_sync = true;
@@ -4798,7 +4843,8 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
  * This function should be called when the active VMCS is L1's (vmcs01).
  */
 static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
-				   struct vmcs12 *vmcs12)
+				   struct vmcs12 *vmcs12,
+				   enum nested_l2_state l2_state)
 {
 	enum vm_entry_failure_code ignored;
 	struct kvm_segment seg;
@@ -4856,12 +4902,13 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	/*
 	 * Load CET state from host state if VM_EXIT_LOAD_CET_STATE is set.
 	 * otherwise CET state should be retained across VM-exit, i.e.,
-	 * guest values should be propagated from vmcs12 to vmcs01.
+	 * guest values should be propagated from vmcs12 to vmcs01, but only if
+	 * L2's CET state is live in hardware.
 	 */
 	if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_CET_STATE)
 		vmcs_write_cet_state(vcpu, vmcs12->host_s_cet, vmcs12->host_ssp,
 				     vmcs12->host_ssp_tbl);
-	else
+	else if (nested_l2_state_is_live(vmcs12, VM_ENTRY_LOAD_CET_STATE, l2_state))
 		vmcs_write_cet_state(vcpu, vmcs12->guest_s_cet, vmcs12->guest_ssp,
 				     vmcs12->guest_ssp_tbl);
 
@@ -5193,7 +5240,7 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
 						       vmcs12->vm_exit_intr_error_code,
 						       KVM_ISA_VMX);
 
-		load_vmcs12_host_state(vcpu, vmcs12);
+		load_vmcs12_host_state(vcpu, vmcs12, L2_STATE_SAVED_TO_VMCS12);
 
 		/*
 		 * Process events if an injectable IRQ or NMI is pending, even
-- 
2.34.1