On Tue, Mar 10, 2015 at 5:22 PM, Ian Campbell <ian.campb...@citrix.com> wrote:
> On Tue, 2015-03-10 at 11:45 +0000, Julien Grall wrote:
>> On 09/03/15 16:08, Vijay Kilari wrote:
>> > On Mon, Mar 9, 2015 at 5:46 PM, Julien Grall <julien.gr...@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.kil...@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).
>
> Right, I think we should just remove MAP_SMALL_PAGES on ARM and if/when
> it turns out we need it we can add it back and implement it in the PT
> creation code.
>
> In any case the fix to make vmap_init use the new function should
> certainly be in a separate patch to anything which is fixing
> MAP_SMALL_PAGES.
My initial approach with RFC patch was right.
http://lists.xen.org/archives/html/xen-devel/2015-02/msg01919.html
So I will add populate_pt_range() and remove MAP_SMALL_PAGES.
Let me know your stance.
Regards
Vijay
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
