Re: [Qemu-devel] [PATCH v2] Fix X86 CPU topology in KVM mode

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2011-09-08 07:33, 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 comment about
> bringing back (smp_cpus > 1) check.
> 
> The previous version was posted here:
> http://lists.gnu.org/archive/html/qemu-devel/2011-09/msg00892.html
> 
> I coudn't boot a 486 kernel successfully with qemu and hence not
> sure if and how this fix breaks i486. Any help from Jan or others
> who might have easy means to boot 486 would be good.

At least it preserves the current logic, just moves it up in the
initialization path.

> 
>  hw/pc.c              |    1 -
>  target-i386/helper.c |    5 +++++
>  2 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 5bc845a..f9cca4d 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -933,7 +933,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..139a193 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1256,6 +1256,11 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
>          cpu_x86_close(env);
>          return NULL;
>      }
> +
> +    if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> +        env->cpuid_apic_id = env->cpu_index;
> +    }
> +
>      mce_init(env);
>  
>      qemu_init_vcpu(env);

Tested-and-acked-by: Jan Kiszka <jan.kiszka@siemens.com>

Just in time, we happen to hit this bug today too. It confused libvirt
in the guest quite a bit...

Jan

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