From: "Radim Krčmář" <rkrcmar@ventanamicro.com>
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>
Subject: Re: [PATCH] riscv: KVM: Remove unnecessary vcpu kick
Date: Wed, 19 Feb 2025 09:51:05 +0100 [thread overview]
Message-ID: <D7WALEFMK28X.13HQ0UL1S3NM5@ventanamicro.com> (raw)
In-Reply-To: <20250219015426.1939-1-xiangwencheng@lanxincomputing.com>
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.
next prev parent reply other threads:[~2025-02-19 8:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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ář [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D7WALEFMK28X.13HQ0UL1S3NM5@ventanamicro.com \
--to=rkrcmar@ventanamicro.com \
--cc=ajones@ventanamicro.com \
--cc=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atishp@atishpatra.org \
--cc=kvm-riscv@lists.infradead.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv-bounces@lists.infradead.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=xiangwencheng@lanxincomputing.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox