[RFC,0/2] Fix PAGE_SIZE issues in public headers

Commit Message

Stefano Stabellini Oct. 19, 2016, 7:20 p.m. UTC

On Wed, 19 Oct 2016, Stefano Stabellini wrote:
> On Wed, 19 Oct 2016, Juergen Gross wrote:
> > On 19/10/16 12:46, Wei Liu wrote:
> > > Reference to PAGE_SIZE slipped in to two public header files. QEMU build on
> > > ARM64 is broken by this.
> > > 
> > > This series tries to fix that. The approach is discussed at [0].
> > > 
> > > QEMU will need to update its copy of usbif.h.
> > 
> > There is no QEMU copy of Xen headers. This is the reason why an
> > imcompatible change is not possible. With the patches there is no
> > longer an urgent need for qemu to be modified.
> 
> Yes, as long as we backport the fixes to the Xen stable trees as soon as
> possible.

I spoke too soon. Actually QEMU xen-usb.c references PAGE_SIZE
directly, which also needs to be fixed. In this case, I think I would
use XC_PAGE_SIZE, because we are already using it in that file and
because this way we don't tie the fix with the public headers fixes in
Xen.

---

xen-usb: do not reference PAGE_SIZE

PAGE_SIZE is undefined on ARM64. Use XC_PAGE_SIZE instead, which is
always 4096 even when page granularity is 64K.

For this to actually work with 64K pages, more changes are required.

Signed-off-by: Stefano Stabellini <[e-mail]>

Comments

Juergen Gross Oct. 20, 2016, 5:11 a.m. UTC | #1

On 19/10/16 21:20, Stefano Stabellini wrote:
> On Wed, 19 Oct 2016, Stefano Stabellini wrote:
>> On Wed, 19 Oct 2016, Juergen Gross wrote:
>>> On 19/10/16 12:46, Wei Liu wrote:
>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on
>>>> ARM64 is broken by this.
>>>>
>>>> This series tries to fix that. The approach is discussed at [0].
>>>>
>>>> QEMU will need to update its copy of usbif.h.
>>>
>>> There is no QEMU copy of Xen headers. This is the reason why an
>>> imcompatible change is not possible. With the patches there is no
>>> longer an urgent need for qemu to be modified.
>>
>> Yes, as long as we backport the fixes to the Xen stable trees as soon as
>> possible.
> 
> I spoke too soon. Actually QEMU xen-usb.c references PAGE_SIZE
> directly, which also needs to be fixed. In this case, I think I would
> use XC_PAGE_SIZE, because we are already using it in that file and
> because this way we don't tie the fix with the public headers fixes in
> Xen.
> 
> ---
> 
> xen-usb: do not reference PAGE_SIZE
> 
> PAGE_SIZE is undefined on ARM64. Use XC_PAGE_SIZE instead, which is
> always 4096 even when page granularity is 64K.
> 
> For this to actually work with 64K pages, more changes are required.
> 
> Signed-off-by: Stefano Stabellini <[e-mail]>

Reviewed-by: Juergen Gross <[e-mail]>

> 
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index 174d715..de2ebd6 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -160,7 +160,7 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
>  
>      for (i = 0; i < nr_segs; i++) {
>          if ((unsigned)usbback_req->req.seg[i].offset +
> -            (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
> +            (unsigned)usbback_req->req.seg[i].length > XC_PAGE_SIZE) {
>              xen_be_printf(xendev, 0, "segment crosses page boundary\n");
>              return -EINVAL;
>          }
> @@ -183,7 +183,7 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
>  
>          for (i = 0; i < usbback_req->nr_buffer_segs; i++) {
>              seg = usbback_req->req.seg + i;
> -            addr = usbback_req->buffer + i * PAGE_SIZE + seg->offset;
> +            addr = usbback_req->buffer + i * XC_PAGE_SIZE + seg->offset;
>              qemu_iovec_add(&usbback_req->packet.iov, addr, seg->length);
>          }
>      }
> 
>

Patch

diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 174d715..de2ebd6 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -160,7 +160,7 @@  static int usbback_gnttab_map(struct usbback_req *usbback_req)
 
     for (i = 0; i < nr_segs; i++) {
         if ((unsigned)usbback_req->req.seg[i].offset +
-            (unsigned)usbback_req->req.seg[i].length > PAGE_SIZE) {
+            (unsigned)usbback_req->req.seg[i].length > XC_PAGE_SIZE) {
             xen_be_printf(xendev, 0, "segment crosses page boundary\n");
             return -EINVAL;
         }
@@ -183,7 +183,7 @@  static int usbback_gnttab_map(struct usbback_req *usbback_req)
 
         for (i = 0; i < usbback_req->nr_buffer_segs; i++) {
             seg = usbback_req->req.seg + i;
-            addr = usbback_req->buffer + i * PAGE_SIZE + seg->offset;
+            addr = usbback_req->buffer + i * XC_PAGE_SIZE + seg->offset;
             qemu_iovec_add(&usbback_req->packet.iov, addr, seg->length);
         }
     }