Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Tue, Jan 09, 2018 at 05:46:40PM +0100, Christoph Hellwig wrote: > On Mon, Jan 08, 2018 at 12:49:50PM -0700, Jason Gunthorpe wrote: > > Pretty sure P2P capable IOMMU hardware exists. > > > > With SOC's we also have the scenario that an DMA originated from an > > on-die device wishes to target

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Tue, Jan 09, 2018 at 05:46:40PM +0100, Christoph Hellwig wrote: > On Mon, Jan 08, 2018 at 12:49:50PM -0700, Jason Gunthorpe wrote: > > Pretty sure P2P capable IOMMU hardware exists. > > > > With SOC's we also have the scenario that an DMA originated from an > > on-die device wishes to target

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:01:16PM -0700, Jason Gunthorpe wrote: > > So I very much disagree about where to place that workaround - the > > RDMA code is exactly the right place. > > But why? RDMA is using core code to do this. It uses dma_ops in struct > device and it uses normal dma_map SG. How

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:01:16PM -0700, Jason Gunthorpe wrote: > > So I very much disagree about where to place that workaround - the > > RDMA code is exactly the right place. > > But why? RDMA is using core code to do this. It uses dma_ops in struct > device and it uses normal dma_map SG. How

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:05:57PM -0700, Logan Gunthorpe wrote: > Ok, so if we shouldn't touch the dma_map infrastructure how should the > workaround to opt-out HFI and QIB look? I'm not that familiar with the RDMA > code. We can add a no_p2p quirk, I'm just not sure what the right place for

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:05:57PM -0700, Logan Gunthorpe wrote: > Ok, so if we shouldn't touch the dma_map infrastructure how should the > workaround to opt-out HFI and QIB look? I'm not that familiar with the RDMA > code. We can add a no_p2p quirk, I'm just not sure what the right place for

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:49:50PM -0700, Jason Gunthorpe wrote: > Pretty sure P2P capable IOMMU hardware exists. > > With SOC's we also have the scenario that an DMA originated from an > on-die device wishes to target an off-die PCI BAR (through the IOMMU), > that definitely exists today, and

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 12:49:50PM -0700, Jason Gunthorpe wrote: > Pretty sure P2P capable IOMMU hardware exists. > > With SOC's we also have the scenario that an DMA originated from an > on-die device wishes to target an off-die PCI BAR (through the IOMMU), > that definitely exists today, and

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 8, 2018 at 11:57 AM, Christoph Hellwig wrote: >> (c) setup/manage any security permissions on mappings >> Which P2P may at some point be concerned with. > > Maybe once root complexes with iommus actually support P2P. But until > then we have a lot more more important

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 8, 2018 at 11:57 AM, Christoph Hellwig wrote: >> (c) setup/manage any security permissions on mappings >> Which P2P may at some point be concerned with. > > Maybe once root complexes with iommus actually support P2P. But until > then we have a lot more more important problems to

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:57 AM, Christoph Hellwig wrote: It does, sort of - but in a different way then the normal DMA map ops. And only to work around the fact that we need to map our P2P space into struct pages. Without that we could just pass the bus address around, but the Linux stack and VM isn't

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:57 AM, Christoph Hellwig wrote: It does, sort of - but in a different way then the normal DMA map ops. And only to work around the fact that we need to map our P2P space into struct pages. Without that we could just pass the bus address around, but the Linux stack and VM isn't

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 07:34:34PM +0100, Christoph Hellwig wrote: > > > > And on that topic, does this scheme work with HFI? > > > > > > No, and I guess we need an opt-out. HFI generally seems to be > > > extremely weird. > > > > This series needs some kind of fix so HFI, QIB, rxe, etc don't

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 07:34:34PM +0100, Christoph Hellwig wrote: > > > > And on that topic, does this scheme work with HFI? > > > > > > No, and I guess we need an opt-out. HFI generally seems to be > > > extremely weird. > > > > This series needs some kind of fix so HFI, QIB, rxe, etc don't

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:44:41AM -0700, Logan Gunthorpe wrote: >> Think about what the dma mapping routines do: >> >> (a) translate from host address to bus addresses >> >> and >> >> (b) flush caches (in non-coherent architectures) >> >> Both are obviously not needed for P2P transfers, as

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:44:41AM -0700, Logan Gunthorpe wrote: >> Think about what the dma mapping routines do: >> >> (a) translate from host address to bus addresses >> >> and >> >> (b) flush caches (in non-coherent architectures) >> >> Both are obviously not needed for P2P transfers, as

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:34 AM, Christoph Hellwig wrote: But P2P is _not_ a factor of the dma_ops implementation at all, it is something that happens behind the dma_map implementation. Think about what the dma mapping routines do: (a) translate from host address to bus addresses and (b) flush

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:34 AM, Christoph Hellwig wrote: But P2P is _not_ a factor of the dma_ops implementation at all, it is something that happens behind the dma_map implementation. Think about what the dma mapping routines do: (a) translate from host address to bus addresses and (b) flush

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:09:17AM -0700, Jason Gunthorpe wrote: > > As usual we implement what actually has a consumer. On top of that the > > R/W API is the only core RDMA API that actually does DMA mapping for the > > ULP at the moment. > > Well again the same can be said for dma_map_page vs

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:09:17AM -0700, Jason Gunthorpe wrote: > > As usual we implement what actually has a consumer. On top of that the > > R/W API is the only core RDMA API that actually does DMA mapping for the > > ULP at the moment. > > Well again the same can be said for dma_map_page vs

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:17:38AM -0700, Logan Gunthorpe wrote: > >>If at all it should be in the dma_map* wrappers, but for that we'd need > >>a good identifier. And it still would not solve the whole fake dma > >>ops issue. > > > >Very long term the IOMMUs under the ops will need to care

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 11:17:38AM -0700, Logan Gunthorpe wrote: > >>If at all it should be in the dma_map* wrappers, but for that we'd need > >>a good identifier. And it still would not solve the whole fake dma > >>ops issue. > > > >Very long term the IOMMUs under the ops will need to care

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:09 AM, Jason Gunthorpe wrote: It could, if we had a DMA op for p2p then the drivers that provide their own ops can implement it appropriately or not at all. I was thinking of doing something like this. I'll probably rough out a patch and send it along today or tomorrow. If

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 08/01/18 11:09 AM, Jason Gunthorpe wrote: It could, if we had a DMA op for p2p then the drivers that provide their own ops can implement it appropriately or not at all. I was thinking of doing something like this. I'll probably rough out a patch and send it along today or tomorrow. If

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 03:59:01PM +0100, Christoph Hellwig wrote: > On Thu, Jan 04, 2018 at 09:50:31PM -0700, Jason Gunthorpe wrote: > > Well that argument applies equally to the RDMA RW API wrappers around > > the DMA API. I think it is fine if sgl are defined to only have P2P or > > not, and

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Mon, Jan 08, 2018 at 03:59:01PM +0100, Christoph Hellwig wrote: > On Thu, Jan 04, 2018 at 09:50:31PM -0700, Jason Gunthorpe wrote: > > Well that argument applies equally to the RDMA RW API wrappers around > > the DMA API. I think it is fine if sgl are defined to only have P2P or > > not, and

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 09:50:31PM -0700, Jason Gunthorpe wrote: > Well that argument applies equally to the RDMA RW API wrappers around > the DMA API. I think it is fine if sgl are defined to only have P2P or > not, and that debugging support seemed reasonable to me.. > > > It's also very

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 09:50:31PM -0700, Jason Gunthorpe wrote: > Well that argument applies equally to the RDMA RW API wrappers around > the DMA API. I think it is fine if sgl are defined to only have P2P or > not, and that debugging support seemed reasonable to me.. > > > It's also very

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 04:44:00PM -0700, Logan Gunthorpe wrote: > On 04/01/18 03:13 PM, Jason Gunthorpe wrote: > >On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: > >>We tried things like this in an earlier iteration[1] which assumed the SG > >>was homogenous (all P2P or all

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 04:44:00PM -0700, Logan Gunthorpe wrote: > On 04/01/18 03:13 PM, Jason Gunthorpe wrote: > >On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: > >>We tried things like this in an earlier iteration[1] which assumed the SG > >>was homogenous (all P2P or all

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 04/01/18 03:13 PM, Jason Gunthorpe wrote: On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: We tried things like this in an earlier iteration[1] which assumed the SG was homogenous (all P2P or all regular memory). This required serious ugliness to try and ensure SGs were in

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 04/01/18 03:13 PM, Jason Gunthorpe wrote: On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: We tried things like this in an earlier iteration[1] which assumed the SG was homogenous (all P2P or all regular memory). This required serious ugliness to try and ensure SGs were in

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: > >I'd be much happier if dma_map_sg can tell the memory is P2P or not > >from the scatterlist or dir arguments and not require the callers to > >have this. > > We tried things like this in an earlier iteration[1] which assumed the

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 12:52:24PM -0700, Logan Gunthorpe wrote: > >I'd be much happier if dma_map_sg can tell the memory is P2P or not > >from the scatterlist or dir arguments and not require the callers to > >have this. > > We tried things like this in an earlier iteration[1] which assumed the

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 04/01/18 12:22 PM, Jason Gunthorpe wrote: This seems really clunky since we are going to want to do this same logic all over the place. I'd be much happier if dma_map_sg can tell the memory is P2P or not from the scatterlist or dir arguments and not require the callers to have this. We

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On 04/01/18 12:22 PM, Jason Gunthorpe wrote: This seems really clunky since we are going to want to do this same logic all over the place. I'd be much happier if dma_map_sg can tell the memory is P2P or not from the scatterlist or dir arguments and not require the callers to have this. We

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 12:01:31PM -0700, Logan Gunthorpe wrote: > > @@ -269,18 +270,24 @@ static int rdma_rw_init_single_wr(struct rdma_rw_ctx > *ctx, struct ib_qp *qp, > * @remote_addr:remote address to read/write (relative to @rkey) > * @rkey:remote key to operate on > * @dir:

Re: [PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

On Thu, Jan 04, 2018 at 12:01:31PM -0700, Logan Gunthorpe wrote: > > @@ -269,18 +270,24 @@ static int rdma_rw_init_single_wr(struct rdma_rw_ctx > *ctx, struct ib_qp *qp, > * @remote_addr:remote address to read/write (relative to @rkey) > * @rkey:remote key to operate on > * @dir:

[PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

In order to use PCI P2P memory pci_p2pmem_[un]map_sg() functions must be called to map the correct DMA address. To do this, we add a flags variable and the RDMA_RW_CTX_FLAG_PCI_P2P flag. When the flag is specified use the appropriate map function. Signed-off-by: Logan Gunthorpe

[PATCH 06/12] IB/core: Add optional PCI P2P flag to rdma_rw_ctx_[init|destroy]()

In order to use PCI P2P memory pci_p2pmem_[un]map_sg() functions must be called to map the correct DMA address. To do this, we add a flags variable and the RDMA_RW_CTX_FLAG_PCI_P2P flag. When the flag is specified use the appropriate map function. Signed-off-by: Logan Gunthorpe ---