Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jes Sorensen

Zhang, Xiantao wrote:

--- qemu-kvm.orig/vl.c
+++ qemu-kvm/vl.c
@@ -4417,13 +4417,11 @@
 }
 if (cpu_can_run(env))
 ret = qemu_cpu_exec(env);
-#ifndef CONFIG_GDBSTUB
^^^
Don't know why change #ifdef to #ifndef in upstream, and I remember it should be ifdef before. I believe this stuff should be compiled only if CONFIG_GDBSTUB is defined.  


Yes, I made get_set_top_cpu() a noop instead when compiled with
CONFIG_GDBSTUB disabled, that way we avoid a bunch of these silly
#ifdefs.

Cheers,
Jes
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Avi Kivity

Zhang, Xiantao wrote:

Jes Sorensen wrote:
  

Hi,

The latest changes to qemu-kvm breaks miserably if one tries to build
without CONFIG_GDBSTUB.

Jes



--- qemu-kvm.orig/vl.c
+++ qemu-kvm/vl.c
@@ -4417,13 +4417,11 @@
 }
 if (cpu_can_run(env))
 ret = qemu_cpu_exec(env);
-#ifndef CONFIG_GDBSTUB
^^^
Don't know why change #ifdef to #ifndef in upstream, and I remember it should be ifdef before. I believe this stuff should be compiled only if CONFIG_GDBSTUB is defined.  
  


This was introduced by

commit 704aec581c1683750e313832ba3aa4813d59cbd0
Author: Xiantao Zhang xiantao.zh...@intel.com
Date:   Thu Nov 27 17:23:27 2008 +0800

   Build fix for !CONFIG_GDBSTUB case
  
   Once CONFIG_GDBSTUB not configured, compile will generate error


In upstream.  Please fix it in upstream and qemu-kvm.git will get the 
fix from there.



--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Zhang, Xiantao
Avi Kivity wrote:
 Zhang, Xiantao wrote:
 Jes Sorensen wrote:
 
 Hi,
 
 The latest changes to qemu-kvm breaks miserably if one tries to
 build without CONFIG_GDBSTUB. 
 
 Jes
 
 
 --- qemu-kvm.orig/vl.c
 +++ qemu-kvm/vl.c
 @@ -4417,13 +4417,11 @@
  }
  if (cpu_can_run(env))
  ret = qemu_cpu_exec(env);
 -#ifndef CONFIG_GDBSTUB
 ^^^
 Don't know why change #ifdef to #ifndef in upstream, and I remember
 it should be ifdef before. I believe this stuff should be compiled
 only if CONFIG_GDBSTUB is defined.  
 
 
 This was introduced by
 
 commit 704aec581c1683750e313832ba3aa4813d59cbd0
 Author: Xiantao Zhang xiantao.zh...@intel.com
 Date:   Thu Nov 27 17:23:27 2008 +0800
 
 Build fix for !CONFIG_GDBSTUB case
 
 Once CONFIG_GDBSTUB not configured, compile will generate error
 
 In upstream.  Please fix it in upstream and qemu-kvm.git will get the
 fix from there.

The original patch is 8c4379cc made by me in kvm-userspace.git and I used 
#ifdef, but accidently changed to #ifndef in qemu-kvm.git. So maybe this typo 
is introduced by Anthony Liguori when rewrote this patch for qemu-kvm.git ?  
Xiantao


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Avi Kivity

Zhang, Xiantao wrote:

Avi Kivity wrote:
  

Zhang, Xiantao wrote:


Jes Sorensen wrote:

  

Hi,

The latest changes to qemu-kvm breaks miserably if one tries to
build without CONFIG_GDBSTUB. 


Jes



--- qemu-kvm.orig/vl.c
+++ qemu-kvm/vl.c
@@ -4417,13 +4417,11 @@
 }
 if (cpu_can_run(env))
 ret = qemu_cpu_exec(env);
-#ifndef CONFIG_GDBSTUB
^^^
Don't know why change #ifdef to #ifndef in upstream, and I remember
it should be ifdef before. I believe this stuff should be compiled
only if CONFIG_GDBSTUB is defined.  

  

This was introduced by

commit 704aec581c1683750e313832ba3aa4813d59cbd0
Author: Xiantao Zhang xiantao.zh...@intel.com
Date:   Thu Nov 27 17:23:27 2008 +0800

Build fix for !CONFIG_GDBSTUB case

Once CONFIG_GDBSTUB not configured, compile will generate error

In upstream.  Please fix it in upstream and qemu-kvm.git will get the
fix from there.



The original patch is 8c4379cc made by me in kvm-userspace.git and I used #ifdef, but accidently changed to #ifndef in qemu-kvm.git. So maybe this typo is introduced by Anthony Liguori when rewrote this patch for qemu-kvm.git ?  
Xiantao
  


That commit got translated to fe538e6f in qemu-kvm.git.

Anthony, what's the reason for the difference between fe538e6f and 704aec58?

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jan Kiszka
Avi Kivity wrote:
 Zhang, Xiantao wrote:
 Jes Sorensen wrote:
  
 Hi,

 The latest changes to qemu-kvm breaks miserably if one tries to build
 without CONFIG_GDBSTUB.

 Jes
 

 --- qemu-kvm.orig/vl.c
 +++ qemu-kvm/vl.c
 @@ -4417,13 +4417,11 @@
  }
  if (cpu_can_run(env))
  ret = qemu_cpu_exec(env);
 -#ifndef CONFIG_GDBSTUB
 ^^^
 Don't know why change #ifdef to #ifndef in upstream, and I remember it
 should be ifdef before. I believe this stuff should be compiled only
 if CONFIG_GDBSTUB is defined.
 
 This was introduced by
 
 commit 704aec581c1683750e313832ba3aa4813d59cbd0
 Author: Xiantao Zhang xiantao.zh...@intel.com
 Date:   Thu Nov 27 17:23:27 2008 +0800
 
Build fix for !CONFIG_GDBSTUB case
  Once CONFIG_GDBSTUB not configured, compile will generate error
 
 In upstream.  Please fix it in upstream and qemu-kvm.git will get the
 fix from there.

Given that CONFIG_GDBSTUB is always true upstream, I tend to say:
Finally drop it upstream and keep this (or a more arch-local) workaround
for missing ia64 gdbstub support downstream.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Avi Kivity

Jan Kiszka wrote:

In upstream.  Please fix it in upstream and qemu-kvm.git will get the
fix from there.



Given that CONFIG_GDBSTUB is always true upstream, I tend to say:
Finally drop it upstream and keep this (or a more arch-local) workaround
for missing ia64 gdbstub support downstream.
  


I agree, unless

- we want to make gdbstub support configurable (don't see any 
overwhelming reason for this, but maybe others do)
- we want to merge ia64 kvm support upstream, and don't want to impose 
gdbstub support (though I'd recommend properly implementing gdbstub)


In any case, I'm okay with dropping the check upstream and applying the 
local fixup.


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jan Kiszka
Avi Kivity wrote:
 Jan Kiszka wrote:
 In upstream.  Please fix it in upstream and qemu-kvm.git will get the
 fix from there.
 

 Given that CONFIG_GDBSTUB is always true upstream, I tend to say:
 Finally drop it upstream and keep this (or a more arch-local) workaround
 for missing ia64 gdbstub support downstream.
   
 
 I agree, unless
 
 - we want to make gdbstub support configurable (don't see any
 overwhelming reason for this, but maybe others do)
 - we want to merge ia64 kvm support upstream, and don't want to impose
 gdbstub support (though I'd recommend properly implementing gdbstub)
 
 In any case, I'm okay with dropping the check upstream and applying the
 local fixup.

The last ia64 tcg patch I saw one or two days ago was missing gdb
support, too (and may suffer from some brokenness for that reason). But
I would suggest for both cases (tcg and kvm) to temporarily provide
stubs/placeholders instead of keeping the central #ifdefs.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Avi Kivity

Jan Kiszka wrote:

I agree, unless

- we want to make gdbstub support configurable (don't see any
overwhelming reason for this, but maybe others do)
- we want to merge ia64 kvm support upstream, and don't want to impose
gdbstub support (though I'd recommend properly implementing gdbstub)

In any case, I'm okay with dropping the check upstream and applying the
local fixup.



The last ia64 tcg patch I saw one or two days ago was missing gdb
support, too (and may suffer from some brokenness for that reason).


I didn't see ia64/tcg.  Did you mean s390?  That was host only IIRC.


 But
I would suggest for both cases (tcg and kvm) to temporarily provide
stubs/placeholders instead of keeping the central #ifdefs.
  


Agreed.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jan Kiszka
Avi Kivity wrote:
 Jan Kiszka wrote:
 I agree, unless

 - we want to make gdbstub support configurable (don't see any
 overwhelming reason for this, but maybe others do)
 - we want to merge ia64 kvm support upstream, and don't want to impose
 gdbstub support (though I'd recommend properly implementing gdbstub)

 In any case, I'm okay with dropping the check upstream and applying the
 local fixup.
 

 The last ia64 tcg patch I saw one or two days ago was missing gdb
 support, too (and may suffer from some brokenness for that reason).
 
 I didn't see ia64/tcg.  Did you mean s390?  That was host only IIRC.

Ah, you are right.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jes Sorensen

Avi Kivity wrote:

I agree, unless

- we want to make gdbstub support configurable (don't see any 
overwhelming reason for this, but maybe others do)
- we want to merge ia64 kvm support upstream, and don't want to impose 
gdbstub support (though I'd recommend properly implementing gdbstub)


In any case, I'm okay with dropping the check upstream and applying the 
local fixup.


Hi,

Here's a patch that fixes the #ifndef to make it the #ifdef as it was
intended.

I am quite fine with us trying to drop all the #ifdefs and introduce
noop wrappers for archs that do not provide the gdbstubs (ie. ia64), but
to start with we better just fix the #ifndef to make it behave like it
was originally intended.

Cheers,
Jes



Fix incorrect #ifndef for CONFIG_GETSTUB, which should have been an
#ifdef.

Signed-off-by: Jes Sorensen j...@sgi.com

---
 vl.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: qemu/vl.c
===
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -4350,7 +4350,7 @@
 }
 if (cpu_can_run(env))
 ret = qemu_cpu_exec(env);
-#ifndef CONFIG_GDBSTUB
+#ifdef CONFIG_GDBSTUB
 if (ret == EXCP_DEBUG) {
 gdb_set_stop_cpu(env);
 debug_requested = 1;


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jan Kiszka
Jes Sorensen wrote:
 Avi Kivity wrote:
 I agree, unless

 - we want to make gdbstub support configurable (don't see any
 overwhelming reason for this, but maybe others do)
 - we want to merge ia64 kvm support upstream, and don't want to impose
 gdbstub support (though I'd recommend properly implementing gdbstub)

 In any case, I'm okay with dropping the check upstream and applying
 the local fixup.
 
 Hi,
 
 Here's a patch that fixes the #ifndef to make it the #ifdef as it was
 intended.
 
 I am quite fine with us trying to drop all the #ifdefs and introduce
 noop wrappers for archs that do not provide the gdbstubs (ie. ia64), but
 to start with we better just fix the #ifndef to make it behave like it
 was originally intended.

As the original change this patch (correctly) fixes broke gdbstub
support in qemu-kvm for all archs + it is still a deviation from
upstream, please take the chance and go for ia64 stubs.

BTW, I'm planning to submit a cleanup patch for CONFIG_GDBSTUB to
upstream soon.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jes Sorensen

Jan Kiszka wrote:

As the original change this patch (correctly) fixes broke gdbstub
support in qemu-kvm for all archs + it is still a deviation from
upstream, please take the chance and go for ia64 stubs.


Jan,

There's far more to do in upstream than just gdbstubs for ia64 to be
usable :-(

We'll get to gdbstubs at some point, but there's too many other higher
priority items to deal with first.

Cheers,
Jes
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jan Kiszka
Jes Sorensen wrote:
 Jan Kiszka wrote:
 As the original change this patch (correctly) fixes broke gdbstub
 support in qemu-kvm for all archs + it is still a deviation from
 upstream, please take the chance and go for ia64 stubs.
 
 Jan,
 
 There's far more to do in upstream than just gdbstubs for ia64 to be
 usable :-(
 
 We'll get to gdbstubs at some point, but there's too many other higher
 priority items to deal with first.

I'm not asking for full gdbstub support at this point, just for a build
workaround at arch-level (or in arch-specific blocks, in gdbstub.c e.g.)
instead of patching generic code with global risk of regressions.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-07 Thread Jes Sorensen

Jan Kiszka wrote:

I'm not asking for full gdbstub support at this point, just for a build
workaround at arch-level (or in arch-specific blocks, in gdbstub.c e.g.)
instead of patching generic code with global risk of regressions.


Oh, I see.

I already submitted that to Avi, making gdb_set_stop_cpu() a noop on
ia64. Since there's no target-ia64/ directory in mainline qemu.git yet,
that fix will have to live in qemu-kvm for the time being.

I am fine if you wish to remove all the #ifdef CONFIG_GDBSTUB bits.

Cheers,
Jes

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [patch] fix qemu-kvm to build when gdbstub is disabled

2009-05-06 Thread Zhang, Xiantao
Jes Sorensen wrote:
 Hi,
 
 The latest changes to qemu-kvm breaks miserably if one tries to build
 without CONFIG_GDBSTUB.
 
 Jes

--- qemu-kvm.orig/vl.c
+++ qemu-kvm/vl.c
@@ -4417,13 +4417,11 @@
 }
 if (cpu_can_run(env))
 ret = qemu_cpu_exec(env);
-#ifndef CONFIG_GDBSTUB
^^^
Don't know why change #ifdef to #ifndef in upstream, and I remember it should 
be ifdef before. I believe this stuff should be compiled only if CONFIG_GDBSTUB 
is defined.  




 if (ret == EXCP_DEBUG) {
 gdb_set_stop_cpu(env);
 debug_requested = 1;
 break;
 }
-#endif
 }
 }--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html