On Wed, Nov 1, 2023 at 3:26 PM Anish Moorthy <amoorthy@xxxxxxxxxx> wrote: > > On Wed, Nov 1, 2023 at 3:03 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Wed, Nov 01, 2023, Anish Moorthy wrote: > > > On Wed, Oct 4, 2023 at 6:44 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > > > > > Eh, the shortlog basically says "do work" with a lot of fancy words. It really > > > > just boils down to: > > > > > > > > KVM: Let callers of __gfn_to_pfn_memslot() opt-out of USERFAULT_ON_MISSING > > > > > > Proposed commit message for v6: > > > > > > > KVM: Implement KVM_CAP EXIT_ON_MISSING by checking memslot flag in __gfn_to_pfn_memslot() > > > > > > > > When the slot flag is enabled, forbid __gfn_to_pfn_memslot() from > > > > faulting in pages for which mappings are absent. However, some callers of > > > > __gfn_to_pfn_memslot() (such as kvm_vcpu_map()) must be able to opt out > > > > of this behavior: allow doing so via the new can_exit_on_missing > > > > parameter. > > > > > > Although separately, I don't think the parameter should be named > > > can_exit_on_missing (or, as you suggested, can_do_userfault)- > > > __gfn_to_pfn_memslot() shouldn't know or care how its callers are > > > setting up KVM exits, after all. > > > > Why not? __gfn_to_pfn_memslot() gets passed all kinds of constraints, I don't > > see how "I can't handle exits to userspace" is any different. > > Well the thing is that __gfn_to_pfn_memslot() is orthogonal to KVM > exits. Its callers are just using it to try resolving a pfn, and what > they do with the results is up to them. > > Put more concretely, __gfn_to_pfn_memslot() has many callers of which > only two (the stage-2 fault handlers) actually use it to set up a KVM > exit- how does a parameter named "can_exit_on_missing" make sense to > its callers in general? "exit" is shorthand for "return_to_userspace" and all KVM code runs in the context of ioctls (which eventually return to userspace). So I think it makes sense in general. And FWIW I found the MEMSLOT_ACCESS_ enum hard to read and agree a boolean would be simpler.
