On Wed, Apr 01, 2020 at 04:04:14PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> >Daniel Vetter
> >Sent: Wednesday, April 1, 2020 7:35 AM
> >To: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> >Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >Subject: Re: [PATCH 1/6] dma-buf: add peer2peer flag
> >
> >On Mon, Mar 30, 2020 at 03:55:31PM +0200, Christian König wrote:
> >> Add a peer2peer flag noting that the importer can deal with device
> >> resources which are not backed by pages.
> >>
> >> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> >> ---
> >> drivers/dma-buf/dma-buf.c | 2 ++
> >> include/linux/dma-buf.h | 10 ++++++++++
> >> 2 files changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >> index ccc9eda1bc28..570c923023e6 100644
> >> --- a/drivers/dma-buf/dma-buf.c
> >> +++ b/drivers/dma-buf/dma-buf.c
> >> @@ -690,6 +690,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf,
> >struct device *dev,
> >>
> >> attach->dev = dev;
> >> attach->dmabuf = dmabuf;
> >> + if (importer_ops)
> >> + attach->peer2peer = importer_ops->allow_peer2peer;
> >
> >So an idea that crossed my mind to validate this, since we need quite some
> >bad amounts of bad luck if someone accidentally introduces and access to
> >struct_page in sg lists in some slowpath.
> >
> >On map_sg, if ->peer2peer is set, we could mangle the struct_page
> >pointers, e.g. swap high bits for low bits (so that NULL stays NULL). On
> >unmap_sg we obviously need to undo that, in case the exporter needs those
> >pointers for its own book-keeping for some reason. I was also pondering
> >just setting them all to NULL, but that might break some exporters. With
> >the pointer mangling trick (especially if we flip high for low bits on 64
> >where this should result in invalid addresses in almost all cases) we
> >should be able to catch buggy p2p importers quite quickly.
>
> The scatter list usage of the struct page pointer has other information in the
> lower bits for keeping track of linking and other stuff. Swizzling the page
> pointers will probably make the scatter list unusable.
We'd need to swizzle only the pointers that are actual struct page
pointers. Plus keep the low bits as-is, and maybe only flip the top-most
60 bits or so. Doesn't break the idea fundamentally I think.
-Daniel
>
> Mike
>
> >Thoughts? Maybe add as a follow-up patch for testing?
> >-Daniel
> >> attach->importer_ops = importer_ops;
> >> attach->importer_priv = importer_priv;
> >>
> >> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> >> index 1ade486fc2bb..82e0a4a64601 100644
> >> --- a/include/linux/dma-buf.h
> >> +++ b/include/linux/dma-buf.h
> >> @@ -334,6 +334,14 @@ struct dma_buf {
> >> * Attachment operations implemented by the importer.
> >> */
> >> struct dma_buf_attach_ops {
> >> + /**
> >> + * @allow_peer2peer:
> >> + *
> >> + * If this is set to true the importer must be able to handle peer
> >> + * resources without struct pages.
> >> + */
> >> + bool allow_peer2peer;
> >> +
> >> /**
> >> * @move_notify
> >> *
> >> @@ -362,6 +370,7 @@ struct dma_buf_attach_ops {
> >> * @node: list of dma_buf_attachment, protected by dma_resv lock of the
> >dmabuf.
> >> * @sgt: cached mapping.
> >> * @dir: direction of cached mapping.
> >> + * @peer2peer: true if the importer can handle peer resources without
> >pages.
> >> * @priv: exporter specific attachment data.
> >> * @importer_ops: importer operations for this attachment, if provided
> >> * dma_buf_map/unmap_attachment() must be called with the dma_resv
> >lock held.
> >> @@ -382,6 +391,7 @@ struct dma_buf_attachment {
> >> struct list_head node;
> >> struct sg_table *sgt;
> >> enum dma_data_direction dir;
> >> + bool peer2peer;
> >> const struct dma_buf_attach_ops *importer_ops;
> >> void *importer_priv;
> >> void *priv;
> >> --
> >> 2.17.1
> >>
> >
> >--
> >Daniel Vetter
> >Software Engineer, Intel Corporation
> >http://blog.ffwll.ch
> >_______________________________________________
> >dri-devel mailing list
> >dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
