[PATCH] xen: grant-table: Simplify get_page_frame

[PATCH] xen: grant-table: Simplify get_page_frame

[Julien Grall]

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(-)

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;
 }
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xen: grant-table: Simplify get_page_frame

[Jan Beulich]

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xen: grant-table: Simplify get_page_frame

[Julien Grall]

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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel