Re: [PATCH V1 3/5] vfio: detect closed container

Steven Sistare
2021-01-11T21:13:22Z

On 1/8/2021 2:39 PM, Alex Williamson wrote:
> On Tue, 5 Jan 2021 07:36:51 -0800
> Steve Sistare <steven.sistare@oracle.com> wrote:
>
>> Add a function that detects if an iommu_group has a valid container.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>> drivers/vfio/vfio.c | 12 ++++++++++++
>> include/linux/vfio.h | 1 +
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index 262ab0e..f89ab80 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -61,6 +61,7 @@ struct vfio_container {
>> struct vfio_iommu_driver *iommu_driver;
>> void *iommu_data;
>> bool noiommu;
>> + bool closed;
>> };
>>
>> struct vfio_unbound_dev {
>> @@ -1223,6 +1224,7 @@ static int vfio_fops_release(struct inode *inode, struct file *filep)
>>
>> filep->private_data = NULL;
>>
>> + container->closed = true;
>> vfio_container_put(container);
>>
>> return 0;
>> @@ -2216,6 +2218,16 @@ void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
>> }
>> EXPORT_SYMBOL_GPL(vfio_group_set_kvm);
>>
>> +bool vfio_iommu_group_contained(struct iommu_group *iommu_group)
>> +{
>> + struct vfio_group *group = vfio_group_get_from_iommu(iommu_group);
>> + bool ret = group && group->container && !group->container->closed;
>> +
>> + vfio_group_put(group);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_iommu_group_contained);
>
> This seems like a pointless interface, the result is immediately stale.
> Anything that relies on the container staying open needs to add itself
> as a user. We already have some interfaces for that, ex.
> vfio_group_get_external_user(). Thanks,

The significant part here is container->closed, which is only set if userland closes the
container fd, which is a one-way trip -- the fd for this instance can never be opened
again. The container object may still have other references, and not be destroyed yet.
In patch 5, kernel code that waits for the RESUME ioctl calls this accessor to test if
the ioctl will never arrive.

- Steve

>> +
>> static int vfio_register_group_notifier(struct vfio_group *group,
>> unsigned long *events,
>> struct notifier_block *nb)
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index 38d3c6a..b2724e7 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -148,6 +148,7 @@ extern int vfio_unregister_notifier(struct device *dev,
>>
>> struct kvm;
>> extern void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
>> +extern bool vfio_iommu_group_contained(struct iommu_group *group);
>>
>> /*
>> * Sub-module helpers
>