On Tue, Jan 30, 2024 at 4:26 PM James Houghton <jthoughton@xxxxxxxxxx> wrote:
>
> Feel free to add:
>
> Reviewed-by: James Houghton <jthoughton@xxxxxxxxxx>
> If we include KVM_MEM_GUEST_MEMFD here, we should point the reader to
> KVM_SET_USER_MEMORY_REGION2 and explain that using
> KVM_SET_USER_MEMORY_REGION with this flag will always fail.
Done and done (I've split the guest memfd doc update off into its own
commit too).
> > @@ -3070,6 +3074,15 @@ kvm_pfn_t __gfn_to_pfn_memslot(const struct kvm_memory_slot *slot, gfn_t gfn,
> > writable = NULL;
> > }
> >
> > + if (!atomic && can_exit_on_missing
> > + && kvm_is_slot_exit_on_missing(slot)) {
> > + atomic = true;
> > + if (async) {
> > + *async = false;
> > + async = NULL;
> > + }
> > + }
> > +
>
> Perhaps we should have a comment for this? Maybe something like: "If
> we want to exit-on-missing, we want to bail out if fast GUP fails, and
> we do not want to go into slow GUP. Setting atomic=true does exactly
> this."
I was going to push back on the use of "we" but I see that it's all
over kvm_main.c :).
I agree that a comment would be good, but isn't the "fast GUP only iff
atomic=true" statement a tautology? That's an actual question, my
memory's fuzzy. What about
> When the slot is exit-on-missing (and when we should respect that)
> set atomic=true to prevent GUP from faulting in the userspace
> mappings.
instead?
