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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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


> 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@xxxxxxx>
> Reviewed-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
> 
> ---
>     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
> 

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