Hi Anish,
Sorry to get back to you so late. :) I was hoping others would provide
more feedback, but I have a little bit to give anyway. Overall the
series looks good to me.
On Thu, Nov 9, 2023 at 1:03 PM Anish Moorthy <amoorthy@xxxxxxxxxx> wrote:
>
> Prevent the stage-2 fault handler from faulting in pages when
> KVM_MEM_EXIT_ON_MISSING is set by allowing its __gfn_to_pfn_memslot()
> calls to check the memslot flag.
>
> To actually make that behavior useful, prepare a KVM_EXIT_MEMORY_FAULT
> when the stage-2 handler cannot resolve the pfn for a fault. With
> KVM_MEM_EXIT_ON_MISSING enabled this effects the delivery of stage-2
> faults as vCPU exits, which userspace can attempt to resolve without
> terminating the guest.
>
> Delivering stage-2 faults to userspace in this way sidesteps the
> significant scalabiliy issues associated with using userfaultfd for the
> same purpose.
>
> Signed-off-by: Anish Moorthy <amoorthy@xxxxxxxxxx>
> ---
> Documentation/virt/kvm/api.rst | 2 +-
> arch/arm64/kvm/Kconfig | 1 +
> arch/arm64/kvm/mmu.c | 7 +++++--
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index fd87bbfbfdf2..67fcb9dbe855 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -8068,7 +8068,7 @@ See KVM_EXIT_MEMORY_FAULT for more information.
> 7.35 KVM_CAP_EXIT_ON_MISSING
> ----------------------------
>
> -:Architectures: x86
> +:Architectures: x86, arm64
> :Returns: Informational only, -EINVAL on direct KVM_ENABLE_CAP.
>
> The presence of this capability indicates that userspace may set the
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 1a777715199f..d6fae31f7e1a 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -43,6 +43,7 @@ menuconfig KVM
> select GUEST_PERF_EVENTS if PERF_EVENTS
> select INTERVAL_TREE
> select XARRAY_MULTI
> + select HAVE_KVM_EXIT_ON_MISSING
> help
> Support hosting virtualized guest machines.
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 13066a6fdfff..3b9fb80672ac 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1486,13 +1486,16 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> mmap_read_unlock(current->mm);
>
> pfn = __gfn_to_pfn_memslot(memslot, gfn, false, false, NULL,
> - write_fault, &writable, false, NULL);
> + write_fault, &writable, true, NULL);
> if (pfn == KVM_PFN_ERR_HWPOISON) {
> kvm_send_hwpoison_signal(hva, vma_shift);
> return 0;
> }
> - if (is_error_noslot_pfn(pfn))
> + if (is_error_noslot_pfn(pfn)) {
> + kvm_prepare_memory_fault_exit(vcpu, gfn * PAGE_SIZE, PAGE_SIZE,
> + write_fault, exec_fault, false);
I think that either (1) we move this kvm_prepare_memory_fault_exit
logic into the previous patch[1], or (2) we merge this patch with the
previous one. IIUC, we can only advertise KVM_CAP_MEMORY_FAULT_INFO on
arm64 if this logic is present.
As for the changelog in the previous patch[1], if you leave it
unmerged with this one, something like "Enable
KVM_CAP_MEMORY_FAULT_INFO to make KVM_CAP_EXIT_ON_MISSING useful, as
without it, userspace doesn't know which page(s) of memory it needs to
fix" works for me.
Also, I think we need to update the documentation for
KVM_CAP_MEMORY_FAULT_INFO to say that it is available for arm64 now
(just like you have done for KVM_CAP_EXIT_ON_MISSING).
Thanks!
[1]: https://lore.kernel.org/kvm/20231109210325.3806151-9-amoorthy@xxxxxxxxxx/
> return -EFAULT;
> + }
>
> if (kvm_is_device_pfn(pfn)) {
> /*
> --
> 2.42.0.869.gea05f2083d-goog
>
