On Fri, Apr 26, 2024 at 12:17 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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.
