On Wed, Jan 25, 2017 at 06:11:37PM +0100, Paolo Bonzini wrote:
[...]
> > The comment from v4 still needs input from Paolo, is it valid to make
> > use of vaddr (based on address_space_translate ->
> > memory_region_get_ram_ptr) outside of the rcu read lock or could future
> > BQL reduction efforts allow this to race?
>
> You need to keep a reference to the MemoryRegion if you do
> rcu_read_unlock. But it's simpler to call vfio_get_vaddr within
> rcu_read_lock, and keep the lock/unlock in vfio_iommu_map_notify.
>
> You probably should also put a comment about why VFIO does *not* need to
> keep a reference between vfio_dma_map and vfio_dma_unmap (which doesn't
> sound easy to do either).
How about this one?
/*
* Here, we need to have the lock not only for vfio_get_vaddr(),
* but also needs to make sure that the vaddr will be valid for
* further operations.
*
* When we map new pages, we need the lock to make sure that vaddr
* is valid along the way we build up the IO page table (via
* vfio_dma_map()). Then, as long as the mapping is set up, we can
* unlock since those pages will be pinned in kernel (which makes
* sure that the RAM backend of vaddr will always be there, even
* if the memory object is destroyed and RAM released).
*
* For unmapping case, we don't really need the protection since
* the pages are in all cases locked in kernel, so we'll probably
* be safe even without the lock. However, it won't hurt we have
* the lock as well here.
*/
Thanks,
-- peterx
