Re: [PATCH V1 4/5] vfio: VA suspend interface

Steven Sistare
2021-01-19T20:13:44Z

On 1/11/2021 4:15 PM, Steven Sistare wrote:
> On 1/8/2021 4:15 PM, Alex Williamson wrote:
>> On Tue, 5 Jan 2021 07:36:52 -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 host virtual address in the host process:
>>>
>>> - VFIO_DMA_UNMAP_FLAG_SUSPEND for VFIO_IOMMU_UNMAP_DMA
>>> - VFIO_DMA_MAP_FLAG_RESUME flag for VFIO_IOMMU_MAP_DMA
>>> - VFIO_SUSPEND extension for VFIO_CHECK_EXTENSION
>>
>> Suspend and Resume can imply many things other than what's done here.
>> Should these be something more akin to INVALIDATE_VADDR and
>> REPLACE_VADDR?
>
> Agreed. I suspected we would discuss the names. Some possibilities:
>
> INVALIDATE_VADDR REPLACE_VADDR
> INV_VADDR SET_VADDR
> CLEAR_VADDR SET_VADDR
> SUSPEND_VADDR RESUME_VADDR
>
>>> The suspend interface blocks vfio translation of host virtual
>>> addresses in a range, but DMA to already-mapped pages continues.
>>> The resume interface records the new base VA and resumes translation.
>>> See comments in uapi/linux/vfio.h for more details.
>>>
>>> This is a partial implementation. Blocking is added in the next patch.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>> drivers/vfio/vfio_iommu_type1.c | 47 +++++++++++++++++++++++++++++++++++------
>>> include/uapi/linux/vfio.h | 16 ++++++++++++++
>>> 2 files changed, 57 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>> index 3dc501d..2c164a6 100644
>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>> @@ -92,6 +92,7 @@ struct vfio_dma {
>>> int prot; /* IOMMU_READ/WRITE */
>>> bool iommu_mapped;
>>> bool lock_cap; /* capable(CAP_IPC_LOCK) */
>>> + bool suspended;
>>
>> Is there a value we could use for vfio_dma.vaddr that would always be
>> considered invalid, ex. ULONG_MAX?
>
> Yes, that could replace the suspend flag. That, plus changing the language from suspend
> to invalidate, will probably yield equally understandable code. I'll try it.

Hi Alex, I was not able to implement this suggestion in V2, because the old vaddr must be
be preserved throughout the loop in vfio_dma_do_unmap so that unwind can recover it. I
renamed the suspended flag to vaddr_valid.

- Steve