Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Thu, Jun 24, 2021 at 11:52:47AM +0200, Christian König wrote: > I've already converted a bunch of the GPU drivers, but there are at least 6 > GPU still needing to be fixed and on top of that comes VA-API and a few > others. > > What are your plans for the DMA mapping subsystem? Building a new API that allows batched DMA mapping without the scatterlist. The main input for my use case would be bio_vecs, but I plan to make it a little flexible, and the output would be a list of [dma_addr,len] tuples, with the API being flexible enough to just return a single [dma_addr,len] for the common IOMMU coalescing case. > >> Btw, one thing I noticed when looking over the dma-buf instances is that >> there is a lot of duplicated code for creating a sg_table from pages, >> and then mapping it. It would be good if we could move toward common >> helpers instead of duplicating that all over again. > > Can you give an example? Take a look at the get_sg_table and put_sg_table helpers in udmabuf. Those would also be useful in armda, i915, tegra, gntdev-dmabuf, mbochs in one form or another. Similar for variants that use a contigous regions. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Thu, Jun 24, 2021 at 10:07:14AM +0200, Christian König wrote: > The key point is that accessing the underlying pages even when DMA-bufs are > backed by system memory is illegal. Daniel even created a patch which > mangles the page pointers in sg_tables used by DMA-buf to make sure that > people don't try to use them. Which is another goddamn layering violation of a subsystem that has no business at all poking into the scatterlist structure, yes. > So the conclusion is that using sg_table in the DMA-buf framework was just > the wrong data structure and we should have invented a new one. I think so. > But then people would have complained that we have a duplicated > infrastructure (which is essentially true). I doubt it. At least if you had actually talked to the relevant people. Which seems to be a major issue with what is going on GPU land. > My best plan to get out of this mess is that we change the DMA-buf > interface to use an array of dma_addresses instead of the sg_table object > and I have already been working on this actively the last few month. Awesome! I have a bit of related work on the DMA mapping subsystems, so let's sync up as soon as you have some first sketches. Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 24.06.21 um 10:12 schrieb Christoph Hellwig: On Thu, Jun 24, 2021 at 10:07:14AM +0200, Christian König wrote: The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them. Which is another goddamn layering violation of a subsystem that has no business at all poking into the scatterlist structure, yes. Completely agree, but it is also the easiest way to get away from the scatterlist as trasnport vehicle for the dma_addresses. [SNIP] My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month. Awesome! I have a bit of related work on the DMA mapping subsystems, so let's sync up as soon as you have some first sketches. Don't start cheering to fast. I've already converted a bunch of the GPU drivers, but there are at least 6 GPU still needing to be fixed and on top of that comes VA-API and a few others. What are your plans for the DMA mapping subsystem? Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again. Can you give an example? Thanks, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 24.06.21 um 07:34 schrieb Christoph Hellwig: On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: I understand the argument and I agree that for the generic case, the top of the stack can't assume anything. Having said that, in this case the SGL is encapsulated inside a dma-buf object. But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable. The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them. So the conclusion is that using sg_table in the DMA-buf framework was just the wrong data structure and we should have invented a new one. But then people would have complained that we have a duplicated infrastructure (which is essentially true). My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month. Regards, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:39:48PM +0300, Oded Gabbay wrote: > On Wed, Jun 23, 2021 at 10:34 PM Jason Gunthorpe wrote: > > > > On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > > > On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > > > > > > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > > > > > > > Can you please explain why it is so important to (allow) access them > > > > > through the CPU ? > > > > > > > > It is not so much important, as it reflects significant design choices > > > > that are already tightly baked into alot of our stacks. > > > > > > > > A SGL is CPU accessible by design - that is baked into this thing and > > > > places all over the place assume it. Even in RDMA we have > > > > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > > > > to see) > > > > > > > > So, the thing at the top of the stack - in this case the gaudi driver > > > > - simply can't assume what the rest of the stack is going to do and > > > > omit the CPU side. It breaks everything. > > > > > > > > Logan's patch series is the most fully developed way out of this > > > > predicament so far. > > > > > > I understand the argument and I agree that for the generic case, the > > > top of the stack can't assume anything. > > > Having said that, in this case the SGL is encapsulated inside a dma-buf > > > object. > > > > > > Maybe its a stupid/over-simplified suggestion, but can't we add a > > > property to the dma-buf object, > > > that will be set by the exporter, which will "tell" the importer it > > > can't use any CPU fallback ? Only "real" p2p ? > > > > The block stack has been trying to do something like this. > > > > The flag doesn't solve the DMA API/IOMMU problems though. > hmm, I thought using dma_map_resource will solve the IOMMU issues, > no ? dma_map_resource() will configure the IOMMU but it is not the correct API to use when building a SG list for DMA, that would be dma_map_sg or sgtable. So it works, but it is an API abuse to build things this way. > If I use dma_map_resource to set the addresses inside the SGL before I > export the dma-buf, and guarantee no one will use the SGL in the > dma-buf for any other purpose than device p2p, what else is needed ? You still have to check the p2p stuff to ensure that p2p is even possible And this approach is misusing all the APIs and has been NAK'd by Christoph, so up to Greg if he wants to take it or insist you work with Logan to get the proper generlized solution finished. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:39:48PM +0300, Oded Gabbay wrote: > hmm, I thought using dma_map_resource will solve the IOMMU issues, no ? > We talked about it yesterday, and you said that it will "work" > (although I noticed a tone of reluctance when you said that). > > If I use dma_map_resource to set the addresses inside the SGL before I > export the dma-buf, and guarantee no one will use the SGL in the > dma-buf for any other purpose than device p2p, what else is needed ? dma_map_resource works in the sense of that helps with mapping an arbitrary phys_addr_t for DMA. It does not take various pitfalls of PCI P2P into account, such as the offset between the CPU physical address and the PCIe bus address, or the whole support of mapping between two devices behding a switch and not going through the limited root port support. Comparing dma_direct_map_resource/iommu_dma_map_resource with with pci_p2pdma_map_sg_attrs/__pci_p2pdma_map_sg should make that very clear. So if you want a non-page based mapping you need a "resource"-level version of pci_p2pdma_map_sg_attrs. Which totall doable, and in fact mostly trivial. But no one has even looked into providing one and just keeps arguing. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > I understand the argument and I agree that for the generic case, the > top of the stack can't assume anything. > Having said that, in this case the SGL is encapsulated inside a dma-buf > object. But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > > > Can you please explain why it is so important to (allow) access them > > > through the CPU ? > > > > It is not so much important, as it reflects significant design choices > > that are already tightly baked into alot of our stacks. > > > > A SGL is CPU accessible by design - that is baked into this thing and > > places all over the place assume it. Even in RDMA we have > > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > > to see) > > > > So, the thing at the top of the stack - in this case the gaudi driver > > - simply can't assume what the rest of the stack is going to do and > > omit the CPU side. It breaks everything. > > > > Logan's patch series is the most fully developed way out of this > > predicament so far. > > I understand the argument and I agree that for the generic case, the > top of the stack can't assume anything. > Having said that, in this case the SGL is encapsulated inside a dma-buf > object. > > Maybe its a stupid/over-simplified suggestion, but can't we add a > property to the dma-buf object, > that will be set by the exporter, which will "tell" the importer it > can't use any CPU fallback ? Only "real" p2p ? The block stack has been trying to do something like this. The flag doesn't solve the DMA API/IOMMU problems though. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:57:35AM +0200, Christian König wrote: > > > No it isn't. It makes devices depend on allocating struct pages for their > > > BARs which is not necessary nor desired. > > Which dramatically reduces the cost of establishing DMA mappings, a > > loop of dma_map_resource() is very expensive. > > Yeah, but that is perfectly ok. Our BAR allocations are either in chunks of > at least 2MiB or only a single 4KiB page. And very small apparently > > > Allocating a struct pages has their use case, for example for exposing > > > VRAM > > > as memory for HMM. But that is something very specific and should not > > > limit > > > PCIe P2P DMA in general. > > Sure, but that is an ideal we are far from obtaining, and nobody wants > > to work on it prefering to do hacky hacky like this. > > > > If you believe in this then remove the scatter list from dmabuf, add a > > new set of dma_map* APIs to work on physical addresses and all the > > other stuff needed. > > Yeah, that's what I totally agree on. And I actually hoped that the new P2P > work for PCIe would go into that direction, but that didn't materialized. It is a lot of work and the only gain is to save a bit of memory for struct pages. Not a very big pay off. > But allocating struct pages for PCIe BARs which are essentially registers > and not memory is much more hacky than the dma_resource_map() approach. It doesn't really matter. The pages are in a special zone and are only being used as handles for the BAR memory. > By using PCIe P2P we want to avoid the round trip to the CPU when one device > has filled the ring buffer and another device must be woken up to process > it. Sure, we all have these scenarios, what is inside the memory doesn't realy matter. The mechanism is generic and the struct pages don't care much if they point at something memory-like or at something register-like. They are already in big trouble because you can't portably use CPU instructions to access them anyhow. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > Can you please explain why it is so important to (allow) access them > through the CPU ? It is not so much important, as it reflects significant design choices that are already tightly baked into alot of our stacks. A SGL is CPU accessible by design - that is baked into this thing and places all over the place assume it. Even in RDMA we have RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page to see) So, the thing at the top of the stack - in this case the gaudi driver - simply can't assume what the rest of the stack is going to do and omit the CPU side. It breaks everything. Logan's patch series is the most fully developed way out of this predicament so far. > The whole purpose is that the other device accesses my device, > bypassing the CPU. Sure, but you don't know that will happen, or if it is even possible in any given system configuration. The purpose is to allow for that optimization when possible, not exclude CPU based approaches. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:34 PM Jason Gunthorpe wrote: > > On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > > On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > > > > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > > > > > Can you please explain why it is so important to (allow) access them > > > > through the CPU ? > > > > > > It is not so much important, as it reflects significant design choices > > > that are already tightly baked into alot of our stacks. > > > > > > A SGL is CPU accessible by design - that is baked into this thing and > > > places all over the place assume it. Even in RDMA we have > > > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > > > to see) > > > > > > So, the thing at the top of the stack - in this case the gaudi driver > > > - simply can't assume what the rest of the stack is going to do and > > > omit the CPU side. It breaks everything. > > > > > > Logan's patch series is the most fully developed way out of this > > > predicament so far. > > > > I understand the argument and I agree that for the generic case, the > > top of the stack can't assume anything. > > Having said that, in this case the SGL is encapsulated inside a dma-buf > > object. > > > > Maybe its a stupid/over-simplified suggestion, but can't we add a > > property to the dma-buf object, > > that will be set by the exporter, which will "tell" the importer it > > can't use any CPU fallback ? Only "real" p2p ? > > The block stack has been trying to do something like this. > > The flag doesn't solve the DMA API/IOMMU problems though. hmm, I thought using dma_map_resource will solve the IOMMU issues, no ? We talked about it yesterday, and you said that it will "work" (although I noticed a tone of reluctance when you said that). If I use dma_map_resource to set the addresses inside the SGL before I export the dma-buf, and guarantee no one will use the SGL in the dma-buf for any other purpose than device p2p, what else is needed ? Oded > > Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > Can you please explain why it is so important to (allow) access them > > through the CPU ? > > It is not so much important, as it reflects significant design choices > that are already tightly baked into alot of our stacks. > > A SGL is CPU accessible by design - that is baked into this thing and > places all over the place assume it. Even in RDMA we have > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > to see) > > So, the thing at the top of the stack - in this case the gaudi driver > - simply can't assume what the rest of the stack is going to do and > omit the CPU side. It breaks everything. > > Logan's patch series is the most fully developed way out of this > predicament so far. I understand the argument and I agree that for the generic case, the top of the stack can't assume anything. Having said that, in this case the SGL is encapsulated inside a dma-buf object. Maybe its a stupid/over-simplified suggestion, but can't we add a property to the dma-buf object, that will be set by the exporter, which will "tell" the importer it can't use any CPU fallback ? Only "real" p2p ? Won't that solve the problem by eliminating the unsupported access methods ? Oded > > > The whole purpose is that the other device accesses my device, > > bypassing the CPU. > > Sure, but you don't know that will happen, or if it is even possible > in any given system configuration. The purpose is to allow for that > optimization when possible, not exclude CPU based approaches. > > Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 9:24 PM Jason Gunthorpe wrote: > > On Wed, Jun 23, 2021 at 10:57:35AM +0200, Christian König wrote: > > > > > No it isn't. It makes devices depend on allocating struct pages for > > > > their > > > > BARs which is not necessary nor desired. > > > Which dramatically reduces the cost of establishing DMA mappings, a > > > loop of dma_map_resource() is very expensive. > > > > Yeah, but that is perfectly ok. Our BAR allocations are either in chunks of > > at least 2MiB or only a single 4KiB page. > > And very small apparently > > > > > Allocating a struct pages has their use case, for example for exposing > > > > VRAM > > > > as memory for HMM. But that is something very specific and should not > > > > limit > > > > PCIe P2P DMA in general. > > > Sure, but that is an ideal we are far from obtaining, and nobody wants > > > to work on it prefering to do hacky hacky like this. > > > > > > If you believe in this then remove the scatter list from dmabuf, add a > > > new set of dma_map* APIs to work on physical addresses and all the > > > other stuff needed. > > > > Yeah, that's what I totally agree on. And I actually hoped that the new P2P > > work for PCIe would go into that direction, but that didn't materialized. > > It is a lot of work and the only gain is to save a bit of memory for > struct pages. Not a very big pay off. > > > But allocating struct pages for PCIe BARs which are essentially registers > > and not memory is much more hacky than the dma_resource_map() approach. > > It doesn't really matter. The pages are in a special zone and are only > being used as handles for the BAR memory. > > > By using PCIe P2P we want to avoid the round trip to the CPU when one device > > has filled the ring buffer and another device must be woken up to process > > it. > > Sure, we all have these scenarios, what is inside the memory doesn't > realy matter. The mechanism is generic and the struct pages don't care > much if they point at something memory-like or at something > register-like. > > They are already in big trouble because you can't portably use CPU > instructions to access them anyhow. > > Jason Jason, Can you please explain why it is so important to (allow) access them through the CPU ? In regard to p2p, where is the use-case for that ? The whole purpose is that the other device accesses my device, bypassing the CPU. Thanks, Oded ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 11:57 AM Christian König wrote: > > Am 22.06.21 um 18:05 schrieb Jason Gunthorpe: > > On Tue, Jun 22, 2021 at 05:48:10PM +0200, Christian König wrote: > >> Am 22.06.21 um 17:40 schrieb Jason Gunthorpe: > >>> On Tue, Jun 22, 2021 at 05:29:01PM +0200, Christian König wrote: > [SNIP] > No absolutely not. NVidia GPUs work exactly the same way. > > And you have tons of similar cases in embedded and SoC systems where > intermediate memory between devices isn't directly addressable with the > CPU. > >>> None of that is PCI P2P. > >>> > >>> It is all some specialty direct transfer. > >>> > >>> You can't reasonably call dma_map_resource() on non CPU mapped memory > >>> for instance, what address would you pass? > >>> > >>> Do not confuse "I am doing transfers between two HW blocks" with PCI > >>> Peer to Peer DMA transfers - the latter is a very narrow subcase. > >>> > No, just using the dma_map_resource() interface. > >>> Ik, but yes that does "work". Logan's series is better. > >> No it isn't. It makes devices depend on allocating struct pages for their > >> BARs which is not necessary nor desired. > > Which dramatically reduces the cost of establishing DMA mappings, a > > loop of dma_map_resource() is very expensive. > > Yeah, but that is perfectly ok. Our BAR allocations are either in chunks > of at least 2MiB or only a single 4KiB page. > > Oded might run into more performance problems, but those DMA-buf > mappings are usually set up only once. > > >> How do you prevent direct I/O on those pages for example? > > GUP fails. > > At least that is calming. > > >> Allocating a struct pages has their use case, for example for exposing VRAM > >> as memory for HMM. But that is something very specific and should not limit > >> PCIe P2P DMA in general. > > Sure, but that is an ideal we are far from obtaining, and nobody wants > > to work on it prefering to do hacky hacky like this. > > > > If you believe in this then remove the scatter list from dmabuf, add a > > new set of dma_map* APIs to work on physical addresses and all the > > other stuff needed. > > Yeah, that's what I totally agree on. And I actually hoped that the new > P2P work for PCIe would go into that direction, but that didn't > materialized. > > But allocating struct pages for PCIe BARs which are essentially > registers and not memory is much more hacky than the dma_resource_map() > approach. > > To re-iterate why I think that having struct pages for those BARs is a > bad idea: Our doorbells on AMD GPUs are write and read pointers for ring > buffers. > > When you write to the BAR you essentially tell the firmware that you > have either filled the ring buffer or read a bunch of it. This in turn > then triggers an interrupt in the hardware/firmware which was eventually > asleep. > > By using PCIe P2P we want to avoid the round trip to the CPU when one > device has filled the ring buffer and another device must be woken up to > process it. > > Think of it as MSI-X in reverse and allocating struct pages for those > BARs just to work around the shortcomings of the DMA API makes no sense > at all to me. We would also like to do that *in the future*. In Gaudi it will never be supported (due to security limitations) but I definitely see it happening in future ASICs. Oded > > > We also do have the VRAM BAR, and for HMM we do allocate struct pages > for the address range exposed there. But this is a different use case. > > Regards, > Christian. > > > > > Otherwise, we have what we have and drivers don't get to opt out. This > > is why the stuff in AMDGPU was NAK'd. > > > > Jason > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 18:05 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 05:48:10PM +0200, Christian König wrote: Am 22.06.21 um 17:40 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 05:29:01PM +0200, Christian König wrote: [SNIP] No absolutely not. NVidia GPUs work exactly the same way. And you have tons of similar cases in embedded and SoC systems where intermediate memory between devices isn't directly addressable with the CPU. None of that is PCI P2P. It is all some specialty direct transfer. You can't reasonably call dma_map_resource() on non CPU mapped memory for instance, what address would you pass? Do not confuse "I am doing transfers between two HW blocks" with PCI Peer to Peer DMA transfers - the latter is a very narrow subcase. No, just using the dma_map_resource() interface. Ik, but yes that does "work". Logan's series is better. No it isn't. It makes devices depend on allocating struct pages for their BARs which is not necessary nor desired. Which dramatically reduces the cost of establishing DMA mappings, a loop of dma_map_resource() is very expensive. Yeah, but that is perfectly ok. Our BAR allocations are either in chunks of at least 2MiB or only a single 4KiB page. Oded might run into more performance problems, but those DMA-buf mappings are usually set up only once. How do you prevent direct I/O on those pages for example? GUP fails. At least that is calming. Allocating a struct pages has their use case, for example for exposing VRAM as memory for HMM. But that is something very specific and should not limit PCIe P2P DMA in general. Sure, but that is an ideal we are far from obtaining, and nobody wants to work on it prefering to do hacky hacky like this. If you believe in this then remove the scatter list from dmabuf, add a new set of dma_map* APIs to work on physical addresses and all the other stuff needed. Yeah, that's what I totally agree on. And I actually hoped that the new P2P work for PCIe would go into that direction, but that didn't materialized. But allocating struct pages for PCIe BARs which are essentially registers and not memory is much more hacky than the dma_resource_map() approach. To re-iterate why I think that having struct pages for those BARs is a bad idea: Our doorbells on AMD GPUs are write and read pointers for ring buffers. When you write to the BAR you essentially tell the firmware that you have either filled the ring buffer or read a bunch of it. This in turn then triggers an interrupt in the hardware/firmware which was eventually asleep. By using PCIe P2P we want to avoid the round trip to the CPU when one device has filled the ring buffer and another device must be woken up to process it. Think of it as MSI-X in reverse and allocating struct pages for those BARs just to work around the shortcomings of the DMA API makes no sense at all to me. We also do have the VRAM BAR, and for HMM we do allocate struct pages for the address range exposed there. But this is a different use case. Regards, Christian. Otherwise, we have what we have and drivers don't get to opt out. This is why the stuff in AMDGPU was NAK'd. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 2021-06-22 um 11:29 a.m. schrieb Christian König: > Am 22.06.21 um 17:23 schrieb Jason Gunthorpe: >> On Tue, Jun 22, 2021 at 02:23:03PM +0200, Christian König wrote: >>> Am 22.06.21 um 14:01 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > On Tue, Jun 22, 2021 at 9:37 AM Christian König > wrote: >> Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: >>> On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: >>> Another thing I want to emphasize is that we are doing p2p only through the export/import of the FD. We do *not* allow the user to mmap the dma-buf as we do not support direct IO. So there is no access to these pages through the userspace. >>> Arguably mmaping the memory is a better choice, and is the >>> direction >>> that Logan's series goes in. Here the use of DMABUF was >>> specifically >>> designed to allow hitless revokation of the memory, which this >>> isn't >>> even using. >> The major problem with this approach is that DMA-buf is also used >> for >> memory which isn't CPU accessible. That isn't an issue here because the memory is only intended to be used with P2P transfers so it must be CPU accessible. >>> No, especially P2P is often done on memory resources which are not even >>> remotely CPU accessible. >> That is a special AMD thing, P2P here is PCI P2P and all PCI memory is >> CPU accessible. > > No absolutely not. NVidia GPUs work exactly the same way. > > And you have tons of similar cases in embedded and SoC systems where > intermediate memory between devices isn't directly addressable with > the CPU. > >>> So you are taking the hit of very limited hardware support and >>> reduced >>> performance just to squeeze into DMABUF.. You still have the issue that this patch is doing all of this P2P stuff wrong - following the already NAK'd AMD approach. >>> Well that stuff was NAKed because we still use sg_tables, not >>> because we >>> don't want to allocate struct pages. >> sg lists in general. >> >>> The plan is to push this forward since DEVICE_PRIVATE clearly can't >>> handle >>> all of our use cases and is not really a good fit to be honest. >>> >>> IOMMU is now working as well, so as far as I can see we are all good >>> here. >> How? Is that more AMD special stuff? > > No, just using the dma_map_resource() interface. > > We have that working on tons of IOMMU enabled systems. > >> This patch series never calls to the iommu driver, AFAICT. >> > I'll go and read Logan's patch-set to see if that will work for us in > the future. Please remember, as Daniel said, we don't have struct > page > backing our device memory, so if that is a requirement to connect to > Logan's work, then I don't think we will want to do it at this point. It is trivial to get the struct page for a PCI BAR. >>> Yeah, but it doesn't make much sense. Why should we create a struct >>> page for >>> something that isn't even memory in a lot of cases? >> Because the iommu and other places need this handle to setup their >> stuff. Nobody has yet been brave enough to try to change those flows >> to be able to use a physical CPU address. > > Well that is certainly not true. I'm just not sure if that works with > all IOMMU drivers thought. > > Would need to ping Felix when the support for this was merged. We have been working on IOMMU support for all our multi-GPU memory mappings in KFD. The PCIe P2P side of this is currently only merged on our internal branch. Before we can actually use this, we need CONFIG_DMABUF_MOVE_NOTIFY enabled (which is still documented as experimental and disabled by default). Otherwise we'll end up pinning all our VRAM. I think we'll try to put together an upstream patch series of all our PCIe P2P support in a few weeks or so. This will include IOMMU mappings, checking that PCIe P2P is actually possible between two devices, and KFD topology updates to correctly report those capabilities to user mode. It will not use struct pages for exported VRAM buffers. Regards, Felix > > Regards, > Christian. > >> >> This is why we have a special struct page type just for PCI BAR >> memory. >> >> Jason > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 17:40 schrieb Oded Gabbay: On Tue, Jun 22, 2021 at 6:31 PM Christian König wrote: Am 22.06.21 um 17:28 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 05:24:08PM +0200, Christian König wrote: I will take two GAUDI devices and use one as an exporter and one as an importer. I want to see that the solution works end-to-end, with real device DMA from importer to exporter. I can tell you it doesn't. Stuffing physical addresses directly into the sg list doesn't involve any of the IOMMU code so any configuration that requires IOMMU page table setup will not work. Sure it does. See amdgpu_vram_mgr_alloc_sgt: amdgpu_res_first(res, offset, length, ); ^^ I'm not talking about the AMD driver, I'm talking about this patch. + bar_address = hdev->dram_pci_bar_start + + (pages[cur_page] - prop->dram_base_address); + sg_dma_address(sg) = bar_address; Yeah, that is indeed not working. Oded you need to use dma_map_resource() for this. Christian. Yes, of course. But will it be enough ? Jason said that supporting IOMMU isn't nice when we don't have struct pages. I fail to understand the connection, I need to dig into this. Question is what you want to do with this? A struct page is always needed if you want to do stuff like HMM with it, if you only want P2P between device I actually recommend to avoid it. Christian. Oded Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 17:40 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 05:29:01PM +0200, Christian König wrote: [SNIP] No absolutely not. NVidia GPUs work exactly the same way. And you have tons of similar cases in embedded and SoC systems where intermediate memory between devices isn't directly addressable with the CPU. None of that is PCI P2P. It is all some specialty direct transfer. You can't reasonably call dma_map_resource() on non CPU mapped memory for instance, what address would you pass? Do not confuse "I am doing transfers between two HW blocks" with PCI Peer to Peer DMA transfers - the latter is a very narrow subcase. No, just using the dma_map_resource() interface. Ik, but yes that does "work". Logan's series is better. No it isn't. It makes devices depend on allocating struct pages for their BARs which is not necessary nor desired. How do you prevent direct I/O on those pages for example? Allocating a struct pages has their use case, for example for exposing VRAM as memory for HMM. But that is something very specific and should not limit PCIe P2P DMA in general. [SNIP] Well that is certainly not true. I'm just not sure if that works with all IOMMU drivers thought. Huh? All the iommu interfaces except for the dma_map_resource() are struct page based. dma_map_resource() is slow ad limited in what it can do. Yeah, but that is exactly the functionality we need. And as far as I can see that is also what Oded wants here. Mapping stuff into userspace and then doing direct DMA to it is only a very limited use case and we need to be more flexible here. Christian. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 17:28 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 05:24:08PM +0200, Christian König wrote: I will take two GAUDI devices and use one as an exporter and one as an importer. I want to see that the solution works end-to-end, with real device DMA from importer to exporter. I can tell you it doesn't. Stuffing physical addresses directly into the sg list doesn't involve any of the IOMMU code so any configuration that requires IOMMU page table setup will not work. Sure it does. See amdgpu_vram_mgr_alloc_sgt: amdgpu_res_first(res, offset, length, ); ^^ I'm not talking about the AMD driver, I'm talking about this patch. + bar_address = hdev->dram_pci_bar_start + + (pages[cur_page] - prop->dram_base_address); + sg_dma_address(sg) = bar_address; Yeah, that is indeed not working. Oded you need to use dma_map_resource() for this. Christian. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 17:23 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 02:23:03PM +0200, Christian König wrote: Am 22.06.21 um 14:01 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: On Tue, Jun 22, 2021 at 9:37 AM Christian König wrote: Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: Another thing I want to emphasize is that we are doing p2p only through the export/import of the FD. We do *not* allow the user to mmap the dma-buf as we do not support direct IO. So there is no access to these pages through the userspace. Arguably mmaping the memory is a better choice, and is the direction that Logan's series goes in. Here the use of DMABUF was specifically designed to allow hitless revokation of the memory, which this isn't even using. The major problem with this approach is that DMA-buf is also used for memory which isn't CPU accessible. That isn't an issue here because the memory is only intended to be used with P2P transfers so it must be CPU accessible. No, especially P2P is often done on memory resources which are not even remotely CPU accessible. That is a special AMD thing, P2P here is PCI P2P and all PCI memory is CPU accessible. No absolutely not. NVidia GPUs work exactly the same way. And you have tons of similar cases in embedded and SoC systems where intermediate memory between devices isn't directly addressable with the CPU. So you are taking the hit of very limited hardware support and reduced performance just to squeeze into DMABUF.. You still have the issue that this patch is doing all of this P2P stuff wrong - following the already NAK'd AMD approach. Well that stuff was NAKed because we still use sg_tables, not because we don't want to allocate struct pages. sg lists in general. The plan is to push this forward since DEVICE_PRIVATE clearly can't handle all of our use cases and is not really a good fit to be honest. IOMMU is now working as well, so as far as I can see we are all good here. How? Is that more AMD special stuff? No, just using the dma_map_resource() interface. We have that working on tons of IOMMU enabled systems. This patch series never calls to the iommu driver, AFAICT. I'll go and read Logan's patch-set to see if that will work for us in the future. Please remember, as Daniel said, we don't have struct page backing our device memory, so if that is a requirement to connect to Logan's work, then I don't think we will want to do it at this point. It is trivial to get the struct page for a PCI BAR. Yeah, but it doesn't make much sense. Why should we create a struct page for something that isn't even memory in a lot of cases? Because the iommu and other places need this handle to setup their stuff. Nobody has yet been brave enough to try to change those flows to be able to use a physical CPU address. Well that is certainly not true. I'm just not sure if that works with all IOMMU drivers thought. Would need to ping Felix when the support for this was merged. Regards, Christian. This is why we have a special struct page type just for PCI BAR memory. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 17:11 schrieb Jason Gunthorpe:
On Tue, Jun 22, 2021 at 04:12:26PM +0300, Oded Gabbay wrote:
1) Setting sg_page to NULL
2) 'mapping' pages for P2P DMA without going through the iommu
3) Allowing P2P DMA without using the p2p dma API to validate that it
can work at all in the first place.
All of these result in functional bugs in certain system
configurations.
Jason
Hi Jason,
Thanks for the feedback.
Regarding point 1, why is that a problem if we disable the option to
mmap the dma-buf from user-space ?
Userspace has nothing to do with needing struct pages or not
Point 1 and 2 mostly go together, you supporting the iommu is not nice
if you dont have struct pages.
You should study Logan's patches I pointed you at as they are solving
exactly this problem.
In addition, I didn't see any problem with sg_page being NULL in the
RDMA p2p dma-buf code. Did I miss something here ?
No, the design of the dmabuf requires the exporter to do the dma maps
and so it is only the exporter that is wrong to omit all the iommu and
p2p logic.
RDMA is OK today only because nobody has implemented dma buf support
in rxe/si - mainly because the only implementations of exporters don't
set the struct page and are thus buggy.
I will take two GAUDI devices and use one as an exporter and one as an
importer. I want to see that the solution works end-to-end, with real
device DMA from importer to exporter.
I can tell you it doesn't. Stuffing physical addresses directly into
the sg list doesn't involve any of the IOMMU code so any configuration
that requires IOMMU page table setup will not work.
Sure it does. See amdgpu_vram_mgr_alloc_sgt:
amdgpu_res_first(res, offset, length, );
for_each_sgtable_sg((*sgt), sg, i) {
phys_addr_t phys = cursor.start + adev->gmc.aper_base;
size_t size = cursor.size;
dma_addr_t addr;
addr = dma_map_resource(dev, phys, size, dir,
DMA_ATTR_SKIP_CPU_SYNC);
r = dma_mapping_error(dev, addr);
if (r)
goto error_unmap;
sg_set_page(sg, NULL, size, 0);
sg_dma_address(sg) = addr;
sg_dma_len(sg) = size;
amdgpu_res_next(, cursor.size);
}
dma_map_resource() does the IOMMU mapping for us.
Regards,
Christian.
Jason
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 02:23:03PM +0200, Christian König wrote: > Am 22.06.21 um 14:01 schrieb Jason Gunthorpe: > > On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > > > On Tue, Jun 22, 2021 at 9:37 AM Christian König > > > wrote: > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > > > > > > > Another thing I want to emphasize is that we are doing p2p only > > > > > > through the export/import of the FD. We do *not* allow the user to > > > > > > mmap the dma-buf as we do not support direct IO. So there is no > > > > > > access > > > > > > to these pages through the userspace. > > > > > Arguably mmaping the memory is a better choice, and is the direction > > > > > that Logan's series goes in. Here the use of DMABUF was specifically > > > > > designed to allow hitless revokation of the memory, which this isn't > > > > > even using. > > > > The major problem with this approach is that DMA-buf is also used for > > > > memory which isn't CPU accessible. > > That isn't an issue here because the memory is only intended to be > > used with P2P transfers so it must be CPU accessible. > > No, especially P2P is often done on memory resources which are not even > remotely CPU accessible. That is a special AMD thing, P2P here is PCI P2P and all PCI memory is CPU accessible. > > > > > So you are taking the hit of very limited hardware support and reduced > > > > > performance just to squeeze into DMABUF.. > > You still have the issue that this patch is doing all of this P2P > > stuff wrong - following the already NAK'd AMD approach. > > Well that stuff was NAKed because we still use sg_tables, not because we > don't want to allocate struct pages. sg lists in general. > The plan is to push this forward since DEVICE_PRIVATE clearly can't handle > all of our use cases and is not really a good fit to be honest. > > IOMMU is now working as well, so as far as I can see we are all good here. How? Is that more AMD special stuff? This patch series never calls to the iommu driver, AFAICT. > > > I'll go and read Logan's patch-set to see if that will work for us in > > > the future. Please remember, as Daniel said, we don't have struct page > > > backing our device memory, so if that is a requirement to connect to > > > Logan's work, then I don't think we will want to do it at this point. > > It is trivial to get the struct page for a PCI BAR. > > Yeah, but it doesn't make much sense. Why should we create a struct page for > something that isn't even memory in a lot of cases? Because the iommu and other places need this handle to setup their stuff. Nobody has yet been brave enough to try to change those flows to be able to use a physical CPU address. This is why we have a special struct page type just for PCI BAR memory. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 05:48:10PM +0200, Christian König wrote: > Am 22.06.21 um 17:40 schrieb Jason Gunthorpe: > > On Tue, Jun 22, 2021 at 05:29:01PM +0200, Christian König wrote: > > > [SNIP] > > > No absolutely not. NVidia GPUs work exactly the same way. > > > > > > And you have tons of similar cases in embedded and SoC systems where > > > intermediate memory between devices isn't directly addressable with the > > > CPU. > > None of that is PCI P2P. > > > > It is all some specialty direct transfer. > > > > You can't reasonably call dma_map_resource() on non CPU mapped memory > > for instance, what address would you pass? > > > > Do not confuse "I am doing transfers between two HW blocks" with PCI > > Peer to Peer DMA transfers - the latter is a very narrow subcase. > > > > > No, just using the dma_map_resource() interface. > > Ik, but yes that does "work". Logan's series is better. > > No it isn't. It makes devices depend on allocating struct pages for their > BARs which is not necessary nor desired. Which dramatically reduces the cost of establishing DMA mappings, a loop of dma_map_resource() is very expensive. > How do you prevent direct I/O on those pages for example? GUP fails. > Allocating a struct pages has their use case, for example for exposing VRAM > as memory for HMM. But that is something very specific and should not limit > PCIe P2P DMA in general. Sure, but that is an ideal we are far from obtaining, and nobody wants to work on it prefering to do hacky hacky like this. If you believe in this then remove the scatter list from dmabuf, add a new set of dma_map* APIs to work on physical addresses and all the other stuff needed. Otherwise, we have what we have and drivers don't get to opt out. This is why the stuff in AMDGPU was NAK'd. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 05:29:01PM +0200, Christian König wrote: > Am 22.06.21 um 17:23 schrieb Jason Gunthorpe: > > On Tue, Jun 22, 2021 at 02:23:03PM +0200, Christian König wrote: > > > Am 22.06.21 um 14:01 schrieb Jason Gunthorpe: > > > > On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > > > > > On Tue, Jun 22, 2021 at 9:37 AM Christian König > > > > > wrote: > > > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > > > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > > > > > > > > > > > Another thing I want to emphasize is that we are doing p2p only > > > > > > > > through the export/import of the FD. We do *not* allow the user > > > > > > > > to > > > > > > > > mmap the dma-buf as we do not support direct IO. So there is no > > > > > > > > access > > > > > > > > to these pages through the userspace. > > > > > > > Arguably mmaping the memory is a better choice, and is the > > > > > > > direction > > > > > > > that Logan's series goes in. Here the use of DMABUF was > > > > > > > specifically > > > > > > > designed to allow hitless revokation of the memory, which this > > > > > > > isn't > > > > > > > even using. > > > > > > The major problem with this approach is that DMA-buf is also used > > > > > > for > > > > > > memory which isn't CPU accessible. > > > > That isn't an issue here because the memory is only intended to be > > > > used with P2P transfers so it must be CPU accessible. > > > No, especially P2P is often done on memory resources which are not even > > > remotely CPU accessible. > > That is a special AMD thing, P2P here is PCI P2P and all PCI memory is > > CPU accessible. > > No absolutely not. NVidia GPUs work exactly the same way. > > And you have tons of similar cases in embedded and SoC systems where > intermediate memory between devices isn't directly addressable with the CPU. None of that is PCI P2P. It is all some specialty direct transfer. You can't reasonably call dma_map_resource() on non CPU mapped memory for instance, what address would you pass? Do not confuse "I am doing transfers between two HW blocks" with PCI Peer to Peer DMA transfers - the latter is a very narrow subcase. > No, just using the dma_map_resource() interface. Ik, but yes that does "work". Logan's series is better. > > > > > I'll go and read Logan's patch-set to see if that will work for us in > > > > > the future. Please remember, as Daniel said, we don't have struct page > > > > > backing our device memory, so if that is a requirement to connect to > > > > > Logan's work, then I don't think we will want to do it at this point. > > > > It is trivial to get the struct page for a PCI BAR. > > > Yeah, but it doesn't make much sense. Why should we create a struct page > > > for > > > something that isn't even memory in a lot of cases? > > Because the iommu and other places need this handle to setup their > > stuff. Nobody has yet been brave enough to try to change those flows > > to be able to use a physical CPU address. > > Well that is certainly not true. I'm just not sure if that works with all > IOMMU drivers thought. Huh? All the iommu interfaces except for the dma_map_resource() are struct page based. dma_map_resource() is slow ad limited in what it can do. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 06:24:28PM +0300, Oded Gabbay wrote: > On Tue, Jun 22, 2021 at 6:11 PM Jason Gunthorpe wrote: > > > > On Tue, Jun 22, 2021 at 04:12:26PM +0300, Oded Gabbay wrote: > > > > > > 1) Setting sg_page to NULL > > > > 2) 'mapping' pages for P2P DMA without going through the iommu > > > > 3) Allowing P2P DMA without using the p2p dma API to validate that it > > > >can work at all in the first place. > > > > > > > > All of these result in functional bugs in certain system > > > > configurations. > > > > > > > > Jason > > > > > > Hi Jason, > > > Thanks for the feedback. > > > Regarding point 1, why is that a problem if we disable the option to > > > mmap the dma-buf from user-space ? > > > > Userspace has nothing to do with needing struct pages or not > > > > Point 1 and 2 mostly go together, you supporting the iommu is not nice > > if you dont have struct pages. > > > > You should study Logan's patches I pointed you at as they are solving > > exactly this problem. > Yes, I do need to study them. I agree with you here. It appears I > have a hole in my understanding. I'm missing the connection between > iommu support (which I must have of course) and struct pages. Chistian explained what the AMD driver is doing by calling dma_map_resource(). Which is a hacky and slow way of achieving what Logan's series is doing. > > No, the design of the dmabuf requires the exporter to do the dma maps > > and so it is only the exporter that is wrong to omit all the iommu and > > p2p logic. > > > > RDMA is OK today only because nobody has implemented dma buf support > > in rxe/si - mainly because the only implementations of exporters don't > > Can you please educate me, what is rxe/si ? Sorry, rxe/siw - these are the all-software implementations of RDMA and they require the struct page to do a SW memory copy. They can't implement dmabuf without it. > ok... > so how come that patch-set was merged into 5.12 if it's buggy ? We only implemented true dma devices for RDMA DMABUF support, so it is isn't buggy right now. > Yes, that's what I expect to see. But I want to see it with my own > eyes and then figure out how to solve this. It might be tricky to test because you have to ensure the iommu is turned on and has a non-idenity page table. Basically if it doesn't trigger a IOMMU failure then the IOMMU isn't setup properly. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 04:12:26PM +0300, Oded Gabbay wrote: > > 1) Setting sg_page to NULL > > 2) 'mapping' pages for P2P DMA without going through the iommu > > 3) Allowing P2P DMA without using the p2p dma API to validate that it > >can work at all in the first place. > > > > All of these result in functional bugs in certain system > > configurations. > > > > Jason > > Hi Jason, > Thanks for the feedback. > Regarding point 1, why is that a problem if we disable the option to > mmap the dma-buf from user-space ? Userspace has nothing to do with needing struct pages or not Point 1 and 2 mostly go together, you supporting the iommu is not nice if you dont have struct pages. You should study Logan's patches I pointed you at as they are solving exactly this problem. > In addition, I didn't see any problem with sg_page being NULL in the > RDMA p2p dma-buf code. Did I miss something here ? No, the design of the dmabuf requires the exporter to do the dma maps and so it is only the exporter that is wrong to omit all the iommu and p2p logic. RDMA is OK today only because nobody has implemented dma buf support in rxe/si - mainly because the only implementations of exporters don't set the struct page and are thus buggy. > I will take two GAUDI devices and use one as an exporter and one as an > importer. I want to see that the solution works end-to-end, with real > device DMA from importer to exporter. I can tell you it doesn't. Stuffing physical addresses directly into the sg list doesn't involve any of the IOMMU code so any configuration that requires IOMMU page table setup will not work. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 05:24:08PM +0200, Christian König wrote: > > > I will take two GAUDI devices and use one as an exporter and one as an > > > importer. I want to see that the solution works end-to-end, with real > > > device DMA from importer to exporter. > > I can tell you it doesn't. Stuffing physical addresses directly into > > the sg list doesn't involve any of the IOMMU code so any configuration > > that requires IOMMU page table setup will not work. > > Sure it does. See amdgpu_vram_mgr_alloc_sgt: > > amdgpu_res_first(res, offset, length, ); ^^ I'm not talking about the AMD driver, I'm talking about this patch. + bar_address = hdev->dram_pci_bar_start + + (pages[cur_page] - prop->dram_base_address); + sg_dma_address(sg) = bar_address; Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 14:01 schrieb Jason Gunthorpe: On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: On Tue, Jun 22, 2021 at 9:37 AM Christian König wrote: Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: Another thing I want to emphasize is that we are doing p2p only through the export/import of the FD. We do *not* allow the user to mmap the dma-buf as we do not support direct IO. So there is no access to these pages through the userspace. Arguably mmaping the memory is a better choice, and is the direction that Logan's series goes in. Here the use of DMABUF was specifically designed to allow hitless revokation of the memory, which this isn't even using. The major problem with this approach is that DMA-buf is also used for memory which isn't CPU accessible. That isn't an issue here because the memory is only intended to be used with P2P transfers so it must be CPU accessible. No, especially P2P is often done on memory resources which are not even remotely CPU accessible. That's one of the major reasons why we use P2P in the first place. See the whole XGMI implementation for example. Thanks Jason for the clarification, but I honestly prefer to use DMA-BUF at the moment. It gives us just what we need (even more than what we need as you pointed out), it is *already* integrated and tested in the RDMA subsystem, and I'm feeling comfortable using it as I'm somewhat familiar with it from my AMD days. That was one of the reasons we didn't even considered using the mapping memory approach for GPUs. Well, now we have DEVICE_PRIVATE memory that can meet this need too.. Just nobody has wired it up to hmm_range_fault() So you are taking the hit of very limited hardware support and reduced performance just to squeeze into DMABUF.. You still have the issue that this patch is doing all of this P2P stuff wrong - following the already NAK'd AMD approach. Well that stuff was NAKed because we still use sg_tables, not because we don't want to allocate struct pages. The plan is to push this forward since DEVICE_PRIVATE clearly can't handle all of our use cases and is not really a good fit to be honest. IOMMU is now working as well, so as far as I can see we are all good here. I'll go and read Logan's patch-set to see if that will work for us in the future. Please remember, as Daniel said, we don't have struct page backing our device memory, so if that is a requirement to connect to Logan's work, then I don't think we will want to do it at this point. It is trivial to get the struct page for a PCI BAR. Yeah, but it doesn't make much sense. Why should we create a struct page for something that isn't even memory in a lot of cases? Regards, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 6:31 PM Christian König wrote: > > > > Am 22.06.21 um 17:28 schrieb Jason Gunthorpe: > > On Tue, Jun 22, 2021 at 05:24:08PM +0200, Christian König wrote: > > > I will take two GAUDI devices and use one as an exporter and one as an > importer. I want to see that the solution works end-to-end, with real > device DMA from importer to exporter. > >>> I can tell you it doesn't. Stuffing physical addresses directly into > >>> the sg list doesn't involve any of the IOMMU code so any configuration > >>> that requires IOMMU page table setup will not work. > >> Sure it does. See amdgpu_vram_mgr_alloc_sgt: > >> > >> amdgpu_res_first(res, offset, length, ); > > ^^ > > > > I'm not talking about the AMD driver, I'm talking about this patch. > > > > + bar_address = hdev->dram_pci_bar_start + > > + (pages[cur_page] - prop->dram_base_address); > > + sg_dma_address(sg) = bar_address; > > Yeah, that is indeed not working. > > Oded you need to use dma_map_resource() for this. > > Christian. Yes, of course. But will it be enough ? Jason said that supporting IOMMU isn't nice when we don't have struct pages. I fail to understand the connection, I need to dig into this. Oded > > > > > > > Jason > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 6:28 PM Jason Gunthorpe wrote: > > On Tue, Jun 22, 2021 at 05:24:08PM +0200, Christian König wrote: > > > > > I will take two GAUDI devices and use one as an exporter and one as an > > > > importer. I want to see that the solution works end-to-end, with real > > > > device DMA from importer to exporter. > > > I can tell you it doesn't. Stuffing physical addresses directly into > > > the sg list doesn't involve any of the IOMMU code so any configuration > > > that requires IOMMU page table setup will not work. > > > > Sure it does. See amdgpu_vram_mgr_alloc_sgt: > > > > amdgpu_res_first(res, offset, length, ); > ^^ > > I'm not talking about the AMD driver, I'm talking about this patch. > > + bar_address = hdev->dram_pci_bar_start + > + (pages[cur_page] - prop->dram_base_address); > + sg_dma_address(sg) = bar_address; > > Jason Yes, you are correct of course, but what will happen Jason, If I will add a call to dma_map_resource() like Christian said ? Won't that solve that specific issue ? That's why I want to try it... Oded ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 6:11 PM Jason Gunthorpe wrote: > > On Tue, Jun 22, 2021 at 04:12:26PM +0300, Oded Gabbay wrote: > > > > 1) Setting sg_page to NULL > > > 2) 'mapping' pages for P2P DMA without going through the iommu > > > 3) Allowing P2P DMA without using the p2p dma API to validate that it > > >can work at all in the first place. > > > > > > All of these result in functional bugs in certain system > > > configurations. > > > > > > Jason > > > > Hi Jason, > > Thanks for the feedback. > > Regarding point 1, why is that a problem if we disable the option to > > mmap the dma-buf from user-space ? > > Userspace has nothing to do with needing struct pages or not > > Point 1 and 2 mostly go together, you supporting the iommu is not nice > if you dont have struct pages. > > You should study Logan's patches I pointed you at as they are solving > exactly this problem. Yes, I do need to study them. I agree with you here. It appears I have a hole in my understanding. I'm missing the connection between iommu support (which I must have of course) and struct pages. > > > In addition, I didn't see any problem with sg_page being NULL in the > > RDMA p2p dma-buf code. Did I miss something here ? > > No, the design of the dmabuf requires the exporter to do the dma maps > and so it is only the exporter that is wrong to omit all the iommu and > p2p logic. > > RDMA is OK today only because nobody has implemented dma buf support > in rxe/si - mainly because the only implementations of exporters don't Can you please educate me, what is rxe/si ? > set the struct page and are thus buggy. ok... so how come that patch-set was merged into 5.12 if it's buggy ? Because the current exporters are buggy ? I probably need a history lesson here. But I understand why you think it's a bad idea to add a new buggy exporter. > > > I will take two GAUDI devices and use one as an exporter and one as an > > importer. I want to see that the solution works end-to-end, with real > > device DMA from importer to exporter. > > I can tell you it doesn't. Stuffing physical addresses directly into > the sg list doesn't involve any of the IOMMU code so any configuration > that requires IOMMU page table setup will not work. > > Jason Yes, that's what I expect to see. But I want to see it with my own eyes and then figure out how to solve this. Maybe the result will be going to Logan's path, maybe something else, but I need to start by seeing the failure in a real system. Thanks for the information, it is really helpful. Oded ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 3:15 PM Jason Gunthorpe wrote: > > On Tue, Jun 22, 2021 at 03:04:30PM +0300, Oded Gabbay wrote: > > On Tue, Jun 22, 2021 at 3:01 PM Jason Gunthorpe wrote: > > > > > > On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > > > > On Tue, Jun 22, 2021 at 9:37 AM Christian König > > > > wrote: > > > > > > > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > > > > > > > >> Another thing I want to emphasize is that we are doing p2p only > > > > > >> through the export/import of the FD. We do *not* allow the user to > > > > > >> mmap the dma-buf as we do not support direct IO. So there is no > > > > > >> access > > > > > >> to these pages through the userspace. > > > > > > Arguably mmaping the memory is a better choice, and is the direction > > > > > > that Logan's series goes in. Here the use of DMABUF was specifically > > > > > > designed to allow hitless revokation of the memory, which this isn't > > > > > > even using. > > > > > > > > > > The major problem with this approach is that DMA-buf is also used for > > > > > memory which isn't CPU accessible. > > > > > > That isn't an issue here because the memory is only intended to be > > > used with P2P transfers so it must be CPU accessible. > > > > > > > > That was one of the reasons we didn't even considered using the > > > > > mapping > > > > > memory approach for GPUs. > > > > > > Well, now we have DEVICE_PRIVATE memory that can meet this need > > > too.. Just nobody has wired it up to hmm_range_fault() > > > > > > > > > So you are taking the hit of very limited hardware support and > > > > > > reduced > > > > > > performance just to squeeze into DMABUF.. > > > > > > > > Thanks Jason for the clarification, but I honestly prefer to use > > > > DMA-BUF at the moment. > > > > It gives us just what we need (even more than what we need as you > > > > pointed out), it is *already* integrated and tested in the RDMA > > > > subsystem, and I'm feeling comfortable using it as I'm somewhat > > > > familiar with it from my AMD days. > > > > > > You still have the issue that this patch is doing all of this P2P > > > stuff wrong - following the already NAK'd AMD approach. > > > > Could you please point me exactly to the lines of code that are wrong > > in your opinion ? > > 1) Setting sg_page to NULL > 2) 'mapping' pages for P2P DMA without going through the iommu > 3) Allowing P2P DMA without using the p2p dma API to validate that it >can work at all in the first place. > > All of these result in functional bugs in certain system > configurations. > > Jason Hi Jason, Thanks for the feedback. Regarding point 1, why is that a problem if we disable the option to mmap the dma-buf from user-space ? We don't want to support CPU fallback/Direct IO. In addition, I didn't see any problem with sg_page being NULL in the RDMA p2p dma-buf code. Did I miss something here ? Regarding points 2 & 3, I want to examine them more closely in a KVM virtual machine environment with IOMMU enabled. I will take two GAUDI devices and use one as an exporter and one as an importer. I want to see that the solution works end-to-end, with real device DMA from importer to exporter. I fear that the dummy importer I wrote is bypassing these two issues you brought up. So thanks again and I'll get back and update once I've finished testing it. Oded ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 03:04:30PM +0300, Oded Gabbay wrote: > On Tue, Jun 22, 2021 at 3:01 PM Jason Gunthorpe wrote: > > > > On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > > > On Tue, Jun 22, 2021 at 9:37 AM Christian König > > > wrote: > > > > > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > > > > > >> Another thing I want to emphasize is that we are doing p2p only > > > > >> through the export/import of the FD. We do *not* allow the user to > > > > >> mmap the dma-buf as we do not support direct IO. So there is no > > > > >> access > > > > >> to these pages through the userspace. > > > > > Arguably mmaping the memory is a better choice, and is the direction > > > > > that Logan's series goes in. Here the use of DMABUF was specifically > > > > > designed to allow hitless revokation of the memory, which this isn't > > > > > even using. > > > > > > > > The major problem with this approach is that DMA-buf is also used for > > > > memory which isn't CPU accessible. > > > > That isn't an issue here because the memory is only intended to be > > used with P2P transfers so it must be CPU accessible. > > > > > > That was one of the reasons we didn't even considered using the mapping > > > > memory approach for GPUs. > > > > Well, now we have DEVICE_PRIVATE memory that can meet this need > > too.. Just nobody has wired it up to hmm_range_fault() > > > > > > > So you are taking the hit of very limited hardware support and reduced > > > > > performance just to squeeze into DMABUF.. > > > > > > Thanks Jason for the clarification, but I honestly prefer to use > > > DMA-BUF at the moment. > > > It gives us just what we need (even more than what we need as you > > > pointed out), it is *already* integrated and tested in the RDMA > > > subsystem, and I'm feeling comfortable using it as I'm somewhat > > > familiar with it from my AMD days. > > > > You still have the issue that this patch is doing all of this P2P > > stuff wrong - following the already NAK'd AMD approach. > > Could you please point me exactly to the lines of code that are wrong > in your opinion ? 1) Setting sg_page to NULL 2) 'mapping' pages for P2P DMA without going through the iommu 3) Allowing P2P DMA without using the p2p dma API to validate that it can work at all in the first place. All of these result in functional bugs in certain system configurations. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > On Tue, Jun 22, 2021 at 9:37 AM Christian König > wrote: > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > >> Another thing I want to emphasize is that we are doing p2p only > > >> through the export/import of the FD. We do *not* allow the user to > > >> mmap the dma-buf as we do not support direct IO. So there is no access > > >> to these pages through the userspace. > > > Arguably mmaping the memory is a better choice, and is the direction > > > that Logan's series goes in. Here the use of DMABUF was specifically > > > designed to allow hitless revokation of the memory, which this isn't > > > even using. > > > > The major problem with this approach is that DMA-buf is also used for > > memory which isn't CPU accessible. That isn't an issue here because the memory is only intended to be used with P2P transfers so it must be CPU accessible. > > That was one of the reasons we didn't even considered using the mapping > > memory approach for GPUs. Well, now we have DEVICE_PRIVATE memory that can meet this need too.. Just nobody has wired it up to hmm_range_fault() > > > So you are taking the hit of very limited hardware support and reduced > > > performance just to squeeze into DMABUF.. > > Thanks Jason for the clarification, but I honestly prefer to use > DMA-BUF at the moment. > It gives us just what we need (even more than what we need as you > pointed out), it is *already* integrated and tested in the RDMA > subsystem, and I'm feeling comfortable using it as I'm somewhat > familiar with it from my AMD days. You still have the issue that this patch is doing all of this P2P stuff wrong - following the already NAK'd AMD approach. > I'll go and read Logan's patch-set to see if that will work for us in > the future. Please remember, as Daniel said, we don't have struct page > backing our device memory, so if that is a requirement to connect to > Logan's work, then I don't think we will want to do it at this point. It is trivial to get the struct page for a PCI BAR. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 3:01 PM Jason Gunthorpe wrote: > > On Tue, Jun 22, 2021 at 11:42:27AM +0300, Oded Gabbay wrote: > > On Tue, Jun 22, 2021 at 9:37 AM Christian König > > wrote: > > > > > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > > > > > >> Another thing I want to emphasize is that we are doing p2p only > > > >> through the export/import of the FD. We do *not* allow the user to > > > >> mmap the dma-buf as we do not support direct IO. So there is no access > > > >> to these pages through the userspace. > > > > Arguably mmaping the memory is a better choice, and is the direction > > > > that Logan's series goes in. Here the use of DMABUF was specifically > > > > designed to allow hitless revokation of the memory, which this isn't > > > > even using. > > > > > > The major problem with this approach is that DMA-buf is also used for > > > memory which isn't CPU accessible. > > That isn't an issue here because the memory is only intended to be > used with P2P transfers so it must be CPU accessible. > > > > That was one of the reasons we didn't even considered using the mapping > > > memory approach for GPUs. > > Well, now we have DEVICE_PRIVATE memory that can meet this need > too.. Just nobody has wired it up to hmm_range_fault() > > > > > So you are taking the hit of very limited hardware support and reduced > > > > performance just to squeeze into DMABUF.. > > > > Thanks Jason for the clarification, but I honestly prefer to use > > DMA-BUF at the moment. > > It gives us just what we need (even more than what we need as you > > pointed out), it is *already* integrated and tested in the RDMA > > subsystem, and I'm feeling comfortable using it as I'm somewhat > > familiar with it from my AMD days. > > You still have the issue that this patch is doing all of this P2P > stuff wrong - following the already NAK'd AMD approach. Could you please point me exactly to the lines of code that are wrong in your opinion ? I find it hard to understand from your statement what exactly you think that we are doing wrong. The implementation is found in the second patch in this patch-set. Thanks, Oded > > > I'll go and read Logan's patch-set to see if that will work for us in > > the future. Please remember, as Daniel said, we don't have struct page > > backing our device memory, so if that is a requirement to connect to > > Logan's work, then I don't think we will want to do it at this point. > > It is trivial to get the struct page for a PCI BAR. > > Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Tue, Jun 22, 2021 at 9:37 AM Christian König wrote: > > Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: > > On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: > > > >> Another thing I want to emphasize is that we are doing p2p only > >> through the export/import of the FD. We do *not* allow the user to > >> mmap the dma-buf as we do not support direct IO. So there is no access > >> to these pages through the userspace. > > Arguably mmaping the memory is a better choice, and is the direction > > that Logan's series goes in. Here the use of DMABUF was specifically > > designed to allow hitless revokation of the memory, which this isn't > > even using. > > The major problem with this approach is that DMA-buf is also used for > memory which isn't CPU accessible. > > That was one of the reasons we didn't even considered using the mapping > memory approach for GPUs. > > Regards, > Christian. > > > > > So you are taking the hit of very limited hardware support and reduced > > performance just to squeeze into DMABUF.. Thanks Jason for the clarification, but I honestly prefer to use DMA-BUF at the moment. It gives us just what we need (even more than what we need as you pointed out), it is *already* integrated and tested in the RDMA subsystem, and I'm feeling comfortable using it as I'm somewhat familiar with it from my AMD days. I'll go and read Logan's patch-set to see if that will work for us in the future. Please remember, as Daniel said, we don't have struct page backing our device memory, so if that is a requirement to connect to Logan's work, then I don't think we will want to do it at this point. Thanks, Oded > > > > Jason > > ___ > > Linaro-mm-sig mailing list > > linaro-mm-...@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 22.06.21 um 01:29 schrieb Jason Gunthorpe: On Mon, Jun 21, 2021 at 10:24:16PM +0300, Oded Gabbay wrote: Another thing I want to emphasize is that we are doing p2p only through the export/import of the FD. We do *not* allow the user to mmap the dma-buf as we do not support direct IO. So there is no access to these pages through the userspace. Arguably mmaping the memory is a better choice, and is the direction that Logan's series goes in. Here the use of DMABUF was specifically designed to allow hitless revokation of the memory, which this isn't even using. The major problem with this approach is that DMA-buf is also used for memory which isn't CPU accessible. That was one of the reasons we didn't even considered using the mapping memory approach for GPUs. Regards, Christian. So you are taking the hit of very limited hardware support and reduced performance just to squeeze into DMABUF.. Jason ___ Linaro-mm-sig mailing list linaro-mm-...@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-mm-sig ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
