[PATCH] [RFC] Activate VMX on demand

[PATCH] [RFC] Activate VMX on demand

[Alexander Graf]

This patch moves the actual VMX enablement from the initialization
of the module to the creation of a VCPU.

With this approach, KVM can be easily autoloaded and does not block
other VMMs while being modprobe'd.
This improves the user experience a lot, since now other VMMs can
run, even though KVM is loaded, so users do not have to manually
load/unload KVM or any other VMM module.

Compared to the previously suggested approach "2", which would
introduce a complete framework that brings almost no benefit,
this approach enables coexistence of multiple VMMs without much
intervention. Thanks to Gerd for pointing that out.

I verified that this approach works with VirtualBox.

Signed-off-by: Alexander Graf <agraf@suse.de>

---
 arch/x86/kvm/vmx.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 64e2439..2c48590 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -116,6 +116,9 @@ static struct page *vmx_msr_bitmap;
 static DECLARE_BITMAP(vmx_vpid_bitmap, VMX_NR_VPIDS);
 static DEFINE_SPINLOCK(vmx_vpid_lock);
 
+static int vmx_usage_count = 0;
+static DEFINE_SPINLOCK(vmx_usage_lock);
+
 static struct vmcs_config {
 	int size;
 	int order;
@@ -1059,10 +1062,68 @@ static __init int vmx_disabled_by_bios(void)
 	/* locked but not enabled */
 }
 
-static void hardware_enable(void *garbage)
+static void __vmx_off(void *garbage)
+{
+	asm volatile (__ex(ASM_VMX_VMXOFF) : : : "cc");
+	write_cr4(read_cr4() & ~X86_CR4_VMXE);
+}
+
+static void vmx_off(void)
+{
+	if (!vmx_usage_count)
+		return;
+
+	spin_lock(&vmx_usage_lock);
+
+	vmx_usage_count--;
+	if (vmx_usage_count == 0)
+		on_each_cpu(__vmx_off, NULL, 1);
+	
+	spin_unlock(&vmx_usage_lock);
+}
+
+static void __vmx_on(void *garbage)
 {
 	int cpu = raw_smp_processor_id();
 	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
+
+	write_cr4(read_cr4() | X86_CR4_VMXE); /* FIXME: not cpu hotplug safe */
+	asm volatile (ASM_VMX_VMXON_RAX
+		      : : "a"(&phys_addr), "m"(phys_addr)
+		      : "memory", "cc");
+}
+
+static void __vmx_check(void *r)
+{
+	if (read_cr4() & X86_CR4_VMXE)
+		*((int*)r) = -EBUSY;
+}
+
+static int vmx_on(void)
+{
+	int r = 0;
+	spin_lock(&vmx_usage_lock);
+	vmx_usage_count++;
+	if (vmx_usage_count == 1) {
+		on_each_cpu(__vmx_check, &r, 1);
+		if (r)
+			goto out_1;
+
+		on_each_cpu(__vmx_on, NULL, 1);
+	}
+
+	goto out;
+
+out_1:
+	vmx_usage_count--;
+out:
+	spin_unlock(&vmx_usage_lock);
+	return r;
+}
+
+static void hardware_enable(void *garbage)
+{
+	int cpu = raw_smp_processor_id();
 	u64 old;
 
 	INIT_LIST_HEAD(&per_cpu(vcpus_on_cpu, cpu));
@@ -1075,10 +1136,9 @@ static void hardware_enable(void *garbage)
 		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
 		       FEATURE_CONTROL_LOCKED |
 		       FEATURE_CONTROL_VMXON_ENABLED);
-	write_cr4(read_cr4() | X86_CR4_VMXE); /* FIXME: not cpu hotplug safe */
-	asm volatile (ASM_VMX_VMXON_RAX
-		      : : "a"(&phys_addr), "m"(phys_addr)
-		      : "memory", "cc");
+
+	if (vmx_usage_count)
+		__vmx_on(garbage);
 }
 
 static void vmclear_local_vcpus(void)
@@ -1094,8 +1154,9 @@ static void vmclear_local_vcpus(void)
 static void hardware_disable(void *garbage)
 {
 	vmclear_local_vcpus();
-	asm volatile (__ex(ASM_VMX_VMXOFF) : : : "cc");
-	write_cr4(read_cr4() & ~X86_CR4_VMXE);
+
+	if (vmx_usage_count)
+		__vmx_off(garbage);
 }
 
 static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
@@ -3490,6 +3551,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
 	kfree(vmx->guest_msrs);
 	kvm_vcpu_uninit(vcpu);
 	kmem_cache_free(kvm_vcpu_cache, vmx);
+	vmx_off();
 }
 
 static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
@@ -3501,6 +3563,10 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 	if (!vmx)
 		return ERR_PTR(-ENOMEM);
 
+	err = vmx_on();
+	if (err)
+		goto free_vmx;
+
 	allocate_vpid(vmx);
 
 	err = kvm_vcpu_init(&vmx->vcpu, kvm, id);
@@ -3550,6 +3616,8 @@ uninit_vcpu:
 	kvm_vcpu_uninit(&vmx->vcpu);
 free_vcpu:
 	kmem_cache_free(kvm_vcpu_cache, vmx);
+free_vmx:
+	vmx_off();
 	return ERR_PTR(err);
 }
 
-- 
1.6.0.2

Re: [PATCH] [RFC] Activate VMX on demand

[Avi Kivity]

Alexander Graf wrote:
> This patch moves the actual VMX enablement from the initialization
> of the module to the creation of a VCPU.
>
> With this approach, KVM can be easily autoloaded and does not block
> other VMMs while being modprobe'd.
> This improves the user experience a lot, since now other VMMs can
> run, even though KVM is loaded, so users do not have to manually
> load/unload KVM or any other VMM module.
>
> Compared to the previously suggested approach "2", which would
> introduce a complete framework that brings almost no benefit,
> this approach enables coexistence of multiple VMMs without much
> intervention. Thanks to Gerd for pointing that out.
>
> I verified that this approach works with VirtualBox.
>
>   

This should be done in x86.c and made to apply to svm as well.  Other 
VMMs would use efer.svme as an indication that svm is in use, similar to 
cr4.vmxe today.

I'm not thrilled about this (it could be done, much more simply, by 
having the other hypervisor rmmod kvm), but okay.

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

Re: [PATCH] [RFC] Activate VMX on demand

[Alexander Graf]

On 04.11.2008, at 11:36, Avi Kivity wrote:

> Alexander Graf wrote:
>> This patch moves the actual VMX enablement from the initialization
>> of the module to the creation of a VCPU.
>>
>> With this approach, KVM can be easily autoloaded and does not block
>> other VMMs while being modprobe'd.
>> This improves the user experience a lot, since now other VMMs can
>> run, even though KVM is loaded, so users do not have to manually
>> load/unload KVM or any other VMM module.
>>
>> Compared to the previously suggested approach "2", which would
>> introduce a complete framework that brings almost no benefit,
>> this approach enables coexistence of multiple VMMs without much
>> intervention. Thanks to Gerd for pointing that out.
>>
>> I verified that this approach works with VirtualBox.
>>
>>
>
> This should be done in x86.c and made to apply to svm as well.   
> Other VMMs would use efer.svme as an indication that svm is in use,  
> similar to cr4.vmxe today.

Right, that sounds good. So just track the usage counter in x86.c and  
then split the hardware_{en,dis}able functions? Or do you want to  
actually call hardware_{en,dis}able on the usage counter events? There  
is no real need to enable/disable everything just because of the usage  
counter.

> I'm not thrilled about this (it could be done, much more simply, by  
> having the other hypervisor rmmod kvm), but okay.

It's not that I'm thrilled either, but it seriously annoys me to block  
others for "no reason". And having all other hypervisors support kvm  
module unloading just because doesn't sound right. Plus, this approach  
is unprivileged user safe. A user that has access to /dev/vbox (or  
whatever it's called) and /dev/kvm can still use both hypervisors  
without the need for root access.

Alex

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

Re: [PATCH] [RFC] Activate VMX on demand

[Avi Kivity]

Alexander Graf wrote:
>>
>> This should be done in x86.c and made to apply to svm as well.  Other 
>> VMMs would use efer.svme as an indication that svm is in use, similar 
>> to cr4.vmxe today.
>
> Right, that sounds good. So just track the usage counter in x86.c and 
> then split the hardware_{en,dis}able functions? 

What's there to split in hardware_enable()?

> Or do you want to actually call hardware_{en,dis}able on the usage 
> counter events? 

Yes.

> There is no real need to enable/disable everything just because of the 
> usage counter.

All it does is enable the feature, enable vmxe, and vmxon.  I don't 
think we gain anything by splitting it.

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

Re: [PATCH] [RFC] Activate VMX on demand

[Alexander Graf]

On 04.11.2008, at 14:37, Avi Kivity wrote:

> Alexander Graf wrote:
>>>
>>> This should be done in x86.c and made to apply to svm as well.   
>>> Other VMMs would use efer.svme as an indication that svm is in  
>>> use, similar to cr4.vmxe today.
>>
>> Right, that sounds good. So just track the usage counter in x86.c  
>> and then split the hardware_{en,dis}able functions?
>
> What's there to split in hardware_enable()?

enable allocates local vcpus and sets the vmx feature MSR, disable  
clears the vcpus. But yeah, no need to split that.

>
>
>> Or do you want to actually call hardware_{en,dis}able on the usage  
>> counter events?
>
> Yes.
>
>> There is no real need to enable/disable everything just because of  
>> the usage counter.
>
> All it does is enable the feature, enable vmxe, and vmxon.  I don't  
> think we gain anything by splitting it.

Ok. But I think this would change kvm_main.c, not x86.c, right? We'd  
basically move hardware_enable and disable generically from the module  
initialization to the vcpu creation. Could this break other  
architectures?

Alex

Re: [PATCH] [RFC] Activate VMX on demand

[Avi Kivity]

Alexander Graf wrote:
>
> Ok. But I think this would change kvm_main.c, not x86.c, right? 

Yes, thinko/remembero.

> We'd basically move hardware_enable and disable generically from the 
> module initialization to the vcpu creation. Could this break other 
> architectures?

Only if they do something in between.  Can't see how they could.

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