Message ID | 20240726235234.228822-1-seanjc@google.com |
---|---|
Headers | show |
Series | KVM: Stop grabbing references to PFNMAP'd pages | expand |
On 7/27/24 01:51, Sean Christopherson wrote: > arm64 folks, the first two patches are bug fixes, but I have very low > confidence that they are correct and/or desirable. If they are more or > less correct, I can post them separately if that'd make life easier. I > included them here to avoid conflicts, and because I'm pretty sure how > KVM deals with MTE tags vs. dirty logging will impact what APIs KVM needs > to provide to arch code. > > On to the series... The TL;DR is that I would like to get input on two > things: > > 1. Marking folios dirty/accessed only on the intial stage-2 page fault > 2. The new APIs for faulting, prefetching, and doing "lookups" on pfns Wow! Splitting out prefetching makes a lot of sense, as it's the only one with npages > 1 and it doesn't need all the complexity of hva_to_pfn(). I've left a comment on the lookup API, which is probably the only one that can be simplified further. The faulting API looks good as a first iteration. Code-wise, kvm_resolve_pfn() is probably unnecessary at the end of the series but I can see why you had to restrain yourself and declare it done. :) An interesting evolution of the API could be to pass a struct kvm_follow_pfn pointer to {,__}kvm_faultin_pfn() and __gfn_to_page() (the "constructors"); and on the other side to kvm_release_faultin_page() and kvm_release_page_*(). The struct kvm_follow_pfn could be embedded in the (x86) kvm_page_fault and (generic) kvm_host_map structs. But certainly not as part of this already huge work. Paolo
On Tue, Jul 30, 2024, Paolo Bonzini wrote: > An interesting evolution of the API could be to pass a struct kvm_follow_pfn > pointer to {,__}kvm_faultin_pfn() and __gfn_to_page() (the "constructors"); > and on the other side to kvm_release_faultin_page() and > kvm_release_page_*(). The struct kvm_follow_pfn could be embedded in the > (x86) kvm_page_fault and (generic) kvm_host_map structs. But certainly not > as part of this already huge work. For kvm_faultin_pfn(), my hope/dream is to make kvm_page_fault a common struct, with an arch member (a la kvm_vcpu), and get to something like: static int arch_page_fault_handler(...) { struct kvm_page_fault fault = { <const common stuff>, .arch.xxx = <arch stuff>, }; <arch code> r = kvm_faultin_pfn(); ... } In theory, that would allow moving the kvm->mmu_invalidate_seq handling, memslot lookup, etc. into kvm_faultin_pfn(), or maybe another helper that is invoked to setup the fault structure. I.e. it would give us a way to drive convergence for at least some of the fault handling logic, without having to tackle gory arch details, at least not right away.
Sean Christopherson <seanjc@google.com> writes: > arm64 folks, the first two patches are bug fixes, but I have very low > confidence that they are correct and/or desirable. If they are more or > less correct, I can post them separately if that'd make life easier. I > included them here to avoid conflicts, and because I'm pretty sure how > KVM deals with MTE tags vs. dirty logging will impact what APIs KVM needs > to provide to arch code. > > On to the series... The TL;DR is that I would like to get input on two > things: > > 1. Marking folios dirty/accessed only on the intial stage-2 page fault > 2. The new APIs for faulting, prefetching, and doing "lookups" on > pfns I've finally managed to get virtio-vulkan working on my Arm64 devbox with an AMD graphics card plugged into the PCI. I'm confident that the graphics path is using the discrete card memory (as it has been mapped as device memory with alignment handlers to deal with the broken Altra PCI). However aside from running graphics workloads in KVM guests is their anything else I can check to see things are behaving as expected? The predecessor series did break launching some KVM guests on my x86 system but with this series launching guests works fine and I haven't noticed any weirdness. So for those caveats you can certainly have a: Tested-by: Alex Bennée <alex.bennee@linaro.org> However if there is anything else I can do to further stress test this code do let me know.