[Xen-devel,for-4.12,v2,2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn

In a follow-up patches, we will need to handle get_page_from_gfn
differently for DOMID_XEN. To keep the code simple move the current
content in a new separate helper p2m_get_page_from_gfn.

Note the new helper is a not anymore a static inline function as the helper
is quite complex.

Finally, take the opportunity to use typesafe gfn as the change is
minor.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/p2m.c        | 32 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h | 33 ++++-----------------------------
 2 files changed, 36 insertions(+), 29 deletions(-)

On Thu, 20 Dec 2018, Julien Grall wrote:
> In a follow-up patches, we will need to handle get_page_from_gfn
     ^ remove a

Aside from that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> differently for DOMID_XEN. To keep the code simple move the current
> content in a new separate helper p2m_get_page_from_gfn.
> 
> Note the new helper is a not anymore a static inline function as the helper
> is quite complex.
> 
> Finally, take the opportunity to use typesafe gfn as the change is
> minor.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/p2m.c        | 32 ++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/p2m.h | 33 ++++-----------------------------
>  2 files changed, 36 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2b5e43f50a..cd34149d13 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -406,6 +406,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
>      return mfn;
>  }
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t)
> +{
> +    struct page_info *page;
> +    p2m_type_t p2mt;
> +    mfn_t mfn = p2m_lookup(d, gfn, &p2mt);
> +
> +    if (t)
> +        *t = p2mt;
> +
> +    if ( !p2m_is_any_ram(p2mt) )
> +        return NULL;
> +
> +    if ( !mfn_valid(mfn) )
> +        return NULL;
> +    page = mfn_to_page(mfn);
> +
> +    /*
> +     * get_page won't work on foreign mapping because the page doesn't
> +     * belong to the current domain.
> +     */
> +    if ( p2m_is_foreign(p2mt) )
> +    {
> +        struct domain *fdom = page_get_owner_and_reference(page);
> +        ASSERT(fdom != NULL);
> +        ASSERT(fdom != d);
> +        return page;
> +    }
> +
> +    return (get_page(page, d) ? page: NULL);
> +}
> +
>  int guest_physmap_mark_populate_on_demand(struct domain *d,
>                                            unsigned long gfn,
>                                            unsigned int order)
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 01cd3ee4b5..4db8e8709d 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -289,38 +289,13 @@ typedef unsigned int p2m_query_t;
>  #define P2M_ALLOC    (1u<<0)   /* Populate PoD and paged-out entries */
>  #define P2M_UNSHARE  (1u<<1)   /* Break CoW sharing */
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t);
> +
>  static inline struct page_info *get_page_from_gfn(
>      struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>  {
> -    struct page_info *page;
> -    p2m_type_t p2mt;
> -    mfn_t mfn = p2m_lookup(d, _gfn(gfn), &p2mt);
> -
> -    if (t)
> -        *t = p2mt;
> -
> -    if ( !p2m_is_any_ram(p2mt) )
> -        return NULL;
> -
> -    if ( !mfn_valid(mfn) )
> -        return NULL;
> -    page = mfn_to_page(mfn);
> -
> -    /*
> -     * get_page won't work on foreign mapping because the page doesn't
> -     * belong to the current domain.
> -     */
> -    if ( p2m_is_foreign(p2mt) )
> -    {
> -        struct domain *fdom = page_get_owner_and_reference(page);
> -        ASSERT(fdom != NULL);
> -        ASSERT(fdom != d);
> -        return page;
> -    }
> -
> -    if ( !get_page(page, d) )
> -        return NULL;
> -    return page;
> +    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
>  }
>  
>  int get_page_type(struct page_info *page, unsigned long type);
> -- 
> 2.11.0
>

Hi Stefano,

On 20/12/2018 22:56, Stefano Stabellini wrote:
> On Thu, 20 Dec 2018, Julien Grall wrote:
>> In a follow-up patches, we will need to handle get_page_from_gfn
>       ^ remove a

I have removed the "es" from "patches" and keep the "a" instead.

> 
> Aside from that:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you for the review!

Cheers,

On 20/12/2018 19:23, Julien Grall wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2b5e43f50a..cd34149d13 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -406,6 +406,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
>      return mfn;
>  }
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t)
> +{
> +    struct page_info *page;
> +    p2m_type_t p2mt;
> +    mfn_t mfn = p2m_lookup(d, gfn, &p2mt);
> +
> +    if (t)

Spaces

> +        *t = p2mt;
> +
> +    if ( !p2m_is_any_ram(p2mt) )
> +        return NULL;
> +
> +    if ( !mfn_valid(mfn) )
> +        return NULL;

Newline

> +    page = mfn_to_page(mfn);
> +
> +    /*
> +     * get_page won't work on foreign mapping because the page doesn't
> +     * belong to the current domain.
> +     */
> +    if ( p2m_is_foreign(p2mt) )
> +    {
> +        struct domain *fdom = page_get_owner_and_reference(page);
> +        ASSERT(fdom != NULL);
> +        ASSERT(fdom != d);
> +        return page;
> +    }
> +
> +    return (get_page(page, d) ? page: NULL);

No need for the outer brackets.

All trivial style issues, so can be fixed on commit.

~Andrew