forward cpuid leaves when using -cpu host

PATCH Superseded Benoît Canet 4 participants 6 messages Aug 27, 2013 – Sep 2, 2013 KVM

Share & tools

Benoît Canet

author

2013-08-27 20:36 UTC #0 lore raw

PATCH 1/1 forward cpuid leaves when using -cpu host

This patch uses directly cpuid_host to forward the informations instead of storing a variable number of leaves in the cpu states.

Benoît Canet (1):
i386: forward CPUID cache leaves when -cpu host is used

 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)

Signature · 1 line

1.7.10.4

Benoît Canet

author

2013-08-27 20:36 UTC #1 lore raw

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

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 <[email protected]>

---
 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)

target-i386/cpu.c +19 -0

index 42c5de0..2c8eaf7 100644
@@ -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;

target-i386/cpu.h +1 -0

index 8a3d0fd..1ec32fa 100644
@@ -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 */

Signature · 1 line

1.7.10.4

Andreas Färber

2013-09-02 12:55 UTC #2 lore raw

Re: [Qemu-devel] [PATCH] i386: forward CPUID cache leaves when -cpu host is used

Hi,

"target-i386:" please.

Am 27.08.2013 22:38, schrieb Benoît Canet:

Quoted 74 lines

This patch fix this.

Signed-off-by Benoit Canet <[email protected]>

---
 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)

target-i386/cpu.c +15 -0

index 42c5de0..2c8eaf7 100644
@@ -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;

target-i386/cpu.c void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, · line 2133

Quoted 10 lines

target-i386/cpu.c +4 -0

@@ -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.

target-i386/cpu.h typedef struct CPUX86State { · line 871

Quoted 12 lines

target-i386/cpu.h +1 -0

index 8a3d0fd..1ec32fa 100644
@@ -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

Signature · 2 lines

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Eduardo Habkost

2013-09-02 13:09 UTC #3 lore raw

Re: [Qemu-devel] [PATCH] i386: forward CPUID cache leaves when -cpu host is used

On Mon, Sep 02, 2013 at 02:55:36PM +0200, Andreas Färber wrote:
[...]

Quoted 10 lines

target-i386/cpu.h +1 -0

index 8a3d0fd..1ec32fa 100644
@@ -865,6 +865,7 @@ typedef struct CPUX86State {
     bool tsc_valid;
     int tsc_khz;
     void *kvm_xsave_buf;
+    bool fwd_cpuid_cache_leaves;

Quoted 4 lines

target-i386/cpu.h typedef struct CPUX86State { · line 871
Quoted 2 lines
target-i386/cpu.h +0 -0
     /* 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.

Signature · 1 line

Eduardo

Benoît Canet

2013-09-02 12:44 UTC #4 lore raw

In reply to Benoît Canet

Ping,

I am aware that this patch must be a QObject/CPUState monstruosity but I don't see how I could store the fact that the cpu derive from a "host" type cpu. The alternative would be to remember a variable number of leaves and serving them to the guest as needed but I don't find that much cleaner.

Best regards

Benoît

Quoted 14 lines · Benoît Canet

This patch uses directly cpuid_host to forward the informations instead of storing a variable number of leaves in the cpu states.
Benoît Canet (1):
i386: forward CPUID cache leaves when -cpu host is used
 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)
Signature · 1 line
1.7.10.4

Eduardo Habkost

2013-09-02 12:58 UTC #5 lore raw

Quoted 7 lines · Benoît Canet

Ping,
I am aware that this patch must be a QObject/CPUState monstruosity but I don't see how I could store the fact that the cpu derive from a "host" type cpu. The alternative would be to remember a variable number of leaves and serving them to the guest as needed but I don't find that much cleaner.

The idea makes sense for -cpu host, and it is not intrusive for all other CPU models. I just didn't take the time to look at the code to send my "reviewed-by:" line yet. I plan to take a look today or tomorrow.

Quoted 19 lines

Best regards
Benoît
Quoted 14 lines · Benoît Canet
This patch uses directly cpuid_host to forward the informations instead of storing a variable number of leaves in the cpu states.
Benoît Canet (1):
i386: forward CPUID cache leaves when -cpu host is used
 target-i386/cpu.c |   19 +++++++++++++++++++
 target-i386/cpu.h |    1 +
 2 files changed, 20 insertions(+)
Signature · 1 line
1.7.10.4

Signature · 1 line

Eduardo