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

[Penny Zheng <Penny.Zheng@xxxxxxx>]

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Sunday, January 8, 2023 8:23 PM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v1 05/13] xen/arm: allocate shared memory from heap
> when host address not provided
> 
> Hi Penny,
> 

Hi Julien,

> On 15/11/2022 02: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.
> 
> Please don't move pages from the {xen,dom}heap to the static heap. If you
> need to keep the pages for longer, then get an extra reference so they will
> not be released.
> 
> >
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > ---
> >   xen/arch/arm/domain_build.c | 83
> ++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 82 insertions(+), 1 deletion(-)
> >
> > +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));
> 
> Where is this freed?
> 

These kinds of info will be only used in boot-time, so maybe I shall 
free them in init_done()?
Or just after process_shm() ?

> > +    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
> 
> Cheers,
> 
> --
> Julien Grall

Cheers,

--
Penny Zheng