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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-09-30 at 11:50 +0100, Jan Beulich wrote:
> >>> On 30.09.11 at 12:14, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> > On 30/09/11 09:08, Jan Beulich wrote:
> >>>>> On 29.09.11 at 18:45, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> >>> On 29/09/11 17:07, Jan Beulich wrote:
> >>>>>>> On 29.09.11 at 17:53, David Vrabel <david.vrabel@xxxxxxxxxx> 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.
> 
> And you verified that no other hypercall gets, perhaps indirectly,
> passed alloc_vm_area()ed memory?

In the specific case of these functions, which map the rings, we believe
this is the case.

If there are other unrelated callsites then the failure to deal with
this is an existing bug at that callsite, before or after this series
(modulo the short term sticking plaster of putting the vmalloc_sync_all
back, which was known not to be acceptable longterm when it went in --
hence this series).

The only other use of alloc_vm_area is arch_gnttab_map_shared. IIRC the
hypervisor does not use the linear map to get a guests grant table but
those pages are owned by Xen and hence it has and uses its own mapping
of them.

Ian.

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel