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

From: Sean Christopherson <seanjc@google.com>
To: Anish Moorthy <amoorthy@google.com>
Cc: David Matlack <dmatlack@google.com>,
	oliver.upton@linux.dev, kvm@vger.kernel.org,
	 kvmarm@lists.linux.dev, pbonzini@redhat.com, maz@kernel.org,
	 robert.hoo.linux@gmail.com, jthoughton@google.com,
	axelrasmussen@google.com,  peterx@redhat.com,
	nadav.amit@gmail.com, isaku.yamahata@gmail.com,
	 kconsul@linux.vnet.ibm.com
Subject: Re: [PATCH v5 10/17] KVM: Implement KVM_CAP_USERFAULT_ON_MISSING by atomizing __gfn_to_pfn_memslot() calls
Date: Wed, 1 Nov 2023 15:03:33 -0700	[thread overview]
Message-ID: <ZULLNdp6XKD6Twuc@google.com> (raw)
In-Reply-To: <CAF7b7mrka8ASjp2UWWunCORjYbjUaOzSyzy_p-0KZXdrfOBOHQ@mail.gmail.com>

On Wed, Nov 01, 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
> 
> Proposed commit message for v6:
> 
> > KVM: Implement KVM_CAP EXIT_ON_MISSING by checking memslot flag in __gfn_to_pfn_memslot()
> >
> > When the slot flag is enabled, forbid __gfn_to_pfn_memslot() from
> > faulting in pages for which mappings are absent. However, some callers of
> > __gfn_to_pfn_memslot() (such as kvm_vcpu_map()) must be able to opt out
> > of this behavior: allow doing so via the new can_exit_on_missing
> > parameter.
> 
> Although separately, I don't think the parameter should be named
> can_exit_on_missing (or, as you suggested, can_do_userfault)-
> __gfn_to_pfn_memslot() shouldn't know or care how its callers are
> setting up KVM exits, after all.

Why not?  __gfn_to_pfn_memslot() gets passed all kinds of constraints, I don't
see how "I can't handle exits to userspace" is any different.

> I think it makes sense to rename the new parameter and, for the same
> reasoning, the memslot flag to "forbid_fault_on_missing" and
> KVM_MEM_FORBID_FAULT_ON_MISSING respectively. Objections?

Yes.  I very strongly prefer KVM_MEM_EXIT_ON_MISSING.  As David pointed out, KVM
already has established nomenclature around exit, i.e. "exit on" should be quite
intuitive for most KVM developers.

"Forbid fault" is rather nonsensical because a fault has already happened.  The
confusion between "page fault VM-Exit" and "faulting in memory in the host" is
the main reason we wandered away from anything with "fault" in the name.

That said, I do agree that can_do_userfault doesn't work with KVM_MEM_EXIT_ON_MISSING.
Maybe just a boring can_exit_on_missing?