On Mon, Sep 30, 2019 at 12:34 AM Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
>
> Liran Alon <liran.alon@xxxxxxxxxx> writes:
>
> > Commit bf653b78f960 ("KVM: vmx: Introduce handle_unexpected_vmexit
> > and handle WAITPKG vmexit") introduced proprietary handling of
> > specific exit-reasons that should not be raised by CPU because
> > KVM configures VMCS such that they should never be raised.
> >
> > However, since commit 7396d337cfad ("KVM: x86: Return to userspace
> > with internal error on unexpected exit reason"), VMX & SVM
> > exit handlers were modified to generically handle all unexpected
> > exit-reasons by returning to userspace with internal error.
> >
> > Therefore, there is no need for proprietary handling of specific
> > unexpected exit-reasons (This proprietary handling also introduced
> > inconsistency for these exit-reasons to silently skip guest instruction
> > instead of return to userspace on internal-error).
> >
> > Fixes: bf653b78f960 ("KVM: vmx: Introduce handle_unexpected_vmexit and handle WAITPKG vmexit")
> >
> > Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
>
> (It's been awhile since in software world the word 'proprietary' became
> an opposite of free/open-source to me so I have to admit your subject
> line really got me interested :-)
>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
I agree that proprietary is an unusual word choice.
Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/vmx/vmx.c | 12 ------------
> > 1 file changed, 12 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index d4575ffb3cec..e31317fc8c95 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -5538,14 +5538,6 @@ static int handle_encls(struct kvm_vcpu *vcpu)
> > return 1;
> > }
> >
> > -static int handle_unexpected_vmexit(struct kvm_vcpu *vcpu)
> > -{
> > - kvm_skip_emulated_instruction(vcpu);
> > - WARN_ONCE(1, "Unexpected VM-Exit Reason = 0x%x",
> > - vmcs_read32(VM_EXIT_REASON));
> > - return 1;
> > -}
> > -
> > /*
> > * The exit handlers return 1 if the exit was handled fully and guest execution
> > * may resume. Otherwise they set the kvm_run parameter to indicate what needs
> > @@ -5597,15 +5589,11 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
> > [EXIT_REASON_INVVPID] = handle_vmx_instruction,
> > [EXIT_REASON_RDRAND] = handle_invalid_op,
> > [EXIT_REASON_RDSEED] = handle_invalid_op,
> > - [EXIT_REASON_XSAVES] = handle_unexpected_vmexit,
> > - [EXIT_REASON_XRSTORS] = handle_unexpected_vmexit,
> > [EXIT_REASON_PML_FULL] = handle_pml_full,
> > [EXIT_REASON_INVPCID] = handle_invpcid,
> > [EXIT_REASON_VMFUNC] = handle_vmx_instruction,
> > [EXIT_REASON_PREEMPTION_TIMER] = handle_preemption_timer,
> > [EXIT_REASON_ENCLS] = handle_encls,
> > - [EXIT_REASON_UMWAIT] = handle_unexpected_vmexit,
> > - [EXIT_REASON_TPAUSE] = handle_unexpected_vmexit,
> > };
> >
> > static const int kvm_vmx_max_exit_handlers =
>
> --
> Vitaly
