Re: [Xen-devel] [PATCH V1] xen/arm: domain_build: introduce dom0_lowmem bootargs

[Julien Grall <julien.grall@xxxxxxx>]

Hello,

On 14/09/2016 06:12, Peng Fan wrote:
> On AArch64 SoCs, some IPs may only have the capability to access
> 32bits address space. The physical memory assigned for Dom0 maybe
> not in 4GB address space, then the IPs will not work properly.
>
> Introduce dom0_lowmem bootargs, user could pass "dom0_lowmem=xx"
> to xen. It means how much memory user would like to be allocated
> in lower 4GB memory space. If there is not enough memory for
> dom0_lowmem, higher memory will be allocated.
>
> Thinking such a memory layout on an AArch64 SoC:
> Region 0: 2GB(0x80000000 - 0xffffffff)
> Region 1: 4GB(0x880000000 - 0x97fffffff)
> If user would like to assign 2GB for Dom0 and 1GB of the 2GB memory
> in Region 0, user could pass "dom0=2048M dom0_lowmem=1024M" to xen.
See my comments on v1, I still don't think this new parameter is useful.

The commit message does not explain what is the advantage to only 
allocate a bit of low mem and not as much as we can (as we do on for 
32-bit dom0).
> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>
> RFC->V1:
>  This patch is to resolve the issue in 
> https://lists.xen.org/archives/html/xen-devel/2016-09/msg00235.html
>  No code change since RFC.
>  Tested on xen-4.8 unstable with AArch64. See partial log:
>  "dom0_mem = 2048M  dom0_lowmem=128M"
>  (XEN) Allocated 0x00000088000000-0x00000090000000 (128MB/2048MB, order 15)
>  (XEN) Allocated 0x00000880000000-0x000008c0000000 (1024MB/1920MB, order 18)
>  (XEN) Allocated 0x000009c0000000-0x000009e0000000 (512MB/896MB, order 17)
>  (XEN) Allocated 0x000009e0000000-0x000009f0000000 (256MB/384MB, order 16)
>  (XEN) Allocated 0x000008f8000000-0x00000900000000 (128MB/128MB, order 15)
>
>  "dom0_mem = 2048M  dom0_lowmem=1024M"
>  (XEN) Allocated 0x000000a0000000-0x000000c0000000 (512MB/2048MB, order 17)
>  (XEN) Allocated 0x000000c0000000-0x000000e0000000 (512MB/1536MB, order 17)
>  (XEN) Allocated 0x00000880000000-0x000008c0000000 (1024MB/1024MB, order 18)
>
>  "dom0_mem = 1024M  dom0_lowmem=1024M"
>  (XEN) Allocated 0x000000a0000000-0x000000c0000000 (512MB/1024MB, order 17)
>  (XEN) Allocated 0x000000c0000000-0x000000e0000000 (512MB/512MB, order 17)
>
>  xen/arch/arm/domain_build.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 35ab08d..0f53bba 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -33,6 +33,8 @@ int dom0_11_mapping = 1;
>
>  #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
>  static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT;
> +/* Only for AArch64 */
> +static u64 __initdata dom0_lowmem;
>
>  static void __init parse_dom0_mem(const char *s)
>  {
> @@ -42,6 +44,12 @@ static void __init parse_dom0_mem(const char *s)
>  }
>  custom_param("dom0_mem", parse_dom0_mem);
>
> +static void __init parse_dom0_lowmem(const char *s)
> +{
> +    dom0_lowmem = parse_size_and_unit(s, &s);
> +}
> +custom_param("dom0_lowmem", parse_dom0_lowmem);
> +
>  //#define DEBUG_11_ALLOCATION
>  #ifdef DEBUG_11_ALLOCATION
>  # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> @@ -244,7 +252,7 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
The comment on top of allocate_memory should be updated.

>      unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
>      int i;
>
> -    bool_t lowmem = is_32bit_domain(d);
> +    bool_t lowmem = is_32bit_domain(d) || !!dom0_lowmem;
>      unsigned int bits;
>
>      /*
> @@ -263,6 +271,9 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
>       * First try and allocate the largest thing we can as low as
>       * possible to be bank 0.
>       */
> +    if ( dom0_lowmem )
> +        order = get_order_from_bytes(dom0_lowmem);
> +
>      while ( order >= min_low_order )
>      {
>          for ( bits = order ; bits <= (lowmem ? 32 : PADDR_BITS); bits++ )
> @@ -278,6 +289,11 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
>
>   got_bank0:
>
> +    if ( dom0_lowmem ) {
> +        dom0_lowmem -= pfn_to_paddr((1 << order));
> +        lowmem = is_32bit_domain(d) || !!dom0_lowmem;
Why do you spread the lowmem = is_32_bit_domain(d) || !!dom0_lowmem 
everywhere?
> +    }
> +
>      if ( !insert_11_bank(d, kinfo, pg, order) )
>          BUG(); /* Cannot fail for first bank */
>
> @@ -325,6 +341,16 @@ static void allocate_memory(struct domain *d, struct 
> kernel_info *kinfo)
>              }
>          }
>
> +        if ( dom0_lowmem && lowmem )
> +        {
> +            dom0_lowmem -= pfn_to_paddr((1 << order));
> +            lowmem = is_32bit_domain(d) || !!dom0_lowmem;
> +        }
> +        else
> +        {
> +            lowmem = false;
> +        }
> +
>          /*
>           * Success, next time around try again to get the largest order
>           * allocation possible.
> @@ -2098,6 +2124,8 @@ int construct_dom0(struct domain *d)
>
>      d->max_pages = ~0U;
>
> +    BUG_ON(dom0_mem < dom0_lowmem);
> +
BUG_ON should not be used to check user input validity.

>      kinfo.unassigned_mem = dom0_mem;
>
>      rc = kernel_probe(&kinfo);
Regards,

--
Julien Grall

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