On Thu, Jan 09, 2025, James Houghton wrote:
> Adhering to the requirements of KVM Userfault:
>
> 1. Zap all sptes for the memslot when KVM_MEM_USERFAULT is toggled on
> with kvm_arch_flush_shadow_memslot().
> 2. Only all PAGE_SIZE sptes when KVM_MEM_USERFAULT is enabled (for both
> normal/GUP memory and guest_memfd memory).
> 3. Reconstruct huge mappings when KVM_MEM_USERFAULT is toggled off with
> kvm_mmu_recover_huge_pages(). This is the behavior when dirty logging
> is disabled; remain consistent with it.
>
> With the new logic in kvm_mmu_slot_apply_flags(), I've simplified the
> two dirty-logging-toggle checks into one, and I have dropped the
> WARN_ON() that was there.
>
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> arch/x86/kvm/Kconfig | 1 +
> arch/x86/kvm/mmu/mmu.c | 27 +++++++++++++++++++++----
> arch/x86/kvm/mmu/mmu_internal.h | 20 +++++++++++++++---
> arch/x86/kvm/x86.c | 36 ++++++++++++++++++++++++---------
> include/linux/kvm_host.h | 5 ++++-
> 5 files changed, 71 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index ea2c4f21c1ca..286c6825cd1c 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -47,6 +47,7 @@ config KVM_X86
> select KVM_GENERIC_PRE_FAULT_MEMORY
> select KVM_GENERIC_PRIVATE_MEM if KVM_SW_PROTECTED_VM
> select KVM_WERROR if WERROR
> + select HAVE_KVM_USERFAULT
>
> config KVM
> tristate "Kernel-based Virtual Machine (KVM) support"
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 2401606db260..5cab2785b97f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4280,14 +4280,19 @@ static inline u8 kvm_max_level_for_order(int order)
> return PG_LEVEL_4K;
> }
>
> -static u8 kvm_max_private_mapping_level(struct kvm *kvm, kvm_pfn_t pfn,
> - u8 max_level, int gmem_order)
> +static u8 kvm_max_private_mapping_level(struct kvm *kvm,
> + struct kvm_memory_slot *slot,
> + kvm_pfn_t pfn, u8 max_level,
> + int gmem_order)
> {
> u8 req_max_level;
>
> if (max_level == PG_LEVEL_4K)
> return PG_LEVEL_4K;
>
> + if (kvm_memslot_userfault(slot))
Unless I'm missing something, this can go in kvm_mmu_hugepage_adjust():
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a4439e9e0726..49eb6b9b268c 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3304,7 +3304,7 @@ void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
if (is_error_noslot_pfn(fault->pfn))
return;
- if (kvm_slot_dirty_track_enabled(slot))
+ if (kvm_slot_dirty_track_enabled(slot) || kvm_is_userfault_memslot(slot))
return;
> static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1b04092ec76a..2abb425a6514 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -13053,12 +13053,36 @@ static void kvm_mmu_slot_apply_flags(struct kvm *kvm,
> u32 new_flags = new ? new->flags : 0;
> bool log_dirty_pages = new_flags & KVM_MEM_LOG_DIRTY_PAGES;
>
> + /*
> + * When toggling KVM Userfault on, zap all sptes so that userfault-ness
> + * will be respected at refault time. All new faults will only install
> + * small sptes. Therefore, when toggling it off, recover hugepages.
> + *
> + * For MOVE and DELETE, there will be nothing to do, as the old
> + * mappings will have already been deleted by
> + * kvm_arch_flush_shadow_memslot().
> + *
> + * For CREATE, no mappings will have been created yet.
> + */
Eh, trim this down and the reference the comment below to explain why FLAGS_ONLY
is the only case that needs to be handled.
> + if ((old_flags ^ new_flags) & KVM_MEM_USERFAULT &&
> + (change == KVM_MR_FLAGS_ONLY)) {
> + if (old_flags & KVM_MEM_USERFAULT)
> + kvm_mmu_recover_huge_pages(kvm, new);
> + else
> + kvm_arch_flush_shadow_memslot(kvm, old);
The call to kvm_arch_flush_shadow_memslot() should definitely go in common code.
The fancy recovery logic is arch specific, but blasting the memslot when userfault
is toggled on is not.
