diff mbox series

[v1,03/13] xen/arm: allocate static shared memory to dom_shared

Message ID 20220311061123.1883189-4-Penny.Zheng@arm.com (mailing list archive)
State New, archived
Headers show
Series Static shared memory on dom0less system | expand

Commit Message

Penny Zheng March 11, 2022, 6:11 a.m. UTC
From: Penny Zheng <penny.zheng@arm.com>

This commit introduces process_shm to cope with static shared memory in
domain construction.

This commit only considers allocating static shared memory to dom_shared
when owner domain is not explicitly defined in device tree, the other
scenario will be covered in the following patches.

Static shared memory could reuse acquire_static_memory_bank() to acquire
and allocate static memory.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
 xen/arch/arm/domain_build.c | 116 +++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini March 18, 2022, 1:59 a.m. UTC | #1
On Fri, 11 Mar 2022, Penny Zheng wrote:
> From: Penny Zheng <penny.zheng@arm.com>
> 
> This commit introduces process_shm to cope with static shared memory in
> domain construction.
> 
> This commit only considers allocating static shared memory to dom_shared
> when owner domain is not explicitly defined in device tree, the other
> scenario will be covered in the following patches.
> 
> Static shared memory could reuse acquire_static_memory_bank() to acquire
> and allocate static memory.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
>  xen/arch/arm/domain_build.c | 116 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 8be01678de..6e6349caac 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -527,7 +527,8 @@ static mfn_t __init acquire_static_memory_bank(struct domain *d,
>      mfn_t smfn;
>      int res;
>  
> -    device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
> +    if ( cell )
> +        device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);

Why this change?


>      ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize, PAGE_SIZE));
>      if ( PFN_DOWN(*psize) > UINT_MAX )
>      {
> @@ -751,6 +752,113 @@ static void __init assign_static_memory_11(struct domain *d,
>      panic("Failed to assign requested static memory for direct-map domain %pd.",
>            d);
>  }
> +
> +#ifdef CONFIG_STATIC_SHM
> +static __initdata DECLARE_BITMAP(shm_mask, NR_MEM_BANKS);
> +
> +static mfn_t __init acquire_shared_memory_bank(struct domain *d,
> +                                               u32 addr_cells, u32 size_cells,
> +                                               paddr_t *pbase, paddr_t *psize)
> +{
> +    /*
> +     * Pages of statically shared memory shall be included
> +     * in domain_tot_pages().
> +     */
> +    d->max_pages += PFN_DOWN(*psize);
> +
> +    return acquire_static_memory_bank(d, NULL, addr_cells, size_cells,
> +                                      pbase, psize);
> +
> +}
> +
> +static int __init allocate_shared_memory(struct domain *d,
> +                                         u32 addr_cells, u32 size_cells,
> +                                         paddr_t pbase, paddr_t psize,
> +                                         paddr_t gbase)
> +{
> +    mfn_t smfn;
> +    int ret = 0;
> +
> +    printk(XENLOG_INFO "Allocate static shared memory BANK %#"PRIpaddr"-%#"PRIpaddr"\n",
> +           pbase, pbase + psize);
> +
> +    smfn = acquire_shared_memory_bank(d, addr_cells, size_cells, &pbase,
> +                                      &psize);
> +    if ( mfn_eq(smfn, INVALID_MFN) )
> +        return -EINVAL;
> +
> +    ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, PFN_DOWN(psize));
> +    if ( ret )
> +    {
> +        dprintk(XENLOG_ERR, "Failed to map shared memory to %pd.\n", d);
> +        return ret;
> +    }
> +
> +    return ret;
> +}
> +
> +static int __init process_shm(struct domain *d,
> +                              const struct dt_device_node *node)
> +{
> +    struct dt_device_node *shm_node;
> +    int ret = 0;
> +    const struct dt_property *prop;
> +    const __be32 *cells;
> +    u32 shm_id;
> +    u32 addr_cells, size_cells;
> +    paddr_t gbase, pbase, psize;
> +
> +    dt_for_each_child_node(node, shm_node)
> +    {
> +        if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-v1") )
> +            continue;
> +
> +        if ( !dt_property_read_u32(shm_node, "xen,shm-id", &shm_id) )
> +        {
> +            printk("Shared memory node does not provide \"xen,shm-id\" property.\n");
> +            return -ENOENT;
> +        }
> +
> +        addr_cells = dt_n_addr_cells(shm_node);
> +        size_cells = dt_n_size_cells(shm_node);
> +        prop = dt_find_property(shm_node, "xen,shared-mem", NULL);
> +        if ( !prop )
> +        {
> +            printk("Shared memory node does not provide \"xen,shared-mem\" property.\n");
> +            return -ENOENT;
> +        }
> +        cells = (const __be32 *)prop->value;
> +        /* xen,shared-mem = <pbase, psize, gbase>; */
> +        device_tree_get_reg(&cells, addr_cells, size_cells, &pbase, &psize);
> +        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_shared. */
> +        /*
> +         * Per shared memory region could be shared between multiple domains.
> +         * In case re-allocating the same shared memory region, we use bitmask
> +         * shm_mask to record whether this shared memory region has ever been
> +         * allocated already.
> +         */
> +        if ( !test_bit(shm_id, shm_mask) )
> +        {
> +            /*
> +             * Allocate statically shared pages to the default dom_shared.
> +             * Set up P2M, and dom_shared is a direct-map domain,
> +             * so GFN == PFN.
> +             */
> +            ret = allocate_shared_memory(dom_shared, addr_cells, size_cells,
> +                                         pbase, psize, pbase);
                                                          ^gbase

The last parameter should be gbase instead of pbase?


Reading this patch is not clear that only the "owner" code path is
implemented here. The "borrower" code path is implemented later and
missing in this patch. I think it would be good to clarify that in the
commit message.

Under this light, allocate_shared_memory is supposed to be only called
for the owner. I think we should probably mention that in the in-code
comment too.

I don't think we need to define a second copy of shm_mask. Can we reuse
the one in bootfdt.c?

Or we could get rid of shm_mask entirely here if we check whether the
pages were already allocated in the owner p2m.


> +            if ( ret )
> +                return ret;
> +
> +            set_bit(shm_id, shm_mask);
> +        }
> +    }
> +
> +    return 0;
> +}
> +#endif /* CONFIG_STATIC_SHM */
>  #else
>  static void __init allocate_static_memory(struct domain *d,
>                                            struct kernel_info *kinfo,
> @@ -3150,6 +3258,12 @@ static int __init construct_domU(struct domain *d,
>      else
>          assign_static_memory_11(d, &kinfo, node);
>  
> +#ifdef CONFIG_STATIC_SHM
> +    rc = process_shm(d, node);
> +    if ( rc < 0 )
> +        return rc;
> +#endif
> +
>      /*
>       * Base address and irq number are needed when creating vpl011 device
>       * tree node in prepare_dtb_domU, so initialization on related variables
> -- 
> 2.25.1
>
Penny Zheng March 18, 2022, 8:35 a.m. UTC | #2
Hi Stefano

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: Friday, March 18, 2022 10:00 AM
> To: Penny Zheng <Penny.Zheng@arm.com>
> Cc: xen-devel@lists.xenproject.org; nd <nd@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>
> Subject: Re: [PATCH v1 03/13] xen/arm: allocate static shared memory to
> dom_shared
> 
> On Fri, 11 Mar 2022, Penny Zheng wrote:
> > From: Penny Zheng <penny.zheng@arm.com>
> >
> > This commit introduces process_shm to cope with static shared memory
> > in domain construction.
> >
> > This commit only considers allocating static shared memory to
> > dom_shared when owner domain is not explicitly defined in device tree,
> > the other scenario will be covered in the following patches.
> >
> > Static shared memory could reuse acquire_static_memory_bank() to
> > acquire and allocate static memory.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > ---
> >  xen/arch/arm/domain_build.c | 116
> > +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 115 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 8be01678de..6e6349caac 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -527,7 +527,8 @@ static mfn_t __init
> acquire_static_memory_bank(struct domain *d,
> >      mfn_t smfn;
> >      int res;
> >
> > -    device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
> > +    if ( cell )
> > +        device_tree_get_reg(cell, addr_cells, size_cells, pbase,
> > + psize);
> 
> Why this change?
> 

This helper is also used for acquiring static memory as guest RAM for statically configured
domain.

And since we are reusing it for static shared memory, but try to avoid parsing the property
here, the "xen,static-shm" property getting parsed in different ways in process_shm.
So this change is needed here.

And I think I need to add in-code comment to explain. ;)

> 
> >      ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize,
> PAGE_SIZE));
> >      if ( PFN_DOWN(*psize) > UINT_MAX )
> >      {
> > @@ -751,6 +752,113 @@ static void __init assign_static_memory_11(struct
> domain *d,
> >      panic("Failed to assign requested static memory for direct-map
> domain %pd.",
> >            d);
> >  }
> > +
> > +#ifdef CONFIG_STATIC_SHM
> > +static __initdata DECLARE_BITMAP(shm_mask, NR_MEM_BANKS);
> > +
> > +static mfn_t __init acquire_shared_memory_bank(struct domain *d,
> > +                                               u32 addr_cells, u32 size_cells,
> > +                                               paddr_t *pbase,
> > +paddr_t *psize) {
> > +    /*
> > +     * Pages of statically shared memory shall be included
> > +     * in domain_tot_pages().
> > +     */
> > +    d->max_pages += PFN_DOWN(*psize);
> > +
> > +    return acquire_static_memory_bank(d, NULL, addr_cells, size_cells,
> > +                                      pbase, psize);
> > +
> > +}
> > +
> > +static int __init allocate_shared_memory(struct domain *d,
> > +                                         u32 addr_cells, u32 size_cells,
> > +                                         paddr_t pbase, paddr_t psize,
> > +                                         paddr_t gbase) {
> > +    mfn_t smfn;
> > +    int ret = 0;
> > +
> > +    printk(XENLOG_INFO "Allocate static shared memory
> BANK %#"PRIpaddr"-%#"PRIpaddr"\n",
> > +           pbase, pbase + psize);
> > +
> > +    smfn = acquire_shared_memory_bank(d, addr_cells, size_cells, &pbase,
> > +                                      &psize);
> > +    if ( mfn_eq(smfn, INVALID_MFN) )
> > +        return -EINVAL;
> > +
> > +    ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn,
> PFN_DOWN(psize));
> > +    if ( ret )
> > +    {
> > +        dprintk(XENLOG_ERR, "Failed to map shared memory to %pd.\n", d);
> > +        return ret;
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +static int __init process_shm(struct domain *d,
> > +                              const struct dt_device_node *node) {
> > +    struct dt_device_node *shm_node;
> > +    int ret = 0;
> > +    const struct dt_property *prop;
> > +    const __be32 *cells;
> > +    u32 shm_id;
> > +    u32 addr_cells, size_cells;
> > +    paddr_t gbase, pbase, psize;
> > +
> > +    dt_for_each_child_node(node, shm_node)
> > +    {
> > +        if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-
> v1") )
> > +            continue;
> > +
> > +        if ( !dt_property_read_u32(shm_node, "xen,shm-id", &shm_id) )
> > +        {
> > +            printk("Shared memory node does not provide \"xen,shm-id\"
> property.\n");
> > +            return -ENOENT;
> > +        }
> > +
> > +        addr_cells = dt_n_addr_cells(shm_node);
> > +        size_cells = dt_n_size_cells(shm_node);
> > +        prop = dt_find_property(shm_node, "xen,shared-mem", NULL);
> > +        if ( !prop )
> > +        {
> > +            printk("Shared memory node does not provide \"xen,shared-mem\"
> property.\n");
> > +            return -ENOENT;
> > +        }
> > +        cells = (const __be32 *)prop->value;
> > +        /* xen,shared-mem = <pbase, psize, gbase>; */
> > +        device_tree_get_reg(&cells, addr_cells, size_cells, &pbase, &psize);
> > +        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_shared. */
> > +        /*
> > +         * Per shared memory region could be shared between multiple
> domains.
> > +         * In case re-allocating the same shared memory region, we use bitmask
> > +         * shm_mask to record whether this shared memory region has ever
> been
> > +         * allocated already.
> > +         */
> > +        if ( !test_bit(shm_id, shm_mask) )
> > +        {
> > +            /*
> > +             * Allocate statically shared pages to the default dom_shared.
> > +             * Set up P2M, and dom_shared is a direct-map domain,
> > +             * so GFN == PFN.
> > +             */
> > +            ret = allocate_shared_memory(dom_shared, addr_cells, size_cells,
> > +                                         pbase, psize, pbase);
>                                                           ^gbase
> 
> The last parameter should be gbase instead of pbase?
> 

Yes, and since dom_shared is a direct-map domain, GFN == PFN, so pbase should be
ok here. I mentioned it on comments.

And Why I make dom_shared direct-map domain is that in this way we don't need to decode
owner GFN when doing foreign memory mapping for borrower domain.

> 
> Reading this patch is not clear that only the "owner" code path is
> implemented here. The "borrower" code path is implemented later and
> missing in this patch. I think it would be good to clarify that in the commit
> message.
> 
> Under this light, allocate_shared_memory is supposed to be only called for the
> owner. I think we should probably mention that in the in-code comment too.
> 

Yes, only owner domain could allocate memory, I'll add it to in-code comment.

> I don't think we need to define a second copy of shm_mask. Can we reuse the
> one in bootfdt.c?
> 

Yes, maybe I should reuse than reintroduce. And before using the bitmap here,
I need to clear it totally to clean away all the stale info from bootfdt.c.

> Or we could get rid of shm_mask entirely here if we check whether the pages
> were already allocated in the owner p2m.
> 
> 

Hmm, that means that we need to do the page walk each time? That's kinds of
time-consuming, or am I missing some convenient way to check?

> > +            if ( ret )
> > +                return ret;
> > +
> > +            set_bit(shm_id, shm_mask);
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +#endif /* CONFIG_STATIC_SHM */
> >  #else
> >  static void __init allocate_static_memory(struct domain *d,
> >                                            struct kernel_info *kinfo,
> > @@ -3150,6 +3258,12 @@ static int __init construct_domU(struct domain *d,
> >      else
> >          assign_static_memory_11(d, &kinfo, node);
> >
> > +#ifdef CONFIG_STATIC_SHM
> > +    rc = process_shm(d, node);
> > +    if ( rc < 0 )
> > +        return rc;
> > +#endif
> > +
> >      /*
> >       * Base address and irq number are needed when creating vpl011 device
> >       * tree node in prepare_dtb_domU, so initialization on related
> > variables
> > --
> > 2.25.1

Cheers,

---
Penny Zheng
> >
Stefano Stabellini March 18, 2022, 10:27 p.m. UTC | #3
On Fri, 18 Mar 2022, Penny Zheng wrote:
> > On Fri, 11 Mar 2022, Penny Zheng wrote:
> > > From: Penny Zheng <penny.zheng@arm.com>
> > >
> > > This commit introduces process_shm to cope with static shared memory
> > > in domain construction.
> > >
> > > This commit only considers allocating static shared memory to
> > > dom_shared when owner domain is not explicitly defined in device tree,
> > > the other scenario will be covered in the following patches.
> > >
> > > Static shared memory could reuse acquire_static_memory_bank() to
> > > acquire and allocate static memory.
> > >
> > > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > > ---
> > >  xen/arch/arm/domain_build.c | 116
> > > +++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 115 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index 8be01678de..6e6349caac 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -527,7 +527,8 @@ static mfn_t __init
> > acquire_static_memory_bank(struct domain *d,
> > >      mfn_t smfn;
> > >      int res;
> > >
> > > -    device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
> > > +    if ( cell )
> > > +        device_tree_get_reg(cell, addr_cells, size_cells, pbase,
> > > + psize);
> > 
> > Why this change?
> > 
> 
> This helper is also used for acquiring static memory as guest RAM for statically configured
> domain.
> 
> And since we are reusing it for static shared memory, but try to avoid parsing the property
> here, the "xen,static-shm" property getting parsed in different ways in process_shm.
> So this change is needed here.
> 
> And I think I need to add in-code comment to explain. ;)
> 
> > 
> > >      ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize,
> > PAGE_SIZE));
> > >      if ( PFN_DOWN(*psize) > UINT_MAX )
> > >      {
> > > @@ -751,6 +752,113 @@ static void __init assign_static_memory_11(struct
> > domain *d,
> > >      panic("Failed to assign requested static memory for direct-map
> > domain %pd.",
> > >            d);
> > >  }
> > > +
> > > +#ifdef CONFIG_STATIC_SHM
> > > +static __initdata DECLARE_BITMAP(shm_mask, NR_MEM_BANKS);
> > > +
> > > +static mfn_t __init acquire_shared_memory_bank(struct domain *d,
> > > +                                               u32 addr_cells, u32 size_cells,
> > > +                                               paddr_t *pbase,
> > > +paddr_t *psize) {
> > > +    /*
> > > +     * Pages of statically shared memory shall be included
> > > +     * in domain_tot_pages().
> > > +     */
> > > +    d->max_pages += PFN_DOWN(*psize);
> > > +
> > > +    return acquire_static_memory_bank(d, NULL, addr_cells, size_cells,
> > > +                                      pbase, psize);
> > > +
> > > +}
> > > +
> > > +static int __init allocate_shared_memory(struct domain *d,
> > > +                                         u32 addr_cells, u32 size_cells,
> > > +                                         paddr_t pbase, paddr_t psize,
> > > +                                         paddr_t gbase) {
> > > +    mfn_t smfn;
> > > +    int ret = 0;
> > > +
> > > +    printk(XENLOG_INFO "Allocate static shared memory
> > BANK %#"PRIpaddr"-%#"PRIpaddr"\n",
> > > +           pbase, pbase + psize);
> > > +
> > > +    smfn = acquire_shared_memory_bank(d, addr_cells, size_cells, &pbase,
> > > +                                      &psize);
> > > +    if ( mfn_eq(smfn, INVALID_MFN) )
> > > +        return -EINVAL;
> > > +
> > > +    ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn,
> > PFN_DOWN(psize));
> > > +    if ( ret )
> > > +    {
> > > +        dprintk(XENLOG_ERR, "Failed to map shared memory to %pd.\n", d);
> > > +        return ret;
> > > +    }
> > > +
> > > +    return ret;
> > > +}
> > > +
> > > +static int __init process_shm(struct domain *d,
> > > +                              const struct dt_device_node *node) {
> > > +    struct dt_device_node *shm_node;
> > > +    int ret = 0;
> > > +    const struct dt_property *prop;
> > > +    const __be32 *cells;
> > > +    u32 shm_id;
> > > +    u32 addr_cells, size_cells;
> > > +    paddr_t gbase, pbase, psize;
> > > +
> > > +    dt_for_each_child_node(node, shm_node)
> > > +    {
> > > +        if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-
> > v1") )
> > > +            continue;
> > > +
> > > +        if ( !dt_property_read_u32(shm_node, "xen,shm-id", &shm_id) )
> > > +        {
> > > +            printk("Shared memory node does not provide \"xen,shm-id\"
> > property.\n");
> > > +            return -ENOENT;
> > > +        }
> > > +
> > > +        addr_cells = dt_n_addr_cells(shm_node);
> > > +        size_cells = dt_n_size_cells(shm_node);
> > > +        prop = dt_find_property(shm_node, "xen,shared-mem", NULL);
> > > +        if ( !prop )
> > > +        {
> > > +            printk("Shared memory node does not provide \"xen,shared-mem\"
> > property.\n");
> > > +            return -ENOENT;
> > > +        }
> > > +        cells = (const __be32 *)prop->value;
> > > +        /* xen,shared-mem = <pbase, psize, gbase>; */
> > > +        device_tree_get_reg(&cells, addr_cells, size_cells, &pbase, &psize);
> > > +        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_shared. */
> > > +        /*
> > > +         * Per shared memory region could be shared between multiple
> > domains.
> > > +         * In case re-allocating the same shared memory region, we use bitmask
> > > +         * shm_mask to record whether this shared memory region has ever
> > been
> > > +         * allocated already.
> > > +         */
> > > +        if ( !test_bit(shm_id, shm_mask) )
> > > +        {
> > > +            /*
> > > +             * Allocate statically shared pages to the default dom_shared.
> > > +             * Set up P2M, and dom_shared is a direct-map domain,
> > > +             * so GFN == PFN.
> > > +             */
> > > +            ret = allocate_shared_memory(dom_shared, addr_cells, size_cells,
> > > +                                         pbase, psize, pbase);
> >                                                           ^gbase
> > 
> > The last parameter should be gbase instead of pbase?
> > 
> 
> Yes, and since dom_shared is a direct-map domain, GFN == PFN, so pbase should be
> ok here. I mentioned it on comments.
> 
> And Why I make dom_shared direct-map domain is that in this way we don't need to decode
> owner GFN when doing foreign memory mapping for borrower domain.
> 
> > 
> > Reading this patch is not clear that only the "owner" code path is
> > implemented here. The "borrower" code path is implemented later and
> > missing in this patch. I think it would be good to clarify that in the commit
> > message.
> > 
> > Under this light, allocate_shared_memory is supposed to be only called for the
> > owner. I think we should probably mention that in the in-code comment too.
> > 
> 
> Yes, only owner domain could allocate memory, I'll add it to in-code comment.
> 
> > I don't think we need to define a second copy of shm_mask. Can we reuse the
> > one in bootfdt.c?
> > 
> 
> Yes, maybe I should reuse than reintroduce. And before using the bitmap here,
> I need to clear it totally to clean away all the stale info from bootfdt.c.
> 
> > Or we could get rid of shm_mask entirely here if we check whether the pages
> > were already allocated in the owner p2m.
> > 
> > 
> 
> Hmm, that means that we need to do the page walk each time? That's kinds of
> time-consuming, or am I missing some convenient way to check?

No page walk. I think it should be possible with:

- mfn_to_page
- page_get_owner

both of them are direct access. Assuming that the page owner is set
correctly.
diff mbox series

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 8be01678de..6e6349caac 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -527,7 +527,8 @@  static mfn_t __init acquire_static_memory_bank(struct domain *d,
     mfn_t smfn;
     int res;
 
-    device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
+    if ( cell )
+        device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
     ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize, PAGE_SIZE));
     if ( PFN_DOWN(*psize) > UINT_MAX )
     {
@@ -751,6 +752,113 @@  static void __init assign_static_memory_11(struct domain *d,
     panic("Failed to assign requested static memory for direct-map domain %pd.",
           d);
 }
+
+#ifdef CONFIG_STATIC_SHM
+static __initdata DECLARE_BITMAP(shm_mask, NR_MEM_BANKS);
+
+static mfn_t __init acquire_shared_memory_bank(struct domain *d,
+                                               u32 addr_cells, u32 size_cells,
+                                               paddr_t *pbase, paddr_t *psize)
+{
+    /*
+     * Pages of statically shared memory shall be included
+     * in domain_tot_pages().
+     */
+    d->max_pages += PFN_DOWN(*psize);
+
+    return acquire_static_memory_bank(d, NULL, addr_cells, size_cells,
+                                      pbase, psize);
+
+}
+
+static int __init allocate_shared_memory(struct domain *d,
+                                         u32 addr_cells, u32 size_cells,
+                                         paddr_t pbase, paddr_t psize,
+                                         paddr_t gbase)
+{
+    mfn_t smfn;
+    int ret = 0;
+
+    printk(XENLOG_INFO "Allocate static shared memory BANK %#"PRIpaddr"-%#"PRIpaddr"\n",
+           pbase, pbase + psize);
+
+    smfn = acquire_shared_memory_bank(d, addr_cells, size_cells, &pbase,
+                                      &psize);
+    if ( mfn_eq(smfn, INVALID_MFN) )
+        return -EINVAL;
+
+    ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, PFN_DOWN(psize));
+    if ( ret )
+    {
+        dprintk(XENLOG_ERR, "Failed to map shared memory to %pd.\n", d);
+        return ret;
+    }
+
+    return ret;
+}
+
+static int __init process_shm(struct domain *d,
+                              const struct dt_device_node *node)
+{
+    struct dt_device_node *shm_node;
+    int ret = 0;
+    const struct dt_property *prop;
+    const __be32 *cells;
+    u32 shm_id;
+    u32 addr_cells, size_cells;
+    paddr_t gbase, pbase, psize;
+
+    dt_for_each_child_node(node, shm_node)
+    {
+        if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-v1") )
+            continue;
+
+        if ( !dt_property_read_u32(shm_node, "xen,shm-id", &shm_id) )
+        {
+            printk("Shared memory node does not provide \"xen,shm-id\" property.\n");
+            return -ENOENT;
+        }
+
+        addr_cells = dt_n_addr_cells(shm_node);
+        size_cells = dt_n_size_cells(shm_node);
+        prop = dt_find_property(shm_node, "xen,shared-mem", NULL);
+        if ( !prop )
+        {
+            printk("Shared memory node does not provide \"xen,shared-mem\" property.\n");
+            return -ENOENT;
+        }
+        cells = (const __be32 *)prop->value;
+        /* xen,shared-mem = <pbase, psize, gbase>; */
+        device_tree_get_reg(&cells, addr_cells, size_cells, &pbase, &psize);
+        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_shared. */
+        /*
+         * Per shared memory region could be shared between multiple domains.
+         * In case re-allocating the same shared memory region, we use bitmask
+         * shm_mask to record whether this shared memory region has ever been
+         * allocated already.
+         */
+        if ( !test_bit(shm_id, shm_mask) )
+        {
+            /*
+             * Allocate statically shared pages to the default dom_shared.
+             * Set up P2M, and dom_shared is a direct-map domain,
+             * so GFN == PFN.
+             */
+            ret = allocate_shared_memory(dom_shared, addr_cells, size_cells,
+                                         pbase, psize, pbase);
+            if ( ret )
+                return ret;
+
+            set_bit(shm_id, shm_mask);
+        }
+    }
+
+    return 0;
+}
+#endif /* CONFIG_STATIC_SHM */
 #else
 static void __init allocate_static_memory(struct domain *d,
                                           struct kernel_info *kinfo,
@@ -3150,6 +3258,12 @@  static int __init construct_domU(struct domain *d,
     else
         assign_static_memory_11(d, &kinfo, node);
 
+#ifdef CONFIG_STATIC_SHM
+    rc = process_shm(d, node);
+    if ( rc < 0 )
+        return rc;
+#endif
+
     /*
      * Base address and irq number are needed when creating vpl011 device
      * tree node in prepare_dtb_domU, so initialization on related variables