[PATCH] riscv: KVM: Remove unnecessary vcpu kick

[xiangwencheng]

> From: "Andrew Jones"<ajones at ventanamicro.com>
> Date:  Thu, Feb 20, 2025, 16:01
> Subject:  Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
> To: "xiangwencheng"<xiangwencheng at lanxincomputing.com>
> Cc: "Radim Krčmář"<rkrcmar at ventanamicro.com>, <anup at brainfault.org>, <kvm-riscv at lists.infradead.org>, <kvm at vger.kernel.org>, <linux-riscv at lists.infradead.org>, <linux-kernel at vger.kernel.org>, <atishp at atishpatra.org>, <paul.walmsley at sifive.com>, <palmer at dabbelt.com>, <aou at eecs.berkeley.edu>, "linux-riscv"<linux-riscv-bounces at lists.infradead.org>
> On Thu, Feb 20, 2025 at 03:12:58PM +0800, xiangwencheng wrote:

> ...

> > As "KVM:  WFI wake-up using IMSIC VS-files" that described in [1], writing to 

> > VS-FILE will wake up vCPU.

> > 

> > KVM has also handled the situation of WFI. Here is the WFI emulation process:

> > kvm_riscv_vcpu_exit 

> >     -> kvm_riscv_vcpu_virtual_insn 

> >          -> system_opcode_insn 

> >               -> wfi_insn 

> >                  -> kvm_riscv_vcpu_wfi

> >                      -> kvm_vcpu_halt

> >                          -> kvm_vcpu_block

> >                              -> kvm_arch_vcpu_blocking

> >                                    -> kvm_riscv_aia_wakeon_hgei

> >                                          -> csr_set(CSR_HGEIE, BIT(hgei));

> >                              -> set_current_state(TASK_INTERRUPTIBLE);

> >                              -> schedule

> > 

> > In kvm_arch_vcpu_blocking it will enable guest external interrupt, which

> > means wirting to VS_FILE will cause an interrupt. And the interrupt handler

> > hgei_interrupt which is setted in aia_hgei_init will finally call kvm_vcpu_kick

> > to wake up vCPU.

> > 

> > So I still think is not necessary to call another kvm_vcpu_kick after writing to

> > VS_FILE.

> > 

> > Waiting for more info. Thanks.

> > 

> > [1]  https://kvm-forum.qemu.org/2022/AIA_Virtualization_in_KVM_RISCV_final.pdf

> >

> 
> Right, we don't need anything since hgei_interrupt() kicks for us, but if

> we do

> 
> @@ -973,8 +973,8 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,

>         read_lock_irqsave(&imsic->vsfile_lock, flags);

> 
>         if (imsic->vsfile_cpu >= 0) {

> +               kvm_vcpu_wake_up(vcpu);

>                 writel(iid, imsic->vsfile_va + IMSIC_MMIO_SETIPNUM_LE);

> -               kvm_vcpu_kick(vcpu);

>         } else {

>                 eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];

>                 set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);

> 
> then we should be able to avoid taking a host interrupt.

But it may schedule again in the for(;;) loop of kvm_vcpu_block after kvm_vcpu_wake_up but 
before the write of vsfile, and we will still get a host interrupt.
@@ -3573,6 +3573,8 @@ bool kvm_vcpu_block(struct kvm_vcpu *vcpu)
        for (;;) {
                set_current_state(TASK_INTERRUPTIBLE);

+               // Here will not break before the write of vsfile,
+               // and then we will schedule again.
                if (kvm_vcpu_check_block(vcpu) < 0)
                        break;

                waited = true;
                schedule();
        }

Thanks 

> 
> Thanks,

> drew
>