From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:37008)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VROUK-0004eD-M8
	for qemu-devel@nongnu.org; Wed, 02 Oct 2013 11:37:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VROUD-00060K-EX
	for qemu-devel@nongnu.org; Wed, 02 Oct 2013 11:37:20 -0400
Received: from mx1.redhat.com ([209.132.183.28]:46456)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VROUD-00060A-55
	for qemu-devel@nongnu.org; Wed, 02 Oct 2013 11:37:13 -0400
Message-ID: <524C3DBB.4040300@redhat.com>
Date: Wed, 02 Oct 2013 17:37:31 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1379080558-16499-1-git-send-email-pbonzini@redhat.com>
	<1379080558-16499-3-git-send-email-pbonzini@redhat.com>
	<20131002152126.GM17294@redhat.com>
In-Reply-To: <20131002152126.GM17294@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 uq/master 2/2] x86: cpuid: reconstruct
	leaf 0Dh data
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gleb Natapov <gleb@redhat.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org

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.

Paolo