Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Stefano Stabellini 
On Wed, 19 Oct 2016, Wei Liu wrote:
> On Wed, Oct 19, 2016 at 11:40:40AM +0200, Juergen Gross wrote:
> > On 19/10/16 11:22, Wei Liu wrote:
> > > On Wed, Oct 19, 2016 at 11:09:21AM +0200, Juergen Gross wrote:
> > >> On 19/10/16 10:50, Wei Liu wrote:
> > >>> On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
> >  Hi all,
> > 
> >  While I was investigating the recent libxl ARM64 build issue, I found
> >  another ARM64 build problem. This one is in QEMU:
> > 
> > 
> >    CChw/usb/xen-usb.o
> >  hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
> >  hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in 
> >  this function)
> >   (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
> >  ^
> >  hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported 
> >  only once for each function it appears in
> >  In file included from hw/usb/xen-usb.c:36:0:
> >  hw/usb/xen-usb.c: In function ‘usbback_alloc’:
> >  /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: 
> >  error: ‘PAGE_SIZE’ undeclared (first use in this function)
> >   #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> >  ^
> >  /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: 
> >  in definition of macro ‘__RD32’
> >   #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : 
> >  __RD16(_x))
> > ^
> >  /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: 
> >  note: in expansion of macro ‘__CONST_RING_SIZE’
> >   #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> > ^
> >  hw/usb/xen-usb.c:1029:51: note: in expansion of macro 
> >  ‘USB_URB_RING_SIZE’
> >   max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 
> >  2;
> > ^
> >  make: *** [hw/usb/xen-usb.o] Error 1
> > 
> > 
> >  It is caused by PAGE_SIZE being utilized in
> >  xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
> >  QEMU includes xen/include/public/io/usbif.h directly, therefore
> >  hw/usb/xen-usb.c doesn't build.
> > 
> >  I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
> >  that file is present in too many Xen releases to ignore now. So I am
> >  going to work around the issue in QEMU.
> > 
> > >>>
> > >>> Actually it was introduced in 4.7.
> > >>>
> >  ---
> > 
> >  xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
> >  which uses PAGE_SIZE without defining it. The header file has been
> >  shipped with too many Xen releases to be able to solve the problem at
> >  the source.
> > 
> >  This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.
> > 
> >  Signed-off-by: Stefano Stabellini 
> > 
> >  diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> >  index 174d715..ca9df87 100644
> >  --- a/hw/usb/xen-usb.c
> >  +++ b/hw/usb/xen-usb.c
> >  @@ -34,6 +34,11 @@
> >   #include "qapi/qmp/qstring.h"
> >   
> >   #include 
> >  +
> >  +/* xen/io/usbif.h references PAGE_SIZE without defining it */
> >  +#ifndef PAGE_SIZE
> >  +#define PAGE_SIZE XC_PAGE_SIZE
> >  +#endif
> >   #include 
> > >>>
> > >>>
> > >>> Can we just change the macro to
> > >>>
> > >>>   #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, 
> > >>> (pg_size)
> > >>>
> > >>> ?
> > >>
> > >> Not easily. On x86 qemu builds just fine and modifying the macro to take
> > >> a parameter would break the build.
> > >>
> > >> We could do something like:
> > >>
> > >> #define USB_CONN_RING_SZ(pgsz) __CONST_RING_SIZE(usbif_conn, (pgsz)
> > >> #ifdef PAGE_SIZE
> > >> #define USB_CONN_RING_SIZE USB_CONN_RING_SZ(PAGE_SIZE)
> > >> #endif
> > >>
> > >> And then modify qemu to use USB_CONN_RING_SZ()
> > >>
> > > 
> > > This would also be problematic I'm afraid.  One's build could break
> > > randomly depending on the order of header files inclusion. I think it
> > > would even be harder to debug such bug than the existing one.
> > > 
> > > Inspired by Andrew on IRC. I think we can do:
> > > 
> > > /* Backward-compatible macro, use _SIZE2 variant in new code instead */
> > > #define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, 4096)
> > > 
> > > #define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, 
> > > (pgsize))
> > > 
> > > The assumption is that all existing implementations of the said protocol
> > > use 4k page size. Juergen, do you think this would work?
> > 
> > In theory, yes. I'd still prefer using PAGE_SIZE if it is defined.
> > 
> > What about:
> > 
> >

Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Wei Liu 
On Wed, Oct 19, 2016 at 11:40:40AM +0200, Juergen Gross wrote:
> On 19/10/16 11:22, Wei Liu wrote:
> > On Wed, Oct 19, 2016 at 11:09:21AM +0200, Juergen Gross wrote:
> >> On 19/10/16 10:50, Wei Liu wrote:
> >>> On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
>  Hi all,
> 
>  While I was investigating the recent libxl ARM64 build issue, I found
>  another ARM64 build problem. This one is in QEMU:
> 
> 
>    CChw/usb/xen-usb.o
>  hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
>  hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in 
>  this function)
>   (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
>  ^
>  hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported 
>  only once for each function it appears in
>  In file included from hw/usb/xen-usb.c:36:0:
>  hw/usb/xen-usb.c: In function ‘usbback_alloc’:
>  /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
>  ‘PAGE_SIZE’ undeclared (first use in this function)
>   #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
>  ^
>  /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
>  definition of macro ‘__RD32’
>   #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : 
>  __RD16(_x))
> ^
>  /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: 
>  in expansion of macro ‘__CONST_RING_SIZE’
>   #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> ^
>  hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
>   max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
> ^
>  make: *** [hw/usb/xen-usb.o] Error 1
> 
> 
>  It is caused by PAGE_SIZE being utilized in
>  xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
>  QEMU includes xen/include/public/io/usbif.h directly, therefore
>  hw/usb/xen-usb.c doesn't build.
> 
>  I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
>  that file is present in too many Xen releases to ignore now. So I am
>  going to work around the issue in QEMU.
> 
> >>>
> >>> Actually it was introduced in 4.7.
> >>>
>  ---
> 
>  xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
>  which uses PAGE_SIZE without defining it. The header file has been
>  shipped with too many Xen releases to be able to solve the problem at
>  the source.
> 
>  This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.
> 
>  Signed-off-by: Stefano Stabellini 
> 
>  diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
>  index 174d715..ca9df87 100644
>  --- a/hw/usb/xen-usb.c
>  +++ b/hw/usb/xen-usb.c
>  @@ -34,6 +34,11 @@
>   #include "qapi/qmp/qstring.h"
>   
>   #include 
>  +
>  +/* xen/io/usbif.h references PAGE_SIZE without defining it */
>  +#ifndef PAGE_SIZE
>  +#define PAGE_SIZE XC_PAGE_SIZE
>  +#endif
>   #include 
> >>>
> >>>
> >>> Can we just change the macro to
> >>>
> >>>   #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, 
> >>> (pg_size)
> >>>
> >>> ?
> >>
> >> Not easily. On x86 qemu builds just fine and modifying the macro to take
> >> a parameter would break the build.
> >>
> >> We could do something like:
> >>
> >> #define USB_CONN_RING_SZ(pgsz) __CONST_RING_SIZE(usbif_conn, (pgsz)
> >> #ifdef PAGE_SIZE
> >> #define USB_CONN_RING_SIZE USB_CONN_RING_SZ(PAGE_SIZE)
> >> #endif
> >>
> >> And then modify qemu to use USB_CONN_RING_SZ()
> >>
> > 
> > This would also be problematic I'm afraid.  One's build could break
> > randomly depending on the order of header files inclusion. I think it
> > would even be harder to debug such bug than the existing one.
> > 
> > Inspired by Andrew on IRC. I think we can do:
> > 
> > /* Backward-compatible macro, use _SIZE2 variant in new code instead */
> > #define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, 4096)
> > 
> > #define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, (pgsize))
> > 
> > The assumption is that all existing implementations of the said protocol
> > use 4k page size. Juergen, do you think this would work?
> 
> In theory, yes. I'd still prefer using PAGE_SIZE if it is defined.
> 
> What about:
> 
> #ifdef PAGE_SIZE
> #define USB_RING_PAGE_SIZE PAGE_SIZE
> #else
> #define USB_RING_PAGE_SIZE 4096
> #endif
> #define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, USB_RING_PAGE_SIZE)
> #define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, (pgsize))
> 

I wish to eliminate PAGE_SIZE all

Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Juergen Gross 
On 19/10/16 11:22, Wei Liu wrote:
> On Wed, Oct 19, 2016 at 11:09:21AM +0200, Juergen Gross wrote:
>> On 19/10/16 10:50, Wei Liu wrote:
>>> On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
 Hi all,

 While I was investigating the recent libxl ARM64 build issue, I found
 another ARM64 build problem. This one is in QEMU:


   CChw/usb/xen-usb.o
 hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
 hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in this 
 function)
  (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
 ^
 hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported only 
 once for each function it appears in
 In file included from hw/usb/xen-usb.c:36:0:
 hw/usb/xen-usb.c: In function ‘usbback_alloc’:
 /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
 ‘PAGE_SIZE’ undeclared (first use in this function)
  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
 ^
 /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
 definition of macro ‘__RD32’
  #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : 
 __RD16(_x))
^
 /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: in 
 expansion of macro ‘__CONST_RING_SIZE’
  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
^
 hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
  max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
^
 make: *** [hw/usb/xen-usb.o] Error 1


 It is caused by PAGE_SIZE being utilized in
 xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
 QEMU includes xen/include/public/io/usbif.h directly, therefore
 hw/usb/xen-usb.c doesn't build.

 I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
 that file is present in too many Xen releases to ignore now. So I am
 going to work around the issue in QEMU.

>>>
>>> Actually it was introduced in 4.7.
>>>
 ---

 xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
 which uses PAGE_SIZE without defining it. The header file has been
 shipped with too many Xen releases to be able to solve the problem at
 the source.

 This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.

 Signed-off-by: Stefano Stabellini 

 diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
 index 174d715..ca9df87 100644
 --- a/hw/usb/xen-usb.c
 +++ b/hw/usb/xen-usb.c
 @@ -34,6 +34,11 @@
  #include "qapi/qmp/qstring.h"
  
  #include 
 +
 +/* xen/io/usbif.h references PAGE_SIZE without defining it */
 +#ifndef PAGE_SIZE
 +#define PAGE_SIZE XC_PAGE_SIZE
 +#endif
  #include 
>>>
>>>
>>> Can we just change the macro to
>>>
>>>   #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, 
>>> (pg_size)
>>>
>>> ?
>>
>> Not easily. On x86 qemu builds just fine and modifying the macro to take
>> a parameter would break the build.
>>
>> We could do something like:
>>
>> #define USB_CONN_RING_SZ(pgsz) __CONST_RING_SIZE(usbif_conn, (pgsz)
>> #ifdef PAGE_SIZE
>> #define USB_CONN_RING_SIZE USB_CONN_RING_SZ(PAGE_SIZE)
>> #endif
>>
>> And then modify qemu to use USB_CONN_RING_SZ()
>>
> 
> This would also be problematic I'm afraid.  One's build could break
> randomly depending on the order of header files inclusion. I think it
> would even be harder to debug such bug than the existing one.
> 
> Inspired by Andrew on IRC. I think we can do:
> 
> /* Backward-compatible macro, use _SIZE2 variant in new code instead */
> #define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, 4096)
> 
> #define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, (pgsize))
> 
> The assumption is that all existing implementations of the said protocol
> use 4k page size. Juergen, do you think this would work?

In theory, yes. I'd still prefer using PAGE_SIZE if it is defined.

What about:

#ifdef PAGE_SIZE
#define USB_RING_PAGE_SIZE PAGE_SIZE
#else
#define USB_RING_PAGE_SIZE 4096
#endif
#define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, USB_RING_PAGE_SIZE)
#define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, (pgsize))


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Wei Liu 
On Wed, Oct 19, 2016 at 11:09:21AM +0200, Juergen Gross wrote:
> On 19/10/16 10:50, Wei Liu wrote:
> > On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
> >> Hi all,
> >>
> >> While I was investigating the recent libxl ARM64 build issue, I found
> >> another ARM64 build problem. This one is in QEMU:
> >>
> >>
> >>   CChw/usb/xen-usb.o
> >> hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
> >> hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in this 
> >> function)
> >>  (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
> >> ^
> >> hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported only 
> >> once for each function it appears in
> >> In file included from hw/usb/xen-usb.c:36:0:
> >> hw/usb/xen-usb.c: In function ‘usbback_alloc’:
> >> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
> >> ‘PAGE_SIZE’ undeclared (first use in this function)
> >>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> >> ^
> >> /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
> >> definition of macro ‘__RD32’
> >>  #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : 
> >> __RD16(_x))
> >>^
> >> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: in 
> >> expansion of macro ‘__CONST_RING_SIZE’
> >>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> >>^
> >> hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
> >>  max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
> >>^
> >> make: *** [hw/usb/xen-usb.o] Error 1
> >>
> >>
> >> It is caused by PAGE_SIZE being utilized in
> >> xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
> >> QEMU includes xen/include/public/io/usbif.h directly, therefore
> >> hw/usb/xen-usb.c doesn't build.
> >>
> >> I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
> >> that file is present in too many Xen releases to ignore now. So I am
> >> going to work around the issue in QEMU.
> >>
> > 
> > Actually it was introduced in 4.7.
> > 
> >> ---
> >>
> >> xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
> >> which uses PAGE_SIZE without defining it. The header file has been
> >> shipped with too many Xen releases to be able to solve the problem at
> >> the source.
> >>
> >> This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.
> >>
> >> Signed-off-by: Stefano Stabellini 
> >>
> >> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> >> index 174d715..ca9df87 100644
> >> --- a/hw/usb/xen-usb.c
> >> +++ b/hw/usb/xen-usb.c
> >> @@ -34,6 +34,11 @@
> >>  #include "qapi/qmp/qstring.h"
> >>  
> >>  #include 
> >> +
> >> +/* xen/io/usbif.h references PAGE_SIZE without defining it */
> >> +#ifndef PAGE_SIZE
> >> +#define PAGE_SIZE XC_PAGE_SIZE
> >> +#endif
> >>  #include 
> > 
> > 
> > Can we just change the macro to
> > 
> >   #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, 
> > (pg_size)
> > 
> > ?
> 
> Not easily. On x86 qemu builds just fine and modifying the macro to take
> a parameter would break the build.
> 
> We could do something like:
> 
> #define USB_CONN_RING_SZ(pgsz) __CONST_RING_SIZE(usbif_conn, (pgsz)
> #ifdef PAGE_SIZE
> #define USB_CONN_RING_SIZE USB_CONN_RING_SZ(PAGE_SIZE)
> #endif
> 
> And then modify qemu to use USB_CONN_RING_SZ()
> 

This would also be problematic I'm afraid.  One's build could break
randomly depending on the order of header files inclusion. I think it
would even be harder to debug such bug than the existing one.

Inspired by Andrew on IRC. I think we can do:

/* Backward-compatible macro, use _SIZE2 variant in new code instead */
#define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, 4096)

#define USB_CONN_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_conn, (pgsize))

The assumption is that all existing implementations of the said protocol
use 4k page size. Juergen, do you think this would work?

Any thought?

Wei.

> 
> Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Juergen Gross 
On 19/10/16 10:50, Wei Liu wrote:
> On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
>> Hi all,
>>
>> While I was investigating the recent libxl ARM64 build issue, I found
>> another ARM64 build problem. This one is in QEMU:
>>
>>
>>   CChw/usb/xen-usb.o
>> hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
>> hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in this 
>> function)
>>  (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
>> ^
>> hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported only 
>> once for each function it appears in
>> In file included from hw/usb/xen-usb.c:36:0:
>> hw/usb/xen-usb.c: In function ‘usbback_alloc’:
>> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
>> ‘PAGE_SIZE’ undeclared (first use in this function)
>>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
>> ^
>> /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
>> definition of macro ‘__RD32’
>>  #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : __RD16(_x))
>>^
>> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: in 
>> expansion of macro ‘__CONST_RING_SIZE’
>>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
>>^
>> hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
>>  max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
>>^
>> make: *** [hw/usb/xen-usb.o] Error 1
>>
>>
>> It is caused by PAGE_SIZE being utilized in
>> xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
>> QEMU includes xen/include/public/io/usbif.h directly, therefore
>> hw/usb/xen-usb.c doesn't build.
>>
>> I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
>> that file is present in too many Xen releases to ignore now. So I am
>> going to work around the issue in QEMU.
>>
> 
> Actually it was introduced in 4.7.
> 
>> ---
>>
>> xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
>> which uses PAGE_SIZE without defining it. The header file has been
>> shipped with too many Xen releases to be able to solve the problem at
>> the source.
>>
>> This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.
>>
>> Signed-off-by: Stefano Stabellini 
>>
>> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
>> index 174d715..ca9df87 100644
>> --- a/hw/usb/xen-usb.c
>> +++ b/hw/usb/xen-usb.c
>> @@ -34,6 +34,11 @@
>>  #include "qapi/qmp/qstring.h"
>>  
>>  #include 
>> +
>> +/* xen/io/usbif.h references PAGE_SIZE without defining it */
>> +#ifndef PAGE_SIZE
>> +#define PAGE_SIZE XC_PAGE_SIZE
>> +#endif
>>  #include 
> 
> 
> Can we just change the macro to
> 
>   #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, (pg_size)
> 
> ?

Not easily. On x86 qemu builds just fine and modifying the macro to take
a parameter would break the build.

We could do something like:

#define USB_CONN_RING_SZ(pgsz) __CONST_RING_SIZE(usbif_conn, (pgsz)
#ifdef PAGE_SIZE
#define USB_CONN_RING_SIZE USB_CONN_RING_SZ(PAGE_SIZE)
#endif

And then modify qemu to use USB_CONN_RING_SZ()


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-19 Thread Wei Liu 
On Tue, Oct 18, 2016 at 12:09:55PM -0700, Stefano Stabellini wrote:
> Hi all,
> 
> While I was investigating the recent libxl ARM64 build issue, I found
> another ARM64 build problem. This one is in QEMU:
> 
> 
>   CChw/usb/xen-usb.o
> hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
> hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in this 
> function)
>  (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
> ^
> hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported only 
> once for each function it appears in
> In file included from hw/usb/xen-usb.c:36:0:
> hw/usb/xen-usb.c: In function ‘usbback_alloc’:
> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
> ‘PAGE_SIZE’ undeclared (first use in this function)
>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> ^
> /users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
> definition of macro ‘__RD32’
>  #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : __RD16(_x))
>^
> /users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: in 
> expansion of macro ‘__CONST_RING_SIZE’
>  #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
>^
> hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
>  max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
>^
> make: *** [hw/usb/xen-usb.o] Error 1
> 
> 
> It is caused by PAGE_SIZE being utilized in
> xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
> QEMU includes xen/include/public/io/usbif.h directly, therefore
> hw/usb/xen-usb.c doesn't build.
> 
> I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
> that file is present in too many Xen releases to ignore now. So I am
> going to work around the issue in QEMU.
> 

Actually it was introduced in 4.7.

> ---
> 
> xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
> which uses PAGE_SIZE without defining it. The header file has been
> shipped with too many Xen releases to be able to solve the problem at
> the source.
> 
> This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.
> 
> Signed-off-by: Stefano Stabellini 
> 
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index 174d715..ca9df87 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -34,6 +34,11 @@
>  #include "qapi/qmp/qstring.h"
>  
>  #include 
> +
> +/* xen/io/usbif.h references PAGE_SIZE without defining it */
> +#ifndef PAGE_SIZE
> +#define PAGE_SIZE XC_PAGE_SIZE
> +#endif
>  #include 


Can we just change the macro to

  #define USB_CONN_RING_SIZE(pg_size) __CONST_RING_SIZE(usbif_conn, (pg_size)

?

I also found a similar issue in vscsiif.h.

Wei.


>  
>  /*


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen-usb.c/usbif.h: fix ARM64 build issue

2016-10-18 Thread Stefano Stabellini 
Hi all,

While I was investigating the recent libxl ARM64 build issue, I found
another ARM64 build problem. This one is in QEMU:


  CChw/usb/xen-usb.o
hw/usb/xen-usb.c: In function ‘usbback_gnttab_map’:
hw/usb/xen-usb.c:163:56: error: ‘PAGE_SIZE’ undeclared (first use in this 
function)
 (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
^
hw/usb/xen-usb.c:163:56: note: each undeclared identifier is reported only once 
for each function it appears in
In file included from hw/usb/xen-usb.c:36:0:
hw/usb/xen-usb.c: In function ‘usbback_alloc’:
/users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:56: error: 
‘PAGE_SIZE’ undeclared (first use in this function)
 #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
^
/users/stefanos/xen/tools/../tools/include/xen/io/ring.h:45:23: note: in 
definition of macro ‘__RD32’
 #define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : __RD16(_x))
   ^
/users/stefanos/xen/tools/../tools/include/xen/io/usbif.h:229:27: note: in 
expansion of macro ‘__CONST_RING_SIZE’
 #define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
   ^
hw/usb/xen-usb.c:1029:51: note: in expansion of macro ‘USB_URB_RING_SIZE’
 max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
   ^
make: *** [hw/usb/xen-usb.o] Error 1


It is caused by PAGE_SIZE being utilized in
xen/include/public/io/usbif.h, but undefined on ARM64 (in userspace).
QEMU includes xen/include/public/io/usbif.h directly, therefore
hw/usb/xen-usb.c doesn't build.

I don't think we should be using "PAGE_SIZE" in public/io/usbif.h, but
that file is present in too many Xen releases to ignore now. So I am
going to work around the issue in QEMU.

---

xen-usb.c does not build on ARM64 because it includes xen/io/usbif.h
which uses PAGE_SIZE without defining it. The header file has been
shipped with too many Xen releases to be able to solve the problem at
the source.

This patch define PAGE_SIZE as XC_PAGE_SIZE when undefined.

Signed-off-by: Stefano Stabellini 

diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 174d715..ca9df87 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -34,6 +34,11 @@
 #include "qapi/qmp/qstring.h"
 
 #include 
+
+/* xen/io/usbif.h references PAGE_SIZE without defining it */
+#ifndef PAGE_SIZE
+#define PAGE_SIZE XC_PAGE_SIZE
+#endif
 #include 
 
 /*___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel