Re: [PATCH RFC 13/18] xen: introduce and use 'dom0_rambase_pfn' setting for kernel Dom0

[Julien Grall <julien.grall@arm.com>]

Hello,

On 18/05/16 17:32, Andrii Anisov wrote:
> From: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
>
> This setting is used to adjust starting memory address allocated
> for kernel Dom0. To use 'rambase_pfn' setting just add for example
> 'dom0_rambase_pfn=0x80000' to the hypervisor command line. Note that
> 'dom0_rambase_pfn' should be aligned with the smallest memory chunk
> which use xen memory allocator.

Why would a user want to allocate DOM0 RAM bank to a specific address?

If I understand correctly your patch, DOM0 will only able to allocate 
one bank of the given size at the specific address. You also add this 
possibility for guest domain (see patch #4) and try to control where the 
guest memory will be allocated. This will increase a lot the chance of 
the memory allocation to fail.

For instance, the RAM region requested for DOM0 may have been used to 
allocate memory for Xen internal. So you need a way to reserve memory in 
order to avoid Xen using it.

I expect most of the users who want to use direct memory mapped guest to 
know the number of guests which will use this feature.

A such feature is only useful when pass-through a device to the guest on 
platfom without SMMU, so it is by default insecure.

So I would suggest to create a new device-tree binding (or re-use an 
actual one) to reserve memory region to be used for direct memory mapped 
domain.

Those regions could have an identifier to be used later during the 
allocation. This would avoid memory fragmentation, allow multiple RAM 
bank for DOM0,...

Any opinions?

>
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
> ---
>   xen/arch/arm/domain_build.c | 24 +++++++++++++---
>   xen/common/page_alloc.c     | 68 +++++++++++++++++++++++++++++++++++----------
>   xen/include/xen/mm.h        |  2 ++
>   3 files changed, 75 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 2937ff7..b48718d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -27,6 +27,9 @@
>   static unsigned int __initdata opt_dom0_max_vcpus;
>   integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>
> +static u64 __initdata opt_dom0_rambase_pfn = 0;
> +integer_param("dom0_rambase_pfn", opt_dom0_rambase_pfn);
> +
>   int dom0_11_mapping = 1;
>
>   #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
> @@ -248,6 +251,8 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
>       const unsigned int min_order = get_order_from_bytes(MB(4));
>       struct page_info *pg;
>       unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
> +    u64 rambase_pfn = opt_dom0_rambase_pfn;
> +    paddr_t mem_size = kinfo->unassigned_mem;
>       int i;
>
>       bool_t lowmem = is_32bit_domain(d);
> @@ -267,7 +272,7 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
>       {
>           for ( bits = order ; bits <= (lowmem ? 32 : PADDR_BITS); bits++ )
>           {
> -            pg = alloc_domheap_pages(d, order, MEMF_bits(bits));
> +            pg = alloc_domheap_pages_pfn(d, order, MEMF_bits(bits), rambase_pfn);
>               if ( pg != NULL )
>                   goto got_bank0;
>           }
> @@ -284,16 +289,21 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
>       /* Now allocate more memory and fill in additional banks */
>
>       order = get_11_allocation_size(kinfo->unassigned_mem);
> +    if ( opt_dom0_rambase_pfn )
> +        rambase_pfn += (mem_size - kinfo->unassigned_mem) >> PAGE_SHIFT;
> +
>       while ( kinfo->unassigned_mem && kinfo->mem.nr_banks < NR_MEM_BANKS )
>       {
> -        pg = alloc_domheap_pages(d, order, lowmem ? MEMF_bits(32) : 0);
> +        pg = alloc_domheap_pages_pfn(d, order, lowmem ? MEMF_bits(32) : 0,
> +                                     rambase_pfn);

 From my understanding, when rambase_pfn is not 0, the memory must be 
allocated contiguously at this specific address. So if the first call of 
alloc_domheap_pages (see a bit above) as failed, then this one will 
always fail because it means that someone has allocated some page in 
this region.

>           if ( !pg )
>           {
>               order --;
>
>               if ( lowmem && order < min_low_order)
>               {
> -                D11PRINT("Failed at min_low_order, allow high allocations\n");
> +                if ( !opt_dom0_rambase_pfn )
> +                    D11PRINT("Failed at min_low_order, allow high allocations\n");
>                   order = get_11_allocation_size(kinfo->unassigned_mem);
>                   lowmem = false;
>                   continue;
> @@ -313,7 +323,8 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
>
>               if ( lowmem )
>               {
> -                D11PRINT("Allocation below bank 0, allow high allocations\n");
> +                if ( !opt_dom0_rambase_pfn )
> +                    D11PRINT("Allocation below bank 0, allow high allocations\n");
>                   order = get_11_allocation_size(kinfo->unassigned_mem);
>                   lowmem = false;
>                   continue;
> @@ -330,6 +341,11 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
>            * allocation possible.
>            */
>           order = get_11_allocation_size(kinfo->unassigned_mem);
> +        if ( opt_dom0_rambase_pfn )
> +        {
> +            rambase_pfn += (mem_size - kinfo->unassigned_mem) >> PAGE_SHIFT;
> +            mem_size = kinfo->unassigned_mem;
> +        }
>       }
>
>       if ( kinfo->unassigned_mem )
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 74fc1de..d0c0fbb 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -583,16 +583,17 @@ static void check_low_mem_virq(void)
>       }
>   }
>
> -/* Allocate 2^@order contiguous pages. */
> -static struct page_info *alloc_heap_pages(
> +/* Allocate 2^@order contiguous pages at given pfn. */
> +static struct page_info *alloc_heap_pages_pfn(
>       unsigned int zone_lo, unsigned int zone_hi,
>       unsigned int order, unsigned int memflags,
> -    struct domain *d)
> +    struct domain *d, xen_pfn_t pfn)
>   {
>       unsigned int i, j, zone = 0, nodemask_retry = 0;
>       nodeid_t first_node, node = MEMF_get_node(memflags), req_node = node;
>       unsigned long request = 1UL << order;
> -    struct page_info *pg;
> +    struct page_info *pg, *tmp_pg;
> +    struct page_list_head *pg_list;
>       nodemask_t nodemask = (d != NULL ) ? d->node_affinity : node_online_map;
>       bool_t need_tlbflush = 0;
>       uint32_t tlbflush_timestamp = 0;
> @@ -657,9 +658,25 @@ static struct page_info *alloc_heap_pages(
>                   continue;
>
>               /* Find smallest order which can satisfy the request. */
> -            for ( j = order; j <= MAX_ORDER; j++ )
> -                if ( (pg = page_list_remove_head(&heap(node, zone, j))) )
> -                    goto found;
> +            for ( j = order; j <= MAX_ORDER; j++ ) {
> +                pg_list = &heap(node, zone, j);
> +                if ( pfn )
> +                {
> +                    page_list_for_each_safe( pg, tmp_pg, pg_list )
> +                    {
> +                        if ( pfn == page_to_mfn(pg) )
> +                        {
> +                            page_list_del(pg, pg_list);
> +                            goto found;
> +                        }
> +                    }
> +                }
> +                else
> +                {
> +                    if ( (pg = page_list_remove_head(pg_list)) )
> +                        goto found;
> +                }
> +            }
>           } while ( zone-- > zone_lo ); /* careful: unsigned zone may wrap */
>
>           if ( (memflags & MEMF_exact_node) && req_node != NUMA_NO_NODE )
> @@ -706,9 +723,15 @@ static struct page_info *alloc_heap_pages(
>       /* We may have to halve the chunk a number of times. */
>       while ( j != order )
>       {
> -        PFN_ORDER(pg) = --j;
> -        page_list_add_tail(pg, &heap(node, zone, j));
> -        pg += 1 << j;
> +        tmp_pg = pg;
> +        if ( pfn )
> +            tmp_pg += 1 << (j - 1);
> +
> +        PFN_ORDER(tmp_pg) = --j;
> +        page_list_add_tail(tmp_pg, &heap(node, zone, j));
> +
> +        if ( !pfn )
> +            pg += 1 << j;
>       }
>
>       ASSERT(avail[node][zone] >= request);
> @@ -762,6 +785,15 @@ static struct page_info *alloc_heap_pages(
>       return pg;
>   }
>
> +/* Allocate 2^@order contiguous pages. */
> +static struct page_info *alloc_heap_pages(
> +    unsigned int zone_lo, unsigned int zone_hi,
> +    unsigned int order, unsigned int memflags,
> +    struct domain *d)
> +{
> +    return alloc_heap_pages_pfn(zone_lo, zone_hi, order, memflags, d, 0);
> +}
> +
>   /* Remove any offlined page in the buddy pointed to by head. */
>   static int reserve_offlined_page(struct page_info *head)
>   {
> @@ -1687,8 +1719,8 @@ int assign_pages(
>   }
>
>
> -struct page_info *alloc_domheap_pages(
> -    struct domain *d, unsigned int order, unsigned int memflags)
> +struct page_info *alloc_domheap_pages_pfn(
> +    struct domain *d, unsigned int order, unsigned int memflags, xen_pfn_t pfn)
>   {
>       struct page_info *pg = NULL;
>       unsigned int bits = memflags >> _MEMF_bits, zone_hi = NR_ZONES - 1;
> @@ -1705,12 +1737,12 @@ struct page_info *alloc_domheap_pages(
>           memflags |= MEMF_no_refcount;
>
>       if ( dma_bitsize && ((dma_zone = bits_to_zone(dma_bitsize)) < zone_hi) )
> -        pg = alloc_heap_pages(dma_zone + 1, zone_hi, order, memflags, d);
> +        pg = alloc_heap_pages_pfn(dma_zone + 1, zone_hi, order, memflags, d, pfn);
>
>       if ( (pg == NULL) &&
>            ((memflags & MEMF_no_dma) ||
> -          ((pg = alloc_heap_pages(MEMZONE_XEN + 1, zone_hi, order,
> -                                  memflags, d)) == NULL)) )
> +          ((pg = alloc_heap_pages_pfn(MEMZONE_XEN + 1, zone_hi, order,
> +                                  memflags, d, pfn)) == NULL)) )
>            return NULL;
>
>       if ( d && !(memflags & MEMF_no_owner) &&
> @@ -1723,6 +1755,12 @@ struct page_info *alloc_domheap_pages(
>       return pg;
>   }
>
> +struct page_info *alloc_domheap_pages(
> +    struct domain *d, unsigned int order, unsigned int memflags)
> +{
> +    return alloc_domheap_pages_pfn(d, order, memflags, 0);
> +}
> +
>   void free_domheap_pages(struct page_info *pg, unsigned int order)
>   {
>       struct domain *d = page_get_owner(pg);
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index a795dd6..83e4913 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -117,6 +117,8 @@ void get_outstanding_claims(uint64_t *free_pages, uint64_t *outstanding_pages);
>
>   /* Domain suballocator. These functions are *not* interrupt-safe.*/
>   void init_domheap_pages(paddr_t ps, paddr_t pe);
> +struct page_info *alloc_domheap_pages_pfn(
> +    struct domain *d, unsigned int order, unsigned int memflags, xen_pfn_t pfn);
>   struct page_info *alloc_domheap_pages(
>       struct domain *d, unsigned int order, unsigned int memflags);
>   void free_domheap_pages(struct page_info *pg, unsigned int order);
>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel