From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45180) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0TOd-0001mg-4m for qemu-devel@nongnu.org; Wed, 11 May 2016 08:37:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b0TOY-0005fH-Q3 for qemu-devel@nongnu.org; Wed, 11 May 2016 08:37:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37871) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0TOY-0005eX-HW for qemu-devel@nongnu.org; Wed, 11 May 2016 08:37:42 -0400 Date: Wed, 11 May 2016 14:37:38 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Message-ID: <20160511123733.GG12472@potion> References: <1462826940-2422-1-git-send-email-rkrcmar@redhat.com> <20160510195336.GA4457@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160510195336.GA4457@thinpad.lan.raisama.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-i386: implement CPUID[0xB] (Extended Topology Enumeration) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Paolo Bonzini , Richard Henderson 2016-05-10 16:53-0300, Eduardo Habkost: > On Mon, May 09, 2016 at 10:49:00PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: >> I looked at a dozen Intel CPU that have this CPUID and all of them >> always had Core offset as 1 (a wasted bit when hyperthreading is >> disabled) and Package offset at least 4 (wasted bits at <=3D 4 cores). >>=20 >> QEMU uses more compact IDs and it doesn't make much sense to change it >> now. I keep the SMT and Core sub-leaves even if there is just one >> thread/core; it makes the code simpler and there should be no harm. >=20 > If in the future we really want to make the APIC ID offsets match > the CPUs you looked at, we can add extra X86CPU properties to > allow configuration of APIC ID bit offsets larger than the ones > calculated from smp_cores and smp_threads. Sounds good. No hurry with that as virt has no problem with routing a x2APIC cluster that covers two packages and I'm not sure what the reasoning for HT is. > What about compatiblity on migration? With this patch, guests > will see CPUID data change when upgrading QEMU. Ah, thanks, I always forget that QEMU doesn't migrate all configuration and has machine types ... I don't think that we can directly override values from cpu_x86_cpuid() with machine type wrappers. What about adding a compatibility flags into X86CPUDefinition and assigning one flag to disable CPUID 0xB, or a global flag? >> Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >> --- >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> @@ -2460,6 +2461,32 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t i= ndex, uint32_t count, >> + switch (*ecx) { >> + case 0: >> + *eax =3D apicid_core_offset(smp_cores, smp_threads); >> + *ebx =3D smp_threads; >> + *ecx |=3D CPUID_TOPOLOGY_LEVEL_SMT; >> + break; >> + case 1: >> + *eax =3D apicid_pkg_offset(smp_cores, smp_threads); >> + *ebx =3D smp_cores * smp_threads; >> + *ecx |=3D CPUID_TOPOLOGY_LEVEL_CORE; >> + break; >> + default: >> + *eax =3D 0; >> + *ebx =3D 0; >> + *ecx |=3D CPUID_TOPOLOGY_LEVEL_INVALID; >> + } >> + >> + /* Preserve reserved bits. Extremely unlikely to make a diff= erence. */ >> + *eax &=3D 0x1f; >> + *ebx &=3D 0xffff; >=20 > That means we don't know how to handle apicid_*_offset() >=3D 32, > smp_threads > 2^16, or smp_cores * smp_threads > 2^16. If that > happen one day, I would prefer to see QEMU abort than silently > truncating data in CPUID[0xB]. What about an assert()? I skipped an assert because the spec says that *ebx cannot be taken seriously, so killing the guest didn't seem worth it: Software must not use EBX[15:0] to enumerate processor topology of the system. This value in this field (EBX[15:0]) is only intended for display/diagnostic purposes. The actual number of logical processors available to BIOS/OS/Applications may be different from the value of EBX[15:0], depending on software and platform hardware configurations. I'd warn, but I don't know if 'printf' is ok, so I skipped it too, because the overflow really doesn't matter.