Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

2012-11-29 Thread Marcelo Tosatti
On Thu, Nov 29, 2012 at 07:21:28PM +, Auld, Will wrote:
 Marcelo,
 
 The behavior on reset is to return the TSC_AJUST msr value to 0x0. I am 
 currently initializing this emulated msr in kvm_arch_vcpu_init(). 

Will,

Reset is handled by QEMU. kvm_arch_vcpu_init is only called during vcpu
allocation (ie vm creation). See x86_cpu_reset in QEMU's
target-i386/cpu.c file.

 
 - Behaviour on reset: what is the behaviour on RESET?
 
 I am testing the rebase now. I would like to get any needed changes for this 
 initialization into my next patch set version (v6) if possible.

For the kernel patch, everything is fine. Apparently for the QEMU patch
it is  necessary to set value to zero on reset.

--
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 V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

2012-11-29 Thread Auld, Will
Marcelo, 

My latest QEMU patch ([PATCH V5] target-i386: Enabling IA32_TSC_ADJUST for QEMU 
KVM guest VMs) seems to be OK with regards to reset. The tsc_adjust variable is 
being zeroed by the memset() in x86_cpu_reset(). Later code seems to write 
these values through kvm_put_msr() by way of cpu_synchronize_all_post_reset(). 

Andreas said: I'm expecting this to go through Marcello's queue unless I'm told 
otherwise.

Does this meet your expectations as well?

Thanks,

Will

 -Original Message-
 From: Marcelo Tosatti [mailto:mtosa...@redhat.com]
 Sent: Thursday, November 29, 2012 12:44 PM
 To: Auld, Will
 Cc: kvm@vger.kernel.org; Dugger, Donald D; Liu, Jinsong; Zhang,
 Xiantao; a...@redhat.com; qemu-devel; Gleb
 Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM
 support
 
 On Thu, Nov 29, 2012 at 07:21:28PM +, Auld, Will wrote:
  Marcelo,
 
  The behavior on reset is to return the TSC_AJUST msr value to 0x0. I
 am currently initializing this emulated msr in kvm_arch_vcpu_init().
 
 Will,
 
 Reset is handled by QEMU. kvm_arch_vcpu_init is only called during vcpu
 allocation (ie vm creation). See x86_cpu_reset in QEMU's target-
 i386/cpu.c file.
 
 
  - Behaviour on reset: what is the behaviour on RESET?
 
  I am testing the rebase now. I would like to get any needed changes
 for this initialization into my next patch set version (v6) if
 possible.
 
 For the kernel patch, everything is fine. Apparently for the QEMU patch
 it is  necessary to set value to zero on reset.

--
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 V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

2012-11-27 Thread Will Auld
CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported

Basic design is to emulate the MSR by allowing reads and writes to a guest
vcpu specific location to store the value of the emulated MSR while adding
the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will
be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This
is of course as long as the use TSC counter offsetting VM-execution
control is enabled as well as the IA32_TSC_ADJUST control.

However, because hardware will only return the TSC + IA32_TSC_ADJUST + vmsc
tsc_offset for a guest process when it does and rdtsc (with the correct
settings) the value of our virtualized IA32_TSC_ADJUST must be stored in
one of these three locations. The argument against storing it in the actual
MSR is performance. This is likely to be seldom used while the save/restore
is required on every transition. IA32_TSC_ADJUST was created as a way to
solve some issues with writing TSC itself so that is not an option either.
The remaining option, defined above as our solution has the problem of
returning incorrect vmcs tsc_offset values (unless we intercept and fix, not
done here) as mentioned above. However, more problematic is that storing the
data in vmcs tsc_offset will have a different semantic effect on the system
than does using the actual MSR. This is illustrated in the following example:
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest
process performs a rdtsc. In this case the guest process will get TSC +
IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including IA32_TSC_ADJUST_guest.
While the total system semantics changed the semantics as seen by the guest
do not and hence this will not cause a problem.

Signed-off-by: Will Auld will.a...@intel.com
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c  |  2 ++
 arch/x86/kvm/cpuid.h  |  8 
 arch/x86/kvm/svm.c|  7 +++
 arch/x86/kvm/vmx.c|  9 +
 arch/x86/kvm/x86.c| 22 ++
 8 files changed, 53 insertions(+)

diff --git a/arch/x86/include/asm/cpufeature.h 
b/arch/x86/include/asm/cpufeature.h
index 6b7ee5f..e574d81 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x0007:0 (ebx), word 9 */
 #define X86_FEATURE_FSGSBASE   (9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b */
 #define X86_FEATURE_BMI1   (9*32+ 3) /* 1st group bit manipulation 
extensions */
 #define X86_FEATURE_HLE(9*32+ 4) /* Hardware Lock Elision */
 #define X86_FEATURE_AVX2   (9*32+ 5) /* AVX2 instructions */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index da34027..cf8c7e0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
u32 virtual_tsc_mult;
u32 virtual_tsc_khz;
 
+   s64 ia32_tsc_adjust_msr;
+
atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
unsigned nmi_pending; /* NMI queued after currently running handler */
bool nmi_injected;/* Trying to inject an NMI this entry */
@@ -690,6 +692,7 @@ struct kvm_x86_ops {
bool (*has_wbinvd_exit)(void);
 
void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool 
scale);
+   u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 957ec87..6486569 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON0x002a
 #define MSR_EBC_FREQUENCY_ID   0x002c
 #define MSR_IA32_FEATURE_CONTROL0x003a
+#define MSR_IA32_TSC_ADJUST 0x003b
 
 #define FEATURE_CONTROL_LOCKED (10)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX   (11)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0595f13..e817bac 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 
function,
if (index == 0) {
entry-ebx = kvm_supported_word9_x86_features;
cpuid_mask(entry-ebx, 9);
+   // TSC_ADJUST is emulated 
+   entry-ebx |= F(TSC_ADJUST);
} else
entry-ebx = 0;
entry-eax = 0;
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 

Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

2012-11-27 Thread Marcelo Tosatti

Hi Will,

On Tue, Nov 27, 2012 at 11:09:00AM -0800, Will Auld wrote:
 CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
 
 Basic design is to emulate the MSR by allowing reads and writes to a guest
 vcpu specific location to store the value of the emulated MSR while adding
 the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will
 be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This
 is of course as long as the use TSC counter offsetting VM-execution
 control is enabled as well as the IA32_TSC_ADJUST control.
 
 However, because hardware will only return the TSC + IA32_TSC_ADJUST + vmsc
 tsc_offset for a guest process when it does and rdtsc (with the correct
 settings) the value of our virtualized IA32_TSC_ADJUST must be stored in
 one of these three locations. The argument against storing it in the actual
 MSR is performance. This is likely to be seldom used while the save/restore
 is required on every transition. IA32_TSC_ADJUST was created as a way to
 solve some issues with writing TSC itself so that is not an option either.
 The remaining option, defined above as our solution has the problem of
 returning incorrect vmcs tsc_offset values (unless we intercept and fix, not
 done here) as mentioned above. However, more problematic is that storing the
 data in vmcs tsc_offset will have a different semantic effect on the system
 than does using the actual MSR. This is illustrated in the following example:
 The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest
 process performs a rdtsc. In this case the guest process will get TSC +
 IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including IA32_TSC_ADJUST_guest.
 While the total system semantics changed the semantics as seen by the guest
 do not and hence this will not cause a problem.
 
 Signed-off-by: Will Auld will.a...@intel.com
 ---
  arch/x86/include/asm/cpufeature.h |  1 +
  arch/x86/include/asm/kvm_host.h   |  3 +++
  arch/x86/include/asm/msr-index.h  |  1 +
  arch/x86/kvm/cpuid.c  |  2 ++
  arch/x86/kvm/cpuid.h  |  8 
  arch/x86/kvm/svm.c|  7 +++
  arch/x86/kvm/vmx.c|  9 +
  arch/x86/kvm/x86.c| 22 ++
  8 files changed, 53 insertions(+)
 
 diff --git a/arch/x86/include/asm/cpufeature.h 
 b/arch/x86/include/asm/cpufeature.h
 index 6b7ee5f..e574d81 100644
 --- a/arch/x86/include/asm/cpufeature.h
 +++ b/arch/x86/include/asm/cpufeature.h
 @@ -199,6 +199,7 @@
  
  /* Intel-defined CPU features, CPUID level 0x0007:0 (ebx), word 9 */
  #define X86_FEATURE_FSGSBASE (9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
 +#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b */
  #define X86_FEATURE_BMI1 (9*32+ 3) /* 1st group bit manipulation 
 extensions */
  #define X86_FEATURE_HLE  (9*32+ 4) /* Hardware Lock Elision */
  #define X86_FEATURE_AVX2 (9*32+ 5) /* AVX2 instructions */
 diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
 index da34027..cf8c7e0 100644
 --- a/arch/x86/include/asm/kvm_host.h
 +++ b/arch/x86/include/asm/kvm_host.h
 @@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
   u32 virtual_tsc_mult;
   u32 virtual_tsc_khz;
  
 + s64 ia32_tsc_adjust_msr;
 +
   atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
   unsigned nmi_pending; /* NMI queued after currently running handler */
   bool nmi_injected;/* Trying to inject an NMI this entry */
 @@ -690,6 +692,7 @@ struct kvm_x86_ops {
   bool (*has_wbinvd_exit)(void);
  
   void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool 
 scale);
 + u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
   void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
  
   u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
 diff --git a/arch/x86/include/asm/msr-index.h 
 b/arch/x86/include/asm/msr-index.h
 index 957ec87..6486569 100644
 --- a/arch/x86/include/asm/msr-index.h
 +++ b/arch/x86/include/asm/msr-index.h
 @@ -231,6 +231,7 @@
  #define MSR_IA32_EBL_CR_POWERON  0x002a
  #define MSR_EBC_FREQUENCY_ID 0x002c
  #define MSR_IA32_FEATURE_CONTROL0x003a
 +#define MSR_IA32_TSC_ADJUST 0x003b
  
  #define FEATURE_CONTROL_LOCKED   (10)
  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX (11)
 diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
 index 0595f13..e817bac 100644
 --- a/arch/x86/kvm/cpuid.c
 +++ b/arch/x86/kvm/cpuid.c
 @@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, 
 u32 function,
   if (index == 0) {
   entry-ebx = kvm_supported_word9_x86_features;
   cpuid_mask(entry-ebx, 9);
 + // TSC_ADJUST is emulated 
 + entry-ebx |= F(TSC_ADJUST);
   } else
 

RE: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

2012-11-27 Thread Auld, Will
Thanks Marcelo, 

I'll address these items.

- Please rebase against queue branch on kvm.git.

I am not sure how to do this. The repository I have been working against is:

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

I assume that I need to change this in some way but I am not sure what I need 
to do. Will you explain or give me the needed path?

Thanks,

Will

 -Original Message-
 From: Marcelo Tosatti [mailto:mtosa...@redhat.com]
 Sent: Tuesday, November 27, 2012 5:00 PM
 To: Auld, Will
 Cc: kvm@vger.kernel.org; Dugger, Donald D; Liu, Jinsong; Zhang,
 Xiantao; a...@redhat.com; qemu-devel; Gleb
 Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM
 support
 
 
 Hi Will,
 
 On Tue, Nov 27, 2012 at 11:09:00AM -0800, Will Auld wrote:
  CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
 
  Basic design is to emulate the MSR by allowing reads and writes to a
  guest vcpu specific location to store the value of the emulated MSR
  while adding the value to the vmcs tsc_offset. In this way the
  IA32_TSC_ADJUST value will be included in all reads to the TSC MSR
  whether through rdmsr or rdtsc. This is of course as long as the use
  TSC counter offsetting VM-execution control is enabled as well as
 the IA32_TSC_ADJUST control.
 
  However, because hardware will only return the TSC + IA32_TSC_ADJUST
 +
  vmsc tsc_offset for a guest process when it does and rdtsc (with the
  correct
  settings) the value of our virtualized IA32_TSC_ADJUST must be stored
  in one of these three locations. The argument against storing it in
  the actual MSR is performance. This is likely to be seldom used while
  the save/restore is required on every transition. IA32_TSC_ADJUST was
  created as a way to solve some issues with writing TSC itself so that
 is not an option either.
  The remaining option, defined above as our solution has the problem
 of
  returning incorrect vmcs tsc_offset values (unless we intercept and
  fix, not done here) as mentioned above. However, more problematic is
  that storing the data in vmcs tsc_offset will have a different
  semantic effect on the system than does using the actual MSR. This is
 illustrated in the following example:
  The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a
  guest process performs a rdtsc. In this case the guest process will
  get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including
 IA32_TSC_ADJUST_guest.
  While the total system semantics changed the semantics as seen by the
  guest do not and hence this will not cause a problem.
 
  Signed-off-by: Will Auld will.a...@intel.com
  ---
   arch/x86/include/asm/cpufeature.h |  1 +
   arch/x86/include/asm/kvm_host.h   |  3 +++
   arch/x86/include/asm/msr-index.h  |  1 +
   arch/x86/kvm/cpuid.c  |  2 ++
   arch/x86/kvm/cpuid.h  |  8 
   arch/x86/kvm/svm.c|  7 +++
   arch/x86/kvm/vmx.c|  9 +
   arch/x86/kvm/x86.c| 22 ++
   8 files changed, 53 insertions(+)
 
  diff --git a/arch/x86/include/asm/cpufeature.h
  b/arch/x86/include/asm/cpufeature.h
  index 6b7ee5f..e574d81 100644
  --- a/arch/x86/include/asm/cpufeature.h
  +++ b/arch/x86/include/asm/cpufeature.h
  @@ -199,6 +199,7 @@
 
   /* Intel-defined CPU features, CPUID level 0x0007:0 (ebx), word
 9 */
   #define X86_FEATURE_FSGSBASE   (9*32+ 0) /* {RD/WR}{FS/GS}BASE
 instructions*/
  +#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b
  +*/
   #define X86_FEATURE_BMI1   (9*32+ 3) /* 1st group bit manipulation
 extensions */
   #define X86_FEATURE_HLE(9*32+ 4) /* Hardware Lock Elision
 */
   #define X86_FEATURE_AVX2   (9*32+ 5) /* AVX2 instructions */
  diff --git a/arch/x86/include/asm/kvm_host.h
  b/arch/x86/include/asm/kvm_host.h index da34027..cf8c7e0 100644
  --- a/arch/x86/include/asm/kvm_host.h
  +++ b/arch/x86/include/asm/kvm_host.h
  @@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
  u32 virtual_tsc_mult;
  u32 virtual_tsc_khz;
 
  +   s64 ia32_tsc_adjust_msr;
  +
  atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
  unsigned nmi_pending; /* NMI queued after currently running
 handler */
  bool nmi_injected;/* Trying to inject an NMI this entry */
  @@ -690,6 +692,7 @@ struct kvm_x86_ops {
  bool (*has_wbinvd_exit)(void);
 
  void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
  scale);
  +   u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
  void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
  u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
  diff --git a/arch/x86/include/asm/msr-index.h
  b/arch/x86/include/asm/msr-index.h
  index 957ec87..6486569 100644
  --- a/arch/x86/include/asm/msr-index.h
  +++ b/arch/x86/include/asm/msr-index.h
  @@ -231,6 +231,7 @@
   #define MSR_IA32_EBL_CR_POWERON0x002a
   #define MSR_EBC_FREQUENCY_ID