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 we reached to an agreement that code would not impose
>> any limits on what user wants.
>>
>> What happened to all the emails we exchanged?
> 
> 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. Nobody else seems keen on 
> allowing the user to enable this on hardware that doesn't work. The easiest 
> solution is still limiting it to using a switch. From there, if someone wants 
> to start creating a white-list then that's probably the way forward to 
> support root ports.

I thought only few root ports had problem. Thanks for clarifying that.

> 
> 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 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.

+int pci_p2pdma_disable_acs(struct pci_dev *pdev)
+{
+       struct pci_dev *up;
+       int pos;
+       u16 ctrl;
+
+       up = get_upstream_bridge_port(pdev);
+       if (!up)
+               return 0;

Some future device would only deal with pci_p2pdma_add_client(() for 
whitelisting
instead of changing all of your code.

We should at least limit the usage of get_upstream_bridge_port() family of 
functions
to probe time only.

> Additionally, once you allow for root ports you may find the IOMMU getting in 
> the way.

We can tell iommu to do one to one translation by passing iommu.passthrough=1 
to kernel
command line to have identical behavior to your switch case.

> 
> So there are great deal more issues to sort out if you don't restrict to 
> devices behind switches.
> 
> 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.

Reply via email to