hfi1 use of PCI internals
Subsystems (2)
I noticed drivers/infiniband/hw/hfi1 got moved from staging to drivers/ for v4.7. It does a bunch of grubbing around in PCIe ASPM configuration, e.g., see drivers/infiniband/hw/hfi1/aspm.h.
I know there have been lots of ASPM issues, both hardware problems and Linux kernel problems, but it is *supposed* to be manageable by the core, without special driver support. What's the justification for having to do this in the hfi1 driver?
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 8 lines · Bjorn Helgaas
I noticed
drivers/infiniband/hw/hfi1got moved from staging todrivers/for v4.7. It does a bunch of grubbing around in PCIe ASPM configuration, e.g., seedrivers/infiniband/hw/hfi1/aspm.h.I know there have been lots of ASPM issues, both hardware problems and Linux kernel problems, but it is *supposed* to be manageable by the core, without special driver support. What's the justification for having to do this in the hfi1 driver?
The description for commit affa48de84 "staging/rdma/hfi1: Add support for enabling/disabling PCIe ASPM" anticipates this question and describes why this was done in the hfi1 driver:
Finally, the kernel ASPM API is not used in this patch. This is because this patch does several non-standard things as SW workarounds for HW issues. As mentioned above, it enables ASPM even when advertised actual latencies are greater than acceptable latencies. Also, whereas the kernel API only allows drivers to disable ASPM from driver probe, this patch enables/disables ASPM directly from interrupt context. Due to these reasons the kernel ASPM API was not used.
Quoted 14 lines · Ashutosh Dixit
Quoted 8 lines · Bjorn Helgaas
I noticed
drivers/infiniband/hw/hfi1got moved from staging todrivers/for v4.7. It does a bunch of grubbing around in PCIe ASPM configuration, e.g., seedrivers/infiniband/hw/hfi1/aspm.h.I know there have been lots of ASPM issues, both hardware problems and Linux kernel problems, but it is *supposed* to be manageable by the core, without special driver support. What's the justification for having to do this in the hfi1 driver?
The description for commit
affa48de84"staging/rdma/hfi1: Add support for enabling/disabling PCIe ASPM" anticipates this question and describes why this was done in the hfi1 driver:
Quoted 8 lines
Finally, the kernel ASPM API is not used in this patch. This is because this patch does several non-standard things as SW workarounds for HW issues. As mentioned above, it enables ASPM even when advertised actual latencies are greater than acceptable latencies. Also, whereas the kernel API only allows drivers to disable ASPM from driver probe, this patch enables/disables ASPM directly from interrupt context. Due to these reasons the kernel ASPM API was not used.
That's a good start, but leads to more questions. For example, it doesn't answer the obvious question of why the driver needs to enable/disable ASPM from interrupt context.
Disabling ASPM should only require writing the device's Link Control register. The PCI core could probably provide an interface to do that in interrupt context.
Enabling ASPM is not latency-critical and could probably be done from a work queue outside interrupt context, although conceptually there shouldn't be much required here either, and possibly the PCI core interface could be improved.
It's possible the latency problem could be handled by some sort of quirk that overrides the acceptable latency.
It's hard enough to get ASPM support in the PCI core correct without having to worry about drivers doing their own thing behind the back of the core.
As far as I can tell, none of these PCI questions were raised on linux-pci, so we never even had a chance to have a conversation about them.
Bjorn
Quoted 3 lines · Bjorn Helgaas
As far as I can tell, none of these PCI questions were raised on linux-pci, so we never even had a chance to have a conversation about them.
I'll let Ashutosh handle the technical details here since he is most familiar with the code in question. I just want to mention that the move out of staging doesn't imply the driver is done being developed. We are very much open to discussing whatever the PCI folks see as needing to be addressed.
-Denny
Quoted 4 lines · Bjorn Helgaas
That's a good start, but leads to more questions. For example, it doesn't answer the obvious question of why the driver needs to enable/disable ASPM from interrupt context.
For power saving reasons we keep ASPM L1 enabled, but implement a heuristic to "quickly" disable ASPM L1 when we notice PCIe traffic (as measured by the interrupt rate) starting up. If interrupt activity ceases ASPM L1 is re-enabled.
Quoted 8 lines
Disabling ASPM should only require writing the device's Link Control register. The PCI core could probably provide an interface to do that in interrupt context.
Enabling ASPM is not latency-critical and could probably be done from a work queue outside interrupt context, although conceptually there shouldn't be much required here either, and possibly the PCI core interface could be improved.
That is true, to keep latencies low we need to disable ASPM from interrupt context, but re-enabling ASPM is not latency critical.
Quoted 2 lines
It's possible the latency problem could be handled by some sort of quirk that overrides the acceptable latency.
Correct, this is another issue that needs to be resolved.
Quoted 15 lines · Ashutosh Dixit
Quoted 4 lines · Bjorn Helgaas
That's a good start, but leads to more questions. For example, it doesn't answer the obvious question of why the driver needs to enable/disable ASPM from interrupt context.
For power saving reasons we keep ASPM L1 enabled, but implement a heuristic to "quickly" disable ASPM L1 when we notice PCIe traffic (as measured by the interrupt rate) starting up. If interrupt activity ceases ASPM L1 is re-enabled.
Quoted 4 lines
Disabling ASPM should only require writing the device's Link Control register. The PCI core could probably provide an interface to do that in interrupt context.
Quoted 7 lines
Quoted 4 lines
Enabling ASPM is not latency-critical and could probably be done from a work queue outside interrupt context, although conceptually there shouldn't be much required here either, and possibly the PCI core interface could be improved.
That is true, to keep latencies low we need to disable ASPM from interrupt context, but re-enabling ASPM is not latency critical.
For endpoint devices, it should be theoretically possible to enable/disable ASPM very quickly, by touching only that device. We don't do that today because pcie/aspm.c does all sorts of buffoonery and path walking. I think that could be simplified, assuming we think this sort of intensive ASPM-management is desirable.
Quoted 4 lines
Quoted 2 lines
It's possible the latency problem could be handled by some sort of quirk that overrides the acceptable latency.
Correct, this is another issue that needs to be resolved.