linux-kernel - Re: [PATCH 4.19 067/306] KVM: nVMX: move check_vmentry_postreqs() call to nested_vmx_enter_non_root

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20191205205206.2mnr3dj2slamuhrl@xps.therub.org>
Date:   Thu, 5 Dec 2019 14:52:06 -0600
From:   Dan Rue <dan.rue@...aro.org>
To:     Jack Wang <jack.wang.usish@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, stable <stable@...r.kernel.org>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Jim Mattson <jmattson@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 067/306] KVM: nVMX: move check_vmentry_postreqs()
 call to nested_vmx_enter_non_root_mode()

On Thu, Dec 05, 2019 at 10:51:18AM +0100, Jack Wang wrote:
> Dan Rue <dan.rue@...aro.org> 于2019年12月4日周三 下午6:50写道：
> >
> > On Mon, Dec 02, 2019 at 03:40:04PM +0100, Jack Wang wrote:
> > > Greg Kroah-Hartman <gregkh@...uxfoundation.org> 于2019年11月27日周三 下午10:30写道：
> > > >
> > > > From: Sean Christopherson <sean.j.christopherson@...el.com>
> > > >
> > > > [ Upstream commit 7671ce21b13b9596163a29f4712cb2451a9b97dc ]
> > > >
> > > > In preparation of supporting checkpoint/restore for nested state,
> > > > commit ca0bde28f2ed ("kvm: nVMX: Split VMCS checks from nested_vmx_run()")
> > > > modified check_vmentry_postreqs() to only perform the guest EFER
> > > > consistency checks when nested_run_pending is true.  But, in the
> > > > normal nested VMEntry flow, nested_run_pending is only set after
> > > > check_vmentry_postreqs(), i.e. the consistency check is being skipped.
> > > >
> > > > Alternatively, nested_run_pending could be set prior to calling
> > > > check_vmentry_postreqs() in nested_vmx_run(), but placing the
> > > > consistency checks in nested_vmx_enter_non_root_mode() allows us
> > > > to split prepare_vmcs02() and interleave the preparation with
> > > > the consistency checks without having to change the call sites
> > > > of nested_vmx_enter_non_root_mode().  In other words, the rest
> > > > of the consistency check code in nested_vmx_run() will be joining
> > > > the postreqs checks in future patches.
> > > >
> > > > Fixes: ca0bde28f2ed ("kvm: nVMX: Split VMCS checks from nested_vmx_run()")
> > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com>
> > > > Cc: Jim Mattson <jmattson@...gle.com>
> > > > Reviewed-by: Jim Mattson <jmattson@...gle.com>
> > > > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> > > > Signed-off-by: Sasha Levin <sashal@...nel.org>
> > > > ---
> > > >  arch/x86/kvm/vmx.c | 10 +++-------
> > > >  1 file changed, 3 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > > index fe7fdd666f091..bdf019f322117 100644
> > > > --- a/arch/x86/kvm/vmx.c
> > > > +++ b/arch/x86/kvm/vmx.c
> > > > @@ -12694,6 +12694,9 @@ static int enter_vmx_non_root_mode(struct kvm_vcpu *vcpu, u32 *exit_qual)
> > > >         if (likely(!evaluate_pending_interrupts) && kvm_vcpu_apicv_active(vcpu))
> > > >                 evaluate_pending_interrupts |= vmx_has_apicv_interrupt(vcpu);
> > > >
> > > > +       if (from_vmentry && check_vmentry_postreqs(vcpu, vmcs12, exit_qual))
> > > > +               return EXIT_REASON_INVALID_STATE;
> > > > +
> > > >         enter_guest_mode(vcpu);
> > > >
> > > >         if (!(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
> > > > @@ -12836,13 +12839,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> > > >          */
> > > >         skip_emulated_instruction(vcpu);
> > > >
> > > > -       ret = check_vmentry_postreqs(vcpu, vmcs12, &exit_qual);
> > > > -       if (ret) {
> > > > -               nested_vmx_entry_failure(vcpu, vmcs12,
> > > > -                                        EXIT_REASON_INVALID_STATE, exit_qual);
> > > > -               return 1;
> > > > -       }
> > > > -
> > > >         /*
> > > >          * We're finally done with prerequisite checking, and can start with
> > > >          * the nested entry.
> > > > --
> > > > 2.20.1
> > > >
> > > >
> > > >
> > > Hi all,
> > >
> > > This commit caused many kvm-unit-tests regression, cherry-pick
> > > following commits from 4.20 fix the regression:
> >
> > Hi Jack - can you be more specific about the failing tests? What type of
> > environment and which tests failed, which version of kvm-unit-tests? Do
> > you have any logs available? I ask because we do run kvm-unit-tests on
> > x86 and arm64 but we did not see these regressions.
> >
> > Thanks,
> > Dan
> >
> Hi Dan,
> 
> I'm running at kvm-unit-tests commit b1414c5f0142 ("x86: vmx: fix
> required alignment for posted interrupt descriptor")
> 
> using "run_tests.sh -a -t -j8" with qemu-2.7.1
> 
> Left side has only 78 tests ok, and right side has 112 tests ok.

Thanks - so we run it with "run_tests.sh -v" and only see 43 passes in
the best case. Besides missing -a, we see a skip for the vmx related
tests because vmx isn't enabled in our environment.

We will fix those problems in LKFT so that we can catch regressions like
this before they are released.

Dan