Re: [Qemu-devel] [PATCH] i386: wire up MSR_IA32_MISC_ENABLE

[Jan Kiszka]

On 2011-10-04 16:26, Avi Kivity wrote:
> It's needed for its default value - bit 0 specifies that "rep movs" is
> good enough for memcpy, and Linux may use a slower memcpu if it is not set,
> depending on cpu family/model.
> 
> Signed-off-by: Avi Kivity <address@hidden>
> ---
>  target-i386/cpu.h       |    5 +++++
>  target-i386/helper.c    |    1 +
>  target-i386/kvm.c       |   15 +++++++++++++++
>  target-i386/machine.c   |   21 +++++++++++++++++++++
>  target-i386/op_helper.c |    6 ++++++
>  5 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index ae36489..5416809 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -299,6 +299,10 @@
>  
>  #define MSR_IA32_PERF_STATUS            0x198
>  
> +#define MSR_IA32_MISC_ENABLE         0x1a0

I smell tabs...

> +/* Indicates good rep/movs microcode on some processors: */
> +#define MSR_IA32_MISC_ENABLE_DEFAULT    1
> +
>  #define MSR_MTRRphysBase(reg)                (0x200 + 2 * (reg))
>  #define MSR_MTRRphysMask(reg)                (0x200 + 2 * (reg) + 1)
>  
> @@ -689,6 +693,7 @@ typedef struct CPUX86State {
>      uint64_t tsc;
>  
>      uint64_t mcg_status;
> +    uint64_t msr_ia32_misc_enable;
>  
>      /* exception/interrupt handling */
>      int error_code;
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 5df40d4..6c6a167 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -98,6 +98,7 @@ void cpu_reset(CPUX86State *env)
>      env->mxcsr = 0x1f80;
>  
>      env->pat = 0x0007040600070406ULL;
> +    env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
>  
>      memset(env->dr, 0, sizeof(env->dr));
>      env->dr[6] = DR6_FIXED_1;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index b6eef04..b3a650b 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -60,6 +60,7 @@
>  static bool has_msr_star;
>  static bool has_msr_hsave_pa;
>  static bool has_msr_async_pf_en;
> +static bool has_msr_misc_enable;
>  static int lm_capable_kernel;
>  
>  static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
> @@ -568,6 +569,10 @@ static int kvm_get_supported_msrs(KVMState *s)
>                      has_msr_hsave_pa = true;
>                      continue;
>                  }
> +                if (kvm_msr_list->indices[i] == MSR_IA32_MISC_ENABLE) {
> +                    has_msr_misc_enable = true;
> +                    continue;
> +                }
>              }
>          }
>  
> @@ -881,6 +886,10 @@ static int kvm_put_msrs(CPUState *env, int level)
>      if (has_msr_hsave_pa) {
>          kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
>      }
> +    if (has_msr_misc_enable) {
> +        kvm_msr_entry_set(&msrs[n++], MSR_IA32_MISC_ENABLE,
> +                          env->msr_ia32_misc_enable);
> +    }
>  #ifdef TARGET_X86_64
>      if (lm_capable_kernel) {
>          kvm_msr_entry_set(&msrs[n++], MSR_CSTAR, env->cstar);
> @@ -1127,6 +1136,9 @@ static int kvm_get_msrs(CPUState *env)
>      if (has_msr_hsave_pa) {
>          msrs[n++].index = MSR_VM_HSAVE_PA;
>      }
> +    if (has_msr_misc_enable) {
> +        msrs[n++].index = MSR_IA32_MISC_ENABLE;
> +    }
>  
>      if (!env->tsc_valid) {
>          msrs[n++].index = MSR_IA32_TSC;
> @@ -1210,6 +1222,9 @@ static int kvm_get_msrs(CPUState *env)
>          case MSR_MCG_CTL:
>              env->mcg_ctl = msrs[i].data;
>              break;
> +        case MSR_IA32_MISC_ENABLE:
> +            env->msr_ia32_misc_enable = msrs[i].data;
> +            break;
>          default:
>              if (msrs[i].index >= MSR_MC0_CTL &&
>                  msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 9aca8e0..73c1ffe 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -310,6 +310,24 @@ static bool fpop_ip_dp_needed(void *opaque)
>      }
>  };
>  
> +static bool misc_enable_needed(void *opaque)
> +{
> +    CPUState *env = opaque;
> +
> +    return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> +}
> +
> +static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> +    .name = "cpu/msr_ia32_misc_enable",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField []) {
> +        VMSTATE_UINT64(msr_ia32_misc_enable, CPUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +

We are about to bump the CPU_SAVE_VERSION for the sake of APIC deadline
timer, so you can jump on that train and avoid this subsection.

>  static const VMStateDescription vmstate_cpu = {
>      .name = "cpu",
>      .version_id = CPU_SAVE_VERSION,
> @@ -420,6 +438,9 @@ static bool fpop_ip_dp_needed(void *opaque)
>          } , {
>              .vmsd = &vmstate_fpop_ip_dp,
>              .needed = fpop_ip_dp_needed,
> +        }, {
> +            .vmsd = &vmstate_msr_ia32_misc_enable,
> +            .needed = misc_enable_needed,
>          } , {
>              /* empty */
>          }
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index 3bb5a91..c89e4a4 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -3280,6 +3280,9 @@ void helper_wrmsr(void)
>      case MSR_TSC_AUX:
>          env->tsc_aux = val;
>          break;
> +    case MSR_IA32_MISC_ENABLE:
> +        env->msr_ia32_misc_enable = val;
> +        break;

This MSR is Intel-specific, isn't it? Then I guess it should be limited
to Intel CPU types.

>      default:
>          if ((uint32_t)ECX >= MSR_MC0_CTL
>              && (uint32_t)ECX < MSR_MC0_CTL + (4 * env->mcg_cap & 0xff)) {
> @@ -3413,6 +3416,9 @@ void helper_rdmsr(void)
>      case MSR_MCG_STATUS:
>          val = env->mcg_status;
>          break;
> +    case MSR_IA32_MISC_ENABLE:
> +        val = env->msr_ia32_misc_enable;
> +        break;
>      default:
>          if ((uint32_t)ECX >= MSR_MC0_CTL
>              && (uint32_t)ECX < MSR_MC0_CTL + (4 * env->mcg_cap & 0xff)) {

Jan

signature.asc
Description: OpenPGP digital signature