| Message ID | 20170914164945.11566-1-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Xen-devel] xen: grant-table: Simplify get_page_frame | expand |
>>> On 14.09.17 at 18:49, <julien.grall@arm.com> wrote: > get_page_from_gfn will be able to get reference on foreign page and as > per my understanding will allow to grant page on foreign memory. > > This was not allowed with a simple get_page(...) on the ARM > implementation (no sharing nor paging supprot) but is allowed on the x86 > implementation due to get_page_from_gfn. > > On x86, foreign pages are currently only allowed for PVH dom0, so I > think it is not a big deal for now. > > On Arm, foreign pages can be present on any domain. So this patch would > permit grant on foreing pages. > > So I would like to check whether it would be valid to grant a mapping on > foreign pages? No, I don't think grants should be permitted for other domains' pages. > If not, I could add a check if ( p2m_is_foreign(...) ) return > GTNST_bad_page. Please do. > @@ -267,26 +266,20 @@ static int get_paged_frame(unsigned long gfn, unsigned long *frame, > if ( !(*page) ) > { > *frame = mfn_x(INVALID_MFN); > +#if defined(P2M_SHARED_TYPES) #ifdef please. > if ( p2m_is_shared(p2mt) ) > return GNTST_eagain; > +#endif > +#if defined(P2M_PAGES_TYPES) Same here. Jan
Hi Jan, On 09/15/2017 01:15 PM, Jan Beulich wrote: >>>> On 14.09.17 at 18:49, <julien.grall@arm.com> wrote: >> get_page_from_gfn will be able to get reference on foreign page and as >> per my understanding will allow to grant page on foreign memory. >> >> This was not allowed with a simple get_page(...) on the ARM >> implementation (no sharing nor paging supprot) but is allowed on the x86 >> implementation due to get_page_from_gfn. >> >> On x86, foreign pages are currently only allowed for PVH dom0, so I >> think it is not a big deal for now. >> >> On Arm, foreign pages can be present on any domain. So this patch would >> permit grant on foreing pages. >> >> So I would like to check whether it would be valid to grant a mapping on >> foreign pages? > > No, I don't think grants should be permitted for other domains' > pages. Which seems to match the PV behavior. I will update the patch to forbid foreign pages and use #ifdef. Cheers,
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index c3895e6201..97d3200313 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -259,7 +259,6 @@ static int get_paged_frame(unsigned long gfn, unsigned long *frame, struct domain *rd) { int rc = GNTST_okay; -#if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES) p2m_type_t p2mt; *page = get_page_from_gfn(rd, gfn, &p2mt, @@ -267,26 +266,20 @@ static int get_paged_frame(unsigned long gfn, unsigned long *frame, if ( !(*page) ) { *frame = mfn_x(INVALID_MFN); +#if defined(P2M_SHARED_TYPES) if ( p2m_is_shared(p2mt) ) return GNTST_eagain; +#endif +#if defined(P2M_PAGES_TYPES) if ( p2m_is_paging(p2mt) ) { p2m_mem_paging_populate(rd, gfn); return GNTST_eagain; } +#endif return GNTST_bad_page; } *frame = page_to_mfn(*page); -#else - *frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn))); - *page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL; - if ( (!(*page)) || (!get_page(*page, rd)) ) - { - *frame = mfn_x(INVALID_MFN); - *page = NULL; - rc = GNTST_bad_page; - } -#endif return rc; }
The implementation of get_page_frame is currently different whether the architecture support sharing memory or paging memory. Both version are extremely similar so it is possible to consolidate in a single implementation. The check for shared/paged memory are gated with the respective ifdef. Potentially, dummy p2m_is_shared/p2m_is_paging could be implemented for Arm. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wei.liu2@citrix.com> get_page_from_gfn will be able to get reference on foreign page and as per my understanding will allow to grant page on foreign memory. This was not allowed with a simple get_page(...) on the ARM implementation (no sharing nor paging supprot) but is allowed on the x86 implementation due to get_page_from_gfn. On x86, foreign pages are currently only allowed for PVH dom0, so I think it is not a big deal for now. On Arm, foreign pages can be present on any domain. So this patch would permit grant on foreing pages. So I would like to check whether it would be valid to grant a mapping on foreign pages? If not, I could add a check if ( p2m_is_foreign(...) ) return GTNST_bad_page. --- xen/common/grant_table.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)