* [PATCH] riscv: KVM: Remove unnecessary vcpu kick
@ 2025-02-19 1:54 BillXiang
2025-02-19 8:36 ` Andrew Jones
2025-02-19 8:51 ` Radim Krčmář
0 siblings, 2 replies; 10+ messages in thread
From: BillXiang @ 2025-02-19 1:54 UTC (permalink / raw)
To: anup
Cc: ajones, xiangwencheng, kvm-riscv, kvm, linux-riscv, linux-kernel,
atishp, paul.walmsley, palmer, aou
Thank you Andrew Jones, forgive my errors in the last email.
I'm wondering whether it's necessary to kick the virtual hart
after writing to the vsfile of IMSIC.
From my understanding, writing to the vsfile should directly
forward the interrupt as MSI to the virtual hart. This means that
an additional kick should not be necessary, as it would cause the
vCPU to exit unnecessarily and potentially degrade performance.
I've tested this behavior in QEMU, and it seems to work perfectly
fine without the extra kick.
Would appreciate any insights or confirmation on this!
Best regards.
Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
---
arch/riscv/kvm/aia_imsic.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index a8085cd8215e..29ef9c2133a9 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -974,7 +974,6 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,
if (imsic->vsfile_cpu >= 0) {
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);
--
2.46.2
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-19 1:54 [PATCH] riscv: KVM: Remove unnecessary vcpu kick BillXiang
@ 2025-02-19 8:36 ` Andrew Jones
2025-02-19 8:51 ` Radim Krčmář
1 sibling, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-02-19 8:36 UTC (permalink / raw)
To: BillXiang
Cc: anup, kvm-riscv, kvm, linux-riscv, linux-kernel, atishp,
paul.walmsley, palmer, aou
On Wed, Feb 19, 2025 at 09:54:26AM +0800, BillXiang wrote:
> Thank you Andrew Jones, forgive my errors in the last email.
From here down is all exactly the same as your first email, which I
already completely replied to.
> I'm wondering whether it's necessary to kick the virtual hart
> after writing to the vsfile of IMSIC.
> From my understanding, writing to the vsfile should directly
> forward the interrupt as MSI to the virtual hart. This means that
> an additional kick should not be necessary, as it would cause the
> vCPU to exit unnecessarily and potentially degrade performance.
> I've tested this behavior in QEMU, and it seems to work perfectly
> fine without the extra kick.
> Would appreciate any insights or confirmation on this!
> Best regards.
>
> Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
> ---
> arch/riscv/kvm/aia_imsic.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> index a8085cd8215e..29ef9c2133a9 100644
> --- a/arch/riscv/kvm/aia_imsic.c
> +++ b/arch/riscv/kvm/aia_imsic.c
> @@ -974,7 +974,6 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,
>
> if (imsic->vsfile_cpu >= 0) {
> 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);
> --
> 2.46.2
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-19 1:54 [PATCH] riscv: KVM: Remove unnecessary vcpu kick BillXiang
2025-02-19 8:36 ` Andrew Jones
@ 2025-02-19 8:51 ` Radim Krčmář
2025-02-20 7:12 ` xiangwencheng
1 sibling, 1 reply; 10+ messages in thread
From: Radim Krčmář @ 2025-02-19 8:51 UTC (permalink / raw)
To: BillXiang, anup
Cc: ajones, kvm-riscv, kvm, linux-riscv, linux-kernel, atishp,
paul.walmsley, palmer, aou, linux-riscv
2025-02-19T09:54:26+08:00, BillXiang <xiangwencheng@lanxincomputing.com>:
> Thank you Andrew Jones, forgive my errors in the last email.
> I'm wondering whether it's necessary to kick the virtual hart
> after writing to the vsfile of IMSIC.
> From my understanding, writing to the vsfile should directly
> forward the interrupt as MSI to the virtual hart. This means that
> an additional kick should not be necessary, as it would cause the
> vCPU to exit unnecessarily and potentially degrade performance.
Andrew proposed to avoid the exit overhead, but do a wakeup if the VCPU
is "sleeping". I talked with Andrew and thought so as well, but now I
agree with you that we shouldn't have anything extra here.
Direct MSIs from IOMMU or other harts won't perform anything afterwards,
so what you want to do correct and KVM has to properly handle the memory
write alone.
> I've tested this behavior in QEMU, and it seems to work perfectly
> fine without the extra kick.
If the rest of KVM behaves correctly is a different question.
A mistake might result in a very rare race condition, so it's better to
do verification rather than generic testing.
For example, is `vsfile_cpu >= 0` the right condition for using direct
interrupts?
I don't see KVM setting vsfile_cpu to -1 before descheduling after
emulating WFI, which could cause a bug as a MSI would never cause a wake
up. It might still look like it works, because something else could be
waking the VCPU up and then the VCPU would notice this MSI as well.
Please note that I didn't actualy verify the KVM code, so it can be
correct, I just used this to give you an example of what can go wrong
without being able to see it in testing.
I would like to know if KVM needs fixing before this change is accepted.
(It could make bad things worse.)
> Would appreciate any insights or confirmation on this!
Your patch is not acceptable because of its commit message, though.
Please look again at the document that Andrew posted and always reply
the previous thread if you do not send a new patch version.
The commit message should be on point.
Please avoid extraneous information that won't help anyone reading the
commit. Greeting and commentary can go below the "---" line.
(And possibly above a "---8<---" line, although that is not official and
may cause issues with some maintainers.)
Thanks.
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-19 8:51 ` Radim Krčmář
@ 2025-02-20 7:12 ` xiangwencheng
2025-02-20 8:01 ` Andrew Jones
0 siblings, 1 reply; 10+ messages in thread
From: xiangwencheng @ 2025-02-20 7:12 UTC (permalink / raw)
To: Radim Krčmář
Cc: anup, ajones, kvm-riscv, kvm, linux-riscv, linux-kernel, atishp,
paul.walmsley, palmer, aou, linux-riscv
> From: "Radim Krčmář"<rkrcmar@ventanamicro.com>
> Date: Wed, Feb 19, 2025, 16:51
> Subject: Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
> To: "BillXiang"<xiangwencheng@lanxincomputing.com>, <anup@brainfault.org>
> Cc: <ajones@ventanamicro.com>, <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>
> 2025-02-19T09:54:26+08:00, BillXiang <xiangwencheng@lanxincomputing.com>:
> > Thank you Andrew Jones, forgive my errors in the last email.
> > I'm wondering whether it's necessary to kick the virtual hart
> > after writing to the vsfile of IMSIC.
> > From my understanding, writing to the vsfile should directly
> > forward the interrupt as MSI to the virtual hart. This means that
> > an additional kick should not be necessary, as it would cause the
> > vCPU to exit unnecessarily and potentially degrade performance.
>
> Andrew proposed to avoid the exit overhead, but do a wakeup if the VCPU
> is "sleeping". I talked with Andrew and thought so as well, but now I
> agree with you that we shouldn't have anything extra here.
>
> Direct MSIs from IOMMU or other harts won't perform anything afterwards,
> so what you want to do correct and KVM has to properly handle the memory
> write alone.
>
> > I've tested this behavior in QEMU, and it seems to work perfectly
> > fine without the extra kick.
>
> If the rest of KVM behaves correctly is a different question.
> A mistake might result in a very rare race condition, so it's better to
> do verification rather than generic testing.
>
> For example, is `vsfile_cpu >= 0` the right condition for using direct
> interrupts?
>
> I don't see KVM setting vsfile_cpu to -1 before descheduling after
It's not necessary to set vsfile_cpu to -1 as it doesn't release it, and
the vsfile still belongs to the vCPU after WFI.
> emulating WFI, which could cause a bug as a MSI would never cause a wake
> up. It might still look like it works, because something else could be
> waking the VCPU up and then the VCPU would notice this MSI as well.
>
> Please note that I didn't actualy verify the KVM code, so it can be
> correct, I just used this to give you an example of what can go wrong
> without being able to see it in testing.
>
> I would like to know if KVM needs fixing before this change is accepted.
> (It could make bad things worse.)
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
>
> > Would appreciate any insights or confirmation on this!
>
> Your patch is not acceptable because of its commit message, though.
> Please look again at the document that Andrew posted and always reply
> the previous thread if you do not send a new patch version.
>
> The commit message should be on point.
> Please avoid extraneous information that won't help anyone reading the
> commit. Greeting and commentary can go below the "---" line.
> (And possibly above a "---8<---" line, although that is not official and
> may cause issues with some maintainers.)
>
> Thanks.
>
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-20 7:12 ` xiangwencheng
@ 2025-02-20 8:01 ` Andrew Jones
2025-02-20 8:17 ` xiangwencheng
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2025-02-20 8:01 UTC (permalink / raw)
To: xiangwencheng
Cc: Radim Krčmář, anup, kvm-riscv, kvm, linux-riscv,
linux-kernel, atishp, paul.walmsley, palmer, aou, linux-riscv
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.
Thanks,
drew
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-20 8:01 ` Andrew Jones
@ 2025-02-20 8:17 ` xiangwencheng
2025-02-20 8:50 ` Radim Krčmář
0 siblings, 1 reply; 10+ messages in thread
From: xiangwencheng @ 2025-02-20 8:17 UTC (permalink / raw)
To: Andrew Jones
Cc: Radim Krčmář, anup, kvm-riscv, kvm, linux-riscv,
linux-kernel, atishp, paul.walmsley, palmer, aou, linux-riscv
> From: "Andrew Jones"<ajones@ventanamicro.com>
> Date: Thu, Feb 20, 2025, 16:01
> Subject: Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
> To: "xiangwencheng"<xiangwencheng@lanxincomputing.com>
> Cc: "Radim Krčmář"<rkrcmar@ventanamicro.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>
> 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
>
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-20 8:17 ` xiangwencheng
@ 2025-02-20 8:50 ` Radim Krčmář
2025-02-20 12:14 ` Andrew Jones
0 siblings, 1 reply; 10+ messages in thread
From: Radim Krčmář @ 2025-02-20 8:50 UTC (permalink / raw)
To: xiangwencheng, Andrew Jones
Cc: anup, kvm-riscv, kvm, linux-riscv, linux-kernel, atishp,
paul.walmsley, palmer, aou, linux-riscv
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.)
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-20 8:50 ` Radim Krčmář
@ 2025-02-20 12:14 ` Andrew Jones
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-02-20 12:14 UTC (permalink / raw)
To: Radim Krčmář
Cc: xiangwencheng, anup, kvm-riscv, kvm, linux-riscv, linux-kernel,
atishp, paul.walmsley, palmer, aou, linux-riscv
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] riscv: KVM: Remove unnecessary vcpu kick
@ 2025-02-18 8:00 项文成
2025-02-18 17:48 ` Andrew Jones
0 siblings, 1 reply; 10+ messages in thread
From: 项文成 @ 2025-02-18 8:00 UTC (permalink / raw)
To: kvm-riscv@lists.infradead.org
Cc: anup@brainfault.org, atishp@atishpatra.org,
paul.walmsley@sifive.com, palmer@dabbelt.com,
aou@eecs.berkeley.edu, kvm@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
From 30dd00f6886119ecc5c39b6b88f8617a57e598fc Mon Sep 17 00:00:00 2001
From: BillXiang <xiangwencheng@lanxincomputing.com>
Date: Tue, 18 Feb 2025 15:45:52 +0800
Subject: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
Hello everyone,
I'm wondering whether it's necessary to kick the virtual hart
after writing to the vsfile of IMSIC.
From my understanding, writing to the vsfile should directly
forward the interrupt as MSI to the virtual hart. This means that
an additional kick should not be necessary, as it would cause the
vCPU to exit unnecessarily and potentially degrade performance.
I've tested this behavior in QEMU, and it seems to work perfectly
fine without the extra kick.
Would appreciate any insights or confirmation on this!
Best regards.
Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
---
arch/riscv/kvm/aia_imsic.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index a8085cd8215e..29ef9c2133a9 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -974,7 +974,6 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,
if (imsic->vsfile_cpu >= 0) {
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);
--
2.46.2
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
2025-02-18 8:00 项文成
@ 2025-02-18 17:48 ` Andrew Jones
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-02-18 17:48 UTC (permalink / raw)
To: 项文成
Cc: kvm-riscv@lists.infradead.org, anup@brainfault.org,
atishp@atishpatra.org, paul.walmsley@sifive.com,
palmer@dabbelt.com, aou@eecs.berkeley.edu, kvm@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
On Tue, Feb 18, 2025 at 04:00:24PM +0800, 项文成 wrote:
> From 30dd00f6886119ecc5c39b6b88f8617a57e598fc Mon Sep 17 00:00:00 2001
> From: BillXiang <xiangwencheng@lanxincomputing.com>
> Date: Tue, 18 Feb 2025 15:45:52 +0800
> Subject: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
>
> Hello everyone,
> I'm wondering whether it's necessary to kick the virtual hart
> after writing to the vsfile of IMSIC.
> From my understanding, writing to the vsfile should directly
> forward the interrupt as MSI to the virtual hart. This means that
> an additional kick should not be necessary, as it would cause the
> vCPU to exit unnecessarily and potentially degrade performance.
> I've tested this behavior in QEMU, and it seems to work perfectly
> fine without the extra kick.
> Would appreciate any insights or confirmation on this!
> Best regards.
The above should be in a cover letter so the commit message can
be written following the guidelines of [1]
[1] Documentation/process/submitting-patches.rst
>
> Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
> ---
> arch/riscv/kvm/aia_imsic.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> index a8085cd8215e..29ef9c2133a9 100644
> --- a/arch/riscv/kvm/aia_imsic.c
> +++ b/arch/riscv/kvm/aia_imsic.c
> @@ -974,7 +974,6 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,
>
> if (imsic->vsfile_cpu >= 0) {
> writel(iid, imsic->vsfile_va + IMSIC_MMIO_SETIPNUM_LE);
> - kvm_vcpu_kick(vcpu);
We can't completely remove the kick, but we could replace it with a
kvm_vcpu_wake_up().
There's also a kick in kvm_riscv_vcpu_vstimer_expired() which could be a
kvm_vcpu_wake_up() when hideleg has IRQ_VS_TIMER set (which it currently
always does).
Thanks,
drew
> } else {
> eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];
> set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);
> --
> 2.46.2
>
> --
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-02-20 12:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-19 1:54 [PATCH] riscv: KVM: Remove unnecessary vcpu kick BillXiang
2025-02-19 8:36 ` Andrew Jones
2025-02-19 8:51 ` Radim Krčmář
2025-02-20 7:12 ` xiangwencheng
2025-02-20 8:01 ` Andrew Jones
2025-02-20 8:17 ` xiangwencheng
2025-02-20 8:50 ` Radim Krčmář
2025-02-20 12:14 ` Andrew Jones
-- strict thread matches above, loose matches on Subject: below --
2025-02-18 8:00 项文成
2025-02-18 17:48 ` Andrew Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).