Re: [PATCH 4/4 v3] KVM: VMX: VMXON/VMXOFF usage changes.

2010-05-11 Thread Avi Kivity 

On 05/07/2010 05:43 AM, Xu, Dongxiao wrote:

From: Dongxiao Xudongxiao...@intel.com

SDM suggests VMXON should be called before VMPTRLD, and VMXOFF
should be called after doing VMCLEAR.

Therefore in vmm coexistence case, we should firstly call VMXON
before any VMCS operation, and then call VMXOFF after the
operation is done.


@@ -4118,7 +4143,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
struct kvm_cpuid_entry2 *best;
struct vcpu_vmx *vmx = to_vmx(vcpu);
u32 exec_control;
+   int cpu;

+   cpu = get_cpu();
+   vmx_vcpu_load(vmx-vcpu, cpu);
vmx-rdtscp_enabled = false;
if (vmx_rdtscp_supported()) {
exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
@@ -4133,6 +4161,9 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
}
}
}
+   vmx_vcpu_put(vmx-vcpu);
+   put_cpu();
+
  }

  static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
   


I'm not sure why this is needed.  vmx_cpuid_update() is called from a 
vcpu ioctl which should have called vcpu_load() before.


Apart from that, everything looks good for merging.

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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/4 v3] KVM: VMX: VMXON/VMXOFF usage changes.

2010-05-11 Thread Xu, Dongxiao 
Avi Kivity wrote:
 On 05/07/2010 05:43 AM, Xu, Dongxiao wrote:
 From: Dongxiao Xudongxiao...@intel.com
 
 SDM suggests VMXON should be called before VMPTRLD, and VMXOFF
 should be called after doing VMCLEAR.
 
 Therefore in vmm coexistence case, we should firstly call VMXON
 before any VMCS operation, and then call VMXOFF after the
 operation is done.
 
 
 @@ -4118,7 +4143,10 @@ static void vmx_cpuid_update(struct kvm_vcpu
  *vcpu) struct kvm_cpuid_entry2 *best;
  struct vcpu_vmx *vmx = to_vmx(vcpu);
  u32 exec_control;
 +int cpu;
 
 +cpu = get_cpu();
 +vmx_vcpu_load(vmx-vcpu, cpu);
  vmx-rdtscp_enabled = false;
  if (vmx_rdtscp_supported()) {
  exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
 @@ -4133,6 +4161,9 @@ static void vmx_cpuid_update(struct kvm_vcpu
  *vcpu)  } }
  }
 +vmx_vcpu_put(vmx-vcpu);
 +put_cpu();
 +
   }
 
   static void vmx_set_supported_cpuid(u32 func, struct
 kvm_cpuid_entry2 *entry) 
 
 
 I'm not sure why this is needed.  vmx_cpuid_update() is called from a
 vcpu ioctl which should have called vcpu_load() before.
 
 Apart from that, everything looks good for merging.

Vcpu_load() and vcpu_put() is not called in that ioctl. I will add that and
send out another version of patchset.

Thanks,
Dongxiao

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4 v3] KVM: VMX: VMXON/VMXOFF usage changes.

2010-05-11 Thread Avi Kivity 

On 05/11/2010 12:38 PM, Xu, Dongxiao wrote:



I'm not sure why this is needed.  vmx_cpuid_update() is called from a
vcpu ioctl which should have called vcpu_load() before.

Apart from that, everything looks good for merging.
 

Vcpu_load() and vcpu_put() is not called in that ioctl. I will add that and
send out another version of patchset.
   


That's a serious bug even before this patchset, since vmx_cpuid_update 
accesses the vmcs.  Please send the fix as a separate patch (which will 
be backported to -stable).


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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/4 v3] KVM: VMX: VMXON/VMXOFF usage changes.

2010-05-11 Thread Xu, Dongxiao 
Avi Kivity wrote:
 On 05/11/2010 12:38 PM, Xu, Dongxiao wrote:
 
 I'm not sure why this is needed.  vmx_cpuid_update() is called from
 a vcpu ioctl which should have called vcpu_load() before.
 
 Apart from that, everything looks good for merging.
 
 Vcpu_load() and vcpu_put() is not called in that ioctl. I will add
 that and send out another version of patchset.
 
 
 That's a serious bug even before this patchset, since vmx_cpuid_update
 accesses the vmcs.  Please send the fix as a separate patch (which
 will be backported to -stable).

That's OK. 

Thanks
Dongxiao--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4 v3] KVM: VMX: VMXON/VMXOFF usage changes.

2010-05-06 Thread Xu, Dongxiao 
From: Dongxiao Xu dongxiao...@intel.com

SDM suggests VMXON should be called before VMPTRLD, and VMXOFF
should be called after doing VMCLEAR.

Therefore in vmm coexistence case, we should firstly call VMXON
before any VMCS operation, and then call VMXOFF after the
operation is done.

Signed-off-by: Dongxiao Xu dongxiao...@intel.com
---
 arch/x86/kvm/vmx.c |   45 ++---
 1 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c536b9d..59d7443 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -168,6 +168,8 @@ static inline struct vcpu_vmx *to_vmx(struct kvm_vcpu *vcpu)
 
 static int init_rmode(struct kvm *kvm);
 static u64 construct_eptp(unsigned long root_hpa);
+static void kvm_cpu_vmxon(u64 addr);
+static void kvm_cpu_vmxoff(void);
 
 static DEFINE_PER_CPU(struct vmcs *, vmxarea);
 static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
@@ -786,8 +788,11 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
u64 tsc_this, delta, new_offset;
+   u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
 
-   if (vmm_exclusive  vcpu-cpu != cpu)
+   if (!vmm_exclusive)
+   kvm_cpu_vmxon(phys_addr);
+   else if (vcpu-cpu != cpu)
vcpu_clear(vmx);
 
if (per_cpu(current_vmcs, cpu) != vmx-vmcs) {
@@ -833,8 +838,10 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 static void vmx_vcpu_put(struct kvm_vcpu *vcpu)
 {
__vmx_load_host_state(to_vmx(vcpu));
-   if (!vmm_exclusive)
+   if (!vmm_exclusive) {
__vcpu_clear(to_vmx(vcpu));
+   kvm_cpu_vmxoff();
+   }
 }
 
 static void vmx_fpu_activate(struct kvm_vcpu *vcpu)
@@ -1257,9 +1264,11 @@ static int hardware_enable(void *garbage)
   FEATURE_CONTROL_LOCKED |
   FEATURE_CONTROL_VMXON_ENABLED);
write_cr4(read_cr4() | X86_CR4_VMXE); /* FIXME: not cpu hotplug safe */
-   kvm_cpu_vmxon(phys_addr);
 
-   ept_sync_global();
+   if (vmm_exclusive) {
+   kvm_cpu_vmxon(phys_addr);
+   ept_sync_global();
+   }
 
return 0;
 }
@@ -1285,8 +1294,10 @@ static void kvm_cpu_vmxoff(void)
 
 static void hardware_disable(void *garbage)
 {
-   vmclear_local_vcpus();
-   kvm_cpu_vmxoff();
+   if (vmm_exclusive) {
+   vmclear_local_vcpus();
+   kvm_cpu_vmxoff();
+   }
write_cr4(read_cr4()  ~X86_CR4_VMXE);
 }
 
@@ -3949,6 +3960,19 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
kmem_cache_free(kvm_vcpu_cache, vmx);
 }
 
+static inline void vmcs_init(struct vmcs *vmcs)
+{
+   u64 phys_addr = __pa(per_cpu(vmxarea, raw_smp_processor_id()));
+
+   if (!vmm_exclusive)
+   kvm_cpu_vmxon(phys_addr);
+
+   vmcs_clear(vmcs);
+
+   if (!vmm_exclusive)
+   kvm_cpu_vmxoff();
+}
+
 static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 {
int err;
@@ -3974,13 +3998,14 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
*kvm, unsigned int id)
if (!vmx-vmcs)
goto free_msrs;
 
-   vmcs_clear(vmx-vmcs);
+   vmcs_init(vmx-vmcs);
 
cpu = get_cpu();
vmx_vcpu_load(vmx-vcpu, cpu);
err = vmx_vcpu_setup(vmx);
vmx_vcpu_put(vmx-vcpu);
put_cpu();
+
if (err)
goto free_vmcs;
if (vm_need_virtualize_apic_accesses(kvm))
@@ -4118,7 +4143,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
struct kvm_cpuid_entry2 *best;
struct vcpu_vmx *vmx = to_vmx(vcpu);
u32 exec_control;
+   int cpu;
 
+   cpu = get_cpu();
+   vmx_vcpu_load(vmx-vcpu, cpu);
vmx-rdtscp_enabled = false;
if (vmx_rdtscp_supported()) {
exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
@@ -4133,6 +4161,9 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
}
}
}
+   vmx_vcpu_put(vmx-vcpu);
+   put_cpu();
+
 }
 
 static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
-- 
1.6.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html