[Qemu-devel] X86 CPU topology broken in qemu ?

[Qemu-devel] X86 CPU topology broken in qemu ?

[Bharata B Rao]

Hi,

I see that x86 CPU topology inside VM is not showing up as specified.
With some debugging, I found out that the root cause for this: qemu is
not enumerating the apic ids correctly for vcpus. I made the below
hackish change to get it working.  Has anybody else seen this problem
?  This patch is on qemu-kvm-0.14.1. Using 2.6.39 for guest.

***************************
Fix apic id enumeration

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.

Fix this by setting cpuid_apic_id before cpuid information is passed to
the host kernel.

Signed-off-by: Bharata B Rao <bharata.rao@gmail.com>
---
 hw/pc.c           |    4 +---
 target-i386/kvm.c |    3 +++
 2 files changed, 4 insertions(+), 3 deletions(-)

Index: qemu-kvm-0.14.1/hw/pc.c
===================================================================
--- qemu-kvm-0.14.1.orig/hw/pc.c
+++ qemu-kvm-0.14.1/hw/pc.c
@@ -930,10 +930,8 @@ CPUState *pc_new_cpu(const char *cpu_mod
        fprintf(stderr, "Unable to find x86 CPU definition\n");
        exit(1);
    }
-    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-        env->cpuid_apic_id = env->cpu_index;
+    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1)
        env->apic_state = apic_init(env, env->cpuid_apic_id);
-    }
    qemu_register_reset(pc_cpu_reset, env);
    pc_cpu_reset(env);
    return env;
Index: qemu-kvm-0.14.1/target-i386/kvm.c
===================================================================
--- qemu-kvm-0.14.1.orig/target-i386/kvm.c
+++ qemu-kvm-0.14.1/target-i386/kvm.c
@@ -340,6 +340,9 @@ int kvm_arch_init_vcpu(CPUState *env)

    cpuid_i = 0;

+    if (env->cpuid_features & CPUID_APIC)
+        env->cpuid_apic_id = env->cpu_index;
+
 #ifdef CONFIG_KVM_PARA
    /* Paravirtualization CPUIDs */
    memcpy(signature, "KVMKVMKVM\0\0\0", 12);

**************************

This is how various fields look like before and after this change with
qemu command line option of "-smp ,sockets=1,cores=4,threads=2"

Before
------
root@sqzy:~# grep "core id" /proc/cpuinfo
core id         : 0
core id         : 0
core id         : 0
core id         : 0
core id         : 0
core id         : 0
core id         : 0
core id         : 0

After
-----
root@sqzy:~# grep "core id" /proc/cpuinfo
core id         : 0
core id         : 0
core id         : 1
core id         : 1
core id         : 2
core id         : 2
core id         : 3
core id         : 3

Before
------
root@sqzy:~# grep "cpu cores" /proc/cpuinfo
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1
cpu cores       : 1

After
-----
root@sqzy:~# grep "cpu cores" /proc/cpuinfo
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4
cpu cores       : 4

Before
------
root@sqzy:~# grep apicid /proc/cpuinfo
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0
apicid          : 0
initial apicid  : 0

After
-----
root@sqzy:~# grep apicid /proc/cpuinfo
apicid          : 0
initial apicid  : 0
apicid          : 1
initial apicid  : 1
apicid          : 2
initial apicid  : 2
apicid          : 3
initial apicid  : 3
apicid          : 4
initial apicid  : 4
apicid          : 5
initial apicid  : 5
apicid          : 6
initial apicid  : 6
apicid          : 7
initial apicid  : 7

Before
------
root@sqzy:/sys/devices/system/cpu# cat cpu*/topology/core_siblings_list
0-7
0-7
0-7
0-7
0-7
0-7
0-7
0-7
root@sqzy:/sys/devices/system/cpu# cat cpu*/topology/thread_siblings_list
0-7
0-7
0-7
0-7
0-7
0-7
0-7
0-7

After
-----
root@sqzy:/sys/devices/system/cpu# cat cpu*/topology/core_siblings_list
0-7
0-7
0-7
0-7
0-7
0-7
0-7
0-7
root@sqzy:/sys/devices/system/cpu# cat cpu*/topology/thread_siblings_list
0-1
0-1
2-3
2-3
4-5
4-5
6-7
6-7

Regards,
Bharata.
--
http://bharata.sulekha.com/blog/posts.htm, http://raobharata.wordpress.com/

Re: [Qemu-devel] X86 CPU topology broken in qemu ?

[Bharata B Rao]

On Wed, Aug 17, 2011 at 10:55 AM, Bharata B Rao <bharata.rao@gmail.com> wrote:
> Hi,
>
> I see that x86 CPU topology inside VM is not showing up as specified.
> With some debugging, I found out that the root cause for this: qemu is
> not enumerating the apic ids correctly for vcpus. I made the below
> hackish change to get it working.  Has anybody else seen this problem
> ?  This patch is on qemu-kvm-0.14.1. Using 2.6.39 for guest.
>
> ***************************
> Fix apic id enumeration
>
> 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.
>
> Fix this by setting cpuid_apic_id before cpuid information is passed to
> the host kernel.

Going by the lack of response I assume not many people are using - smp
option with sockets, cores and threads specified explicitly ?

Or is it that I hit a corner case and not using the -smp option
correctly enough ?

Regards,
Bharata.

Re: [Qemu-devel] X86 CPU topology broken in qemu ?

[Avi Kivity]

> Hi,
> 
> I see that x86 CPU topology inside VM is not showing up as specified.
> With some debugging, I found out that the root cause for this: qemu is
> not enumerating the apic ids correctly for vcpus. I made the below
> hackish change to get it working. Has anybody else seen this problem
> ? This patch is on qemu-kvm-0.14.1. Using 2.6.39 for guest.
> 
> ***************************
> Fix apic id enumeration
> 
> 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.
> 
> Fix this by setting cpuid_apic_id before cpuid information is passed
> to
> the host kernel.
> 

> Index: qemu-kvm-0.14.1/hw/pc.c
> ===================================================================
> --- qemu-kvm-0.14.1.orig/hw/pc.c
> +++ qemu-kvm-0.14.1/hw/pc.c

Please post a patch against qemu.git master branch.

> 
> + if (env->cpuid_features & CPUID_APIC)
> +     env->cpuid_apic_id = env->cpu_index;
> +

qemu coding style requires braces even around single statements in if () blocks.

Re: [Qemu-devel] X86 CPU topology broken in qemu ?

[Jan Kiszka]

On 2011-08-17 07:25, Bharata B Rao wrote:
> Hi,
> 
> I see that x86 CPU topology inside VM is not showing up as specified.
> With some debugging, I found out that the root cause for this: qemu is
> not enumerating the apic ids correctly for vcpus. I made the below
> hackish change to get it working.  Has anybody else seen this problem
> ?  This patch is on qemu-kvm-0.14.1. Using 2.6.39 for guest.
> 
> ***************************
> Fix apic id enumeration
> 
> 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.
> 
> Fix this by setting cpuid_apic_id before cpuid information is passed to
> the host kernel.
> 
> Signed-off-by: Bharata B Rao <bharata.rao@gmail.com>
> ---
>  hw/pc.c           |    4 +---
>  target-i386/kvm.c |    3 +++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> Index: qemu-kvm-0.14.1/hw/pc.c
> ===================================================================
> --- qemu-kvm-0.14.1.orig/hw/pc.c
> +++ qemu-kvm-0.14.1/hw/pc.c
> @@ -930,10 +930,8 @@ CPUState *pc_new_cpu(const char *cpu_mod
>         fprintf(stderr, "Unable to find x86 CPU definition\n");
>         exit(1);
>     }
> -    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> -        env->cpuid_apic_id = env->cpu_index;
> +    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1)
>         env->apic_state = apic_init(env, env->cpuid_apic_id);
> -    }
>     qemu_register_reset(pc_cpu_reset, env);
>     pc_cpu_reset(env);
>     return env;
> Index: qemu-kvm-0.14.1/target-i386/kvm.c
> ===================================================================
> --- qemu-kvm-0.14.1.orig/target-i386/kvm.c
> +++ qemu-kvm-0.14.1/target-i386/kvm.c
> @@ -340,6 +340,9 @@ int kvm_arch_init_vcpu(CPUState *env)
> 
>     cpuid_i = 0;
> 
> +    if (env->cpuid_features & CPUID_APIC)
> +        env->cpuid_apic_id = env->cpu_index;
> +

Moving it only here will break TCG mode. Make sure to test both. I guess
it's best to move cpuid_apic_id initialization into cpu_x86_init. And
you need to take care of the external APIC case (i486) as well.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] X86 CPU topology broken in qemu ?

[Bharata B Rao]

On Thu, Aug 25, 2011 at 3:31 PM, Avi Kivity <avi@redhat.com> wrote:
>> Hi,
>>
>> I see that x86 CPU topology inside VM is not showing up as specified.
>> With some debugging, I found out that the root cause for this: qemu is
>> not enumerating the apic ids correctly for vcpus. I made the below
>> hackish change to get it working. Has anybody else seen this problem
>> ? This patch is on qemu-kvm-0.14.1. Using 2.6.39 for guest.
>>
>
>> Index: qemu-kvm-0.14.1/hw/pc.c
>> ===================================================================
>> --- qemu-kvm-0.14.1.orig/hw/pc.c
>> +++ qemu-kvm-0.14.1/hw/pc.c
>
> Please post a patch against qemu.git master branch.

Sure I will rebase. The previous version was just a quick and hackish
change, I just wanted pass by the list.

>
>>
>> + if (env->cpuid_features & CPUID_APIC)
>> +     env->cpuid_apic_id = env->cpu_index;
>> +
>
> qemu coding style requires braces even around single statements in if () blocks.

Sure.

Thanks,
Bharata.

Re: [Qemu-devel] X86 CPU topology broken in qemu ?

[Bharata B Rao]

On Thu, Aug 25, 2011 at 4:17 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-08-17 07:25, Bharata B Rao wrote:
>> Index: qemu-kvm-0.14.1/target-i386/kvm.c
>> ===================================================================
>> --- qemu-kvm-0.14.1.orig/target-i386/kvm.c
>> +++ qemu-kvm-0.14.1/target-i386/kvm.c
>> @@ -340,6 +340,9 @@ int kvm_arch_init_vcpu(CPUState *env)
>>
>>     cpuid_i = 0;
>>
>> +    if (env->cpuid_features & CPUID_APIC)
>> +        env->cpuid_apic_id = env->cpu_index;
>> +
>
> Moving it only here will break TCG mode. Make sure to test both. I guess
> it's best to move cpuid_apic_id initialization into cpu_x86_init. And
> you need to take care of the external APIC case (i486) as well.

Thanks for the review. The patch was just a quick fix. Let me work on
a proper fix that addresses your comments.

Regards,
Bharata.