Re: [PATCH 0/4] xen: map foreign pages for shared rings by updating the PTEs directly

[David Vrabel <david.vrabel@citrix.com>]

On 30/09/11 09:08, Jan Beulich wrote:
>>>> On 29.09.11 at 18:45, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 29/09/11 17:07, Jan Beulich wrote:
>>>>>> On 29.09.11 at 17:53, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> [Resend as requested by Konrad.]
>>>>
>>>> This series of patches allows the vmalloc_sync_all() to be removed
>>>> from alloc_vm_area() by getting the hypervisor to update the PTEs (in
>>>> init_mm) directly rather than having the hypervisor look in the
>>>> current page tables to find the PTEs.
>>>>
>>>> Once the hypervisor has updated the PTEs, the normal mechanism of
>>>> syncing the page tables after a fault works as expected.
>>>
>>> Did you actually test that, and namely the case where alloc_vm_area()
>>> would result in a new top level page directory entry to get populated?
>>>
>>> I cannot see how this new entry would propagate into other mm-s, and
>>> hence I cannot see how you can do away with calling vmalloc_sync_all()
>>> just by changing how leaf page table entries get populated.
>>
>> I don't think this new behaviour of alloc_vm_area() is any different
>> from how a regular vmalloc() works.
> 
> Of course not. Callers of vmalloc() need to use vmalloc_sync_all() too
> before it is permitted to access the allocated space from an NMI
> handler or pass it into a hypercall.

The virtual addresses of the mapped rings are not accessed in an NMI
handler or a hypercall (before this patch set they were accessed in the
GNTTABOP_map_grant_ref hypercall, but no longer).

In the future, if something did want to pass the virtual address to a
hypercall it wouldn't need the vmalloc_sync() as it could disable
preemption, touch the page (so current->active_mm is updated), do the
hypercall, then disable preemption.

>> vmalloc_fault() copies the page table entries from init_mm into the
>> current MM (on 32-bit it calls vmalloc_sync_one() which makes it
>> obviously correct I think).
> 
> No, vmalloc_fault() copies pmd-s (32-bit) or pgd-s (64-bit), but never
> pte-s. Avoiding this to be done in a fault (precisely for the NMI and
> hypercall cases named above) is what vmalloc_sync_all() was
> introduced for (really, the hypercall aspect didn't matter back then,
> and alloc_vm_area() didn't exist outside of Xenolinux then either).
> 
> So eliminating it from alloc_vm_area() just pushes the need to call it
> to all callers that may have the obtained space accessed in NMI
> context (none at present, as only Xen code appears to call this
> function) or want to pass it to a hypercall without running on init_mm.
> 
> Just to repeat the essence of my first reply: Fiddling with how pte-s
> get populated can not possibly eliminate the need to call a function
> that populates top level page directory entries (pmd-s/pgd-s).
> 
> Jan
> 
>>>> This mechanism doesn't currently work on the ia64 port as that does
>>>> not support the GNTMAP_contains_pte flag.
>>>>
>>>> David