Re: [PATCH v5 10/17] KVM: Implement KVM_CAP_USERFAULT_ON_MISSING by atomizing __gfn_to_pfn_memslot() calls

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 05, 2023, Anish Moorthy wrote:
> On Wed, Oct 4, 2023 at 6:44 PM Sean Christopherson <seanjc@google.com> 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
> >
> > On Fri, Sep 08, 2023, Anish Moorthy wrote:
> > > Change the "atomic" parameter of __gfn_to_pfn_memslot() to an enum which
> >
> > I've pushed back on more booleans multiple times, but IMO this is even worse.
> > E.g. what does an "upgrade" to atomic even mean?
> 
> Oh, that bad huh? Based on what you mentioned earlier about some
> callers of __gfn_to_pfn_memslot() needing to opt out of the memslot
> flag, it seemed like there were basically three ways (wrt to @atomic)
> that function needed to be called
> 
> 1. With atomic = true
> 2. With atomic = false, and some way to make sure that's respected
> whatever the memslot flag says
> 3. With atomic = false, but respecting the memslot flag (ie, changing
> to atomic = true when it's set).
> 
> An "upgrade" in this context was meant to describe case 3 (when the
> memslot flag is set). Anyways despite terminology issues, the idea of
> an enum encapsulating those three cases seems like, essentially, the
> right thing to do. Though of course, let me know if you disagree :)

The problem is that the three possibilities aren't directly related.  The existing
use of atomic truly means "this call can't sleep/block".  The userfault-on-missing
case has nothing to do with sleeping being illegal, the behavior of @atomic just
happens to align exactly with what is needed *today*.

E.g. if there was a flavor of gup() that could fault in memory without sleeping,
that could be used for the @atomic case but not the userfault-on-missing case.
I doubt such a variation will ever exist, but "that probably won't happen" isn't
a good reason to conflate the two things.

> > Since we have line of sight to getting out of boolean hell via David's series,
> > just bite the bullet for now.  Deciphering the callers will suck, but not really
> > anymore than it already sucks.
> 
> Sorry, what exactly are you suggesting via "bite the bullet" here?

Add another boolean, the "bool can_do_userfault" from the diff that got snipped.