From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33377) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfTqa-000830-Sk for qemu-devel@nongnu.org; Tue, 07 Apr 2015 09:47:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfTqV-0007CV-9P for qemu-devel@nongnu.org; Tue, 07 Apr 2015 09:47:20 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33385 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfTqV-0007C8-2q for qemu-devel@nongnu.org; Tue, 07 Apr 2015 09:47:15 -0400 Message-ID: <5523DFE0.4040301@suse.de> Date: Tue, 07 Apr 2015 15:47:12 +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> <5523C62E.6010507@suse.de> <5523D90A.1040604@redhat.com> <5523DAEC.6070605@openvz.org> <5523DBC2.2050007@redhat.com> <5523DF4A.7050905@openvz.org> In-Reply-To: <5523DF4A.7050905@openvz.org> 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: "Denis V. Lunev" , 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 15:44 schrieb Denis V. Lunev: > On 07/04/15 16:29, Paolo Bonzini wrote: >> >> >> On 07/04/2015 15:26, Denis V. Lunev wrote: >>> On 07/04/15 16:18, Paolo Bonzini wrote: >>>> >>>> >>>> On 07/04/2015 13:57, Andreas F=E4rber wrote: >>>>>>> If this is some issue with sync'ing state back and forth before >>>>>>> QEMU and >>>>>>> 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 res= et >>>>> 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; >>>> >>>> Yes, this is more readable. >>> >>> just $0.02 :) >>> >>> why don't >>> bsp =3D s->apicbase & MSR_IA32_APICBASE_BSP; >>> s->apicbase =3D >>> APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE; >>> in this case. This looks the same from the textual point of view. >> >> Yes. Would you like to send a patch? >> >> Paolo >> >=20 > no prob, just give me 2 minutes. Side note, bsp will become uint32_t > and we will loose tracepoint inside cpu_get_apic_base() on this > path... Yes, I intentionally left it out above as I don't think we need to trace the local usage here. Worth mentioning in the commit message though. Regards, 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)