On Wed, Apr 30, 2025 at 03:09:54PM -0700, Sean Christopherson wrote:
> When changing memory attributes on a subset of a potential hugepage, add
> the hugepage to the invalidation range tracking to prevent installing a
> hugepage until the attributes are fully updated. Like the actual hugepage
> tracking updates in kvm_arch_post_set_memory_attributes(), process only
> the head and tail pages, as any potential hugepages that are entirely
> covered by the range will already be tracked.
>
> Note, only hugepage chunks whose current attributes are NOT mixed need to
> be added to the invalidation set, as mixed attributes already prevent
> installing a hugepage, and it's perfectly safe to install a smaller
> mapping for a gfn whose attributes aren't changing.
>
> Fixes: 8dd2eee9d526 ("KVM: x86/mmu: Handle page fault for private memory")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Michael Roth <michael.roth@xxxxxxx>
> Tested-by: Michael Roth <michael.roth@xxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>
> Mike, if you haven't arleady, can you rerun your testcase to double check
> that adding the "(end + nr_pages) > range->end" check didn't break anything?
I can confirm that the fix still works as expected even with the
additional check added.
Thanks!
-Mike
>
> v2: Don't add the tail page if its wholly contained by the range whose
> attributes are being modified. [Yan]
> v1: https://lore.kernel.org/all/20250426001056.1025157-1-seanjc@xxxxxxxxxx
>
> arch/x86/kvm/mmu/mmu.c | 69 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 63bb77ee1bb1..de7fd6d4b9d7 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7669,9 +7669,30 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)
> }
>
> #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> +static bool hugepage_test_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> + int level)
> +{
> + return lpage_info_slot(gfn, slot, level)->disallow_lpage & KVM_LPAGE_MIXED_FLAG;
> +}
> +
> +static void hugepage_clear_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> + int level)
> +{
> + lpage_info_slot(gfn, slot, level)->disallow_lpage &= ~KVM_LPAGE_MIXED_FLAG;
> +}
> +
> +static void hugepage_set_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> + int level)
> +{
> + lpage_info_slot(gfn, slot, level)->disallow_lpage |= KVM_LPAGE_MIXED_FLAG;
> +}
> +
> bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> struct kvm_gfn_range *range)
> {
> + struct kvm_memory_slot *slot = range->slot;
> + int level;
> +
> /*
> * Zap SPTEs even if the slot can't be mapped PRIVATE. KVM x86 only
> * supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
> @@ -7686,6 +7707,38 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
> return false;
>
> + if (WARN_ON_ONCE(range->end <= range->start))
> + return false;
> +
> + /*
> + * If the head and tail pages of the range currently allow a hugepage,
> + * i.e. reside fully in the slot and don't have mixed attributes, then
> + * add each corresponding hugepage range to the ongoing invalidation,
> + * e.g. to prevent KVM from creating a hugepage in response to a fault
> + * for a gfn whose attributes aren't changing. Note, only the range
> + * of gfns whose attributes are being modified needs to be explicitly
> + * unmapped, as that will unmap any existing hugepages.
> + */
> + for (level = PG_LEVEL_2M; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {
> + gfn_t start = gfn_round_for_level(range->start, level);
> + gfn_t end = gfn_round_for_level(range->end - 1, level);
> + gfn_t nr_pages = KVM_PAGES_PER_HPAGE(level);
> +
> + if ((start != range->start || start + nr_pages > range->end) &&
> + start >= slot->base_gfn &&
> + start + nr_pages <= slot->base_gfn + slot->npages &&
> + !hugepage_test_mixed(slot, start, level))
> + kvm_mmu_invalidate_range_add(kvm, start, start + nr_pages);
> +
> + if (end == start)
> + continue;
> +
> + if ((end + nr_pages) > range->end &&
> + (end + nr_pages) <= (slot->base_gfn + slot->npages) &&
> + !hugepage_test_mixed(slot, end, level))
> + kvm_mmu_invalidate_range_add(kvm, end, end + nr_pages);
> + }
> +
> /* Unmap the old attribute page. */
> if (range->arg.attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)
> range->attr_filter = KVM_FILTER_SHARED;
> @@ -7695,23 +7748,7 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> return kvm_unmap_gfn_range(kvm, range);
> }
>
> -static bool hugepage_test_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> - int level)
> -{
> - return lpage_info_slot(gfn, slot, level)->disallow_lpage & KVM_LPAGE_MIXED_FLAG;
> -}
>
> -static void hugepage_clear_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> - int level)
> -{
> - lpage_info_slot(gfn, slot, level)->disallow_lpage &= ~KVM_LPAGE_MIXED_FLAG;
> -}
> -
> -static void hugepage_set_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
> - int level)
> -{
> - lpage_info_slot(gfn, slot, level)->disallow_lpage |= KVM_LPAGE_MIXED_FLAG;
> -}
>
> static bool hugepage_has_attrs(struct kvm *kvm, struct kvm_memory_slot *slot,
> gfn_t gfn, int level, unsigned long attrs)
>
> base-commit: 2d7124941a273c7233849a7a2bbfbeb7e28f1caa
> --
> 2.49.0.906.g1f30a19c02-goog
>
