Re: [RFC V1 0/4] iommufd live update

[Steven Sistare <steven.sistare@oracle.com>]

On 8/19/2024 10:59 AM, Jason Gunthorpe wrote:
> On Mon, Aug 12, 2024 at 01:41:46PM -0400, Steven Sistare wrote:
>> On 8/8/2024 3:52 PM, Jason Gunthorpe wrote:
>>> On Thu, Aug 08, 2024 at 03:15:02PM -0400, Steven Sistare wrote:
>>>> On 8/6/2024 8:56 AM, Jason Gunthorpe wrote:
>>>>> On Mon, Aug 05, 2024 at 03:03:30PM -0400, Steven Sistare wrote:
>>>>>> On 7/22/2024 11:55 AM, Jason Gunthorpe wrote:
>>>>>>> On Sat, Jul 20, 2024 at 11:56:40AM -0700, Steve Sistare wrote:
>>>>>>>> Live update is a technique wherein an application saves its state, launches
>>>>>>>> an updated version of itself, and restores its state.  Clients of the
>>>>>>>> application experience a brief suspension of service, on the order of
>>>>>>>> 100's of milliseconds, but are otherwise unaffected.
>>>>>>>>
>>>>>>>> Define the IOMMU_IOAS_CHANGE_PROCESS ioctl to allow management and use
>>>>>>>> of an iommufd device to be transferred from one process to another.  The
>>>>>>>> application is responsible for transferring the device descriptor to the new
>>>>>>>> process, eg either by preservation across fork and exec or via SCM_RIGHTS.
>>>>>>>
>>>>>>> It seems Ok to me, I'm glad it worked out for you
>>>>>>>
>>>>>>> But have you considered using something like the new
>>>>>>> memfd_pin_folios() system so that iommufd is bound to the FDs backing
>>>>>>> the memory instead of VMAs?
>>>>>>>
>>>>>>> https://lore.kernel.org/all/20240624063952.1572359-1-vivek.kasireddy@intel.com/
>>>>>>>
>>>>>>> I've been expecting to add support for that, but does it help this scenario?
>>>>>>
>>>>>> Thanks for the pointer, I had not seen it.
>>>>>> AFAICT it does not affect live update.  The memfd is passed to new qemu, and
>>>>>> the manner in which its pages were pinned does not matter, as long as the effect
>>>>>> on the mm fields that we manipulate is the same.
>>>>>
>>>>> I mean instead of using mmap's() and telling iommfd to take the pages
>>>>> from a VMA you'd use a memfd and tell iommufd to take the pages from
>>>>> the memfd directly.
>>>>>
>>>>> Since the memfd is not part of a process or mm_struct it is not
>>>>> effected by live update's exec() and none of these gyrations are
>>>>> necessary.
>>>>
>>>> The problem is that kernel clients (eg mdevs) use userland VA to identify
>>>> memory when calling iommufd, so we must update the VA's after exec.
>>>
>>> Technically no, they use IOVA too and iommufd translates IOVA into a
>>> VMA and what not.
>>>
>>> So if we teach iommufd how to do memfd it would also learn how to
>>> adapt it to mdevs as well.
>>>
>>>> vdpa does the same, if/when it converts to iommufd.  I cannot see us
>>>> changing vaddr to (file, offset) everywhere in iommufd and its clients,
>>>> up through the mdev code stack, can you?
>>>
>>> That is exactly what I imagine, because it isn't vaddr already, it is
>>> IOVA and IOVA always already translates to an area which gets you the
>>> vaddr.
>>>
>>> It is why this series can remap the vaddrs on the fly without reaching
>>> outside the area struct.
>>
>> OK, that looks tractable.  There are not too many instances of
>> struct iopt_pages uptr to fiddle with, adding support for
>> file+offset.  We must of course keep uptr to continue to support
>> anonymous memory for iommufd, but such memory will not be supported
>> for live update.
>>
>> Do you envision a new userland interface variant of IOMMU_IOAS_MAP
>> that takes fd and offset?
> 
> Yes
> 
>> Or have userland pass user_va as usual, but have the kernel check if it maps to a file,
>> and save the file?  The latter is more work in the kernel but requires no change in
>> applications.
> 
> Maybe this is possible too..
> 
>> Do you plan to work on this any time soon?  Do you want me to?
> 
> I wasn't at the point of this yet, if you are interested I suggest
> taking a stab. Now that the the infrastructure is in the mm it should
> mostly just be changing pin_user_pages() to the other one. It might be
> quite shore

I'm on the fence about taking a stab. It will not be short!

memfd_pin_folios() returns an array of folios which must be saved and
later passed back to unpin_folios().   That's a slight bummer, since
currently you save memory by retrieving PFNs from the iommu as needed,
rather than saving all of them explicitly (with the exception of pinned_pfns).

Also, the current map() call stack uses pfn_reader, which pins in small batches.
This offers no advantage for memfd since we must save all folios regardless.
So, we could short-circuit the call stack above the level where pfn_reader
is used.  Or, add folios to pfn_reader, and continue to pin in batches, and manage
the separate arrays of folios that result.  Or, add folios to pfn_reader, and pin
using a single giant batch.

Also, I must truncate the unmap() call stacks above where PFNs are used.

This is mostly FYI, but if you prefer one approach, let me know.

- Steve'