Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But
as the whole function domain_pause_for_debugger() is pointless to be
compiled when there''s no arch support, simply introduce another HAS_*
macro, enabled only on x86.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE
CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS
CFLAGS-$(lock_profile) += -DLOCK_PROFILE
CFLAGS-$(HAS_ACPI) += -DHAS_ACPI
+CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX
CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH
CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y
HAS_NS16550 := y
HAS_EHCI := y
HAS_KEXEC := y
+HAS_GDBSX := y
xenoprof := y
#
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v
vcpu_check_shutdown(v);
}
+#ifdef HAS_GDBSX
void domain_pause_for_debugger(void)
{
struct domain *d = current->domain;
@@ -628,6 +629,7 @@ void domain_pause_for_debugger(void)
if (current->arch.gdbsx_vcpu_event == 0)
send_global_virq(VIRQ_DEBUGGER);
}
+#endif
/* Complete domain destroy after RCU readers are not holding old references. */
static void complete_domain_destroy(struct rcu_head *head)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Ian Campbell
2012-Jun-25 11:05 UTC
Re: [PATCH] arm: fix build after c/s 25477:e12e0b038219
On Mon, 2012-06-25 at 11:54 +0100, Jan Beulich wrote:> Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But > as the whole function domain_pause_for_debugger() is pointless to be > compiled when there''s no arch support, simply introduce another HAS_* > macro, enabled only on x86. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE > CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS > CFLAGS-$(lock_profile) += -DLOCK_PROFILE > CFLAGS-$(HAS_ACPI) += -DHAS_ACPI > +CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX > CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH > CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER > > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y > HAS_NS16550 := y > HAS_EHCI := y > HAS_KEXEC := y > +HAS_GDBSX := y > xenoprof := y > > # > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v > vcpu_check_shutdown(v); > } > > +#ifdef HAS_GDBSX > void domain_pause_for_debugger(void) > { > struct domain *d = current->domain; > @@ -628,6 +629,7 @@ void domain_pause_for_debugger(void) > if (current->arch.gdbsx_vcpu_event == 0) > send_global_virq(VIRQ_DEBUGGER); > } > +#endif > > /* Complete domain destroy after RCU readers are not holding old references. */ > static void complete_domain_destroy(struct rcu_head *head) > > >
>>> On 25.06.12 at 13:05, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Mon, 2012-06-25 at 11:54 +0100, Jan Beulich wrote: >> Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But >> as the whole function domain_pause_for_debugger() is pointless to be >> compiled when there''s no arch support, simply introduce another HAS_* >> macro, enabled only on x86. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>Hi Keir, the subject was probably badly chosen to catch your attention, but as the patch touches only non-ARM code (in order to make ARM build again) it needs your approval for me to commit. Jan>> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE >> CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS >> CFLAGS-$(lock_profile) += -DLOCK_PROFILE >> CFLAGS-$(HAS_ACPI) += -DHAS_ACPI >> +CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX >> CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH >> CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER >> >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y >> HAS_NS16550 := y >> HAS_EHCI := y >> HAS_KEXEC := y >> +HAS_GDBSX := y >> xenoprof := y >> >> # >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v >> vcpu_check_shutdown(v); >> } >> >> +#ifdef HAS_GDBSX >> void domain_pause_for_debugger(void) >> { >> struct domain *d = current->domain; >> @@ -628,6 +629,7 @@ void domain_pause_for_debugger(void) >> if (current->arch.gdbsx_vcpu_event == 0) >> send_global_virq(VIRQ_DEBUGGER); >> } >> +#endif >> >> /* Complete domain destroy after RCU readers are not holding old > references. */ >> static void complete_domain_destroy(struct rcu_head *head) >> >> >>
On 25/06/2012 11:54, "Jan Beulich" <JBeulich@suse.com> wrote:> Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But > as the whole function domain_pause_for_debugger() is pointless to be > compiled when there''s no arch support, simply introduce another HAS_* > macro, enabled only on x86. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>How did arch/arm build at all then, previously, with this function present for a long time in common code? Would a better fix be just to move the function to arch/x86/domain.c, since it is only called by arch/x86? -- Keir> --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE > CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS > CFLAGS-$(lock_profile) += -DLOCK_PROFILE > CFLAGS-$(HAS_ACPI) += -DHAS_ACPI > +CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX > CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH > CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER > > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y > HAS_NS16550 := y > HAS_EHCI := y > HAS_KEXEC := y > +HAS_GDBSX := y > xenoprof := y > > # > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v > vcpu_check_shutdown(v); > } > > +#ifdef HAS_GDBSX > void domain_pause_for_debugger(void) > { > struct domain *d = current->domain; > @@ -628,6 +629,7 @@ void domain_pause_for_debugger(void) > if (current->arch.gdbsx_vcpu_event == 0) > send_global_virq(VIRQ_DEBUGGER); > } > +#endif > > /* Complete domain destroy after RCU readers are not holding old references. > */ > static void complete_domain_destroy(struct rcu_head *head) > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> On 28.06.12 at 15:03, Keir Fraser <keir@xen.org> wrote: > On 25/06/2012 11:54, "Jan Beulich" <JBeulich@suse.com> wrote: > >> Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But >> as the whole function domain_pause_for_debugger() is pointless to be >> compiled when there''s no arch support, simply introduce another HAS_* >> macro, enabled only on x86. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > How did arch/arm build at all then, previously, with this function present > for a long time in common code?Prior to said c/s adding a reference to ->arch.gdbsx_vcpu_event, the function wasn''t arch-specific at all.> Would a better fix be just to move the > function to arch/x86/domain.c, since it is only called by arch/x86?I''d say no - the function really ought to be generic. Any arch wanting gdbsx support would have to add a respective field to its arch_vcpu. Or alternatively the field itself should get moved into struct vcpu, perhaps dependent upon some CONFIG_*. (This is also why I didn''t want to put in e.g. a CONFIG_X86 conditional, which would have been a smaller change.) Jan>> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE >> CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS >> CFLAGS-$(lock_profile) += -DLOCK_PROFILE >> CFLAGS-$(HAS_ACPI) += -DHAS_ACPI >> +CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX >> CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH >> CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER >> >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y >> HAS_NS16550 := y >> HAS_EHCI := y >> HAS_KEXEC := y >> +HAS_GDBSX := y >> xenoprof := y >> >> # >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v >> vcpu_check_shutdown(v); >> } >> >> +#ifdef HAS_GDBSX >> void domain_pause_for_debugger(void) >> { >> struct domain *d = current->domain; >> @@ -628,6 +629,7 @@ void domain_pause_for_debugger(void) >> if (current->arch.gdbsx_vcpu_event == 0) >> send_global_virq(VIRQ_DEBUGGER); >> } >> +#endif >> >> /* Complete domain destroy after RCU readers are not holding old > references. >> */ >> static void complete_domain_destroy(struct rcu_head *head) >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel
On 28/06/2012 14:12, "Jan Beulich" <JBeulich@suse.com> wrote:>>>> On 28.06.12 at 15:03, Keir Fraser <keir@xen.org> wrote: >> On 25/06/2012 11:54, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> Only x86 currently has a struct vcpu field arch.gdbsx_vcpu_event. But >>> as the whole function domain_pause_for_debugger() is pointless to be >>> compiled when there''s no arch support, simply introduce another HAS_* >>> macro, enabled only on x86. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> How did arch/arm build at all then, previously, with this function present >> for a long time in common code? > > Prior to said c/s adding a reference to ->arch.gdbsx_vcpu_event, > the function wasn''t arch-specific at all.Ah yes, of course. :)>> Would a better fix be just to move the >> function to arch/x86/domain.c, since it is only called by arch/x86? > > I''d say no - the function really ought to be generic. Any arch > wanting gdbsx support would have to add a respective field to > its arch_vcpu. Or alternatively the field itself should get moved > into struct vcpu, perhaps dependent upon some CONFIG_*. > (This is also why I didn''t want to put in e.g. a CONFIG_X86 > conditional, which would have been a smaller change.)Well, fine then. Acked-by: Keir Fraser <keir@xen.org>> Jan > >>> --- a/xen/Rules.mk >>> +++ b/xen/Rules.mk >>> @@ -51,6 +51,7 @@ CFLAGS-$(perfc) += -DPERF_COUNTE >>> CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS >>> CFLAGS-$(lock_profile) += -DLOCK_PROFILE >>> CFLAGS-$(HAS_ACPI) += -DHAS_ACPI >>> +CFLAGS-$(HAS_GDBSX) += -DHAS_GDBSX >>> CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH >>> CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER >>> >>> --- a/xen/arch/x86/Rules.mk >>> +++ b/xen/arch/x86/Rules.mk >>> @@ -9,6 +9,7 @@ HAS_PASSTHROUGH := y >>> HAS_NS16550 := y >>> HAS_EHCI := y >>> HAS_KEXEC := y >>> +HAS_GDBSX := y >>> xenoprof := y >>> >>> # >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -612,6 +612,7 @@ void vcpu_end_shutdown_deferral(struct v >>> vcpu_check_shutdown(v); >>> } >>> >>> +#ifdef HAS_GDBSX >>> void domain_pause_for_debugger(void) >>> { >>> struct domain *d = current->domain; >>> @@ -628,6 +629,7 @@ void domain_pause_for_debugger(void) >>> if (current->arch.gdbsx_vcpu_event == 0) >>> send_global_virq(VIRQ_DEBUGGER); >>> } >>> +#endif >>> >>> /* Complete domain destroy after RCU readers are not holding old >> references. >>> */ >>> static void complete_domain_destroy(struct rcu_head *head) >>> >>> >>> >>> _______________________________________________ >>> Xen-devel mailing list >>> Xen-devel@lists.xen.org >>> http://lists.xen.org/xen-devel > > >