* [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode
@ 2011-11-02 8:46 bharata.rao
2011-11-02 8:50 ` Jan Kiszka
2011-11-02 12:56 ` Anthony Liguori
0 siblings, 2 replies; 3+ messages in thread
From: bharata.rao @ 2011-11-02 8:46 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, avi, Bharata B Rao
From: Bharata B Rao <bharata.rao@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.rao@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);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode
2011-11-02 8:46 [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode bharata.rao
@ 2011-11-02 8:50 ` Jan Kiszka
2011-11-02 12:56 ` Anthony Liguori
1 sibling, 0 replies; 3+ messages in thread
From: Jan Kiszka @ 2011-11-02 8:50 UTC (permalink / raw)
To: bharata.rao@gmail.com; +Cc: qemu-devel@nongnu.org, avi@redhat.com
On 2011-11-02 09:46, bharata.rao@gmail.com wrote:
> From: Bharata B Rao <bharata.rao@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.rao@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.kiszka@siemens.com>
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode
2011-11-02 8:46 [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode bharata.rao
2011-11-02 8:50 ` Jan Kiszka
@ 2011-11-02 12:56 ` Anthony Liguori
1 sibling, 0 replies; 3+ messages in thread
From: Anthony Liguori @ 2011-11-02 12:56 UTC (permalink / raw)
To: bharata.rao; +Cc: jan.kiszka, qemu-devel, avi
On 11/02/2011 03:46 AM, bharata.rao@gmail.com wrote:
> From: Bharata B Rao<bharata.rao@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.rao@gmail.com>
Applied. Thanks.
Regards,
Anthony Liguori
> ---
> 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);
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-11-02 12:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-02 8:46 [Qemu-devel] [PATCH v3] Fix X86 CPU topology in KVM mode bharata.rao
2011-11-02 8:50 ` Jan Kiszka
2011-11-02 12:56 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).