Re: [PATCH V3 0/9] vfio virtual address update

[Alex Williamson <alex.williamson@redhat.com>]

On Fri, 29 Jan 2021 08:54:03 -0800
Steve Sistare <steven.sistare@oracle.com> wrote:

> Add interfaces that allow the underlying memory object of an iova range
> to be mapped to a new virtual address in the host process:
> 
>   - VFIO_DMA_UNMAP_FLAG_VADDR for VFIO_IOMMU_UNMAP_DMA
>   - VFIO_DMA_MAP_FLAG_VADDR flag for VFIO_IOMMU_MAP_DMA
>   - VFIO_UPDATE_VADDR for VFIO_CHECK_EXTENSION
>   - VFIO_DMA_UNMAP_FLAG_ALL for VFIO_IOMMU_UNMAP_DMA
>   - VFIO_UNMAP_ALL for VFIO_CHECK_EXTENSION
> 
> Unmap-vaddr invalidates the host virtual address in an iova range and blocks
> vfio translation of host virtual addresses, but DMA to already-mapped pages
> continues.  Map-vaddr updates the base VA and resumes translation.  The
> implementation supports iommu type1 and mediated devices.  Unmap-all allows
> all ranges to be unmapped or invalidated in a single ioctl, which simplifies
> userland code.
> 
> This functionality is necessary for live update, in which a host process
> such as qemu exec's an updated version of itself, while preserving its
> guest and vfio devices.  The process blocks vfio VA translation, exec's
> its new self, mmap's the memory object(s) underlying vfio object, updates
> the VA, and unblocks translation.  For a working example that uses these
> new interfaces, see the QEMU patch series "[PATCH V2] Live Update" at
> https://lore.kernel.org/qemu-devel/1609861330-129855-1-git-send-email-steven.sistare@oracle.com
> 
> Patches 1-3 define and implement the flag to unmap all ranges.
> Patches 4-6 define and implement the flags to update vaddr.
> Patches 7-9 add blocking to complete the implementation.

Hi Steve,

It looks pretty good to me, but I have some nit-picky comments that
I'll follow-up with on the individual patches.  However, I've made the
changes I suggest in a branch that you can find here:

git://github.com/awilliam/linux-vfio.git vaddr-v3

If the changes look ok, just send me an ack, I don't want to attribute
something to you that you don't approve of.  Thanks,

Alex
 
> Changes in V2:
>  - define a flag for unmap all instead of special range values
>  - define the VFIO_UNMAP_ALL extension
>  - forbid the combination of unmap-all and get-dirty-bitmap
>  - unwind in unmap on vaddr error
>  - add a new function to find first dma in a range instead of modifying
>    an existing function
>  - change names of update flags
>  - fix concurrency bugs due to iommu lock being dropped
>  - call down from from vfio to a new backend interface instead of up from
>    driver to detect container close
>  - use wait/wake instead of sleep and polling
>  - refine the uapi specification
>  - split patches into vfio vs type1
> 
> Changes in V3:
>  - add vaddr_invalid_count to fix pin_pages race with unmap
>  - refactor the wait helper functions
>  - traverse dma entries more efficiently in unmap
>  - check unmap flag conflicts more explicitly
>  - rename some local variables and functions
> 
> Steve Sistare (9):
>   vfio: option to unmap all
>   vfio/type1: unmap cleanup
>   vfio/type1: implement unmap all
>   vfio: interfaces to update vaddr
>   vfio/type1: massage unmap iteration
>   vfio/type1: implement interfaces to update vaddr
>   vfio: iommu driver notify callback
>   vfio/type1: implement notify callback
>   vfio/type1: block on invalid vaddr
> 
>  drivers/vfio/vfio.c             |   5 +
>  drivers/vfio/vfio_iommu_type1.c | 251 +++++++++++++++++++++++++++++++++++-----
>  include/linux/vfio.h            |   5 +
>  include/uapi/linux/vfio.h       |  27 +++++
>  4 files changed, 256 insertions(+), 32 deletions(-)
>