diff mbox

[V3] xen/arm: domain_build: allocate lowmem for dom0 as much as possible

Message ID 1474535761-22079-1-git-send-email-peng.fan@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peng Fan Sept. 22, 2016, 9:16 a.m. UTC
On AArch64 SoCs, some IPs may only have the capability to access
32 bits address space. The physical memory assigned for Dom0 maybe
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.

This patch does not affect 32-bit domain, because Variable "lowmem" is
set to true at the beginning. If failed to allocate bank0 under 4GB,
need to panic for 32-bit domain, because 32-bit domain requires bank0
be allocated under 4GB.

For 64-bit domain, set "lowmem" to false, and continue allocating
memory from higher memory space.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---

This patch is to resolve the issue mentioned in
https://lists.xen.org/archives/html/xen-devel/2016-09/msg00235.html

V3:
 Add more commit log
 Add more comments
 Add back panic if failed to allocate bank0 under 4GB for 32-bit domain.

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 | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

Comments

Julien Grall Sept. 22, 2016, 3:23 p.m. UTC | #1
Hello Peng,

On 22/09/16 10:16, Peng Fan wrote:
> On AArch64 SoCs, some IPs may only have the capability to access
> 32 bits address space. The physical memory assigned for Dom0 maybe
> 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.

The last sentence is confusing. I would avoid to mention domU use case here.

>
> This patch does not affect 32-bit domain, because Variable "lowmem" is
> set to true at the beginning. If failed to allocate bank0 under 4GB,
> need to panic for 32-bit domain, because 32-bit domain requires bank0
> be allocated under 4GB.
>
> For 64-bit domain, set "lowmem" to false, and continue allocating
> memory from higher memory space.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>
> This patch is to resolve the issue mentioned in
> https://lists.xen.org/archives/html/xen-devel/2016-09/msg00235.html
>
> V3:
>  Add more commit log
>  Add more comments
>  Add back panic if failed to allocate bank0 under 4GB for 32-bit domain.
>
> 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 | 44 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 35ab08d..a9c37c4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -195,9 +195,10 @@ fail:
>   *    bank. Partly this is just easier for us to deal with, but also
>   *    the ramdisk and DTB must be placed within a certain proximity of
>   *    the kernel within RAM.
> - * 3. For 32-bit dom0 we want to place as much of the RAM as we
> - *    reasonably can below 4GB, so that it can be used by non-LPAE
> - *    enabled kernels.
> + * 3. For dom0 we want to place as much of the RAM as we reasonably can
> + *    below 4GB, so that the devices have the limitation to access 64 bits
> + *    address space and work properly and it can be used by non-LPAE enabled
> + *    kernels.

I would say: "For dom0 we want to place as much of the RAM as we 
reasonably can below 4GB, so that it can be used by non-LPAE enabled 
kernels (32-bit) or when a device assigned to dom0 can only do 32-bit 
DMA access".

>   * 4. For 32-bit dom0 the kernel must be located below 4GB.
>   * 5. We want to have a few largers banks rather than many smaller ones.
>   *
> @@ -230,7 +231,8 @@ fail:
>   * we give up.
>   *
>   * For 32-bit domain we require that the initial allocation for the
> - * first bank is under 4G. Then for the subsequent allocations we
> + * first bank is under 4G. For 64-bit domain, the first bank is preferred
> + * to be allocated under 4G. Then for the subsequent allocations we
>   * initially allocate memory only from below 4GB. Once that runs out
>   * (as described above) we allow higher allocations and continue until
>   * that runs out (or we have allocated sufficient dom0 memory).
> @@ -240,11 +242,11 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>      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,24 +267,40 @@ 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++ )

Why this change? If we ever decide to set lowmen to false, it will not 
work anymore.

>          {
>              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;
> +	    }

The indentation looks wrong here.

>          }
>          order--;
>      }
>
> -    panic("Unable to allocate first memory bank");
> -
> - got_bank0:
> +    /* Failed to allocate bank0 under 4GB */
> +    if ( pg == NULL )

This check is not necessary because you can only be here when (pg == NULL).

> +    {
> +        if ( is_32bit_domain(d) )
> +            panic("Unable to allocate first memory bank.");
>
> -    if ( !insert_11_bank(d, kinfo, pg, order) )
> -        BUG(); /* Cannot fail for first bank */
> +        /* Try allocating higher memory */

Try to allocate higher memory

> +        printk(XENLOG_INFO "No bank has been allocated below 4GB.\n");
> +        lowmem = false;
> +    }
>
> -    /* Now allocate more memory and fill in additional banks */
> + got_bank0:
>
> +    /*
> +     * If failed to allocate bank0 under 4GB, continue allocating

If we failed to allocate

> +     * memory from higher memory space and fill in banks.

"higher" is not clear. So I would say "from above 4GB"

> +     * Otherwise allocate more memory under 4GB and fill in additional
> +     * banks. If there is not enough memory under 4GB, use higher

Ditto.

> +     * memory.
> +     */

Although, I would just drop the second sentence. It is a reword of the 
big description on top of the function.

>      order = get_11_allocation_size(kinfo->unassigned_mem);
>      while ( kinfo->unassigned_mem && kinfo->mem.nr_banks < NR_MEM_BANKS )
>      {
>

Regards,
Peng Fan Sept. 23, 2016, 2:01 a.m. UTC | #2
On Thu, Sep 22, 2016 at 04:23:05PM +0100, Julien Grall wrote:
>Hello Peng,
>
>On 22/09/16 10:16, Peng Fan wrote:
>>On AArch64 SoCs, some IPs may only have the capability to access
>>32 bits address space. The physical memory assigned for Dom0 maybe
>>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.
>
>The last sentence is confusing. I would avoid to mention domU use case here.

Drop it in V4.

>
>>
>>This patch does not affect 32-bit domain, because Variable "lowmem" is
>>set to true at the beginning. If failed to allocate bank0 under 4GB,
>>need to panic for 32-bit domain, because 32-bit domain requires bank0
>>be allocated under 4GB.
>>
>>For 64-bit domain, set "lowmem" to false, and continue allocating
>>memory from higher memory space.
>>
>>Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>Cc: Stefano Stabellini <sstabellini@kernel.org>
>>Cc: Julien Grall <julien.grall@arm.com>
>>---
>>
>>This patch is to resolve the issue mentioned in
>>https://lists.xen.org/archives/html/xen-devel/2016-09/msg00235.html
>>
>>V3:
>> Add more commit log
>> Add more comments
>> Add back panic if failed to allocate bank0 under 4GB for 32-bit domain.
>>
>>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 | 44 +++++++++++++++++++++++++++++++-------------
>> 1 file changed, 31 insertions(+), 13 deletions(-)
>>
>>diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>index 35ab08d..a9c37c4 100644
>>--- a/xen/arch/arm/domain_build.c
>>+++ b/xen/arch/arm/domain_build.c
>>@@ -195,9 +195,10 @@ fail:
>>  *    bank. Partly this is just easier for us to deal with, but also
>>  *    the ramdisk and DTB must be placed within a certain proximity of
>>  *    the kernel within RAM.
>>- * 3. For 32-bit dom0 we want to place as much of the RAM as we
>>- *    reasonably can below 4GB, so that it can be used by non-LPAE
>>- *    enabled kernels.
>>+ * 3. For dom0 we want to place as much of the RAM as we reasonably can
>>+ *    below 4GB, so that the devices have the limitation to access 64 bits
>>+ *    address space and work properly and it can be used by non-LPAE enabled
>>+ *    kernels.
>
>I would say: "For dom0 we want to place as much of the RAM as we reasonably
>can below 4GB, so that it can be used by non-LPAE enabled kernels (32-bit) or
>when a device assigned to dom0 can only do 32-bit DMA access".

Thanks. Fix in V4.

>
>>  * 4. For 32-bit dom0 the kernel must be located below 4GB.
>>  * 5. We want to have a few largers banks rather than many smaller ones.
>>  *
>>@@ -230,7 +231,8 @@ fail:
>>  * we give up.
>>  *
>>  * For 32-bit domain we require that the initial allocation for the
>>- * first bank is under 4G. Then for the subsequent allocations we
>>+ * first bank is under 4G. For 64-bit domain, the first bank is preferred
>>+ * to be allocated under 4G. Then for the subsequent allocations we
>>  * initially allocate memory only from below 4GB. Once that runs out
>>  * (as described above) we allow higher allocations and continue until
>>  * that runs out (or we have allocated sufficient dom0 memory).
>>@@ -240,11 +242,11 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>>     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,24 +267,40 @@ 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++ )
>
>Why this change? If we ever decide to set lowmen to false, it will not work
>anymore.

Reasonable. Fix in V4.

>
>>         {
>>             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;
>>+	    }
>
>The indentation looks wrong here.

Fix in V4.

>
>>         }
>>         order--;
>>     }
>>
>>-    panic("Unable to allocate first memory bank");
>>-
>>- got_bank0:
>>+    /* Failed to allocate bank0 under 4GB */
>>+    if ( pg == NULL )
>
>This check is not necessary because you can only be here when (pg == NULL).

Right. Drop it in V4.

>
>>+    {
>>+        if ( is_32bit_domain(d) )
>>+            panic("Unable to allocate first memory bank.");
>>
>>-    if ( !insert_11_bank(d, kinfo, pg, order) )
>>-        BUG(); /* Cannot fail for first bank */
>>+        /* Try allocating higher memory */
>
>Try to allocate higher memory

Fix in V4.

>
>>+        printk(XENLOG_INFO "No bank has been allocated below 4GB.\n");
>>+        lowmem = false;
>>+    }
>>
>>-    /* Now allocate more memory and fill in additional banks */
>>+ got_bank0:
>>
>>+    /*
>>+     * If failed to allocate bank0 under 4GB, continue allocating
>
>If we failed to allocate

Fix in V4.

>
>>+     * memory from higher memory space and fill in banks.
>
>"higher" is not clear. So I would say "from above 4GB"

Fix in V4.

>
>>+     * Otherwise allocate more memory under 4GB and fill in additional
>>+     * banks. If there is not enough memory under 4GB, use higher
>
>Ditto.
>
>>+     * memory.
>>+     */
>
>Although, I would just drop the second sentence. It is a reword of the big
>description on top of the function.

Ok.

Thanks,
Peng.
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 35ab08d..a9c37c4 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -195,9 +195,10 @@  fail:
  *    bank. Partly this is just easier for us to deal with, but also
  *    the ramdisk and DTB must be placed within a certain proximity of
  *    the kernel within RAM.
- * 3. For 32-bit dom0 we want to place as much of the RAM as we
- *    reasonably can below 4GB, so that it can be used by non-LPAE
- *    enabled kernels.
+ * 3. For dom0 we want to place as much of the RAM as we reasonably can
+ *    below 4GB, so that the devices have the limitation to access 64 bits
+ *    address space and work properly and it can be used by non-LPAE enabled
+ *    kernels.
  * 4. For 32-bit dom0 the kernel must be located below 4GB.
  * 5. We want to have a few largers banks rather than many smaller ones.
  *
@@ -230,7 +231,8 @@  fail:
  * we give up.
  *
  * For 32-bit domain we require that the initial allocation for the
- * first bank is under 4G. Then for the subsequent allocations we
+ * first bank is under 4G. For 64-bit domain, the first bank is preferred
+ * to be allocated under 4G. Then for the subsequent allocations we
  * initially allocate memory only from below 4GB. Once that runs out
  * (as described above) we allow higher allocations and continue until
  * that runs out (or we have allocated sufficient dom0 memory).
@@ -240,11 +242,11 @@  static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
     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,24 +267,40 @@  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");
-
- got_bank0:
+    /* Failed to allocate bank0 under 4GB */
+    if ( pg == NULL )
+    {
+        if ( is_32bit_domain(d) )
+            panic("Unable to allocate first memory bank.");
 
-    if ( !insert_11_bank(d, kinfo, pg, order) )
-        BUG(); /* Cannot fail for first bank */
+        /* Try allocating higher memory */
+        printk(XENLOG_INFO "No bank has been allocated below 4GB.\n");
+        lowmem = false;
+    }
 
-    /* Now allocate more memory and fill in additional banks */
+ got_bank0:
 
+    /*
+     * If failed to allocate bank0 under 4GB, continue allocating
+     * memory from higher memory space and fill in banks.
+     * Otherwise allocate more memory under 4GB and fill in additional
+     * banks. If there is not enough memory under 4GB, use higher
+     * memory.
+     */
     order = get_11_allocation_size(kinfo->unassigned_mem);
     while ( kinfo->unassigned_mem && kinfo->mem.nr_banks < NR_MEM_BANKS )
     {