[PATCH] Add cpu consistence check in vmx.

[PATCH] Add cpu consistence check in vmx.

[Yang, Sheng]

[-- Attachment #1: Type: text/plain, Size: 5745 bytes --]

All the physical CPUs on the board should support the same VMX feature
set.
The hardware_enable() do the per-cpu check now.
In case of vmx/svm enabling failure, transfer a variable in
hardware_enable()
for error report.

Signed-off-by: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/kvm/kvm_main.c |    9 +++--
 drivers/kvm/svm.c      |    3 +-
 drivers/kvm/vmx.c      |   84
+++++++++++++++++++++++++++++-------------------
 3 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index bc11c2d..10443ea 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -2974,14 +2974,14 @@ static void decache_vcpus_on_cpu(int cpu)
 	spin_unlock(&kvm_lock);
 }
 
-static void hardware_enable(void *junk)
+static void hardware_enable(void *rtn)
 {
 	int cpu = raw_smp_processor_id();
 
 	if (cpu_isset(cpu, cpus_hardware_enabled))
 		return;
 	cpu_set(cpu, cpus_hardware_enabled);
-	kvm_arch_ops->hardware_enable(NULL);
+	kvm_arch_ops->hardware_enable(rtn);
 }
 
 static void hardware_disable(void *junk)
@@ -3173,7 +3173,10 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
struct module *module)
 	if (r < 0)
 		goto out;
 
-	on_each_cpu(hardware_enable, NULL, 0, 1);
+	on_each_cpu(hardware_enable, &r, 0, 1);
+	if (r < 0)
+		goto out;
+
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
 		goto out_free_1;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 850a1b1..1e76417 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -285,7 +285,7 @@ static void svm_hardware_disable(void *garbage)
 	}
 }
 
-static void svm_hardware_enable(void *garbage)
+static void svm_hardware_enable(void *rtn)
 {
 
 	struct svm_cpu_data *svm_data;
@@ -298,6 +298,7 @@ static void svm_hardware_enable(void *garbage)
 	struct desc_struct *gdt;
 	int me = raw_smp_processor_id();
 
+	*(int*)rtn = 0;
 	if (!has_svm()) {
 		printk(KERN_ERR "svm_cpu_init: err EOPNOTSUPP on %d\n",
me);
 		return;
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index e5721a5..8032c7e 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -756,31 +756,6 @@ static __init int vmx_disabled_by_bios(void)
 	/* locked but not enabled */
 }
 
-static void hardware_enable(void *garbage)
-{
-	int cpu = raw_smp_processor_id();
-	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
-	u64 old;
-
-	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
-	if ((old & (MSR_IA32_FEATURE_CONTROL_LOCKED |
-		    MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
-	    != (MSR_IA32_FEATURE_CONTROL_LOCKED |
-		MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
-		/* enable and lock */
-		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
-		       MSR_IA32_FEATURE_CONTROL_LOCKED |
-		       MSR_IA32_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");
-}
-
-static void hardware_disable(void *garbage)
-{
-	asm volatile (ASM_VMX_VMXOFF : : : "cc");
-}
-
 static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
 				      u32 msr, u32* result)
 {
@@ -856,18 +831,61 @@ static __init int setup_vmcs_config(void)
 	if (((vmx_msr_high >> 18) & 15) != 6)
 		return -1;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
-
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	if (vmcs_config.size == 0) {
+		/* called in hardware_setup(), initialization */
+		vmcs_config.size = vmx_msr_high & 0x1fff;
+		vmcs_config.order = get_order(vmcs_config.size);
+		vmcs_config.revision_id = vmx_msr_low;
+
+		vmcs_config.pin_based_exec_ctrl =
_pin_based_exec_control;
+		vmcs_config.cpu_based_exec_ctrl =
_cpu_based_exec_control;
+		vmcs_config.vmexit_ctrl         = _vmexit_control;
+		vmcs_config.vmentry_ctrl        = _vmentry_control;
+	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
+		|| (vmcs_config.revision_id != vmx_msr_low)
+		|| (vmcs_config.pin_based_exec_ctrl !=
_pin_based_exec_control)
+		|| (vmcs_config.cpu_based_exec_ctrl !=
_cpu_based_exec_control)
+		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
+		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
+		/* called in hardware_enable(), check consistence */
+		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
+		return -1;
+	}
 
 	return 0;
 }
 
+static void hardware_enable(void *rtn)
+{
+	int cpu = raw_smp_processor_id();
+	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
+	u64 old;
+
+	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
+	if ((old & (MSR_IA32_FEATURE_CONTROL_LOCKED |
+		    MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
+	    != (MSR_IA32_FEATURE_CONTROL_LOCKED |
+		MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
+		/* enable and lock */
+		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
+		       MSR_IA32_FEATURE_CONTROL_LOCKED |
+		       MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED);
+
+	*(int*)rtn = setup_vmcs_config(); /* check cpus' feature
consistence */
+	if (rtn < 0)
+		return;
+
+	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 hardware_disable(void *garbage)
+{
+	asm volatile (ASM_VMX_VMXOFF : : : "cc");
+}
+
 static struct vmcs *alloc_vmcs_cpu(int cpu)
 {
 	int node = cpu_to_node(cpu);
-- 
1.5.2

[-- Attachment #2: Add-cpu-consistence-check-in-vmx.patch --]
[-- Type: application/octet-stream, Size: 5727 bytes --]

From 2775d63d659bc1ed812ecb774d70aeca1f78ec2e Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Fri, 27 Jul 2007 11:27:45 +0800
Subject: [PATCH] Add cpu consistence check in vmx.

All the physical CPUs on the board should support the same VMX feature set.
The hardware_enable() do the per-cpu check now.
In case of vmx/svm enable failure, transfer a variable in hardware_enable()
for error report.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 drivers/kvm/kvm_main.c |    9 +++--
 drivers/kvm/svm.c      |    3 +-
 drivers/kvm/vmx.c      |   84 +++++++++++++++++++++++++++++-------------------
 3 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index bc11c2d..10443ea 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -2974,14 +2974,14 @@ static void decache_vcpus_on_cpu(int cpu)
 	spin_unlock(&kvm_lock);
 }
 
-static void hardware_enable(void *junk)
+static void hardware_enable(void *rtn)
 {
 	int cpu = raw_smp_processor_id();
 
 	if (cpu_isset(cpu, cpus_hardware_enabled))
 		return;
 	cpu_set(cpu, cpus_hardware_enabled);
-	kvm_arch_ops->hardware_enable(NULL);
+	kvm_arch_ops->hardware_enable(rtn);
 }
 
 static void hardware_disable(void *junk)
@@ -3173,7 +3173,10 @@ int kvm_init_arch(struct kvm_arch_ops *ops, struct module *module)
 	if (r < 0)
 		goto out;
 
-	on_each_cpu(hardware_enable, NULL, 0, 1);
+	on_each_cpu(hardware_enable, &r, 0, 1);
+	if (r < 0)
+		goto out;
+
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
 		goto out_free_1;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 850a1b1..1e76417 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -285,7 +285,7 @@ static void svm_hardware_disable(void *garbage)
 	}
 }
 
-static void svm_hardware_enable(void *garbage)
+static void svm_hardware_enable(void *rtn)
 {
 
 	struct svm_cpu_data *svm_data;
@@ -298,6 +298,7 @@ static void svm_hardware_enable(void *garbage)
 	struct desc_struct *gdt;
 	int me = raw_smp_processor_id();
 
+	*(int*)rtn = 0;
 	if (!has_svm()) {
 		printk(KERN_ERR "svm_cpu_init: err EOPNOTSUPP on %d\n", me);
 		return;
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index e5721a5..8032c7e 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -756,31 +756,6 @@ static __init int vmx_disabled_by_bios(void)
 	/* locked but not enabled */
 }
 
-static void hardware_enable(void *garbage)
-{
-	int cpu = raw_smp_processor_id();
-	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
-	u64 old;
-
-	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
-	if ((old & (MSR_IA32_FEATURE_CONTROL_LOCKED |
-		    MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
-	    != (MSR_IA32_FEATURE_CONTROL_LOCKED |
-		MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
-		/* enable and lock */
-		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
-		       MSR_IA32_FEATURE_CONTROL_LOCKED |
-		       MSR_IA32_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");
-}
-
-static void hardware_disable(void *garbage)
-{
-	asm volatile (ASM_VMX_VMXOFF : : : "cc");
-}
-
 static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
 				      u32 msr, u32* result)
 {
@@ -856,18 +831,61 @@ static __init int setup_vmcs_config(void)
 	if (((vmx_msr_high >> 18) & 15) != 6)
 		return -1;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
-
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	if (vmcs_config.size == 0) {
+		/* called in hardware_setup(), initialization */
+		vmcs_config.size = vmx_msr_high & 0x1fff;
+		vmcs_config.order = get_order(vmcs_config.size);
+		vmcs_config.revision_id = vmx_msr_low;
+
+		vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
+		vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
+		vmcs_config.vmexit_ctrl         = _vmexit_control;
+		vmcs_config.vmentry_ctrl        = _vmentry_control;
+	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
+		|| (vmcs_config.revision_id != vmx_msr_low)
+		|| (vmcs_config.pin_based_exec_ctrl != _pin_based_exec_control)
+		|| (vmcs_config.cpu_based_exec_ctrl != _cpu_based_exec_control)
+		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
+		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
+		/* called in hardware_enable(), check consistence */
+		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
+		return -1;
+	}
 
 	return 0;
 }
 
+static void hardware_enable(void *rtn)
+{
+	int cpu = raw_smp_processor_id();
+	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
+	u64 old;
+
+	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
+	if ((old & (MSR_IA32_FEATURE_CONTROL_LOCKED |
+		    MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
+	    != (MSR_IA32_FEATURE_CONTROL_LOCKED |
+		MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
+		/* enable and lock */
+		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
+		       MSR_IA32_FEATURE_CONTROL_LOCKED |
+		       MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED);
+
+	*(int*)rtn = setup_vmcs_config(); /* check cpus' feature consistence */
+	if (rtn < 0)
+		return;
+
+	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 hardware_disable(void *garbage)
+{
+	asm volatile (ASM_VMX_VMXOFF : : : "cc");
+}
+
 static struct vmcs *alloc_vmcs_cpu(int cpu)
 {
 	int node = cpu_to_node(cpu);
-- 
1.5.2


[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] Add cpu consistence check in vmx.

[Avi Kivity]

Yang, Sheng wrote:
> All the physical CPUs on the board should support the same VMX feature
> set.
> The hardware_enable() do the per-cpu check now.
> In case of vmx/svm enabling failure, transfer a variable in
> hardware_enable()
> for error report.
>
>  
>  static void hardware_disable(void *junk)
> @@ -3173,7 +3173,10 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
> struct module *module)
>  	if (r < 0)
>  		goto out;
>  
> -	on_each_cpu(hardware_enable, NULL, 0, 1);
> +	on_each_cpu(hardware_enable, &r, 0, 1);
> +	if (r < 0)
> +		goto out;
> +
>   

Looks like the checking and assignment to r are done in parallel; this 
is racy.

I suggest adding a new arch op, ->check_processor_compatibility(), and 
running it from kvm_main as follows:

    for_each_online_cpu(cpu) {
        smp_call_function_single(cpu, 
kvm_arch_ops->check_processor_compatibility, &r, ...);
        if (r < 0)
             break;
    }

>  
> -static void hardware_enable(void *garbage)
> -{
> -	int cpu = raw_smp_processor_id();
> -	u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
> -	u64 old;
> -
> -	rdmsrl(MSR_IA32_FEATURE_CONTROL, old);
> -	if ((old & (MSR_IA32_FEATURE_CONTROL_LOCKED |
> -		    MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
> -	    != (MSR_IA32_FEATURE_CONTROL_LOCKED |
> -		MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED))
> -		/* enable and lock */
> -		wrmsrl(MSR_IA32_FEATURE_CONTROL, old |
> -		       MSR_IA32_FEATURE_CONTROL_LOCKED |
> -		       MSR_IA32_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");
> -}
> -
> -static void hardware_disable(void *garbage)
> -{
> -	asm volatile (ASM_VMX_VMXOFF : : : "cc");
> -}
> -
>   

Please avoid moving a function and changing it in the same patch.  That 
makes it very hard to review.

>  static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
>  				      u32 msr, u32* result)
>  {
> @@ -856,18 +831,61 @@ static __init int setup_vmcs_config(void)
>  	if (((vmx_msr_high >> 18) & 15) != 6)
>  		return -1;
>  
> -	vmcs_config.size = vmx_msr_high & 0x1fff;
> -	vmcs_config.order = get_order(vmcs_config.size);
> -	vmcs_config.revision_id = vmx_msr_low;
> -
> -	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
> -	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
> -	vmcs_config.vmexit_ctrl         = _vmexit_control;
> -	vmcs_config.vmentry_ctrl        = _vmentry_control;
> +	if (vmcs_config.size == 0) {
> +		/* called in hardware_setup(), initialization */
> +		vmcs_config.size = vmx_msr_high & 0x1fff;
> +		vmcs_config.order = get_order(vmcs_config.size);
> +		vmcs_config.revision_id = vmx_msr_low;
> +
> +		vmcs_config.pin_based_exec_ctrl =
> _pin_based_exec_control;
> +		vmcs_config.cpu_based_exec_ctrl =
> _cpu_based_exec_control;
> +		vmcs_config.vmexit_ctrl         = _vmexit_control;
> +		vmcs_config.vmentry_ctrl        = _vmentry_control;
> +	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
> +		|| (vmcs_config.revision_id != vmx_msr_low)
> +		|| (vmcs_config.pin_based_exec_ctrl !=
> _pin_based_exec_control)
> +		|| (vmcs_config.cpu_based_exec_ctrl !=
> _cpu_based_exec_control)
> +		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
> +		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
> +		/* called in hardware_enable(), check consistence */
> +		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
> +		return -1;
> +	}
>  
>  	return 0;
>  }
>   

This is called from ->hardware_enable(), right?  If so, then it is racy.



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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Yang, Sheng]

[-- Attachment #1: Type: text/plain, Size: 4941 bytes --]

Thank you for point out my fault.

Here is a modified version which is clearer. And I have tested it with
version d9feefe(for the latest git repository broken).

Thanks
Yang, Sheng
--
From: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Tue, 31 Jul 2007 10:21:32 +0800
Subject: [PATCH] Add cpu consistence check in vmx.

All the physical CPUs on the board should support the same VMX feature
set.
Add check_processor_compatibility to kvm_arch_ops for the consistence
check.
---
 drivers/kvm/kvm.h      |    1 +
 drivers/kvm/kvm_main.c |    9 +++++++++
 drivers/kvm/svm.c      |    6 ++++++
 drivers/kvm/vmx.c      |   36 ++++++++++++++++++++++++++++--------
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index f78729c..e4f11b6 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -420,6 +420,7 @@ struct kvm_arch_ops {
 	int (*disabled_by_bios)(void);             /* __init */
 	void (*hardware_enable)(void *dummy);      /* __init */
 	void (*hardware_disable)(void *dummy);
+	void (*check_processor_compatibility)(void *rtn);
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 4fd2074..ae14163 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -3095,6 +3095,7 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
unsigned int vcpu_size,
 		  struct module *module)
 {
 	int r;
+	int cpu;
 
 	if (kvm_arch_ops) {
 		printk(KERN_ERR "kvm: already loaded the other
module\n");
@@ -3116,6 +3117,14 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
unsigned int vcpu_size,
 	if (r < 0)
 		goto out;
 
+	for_each_online_cpu(cpu) {
+		smp_call_function_single(cpu,
+
kvm_arch_ops->check_processor_compatibility,
+				&r, 0, 1);
+		if (r < 0)
+			goto out;
+	}
+
 	on_each_cpu(hardware_enable, NULL, 0, 1);
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 5277084..827bc27 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1798,11 +1798,17 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu,
unsigned char *hypercall)
 	hypercall[3] = 0xc3;
 }
 
+static void svm_check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+}
+
 static struct kvm_arch_ops svm_arch_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
 	.hardware_setup = svm_hardware_setup,
 	.hardware_unsetup = svm_hardware_unsetup,
+	.check_processor_compatibility = svm_check_processor_compat,
 	.hardware_enable = svm_hardware_enable,
 	.hardware_disable = svm_hardware_disable,
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 6e23600..41a4986 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -902,14 +902,26 @@ static __init int setup_vmcs_config(void)
 	if (((vmx_msr_high >> 18) & 15) != 6)
 		return -1;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
-
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	if (vmcs_config.size == 0) {
+		/* called in hardware_setup(), initialization */
+		vmcs_config.size = vmx_msr_high & 0x1fff;
+		vmcs_config.order = get_order(vmcs_config.size);
+		vmcs_config.revision_id = vmx_msr_low;
+
+		vmcs_config.pin_based_exec_ctrl =
_pin_based_exec_control;
+		vmcs_config.cpu_based_exec_ctrl =
_cpu_based_exec_control;
+		vmcs_config.vmexit_ctrl         = _vmexit_control;
+		vmcs_config.vmentry_ctrl        = _vmentry_control;
+	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
+		|| (vmcs_config.revision_id != vmx_msr_low)
+		|| (vmcs_config.pin_based_exec_ctrl !=
_pin_based_exec_control)
+		|| (vmcs_config.cpu_based_exec_ctrl !=
_cpu_based_exec_control)
+		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
+		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
+		/* called check_processor_compat(), check consistence */
+		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
+		return -1;
+	}
 
 	return 0;
 }
@@ -2412,11 +2424,19 @@ free_vcpu:
 	return ERR_PTR(err);
 }
 
+void __init check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+	if (setup_vmcs_config() < 0)
+		*(int *)rtn = -1;
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
 	.hardware_setup = hardware_setup,
 	.hardware_unsetup = hardware_unsetup,
+	.check_processor_compatibility = check_processor_compat,
 	.hardware_enable = hardware_enable,
 	.hardware_disable = hardware_disable,
 
-- 
1.5.2


[-- Attachment #2: Add-cpu-consistence-check-in-vmx.patch --]
[-- Type: application/octet-stream, Size: 4646 bytes --]

From f4b732d0c3b5cb5467dc6c1275202728e43430d5 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Tue, 31 Jul 2007 10:21:32 +0800
Subject: [PATCH] Add cpu consistence check in vmx.

All the physical CPUs on the board should support the same VMX feature set.
Add check_processor_compatibility to kvm_arch_ops for the consistence
check.
---
 drivers/kvm/kvm.h      |    1 +
 drivers/kvm/kvm_main.c |    9 +++++++++
 drivers/kvm/svm.c      |    6 ++++++
 drivers/kvm/vmx.c      |   36 ++++++++++++++++++++++++++++--------
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index f78729c..e4f11b6 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -420,6 +420,7 @@ struct kvm_arch_ops {
 	int (*disabled_by_bios)(void);             /* __init */
 	void (*hardware_enable)(void *dummy);      /* __init */
 	void (*hardware_disable)(void *dummy);
+	void (*check_processor_compatibility)(void *rtn);
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 4fd2074..ae14163 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -3095,6 +3095,7 @@ int kvm_init_arch(struct kvm_arch_ops *ops, unsigned int vcpu_size,
 		  struct module *module)
 {
 	int r;
+	int cpu;
 
 	if (kvm_arch_ops) {
 		printk(KERN_ERR "kvm: already loaded the other module\n");
@@ -3116,6 +3117,14 @@ int kvm_init_arch(struct kvm_arch_ops *ops, unsigned int vcpu_size,
 	if (r < 0)
 		goto out;
 
+	for_each_online_cpu(cpu) {
+		smp_call_function_single(cpu,
+				kvm_arch_ops->check_processor_compatibility,
+				&r, 0, 1);
+		if (r < 0)
+			goto out;
+	}
+
 	on_each_cpu(hardware_enable, NULL, 0, 1);
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 5277084..827bc27 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1798,11 +1798,17 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall)
 	hypercall[3] = 0xc3;
 }
 
+static void svm_check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+}
+
 static struct kvm_arch_ops svm_arch_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
 	.hardware_setup = svm_hardware_setup,
 	.hardware_unsetup = svm_hardware_unsetup,
+	.check_processor_compatibility = svm_check_processor_compat,
 	.hardware_enable = svm_hardware_enable,
 	.hardware_disable = svm_hardware_disable,
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 6e23600..41a4986 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -902,14 +902,26 @@ static __init int setup_vmcs_config(void)
 	if (((vmx_msr_high >> 18) & 15) != 6)
 		return -1;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
-
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	if (vmcs_config.size == 0) {
+		/* called in hardware_setup(), initialization */
+		vmcs_config.size = vmx_msr_high & 0x1fff;
+		vmcs_config.order = get_order(vmcs_config.size);
+		vmcs_config.revision_id = vmx_msr_low;
+
+		vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
+		vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
+		vmcs_config.vmexit_ctrl         = _vmexit_control;
+		vmcs_config.vmentry_ctrl        = _vmentry_control;
+	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
+		|| (vmcs_config.revision_id != vmx_msr_low)
+		|| (vmcs_config.pin_based_exec_ctrl != _pin_based_exec_control)
+		|| (vmcs_config.cpu_based_exec_ctrl != _cpu_based_exec_control)
+		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
+		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
+		/* called check_processor_compat(), check consistence */
+		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
+		return -1;
+	}
 
 	return 0;
 }
@@ -2412,11 +2424,19 @@ free_vcpu:
 	return ERR_PTR(err);
 }
 
+void __init check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+	if (setup_vmcs_config() < 0)
+		*(int *)rtn = -1;
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
 	.hardware_setup = hardware_setup,
 	.hardware_unsetup = hardware_unsetup,
+	.check_processor_compatibility = check_processor_compat,
 	.hardware_enable = hardware_enable,
 	.hardware_disable = hardware_disable,
 
-- 
1.5.2


[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] Add cpu consistence check in vmx.

[Avi Kivity]

Yang, Sheng wrote:
> Thank you for point out my fault.
>
> Here is a modified version which is clearer. And I have tested it with
> version d9feefe(for the latest git repository broken).
>   

I recommend building kvm.git, not the external module.  kvm.git is not 
broken at the moment.

> All the physical CPUs on the board should support the same VMX feature
> set.
> Add check_processor_compatibility to kvm_arch_ops for the consistence
> check.
>   

> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -3095,6 +3095,7 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
> unsigned int vcpu_size,
>  		  struct module *module)
>  {
>  	int r;
> +	int cpu;
>  
>  	if (kvm_arch_ops) {
>  		printk(KERN_ERR "kvm: already loaded the other
> module\n");
> @@ -3116,6 +3117,14 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
> unsigned int vcpu_size,
>  	if (r < 0)
>  		goto out;
>  
> +	for_each_online_cpu(cpu) {
> +		smp_call_function_single(cpu,
> +
> kvm_arch_ops->check_processor_compatibility,
> +				&r, 0, 1);
> +		if (r < 0)
> +			goto out;
>   

You need to call ->hardware_unsetup() in case of an error here.

> +	}
> +
>  	on_each_cpu(hardware_enable, NULL, 0, 1);
>  	r = register_cpu_notifier(&kvm_cpu_notifier);
>  	if (r)
> diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
> index 5277084..827bc27 100644
> --- a/drivers/kvm/svm.c
> +++ b/drivers/kvm/svm.c
> @@ -1798,11 +1798,17 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu,
> unsigned char *hypercall)
>  	hypercall[3] = 0xc3;
>  }
>  
> +static void svm_check_processor_compat(void *rtn)
> +{
> +	*(int *)rtn = 0;
> +}
> +
>  static struct kvm_arch_ops svm_arch_ops = {
>  	.cpu_has_kvm_support = has_svm,
>  	.disabled_by_bios = is_disabled,
>  	.hardware_setup = svm_hardware_setup,
>  	.hardware_unsetup = svm_hardware_unsetup,
> +	.check_processor_compatibility = svm_check_processor_compat,
>  	.hardware_enable = svm_hardware_enable,
>  	.hardware_disable = svm_hardware_disable,
>  
> diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
> index 6e23600..41a4986 100644
> --- a/drivers/kvm/vmx.c
> +++ b/drivers/kvm/vmx.c
> @@ -902,14 +902,26 @@ static __init int setup_vmcs_config(void)
>  	if (((vmx_msr_high >> 18) & 15) != 6)
>  		return -1;
>  
> -	vmcs_config.size = vmx_msr_high & 0x1fff;
> -	vmcs_config.order = get_order(vmcs_config.size);
> -	vmcs_config.revision_id = vmx_msr_low;
> -
> -	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
> -	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
> -	vmcs_config.vmexit_ctrl         = _vmexit_control;
> -	vmcs_config.vmentry_ctrl        = _vmentry_control;
> +	if (vmcs_config.size == 0) {
> +		/* called in hardware_setup(), initialization */
> +		vmcs_config.size = vmx_msr_high & 0x1fff;
> +		vmcs_config.order = get_order(vmcs_config.size);
> +		vmcs_config.revision_id = vmx_msr_low;
> +
> +		vmcs_config.pin_based_exec_ctrl =
> _pin_based_exec_control;
> +		vmcs_config.cpu_based_exec_ctrl =
> _cpu_based_exec_control;
> +		vmcs_config.vmexit_ctrl         = _vmexit_control;
> +		vmcs_config.vmentry_ctrl        = _vmentry_control;
> +	} else if ((vmcs_config.size != (vmx_msr_high & 0x1fff))
> +		|| (vmcs_config.revision_id != vmx_msr_low)
> +		|| (vmcs_config.pin_based_exec_ctrl !=
> _pin_based_exec_control)
> +		|| (vmcs_config.cpu_based_exec_ctrl !=
> _cpu_based_exec_control)
> +		|| (vmcs_config.vmexit_ctrl != _vmexit_control)
> +		|| (vmcs_config.vmentry_ctrl != _vmentry_control)) {
> +		/* called check_processor_compat(), check consistence */
> +		printk(KERN_ERR "kvm: CPUs feature inconsistence!\n");
>   

Spelling: "CPUs" -> "CPU%d", "inconsistence" -> "inconsistency".

> +		return -1;
>   

-1 is -EPERM.  We need a real, more suitable, error code here.

Also, having a single function either construct vmcs_config or verify, 
depending on whether it is first called or not, it is a bit ugly.  A 
check_... function shouldn't actually set up global variables.  How 
about the following:

- setup_vmcs_config() takes a vmcs_config parameter instead of using a 
global.
- it is called once by vmx_hardware_setup() with the global config
- vmx_check_processor_compat() calls setup_vmcs_config() to set up a 
local variable, and then calls vmx_verify_config() to compare the two 
configurations.  Perhaps we can use memcmp() for the comparison.
> +	}
>  
>  	return 0;
>  }
> @@ -2412,11 +2424,19 @@ free_vcpu:
>  	return ERR_PTR(err);
>  }
>  
> +void __init check_processor_compat(void *rtn)
>   

Call this vmx_processor_compat() for consistency?


btw, what about cpu hotplug?  we need to deal with that too.  Do we 
error out and refuse to enable the cpu if it isn't compatible enough?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Li, Xin B]

>
>btw, what about cpu hotplug?  we need to deal with that too.  Do we 
>error out and refuse to enable the cpu if it isn't compatible enough?

I think we shouldn't enable it, or the code becomes tooooo complex.
-xin

>
>-- 
>error compiling committee.c: too many arguments to function
>
>
>---------------------------------------------------------------
>----------
>This SF.net email is sponsored by: Splunk Inc.
>Still grepping through log files to find problems?  Stop.
>Now Search log events and configuration files using AJAX and a browser.
>Download your FREE copy of Splunk now >>  http://get.splunk.com/
>_______________________________________________
>kvm-devel mailing list
>kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>https://lists.sourceforge.net/lists/listinfo/kvm-devel
>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Avi Kivity]

Li, Xin B wrote:
>> btw, what about cpu hotplug?  we need to deal with that too.  Do we 
>> error out and refuse to enable the cpu if it isn't compatible enough?
>>     
>
> I think we shouldn't enable it, or the code becomes tooooo complex.
> -xin
>
>   

What do you mean exactly?

Don't enable cpu hotplug?  it's needed for suspend/resume.
Don't enable the cpu if it isn't compatible?  I think that's the way to go.
Don't enable feature compatibility checking on hotplug?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Li, Xin B]

>-----Original Message-----
>From: Avi Kivity [mailto:avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org] 
>Sent: Tuesday, July 31, 2007 4:10 PM
>To: Li, Xin B
>Cc: Yang, Sheng; kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] [PATCH] Add cpu consistence check in vmx.
>
>Li, Xin B wrote:
>>> btw, what about cpu hotplug?  we need to deal with that too.  Do we 
>>> error out and refuse to enable the cpu if it isn't 
>compatible enough?
>>>     
>>
>> I think we shouldn't enable it, or the code becomes tooooo complex.
>> -xin
>>
>>   
>
>What do you mean exactly?
>
>Don't enable cpu hotplug?  it's needed for suspend/resume.
>Don't enable the cpu if it isn't compatible?  I think that's 
>the way to go.

Yes, this is what I meant.


>Don't enable feature compatibility checking on hotplug?
>
>-- 
>error compiling committee.c: too many arguments to function
>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Yang, Sheng]

>> +		return -1;
>>
>
>-1 is -EPERM.  We need a real, more suitable, error code here.

How about using -EINVAL? But the comment of it is "Invalid argument"...
I can't find more one here...

>Also, having a single function either construct vmcs_config or verify,
>depending on whether it is first called or not, it is a bit ugly.  A
>check_... function shouldn't actually set up global variables.  How
>about the following:
>
>- setup_vmcs_config() takes a vmcs_config parameter instead of using a
>global.
>- it is called once by vmx_hardware_setup() with the global config
>- vmx_check_processor_compat() calls setup_vmcs_config() to set up a
>local variable, and then calls vmx_verify_config() to compare the two
>configurations.  Perhaps we can use memcmp() for the comparison.

Good approach. I will do it.

>> +void __init check_processor_compat(void *rtn)
>>
>
>Call this vmx_processor_compat() for consistency?
Ok. 

Thanks.
Yang, Sheng

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Avi Kivity]

Yang, Sheng wrote:
>>> +		return -1;
>>>
>>>       
>> -1 is -EPERM.  We need a real, more suitable, error code here.
>>     
>
> How about using -EINVAL? But the comment of it is "Invalid argument"...
> I can't find more one here...
>
>   

EINVAL usually means a user-supplied argument is incorrect.  The best 
fit I can find is EIO, as it implies a hardware issue.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] Add cpu consistence check in vmx.

[Yang, Sheng]

[-- Attachment #1: Type: text/plain, Size: 6821 bytes --]

I modified the patch as you suggest.
For any failure in this due to hardware error, I replace all the error
return code with -EIO.

Thanks.
---
From: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Tue, 31 Jul 2007 17:23:20 +0800
Subject: [PATCH] Add cpu consistence check in vmx.

All the physical CPUs on the board should support the same VMX feature
set.
Add check_processor_compatibility to kvm_arch_ops for the consistence
check.

Signed-off-by: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/kvm/kvm.h      |    1 +
 drivers/kvm/kvm_main.c |   10 +++++++++
 drivers/kvm/svm.c      |    6 +++++
 drivers/kvm/vmx.c      |   51
+++++++++++++++++++++++++++++++----------------
 4 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index f78729c..e4f11b6 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -420,6 +420,7 @@ struct kvm_arch_ops {
 	int (*disabled_by_bios)(void);             /* __init */
 	void (*hardware_enable)(void *dummy);      /* __init */
 	void (*hardware_disable)(void *dummy);
+	void (*check_processor_compatibility)(void *rtn);
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 4fd2074..1811ed4 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -3095,6 +3095,7 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
unsigned int vcpu_size,
 		  struct module *module)
 {
 	int r;
+	int cpu;
 
 	if (kvm_arch_ops) {
 		printk(KERN_ERR "kvm: already loaded the other
module\n");
@@ -3116,6 +3117,14 @@ int kvm_init_arch(struct kvm_arch_ops *ops,
unsigned int vcpu_size,
 	if (r < 0)
 		goto out;
 
+	for_each_online_cpu(cpu) {
+		smp_call_function_single(cpu,
+
kvm_arch_ops->check_processor_compatibility,
+				&r, 0, 1);
+		if (r < 0)
+			goto out_free_0;
+	}
+
 	on_each_cpu(hardware_enable, NULL, 0, 1);
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
@@ -3162,6 +3171,7 @@ out_free_2:
 	unregister_cpu_notifier(&kvm_cpu_notifier);
 out_free_1:
 	on_each_cpu(hardware_disable, NULL, 0, 1);
+out_free_0:
 	kvm_arch_ops->hardware_unsetup();
 out:
 	kvm_arch_ops = NULL;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 5277084..827bc27 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1798,11 +1798,17 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu,
unsigned char *hypercall)
 	hypercall[3] = 0xc3;
 }
 
+static void svm_check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+}
+
 static struct kvm_arch_ops svm_arch_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
 	.hardware_setup = svm_hardware_setup,
 	.hardware_unsetup = svm_hardware_unsetup,
+	.check_processor_compatibility = svm_check_processor_compat,
 	.hardware_enable = svm_hardware_enable,
 	.hardware_disable = svm_hardware_disable,
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 6e23600..fccaf35 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -840,13 +840,13 @@ static __init int adjust_vmx_controls(u32 ctl_min,
u32 ctl_opt,
 
 	/* Ensure minimum (required) set of control bits are supported.
*/
 	if (ctl_min & ~ctl)
-		return -1;
+		return -EIO;
 
 	*result = ctl;
 	return 0;
 }
 
-static __init int setup_vmcs_config(void)
+static __init int setup_vmcs_config(struct vmcs_config* vmcs_conf)
 {
 	u32 vmx_msr_low, vmx_msr_high;
 	u32 min, opt;
@@ -859,7 +859,7 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PINBASED_CTLS,
 				&_pin_based_exec_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = CPU_BASED_HLT_EXITING |
 	      CPU_BASED_CR8_LOAD_EXITING |
@@ -870,7 +870,7 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
 				&_cpu_based_exec_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = 0;
 #ifdef CONFIG_X86_64
@@ -879,37 +879,37 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
 				&_vmentry_control) < 0)
-		return -1;
+		return -EIO;
 
 	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
 
 	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
 	if ((vmx_msr_high & 0x1fff) > PAGE_SIZE)
-		return -1;
+		return -EIO;
 
 #ifdef CONFIG_X86_64
 	/* IA-32 SDM Vol 3B: 64-bit CPUs always have
VMX_BASIC_MSR[48]==0. */
 	if (vmx_msr_high & (1u<<16))
-		return -1;
+		return -EIO;
 #endif
 
 	/* Require Write-Back (WB) memory type for VMCS accesses. */
 	if (((vmx_msr_high >> 18) & 15) != 6)
-		return -1;
+		return -EIO;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
+	vmcs_conf->size = vmx_msr_high & 0x1fff;
+	vmcs_conf->order = get_order(vmcs_config.size);
+	vmcs_conf->revision_id = vmx_msr_low;
 
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
+	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;
+	vmcs_conf->vmexit_ctrl         = _vmexit_control;
+	vmcs_conf->vmentry_ctrl        = _vmentry_control;
 
 	return 0;
 }
@@ -969,8 +969,8 @@ static __init int alloc_kvm_area(void)
 
 static __init int hardware_setup(void)
 {
-	if (setup_vmcs_config() < 0)
-		return -1;
+	if (setup_vmcs_config(&vmcs_config) < 0)
+		return -EIO;
 	return alloc_kvm_area();
 }
 
@@ -2412,11 +2412,26 @@ free_vcpu:
 	return ERR_PTR(err);
 }
 
+void __init vmx_check_processor_compat(void *rtn)
+{
+	struct vmcs_config vmcs_conf;
+
+	*(int *)rtn = 0;
+	if (setup_vmcs_config(&vmcs_conf) < 0)
+		*(int *)rtn = -EIO;
+	if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config))
!= 0) {
+		printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n",
+				smp_processor_id());
+		*(int *)rtn = -EIO;
+	}
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
 	.hardware_setup = hardware_setup,
 	.hardware_unsetup = hardware_unsetup,
+	.check_processor_compatibility = vmx_check_processor_compat,
 	.hardware_enable = hardware_enable,
 	.hardware_disable = hardware_disable,
 
-- 
1.5.2


[-- Attachment #2: Add-cpu-consistence-check-in-vmx.patch --]
[-- Type: application/octet-stream, Size: 6410 bytes --]

From 0b5ecca59c5128990acf0953adb182bb5f700658 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Tue, 31 Jul 2007 17:23:20 +0800
Subject: [PATCH] Add cpu consistence check in vmx.

All the physical CPUs on the board should support the same VMX feature set.
Add check_processor_compatibility to kvm_arch_ops for the consistence
check.
---
 drivers/kvm/kvm.h      |    1 +
 drivers/kvm/kvm_main.c |   10 +++++++++
 drivers/kvm/svm.c      |    6 +++++
 drivers/kvm/vmx.c      |   51 +++++++++++++++++++++++++++++++----------------
 4 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index f78729c..e4f11b6 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -420,6 +420,7 @@ struct kvm_arch_ops {
 	int (*disabled_by_bios)(void);             /* __init */
 	void (*hardware_enable)(void *dummy);      /* __init */
 	void (*hardware_disable)(void *dummy);
+	void (*check_processor_compatibility)(void *rtn);
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 4fd2074..1811ed4 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -3095,6 +3095,7 @@ int kvm_init_arch(struct kvm_arch_ops *ops, unsigned int vcpu_size,
 		  struct module *module)
 {
 	int r;
+	int cpu;
 
 	if (kvm_arch_ops) {
 		printk(KERN_ERR "kvm: already loaded the other module\n");
@@ -3116,6 +3117,14 @@ int kvm_init_arch(struct kvm_arch_ops *ops, unsigned int vcpu_size,
 	if (r < 0)
 		goto out;
 
+	for_each_online_cpu(cpu) {
+		smp_call_function_single(cpu,
+				kvm_arch_ops->check_processor_compatibility,
+				&r, 0, 1);
+		if (r < 0)
+			goto out_free_0;
+	}
+
 	on_each_cpu(hardware_enable, NULL, 0, 1);
 	r = register_cpu_notifier(&kvm_cpu_notifier);
 	if (r)
@@ -3162,6 +3171,7 @@ out_free_2:
 	unregister_cpu_notifier(&kvm_cpu_notifier);
 out_free_1:
 	on_each_cpu(hardware_disable, NULL, 0, 1);
+out_free_0:
 	kvm_arch_ops->hardware_unsetup();
 out:
 	kvm_arch_ops = NULL;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 5277084..827bc27 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1798,11 +1798,17 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall)
 	hypercall[3] = 0xc3;
 }
 
+static void svm_check_processor_compat(void *rtn)
+{
+	*(int *)rtn = 0;
+}
+
 static struct kvm_arch_ops svm_arch_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
 	.hardware_setup = svm_hardware_setup,
 	.hardware_unsetup = svm_hardware_unsetup,
+	.check_processor_compatibility = svm_check_processor_compat,
 	.hardware_enable = svm_hardware_enable,
 	.hardware_disable = svm_hardware_disable,
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 6e23600..fccaf35 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -840,13 +840,13 @@ static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
 
 	/* Ensure minimum (required) set of control bits are supported. */
 	if (ctl_min & ~ctl)
-		return -1;
+		return -EIO;
 
 	*result = ctl;
 	return 0;
 }
 
-static __init int setup_vmcs_config(void)
+static __init int setup_vmcs_config(struct vmcs_config* vmcs_conf)
 {
 	u32 vmx_msr_low, vmx_msr_high;
 	u32 min, opt;
@@ -859,7 +859,7 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PINBASED_CTLS,
 				&_pin_based_exec_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = CPU_BASED_HLT_EXITING |
 	      CPU_BASED_CR8_LOAD_EXITING |
@@ -870,7 +870,7 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
 				&_cpu_based_exec_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = 0;
 #ifdef CONFIG_X86_64
@@ -879,37 +879,37 @@ static __init int setup_vmcs_config(void)
 	opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
-		return -1;
+		return -EIO;
 
 	min = opt = 0;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
 				&_vmentry_control) < 0)
-		return -1;
+		return -EIO;
 
 	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
 
 	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
 	if ((vmx_msr_high & 0x1fff) > PAGE_SIZE)
-		return -1;
+		return -EIO;
 
 #ifdef CONFIG_X86_64
 	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
 	if (vmx_msr_high & (1u<<16))
-		return -1;
+		return -EIO;
 #endif
 
 	/* Require Write-Back (WB) memory type for VMCS accesses. */
 	if (((vmx_msr_high >> 18) & 15) != 6)
-		return -1;
+		return -EIO;
 
-	vmcs_config.size = vmx_msr_high & 0x1fff;
-	vmcs_config.order = get_order(vmcs_config.size);
-	vmcs_config.revision_id = vmx_msr_low;
+	vmcs_conf->size = vmx_msr_high & 0x1fff;
+	vmcs_conf->order = get_order(vmcs_config.size);
+	vmcs_conf->revision_id = vmx_msr_low;
 
-	vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control;
-	vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control;
-	vmcs_config.vmexit_ctrl         = _vmexit_control;
-	vmcs_config.vmentry_ctrl        = _vmentry_control;
+	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
+	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;
+	vmcs_conf->vmexit_ctrl         = _vmexit_control;
+	vmcs_conf->vmentry_ctrl        = _vmentry_control;
 
 	return 0;
 }
@@ -969,8 +969,8 @@ static __init int alloc_kvm_area(void)
 
 static __init int hardware_setup(void)
 {
-	if (setup_vmcs_config() < 0)
-		return -1;
+	if (setup_vmcs_config(&vmcs_config) < 0)
+		return -EIO;
 	return alloc_kvm_area();
 }
 
@@ -2412,11 +2412,26 @@ free_vcpu:
 	return ERR_PTR(err);
 }
 
+void __init vmx_check_processor_compat(void *rtn)
+{
+	struct vmcs_config vmcs_conf;
+
+	*(int *)rtn = 0;
+	if (setup_vmcs_config(&vmcs_conf) < 0)
+		*(int *)rtn = -EIO;
+	if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config)) != 0) {
+		printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n",
+				smp_processor_id());
+		*(int *)rtn = -EIO;
+	}
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
 	.hardware_setup = hardware_setup,
 	.hardware_unsetup = hardware_unsetup,
+	.check_processor_compatibility = vmx_check_processor_compat,
 	.hardware_enable = hardware_enable,
 	.hardware_disable = hardware_disable,
 
-- 
1.5.2


[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] Add cpu consistence check in vmx.

[Avi Kivity]

Yang, Sheng wrote:
> I modified the patch as you suggest.
> For any failure in this due to hardware error, I replace all the error
> return code with -EIO.
>
> Thanks.
> ---
> From: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Tue, 31 Jul 2007 17:23:20 +0800
> Subject: [PATCH] Add cpu consistence check in vmx.
>
> All the physical CPUs on the board should support the same VMX feature
> set.
> Add check_processor_compatibility to kvm_arch_ops for the consistence
> check.
>
>   


Applied, thanks.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/