linux-kernel - Re: [PATCH v2 2/4] KVM: nSVM: correctly restore nested_run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <98f35e0a-d82b-cac0-b267-00fcba00c185@redhat.com>
Date:   Thu, 7 Jan 2021 19:03:18 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>,
        Jim Mattson <jmattson@...gle.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH v2 2/4] KVM: nSVM: correctly restore nested_run_pending on
 migration

On 07/01/21 10:38, Maxim Levitsky wrote:
> The code to store it on the migration exists, but no code was restoring it.
> 
> One of the side effects of fixing this is that L1->L2 injected events
> are no longer lost when migration happens with nested run pending.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
>   arch/x86/kvm/svm/nested.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index ee4f2082ad1bd..cc3130ab612e5 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1200,6 +1200,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>   	 * in the registers, the save area of the nested state instead
>   	 * contains saved L1 state.
>   	 */
> +
> +	svm->nested.nested_run_pending =
> +		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
> +
>   	copy_vmcb_control_area(&hsave->control, &svm->vmcb->control);
>   	hsave->save = *save;
>   
> 

Nice fix and we need to do it anyway.

That said, the v1 change had some appeal to it.  In the VMX case (if 
properly implemented) it would allow removing the weird 
nested_run_pending case from prepare_vmcs02_early.  I think it's a 
valuable invariant that there are no events in the VMCS after each 
KVM_RUN iteration, and this special case is breaking the invariant.

Paolo