diff mbox series

[07/13] KVM: x86/mmu: Track PRIVATE impact on hugepage mappings for all memslots

Message ID 20230921203331.3746712-8-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: guest_memfd fixes | expand

Commit Message

Sean Christopherson Sept. 21, 2023, 8:33 p.m. UTC
Track the effects of private attributes on potential hugepage mappings if
the VM supports private memory, i.e. even if the target memslot can only
ever be mapped shared.  If userspace configures a chunk of memory as
private, KVM must not allow that memory to be mapped shared regardless of
whether or not the *current* memslot can be mapped private.  E.g. if the
guest accesses a private range using a shared memslot, then KVM must exit
to userspace.

Fixes: 5bb0b4e162d1 ("KVM: x86: Disallow hugepages when memory attributes are mixed")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Binbin Wu Sept. 27, 2023, 6:01 a.m. UTC | #1
On 9/22/2023 4:33 AM, Sean Christopherson wrote:
> Track the effects of private attributes on potential hugepage mappings if
> the VM supports private memory, i.e. even if the target memslot can only
> ever be mapped shared.  If userspace configures a chunk of memory as
> private, KVM must not allow that memory to be mapped shared regardless of
> whether or not the *current* memslot can be mapped private.  E.g. if the
> guest accesses a private range using a shared memslot, then KVM must exit
> to userspace.
How does usersapce handle this case?
IIRC, in gmem v12 patch set, it says a memslot can not be convert to 
private from shared.
So, userspace should delete the old memslot and and a new one?


> Fixes: 5bb0b4e162d1 ("KVM: x86: Disallow hugepages when memory attributes are mixed")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/mmu/mmu.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 269d4dc47c98..148931cf9dba 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7314,10 +7314,12 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
>   	lockdep_assert_held(&kvm->slots_lock);
>   
>   	/*
> -	 * KVM x86 currently only supports KVM_MEMORY_ATTRIBUTE_PRIVATE, skip
> -	 * the slot if the slot will never consume the PRIVATE attribute.
> +	 * Calculate which ranges can be mapped with hugepages even if the slot
> +	 * can't map memory PRIVATE.  KVM mustn't create a SHARED hugepage over
> +	 * a range that has PRIVATE GFNs, and conversely converting a range to
> +	 * SHARED may now allow hugepages.
>   	 */
> -	if (!kvm_slot_can_be_private(slot))
> +	if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
>   		return false;
>   
>   	/*
> @@ -7372,7 +7374,7 @@ void kvm_mmu_init_memslot_memory_attributes(struct kvm *kvm,
>   {
>   	int level;
>   
> -	if (!kvm_slot_can_be_private(slot))
> +	if (!kvm_arch_has_private_mem(kvm))
>   		return;
>   
>   	for (level = PG_LEVEL_2M; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {
Sean Christopherson Sept. 27, 2023, 2:25 p.m. UTC | #2
On Wed, Sep 27, 2023, Binbin Wu wrote:
> 
> On 9/22/2023 4:33 AM, Sean Christopherson wrote:
> > Track the effects of private attributes on potential hugepage mappings if
> > the VM supports private memory, i.e. even if the target memslot can only
> > ever be mapped shared.  If userspace configures a chunk of memory as
> > private, KVM must not allow that memory to be mapped shared regardless of
> > whether or not the *current* memslot can be mapped private.  E.g. if the
> > guest accesses a private range using a shared memslot, then KVM must exit
> > to userspace.
> How does usersapce handle this case?
> IIRC, in gmem v12 patch set, it says a memslot can not be convert to private
> from shared.
> So, userspace should delete the old memslot and and a new one?

That depends on the contract between userspace and the VM, e.g. if the access is
to a range that the VMM and the guest have agreed is shared-only, then the VMM
could also "resolve" the access by injecting an error or killing the VM.

But yes, deleting the memslot and creating a new one is the only approach that
will work if userspace wants to map the gfn as private.  I don't actually expect
any real world VMMs to actually do anything like this, the purpose of this change
is mainly to ensure KVM has robust, consistent behavior.
diff mbox series

Patch

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 269d4dc47c98..148931cf9dba 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -7314,10 +7314,12 @@  bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 	lockdep_assert_held(&kvm->slots_lock);
 
 	/*
-	 * KVM x86 currently only supports KVM_MEMORY_ATTRIBUTE_PRIVATE, skip
-	 * the slot if the slot will never consume the PRIVATE attribute.
+	 * Calculate which ranges can be mapped with hugepages even if the slot
+	 * can't map memory PRIVATE.  KVM mustn't create a SHARED hugepage over
+	 * a range that has PRIVATE GFNs, and conversely converting a range to
+	 * SHARED may now allow hugepages.
 	 */
-	if (!kvm_slot_can_be_private(slot))
+	if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
 		return false;
 
 	/*
@@ -7372,7 +7374,7 @@  void kvm_mmu_init_memslot_memory_attributes(struct kvm *kvm,
 {
 	int level;
 
-	if (!kvm_slot_can_be_private(slot))
+	if (!kvm_arch_has_private_mem(kvm))
 		return;
 
 	for (level = PG_LEVEL_2M; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {