Re: [PATCH v2 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data

[Gleb Natapov <gleb@redhat.com>]

On Wed, Oct 02, 2013 at 05:37:31PM +0200, Paolo Bonzini wrote:
> Il 02/10/2013 17:21, Gleb Natapov ha scritto:
> >> -        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 & 3;
> > Lets use define from previous patch.
> 
> Right.
> 
> >> +            *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;
> > Why do you hard code them instead of querying kernel? What if they
> > depend on cpu type? (well if this happens we can forget about
> > migration, but still...)
> 
> HPA confirmed (on xen-devel) that they will not depend on the CPU type.
>  All offsets are documented in the SDM and in the additional Skylake
> manual except for MPX, and he reported that he'd ask for MPX to be
> documented as well.  As you said, if they changed it would be a total mess.
> 
> I hardcoded them because this is not KVM-specific knowledge.  TCG could
> in principle reuse the same code, just skipping the part where it masks
> away features not supported by KVM.
> 
OK. Can you send new version with defines please?

--
			Gleb.