Re: [PATCH v1 06/13] xen/arm: set up shared memory foreign mapping for borrower domain

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 11 Mar 2022, Penny Zheng wrote:
> From: Penny Zheng <penny.zheng@xxxxxxx>
> 
> This commits introduces a new helper guest_physmap_add_shm to set up shared
> memory foreign mapping for borrower domain.
> 
> Firstly it should get and take reference of statically shared pages from
> owner dom_shared. Then it will setup P2M foreign memory map of these 
> statically
> shared pages for borrower domain.
> 
> This commits only considers owner domain is the default dom_shared, the
> other scenario will be covered in the following patches.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 52 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 984e70e5fc..8cee5ffbd1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -798,6 +798,48 @@ static int __init allocate_shared_memory(struct domain 
> *d,
>      return ret;
>  }
>  
> +static int __init guest_physmap_add_shm(struct domain *od, struct domain *bd,
> +                                        unsigned long o_gfn,
> +                                        unsigned long b_gfn,
> +                                        unsigned long nr_gfns)

They should probably be gfn_t type


> +{
> +    struct page_info **pages = NULL;
> +    p2m_type_t p2mt, t;
> +    int ret = 0;
> +
> +    pages = xmalloc_array(struct page_info *, nr_gfns);
> +    if ( !pages )
> +        return -ENOMEM;
> +
> +    /*
> +     * Take reference of statically shared pages from owner domain.
> +     * Reference will be released when destroying shared memory region.
> +     */
> +    ret = get_pages_from_gfn(od, o_gfn, nr_gfns, pages, &p2mt, P2M_ALLOC);
> +    if ( ret )
> +    {
> +        ret = -EINVAL;
> +        goto fail_pages;
> +    }
> +
> +    if ( p2m_is_ram(p2mt) )
> +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
> +    else
> +    {
> +        ret = -EINVAL;
> +        goto fail_pages;
> +    }

One idea is to initialize p2mt = p2m_ram_rw and pass it to
get_pages_from_gfn. Then get_pages_from_gfn can return error immediately
if any of the pages are of different type.

This way there is no need for checking again here.


> +    /* Set up guest foreign map. */
> +    ret = guest_physmap_add_pages(bd, _gfn(b_gfn), page_to_mfn(pages[0]),
> +                                  nr_gfns, t);
> +
> + fail_pages:
> +        xfree(pages);
> +
> +    return ret;
> +}
> +
>  static int __init process_shm(struct domain *d,
>                                const struct dt_device_node *node)
>  {
> @@ -855,6 +897,16 @@ static int __init process_shm(struct domain *d,
>  
>              set_bit(shm_id, shm_mask);
>          }
> +
> +        /*
> +         * All domains are borrower domains when owner domain is the
> +         * default dom_shared, so here we could just set up P2M foreign
> +         * mapping for borrower domain immediately.
> +         */
> +        ret = guest_physmap_add_shm(dom_shared, d, PFN_DOWN(pbase),
> +                                    PFN_DOWN(gbase), PFN_DOWN(psize));
> +        if ( ret )
> +            return ret;
>      }
>  
>      return 0;
> -- 
> 2.25.1
>