| Message ID | 20181204190306.9928-2-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM nVMX: MSRs should not be stored if VM-entry fails during or after loading guest state | expand |
On Tue, Dec 4, 2018 at 11:27 AM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > > According to section "VM-entry Failures During or After Loading Guest State" > in Intel SDM vol 3C, > > "No MSRs are saved into the VM-exit MSR-store area." > > when bit 31 of the exit reason is set. > > Reported-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Suggested-by: Jim Mattson <jmattson@google.com> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > --- > arch/x86/kvm/vmx.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 4555077..9e1a640 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -13827,6 +13827,19 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > * L2 to IDT_VECTORING_INFO_FIELD. > */ > vmcs12_save_pending_event(vcpu, vmcs12); > + > + /* > + * There's no need to store the guest's MSRs if we exit > + * due to a VM-entry failure that occurs during or after > + * loading the guest state, because the guest has not > + * executed any instruction in that case. However, here we > + * are past that point. So we need to save the MSRs. > + */ Executing instructions is a distraction. The specification indicates that these MSRs are stored even in cases where no guest instruction has executed, so long as the VM-exit was not due to a VM-entry failure during or after loading guest state. There's no need to store the guest's MSRs if the exit was due to a VM-entry failure during or after loading guest state (and, in fact, it is incorrect to do so), because the specification indicates that these MSRs are not stored in that case. > + if (nested_vmx_store_msr(vcpu, > + vmcs12->vm_exit_msr_store_addr, > + vmcs12->vm_exit_msr_store_count)) > + nested_vmx_abort(vcpu, > + VMX_ABORT_SAVE_GUEST_MSR_FAIL); > } > > /* > @@ -14159,10 +14172,6 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, > * immutable. > */ > nested_flush_cached_shadow_vmcs12(vcpu, vmcs12); > - > - if (nested_vmx_store_msr(vcpu, vmcs12->vm_exit_msr_store_addr, > - vmcs12->vm_exit_msr_store_count)) > - nested_vmx_abort(vcpu, VMX_ABORT_SAVE_GUEST_MSR_FAIL); > } else { > /* > * The only expected VM-instruction error is "VM entry with > -- > 2.9.5 >
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 4555077..9e1a640 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -13827,6 +13827,19 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, * L2 to IDT_VECTORING_INFO_FIELD. */ vmcs12_save_pending_event(vcpu, vmcs12); + + /* + * There's no need to store the guest's MSRs if we exit + * due to a VM-entry failure that occurs during or after + * loading the guest state, because the guest has not + * executed any instruction in that case. However, here we + * are past that point. So we need to save the MSRs. + */ + if (nested_vmx_store_msr(vcpu, + vmcs12->vm_exit_msr_store_addr, + vmcs12->vm_exit_msr_store_count)) + nested_vmx_abort(vcpu, + VMX_ABORT_SAVE_GUEST_MSR_FAIL); } /* @@ -14159,10 +14172,6 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, * immutable. */ nested_flush_cached_shadow_vmcs12(vcpu, vmcs12); - - if (nested_vmx_store_msr(vcpu, vmcs12->vm_exit_msr_store_addr, - vmcs12->vm_exit_msr_store_count)) - nested_vmx_abort(vcpu, VMX_ABORT_SAVE_GUEST_MSR_FAIL); } else { /* * The only expected VM-instruction error is "VM entry with