Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Jan Kiszka 
On 2011-09-07 06:21, Bharata B Rao wrote:
 Hi,
 
 Sometime back I posted a patch for fixing x86 CPU topology (
 http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02022.html).
 Here is the next version of the fix which addresses all but one
 comment received during that post.
 
 - Fixed code style issues
 - Ensured that the fix doesn't break TCG mode
 - I am not sure what is the problem with i486 as I haven't been able
 to boot an i486 VM successfully, hence haven't attempted to fix this.

-smp 2 -cpu i486 boots fine here (granted, I don't have some i486 SMP
kernel at hand).

 
 I have tested following scenarios and found the fix to be working fine.
 
 KVM: (with --enable-kvm)
 -smp sockets=1,cores=4,threads=2
 -smp sockets=4,cores=4,threads=2
 -cpu core2duo sockets=1,cores=4,threads=2
 -cpu core2duo sockets=2,cores=4,threads=2
 
 TCG: (without --enable-kvm)
 -cpu core2duo sockets=1,cores=4,threads=2
 -cpu core2duo sockets=2,cores=4,threads=2
 
 Here is the updated patch which now applies against qemu.git.
 
 
 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. 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
 ---
  hw/pc.c  |1 -
  target-i386/helper.c |5 +
  2 files changed, 5 insertions(+), 1 deletion(-)
 
 Index: qemu/hw/pc.c
 ===
 --- qemu.orig/hw/pc.c
 +++ qemu/hw/pc.c
 @@ -933,7 +933,6 @@ static CPUState *pc_new_cpu(const char *
  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);
 Index: qemu/target-i386/helper.c
 ===
 --- qemu.orig/target-i386/helper.c
 +++ qemu/target-i386/helper.c
 @@ -1256,6 +1256,11 @@ CPUX86State *cpu_x86_init(const char *cp
  cpu_x86_close(env);
  return NULL;
  }
 +
 +if (env-cpuid_features  CPUID_APIC) {

|| smp_cpus  1

Should be obvious when looking at the hunk you took this from.

 +env-cpuid_apic_id = env-cpu_index;
 +}
 +
  mce_init(env);
 
  qemu_init_vcpu(env);
 *
 
 Regards,
 Bharata.
 --
  http://bharata.sulekha.com/blog/posts.htm, http://raobharata.wordpress.com/
 
 

Jan

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

Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Bharata B Rao 
On Wed, Sep 7, 2011 at 1:37 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2011-09-07 06:21, Bharata B Rao wrote:
 - I am not sure what is the problem with i486 as I haven't been able
 to boot an i486 VM successfully, hence haven't attempted to fix this.

 -smp 2 -cpu i486 boots fine here (granted, I don't have some i486 SMP
 kernel at hand).

I am getting Unable to find x86 CPU definition error with -cpu i486.
Need to investigate more.

 +
 +    if (env-cpuid_features  CPUID_APIC) {

 || smp_cpus  1

 Should be obvious when looking at the hunk you took this from.

Yes, but I thought no harm in initializing it for uni processor case too, no ?

Regards,
Bharata.

Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Jan Kiszka 
On 2011-09-07 10:19, Bharata B Rao wrote:
 On Wed, Sep 7, 2011 at 1:37 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2011-09-07 06:21, Bharata B Rao wrote:
 - I am not sure what is the problem with i486 as I haven't been able
 to boot an i486 VM successfully, hence haven't attempted to fix this.

 -smp 2 -cpu i486 boots fine here (granted, I don't have some i486 SMP
 kernel at hand).
 
 I am getting Unable to find x86 CPU definition error with -cpu i486.
 Need to investigate more.

Err, sorry: -cpu 486

 
 +
 +if (env-cpuid_features  CPUID_APIC) {

 || smp_cpus  1

 Should be obvious when looking at the hunk you took this from.
 
 Yes, but I thought no harm in initializing it for uni processor case too, no ?

486 CPUs do not have the CPUID_APIC feature set as they do not include a
local APIC. But those SMP systems have external APICs.

Jan

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

Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Anthony Liguori 

On 09/06/2011 11:21 PM, Bharata B Rao wrote:

Hi,

Sometime back I posted a patch for fixing x86 CPU topology (
http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02022.html).
Here is the next version of the fix which addresses all but one
comment received during that post.

- Fixed code style issues
- Ensured that the fix doesn't break TCG mode
- I am not sure what is the problem with i486 as I haven't been able
to boot an i486 VM successfully, hence haven't attempted to fix this.

I have tested following scenarios and found the fix to be working fine.

KVM: (with --enable-kvm)
-smp sockets=1,cores=4,threads=2
-smp sockets=4,cores=4,threads=2
-cpu core2duo sockets=1,cores=4,threads=2
-cpu core2duo sockets=2,cores=4,threads=2

TCG: (without --enable-kvm)
-cpu core2duo sockets=1,cores=4,threads=2
-cpu core2duo sockets=2,cores=4,threads=2

Here is the updated patch which now applies against qemu.git.


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


Please post patches as top-level threads with [PATCH] in the subject. 
Please use git diff or better yet, git-send-email.


Regards,

Anthony Liguori


---
  hw/pc.c  |1 -
  target-i386/helper.c |5 +
  2 files changed, 5 insertions(+), 1 deletion(-)

Index: qemu/hw/pc.c
===
--- qemu.orig/hw/pc.c
+++ qemu/hw/pc.c
@@ -933,7 +933,6 @@ static CPUState *pc_new_cpu(const char *
  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);
Index: qemu/target-i386/helper.c
===
--- qemu.orig/target-i386/helper.c
+++ qemu/target-i386/helper.c
@@ -1256,6 +1256,11 @@ CPUX86State *cpu_x86_init(const char *cp
  cpu_x86_close(env);
  return NULL;
  }
+
+if (env-cpuid_features  CPUID_APIC) {
+env-cpuid_apic_id = env-cpu_index;
+}
+
  mce_init(env);

  qemu_init_vcpu(env);
*

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

Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Bharata B Rao 
On Wed, Sep 7, 2011 at 6:29 PM, Anthony Liguori anth...@codemonkey.ws wrote:
 On 09/06/2011 11:21 PM, Bharata B Rao wrote:

 Hi,

 Please post patches as top-level threads with [PATCH] in the subject.

I posted a new thread and hence it has appeared as a top-level thread.

This was a fix and hence used [FIX], but if this mailing list expects
[PATCH], then will use it from next time.

 Please
 use git diff or better yet, git-send-email.

This was a small patch and hence used quilt. If you insist, I can use
git for the next post :)

Regards,
Bharata.

Re: [Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-07 Thread Anthony Liguori 

On 09/07/2011 08:24 AM, Bharata B Rao wrote:

On Wed, Sep 7, 2011 at 6:29 PM, Anthony Liguorianth...@codemonkey.ws  wrote:

On 09/06/2011 11:21 PM, Bharata B Rao wrote:


Hi,


Please post patches as top-level threads with [PATCH] in the subject.


I posted a new thread and hence it has appeared as a top-level thread.

This was a fix and hence used [FIX], but if this mailing list expects
[PATCH], then will use it from next time.


Please
use git diff or better yet, git-send-email.


This was a small patch and hence used quilt. If you insist, I can use
git for the next post :)


It's not strictly required, but git includes extra information in the 
patch it generates (base revision) which git-am can use to merge the 
patch via a 3way merge instead of patch fuzz.  That makes life a bit 
easier when you're applying a lot of patches.


Regards,

Anthony Liguori



Regards,
Bharata.

[Qemu-devel] [FIX] X86 CPU topology broken in KVM mode

2011-09-06 Thread Bharata B Rao 
Hi,

Sometime back I posted a patch for fixing x86 CPU topology (
http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02022.html).
Here is the next version of the fix which addresses all but one
comment received during that post.

- Fixed code style issues
- Ensured that the fix doesn't break TCG mode
- I am not sure what is the problem with i486 as I haven't been able
to boot an i486 VM successfully, hence haven't attempted to fix this.

I have tested following scenarios and found the fix to be working fine.

KVM: (with --enable-kvm)
-smp sockets=1,cores=4,threads=2
-smp sockets=4,cores=4,threads=2
-cpu core2duo sockets=1,cores=4,threads=2
-cpu core2duo sockets=2,cores=4,threads=2

TCG: (without --enable-kvm)
-cpu core2duo sockets=1,cores=4,threads=2
-cpu core2duo sockets=2,cores=4,threads=2

Here is the updated patch which now applies against qemu.git.


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. 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
---
 hw/pc.c  |1 -
 target-i386/helper.c |5 +
 2 files changed, 5 insertions(+), 1 deletion(-)

Index: qemu/hw/pc.c
===
--- qemu.orig/hw/pc.c
+++ qemu/hw/pc.c
@@ -933,7 +933,6 @@ static CPUState *pc_new_cpu(const char *
 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);
Index: qemu/target-i386/helper.c
===
--- qemu.orig/target-i386/helper.c
+++ qemu/target-i386/helper.c
@@ -1256,6 +1256,11 @@ CPUX86State *cpu_x86_init(const char *cp
 cpu_x86_close(env);
 return NULL;
 }
+
+if (env-cpuid_features  CPUID_APIC) {
+env-cpuid_apic_id = env-cpu_index;
+}
+
 mce_init(env);

 qemu_init_vcpu(env);
*

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