Re: [PATCH v3] xen/arm: Do not allocate pte entries for MAP_SMALL_PAGES

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

On 09/03/15 16:08, Vijay Kilari wrote:
> On Mon, Mar 9, 2015 at 5:46 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> Hi Vijay,
>>
>> Given the introduction of the new helper, the title looks wrong to me.
>>
>>
>> On 09/03/2015 08:59, vijay.kilari@gmail.com wrote:
>>>
>>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>>> index 7d4ba0c..e0be36b 100644
>>> --- a/xen/arch/arm/mm.c
>>> +++ b/xen/arch/arm/mm.c
>>> @@ -827,14 +827,15 @@ static int create_xen_table(lpae_t *entry)
>>>
>>>   enum xenmap_operation {
>>>       INSERT,
>>> -    REMOVE
>>> +    REMOVE,
>>> +    RESERVE
>>>   };
>>>
>>>   static int create_xen_entries(enum xenmap_operation op,
>>>                                 unsigned long virt,
>>>                                 unsigned long mfn,
>>>                                 unsigned long nr_mfns,
>>> -                              unsigned int ai)
>>> +                              unsigned int flags)
>>>   {
>>>       int rc;
>>>       unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>>> @@ -859,13 +860,17 @@ static int create_xen_entries(enum xenmap_operation
>>> op,
>>>
>>>           switch ( op ) {
>>>               case INSERT:
>>> +            case RESERVE:
>>>                   if ( third[third_table_offset(addr)].pt.valid )
>>>                   {
>>>                       printk("create_xen_entries: trying to replace an
>>> existing mapping addr=%lx mfn=%lx\n",
>>>                              addr, mfn);
>>>                       return -EINVAL;
>>>                   }
>>> -                pte = mfn_to_xen_entry(mfn, ai);
>>> +                if ( op == RESERVE || !is_pte_present(flags) )
>>
>>
>> As you have a new operation (only used by populate_pt_range), why do you
>> need to check is_pte_present?
> 
> map_pages_to_xen() can still take MAP_SMALL_PAGES as flags.
> In future if any common code requires  MAP_SMALL_PAGES then,
> this can be used.

The only usage was in vmap that you removed in this patch...

Furthermore, we decided to use to introduce populate_pt_range in order
to avoid using MAP_SMALL_PAGES on ARM...

It's pointless to keep to different way to population page table...

[..]

>>
>> And, therefore, MAP_SMALL_PAGES could be dropped.
> 
> MAP_SMALL_PAGES is still used in common code esp. EFI code.
> We can remove this provided if we clean up this. But I still think
> MAP_SMALL_PAGES is required to keep equivalent functionality of x86.

If you looked at the code you would have notice that the code is only
compiled for x86 and would never work for ARM (_PAGE_PAT, _PAGE_PWT...
doesn't exist).

Regards,

-- 
Julien Grall