[PATCH 29/37] PCI: Add pci_enable_atomic_ops_to_root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2017-12-12 06:27 PM, Bjorn Helgaas wrote:
> [+cc Ram, Michal, Ariel, Doug, Jason]
>
> The [29/37] in the subject makes it look like this is part of a larger
> series, but I can't find the rest of it on linux-pci or linux-kernel.
>
> I don't want to merge a new interface unless there's an in-tree user
> of it.  I assume the rest of the series includes a user.
>
> On Fri, Dec 08, 2017 at 11:09:07PM -0500, Felix Kuehling wrote:
[snip]
>> + * all upstream bridges support AtomicOp routing, egress blocking is disabled
>> + * on all upstream ports, and the root port supports 32-bit, 64-bit and/or
>> + * 128-bit AtomicOp completion, or negative otherwise.
>> + */
>> +int pci_enable_atomic_ops_to_root(struct pci_dev *dev)
>> +{
>> +	struct pci_bus *bus = dev->bus;
>> +
>> +	if (!pci_is_pcie(dev))
>> +		return -EINVAL;
>> +
>> +	switch (pci_pcie_type(dev)) {
>> +	/*
>> +	 * PCIe 3.0, 6.15 specifies that endpoints and root ports are permitted
>> +	 * to implement AtomicOp requester capabilities.
>> +	 */
>> +	case PCI_EXP_TYPE_ENDPOINT:
>> +	case PCI_EXP_TYPE_LEG_END:
>> +	case PCI_EXP_TYPE_RC_END:
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	while (bus->parent) {
>> +		struct pci_dev *bridge = bus->self;
>> +		u32 cap;
>> +
>> +		pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
>> +
>> +		switch (pci_pcie_type(bridge)) {
>> +		/*
>> +		 * Upstream, downstream and root ports may implement AtomicOp
>> +		 * routing capabilities. AtomicOp routing via a root port is
>> +		 * not considered.
>> +		 */
>> +		case PCI_EXP_TYPE_UPSTREAM:
>> +		case PCI_EXP_TYPE_DOWNSTREAM:
>> +			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
>> +				return -EINVAL;
>> +			break;
>> +
>> +		/*
>> +		 * Root ports are permitted to implement AtomicOp completion
>> +		 * capabilities.
>> +		 */
>> +		case PCI_EXP_TYPE_ROOT_PORT:
>> +			if (!(cap & (PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
>> +				     PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
>> +				     PCI_EXP_DEVCAP2_ATOMIC_COMP128)))
>> +				return -EINVAL;
>> +			break;
>> +		}
> IIUC, you want to enable an endpoint, e.g., an AMD Fiji-class GPU, to
> initiate AtomicOps that target system memory.  This interface
> (pci_enable_atomic_ops_to_root()) doesn't specify what size operations
> the driver wants to do.  If the GPU requests a 128-bit op and the Root
> Port doesn't support it, I think we'll see an Unsupported Request
> error.
>
> Do you need to extend this interface so the driver can specify what
> sizes it wants?
>
> The existing code in qedr_pci_set_atomic() is very similar.  We should
> make this new interface work for both places, then actually use it in
> qedr_pci_set_atomic().

Hi Bjorn, Doug, Ram,

I just discussed this with Jay, and he noticed that qedr_pci_set_atomic
seems to use a different criteria to find the completer for atomic
requests. Jay's function expects the root port to have a parent, which
was the case on the systems he tested. But Ram's function looks for a
bridge without a parent and checks completion capabilities on that. Jay
believes that to be a root complex, not a root port.

According to the spec, "Root ports are permitted to implement AtomicOp
completion capabilities." It talks about a root port, not a root complex.

Can you help us understand, which interpretation is correct? And how to
correctly identify the root port for checking completion capabilities?
Are there valid topologies where a root port does not have a parent?

Regards,
  Felix



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux