From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35249) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfS8U-0007jx-Kv for qemu-devel@nongnu.org; Tue, 07 Apr 2015 07:57:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfS8P-0005Ei-H0 for qemu-devel@nongnu.org; Tue, 07 Apr 2015 07:57:42 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52048 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfS8P-0005EZ-BR for qemu-devel@nongnu.org; Tue, 07 Apr 2015 07:57:37 -0400 Message-ID: <5523C62E.6010507@suse.de> Date: Tue, 07 Apr 2015 13:57:34 +0200 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1427932716-11800-1-git-send-email-namit@cs.technion.ac.il> <551D3768.9090404@redhat.com> <5523AE38.6000701@suse.de> <5523B2C6.5080601@redhat.com> <5523B518.5050902@suse.de> <5523B755.2080909@redhat.com> <5523BB00.3040404@suse.de> In-Reply-To: <5523BB00.3040404@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] target-i386: clear bsp bit when designating bsp List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: Eduardo Habkost , Nadav Amit , mst@redhat.com, Igor Mammedov , nadav.amit@gmail.com, rth@twiddle.net Am 07.04.2015 um 13:09 schrieb Andreas F=E4rber: > Am 07.04.2015 um 12:54 schrieb Paolo Bonzini: >> On 07/04/2015 12:44, Andreas F=E4rber wrote: >>>>> It can change at runtime, though, if you're using the KVM in-kernel= LAPIC. >>> Got a pointer? A quick git-grep doesn't show anything in hw/ or >>> kvm-all.c or target-i386/ assigning cpu_index, so it'll always have t= he >>> initial value. >> >> Not cpu_index, s->apicbase's MSR_IA32_APICBASE_BSP bit can change with >> KVM in-kernel LAPIC. It cannot change with QEMU's userspace LAPIC. >> >> Because it can change, you have to call apic_designate_bsp for all CPU= s >> and not only on CPU 0. >=20 > Now I'm even more confused. Surely CPUState is initially > zero-initialized. Then we designate one as BSP on reset. That should be > the same result as designating all non-BSP CPUs, no? The only way that > would change is then cpu_index =3D=3D 0 goes away (hot-unplug, not > implemented yet), and in that case it would be about designating a > different CPU, not about un-designating one. >=20 > If this is some issue with sync'ing state back and forth before QEMU an= d > KVM then the real issue has not been explained. Hm, hw/intc/apic_common.c:apic_reset_common() has: bsp =3D cpu_is_bsp(s->cpu); s->apicbase =3D APIC_DEFAULT_ADDRESS | (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE; What this is doing is really: bsp =3D cpu_get_apic_base(s->cpu->apic_state) & MSR_IA32_APICBASE_BSP= ; s->apicbase =3D APIC_DEFAULT_ADDRESS | (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE; Unless I'm missing something, since we are in the APIC device's reset function, this is effectively a twisted way of writing: bsp =3D s->apicbase & MSR_IA32_APICBASE_BSP; s->apicbase =3D APIC_DEFAULT_ADDRESS | (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE; In which case we already relied on s->cpu and could thus simply change this to something like: bsp =3D CPU(s->cpu)->cpu_index =3D=3D 0; s->apicbase =3D APIC_DEFAULT_ADDRESS | (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE; Then the apicbase manipulation would be nicely encapsulated in the APIC rather than the APIC reset retaining it and the CPU reset meddling with its state. Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)