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

From: Anish Moorthy <amoorthy@google.com>
To: Sean Christopherson <seanjc@google.com>,
	David Matlack <dmatlack@google.com>
Cc: 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: Thu, 2 Nov 2023 12:14:42 -0700	[thread overview]
Message-ID: <CAF7b7mpzkjvBTybbaEUSp7iL3dVURVi+rDtkkojOcXAY=Bk9=g@mail.gmail.com> (raw)
In-Reply-To: <CAF7b7mrka8ASjp2UWWunCORjYbjUaOzSyzy_p-0KZXdrfOBOHQ@mail.gmail.com>

I'm going to squash this into patch 9: the fact that it's separated is
a holdover from when the memslot flag check lived in arch-specific
code.

Proposed commit message for the squashed commit

> KVM: Add KVM_CAP_EXIT_ON_MISSING which forbids page faults in stage-2 fault handlers

> Faulting in pages via the stage-2 fault handlers can be undesirable in
> the context of userfaultfd-based postcopy: contention for userfaultfd's
> internal wait queues can cause significant slowness when delivering
> faults to userspace. A performant alternative is to allow the stage-2
> fault handlers to, where they would currently page-fault on the
> userspace mappings, exit from KVM_RUN and deliver relevant information
> to userspace via KVM_CAP_MEMORY_FAULT_INFO. This approach avoids
> contention-related issues.

> Add, and mostly implement, a capability through which userspace can
> indicate to KVM (through the new KVM_MEM_EXIT_ON_MISSING memslot flag)
> that it should not fault in pages from the stage-2 fault handlers.

> The basic implementation strategy is to check the memslot flag from
> within __gfn_to_pfn_memslot() and override the "atomic" parameter
> accordingly. Some callers (such as kvm_vcpu_map()) must be able to opt
> out of this behavior, and do so by passing the new can_exit_on_missing
> parameter as false.

> No functional change intended: nothing sets KVM_MEM_EXIT_ON_MISSING.

One comment/question though- as I have the (sqaushed) patches/new
commit message written, I think it could mislead readers into thinking
that callers that pass can_exit_on_missing=false to
__gfn_to_pfn_memslot() *must* do so. But at least some of these cases,
such as the ones in the powerpc mmu, I think we're just punting on.

I see a few options here

1. Make all callers pass can_exit_on_missing=false, and leave the
=true update to the x86/arm64 specific "enable/annotate
KVM_EXIT_ON_MISSING" commits [my preference]

2. Make the powerpc callers pass can_exit_on_missing=true as well,
even though we're not adding KVM_CAP_EXIT_ON_MISSING for them

3. Add a disclaimer in the commit message that some cases where
can_exit_on_missing=false could become =true in the future

4. Things are fine as-is and I'm making a mountain out of a molehill

Any thoughts?