| Message ID | 20190929145018.120753-1-liran.alon@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: Remove proprietary handling of unexpected exit-reasons | expand |
Liran Alon <liran.alon@oracle.com> 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@oracle.com> (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@redhat.com> > --- > 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 =
On Mon, Sep 30, 2019 at 12:34 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > Liran Alon <liran.alon@oracle.com> 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@oracle.com> > > (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@redhat.com> I agree that proprietary is an unusual word choice. Reviewed-by: Jim Mattson <jmattson@google.com> > > --- > > 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
On Mon, Sep 30, 2019 at 09:35:59AM -0700, Jim Mattson wrote: > On Mon, Sep 30, 2019 at 12:34 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > > > Liran Alon <liran.alon@oracle.com> 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@oracle.com> > > > > (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@redhat.com> > > I agree that proprietary is an unusual word choice. It's one way to get quick reviews though :-)
> On 30 Sep 2019, at 20:20, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Mon, Sep 30, 2019 at 09:35:59AM -0700, Jim Mattson wrote: >> On Mon, Sep 30, 2019 at 12:34 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote: >>> >>> Liran Alon <liran.alon@oracle.com> 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@oracle.com> >>> >>> (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@redhat.com> >> >> I agree that proprietary is an unusual word choice. > > It's one way to get quick reviews though :-) OK Ok I apologise for my bad English. ^_^ Paolo, feel free to reword this commit title & message to something else when applying… -Liran
On 01/10/19 02:33, Liran Alon wrote: > OK Ok I apologise for my bad English. ^_^ > Paolo, feel free to reword this commit title & message to something else when applying… I'll replace it with "specialized", no problem. I agree with Sean's assessment. :) Paolo
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 =
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@oracle.com> --- arch/x86/kvm/vmx/vmx.c | 12 ------------ 1 file changed, 12 deletions(-)