From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43074) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfQXZ-000298-ES for qemu-devel@nongnu.org; Tue, 07 Apr 2015 06:15:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfQXU-0005v0-DG for qemu-devel@nongnu.org; Tue, 07 Apr 2015 06:15:29 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44174 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfQXU-0005uY-3W for qemu-devel@nongnu.org; Tue, 07 Apr 2015 06:15:24 -0400 Message-ID: <5523AE38.6000701@suse.de> Date: Tue, 07 Apr 2015 12:15:20 +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> In-Reply-To: <551D3768.9090404@redhat.com> 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 , Nadav Amit , mst@redhat.com, qemu-devel@nongnu.org Cc: nadav.amit@gmail.com, Eduardo Habkost , rth@twiddle.net +Eduardo as target-i386 maintainer Am 02.04.2015 um 14:34 schrieb Paolo Bonzini: >=20 >=20 > On 02/04/2015 01:58, Nadav Amit wrote: >> Since the BSP bit is writable on real hardware, during reset all the C= PUs which >> were not chosen to be the BSP should have their BSP bit cleared. This = fix is >> required for KVM to work correctly when it changes the BSP bit. >> >> An additional fix is required for QEMU tcg to allow software to change= the BSP >> bit. >> >> Signed-off-by: Nadav Amit >> --- >> hw/intc/apic_common.c | 8 ++++++-- >> include/hw/i386/apic.h | 2 +- >> target-i386/cpu.c | 4 +--- >> 3 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c >> index 0858b45..042e960 100644 >> --- a/hw/intc/apic_common.c >> +++ b/hw/intc/apic_common.c >> @@ -215,14 +215,18 @@ void apic_init_reset(DeviceState *dev) >> } >> } >> =20 >> -void apic_designate_bsp(DeviceState *dev) >> +void apic_designate_bsp(DeviceState *dev, bool bsp) >> { >> if (dev =3D=3D NULL) { >> return; >> } >> =20 >> APICCommonState *s =3D APIC_COMMON(dev); >> - s->apicbase |=3D MSR_IA32_APICBASE_BSP; >> + if (bsp) { >> + s->apicbase |=3D MSR_IA32_APICBASE_BSP; >> + } else { >> + s->apicbase &=3D ~MSR_IA32_APICBASE_BSP; >> + } >> } >> =20 >> static void apic_reset_common(DeviceState *dev) >> diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h >> index 1d48e02..51eb6d3 100644 >> --- a/include/hw/i386/apic.h >> +++ b/include/hw/i386/apic.h >> @@ -21,7 +21,7 @@ void apic_sipi(DeviceState *s); >> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, >> TPRAccess access); >> void apic_poll_irq(DeviceState *d); >> -void apic_designate_bsp(DeviceState *d); >> +void apic_designate_bsp(DeviceState *d, bool bsp); >> =20 >> /* pc.c */ >> DeviceState *cpu_get_current_apic(void); >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index b2d1c95..03b33cf 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -2714,9 +2714,7 @@ static void x86_cpu_reset(CPUState *s) >> =20 >> #if !defined(CONFIG_USER_ONLY) >> /* We hard-wire the BSP to the first CPU. */ >> - if (s->cpu_index =3D=3D 0) { >> - apic_designate_bsp(cpu->apic_state); >> - } >> + apic_designate_bsp(cpu->apic_state, s->cpu_index =3D=3D 0); >> =20 >> s->halted =3D !cpu_is_bsp(cpu); >> =20 >> >=20 > Thanks, applied locally. I don't understand why this is necessary: The cpu_index doesn't change, therefore the BSP designation won't change either. Is this a hot-unplug preparation? Or is a KVM-specific call missing here? Either way, the commit message could use some clarification. The API change itself looks okay. 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)