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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

Jan



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