Patchwork [V2] Resend - Enabling IA32_TSC_ADJUST for Qemu KVM guest VMs

login
register
mail settings
Submitter Will Auld
Date Nov. 26, 2012, 6:42 p.m.
Message ID <1353955371.5921.0.camel@WillAuldHomeLinux>
Download mbox | patch
Permalink /patch/202002/
State New
Headers show

Comments

Will Auld - Nov. 26, 2012, 6:42 p.m.
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 the
hypervisor vcpu specific locations to store the value of the emulated MSRs.
In this way the IA32_TSC_ADJUST value will be included in all reads to
the TSC MSR whether through rdmsr or rdtsc.

As this is a new MSR that the guest may access and modify its value needs
to be migrated along with the other MRSs. The changes here are specifically
for recognizing when IA32_TSC_ADJUST is enabled in CPUID and code added
for migrating its value.

Signed-off-by: Will Auld <will.auld@intel.com>
---
 target-i386/cpu.h     |  2 ++
 target-i386/kvm.c     | 15 +++++++++++++++
 target-i386/machine.c | 21 +++++++++++++++++++++
 3 files changed, 38 insertions(+)
Andreas Färber - Nov. 26, 2012, 7:05 p.m.
Hello,

Am 26.11.2012 19:42, schrieb 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 the
> hypervisor vcpu specific locations to store the value of the emulated MSRs.
> In this way the IA32_TSC_ADJUST value will be included in all reads to
> the TSC MSR whether through rdmsr or rdtsc.
> 
> As this is a new MSR that the guest may access and modify its value needs
> to be migrated along with the other MRSs. The changes here are specifically
> for recognizing when IA32_TSC_ADJUST is enabled in CPUID and code added
> for migrating its value.
> 
> Signed-off-by: Will Auld <will.auld@intel.com>

$subject should get a prefix of "target-i386: " and "resend" is better
used inside a tag so that it doesn't end up in the commit.
And it's "QEMU". ;)

Some more stylistic issues inline:

> ---
>  target-i386/cpu.h     |  2 ++
>  target-i386/kvm.c     | 15 +++++++++++++++
>  target-i386/machine.c | 21 +++++++++++++++++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index aabf993..13d4152 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -284,6 +284,7 @@
>  #define MSR_IA32_APICBASE_BSP           (1<<8)
>  #define MSR_IA32_APICBASE_ENABLE        (1<<11)
>  #define MSR_IA32_APICBASE_BASE          (0xfffff<<12)
> +#define MSR_TSC_ADJUST	 		0x0000003b

Tabs. You can use scripts/checkpatch.pl to verify.

>  #define MSR_IA32_TSCDEADLINE            0x6e0
>  
>  #define MSR_MTRRcap			0xfe
> @@ -701,6 +702,7 @@ typedef struct CPUX86State {
>      uint64_t async_pf_en_msr;
>  
>      uint64_t tsc;
> +    uint64_t tsc_adjust;
>      uint64_t tsc_deadline;
>  
>      uint64_t mcg_status;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 696b14a..e974c42 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -61,6 +61,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  
>  static bool has_msr_star;
>  static bool has_msr_hsave_pa;
> +static bool has_msr_tsc_adjust;
>  static bool has_msr_tsc_deadline;
>  static bool has_msr_async_pf_en;
>  static bool has_msr_misc_enable;
> @@ -641,6 +642,10 @@ static int kvm_get_supported_msrs(KVMState *s)
>                      has_msr_hsave_pa = true;
>                      continue;
>                  }
> +                if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
> +                    has_msr_tsc_adjust = true;
> +                    continue;
> +                }
>                  if (kvm_msr_list->indices[i] == MSR_IA32_TSCDEADLINE) {
>                      has_msr_tsc_deadline = true;
>                      continue;
> @@ -978,6 +983,10 @@ static int kvm_put_msrs(CPUX86State *env, int level)
>      if (has_msr_hsave_pa) {
>          kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
>      }
> +    if (has_msr_tsc_adjust) {
> +        kvm_msr_entry_set(&msrs[n++], 
> +			MSR_TSC_ADJUST, env->tsc_adjust);

Tabs.

> +    }
>      if (has_msr_tsc_deadline) {
>          kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSCDEADLINE, env->tsc_deadline);
>      }
> @@ -1234,6 +1243,9 @@ static int kvm_get_msrs(CPUX86State *env)
>      if (has_msr_hsave_pa) {
>          msrs[n++].index = MSR_VM_HSAVE_PA;
>      }
> +    if (has_msr_tsc_adjust) {
> +        msrs[n++].index = MSR_TSC_ADJUST;
> +    }
>      if (has_msr_tsc_deadline) {
>          msrs[n++].index = MSR_IA32_TSCDEADLINE;
>      }
> @@ -1308,6 +1320,9 @@ static int kvm_get_msrs(CPUX86State *env)
>          case MSR_IA32_TSC:
>              env->tsc = msrs[i].data;
>              break;
> +        case MSR_TSC_ADJUST:
> +            env->tsc_adjust = msrs[i].data;
> +            break;
>          case MSR_IA32_TSCDEADLINE:
>              env->tsc_deadline = msrs[i].data;
>              break;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a8be058..95bda9b 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -310,6 +310,24 @@ static const VMStateDescription vmstate_fpop_ip_dp = {
>      }
>  };
>  
> +static bool tsc_adjust_needed(void *opaque)
> +{
> +    CPUX86State *cpu = opaque;

Please name this "env" to differentiate from CPUState / X86CPU.
Since there are other tsc_* fields already I won't request that you move
your new field to the containing X86CPU struct but at some point we will
need to convert the VMSDs to X86CPU.

> +
> +    return cpu->tsc_adjust != 0;
> +}
> +
> +static const VMStateDescription vmstate_msr_tsc_adjust = {
> +    .name = "cpu/msr_tsc_adjust",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField []) {
> +        VMSTATE_UINT64(tsc_adjust, CPUX86State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static bool tscdeadline_needed(void *opaque)
>  {
>      CPUX86State *env = opaque;
> @@ -457,6 +475,9 @@ static const VMStateDescription vmstate_cpu = {
>              .vmsd = &vmstate_fpop_ip_dp,
>              .needed = fpop_ip_dp_needed,
>          }, {
> +            .vmsd = &vmstate_msr_tsc_adjust,
> +            .needed = tsc_adjust_needed,
> +        }, {
>              .vmsd = &vmstate_msr_tscdeadline,
>              .needed = tscdeadline_needed,
>          }, {

Otherwise looks okay to me. I'm expecting this to go through Marcello's
queue unless I'm told otherwise.

Regards,
Andreas
Auld, Will - Nov. 27, 2012, 1:42 a.m.
Andreas, 

Thanks. I just sent the update patch (V3) to address your comments. 

Will

> -----Original Message-----

> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On

> Behalf Of Andreas Färber

> Sent: Monday, November 26, 2012 11:05 AM

> To: Auld, Will

> Cc: Will Auld; qemu-devel; Gleb; Marcelo Tosatti; kvm@vger.kernel.org;

> Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; avi@redhat.com

> Subject: Re: [Qemu-devel] [PATCH V2] Resend - Enabling IA32_TSC_ADJUST

> for Qemu KVM guest VMs

> 

> Hello,

> 

> Am 26.11.2012 19:42, schrieb 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

> the

> > hypervisor vcpu specific locations to store the value of the emulated

> MSRs.

> > In this way the IA32_TSC_ADJUST value will be included in all reads

> to

> > the TSC MSR whether through rdmsr or rdtsc.

> >

> > As this is a new MSR that the guest may access and modify its value

> > needs to be migrated along with the other MRSs. The changes here are

> > specifically for recognizing when IA32_TSC_ADJUST is enabled in CPUID

> > and code added for migrating its value.

> >

> > Signed-off-by: Will Auld <will.auld@intel.com>

> 

> $subject should get a prefix of "target-i386: " and "resend" is better

> used inside a tag so that it doesn't end up in the commit.

> And it's "QEMU". ;)

> 

> Some more stylistic issues inline:

> 

> > ---

> >  target-i386/cpu.h     |  2 ++

> >  target-i386/kvm.c     | 15 +++++++++++++++

> >  target-i386/machine.c | 21 +++++++++++++++++++++

> >  3 files changed, 38 insertions(+)

> >

> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h index

> > aabf993..13d4152 100644

> > --- a/target-i386/cpu.h

> > +++ b/target-i386/cpu.h

> > @@ -284,6 +284,7 @@

> >  #define MSR_IA32_APICBASE_BSP           (1<<8)

> >  #define MSR_IA32_APICBASE_ENABLE        (1<<11)

> >  #define MSR_IA32_APICBASE_BASE          (0xfffff<<12)

> > +#define MSR_TSC_ADJUST	 		0x0000003b

> 

> Tabs. You can use scripts/checkpatch.pl to verify.

> 

> >  #define MSR_IA32_TSCDEADLINE            0x6e0

> >

> >  #define MSR_MTRRcap			0xfe

> > @@ -701,6 +702,7 @@ typedef struct CPUX86State {

> >      uint64_t async_pf_en_msr;

> >

> >      uint64_t tsc;

> > +    uint64_t tsc_adjust;

> >      uint64_t tsc_deadline;

> >

> >      uint64_t mcg_status;

> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c index

> > 696b14a..e974c42 100644

> > --- a/target-i386/kvm.c

> > +++ b/target-i386/kvm.c

> > @@ -61,6 +61,7 @@ const KVMCapabilityInfo

> > kvm_arch_required_capabilities[] = {

> >

> >  static bool has_msr_star;

> >  static bool has_msr_hsave_pa;

> > +static bool has_msr_tsc_adjust;

> >  static bool has_msr_tsc_deadline;

> >  static bool has_msr_async_pf_en;

> >  static bool has_msr_misc_enable;

> > @@ -641,6 +642,10 @@ static int kvm_get_supported_msrs(KVMState *s)

> >                      has_msr_hsave_pa = true;

> >                      continue;

> >                  }

> > +                if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {

> > +                    has_msr_tsc_adjust = true;

> > +                    continue;

> > +                }

> >                  if (kvm_msr_list->indices[i] ==

> MSR_IA32_TSCDEADLINE) {

> >                      has_msr_tsc_deadline = true;

> >                      continue;

> > @@ -978,6 +983,10 @@ static int kvm_put_msrs(CPUX86State *env, int

> level)

> >      if (has_msr_hsave_pa) {

> >          kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env-

> >vm_hsave);

> >      }

> > +    if (has_msr_tsc_adjust) {

> > +        kvm_msr_entry_set(&msrs[n++],

> > +			MSR_TSC_ADJUST, env->tsc_adjust);

> 

> Tabs.

> 

> > +    }

> >      if (has_msr_tsc_deadline) {

> >          kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSCDEADLINE, env-

> >tsc_deadline);

> >      }

> > @@ -1234,6 +1243,9 @@ static int kvm_get_msrs(CPUX86State *env)

> >      if (has_msr_hsave_pa) {

> >          msrs[n++].index = MSR_VM_HSAVE_PA;

> >      }

> > +    if (has_msr_tsc_adjust) {

> > +        msrs[n++].index = MSR_TSC_ADJUST;

> > +    }

> >      if (has_msr_tsc_deadline) {

> >          msrs[n++].index = MSR_IA32_TSCDEADLINE;

> >      }

> > @@ -1308,6 +1320,9 @@ static int kvm_get_msrs(CPUX86State *env)

> >          case MSR_IA32_TSC:

> >              env->tsc = msrs[i].data;

> >              break;

> > +        case MSR_TSC_ADJUST:

> > +            env->tsc_adjust = msrs[i].data;

> > +            break;

> >          case MSR_IA32_TSCDEADLINE:

> >              env->tsc_deadline = msrs[i].data;

> >              break;

> > diff --git a/target-i386/machine.c b/target-i386/machine.c index

> > a8be058..95bda9b 100644

> > --- a/target-i386/machine.c

> > +++ b/target-i386/machine.c

> > @@ -310,6 +310,24 @@ static const VMStateDescription

> vmstate_fpop_ip_dp = {

> >      }

> >  };

> >

> > +static bool tsc_adjust_needed(void *opaque) {

> > +    CPUX86State *cpu = opaque;

> 

> Please name this "env" to differentiate from CPUState / X86CPU.

> Since there are other tsc_* fields already I won't request that you

> move your new field to the containing X86CPU struct but at some point

> we will need to convert the VMSDs to X86CPU.

> 

> > +

> > +    return cpu->tsc_adjust != 0;

> > +}

> > +

> > +static const VMStateDescription vmstate_msr_tsc_adjust = {

> > +    .name = "cpu/msr_tsc_adjust",

> > +    .version_id = 1,

> > +    .minimum_version_id = 1,

> > +    .minimum_version_id_old = 1,

> > +    .fields      = (VMStateField []) {

> > +        VMSTATE_UINT64(tsc_adjust, CPUX86State),

> > +        VMSTATE_END_OF_LIST()

> > +    }

> > +};

> > +

> >  static bool tscdeadline_needed(void *opaque)  {

> >      CPUX86State *env = opaque;

> > @@ -457,6 +475,9 @@ static const VMStateDescription vmstate_cpu = {

> >              .vmsd = &vmstate_fpop_ip_dp,

> >              .needed = fpop_ip_dp_needed,

> >          }, {

> > +            .vmsd = &vmstate_msr_tsc_adjust,

> > +            .needed = tsc_adjust_needed,

> > +        }, {

> >              .vmsd = &vmstate_msr_tscdeadline,

> >              .needed = tscdeadline_needed,

> >          }, {

> 

> Otherwise looks okay to me. I'm expecting this to go through Marcello's

> queue unless I'm told otherwise.

> 

> Regards,

> Andreas

> 

> --

> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany

> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

> --

> To unsubscribe from this list: send the line "unsubscribe kvm" in the

> body of a message to majordomo@vger.kernel.org More majordomo info at

> http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index aabf993..13d4152 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -284,6 +284,7 @@ 
 #define MSR_IA32_APICBASE_BSP           (1<<8)
 #define MSR_IA32_APICBASE_ENABLE        (1<<11)
 #define MSR_IA32_APICBASE_BASE          (0xfffff<<12)
+#define MSR_TSC_ADJUST	 		0x0000003b
 #define MSR_IA32_TSCDEADLINE            0x6e0
 
 #define MSR_MTRRcap			0xfe
@@ -701,6 +702,7 @@  typedef struct CPUX86State {
     uint64_t async_pf_en_msr;
 
     uint64_t tsc;
+    uint64_t tsc_adjust;
     uint64_t tsc_deadline;
 
     uint64_t mcg_status;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 696b14a..e974c42 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -61,6 +61,7 @@  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool has_msr_star;
 static bool has_msr_hsave_pa;
+static bool has_msr_tsc_adjust;
 static bool has_msr_tsc_deadline;
 static bool has_msr_async_pf_en;
 static bool has_msr_misc_enable;
@@ -641,6 +642,10 @@  static int kvm_get_supported_msrs(KVMState *s)
                     has_msr_hsave_pa = true;
                     continue;
                 }
+                if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
+                    has_msr_tsc_adjust = true;
+                    continue;
+                }
                 if (kvm_msr_list->indices[i] == MSR_IA32_TSCDEADLINE) {
                     has_msr_tsc_deadline = true;
                     continue;
@@ -978,6 +983,10 @@  static int kvm_put_msrs(CPUX86State *env, int level)
     if (has_msr_hsave_pa) {
         kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
     }
+    if (has_msr_tsc_adjust) {
+        kvm_msr_entry_set(&msrs[n++], 
+			MSR_TSC_ADJUST, env->tsc_adjust);
+    }
     if (has_msr_tsc_deadline) {
         kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSCDEADLINE, env->tsc_deadline);
     }
@@ -1234,6 +1243,9 @@  static int kvm_get_msrs(CPUX86State *env)
     if (has_msr_hsave_pa) {
         msrs[n++].index = MSR_VM_HSAVE_PA;
     }
+    if (has_msr_tsc_adjust) {
+        msrs[n++].index = MSR_TSC_ADJUST;
+    }
     if (has_msr_tsc_deadline) {
         msrs[n++].index = MSR_IA32_TSCDEADLINE;
     }
@@ -1308,6 +1320,9 @@  static int kvm_get_msrs(CPUX86State *env)
         case MSR_IA32_TSC:
             env->tsc = msrs[i].data;
             break;
+        case MSR_TSC_ADJUST:
+            env->tsc_adjust = msrs[i].data;
+            break;
         case MSR_IA32_TSCDEADLINE:
             env->tsc_deadline = msrs[i].data;
             break;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a8be058..95bda9b 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -310,6 +310,24 @@  static const VMStateDescription vmstate_fpop_ip_dp = {
     }
 };
 
+static bool tsc_adjust_needed(void *opaque)
+{
+    CPUX86State *cpu = opaque;
+
+    return cpu->tsc_adjust != 0;
+}
+
+static const VMStateDescription vmstate_msr_tsc_adjust = {
+    .name = "cpu/msr_tsc_adjust",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT64(tsc_adjust, CPUX86State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool tscdeadline_needed(void *opaque)
 {
     CPUX86State *env = opaque;
@@ -457,6 +475,9 @@  static const VMStateDescription vmstate_cpu = {
             .vmsd = &vmstate_fpop_ip_dp,
             .needed = fpop_ip_dp_needed,
         }, {
+            .vmsd = &vmstate_msr_tsc_adjust,
+            .needed = tsc_adjust_needed,
+        }, {
             .vmsd = &vmstate_msr_tscdeadline,
             .needed = tscdeadline_needed,
         }, {