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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 7 Jan 2021 12:19:30 -0800 From: Sean Christopherson <seanjc@...gle.com> To: Paolo Bonzini <pbonzini@...hat.com> Cc: Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org, 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> Subject: Re: [PATCH v2 2/4] KVM: nSVM: correctly restore nested_run_pending on migration On Thu, Jan 07, 2021, Paolo Bonzini wrote: > 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. Which v1 change are you referring to? > 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. Hmm, as weird as that code is, I think it's actually the most architecturally correct behavior. Technically, the clearing of VM_ENTRY_INTR_INFO.VALID shouldn't be visible in vmcs12 until a nested VM-Exit occurs, e.g. copying the vmcs02 value to vmcs12 in vmx_get_nested_state() would work, but it's wrong at the same time. Ditto for L1 (or L2) writing vmcs12.VM_ENTRY_INTR_INFO while L2 is running (ignoring the SDM's very clear warning that doing so is bad); from L1/L2's perspective, there is no VM-Entry so writing vmcs12.VM_ENTRY_INTR_INFO should never generate an event in L2, even on CPUs without VMCS caching.
Powered by blists - more mailing lists