Vmap allocator fails to allocate beyond 128MB

Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

Hi,

  When devices like SMMU request large ioremap space and if the total allocation
of vmap space is beyond 128MB the allocation fails for next requests
and following warning is seen

create_xen_entries: trying to replace an existing mapping
addr=40001000 mfn=fffd6

I found that vm_top is allocated with only 1 page which can hold
bitmap for only 128MB
space though 1GB of vmap space is assigned.

With 1GB vmap space following are the calculations

vm_base = 0x4000000
vm_end = 0x3ffff
vm_low = 0x8
nr = 1
vm_top = 0x8000

With the below patch, I could get allocations beyond 128MB.

where nr = 8 for 1GB vmap space

diff --git a/xen/common/vmap.c b/xen/common/vmap.c
index 783cea3..369212d 100644
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -27,7 +27,7 @@ void __init vm_init(void)
     vm_base = (void *)VMAP_VIRT_START;
     vm_end = PFN_DOWN(arch_vmap_virt_end() - vm_base);
     vm_low = PFN_UP((vm_end + 7) / 8);
-    nr = PFN_UP((vm_low + 7) / 8);
+    nr = PFN_UP((vm_end + 7) / 8);
     vm_top = nr * PAGE_SIZE * 8;

     for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )

Regards
Vijay

Re: Vmap allocator fails to allocate beyond 128MB

[Jan Beulich]

>>> On 26.09.14 at 14:17, <vijay.kilari@gmail.com> wrote:
>   When devices like SMMU request large ioremap space and if the total 
> allocation
> of vmap space is beyond 128MB the allocation fails for next requests
> and following warning is seen
> 
> create_xen_entries: trying to replace an existing mapping
> addr=40001000 mfn=fffd6
> 
> I found that vm_top is allocated with only 1 page which can hold
> bitmap for only 128MB
> space though 1GB of vmap space is assigned.
> 
> With 1GB vmap space following are the calculations
> 
> vm_base = 0x4000000
> vm_end = 0x3ffff
> vm_low = 0x8
> nr = 1
> vm_top = 0x8000
> 
> With the below patch, I could get allocations beyond 128MB.
> 
> where nr = 8 for 1GB vmap space
> 
> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
> index 783cea3..369212d 100644
> --- a/xen/common/vmap.c
> +++ b/xen/common/vmap.c
> @@ -27,7 +27,7 @@ void __init vm_init(void)
>      vm_base = (void *)VMAP_VIRT_START;
>      vm_end = PFN_DOWN(arch_vmap_virt_end() - vm_base);
>      vm_low = PFN_UP((vm_end + 7) / 8);
> -    nr = PFN_UP((vm_low + 7) / 8);
> +    nr = PFN_UP((vm_end + 7) / 8);
>      vm_top = nr * PAGE_SIZE * 8;
> 
>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )

Maybe there's a bug somewhere, but what you suggest as a change
above doesn't look correct: You make nr == vm_low, and hence the
map_pages_to_xen() after the loop do nothing. That can't be right.
Is it perhaps that this second map_pages_to_xen() doesn't have the
intended effect on ARM?

In any event is allocating just a single page for the bitmap initially
the expected behavior. Further bitmap pages will get allocated on
demand in vm_alloc().

Jan

Jan

Re: Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

Hi Jan,

On Fri, Sep 26, 2014 at 6:16 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.09.14 at 14:17, <vijay.kilari@gmail.com> wrote:
>>   When devices like SMMU request large ioremap space and if the total
>> allocation
>> of vmap space is beyond 128MB the allocation fails for next requests
>> and following warning is seen
>>
>> create_xen_entries: trying to replace an existing mapping
>> addr=40001000 mfn=fffd6
>>
>> I found that vm_top is allocated with only 1 page which can hold
>> bitmap for only 128MB
>> space though 1GB of vmap space is assigned.
>>
>> With 1GB vmap space following are the calculations
>>
>> vm_base = 0x4000000
>> vm_end = 0x3ffff
>> vm_low = 0x8
>> nr = 1
>> vm_top = 0x8000
>>
>> With the below patch, I could get allocations beyond 128MB.
>>
>> where nr = 8 for 1GB vmap space
>>
>> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
>> index 783cea3..369212d 100644
>> --- a/xen/common/vmap.c
>> +++ b/xen/common/vmap.c
>> @@ -27,7 +27,7 @@ void __init vm_init(void)
>>      vm_base = (void *)VMAP_VIRT_START;
>>      vm_end = PFN_DOWN(arch_vmap_virt_end() - vm_base);
>>      vm_low = PFN_UP((vm_end + 7) / 8);
>> -    nr = PFN_UP((vm_low + 7) / 8);
>> +    nr = PFN_UP((vm_end + 7) / 8);
>>      vm_top = nr * PAGE_SIZE * 8;
>>
>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )
>
> Maybe there's a bug somewhere, but what you suggest as a change
> above doesn't look correct: You make nr == vm_low, and hence the
> map_pages_to_xen() after the loop do nothing. That can't be right.
> Is it perhaps that this second map_pages_to_xen() doesn't have the
> intended effect on ARM?

Note: I am testing on arm64 platform.

In the call map_pages_to_xen() after for loop is performing the mapping
for next vm_bitmap pages. In case of arm  this call will set valid bit
is set to 1 in pte
entry for this mapping.

void __init vm_init(void)
{
     ....
     for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )
     {
         struct page_info *pg = alloc_domheap_page(NULL, 0);

         map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
         clear_page((void *)va);
     }
     bitmap_fill(vm_bitmap, vm_low);

     /* Populate page tables for the bitmap if necessary. */
     map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
 }

In vma_alloc()  below map_pages_to_xen() is failing for >128MB because
for this next vm_bitmap
page the mapping is already set in vm_init(). So map_pages_to_xen() in
ARM returns error.

          if ( start >= vm_top )
         {
             unsigned long va = (unsigned long)vm_bitmap + vm_top / 8;

             if ( !map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR) )
           ...
         }

So my patch is making map_pages_to_xen() call in vm_init is not doing anything
because nr_mfns is 0 ( parameter 3 is 0).

Queries: 1) How is x86 is updating tables even if present/valid bit is set?
             2) Can we allocate all the pages required for vm_bitmap
in vm_init()?. we may be wasting few pages but
                this makes work for both x86&arm.
             3) Can we split vm_init() into generic and arch specific?

 Hi Ian,
       Can we one of following for arm
        1) Add new option in create_xen_entries() something like
RESERVE where valid bit is not set
            and map_pages_to_xen() should choose this option in case
of mfn is 0?
        2) Just return without doing an mapping if mfn is 0 in
map_page_to_xen()?

>
> In any event is allocating just a single page for the bitmap initially
> the expected behavior. Further bitmap pages will get allocated on
> demand in vm_alloc().
>
> Jan
>
> Jan
>

Re: Vmap allocator fails to allocate beyond 128MB

[Jan Beulich]

>>> On 26.09.14 at 17:23, <vijay.kilari@gmail.com> wrote:
> Hi Jan,
> 
> On Fri, Sep 26, 2014 at 6:16 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 26.09.14 at 14:17, <vijay.kilari@gmail.com> wrote:
>>>   When devices like SMMU request large ioremap space and if the total
>>> allocation
>>> of vmap space is beyond 128MB the allocation fails for next requests
>>> and following warning is seen
>>>
>>> create_xen_entries: trying to replace an existing mapping
>>> addr=40001000 mfn=fffd6
>>>
>>> I found that vm_top is allocated with only 1 page which can hold
>>> bitmap for only 128MB
>>> space though 1GB of vmap space is assigned.
>>>
>>> With 1GB vmap space following are the calculations
>>>
>>> vm_base = 0x4000000
>>> vm_end = 0x3ffff
>>> vm_low = 0x8
>>> nr = 1
>>> vm_top = 0x8000
>>>
>>> With the below patch, I could get allocations beyond 128MB.
>>>
>>> where nr = 8 for 1GB vmap space
>>>
>>> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
>>> index 783cea3..369212d 100644
>>> --- a/xen/common/vmap.c
>>> +++ b/xen/common/vmap.c
>>> @@ -27,7 +27,7 @@ void __init vm_init(void)
>>>      vm_base = (void *)VMAP_VIRT_START;
>>>      vm_end = PFN_DOWN(arch_vmap_virt_end() - vm_base);
>>>      vm_low = PFN_UP((vm_end + 7) / 8);
>>> -    nr = PFN_UP((vm_low + 7) / 8);
>>> +    nr = PFN_UP((vm_end + 7) / 8);
>>>      vm_top = nr * PAGE_SIZE * 8;
>>>
>>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE 
> )
>>
>> Maybe there's a bug somewhere, but what you suggest as a change
>> above doesn't look correct: You make nr == vm_low, and hence the
>> map_pages_to_xen() after the loop do nothing. That can't be right.
>> Is it perhaps that this second map_pages_to_xen() doesn't have the
>> intended effect on ARM?
> 
> Note: I am testing on arm64 platform.
> 
> In the call map_pages_to_xen() after for loop is performing the mapping
> for next vm_bitmap pages. In case of arm  this call will set valid bit
> is set to 1 in pte
> entry for this mapping.

So _that_ is the bug then, because ...

> void __init vm_init(void)
> {
>      ....
>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )
>      {
>          struct page_info *pg = alloc_domheap_page(NULL, 0);
> 
>          map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
>          clear_page((void *)va);
>      }
>      bitmap_fill(vm_bitmap, vm_low);
> 
>      /* Populate page tables for the bitmap if necessary. */
>      map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);

... here we don't request any valid leaf entries to be created. All we
want are the non-leaf page table structures.

> Queries: 1) How is x86 is updating tables even if present/valid bit is set?

When not asked to set the present bit, it also doesn't set it, and
hence also doesn't find any collisions later.

>              2) Can we allocate all the pages required for vm_bitmap
> in vm_init()?. we may be wasting few pages but
>                 this makes work for both x86&arm.

No - we shouldn't be wasting memory here.

>              3) Can we split vm_init() into generic and arch specific?

That would be kind of a last resort.

Jan

Re: Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

On Fri, Sep 26, 2014 at 9:21 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.09.14 at 17:23, <vijay.kilari@gmail.com> wrote:
>> Hi Jan,
>>
>> On Fri, Sep 26, 2014 at 6:16 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 26.09.14 at 14:17, <vijay.kilari@gmail.com> wrote:
>>>>   When devices like SMMU request large ioremap space and if the total
>>>> allocation
>>>> of vmap space is beyond 128MB the allocation fails for next requests
>>>> and following warning is seen
>>>>
>>>> create_xen_entries: trying to replace an existing mapping
>>>> addr=40001000 mfn=fffd6
>>>>
>>>> I found that vm_top is allocated with only 1 page which can hold
>>>> bitmap for only 128MB
>>>> space though 1GB of vmap space is assigned.
>>>>
>>>> With 1GB vmap space following are the calculations
>>>>
>>>> vm_base = 0x4000000
>>>> vm_end = 0x3ffff
>>>> vm_low = 0x8
>>>> nr = 1
>>>> vm_top = 0x8000
>>>>
>>>> With the below patch, I could get allocations beyond 128MB.
>>>>
>>>> where nr = 8 for 1GB vmap space
>>>>
>>>> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
>>>> index 783cea3..369212d 100644
>>>> --- a/xen/common/vmap.c
>>>> +++ b/xen/common/vmap.c
>>>> @@ -27,7 +27,7 @@ void __init vm_init(void)
>>>>      vm_base = (void *)VMAP_VIRT_START;
>>>>      vm_end = PFN_DOWN(arch_vmap_virt_end() - vm_base);
>>>>      vm_low = PFN_UP((vm_end + 7) / 8);
>>>> -    nr = PFN_UP((vm_low + 7) / 8);
>>>> +    nr = PFN_UP((vm_end + 7) / 8);
>>>>      vm_top = nr * PAGE_SIZE * 8;
>>>>
>>>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE
>> )
>>>
>>> Maybe there's a bug somewhere, but what you suggest as a change
>>> above doesn't look correct: You make nr == vm_low, and hence the
>>> map_pages_to_xen() after the loop do nothing. That can't be right.
>>> Is it perhaps that this second map_pages_to_xen() doesn't have the
>>> intended effect on ARM?
>>
>> Note: I am testing on arm64 platform.
>>
>> In the call map_pages_to_xen() after for loop is performing the mapping
>> for next vm_bitmap pages. In case of arm  this call will set valid bit
>> is set to 1 in pte
>> entry for this mapping.
>
> So _that_ is the bug then, because ...
>
>> void __init vm_init(void)
>> {
>>      ....
>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE )
>>      {
>>          struct page_info *pg = alloc_domheap_page(NULL, 0);
>>
>>          map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
>>          clear_page((void *)va);
>>      }
>>      bitmap_fill(vm_bitmap, vm_low);
>>
>>      /* Populate page tables for the bitmap if necessary. */
>>      map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
>
> ... here we don't request any valid leaf entries to be created. All we
> want are the non-leaf page table structures.

Do we need to reserve this non-leaf page table structures?
If vm_low is reserving the virtual address, vm_alloc will not allocate
vm_bitmap reserved pages.

>
>> Queries: 1) How is x86 is updating tables even if present/valid bit is set?
>
> When not asked to set the present bit, it also doesn't set it, and
> hence also doesn't find any collisions later.
>
>>              2) Can we allocate all the pages required for vm_bitmap
>> in vm_init()?. we may be wasting few pages but
>>                 this makes work for both x86&arm.
>
> No - we shouldn't be wasting memory here.
>
>>              3) Can we split vm_init() into generic and arch specific?
>
> That would be kind of a last resort.
>
> Jan
>

Re: Vmap allocator fails to allocate beyond 128MB

[Jan Beulich]

>>> On 29.09.14 at 07:42, <vijay.kilari@gmail.com> wrote:
> On Fri, Sep 26, 2014 at 9:21 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 26.09.14 at 17:23, <vijay.kilari@gmail.com> wrote:
>>> In the call map_pages_to_xen() after for loop is performing the mapping
>>> for next vm_bitmap pages. In case of arm  this call will set valid bit
>>> is set to 1 in pte
>>> entry for this mapping.
>>
>> So _that_ is the bug then, because ...
>>
>>> void __init vm_init(void)
>>> {
>>>      ....
>>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE 
> )
>>>      {
>>>          struct page_info *pg = alloc_domheap_page(NULL, 0);
>>>
>>>          map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
>>>          clear_page((void *)va);
>>>      }
>>>      bitmap_fill(vm_bitmap, vm_low);
>>>
>>>      /* Populate page tables for the bitmap if necessary. */
>>>      map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
>>
>> ... here we don't request any valid leaf entries to be created. All we
>> want are the non-leaf page table structures.
> 
> Do we need to reserve this non-leaf page table structures?
> If vm_low is reserving the virtual address, vm_alloc will not allocate
> vm_bitmap reserved pages.

Not sure what you mean with "reserve" here. Again - we want the
page table structures to be created (without handing out the
respective virtual addresses to anyone), so that we don't need to
be bothered with this when actual allocation requests come in.

Jan

Re: Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

On Mon, Sep 29, 2014 at 12:11 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 29.09.14 at 07:42, <vijay.kilari@gmail.com> wrote:
>> On Fri, Sep 26, 2014 at 9:21 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 26.09.14 at 17:23, <vijay.kilari@gmail.com> wrote:
>>>> In the call map_pages_to_xen() after for loop is performing the mapping
>>>> for next vm_bitmap pages. In case of arm  this call will set valid bit
>>>> is set to 1 in pte
>>>> entry for this mapping.
>>>
>>> So _that_ is the bug then, because ...
>>>
>>>> void __init vm_init(void)
>>>> {
>>>>      ....
>>>>      for ( i = 0, va = (unsigned long)vm_bitmap; i < nr; ++i, va += PAGE_SIZE
>> )
>>>>      {
>>>>          struct page_info *pg = alloc_domheap_page(NULL, 0);
>>>>
>>>>          map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
>>>>          clear_page((void *)va);
>>>>      }
>>>>      bitmap_fill(vm_bitmap, vm_low);
>>>>
>>>>      /* Populate page tables for the bitmap if necessary. */
>>>>      map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
>>>
>>> ... here we don't request any valid leaf entries to be created. All we
>>> want are the non-leaf page table structures.
>>
>> Do we need to reserve this non-leaf page table structures?
>> If vm_low is reserving the virtual address, vm_alloc will not allocate
>> vm_bitmap reserved pages.
>
> Not sure what you mean with "reserve" here. Again - we want the
> page table structures to be created (without handing out the
> respective virtual addresses to anyone), so that we don't need to
> be bothered with this when actual allocation requests come in.

I mean, vm_low holds the lowest clear bit in vm_bitmap from where
vm_alloc starts allocation. By setting vm_low = PFN_UP((vm_end + 7) / 8);
vm_low is reserving the all the virtual address/pages required for
vm_bitmap in future.

Ex: In case of 1GB vmap space. vm_low is 8. meaning the first
8 pages are reserved for vm_bitmap.

We don't need to allocate page table structures in vm_init
for vm_bitmap reserved pages  because this  virtual addresses are
reserved for vm_bitmap and are not given to anyone by vm_alloc.
So page table structures can be created when actual allocation
requests comes in for vm_bitmap.

So I feel map_pages_to_xen() after for loop in vm_init
is not required.

Vijay

Re: Vmap allocator fails to allocate beyond 128MB

[Jan Beulich]

>>> On 29.09.14 at 09:10, <vijay.kilari@gmail.com> wrote:
> We don't need to allocate page table structures in vm_init
> for vm_bitmap reserved pages  because this  virtual addresses are
> reserved for vm_bitmap and are not given to anyone by vm_alloc.
> So page table structures can be created when actual allocation
> requests comes in for vm_bitmap.

They could, but did you check what that would mean to code
complexity? In particular the current approach avoids calling
map_pages_to_xen() in ways where it might need to do allocations
(to establish page tables) while already holding a spin lock in the
caller. Please remember that while it is not strictly forbidden in Xen
to do (page) allocations with a lock held, it is bad practice and
should hence be avoided where possible.

Jan

Re: Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

Hi Ian,

  For ThunderX/arm64 this issue needs to be fixed.
Could you please comment on this?

I could think of below

        1) Add new call for ARM under CONFIG_ARM_32/CONFIG_ARM_64
in vm_init() and manage map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
with different function call for arm that would not make any pte
entries for vm_bitmap pages.
This avoids change to x86

        2) Remove map_pages_to_xen(va, 0, vm_low - nr,
MAP_SMALL_PAGES) from vm_init
and add new architecture specific for initializing vm_bitmap pages.
But this touches
x86 code.


Regards
Vijay

On Mon, Sep 29, 2014 at 12:56 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 29.09.14 at 09:10, <vijay.kilari@gmail.com> wrote:
>> We don't need to allocate page table structures in vm_init
>> for vm_bitmap reserved pages  because this  virtual addresses are
>> reserved for vm_bitmap and are not given to anyone by vm_alloc.
>> So page table structures can be created when actual allocation
>> requests comes in for vm_bitmap.
>
> They could, but did you check what that would mean to code
> complexity? In particular the current approach avoids calling
> map_pages_to_xen() in ways where it might need to do allocations
> (to establish page tables) while already holding a spin lock in the
> caller. Please remember that while it is not strictly forbidden in Xen
> to do (page) allocations with a lock held, it is bad practice and
> should hence be avoided where possible.
>
> Jan
>

Re: Vmap allocator fails to allocate beyond 128MB

[Julien Grall]

On 16/02/15 10:17, Vijay Kilari wrote:

Hello Vijay,

>   For ThunderX/arm64 this issue needs to be fixed.
> Could you please comment on this?

AFAICS, x86 is also using a 1G area for the vmap. Does it mean that x86
never use vmap for more than 128M?

> I could think of below
> 
>         1) Add new call for ARM under CONFIG_ARM_32/CONFIG_ARM_64
> in vm_init() and manage map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
> with different function call for arm that would not make any pte
> entries for vm_bitmap pages.
> This avoids change to x86
> 
>         2) Remove map_pages_to_xen(va, 0, vm_low - nr,
> MAP_SMALL_PAGES) from vm_init
> and add new architecture specific for initializing vm_bitmap pages.
> But this touches
> x86 code.

I don't know which approach is the best, and I though you already talked
about it with Jan... But in general a generic approach is better than a
per-architecture solution.

Regards,

-- 
Julien Grall

Re: Vmap allocator fails to allocate beyond 128MB

[Vijay Kilari]

On Mon, Feb 16, 2015 at 3:58 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 16/02/15 10:17, Vijay Kilari wrote:
>
> Hello Vijay,
>
>>   For ThunderX/arm64 this issue needs to be fixed.
>> Could you please comment on this?
>
> AFAICS, x86 is also using a 1G area for the vmap. Does it mean that x86
> never use vmap for more than 128M?

I think for x86 there is no problem. It works beyond 128M

>
>> I could think of below
>>
>>         1) Add new call for ARM under CONFIG_ARM_32/CONFIG_ARM_64
>> in vm_init() and manage map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
>> with different function call for arm that would not make any pte
>> entries for vm_bitmap pages.
>> This avoids change to x86
>>
>>         2) Remove map_pages_to_xen(va, 0, vm_low - nr,
>> MAP_SMALL_PAGES) from vm_init
>> and add new architecture specific for initializing vm_bitmap pages.
>> But this touches
>> x86 code.
>
> I don't know which approach is the best, and I though you already talked
> about it with Jan... But in general a generic approach is better than a
> per-architecture solution.

When I say architecture specific, I mean, a common function will be
created and implemented in architecture specific files similar to
map_pages_to_xen. This new function will wrap the required code
for per-architecture.

Vijay

Re: Vmap allocator fails to allocate beyond 128MB

[Julien Grall]

On 16/02/15 10:50, Vijay Kilari wrote:
> On Mon, Feb 16, 2015 at 3:58 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> On 16/02/15 10:17, Vijay Kilari wrote:
>>
>> Hello Vijay,
>>
>>>   For ThunderX/arm64 this issue needs to be fixed.
>>> Could you please comment on this?
>>
>> AFAICS, x86 is also using a 1G area for the vmap. Does it mean that x86
>> never use vmap for more than 128M?
> 
> I think for x86 there is no problem. It works beyond 128M

Did you test it? The code seems common enough to make the problem
appears on x86.

>>
>>> I could think of below
>>>
>>>         1) Add new call for ARM under CONFIG_ARM_32/CONFIG_ARM_64
>>> in vm_init() and manage map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
>>> with different function call for arm that would not make any pte
>>> entries for vm_bitmap pages.
>>> This avoids change to x86
>>>
>>>         2) Remove map_pages_to_xen(va, 0, vm_low - nr,
>>> MAP_SMALL_PAGES) from vm_init
>>> and add new architecture specific for initializing vm_bitmap pages.
>>> But this touches
>>> x86 code.
>>
>> I don't know which approach is the best, and I though you already talked
>> about it with Jan... But in general a generic approach is better than a
>> per-architecture solution.
> 
> When I say architecture specific, I mean, a common function will be
> created and implemented in architecture specific files similar to
> map_pages_to_xen. This new function will wrap the required code
> for per-architecture.

I would advice you to send an RFC on the mailing list. So we have a base
to talk about it.

Regards,

-- 
Julien Grall

Re: Vmap allocator fails to allocate beyond 128MB

[Julien Grall]

On 16/02/15 10:57, Julien Grall wrote:
> On 16/02/15 10:50, Vijay Kilari wrote:
>> On Mon, Feb 16, 2015 at 3:58 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>> On 16/02/15 10:17, Vijay Kilari wrote:
>>>
>>> Hello Vijay,
>>>
>>>>   For ThunderX/arm64 this issue needs to be fixed.
>>>> Could you please comment on this?
>>>
>>> AFAICS, x86 is also using a 1G area for the vmap. Does it mean that x86
>>> never use vmap for more than 128M?
>>
>> I think for x86 there is no problem. It works beyond 128M
> 
> Did you test it? The code seems common enough to make the problem
> appears on x86.

So I've looked at the code and read twice the whole thread.

On a previous mail [1], Jan said that x86 doesn't set the present bit
when it's not required.

The first time that map_pages_to_xen is called to this region (in
vm_init), MAP_SMALL_PAGES is used. Looking to the x86 definition, the
present bit is not set.

On the second call (see vm_alloc), PAGE_HYPERVISOR is used. This define
will set the present bit.

Now about ARM... MAP_SMALL_PAGES is defined as PAGE_HYPERVISOR and the
page is mapped in any case.

So it looks like to me that the buggy code is not the vmap code but the
ARM Xen Page Table code.

IHMO, we have to introduce the concept of mapping region with
non-present bit on ARM. This would allow us to populate the table during
the initialization and allow to map a page (such as the bitmap) in
constant time later.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2014-09/msg04307.html

-- 
Julien Grall