On 2011-11-02 09:46, bharata....@gmail.com wrote: > From: Bharata B Rao <bharata....@gmail.com> > > apic id returned to guest kernel in ebx for cpuid(function=1) depends on > CPUX86State->cpuid_apic_id which gets populated after the cpuid information > is cached in the host kernel. This results in broken CPU topology in guest. > > Fix this by setting cpuid_apic_id before cpuid information is passed to > the host kernel. This is done by moving the setting of cpuid_apic_id > to cpu_x86_init() where it will work for both KVM as well as TCG modes. > > Signed-off-by: Bharata B Rao <bharata....@gmail.com> > --- > This is the next post of the fix that addresses Jan's last comment from > my v2 post > (http://lists.gnu.org/archive/html/qemu-devel/2011-09/msg01049.html) > > Compile tested for all targets. > Tested that it works for x86_64-softmmu with KVM and TCG modes > As with the earlier version, I haven't tested i486. > > A quick recap: Without this fix, the APIC IDs will not be set correctly > for vCPUs. There are two effects of this in KVM mode: > > 1 The final topology the guest comes up with will not be correct for > options like -smp sockets=2,cores=4,threads=2. > > 2. The guest kernel crashes for options like > -smp sockets=2,threads=4,cores=2 -numa node,nodeid=0,cpus=0-7 > -numa node,nodeid=1,cpus=8-15 > > [ 0.256180] divide error: 0000 [#1] SMP > [ 0.256970] last sysfs file: > [ 0.256970] CPU 1 > [ 0.256970] Modules linked in: > [ 0.256970] > [ 0.256970] Pid: 2, comm: kthreadd Not tainted 2.6.38.6-26.rc1.fc15.x86_64 > #1 Bochs Bochs > [ 0.256970] RIP: 0010:[<ffffffff8104f4d4>] [<ffffffff8104f4d4>] > select_task_rq_fair+0x44a/0x571 > [ 0.256970] RSP: 0000:ffff880095a97c60 EFLAGS: 00010006 > [ 0.256970] RAX: 0000000000100000 RBX: ffff880095a8cfc0 RCX: > 0000000000000000 > [ 0.256970] RDX: 0000000000000000 RSI: 0000000000000100 RDI: > 0000000000000000 > [ 0.256970] RBP: ffff880095a97d10 R08: 0000000000000100 R09: > ffff880095a8cff8 > [ 0.256970] R10: 0000000000013840 R11: 0000000000800711 R12: > 00000000ffffffff > [ 0.256970] R13: ffff88009fc4f810 R14: 0000000000000001 R15: > 0000000000000000 > [ 0.256970] FS: 0000000000000000(0000) GS:ffff88009fc40000(0000) > knlGS:0000000000000000 > [ 0.256970] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [ 0.256970] CR2: 00000000ffffffff CR3: 0000000001a03000 CR4: > 00000000000006e0 > [ 0.256970] DR0: 0000000000000000 DR1: 0000000000000000 DR2: > 0000000000000000 > [ 0.256970] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: > 0000000000000400 > [ 0.256970] Process kthreadd (pid: 2, threadinfo ffff880095a96000, task > ffff880095a99720) > [ 0.256970] Stack: > [ 0.256970] ffffffff81475873 ffffffff81a02140 ffff880095a97ce0 > ffffffff8106c5a3 > [ 0.256970] ffff880095a8cfd8 0000000100000007 0000000000013840 > 0000000000013840 > [ 0.256970] ffff880095a8cfd8 0000007d00000001 ffff880000000000 > ffff8800954081e8 > [ 0.256970] Call Trace: > [ 0.256970] [<ffffffff81475873>] ? _raw_spin_lock_irq+0x1c/0x1e > [ 0.256970] [<ffffffff8106c5a3>] ? alloc_pid+0x2e6/0x335 > [ 0.256970] [<ffffffff81048960>] select_task_rq+0x16/0x46 > [ 0.256970] [<ffffffff8104e29a>] wake_up_new_task+0x3a/0xde > [ 0.256970] [<ffffffff810546ce>] do_fork+0x1f1/0x2bf > [ 0.256970] [<ffffffff8100804e>] ? load_TLS+0x10/0x14 > [ 0.256970] [<ffffffff81008714>] ? __switch_to+0xc6/0x220 > [ 0.256970] [<ffffffff81010c1a>] kernel_thread+0x75/0x77 > [ 0.256970] [<ffffffff8106eacf>] ? kthread+0x0/0x8c > [ 0.256970] [<ffffffff8100a9e0>] ? kernel_thread_helper+0x0/0x10 > [ 0.256970] [<ffffffff8106ee93>] kthreadd+0xe7/0x124 > [ 0.256970] [<ffffffff8100a9e4>] kernel_thread_helper+0x4/0x10 > [ 0.256970] [<ffffffff8106edac>] ? kthreadd+0x0/0x124 > [ 0.256970] [<ffffffff8100a9e0>] ? kernel_thread_helper+0x0/0x10 > > Anthony, could you please consider this for 1.0 ? > > hw/pc.c | 1 - > target-i386/helper.c | 1 + > 2 files changed, 1 insertions(+), 1 deletions(-) > > > diff --git a/hw/pc.c b/hw/pc.c > index eb4c2d8..25bfa85 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -941,7 +941,6 @@ static CPUState *pc_new_cpu(const char *cpu_model) > exit(1); > } > if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) { > - env->cpuid_apic_id = env->cpu_index; > env->apic_state = apic_init(env, env->cpuid_apic_id); > } > qemu_register_reset(pc_cpu_reset, env); > diff --git a/target-i386/helper.c b/target-i386/helper.c > index 5df40d4..9302823 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1256,6 +1256,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model) > cpu_x86_close(env); > return NULL; > } > + env->cpuid_apic_id = env->cpu_index; > mce_init(env); > > qemu_init_vcpu(env);
Acked-by: Jan Kiszka <jan.kis...@siemens.com> -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
