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

[Wei Liu <wei.liu2@citrix.com>]

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