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

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

[Wei Liu]

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.

It would be good if we can have this fixed either by this series or some other
method for 4.8 to avoid releasing problematic headers for another release.

Wei.

[0] <alpine.DEB.2.10.1610181145470.16441@sstabellini-ThinkPad-X260>

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Juergen Gross <jgross@suse.com>

Wei Liu (2):
  usbif.h: don't require PAGE_SIZE to be present
  vscsiif.h: don't require PAGE_SIZE to be present

 xen/include/public/io/usbif.h   | 24 ++++++++++++++++++++++--
 xen/include/public/io/vscsiif.h | 18 +++++++++++++++++-
 2 files changed, 39 insertions(+), 3 deletions(-)

--
2.1.4


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

[PATCH RFC 1/2] usbif.h: don't require PAGE_SIZE to be present

[Wei Liu]

If it is present, use it; otherwise use 4096. Also provide two new
macros to let user have control over the page size used to do
calculation.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
 xen/include/public/io/usbif.h | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h
index 4053c24..ac38318 100644
--- a/xen/include/public/io/usbif.h
+++ b/xen/include/public/io/usbif.h
@@ -216,6 +216,16 @@ struct usbif_urb_request {
 };
 typedef struct usbif_urb_request usbif_urb_request_t;
 
+/*
+ * Reference to PAGE_SIZE was overlooked when this header file was
+ * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
+ */
+#ifdef PAGE_SIZE
+#define _USB_RING_PAGE_SIZE PAGE_SIZE
+#else
+#define _USB_RING_PAGE_SIZE 4096
+#endif
+
 struct usbif_urb_response {
 	uint16_t id; /* request id */
 	uint16_t start_frame;  /* start frame (ISO) */
@@ -226,7 +236,12 @@ struct usbif_urb_response {
 typedef struct usbif_urb_response usbif_urb_response_t;
 
 DEFINE_RING_TYPES(usbif_urb, struct usbif_urb_request, struct usbif_urb_response);
-#define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
+/*
+ * Please use _SIZE2 variant in new code, _SIZE variant is kept for
+ * backward-compatibility.
+ */
+#define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, _USB_RING_PAGE_SIZE)
+#define USB_URB_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_urb, (pgsize))
 
 /*
  * RING for notifying connect/disconnect events to frontend
@@ -248,6 +263,11 @@ struct usbif_conn_response {
 typedef struct usbif_conn_response usbif_conn_response_t;
 
 DEFINE_RING_TYPES(usbif_conn, struct usbif_conn_request, struct usbif_conn_response);
-#define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, PAGE_SIZE)
+/*
+ * Please use _SIZE2 variant in new code, _SIZE variant is kept for
+ * backward-compatibility.
+ */
+#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))
 
 #endif /* __XEN_PUBLIC_IO_USBIF_H__ */
-- 
2.1.4


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

Re: [PATCH RFC 1/2] usbif.h: don't require PAGE_SIZE to be present

[Andrew Cooper]

On 19/10/16 11:46, Wei Liu wrote:
> If it is present, use it; otherwise use 4096. Also provide two new
> macros to let user have control over the page size used to do
> calculation.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>
> ---
>  xen/include/public/io/usbif.h | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h
> index 4053c24..ac38318 100644
> --- a/xen/include/public/io/usbif.h
> +++ b/xen/include/public/io/usbif.h
> @@ -216,6 +216,16 @@ struct usbif_urb_request {
>  };
>  typedef struct usbif_urb_request usbif_urb_request_t;
>  
> +/*
> + * Reference to PAGE_SIZE was overlooked when this header file was
> + * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
> + */
> +#ifdef PAGE_SIZE
> +#define _USB_RING_PAGE_SIZE PAGE_SIZE
> +#else
> +#define _USB_RING_PAGE_SIZE 4096
> +#endif

Tokens starting with an single underscore and a capital letter are reserved.

Also, I am -1 to screwing around like this.  The header file should not
reference PAGE_SIZE as it creates ABI-incompatible code when compiled on
different systems.

If we really want to get into a rant, then we *should not* be using C to
describe our ABI in the first place, for precisely reasons like this. 
ABIs should be written down in a text document.  It is fine to provide C
header files to implement an ABI described elsewhere, but using C as the
ABI statement causes repeated disasters like this.

> +
>  struct usbif_urb_response {
>  	uint16_t id; /* request id */
>  	uint16_t start_frame;  /* start frame (ISO) */
> @@ -226,7 +236,12 @@ struct usbif_urb_response {
>  typedef struct usbif_urb_response usbif_urb_response_t;
>  
>  DEFINE_RING_TYPES(usbif_urb, struct usbif_urb_request, struct usbif_urb_response);
> -#define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, PAGE_SIZE)
> +/*
> + * Please use _SIZE2 variant in new code, _SIZE variant is kept for
> + * backward-compatibility.
> + */
> +#define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, _USB_RING_PAGE_SIZE)
> +#define USB_URB_RING_SIZE2(pgsize) __CONST_RING_SIZE(usbif_urb, (pgsize))

No brackets are needed around pgsize, as it is just a passed parameter.

~Andrew

>  
>  /*
>   * RING for notifying connect/disconnect events to frontend
> @@ -248,6 +263,11 @@ struct usbif_conn_response {
>  typedef struct usbif_conn_response usbif_conn_response_t;
>  
>  DEFINE_RING_TYPES(usbif_conn, struct usbif_conn_request, struct usbif_conn_response);
> -#define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, PAGE_SIZE)
> +/*
> + * Please use _SIZE2 variant in new code, _SIZE variant is kept for
> + * backward-compatibility.
> + */
> +#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))
>  
>  #endif /* __XEN_PUBLIC_IO_USBIF_H__ */


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

Re: [PATCH RFC 1/2] usbif.h: don't require PAGE_SIZE to be present

[Wei Liu]

On Wed, Oct 19, 2016 at 12:45:31PM +0100, Andrew Cooper wrote:
> On 19/10/16 11:46, Wei Liu wrote:
> > If it is present, use it; otherwise use 4096. Also provide two new
> > macros to let user have control over the page size used to do
> > calculation.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > ---
> >  xen/include/public/io/usbif.h | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h
> > index 4053c24..ac38318 100644
> > --- a/xen/include/public/io/usbif.h
> > +++ b/xen/include/public/io/usbif.h
> > @@ -216,6 +216,16 @@ struct usbif_urb_request {
> >  };
> >  typedef struct usbif_urb_request usbif_urb_request_t;
> >  
> > +/*
> > + * Reference to PAGE_SIZE was overlooked when this header file was
> > + * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
> > + */
> > +#ifdef PAGE_SIZE
> > +#define _USB_RING_PAGE_SIZE PAGE_SIZE
> > +#else
> > +#define _USB_RING_PAGE_SIZE 4096
> > +#endif
> 
> Tokens starting with an single underscore and a capital letter are reserved.
> 

Right, this can be fixed.

> Also, I am -1 to screwing around like this.  The header file should not
> reference PAGE_SIZE as it creates ABI-incompatible code when compiled on
> different systems.

I also dislike keeping PAGE_SIZE here, but what if existing code really
expects such behaviour?  :-/

I don't know enough about the history and / or users of these macros
TBH. I trust Juergen's judgement because he knows more about this than
me -- both this and vscsi came from SuSE and predates upstream PVOPS
kernel AFAICT.

> 
> If we really want to get into a rant, then we *should not* be using C to
> describe our ABI in the first place, for precisely reasons like this. 
> ABIs should be written down in a text document.  It is fine to provide C
> header files to implement an ABI described elsewhere, but using C as the
> ABI statement causes repeated disasters like this.
> 

Agreed.

Wei.

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

Re: [PATCH RFC 1/2] usbif.h: don't require PAGE_SIZE to be present

[David Vrabel]

On 19/10/16 11:46, Wei Liu wrote:
> If it is present, use it; otherwise use 4096. Also provide two new
> macros to let user have control over the page size used to do
> calculation.
[...]
> index 4053c24..ac38318 100644
> --- a/xen/include/public/io/usbif.h
> +++ b/xen/include/public/io/usbif.h
> @@ -216,6 +216,16 @@ struct usbif_urb_request {
>  };
>  typedef struct usbif_urb_request usbif_urb_request_t;
>  
> +/*
> + * Reference to PAGE_SIZE was overlooked when this header file was
> + * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
> + */
> +#ifdef PAGE_SIZE
> +#define _USB_RING_PAGE_SIZE PAGE_SIZE

#define XEN_USBIF_RING_SIZE 4096

Since PAGE_SIZE on the backend is not necessarily PAGE_SIZE in the
frontend.  This is what the size has always been so it won't break anything.

And similarly for vscsiif.

David

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

Re: [PATCH RFC 1/2] usbif.h: don't require PAGE_SIZE to be present

[Stefano Stabellini]

On Wed, 19 Oct 2016, David Vrabel wrote:
> On 19/10/16 11:46, Wei Liu wrote:
> > If it is present, use it; otherwise use 4096. Also provide two new
> > macros to let user have control over the page size used to do
> > calculation.
> [...]
> > index 4053c24..ac38318 100644
> > --- a/xen/include/public/io/usbif.h
> > +++ b/xen/include/public/io/usbif.h
> > @@ -216,6 +216,16 @@ struct usbif_urb_request {
> >  };
> >  typedef struct usbif_urb_request usbif_urb_request_t;
> >  
> > +/*
> > + * Reference to PAGE_SIZE was overlooked when this header file was
> > + * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
> > + */
> > +#ifdef PAGE_SIZE
> > +#define _USB_RING_PAGE_SIZE PAGE_SIZE
> 
> #define XEN_USBIF_RING_SIZE 4096
> 
> Since PAGE_SIZE on the backend is not necessarily PAGE_SIZE in the
> frontend.  This is what the size has always been so it won't break anything.
> 
> And similarly for vscsiif.

That is exactly right, thank you.

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

[PATCH RFC 2/2] vscsiif.h: don't require PAGE_SIZE to be present

[Wei Liu]

If it is present, use it; otherwise use 4096. Also provide a new macro
to let user have control over the page size used to do calculation.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
 xen/include/public/io/vscsiif.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
index 0a4709f..00e8b74 100644
--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -256,7 +256,23 @@ struct scsiif_request_segment {
 };
 typedef struct scsiif_request_segment vscsiif_segment_t;
 
-#define VSCSIIF_SG_PER_PAGE (PAGE_SIZE / sizeof(struct scsiif_request_segment))
+/*
+ * Reference to PAGE_SIZE was overlooked when this header file was
+ * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
+ */
+#ifdef PAGE_SIZE
+#define _VSCSIIF_PAGE_SIZE PAGE_SIZE
+#else
+#define _VSCSIIF_PAGE_SIZE 4096
+#endif
+/*
+ * Please use _PAGE2 variant in new code, _PAGE variant is kept for
+ * backward-compatibility.
+ */
+#define VSCSIIF_SG_PER_PAGE (_VSCSI_PAGE_SIZE / \
+                             sizeof(struct scsiif_request_segment))
+#define VSCSIIF_SG_PER_PAGE2(pgsize) ((pgsize) / \
+                                      sizeof(struct scsiif_request_segment))
 
 /* Size of one request is 252 bytes */
 struct vscsiif_request {
-- 
2.1.4


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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Juergen Gross]

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.

> It would be good if we can have this fixed either by this series or some other
> method for 4.8 to avoid releasing problematic headers for another release.

Both patches:

Reviewed-by: Juergen Gross <jgross@suse.com>


Thanks,

Juergen

> 
> Wei.
> 
> [0] <alpine.DEB.2.10.1610181145470.16441@sstabellini-ThinkPad-X260>
> 
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>
> 
> Wei Liu (2):
>   usbif.h: don't require PAGE_SIZE to be present
>   vscsiif.h: don't require PAGE_SIZE to be present
> 
>  xen/include/public/io/usbif.h   | 24 ++++++++++++++++++++++--
>  xen/include/public/io/vscsiif.h | 18 +++++++++++++++++-
>  2 files changed, 39 insertions(+), 3 deletions(-)
> 
> --
> 2.1.4
> 
> 


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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Stefano Stabellini]

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.


> > It would be good if we can have this fixed either by this series or some other
> > method for 4.8 to avoid releasing problematic headers for another release.
> 
> Both patches:
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>
> 
> 
> Thanks,
> 
> Juergen
> 
> > 
> > Wei.
> > 
> > [0] <alpine.DEB.2.10.1610181145470.16441@sstabellini-ThinkPad-X260>
> > 
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > 
> > Wei Liu (2):
> >   usbif.h: don't require PAGE_SIZE to be present
> >   vscsiif.h: don't require PAGE_SIZE to be present
> > 
> >  xen/include/public/io/usbif.h   | 24 ++++++++++++++++++++++--
> >  xen/include/public/io/vscsiif.h | 18 +++++++++++++++++-
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> > 
> > --
> > 2.1.4
> > 
> > 
> 

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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Stefano Stabellini]

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 <sstabellini@kernel.org>

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


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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Juergen Gross]

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 <sstabellini@kernel.org>

Reviewed-by: Juergen Gross <jgross@suse.com>

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


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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Ian Jackson]

Stefano Stabellini writes ("Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers"):
> Yes, as long as we backport the fixes to the Xen stable trees as soon as
> possible.

I have queued both of these for backport:

d93539cc486aa6022195305dbea5fe12f90b69fe
vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE

04535bee06858fd949c743cfecc4d7b96333a16c
usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE


Thanks,
Ian.

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

Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers

[Stefano Stabellini]

On Mon, 24 Oct 2016, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH RFC 0/2] Fix PAGE_SIZE issues in public headers"):
> > Yes, as long as we backport the fixes to the Xen stable trees as soon as
> > possible.
> 
> I have queued both of these for backport:
> 
> d93539cc486aa6022195305dbea5fe12f90b69fe
> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE
> 
> 04535bee06858fd949c743cfecc4d7b96333a16c
> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE

Thank you!

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