linux-kernel - Re: [PATCH v3 2/5] KVM: X86: Introduce need_cancel_enter

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: <20200428010315.GE14870@linux.intel.com>
Date:   Mon, 27 Apr 2020 18:03:15 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Wanpeng Li <kernellwp@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, kvm <kvm@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Haiwei Li <lihaiwei@...cent.com>
Subject: Re: [PATCH v3 2/5] KVM: X86: Introduce need_cancel_enter_guest helper

On Tue, Apr 28, 2020 at 08:44:13AM +0800, Wanpeng Li wrote:
> On Tue, 28 Apr 2020 at 02:36, Sean Christopherson
> <sean.j.christopherson@...el.com> wrote:
> >
> > > @@ -6771,12 +6774,10 @@ static enum exit_fastpath_completion
> > > vmx_vcpu_run(struct kvm_vcpu *vcpu)
> > >      vmx_recover_nmi_blocking(vmx);
> > >      vmx_complete_interrupts(vmx);
> > >
> > > -    if (!(kvm_need_cancel_enter_guest(vcpu))) {
> > > -        exit_fastpath = vmx_exit_handlers_fastpath(vcpu);
> > > -        if (exit_fastpath == EXIT_FASTPATH_CONT_RUN) {
> > > -            vmx_sync_pir_to_irr(vcpu);
> > > -            goto cont_run;
> > > -        }
> > > +    exit_fastpath = vmx_exit_handlers_fastpath(vcpu);
> > > +    if (exit_fastpath == EXIT_FASTPATH_CONT_RUN) {
> >
> > Relying on the handlers to check kvm_need_cancel_enter_guest() will be
> > error prone and costly to maintain.  I also don't like that it buries the
> > logic.
> >
> > What about adding another flavor, e.g.:
> >
> >         exit_fastpath = vmx_exit_handlers_fastpath(vcpu);
> >         if (exit_fastpath == EXIT_FASTPATH_CONT_RUN &&
> >             kvm_need_cancel_enter_guest(vcpu))
> >                 exit_fastpath = EXIT_FASTPATH_NOP;
> >
> > That would also allow you to enable preemption timer without first having
> > to add CONT_RUN, which would be a very good thing for bisection.
> 
> I miss understand the second part, do you mean don't need to add
> CONT_RUN in patch 1/5?

Yes, with the disclaimer that I haven't worked through all the flows to
ensure it's actually doable and/or a good idea. 

The idea is to add EXIT_FASTPATH_NOP and use that for the preemption timer
fastpath.  KVM would still go through it's full run loop, but it would skip
invoking the exit handler.  In theory that would disassociate fast handling
of the preemption timer from resuming the guest without going back to the
run loop, i.e. provide a separate bisection point for enabling CONT_RUN.

Like I said, might not be a good idea, e.g. if preemption timer ends up
being the only user of EXIT_FASTPATH_CONT_RUN then EXIT_FASTPATH_NOP is a
waste of space.

Side topic, what about EXIT_FASTPATH_RESUME instead of CONT_RUN?  Or maybe
REENTER_GUEST?  Something that start with RE :-)