> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 19 September 2018 07:03 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper > <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>; Ian > Jackson <ian.jack...@citrix.com>; Wei Liu <wei.l...@citrix.com>; Stefano > Stabellini <sstabell...@kernel.org>; xen-devel <xen- > de...@lists.xenproject.org>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> > 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.durr...@citrix.com> wrote: > >> -----Original Message----- > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 18 September 2018 15:51 > >> To: Paul Durrant <paul.durr...@citrix.com> > >> Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper > >> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>; > Ian > >> Jackson <ian.jack...@citrix.com>; Wei Liu <wei.l...@citrix.com>; > Stefano > >> Stabellini <sstabell...@kernel.org>; xen-devel <xen- > >> de...@lists.xenproject.org>; Konrad Rzeszutek Wilk > >> <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> > >> 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.durr...@citrix.com> wrote: > >> >> -----Original Message----- > >> >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On > >> Behalf > >> >> Of Jan Beulich > >> >> Sent: 18 September 2018 14:17 > >> >> To: Paul Durrant <paul.durr...@citrix.com> > >> >> Cc: Stefano Stabellini <sstabell...@kernel.org>; Wei Liu > >> >> <wei.l...@citrix.com>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; > >> >> George Dunlap <george.dun...@citrix.com>; Andrew Cooper > >> >> <andrew.coop...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; > Tim > >> >> (Xen.org) <t...@xen.org>; Julien Grall <julien.gr...@arm.com>; xen- > devel > >> >> <xen-devel@lists.xenproject.org> > >> >> 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.durr...@citrix.com> 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 <jbeul...@suse.com> > >> >> > >> > > >> > 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? Paul > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
