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

[Peng Fan <van.freenix@xxxxxxxxx>]

On Wed, Sep 21, 2016 at 11:39:11AM +0100, Julien Grall wrote:
>
>
>On 20/09/16 10:10, Peng Fan wrote:
>>Hello Julien,
>
>Hello Peng,
>
>>On Tue, Sep 20, 2016 at 10:36:27AM +0200, Julien Grall wrote:
>>>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/
>>
>>Fix in V3.
>>
>>>
>>>>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.
>>
>>About this part:
>>* 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.
>>
>>Changed to this V3:
>>For dom0 we want to place as much of the RAM as we reasonably can
>>below 4GB, so the devices have the limitation to access 64 bits
>>address space can work properly and it can also be used by
>>non-LPAE enabled kernels for 32-bit dom0.
>>
>>About
>>"
>> * For 32-bit domain we require that the initial allocation for the
>> * first bank is under 4G.
>>"
>>
>>Changed to this in V3:
>>
>>"
>>For 32-bit domain we require that the initial allocation for the
>>first bank is under 4G. For 64-bit domain, the first bank is preferred
>>to be allocated under 4GB.
>>"
>>
>>Is this ok?
>
>I think so.

Thanks. Update in v3.

>
>>
>>>
>>>>    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.
>>
>>Consider the comments of the function:
>>  1. The dom0 kernel should be loaded within the first 128MB of RAM. This
>>     is necessary at least for Linux zImage kernels, which are all we
>>     support today.
>> 4. For 32-bit dom0 the kernel must be located below 4GB.
>>
>>I think I may need to add back the panic when first bank is failed to be 
>>allocated
>>under 4GB.
>
>It occurred to me that the first bank below 4GB is a mandatory for 32-bit
>domain, because the kernel is booted with paging disabled.

Yeah. I'll add back the panic for 32bit domain.
Add document this in commit message.

>
>>
>>Will use the following code in V3 and document this in commit log.
>>    if ( pg == NULL )
>>    {
>>        if ( is_32bit_domain(d) )
>>            panic("Unable to allocate first memory bank");
>>
>>        printk(XENLOG_INFO "No bank has been allocated below 4GB.\n");
>>        lowmem = false;
>>    }
>>
>>
>>>
>>>>
>>>> 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.
>>
>>Add more comments? I did not modify code below the comment and the comment
>>is correct I think.
>
>Well, you can reach here with the bank 0 not allocated, so "now allocate more
>memory..." is inaccurate for me. When you do change, you also need to see
>whether it fits with the rest of the function.

ok. Will update this in v3.

Thanks,
Peng.
>
>Regards,
>
>-- 
>Julien Grall

-- 

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