[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 uq/master 2/2] x86: cpuid: reconstruct leaf 0
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v3 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data |
Date: |
Thu, 3 Oct 2013 12:14:03 +0200 |
On Thu, 3 Oct 2013 13:01:54 +0300
Gleb Natapov <address@hidden> 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 <address@hidden> 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 <address@hidden>
> > > ---
> > > 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.
>