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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxxx 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@xxxxxxxxxx>,
> > > 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@xxxxxxxxxx>

I sent a pull request to qemu with the patch and applied it to staging.

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