Re: [PATCH v5 11/17] KVM: x86: Enable KVM_CAP_USERFAULT_ON_MISSING

[Sean Christopherson <seanjc@google.com>]

On Fri, Sep 08, 2023, Anish Moorthy wrote:
> The relevant __gfn_to_pfn_memslot() calls in __kvm_faultin_pfn()
> already use MEMSLOT_ACCESS_NONATOMIC_MAY_UPGRADE.

--verbose

> Signed-off-by: Anish Moorthy <amoorthy@google.com>
> ---
>  Documentation/virt/kvm/api.rst | 2 +-
>  arch/x86/kvm/Kconfig           | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index c2eaacb6dc63..a74d721a18f6 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -7788,7 +7788,7 @@ error/annotated fault.
>  7.35 KVM_CAP_USERFAULT_ON_MISSING
>  ---------------------------------
>  
> -:Architectures: None
> +:Architectures: x86
>  :Returns: Informational only, -EINVAL on direct KVM_ENABLE_CAP.
>  
>  The presence of this capability indicates that userspace may set the
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index ed90f148140d..11d956f17a9d 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -49,6 +49,7 @@ config KVM
>  	select INTERVAL_TREE
>  	select HAVE_KVM_PM_NOTIFIER if PM
>  	select KVM_GENERIC_HARDWARE_ENABLING
> +        select HAVE_KVM_USERFAULT_ON_MISSING


Hmm, I vote to squash this patch with

  KVM: x86: Annotate -EFAULTs from kvm_handle_error_pfn()

or rather, squash that code into this patch.  Ditto for the ARM patches.

Since we're making KVM_EXIT_MEMORY_FAULT informational-only for flows that KVM
isn't committing to supporting, I think it makes sense to enable the arch specific
paths that *need* to return KVM_EXIT_MEMORY_FAULT at the same time as the feature
that adds the requirement.

E.g. without HAVE_KVM_USERFAULT_ON_MISSING support, per the contract we are creating,
it would be totally fine for KVM to not use KVM_EXIT_MEMORY_FAULT for the page
fault paths.  And that obviously has to be the case since KVM_CAP_MEMORY_FAULT_INFO
is introduced before the arch code is enabled.

But as of this path, KVM *must* return KVM_EXIT_MEMORY_FAULT, so we should make
it impossible to do a straight revert of that dependency.  That should also help
with the changelogs, e.g. it'll give you a prompt for why only kvm_handle_error_pfn()
is getting treatment.