kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Steven Sistare <steven.sistare@oracle.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 12:44:53 -0700	[thread overview]
Message-ID: <20210113124453.0ff1c756@omen.home.shazbot.org> (raw)
In-Reply-To: <20210113122609.2bedad55@omen.home.shazbot.org>

On Wed, 13 Jan 2021 12:26:09 -0700
Alex Williamson <alex.williamson@redhat.com> 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.

A callback into the iommu backend would also allow you to use a
waitqueue and wake_up for the blocking mechanism while vaddr is
invalid.  Seems like an improvement over the polling model.  Thanks,

Alex


  parent reply	other threads:[~2021-01-13 19:46 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
2021-01-13 19:44         ` Alex Williamson [this message]
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=20210113124453.0ff1c756@omen.home.shazbot.org \
    --to=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=steven.sistare@oracle.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).