kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Sistare <steven.sistare@oracle.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: kvm@vger.kernel.org, Cornelia Huck <cohuck@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>
Subject: Re: [PATCH V1 3/5] vfio: detect closed container
Date: Wed, 13 Jan 2021 14:44:26 -0500	[thread overview]
Message-ID: <c6393c85-0aed-8235-7cda-c9376df189bf@oracle.com> (raw)
In-Reply-To: <20210113122609.2bedad55@omen.home.shazbot.org>

On 1/13/2021 2:26 PM, Alex Williamson wrote:
> On Mon, 11 Jan 2021 16:12:18 -0500
> Steven Sistare <steven.sistare@oracle.com> wrote:
> 
>> 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.
> 
> Ok, that makes sense, the "contained" naming notsomuch.  You mention on
> 5/5 that you considered defining a new backend interface to call from
> the core, but considered it overkill.  I'm not sure what you're
> imagining that would be overkill.  We can pretty simply add an optional
> callback to struct vfio_iommu_drivers_ops that would allow the iommu
> driver to be notified when the container fd is released.  That seems
> quite simple and avoids the inverted calling structure presented here.
> Thanks,

OK.

Thanks very much for all your comments.  Off to write PATCH V2 ...

- Steve

  reply	other threads:[~2021-01-13 19:45 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-05 15:36 [PATCH V1 0/5] vfio virtual address update Steve Sistare
2021-01-05 15:36 ` [PATCH V1 1/5] vfio: maintain dma_list order Steve Sistare
2021-01-05 18:48   ` kernel test robot
2021-01-06  0:02   ` Alex Williamson
2021-01-06 14:50     ` Steven Sistare
2021-01-05 15:36 ` [PATCH V1 2/5] vfio: option to unmap all Steve Sistare
2021-01-08 19:35   ` Alex Williamson
2021-01-11 21:09     ` Steven Sistare
2021-01-13 19:41       ` Alex Williamson
2021-01-05 15:36 ` [PATCH V1 3/5] vfio: detect closed container Steve Sistare
2021-01-08 19:39   ` Alex Williamson
2021-01-11 21:12     ` Steven Sistare
2021-01-13 19:26       ` Alex Williamson
2021-01-13 19:44         ` Steven Sistare [this message]
2021-01-13 19:44         ` Alex Williamson
2021-01-05 15:36 ` [PATCH V1 4/5] vfio: VA suspend interface Steve Sistare
2021-01-08 21:15   ` Alex Williamson
2021-01-11 21:15     ` Steven Sistare
2021-01-12 15:47       ` Jason Zeng
2021-01-12 22:09         ` Alex Williamson
2021-01-13  4:37           ` Jason Zeng
2021-01-12 22:47       ` Alex Williamson
2021-01-13  4:10         ` Alex Williamson
2021-01-13 18:02           ` Steven Sistare
2021-01-13 18:34             ` Alex Williamson
2021-01-13 18:01         ` Steven Sistare
2021-01-19 20:11       ` Steven Sistare
2021-01-05 15:36 ` [PATCH V1 5/5] vfio: block during VA suspend Steve Sistare
2021-01-05 18:08   ` kernel test robot
2021-01-05 20:03   ` kernel test robot
2021-01-07 15:17     ` Steven Sistare
2021-01-05 20:03   ` [RFC PATCH] vfio: vfio_vaddr_valid() can be static kernel test robot
2021-01-08 21:32   ` [PATCH V1 5/5] vfio: block during VA suspend Alex Williamson
2021-01-11 21:16     ` Steven Sistare
2021-01-12 21:52 ` [PATCH V1 0/5] vfio virtual address update Steven Sistare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c6393c85-0aed-8235-7cda-c9376df189bf@oracle.com \
    --to=steven.sistare@oracle.com \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).