On Thu, Nov 01, 2012 at 03:04:05PM +0100, Igor Mammedov wrote: > On Wed, 24 Oct 2012 15:49:56 -0200 > Eduardo Habkost <ehabk...@redhat.com> 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 <ehabk...@redhat.com> > > --- > > 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.
I was considering doing that, but I thought it could be overkill. I will do it in the next version. > > > + > > x86_cpu_realize(OBJECT(cpu), &err); > > if (err) { > > error_report("pc_cpu_init: %s\n", error_get_pretty(err)); > -- Eduardo