Konrad Rzeszutek Wilk
2012-Nov-06 22:13 UTC
[PATCH] Fix various compile errors/warnings on ARM with CONFIG_XEN_*=m
Two patches that I was thinking for v3.7 to fix compile issues. [PATCH 1/2] xen/generic: Disable fallback build on ARM. This one, requires Stefano''s Ack: [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as
Konrad Rzeszutek Wilk
2012-Nov-06 22:13 UTC
[PATCH 1/2] xen/generic: Disable fallback build on ARM.
As there is no need for it (the fallback code is for older hypervisors and they won''t run under ARM), and also b/c we get: drivers/xen/fallback.c: In function ''xen_event_channel_op_compat'': drivers/xen/fallback.c:10:19: error: storage size of ''op'' isn''t known drivers/xen/fallback.c:15:2: error: implicit declaration of function ''_hypercall1'' [-Werror=implicit-function-declaration] drivers/xen/fallback.c:15:19: error: expected expression before ''int'' drivers/xen/fallback.c:18:7: error: ''EVTCHNOP_close'' undeclared (first use in this function) drivers/xen/fallback.c:18:7: note: each undeclared identifier is reported only once for each function it appears in .. and more Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/xen/Makefile | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 46de6cd..273d2b9 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -1,8 +1,8 @@ ifneq ($(CONFIG_ARM),y) -obj-y += manage.o balloon.o +obj-y += manage.o balloon.o fallback.o obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o endif -obj-y += grant-table.o features.o events.o fallback.o +obj-y += grant-table.o features.o events.o obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) -- 1.7.7.6
Konrad Rzeszutek Wilk
2012-Nov-06 22:13 UTC
[PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.
We end up with: ERROR: "HYPERVISOR_event_channel_op" [drivers/xen/xen-gntdev.ko] undefined! ERROR: "privcmd_call" [drivers/xen/xen-privcmd.ko] undefined! ERROR: "HYPERVISOR_grant_table_op" [drivers/net/xen-netback/xen-netback.ko] undefined! and this patch exports said function (which is implemented in hypercall.S). Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/arm/xen/enlighten.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 59bcb96..96d969d 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -166,3 +166,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages) *pages = NULL; } EXPORT_SYMBOL_GPL(free_xenballooned_pages); + +/* In the hypervisor.S file. */ +EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op); +EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op); +EXPORT_SYMBOL_GPL(privcmd_call); -- 1.7.7.6
Ian Campbell
2012-Nov-07 07:19 UTC
Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.
On Tue, 2012-11-06 at 22:13 +0000, Konrad Rzeszutek Wilk wrote:> As there is no need for it (the fallback code is for older > hypervisors and they won''t run under ARM),I think more specifically they won''t run on anything other than x86. [...]> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > index 46de6cd..273d2b9 100644 > --- a/drivers/xen/Makefile > +++ b/drivers/xen/Makefile > @@ -1,8 +1,8 @@ > ifneq ($(CONFIG_ARM),y) > -obj-y += manage.o balloon.o > +obj-y += manage.o balloon.o fallback.o > obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o > endifI think : obj-$(CONFIG_X86) += fallback.o would better reflect what is going on here.
Ian Campbell
2012-Nov-07 07:21 UTC
Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.
CCing Russell since I believe this is the fix for the "BUG: ARM build failures due to Xen" failure he reported yesterday, On Tue, 2012-11-06 at 22:13 +0000, Konrad Rzeszutek Wilk wrote:> We end up with: > > ERROR: "HYPERVISOR_event_channel_op" [drivers/xen/xen-gntdev.ko] undefined! > ERROR: "privcmd_call" [drivers/xen/xen-privcmd.ko] undefined! > ERROR: "HYPERVISOR_grant_table_op" [drivers/net/xen-netback/xen-netback.ko] undefined! > > and this patch exports said function (which is implemented in hypercall.S). > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > arch/arm/xen/enlighten.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 59bcb96..96d969d 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -166,3 +166,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages) > *pages = NULL; > } > EXPORT_SYMBOL_GPL(free_xenballooned_pages); > + > +/* In the hypervisor.S file. */ > +EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op); > +EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op); > +EXPORT_SYMBOL_GPL(privcmd_call);
Jan Beulich
2012-Nov-07 08:38 UTC
Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.
>>> On 07.11.12 at 08:19, Ian Campbell <ian.campbell@citrix.com> wrote: > On Tue, 2012-11-06 at 22:13 +0000, Konrad Rzeszutek Wilk wrote: >> As there is no need for it (the fallback code is for older >> hypervisors and they won''t run under ARM), > > I think more specifically they won''t run on anything other than x86. > > [...] >> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile >> index 46de6cd..273d2b9 100644 >> --- a/drivers/xen/Makefile >> +++ b/drivers/xen/Makefile >> @@ -1,8 +1,8 @@ >> ifneq ($(CONFIG_ARM),y) >> -obj-y += manage.o balloon.o >> +obj-y += manage.o balloon.o fallback.o >> obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o >> endif > > I think : > obj-$(CONFIG_X86) += fallback.o > would better reflect what is going on here.Yes please. Jan
Stefano Stabellini
2012-Nov-07 10:25 UTC
Re: [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.
On Tue, 6 Nov 2012, Konrad Rzeszutek Wilk wrote:> We end up with: > > ERROR: "HYPERVISOR_event_channel_op" [drivers/xen/xen-gntdev.ko] undefined! > ERROR: "privcmd_call" [drivers/xen/xen-privcmd.ko] undefined! > ERROR: "HYPERVISOR_grant_table_op" [drivers/net/xen-netback/xen-netback.ko] undefined! > > and this patch exports said function (which is implemented in hypercall.S). > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Thank you very much for going out of your way to fix this issue (I am currently at LinuxCon).> arch/arm/xen/enlighten.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index 59bcb96..96d969d 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -166,3 +166,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages) > *pages = NULL; > } > EXPORT_SYMBOL_GPL(free_xenballooned_pages); > + > +/* In the hypervisor.S file. */ > +EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op); > +EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op); > +EXPORT_SYMBOL_GPL(privcmd_call);I think the patch is OK and I tested it: it fixes the issue reported by Russell. However I am wondering, does it actually make sense only to export 3 hypercalls among the set implemented in hypercall.S? Maybe it does make sense only to export a subset, but I wouldn''t necessarly do the differentiation here, I would just export all the hypercalls implemented in hypercalls.S. In fact if we separate the hypercalls in two sets, I would like to see a similar differentiation on x86 too.
Ian Campbell
2012-Nov-07 10:57 UTC
Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.
On Wed, 2012-11-07 at 10:25 +0000, Stefano Stabellini wrote:> On Tue, 6 Nov 2012, Konrad Rzeszutek Wilk wrote: > > We end up with: > > > > ERROR: "HYPERVISOR_event_channel_op" [drivers/xen/xen-gntdev.ko] undefined! > > ERROR: "privcmd_call" [drivers/xen/xen-privcmd.ko] undefined! > > ERROR: "HYPERVISOR_grant_table_op" [drivers/net/xen-netback/xen-netback.ko] undefined! > > > > and this patch exports said function (which is implemented in hypercall.S). > > > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > Thank you very much for going out of your way to fix this issue (I am > currently at LinuxCon). > > > > arch/arm/xen/enlighten.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index 59bcb96..96d969d 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -166,3 +166,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages) > > *pages = NULL; > > } > > EXPORT_SYMBOL_GPL(free_xenballooned_pages); > > + > > +/* In the hypervisor.S file. */ > > +EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op); > > +EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op); > > +EXPORT_SYMBOL_GPL(privcmd_call); > > I think the patch is OK and I tested it: it fixes the issue reported > by Russell. > However I am wondering, does it actually make sense only to export 3 > hypercalls among the set implemented in hypercall.S? > Maybe it does make sense only to export a subset, but I wouldn''t > necessarly do the differentiation here, I would just export all the > hypercalls implemented in hypercalls.S. > In fact if we separate the hypercalls in two sets, I would like to > see a similar differentiation on x86 too.On x86 these functions are static inline (all of them, I think) so exporting them is not necessary or possible. I don''t think you can export from a .S, otherwise the obvious answer would be to integrate it with the macro in hypercall.S. Possibly mad idea: auto-generate hypercall.[Sc] (.S=stubs & .c=exports) from include/xen/interface/xen.h:HYPERVISOR_* ? (Aside: the whitespace in hypercall.S is a bit fubarred, wrt the line wrapping in the macros) Ian.
Stefano Stabellini
2012-Nov-07 13:01 UTC
Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.
On Wed, 7 Nov 2012, Ian Campbell wrote:> On Wed, 2012-11-07 at 10:25 +0000, Stefano Stabellini wrote: > > On Tue, 6 Nov 2012, Konrad Rzeszutek Wilk wrote: > > > We end up with: > > > > > > ERROR: "HYPERVISOR_event_channel_op" [drivers/xen/xen-gntdev.ko] undefined! > > > ERROR: "privcmd_call" [drivers/xen/xen-privcmd.ko] undefined! > > > ERROR: "HYPERVISOR_grant_table_op" [drivers/net/xen-netback/xen-netback.ko] undefined! > > > > > > and this patch exports said function (which is implemented in hypercall.S). > > > > > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > Thank you very much for going out of your way to fix this issue (I am > > currently at LinuxCon). > > > > > > > arch/arm/xen/enlighten.c | 5 +++++ > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > > index 59bcb96..96d969d 100644 > > > --- a/arch/arm/xen/enlighten.c > > > +++ b/arch/arm/xen/enlighten.c > > > @@ -166,3 +166,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages) > > > *pages = NULL; > > > } > > > EXPORT_SYMBOL_GPL(free_xenballooned_pages); > > > + > > > +/* In the hypervisor.S file. */ > > > +EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op); > > > +EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op); > > > +EXPORT_SYMBOL_GPL(privcmd_call); > > > > I think the patch is OK and I tested it: it fixes the issue reported > > by Russell. > > However I am wondering, does it actually make sense only to export 3 > > hypercalls among the set implemented in hypercall.S? > > Maybe it does make sense only to export a subset, but I wouldn''t > > necessarly do the differentiation here, I would just export all the > > hypercalls implemented in hypercalls.S. > > In fact if we separate the hypercalls in two sets, I would like to > > see a similar differentiation on x86 too. > > On x86 these functions are static inline (all of them, I think) so > exporting them is not necessary or possible.What I meant is that on x86 we could move some of the hypercalls out of hypercall.h. That would be similar to the split that this patch is introducing.> I don''t think you can export from a .S, otherwise the obvious answer > would be to integrate it with the macro in hypercall.S.yeah..> Possibly mad idea: auto-generate hypercall.[Sc] (.S=stubs & .c=exports) > from include/xen/interface/xen.h:HYPERVISOR_* ? > > (Aside: the whitespace in hypercall.S is a bit fubarred, wrt the line > wrapping in the macros)I would leave this idea aside for the moment :)
Konrad Rzeszutek Wilk
2012-Nov-07 15:47 UTC
Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.
On Wed, Nov 07, 2012 at 08:38:44AM +0000, Jan Beulich wrote:> >>> On 07.11.12 at 08:19, Ian Campbell <ian.campbell@citrix.com> wrote: > > On Tue, 2012-11-06 at 22:13 +0000, Konrad Rzeszutek Wilk wrote: > >> As there is no need for it (the fallback code is for older > >> hypervisors and they won''t run under ARM), > > > > I think more specifically they won''t run on anything other than x86. > > > > [...] > >> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > >> index 46de6cd..273d2b9 100644 > >> --- a/drivers/xen/Makefile > >> +++ b/drivers/xen/Makefile > >> @@ -1,8 +1,8 @@ > >> ifneq ($(CONFIG_ARM),y) > >> -obj-y += manage.o balloon.o > >> +obj-y += manage.o balloon.o fallback.o > >> obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o > >> endif > > > > I think : > > obj-$(CONFIG_X86) += fallback.o > > would better reflect what is going on here. > > Yes please.From 6bf926ddd44ddc67edbeb28d4069f207f2c6e07e Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Date: Tue, 6 Nov 2012 15:49:27 -0500 Subject: [PATCH 1/2] xen/generic: Disable fallback build on ARM. As there is no need for it (the fallback code is for older hypervisors and they only run under x86), and also b/c we get: drivers/xen/fallback.c: In function ''xen_event_channel_op_compat'': drivers/xen/fallback.c:10:19: error: storage size of ''op'' isn''t known drivers/xen/fallback.c:15:2: error: implicit declaration of function ''_hypercall1'' [-Werror=implicit-function-declaration] drivers/xen/fallback.c:15:19: error: expected expression before ''int'' drivers/xen/fallback.c:18:7: error: ''EVTCHNOP_close'' undeclared (first use in this function) drivers/xen/fallback.c:18:7: note: each undeclared identifier is reported only once for each function it appears in .. and more [v1: Moved the enablement to be covered by CONFIG_X86 per Ian''s suggestion] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/xen/Makefile | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 46de6cd..7435470 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,7 +2,8 @@ ifneq ($(CONFIG_ARM),y) obj-y += manage.o balloon.o obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o endif -obj-y += grant-table.o features.o events.o fallback.o +obj-$(CONFIG_X86) += fallback.o +obj-y += grant-table.o features.o events.o obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) -- 1.7.7.6> > Jan