From: Kele Huang <kele@cs.columbia.edu>
To: Sean Christopherson <seanjc@google.com>
Cc: Zide Chen <zide.chen@intel.com>,
pbonzini@redhat.com, kvm@vger.kernel.org
Subject: Re: [1/1] KVM: restrict kvm_gfn_to_hva_cache_init() to only accept address ranges within one page
Date: Sat, 27 Apr 2024 00:27:33 -0400 [thread overview]
Message-ID: <CAOfLF_L+bxOo4kK5H6WAUcOeTu5wFiU57UtR5qmr1rQBT5mAfA@mail.gmail.com> (raw)
In-Reply-To: <ZivTmpMmeuIShbcC@google.com>
On Fri, Apr 26, 2024 at 12:17 PM Sean Christopherson <seanjc@google.com> wrote:
> Please don't top post. https://people.kernel.org/tglx/notes-about-netiquette
Thanks for the tip!
> The exports from kvm.ko are intended for use only by KVM itself, e.g. by
> kvm-intel.ko and kvm-amd.ko on x86. Anyone trying to use KVM's exports in random
> drivers is in for a world of hurt, as there many, many subtleties throughout KVM
> that bleed all over the exports.
>
> It's gross that KVM has "internal" exports, and we have line of sight to removing
> them for most architectures, including x86, but that isn't the easiest of changes.
>
> If there is a real problem with in-tree upstream KVM, then we'll fix it, but
> changing the behavior of KVM functions just because they are exported isn't going
> to happen.
Yes, I agree with this.
> > For example, function kvm_lapic_set_vapic_addr()
> > called `kvm_gfn_to_hva_cache_init()` and simply assumes the cache is
> > successfully initialized by checking the return value.
>
> I don't follow the concern here. It's userspace's responsibility to provide a
> page-aligned address. KVM needs to not explode on an unaligned address, but
> ensuring that KVM can actually use the fast path isn't KVM's problem.
Yes, you are right. For the cross page address range, the return value 0 does
not mean the cache is successfully initialized, but the following read and
write to the corresponding guest memory would still check if the ghc->memslot
is nullified before directly using the cached hva address.
> > My thought is there probably should be another function to provide correct
> > cross page cache initialization but I am not sure if this is really needed.
>
> If there were a legitimate use case where it was truly valuable, then we could
> add that functionality. But all of the usage in KVM either deals with assets
> that are exactly one page in size and must be page aligned, or with assets that
> are userspace or guest controlled and not worth optimizing for page splits because
> a well-behavior userspace/guest should ensure the asset doesn't split a page.
Yes, I agree.
> > Nevertheless, I think we could at least make the existing function
> > more accurate?
>
> More accurate with respect to what?
The correctness of the whole gfn to hva cache init logic looks good to me now.
Thanks for the clarifications from all of you.
Although, it is a bit not straightforward to me because it needs to
call designed
functions to do the guest memory read and write even though people assume
the cache is initialized, or need to do the ghc->memslot checking manually
before using the fast path.
next prev parent reply other threads:[~2024-04-27 4:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-23 2:49 [1/1] KVM: restrict kvm_gfn_to_hva_cache_init() to only accept address ranges within one page Kele Huang
2024-04-26 1:16 ` Chen, Zide
[not found] ` <CAOfLF_L2UgSUyUsbiBDhLPskt2xLWujy1GBAhpcWzi2i3brAww@mail.gmail.com>
2024-04-26 4:18 ` Kele Huang
2024-04-26 16:17 ` Sean Christopherson
2024-04-27 4:27 ` Kele Huang [this message]
2024-04-29 20:13 ` Sean Christopherson
2024-04-30 19:29 ` Kele Huang
2024-04-26 6:25 ` Christoph Hellwig
2024-04-27 3:50 ` Kele Huang
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=CAOfLF_L+bxOo4kK5H6WAUcOeTu5wFiU57UtR5qmr1rQBT5mAfA@mail.gmail.com \
--to=kele@cs.columbia.edu \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=zide.chen@intel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).