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. But I was told this by developers who work know switches. Hopefully Stephen can find the reference. But it's a bit of a moot point. Devices can DMA to themselves if they are designed to do so. For example, some NVMe cards can read and write their own CMB for certain types of DMA request. There is a register in the spec (CMBSZ) which specifies which types of requests are supported. (See 3.1.12 in NVMe 1.3a). > 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 common bridges is a way to ensure > that peers directly below Root Ports don't try to DMA to each other. No, I don't get where you think we need to have two common upstream bridges. I'm not sure when such a case would ever happen. But you seem to understand based on what you wrote below. > So I guess the first order of business is to nail down whether peers > below a Root Port are prohibited from DMAing to each other. My > assumption, based on 6.12.1.2 and the fact that I haven't yet found > a prohibition, is that they can. If you have a multifunction device designed to DMA to itself below a root port, it can. But determining this is on a device by device basis, just as determining whether a root complex can do peer to peer is on a per device basis. So I'd say we don't want to allow it by default and let someone who has such a device figure out what's necessary if and when one comes along. > You already have upstream_bridges_match(), which takes two pci_devs. > I think it should walk up the PCI hierarchy from the first device, > checking whether the bridge at each level is also a parent of the > second device. Yes, this is what I meant when I said walking the entire tree. I've been kicking the can down the road on implementing this as getting ref counting right and testing it is going to be quite tricky. The single switch approach we implemented now is just a simplification which works for a single switch. But I guess we can look at implementing it this way for v4. Logan