From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48135) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TTvNV-00008J-WF for qemu-devel@nongnu.org; Thu, 01 Nov 2012 10:04:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TTvNQ-0007Zm-QA for qemu-devel@nongnu.org; Thu, 01 Nov 2012 10:04:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35938) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TTvNQ-0007Za-G3 for qemu-devel@nongnu.org; Thu, 01 Nov 2012 10:04:08 -0400 Date: Thu, 1 Nov 2012 15:04:05 +0100 From: Igor Mammedov Message-ID: <20121101150405.4e5cce1e@nial.usersys.redhat.com> In-Reply-To: <1351101001-14589-23-git-send-email-ehabkost@redhat.com> References: <1351101001-14589-1-git-send-email-ehabkost@redhat.com> <1351101001-14589-23-git-send-email-ehabkost@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 22/27] pc: set CPU APIC ID explicitly List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Paolo Bonzini , qemu-devel@nongnu.org, Andreas =?ISO-8859-1?B?RuRyYmVy?= On Wed, 24 Oct 2012 15:49:56 -0200 Eduardo Habkost wrote: > The PC code takes care of CPU topology, and CPU topology affect the CPU > APIC ID. So the PC CPU initialization code needs to set the APIC ID > explicitly. > > By now, keep the existing behavior but create a apic_id_for_cpu() > function that will be changed later to implement appropriate > topology-dependent behavior. > > The cpuid_apic_id field is used only at: > > - x86_cpu_apic_init(), called from x86_cpu_realize() > - kvm_init_vcpu(), that is called from the VCPU thread > created by qemu_init_vcpu(), called by x86_cpu_realize() > - helper_cpuid(), called only when the VCPU is already running > - kvm_arch_init_vcpu(), that's called by kvm_init_vcpu() > > So it's safe to change it before x86_cpu_realize() is called. > > Signed-off-by: Eduardo Habkost > --- > This is based on the patch that I have originally suybmitted as: > Subject: pc: create apic_id_for_cpu() function (v3) > --- > hw/pc.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/hw/pc.c b/hw/pc.c > index 0e9a00f..eb68851 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -553,6 +553,21 @@ static void bochs_bios_write(void *opaque, uint32_t > addr, uint32_t val) } > } > > +/* Calculates initial APIC ID for a specific CPU index > + * > + * Currently we need to be able to calculate the APIC ID from the CPU index > + * alone (without requiring a CPU object), as the QEMU<->Seabios > interfaces have > + * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC > ID of > + * all CPUs up to max_cpus. > + */ > +static uint32_t apic_id_for_cpu(PCInitArgs *args, int cpu_index) > +{ > + /* right now APIC ID == CPU index. this will eventually change to use > + * the CPU topology configuration properly > + */ > + return cpu_index; > +} > + > int e820_add_entry(uint64_t address, uint64_t length, uint32_t type) > { > int index = le32_to_cpu(e820_table.count); > @@ -870,6 +885,13 @@ static void pc_cpu_init(PCInitArgs *args, int > cpu_index) exit(1); > } > > + /* Override the default APIC set by the X86CPU init function. > + * We need to do that because: > + * - The APIC ID depends on the CPU topology; > + * - The exact APIC ID used may depend on the machine-type init > arguments. > + */ > + cpu->env.cpuid_apic_id = apic_id_for_cpu(args, cpu_index); Could you create property setter for apic_id and use it here, please. > + > x86_cpu_realize(OBJECT(cpu), &err); > if (err) { > error_report("pc_cpu_init: %s\n", error_get_pretty(err));