> -----Original Message----- > From: Daniel Vetter <daniel@xxxxxxxx> > Sent: Friday, October 23, 2020 6:45 PM > To: Jason Gunthorpe <jgg@xxxxxxxx> > Cc: Xiong, Jianxin <jianxin.xiong@xxxxxxxxx>; linux-rdma <linux-rdma@xxxxxxxxxxxxxxx>; dri-devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; Leon > Romanovsky <leon@xxxxxxxxxx>; Doug Ledford <dledford@xxxxxxxxxx>; Vetter, Daniel <daniel.vetter@xxxxxxxxx>; Christian Koenig > <christian.koenig@xxxxxxx> > Subject: Re: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user memory region > > > > > + > > > > +#ifdef CONFIG_DMA_VIRT_OPS > > > > + if (device->dma_device->dma_ops == &dma_virt_ops) > > > > + return ERR_PTR(-EINVAL); #endif > > > > > > Maybe I'm confused, but should we have this check in dma_buf_attach, > > > or at least in dma_buf_dynamic_attach? The p2pdma functions use that > > > too, and I can't imagine how zerocopy should work (which is like the > > > entire point of > > > dma-buf) when we have dma_virt_ops. > > > > The problem is we have RDMA drivers that assume SGL's have a valid > > struct page, and these hacky/wrong P2P sgls that DMABUF creates cannot > > be passed into those drivers. > > > > But maybe this is just a 'drivers are using it wrong' if they call > > this function and expect struct pages.. > > > > The check in the p2p stuff was done to avoid this too, but it was on a > > different flow. > > Yeah definitely don't call dma_buf_map_attachment and expect a page back. In practice you'll get a page back fairly often, but I don't think > we want to bake that in, maybe we eventually get to non-hacky dma_addr_t only sgl. > > What I'm wondering is whether dma_buf_attach shouldn't reject such devices directly, instead of each importer having to do that. Come back here to see if consensus can be reached on who should do the check. My thinking is that it could be over restrictive for dma_buf_attach to always reject dma_virt_ops. According to dma-buf documentation the back storage would be moved to system area upon mapping unless p2p is requested and can be supported by the exporter. The sg_list for system memory would have struct page present. -Jianxin > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
