From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>,
Alex Williamson <alex.williamson@redhat.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH v1 2/2] vfio/pci: Allow MMIO regions to be exported through dma-buf
Date: Wed, 8 May 2024 10:23:09 +0200 [thread overview]
Message-ID: <Zjs2bVVxBHEGUhF_@phenom.ffwll.local> (raw)
In-Reply-To: <20240508003153.GC4650@nvidia.com>
On Tue, May 07, 2024 at 09:31:53PM -0300, Jason Gunthorpe wrote:
> On Thu, May 02, 2024 at 07:50:36AM +0000, Kasireddy, Vivek wrote:
> > Hi Jason,
> >
> > >
> > > On Tue, Apr 30, 2024 at 04:24:50PM -0600, Alex Williamson wrote:
> > > > > +static vm_fault_t vfio_pci_dma_buf_fault(struct vm_fault *vmf)
> > > > > +{
> > > > > + struct vm_area_struct *vma = vmf->vma;
> > > > > + struct vfio_pci_dma_buf *priv = vma->vm_private_data;
> > > > > + pgoff_t pgoff = vmf->pgoff;
> > > > > +
> > > > > + if (pgoff >= priv->nr_pages)
> > > > > + return VM_FAULT_SIGBUS;
> > > > > +
> > > > > + return vmf_insert_pfn(vma, vmf->address,
> > > > > + page_to_pfn(priv->pages[pgoff]));
> > > > > +}
> > > >
> > > > How does this prevent the MMIO space from being mmap'd when disabled
> > > at
> > > > the device? How is the mmap revoked when the MMIO becomes disabled?
> > > > Is it part of the move protocol?
> > In this case, I think the importers that mmap'd the dmabuf need to be tracked
> > separately and their VMA PTEs need to be zapped when MMIO access is revoked.
>
> Which, as we know, is quite hard.
>
> > > Yes, we should not have a mmap handler for dmabuf. vfio memory must be
> > > mmapped in the normal way.
> > Although optional, I think most dmabuf exporters (drm ones) provide a mmap
> > handler. Otherwise, there is no easy way to provide CPU access (backup slow path)
> > to the dmabuf for the importer.
>
> Here we should not, there is no reason since VFIO already provides a
> mmap mechanism itself. Anything using this API should just call the
> native VFIO function instead of trying to mmap the DMABUF. Yes, it
> will be inconvient for the scatterlist case you have, but the kernel
> side implementation is much easier ..
Just wanted to confirm that it's entirely legit to not implement dma-buf
mmap. Same for the in-kernel vmap functions. Especially for really funny
buffers like these it's just not a good idea, and the dma-buf interfaces
are intentionally "everything is optional".
Similarly you can (and should) reject and dma_buf_attach to devices where
p2p connectevity isn't there, or well really for any other reason that
makes stuff complicated and is out of scope for your use-case. It's better
to reject strictly and than accidentally support something really horrible
(we've been there).
The only real rule with all the interfaces is that when attach() worked,
then map must too (except when you're in OOM). Because at least for some
drivers/subsystems, that's how userspace figures out whether a buffer can
be shared.
-Sima
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
prev parent reply other threads:[~2024-05-08 8:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 6:30 [PATCH v1 0/2] vfio/pci: Allow MMIO regions to be exported through dma-buf Vivek Kasireddy
2024-04-22 6:30 ` [PATCH v1 1/2] vfio: Export vfio device get and put registration helpers Vivek Kasireddy
2024-04-22 6:30 ` [PATCH v1 2/2] vfio/pci: Allow MMIO regions to be exported through dma-buf Vivek Kasireddy
2024-04-22 14:44 ` Zhu Yanjun
2024-04-30 22:24 ` Alex Williamson
2024-05-01 12:53 ` Jason Gunthorpe
2024-05-02 7:50 ` Kasireddy, Vivek
2024-05-02 12:57 ` Leon Romanovsky
2024-05-08 0:31 ` Jason Gunthorpe
2024-05-08 8:23 ` Daniel Vetter [this message]
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=Zjs2bVVxBHEGUhF_@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=alex.williamson@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@infradead.org \
--cc=jgg@nvidia.com \
--cc=kvm@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=vivek.kasireddy@intel.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 read-only IMAP folder(s) and NNTP newsgroup(s).