Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick

From: Andrew Jones <ajones@ventanamicro.com>
To: "Radim Krčmář" <rkrcmar@ventanamicro.com>
Cc: xiangwencheng <xiangwencheng@lanxincomputing.com>,
	anup@brainfault.org,  kvm-riscv@lists.infradead.org,
	kvm@vger.kernel.org, linux-riscv@lists.infradead.org,
	 linux-kernel@vger.kernel.org, atishp@atishpatra.org,
	paul.walmsley@sifive.com,  palmer@dabbelt.com,
	aou@eecs.berkeley.edu,
	 linux-riscv <linux-riscv-bounces@lists.infradead.org>
Subject: Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
Date: Thu, 20 Feb 2025 13:14:33 +0100	[thread overview]
Message-ID: <20250220-1581569f8559049399549cae@orel> (raw)
In-Reply-To: <D7X576NHG512.2HBBO3JLIA1JH@ventanamicro.com>

On Thu, Feb 20, 2025 at 09:50:06AM +0100, Radim Krčmář wrote:
> 2025-02-20T16:17:33+08:00, xiangwencheng <xiangwencheng@lanxincomputing.com>:
> >> From: "Andrew Jones"<ajones@ventanamicro.com>
> >> On Thu, Feb 20, 2025 at 03:12:58PM +0800, xiangwencheng wrote:
> >> > 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.
> 
> (Configure your mail client, so it doesn't add a newline between each
>  quoted line when replying.)
> 
> >> > So I still think is not necessary to call another kvm_vcpu_kick after writing to
> >> > VS_FILE.
> 
> So the kick wasn't there to mask some other bug, thanks.
> 
> >> 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.
> 
> The wakeup is asynchronous, and this would practically never avoid the
> host interrupt, but we'd do extra pointless work...
> I think it's much better just with the write.  (The wakeup would again
> make KVM look like it has a bug elsewhere.)

Ah yes, the wakeup is asynchronous. Just dropping the kick is the right
way to go then.

Thanks,
drew

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv