PCIe 3.0 AtomicOp capabilities

PCIe 3.0 AtomicOp capabilities

[Jay Cornwall]

Hi,

There is some interest in using the PCIe 3.0 AtomicOp capability with a 
subset of devices supported by drm/amdgpu.

Adding this line to drm/amdgpu produced correct results in our tests:
pcie_capability_set_word(adev->pdev, PCI_EXP_DEVCTL2, 0x40);

I have found only a handful of examples of capability use throughout 
drivers/. I see that the ARI capability is enabled if supported in 
drivers/pci/pci.c.

Should the AtomicOp capabilities be similarly enabled if available? Or 
might there be a reason for doing this on a per-driver basis?

-- 
Jay Cornwall

Re: PCIe 3.0 AtomicOp capabilities

[Bjorn Helgaas]

Hi Jay,

On Mon, Aug 10, 2015 at 1:36 PM, Jay Cornwall <jay@jcornwall.me> wrote:
> There is some interest in using the PCIe 3.0 AtomicOp capability with a
> subset of devices supported by drm/amdgpu.
>
> Adding this line to drm/amdgpu produced correct results in our tests:
> pcie_capability_set_word(adev->pdev, PCI_EXP_DEVCTL2, 0x40);
>
> I have found only a handful of examples of capability use throughout
> drivers/. I see that the ARI capability is enabled if supported in
> drivers/pci/pci.c.
>
> Should the AtomicOp capabilities be similarly enabled if available? Or might
> there be a reason for doing this on a per-driver basis?

I think the PCIe capability structure (including device, link slot,
etc., capability, status, and control registers) should be managed by
the PCI core, not by individual drivers, because these things often
have implications beyond the scope of a driver.  There are some
existing cases where drivers fiddle with the PCIe capability, but I
try to avoid adding new ones.

I'm not very familiar with the AtomicOp functionality, but a quick
skim of the spec suggests that it does have system implications and
should be handled by the core.  For example, AtomicOp support is
optional, and it looks like it would be a bad idea to enable it in an
endpoint if the upstream switch didn't support it.

Bjorn

Re: PCIe 3.0 AtomicOp capabilities

[Jay Cornwall]

On 2015-08-11 10:10, Bjorn Helgaas wrote:

> On Mon, Aug 10, 2015 at 1:36 PM, Jay Cornwall <jay@jcornwall.me> wrote:
> 
>> Should the AtomicOp capabilities be similarly enabled if available? Or 
>> might
>> there be a reason for doing this on a per-driver basis?
> 
> I'm not very familiar with the AtomicOp functionality, but a quick
> skim of the spec suggests that it does have system implications and
> should be handled by the core. For example, AtomicOp support is
> optional, and it looks like it would be a bad idea to enable it in an
> endpoint if the upstream switch didn't support it.

This makes sense, but I think there are some cases in which upstream is 
ambiguous.

For example, consider a root complex which does not support AtomicOp 
completion but supports routing to another endpoint which does. Would 
the necessary condition for enabling AtomicOp requests be that at least 
one other completion-capable endpoint is reachable?

-- 
Jay Cornwall

Re: PCIe 3.0 AtomicOp capabilities

[Bjorn Helgaas]

On Tue, Aug 11, 2015 at 7:39 PM, Jay Cornwall <jay@jcornwall.me> wrote:
> On 2015-08-11 10:10, Bjorn Helgaas wrote:
>
>> On Mon, Aug 10, 2015 at 1:36 PM, Jay Cornwall <jay@jcornwall.me> wrote:
>>
>>> Should the AtomicOp capabilities be similarly enabled if available? Or
>>> might
>>> there be a reason for doing this on a per-driver basis?
>>
>>
>> I'm not very familiar with the AtomicOp functionality, but a quick
>> skim of the spec suggests that it does have system implications and
>> should be handled by the core. For example, AtomicOp support is
>> optional, and it looks like it would be a bad idea to enable it in an
>> endpoint if the upstream switch didn't support it.
>
>
> This makes sense, but I think there are some cases in which upstream is
> ambiguous.
>
> For example, consider a root complex which does not support AtomicOp
> completion but supports routing to another endpoint which does. Would the
> necessary condition for enabling AtomicOp requests be that at least one
> other completion-capable endpoint is reachable?

I don't know enough about AtomicOps to get into a discussion about
what the exact conditions for enabling it are.

I thought your original questions were basically (1) should the PCI
core automatically enable AtomicOps? and (2) should drivers contain
code to enable AtomicOps?

I think drivers should *not* contain code to directly enable
AtomicOps, because it looks like doing that safely requires knowledge
about the topology beyond the driver's device.  But I think it *would*
be reasonable a driver to call a PCI interface like
pci_enable_atomic() or something, and the PCI core could figure out
whether it's safe and enable it or fail.

I don't know whether we should try to automatically enable AtomicOps
even without an explicit driver request.  I don't say that to
discourage that approach; I just don't know whether it makes sense or
can be done safely.  I'm certainly open to reviewing patches that
would do that.

Bjorn