Re: [PATCH v2 3/9] xen/arm: allocate static shared memory to a specific owner domain

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 6 May 2022, Penny Zheng wrote:
> If owner property is defined, then owner domain of a static shared memory
> region is not the default dom_io anymore, but a specific domain.
> 
> This commit implements allocating static shared memory to a specific domain
> when owner property is defined.
> 
> Coding flow for dealing borrower domain will be introduced later in the
> following commits.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
> v2 change:
> - P2M mapping is restricted to normal domain
> - in-code comment fix
> ---
>  xen/arch/arm/domain_build.c | 55 +++++++++++++++++++++++++++++--------
>  1 file changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e97bb6eeba..f43378227a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -798,9 +798,11 @@ static mfn_t __init acquire_shared_memory_bank(struct 
> domain *d,
>   */
>  static int __init allocate_shared_memory(struct domain *d,
>                                           u32 addr_cells, u32 size_cells,
> -                                         paddr_t pbase, paddr_t psize)
> +                                         paddr_t pbase, paddr_t psize,
> +                                         paddr_t gbase)
>  {
>      mfn_t smfn;
> +    int ret = 0;
>  
>      dprintk(XENLOG_INFO,
>              "Allocate static shared memory BANK 
> %#"PRIpaddr"-%#"PRIpaddr".\n",
> @@ -813,10 +815,20 @@ static int __init allocate_shared_memory(struct domain 
> *d,
>  
>      /*
>       * DOMID_IO is the domain, like DOMID_XEN, that is not auto-translated.
> -     * It sees RAM 1:1 and we do not need to create P2M mapping for it
> +     * It sees RAM 1:1 and we do not need to create P2M mapping for it.
>       */
> -    ASSERT(d == dom_io);
> -    return 0;
> +    if ( d != dom_io )
> +    {
> +        ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, 
> PFN_DOWN(psize));
> +        if ( ret )
> +        {
> +            printk(XENLOG_ERR
> +                   "Failed to map shared memory to %pd.\n", d);
> +            return ret;
> +        }
> +    }
> +
> +    return ret;
>  }
>  
>  static int __init process_shm(struct domain *d,
> @@ -829,6 +841,8 @@ static int __init process_shm(struct domain *d,
>      u32 shm_id;
>      u32 addr_cells, size_cells;
>      paddr_t gbase, pbase, psize;
> +    const char *role_str;
> +    bool owner_dom_io = true;
>  
>      dt_for_each_child_node(node, shm_node)
>      {
> @@ -855,19 +869,36 @@ static int __init process_shm(struct domain *d,
>          ASSERT(IS_ALIGNED(pbase, PAGE_SIZE) && IS_ALIGNED(psize, PAGE_SIZE));
>          gbase = dt_read_number(cells, addr_cells);
>  
> -        /* TODO: Consider owner domain is not the default dom_io. */
> +        /*
> +         * "role" property is optional and if it is defined explicitly,
> +         * then the owner domain is not the default "dom_io" domain.
> +         */
> +        if ( dt_property_read_string(shm_node, "role", &role_str) == 0 )
> +            owner_dom_io = false;
> +
>          /*
>           * Per static shared memory region could be shared between multiple
>           * domains.
> -         * In case re-allocating the same shared memory region, we check
> -         * if it is already allocated to the default owner dom_io before
> -         * the actual allocation.
> +         * So when owner domain is the default dom_io, in case re-allocating
> +         * the same shared memory region, we check if it is already allocated
> +         * to the default owner dom_io before the actual allocation.
>           */
> -        if ( !is_shm_allocated_to_domio(pbase) )
> +        if ( (owner_dom_io && !is_shm_allocated_to_domio(pbase)) ||
> +             (!owner_dom_io && strcmp(role_str, "owner") == 0) )
>          {
> -            /* Allocate statically shared pages to the default owner dom_io. 
> */
> -            ret = allocate_shared_memory(dom_io, addr_cells, size_cells,
> -                                         pbase, psize);
> +            struct domain *od = d;
> +            paddr_t o_gbase = gbase;
> +
> +            if ( owner_dom_io )
> +            {
> +                od = dom_io;
> +                /* For dom_io, GFN is always equal to PFN. */
> +                o_gbase = pbase;
> +            }
>

o_gbase is not used if the domain is dom_io, so I would simplify it
further to:

ret = allocate_shared_memory(owner_dom_io ? dom_io : d,
                             addr_cells, size_cells,
                             pbase, psize, gbase);





> +            /* Allocate statically shared pages to the owner domain. */
> +            ret = allocate_shared_memory(od, addr_cells, size_cells,
> +                                         pbase, psize, o_gbase);
>              if ( ret )
>                  return ret;
>          }
> -- 
> 2.25.1
>