* [PATCH v2 0/2] Remove PAGE_SIZE from public headers @ 2016-10-19 19:21 stefano 2016-10-19 19:22 ` [PATCH v2 1/2] usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE Stefano Stabellini 2016-10-20 5:10 ` [PATCH v2 0/2] Remove PAGE_SIZE from public headers Juergen Gross 0 siblings, 2 replies; 13+ messages in thread From: stefano @ 2016-10-19 19:21 UTC (permalink / raw) To: Xen-devel Cc: Juergen Gross, Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Jan Beulich Reference to PAGE_SIZE slipped in to two public header files. QEMU build on ARM64 is broken by this. PAGE_SIZE should not be used because it could be undefined or it could be defined differently on different operating systems. Stefano Stabellini (2): usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE xen/include/public/io/usbif.h | 5 +++-- xen/include/public/io/vscsiif.h | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE 2016-10-19 19:21 [PATCH v2 0/2] Remove PAGE_SIZE from public headers stefano @ 2016-10-19 19:22 ` Stefano Stabellini 2016-10-19 19:22 ` [PATCH v2 2/2] vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE Stefano Stabellini 2016-10-20 5:10 ` [PATCH v2 0/2] Remove PAGE_SIZE from public headers Juergen Gross 1 sibling, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2016-10-19 19:22 UTC (permalink / raw) To: xen-devel Cc: jgross, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson, tim, jbeulich Do not reference PAGE_SIZE directly: it could be undefined, or it could have different values in the frontend or in the backend. Define USBIF_RING_SIZE as 4096, assuming all users of usbif.h have 4K page granularity. Replace PAGE_SIZE with USBIF_RING_SIZE. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- xen/include/public/io/usbif.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h index 4053c24..c6a5863 100644 --- a/xen/include/public/io/usbif.h +++ b/xen/include/public/io/usbif.h @@ -170,6 +170,7 @@ enum usb_spec_version { #define USBIF_MAX_SEGMENTS_PER_REQUEST (16) #define USBIF_MAX_PORTNR 31 +#define USBIF_RING_SIZE 4096 /* * RING for transferring urbs. @@ -226,7 +227,7 @@ 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) +#define USB_URB_RING_SIZE __CONST_RING_SIZE(usbif_urb, USBIF_RING_SIZE) /* * RING for notifying connect/disconnect events to frontend @@ -248,6 +249,6 @@ 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) +#define USB_CONN_RING_SIZE __CONST_RING_SIZE(usbif_conn, USBIF_RING_SIZE) #endif /* __XEN_PUBLIC_IO_USBIF_H__ */ -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE 2016-10-19 19:22 ` [PATCH v2 1/2] usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE Stefano Stabellini @ 2016-10-19 19:22 ` Stefano Stabellini 0 siblings, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2016-10-19 19:22 UTC (permalink / raw) To: xen-devel Cc: jgross, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson, tim, jbeulich Do not reference PAGE_SIZE directly: it could be undefined, or it could have different values in the frontend or in the backend. Define VSCSIIF_PAGE_SIZE as 4096, assuming all users of vscsiif.h have 4K page granularity. Replace PAGE_SIZE with VSCSIIF_PAGE_SIZE. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- xen/include/public/io/vscsiif.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h index 0a4709f..d0bd3b5 100644 --- a/xen/include/public/io/vscsiif.h +++ b/xen/include/public/io/vscsiif.h @@ -248,6 +248,7 @@ */ #define VSCSIIF_MAX_COMMAND_SIZE 16 #define VSCSIIF_SENSE_BUFFERSIZE 96 +#define VSCSIIF_PAGE_SIZE 4096 struct scsiif_request_segment { grant_ref_t gref; @@ -256,7 +257,7 @@ struct scsiif_request_segment { }; typedef struct scsiif_request_segment vscsiif_segment_t; -#define VSCSIIF_SG_PER_PAGE (PAGE_SIZE / sizeof(struct scsiif_request_segment)) +#define VSCSIIF_SG_PER_PAGE (VSCSIIF_PAGE_SIZE / sizeof(struct scsiif_request_segment)) /* Size of one request is 252 bytes */ struct vscsiif_request { -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-19 19:21 [PATCH v2 0/2] Remove PAGE_SIZE from public headers stefano 2016-10-19 19:22 ` [PATCH v2 1/2] usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE Stefano Stabellini @ 2016-10-20 5:10 ` Juergen Gross 2016-10-20 19:09 ` Stefano Stabellini 1 sibling, 1 reply; 13+ messages in thread From: Juergen Gross @ 2016-10-20 5:10 UTC (permalink / raw) To: stefano, Xen-devel Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Jan Beulich On 19/10/16 21:21, stefano@aporeto.com wrote: > Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > ARM64 is broken by this. PAGE_SIZE should not be used because it could > be undefined or it could be defined differently on different operating > systems. > > Stefano Stabellini (2): > usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > > xen/include/public/io/usbif.h | 5 +++-- > xen/include/public/io/vscsiif.h | 3 ++- > 2 files changed, 5 insertions(+), 3 deletions(-) > In principle I'm okay with these changes. But wouldn't it be better to add #define XEN_RING_PAGE_SIZE 4096 to xen/include/public/io/ring.h instead of having each interface file its own definition? Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 5:10 ` [PATCH v2 0/2] Remove PAGE_SIZE from public headers Juergen Gross @ 2016-10-20 19:09 ` Stefano Stabellini 2016-10-20 19:42 ` Andrew Cooper 0 siblings, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2016-10-20 19:09 UTC (permalink / raw) To: Juergen Gross Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, stefano, Jan Beulich, Xen-devel On Thu, 20 Oct 2016, Juergen Gross wrote: > On 19/10/16 21:21, stefano@aporeto.com wrote: > > Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > > ARM64 is broken by this. PAGE_SIZE should not be used because it could > > be undefined or it could be defined differently on different operating > > systems. > > > > Stefano Stabellini (2): > > usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > > vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > > > > xen/include/public/io/usbif.h | 5 +++-- > > xen/include/public/io/vscsiif.h | 3 ++- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > In principle I'm okay with these changes. But wouldn't it be > better to add > > #define XEN_RING_PAGE_SIZE 4096 > > to xen/include/public/io/ring.h instead of having each interface > file its own definition? I would prefer to avoid having to spell out the page size for all the io interfaces, because the same interfaces should work fine with other page granularities, if the grant table hypercalls supported them. However in reality frontend and backend would have to agree on the page granularity to use beforehand. Some sort of consensus protocol is required. When that protocol is introduced, we could get rid of #define XEN_RING_PAGE_SIZE 4096. What do people think about this? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 19:09 ` Stefano Stabellini @ 2016-10-20 19:42 ` Andrew Cooper 2016-10-20 20:04 ` Stefano Stabellini 0 siblings, 1 reply; 13+ messages in thread From: Andrew Cooper @ 2016-10-20 19:42 UTC (permalink / raw) To: Stefano Stabellini, Juergen Gross Cc: Wei Liu, George Dunlap, Tim Deegan, Ian Jackson, stefano, Jan Beulich, Xen-devel On 20/10/16 20:09, Stefano Stabellini wrote: > On Thu, 20 Oct 2016, Juergen Gross wrote: >> On 19/10/16 21:21, stefano@aporeto.com wrote: >>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on >>> ARM64 is broken by this. PAGE_SIZE should not be used because it could >>> be undefined or it could be defined differently on different operating >>> systems. >>> >>> Stefano Stabellini (2): >>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE >>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE >>> >>> xen/include/public/io/usbif.h | 5 +++-- >>> xen/include/public/io/vscsiif.h | 3 ++- >>> 2 files changed, 5 insertions(+), 3 deletions(-) >>> >> In principle I'm okay with these changes. But wouldn't it be >> better to add >> >> #define XEN_RING_PAGE_SIZE 4096 >> >> to xen/include/public/io/ring.h instead of having each interface >> file its own definition? > I would prefer to avoid having to spell out the page size for all the io > interfaces, because the same interfaces should work fine with other page > granularities, if the grant table hypercalls supported them. > > However in reality frontend and backend would have to agree on the page > granularity to use beforehand. Some sort of consensus protocol is > required. When that protocol is introduced, we could get rid of > #define XEN_RING_PAGE_SIZE 4096. > > What do people think about this? Not all current rings are 4k in size, and not all future rings should be implicitly restricted to 4k. -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing than helpful. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 19:42 ` Andrew Cooper @ 2016-10-20 20:04 ` Stefano Stabellini 2016-10-20 20:05 ` Andrew Cooper 0 siblings, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2016-10-20 20:04 UTC (permalink / raw) To: Andrew Cooper Cc: Juergen Gross, Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson, stefano, Jan Beulich, Xen-devel On Thu, 20 Oct 2016, Andrew Cooper wrote: > On 20/10/16 20:09, Stefano Stabellini wrote: > > On Thu, 20 Oct 2016, Juergen Gross wrote: > >> On 19/10/16 21:21, stefano@aporeto.com wrote: > >>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > >>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > >>> be undefined or it could be defined differently on different operating > >>> systems. > >>> > >>> Stefano Stabellini (2): > >>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > >>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > >>> > >>> xen/include/public/io/usbif.h | 5 +++-- > >>> xen/include/public/io/vscsiif.h | 3 ++- > >>> 2 files changed, 5 insertions(+), 3 deletions(-) > >>> > >> In principle I'm okay with these changes. But wouldn't it be > >> better to add > >> > >> #define XEN_RING_PAGE_SIZE 4096 > >> > >> to xen/include/public/io/ring.h instead of having each interface > >> file its own definition? > > I would prefer to avoid having to spell out the page size for all the io > > interfaces, because the same interfaces should work fine with other page > > granularities, if the grant table hypercalls supported them. > > > > However in reality frontend and backend would have to agree on the page > > granularity to use beforehand. Some sort of consensus protocol is > > required. When that protocol is introduced, we could get rid of > > #define XEN_RING_PAGE_SIZE 4096. > > > > What do people think about this? > > Not all current rings are 4k in size, and not all future rings should be > implicitly restricted to 4k. > > -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > than helpful. Indeed, you are right. Since you are at it, feel like acking the current patches? :-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 20:04 ` Stefano Stabellini @ 2016-10-20 20:05 ` Andrew Cooper 2016-10-20 20:38 ` Stefano Stabellini 2016-10-21 8:59 ` Wei Liu 0 siblings, 2 replies; 13+ messages in thread From: Andrew Cooper @ 2016-10-20 20:05 UTC (permalink / raw) To: Stefano Stabellini Cc: Juergen Gross, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson, stefano, Jan Beulich, Xen-devel On 20/10/16 21:04, Stefano Stabellini wrote: > On Thu, 20 Oct 2016, Andrew Cooper wrote: >> On 20/10/16 20:09, Stefano Stabellini wrote: >>> On Thu, 20 Oct 2016, Juergen Gross wrote: >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could >>>>> be undefined or it could be defined differently on different operating >>>>> systems. >>>>> >>>>> Stefano Stabellini (2): >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE >>>>> >>>>> xen/include/public/io/usbif.h | 5 +++-- >>>>> xen/include/public/io/vscsiif.h | 3 ++- >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) >>>>> >>>> In principle I'm okay with these changes. But wouldn't it be >>>> better to add >>>> >>>> #define XEN_RING_PAGE_SIZE 4096 >>>> >>>> to xen/include/public/io/ring.h instead of having each interface >>>> file its own definition? >>> I would prefer to avoid having to spell out the page size for all the io >>> interfaces, because the same interfaces should work fine with other page >>> granularities, if the grant table hypercalls supported them. >>> >>> However in reality frontend and backend would have to agree on the page >>> granularity to use beforehand. Some sort of consensus protocol is >>> required. When that protocol is introduced, we could get rid of >>> #define XEN_RING_PAGE_SIZE 4096. >>> >>> What do people think about this? >> Not all current rings are 4k in size, and not all future rings should be >> implicitly restricted to 4k. >> >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing >> than helpful. > Indeed, you are right. Since you are at it, feel like acking the current > patches? :-) Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, although you also need a release ack at this point. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 20:05 ` Andrew Cooper @ 2016-10-20 20:38 ` Stefano Stabellini 2016-10-21 9:00 ` Wei Liu 2016-10-21 8:59 ` Wei Liu 1 sibling, 1 reply; 13+ messages in thread From: Stefano Stabellini @ 2016-10-20 20:38 UTC (permalink / raw) To: Andrew Cooper Cc: Juergen Gross, Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson, stefano, Jan Beulich, Xen-devel On Thu, 20 Oct 2016, Andrew Cooper wrote: > On 20/10/16 21:04, Stefano Stabellini wrote: > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > >> On 20/10/16 20:09, Stefano Stabellini wrote: > >>> On Thu, 20 Oct 2016, Juergen Gross wrote: > >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: > >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > >>>>> be undefined or it could be defined differently on different operating > >>>>> systems. > >>>>> > >>>>> Stefano Stabellini (2): > >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > >>>>> > >>>>> xen/include/public/io/usbif.h | 5 +++-- > >>>>> xen/include/public/io/vscsiif.h | 3 ++- > >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) > >>>>> > >>>> In principle I'm okay with these changes. But wouldn't it be > >>>> better to add > >>>> > >>>> #define XEN_RING_PAGE_SIZE 4096 > >>>> > >>>> to xen/include/public/io/ring.h instead of having each interface > >>>> file its own definition? > >>> I would prefer to avoid having to spell out the page size for all the io > >>> interfaces, because the same interfaces should work fine with other page > >>> granularities, if the grant table hypercalls supported them. > >>> > >>> However in reality frontend and backend would have to agree on the page > >>> granularity to use beforehand. Some sort of consensus protocol is > >>> required. When that protocol is introduced, we could get rid of > >>> #define XEN_RING_PAGE_SIZE 4096. > >>> > >>> What do people think about this? > >> Not all current rings are 4k in size, and not all future rings should be > >> implicitly restricted to 4k. > >> > >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > >> than helpful. > > Indeed, you are right. Since you are at it, feel like acking the current > > patches? :-) > > Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, > although you also need a release ack at this point. Wei, I also need a release ack for http://marc.info/?l=xen-devel&m=147690490302553 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 20:38 ` Stefano Stabellini @ 2016-10-21 9:00 ` Wei Liu 2016-10-21 20:28 ` Stefano Stabellini 0 siblings, 1 reply; 13+ messages in thread From: Wei Liu @ 2016-10-21 9:00 UTC (permalink / raw) To: Stefano Stabellini Cc: Juergen Gross, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, stefano, Jan Beulich, Xen-devel On Thu, Oct 20, 2016 at 01:38:35PM -0700, Stefano Stabellini wrote: > On Thu, 20 Oct 2016, Andrew Cooper wrote: > > On 20/10/16 21:04, Stefano Stabellini wrote: > > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > > >> On 20/10/16 20:09, Stefano Stabellini wrote: > > >>> On Thu, 20 Oct 2016, Juergen Gross wrote: > > >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: > > >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > > >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > > >>>>> be undefined or it could be defined differently on different operating > > >>>>> systems. > > >>>>> > > >>>>> Stefano Stabellini (2): > > >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > > >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > > >>>>> > > >>>>> xen/include/public/io/usbif.h | 5 +++-- > > >>>>> xen/include/public/io/vscsiif.h | 3 ++- > > >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) > > >>>>> > > >>>> In principle I'm okay with these changes. But wouldn't it be > > >>>> better to add > > >>>> > > >>>> #define XEN_RING_PAGE_SIZE 4096 > > >>>> > > >>>> to xen/include/public/io/ring.h instead of having each interface > > >>>> file its own definition? > > >>> I would prefer to avoid having to spell out the page size for all the io > > >>> interfaces, because the same interfaces should work fine with other page > > >>> granularities, if the grant table hypercalls supported them. > > >>> > > >>> However in reality frontend and backend would have to agree on the page > > >>> granularity to use beforehand. Some sort of consensus protocol is > > >>> required. When that protocol is introduced, we could get rid of > > >>> #define XEN_RING_PAGE_SIZE 4096. > > >>> > > >>> What do people think about this? > > >> Not all current rings are 4k in size, and not all future rings should be > > >> implicitly restricted to 4k. > > >> > > >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > > >> than helpful. > > > Indeed, you are right. Since you are at it, feel like acking the current > > > patches? :-) > > > > Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, > > although you also need a release ack at this point. > > Wei, I also need a release ack for > > http://marc.info/?l=xen-devel&m=147690490302553 Presumably you want to apply that to our QEMU tree? If so: Release-acked-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-21 9:00 ` Wei Liu @ 2016-10-21 20:28 ` Stefano Stabellini 0 siblings, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2016-10-21 20:28 UTC (permalink / raw) To: Wei Liu Cc: Juergen Gross, Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, stefano, Jan Beulich, Xen-devel On Fri, 21 Oct 2016, Wei Liu wrote: > On Thu, Oct 20, 2016 at 01:38:35PM -0700, Stefano Stabellini wrote: > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > > > On 20/10/16 21:04, Stefano Stabellini wrote: > > > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > > > >> On 20/10/16 20:09, Stefano Stabellini wrote: > > > >>> On Thu, 20 Oct 2016, Juergen Gross wrote: > > > >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: > > > >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > > > >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > > > >>>>> be undefined or it could be defined differently on different operating > > > >>>>> systems. > > > >>>>> > > > >>>>> Stefano Stabellini (2): > > > >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > > > >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > > > >>>>> > > > >>>>> xen/include/public/io/usbif.h | 5 +++-- > > > >>>>> xen/include/public/io/vscsiif.h | 3 ++- > > > >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) > > > >>>>> > > > >>>> In principle I'm okay with these changes. But wouldn't it be > > > >>>> better to add > > > >>>> > > > >>>> #define XEN_RING_PAGE_SIZE 4096 > > > >>>> > > > >>>> to xen/include/public/io/ring.h instead of having each interface > > > >>>> file its own definition? > > > >>> I would prefer to avoid having to spell out the page size for all the io > > > >>> interfaces, because the same interfaces should work fine with other page > > > >>> granularities, if the grant table hypercalls supported them. > > > >>> > > > >>> However in reality frontend and backend would have to agree on the page > > > >>> granularity to use beforehand. Some sort of consensus protocol is > > > >>> required. When that protocol is introduced, we could get rid of > > > >>> #define XEN_RING_PAGE_SIZE 4096. > > > >>> > > > >>> What do people think about this? > > > >> Not all current rings are 4k in size, and not all future rings should be > > > >> implicitly restricted to 4k. > > > >> > > > >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > > > >> than helpful. > > > > Indeed, you are right. Since you are at it, feel like acking the current > > > > patches? :-) > > > > > > Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, > > > although you also need a release ack at this point. > > > > Wei, I also need a release ack for > > > > http://marc.info/?l=xen-devel&m=147690490302553 > > Presumably you want to apply that to our QEMU tree? If so: > > Release-acked-by: Wei Liu <wei.liu2@citrix.com> I sent a pull request to qemu with the patch and applied it to staging. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-20 20:05 ` Andrew Cooper 2016-10-20 20:38 ` Stefano Stabellini @ 2016-10-21 8:59 ` Wei Liu 2016-10-21 20:08 ` Stefano Stabellini 1 sibling, 1 reply; 13+ messages in thread From: Wei Liu @ 2016-10-21 8:59 UTC (permalink / raw) To: Andrew Cooper Cc: Juergen Gross, Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson, stefano, Jan Beulich, Xen-devel On Thu, Oct 20, 2016 at 09:05:26PM +0100, Andrew Cooper wrote: > On 20/10/16 21:04, Stefano Stabellini wrote: > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > >> On 20/10/16 20:09, Stefano Stabellini wrote: > >>> On Thu, 20 Oct 2016, Juergen Gross wrote: > >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: > >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > >>>>> be undefined or it could be defined differently on different operating > >>>>> systems. > >>>>> > >>>>> Stefano Stabellini (2): > >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > >>>>> > >>>>> xen/include/public/io/usbif.h | 5 +++-- > >>>>> xen/include/public/io/vscsiif.h | 3 ++- > >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) > >>>>> > >>>> In principle I'm okay with these changes. But wouldn't it be > >>>> better to add > >>>> > >>>> #define XEN_RING_PAGE_SIZE 4096 > >>>> > >>>> to xen/include/public/io/ring.h instead of having each interface > >>>> file its own definition? > >>> I would prefer to avoid having to spell out the page size for all the io > >>> interfaces, because the same interfaces should work fine with other page > >>> granularities, if the grant table hypercalls supported them. > >>> > >>> However in reality frontend and backend would have to agree on the page > >>> granularity to use beforehand. Some sort of consensus protocol is > >>> required. When that protocol is introduced, we could get rid of > >>> #define XEN_RING_PAGE_SIZE 4096. > >>> > >>> What do people think about this? > >> Not all current rings are 4k in size, and not all future rings should be > >> implicitly restricted to 4k. > >> > >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > >> than helpful. > > Indeed, you are right. Since you are at it, feel like acking the current > > patches? :-) > > Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, > although you also need a release ack at this point. > Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> And applied. > ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] Remove PAGE_SIZE from public headers 2016-10-21 8:59 ` Wei Liu @ 2016-10-21 20:08 ` Stefano Stabellini 0 siblings, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2016-10-21 20:08 UTC (permalink / raw) To: Wei Liu Cc: Juergen Gross, Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, stefano, Jan Beulich, Xen-devel On Fri, 21 Oct 2016, Wei Liu wrote: > On Thu, Oct 20, 2016 at 09:05:26PM +0100, Andrew Cooper wrote: > > On 20/10/16 21:04, Stefano Stabellini wrote: > > > On Thu, 20 Oct 2016, Andrew Cooper wrote: > > >> On 20/10/16 20:09, Stefano Stabellini wrote: > > >>> On Thu, 20 Oct 2016, Juergen Gross wrote: > > >>>> On 19/10/16 21:21, stefano@aporeto.com wrote: > > >>>>> Reference to PAGE_SIZE slipped in to two public header files. QEMU build on > > >>>>> ARM64 is broken by this. PAGE_SIZE should not be used because it could > > >>>>> be undefined or it could be defined differently on different operating > > >>>>> systems. > > >>>>> > > >>>>> Stefano Stabellini (2): > > >>>>> usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE > > >>>>> vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE > > >>>>> > > >>>>> xen/include/public/io/usbif.h | 5 +++-- > > >>>>> xen/include/public/io/vscsiif.h | 3 ++- > > >>>>> 2 files changed, 5 insertions(+), 3 deletions(-) > > >>>>> > > >>>> In principle I'm okay with these changes. But wouldn't it be > > >>>> better to add > > >>>> > > >>>> #define XEN_RING_PAGE_SIZE 4096 > > >>>> > > >>>> to xen/include/public/io/ring.h instead of having each interface > > >>>> file its own definition? > > >>> I would prefer to avoid having to spell out the page size for all the io > > >>> interfaces, because the same interfaces should work fine with other page > > >>> granularities, if the grant table hypercalls supported them. > > >>> > > >>> However in reality frontend and backend would have to agree on the page > > >>> granularity to use beforehand. Some sort of consensus protocol is > > >>> required. When that protocol is introduced, we could get rid of > > >>> #define XEN_RING_PAGE_SIZE 4096. > > >>> > > >>> What do people think about this? > > >> Not all current rings are 4k in size, and not all future rings should be > > >> implicitly restricted to 4k. > > >> > > >> -1 to this introducing XEN_RING_PAGE_SIZE. It will be more confusing > > >> than helpful. > > > Indeed, you are right. Since you are at it, feel like acking the current > > > patches? :-) > > > > Good point. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, > > although you also need a release ack at this point. > > > > Acked-by: Wei Liu <wei.liu2@citrix.com> > Release-acked-by: Wei Liu <wei.liu2@citrix.com> > > And applied. Thanks. Please backport to stable trees. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-10-21 20:28 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-19 19:21 [PATCH v2 0/2] Remove PAGE_SIZE from public headers stefano 2016-10-19 19:22 ` [PATCH v2 1/2] usbif.h: replace PAGE_SIZE with USBIF_RING_SIZE Stefano Stabellini 2016-10-19 19:22 ` [PATCH v2 2/2] vscsiif.h: replace PAGE_SIZE with VSCSIIF_PAGE_SIZE Stefano Stabellini 2016-10-20 5:10 ` [PATCH v2 0/2] Remove PAGE_SIZE from public headers Juergen Gross 2016-10-20 19:09 ` Stefano Stabellini 2016-10-20 19:42 ` Andrew Cooper 2016-10-20 20:04 ` Stefano Stabellini 2016-10-20 20:05 ` Andrew Cooper 2016-10-20 20:38 ` Stefano Stabellini 2016-10-21 9:00 ` Wei Liu 2016-10-21 20:28 ` Stefano Stabellini 2016-10-21 8:59 ` Wei Liu 2016-10-21 20:08 ` Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.