[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <X/X13wD58Oi/0XpX@google.com>
Date: Wed, 6 Jan 2021 09:39:43 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: kvm@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <sean.j.christopherson@...el.com>,
"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@...r.kernel.org>, Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
Wanpeng Li <wanpengli@...cent.com>,
"H. Peter Anvin" <hpa@...or.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH 5/6] KVM: nSVM: always leave the nested state first on
KVM_SET_NESTED_STATE
On Wed, Jan 06, 2021, Maxim Levitsky wrote:
> This should prevent bad things from happening if the user calls the
> KVM_SET_NESTED_STATE twice.
This doesn't exactly inspire confidence, nor does it provide much help to
readers that don't already know why KVM should "leave nested" before processing
the rest of kvm_state.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
> arch/x86/kvm/svm/nested.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index c1a3d0e996add..3aa18016832d0 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1154,8 +1154,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> if (is_smm(vcpu) && (kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
> return -EINVAL;
>
> + svm_leave_nested(svm);
nVMX sets a really bad example in that it does vmx_leave_nested(), and many
other things, long before it has vetted the incoming state. That's not the end
of the word as the caller is likely going to exit if this ioctl() fails, but it
would be nice to avoid such behavior with nSVM, especially since it appears to
be trivially easy to do svm_leave_nested() iff the ioctl() will succeed.
> +
> if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) {
> - svm_leave_nested(svm);
> svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
> return 0;
> }
> --
> 2.26.2
>
Powered by blists - more mailing lists