Re: [Xen-devel] [PATCH V2] xen/arm: domain_build: allocate lowmem for dom0 as much as possible

[Julien Grall <julien.grall@xxxxxxx>]

Hello Peng,

On 20/09/2016 07:52, van.freenix@xxxxxxxxx wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
>
> On AArch64 SoCs, some IPs may only have the capability to access
> 32bits address space. The physical memory assigned for Dom0 maybe
s/32bits/32 bits/

> not in 4GB address space, then the IPs will not work properly.
> So need to allocate memory under 4GB for Dom0.
>
> There is no restriction that how much lowmem needs to be allocated for
> Dom0. Dom0 now use 1:1 mapping, but DomU does not use 1:1 mapping,
> there is no need to reserve lowmem for DomU, so allocate lowmem as much
> as possible for Dom0.
>
> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>
> This patch is to resolve the issue mentioned in
> https://lists.xen.org/archives/html/xen-devel/2016-09/msg00235.html
>
> V2:
>  Remove the bootargs dom0_lowmem introduced in V1.
>  Following 
> "https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01459.html";
>  to allocate as much as possible lowmem.
>
>  Tested results:
>  (XEN) Allocating 1:1 mappings totalling 2048MB for dom0:
>  (XEN) BANK[0] 0x00000088000000-0x000000f8000000 (1792MB)
>  (XEN) BANK[1] 0x000009e0000000-0x000009f0000000 (256MB)
>  1792M allocated in 4GB address space.
>
>  xen/arch/arm/domain_build.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 35ab08d..0b9b85c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -240,11 +240,11 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
As mentioned on V1, please update the comment above this function.

>      const unsigned int min_low_order =
>          get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
>      const unsigned int min_order = get_order_from_bytes(MB(4));
> -    struct page_info *pg;
> +    struct page_info *pg = NULL;
>      unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
>      int i;
>
> -    bool_t lowmem = is_32bit_domain(d);
> +    bool_t lowmem = true;
>      unsigned int bits;
>
>      /*
> @@ -265,22 +265,28 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
>       */
>      while ( order >= min_low_order )
>      {
> -        for ( bits = order ; bits <= (lowmem ? 32 : PADDR_BITS); bits++ )
> +        for ( bits = order ; bits <= 32 ; bits++ )
>          {
>              pg = alloc_domheap_pages(d, order, MEMF_bits(bits));
>              if ( pg != NULL )
> +            {
> +                if ( !insert_11_bank(d, kinfo, pg, order) )
> +                    BUG(); /* Cannot fail for first bank */
> +
>                  goto got_bank0;
> +           }
>          }
>          order--;
>      }
>
> -    panic("Unable to allocate first memory bank");
> +    if ( pg == NULL )
> +    {
> +        printk(XENLOG_INFO "No bank has been allocated below 4GB.\n");
> +        lowmem = false;
> +    }
This new behavior should be documented in the commit message, with some 
rationale why it is fine to do it for ARM32.
>   got_bank0:
>
> -    if ( !insert_11_bank(d, kinfo, pg, order) )
> -        BUG(); /* Cannot fail for first bank */
> -
>      /* Now allocate more memory and fill in additional banks */
This comment should be updated.

>      order = get_11_allocation_size(kinfo->unassigned_mem);
Regards,

--
Julien Grall

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