Shortlog says nVMX, code says nSVM :-)
On Wed, May 27, 2020 at 10:29:21AM +0200, Vitaly Kuznetsov wrote:
> L2 guest hang is observed after 'exit_required' was dropped and nSVM
> switched to check_nested_events() completely. The hang is a busy loop when
> e.g. KVM is emulating an instruction (e.g. L2 is accessing MMIO space and
> we drop to userspace). After nested_svm_vmexit() and when L1 is doing VMRUN
> nested guest's RIP is not advanced so KVM goes into emulating the same
> instruction which cased nested_svm_vmexit() and the loop continues.
s/cased/caused?
> nested_svm_vmexit() is not new, however, with check_nested_events() we're
> now calling it later than before. In case by that time KVM has modified
> register state we may pick stale values from VMCS when trying to save
s/VMCS/VMCB
> nested guest state to nested VMCB.
>
> VMX code handles this case correctly: sync_vmcs02_to_vmcs12() called from
> nested_vmx_vmexit() does 'vmcs12->guest_rip = kvm_rip_read(vcpu)' and this
> ensures KVM-made modifications are preserved. Do the same for nVMX.
s/nVMX/nSVM
>
> Generally, nested_vmx_vmexit()/nested_svm_vmexit() need to pick up all
> nested guest state modifications done by KVM after vmexit. It would be
> great to find a way to express this in a way which would not require to
> manually track these changes, e.g. nested_{vmcb,vmcs}_get_field().
>
> Co-debugged-with: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> - To certain extent we're fixing currently-pending 'KVM: SVM: immediately
> inject INTR vmexit' commit but I'm not certain about that. We had so many
> problems with nested events before switching to check_nested_events() that
> what worked before could just be treated as a miracle. Miracles tend to
> appear and disappear all of a sudden.
> ---
> arch/x86/kvm/svm/nested.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 0f02521550b9..6b1049148c1b 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -537,9 +537,9 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> nested_vmcb->save.cr2 = vmcb->save.cr2;
> nested_vmcb->save.cr4 = svm->vcpu.arch.cr4;
> nested_vmcb->save.rflags = kvm_get_rflags(&svm->vcpu);
> - nested_vmcb->save.rip = vmcb->save.rip;
> - nested_vmcb->save.rsp = vmcb->save.rsp;
> - nested_vmcb->save.rax = vmcb->save.rax;
> + nested_vmcb->save.rip = kvm_rip_read(&svm->vcpu);
> + nested_vmcb->save.rsp = kvm_rsp_read(&svm->vcpu);
> + nested_vmcb->save.rax = kvm_rax_read(&svm->vcpu);
> nested_vmcb->save.dr7 = vmcb->save.dr7;
> nested_vmcb->save.dr6 = svm->vcpu.arch.dr6;
> nested_vmcb->save.cpl = vmcb->save.cpl;
> --
> 2.25.4
>
