Message ID | 1380729297-6282-1-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, Oct 02, 2013 at 05:54:57PM +0200, Paolo Bonzini wrote: > The data in leaf 0Dh depends on information from other feature bits. > Instead of passing it blindly from the host, compute it based on > whether these feature bits are enabled. > Applied both. Thanks. > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target-i386/cpu.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 48 insertions(+), 17 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index ac83106..1addb18 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { > }; > #undef REGISTER > > +typedef struct ExtSaveArea { > + uint32_t feature, bits; > + uint32_t offset, size; > +} ExtSaveArea; > + > +static const ExtSaveArea ext_save_areas[] = { > + [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, > + .offset = 0x100, .size = 0x240 }, > +}; > > const char *get_register_name_32(unsigned int reg) > { > @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *edx = 0; > } > break; > - case 0xD: > + case 0xD: { > + KVMState *s = cs->kvm_state; > + uint64_t kvm_mask; > + int i; > + > /* Processor Extended State */ > - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { > - *eax = 0; > - *ebx = 0; > - *ecx = 0; > - *edx = 0; > + *eax = 0; > + *ebx = 0; > + *ecx = 0; > + *edx = 0; > + if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { > break; > } > - if (kvm_enabled()) { > - KVMState *s = cs->kvm_state; > + kvm_mask = > + kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) | > + ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32); > > - *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX); > - *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX); > - *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX); > - *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX); > - } else { > - *eax = 0; > - *ebx = 0; > - *ecx = 0; > - *edx = 0; > + if (count == 0) { > + *ecx = 0x240; > + for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) { > + const ExtSaveArea *esa = &ext_save_areas[i]; > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > + (kvm_mask & (1 << i)) != 0) { > + if (i < 32) { > + *eax |= 1 << i; > + } else { > + *edx |= 1 << (i - 32); > + } > + *ecx = MAX(*ecx, esa->offset + esa->size); > + } > + } > + *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE); > + *ebx = *ecx; > + } else if (count == 1) { > + *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX); > + } else if (count < ARRAY_SIZE(ext_save_areas)) { > + const ExtSaveArea *esa = &ext_save_areas[count]; > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > + (kvm_mask & (1 << count)) != 0) { > + *eax = esa->offset; > + *ebx = esa->size; > + } > } > break; > + } > case 0x80000000: > *eax = env->cpuid_xlevel; > *ebx = env->cpuid_vendor1; > -- > 1.8.3.1 -- Gleb.
On Wed, 2 Oct 2013 17:54:57 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > The data in leaf 0Dh depends on information from other feature bits. > Instead of passing it blindly from the host, compute it based on > whether these feature bits are enabled. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target-i386/cpu.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 48 insertions(+), 17 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index ac83106..1addb18 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { > }; > #undef REGISTER > > +typedef struct ExtSaveArea { > + uint32_t feature, bits; > + uint32_t offset, size; > +} ExtSaveArea; > + > +static const ExtSaveArea ext_save_areas[] = { > + [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, > + .offset = 0x100, .size = 0x240 }, > +}; > > const char *get_register_name_32(unsigned int reg) > { > @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *edx = 0; > } > break; > - case 0xD: > + case 0xD: { > + KVMState *s = cs->kvm_state; > + uint64_t kvm_mask; > + int i; > + > /* Processor Extended State */ > - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { > - *eax = 0; > - *ebx = 0; > - *ecx = 0; > - *edx = 0; > + *eax = 0; > + *ebx = 0; > + *ecx = 0; > + *edx = 0; > + if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { > break; > } > - if (kvm_enabled()) { > - KVMState *s = cs->kvm_state; > + kvm_mask = > + kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) | > + ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32); calling kvm_arch_get_supported_cpuid() without kvm_enabled() guard could regress TCG mode on non KVM host: kvm_arch_get_supported_cpuid -> get_supported_cpuid -> try_get_cpuid -> r = kvm_ioctl(s, KVM_GET_SUPPORTED_CPUID, cpuid); ... if (r < 0) { if (r == -E2BIG) { g_free(cpuid); return NULL; } else { fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n", strerror(-r)); exit(1); ^^^^^^^^^ guest suddenly dies > > - *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX); > - *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX); > - *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX); > - *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX); > - } else { > - *eax = 0; > - *ebx = 0; > - *ecx = 0; > - *edx = 0; > + if (count == 0) { > + *ecx = 0x240; > + for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) { > + const ExtSaveArea *esa = &ext_save_areas[i]; > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > + (kvm_mask & (1 << i)) != 0) { > + if (i < 32) { > + *eax |= 1 << i; > + } else { > + *edx |= 1 << (i - 32); > + } > + *ecx = MAX(*ecx, esa->offset + esa->size); > + } > + } > + *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE); > + *ebx = *ecx; > + } else if (count == 1) { > + *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX); > + } else if (count < ARRAY_SIZE(ext_save_areas)) { > + const ExtSaveArea *esa = &ext_save_areas[count]; > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > + (kvm_mask & (1 << count)) != 0) { > + *eax = esa->offset; > + *ebx = esa->size; > + } > } > break; > + } > case 0x80000000: > *eax = env->cpuid_xlevel; > *ebx = env->cpuid_vendor1;
On Thu, Oct 03, 2013 at 11:59:24AM +0200, Igor Mammedov wrote: > On Wed, 2 Oct 2013 17:54:57 +0200 > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > The data in leaf 0Dh depends on information from other feature bits. > > Instead of passing it blindly from the host, compute it based on > > whether these feature bits are enabled. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > target-i386/cpu.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------- > > 1 file changed, 48 insertions(+), 17 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index ac83106..1addb18 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { > > }; > > #undef REGISTER > > > > +typedef struct ExtSaveArea { > > + uint32_t feature, bits; > > + uint32_t offset, size; > > +} ExtSaveArea; > > + > > +static const ExtSaveArea ext_save_areas[] = { > > + [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, > > + .offset = 0x100, .size = 0x240 }, > > +}; > > > > const char *get_register_name_32(unsigned int reg) > > { > > @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > *edx = 0; > > } > > break; > > - case 0xD: > > + case 0xD: { > > + KVMState *s = cs->kvm_state; > > + uint64_t kvm_mask; > > + int i; > > + > > /* Processor Extended State */ > > - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { > > - *eax = 0; > > - *ebx = 0; > > - *ecx = 0; > > - *edx = 0; > > + *eax = 0; > > + *ebx = 0; > > + *ecx = 0; > > + *edx = 0; > > + if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { > > break; > > } > > - if (kvm_enabled()) { > > - KVMState *s = cs->kvm_state; > > + kvm_mask = > > + kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) | > > + ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32); > calling kvm_arch_get_supported_cpuid() without kvm_enabled() guard > could regress TCG mode on non KVM host: > But there is kvm_enabled() guard above. > kvm_arch_get_supported_cpuid -> get_supported_cpuid -> try_get_cpuid -> > r = kvm_ioctl(s, KVM_GET_SUPPORTED_CPUID, cpuid); > ... > if (r < 0) { > if (r == -E2BIG) { > g_free(cpuid); > return NULL; > } else { > fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n", > strerror(-r)); > exit(1); > ^^^^^^^^^ guest suddenly dies > > > > > - *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX); > > - *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX); > > - *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX); > > - *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX); > > - } else { > > - *eax = 0; > > - *ebx = 0; > > - *ecx = 0; > > - *edx = 0; > > + if (count == 0) { > > + *ecx = 0x240; > > + for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) { > > + const ExtSaveArea *esa = &ext_save_areas[i]; > > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > > + (kvm_mask & (1 << i)) != 0) { > > + if (i < 32) { > > + *eax |= 1 << i; > > + } else { > > + *edx |= 1 << (i - 32); > > + } > > + *ecx = MAX(*ecx, esa->offset + esa->size); > > + } > > + } > > + *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE); > > + *ebx = *ecx; > > + } else if (count == 1) { > > + *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX); > > + } else if (count < ARRAY_SIZE(ext_save_areas)) { > > + const ExtSaveArea *esa = &ext_save_areas[count]; > > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > > + (kvm_mask & (1 << count)) != 0) { > > + *eax = esa->offset; > > + *ebx = esa->size; > > + } > > } > > break; > > + } > > case 0x80000000: > > *eax = env->cpuid_xlevel; > > *ebx = env->cpuid_vendor1; -- Gleb.
On Thu, 3 Oct 2013 13:01:54 +0300 Gleb Natapov <gleb@redhat.com> wrote: > On Thu, Oct 03, 2013 at 11:59:24AM +0200, Igor Mammedov wrote: > > On Wed, 2 Oct 2013 17:54:57 +0200 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > The data in leaf 0Dh depends on information from other feature bits. > > > Instead of passing it blindly from the host, compute it based on > > > whether these feature bits are enabled. > > > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > --- > > > target-i386/cpu.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------- > > > 1 file changed, 48 insertions(+), 17 deletions(-) > > > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > > index ac83106..1addb18 100644 > > > --- a/target-i386/cpu.c > > > +++ b/target-i386/cpu.c > > > @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { > > > }; > > > #undef REGISTER > > > > > > +typedef struct ExtSaveArea { > > > + uint32_t feature, bits; > > > + uint32_t offset, size; > > > +} ExtSaveArea; > > > + > > > +static const ExtSaveArea ext_save_areas[] = { > > > + [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, > > > + .offset = 0x100, .size = 0x240 }, > > > +}; > > > > > > const char *get_register_name_32(unsigned int reg) > > > { > > > @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > > *edx = 0; > > > } > > > break; > > > - case 0xD: > > > + case 0xD: { > > > + KVMState *s = cs->kvm_state; > > > + uint64_t kvm_mask; > > > + int i; > > > + > > > /* Processor Extended State */ > > > - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { > > > - *eax = 0; > > > - *ebx = 0; > > > - *ecx = 0; > > > - *edx = 0; > > > + *eax = 0; > > > + *ebx = 0; > > > + *ecx = 0; > > > + *edx = 0; > > > + if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { > > > break; > > > } > > > - if (kvm_enabled()) { > > > - KVMState *s = cs->kvm_state; > > > + kvm_mask = > > > + kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) | > > > + ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32); > > calling kvm_arch_get_supported_cpuid() without kvm_enabled() guard > > could regress TCG mode on non KVM host: > > > But there is kvm_enabled() guard above. Ah, I'm sorry for noise. I've not noticed it in previous hunk. > > > kvm_arch_get_supported_cpuid -> get_supported_cpuid -> try_get_cpuid -> > > r = kvm_ioctl(s, KVM_GET_SUPPORTED_CPUID, cpuid); > > ... > > if (r < 0) { > > if (r == -E2BIG) { > > g_free(cpuid); > > return NULL; > > } else { > > fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n", > > strerror(-r)); > > exit(1); > > ^^^^^^^^^ guest suddenly dies > > > > > > > > - *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX); > > > - *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX); > > > - *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX); > > > - *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX); > > > - } else { > > > - *eax = 0; > > > - *ebx = 0; > > > - *ecx = 0; > > > - *edx = 0; > > > + if (count == 0) { > > > + *ecx = 0x240; > > > + for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) { > > > + const ExtSaveArea *esa = &ext_save_areas[i]; > > > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > > > + (kvm_mask & (1 << i)) != 0) { > > > + if (i < 32) { > > > + *eax |= 1 << i; > > > + } else { > > > + *edx |= 1 << (i - 32); > > > + } > > > + *ecx = MAX(*ecx, esa->offset + esa->size); > > > + } > > > + } > > > + *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE); > > > + *ebx = *ecx; > > > + } else if (count == 1) { > > > + *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX); > > > + } else if (count < ARRAY_SIZE(ext_save_areas)) { > > > + const ExtSaveArea *esa = &ext_save_areas[count]; > > > + if ((env->features[esa->feature] & esa->bits) == esa->bits && > > > + (kvm_mask & (1 << count)) != 0) { > > > + *eax = esa->offset; > > > + *ebx = esa->size; > > > + } > > > } > > > break; > > > + } > > > case 0x80000000: > > > *eax = env->cpuid_xlevel; > > > *ebx = env->cpuid_vendor1; > > -- > Gleb. >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index ac83106..1addb18 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { }; #undef REGISTER +typedef struct ExtSaveArea { + uint32_t feature, bits; + uint32_t offset, size; +} ExtSaveArea; + +static const ExtSaveArea ext_save_areas[] = { + [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, + .offset = 0x100, .size = 0x240 }, +}; const char *get_register_name_32(unsigned int reg) { @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *edx = 0; } break; - case 0xD: + case 0xD: { + KVMState *s = cs->kvm_state; + uint64_t kvm_mask; + int i; + /* Processor Extended State */ - if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) { - *eax = 0; - *ebx = 0; - *ecx = 0; - *edx = 0; + *eax = 0; + *ebx = 0; + *ecx = 0; + *edx = 0; + if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) { break; } - if (kvm_enabled()) { - KVMState *s = cs->kvm_state; + kvm_mask = + kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) | + ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32); - *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX); - *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX); - *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX); - *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX); - } else { - *eax = 0; - *ebx = 0; - *ecx = 0; - *edx = 0; + if (count == 0) { + *ecx = 0x240; + for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) { + const ExtSaveArea *esa = &ext_save_areas[i]; + if ((env->features[esa->feature] & esa->bits) == esa->bits && + (kvm_mask & (1 << i)) != 0) { + if (i < 32) { + *eax |= 1 << i; + } else { + *edx |= 1 << (i - 32); + } + *ecx = MAX(*ecx, esa->offset + esa->size); + } + } + *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE); + *ebx = *ecx; + } else if (count == 1) { + *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX); + } else if (count < ARRAY_SIZE(ext_save_areas)) { + const ExtSaveArea *esa = &ext_save_areas[count]; + if ((env->features[esa->feature] & esa->bits) == esa->bits && + (kvm_mask & (1 << count)) != 0) { + *eax = esa->offset; + *ebx = esa->size; + } } break; + } case 0x80000000: *eax = env->cpuid_xlevel; *ebx = env->cpuid_vendor1;
The data in leaf 0Dh depends on information from other feature bits. Instead of passing it blindly from the host, compute it based on whether these feature bits are enabled. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target-i386/cpu.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 17 deletions(-)