From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH 0/4] xen: map foreign pages for shared rings by updating the PTEs directly Date: Fri, 30 Sep 2011 11:14:47 +0100 Message-ID: <4E859697.1050906@citrix.com> References: <1317311612-15220-1-git-send-email-david.vrabel@citrix.com> <4E84B3FE02000078000588A9@nat28.tlf.novell.com> <4E84A0C0.6000804@citrix.com> <4E85951B0200007800058AF8@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4E85951B0200007800058AF8@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" , KonradRzeszutek Wilk List-Id: xen-devel@lists.xenproject.org On 30/09/11 09:08, Jan Beulich wrote: >>>> On 29.09.11 at 18:45, David Vrabel wrote: >> On 29/09/11 17:07, Jan Beulich wrote: >>>>>> On 29.09.11 at 17:53, David Vrabel 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