| Message ID | 706158FABBBA044BAD4FE898A02E4BC236AC6F59@pdsmsx503.ccr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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.
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.zhang@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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?
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.zhang@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
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.
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
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.
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
--- 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;