Re: [PATCH v3 8/8] xen: arm: allocate more than one bank for 1:1 domain 0 if needed

[Julien Grall <julien.grall@linaro.org>]

On 27/06/14 10:24, Ian Campbell wrote:
> On Thu, 2014-06-26 at 19:20 +0100, Julien Grall wrote:
>> On 06/26/2014 11:17 AM, Ian Campbell wrote:
>>> Depending on where Xen and the initial modules were loaded into RAM we may not
>>> be able to allocate a suitably contiguous block of memory for dom0. Especially
>>> since the allocations made by alloc_domheap_pages are naturally aligned (e.g. a
>>> 1GB allocation must be at a 1GB boundary).
>>>
>>> The alignment requirement in particular is a huge limitation, meaning that even
>>> dom0_mem0=1G on a 2GB system is pretty likely to fail unless you are very
>>> careful with your load addresses. People were also having trouble with dom0 >
>>> 128MB on the 1GB cubieboard2 when using fairly standard load addresses for
>>> things.
>>>
>>> This patch tries to allocate multiple banks of memory in order to try and
>>> satisfy the entire requested domain 0 allocation. Sadly this turns out to be
>>> pretty tricky to arrange (see the large comment in the code).
>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> Cc: Karim Raslan <karim.allah.ahmed@gmail.com>
>>> ---
>>> v3:
>>>    Handle running out of banks more gracefully
>>>    Allow order >= min_low_order, not strictly greater. Otherwise
>>>    dom0_mem=128M doesn't work.
>>
>> dom0_mem still not working on the versatile express (see my explanation
>> a below).
>
>>
>>> +static void allocate_memory_11(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;
>>> +    unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
>>> +    int i;
>>> +
>>> +    bool_t lowmem = is_32bit_domain(d);
>>> +    unsigned int bits;
>>> +
>>> +    printk("Allocating 1:1 mappings totalling %ldMB for dom0:\n",
>>> +           /* Don't want format this as PRIpaddr (16 digit hex) */
>>> +           (unsigned long)(kinfo->unassigned_mem >> 20));
>>> +
>>> +    kinfo->mem.nr_banks = 0;
>>> +
>>> +    /*
>>> +     * First try and allocate the largest thing we can as low as
>>> +     * possible to be bank 0.
>>> +     */
>>> +    while ( order >= min_low_order )
>>> +    {
>>> +        for ( bits = order ; bits < (lowmem ? 32 : PADDR_BITS); bits++ )
>>
>> This has to be: bits <= (...). Indeed, we want to try all possible zone.

After digging into the code, I confirm that we need the "<=".

the zone is retrieved with this formula: fls(page_to_mfn(pg)), in the 
case of the versatile express the bank start mfn is 0x8000.
So every page will reach the zone 20. At the same time zone 20 is equals 
to bits=32.

Futhermore, it looks like we always allocate memory from the top of this 
zone. Is it normal?

Regards,

-- 
Julien Grall