From: "Tian, Kevin" <kevin.tian@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>,
        Kirti Wankhede <kwankhede@nvidia.com>
CC: "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "kraxel@redhat.com" <kraxel@redhat.com>,
        "cjia@nvidia.com" <cjia@nvidia.com>,
        "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "Song, Jike" <jike.song@intel.com>,
        "bjsdjshi@linux.vnet.ibm.com" <bjsdjshi@linux.vnet.ibm.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v9 04/12] vfio iommu: Add support for mediated devices
Thread-Topic: [PATCH v9 04/12] vfio iommu: Add support for mediated devices
Thread-Index: AQHSKkwqKQB3u1PQIkaGlquXqmSvMKC6YvIQgAAExrA=
Date: Wed, 26 Oct 2016 07:54:56 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D18DFE3484@SHSMSX101.ccr.corp.intel.com>
References: <1476739332-4911-1-git-send-email-kwankhede@nvidia.com>
        <1476739332-4911-5-git-send-email-kwankhede@nvidia.com>
 <20161019150231.19e32b84@t450s.home> 
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2383
Lines: 54

> From: Tian, Kevin
> Sent: Wednesday, October 26, 2016 3:54 PM
> 
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Thursday, October 20, 2016 5:03 AM
> > > @@ -83,6 +92,21 @@ struct vfio_group {
> > >  };
> > >
> > >  /*
> > > + * Guest RAM pinning working set or DMA target
> > > + */
> > > +struct vfio_pfn {
> > > +	struct rb_node		node;
> > > +	unsigned long		vaddr;		/* virtual addr */
> > > +	dma_addr_t		iova;		/* IOVA */
> > > +	unsigned long		pfn;		/* Host pfn */
> > > +	int			prot;
> > > +	atomic_t		ref_count;
> > > +};
> >
> > Somehow we're going to need to fit an invalidation callback here too.
> > How would we handle a case where there are multiple mdev devices, from
> > different vendor drivers, that all have the same pfn pinned?  I'm
> > already concerned about the per pfn overhead we're introducing here so
> > clearly we cannot store an invalidation callback per pinned page, per
> > vendor driver.  Perhaps invalidations should be done using a notifier
> > chain per vfio_iommu, the vendor drivers are required to register on
> > that chain (fail pinning with empty notifier list) user unmapping
> > will be broadcast to the notifier chain, the vendor driver will be
> > responsible for deciding if each unmap is relevant to them (potentially
> > it's for a pinning from another driver).
> >
> > I expect we also need to enforce that vendors perform a synchronous
> > unmap such that after returning from the notifier list call, the
> > vfio_pfn should no longer exist.  If it does we might need to BUG_ON.
> > Also be careful to pay attention to the locking of the notifier vs
> > unpin callbacks to avoid deadlocks.
> >
> 
> What about just requesting vendor driver to provide a callback in parent
> device ops?
> 
> Curious in which scenario the user application (say Qemu here) may
> unmap memory pages which are still pinned by vendor driver... Is it
> purely about a corner case which we want to handle elegantly?
> 
> If yes, possibly a simpler way is to force destroying mdev instead of
> asking vendor driver to take care of each invalidation request under
> such situation. Since anyway the mdev device won't be in an usable
> state anymore... (sorry if I missed the key problem here.)
> 

or calling reset callback of parent device driver, if we don't want to
break libvirt's expectation by blindly removing mdev device...