> -----Original Message-----
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Sent: Wednesday, October 19, 2022 11:54 PM
> To: Li,Rongqing <lirongqing@xxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx; Peter Shier <pshier@xxxxxxxxxx>; Jim Mattson
> <jmattson@xxxxxxxxxx>; Wanpeng Li <wanpengli@xxxxxxxxxxx>
> Subject: Re: [PATCH][RFC] KVM: x86: Don't reset deadline to period when timer
> is in one shot mode
>
> On Mon, Oct 17, 2022, Li,Rongqing wrote:
> > > +Jim, Peter, and Wanpeng
> > >
> > > On Wed, Oct 12, 2022, Li RongQing wrote:
> > > > In one-shot mode, the APIC timer stops counting when the timer
> > > > reaches zero, so don't reset deadline to period for one shot mode
> > > >
> > > > Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
> > > > ---
> > > > arch/x86/kvm/lapic.c | 8 ++++++--
> > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> > > > 9dda989..bf39027 100644
> > > > --- a/arch/x86/kvm/lapic.c
> > > > +++ b/arch/x86/kvm/lapic.c
> > > > @@ -1840,8 +1840,12 @@ static bool set_target_expiration(struct
> > > > kvm_lapic
> > > *apic, u32 count_reg)
> > > > if (unlikely(count_reg != APIC_TMICT)) {
> > > > deadline = tmict_to_ns(apic,
> > > > kvm_lapic_get_reg(apic, count_reg));
> > > > - if (unlikely(deadline <= 0))
> > > > - deadline = apic->lapic_timer.period;
> > > > + if (unlikely(deadline <= 0)) {
> > > > + if (apic_lvtt_period(apic))
> > > > + deadline = apic->lapic_timer.period;
> > > > + else
> > > > + deadline = 0;
> > > > + }
> > >
> > > This is not the standard "count has reached zero" path, it's the
> > > "vCPU is migrated and the timer needs to be resumed on the destination"
> path.
> > > Zeroing the deadline here will not squash the timer, IIUC it will
> > > cause the timer to immediately fire.
> > >
> > > That said, I think the patch is actually correct
> >
> > Should we set deadline to 0 when it is expired whether the timer is
> > one shot mode or period mode ?
>
> I'm not sure I follow the question. Are you asking if this path should set the
> deadline to '0' even if the timer is in periodic mode?
Yes
-Li
