On Wed, Nov 8, 2023 at 12:47 PM Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote: > > On Wed, Nov 8, 2023 at 12:10 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Wed, Nov 08, 2023, Peter Xu wrote: > > > On Wed, Nov 08, 2023 at 08:56:22AM -0800, David Matlack wrote: > > > > Thanks for the longer explanation. Yes kvm_read_guest() eventually > > > > calls __copy_from_user() which will trigger a page fault and > > > > UserfaultFD will notify userspace and wait for the page to become > > > > present. In the KVM-specific proposal I outlined, calling > > > > kvm_read_guest() will ultimately result in a check of the VM's present > > > > bitmap and KVM will nnotify userspace and wait for the page to become > > > > present if it's not, before calling __copy_from_user(). So I don't > > > > expect a KVM-specific solution to have any increased maintenance > > > > burden for VGIC (or any other widgets). > > > > > > The question is how to support modules that do not use kvm apis at all, > > > like vhost. I raised the question in my initial reply, too. > > > > > > I think if vhost is going to support gmemfd, it'll need new apis so maybe > > > there'll be a chance to take that into account, but I'm not 100% sure it'll > > > be the same complexity, also not sure if that's the plan even for CoCo. > > > > > > Or is anything like vhost not considered to be supported for gmemfd at all? > > > > vhost shouldn't require new APIs. To support vhost, guest_memfd would first need > > to support virtio for host userspace, i.e. would need to support .mmap(). At that > > point, all of the uaccess and gup() stuff in vhost should work without modification. > > Does this imply the need for some case-specific annotations for the > proposed KVM demand paging implementation to "catch" these accesses? > > IIUC this was one of the larger downsides to our internal > implementation, and something David had hoped to avoid in his RFC > proposal. Whereas, I think this is a point in UFFD's favor, where all > accesses "just work" with one centralized check in the fault handler > path. Yes this is definitely a point in UFFD's favor. The KVM-specific solution would require adding hooks in KVM in the core routines that KVM uses to access guest memory, or in the gfn-to-hva conversion routine. But the number of hooks needed is small, and this isn't exactly a new problem. KVM already needs similar hooks so that it can manually keep the dirty log up-to-date. Most of the pain we've experienced in our internal implementation is because (1) it's not upstream so it's easy for changes to slip in that don't go through the core routines (e.g. see record_steal_time() which hand-rolls its own guest memory access via user_access_begin/end()), and (2) we used to manually annotate all guest memory accesses instead of hooking the core routines. It's still possible for there to be correctness bugs (more likely than UFFD), but I think it's about as likely as KVM missing a dirty log update which I haven't noticed being a major source of upstream bugs or maintenance burden. But again, yes this is definitely a point in favor of a page-table-based approach. :)
