On Wed, Oct 4, 2023 at 6:52 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> 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
Alright, how about the following?
KVM: x86: Enable KVM_CAP_EXIT_ON_MISSING
__gfn_to_pfn_memslot(), used by the stage-2 fault handler, already
checks the memslot flag to avoid faulting in missing pages as required.
Therefore, enabling the capability is just a matter of selecting the kconfig
to allow the flag to actually be set.
> 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.
Should we really be concerned about people reverting the
KVM_CAP_MEMORY_FAULT_INFO commit(s) in this way? I see what you're
saying- but it also seems to me that KVM could add other things akin
to KVM_CAP_EXIT_ON_MISSING in the future, and then we end up in the
exact same situation. Sure the squash might make sense for the
specific commits in the series, but there's a general issue that isn't
really being solved.
Maybe I'm just letting the better be the enemy of the best, but I do
like the current separation/single-focus of the commits. That said,
the squash is nbd and I can go ahead if you're not convinced.
