i386: forward CPUID cache leaves when -cpu host is used

Submitted by Benoît Canet on Aug. 27, 2013, 8:38 p.m.

Details

Message ID 1377635906-17274-2-git-send-email-benoit@irqsave.net
State New
Headers show

Commit Message

Benoît Canet Aug. 27, 2013, 8:38 p.m.
Some users running cpu intensive tasks checking the cache CPUID leaves at
startup and making decisions based on the result reported that the guest was
not reflecting the host CPUID leaves when -cpu host is used.

This patch fix this.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)

Comments

Andreas Färber Sept. 2, 2013, 12:55 p.m.
Hi,

"target-i386:" please.

Am 27.08.2013 22:38, schrieb Benoît Canet:
> Some users running cpu intensive tasks checking the cache CPUID leaves at
> startup and making decisions based on the result reported that the guest was
> not reflecting the host CPUID leaves when -cpu host is used.
> 
> This patch fix this.
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  target-i386/cpu.c |   19 +++++++++++++++++++
>  target-i386/cpu.h |    1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 42c5de0..2c8eaf7 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -374,6 +374,7 @@ typedef struct x86_def_t {
>      int stepping;
>      FeatureWordArray features;
>      char model_id[48];
> +    bool fwd_cpuid_cache_leaves;
>  } x86_def_t;
>  
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -1027,6 +1028,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
>      assert(kvm_enabled());
>  
>      x86_cpu_def->name = "host";
> +    x86_cpu_def->fwd_cpuid_cache_leaves = true;
>      host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
>      x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
>  
> @@ -1776,6 +1778,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>      env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
>      env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
>      env->cpuid_xlevel2 = def->xlevel2;
> +    env->fwd_cpuid_cache_leaves = def->fwd_cpuid_cache_leaves;
>  
>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
>  }
> @@ -1949,6 +1952,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          }
>          break;
>      case 2:
> +        if (env->fwd_cpuid_cache_leaves) {
> +            host_cpuid(0x2, 0, eax, ebx, ecx, edx);
> +            break;
> +        }
>          /* cache info: needed for Pentium Pro compatibility */
>          *eax = 1;
>          *ebx = 0;
> @@ -1956,6 +1963,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *edx = 0x2c307d;
>          break;
>      case 4:
> +        if (env->fwd_cpuid_cache_leaves) {
> +            host_cpuid(0x4, count, eax, ebx, ecx, edx);
> +            break;
> +        }
>          /* cache info: needed for Core compatibility */
>          if (cs->nr_cores > 1) {
>              *eax = (cs->nr_cores - 1) << 26;
> @@ -2102,6 +2113,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          break;
>      case 0x80000005:
>          /* cache info (L1 cache) */
> +        if (env->fwd_cpuid_cache_leaves) {
> +            host_cpuid(0x80000005, 0, eax, ebx, ecx, edx);
> +            break;
> +        }
>          *eax = 0x01ff01ff;
>          *ebx = 0x01ff01ff;
>          *ecx = 0x40020140;
> @@ -2109,6 +2124,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          break;
>      case 0x80000006:
>          /* cache info (L2 cache) */
> +        if (env->fwd_cpuid_cache_leaves) {
> +            host_cpuid(0x80000006, 0, eax, ebx, ecx, edx);
> +            break;
> +        }
>          *eax = 0;
>          *ebx = 0x42004200;
>          *ecx = 0x02008140;

This hunk may trivially conflict with Eduardo's cache flags cleanup.

> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 8a3d0fd..1ec32fa 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -865,6 +865,7 @@ typedef struct CPUX86State {
>      bool tsc_valid;
>      int tsc_khz;
>      void *kvm_xsave_buf;
> +    bool fwd_cpuid_cache_leaves;
>  
>      /* in order to simplify APIC support, we leave this pointer to the
>         user */

Please place the field in X86CPU instead and document it.

Otherwise patch looks okay to me on a brief sight; but since this is
about -cpu host I would prefer this to go through uq/master once fixed
or at least to get some acks.

Regards,
Andreas
Eduardo Habkost Sept. 2, 2013, 1:09 p.m.
On Mon, Sep 02, 2013 at 02:55:36PM +0200, Andreas Färber wrote:
[...]
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 8a3d0fd..1ec32fa 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -865,6 +865,7 @@ typedef struct CPUX86State {
> >      bool tsc_valid;
> >      int tsc_khz;
> >      void *kvm_xsave_buf;
> > +    bool fwd_cpuid_cache_leaves;
> >  
> >      /* in order to simplify APIC support, we leave this pointer to the
> >         user */
> 
> Please place the field in X86CPU instead and document it.

While moving it, I believe the name can be made clearer. I would name it
"fwd_host_cache_info" or something like that, to make it clear that it
will expose the _host CPU_ cache information to the guest.

Patch hide | download patch | download mbox

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 42c5de0..2c8eaf7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -374,6 +374,7 @@  typedef struct x86_def_t {
     int stepping;
     FeatureWordArray features;
     char model_id[48];
+    bool fwd_cpuid_cache_leaves;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -1027,6 +1028,7 @@  static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
     assert(kvm_enabled());
 
     x86_cpu_def->name = "host";
+    x86_cpu_def->fwd_cpuid_cache_leaves = true;
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
     x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
 
@@ -1776,6 +1778,7 @@  static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
     env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
     env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
     env->cpuid_xlevel2 = def->xlevel2;
+    env->fwd_cpuid_cache_leaves = def->fwd_cpuid_cache_leaves;
 
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
@@ -1949,6 +1952,10 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     case 2:
+        if (env->fwd_cpuid_cache_leaves) {
+            host_cpuid(0x2, 0, eax, ebx, ecx, edx);
+            break;
+        }
         /* cache info: needed for Pentium Pro compatibility */
         *eax = 1;
         *ebx = 0;
@@ -1956,6 +1963,10 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *edx = 0x2c307d;
         break;
     case 4:
+        if (env->fwd_cpuid_cache_leaves) {
+            host_cpuid(0x4, count, eax, ebx, ecx, edx);
+            break;
+        }
         /* cache info: needed for Core compatibility */
         if (cs->nr_cores > 1) {
             *eax = (cs->nr_cores - 1) << 26;
@@ -2102,6 +2113,10 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0x80000005:
         /* cache info (L1 cache) */
+        if (env->fwd_cpuid_cache_leaves) {
+            host_cpuid(0x80000005, 0, eax, ebx, ecx, edx);
+            break;
+        }
         *eax = 0x01ff01ff;
         *ebx = 0x01ff01ff;
         *ecx = 0x40020140;
@@ -2109,6 +2124,10 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0x80000006:
         /* cache info (L2 cache) */
+        if (env->fwd_cpuid_cache_leaves) {
+            host_cpuid(0x80000006, 0, eax, ebx, ecx, edx);
+            break;
+        }
         *eax = 0;
         *ebx = 0x42004200;
         *ecx = 0x02008140;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8a3d0fd..1ec32fa 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -865,6 +865,7 @@  typedef struct CPUX86State {
     bool tsc_valid;
     int tsc_khz;
     void *kvm_xsave_buf;
+    bool fwd_cpuid_cache_leaves;
 
     /* in order to simplify APIC support, we leave this pointer to the
        user */