> -----Original Message----- > From: George Dunlap [mailto:[email protected]] > Sent: 11 July 2018 14:05 > To: Paul Durrant <[email protected]>; [email protected] > Cc: Jan Beulich <[email protected]>; Andrew Cooper > <[email protected]>; Ian Jackson <[email protected]>; > Julien Grall <[email protected]>; Konrad Rzeszutek Wilk > <[email protected]>; Stefano Stabellini <[email protected]>; Tim > (Xen.org) <[email protected]>; Wei Liu <[email protected]> > Subject: Re: [PATCH v2 11/13] memory: add get_paged_gfn() as a wrapper... > > On 07/11/2018 01:31 PM, Paul Durrant wrote: > >>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > >>> index c6b99c4116..510f37f100 100644 > >>> --- a/xen/common/grant_table.c > >>> +++ b/xen/common/grant_table.c > >>> @@ -375,39 +375,23 @@ static int get_paged_frame(unsigned long gfn, > >> mfn_t *mfn, > >>> struct page_info **page, bool readonly, > >>> struct domain *rd) > >>> { > >>> - int rc = GNTST_okay; > >>> - p2m_type_t p2mt; > >>> - > >>> - *mfn = INVALID_MFN; > >>> - *page = get_page_from_gfn(rd, gfn, &p2mt, > >>> - readonly ? P2M_ALLOC : P2M_UNSHARE); > >>> - if ( !*page ) > >>> - { > >>> -#ifdef P2M_SHARED_TYPES > >>> - if ( p2m_is_shared(p2mt) ) > >>> - return GNTST_eagain; > >>> -#endif > >>> -#ifdef P2M_PAGES_TYPES > >>> - if ( p2m_is_paging(p2mt) ) > >>> - { > >>> - p2m_mem_paging_populate(rd, gfn); > >>> - return GNTST_eagain; > >>> - } > >>> -#endif > >>> - return GNTST_bad_page; > >>> - } > >>> + int rc; > >>> > >>> - if ( p2m_is_foreign(p2mt) ) > >> [snip] > >>> { > >> [snip] > >>> - put_page(*page); > >>> - *page = NULL; > >>> - > >> > >> Comparing before-and-after, this seems to remove this > 'p2m_is_foreign()' > >> check. Am I missing something? > >> > > > > I may be. I thought p2m_is_ram() ruled out foreign pages > (p2m_is_any_ram() being the way to include foreign maps if required). I'll > check. > > Looks like you're right. But then, are you sure that's what we want for > the other callers? Might we not need to do an emulation that ends up > writing into a foreign mapping, for instance?
If we do then I'd expect the emulation to know the domid that owns the page and thus the page would not be foreign to the specified domid. In fact, for x86 at least, get_page_from_gfn() will fail unless the domid of the page owner is specified so there's no prospect of the page being foreign in the p2mt check anyway. Paul > > -George _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
