RE: [PATCH 09/10] xen/arm: parse `xen,static-mem` info during domain construction

[Penny Zheng <Penny.Zheng@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Tuesday, May 18, 2021 8:09 PM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [PATCH 09/10] xen/arm: parse `xen,static-mem` info during
> domain construction
> 
> Hi Penny,
> 
> On 18/05/2021 06:21, Penny Zheng wrote:
> > This commit parses `xen,static-mem` device tree property, to acquire
> > static memory info reserved for this domain, when constructing domain
> > during boot-up.
> >
> > Related info shall be stored in new static_mem value under per domain
> > struct arch_domain.
> 
> So far, this seems to only be used during boot. So can't this be kept in the
> kinfo structure?
> 

Sure, I'll store it in kinfo

> >
> > Right now, the implementation of allocate_static_memory is missing,
> > and will be introduced later. It just BUG() out at the moment.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > ---
> >   xen/arch/arm/domain_build.c  | 58
> ++++++++++++++++++++++++++++++++----
> >   xen/include/asm-arm/domain.h |  3 ++
> >   2 files changed, 56 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 282416e74d..30b55588b7 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2424,17 +2424,61 @@ static int __init construct_domU(struct domain
> *d,
> >   {
> >       struct kernel_info kinfo = {};
> >       int rc;
> > -    u64 mem;
> > +    u64 mem, static_mem_size = 0;
> > +    const struct dt_property *prop;
> > +    u32 static_mem_len;
> > +    bool static_mem = false;
> > +
> > +    /*
> > +     * Guest RAM could be of static memory from static allocation,
> > +     * which will be specified through "xen,static-mem" property.
> > +     */
> > +    prop = dt_find_property(node, "xen,static-mem", &static_mem_len);
> > +    if ( prop )
> > +    {
> > +        const __be32 *cell;
> > +        u32 addr_cells = 2, size_cells = 2, reg_cells;
> > +        u64 start, size;
> > +        int i, banks;
> > +        static_mem = true;
> > +
> > +        dt_property_read_u32(node, "#address-cells", &addr_cells);
> > +        dt_property_read_u32(node, "#size-cells", &size_cells);
> > +        BUG_ON(size_cells > 2 || addr_cells > 2);
> > +        reg_cells = addr_cells + size_cells;
> > +
> > +        cell = (const __be32 *)prop->value;
> > +        banks = static_mem_len / (reg_cells * sizeof (u32));
> > +        BUG_ON(banks > NR_MEM_BANKS);
> > +
> > +        for ( i = 0; i < banks; i++ )
> > +        {
> > +            device_tree_get_reg(&cell, addr_cells, size_cells, &start, 
> > &size);
> > +            d->arch.static_mem.bank[i].start = start;
> > +            d->arch.static_mem.bank[i].size = size;
> > +            static_mem_size += size;
> > +
> > +            printk(XENLOG_INFO
> > +                    "Static Memory Bank[%d] for Domain %pd:"
> > +                    "0x%"PRIx64"-0x%"PRIx64"\n",
> > +                    i, d,
> > +                    d->arch.static_mem.bank[i].start,
> > +                    d->arch.static_mem.bank[i].start +
> > +                    d->arch.static_mem.bank[i].size);
> > +        }
> > +        d->arch.static_mem.nr_banks = banks;
> > +    }
> 
> Could we allocate the memory as we parse?
> 

Ok. I'll try.

> >
> >       rc = dt_property_read_u64(node, "memory", &mem);
> > -    if ( !rc )
> > +    if ( !static_mem && !rc )
> >       {
> >           printk("Error building DomU: cannot read \"memory\" property\n");
> >           return -EINVAL;
> >       }
> > -    kinfo.unassigned_mem = (paddr_t)mem * SZ_1K;
> > +    kinfo.unassigned_mem = static_mem ? static_mem_size :
> > + (paddr_t)mem * SZ_1K;
> >
> > -    printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d-
> >max_vcpus, mem);
> > +    printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n",
> > +            d->max_vcpus, (kinfo.unassigned_mem) >> 10);
> >
> >       kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
> >
> > @@ -2452,7 +2496,11 @@ static int __init construct_domU(struct domain
> *d,
> >       /* type must be set before allocate memory */
> >       d->arch.type = kinfo.type;
> >   #endif
> > -    allocate_memory(d, &kinfo);
> > +    if ( static_mem )
> > +        /* allocate_static_memory(d, &kinfo); */
> > +        BUG();
> > +    else
> > +        allocate_memory(d, &kinfo);
> >
> >       rc = prepare_dtb_domU(d, &kinfo);
> >       if ( rc < 0 )
> > diff --git a/xen/include/asm-arm/domain.h
> > b/xen/include/asm-arm/domain.h index c9277b5c6d..81b8eb453c 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -10,6 +10,7 @@
> >   #include <asm/gic.h>
> >   #include <asm/vgic.h>
> >   #include <asm/vpl011.h>
> > +#include <asm/setup.h>
> >   #include <public/hvm/params.h>
> >
> >   struct hvm_domain
> > @@ -89,6 +90,8 @@ struct arch_domain
> >   #ifdef CONFIG_TEE
> >       void *tee;
> >   #endif
> > +
> > +    struct meminfo static_mem;
> >   }  __cacheline_aligned;
> >
> >   struct arch_vcpu
> >
> 
> Cheers,
> 
> --
> Julien Grall

Cheers

Penny