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

2011-09-11 Thread Jan Kiszka 
On 2011-09-10 16:37, Bharata B Rao wrote:
 On Fri, Sep 9, 2011 at 10:44 PM, Anthony Liguori anth...@codemonkey.ws 
 wrote:

 This breaks the build:

  CCi386-linux-user/helper.o
 /home/anthony/git/qemu/target-i386/helper.c: In function ‘cpu_x86_init’:
 /home/anthony/git/qemu/target-i386/helper.c:1260:47: error: ‘smp_cpus’
 undeclared (first use in this function)
 /home/anthony/git/qemu/target-i386/helper.c:1260:47: note: each undeclared
 identifier is reported only once for each function it appears in
 make[1]: *** [helper.o] Error 1
 make: *** [subdir-i386-linux-user] Error 2

 Please do a full build before submitting patches.
 
 Sorry about this, the complexity of qemu and its capability to
 support/emulate such wide range of targets is slowly sinking in :)
 
 smp_cpus is defined in vl.c which doesn't even get compiled for
 targets like i386-linux-user and hence we end up in above undefined
 reference error.
 
 I wouldn't attempt another fix until I spend some time understanding
 the code organization for various targets. Meanwhile since the root
 cause of the probem and the solution are known, if anyone more well
 versed with qemu code organization can fix this, please go ahead.

Just assign cpuid_apic_id unconditionally in cpu_x86_init. Will have no
side effects.

Jan



signature.asc
Description: OpenPGP digital signature

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

2011-09-10 Thread Bharata B Rao 
On Fri, Sep 9, 2011 at 10:44 PM, Anthony Liguori anth...@codemonkey.ws wrote:

 This breaks the build:

  CC    i386-linux-user/helper.o
 /home/anthony/git/qemu/target-i386/helper.c: In function ‘cpu_x86_init’:
 /home/anthony/git/qemu/target-i386/helper.c:1260:47: error: ‘smp_cpus’
 undeclared (first use in this function)
 /home/anthony/git/qemu/target-i386/helper.c:1260:47: note: each undeclared
 identifier is reported only once for each function it appears in
 make[1]: *** [helper.o] Error 1
 make: *** [subdir-i386-linux-user] Error 2

 Please do a full build before submitting patches.

Sorry about this, the complexity of qemu and its capability to
support/emulate such wide range of targets is slowly sinking in :)

smp_cpus is defined in vl.c which doesn't even get compiled for
targets like i386-linux-user and hence we end up in above undefined
reference error.

I wouldn't attempt another fix until I spend some time understanding
the code organization for various targets. Meanwhile since the root
cause of the probem and the solution are known, if anyone more well
versed with qemu code organization can fix this, please go ahead.

Regards,
Bharata.

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

2011-09-09 Thread Anthony Liguori 

On 09/08/2011 12:33 AM, bharata@gmail.com wrote:

From: Bharata B Raobharata@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 Raobharata@gmail.com


This breaks the build:

  CCi386-linux-user/helper.o
/home/anthony/git/qemu/target-i386/helper.c: In function ‘cpu_x86_init’:
/home/anthony/git/qemu/target-i386/helper.c:1260:47: error: ‘smp_cpus’ 
undeclared (first use in this function)
/home/anthony/git/qemu/target-i386/helper.c:1260:47: note: each 
undeclared identifier is reported only once for each function it appears in

make[1]: *** [helper.o] Error 1
make: *** [subdir-i386-linux-user] Error 2

Please do a full build before submitting patches.

Regards,

Anthony Liguori


---
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.

  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);

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

2011-09-08 Thread Jan Kiszka 
On 2011-09-08 07:33, 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 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.kis...@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

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

2011-09-07 Thread bharata . rao 
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 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.

 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);
-- 
1.7.3.1