Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
MIME-Version: 1.0
References: <20231116023036.2324371-1-maobibo@loongson.cn> <20231116023036.2324371-2-maobibo@loongson.cn>
 <564a2fd3-ffba-3bc5-70b9-8a9fa9a0f1c6@loongson.cn>
In-Reply-To: <564a2fd3-ffba-3bc5-70b9-8a9fa9a0f1c6@loongson.cn>
From:   Huacai Chen <chenhuacai@kernel.org>
Date:   Tue, 5 Dec 2023 10:20:17 +0800
Message-ID: <CAAhV-H4P_JUewDM7R1ByNR4PZa97=xM_rAJ239J-wFSd6_+0GA@mail.gmail.com>
Subject: Re: [PATCH v4 1/3] LoongArch: KVM: Remove SW timer switch when vcpu
 is halt polling
To:     zhaotianrui <zhaotianrui@loongson.cn>
Cc:     Bibo Mao <maobibo@loongson.cn>, WANG Xuerui <kernel@xen0n.name>,
        kvm@vger.kernel.org, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

This series looks good to me, If Paolo agrees, I will apply to
loongarch-next after [1] is taken into the kvm tree (otherwise there
will be build errors).

[1] https://lore.kernel.org/loongarch/CAAhV-H63QkfSw+Esn8oW2PDEsCnTRPFqkj8X=
-x8i9cH3AS0k9w@mail.gmail.com/T/#t

On Mon, Dec 4, 2023 at 4:45=E2=80=AFPM zhaotianrui <zhaotianrui@loongson.cn=
> wrote:
>
> Reviewed-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>
> =E5=9C=A8 2023/11/16 =E4=B8=8A=E5=8D=8810:30, Bibo Mao =E5=86=99=E9=81=93=
:
> > With halt-polling supported, there is checking for pending events
> > or interrupts when vcpu executes idle instruction. Pending interrupts
> > include injected SW interrupts and passthrough HW interrupts, such as
> > HW timer interrupts, since HW timer works still even if vcpu exists
> > from VM mode.
> >
> > Since HW timer pending interrupt can be set directly with CSR status
> > register, and pending HW timer interrupt checking is used in vcpu block
> > checking function, it is not necessary to switch to sw timer during
> > halt-polling. This patch adds preemption disabling in function
> > kvm_cpu_has_pending_timer, and removes SW timer switching in idle
> > instruction emulation function.
> >
> > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > ---
> >   arch/loongarch/kvm/exit.c  | 13 ++-----------
> >   arch/loongarch/kvm/timer.c | 13 ++++++++++---
> >   arch/loongarch/kvm/vcpu.c  |  9 ++++++++-
> >   3 files changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c
> > index ce8de3fa472c..e708a1786d6b 100644
> > --- a/arch/loongarch/kvm/exit.c
> > +++ b/arch/loongarch/kvm/exit.c
> > @@ -200,17 +200,8 @@ int kvm_emu_idle(struct kvm_vcpu *vcpu)
> >       ++vcpu->stat.idle_exits;
> >       trace_kvm_exit_idle(vcpu, KVM_TRACE_EXIT_IDLE);
> >
> > -     if (!kvm_arch_vcpu_runnable(vcpu)) {
> > -             /*
> > -              * Switch to the software timer before halt-polling/block=
ing as
> > -              * the guest's timer may be a break event for the vCPU, a=
nd the
> > -              * hypervisor timer runs only when the CPU is in guest mo=
de.
> > -              * Switch before halt-polling so that KVM recognizes an e=
xpired
> > -              * timer before blocking.
> > -              */
> > -             kvm_save_timer(vcpu);
> > -             kvm_vcpu_block(vcpu);
> > -     }
> > +     if (!kvm_arch_vcpu_runnable(vcpu))
> > +             kvm_vcpu_halt(vcpu);
> >
> >       return EMULATE_DONE;
> >   }
> > diff --git a/arch/loongarch/kvm/timer.c b/arch/loongarch/kvm/timer.c
> > index 284bf553fefe..437e960d8fdb 100644
> > --- a/arch/loongarch/kvm/timer.c
> > +++ b/arch/loongarch/kvm/timer.c
> > @@ -155,11 +155,18 @@ static void _kvm_save_timer(struct kvm_vcpu *vcpu=
)
> >                */
> >               hrtimer_cancel(&vcpu->arch.swtimer);
> >               hrtimer_start(&vcpu->arch.swtimer, expire, HRTIMER_MODE_A=
BS_PINNED);
> > -     } else
> > +     } else if (vcpu->stat.generic.blocking) {
> >               /*
> > -              * Inject timer interrupt so that hall polling can dectec=
t and exit
> > +              * Inject timer interrupt so that hall polling can dectec=
t and
> > +              * exit.
> > +              * VCPU is scheduled out already and sleeps in rcuwait qu=
eue and
> > +              * will not poll pending events again. kvm_queue_irq is n=
ot
> > +              * enough, hrtimer swtimer should be used here.
> >                */
> > -             kvm_queue_irq(vcpu, INT_TI);
> > +             expire =3D ktime_add_ns(ktime_get(), 10);  // 10ns is eno=
ugh here?
> > +             vcpu->arch.expire =3D expire;
> > +             hrtimer_start(&vcpu->arch.swtimer, expire, HRTIMER_MODE_A=
BS_PINNED);
> > +     }
> >   }
> >
> >   /*
> > diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> > index 73d0c2b9c1a5..42663a345bd1 100644
> > --- a/arch/loongarch/kvm/vcpu.c
> > +++ b/arch/loongarch/kvm/vcpu.c
> > @@ -187,8 +187,15 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu =
*vcpu,
> >
> >   int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> >   {
> > -     return kvm_pending_timer(vcpu) ||
> > +     int ret;
> > +
> > +     /* protect from TOD sync and vcpu_load/put */
> > +     preempt_disable();
> > +     ret =3D kvm_pending_timer(vcpu) ||
> >               kvm_read_hw_gcsr(LOONGARCH_CSR_ESTAT) & (1 << INT_TI);
> > +     preempt_enable();
> > +
> > +     return ret;
> >   }
> >
> >   int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
>
>