On Thu, Sep 20, 2018 at 02:28:28PM -0700, Krish Sadhukhan wrote: > > > On 08/28/2018 09:04 AM, Sean Christopherson wrote: > >Write VM_EXIT_CONTROLS using vm_exit_controls_init() when configuring > >vmcs02, otherwise vm_exit_controls_shadow will be stale. EFER in > >particular can be corrupted if VM_EXIT_LOAD_IA32_EFER is not updated > >due to an incorrect shadow optimization, which can crash L0 due to > >EFER not being loaded on exit. This does not occur with the current > >code base simply because update_transition_efer() unconditionally > >clears VM_EXIT_LOAD_IA32_EFER before conditionally setting it, and > >because a nested guest always starts with VM_EXIT_LOAD_IA32_EFER > >clear, i.e. we'll only ever unnecessarily clear the bit. That is, > >until someone optimizes update_transition_efer()... > > > >Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > >--- > > arch/x86/kvm/vmx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > >index 6451e63847d9..b7aca0edeb59 100644 > >--- a/arch/x86/kvm/vmx.c > >+++ b/arch/x86/kvm/vmx.c > >@@ -12186,7 +12186,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > > * we should use its exit controls. Note that VM_EXIT_LOAD_IA32_EFER > > * bits are further modified by vmx_set_efer() below. > > */ > >- vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl); > >+ vm_exit_controls_init(vmx, vmcs_config.vmexit_ctrl); > > /* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE are > > * emulated by vmx_set_efer(), below. > > We call vm_exit_controls_init() in vmx_vcpu_setup(). Just wondering why > that initialization doesn't persist through prepare_vmcs02(). vm_exit_controls_shadow, which is written by vm_exit_controls_init(), is a shadow of the VM_EXIT_CONTROLS VMCS field. The shadow is used to elide VMREADs and VMWRITEs to VM_EXIT_CONTROLS, i.e. we don't need to VMREAD the field when toggling a bit and we can skip VMWRITE if the desired value matches the shadow (and therefore matches the VMCS). vm_exit_controls_shadow needs to be kept in sync with the VMCS field at all times, otherwise we might incorrectly skip a VMWRITE, e.g. if VM_EXIT_LOAD_IA32_EFER bit is set in the shadow but not the actual VMCS field. When we switch to vmcs02 we need to re-initialize the shadow because the shadow is per-VCPU, not per-VMCS.
