From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MDwTD-0005wh-4q
	for qemu-devel@nongnu.org; Tue, 09 Jun 2009 04:14:11 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MDwT8-0005uq-Ab
	for qemu-devel@nongnu.org; Tue, 09 Jun 2009 04:14:10 -0400
Received: from [199.232.76.173] (port=39858 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MDwT8-0005uh-8a
	for qemu-devel@nongnu.org; Tue, 09 Jun 2009 04:14:06 -0400
Received: from mx2.redhat.com ([66.187.237.31]:52186)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <gleb@redhat.com>) id 1MDwT7-0002hy-Fx
	for qemu-devel@nongnu.org; Tue, 09 Jun 2009 04:14:06 -0400
Date: Tue, 9 Jun 2009 11:14:01 +0300
From: Gleb Natapov <gleb@redhat.com>
Message-ID: <20090609081401.GR27210@redhat.com>
References: <20090608125946.GF27210@redhat.com> <4A2D1A27.4080504@siemens.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4A2D1A27.4080504@siemens.com>
Subject: [Qemu-devel] Re: [PATCH v2] Apic creation should not depend on pci
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: qemu-devel@nongnu.org

On Mon, Jun 08, 2009 at 04:03:19PM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> >     
> > It should depend on whether cpu has APIC.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 0934778..cb49772 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -878,14 +878,10 @@ static void pc_init1(ram_addr_t ram_size,
> >          }
> >          if (i != 0)
> >              env->halted = 1;
> > -        if (smp_cpus > 1) {
> > -            /* XXX: enable it in all cases */
> > -            env->cpuid_features |= CPUID_APIC;
> > -        }
> > -        qemu_register_reset(main_cpu_reset, 0, env);
> > -        if (pci_enabled) {
> > +        if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> 
> Obviously :), I'm fine with that change. Needs testing, though. What
> scenarios did you already check?
> 
> >              apic_init(env);
> >          }
> > +        qemu_register_reset(main_cpu_reset, 0, env);
> 
> But this line silently reorders CPU and APIC reset handlers. If you did
> it intentionally (I vaguely recall it may have some benefit /wrt KVM
> synchronizing kernel and user space states), I would suggest pushing it
> as a separate patch.
> 
BTW relying on order of callback registration is not a good idea
especially since we have "order" parameter now. On the other hand apic
reset handler already resets cpu so if apic is present there is no need to
register main_cpu_reset().

--
			Gleb.