Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

> I'll see if I can get our PCI SIG people to follow this through Hi Jonathan Can you let me know if this moves forward within PCI-SIG? I would like to track it. I can see this being doable between Root Ports that reside in the same Root Complex but might become more challenging to

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 27/03/18 02:47 AM, Jonathan Cameron wrote: > I'll see if I can get our PCI SIG people to follow this through and see if > it is just an omission or as Bjorn suggested, there is some reason we > aren't thinking of that makes it hard. That would be great! Thanks! Logan

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Mon, 26 Mar 2018 09:46:24 -0600 Logan Gunthorpe wrote: > On 26/03/18 08:01 AM, Bjorn Helgaas wrote: > > On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: > >> On Tue, 13 Mar 2018 10:43:55 -0600 > >> Logan Gunthorpe wrote: > >>> It

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 26/03/18 01:35 PM, Jason Gunthorpe wrote: > I think this is another case of the HW can do it but the SW support is > missing. IOMMU configuration and maybe firmware too, for instance. Nope, not sure how you can make this leap. We've been specifically told that peer-to-peer PCIe DMA is not

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Mon, Mar 26, 2018 at 11:30:38AM -0600, Logan Gunthorpe wrote: > > > On 26/03/18 10:41 AM, Jason Gunthorpe wrote: > > On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: > >> On Tue, 13 Mar 2018 10:43:55 -0600 > >> Logan Gunthorpe wrote: > >> > >>> On

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 26/03/18 10:41 AM, Jason Gunthorpe wrote: > On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: >> On Tue, 13 Mar 2018 10:43:55 -0600 >> Logan Gunthorpe wrote: >> >>> On 12/03/18 09:28 PM, Sinan Kaya wrote: On 3/12/2018 3:35 PM, Logan Gunthorpe wrote:

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: > On Tue, 13 Mar 2018 10:43:55 -0600 > Logan Gunthorpe wrote: > > > On 12/03/18 09:28 PM, Sinan Kaya wrote: > > > On 3/12/2018 3:35 PM, Logan Gunthorpe wrote: > > > Regarding the switch business, It is amazing

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 26/03/18 08:01 AM, Bjorn Helgaas wrote: > On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: >> On Tue, 13 Mar 2018 10:43:55 -0600 >> Logan Gunthorpe wrote: >>> It turns out that root ports that support P2P are far less common than >>> anyone thought. So

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 24/03/18 09:28 AM, Stephen Bates wrote: > 1. There is no requirement for a single function to support internal DMAs but > in the case of NVMe we do have a protocol specific way for a NVMe function to > indicate it supports via the CMB BAR. Other protocols may also have such > methods but

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Mon, Mar 26, 2018 at 12:11:38PM +0100, Jonathan Cameron wrote: > On Tue, 13 Mar 2018 10:43:55 -0600 > Logan Gunthorpe wrote: > > It turns out that root ports that support P2P are far less common than > > anyone thought. So it will likely have to be a white list. > > This

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

> That would be very nice but many devices do not support the internal > route. But Logan in the NVMe case we are discussing movement within a single function (i.e. from a NVMe namespace to a NVMe CMB on the same function). Bjorn is discussing movement between two functions (PFs or VFs) in the

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Fri, Mar 23, 2018 at 03:59:14PM -0600, Logan Gunthorpe wrote: > On 23/03/18 03:50 PM, Bjorn Helgaas wrote: > > Popping way up the stack, my original point was that I'm trying to > > remove restrictions on what devices can participate in > > peer-to-peer DMA. I think it's fairly clear that in

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 23/03/18 03:50 PM, Bjorn Helgaas wrote: > Popping way up the stack, my original point was that I'm trying to > remove restrictions on what devices can participate in peer-to-peer > DMA. I think it's fairly clear that in conventional PCI, any devices > in the same PCI hierarchy, i.e., below

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Thu, Mar 22, 2018 at 10:57:32PM +, Stephen Bates wrote: > > I've seen the response that peers directly below a Root Port could not > > DMA to each other through the Root Port because of the "route to self" > > issue, and I'm not disputing that. > > Bjorn > > You asked me for a

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

> I've seen the response that peers directly below a Root Port could not > DMA to each other through the Root Port because of the "route to self" > issue, and I'm not disputing that. Bjorn You asked me for a reference to RTS in the PCIe specification. As luck would have it I ended up in an

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Wed, Mar 14, 2018 at 12:34 PM, Stephen Bates wrote: >> P2P over PCI/PCI-X is quite common in devices like raid controllers. > > Hi Dan > > Do you mean between PCIe devices below the RAID controller? Isn't it pretty > novel to be able to support PCIe EPs below a RAID

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

Stephen, >> It would be useful if those configurations were not left behind so >> that Linux could feasibly deploy offload code to a controller in the >> PCI domain. > > Agreed. I think this would be great. Kind of like the XCOPY framework > that was proposed a while back for SCSI devices

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

> P2P over PCI/PCI-X is quite common in devices like raid controllers. Hi Dan Do you mean between PCIe devices below the RAID controller? Isn't it pretty novel to be able to support PCIe EPs below a RAID controller (as opposed to SCSI based devices)? > It would be useful if those

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 14/03/18 01:28 PM, Dan Williams wrote: > P2P over PCI/PCI-X is quite common in devices like raid controllers. > It would be useful if those configurations were not left behind so > that Linux could feasibly deploy offload code to a controller in the > PCI domain. Thanks for the note. Neat.

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Wed, Mar 14, 2018 at 12:03 PM, Logan Gunthorpe wrote: > > > On 14/03/18 12:51 PM, Bjorn Helgaas wrote: >> You are focused on PCIe systems, and in those systems, most topologies >> do have an upstream switch, which means two upstream bridges. I'm >> trying to remove that

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 14/03/18 12:51 PM, Bjorn Helgaas wrote: > You are focused on PCIe systems, and in those systems, most topologies > do have an upstream switch, which means two upstream bridges. I'm > trying to remove that assumption because I don't think there's a > requirement for it in the spec. Enforcing

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Wed, Mar 14, 2018 at 10:17:34AM -0600, Logan Gunthorpe wrote: > On 13/03/18 08:56 PM, Bjorn Helgaas wrote: > > I agree that peers need to have a common upstream bridge. I think > > you're saying peers need to have *two* common upstream bridges. If I > > understand correctly, requiring two

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 14/03/18 06:16 AM, David Laight wrote: > That surprises me (unless I missed something last time I read the spec). > While P2P writes are relatively easy to handle, reads and any other TLP that > require acks are a completely different proposition. > There are no additional fields that can be

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 08:56 PM, Bjorn Helgaas wrote: > I assume you want to exclude Root Ports because of multi-function > devices and the "route to self" error. I was hoping for a reference > to that so I could learn more about it. I haven't been able to find where in the spec it forbids route to self.

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

>I assume you want to exclude Root Ports because of multi-function > devices and the "route to self" error. I was hoping for a reference > to that so I could learn more about it. Apologies Bjorn. This slipped through my net. I will try and get you a reference for RTS in the next couple of

RE: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

From: Logan Gunthorpe > Sent: 13 March 2018 23:46 ... > As Stephen pointed out, it's a requirement of the PCIe spec that a > switch supports P2P. If you want to sell a switch that does P2P with bad > performance then that's on you to deal with. That surprises me (unless I missed something last

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Tue, Mar 13, 2018 at 05:21:20PM -0600, Logan Gunthorpe wrote: > On 13/03/18 05:08 PM, Bjorn Helgaas wrote: > > On Tue, Mar 13, 2018 at 10:31:55PM +, Stephen Bates wrote: > > If it *is* necessary because Root Ports and devices below them behave > > differently than in conventional PCI, I

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 05:19 PM, Sinan Kaya wrote: > It is still a switch it can move packets but, maybe it can move data at > 100kbps speed. As Stephen pointed out, it's a requirement of the PCIe spec that a switch supports P2P. If you want to sell a switch that does P2P with bad performance then that's

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 05:08 PM, Bjorn Helgaas wrote: > On Tue, Mar 13, 2018 at 10:31:55PM +, Stephen Bates wrote: > If it *is* necessary because Root Ports and devices below them behave > differently than in conventional PCI, I think you should include a > reference to the relevant section of the

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 6:48 PM, Logan Gunthorpe wrote: > > > On 13/03/18 04:29 PM, Sinan Kaya wrote: >> If hardware doesn't support it, blacklisting should have been the right >> path and I still think that you should remove all switch business from the >> code. >> I did not hear enough justification for

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On Tue, Mar 13, 2018 at 10:31:55PM +, Stephen Bates wrote: > >> It sounds like you have very tight hardware expectations for this to work > >> at this moment. You also don't want to generalize this code for others and > >> address the shortcomings. > > No, that's the way the community has

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 04:29 PM, Sinan Kaya wrote: > If hardware doesn't support it, blacklisting should have been the right > path and I still think that you should remove all switch business from the > code. > I did not hear enough justification for having a switch requirement > for P2P. I disagree. >

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

Hi Sinan >If hardware doesn't support it, blacklisting should have been the right >path and I still think that you should remove all switch business from the > code. >I did not hear enough justification for having a switch requirement >for P2P. We disagree. As does the

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

>> It sounds like you have very tight hardware expectations for this to work >> at this moment. You also don't want to generalize this code for others and >> address the shortcomings. > No, that's the way the community has pushed this work Hi Sinan Thanks for all the input. As Logan has pointed

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 6:00 PM, Logan Gunthorpe wrote: > > > On 13/03/18 03:22 PM, Sinan Kaya wrote: >> It sounds like you have very tight hardware expectations for this to work >> at this moment. You also don't want to generalize this code for others and >> address the shortcomings. > > No, that's the

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 03:22 PM, Sinan Kaya wrote: > It sounds like you have very tight hardware expectations for this to work > at this moment. You also don't want to generalize this code for others and > address the shortcomings. No, that's the way the community has pushed this work. Our original work

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 4:46 PM, Logan Gunthorpe wrote: > > > On 13/03/18 01:53 PM, Sinan Kaya wrote: >> I agree disabling globally would be bad. Somebody can always say I have >> ten switches on my system. I want to do peer-to-peer on one switch only. Now, >> this change weakened security for the other

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 01:53 PM, Sinan Kaya wrote: > I agree disabling globally would be bad. Somebody can always say I have > ten switches on my system. I want to do peer-to-peer on one switch only. Now, > this change weakened security for the other switches that I had no intention > with doing P2P. > >

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 3:19 PM, Logan Gunthorpe wrote: > > > On 13/03/18 01:10 PM, Sinan Kaya wrote: >> I was thinking of this for the pci_p2pdma_add_client() case for the >> parent pointer. >> >> +struct pci_p2pdma_client { >> +struct list_head list; >> +struct pci_dev *client; >> +struct

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 01:10 PM, Sinan Kaya wrote: > I was thinking of this for the pci_p2pdma_add_client() case for the > parent pointer. > > +struct pci_p2pdma_client { > + struct list_head list; > + struct pci_dev *client; > + struct pci_dev *provider; > +}; Yeah, that structure only

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 2:44 PM, Logan Gunthorpe wrote: >> Do you think you can keep a pointer to the parent bridge instead of querying >> it >> via get_upstream_bridge_port() here so that we can reuse your >> pci_p2pdma_disable_acs() in the future. > > Keep a pointer where? pci_p2pdma_disable_acs() and

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 13/03/18 11:49 AM, Sinan Kaya wrote: And there's also the ACS problem which means if you want to use P2P on the root ports you'll have to disable ACS on the entire system. (Or preferably, the IOMMU groups need to get more sophisticated to allow for dynamic changes). Do you think you

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 12/03/18 09:28 PM, Sinan Kaya wrote: Maybe, dev parameter should also be struct pci_dev so that you can get rid of all to_pci_dev() calls in this code including find_parent_pci_dev() function. No, this was mentioned in v2. find_parent_pci_dev is necessary because the calling drivers

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/13/2018 12:43 PM, Logan Gunthorpe wrote: > > > On 12/03/18 09:28 PM, Sinan Kaya wrote: >> On 3/12/2018 3:35 PM, Logan Gunthorpe wrote: >> Regarding the switch business, It is amazing how much trouble you went into >> limit this functionality into very specific hardware. >> >> I thought that

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 12/03/18 09:28 PM, Sinan Kaya wrote: On 3/12/2018 3:35 PM, Logan Gunthorpe wrote: Regarding the switch business, It is amazing how much trouble you went into limit this functionality into very specific hardware. I thought that we reached to an agreement that code would not impose any

Re: [PATCH v3 01/11] PCI/P2PDMA: Support peer-to-peer memory

On 3/12/2018 3:35 PM, Logan Gunthorpe wrote: > +int pci_p2pdma_add_client(struct list_head *head, struct device *dev) It feels like code tried to be a generic p2pdma provider first. Then got converted to PCI, yet all dev parameters are still struct device. Maybe, dev parameter should also be