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

[Juergen Gross <jgross@xxxxxxxx>]

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:
>>
>>
>>   CC    hw/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) & 0xffff0000) ? __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 <sstabellini@xxxxxxxxxx>
>>
>> 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/ring.h>
>> +
>> +/* xen/io/usbif.h references PAGE_SIZE without defining it */
>> +#ifndef PAGE_SIZE
>> +#define PAGE_SIZE XC_PAGE_SIZE
>> +#endif
>>  #include <xen/io/usbif.h>
> 
> 
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel