Re: [PATCH v1 05/13] xen/arm: allocate shared memory from heap when host address not provided

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 11/14/22 21:52, Penny Zheng wrote:
> when host address is not provided in "xen,shared-mem", we let Xen
> allocate requested shared memory from heap, and once the shared memory is
> allocated, it will be marked as static(PGC_static), which means that it will 
> be
> reserved as static memory, and will not go back to heap even on freeing.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 83 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fbb196d8a4..3de96882a5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -835,6 +835,72 @@ static bool __init is_shm_allocated_to_domio(struct 
> shm_membank *shm_membank)
>      return true;
>  }
> 
> +static int __init mark_shared_memory_static(struct shm_membank *shm_membank)
> +{
> +    unsigned int bank;
> +    unsigned long i, nr_mfns;
> +    struct page_info *pg;
> +    struct meminfo *meminfo;
> +
> +    BUG_ON(!shm_membank->mem.banks.meminfo);
> +    meminfo = shm_membank->mem.banks.meminfo;
> +    for ( bank = 0; bank < meminfo->nr_banks; bank++ )
> +    {
> +        pg = mfn_to_page(maddr_to_mfn(meminfo->bank[bank].start));
> +        nr_mfns = PFN_DOWN(meminfo->bank[bank].size);
> +
> +        for ( i = 0; i < nr_mfns; i++ )
> +        {
> +            /* The page should be already allocated from heap. */
> +            if ( !pg[i].count_info & PGC_state_inuse )
> +            {
> +                printk(XENLOG_ERR
> +                       "pg[%lu] MFN %"PRI_mfn" c=%#lx\n",
> +                       i, mfn_x(page_to_mfn(pg)) + i, pg[i].count_info);
> +                goto fail;
> +            }
> +
> +           pg[i].count_info |= PGC_static;
> +        }
> +    }
> +
> +    return 0;
> +
> + fail:
> +    while ( bank >= 0 )

When building with EXTRA_CFLAGS_XEN_CORE="-Wtype-limits 
-Wno-error=type-limits", we get the following warning:
arch/arm/domain_build.c: In function ‘mark_shared_memory_static’:
arch/arm/domain_build.c:879:18: warning: comparison of unsigned expression in 
‘>= 0’ is always true [-Wtype-limits]
  879 |     while ( bank >= 0 )
      |                  ^~

> +    {
> +        while ( --i >= 0 )

Similarly here:
arch/arm/domain_build.c:881:21: warning: comparison of unsigned expression in 
‘>= 0’ is always true [-Wtype-limits]
  881 |         while ( --i >= 0 )
      |                     ^~

> +            pg[i].count_info &= ~PGC_static;
> +        i = PFN_DOWN(meminfo->bank[--bank].size);
> +    }
> +
> +    return -EINVAL;
> +}
> +
> +static int __init allocate_shared_memory(struct shm_membank *shm_membank,
> +                                         paddr_t psize)
> +{
> +    struct meminfo *banks;
> +    int ret;
> +
> +    BUG_ON(shm_membank->mem.banks.meminfo != NULL);
> +
> +    banks = xmalloc_bytes(sizeof(struct meminfo));
> +    if ( banks == NULL )
> +        return -ENOMEM;
> +    shm_membank->mem.banks.meminfo = banks;
> +    memset(shm_membank->mem.banks.meminfo, 0, sizeof(struct meminfo));
> +
> +    if ( !allocate_domheap_memory(NULL, psize, 
> shm_membank->mem.banks.meminfo) )
> +        return -EINVAL;
> +
> +    ret = mark_shared_memory_static(shm_membank);
> +    if ( ret )
> +        return ret;
> +
> +    return 0;
> +}
> +
>  static mfn_t __init acquire_shared_memory_bank(struct domain *d,
>                                                 paddr_t pbase, paddr_t psize)
>  {
> @@ -975,7 +1041,7 @@ static int __init process_shm(struct domain *d, struct 
> kernel_info *kinfo,
>          unsigned int i;
>          const char *role_str;
>          const char *shm_id;
> -        bool owner_dom_io = true;
> +        bool owner_dom_io = true, paddr_assigned = true;
>          struct shm_membank *shm_membank;
> 
>          if ( !dt_device_is_compatible(shm_node, 
> "xen,domain-shared-memory-v1") )
> @@ -1035,6 +1101,21 @@ static int __init process_shm(struct domain *d, struct 
> kernel_info *kinfo,
>              return -ENOENT;
>          }
> 
> +        /*
> +         * When host address is not provided in "xen,shared-mem",
> +         * we let Xen allocate requested memory from heap at first domain.
> +         */
> +        if ( !paddr_assigned && !shm_membank->mem.banks.meminfo )
> +        {
> +            ret = allocate_shared_memory(shm_membank, psize);
> +            if ( ret )
> +            {
> +                printk("%pd: failed to allocate shared memory 
> bank(%"PRIpaddr"MB) from heap: %d\n",
> +                       d, psize >> 20, ret);
> +                return ret;
> +            }
> +        }
> +
>          /*
>           * DOMID_IO is a fake domain and is not described in the Device-Tree.
>           * Therefore when the owner of the shared region is DOMID_IO, we will
> --
> 2.25.1
> 
>