On Tue, Apr 20, 2021 at 11:26:34PM +0000, Sean Christopherson wrote:
> On Wed, Apr 21, 2021, Frederic Weisbecker wrote:
> > On Thu, Apr 15, 2021 at 03:21:00PM -0700, Sean Christopherson wrote:
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 16fb39503296..e4d475df1d4a 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -9230,6 +9230,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > > local_irq_disable();
> > > kvm_after_interrupt(vcpu);
> > >
> > > + /*
> > > + * When using tick-based accounting, wait until after servicing IRQs to
> > > + * account guest time so that any ticks that occurred while running the
> > > + * guest are properly accounted to the guest.
> > > + */
> > > + if (!vtime_accounting_enabled_this_cpu())
> > > + vtime_account_guest_exit();
> >
> > Can we rather have instead:
> >
> > static inline void tick_account_guest_exit(void)
> > {
> > if (!vtime_accounting_enabled_this_cpu())
> > current->flags &= ~PF_VCPU;
> > }
> >
> > It duplicates a bit of code but I think this will read less confusing.
>
> Either way works for me. I used vtime_account_guest_exit() to try to keep as
> many details as possible inside vtime, e.g. in case the implemenation is tweaked
> in the future. But I agree that pretending KVM isn't already deeply intertwined
> with the details is a lie.
Ok let's keep it as is then. It reads funny but can we perhaps hope that
other archs have the same issue and in the future we'll need to have this split
generalized outside x86 with:
* guest_exit_vtime()
* guest_exit_tick()
