Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-21 Thread Stefano Stabellini 
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, stef...@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 ,
> > > although you also need a release ack at this point.
> > 
> > Wei, I also need a release ack for
> > 
> > http://marc.info/?l=xen-devel=147690490302553
> 
> Presumably you want to apply that to our QEMU tree? If so:
> 
> Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-21 Thread Stefano Stabellini 
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, stef...@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 ,
> > although you also need a release ack at this point.
> > 
> 
> Acked-by: Wei Liu 
> Release-acked-by: Wei Liu 
> 
> And applied.

Thanks. Please backport to stable trees.

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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-21 Thread Wei Liu 
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, stef...@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 ,
> > although you also need a release ack at this point.
> 
> Wei, I also need a release ack for
> 
> http://marc.info/?l=xen-devel=147690490302553

Presumably you want to apply that to our QEMU tree? If so:

Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-21 Thread Wei Liu 
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, stef...@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 ,
> although you also need a release ack at this point.
> 

Acked-by: Wei Liu 
Release-acked-by: Wei Liu 

And applied.


> ~Andrew

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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-20 Thread Stefano Stabellini 
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, stef...@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 ,
> although you also need a release ack at this point.

Wei, I also need a release ack for

http://marc.info/?l=xen-devel=147690490302553

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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-20 Thread Andrew Cooper 
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, stef...@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 ,
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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-20 Thread Stefano Stabellini 
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, stef...@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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-20 Thread Andrew Cooper 
On 20/10/16 20:09, Stefano Stabellini wrote:
> On Thu, 20 Oct 2016, Juergen Gross wrote:
>> On 19/10/16 21:21, stef...@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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-20 Thread Stefano Stabellini 
On Thu, 20 Oct 2016, Juergen Gross wrote:
> On 19/10/16 21:21, stef...@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

Re: [Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-19 Thread Juergen Gross 
On 19/10/16 21:21, stef...@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

[Xen-devel] [PATCH v2 0/2] Remove PAGE_SIZE from public headers

2016-10-19 Thread stefano 
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