[PATCH] Fix various compile errors/warnings on ARM with CONFIG_XEN_*=m

[PATCH] Fix various compile errors/warnings on ARM with CONFIG_XEN_*=m

[Konrad Rzeszutek Wilk]

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

[PATCH 1/2] xen/generic: Disable fallback build on ARM.

[Konrad Rzeszutek Wilk]

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

Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.

[Ian Campbell]

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.

Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.

[Jan Beulich]

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

Re: [Xen-devel] [PATCH 1/2] xen/generic: Disable fallback build on ARM.

[Konrad Rzeszutek Wilk]

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

[PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.

[Konrad Rzeszutek Wilk]

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

Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.

[Ian Campbell]

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);

Re: [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.

[Stefano Stabellini]

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.

Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.

[Ian Campbell]

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.

Re: [Xen-devel] [PATCH 2/2] xen/arm: Fix compile errors when drivers are compiled as modules.

[Stefano Stabellini]

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 :)