Patchwork [uq/master,V2] kvm: Add CPUID support for VIA CPU

login
register
mail settings
Submitter BrillyWu
Date May 10, 2011, 8:02 a.m.
Message ID <BANLkTimzTSU7A-b9yc9H35b1szfL4z7H-A@mail.gmail.com>
Download mbox | patch
Permalink /patch/94960/
State New
Headers show

Comments

BrillyWu - May 10, 2011, 8:02 a.m.
From: BrillyWu <brillywu@viatech.com.cn>

When KVM is running on VIA CPU with host cpu's model, the
feautures of VIA CPU will be passed into kvm guest by calling
the CPUID instruction for Centaur.

Signed-off-by: BrillyWu<brillywu@viatech.com.cn>
Signed-off-by: KaryJin<karyjin@viatech.com.cn>
---
 target-i386/cpu.h   |    3 +++
 target-i386/cpuid.c |   46 +++++++++++++++++++++++++++++++++++++++++++++-
 target-i386/kvm.c   |   15 +++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
Avi Kivity - May 11, 2011, 11:50 a.m.
On 05/10/2011 11:02 AM, BrillyWu wrote:
> From: BrillyWu<brillywu@viatech.com.cn>
>
> When KVM is running on VIA CPU with host cpu's model, the
> feautures of VIA CPU will be passed into kvm guest by calling
> the CPUID instruction for Centaur.
>

Applied, thanks.
Jan Kiszka - May 28, 2011, 10:28 a.m.
On 2011-05-10 10:02, BrillyWu wrote:
> From: BrillyWu <brillywu@viatech.com.cn>
> 
> When KVM is running on VIA CPU with host cpu's model, the
> feautures of VIA CPU will be passed into kvm guest by calling
> the CPUID instruction for Centaur.
> 
> Signed-off-by: BrillyWu<brillywu@viatech.com.cn>
> Signed-off-by: KaryJin<karyjin@viatech.com.cn>
> ---
>  target-i386/cpu.h   |    3 +++
>  target-i386/cpuid.c |   46 +++++++++++++++++++++++++++++++++++++++++++++-
>  target-i386/kvm.c   |   15 +++++++++++++++
>  3 files changed, 63 insertions(+), 1 deletion(-)
> 
> --- a/target-i386/cpu.h	2011-05-09 09:55:48.624885001 +0800
> +++ b/target-i386/cpu.h	2011-05-09 09:48:53.704885019 +0800
> @@ -721,6 +721,9 @@ typedef struct CPUX86State {
>      uint32_t cpuid_ext3_features;
>      uint32_t cpuid_apic_id;
>      int cpuid_vendor_override;
> +    /* Store the results of Centaur's CPUID instructions */
> +    uint32_t cpuid_xlevel2;
> +    uint32_t cpuid_ext4_features;
> 
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];
> --- a/target-i386/cpuid.c	2011-05-09 09:31:11.754884991 +0800
> +++ b/target-i386/cpuid.c	2011-05-09 10:18:46.204885008 +0800
> @@ -230,6 +230,9 @@ typedef struct x86_def_t {
>      char model_id[48];
>      int vendor_override;
>      uint32_t flags;
> +    /* Store the results of Centaur's CPUID instructions */
> +    uint32_t ext4_features;
> +    uint32_t xlevel2;
>  } x86_def_t;
> 
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -522,6 +525,18 @@ static int cpu_x86_fill_host(x86_def_t *
>      cpu_x86_fill_model_id(x86_cpu_def->model_id);
>      x86_cpu_def->vendor_override = 0;
> 
> +    /* Call Centaur's CPUID instruction. */
> +    if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 &&
> +    x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 &&
> +    x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) {

CPUID_VENDOR_VIA_* definitions in target-i386/cpu.h are missing so that
this patch breaks current uq/master build. Please fix.

Thanks,
Jan
Jan Kiszka - May 28, 2011, 4:20 p.m.
On 2011-05-10 10:02, BrillyWu wrote:
> From: BrillyWu <brillywu@viatech.com.cn>
> 
> When KVM is running on VIA CPU with host cpu's model, the
> feautures of VIA CPU will be passed into kvm guest by calling
> the CPUID instruction for Centaur.
> 
> Signed-off-by: BrillyWu<brillywu@viatech.com.cn>
> Signed-off-by: KaryJin<karyjin@viatech.com.cn>

...

> @@ -855,6 +870,8 @@ int cpu_x86_register (CPUX86State *env,
>      env->cpuid_xlevel = def->xlevel;
>      env->cpuid_kvm_features = def->kvm_features;
>      env->cpuid_svm_features = def->svm_features;
> +    env->cpuid_ext4_features = def->ext4_features;
> +    env->cpuid_xlevel2 = def->xlevel2;
>      if (!kvm_enabled()) {
>          env->cpuid_features &= TCG_FEATURES;
>          env->cpuid_ext_features &= TCG_EXT_FEATURES;
> @@ -1034,7 +1051,12 @@ void cpu_x86_cpuid(CPUX86State *env, uin
>                     uint32_t *ecx, uint32_t *edx)
>  {
>      /* test if maximum index reached */
> -    if (index & 0x80000000) {
> +    if ((index & 0xC000000f) == index) {

This condition can't be correct. It triggers on every index <= 15 and
breaks qemu.

> +        /* Handle the Centaur's CPUID instruction. */
> +        if (index > env->cpuid_xlevel2) {
> +            index = env->cpuid_xlevel2;
> +        }
> +    } else if (index & 0x80000000) {

Your very first version looked like this:

-    if (index & 0x80000000) {
+    if ((index & 0xC0000000) == 0xC0000000) {
+	/* Handle the Centaur's CPUID instruction.*
+	* If cpuid_xlevel2 is "0", then put into the*
+	* default case. */
+	if (env->cpuid_xlevel2 == 0)
+	    index = 0xF0000000;
+	else if (index > env->cpuid_xlevel2)
+	    index = env->cpuid_xlevel2;
+    } else if (index & 0x80000000) {

Something went wrong here, please re-validate the patch carefully.

Jan
BrillyWu@viatech.com.cn - May 30, 2011, 4:02 a.m.
Hi, Jan
> 
> > @@ -855,6 +870,8 @@ int cpu_x86_register (CPUX86State *env,
> >      env->cpuid_xlevel = def->xlevel;
> >      env->cpuid_kvm_features = def->kvm_features;
> >      env->cpuid_svm_features = def->svm_features;
> > +    env->cpuid_ext4_features = def->ext4_features;
> > +    env->cpuid_xlevel2 = def->xlevel2;
> >      if (!kvm_enabled()) {
> >          env->cpuid_features &= TCG_FEATURES;
> >          env->cpuid_ext_features &= TCG_EXT_FEATURES; @@ -1034,7
> > +1051,12 @@ void cpu_x86_cpuid(CPUX86State *env, uin
> >                     uint32_t *ecx, uint32_t *edx)  {
> >      /* test if maximum index reached */
> > -    if (index & 0x80000000) {
> > +    if ((index & 0xC000000f) == index) {
> 
> This condition can't be correct. It triggers on every index <= 15 and 
> breaks qemu.

I'm so sorry to make such a stupid mistake. Thank you very for your revieview.

> 
> > +        /* Handle the Centaur's CPUID instruction. */
> > +        if (index > env->cpuid_xlevel2) {
> > +            index = env->cpuid_xlevel2;
> > +        }
> > +    } else if (index & 0x80000000) {
> 
> Your very first version looked like this:

The first version has some problem, so you could ignore it.

> 
> -    if (index & 0x80000000) {
> +    if ((index & 0xC0000000) == 0xC0000000) {
> +	/* Handle the Centaur's CPUID instruction.*
> +	* If cpuid_xlevel2 is "0", then put into the*
> +	* default case. */
> +	if (env->cpuid_xlevel2 == 0)
> +	    index = 0xF0000000;
> +	else if (index > env->cpuid_xlevel2)
> +	    index = env->cpuid_xlevel2;
> +    } else if (index & 0x80000000) {
> 
> Something went wrong here, please re-validate the patch carefully.

Ok, I will check it soon.

Patch

--- a/target-i386/cpu.h	2011-05-09 09:55:48.624885001 +0800
+++ b/target-i386/cpu.h	2011-05-09 09:48:53.704885019 +0800
@@ -721,6 +721,9 @@  typedef struct CPUX86State {
     uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
     int cpuid_vendor_override;
+    /* Store the results of Centaur's CPUID instructions */
+    uint32_t cpuid_xlevel2;
+    uint32_t cpuid_ext4_features;

     /* MTRRs */
     uint64_t mtrr_fixed[11];
--- a/target-i386/cpuid.c	2011-05-09 09:31:11.754884991 +0800
+++ b/target-i386/cpuid.c	2011-05-09 10:18:46.204885008 +0800
@@ -230,6 +230,9 @@  typedef struct x86_def_t {
     char model_id[48];
     int vendor_override;
     uint32_t flags;
+    /* Store the results of Centaur's CPUID instructions */
+    uint32_t ext4_features;
+    uint32_t xlevel2;
 } x86_def_t;

 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -522,6 +525,18 @@  static int cpu_x86_fill_host(x86_def_t *
     cpu_x86_fill_model_id(x86_cpu_def->model_id);
     x86_cpu_def->vendor_override = 0;

+    /* Call Centaur's CPUID instruction. */
+    if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 &&
+    x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 &&
+    x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) {
+        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
+        if (eax >= 0xC0000001) {
+            /* Support VIA max extended level */
+            x86_cpu_def->xlevel2 = eax;
+            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
+            x86_cpu_def->ext4_features = edx;
+        }
+    }

     /*
      * Every SVM feature requires emulation support in KVM - so we can't just
@@ -855,6 +870,8 @@  int cpu_x86_register (CPUX86State *env,
     env->cpuid_xlevel = def->xlevel;
     env->cpuid_kvm_features = def->kvm_features;
     env->cpuid_svm_features = def->svm_features;
+    env->cpuid_ext4_features = def->ext4_features;
+    env->cpuid_xlevel2 = def->xlevel2;
     if (!kvm_enabled()) {
         env->cpuid_features &= TCG_FEATURES;
         env->cpuid_ext_features &= TCG_EXT_FEATURES;
@@ -1034,7 +1051,12 @@  void cpu_x86_cpuid(CPUX86State *env, uin
                    uint32_t *ecx, uint32_t *edx)
 {
     /* test if maximum index reached */
-    if (index & 0x80000000) {
+    if ((index & 0xC000000f) == index) {
+        /* Handle the Centaur's CPUID instruction. */
+        if (index > env->cpuid_xlevel2) {
+            index = env->cpuid_xlevel2;
+        }
+    } else if (index & 0x80000000) {
         if (index > env->cpuid_xlevel)
             index = env->cpuid_level;
     } else {
@@ -1256,6 +1278,28 @@  void cpu_x86_cpuid(CPUX86State *env, uin
 		*edx = 0;
 	}
         break;
+    case 0xC0000000:
+        *eax = env->cpuid_xlevel2;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        break;
+    case 0xC0000001:
+        /* Support for VIA CPU's CPUID instruction */
+        *eax = env->cpuid_version;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = env->cpuid_ext4_features;
+        break;
+    case 0xC0000002:
+    case 0xC0000003:
+    case 0xC0000004:
+        /* Reserved for the future, and now filled with zero */
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        break;
     default:
         /* reserved values: zero */
         *eax = 0;
--- a/target-i386/kvm.c	2011-05-09 09:31:04.284884996 +0800
+++ b/target-i386/kvm.c	2011-05-09 09:55:42.984885014 +0800
@@ -492,6 +492,21 @@  int kvm_arch_init_vcpu(CPUState *env)
         cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
     }

+    /* Call Centaur's CPUID instructions they are supported. */
+    if (env->cpuid_xlevel2 > 0) {
+        env->cpuid_ext4_features &=
+            kvm_arch_get_supported_cpuid(env, 0xC0000001, 0, R_EDX);
+        cpu_x86_cpuid(env, 0xC0000000, 0, &limit, &unused, &unused, &unused);
+
+        for (i = 0xC0000000; i <= limit; i++) {
+            c = &cpuid_data.entries[cpuid_i++];
+
+            c->function = i;
+            c->flags = 0;
+            cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+        }
+    }
+
     cpuid_data.cpuid.nent = cpuid_i;

 #ifdef KVM_CAP_MCE