Re: [PATCH 1/2] KVM: CPUID takes ecx as input value for some functions

[Anthony Liguori <aliguori@us.ibm.com>]

Amit Shah wrote:
> The CPUID instruction takes the value of ECX as an input parameter
> in addition to the value of EAX as the count for functions 4, 0xb
> and 0xd. Make sure we pass the value to the instruction.
>
> Also convert to the qemu-style whitespace for the surrounding code.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  qemu/target-i386/cpu.h       |    2 +-
>  qemu/target-i386/helper.c    |   34 +++++++++++++++++-----------------
>  qemu/target-i386/kvm.c       |    8 ++++----
>  qemu/target-i386/op_helper.c |    2 +-
>  4 files changed, 23 insertions(+), 23 deletions(-)
>   

Please resubmit against qemu (not kvm-userspace) and I'll apply.

Regards,

Anthony Liguori

> diff --git a/qemu/target-i386/cpu.h b/qemu/target-i386/cpu.h
> index 944e386..d9834db 100644
> --- a/qemu/target-i386/cpu.h
> +++ b/qemu/target-i386/cpu.h
> @@ -743,7 +743,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
>  int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr,
>                               int is_write, int mmu_idx, int is_softmmu);
>  void cpu_x86_set_a20(CPUX86State *env, int a20_state);
> -void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> +void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                     uint32_t *eax, uint32_t *ebx,
>                     uint32_t *ecx, uint32_t *edx);
>
> diff --git a/qemu/target-i386/helper.c b/qemu/target-i386/helper.c
> index cda0390..4c1e541 100644
> --- a/qemu/target-i386/helper.c
> +++ b/qemu/target-i386/helper.c
> @@ -1377,7 +1377,8 @@ static void breakpoint_handler(CPUState *env)
>  }
>  #endif /* !CONFIG_USER_ONLY */
>
> -static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
> +static void host_cpuid(uint32_t function, uint32_t count,
> +                       uint32_t *eax, uint32_t *ebx,
>                         uint32_t *ecx, uint32_t *edx)
>  {
>  #if defined(CONFIG_KVM) || defined(USE_KVM)
> @@ -1385,19 +1386,19 @@ static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
>
>  #ifdef __x86_64__
>      asm volatile("cpuid"
> -		 : "=a"(vec[0]), "=b"(vec[1]),
> -		   "=c"(vec[2]), "=d"(vec[3])
> -		 : "0"(function) : "cc");
> +                 : "=a"(vec[0]), "=b"(vec[1]),
> +                   "=c"(vec[2]), "=d"(vec[3])
> +                 : "0"(function), "c"(count) : "cc");
>  #else
>      asm volatile("pusha \n\t"
> -		 "cpuid \n\t"
> -		 "mov %%eax, 0(%1) \n\t"
> -		 "mov %%ebx, 4(%1) \n\t"
> -		 "mov %%ecx, 8(%1) \n\t"
> -		 "mov %%edx, 12(%1) \n\t"
> -		 "popa"
> -		 : : "a"(function), "S"(vec)
> -		 : "memory", "cc");
> +                 "cpuid \n\t"
> +                 "mov %%eax, 0(%1) \n\t"
> +                 "mov %%ebx, 4(%1) \n\t"
> +                 "mov %%ecx, 8(%1) \n\t"
> +                 "mov %%edx, 12(%1) \n\t"
> +                 "popa"
> +                 : : "a"(function), "c"(count), "S"(vec)
> +                 : "memory", "cc");
>  #endif
>
>      if (eax)
> @@ -1411,7 +1412,7 @@ static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
>  #endif
>  }
>
> -void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> +void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                     uint32_t *eax, uint32_t *ebx,
>                     uint32_t *ecx, uint32_t *edx)
>  {
> @@ -1436,7 +1437,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>           * actuall cpu, and say goodbye to migration between different vendors
>           * is you use compatibility mode. */
>          if (kvm_enabled())
> -            host_cpuid(0, NULL, ebx, ecx, edx);
> +            host_cpuid(0, 0, NULL, ebx, ecx, edx);
>          break;
>      case 1:
>          *eax = env->cpuid_version;
> @@ -1457,7 +1458,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>          break;
>      case 4:
>          /* cache info: needed for Core compatibility */
> -        switch (*ecx) {
> +        switch (count) {
>              case 0: /* L1 dcache info */
>                  *eax = 0x0000121;
>                  *ebx = 0x1c0003f;
> @@ -1483,7 +1484,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>                  *edx = 0;
>                  break;
>          }
> -
>          break;
>      case 5:
>          /* mwait info: needed for Core compatibility */
> @@ -1528,7 +1528,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>          if (kvm_enabled()) {
>              uint32_t h_eax, h_edx;
>
> -            host_cpuid(0x80000001, &h_eax, NULL, NULL, &h_edx);
> +            host_cpuid(index, 0, &h_eax, NULL, NULL, &h_edx);
>
>              /* disable CPU features that the host does not support */
>
> diff --git a/qemu/target-i386/kvm.c b/qemu/target-i386/kvm.c
> index f87bf36..bef3150 100644
> --- a/qemu/target-i386/kvm.c
> +++ b/qemu/target-i386/kvm.c
> @@ -44,13 +44,13 @@ int kvm_arch_init_vcpu(CPUState *env)
>
>      cpuid_i = 0;
>
> -    cpu_x86_cpuid(env, 0, &eax, &ebx, &ecx, &edx);
> +    cpu_x86_cpuid(env, 0, 0, &eax, &ebx, &ecx, &edx);
>      limit = eax;
>
>      for (i = 0; i <= limit; i++) {
>          struct kvm_cpuid_entry *c = &cpuid_data.entries[cpuid_i++];
>
> -        cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
> +        cpu_x86_cpuid(env, i, 0, &eax, &ebx, &ecx, &edx);
>          c->function = i;
>          c->eax = eax;
>          c->ebx = ebx;
> @@ -58,13 +58,13 @@ int kvm_arch_init_vcpu(CPUState *env)
>          c->edx = edx;
>      }
>
> -    cpu_x86_cpuid(env, 0x80000000, &eax, &ebx, &ecx, &edx);
> +    cpu_x86_cpuid(env, 0x80000000, 0, &eax, &ebx, &ecx, &edx);
>      limit = eax;
>
>      for (i = 0x80000000; i <= limit; i++) {
>          struct kvm_cpuid_entry *c = &cpuid_data.entries[cpuid_i++];
>
> -        cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
> +        cpu_x86_cpuid(env, i, 0, &eax, &ebx, &ecx, &edx);
>          c->function = i;
>          c->eax = eax;
>          c->ebx = ebx;
> diff --git a/qemu/target-i386/op_helper.c b/qemu/target-i386/op_helper.c
> index 6e0e32e..74ba643 100644
> --- a/qemu/target-i386/op_helper.c
> +++ b/qemu/target-i386/op_helper.c
> @@ -1898,7 +1898,7 @@ void helper_cpuid(void)
>
>      helper_svm_check_intercept_param(SVM_EXIT_CPUID, 0);
>
> -    cpu_x86_cpuid(env, (uint32_t)EAX, &eax, &ebx, &ecx, &edx);
> +    cpu_x86_cpuid(env, (uint32_t)EAX, (uint32_t)ECX, &eax, &ebx, &ecx, &edx);
>      EAX = eax;
>      EBX = ebx;
>      ECX = ecx;
>