[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aA7aozbc1grlevOm@yzhao56-desk.sh.intel.com>
Date: Mon, 28 Apr 2025 09:32:19 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Michael Roth <michael.roth@....com>
Subject: Re: [PATCH] KVM: x86/mmu: Prevent installing hugepages when mem
attributes are changing
On Fri, Apr 25, 2025 at 05:10:56PM -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@...r.kernel.org
> Reported-by: Michael Roth <michael.roth@....com>
> Tested-by: Michael Roth <michael.roth@....com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 68 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 63bb77ee1bb1..218ba866a40e 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,37 @@ 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))
Instead of checking mixed flag in disallow_lpage, could we check disallow_lpage
directly?
So, if mixed flag is not set but disallow_lpage is 1, there's no need to update
the invalidate range.
> + kvm_mmu_invalidate_range_add(kvm, start, start + nr_pages);
> +
> + if (end == start)
> + continue;
> +
> + if ((end + nr_pages) <= (slot->base_gfn + slot->npages) &&
> + !hugepage_test_mixed(slot, end, level))
if ((end + nr_pages > range->end) &&
((end + nr_pages) <= (slot->base_gfn + slot->npages)) &&
!lpage_info_slot(gfn, slot, level)->disallow_lpage)
?
> + 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 +7747,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.850.g28803427d3-goog
>
>
Powered by blists - more mailing lists