Re: [WIP Patch v2 09/14] KVM: Introduce KVM_CAP_MEMORY_FAULT_NOWAIT without implementation

[Sean Christopherson <seanjc@google.com>]

On Tue, Mar 21, 2023, Oliver Upton wrote:
> On Tue, Mar 21, 2023 at 07:50:35AM -0700, Sean Christopherson wrote:
> > On Mon, Mar 20, 2023, Oliver Upton wrote:
> > > On Fri, Mar 17, 2023 at 01:17:22PM -0700, Sean Christopherson wrote:
> > > > On Fri, Mar 17, 2023, Oliver Upton wrote:
> > > > > I'm not a fan of this architecture-specific dependency. Userspace is already
> > > > > explicitly opting in to this behavior by way of the memslot flag. These sort
> > > > > of exits are entirely orthogonal to the -EFAULT conversion earlier in the
> > > > > series.
> > > > 
> > > > Ya, yet another reason not to speculate on why KVM wasn't able to resolve a fault.
> > > 
> > > Regardless of what we name this memslot flag, we're already getting explicit
> > > opt-in from userspace for new behavior. There seems to be zero value in
> > > supporting memslot_flag && !MEMORY_FAULT_EXIT (i.e. returning EFAULT),
> > > so why even bother?
> > 
> > Because there are use cases for MEMORY_FAULT_EXIT beyond fast-only gup.
> 
> To be abundantly clear -- I have no issue with (nor care about) the other
> MEMORY_FAULT_EXIT changes. If we go the route of explicit user opt-in then
> that deserves its own distinct bit of UAPI. None of my objection pertains
> to the conversion of existing -EFAULT exits.
> 
> > We could have the memslot feature depend on the MEMORY_FAULT_EXIT capability,
> > but I don't see how that adds value for either KVM or userspace.
> 
> That is exactly what I want to avoid! My issue was the language here:
> 
>   +(*) NOTE: On x86, KVM_CAP_X86_MEMORY_FAULT_EXIT must be enabled for the
>   +KVM_MEMFAULT_REASON_ABSENT_MAPPING_reason: otherwise userspace will only receive
>   +a -EFAULT from KVM_RUN without any useful information.
> 
> Which sounds to me as though there are *two* UAPI bits for the whole fast-gup
> failed interaction (flip a bit in the CAP and set a bit on the memslot, but
> only for x86).

It won't be x86 only.  Anish's proposed patch has it as x86 specific, but I think
we're all in agreement that that is undesirable.  There will inevitably be per-arch
enabling and enumeration, e.g. to actually fill information and kick out to
userspace, but I don't see a sane way to avoid that since the common paths don't
have the vCPU (largely by design).

> What I'm asking for is this:
> 
>  1) A capability advertising MEMORY_FAULT_EXIT to userspace. Either usurp
>    EFAULT or require userspace to enable this capability to convert
>    _existing_ EFAULT exits to the new way of the world.
> 
>  2) A capability and a single memslot flag to enable the fast-gup-only
>    behavior (naming TBD). This does not depend on (1) in any way, i.e.
>    only setting (2) should still result in MEMORY_FAULT_EXITs when fast
>    gup fails. IOW, enabling (2) should always yield precise fault
>    information to userspace.

Ah, so 2.2, providing precise fault information on fast-gup-only failures, is the
biggest (only?) point of contention.

My objection to that behavior is that it's either going to annoyingly difficult to
get right in KVM, and even more annoying to maintain, or we'll end up with "fuzzy"
behavior that userspace will inevitably come to rely on, and then we'll be in a real
pickle.  E.g. if KVM sets the information without checking if gup() itself actually
failed, then KVM _might_ fill the info, depending on when KVM detects a problem.

Conversly, if KVM's contract is that it provides precise information if and only
if gup() fails, then KVM needs to precisely propagate back up the stack that gup()
failed.

To avoid spending more time going in circles, I propose we try to usurp -EFAULT
and convert all userspace-exits-from-KVM_RUN -EFAULT paths on x86 (as a guinea pig)
without requiring userspace to opt-in.  If that approach pans out, then this point
of contention goes away because 2.2 Just Works.