On Thu, Nov 02, 2023, Anish Moorthy wrote: > I'm going to squash this into patch 9: the fact that it's separated is > a holdover from when the memslot flag check lived in arch-specific > code. > > Proposed commit message for the squashed commit > > > KVM: Add KVM_CAP_EXIT_ON_MISSING which forbids page faults in stage-2 fault handlers Please don't use "forbids page faults". As I said earlier: : "Forbid fault" is rather nonsensical because a fault has already happened. The : confusion between "page fault VM-Exit" and "faulting in memory in the host" is : the main reason we wandered away from anything with "fault" in the name. The part that is being "forbidden" is not a "page fault", it's the act of faulting in a page. And "forbids" doesn't add any clarity to KVM_CAP_EXIT_ON_MISSING; if anything, it muddies the waters by making it sound like page faults somehow become fatal. Regarding the capability, stating the capability name is both unnecessary and uninteresting. The fact that there's a new capability is an implementation detail, it's in no way needed to understand the basic gist of the patch, which is basically *the* role of the shortlog. The key things to cover in the shortlog: * What does the feature do? Exits when an hva isn't fully mapped * Who controls the behavior? Userspace * How is the feature enabled? Memslot flag I would go with something like: KVM: Add memslot flag to let userspace force an exit on missing hva mappings > > Faulting in pages via the stage-2 fault handlers can be undesirable in > > the context of userfaultfd-based postcopy: contention for userfaultfd's > > internal wait queues can cause significant slowness when delivering > > faults to userspace. A performant alternative is to allow the stage-2 > > fault handlers to, where they would currently page-fault on the > > userspace mappings, exit from KVM_RUN and deliver relevant information > > to userspace via KVM_CAP_MEMORY_FAULT_INFO. This approach avoids > > contention-related issues. > > > Add, and mostly implement, a capability through which userspace can > > indicate to KVM (through the new KVM_MEM_EXIT_ON_MISSING memslot flag) > > that it should not fault in pages from the stage-2 fault handlers. > > > The basic implementation strategy is to check the memslot flag from > > within __gfn_to_pfn_memslot() and override the "atomic" parameter > > accordingly. Some callers (such as kvm_vcpu_map()) must be able to opt > > out of this behavior, and do so by passing the new can_exit_on_missing > > parameter as false. > > > No functional change intended: nothing sets KVM_MEM_EXIT_ON_MISSING. > > One comment/question though- as I have the (sqaushed) patches/new > commit message written, I think it could mislead readers into thinking > that callers that pass can_exit_on_missing=false to > __gfn_to_pfn_memslot() *must* do so. But at least some of these cases, > such as the ones in the powerpc mmu, I think we're just punting on. > > I see a few options here > > 1. Make all callers pass can_exit_on_missing=false, and leave the > =true update to the x86/arm64 specific "enable/annotate > KVM_EXIT_ON_MISSING" commits [my preference] This one. Nothing should pass %true until the caller fully supports KVM_MEM_EXIT_ON_MISSING. > 2. Make the powerpc callers pass can_exit_on_missing=true as well, > even though we're not adding KVM_CAP_EXIT_ON_MISSING for them > > 3. Add a disclaimer in the commit message that some cases where > can_exit_on_missing=false could become =true in the future > > 4. Things are fine as-is and I'm making a mountain out of a molehill > > Any thoughts?
