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

[Sean Christopherson <seanjc@google.com>]

On Fri, Mar 17, 2023, Anish Moorthy wrote:
> On Fri, Mar 17, 2023 at 1:17 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > And as I argued in the last version[*], I am _strongly_ opposed to KVM speculating
> > on why KVM is exiting to userspace.  I.e. KVM should not set a special flag if
> > the memslot has "fast only" behavior.  The only thing the flag should do is control
> > whether or not KVM tries slow paths, what KVM does in response to an unresolved
> > fault should be an orthogonal thing.
> 
> I'm guessing you would want changes to patch 10 of this series [*]
> then, right? Setting a bit/exit reason in kvm_run::memory_fault.flags
> depending on whether the failure originated from a "fast only" fault
> is... exactly what I'm doing :/ I'm not totally clear on your usages
> of the word "flag" above though, the "KVM should not set a special
> flag... the only thing *the* flag should do" part is throwing me off a
> bit. What I think you're saying is

Heh, the second "the flag" is referring to the memslot flag.  Rewriting the above:

  KVM should not set a special flag in kvm_run::memory_fault.flags ... the
  only thing KVM_MEM_FAST_FAULT_ONLY should do is ..."

> "KVM should not set a special bit in kvm_run::memory_fault.flags if
> the memslot has fast-only behavior. The only thing
> KVM_MEM_ABSENT_MAPPING_FAULT should do is..."
> 
> [1] https://lore.kernel.org/all/20230315021738.1151386-11-amoorthy@google.com/
> 
> On Fri, Mar 17, 2023 at 1:54 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Strictly speaking, if y'all buy my argument that the flag shouldn't control the
> > gup behavior, there won't be semantic differences for the memslot flag.  KVM will
> > (obviously) behavior differently if KVM_CAP_MEMORY_FAULT_EXIT is not set, but that
> > will hold true for x86 as well.  The only difference is that x86 will also support
> > an orthogonal flag that makes the fast-only memslot flag useful in practice.
> >
> > So yeah, there will be an arch dependency, but only because arch code needs to
> > actually handle perform the exit, and that's true no matter what.
> >
> > That said, there's zero reason to put X86 in the name.  Just add the capability
> > as KVM_CAP_MEMORY_FAULT_EXIT or whatever and mark it as x86 in the documentation.
> >
> > That said, there's zero reason to put X86 in the name.  Just add the capability
> > as KVM_CAP_MEMORY_FAULT_EXIT or whatever and mark it as x86 in the documentation.
> 
> Again, a little confused on your first "flag" usage here. I figure you
> can't mean the memslot flag because the whole point of that is to
> control the GUP behavior, but I'm not sure what else you'd be
> referring to.
> 
> Anyways the idea of having orthogonal features, one to -EFAULTing
> early before a slow path and another to transform/augment -EFAULTs
> into/with useful information does make sense to me. But I think the
> issue here is that we want the fast-only memslot flag to be useful on
> Arm as well, and with KVM_CAP_MEMORY_FAULT_NOWAIT written as it is now
> there is a semantic differences between x86 and Arm.

If and only if userspace enables the capability that transforms -EFAULT.

> I don't see a way to keep the two features here orthogonal on x86 and
> linked on arm without keeping that semantic difference. Perhaps the
> solution here is a bare-bones implementation of
> KVM_CAP_MEMORY_FAULT_EXIT for Arm? All that actually *needs* to be
> covered to resolve this difference is the one call site in
> user_mem_abort. since KVM_CAP_MEMORY_FAULT_EXIT will be allowed to
> have holes anyways.

As above, so long as userspace must opt into transforming -EFAULT, and can do
so independent of KVM_MEM_FAST_FAULT_ONLY (or whatever we call it), the behavior
of KVM_MEM_FAST_FAULT_ONLY itself is semantically identical across all
architectures.

KVM_MEM_FAST_FAULT_ONLY is obviously not very useful without precise information
about the failing address, but IMO that's not reason enough to tie the two
together.