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.