Commit 5c0b4f3d authored by Sean Christopherson's avatar Sean Christopherson Committed by Paolo Bonzini
Browse files

KVM: Move memslot deletion to helper function 



Move memslot deletion into its own routine so that the success path for
other memslot updates does not need to use kvm_free_memslot(), i.e. can
explicitly destroy the dirty bitmap when necessary.  This paves the way
for dropping @dont from kvm_free_memslot(), i.e. all callers now pass
NULL for @dont.

Add a comment above the code to make a copy of the existing memslot
prior to deletion, it is not at all obvious that the pointer will become
stale during sorting and/or installation of new memslots.

Note, kvm_arch_commit_memory_region() allows an architecture to free
resources when moving a memslot or changing its flags, e.g. x86 frees
its arch specific memslot metadata during commit_memory_region().

Acked-by: default avatarChristoffer Dall <christoffer.dall@arm.com>
Tested-by: default avatarChristoffer Dall <christoffer.dall@arm.com>
Reviewed-by: default avatarPeter Xu <peterx@redhat.com>
Signed-off-by: default avatarSean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 21198846

virt/kvm/kvm_main.c

+46 −29
Original line number Diff line number Diff line
@@ -1043,6 +1043,27 @@ out_slots: 
	return r;

}



static int kvm_delete_memslot(struct kvm *kvm,

			      const struct kvm_userspace_memory_region *mem,

			      struct kvm_memory_slot *old, int as_id)

{

	struct kvm_memory_slot new;

	int r;



	if (!old->npages)

		return -EINVAL;



	memset(&new, 0, sizeof(new));

	new.id = old->id;



	r = kvm_set_memslot(kvm, mem, old, &new, as_id, KVM_MR_DELETE);

	if (r)

		return r;



	kvm_free_memslot(kvm, old, NULL);

	return 0;

}



/*

 * Allocate some memory and give it an address in the guest physical address

 * space.
@@ -1092,7 +1113,17 @@ int __kvm_set_memory_region(struct kvm *kvm, 
	if (npages > KVM_MEM_MAX_NR_PAGES)

		return -EINVAL;



	new = old = *slot;

	/*

	 * Make a full copy of the old memslot, the pointer will become stale

	 * when the memslots are re-sorted by update_memslots(), and the old

	 * memslot needs to be referenced after calling update_memslots(), e.g.

	 * to free its resources and for arch specific behavior.

	 */

	old = *slot;

	if (!mem->memory_size)

		return kvm_delete_memslot(kvm, mem, &old, as_id);



	new = old;



	new.id = id;

	new.base_gfn = base_gfn;
@@ -1100,10 +1131,9 @@ int __kvm_set_memory_region(struct kvm *kvm, 
	new.flags = mem->flags;

	new.userspace_addr = mem->userspace_addr;



	if (npages) {

		if (!old.npages)

	if (!old.npages) {

		change = KVM_MR_CREATE;

		else { /* Modify an existing slot. */

	} else { /* Modify an existing slot. */

		if ((new.userspace_addr != old.userspace_addr) ||

		    (npages != old.npages) ||

		    ((new.flags ^ old.flags) & KVM_MEM_READONLY))
@@ -1116,14 +1146,6 @@ int __kvm_set_memory_region(struct kvm *kvm, 
		else /* Nothing to change. */

			return 0;

	}

	} else {

		if (!old.npages)

			return -EINVAL;



		change = KVM_MR_DELETE;

		new.base_gfn = 0;

		new.flags = 0;

	}



	if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {

		/* Check for overlaps */
@@ -1145,17 +1167,12 @@ int __kvm_set_memory_region(struct kvm *kvm, 
			return r;

	}



	/* actual memory is freed via old in kvm_free_memslot below */

	if (change == KVM_MR_DELETE) {

		new.dirty_bitmap = NULL;

		memset(&new.arch, 0, sizeof(new.arch));

	}



	r = kvm_set_memslot(kvm, mem, &old, &new, as_id, change);

	if (r)

		goto out_bitmap;



	kvm_free_memslot(kvm, &old, &new);

	if (old.dirty_bitmap && !new.dirty_bitmap)

		kvm_destroy_dirty_bitmap(&old);

	return 0;



out_bitmap:

CRA Git | Maintained and supported by SUSTech CRA and CCSE