Re: [patch 3/4] KVM: introduce kvm_arch_can_free_memslot, disallow slot deletion if cached cr3

[Avi Kivity <avi@redhat.com>]

mtosatti@redhat.com wrote:
> Disallow the deletion of memory slots (and aliases, for x86 case), if a
> vcpu contains a cr3 that points to such slot/alias.
>
> This complements commit 6c20e1442bb1c62914bb85b7f4a38973d2a423ba.
>
> v2:
> - set KVM_REQ_TRIPLE_FAULT
> - use __KVM_HAVE_ARCH_CAN_FREE_MEMSLOT to avoid duplication of stub
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-pending/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm-pending.orig/arch/x86/kvm/x86.c
> +++ kvm-pending/arch/x86/kvm/x86.c
> @@ -1636,6 +1636,29 @@ gfn_t unalias_gfn(struct kvm *kvm, gfn_t
>  	return gfn;
>  }
>  
> +static int kvm_root_gfn_in_range(struct kvm *kvm, gfn_t base_gfn,
> +				 gfn_t end_gfn, bool unalias)
> +{
> +	struct kvm_vcpu *vcpu;
> +	gfn_t root_gfn;
> +	int i;
> +
> +	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
> +		vcpu = kvm->vcpus[i];
> +		if (!vcpu)
> +			continue;
> +		root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
>   

The guest may have changed this by now.

> +		if (unalias)
> +			root_gfn = unalias_gfn(kvm, root_gfn);
> +		if (root_gfn >= base_gfn && root_gfn <= end_gfn) {
> +			set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests);
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   

The naming is bad, a function named as a predicate shouldn't have side 
effects.

Also, we should allow deleting the slot.  There's no reason to deny 
userspace something just because the guest is playing around

I think this should be enough:
- take mmu lock
- request an mmu reload from all vcpus
- drop the slot
- release mmu lock

The reload will inject a #GP if cr3 is now out of bounds, should be 
changed to triple fault, but everything is in place (set_cr3 already 
checks).

-- 
error compiling committee.c: too many arguments to function