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 pci_dev *provider; >> +}; > > Yeah, that structure only exists in a list owned by the client and we > only check the upstream bridge once per entry so I don't see the point. > >> But then, Why bother searching for the switch at all? > > Huh? We have to make sure all the client and provider devices are behind > the same switch. How can we do that without "searching" for the switch? > Sorry, I was thinking of ACS case you described below. The only thing code cares is if the device is behind a switch or not at this moment. > In the ACS case, we only disable ACS on downstream ports of switches. No > sense disabling it globally as that's worse from an isolation point of > view and not worth it given we require all P2P transactions to be behind > a switch. 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. Isn't this a problem? Can we specify the BDF of the downstream device we want P2P with during boot via kernel command line? > >> Even if the switch is there, there is no guarantee that it is currently >> being used for P2P. > > IOMMU groups are set at boot time and, at present, there's no way to > dynamically change ACS bits without messing up the groups. So switches > not used for P2P will not have ACS enabled when CONFIG_PCI_P2PDMA is set > and I don't know of any good solution to that. Please see the ACS > discussions in v1 and v2. Given the implementation limitations, this might be OK as a short-term solution. It depends on if Alex is comfortable with this. > >> It seems that we are going with the assumption that enabling this config >> option implies you want P2P, then we can simplify this code as well. > > How so? > > Logan > > > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html