Re: [Xen-devel] [PATCH v9 5/7] memory: add check_get_page_from_gfn() as a wrapper...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
> Of Paul Durrant
> Sent: 19 September 2018 08:56
> To: 'Jan Beulich' <JBeulich@xxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
> George Dunlap <George.Dunlap@xxxxxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v9 5/7] memory: add
> check_get_page_from_gfn() as a wrapper...
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: 19 September 2018 07:03
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
> Ian
> > Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Stefano
> > Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> > <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> > Subject: RE: [Xen-devel] [PATCH v9 5/7] memory: add
> > check_get_page_from_gfn() as a wrapper...
> >
> > >>> On 18.09.18 at 18:37, <Paul.Durrant@xxxxxxxxxx> wrote:
> > >>  -----Original Message-----
> > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > >> Sent: 18 September 2018 15:51
> > >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > >> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > >> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>;
> > Ian
> > >> Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> > Stefano
> > >> Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
> > >> devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> > >> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> > >> Subject: RE: [Xen-devel] [PATCH v9 5/7] memory: add
> > >> check_get_page_from_gfn() as a wrapper...
> > >>
> > >> >>> On 18.09.18 at 16:03, <Paul.Durrant@xxxxxxxxxx> wrote:
> > >> >>  -----Original Message-----
> > >> >> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> > >> Behalf
> > >> >> Of Jan Beulich
> > >> >> Sent: 18 September 2018 14:17
> > >> >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > >> >> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu
> > >> >> <wei.liu2@xxxxxxxxxx>; Konrad Rzeszutek Wilk
> > <konrad.wilk@xxxxxxxxxx>;
> > >> >> George Dunlap <George.Dunlap@xxxxxxxxxx>; Andrew Cooper
> > >> >> <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> > Tim
> > >> >> (Xen.org) <tim@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>; xen-
> > devel
> > >> >> <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> > >> >> Subject: Re: [Xen-devel] [PATCH v9 5/7] memory: add
> > >> >> check_get_page_from_gfn() as a wrapper...
> > >> >>
> > >> >> >>> On 13.09.18 at 17:21, <paul.durrant@xxxxxxxxxx> wrote:
> > >> >> > --- a/xen/include/asm-arm/p2m.h
> > >> >> > +++ b/xen/include/asm-arm/p2m.h
> > >> >> > @@ -303,6 +303,10 @@ static inline struct page_info
> > >> *get_page_from_gfn(
> > >> >> >      return page;
> > >> >> >  }
> > >> >> >
> > >> >> > +int __must_check check_get_page_from_gfn(struct domain *d,
> gfn_t
> > >> gfn,
> > >> >> > +                                         bool readonly,
> > p2m_type_t
> > >> >> *p2mt_p,
> > >> >> > +                                         struct page_info
> > **page_p);
> > >> >> > +
> > >> >> >  int get_page_type(struct page_info *page, unsigned long type);
> > >> >> >  bool is_iomem_page(mfn_t mfn);
> > >> >> >  static inline int get_page_and_type(struct page_info *page,
> > >> >> > --- a/xen/include/asm-x86/p2m.h
> > >> >> > +++ b/xen/include/asm-x86/p2m.h
> > >> >> > @@ -492,6 +492,9 @@ static inline struct page_info
> > >> *get_page_from_gfn(
> > >> >> >      return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page :
> > NULL;
> > >> >> >  }
> > >> >> >
> > >> >> > +int __must_check check_get_page_from_gfn(struct domain *d,
> gfn_t
> > >> gfn,
> > >> >> > +                                         bool readonly,
> > p2m_type_t
> > >> >> *p2mt_p,
> > >> >> > +                                         struct page_info
> > **page_p);
> > >> >> >
> > >> >> >  /* General conversion function from mfn to gfn */
> > >> >> >  static inline unsigned long mfn_to_gfn(struct domain *d, mfn_t
> > mfn)
> > >> >>
> > >> >> Why twice the same declaration? Can't this be put in xen/p2m-
> > common.h?
> > >> >> Or some other suitable common header? With that
> > >> >> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >> >>
> > >> >
> > >> > I was not aware of the common header so I placed it near the
> > >> declarations of
> > >> > get_page_from_gfn(). Do you want be to submit a v10 with this fixed
> > or
> > >> are
> > >> > you happy to move it to wherever you think is appropriate during
> > commit?
> > >>
> > >> I'd prefer a v10, to be sure things compile fine ahead of trying to
> > commit
> > >> this series.
> > >>
> > >
> > > Moving to p2m-common won't work. The function declaration involves a
> > > p2m_type_t argument and that enum is defined in the arm and x86
> specific
> > > headers. I propose therefore to leave this patch as-is.
> >
> > Leaving the duplication in place is just the last resort imo. Does
> > xen/mm.h
> > not work either?
> 
> No, it won't. It has to be something *after* the definition of the
> p2m_type_t enum. I could, as Julien suggested, move the inclusion of p2m-
> common after that point, but it would mean the header guards would cease
> to DTRT of course. I'm not sure whether C will allow me to forward declare
> the enum (not something I've tried) but I could give that a go. Any other
> suggestions?
> 

Forward declaration of the enum does indeed appear to work, so I'll go with 
that.

  Paul

>   Paul
> 
> >
> > Jan
> >
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel