Re: [WIP Patch v2 00/14] Avoiding slow get-user-pages via memory fault exit

[Sean Christopherson <seanjc@google.com>]

On Wed, Mar 15, 2023, Anish Moorthy wrote:
> Still unsure if needs conversion
> --------------------------------
> * __kvm_read_guest_atomic
>   The EFAULT might be propagated though FNAME(sync_page)?
> * kvm_write_guest_offset_cached (virt/kvm/kvm_main.c:3226)
> * __kvm_write_guest_page
>   Called from kvm_write_guest_offset_cached: if that needs change, this does too
> * kvm_write_guest_page
>   Two interesting paths:
>       - kvm_pv_clock_pairing returns a custom KVM_EFAULT error here
>         (arch/x86/kvm/x86.c:9578)
>       - kvm_write_guest_offset_cached returns this directly (so if that needs
>         change, this does too)
> * kvm_read_guest_offset_cached
>   I actually do see a path to userspace, but it's through hyper-v, which we've
>   said is out of scope for round 1.

To clarify: I didn't intend to make Hyper-V explicitly out-of-scope, rather Hyper-V
happened to be out-of-scope because the existing code suppresses -EFAULT.  I don't
think we should make any particular feature/area out-of-scope, as that will lead
to even more arbitrary behavior than we already have.

What I intended, and what I still think we should do, is limit the scope of the
capability to existing paths that return -EFAULT to userspace.  Trying to fix all
of the paths that suppress -EFAULT is going to be ridiculously difficult as so
much of the behavior is arguaby ABI, and there's no authoritative documentation
on what's supposed to happen.  I definitely would love to fix those paths in the
long term, but for the initial implementation/conversion, I think it makes sense
to punt on them, otherwise it'll take months/years to merge this code.

Back to the Hyper-V case, assuming you're referring to the use of kvm_hv_verify_vp_assist()
in nested_svm_vmrun(), that code is a mess.  KVM shouldn't inject a #GP and then
exit to userspace, e.g. the guest might see a spurious #GP if userspace fixes the
fault and resume the instruction.  And just a few lines below, KVM skips the
instruction if kvm_vcpu_map() returns -EFAULT.

As above, ideally that code would be converted to gracefully report the error,
but it's such a snafu that the easiest thing might be to change the "return ret;"
to "return 1;" until we fix all such KVM-on-HyperV code.